Hi again,

Jean Delvare wrote:
>>
>> However, I'm not really satisfied with the way sysfs files are created:
>> I use a lot of preprocessor macros to avoid repetition of code.
>> The files created with these macros in /sys/devices/platform/applesmc are
>> the following (on a Macbook Pro):
>> fan0_actual_speed
>> fan0_manual
>> fan0_maximum_speed
>> fan0_minimum_speed
>> fan0_safe_speed
>> fan0_target_speed
>> fan1_actual_speed
>> fan1_manual
>> fan1_maximum_speed
>> fan1_minimum_speed
>> fan1_safe_speed
>> fan1_target_speed
>> temperature_0
>> temperature_1
>> temperature_2
>> temperature_3
>> temperature_4
>> temperature_5
>> temperature_6
>>     
>
> First of all, please read Documentation/hwmon/sysfs-documentation, and
> rename the entries to match the standard names whenever possible. Also
> make sure that you use the standard units. If you use the standard
> names and units and if you register your device with the hwmon class,
> standard monitoring application will be able to support your driver.
>   

Fixed.

[snip]

>> Also, I never call any sysfs_remove_* function, as the files are
>> deleted when the module is unloaded. Is it safe to do so? Doesn't it
>> cause any memory leak?
>>     
>
> This is considered a bad practice, as in theory you driver shouldn't
> create the device by itself, and the files are associated to the device,
> not the driver. All hardware monitoring drivers have been fixed now, so
> please add the file removal calls in your driver too. You might find it
> easier to use file groups rather than individual files. Again, see for
> example the f71805f driver, and in particular the f71805f_attributes
> array and f71805f_group structure, and the sysfs_create_group() and
> sysfs_remove_group() calls.
>   

Fixed too.

I also added some sanity checks, and some minor features I discovered
using key enumeration (see next patch).

Best regards,

Nicolas

- Standardize applesmc to use sysfs filenames recommended by
  Documentation/hwmon/sysfs-interface, and register the device with the hwmon
  class.
- Use snprintf instead of sprintf in sysfs show handlers.
- Remove the sysfs files properly in case of initialisation problem, and when
  the driver is unloaded.
- Add data buffer length sanity checks.
- Improvements of SMC keys' comments (add data type reported by the device).
- Add temperature sensors to Macbook Pro.
- Add support for reading fan physical position (e.g. "Left Side")

Signed-off-by: Nicolas Boichat <[EMAIL PROTECTED]>
---

 drivers/hwmon/applesmc.c |  280 ++++++++++++++++++++++++++++++++--------------
 1 files changed, 192 insertions(+), 88 deletions(-)

diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index f7b59fc..531bc9a 100644
--- a/drivers/hwmon/applesmc.c
+++ b/drivers/hwmon/applesmc.c
@@ -37,40 +37,48 @@
 #include <linux/hwmon-sysfs.h>
 #include <asm/io.h>
 #include <linux/leds.h>
+#include <linux/hwmon.h>
 
-/* data port used by apple SMC */
+/* data port used by Apple SMC */
 #define APPLESMC_DATA_PORT     0x300
-/* command/status port used by apple SMC */
+/* command/status port used by Apple SMC */
 #define APPLESMC_CMD_PORT      0x304
 
-#define APPLESMC_NR_PORTS      5 /* 0x300-0x304 */
+#define APPLESMC_NR_PORTS      32 /* 0x300-0x31f */
+
+#define APPLESMC_MAX_DATA_LENGTH 32
 
 #define APPLESMC_STATUS_MASK   0x0f
 #define APPLESMC_READ_CMD      0x10
 #define APPLESMC_WRITE_CMD     0x11
 
