* [lm-sensors] [PATCH] hwmon: (gpio-fan) Use is_visible to determine if attributes should be created
@ 2013-03-30 16:19 Guenter Roeck
2013-04-02 10:40 ` [lm-sensors] [PATCH] hwmon: (gpio-fan) Use is_visible to determine if attributes should be creat Simon Guinot
2013-04-03 0:34 ` Guenter Roeck
0 siblings, 2 replies; 3+ messages in thread
From: Guenter Roeck @ 2013-03-30 16:19 UTC (permalink / raw)
To: lm-sensors
Simplify code and reduce object size by more than 300 bytes (x86_64).
Cc: Jamie Lentin <jm@lentin.co.uk>
Cc: Simon Guinot <sguinot@lacie.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/hwmon/gpio-fan.c | 104 +++++++++++++++++-----------------------------
1 file changed, 39 insertions(+), 65 deletions(-)
diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
index 4e02480..3104149 100644
--- a/drivers/hwmon/gpio-fan.c
+++ b/drivers/hwmon/gpio-fan.c
@@ -105,10 +105,6 @@ static int fan_alarm_init(struct gpio_fan_data *fan_data,
if (err)
return err;
- err = device_create_file(&pdev->dev, &dev_attr_fan1_alarm);
- if (err)
- return err;
-
/*
* If the alarm GPIO don't support interrupts, just leave
* without initializing the fail notification support.
@@ -121,23 +117,9 @@ static int fan_alarm_init(struct gpio_fan_data *fan_data,
irq_set_irq_type(alarm_irq, IRQ_TYPE_EDGE_BOTH);
err = devm_request_irq(&pdev->dev, alarm_irq, fan_alarm_irq_handler,
IRQF_SHARED, "GPIO fan alarm", fan_data);
- if (err)
- goto err_free_sysfs;
-
- return 0;
-
-err_free_sysfs:
- device_remove_file(&pdev->dev, &dev_attr_fan1_alarm);
return err;
}
-static void fan_alarm_free(struct gpio_fan_data *fan_data)
-{
- struct platform_device *pdev = fan_data->pdev;
-
- device_remove_file(&pdev->dev, &dev_attr_fan1_alarm);
-}
-
/*
* Control GPIOs.
*/
@@ -327,6 +309,12 @@ exit_unlock:
return ret;
}
+static ssize_t show_name(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "gpio-fan\n");
+}
+
static DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, show_pwm, set_pwm);
static DEVICE_ATTR(pwm1_enable, S_IRUGO | S_IWUSR,
show_pwm_enable, set_pwm_enable);
@@ -336,8 +324,26 @@ static DEVICE_ATTR(fan1_max, S_IRUGO, show_rpm_max, NULL);
static DEVICE_ATTR(fan1_input, S_IRUGO, show_rpm, NULL);
static DEVICE_ATTR(fan1_target, S_IRUGO | S_IWUSR, show_rpm, set_rpm);
-static struct attribute *gpio_fan_ctrl_attributes[] = {
- &dev_attr_pwm1.attr,
+static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
+
+static umode_t gpio_fan_is_visible(struct kobject *kobj,
+ struct attribute *attr, int index)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct gpio_fan_data *data = dev_get_drvdata(dev);
+
+ if (index = 1 && !data->alarm)
+ return 0;
+ if (index > 1 && !data->ctrl)
+ return 0;
+
+ return attr->mode;
+}
+
+static struct attribute *gpio_fan_attributes[] = {
+ &dev_attr_name.attr,
+ &dev_attr_fan1_alarm.attr, /* 1 */
+ &dev_attr_pwm1.attr, /* 2 */
&dev_attr_pwm1_enable.attr,
&dev_attr_pwm1_mode.attr,
&dev_attr_fan1_input.attr,
@@ -347,8 +353,9 @@ static struct attribute *gpio_fan_ctrl_attributes[] = {
NULL
};
-static const struct attribute_group gpio_fan_ctrl_group = {
- .attrs = gpio_fan_ctrl_attributes,
+static const struct attribute_group gpio_fan_group = {
+ .attrs = gpio_fan_attributes,
+ .is_visible = gpio_fan_is_visible,
};
static int fan_ctrl_init(struct gpio_fan_data *fan_data,
@@ -379,30 +386,9 @@ static int fan_ctrl_init(struct gpio_fan_data *fan_data,
if (fan_data->speed_index < 0)
return -ENODEV;
- err = sysfs_create_group(&pdev->dev.kobj, &gpio_fan_ctrl_group);
- return err;
-}
-
-static void fan_ctrl_free(struct gpio_fan_data *fan_data)
-{
- struct platform_device *pdev = fan_data->pdev;
-
- sysfs_remove_group(&pdev->dev.kobj, &gpio_fan_ctrl_group);
-}
-
-/*
- * Platform driver.
- */
-
-static ssize_t show_name(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- return sprintf(buf, "gpio-fan\n");
+ return 0;
}
-static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
-
-
#ifdef CONFIG_OF_GPIO
/*
* Translate OpenFirmware node properties into platform_data
@@ -546,38 +532,30 @@ static int gpio_fan_probe(struct platform_device *pdev)
/* Configure control GPIOs if available. */
if (pdata->ctrl && pdata->num_ctrl > 0) {
- if (!pdata->speed || pdata->num_speed <= 1) {
- err = -EINVAL;
- goto err_free_alarm;
- }
+ if (!pdata->speed || pdata->num_speed <= 1)
+ return -EINVAL;
err = fan_ctrl_init(fan_data, pdata);
if (err)
- goto err_free_alarm;
+ return err;
}
- err = device_create_file(&pdev->dev, &dev_attr_name);
+ err = sysfs_create_group(&pdev->dev.kobj, &gpio_fan_group);
if (err)
- goto err_free_ctrl;
+ return err;
/* Make this driver part of hwmon class. */
fan_data->hwmon_dev = hwmon_device_register(&pdev->dev);
if (IS_ERR(fan_data->hwmon_dev)) {
err = PTR_ERR(fan_data->hwmon_dev);
- goto err_remove_name;
+ goto err_remove;
}
dev_info(&pdev->dev, "GPIO fan initialized\n");
return 0;
-err_remove_name:
- device_remove_file(&pdev->dev, &dev_attr_name);
-err_free_ctrl:
- if (fan_data->ctrl)
- fan_ctrl_free(fan_data);
-err_free_alarm:
- if (fan_data->alarm)
- fan_alarm_free(fan_data);
+err_remove:
+ sysfs_remove_group(&pdev->dev.kobj, &gpio_fan_group);
return err;
}
@@ -586,11 +564,7 @@ static int gpio_fan_remove(struct platform_device *pdev)
struct gpio_fan_data *fan_data = platform_get_drvdata(pdev);
hwmon_device_unregister(fan_data->hwmon_dev);
- device_remove_file(&pdev->dev, &dev_attr_name);
- if (fan_data->alarm)
- fan_alarm_free(fan_data);
- if (fan_data->ctrl)
- fan_ctrl_free(fan_data);
+ sysfs_remove_group(&pdev->dev.kobj, &gpio_fan_group);
return 0;
}
--
1.7.9.7
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [lm-sensors] [PATCH] hwmon: (gpio-fan) Use is_visible to determine if attributes should be creat
2013-03-30 16:19 [lm-sensors] [PATCH] hwmon: (gpio-fan) Use is_visible to determine if attributes should be created Guenter Roeck
@ 2013-04-02 10:40 ` Simon Guinot
2013-04-03 0:34 ` Guenter Roeck
1 sibling, 0 replies; 3+ messages in thread
From: Simon Guinot @ 2013-04-02 10:40 UTC (permalink / raw)
To: lm-sensors
[-- Attachment #1.1: Type: text/plain, Size: 6604 bytes --]
On Sat, Mar 30, 2013 at 09:19:19AM -0700, Guenter Roeck wrote:
> Simplify code and reduce object size by more than 300 bytes (x86_64).
>
> Cc: Jamie Lentin <jm@lentin.co.uk>
> Cc: Simon Guinot <sguinot@lacie.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/hwmon/gpio-fan.c | 104 +++++++++++++++++-----------------------------
> 1 file changed, 39 insertions(+), 65 deletions(-)
Hi Guenter,
It is quite a smart optimization. Just we have to take care about
attribute ordering now.
I have tested you patch on an ns2max board, with several setup
combinations (alarm and controls). For each of them, the expected sysfs
attributes are available and the fan is also working.
Tested-by: Simon Guinot <simon.guinot@sequanux.org>
Thanks,
Simon
>
> diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
> index 4e02480..3104149 100644
> --- a/drivers/hwmon/gpio-fan.c
> +++ b/drivers/hwmon/gpio-fan.c
> @@ -105,10 +105,6 @@ static int fan_alarm_init(struct gpio_fan_data *fan_data,
> if (err)
> return err;
>
> - err = device_create_file(&pdev->dev, &dev_attr_fan1_alarm);
> - if (err)
> - return err;
> -
> /*
> * If the alarm GPIO don't support interrupts, just leave
> * without initializing the fail notification support.
> @@ -121,23 +117,9 @@ static int fan_alarm_init(struct gpio_fan_data *fan_data,
> irq_set_irq_type(alarm_irq, IRQ_TYPE_EDGE_BOTH);
> err = devm_request_irq(&pdev->dev, alarm_irq, fan_alarm_irq_handler,
> IRQF_SHARED, "GPIO fan alarm", fan_data);
> - if (err)
> - goto err_free_sysfs;
> -
> - return 0;
> -
> -err_free_sysfs:
> - device_remove_file(&pdev->dev, &dev_attr_fan1_alarm);
> return err;
> }
>
> -static void fan_alarm_free(struct gpio_fan_data *fan_data)
> -{
> - struct platform_device *pdev = fan_data->pdev;
> -
> - device_remove_file(&pdev->dev, &dev_attr_fan1_alarm);
> -}
> -
> /*
> * Control GPIOs.
> */
> @@ -327,6 +309,12 @@ exit_unlock:
> return ret;
> }
>
> +static ssize_t show_name(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "gpio-fan\n");
> +}
> +
> static DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, show_pwm, set_pwm);
> static DEVICE_ATTR(pwm1_enable, S_IRUGO | S_IWUSR,
> show_pwm_enable, set_pwm_enable);
> @@ -336,8 +324,26 @@ static DEVICE_ATTR(fan1_max, S_IRUGO, show_rpm_max, NULL);
> static DEVICE_ATTR(fan1_input, S_IRUGO, show_rpm, NULL);
> static DEVICE_ATTR(fan1_target, S_IRUGO | S_IWUSR, show_rpm, set_rpm);
>
> -static struct attribute *gpio_fan_ctrl_attributes[] = {
> - &dev_attr_pwm1.attr,
> +static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
> +
> +static umode_t gpio_fan_is_visible(struct kobject *kobj,
> + struct attribute *attr, int index)
> +{
> + struct device *dev = container_of(kobj, struct device, kobj);
> + struct gpio_fan_data *data = dev_get_drvdata(dev);
> +
> + if (index == 1 && !data->alarm)
> + return 0;
> + if (index > 1 && !data->ctrl)
> + return 0;
> +
> + return attr->mode;
> +}
> +
> +static struct attribute *gpio_fan_attributes[] = {
> + &dev_attr_name.attr,
> + &dev_attr_fan1_alarm.attr, /* 1 */
> + &dev_attr_pwm1.attr, /* 2 */
> &dev_attr_pwm1_enable.attr,
> &dev_attr_pwm1_mode.attr,
> &dev_attr_fan1_input.attr,
> @@ -347,8 +353,9 @@ static struct attribute *gpio_fan_ctrl_attributes[] = {
> NULL
> };
>
> -static const struct attribute_group gpio_fan_ctrl_group = {
> - .attrs = gpio_fan_ctrl_attributes,
> +static const struct attribute_group gpio_fan_group = {
> + .attrs = gpio_fan_attributes,
> + .is_visible = gpio_fan_is_visible,
> };
>
> static int fan_ctrl_init(struct gpio_fan_data *fan_data,
> @@ -379,30 +386,9 @@ static int fan_ctrl_init(struct gpio_fan_data *fan_data,
> if (fan_data->speed_index < 0)
> return -ENODEV;
>
> - err = sysfs_create_group(&pdev->dev.kobj, &gpio_fan_ctrl_group);
> - return err;
> -}
> -
> -static void fan_ctrl_free(struct gpio_fan_data *fan_data)
> -{
> - struct platform_device *pdev = fan_data->pdev;
> -
> - sysfs_remove_group(&pdev->dev.kobj, &gpio_fan_ctrl_group);
> -}
> -
> -/*
> - * Platform driver.
> - */
> -
> -static ssize_t show_name(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - return sprintf(buf, "gpio-fan\n");
> + return 0;
> }
>
> -static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
> -
> -
> #ifdef CONFIG_OF_GPIO
> /*
> * Translate OpenFirmware node properties into platform_data
> @@ -546,38 +532,30 @@ static int gpio_fan_probe(struct platform_device *pdev)
>
> /* Configure control GPIOs if available. */
> if (pdata->ctrl && pdata->num_ctrl > 0) {
> - if (!pdata->speed || pdata->num_speed <= 1) {
> - err = -EINVAL;
> - goto err_free_alarm;
> - }
> + if (!pdata->speed || pdata->num_speed <= 1)
> + return -EINVAL;
> err = fan_ctrl_init(fan_data, pdata);
> if (err)
> - goto err_free_alarm;
> + return err;
> }
>
> - err = device_create_file(&pdev->dev, &dev_attr_name);
> + err = sysfs_create_group(&pdev->dev.kobj, &gpio_fan_group);
> if (err)
> - goto err_free_ctrl;
> + return err;
>
> /* Make this driver part of hwmon class. */
> fan_data->hwmon_dev = hwmon_device_register(&pdev->dev);
> if (IS_ERR(fan_data->hwmon_dev)) {
> err = PTR_ERR(fan_data->hwmon_dev);
> - goto err_remove_name;
> + goto err_remove;
> }
>
> dev_info(&pdev->dev, "GPIO fan initialized\n");
>
> return 0;
>
> -err_remove_name:
> - device_remove_file(&pdev->dev, &dev_attr_name);
> -err_free_ctrl:
> - if (fan_data->ctrl)
> - fan_ctrl_free(fan_data);
> -err_free_alarm:
> - if (fan_data->alarm)
> - fan_alarm_free(fan_data);
> +err_remove:
> + sysfs_remove_group(&pdev->dev.kobj, &gpio_fan_group);
> return err;
> }
>
> @@ -586,11 +564,7 @@ static int gpio_fan_remove(struct platform_device *pdev)
> struct gpio_fan_data *fan_data = platform_get_drvdata(pdev);
>
> hwmon_device_unregister(fan_data->hwmon_dev);
> - device_remove_file(&pdev->dev, &dev_attr_name);
> - if (fan_data->alarm)
> - fan_alarm_free(fan_data);
> - if (fan_data->ctrl)
> - fan_ctrl_free(fan_data);
> + sysfs_remove_group(&pdev->dev.kobj, &gpio_fan_group);
>
> return 0;
> }
> --
> 1.7.9.7
>
>
> _______________________________________________
> lm-sensors mailing list
> lm-sensors@lm-sensors.org
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 153 bytes --]
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [lm-sensors] [PATCH] hwmon: (gpio-fan) Use is_visible to determine if attributes should be creat
2013-03-30 16:19 [lm-sensors] [PATCH] hwmon: (gpio-fan) Use is_visible to determine if attributes should be created Guenter Roeck
2013-04-02 10:40 ` [lm-sensors] [PATCH] hwmon: (gpio-fan) Use is_visible to determine if attributes should be creat Simon Guinot
@ 2013-04-03 0:34 ` Guenter Roeck
1 sibling, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2013-04-03 0:34 UTC (permalink / raw)
To: lm-sensors
On Tue, Apr 02, 2013 at 12:40:45PM +0200, Simon Guinot wrote:
> On Sat, Mar 30, 2013 at 09:19:19AM -0700, Guenter Roeck wrote:
> > Simplify code and reduce object size by more than 300 bytes (x86_64).
> >
> > Cc: Jamie Lentin <jm@lentin.co.uk>
> > Cc: Simon Guinot <sguinot@lacie.com>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> > drivers/hwmon/gpio-fan.c | 104 +++++++++++++++++-----------------------------
> > 1 file changed, 39 insertions(+), 65 deletions(-)
>
> Hi Guenter,
>
> It is quite a smart optimization. Just we have to take care about
> attribute ordering now.
>
Agreed, but since this is what the 'index' argument to is_visible is for
in the first place, it should be acceptable.
> I have tested you patch on an ns2max board, with several setup
> combinations (alarm and controls). For each of them, the expected sysfs
> attributes are available and the fan is also working.
>
> Tested-by: Simon Guinot <simon.guinot@sequanux.org>
>
Thanks!
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-04-03 0:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-30 16:19 [lm-sensors] [PATCH] hwmon: (gpio-fan) Use is_visible to determine if attributes should be created Guenter Roeck
2013-04-02 10:40 ` [lm-sensors] [PATCH] hwmon: (gpio-fan) Use is_visible to determine if attributes should be creat Simon Guinot
2013-04-03 0:34 ` Guenter Roeck
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.