Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1650481 > unrolled thread

Re: [PATCH v2 1/1] w1: Add subsystem kernel public interface

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 1/1] w1: Add subsystem kernel public interface Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-05-25 15:10 +0200
    Re: [PATCH v2 1/1] w1: Add subsystem kernel public interface "Andrew F. Davis" <afd@ti.com> - 2017-05-25 15:10 +0200
      Re: [PATCH v2 1/1] w1: Add subsystem kernel public interface "Andrew F. Davis" <afd@ti.com> - 2017-05-25 15:30 +0200
        Re: [PATCH v2 1/1] w1: Add subsystem kernel public interface "Andrew F. Davis" <afd@ti.com> - 2017-05-25 15:50 +0200
          Re: [PATCH v2 1/1] w1: Add subsystem kernel public interface "Andrew F. Davis" <afd@ti.com> - 2017-05-26 18:40 +0200

#1650481 — Re: [PATCH v2 1/1] w1: Add subsystem kernel public interface

On Tue, May 16, 2017 at 03:08:14PM -0500, Andrew F. Davis wrote:
> Like other subsystems we should be able to define slave devices outside
> of the w1 directory. To do this we move public facing interface
> definitions to include/linux/w1.h and rename the internal definition
> file to w1_internal.h.
> 
> As w1_family.h and w1_int.h contained almost entirely public
> driver interface definitions we simply removed these files and
> moved the remaining definitions into w1_internal.h.
> 
> With this we can now start to move slave devices out of w1/slaves and
> into the subsystem based on the function they implement, again like
> other drivers.

Do you have a patch that does that?  I don't want to accept this patch
if that never happens...

thanks,

greg k-h

[toc] | [next] | [standalone]

#1650485

On 05/25/2017 08:00 AM, Greg Kroah-Hartman wrote:
> On Tue, May 16, 2017 at 03:08:14PM -0500, Andrew F. Davis wrote:
>> Like other subsystems we should be able to define slave devices outside
>> of the w1 directory. To do this we move public facing interface
>> definitions to include/linux/w1.h and rename the internal definition
>> file to w1_internal.h.
>>
>> As w1_family.h and w1_int.h contained almost entirely public
>> driver interface definitions we simply removed these files and
>> moved the remaining definitions into w1_internal.h.
>>
>> With this we can now start to move slave devices out of w1/slaves and
>> into the subsystem based on the function they implement, again like
>> other drivers.
> 
> Do you have a patch that does that?  I don't want to accept this patch
> if that never happens...
> 

Yes, this reorganization stems from the need to move the BQ27xxx slave
out, patches were posted and reviewed last cycle here[0] and will be
reposted when this patch is accepted. Other w1/slaves can be moved out
at their owners convenience if/when they wish.

Thanks,
Andrew

[0] https://lkml.org/lkml/2017/3/16/688

> thanks,
> 
> greg k-h
> 

[toc] | [prev] | [next] | [standalone]

#1650502

On 05/25/2017 08:09 AM, Evgeniy Polyakov wrote:
> Hi everyone
> 
> 25.05.2017, 16:07, "Andrew F. Davis" <afd@ti.com>:
> 
>>>  Do you have a patch that does that? I don't want to accept this patch
>>>  if that never happens...
>>
>> Yes, this reorganization stems from the need to move the BQ27xxx slave
>> out, patches were posted and reviewed last cycle here[0] and will be
>> reposted when this patch is accepted. Other w1/slaves can be moved out
>> at their owners convenience if/when they wish.
> 
> Why does BQ27xxx need to move out of w1 tree?
> 

Currently we have to enable a pseudo-platform device driver in the
power/supply BQ27xxx driver, then the w1 driver has to instantiate this
platform device and then they connect and communicate by sharing
callbacks. This is rather hacky.

Other buses USB, SPI, I2C, etc. allow the client device drivers to live
with the subsystem they for whose functionality they expose (e.g an I2C
attached temp sensor goes in drivers/hwmon, not drivers/i2c).

"Need" is a bit strong, but moving this out makes things a lot cleaner
and more consistent.

Andrew

[toc] | [prev] | [next] | [standalone]

#1650524

On 05/25/2017 08:28 AM, Evgeniy Polyakov wrote:
> 
> 
> 25.05.2017, 16:22, "Andrew F. Davis" <afd@ti.com>:
> 
>>>  Why does BQ27xxx need to move out of w1 tree?
>>
>> Currently we have to enable a pseudo-platform device driver in the
>> power/supply BQ27xxx driver, then the w1 driver has to instantiate this
>> platform device and then they connect and communicate by sharing
>> callbacks. This is rather hacky.
> 
> Why do you have to create a pseudo-platform device driver to connect w1 and power/supply?
> 
> I'm not against creating w1 drivers in different places than drivers/w1, but so far
> it was only power drivers which have problem with it (and they easily work it out),
> and this rises a flag.
> 

We could keep it in w1 if we really wanted, but then things like Kconfig
will get difficult to manage (we will jump between menus and have odd
dependencies).

The other w1/slaves seem to mostly be simple EEPROMs and Gauges that
would otherwise end up in misc/ so it is fine if they live in w1, but
BQ27xxx does have a proper home in power/supplies and its i2c interface
is already there, so moving the w1 interface there also makes sense to me.

> I would rather move w1 header into include/linux, will it be enough?
> 

That's what this patch does, we just also re-organize things a bit so
only things that need to be public end up in include/linux. It seems to
be all that is needed for my use-case at least.

[toc] | [prev] | [next] | [standalone]

#1651513

On 05/26/2017 11:13 AM, Evgeniy Polyakov wrote:
> 
> 
> 25.05.2017, 16:50, "Andrew F. Davis" <afd@ti.com>:
> 
>>>  Why do you have to create a pseudo-platform device driver to connect w1 and power/supply?
>>>
>>>  I'm not against creating w1 drivers in different places than drivers/w1, but so far
>>>  it was only power drivers which have problem with it (and they easily work it out),
>>>  and this rises a flag.
>>
>> We could keep it in w1 if we really wanted, but then things like Kconfig
>> will get difficult to manage (we will jump between menus and have odd
>> dependencies).
> 
> One still has to jump between kconfig menus to enable w1 and power devices,
> or to use 'select', nothing really changes.
> 

Well, one less step in that we get to drop the platform shim driver, but
I see what you mean.

>> The other w1/slaves seem to mostly be simple EEPROMs and Gauges that
>> would otherwise end up in misc/ so it is fine if they live in w1, but
>> BQ27xxx does have a proper home in power/supplies and its i2c interface
>> is already there, so moving the w1 interface there also makes sense to me.
>>
>>>  I would rather move w1 header into include/linux, will it be enough?
>>
>> That's what this patch does, we just also re-organize things a bit so
>> only things that need to be public end up in include/linux. It seems to
>> be all that is needed for my use-case at least.
> 
> Ok, lets merge it.

Great, thanks!

> 
> My personal note: please drop TI copyright from headers, at least from those which already exist
> and you move them around, it is your work and not TI. It is up to you to put into driver itself.
> 

9-5 TI owns everything I touch :) so I just figured I should add it, but
I have no personal problem dropping the copyright notice (actually I
already did this as you requested this in v1).

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web