-#define LIGHT_SENSOR_LEFT_KEY  "ALV0" /* r-o length 6 */
-#define LIGHT_SENSOR_RIGHT_KEY "ALV1" /* r-o length 6 */
-#define BACKLIGHT_KEY          "LKSB" /* w-o */
+#define LIGHT_SENSOR_LEFT_KEY  "ALV0" /* r-o {alv (6 bytes) */
+#define LIGHT_SENSOR_RIGHT_KEY "ALV1" /* r-o {alv (6 bytes) */
+#define BACKLIGHT_KEY          "LKSB" /* w-o {lkb (2 bytes) */
 
-#define CLAMSHELL_KEY          "MSLD" /* r-o length 1 (unused) */
+#define CLAMSHELL_KEY          "MSLD" /* r-o ui8 (unused) */
 
-#define MOTION_SENSOR_X_KEY    "MO_X" /* r-o length 2 */
-#define MOTION_SENSOR_Y_KEY    "MO_Y" /* r-o length 2 */
-#define MOTION_SENSOR_Z_KEY    "MO_Z" /* r-o length 2 */
-#define MOTION_SENSOR_KEY      "MOCN" /* r/w length 2 */
+#define MOTION_SENSOR_X_KEY    "MO_X" /* r-o sp78 (2 bytes) */
+#define MOTION_SENSOR_Y_KEY    "MO_Y" /* r-o sp78 (2 bytes) */
+#define MOTION_SENSOR_Z_KEY    "MO_Z" /* r-o sp78 (2 bytes) */
+#define MOTION_SENSOR_KEY      "MOCN" /* r/w ui16 */
 
-#define FANS_COUNT             "FNum" /* r-o length 1 */
-#define FANS_MANUAL            "FS! " /* r-w length 2 */
-#define FAN_ACTUAL_SPEED       "F0Ac" /* r-o length 2 */
-#define FAN_MIN_SPEED          "F0Mn" /* r-o length 2 */
-#define FAN_MAX_SPEED          "F0Mx" /* r-o length 2 */
-#define FAN_SAFE_SPEED         "F0Sf" /* r-o length 2 */
-#define FAN_TARGET_SPEED       "F0Tg" /* r-w length 2 */
+#define FANS_COUNT             "FNum" /* r-o ui8 */
+#define FANS_MANUAL            "FS! " /* r-w ui16 */
+#define FAN_ACTUAL_SPEED       "F0Ac" /* r-o fpe2 (2 bytes) */
+#define FAN_MIN_SPEED          "F0Mn" /* r-o fpe2 (2 bytes) */
+#define FAN_MAX_SPEED          "F0Mx" /* r-o fpe2 (2 bytes) */
+#define FAN_SAFE_SPEED         "F0Sf" /* r-o fpe2 (2 bytes) */
+#define FAN_TARGET_SPEED       "F0Tg" /* r-w fpe2 (2 bytes) */
+#define FAN_POSITION           "F0ID" /* r-o char[16] */
 
