Re: [PATCH] hwmon: applesmc: Add temperature sensor labels to sysfs interface

[Jean Delvare <khali@linux-fr.org>]

Hi Henrik,

On Tue,  4 May 2010 11:55:29 +0200, Henrik Rydberg wrote:
> From: Alex Murray <murray.alex@gmail.com>
> 
> The Apple SMC uses a systematic labeling scheme for the hardware
> temperature sensors. This scheme is currently hidden from
> userland. Since the sensor set, and consequently the numbering,
> differs between models, an extensive database of configurations is
> required for an application such as fan control. This patch adds the
> SMC labels to the hwmon sysfs interface, allowing applications to use
> the sensors more intelligibly.

I like the idea.

> Signed-off-by: Alex Murray <murray.alex@gmail.com>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
>  drivers/hwmon/applesmc.c |  145 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 144 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
> index 36a0d62..8be86ef 100644
> --- a/drivers/hwmon/applesmc.c
> +++ b/drivers/hwmon/applesmc.c
> (...)
> @@ -1252,6 +1387,9 @@ static struct attribute *temperature_attributes[] = {
>  static const struct attribute_group temperature_attributes_group =
>  	{ .attrs = temperature_attributes };
>  
> +static const struct attribute_group label_attributes_group = {
> +	.attrs = label_attributes
> +};

Missing blank like.

>  /* Module stuff */
>  
>  /*
> @@ -1568,7 +1706,8 @@ static int __init applesmc_init(void)
>  	for (i = 0;
>  	     temperature_sensors_sets[applesmc_temperature_set][i] != NULL;
>  	     i++) {
> -		if (temperature_attributes[i] == NULL) {
> +		if (temperature_attributes[i] == NULL ||
> +		    label_attributes[i] == NULL) {
>  			printk(KERN_ERR "applesmc: More temperature sensors "
>  				"in temperature_sensors_sets (at least %i)"
>  				"than available sysfs files in "
> @@ -1578,6 +1717,8 @@ static int __init applesmc_init(void)
>  		}
>  		ret = sysfs_create_file(&pdev->dev.kobj,
>  						temperature_attributes[i]);

Missing error handling.

> +		ret = sysfs_create_file(&pdev->dev.kobj,
> +						label_attributes[i]);
>  		if (ret)
>  			goto out_temperature;
>  	}

Other than that, the patch looks good.

-- 
Jean Delvare