[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20100504185108.01fe2124@hyperion.delvare>
Date: Tue, 4 May 2010 18:51:08 +0200
From: Jean Delvare <khali@...ux-fr.org>
To: "Henrik Rydberg" <rydberg@...omail.se>
Cc: lm-sensors@...sensors.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] hwmon: applesmc: Correct sysfs fan error handling
On Tue, 4 May 2010 15:39:02 +0200, Henrik Rydberg wrote:
> The current code will not remove the sysfs files for fan numbers three
> and up. Also, upon exit, fans one and two are removed regardless of
> their existence. This patch cleans up the sysfs error handling for
> the fans.
>
> Signed-off-by: Henrik Rydberg <rydberg@...omail.se>
> ---
> drivers/hwmon/applesmc.c | 61 +++++++++++++++++++---------------------------
> 1 files changed, 25 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
> index 36a0d62..57c0331 100644
> --- a/drivers/hwmon/applesmc.c
> +++ b/drivers/hwmon/applesmc.c
> @@ -209,6 +209,9 @@ static unsigned int applesmc_accelerometer;
> /* Indicates whether this computer has light sensors and keyboard backlight. */
> static unsigned int applesmc_light;
>
> +/* The number of fans handled by the driver */
> +static unsigned int fans_handled;
> +
> /* Indicates which temperature sensors set to use. */
> static unsigned int applesmc_temperature_set;
>
> @@ -1530,39 +1533,24 @@ static int __init applesmc_init(void)
>
> /* create fan files */
> count = applesmc_get_fan_count();
> - if (count < 0) {
> + if (count < 0)
> printk(KERN_ERR "applesmc: Cannot get the number of fans.\n");
> - } else {
> + else
> printk(KERN_INFO "applesmc: %d fans found.\n", count);
>
> - switch (count) {
> - default:
> - printk(KERN_WARNING "applesmc: More than 4 fans found,"
> - " but at most 4 fans are supported"
> - " by the driver.\n");
> - case 4:
> - ret = sysfs_create_group(&pdev->dev.kobj,
> - &fan_attribute_groups[3]);
> - if (ret)
> - goto out_key_enumeration;
> - case 3:
> - ret = sysfs_create_group(&pdev->dev.kobj,
> - &fan_attribute_groups[2]);
> - if (ret)
> - goto out_key_enumeration;
> - case 2:
> - ret = sysfs_create_group(&pdev->dev.kobj,
> - &fan_attribute_groups[1]);
> - if (ret)
> - goto out_key_enumeration;
> - case 1:
> - ret = sysfs_create_group(&pdev->dev.kobj,
> - &fan_attribute_groups[0]);
> - if (ret)
> - goto out_fan_1;
> - case 0:
> - ;
> - }
> + if (count > 4) {
> + count = 4;
> + printk(KERN_WARNING "applesmc: More than 4 fans found,"
> + " but at most 4 fans are supported"
> + " by the driver.\n");
> + }
> +
> + while (fans_handled < count) {
> + ret = sysfs_create_group(&pdev->dev.kobj,
> + &fan_attribute_groups[fans_handled]);
> + if (ret)
> + goto out_fans;
> + fans_handled++;
> }
>
> for (i = 0;
> @@ -1631,10 +1619,10 @@ out_accelerometer:
> applesmc_release_accelerometer();
> out_temperature:
> sysfs_remove_group(&pdev->dev.kobj, &temperature_attributes_group);
> - sysfs_remove_group(&pdev->dev.kobj, &fan_attribute_groups[0]);
> -out_fan_1:
> - sysfs_remove_group(&pdev->dev.kobj, &fan_attribute_groups[1]);
> -out_key_enumeration:
> +out_fans:
> + while (fans_handled)
> + sysfs_remove_group(&pdev->dev.kobj,
> + &fan_attribute_groups[--fans_handled]);
> sysfs_remove_group(&pdev->dev.kobj, &key_enumeration_group);
> out_name:
> sysfs_remove_file(&pdev->dev.kobj, &dev_attr_name.attr);
> @@ -1660,8 +1648,9 @@ static void __exit applesmc_exit(void)
> if (applesmc_accelerometer)
> applesmc_release_accelerometer();
> sysfs_remove_group(&pdev->dev.kobj, &temperature_attributes_group);
> - sysfs_remove_group(&pdev->dev.kobj, &fan_attribute_groups[0]);
> - sysfs_remove_group(&pdev->dev.kobj, &fan_attribute_groups[1]);
> + while (fans_handled)
> + sysfs_remove_group(&pdev->dev.kobj,
> + &fan_attribute_groups[--fans_handled]);
> sysfs_remove_group(&pdev->dev.kobj, &key_enumeration_group);
> sysfs_remove_file(&pdev->dev.kobj, &dev_attr_name.attr);
> platform_device_unregister(pdev);
Applied, thanks.
--
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists