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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
-- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html