hwmon: How should we represent Apple SMC's temperature sensors?

hwmon: How should we represent Apple SMC's temperature sensors?

[Cengiz Can]

Hello,

As Mr. Roeck knows, I've been refactoring applesmc into a MFD driver and 
cells for a while now.

Refactoring attempt is almost ready to be reviewed however there's a 
question that's keeping me from further progress.

As I read in hwmon api docs, most drivers are not constructing sysfs 
attributes by themselves. They're using helpers like SENSOR_DEVICE_ATTR 
and delegating sysfs management.

And as I read in `drivers/gpu/drm/nouveau/nouveau_hwmon.c` this API 
abstracts sysfs creation/permission tasks. (I assume that nouveau_hwmon is 
a properly implemented example).

However, as you know, applesmc is constructing its sysfs attributes 
manually. (`drivers/hwmon/applesmc.c#L1108`)

I don't have much context (and this is my first hwmon patch attempt) but I 
think this is due to the dynamic nature of 
Apple SMC temperature and fan control values. Not all Apple devices have 
same temperature values nor same number of fans. 

For those who don't know, `drivers/hwmon/applesmc.c` queries an LPC device 
during init and creates sysfs files according to the result that query.

What I would like to know is:  

  1) Should I try to implement a solution that's using SENSOR_DEVICE_ATTR  
or similar helpers to provide sysfs attributes in my refactored Apple SMC 
driver? Is there an example for that?

  2) Should I just keep the manual sysfs creator method?  
(`drivers/hwmon/applesmc.c#L1108`). Is this acceptable?

Thank you

	Cengiz Can

Re: hwmon: How should we represent Apple SMC's temperature sensors?

[Guenter Roeck]

On 05/06/2018 01:36 PM, Cengiz Can wrote:
> Hello,
> 
> As Mr. Roeck knows, I've been refactoring applesmc into a MFD driver and
> cells for a while now.
> 
> Refactoring attempt is almost ready to be reviewed however there's a
> question that's keeping me from further progress.
> 
> As I read in hwmon api docs, most drivers are not constructing sysfs
> attributes by themselves. They're using helpers like SENSOR_DEVICE_ATTR
> and delegating sysfs management.
> 
> And as I read in `drivers/gpu/drm/nouveau/nouveau_hwmon.c` this API
> abstracts sysfs creation/permission tasks. (I assume that nouveau_hwmon is
> a properly implemented example).
> 
> However, as you know, applesmc is constructing its sysfs attributes
> manually. (`drivers/hwmon/applesmc.c#L1108`)
> 
> I don't have much context (and this is my first hwmon patch attempt) but I
> think this is due to the dynamic nature of
> Apple SMC temperature and fan control values. Not all Apple devices have
> same temperature values nor same number of fans.
> 
> For those who don't know, `drivers/hwmon/applesmc.c` queries an LPC device
> during init and creates sysfs files according to the result that query.
> 
> What I would like to know is:
> 
>    1) Should I try to implement a solution that's using SENSOR_DEVICE_ATTR
> or similar helpers to provide sysfs attributes in my refactored Apple SMC
> driver? Is there an example for that?
> 
>    2) Should I just keep the manual sysfs creator method?
> (`drivers/hwmon/applesmc.c#L1108`). Is this acceptable?
> 

The best solution would be to not create any sysfs attributes in the hwmon
driver, but to use the new API (hwmon_device_register_with_info). If that
doesn't work out for some reason (if so, please explain), 2) is fine.

Guenter

> Thank you
> 
> 	Cengiz Can
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>