[PATCH 0/5] hwmon updates for x86 platform drivers

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/5] hwmon updates for x86 platform drivers
@ 2013-11-23 19:03 Guenter Roeck
  2013-11-23 19:03 ` [PATCH 1/5] asus-wmi: Convert to use devm_hwmon_device_register_with_groups Guenter Roeck
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Guenter Roeck @ 2013-11-23 19:03 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Corentin Chary, Cezary Jackiewicz, acpi4asus-user,
	platform-driver-x86, lm-sensors, linux-kernel, Guenter Roeck

Common changes:
- Convert to new hwmon API to simplify code and avoid race conditions
  due to late sysfs attribute registration.
  With this change, hwmon sysfs attributes are now always attached
  to the hwmon device and no longer to its parent device (where this
  was the case).
- Replace SENSOR_DEVICE_ATTR with DEVICE_ATTR where appropriate,
  ie if the additional parameter provided by SENSOR_DEVICE_ATTR
  was not used.
- Use devm_kzalloc() instead of kzalloc() where appropriate to reduce
  code size and simplify error path.

All patches were compile tested only. Unfortunately, I don't have
the necessary hardware to test on real systems.

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

* [PATCH 1/5] asus-wmi: Convert to use devm_hwmon_device_register_with_groups
  2013-11-23 19:03 [PATCH 0/5] hwmon updates for x86 platform drivers Guenter Roeck
@ 2013-11-23 19:03 ` Guenter Roeck
  2013-11-23 19:03 ` [PATCH 2/5] compal-laptop: Replace SENSOR_DEVICE_ATTR with DEVICE_ATTR Guenter Roeck
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2013-11-23 19:03 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Corentin Chary, Cezary Jackiewicz, acpi4asus-user,
	platform-driver-x86, lm-sensors, linux-kernel, Guenter Roeck

Simplify the code and avoid race conditions due to late sysfs attribute
registration. Also replace SENSOR_DEVICE_ATTR with DEVICE_ATTR;
the additional parameter is not used and thus unnecessary.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/platform/x86/asus-wmi.c |   48 +++++++++------------------------------
 1 file changed, 11 insertions(+), 37 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 19c313b..e9e22a5 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -184,7 +184,6 @@ struct asus_wmi {
 
 	struct input_dev *inputdev;
 	struct backlight_device *backlight_device;
-	struct device *hwmon_device;
 	struct platform_device *platform_device;
 
 	struct led_classdev wlan_led;
@@ -1071,20 +1070,12 @@ static ssize_t asus_hwmon_temp1(struct device *dev,
 	return sprintf(buf, "%d\n", value);
 }
 
-static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO, asus_hwmon_pwm1, NULL, 0);
-static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, asus_hwmon_temp1, NULL, 0);
-
-static ssize_t
-show_name(struct device *dev, struct device_attribute *attr, char *buf)
-{
-	return sprintf(buf, "asus\n");
-}
-static SENSOR_DEVICE_ATTR(name, S_IRUGO, show_name, NULL, 0);
+static DEVICE_ATTR(pwm1, S_IRUGO, asus_hwmon_pwm1, NULL);
+static DEVICE_ATTR(temp1_input, S_IRUGO, asus_hwmon_temp1, NULL);
 
 static struct attribute *hwmon_attributes[] = {
-	&sensor_dev_attr_pwm1.dev_attr.attr,
-	&sensor_dev_attr_temp1_input.dev_attr.attr,
-	&sensor_dev_attr_name.dev_attr.attr,
+	&dev_attr_pwm1.attr,
+	&dev_attr_temp1_input.attr,
 	NULL
 };
 
@@ -1098,9 +1089,9 @@ static umode_t asus_hwmon_sysfs_is_visible(struct kobject *kobj,
 	int dev_id = -1;
 	u32 value = ASUS_WMI_UNSUPPORTED_METHOD;
 
-	if (attr == &sensor_dev_attr_pwm1.dev_attr.attr)
+	if (attr == &dev_attr_pwm1.attr)
 		dev_id = ASUS_WMI_DEVID_FAN_CTRL;
-	else if (attr == &sensor_dev_attr_temp1_input.dev_attr.attr)
+	else if (attr == &dev_attr_temp1_input.attr)
 		dev_id = ASUS_WMI_DEVID_THERMAL_CTRL;
 
 	if (dev_id != -1) {
@@ -1135,35 +1126,20 @@ static struct attribute_group hwmon_attribute_group = {
 	.is_visible = asus_hwmon_sysfs_is_visible,
 	.attrs = hwmon_attributes
 };
-
-static void asus_wmi_hwmon_exit(struct asus_wmi *asus)
-{
-	struct device *hwmon;
-
-	hwmon = asus->hwmon_device;
-	if (!hwmon)
-		return;
-	sysfs_remove_group(&hwmon->kobj, &hwmon_attribute_group);
-	hwmon_device_unregister(hwmon);
-	asus->hwmon_device = NULL;
-}
+__ATTRIBUTE_GROUPS(hwmon_attribute);
 
 static int asus_wmi_hwmon_init(struct asus_wmi *asus)
 {
 	struct device *hwmon;
-	int result;
 
-	hwmon = hwmon_device_register(&asus->platform_device->dev);
+	hwmon = hwmon_device_register_with_groups(&asus->platform_device->dev,
+						  "asus", asus,
+						  hwmon_attribute_groups);
 	if (IS_ERR(hwmon)) {
 		pr_err("Could not register asus hwmon device\n");
 		return PTR_ERR(hwmon);
 	}
-	dev_set_drvdata(hwmon, asus);
-	asus->hwmon_device = hwmon;
-	result = sysfs_create_group(&hwmon->kobj, &hwmon_attribute_group);
-	if (result)
-		asus_wmi_hwmon_exit(asus);
-	return result;
+	return 0;
 }
 
 /*
@@ -1834,7 +1810,6 @@ fail_backlight:
 fail_rfkill:
 	asus_wmi_led_exit(asus);
 fail_leds:
-	asus_wmi_hwmon_exit(asus);
 fail_hwmon:
 	asus_wmi_input_exit(asus);
 fail_input:
@@ -1852,7 +1827,6 @@ static int asus_wmi_remove(struct platform_device *device)
 	wmi_remove_notify_handler(asus->driver->event_guid);
 	asus_wmi_backlight_exit(asus);
 	asus_wmi_input_exit(asus);
-	asus_wmi_hwmon_exit(asus);
 	asus_wmi_led_exit(asus);
 	asus_wmi_rfkill_exit(asus);
 	asus_wmi_debugfs_exit(asus);
-- 
1.7.9.7


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

* [PATCH 2/5] compal-laptop: Replace SENSOR_DEVICE_ATTR with DEVICE_ATTR
  2013-11-23 19:03 [PATCH 0/5] hwmon updates for x86 platform drivers Guenter Roeck
  2013-11-23 19:03 ` [PATCH 1/5] asus-wmi: Convert to use devm_hwmon_device_register_with_groups Guenter Roeck
