[lm-sensors] use of struct platform_device vs struct device in

[lm-sensors] use of struct platform_device vs struct device in

[Jim Cromie]

hi Jean,

while LXR'g  (http://sosdg.org/~coywolf/lxr/)
for usage of struct platform_device, I found your code.


What were your reasons for using platform_device, what are the benefits ?

Were they special-case considerations, or is this something that
other hwmon drivers should consider moving towards ?

[lm-sensors] use of struct platform_device vs struct device in

[Jean Delvare]

Hi Jim,

> while LXR'g  (http://sosdg.org/~coywolf/lxr/)
> for usage of struct platform_device, I found your code.
> 
> What were your reasons for using platform_device, what are the benefits ?
> 
> Were they special-case considerations, or is this something that
> other hwmon drivers should consider moving towards ?

Well, the reasons were simply that the Fintek F71805F/FG is not an
I2C/SMBus device. For too long, all hardware monitoring drivers have
been implemented as i2c drivers, even when they were absolutely not
I2C/SMBus related. This was made possible by the i2c-isa driver, which
has an historical reason for existing: some chips (LM78 and W83781D to
name the most popular ones) used to have two possible interfaces, I2C
and ISA. In order to avoid code duplication while keeping the drivers
simple, it was decided to handle the ISA interface as a fake I2C bus.

Now, there are more and more hardware monitoring devices which are
either integrated into PCI devices, of integrated into Super-I/O chips,
with a simple ISA interface. Depending on the whole i2c stack to drive
these is quite boring and inefficient now, so I have been working on a
clean separation between i2c and hwmon for the past few months. This
included a full rewrite of i2c-isa, and moving of all hardware
monitoring drivers from drivers/i2c/chips to drivers/hwmon, and a lot
of cleanups all around the place. Then we introduced the hwmon class,
which makes it possible for user-space to locate hardware monitoring
chips in sysfs regardless of their interface. This was supported by
libsensors in lm_sensors 2.9.2 for the first time.

The ultimate goal is to convert all non-I2C hardware monitoring drivers
to platform drivers. However, we need to wait for lm_sensors 2.9.2 and
later to be widely deployed before we modify the exiting drivers, as it
makes them incompatible with earlier releases of lm_sensors. For the
f71805f driver, it was new so compatibility wasn't an issue. All other
drivers, except the newly ported vt8231 driver, will have to wait at
least another 6 months before we can consider converting them. New
drivers or drivers being ported these days (e.g. vt1211) should be
implemented as platform drivers directly.

I hope I clarified the point :)

-- 
Jean Delvare

[lm-sensors] use of struct platform_device vs struct device in

[Jim Cromie]

Jean Delvare wrote:
> Hi Jim,
>
>   
>> while LXR'g  (http://sosdg.org/~coywolf/lxr/)
>> for usage of struct platform_device, I found your code.
>>
>> What were your reasons for using platform_device, what are the benefits ?
>>
>> Were they special-case considerations, or is this something that
>> other hwmon drivers should consider moving towards ?
>>     
>
> Well, the reasons were simply that the Fintek F71805F/FG is not an
> I2C/SMBus device. For too long, all hardware monitoring drivers have
> been implemented as i2c drivers, even when they were absolutely not
> I2C/SMBus related. This was made possible by the i2c-isa driver, which
> has an historical reason for existing: some chips (LM78 and W83781D to
> name the most popular ones) used to have two possible interfaces, I2C
> and ISA. In order to avoid code duplication while keeping the drivers
> simple, it was decided to handle the ISA interface as a fake I2C bus.
>
> Now, there are more and more hardware monitoring devices which are
> either integrated into PCI devices, of integrated into Super-I/O chips,
> with a simple ISA interface. Depending on the whole i2c stack to drive
> these is quite boring and inefficient now, so I have been working on a
> clean separation between i2c and hwmon for the past few months. This
> included a full rewrite of i2c-isa, and moving of all hardware
> monitoring drivers from drivers/i2c/chips to drivers/hwmon, and a lot
> of cleanups all around the place. Then we introduced the hwmon class,
> which makes it possible for user-space to locate hardware monitoring
> chips in sysfs regardless of their interface. This was supported by
> libsensors in lm_sensors 2.9.2 for the first time.
>
> The ultimate goal is to convert all non-I2C hardware monitoring drivers
> to platform drivers. However, we need to wait for lm_sensors 2.9.2 and
> later to be widely deployed before we modify the exiting drivers, as it
> makes them incompatible with earlier releases of lm_sensors. For the
> f71805f driver, it was new so compatibility wasn't an issue. All other
> drivers, except the newly ported vt8231 driver, will have to wait at
> least another 6 months before we can consider converting them. New
> drivers or drivers being ported these days (e.g. vt1211) should be
> implemented as platform drivers directly.
>
> I hope I clarified the point :)
>
>   
Thanks Jean,

A helpful and thorough answer, thanks.
WRT waiting for 2.9.2,  it would take some months to get updates
thru -mm and then into mainline.   Certainly drivers could sit in -mm
until the time is right.


Actually though, the question I posed was twisted from an OT one;
Im working on pc8736x_gpio, which is an adaptation of scx200_gpio,
so its a char-dev.  I was looking for a way to get a struct device that
I could use in dev_dbg() and friends,

I may imitate your work in pc87360, just to get some familiarity
with platform_device, (along with on-topic help ;-), then apply that
to my gpio project.

[lm-sensors] use of struct platform_device vs struct device in

[Jean Delvare]

Hi Jim,

> WRT waiting for 2.9.2,  it would take some months to get updates
> thru -mm and then into mainline.   Certainly drivers could sit in -mm
> until the time is right.

Bad idea from a maintenance point of view. Converting i2c-isa drivers to
platform drivers will be very intrusive, so it's likely that other
patches (bug fixes etc...) with conflict with them, effectively
doubling the amount of work.

I'd better postpone any work related to such conversions until we are
ready to merge them. This also gives us some time to make sure that the
choices I made for the f71805f driver were correct. See the recent
thread started by Hans de Goede for example.

> Actually though, the question I posed was twisted from an OT one;
> Im working on pc8736x_gpio, which is an adaptation of scx200_gpio,
> so its a char-dev.  I was looking for a way to get a struct device that
> I could use in dev_dbg() and friends,
> 
> I may imitate your work in pc87360, just to get some familiarity
> with platform_device, (along with on-topic help ;-), then apply that
> to my gpio project.

You can still do this in your private sandbox if it helps you.

Thanks,
-- 
Jean Delvare