All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] thinkpad_acpi: revert unintentional device attribute renaming
@ 2015-05-19 17:45 Bjørn Mork
  2015-05-19 17:55 ` Henrique de Moraes Holschuh
  0 siblings, 1 reply; 3+ messages in thread
From: Bjørn Mork @ 2015-05-19 17:45 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Darren Hart, platform-driver-x86, ibm-acpi-devel,
	Bjørn Mork, Bastien Nocera, Henrique de Moraes Holschuh

The conversion to DEVICE_ATTR_* macros failed to fixup a few cases where
the old attribute names didn't match the show/store function names.
Instead of renaming the functions, the attributes were renamed. This
caused an unintentional API change.  The hwmon required 'name' attribute
were among the renamed attribute, causing libsensors to fail to detect
the hwmon device at all.

Fix by using the DEVICE_ATTR macro for these attributes, allowing the
show/store functions to keep their system specific prefixes.

Fixes: b4dd04ac6ef8 ("thinkpad_acpi: use DEVICE_ATTR_* macros")
Cc: Bastien Nocera <hadess@hadess.net>
Cc: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
v2: kept the original function names, using the DEVICE_ATTR instead

 drivers/platform/x86/thinkpad_acpi.c | 37 ++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 9bb9ad6d4a1b..28f328136f0d 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -2897,7 +2897,7 @@ static ssize_t hotkey_wakeup_reason_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "%d\n", hotkey_wakeup_reason);
 }
 
-static DEVICE_ATTR_RO(hotkey_wakeup_reason);
+static DEVICE_ATTR(wakeup_reason, S_IRUGO, hotkey_wakeup_reason_show, NULL);
 
 static void hotkey_wakeup_reason_notify_change(void)
 {
@@ -2913,7 +2913,8 @@ static ssize_t hotkey_wakeup_hotunplug_complete_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "%d\n", hotkey_autosleep_ack);
 }
 
-static DEVICE_ATTR_RO(hotkey_wakeup_hotunplug_complete);
+static DEVICE_ATTR(wakeup_hotunplug_complete, S_IRUGO,
+		   hotkey_wakeup_hotunplug_complete_show, NULL);
 
 static void hotkey_wakeup_hotunplug_complete_notify_change(void)
 {
@@ -2978,8 +2979,8 @@ static struct attribute *hotkey_attributes[] __initdata = {
 	&dev_attr_hotkey_enable.attr,
 	&dev_attr_hotkey_bios_enabled.attr,
 	&dev_attr_hotkey_bios_mask.attr,
-	&dev_attr_hotkey_wakeup_reason.attr,
-	&dev_attr_hotkey_wakeup_hotunplug_complete.attr,
+	&dev_attr_wakeup_reason.attr,
+	&dev_attr_wakeup_hotunplug_complete.attr,
 	&dev_attr_hotkey_mask.attr,
 	&dev_attr_hotkey_all_mask.attr,
 	&dev_attr_hotkey_recommended_mask.attr,
@@ -4393,12 +4394,13 @@ static ssize_t wan_enable_store(struct device *dev,
 			attr, buf, count);
 }
 
-static DEVICE_ATTR_RW(wan_enable);
+static DEVICE_ATTR(wwan_enable, S_IWUSR | S_IRUGO,
+		   wan_enable_show, wan_enable_store);
 
 /* --------------------------------------------------------------------- */
 
 static struct attribute *wan_attributes[] = {
-	&dev_attr_wan_enable.attr,
+	&dev_attr_wwan_enable.attr,
 	NULL
 };
 
@@ -8138,7 +8140,8 @@ static ssize_t fan_pwm1_enable_store(struct device *dev,
 	return count;
 }
 
-static DEVICE_ATTR_RW(fan_pwm1_enable);
+static DEVICE_ATTR(pwm1_enable, S_IWUSR | S_IRUGO,
+		   fan_pwm1_enable_show, fan_pwm1_enable_store);
 
 /* sysfs fan pwm1 ------------------------------------------------------ */
 static ssize_t fan_pwm1_show(struct device *dev,
@@ -8198,7 +8201,7 @@ static ssize_t fan_pwm1_store(struct device *dev,
 	return (rc) ? rc : count;
 }
 
-static DEVICE_ATTR_RW(fan_pwm1);
+static DEVICE_ATTR(pwm1, S_IWUSR | S_IRUGO, fan_pwm1_show, fan_pwm1_store);
 
 /* sysfs fan fan1_input ------------------------------------------------ */
 static ssize_t fan_fan1_input_show(struct device *dev,
@@ -8215,7 +8218,7 @@ static ssize_t fan_fan1_input_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "%u\n", speed);
 }
 
-static DEVICE_ATTR_RO(fan_fan1_input);
+static DEVICE_ATTR(fan1_input, S_IRUGO, fan_fan1_input_show, NULL);
 
 /* sysfs fan fan2_input ------------------------------------------------ */
 static ssize_t fan_fan2_input_show(struct device *dev,
@@ -8232,7 +8235,7 @@ static ssize_t fan_fan2_input_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "%u\n", speed);
 }
 
-static DEVICE_ATTR_RO(fan_fan2_input);
+static DEVICE_ATTR(fan2_input, S_IRUGO, fan_fan2_input_show, NULL);
 
 /* sysfs fan fan_watchdog (hwmon driver) ------------------------------- */
 static ssize_t fan_fan_watchdog_show(struct device_driver *drv,
@@ -8265,8 +8268,8 @@ static DRIVER_ATTR(fan_watchdog, S_IWUSR | S_IRUGO,
 
 /* --------------------------------------------------------------------- */
 static struct attribute *fan_attributes[] = {
-	&dev_attr_fan_pwm1_enable.attr, &dev_attr_fan_pwm1.attr,
-	&dev_attr_fan_fan1_input.attr,
+	&dev_attr_pwm1_enable.attr, &dev_attr_pwm1.attr,
+	&dev_attr_fan1_input.attr,
 	NULL, /* for fan2_input */
 	NULL
 };
@@ -8400,7 +8403,7 @@ static int __init fan_init(struct ibm_init_struct *iibm)
 		if (tp_features.second_fan) {
 			/* attach second fan tachometer */
 			fan_attributes[ARRAY_SIZE(fan_attributes)-2] =
-					&dev_attr_fan_fan2_input.attr;
+					&dev_attr_fan2_input.attr;
 		}
 		rc = sysfs_create_group(&tpacpi_sensors_pdev->dev.kobj,
 					 &fan_attr_group);
@@ -8848,7 +8851,7 @@ static ssize_t thinkpad_acpi_pdev_name_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "%s\n", TPACPI_NAME);
 }
 
-static DEVICE_ATTR_RO(thinkpad_acpi_pdev_name);
+static DEVICE_ATTR(name, S_IRUGO, thinkpad_acpi_pdev_name_show, NULL);
 
 /* --------------------------------------------------------------------- */
 
@@ -9390,8 +9393,7 @@ static void thinkpad_acpi_module_exit(void)
 		hwmon_device_unregister(tpacpi_hwmon);
 
 	if (tp_features.sensors_pdev_attrs_registered)
-		device_remove_file(&tpacpi_sensors_pdev->dev,
-				   &dev_attr_thinkpad_acpi_pdev_name);
+		device_remove_file(&tpacpi_sensors_pdev->dev, &dev_attr_name);
 	if (tpacpi_sensors_pdev)
 		platform_device_unregister(tpacpi_sensors_pdev);
 	if (tpacpi_pdev)
@@ -9512,8 +9514,7 @@ static int __init thinkpad_acpi_module_init(void)
 		thinkpad_acpi_module_exit();
 		return ret;
 	}
-	ret = device_create_file(&tpacpi_sensors_pdev->dev,
-				 &dev_attr_thinkpad_acpi_pdev_name);
+	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();
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] thinkpad_acpi: revert unintentional device attribute renaming
  2015-05-19 17:45 [PATCH v2] thinkpad_acpi: revert unintentional device attribute renaming Bjørn Mork
@ 2015-05-19 17:55 ` Henrique de Moraes Holschuh
  2015-05-20  9:19   ` Darren Hart
  0 siblings, 1 reply; 3+ messages in thread
From: Henrique de Moraes Holschuh @ 2015-05-19 17:55 UTC (permalink / raw)
  To: Bjørn Mork, Henrique de Moraes Holschuh
  Cc: Darren Hart, platform-driver-x86, ibm-acpi-devel, Bastien Nocera

On Tue, May 19, 2015, at 14:45, Bjørn Mork wrote:
> The conversion to DEVICE_ATTR_* macros failed to fixup a few cases where
> the old attribute names didn't match the show/store function names.
> Instead of renaming the functions, the attributes were renamed. This
> caused an unintentional API change.  The hwmon required 'name' attribute
> were among the renamed attribute, causing libsensors to fail to detect
> the hwmon device at all.
> 
> Fix by using the DEVICE_ATTR macro for these attributes, allowing the
> show/store functions to keep their system specific prefixes.
> 
> Fixes: b4dd04ac6ef8 ("thinkpad_acpi: use DEVICE_ATTR_* macros")
> Cc: Bastien Nocera <hadess@hadess.net>
> Cc: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> ---
> v2: kept the original function names, using the DEVICE_ATTR instead

Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] thinkpad_acpi: revert unintentional device attribute renaming
  2015-05-19 17:55 ` Henrique de Moraes Holschuh