@ 2013-11-23 19:03 ` Guenter Roeck
  2013-11-23 19:03 ` [PATCH 3/5] compal-laptop: Use devm_kzalloc to allocate local data structure Guenter Roeck
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2013-11-23 19:03 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Corentin Chary, Cezary Jackiewicz, acpi4asus-user,
	platform-driver-x86, lm-sensors, linux-kernel, Guenter Roeck

The extra argument to SENSOR_DEVICE_ATTR is not used. It is therefore not
necessary to use SENSOR_DEVICE_ATTR in the first place. Replace it with
DEVICE_ATTR.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/platform/x86/compal-laptop.c |   66 +++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/platform/x86/compal-laptop.c b/drivers/platform/x86/compal-laptop.c
index eaa78ed..8b119a7 100644
--- a/drivers/platform/x86/compal-laptop.c
+++ b/drivers/platform/x86/compal-laptop.c
@@ -665,23 +665,23 @@ static DEVICE_ATTR(wake_up_key,
 static DEVICE_ATTR(wake_up_mouse,
 		0644, wake_up_mouse_show,	wake_up_mouse_store);
 
-static SENSOR_DEVICE_ATTR(name,        S_IRUGO, hwmon_name_show,   NULL, 1);
-static SENSOR_DEVICE_ATTR(fan1_input,  S_IRUGO, fan_show,          NULL, 1);
-static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, temp_cpu,          NULL, 1);
-static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, temp_cpu_local,    NULL, 1);
-static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, temp_cpu_DTS,      NULL, 1);
-static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, temp_northbridge,  NULL, 1);
-static SENSOR_DEVICE_ATTR(temp5_input, S_IRUGO, temp_vga,          NULL, 1);
-static SENSOR_DEVICE_ATTR(temp6_input, S_IRUGO, temp_SKIN,         NULL, 1);
-static SENSOR_DEVICE_ATTR(temp1_label, S_IRUGO, label_cpu,         NULL, 1);
-static SENSOR_DEVICE_ATTR(temp2_label, S_IRUGO, label_cpu_local,   NULL, 1);
-static SENSOR_DEVICE_ATTR(temp3_label, S_IRUGO, label_cpu_DTS,     NULL, 1);
-static SENSOR_DEVICE_ATTR(temp4_label, S_IRUGO, label_northbridge, NULL, 1);
-static SENSOR_DEVICE_ATTR(temp5_label, S_IRUGO, label_vga,         NULL, 1);
-static SENSOR_DEVICE_ATTR(temp6_label, S_IRUGO, label_SKIN,        NULL, 1);
-static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, pwm_show, pwm_store, 1);
-static SENSOR_DEVICE_ATTR(pwm1_enable,
-		S_IRUGO | S_IWUSR, pwm_enable_show, pwm_enable_store, 0);
+static DEVICE_ATTR(name,        S_IRUGO, hwmon_name_show,   NULL);
+static DEVICE_ATTR(fan1_input,  S_IRUGO, fan_show,          NULL);
+static DEVICE_ATTR(temp1_input, S_IRUGO, temp_cpu,          NULL);
+static DEVICE_ATTR(temp2_input, S_IRUGO, temp_cpu_local,    NULL);
+static DEVICE_ATTR(temp3_input, S_IRUGO, temp_cpu_DTS,      NULL);
+static DEVICE_ATTR(temp4_input, S_IRUGO, temp_northbridge,  NULL);
+static DEVICE_ATTR(temp5_input, S_IRUGO, temp_vga,          NULL);
+static DEVICE_ATTR(temp6_input, S_IRUGO, temp_SKIN,         NULL);
+static DEVICE_ATTR(temp1_label, S_IRUGO, label_cpu,         NULL);
+static DEVICE_ATTR(temp2_label, S_IRUGO, label_cpu_local,   NULL);
+static DEVICE_ATTR(temp3_label, S_IRUGO, label_cpu_DTS,     NULL);
+static DEVICE_ATTR(temp4_label, S_IRUGO, label_northbridge, NULL);
+static DEVICE_ATTR(temp5_label, S_IRUGO, label_vga,         NULL);
+static DEVICE_ATTR(temp6_label, S_IRUGO, label_SKIN,        NULL);
+static DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, pwm_show, pwm_store);
+static DEVICE_ATTR(pwm1_enable,
+		   S_IRUGO | S_IWUSR, pwm_enable_show, pwm_enable_store);
 
 static struct attribute *compal_attributes[] = {
 	&dev_attr_wake_up_pme.attr,
@@ -692,22 +692,22 @@ static struct attribute *compal_attributes[] = {
 	&dev_attr_wake_up_mouse.attr,
 	/* Maybe put the sensor-stuff in a separate hwmon-driver? That way,
 	 * the hwmon sysfs won't be cluttered with the above files. */
-	&sensor_dev_attr_name.dev_attr.attr,
-	&sensor_dev_attr_pwm1_enable.dev_attr.attr,
-	&sensor_dev_attr_pwm1.dev_attr.attr,
-	&sensor_dev_attr_fan1_input.dev_attr.attr,
-	&sensor_dev_attr_temp1_input.dev_attr.attr,
-	&sensor_dev_attr_temp2_input.dev_attr.attr,
-	&sensor_dev_attr_temp3_input.dev_attr.attr,
-	&sensor_dev_attr_temp4_input.dev_attr.attr,
-	&sensor_dev_attr_temp5_input.dev_attr.attr,
-	&sensor_dev_attr_temp6_input.dev_attr.attr,
-	&sensor_dev_attr_temp1_label.dev_attr.attr,
-	&sensor_dev_attr_temp2_label.dev_attr.attr,
-	&sensor_dev_attr_temp3_label.dev_attr.attr,
-	&sensor_dev_attr_temp4_label.dev_attr.attr,
-	&sensor_dev_attr_temp5_label.dev_attr.attr,
-	&sensor_dev_attr_temp6_label.dev_attr.attr,
+	&dev_attr_name.attr,
+	&dev_attr_pwm1_enable.attr,
+	&dev_attr_pwm1.attr,
+	&dev_attr_fan1_input.attr,
+	&dev_attr_temp1_input.attr,
+	&dev_attr_temp2_input.attr,
+	&dev_attr_temp3_input.attr,
+	&dev_attr_temp4_input.attr,
+	&dev_attr_temp5_input.attr,
+	&dev_attr_temp6_input.attr,
+	&dev_attr_temp1_label.attr,
+	&dev_attr_temp2_label.attr,
+	&dev_attr_temp3_label.attr,
+	&dev_attr_temp4_label.attr,
+	&dev_attr_temp5_label.attr,
+	&dev_attr_temp6_label.attr,
 	NULL
 };
 
-- 
1.7.9.7


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

* [PATCH 3/5] compal-laptop: Use devm_kzalloc to allocate local data structure
  2013-11-23 19:03 [PATCH 0/5] hwmon updates for x86 platform drivers Guenter Roeck
  2013-11-23 19:03 ` [PATCH 1/5] asus-wmi: Convert to use devm_hwmon_device_register_with_groups Guenter Roeck
  2013-11-23 19:03 ` [PATCH 2/5] compal-laptop: Replace SENSOR_DEVICE_ATTR with DEVICE_ATTR Guenter Roeck
@ 2013-11-23 19:03 ` Guenter Roeck
  2013-11-23 19:03 ` [PATCH 4/5] compal-laptop: Use devm_hwmon_device_register_with_groups Guenter Roeck
  2013-11-23 19:03 ` [PATCH 5/5] eeepc-laptop: Convert to use devm_hwmon_device_register_with_groups Guenter Roeck
  4 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2013-11-23 19:03 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Corentin Chary, Cezary Jackiewicz, acpi4asus-user,
	platform-driver-x86, lm-sensors, linux-kernel, Guenter Roeck

Reduce code size and simplify error path.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/platform/x86/compal-laptop.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/x86/compal-laptop.c b/drivers/platform/x86/compal-laptop.c
index 8b119a7..9deb035 100644
--- a/drivers/platform/x86/compal-laptop.c
+++ b/drivers/platform/x86/compal-laptop.c
@@ -1026,24 +1026,21 @@ static int compal_probe(struct platform_device *pdev)
 		return 0;
 
 	/* Fan control */
-	data = kzalloc(sizeof(struct compal_data), GFP_KERNEL);
+	data = devm_kzalloc(&pdev->dev, sizeof(struct compal_data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
 	initialize_fan_control_data(data);
 
 	err = sysfs_create_group(&pdev->dev.kobj, &compal_attribute_group);
-	if (err) {
-		kfree(data);
+	if (err)
 		return err;
-	}
 
 	data->hwmon_dev = hwmon_device_register(&pdev->dev);
 	if (IS_ERR(data->hwmon_dev)) {
 		err = PTR_ERR(data->hwmon_dev);
 		sysfs_remove_group(&pdev->dev.kobj,
 				&compal_attribute_group);
-		kfree(data);
 		return err;
 	}
 
@@ -1083,8 +1080,6 @@ static int compal_remove(struct platform_device *pdev)
 	hwmon_device_unregister(data->hwmon_dev);
 	power_supply_unregister(&data->psy);
 
-	kfree(data);
-
 	sysfs_remove_group(&pdev->dev.kobj, &compal_attribute_group);
 
 	return 0;
-- 
1.7.9.7


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

* [PATCH 4/5] compal-laptop: Use devm_hwmon_device_register_with_groups
  2013-11-23 19:03 [PATCH 0/5] hwmon updates for x86 platform drivers Guenter Roeck
                   ` (2 preceding siblings ...)
  2013-11-23 19:03 ` [PATCH 3/5] compal-laptop: Use devm_kzalloc to allocate local data structure Guenter Roeck
@ 2013-11-23 19:03 ` Guenter Roeck
  2013-11-23 19:03 ` [PATCH 5/5] eeepc-laptop: Convert to use devm_hwmon_device_register_with_groups Guenter Roeck
  4 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2013-11-23 19:03 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Corentin Chary, Cezary Jackiewicz, acpi4asus-user,
	platform-driver-x86, lm-sensors, linux-kernel, Guenter Roeck

Simplify the code and create hwmon attributes as well as hwmon device in one go.

With the new hwmon API, hwmon attributes are now attached to the hwmon device.
Therefore, split hwmon and device attributes into two separate groups.
Platform attributes are still attached to the platform device.

Also use devm_kzalloc to allocate local data structures for further
simplification.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/platform/x86/compal-laptop.c |   52 +++++++++++++++-------------------
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/drivers/platform/x86/compal-laptop.c b/drivers/platform/x86/compal-laptop.c
index 9deb035..7297df2 100644
--- a/drivers/platform/x86/compal-laptop.c
+++ b/drivers/platform/x86/compal-laptop.c
@@ -173,8 +173,7 @@
 /* ======= */
 struct compal_data{
 	/* Fan control */
-	struct device *hwmon_dev;
-	int pwm_enable; /* 0:full on, 1:set by pwm1, 2:control by moterboard */
+	int pwm_enable; /* 0:full on, 1:set by pwm1, 2:control by motherboard */
 	unsigned char curr_pwm;
 
 	/* Power supply */
@@ -402,15 +401,6 @@ SIMPLE_MASKED_STORE_SHOW(wake_up_wlan,	WAKE_UP_ADDR, WAKE_UP_WLAN)
 SIMPLE_MASKED_STORE_SHOW(wake_up_key,	WAKE_UP_ADDR, WAKE_UP_KEY)
 SIMPLE_MASKED_STORE_SHOW(wake_up_mouse,	WAKE_UP_ADDR, WAKE_UP_MOUSE)
 
-
-/* General hwmon interface */
-static ssize_t hwmon_name_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
-{
-	return sprintf(buf, "%s\n", DRIVER_NAME);
-}
-
-
 /* Fan control interface */
 static ssize_t pwm_enable_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
@@ -665,7 +655,6 @@ static DEVICE_ATTR(wake_up_key,
 static DEVICE_ATTR(wake_up_mouse,
 		0644, wake_up_mouse_show,	wake_up_mouse_store);
 
-static DEVICE_ATTR(name,        S_IRUGO, hwmon_name_show,   NULL);
 static DEVICE_ATTR(fan1_input,  S_IRUGO, fan_show,          NULL);
 static DEVICE_ATTR(temp1_input, S_IRUGO, temp_cpu,          NULL);
 static DEVICE_ATTR(temp2_input, S_IRUGO, temp_cpu_local,    NULL);
@@ -683,16 +672,20 @@ static DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, pwm_show, pwm_store);
 static DEVICE_ATTR(pwm1_enable,
 		   S_IRUGO | S_IWUSR, pwm_enable_show, pwm_enable_store);
 
-static struct attribute *compal_attributes[] = {
+static struct attribute *compal_platform_attrs[] = {
 	&dev_attr_wake_up_pme.attr,
 	&dev_attr_wake_up_modem.attr,
 	&dev_attr_wake_up_lan.attr,
 	&dev_attr_wake_up_wlan.attr,
 	&dev_attr_wake_up_key.attr,
 	&dev_attr_wake_up_mouse.attr,
-	/* Maybe put the sensor-stuff in a separate hwmon-driver? That way,
-	 * the hwmon sysfs won't be cluttered with the above files. */
-	&dev_attr_name.attr,
+	NULL
+};
+static struct attribute_group compal_platform_attr_group = {
+	.attrs = compal_platform_attrs
+};
+
+static struct attribute *compal_hwmon_attrs[] = {
 	&dev_attr_pwm1_enable.attr,
 	&dev_attr_pwm1.attr,
 	&dev_attr_fan1_input.attr,
@@ -710,10 +703,7 @@ static struct attribute *compal_attributes[] = {
 	&dev_attr_temp6_label.attr,
 	NULL
 };
-
-static struct attribute_group compal_attribute_group = {
-	.attrs = compal_attributes
-};
+ATTRIBUTE_GROUPS(compal_hwmon);
 
 static int compal_probe(struct platform_device *);
 static int compal_remove(struct platform_device *);
@@ -1021,6 +1011,7 @@ static int compal_probe(struct platform_device *pdev)
 {
 	int err;
 	struct compal_data *data;
+	struct device *hwmon_dev;
 
 	if (!extra_features)
 		return 0;
@@ -1032,16 +1023,16 @@ static int compal_probe(struct platform_device *pdev)
 
 	initialize_fan_control_data(data);
 
-	err = sysfs_create_group(&pdev->dev.kobj, &compal_attribute_group);
+	err = sysfs_create_group(&pdev->dev.kobj, &compal_platform_attr_group);
 	if (err)
 		return err;
 
-	data->hwmon_dev = hwmon_device_register(&pdev->dev);
-	if (IS_ERR(data->hwmon_dev)) {
-		err = PTR_ERR(data->hwmon_dev);
-		sysfs_remove_group(&pdev->dev.kobj,
-				&compal_attribute_group);
-		return err;
+	hwmon_dev = hwmon_device_register_with_groups(&pdev->dev,
+						      DRIVER_NAME, data,
+						      compal_hwmon_groups);
+	if (IS_ERR(hwmon_dev)) {
+		err = PTR_ERR(hwmon_dev);
+		goto remove;
 	}
 
 	/* Power supply */
@@ -1051,6 +1042,10 @@ static int compal_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, data);
 
 	return 0;
+
+remove:
+	sysfs_remove_group(&pdev->dev.kobj, &compal_platform_attr_group);
+	return err;
 }
 
 static void __exit compal_cleanup(void)
@@ -1077,10 +1072,9 @@ static int compal_remove(struct platform_device *pdev)
 	pwm_disable_control();
 
 	data = platform_get_drvdata(pdev);
-	hwmon_device_unregister(data->hwmon_dev);
 	power_supply_unregister(&data->psy);
 
-	sysfs_remove_group(&pdev->dev.kobj, &compal_attribute_group);
+	sysfs_remove_group(&pdev->dev.kobj, &compal_platform_attr_group);
 
 	return 0;
 }
-- 
1.7.9.7


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

* [PATCH 5/5] eeepc-laptop: Convert to use devm_hwmon_device_register_with_groups
  2013-11-23 19:03 [PATCH 0/5] hwmon updates for x86 platform drivers Guenter Roeck
                   ` (3 preceding siblings ...)
  2013-11-23 19:03 ` [PATCH 4/5] compal-laptop: Use devm_hwmon_device_register_with_groups Guenter Roeck
@ 2013-11-23 19:03 ` Guenter Roeck
  4 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2013-11-23 19:03 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Corentin Chary, Cezary Jackiewicz, acpi4asus-user,
	platform-driver-x86, lm-sensors, linux-kernel, Guenter Roeck

Simplify the code and avoid race condition caused by creating sysfs attributes
after creating the hwmon device.

Also replace SENSOR_DEVICE_ATTR with DEVICE_ATTR since the extra argument
is not used and SENSOR_DEVICE_ATTR is not needed.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/platform/x86/eeepc-laptop.c |   52 +++++++----------------------------
 1 file changed, 10 insertions(+), 42 deletions(-)

diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
index aefcc32..c15d339 100644
--- a/drivers/platform/x86/eeepc-laptop.c
+++ b/drivers/platform/x86/eeepc-laptop.c
@@ -166,7 +166,6 @@ struct eeepc_laptop {
 
 	struct platform_device *platform_device;
 	struct acpi_device *device;		/* the device we are in */
-	struct device *hwmon_device;
 	struct backlight_device *backlight_device;
 
 	struct input_dev *inputdev;
@@ -1067,7 +1066,7 @@ static ssize_t show_sys_hwmon(int (*get)(void), char *buf)
 	{								\
 		return store_sys_hwmon(_get, buf, count);		\
 	}								\
-	static SENSOR_DEVICE_ATTR(_name, _mode, show_##_name, store_##_name, 0);
+	static DEVICE_ATTR(_name, _mode, show_##_name, store_##_name);
 
 EEEPC_CREATE_SENSOR_ATTR(fan1_input, S_IRUGO, eeepc_get_fan_rpm, NULL);
 EEEPC_CREATE_SENSOR_ATTR(pwm1, S_IRUGO | S_IWUSR,
@@ -1075,55 +1074,26 @@ EEEPC_CREATE_SENSOR_ATTR(pwm1, S_IRUGO | S_IWUSR,
 EEEPC_CREATE_SENSOR_ATTR(pwm1_enable, S_IRUGO | S_IWUSR,
 			 eeepc_get_fan_ctrl, eeepc_set_fan_ctrl);
 
-static ssize_t
-show_name(struct device *dev, struct device_attribute *attr, char *buf)
-{
-	return sprintf(buf, "eeepc\n");
-}
-static SENSOR_DEVICE_ATTR(name, S_IRUGO, show_name, NULL, 0);
-
-static struct attribute *hwmon_attributes[] = {
-	&sensor_dev_attr_pwm1.dev_attr.attr,
-	&sensor_dev_attr_fan1_input.dev_attr.attr,
-	&sensor_dev_attr_pwm1_enable.dev_attr.attr,
-	&sensor_dev_attr_name.dev_attr.attr,
+static struct attribute *hwmon_attrs[] = {
+	&dev_attr_pwm1.attr,
+	&dev_attr_fan1_input.attr,
+	&dev_attr_pwm1_enable.attr,
 	NULL
 };
-
-static struct attribute_group hwmon_attribute_group = {
-	.attrs = hwmon_attributes
-};
-
-static void eeepc_hwmon_exit(struct eeepc_laptop *eeepc)
-{
-	struct device *hwmon;
-
-	hwmon = eeepc->hwmon_device;
-	if (!hwmon)
-		return;
-	sysfs_remove_group(&hwmon->kobj,
-			   &hwmon_attribute_group);
-	hwmon_device_unregister(hwmon);
-	eeepc->hwmon_device = NULL;
-}
+ATTRIBUTE_GROUPS(hwmon);
 
 static int eeepc_hwmon_init(struct eeepc_laptop *eeepc)
 {
+	struct device *dev = &eeepc->platform_device->dev;
 	struct device *hwmon;
-	int result;
 
-	hwmon = hwmon_device_register(&eeepc->platform_device->dev);
+	hwmon = devm_hwmon_device_register_with_groups(dev, "eeepc", NULL,
+						       hwmon_groups);
 	if (IS_ERR(hwmon)) {
 		pr_err("Could not register eeepc hwmon device\n");
-		eeepc->hwmon_device = NULL;
 		return PTR_ERR(hwmon);
 	}
-	eeepc->hwmon_device = hwmon;
-	result = sysfs_create_group(&hwmon->kobj,
-				    &hwmon_attribute_group);
-	if (result)
-		eeepc_hwmon_exit(eeepc);
-	return result;
+	return 0;
 }
 
 /*
@@ -1481,7 +1451,6 @@ static int eeepc_acpi_add(struct acpi_device *device)
 fail_rfkill:
 	eeepc_led_exit(eeepc);
 fail_led:
-	eeepc_hwmon_exit(eeepc);
 fail_hwmon:
 	eeepc_input_exit(eeepc);
 fail_input:
@@ -1501,7 +1470,6 @@ static int eeepc_acpi_remove(struct acpi_device *device)
 	eeepc_backlight_exit(eeepc);
 	eeepc_rfkill_exit(eeepc);
 	eeepc_input_exit(eeepc);
-	eeepc_hwmon_exit(eeepc);
 	eeepc_led_exit(eeepc);
 	eeepc_platform_exit(eeepc);
 
-- 
1.7.9.7


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

end of thread, other threads:[~2013-11-23 19:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-23 19:03 [PATCH 0/5] hwmon updates for x86 platform drivers Guenter Roeck
2013-11-23 19:03 ` [PATCH 1/5] asus-wmi: Convert to use devm_hwmon_device_register_with_groups Guenter Roeck
2013-11-23 19:03 ` [PATCH 2/5] compal-laptop: Replace SENSOR_DEVICE_ATTR with DEVICE_ATTR Guenter Roeck
2013-11-23 19:03 ` [PATCH 3/5] compal-laptop: Use devm_kzalloc to allocate local data structure Guenter Roeck
2013-11-23 19:03 ` [PATCH 4/5] compal-laptop: Use devm_hwmon_device_register_with_groups Guenter Roeck
2013-11-23 19:03 ` [PATCH 5/5] eeepc-laptop: Convert to use devm_hwmon_device_register_with_groups Guenter Roeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox