* [PATCH] platform/x86: thinkpad_acpi: Fix warning about deprecated hwmon_device_register @ 2017-06-20 3:02 Stanislav Fomichev [not found] ` <20170620030208.15997-1-kernel-klOrIKU+5EClnMjI0IkVqw@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: Stanislav Fomichev @ 2017-06-20 3:02 UTC (permalink / raw) To: ibm-acpi; +Cc: dvhart, andy, ibm-acpi-devel, platform-driver-x86 Use hwmon_device_register_with_groups instead of deprecated hwmon_device_register and fix a dmesg warning. This patch however changes the userspace API. hwmon_device_register_with_groups takes `hwmon' name as an argument and creates a name file in the `hwmon' device, not in the `platform_device'. This allows us to remove custom `name' device attribute, but in order to make lm-sensors happy we also have to move fans and thermal attributes to the `hwmon' device. Even though this patch changes userspace API, it's still compatible with the lm-sensors. Starting with lm-sensors 3.0 (circa 2007), it looks at both hwmon and the backing device for the name and other attributes. Signed-off-by: Stanislav Fomichev <kernel@fomichev.me> --- drivers/platform/x86/thinkpad_acpi.c | 36 ++++++++++-------------------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index 7b6cb0c69b02..0e0a1616f273 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -6401,7 +6401,7 @@ static int __init thermal_init(struct ibm_init_struct *iibm) switch (thermal_read_mode) { case TPACPI_THERMAL_TPEC_16: - res = sysfs_create_group(&tpacpi_sensors_pdev->dev.kobj, + res = sysfs_create_group(&tpacpi_hwmon->kobj, &thermal_temp_input16_group); if (res) return res; @@ -6409,7 +6409,7 @@ static int __init thermal_init(struct ibm_init_struct *iibm) case TPACPI_THERMAL_TPEC_8: case TPACPI_THERMAL_ACPI_TMP07: case TPACPI_THERMAL_ACPI_UPDT: - res = sysfs_create_group(&tpacpi_sensors_pdev->dev.kobj, + res = sysfs_create_group(&tpacpi_hwmon->kobj, &thermal_temp_input8_group); if (res) return res; @@ -6426,13 +6426,13 @@ static void thermal_exit(void) { switch (thermal_read_mode) { case TPACPI_THERMAL_TPEC_16: - sysfs_remove_group(&tpacpi_sensors_pdev->dev.kobj, + sysfs_remove_group(&tpacpi_hwmon->kobj, &thermal_temp_input16_group); break; case TPACPI_THERMAL_TPEC_8: case TPACPI_THERMAL_ACPI_TMP07: case TPACPI_THERMAL_ACPI_UPDT: - sysfs_remove_group(&tpacpi_sensors_pdev->dev.kobj, + sysfs_remove_group(&tpacpi_hwmon->kobj, &thermal_temp_input8_group); break; case TPACPI_THERMAL_NONE: @@ -8773,7 +8773,7 @@ static int __init fan_init(struct ibm_init_struct *iibm) fan_attributes[ARRAY_SIZE(fan_attributes)-2] = &dev_attr_fan2_input.attr; } - rc = sysfs_create_group(&tpacpi_sensors_pdev->dev.kobj, + rc = sysfs_create_group(&tpacpi_hwmon->kobj, &fan_attr_group); if (rc < 0) return rc; @@ -8781,7 +8781,7 @@ static int __init fan_init(struct ibm_init_struct *iibm) rc = driver_create_file(&tpacpi_hwmon_pdriver.driver, &driver_attr_fan_watchdog); if (rc < 0) { - sysfs_remove_group(&tpacpi_sensors_pdev->dev.kobj, + sysfs_remove_group(&tpacpi_hwmon->kobj, &fan_attr_group); return rc; } @@ -8796,7 +8796,7 @@ static void fan_exit(void) "cancelling any pending fan watchdog tasks\n"); /* FIXME: can we really do this unconditionally? */ - sysfs_remove_group(&tpacpi_sensors_pdev->dev.kobj, &fan_attr_group); + sysfs_remove_group(&tpacpi_hwmon->kobj, &fan_attr_group); driver_remove_file(&tpacpi_hwmon_pdriver.driver, &driver_attr_fan_watchdog); @@ -9229,16 +9229,6 @@ static void hotkey_driver_event(const unsigned int scancode) tpacpi_driver_event(TP_HKEY_EV_HOTKEY_BASE + scancode); } -/* sysfs name ---------------------------------------------------------- */ -static ssize_t thinkpad_acpi_pdev_name_show(struct device *dev, - struct device_attribute *attr, - char *buf) -{ - return snprintf(buf, PAGE_SIZE, "%s\n", TPACPI_NAME); -} - -static DEVICE_ATTR(name, S_IRUGO, thinkpad_acpi_pdev_name_show, NULL); - /* --------------------------------------------------------------------- */ /* /proc support */ @@ -9782,8 +9772,6 @@ static void thinkpad_acpi_module_exit(void) if (tpacpi_hwmon) hwmon_device_unregister(tpacpi_hwmon); - if (tp_features.sensors_pdev_attrs_registered) - device_remove_file(&tpacpi_sensors_pdev->dev, &dev_attr_name); if (tpacpi_sensors_pdev) platform_device_unregister(tpacpi_sensors_pdev); if (tpacpi_pdev) @@ -9904,14 +9892,10 @@ static int __init thinkpad_acpi_module_init(void) thinkpad_acpi_module_exit(); return ret; } - ret = device_create_file(&tpacpi_sensors_pdev->dev, &dev_attr_name); - if (ret) { - pr_err("unable to create sysfs hwmon device attributes\n"); - thinkpad_acpi_module_exit(); - return ret; - } tp_features.sensors_pdev_attrs_registered = 1; - tpacpi_hwmon = hwmon_device_register(&tpacpi_sensors_pdev->dev); + tpacpi_hwmon = hwmon_device_register_with_groups( + &tpacpi_sensors_pdev->dev, TPACPI_NAME, NULL, NULL); + if (IS_ERR(tpacpi_hwmon)) { ret = PTR_ERR(tpacpi_hwmon); tpacpi_hwmon = NULL; -- 2.13.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
[parent not found: <20170620030208.15997-1-kernel-klOrIKU+5EClnMjI0IkVqw@public.gmane.org>]
* Re: [PATCH] platform/x86: thinkpad_acpi: Fix warning about deprecated hwmon_device_register [not found] ` <20170620030208.15997-1-kernel-klOrIKU+5EClnMjI0IkVqw@public.gmane.org> @ 2017-06-20 5:16 ` Henrique de Moraes Holschuh 2017-06-21 3:45 ` [PATCH v2] " Stanislav Fomichev 0 siblings, 1 reply; 6+ messages in thread From: Henrique de Moraes Holschuh @ 2017-06-20 5:16 UTC (permalink / raw) To: Stanislav Fomichev Cc: dvhart-wEGCiKHe2LqWVfeAwA7xHQ, platform-driver-x86-u79uwXL29TY76Z2rM5mHXA, andy-wEGCiKHe2LqWVfeAwA7xHQ, ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Mon, 19 Jun 2017, Stanislav Fomichev wrote: > Use hwmon_device_register_with_groups instead of deprecated > hwmon_device_register and fix a dmesg warning. > > This patch however changes the userspace API. > hwmon_device_register_with_groups takes `hwmon' name as an argument and creates > a name file in the `hwmon' device, not in the `platform_device'. This > allows us to remove custom `name' device attribute, but in order to make > lm-sensors happy we also have to move fans and thermal attributes to the > `hwmon' device. Can you clarify that with a ls -l of the "before" and "after"? Also, does it change anything visible to lmsensors users (such as the sensor/adapter name or address)? If so, please post a "before" and "after"... > Even though this patch changes userspace API, it's still compatible with > the lm-sensors. Starting with lm-sensors 3.0 (circa 2007), it looks at both > hwmon and the backing device for the name and other attributes. I am not sure this is OK or not, it *is* an userspace ABI break. I am not really opposed to it, as long as it is done properly. Let's see what our subsystem maintainers think of it. That said: the patch is still incomplete, because this driver has documentation that also needs to be updated... Please update Documentation/laptops/thinkpad-acpi.txt with the changes to the thinkpad_hwmon sensor interfaces. Search for "hwmon" on that text, that should locate everything. Also, please bump "#define TPACPI_SYSFS_VERSION 0x020700" to 0x020800 in drivers/platform/x86/thinkpad_acpi.c, and update the interface changelog table at the end of Documentation/laptops/thinkpad-acpi.txt. (that assumes the change is not a major ABI break, please reply with that "ls -l" I asked above: we might need to bump that TPACPI_SYSFS_VERSION to 0x030000, instead...) > Signed-off-by: Stanislav Fomichev <kernel-klOrIKU+5EClnMjI0IkVqw@public.gmane.org> > --- > drivers/platform/x86/thinkpad_acpi.c | 36 ++++++++++-------------------------- > 1 file changed, 10 insertions(+), 26 deletions(-) > > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c > index 7b6cb0c69b02..0e0a1616f273 100644 > --- a/drivers/platform/x86/thinkpad_acpi.c > +++ b/drivers/platform/x86/thinkpad_acpi.c > @@ -6401,7 +6401,7 @@ static int __init thermal_init(struct ibm_init_struct *iibm) > > switch (thermal_read_mode) { > case TPACPI_THERMAL_TPEC_16: > - res = sysfs_create_group(&tpacpi_sensors_pdev->dev.kobj, > + res = sysfs_create_group(&tpacpi_hwmon->kobj, > &thermal_temp_input16_group); > if (res) > return res; > @@ -6409,7 +6409,7 @@ static int __init thermal_init(struct ibm_init_struct *iibm) > case TPACPI_THERMAL_TPEC_8: > case TPACPI_THERMAL_ACPI_TMP07: > case TPACPI_THERMAL_ACPI_UPDT: > - res = sysfs_create_group(&tpacpi_sensors_pdev->dev.kobj, > + res = sysfs_create_group(&tpacpi_hwmon->kobj, > &thermal_temp_input8_group); > if (res) > return res; > @@ -6426,13 +6426,13 @@ static void thermal_exit(void) > { > switch (thermal_read_mode) { > case TPACPI_THERMAL_TPEC_16: > - sysfs_remove_group(&tpacpi_sensors_pdev->dev.kobj, > + sysfs_remove_group(&tpacpi_hwmon->kobj, > &thermal_temp_input16_group); > break; > case TPACPI_THERMAL_TPEC_8: > case TPACPI_THERMAL_ACPI_TMP07: > case TPACPI_THERMAL_ACPI_UPDT: > - sysfs_remove_group(&tpacpi_sensors_pdev->dev.kobj, > + sysfs_remove_group(&tpacpi_hwmon->kobj, > &thermal_temp_input8_group); > break; > case TPACPI_THERMAL_NONE: > @@ -8773,7 +8773,7 @@ static int __init fan_init(struct ibm_init_struct *iibm) > fan_attributes[ARRAY_SIZE(fan_attributes)-2] = > &dev_attr_fan2_input.attr; > } > - rc = sysfs_create_group(&tpacpi_sensors_pdev->dev.kobj, > + rc = sysfs_create_group(&tpacpi_hwmon->kobj, > &fan_attr_group); > if (rc < 0) > return rc; > @@ -8781,7 +8781,7 @@ static int __init fan_init(struct ibm_init_struct *iibm) > rc = driver_create_file(&tpacpi_hwmon_pdriver.driver, > &driver_attr_fan_watchdog); > if (rc < 0) { > - sysfs_remove_group(&tpacpi_sensors_pdev->dev.kobj, > + sysfs_remove_group(&tpacpi_hwmon->kobj, > &fan_attr_group); > return rc; > } > @@ -8796,7 +8796,7 @@ static void fan_exit(void) > "cancelling any pending fan watchdog tasks\n"); > > /* FIXME: can we really do this unconditionally? */ > - sysfs_remove_group(&tpacpi_sensors_pdev->dev.kobj, &fan_attr_group); > + sysfs_remove_group(&tpacpi_hwmon->kobj, &fan_attr_group); > driver_remove_file(&tpacpi_hwmon_pdriver.driver, > &driver_attr_fan_watchdog); > > @@ -9229,16 +9229,6 @@ static void hotkey_driver_event(const unsigned int scancode) > tpacpi_driver_event(TP_HKEY_EV_HOTKEY_BASE + scancode); > } > > -/* sysfs name ---------------------------------------------------------- */ > -static ssize_t thinkpad_acpi_pdev_name_show(struct device *dev, > - struct device_attribute *attr, > - char *buf) > -{ > - return snprintf(buf, PAGE_SIZE, "%s\n", TPACPI_NAME); > -} > - > -static DEVICE_ATTR(name, S_IRUGO, thinkpad_acpi_pdev_name_show, NULL); > - > /* --------------------------------------------------------------------- */ > > /* /proc support */ > @@ -9782,8 +9772,6 @@ static void thinkpad_acpi_module_exit(void) > if (tpacpi_hwmon) > hwmon_device_unregister(tpacpi_hwmon); > > - if (tp_features.sensors_pdev_attrs_registered) > - device_remove_file(&tpacpi_sensors_pdev->dev, &dev_attr_name); > if (tpacpi_sensors_pdev) > platform_device_unregister(tpacpi_sensors_pdev); > if (tpacpi_pdev) > @@ -9904,14 +9892,10 @@ static int __init thinkpad_acpi_module_init(void) > thinkpad_acpi_module_exit(); > return ret; > } > - ret = device_create_file(&tpacpi_sensors_pdev->dev, &dev_attr_name); > - if (ret) { > - pr_err("unable to create sysfs hwmon device attributes\n"); > - thinkpad_acpi_module_exit(); > - return ret; > - } > tp_features.sensors_pdev_attrs_registered = 1; > - tpacpi_hwmon = hwmon_device_register(&tpacpi_sensors_pdev->dev); > + tpacpi_hwmon = hwmon_device_register_with_groups( > + &tpacpi_sensors_pdev->dev, TPACPI_NAME, NULL, NULL); > + > if (IS_ERR(tpacpi_hwmon)) { > ret = PTR_ERR(tpacpi_hwmon); > tpacpi_hwmon = NULL; -- Henrique Holschuh ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] platform/x86: thinkpad_acpi: Fix warning about deprecated hwmon_device_register 2017-06-20 5:16 ` Henrique de Moraes Holschuh @ 2017-06-21 3:45 ` Stanislav Fomichev 2017-07-01 6:02 ` Stanislav Fomichev 2017-08-18 23:01 ` Darren Hart 0 siblings, 2 replies; 6+ messages in thread From: Stanislav Fomichev @ 2017-06-21 3:45 UTC (permalink / raw) To: hmh Cc: ibm-acpi, dvhart, andy, ibm-acpi-devel, platform-driver-x86, Stanislav Fomichev From: Stanislav Fomichev <sdf@google.com> Use hwmon_device_register_with_groups instead of deprecated hwmon_device_register and fix a dmesg warning. This patch however changes the userspace API. hwmon_device_register_with_groups takes `hwmon' name as an argument and creates a name file in the `hwmon' device, not in the `platform_device'. This allows us to remove custom `name' device attribute, but in order to make lm-sensors happy we also have to move fans and thermal attributes to the `hwmon' device. Even though this patch changes userspace API, it's still compatible with the lm-sensors. Starting with lm-sensors 3.0 (circa 2007), it looks at both hwmon and the backing device for the name and other attributes. before: $ cat /sys/devices/platform/thinkpad_hwmon/{name,fan1_input} thinkpad 2007 $ cat /sys/devices/platform/thinkpad_hwmon/hwmon/hwmon1/{name,fan1_input} cat: /sys/devices/platform/thinkpad_hwmon/hwmon/hwmon1/name: No such file or directory cat: /sys/devices/platform/thinkpad_hwmon/hwmon/hwmon1/fan1_input: No such file or directory $ cat /sys/class/hwmon/hwmon1/{name,fan1_input} cat: /sys/class/hwmon/hwmon1/name: No such file or directory cat: /sys/class/hwmon/hwmon1/fan1_input: No such file or directory $ sensors thinkpad-isa-0000 Adapter: ISA adapter fan1: 3533 RPM after: $ cat /sys/devices/platform/thinkpad_hwmon/{name,fan1_input} cat: /sys/devices/platform/thinkpad_hwmon/name: No such file or directory cat: /sys/devices/platform/thinkpad_hwmon/fan1_input: No such file or directory $ cat /sys/devices/platform/thinkpad_hwmon/hwmon/hwmon1/{name,fan1_input} thinkpad 3478 $ cat /sys/class/hwmon/hwmon1/{name,fan1_input} thinkpad 3478 $ sensors thinkpad-isa-0000 Adapter: ISA adapter fan1: 3489 RPM $ sensors -v sensors version 3.4.0 with libsensors version 3.4.0 Changes since v1: * Bumped TPACPI_SYSFS_VERSION * Updated documentation Signed-off-by: Stanislav Fomichev <kernel@fomichev.me> --- Documentation/laptops/thinkpad-acpi.txt | 9 ++++++-- drivers/platform/x86/thinkpad_acpi.c | 38 ++++++++++----------------------- 2 files changed, 18 insertions(+), 29 deletions(-) diff --git a/Documentation/laptops/thinkpad-acpi.txt b/Documentation/laptops/thinkpad-acpi.txt index ba2e7d254842..1dbef8b8c7b1 100644 --- a/Documentation/laptops/thinkpad-acpi.txt +++ b/Documentation/laptops/thinkpad-acpi.txt @@ -121,8 +121,9 @@ space, for 2.6.23+ this is /sys/devices/platform/thinkpad_acpi/. Sysfs device attributes for the sensors and fan are on the thinkpad_hwmon device's sysfs attribute space, but you should locate it looking for a hwmon device with the name attribute of "thinkpad", or -better yet, through libsensors. - +better yet, through libsensors. For 4.13+ sysfs attributes were moved to the +hwmon device (/sys/bus/platform/devices/thinkpad_hwmon/hwmon/hwmon? or +/sys/class/hwmon/hwmon?). Driver version -------------- @@ -1478,3 +1479,7 @@ not, please contact ibm-acpi-devel@lists.sourceforge.net with a report. 0x020700: Support for mute-only mixers. Volume control in read-only mode by default. Marker for ALSA mixer support. + +0x030000: Thermal and fan sysfs attributes were moved to the hwmon + device instead of being attached to the backing platform + device. diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index 7b6cb0c69b02..4470f59ce874 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -24,7 +24,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #define TPACPI_VERSION "0.25" -#define TPACPI_SYSFS_VERSION 0x020700 +#define TPACPI_SYSFS_VERSION 0x030000 /* * Changelog: @@ -6401,7 +6401,7 @@ static int __init thermal_init(struct ibm_init_struct *iibm) switch (thermal_read_mode) { case TPACPI_THERMAL_TPEC_16: - res = sysfs_create_group(&tpacpi_sensors_pdev->dev.kobj, + res = sysfs_create_group(&tpacpi_hwmon->kobj, &thermal_temp_input16_group); if (res) return res; @@ -6409,7 +6409,7 @@ static int __init thermal_init(struct ibm_init_struct *iibm) case TPACPI_THERMAL_TPEC_8: case TPACPI_THERMAL_ACPI_TMP07: case TPACPI_THERMAL_ACPI_UPDT: - res = sysfs_create_group(&tpacpi_sensors_pdev->dev.kobj, + res = sysfs_create_group(&tpacpi_hwmon->kobj, &thermal_temp_input8_group); if (res) return res; @@ -6426,13 +6426,13 @@ static void thermal_exit(void) { switch (thermal_read_mode) { case TPACPI_THERMAL_TPEC_16: - sysfs_remove_group(&tpacpi_sensors_pdev->dev.kobj, + sysfs_remove_group(&tpacpi_hwmon->kobj, &thermal_temp_input16_group); break; case TPACPI_THERMAL_TPEC_8: case TPACPI_THERMAL_ACPI_TMP07: case TPACPI_THERMAL_ACPI_UPDT: - sysfs_remove_group(&tpacpi_sensors_pdev->dev.kobj, + sysfs_remove_group(&tpacpi_hwmon->kobj, &thermal_temp_input8_group); break; case TPACPI_THERMAL_NONE: @@ -8773,7 +8773,7 @@ static int __init fan_init(struct ibm_init_struct *iibm) fan_attributes[ARRAY_SIZE(fan_attributes)-2] = &dev_attr_fan2_input.attr; } - rc = sysfs_create_group(&tpacpi_sensors_pdev->dev.kobj, + rc = sysfs_create_group(&tpacpi_hwmon->kobj, &fan_attr_group); if (rc < 0) return rc; @@ -8781,7 +8781,7 @@ static int __init fan_init(struct ibm_init_struct *iibm) rc = driver_create_file(&tpacpi_hwmon_pdriver.driver, &driver_attr_fan_watchdog); if (rc < 0) { - sysfs_remove_group(&tpacpi_sensors_pdev->dev.kobj, + sysfs_remove_group(&tpacpi_hwmon->kobj, &fan_attr_group); return rc; } @@ -8796,7 +8796,7 @@ static void fan_exit(void) "cancelling any pending fan watchdog tasks\n"); /* FIXME: can we really do this unconditionally? */ - sysfs_remove_group(&tpacpi_sensors_pdev->dev.kobj, &fan_attr_group); + sysfs_remove_group(&tpacpi_hwmon->kobj, &fan_attr_group); driver_remove_file(&tpacpi_hwmon_pdriver.driver, &driver_attr_fan_watchdog); @@ -9229,16 +9229,6 @@ static void hotkey_driver_event(const unsigned int scancode) tpacpi_driver_event(TP_HKEY_EV_HOTKEY_BASE + scancode); } -/* sysfs name ---------------------------------------------------------- */ -static ssize_t thinkpad_acpi_pdev_name_show(struct device *dev, - struct device_attribute *attr, - char *buf) -{ - return snprintf(buf, PAGE_SIZE, "%s\n", TPACPI_NAME); -} - -static DEVICE_ATTR(name, S_IRUGO, thinkpad_acpi_pdev_name_show, NULL); - /* --------------------------------------------------------------------- */ /* /proc support */ @@ -9782,8 +9772,6 @@ static void thinkpad_acpi_module_exit(void) if (tpacpi_hwmon) hwmon_device_unregister(tpacpi_hwmon); - if (tp_features.sensors_pdev_attrs_registered) - device_remove_file(&tpacpi_sensors_pdev->dev, &dev_attr_name); if (tpacpi_sensors_pdev) platform_device_unregister(tpacpi_sensors_pdev); if (tpacpi_pdev) @@ -9904,14 +9892,10 @@ static int __init thinkpad_acpi_module_init(void) thinkpad_acpi_module_exit(); return ret; } - ret = device_create_file(&tpacpi_sensors_pdev->dev, &dev_attr_name); - if (ret) { - pr_err("unable to create sysfs hwmon device attributes\n"); - thinkpad_acpi_module_exit(); - return ret; - } tp_features.sensors_pdev_attrs_registered = 1; - tpacpi_hwmon = hwmon_device_register(&tpacpi_sensors_pdev->dev); + tpacpi_hwmon = hwmon_device_register_with_groups( + &tpacpi_sensors_pdev->dev, TPACPI_NAME, NULL, NULL); + if (IS_ERR(tpacpi_hwmon)) { ret = PTR_ERR(tpacpi_hwmon); tpacpi_hwmon = NULL; -- 2.13.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] platform/x86: thinkpad_acpi: Fix warning about deprecated hwmon_device_register 2017-06-21 3:45 ` [PATCH v2] " Stanislav Fomichev @ 2017-07-01 6:02 ` Stanislav Fomichev 2017-08-18 23:01 ` Darren Hart 1 sibling, 0 replies; 6+ messages in thread From: Stanislav Fomichev @ 2017-07-01 6:02 UTC (permalink / raw) To: hmh Cc: ibm-acpi, dvhart, andy, ibm-acpi-devel, platform-driver-x86, Stanislav Fomichev Henrique, any comments on the v2? -- Stanislav Fomichev ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] platform/x86: thinkpad_acpi: Fix warning about deprecated hwmon_device_register 2017-06-21 3:45 ` [PATCH v2] " Stanislav Fomichev 2017-07-01 6:02 ` Stanislav Fomichev @ 2017-08-18 23:01 ` Darren Hart 2017-08-31 16:54 ` [ibm-acpi-devel] " Henrique de Moraes Holschuh 1 sibling, 1 reply; 6+ messages in thread From: Darren Hart @ 2017-08-18 23:01 UTC (permalink / raw) To: Stanislav Fomichev Cc: hmh, ibm-acpi, andy, ibm-acpi-devel, platform-driver-x86, Stanislav Fomichev On Tue, Jun 20, 2017 at 08:45:13PM -0700, Stanislav Fomichev wrote: > From: Stanislav Fomichev <sdf@google.com> > Oi, apologies for the long delay on this. > Use hwmon_device_register_with_groups instead of deprecated > hwmon_device_register and fix a dmesg warning. > > This patch however changes the userspace API. > hwmon_device_register_with_groups takes `hwmon' name as an argument and creates > a name file in the `hwmon' device, not in the `platform_device'. This > allows us to remove custom `name' device attribute, but in order to make > lm-sensors happy we also have to move fans and thermal attributes to the > `hwmon' device. > > Even though this patch changes userspace API, it's still compatible with > the lm-sensors. Starting with lm-sensors 3.0 (circa 2007), it looks at both > hwmon and the backing device for the name and other attributes. > > before: > $ cat /sys/devices/platform/thinkpad_hwmon/{name,fan1_input} > thinkpad > 2007 > $ cat /sys/devices/platform/thinkpad_hwmon/hwmon/hwmon1/{name,fan1_input} > cat: /sys/devices/platform/thinkpad_hwmon/hwmon/hwmon1/name: No such file or directory > cat: /sys/devices/platform/thinkpad_hwmon/hwmon/hwmon1/fan1_input: No such file or directory > $ cat /sys/class/hwmon/hwmon1/{name,fan1_input} > cat: /sys/class/hwmon/hwmon1/name: No such file or directory > cat: /sys/class/hwmon/hwmon1/fan1_input: No such file or directory > $ sensors > thinkpad-isa-0000 > Adapter: ISA adapter > fan1: 3533 RPM > > after: > $ cat /sys/devices/platform/thinkpad_hwmon/{name,fan1_input} > cat: /sys/devices/platform/thinkpad_hwmon/name: No such file or directory > cat: /sys/devices/platform/thinkpad_hwmon/fan1_input: No such file or directory > $ cat /sys/devices/platform/thinkpad_hwmon/hwmon/hwmon1/{name,fan1_input} > thinkpad > 3478 > $ cat /sys/class/hwmon/hwmon1/{name,fan1_input} > thinkpad > 3478 > $ sensors > thinkpad-isa-0000 > Adapter: ISA adapter > fan1: 3489 RPM > This looks very reasonable to me. The lm-sensors user experience is effectively unchanged, and the /sys/* changes move from a specific implementation to a generic implementation, taking advantage for the subsystem. > $ sensors -v > sensors version 3.4.0 with libsensors version 3.4.0 > > Changes since v1: > * Bumped TPACPI_SYSFS_VERSION > * Updated documentation > Please see Documentation/process/submitting-patches.rst section 14 regarding the placement of the Changelog below the --- marker line as it is not part of the commit message. (for future patches) > Signed-off-by: Stanislav Fomichev <kernel@fomichev.me> > --- > Documentation/laptops/thinkpad-acpi.txt | 9 ++++++-- > drivers/platform/x86/thinkpad_acpi.c | 38 ++++++++++----------------------- > 2 files changed, 18 insertions(+), 29 deletions(-) > > diff --git a/Documentation/laptops/thinkpad-acpi.txt b/Documentation/laptops/thinkpad-acpi.txt > index ba2e7d254842..1dbef8b8c7b1 100644 > --- a/Documentation/laptops/thinkpad-acpi.txt > +++ b/Documentation/laptops/thinkpad-acpi.txt > @@ -121,8 +121,9 @@ space, for 2.6.23+ this is /sys/devices/platform/thinkpad_acpi/. > Sysfs device attributes for the sensors and fan are on the > thinkpad_hwmon device's sysfs attribute space, but you should locate it > looking for a hwmon device with the name attribute of "thinkpad", or > -better yet, through libsensors. > - > +better yet, through libsensors. For 4.13+ sysfs attributes were moved to the This will be 4.14 because we let it sit too long. I'll correct this. I've queued this to testing for 4.14. Henrique, please shout if you have any objections here. -- Darren Hart VMware Open Source Technology Center ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ibm-acpi-devel] [PATCH v2] platform/x86: thinkpad_acpi: Fix warning about deprecated hwmon_device_register 2017-08-18 23:01 ` Darren Hart @ 2017-08-31 16:54 ` Henrique de Moraes Holschuh 0 siblings, 0 replies; 6+ messages in thread From: Henrique de Moraes Holschuh @ 2017-08-31 16:54 UTC (permalink / raw) To: Darren Hart Cc: Stanislav Fomichev, platform-driver-x86, ibm-acpi-devel, Stanislav Fomichev, andy On Fri, 18 Aug 2017, Darren Hart wrote: > > before: > > $ cat /sys/devices/platform/thinkpad_hwmon/{name,fan1_input} > > after: > > $ cat /sys/devices/platform/thinkpad_hwmon/hwmon/hwmon1/{name,fan1_input} > > thinkpad > > 3478 > > $ cat /sys/class/hwmon/hwmon1/{name,fan1_input} > > thinkpad > > 3478 I wonder what's the point of retaining the thinkpad_hwmon separate device [from the thinkpad_acpi platform device] then... but changing that might break the userspace API even further or cause other annoyances down the road, so I guess it is the lesser evil. > > $ sensors > > thinkpad-isa-0000 > > Adapter: ISA adapter > > fan1: 3489 RPM Yeah, that should cover >90% of the usecases since most people just read these. It *will* break write accesses using /etc/sysfs.conf and similar, though (to set fan mode on boot, etc). It is documented and the userspace ABI is being updated according to the hwmon subsystem rules *and* the thinkpad-acpi rules... this is enough for me, but be warned that people might complain. > This looks very reasonable to me. The lm-sensors user experience is > effectively unchanged, and the /sys/* changes move from a specific > implementation to a generic implementation, taking advantage for the > subsystem. Yes, which is why I am not against the ABI change. > This will be 4.14 because we let it sit too long. I'll correct this. > > I've queued this to testing for 4.14. > > Henrique, please shout if you have any objections here. No objections. -- Henrique Holschuh ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-08-31 16:54 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-06-20 3:02 [PATCH] platform/x86: thinkpad_acpi: Fix warning about deprecated hwmon_device_register Stanislav Fomichev [not found] ` <20170620030208.15997-1-kernel-klOrIKU+5EClnMjI0IkVqw@public.gmane.org> 2017-06-20 5:16 ` Henrique de Moraes Holschuh 2017-06-21 3:45 ` [PATCH v2] " Stanislav Fomichev 2017-07-01 6:02 ` Stanislav Fomichev 2017-08-18 23:01 ` Darren Hart 2017-08-31 16:54 ` [ibm-acpi-devel] " Henrique de Moraes Holschuh
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.