@ 2015-05-20  9:19   ` Darren Hart
  0 siblings, 0 replies; 3+ messages in thread
From: Darren Hart @ 2015-05-20  9:19 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Bjørn Mork, platform-driver-x86, ibm-acpi-devel, Bastien Nocera

On Tue, May 19, 2015 at 02:55:17PM -0300, Henrique de Moraes Holschuh wrote:
> On Tue, May 19, 2015, at 14:45, Bjørn Mork wrote:
> > The conversion to DEVICE_ATTR_* macros failed to fixup a few cases where
> > the old attribute names didn't match the show/store function names.
> > Instead of renaming the functions, the attributes were renamed. This
> > caused an unintentional API change.  The hwmon required 'name' attribute
> > were among the renamed attribute, causing libsensors to fail to detect
> > the hwmon device at all.
> > 
> > Fix by using the DEVICE_ATTR macro for these attributes, allowing the
> > show/store functions to keep their system specific prefixes.
> > 
> > Fixes: b4dd04ac6ef8 ("thinkpad_acpi: use DEVICE_ATTR_* macros")
> > Cc: Bastien Nocera <hadess@hadess.net>
> > Cc: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> > Signed-off-by: Bjørn Mork <bjorn@mork.no>
> > ---
> > v2: kept the original function names, using the DEVICE_ATTR instead
> 
> Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

Thanks for the catch Bjørn, much appreciated. Queued for 4.1 fixes.

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-05-20  9:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-19 17:45 [PATCH v2] thinkpad_acpi: revert unintentional device attribute renaming Bjørn Mork
2015-05-19 17:55 ` Henrique de Moraes Holschuh
2015-05-20  9:19   ` Darren Hart

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.