-/* Temperature sensors keys. First set for Macbook(Pro), second for Macmini */
-static const char* temperature_sensors_sets[][8] = {
-       { "TB0T", "TC0D", "TC0P", "Th0H", "Ts0P", "Th1H", "Ts1P", NULL },
+/*
+ * Temperature sensors keys (sp78 - 2 bytes).
+ * First set for Macbook(Pro), second for Macmini.
+ */
+static const char* temperature_sensors_sets[][13] = {
+       { "TA0P", "TB0T", "TC0D", "TC0P", "TG0H", "TG0P", "TG0T", "Th0H",
+         "Th1H", "Tm0P", "Ts0P", "Ts1P", NULL },
        { "TC0D", "TC0P", NULL }
 };
 
@@ -110,6 +118,7 @@ static s16 rest_x;
 static s16 rest_y;
 static struct timer_list applesmc_timer;
 static struct input_dev *applesmc_idev;
+static struct class_device *hwmon_class_dev;
 
 /* Indicates whether this computer has an accelerometer. */
 static unsigned int applesmc_accelerometer;
@@ -152,17 +161,22 @@ static int __wait_status(u8 val)
  */
 static int applesmc_read_key(const char* key, u8* buffer, u8 len)
 {
-       int ret = -EIO;
        int i;
 
+       if (len > APPLESMC_MAX_DATA_LENGTH) {
+               printk(KERN_ERR "applesmc_read_key: cannot read more than "
+                                       "%d bytes\n", APPLESMC_MAX_DATA_LENGTH);
+               return -EINVAL;
+       }
+
        outb(APPLESMC_READ_CMD, APPLESMC_CMD_PORT);
        if (__wait_status(0x0c))
-               goto out;
+               return -EIO;
        
        for (i = 0; i < 4; i++) {
                outb(key[i], APPLESMC_DATA_PORT);
                if (__wait_status(0x04))
-                       goto out;
+                       return -EIO;
        }
        if (debug)
                printk(KERN_DEBUG "<%s", key);
@@ -173,7 +187,7 @@ static int applesmc_read_key(const char* key, u8* buffer, 
u8 len)
 
        for (i = 0; i < len; i++) {
                if (__wait_status(0x05))
-                       goto out;
+                       return -EIO;
                buffer[i] = inb(APPLESMC_DATA_PORT);
                if (debug)
                        printk(KERN_DEBUG "<%x", buffer[i]);
@@ -181,10 +195,7 @@ static int applesmc_read_key(const char* key, u8* buffer, 
u8 len)
        if (debug)
                printk(KERN_DEBUG "\n");
 
-       ret = 0;
-
-out:
-       return ret;
+       return 0;
 }
 
 /*
@@ -194,30 +205,33 @@ out:
  */
 static int applesmc_write_key(const char* key, u8* buffer, u8 len)
 {
-       int ret = -EIO;
        int i;
 
+       if (len > APPLESMC_MAX_DATA_LENGTH) {
+               printk(KERN_ERR "applesmc_write_key: cannot write more than "
+                                       "%d bytes\n", APPLESMC_MAX_DATA_LENGTH);
+               return -EINVAL;
+       }
+
        outb(APPLESMC_WRITE_CMD, APPLESMC_CMD_PORT);
        if (__wait_status(0x0c))
-               goto out;
+               return -EIO;
        
        for (i = 0; i < 4; i++) {
                outb(key[i], APPLESMC_DATA_PORT);
                if (__wait_status(0x04))
-                       goto out;
+                       return -EIO;
        }
 
        outb(len, APPLESMC_DATA_PORT);
 
        for (i = 0; i < len; i++) {
                if (__wait_status(0x04))
-                       goto out;
+                       return -EIO;
                outb(buffer[i], APPLESMC_DATA_PORT);
        }
 
-       ret = 0;
-out:
-       return ret;
+       return 0;
 }
 
 /*
@@ -415,7 +429,7 @@ out:
        if (ret)
                return ret;
        else
-               return sprintf(buf, "(%d,%d,%d)\n", x, y, z);
+               return snprintf(buf, PAGE_SIZE, "(%d,%d,%d)\n", x, y, z);
 }
 
 static ssize_t applesmc_light_show(struct device *dev,
@@ -439,10 +453,10 @@ out:
        if (ret)
                return ret;
        else
-               return sprintf(sysfsbuf, "(%d,%d)\n", left, right);
+               return snprintf(sysfsbuf, PAGE_SIZE, "(%d,%d)\n", left, right);
 }
 
-/* Displays degree Celsius * 100 */
+/* Displays degree Celsius * 1000 */
 static ssize_t applesmc_show_temperature(struct device *dev,
                        struct device_attribute *devattr, char *sysfsbuf)
 {
@@ -456,15 +470,15 @@ static ssize_t applesmc_show_temperature(struct device 
*dev,
        mutex_lock(&applesmc_lock);
 
        ret = applesmc_read_key(key, buffer, 2);
-       temp = buffer[0]*100;
-       temp += (buffer[1] >> 6) * 25;
+       temp = buffer[0]*1000;
+       temp += (buffer[1] >> 6) * 250;
 
        mutex_unlock(&applesmc_lock);
 
        if (ret)
                return ret;
        else
-               return sprintf(sysfsbuf, "%u\n", temp);
+               return snprintf(sysfsbuf, PAGE_SIZE, "%u\n", temp);
 }
 
 static ssize_t applesmc_show_fan_speed(struct device *dev,
@@ -492,7 +506,7 @@ static ssize_t applesmc_show_fan_speed(struct device *dev,
        if (ret)
                return ret;
        else
-               return sprintf(sysfsbuf, "%u\n", speed);
+               return snprintf(sysfsbuf, PAGE_SIZE, "%u\n", speed);
 }
 
 static ssize_t applesmc_store_fan_speed(struct device *dev,
@@ -547,7 +561,7 @@ static ssize_t applesmc_show_fan_manual(struct device *dev,
        if (ret)
                return ret;
        else
-               return sprintf(sysfsbuf, "%d\n", manual);
+               return snprintf(sysfsbuf, PAGE_SIZE, "%d\n", manual);
 }
 
 static ssize_t applesmc_store_fan_manual(struct device *dev,
@@ -587,10 +601,37 @@ out:
                return count;
 }
 
+static ssize_t applesmc_show_fan_position(struct device *dev,
+                               struct device_attribute *attr, char *sysfsbuf)
+{
+       int ret;
+       char newkey[5];
+       u8 buffer[17];
+       struct sensor_device_attribute_2 *sensor_attr =
+                                               to_sensor_dev_attr_2(attr);
+
+       newkey[0] = FAN_POSITION[0];
+       newkey[1] = '0' + sensor_attr->index;
+       newkey[2] = FAN_POSITION[2];
+       newkey[3] = FAN_POSITION[3];
+       newkey[4] = 0;
+
+       mutex_lock(&applesmc_lock);
+
+       ret = applesmc_read_key(newkey, buffer, 16);
+       buffer[16] = 0;
+
+       mutex_unlock(&applesmc_lock);
+       if (ret)
+               return ret;
+       else
+               return snprintf(sysfsbuf, PAGE_SIZE, "%s\n", buffer+4);
+}
+
 static ssize_t applesmc_calibrate_show(struct device *dev,
                                struct device_attribute *attr, char *sysfsbuf)
 {
-       return sprintf(sysfsbuf, "(%d,%d)\n", rest_x, rest_y);
+       return snprintf(sysfsbuf, PAGE_SIZE, "(%d,%d)\n", rest_x, rest_y);
 }
 
 static ssize_t applesmc_calibrate_store(struct device *dev,
@@ -625,6 +666,15 @@ static DEVICE_ATTR(position, 0444, applesmc_position_show, 
NULL);
 static DEVICE_ATTR(calibrate, 0644,
                        applesmc_calibrate_show, applesmc_calibrate_store);
 
+static struct attribute *accelerometer_attributes[] = {
+       &dev_attr_position.attr,
+       &dev_attr_calibrate.attr,
+       NULL
+};
+
+static const struct attribute_group accelerometer_attributes_group =
+       { .attrs = accelerometer_attributes };
+
 static DEVICE_ATTR(light, 0444, applesmc_light_show, NULL);
 
 /*
@@ -637,31 +687,35 @@ static DEVICE_ATTR(light, 0444, applesmc_light_show, 
NULL);
  *  - show/store manual mode
  */
 #define sysfs_fan_speeds_offset(offset) \
-static SENSOR_DEVICE_ATTR_2(fan##offset##_actual_speed, S_IRUGO, \
-                       applesmc_show_fan_speed, NULL, 0, offset); \
+static SENSOR_DEVICE_ATTR_2(fan##offset##_input, S_IRUGO, \
+                       applesmc_show_fan_speed, NULL, 0, offset-1); \
 \
-static SENSOR_DEVICE_ATTR_2(fan##offset##_minimum_speed, S_IRUGO | S_IWUSR, \
-       applesmc_show_fan_speed, applesmc_store_fan_speed, 1, offset); \
+static SENSOR_DEVICE_ATTR_2(fan##offset##_min, S_IRUGO | S_IWUSR, \
+       applesmc_show_fan_speed, applesmc_store_fan_speed, 1, offset-1); \
 \
-static SENSOR_DEVICE_ATTR_2(fan##offset##_maximum_speed, S_IRUGO, \
-                       applesmc_show_fan_speed, NULL, 2, offset); \
+static SENSOR_DEVICE_ATTR_2(fan##offset##_max, S_IRUGO, \
+                       applesmc_show_fan_speed, NULL, 2, offset-1); \
 \
-static SENSOR_DEVICE_ATTR_2(fan##offset##_safe_speed, S_IRUGO, \
-                       applesmc_show_fan_speed, NULL, 3, offset); \
+static SENSOR_DEVICE_ATTR_2(fan##offset##_safe, S_IRUGO, \
+                       applesmc_show_fan_speed, NULL, 3, offset-1); \
 \
-static SENSOR_DEVICE_ATTR_2(fan##offset##_target_speed, S_IRUGO | S_IWUSR, \
-       applesmc_show_fan_speed, applesmc_store_fan_speed, 4, offset); \
+static SENSOR_DEVICE_ATTR_2(fan##offset##_output, S_IRUGO | S_IWUSR, \
+       applesmc_show_fan_speed, applesmc_store_fan_speed, 4, offset-1); \
 \
 static SENSOR_DEVICE_ATTR(fan##offset##_manual, S_IRUGO | S_IWUSR, \
-       applesmc_show_fan_manual, applesmc_store_fan_manual, offset); \
+       applesmc_show_fan_manual, applesmc_store_fan_manual, offset-1); \
+\
+static SENSOR_DEVICE_ATTR(fan##offset##_position, S_IRUGO, \
+       applesmc_show_fan_position, NULL, offset-1); \
 \
 static struct attribute *fan##offset##_attributes[] = { \
-       &sensor_dev_attr_fan##offset##_actual_speed.dev_attr.attr, \
-       &sensor_dev_attr_fan##offset##_minimum_speed.dev_attr.attr, \
-       &sensor_dev_attr_fan##offset##_maximum_speed.dev_attr.attr, \
-       &sensor_dev_attr_fan##offset##_safe_speed.dev_attr.attr, \
-       &sensor_dev_attr_fan##offset##_target_speed.dev_attr.attr, \
+       &sensor_dev_attr_fan##offset##_input.dev_attr.attr, \
+       &sensor_dev_attr_fan##offset##_min.dev_attr.attr, \
+       &sensor_dev_attr_fan##offset##_max.dev_attr.attr, \
+       &sensor_dev_attr_fan##offset##_safe.dev_attr.attr, \
+       &sensor_dev_attr_fan##offset##_output.dev_attr.attr, \
        &sensor_dev_attr_fan##offset##_manual.dev_attr.attr, \
+       &sensor_dev_attr_fan##offset##_position.dev_attr.attr, \
        NULL \
 };
 
@@ -669,42 +723,61 @@ static struct attribute *fan##offset##_attributes[] = { \
  * Create the needed functions for each fan using the macro defined above 
  * (2 fans are supported)
  */
-sysfs_fan_speeds_offset(0);
 sysfs_fan_speeds_offset(1);
+sysfs_fan_speeds_offset(2);
 
 static const struct attribute_group fan_attribute_groups[] = {
-       { .attrs = fan0_attributes },
-       { .attrs = fan1_attributes }
+       { .attrs = fan1_attributes },
+       { .attrs = fan2_attributes }
 };
 
 /*
  * Temperature sensors sysfs entries.
  */
-static SENSOR_DEVICE_ATTR(temperature_0, S_IRUGO,
+static SENSOR_DEVICE_ATTR(temp_1_input, S_IRUGO,
                                        applesmc_show_temperature, NULL, 0);
-static SENSOR_DEVICE_ATTR(temperature_1, S_IRUGO,
+static SENSOR_DEVICE_ATTR(temp_2_input, S_IRUGO,
                                        applesmc_show_temperature, NULL, 1);
-static SENSOR_DEVICE_ATTR(temperature_2, S_IRUGO,
+static SENSOR_DEVICE_ATTR(temp_3_input, S_IRUGO,
                                        applesmc_show_temperature, NULL, 2);
-static SENSOR_DEVICE_ATTR(temperature_3, S_IRUGO,
+static SENSOR_DEVICE_ATTR(temp_4_input, S_IRUGO,
                                        applesmc_show_temperature, NULL, 3);
-static SENSOR_DEVICE_ATTR(temperature_4, S_IRUGO,
+static SENSOR_DEVICE_ATTR(temp_5_input, S_IRUGO,
                                        applesmc_show_temperature, NULL, 4);
-static SENSOR_DEVICE_ATTR(temperature_5, S_IRUGO,
+static SENSOR_DEVICE_ATTR(temp_6_input, S_IRUGO,
                                        applesmc_show_temperature, NULL, 5);
-static SENSOR_DEVICE_ATTR(temperature_6, S_IRUGO,
+static SENSOR_DEVICE_ATTR(temp_7_input, S_IRUGO,
                                        applesmc_show_temperature, NULL, 6);
+static SENSOR_DEVICE_ATTR(temp_8_input, S_IRUGO,
+                                       applesmc_show_temperature, NULL, 7);
+static SENSOR_DEVICE_ATTR(temp_9_input, S_IRUGO,
+                                       applesmc_show_temperature, NULL, 8);
+static SENSOR_DEVICE_ATTR(temp_10_input, S_IRUGO,
+                                       applesmc_show_temperature, NULL, 9);
+static SENSOR_DEVICE_ATTR(temp_11_input, S_IRUGO,
+                                       applesmc_show_temperature, NULL, 10);
+static SENSOR_DEVICE_ATTR(temp_12_input, S_IRUGO,
+                                       applesmc_show_temperature, NULL, 11);
 
 static struct attribute *temperature_attributes[] = {
-       &sensor_dev_attr_temperature_0.dev_attr.attr,
-       &sensor_dev_attr_temperature_1.dev_attr.attr,
-       &sensor_dev_attr_temperature_2.dev_attr.attr,
-       &sensor_dev_attr_temperature_3.dev_attr.attr,
-       &sensor_dev_attr_temperature_4.dev_attr.attr,
-       &sensor_dev_attr_temperature_5.dev_attr.attr,
-       &sensor_dev_attr_temperature_6.dev_attr.attr,
+       &sensor_dev_attr_temp_1_input.dev_attr.attr,
+       &sensor_dev_attr_temp_2_input.dev_attr.attr,
+       &sensor_dev_attr_temp_3_input.dev_attr.attr,
+       &sensor_dev_attr_temp_4_input.dev_attr.attr,
+       &sensor_dev_attr_temp_5_input.dev_attr.attr,
+       &sensor_dev_attr_temp_6_input.dev_attr.attr,
+       &sensor_dev_attr_temp_7_input.dev_attr.attr,
+       &sensor_dev_attr_temp_8_input.dev_attr.attr,
+       &sensor_dev_attr_temp_9_input.dev_attr.attr,
+       &sensor_dev_attr_temp_10_input.dev_attr.attr,
+       &sensor_dev_attr_temp_11_input.dev_attr.attr,
+       &sensor_dev_attr_temp_12_input.dev_attr.attr,
+       NULL
 };
 
+static const struct attribute_group temperature_attributes_group =
+       { .attrs = temperature_attributes };
+
 /* Module stuff */
 
 /* 
@@ -734,18 +807,15 @@ static int applesmc_create_accelerometer(void)
 {
        int ret;
 
-       ret = sysfs_create_file(&pdev->dev.kobj, &dev_attr_position.attr);
-       if (ret)
-               goto out;
-
-       ret = sysfs_create_file(&pdev->dev.kobj, &dev_attr_calibrate.attr);
+       ret = sysfs_create_group(&pdev->dev.kobj,
+                                       &accelerometer_attributes_group);
        if (ret)
                goto out;
 
        applesmc_idev = input_allocate_device();
        if (!applesmc_idev) {
                ret = -ENOMEM;
-               goto out;
+               goto out_sysfs;
        }
 
        /* initial calibrate for the input device */
@@ -777,6 +847,9 @@ static int applesmc_create_accelerometer(void)
 out_idev:
        input_free_device(applesmc_idev);
 
+out_sysfs:
+       sysfs_remove_group(&pdev->dev.kobj, &accelerometer_attributes_group);   
+
 out:
        printk(KERN_WARNING "applesmc: driver init failed (ret=%d)!\n", ret);
        return ret;
@@ -787,6 +860,7 @@ static void applesmc_release_accelerometer(void)
 {
        del_timer_sync(&applesmc_timer);
        input_unregister_device(applesmc_idev);
+       sysfs_remove_group(&pdev->dev.kobj, &accelerometer_attributes_group);
 }
 
 static __initdata struct dmi_match_data applesmc_dmi_data[] = {
@@ -867,7 +941,7 @@ static int __init applesmc_init(void)
                        ret = sysfs_create_group(&pdev->dev.kobj,
                                                 &fan_attribute_groups[0]);
                        if (ret)
-                               goto out_device;
+                               goto out_fan_1;
                case 0:
                        ;
                }
@@ -876,16 +950,24 @@ static int __init applesmc_init(void)
        for (i = 0;
             temperature_sensors_sets[applesmc_temperature_set][i] != NULL;
             i++) {
+               if (temperature_attributes[i] == NULL) {
+                       printk(KERN_ERR "applesmc: More temperature sensors "
+                               "in temperature_sensors_sets (at least %i)"
+                               "than available sysfs files in "
+                               "temperature_attributes (%i), please report "
+                               "this bug.\n", i, i-1);
+                       goto out_temperature;
+               }
                ret = sysfs_create_file(&pdev->dev.kobj,
                                                temperature_attributes[i]);
                if (ret)
-                       goto out_device;
+                       goto out_temperature;
        }
 
        if (applesmc_accelerometer) {
                ret = applesmc_create_accelerometer();
                if (ret)
-                       goto out_device;
+                       goto out_temperature;
        }
 
        if (applesmc_light) {
@@ -897,15 +979,33 @@ static int __init applesmc_init(void)
                /* register as a led device */
                ret = led_classdev_register(&pdev->dev, &applesmc_backlight);
                if (ret < 0)
-                       goto out_accelerometer;
+                       goto out_light_sysfs;
+       }
+
+       hwmon_class_dev = hwmon_device_register(&pdev->dev);
+       if (IS_ERR(hwmon_class_dev)) {
+               ret = PTR_ERR(hwmon_class_dev);
+               goto out_light;
        }
 
        printk(KERN_INFO "applesmc: driver successfully loaded.\n");
+
        return 0;
 
+out_light:
+       if (applesmc_light)
+               led_classdev_unregister(&applesmc_backlight);
+out_light_sysfs:
+       if (applesmc_light)
+               sysfs_remove_file(&pdev->dev.kobj, &dev_attr_light.attr);
 out_accelerometer:
        if (applesmc_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_device:
        platform_device_unregister(pdev);
 out_driver:
@@ -919,10 +1019,14 @@ out:
 
 static void __exit applesmc_exit(void)
 {
+       hwmon_device_unregister(hwmon_class_dev);       
        if (applesmc_light)
                led_classdev_unregister(&applesmc_backlight);
        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]);
        platform_device_unregister(pdev);
        platform_driver_unregister(&applesmc_driver);
        release_region(APPLESMC_DATA_PORT, APPLESMC_NR_PORTS);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to