Subject: Re: [PATCH 1/1]: thermal driver therm_adt746.c

From: Benjamin Herrenschmidt <[email-masked]>
Date: Thu, 26 Feb 2015 08:12:43 +1100

On Wed, 2015-02-25 at 12:24 +0100, Thomas Haschka wrote:
> Hi,
> 
> Thanks for your comments. Here's a new patch taking them into account.

Thanks. Note that this legalese boilerplate is not useful, the
Signed-off-by: is enough to indicate your acceptance of the Linux
licencing terms. However I do need a properly formatted changeset
comment.

Cheers,
Ben.

> Cheers,
> 
>  
>         (a) The contribution was created in whole or in part by me and I
>             have the right to submit it under the GPL v2 
> 	    (Gnu Public License Version 2) 
> 
>         (b) The contribution is based upon previous work that, to the best
>             of my knowledge, is covered under an appropriate open source
>             license and I have the right under that license to submit that
>             work with modifications, whether created in whole or in part
>             by me, under the same open source license (unless I am
>             permitted to submit under a different license), as indicated
>             in the file; or
> 
>         (c) I understand and agree that this project and the contribution
>             are public and that a record of the contribution (including all
>             personal information I submit with it, including my sign-off) is
>             maintained indefinitely and may be redistributed consistent with
>             this project or the open source license(s) involved.
> 
> 	Signed-off-by: Thomas Haschka <[email-masked]>
> 
> --- linux/drivers/macintosh/therm_adt746x.c.orig	2015-02-25 11:26:43.164000000 +0100
> +++ linux/drivers/macintosh/therm_adt746x.c	2015-02-25 11:40:54.240000000 +0100
> @@ -1,7 +1,8 @@
>  /*
>   * Device driver for the i2c thermostat found on the iBook G4, Albook G4
>   *
> - * Copyright (C) 2003, 2004 Colin Leroy, Rasmus Rohde, Benjamin Herrenschmidt
> + * Copyright (C) 2003, 2004, 2015 
> + *   Colin Leroy, Rasmus Rohde, Benjamin Herrenschmidt, Thomas Haschka
>   *
>   * Documentation from 115254175ADT7467_pra.pdf and 3686221171167ADT7460_b.pdf
>   * http://www.onsemi.com/PowerSolutions/product.do?id=ADT7467
> @@ -45,8 +46,10 @@ static u8 REM_CONTROL[2] = {0x00, 0x40};
>  static u8 FAN_SPEED[2]   = {0x28, 0x2a};
>  static u8 FAN_SPD_SET[2] = {0x30, 0x31};
>  
> -static u8 default_limits_local[3] = {70, 50, 70};    /* local, sensor1, sensor2 */
> -static u8 default_limits_chip[3] = {80, 65, 80};    /* local, sensor1, sensor2 */
> +/* Local sensor is down to 45 in order to assure stable harddrive and 
> + * components temperature on single fan machines */
> +static u8 default_limits_local[3] = {45, 50, 70}; /* local, sensor1, sensor2 */
> +static u8 default_limits_chip[3] = {80, 65, 80}; /* local, sensor1, sensor2 */
>  static const char *sensor_location[3] = { "?", "?", "?" };
>  
>  static int limit_adjust;
> @@ -223,7 +226,7 @@ static void display_stats(struct thermos
>  }
>  #endif
>  
> -static void update_fans_speed (struct thermostat *th)
> +static void update_fans_speed_multifan (struct thermostat *th)
>  {
>  	int lastvar = 0; /* last variation, for iBook */
>  	int i = 0;
> @@ -278,6 +281,83 @@ static void update_fans_speed (struct th
>  	}
>  }
>  
> +static void update_fans_speed_singlefan (struct thermostat *th) {
> +
> +	/* Single Fan Laptops i.e. 12 inch Powerbook, Ibook etc.
> +	 * Necessites the readout of all three thermal sensors
> +	 * in order to avoid the harddisk and other components to overheat
> +	 * in the case of cold CPU. */
> +	
> +	int lastvar = 0; /* last variation, for iBook */
> +	int i = 0;
> +	int var = 0;
> +	int var_sensor[3];
> +	int started = 0;
> +	int fan_number = 0;
> +	for (i = 0; i < 3; i++) {
> +		var_sensor[i] = th->temps[i] - th->limits[i];
> +	}
> +	var = var_sensor[0];
> +	for (i = 1; i < 3; i++) {
> +		if (var_sensor[i] > var) {
> +			var = var_sensor[i]; 
> +		}
> +	}
> +	if (var > -1) {
> +		int step = (255 - fan_speed) / 7;
> +		int new_speed = 0;
> +		/* hysteresis : change fan speed only if variation is
> +		 * more than two degrees */
> +		if (abs(var - th->last_var[fan_number]) > 2) { 
> +
> +			started = 1;
> +			new_speed = fan_speed + ((var-1)*step);
> +
> +			if (new_speed < fan_speed)
> +				new_speed = fan_speed;
> +			if (new_speed > 255)
> +				new_speed = 255;
> +
> +			if (verbose)
> +				printk(KERN_DEBUG "adt746x:"
> +				       " Setting fans speed to %d "
> +				       "(limit exceeded by %d on %s)\n",
> +				       new_speed, var,
> +				       sensor_location[fan_number+1]);
> +			write_both_fan_speed(th, new_speed);
> +			th->last_var[fan_number] = var; 
> +		}
> +	} else if (var < -2) {
> +		/* don't stop fan if sensor2 is cold and sensor1 is not
> +		 * so cold (lastvar >= -1) */
> +		if (i == 2 && lastvar < -1) {
> +			if (th->last_speed[fan_number] != 0)
> +				if (verbose)
> +					printk(KERN_DEBUG "adt746x:"
> +					       " Stopping "
> +					       "fans.\n");
> +			write_both_fan_speed(th, 0);
> +		}
> +	}
> +
> +	lastvar = var;
> +
> +	if (started)
> +		return; /* we don't want to re-stop the fan
> +			 * if sensor1 is heating and sensor2 is not */
> +}
> +
> +static void update_fans_speed(struct thermostat *th) {
> +
> +	if (th->type == ADT7460) {
> +		/* Multifan Laptops */
> +		update_fans_speed_multifan(th);
> +	} else {
> +		/* Singlefan Laptops i.e. 12 inch Powerbook, Ibook etc. */
> +		update_fans_speed_singlefan(th);
> +	}
> +}
> +
>  static int monitor_task(void *arg)
>  {
>  	struct thermostat* th = arg;
> @@ -371,10 +451,18 @@ static ssize_t store_##name(struct devic
>  	return n;						\
>  }
>  
> -BUILD_SHOW_FUNC_INT(sensor1_temperature,	 (read_reg(th, TEMP_REG[1])))
> -BUILD_SHOW_FUNC_INT(sensor2_temperature,	 (read_reg(th, TEMP_REG[2])))
> -BUILD_SHOW_FUNC_INT(sensor1_limit,		 th->limits[1])
> -BUILD_SHOW_FUNC_INT(sensor2_limit,		 th->limits[2])
> +/* Several nits are multiplied by 1000 in order to have
> + * coherent readings with other 
> + * temperature sensors such as intel coretemp. This further
> + * allows programs such as xosview to read these sensors correctly */
> +
> +BUILD_SHOW_FUNC_INT(sensor0_temperature,       (read_reg(th, TEMP_REG[0]))*1000)
> +BUILD_SHOW_FUNC_INT(sensor1_temperature,       (read_reg(th, TEMP_REG[1]))*1000)
> +BUILD_SHOW_FUNC_INT(sensor2_temperature,       (read_reg(th, TEMP_REG[2]))*1000)
> +BUILD_SHOW_FUNC_INT(sensor0_limit,               (th->limits[0])*1000)
> +BUILD_SHOW_FUNC_INT(sensor1_limit,		 (th->limits[1])*1000)
> +BUILD_SHOW_FUNC_INT(sensor2_limit,		 (th->limits[2])*1000)
> +BUILD_SHOW_FUNC_STR(sensor0_location,            sensor_location[0])
>  BUILD_SHOW_FUNC_STR(sensor1_location,		 sensor_location[1])
>  BUILD_SHOW_FUNC_STR(sensor2_location,		 sensor_location[2])
>  
> @@ -387,14 +475,20 @@ BUILD_SHOW_FUNC_FAN(sensor2_fan_speed,
>  BUILD_SHOW_FUNC_INT_LITE(limit_adjust,	 limit_adjust)
>  BUILD_STORE_FUNC_DEG(limit_adjust,	 th)
>  		
> +static DEVICE_ATTR(sensor0_temperature,	S_IRUGO,
> +		   show_sensor0_temperature,NULL);
>  static DEVICE_ATTR(sensor1_temperature,	S_IRUGO,
>  		   show_sensor1_temperature,NULL);
>  static DEVICE_ATTR(sensor2_temperature,	S_IRUGO,
>  		   show_sensor2_temperature,NULL);
> +static DEVICE_ATTR(sensor0_limit, S_IRUGO,
> +		   show_sensor0_limit,	NULL);
>  static DEVICE_ATTR(sensor1_limit, S_IRUGO,
>  		   show_sensor1_limit,	NULL);
>  static DEVICE_ATTR(sensor2_limit, S_IRUGO,
>  		   show_sensor2_limit,	NULL);
> +static DEVICE_ATTR(sensor0_location, S_IRUGO,
> +		   show_sensor0_location, NULL);
>  static DEVICE_ATTR(sensor1_location, S_IRUGO,
>  		   show_sensor1_location, NULL);
>  static DEVICE_ATTR(sensor2_location, S_IRUGO,
> @@ -426,10 +520,13 @@ static void thermostat_create_files(stru
>  		return;
>  	dev = &th->pdev->dev;
>  	dev_set_drvdata(dev, th);
> -	err = device_create_file(dev, &dev_attr_sensor1_temperature);
> +	err = device_create_file(dev, &dev_attr_sensor0_temperature);
> +	err |= device_create_file(dev, &dev_attr_sensor1_temperature);
>  	err |= device_create_file(dev, &dev_attr_sensor2_temperature);
> +	err |= device_create_file(dev, &dev_attr_sensor0_limit);
>  	err |= device_create_file(dev, &dev_attr_sensor1_limit);
>  	err |= device_create_file(dev, &dev_attr_sensor2_limit);
> +	err |= device_create_file(dev, &dev_attr_sensor0_location);
>  	err |= device_create_file(dev, &dev_attr_sensor1_location);
>  	err |= device_create_file(dev, &dev_attr_sensor2_location);
>  	err |= device_create_file(dev, &dev_attr_limit_adjust);
> @@ -449,10 +546,13 @@ static void thermostat_remove_files(stru
>  	if (!th->pdev)
>  		return;
>  	dev = &th->pdev->dev;
> +	device_remove_file(dev, &dev_attr_sensor0_temperature);
>  	device_remove_file(dev, &dev_attr_sensor1_temperature);
>  	device_remove_file(dev, &dev_attr_sensor2_temperature);
> +	device_remove_file(dev, &dev_attr_sensor0_limit);
>  	device_remove_file(dev, &dev_attr_sensor1_limit);
>  	device_remove_file(dev, &dev_attr_sensor2_limit);
> +	device_remove_file(dev, &dev_attr_sensor0_location);
>  	device_remove_file(dev, &dev_attr_sensor1_location);
>  	device_remove_file(dev, &dev_attr_sensor2_location);
>  	device_remove_file(dev, &dev_attr_limit_adjust);
> 
> On Tue, Feb 24, 2015 at 08:18:38AM +1100, Benjamin Herrenschmidt wrote:
> > Hi ! Please try sending patches inline rather than as attachments, it
> > makes replying a bit easier... Also don't CC stable, we can shoot to
> > stable later on if we think it's justified but first we need to get the
> > patch upstream
> > 
> > A few comments:
> > 
> > On Mon, 2015-02-23 at 12:58 +0100, Thomas Haschka wrote:
> > > --- linux/drivers/macintosh/therm_adt746x.c.orig        2015-02-23 12:19:03.984000000 +0100
> > > +++ linux/drivers/macintosh/therm_adt746x.c     2015-02-23 12:22:34.980000000 +0100
> > > @@ -1,7 +1,8 @@
> > >  /*
> > >   * Device driver for the i2c thermostat found on the iBook G4, Albook G4
> > >   *
> > > - * Copyright (C) 2003, 2004 Colin Leroy, Rasmus Rohde, Benjamin Herrenschmidt
> > > + * Copyright (C) 2003, 2004, 2015 
> > > + *   Colin Leroy, Rasmus Rohde, Benjamin Herrenschmidt, Thomas Haschka
> > >   *
> > >   * Documentation from 115254175ADT7467_pra.pdf and 3686221171167ADT7460_b.pdf
> > >   * http://www.onsemi.com/PowerSolutions/product.do?id=ADT7467
> > > @@ -45,7 +46,7 @@ static u8 REM_CONTROL[2] = {0x00, 0x40};
> > >  static u8 FAN_SPEED[2]   = {0x28, 0x2a};
> > >  static u8 FAN_SPD_SET[2] = {0x30, 0x31};
> > >  
> > > -static u8 default_limits_local[3] = {70, 50, 70};    /* local, sensor1, sensor2 */
> > > +static u8 default_limits_local[3] = {45, 50, 70};    /* local, sensor1, sensor2 */
> > 
> > Here you change the limit for the local sensor for existing machines,
> > care to explain ? I *think* that got adjusted a while back due to
> > a bunch of bogus error on some machines.
> > 
> > >  static u8 default_limits_chip[3] = {80, 65, 80};    /* local, sensor1, sensor2 */
> > >  static const char *sensor_location[3] = { "?", "?", "?" };
> > >  
> > > @@ -225,59 +226,123 @@ static void display_stats(struct thermos
> > >  
> > >  static void update_fans_speed (struct thermostat *th)
> > >  {
> > > -       int lastvar = 0; /* last variation, for iBook */
> > > -       int i = 0;
> > > -
> > > -       /* we don't care about local sensor, so we start at sensor 1 */
> > > -       for (i = 1; i < 3; i++) {
> > > -               int started = 0;
> > > -               int fan_number = (th->type == ADT7460 && i == 2);
> > > -               int var = th->temps[i] - th->limits[i];
> > > -
> > > -               if (var > -1) {
> > > -                       int step = (255 - fan_speed) / 7;
> > > -                       int new_speed = 0;
> > > +       
> > > +       /* Multfan Laptops */
> > > +       if ( th->type == ADT7460 ) {
> > > +                int lastvar = 0; /* last variation, for iBook */
> > > +               int i = 0;
> > > +                /* we don't care about local sensor, so we start at sensor 1 */
> > > +               for (i = 1; i < 3; i++) {
> > > +                       int started = 0;
> > > +                       int fan_number = (th->type == ADT7460 && i == 2);
> > > +                       int var = th->temps[i] - th->limits[i];
> > > +
> > > +                       if (var > -1) {
> > > +                               int step = (255 - fan_speed) / 7;
> > > +                               int new_speed = 0;
> > 
> > The function is too big, please break it down into two sub functions,
> > one for multifan and one for single fan.
> > 
> > It is also unclear due to the indentation changes whether you changed
> > the behaviour on "other" laptops. makes the review a bit harder.
> > 
> > >                         /* hysteresis : change fan speed only if variation is
> > >                          * more than two degrees */
> > > -                       if (abs(var - th->last_var[fan_number]) < 2)
> > > -                               continue;
> > > -
> > > -                       started = 1;
> > > -                       new_speed = fan_speed + ((var-1)*step);
> > > +                               if (abs(var - th->last_var[fan_number]) < 2)
> > > +                                       continue;
> > >  
> > > -                       if (new_speed < fan_speed)
> > > -                               new_speed = fan_speed;
> > > -                       if (new_speed > 255)
> > > -                               new_speed = 255;
> > > +                               started = 1;
> > > +                               new_speed = fan_speed + ((var-1)*step);
> > >  
> > > -                       if (verbose)
> > > -                               printk(KERN_DEBUG "adt746x: Setting fans speed to %d "
> > > -                                                "(limit exceeded by %d on %s)\n",
> > > -                                               new_speed, var,
> > > -                                               sensor_location[fan_number+1]);
> > > -                       write_both_fan_speed(th, new_speed);
> > > -                       th->last_var[fan_number] = var;
> > > -               } else if (var < -2) {
> > > +                               if (new_speed < fan_speed)
> > > +                                       new_speed = fan_speed;
> > > +                               if (new_speed > 255)
> > > +                                       new_speed = 255;
> > > +
> > > +                               if (verbose)
> > > +                                       printk(KERN_DEBUG "adt746x: Setting fans speed to %d "
> > > +                                                        "(limit exceeded by %d on %s)\n",
> > > +                                                       new_speed, var,
> > > +                                                       sensor_location[fan_number+1]);
> > > +                               write_both_fan_speed(th, new_speed);
> > > +                               th->last_var[fan_number] = var;
> > > +                       } else if (var < -2) {
> > >                         /* don't stop fan if sensor2 is cold and sensor1 is not
> > >                          * so cold (lastvar >= -1) */
> > > -                       if (i == 2 && lastvar < -1) {
> > > -                               if (th->last_speed[fan_number] != 0)
> > > -                                       if (verbose)
> > > +                               if (i == 2 && lastvar < -1) {
> > > +                                       if (th->last_speed[fan_number] != 0)
> > > +                                               if (verbose)
> > > +                                                       printk(KERN_DEBUG "adt746x: Stopping "
> > > +                                                               "fans.\n");
> > > +                                       write_both_fan_speed(th, 0);
> > > +                               }
> > > +                       }
> > > +
> > > +                       lastvar = var;
> > > +
> > > +                       if (started)
> > > +                               return; /* we don't want to re-stop the fan
> > > +                               * if sensor1 is heating and sensor2 is not */
> > > +               }
> > > +       
> > > +       } else {
> > > +               /*single fan laptops i.e. 12 inch powerbook/ibook*/
> > > +                int lastvar = 0; /* last variation, for iBook */
> > > +               int i = 0;
> > > +               int var = 0;
> > > +               int var_sensor[3];
> > > +               int started = 0;
> > > +               int fan_number = 0;
> > > +               for (i = 0; i < 3; i++) {
> > > +                       var_sensor[i] = th->temps[i] - th->limits[i];
> > > +               }
> > > +               var = var_sensor[0];
> > > +               for (i = 1; i < 3; i++) {
> > > +                       if (var_sensor[i] > var) {
> > > +                               var = var_sensor[i]; 
> > > +                       }
> > > +               }
> > > +               if (var > -1) {
> > > +                       int step = (255 - fan_speed) / 7;
> > > +                       int new_speed = 0;
> > > +
> > > +                       /* hysteresis : change fan speed only if variation is
> > > +                        * more than two degrees */
> > > +                       if (abs(var - th->last_var[fan_number]) > 2) { 
> > > +
> > > +                       started = 1;
> > > +                       new_speed = fan_speed + ((var-1)*step);
> > > +
> > > +                       if (new_speed < fan_speed)
> > > +                               new_speed = fan_speed;
> > > +                       if (new_speed > 255)
> > > +                               new_speed = 255;
> > > +
> > > +                       if (verbose)
> > > +                               printk(
> > > +                                      KERN_DEBUG "adt746x: Setting fans speed to %d "
> > > +                                                "(limit exceeded by %d on %s)\n",
> > > +                              new_speed, var,
> > > +                              sensor_location[fan_number+1]);
> > > +                       write_both_fan_speed(th, new_speed);
> > > +                       th->last_var[fan_number] = var; 
> > > +                        }
> > > +               } else if (var < -2) {
> > > +         /* don't stop fan if sensor2 is cold and sensor1 is not
> > > +          * so cold (lastvar >= -1) */
> > > +                        if (i == 2 && lastvar < -1) {
> > > +                               if (th->last_speed[fan_number] != 0)
> > > +                                       if (verbose)
> > >                                                 printk(KERN_DEBUG "adt746x: Stopping "
> > >                                                         "fans.\n");
> > > -                               write_both_fan_speed(th, 0);
> > > +                                       write_both_fan_speed(th, 0);
> > >                         }
> > >                 }
> > >  
> > > -               lastvar = var;
> > > +               lastvar = var;
> > >  
> > >                 if (started)
> > > -                       return; /* we don't want to re-stop the fan
> > > -                               * if sensor1 is heating and sensor2 is not */
> > > +               return; /* we don't want to re-stop the fan
> > > +                        * if sensor1 is heating and sensor2 is not */
> > >         }
> > >  }
> > >  
> > > +
> > >  static int monitor_task(void *arg)
> > >  {
> > >         struct thermostat* th = arg;
> > > @@ -371,10 +436,13 @@ static ssize_t store_##name(struct devic
> > >         return n;                                               \
> > >  }
> > >  
> > > -BUILD_SHOW_FUNC_INT(sensor1_temperature,        (read_reg(th, TEMP_REG[1])))
> > > -BUILD_SHOW_FUNC_INT(sensor2_temperature,        (read_reg(th, TEMP_REG[2])))
> > > -BUILD_SHOW_FUNC_INT(sensor1_limit,              th->limits[1])
> > > -BUILD_SHOW_FUNC_INT(sensor2_limit,              th->limits[2])
> > > +BUILD_SHOW_FUNC_INT(sensor0_temperature,         (read_reg(th, TEMP_REG[0]))*1000)
> > > +BUILD_SHOW_FUNC_INT(sensor1_temperature,        (read_reg(th, TEMP_REG[1]))*1000)
> > > +BUILD_SHOW_FUNC_INT(sensor2_temperature,        (read_reg(th, TEMP_REG[2]))*1000)
> > > +BUILD_SHOW_FUNC_INT(sensor0_limit,               (th->limits[0])*1000)
> > > +BUILD_SHOW_FUNC_INT(sensor1_limit,              (th->limits[1])*1000)
> > > +BUILD_SHOW_FUNC_INT(sensor2_limit,              (th->limits[2])*1000)
> > 
> > You changed the units here, that should be documented and why.
> > 
> > > +BUILD_SHOW_FUNC_STR(sensor0_location,            sensor_location[0])
> > >  BUILD_SHOW_FUNC_STR(sensor1_location,           sensor_location[1])
> > >  BUILD_SHOW_FUNC_STR(sensor2_location,           sensor_location[2])
> > >  
> > > @@ -387,14 +455,20 @@ BUILD_SHOW_FUNC_FAN(sensor2_fan_speed,
> > >  BUILD_SHOW_FUNC_INT_LITE(limit_adjust,  limit_adjust)
> > >  BUILD_STORE_FUNC_DEG(limit_adjust,      th)
> > >                 
> > > +static DEVICE_ATTR(sensor0_temperature,        S_IRUGO,
> > > +                  show_sensor0_temperature,NULL);
> > >  static DEVICE_ATTR(sensor1_temperature,        S_IRUGO,
> > >                    show_sensor1_temperature,NULL);
> > >  static DEVICE_ATTR(sensor2_temperature,        S_IRUGO,
> > >                    show_sensor2_temperature,NULL);
> > > +static DEVICE_ATTR(sensor0_limit, S_IRUGO,
> > > +                  show_sensor0_limit,  NULL);
> > >  static DEVICE_ATTR(sensor1_limit, S_IRUGO,
> > >                    show_sensor1_limit,  NULL);
> > >  static DEVICE_ATTR(sensor2_limit, S_IRUGO,
> > >                    show_sensor2_limit,  NULL);
> > > +static DEVICE_ATTR(sensor0_location, S_IRUGO,
> > > +                  show_sensor0_location, NULL);
> > >  static DEVICE_ATTR(sensor1_location, S_IRUGO,
> > >                    show_sensor1_location, NULL);
> > >  static DEVICE_ATTR(sensor2_location, S_IRUGO,
> > > @@ -426,10 +500,13 @@ static void thermostat_create_files(stru
> > >                 return;
> > >         dev = &th->pdev->dev;
> > >         dev_set_drvdata(dev, th);
> > > -       err = device_create_file(dev, &dev_attr_sensor1_temperature);
> > > +       err = device_create_file(dev, &dev_attr_sensor0_temperature);
> > > +       err |= device_create_file(dev, &dev_attr_sensor1_temperature);
> > >         err |= device_create_file(dev, &dev_attr_sensor2_temperature);
> > > +       err |= device_create_file(dev, &dev_attr_sensor0_limit);
> > >         err |= device_create_file(dev, &dev_attr_sensor1_limit);
> > >         err |= device_create_file(dev, &dev_attr_sensor2_limit);
> > > +       err |= device_create_file(dev, &dev_attr_sensor0_location);
> > >         err |= device_create_file(dev, &dev_attr_sensor1_location);
> > >         err |= device_create_file(dev, &dev_attr_sensor2_location);
> > >         err |= device_create_file(dev, &dev_attr_limit_adjust);
> > > @@ -449,10 +526,13 @@ static void thermostat_remove_files(stru
> > >         if (!th->pdev)
> > >                 return;
> > >         dev = &th->pdev->dev;
> > > +       device_remove_file(dev, &dev_attr_sensor0_temperature);
> > >         device_remove_file(dev, &dev_attr_sensor1_temperature);
> > >         device_remove_file(dev, &dev_attr_sensor2_temperature);
> > > +       device_remove_file(dev, &dev_attr_sensor0_limit);
> > >         device_remove_file(dev, &dev_attr_sensor1_limit);
> > >         device_remove_file(dev, &dev_attr_sensor2_limit);
> > > +       device_remove_file(dev, &dev_attr_sensor0_location);
> > >         device_remove_file(dev, &dev_attr_sensor1_location);
> > >         device_remove_file(dev, &dev_attr_sensor2_location);
> > >         device_remove_file(dev, &dev_attr_limit_adjust);
> > 
> > Cheers,
> > Ben.
> > 
> >