[PATCH] hwmon: (tmp421) Add nfactor support.

[PATCH] hwmon: (tmp421) Add nfactor support.

[Jeff Angielski]

Add support for reading and writing the n-factor correction
registers.  This is needed to compensate for the characteristics
of a particular sensor hanging off of the remote channels.

Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
---
  drivers/hwmon/tmp421.c |   42 ++++++++++++++++++++++++++++++++++++++++++
  1 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index 738c472..c9e9855 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };

  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
+static const u8 TMP421_NFACTOR[3]		= { 0x21, 0x22, 0x23 };

  /* Flags */
  #define TMP421_CONFIG_SHUTDOWN			0x40
@@ -157,6 +158,38 @@ static ssize_t show_fault(struct device *dev,
  		return sprintf(buf, "0\n");
  }

+static ssize_t show_nfactor(struct device *dev,
+			  struct device_attribute *devattr, char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct tmp421_data *data = i2c_get_clientdata(client);
+	int index = to_sensor_dev_attr(devattr)->index;
+	s8 nfactor;
+
+	mutex_lock(&data->update_lock);
+	nfactor = i2c_smbus_read_byte_data(client, TMP421_NFACTOR[index-1]);
+	mutex_unlock(&data->update_lock);
+
+	return sprintf(buf, "%d\n", nfactor);
+}
+
+static ssize_t set_nfactor(struct device *dev,
+		struct device_attribute *devattr,
+		const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct tmp421_data *data = i2c_get_clientdata(client);
+	int index = to_sensor_dev_attr(devattr)->index;
+	int nfactor = simple_strtol(buf, NULL, 10);
+
+	mutex_lock(&data->update_lock);
+	i2c_smbus_write_byte_data(client, TMP421_NFACTOR[index-1],
+			SENSORS_LIMIT(nfactor, -128, 127));
+	mutex_unlock(&data->update_lock);
+
+	return count;
+}
+
  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
  				int n)
  {
@@ -177,19 +210,28 @@ static mode_t tmp421_is_visible(struct kobject 
*kobj, struct attribute *a,
  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp2_nfactor, S_IRUGO | S_IWUSR,
+		show_nfactor, set_nfactor, 1);
  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp3_nfactor, S_IRUGO | S_IWUSR,
+		show_nfactor, set_nfactor, 2);
  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
+static SENSOR_DEVICE_ATTR(temp4_nfactor, S_IRUGO | S_IWUSR,
+		show_nfactor, set_nfactor, 3);

  static struct attribute *tmp421_attr[] = {
  	&sensor_dev_attr_temp1_input.dev_attr.attr,
  	&sensor_dev_attr_temp2_input.dev_attr.attr,
  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
+	&sensor_dev_attr_temp2_nfactor.dev_attr.attr,
  	&sensor_dev_attr_temp3_input.dev_attr.attr,
  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
+	&sensor_dev_attr_temp3_nfactor.dev_attr.attr,
  	&sensor_dev_attr_temp4_input.dev_attr.attr,
  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
+	&sensor_dev_attr_temp4_nfactor.dev_attr.attr,
  	NULL
  };

-- 
1.7.0.4



-- 
Jeff Angielski
The PTR Group
www.theptrgroup.com

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support.

[Andre Prendel]

On Mon, May 10, 2010 at 10:43:07AM -0400, Jeff Angielski wrote:

Hi Jeff,

A few comments below.
 
> Add support for reading and writing the n-factor correction
> registers.  This is needed to compensate for the characteristics
> of a particular sensor hanging off of the remote channels.
> 
> Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> ---
>  drivers/hwmon/tmp421.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 42 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 738c472..c9e9855 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
> 
>  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
>  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> +static const u8 TMP421_NFACTOR[3]		= { 0x21, 0x22, 0x23 };
> 
>  /* Flags */
>  #define TMP421_CONFIG_SHUTDOWN			0x40
> @@ -157,6 +158,38 @@ static ssize_t show_fault(struct device *dev,
>  		return sprintf(buf, "0\n");
>  }
> 
> +static ssize_t show_nfactor(struct device *dev,
> +			  struct device_attribute *devattr, char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct tmp421_data *data = i2c_get_clientdata(client);
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	s8 nfactor;
> +
> +	mutex_lock(&data->update_lock);
> +	nfactor = i2c_smbus_read_byte_data(client, TMP421_NFACTOR[index-1]);

There should be spaces within the array index, [index - 1].

> +	mutex_unlock(&data->update_lock);
> +
> +	return sprintf(buf, "%d\n", nfactor);
> +}

I'de prefer implementing the sysfs access methods in a consistent way (see other functions). That means adding the nfactor register to the tmp421_data structure and using tmp421_update_device() to update the structure.

> +static ssize_t set_nfactor(struct device *dev,
> +		struct device_attribute *devattr,
> +		const char *buf, size_t count)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct tmp421_data *data = i2c_get_clientdata(client);
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	int nfactor = simple_strtol(buf, NULL, 10);
> +
> +	mutex_lock(&data->update_lock);
> +	i2c_smbus_write_byte_data(client, TMP421_NFACTOR[index-1],

Missing spaces in array index again.

> +			SENSORS_LIMIT(nfactor, -128, 127));
> +	mutex_unlock(&data->update_lock);
> +
> +	return count;
> +}
> +
>  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
>  				int n)
>  {
> @@ -177,19 +210,28 @@ static mode_t tmp421_is_visible(struct kobject
> *kobj, struct attribute *a,
>  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
>  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
>  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> +static SENSOR_DEVICE_ATTR(temp2_nfactor, S_IRUGO | S_IWUSR,
> +		show_nfactor, set_nfactor, 1);
>  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
>  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> +static SENSOR_DEVICE_ATTR(temp3_nfactor, S_IRUGO | S_IWUSR,
> +		show_nfactor, set_nfactor, 2);
>  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
>  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> +static SENSOR_DEVICE_ATTR(temp4_nfactor, S_IRUGO | S_IWUSR,
> +		show_nfactor, set_nfactor, 3);
> 
>  static struct attribute *tmp421_attr[] = {
>  	&sensor_dev_attr_temp1_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp2_nfactor.dev_attr.attr,
>  	&sensor_dev_attr_temp3_input.dev_attr.attr,
>  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp3_nfactor.dev_attr.attr,
>  	&sensor_dev_attr_temp4_input.dev_attr.attr,
>  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp4_nfactor.dev_attr.attr,
>  	NULL
>  };
> 

Regards,
Andre

PS: CC'ed lm-sensors list


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [PATCH] hwmon: (tmp421) Add nfactor support.

[Andre Prendel]

On Mon, May 10, 2010 at 10:43:07AM -0400, Jeff Angielski wrote:

Hi Jeff,

A few comments below.
 
> Add support for reading and writing the n-factor correction
> registers.  This is needed to compensate for the characteristics
> of a particular sensor hanging off of the remote channels.
> 
> Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> ---
>  drivers/hwmon/tmp421.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 42 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 738c472..c9e9855 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
> 
>  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
>  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> +static const u8 TMP421_NFACTOR[3]		= { 0x21, 0x22, 0x23 };
> 
>  /* Flags */
>  #define TMP421_CONFIG_SHUTDOWN			0x40
> @@ -157,6 +158,38 @@ static ssize_t show_fault(struct device *dev,
>  		return sprintf(buf, "0\n");
>  }
> 
> +static ssize_t show_nfactor(struct device *dev,
> +			  struct device_attribute *devattr, char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct tmp421_data *data = i2c_get_clientdata(client);
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	s8 nfactor;
> +
> +	mutex_lock(&data->update_lock);
> +	nfactor = i2c_smbus_read_byte_data(client, TMP421_NFACTOR[index-1]);

There should be spaces within the array index, [index - 1].

> +	mutex_unlock(&data->update_lock);
> +
> +	return sprintf(buf, "%d\n", nfactor);
> +}

I'de prefer implementing the sysfs access methods in a consistent way (see other functions). That means adding the nfactor register to the tmp421_data structure and using tmp421_update_device() to update the structure.

> +static ssize_t set_nfactor(struct device *dev,
> +		struct device_attribute *devattr,
> +		const char *buf, size_t count)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct tmp421_data *data = i2c_get_clientdata(client);
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	int nfactor = simple_strtol(buf, NULL, 10);
> +
> +	mutex_lock(&data->update_lock);
> +	i2c_smbus_write_byte_data(client, TMP421_NFACTOR[index-1],

Missing spaces in array index again.

> +			SENSORS_LIMIT(nfactor, -128, 127));
> +	mutex_unlock(&data->update_lock);
> +
> +	return count;
> +}
> +
>  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
>  				int n)
>  {
> @@ -177,19 +210,28 @@ static mode_t tmp421_is_visible(struct kobject
> *kobj, struct attribute *a,
>  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
>  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
>  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> +static SENSOR_DEVICE_ATTR(temp2_nfactor, S_IRUGO | S_IWUSR,
> +		show_nfactor, set_nfactor, 1);
>  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
>  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> +static SENSOR_DEVICE_ATTR(temp3_nfactor, S_IRUGO | S_IWUSR,
> +		show_nfactor, set_nfactor, 2);
>  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
>  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> +static SENSOR_DEVICE_ATTR(temp4_nfactor, S_IRUGO | S_IWUSR,
> +		show_nfactor, set_nfactor, 3);
> 
>  static struct attribute *tmp421_attr[] = {
>  	&sensor_dev_attr_temp1_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp2_nfactor.dev_attr.attr,
>  	&sensor_dev_attr_temp3_input.dev_attr.attr,
>  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp3_nfactor.dev_attr.attr,
>  	&sensor_dev_attr_temp4_input.dev_attr.attr,
>  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp4_nfactor.dev_attr.attr,
>  	NULL
>  };
> 

Regards,
Andre

PS: CC'ed lm-sensors list

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support.

[Jean Delvare]

On Tue, 11 May 2010 21:03:27 +0200, Andre Prendel wrote:
> On Mon, May 10, 2010 at 10:43:07AM -0400, Jeff Angielski wrote:
> 
> Hi Jeff,
> 
> A few comments below.
>  
> > Add support for reading and writing the n-factor correction
> > registers.  This is needed to compensate for the characteristics
> > of a particular sensor hanging off of the remote channels.
> > 
> > Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> > ---
> >  drivers/hwmon/tmp421.c |   42 ++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 42 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> > index 738c472..c9e9855 100644
> > --- a/drivers/hwmon/tmp421.c
> > +++ b/drivers/hwmon/tmp421.c
> > @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
> > 
> >  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
> >  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> > +static const u8 TMP421_NFACTOR[3]		= { 0x21, 0x22, 0x23 };
> > 
> >  /* Flags */
> >  #define TMP421_CONFIG_SHUTDOWN			0x40
> > @@ -157,6 +158,38 @@ static ssize_t show_fault(struct device *dev,
> >  		return sprintf(buf, "0\n");
> >  }
> > 
> > +static ssize_t show_nfactor(struct device *dev,
> > +			  struct device_attribute *devattr, char *buf)
> > +{
> > +	struct i2c_client *client = to_i2c_client(dev);
> > +	struct tmp421_data *data = i2c_get_clientdata(client);
> > +	int index = to_sensor_dev_attr(devattr)->index;
> > +	s8 nfactor;
> > +
> > +	mutex_lock(&data->update_lock);
> > +	nfactor = i2c_smbus_read_byte_data(client, TMP421_NFACTOR[index-1]);
> 
> There should be spaces within the array index, [index - 1].
> 
> > +	mutex_unlock(&data->update_lock);
> > +
> > +	return sprintf(buf, "%d\n", nfactor);
> > +}
> 
> I'de prefer implementing the sysfs access methods in a consistent way (see other functions). That means adding the nfactor register to the tmp421_data structure and using tmp421_update_device() to update the structure.
> 
> > +static ssize_t set_nfactor(struct device *dev,
> > +		struct device_attribute *devattr,
> > +		const char *buf, size_t count)
> > +{
> > +	struct i2c_client *client = to_i2c_client(dev);
> > +	struct tmp421_data *data = i2c_get_clientdata(client);
> > +	int index = to_sensor_dev_attr(devattr)->index;
> > +	int nfactor = simple_strtol(buf, NULL, 10);
> > +
> > +	mutex_lock(&data->update_lock);
> > +	i2c_smbus_write_byte_data(client, TMP421_NFACTOR[index-1],
> 
> Missing spaces in array index again.
> 
> > +			SENSORS_LIMIT(nfactor, -128, 127));
> > +	mutex_unlock(&data->update_lock);
> > +
> > +	return count;
> > +}
> > +
> >  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
> >  				int n)
> >  {
> > @@ -177,19 +210,28 @@ static mode_t tmp421_is_visible(struct kobject
> > *kobj, struct attribute *a,
> >  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
> >  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
> >  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> > +static SENSOR_DEVICE_ATTR(temp2_nfactor, S_IRUGO | S_IWUSR,
> > +		show_nfactor, set_nfactor, 1);
> >  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
> >  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> > +static SENSOR_DEVICE_ATTR(temp3_nfactor, S_IRUGO | S_IWUSR,
> > +		show_nfactor, set_nfactor, 2);
> >  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
> >  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> > +static SENSOR_DEVICE_ATTR(temp4_nfactor, S_IRUGO | S_IWUSR,
> > +		show_nfactor, set_nfactor, 3);
> > 
> >  static struct attribute *tmp421_attr[] = {
> >  	&sensor_dev_attr_temp1_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp2_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> > +	&sensor_dev_attr_temp2_nfactor.dev_attr.attr,
> >  	&sensor_dev_attr_temp3_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> > +	&sensor_dev_attr_temp3_nfactor.dev_attr.attr,
> >  	&sensor_dev_attr_temp4_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> > +	&sensor_dev_attr_temp4_nfactor.dev_attr.attr,
> >  	NULL
> >  };

Any hope to standardize on the sysfs attribute files and units? So that
other drivers can implement the same interface.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [PATCH] hwmon: (tmp421) Add nfactor support.

[Jean Delvare]

On Tue, 11 May 2010 21:03:27 +0200, Andre Prendel wrote:
> On Mon, May 10, 2010 at 10:43:07AM -0400, Jeff Angielski wrote:
> 
> Hi Jeff,
> 
> A few comments below.
>  
> > Add support for reading and writing the n-factor correction
> > registers.  This is needed to compensate for the characteristics
> > of a particular sensor hanging off of the remote channels.
> > 
> > Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> > ---
> >  drivers/hwmon/tmp421.c |   42 ++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 42 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> > index 738c472..c9e9855 100644
> > --- a/drivers/hwmon/tmp421.c
> > +++ b/drivers/hwmon/tmp421.c
> > @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
> > 
> >  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
> >  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> > +static const u8 TMP421_NFACTOR[3]		= { 0x21, 0x22, 0x23 };
> > 
> >  /* Flags */
> >  #define TMP421_CONFIG_SHUTDOWN			0x40
> > @@ -157,6 +158,38 @@ static ssize_t show_fault(struct device *dev,
> >  		return sprintf(buf, "0\n");
> >  }
> > 
> > +static ssize_t show_nfactor(struct device *dev,
> > +			  struct device_attribute *devattr, char *buf)
> > +{
> > +	struct i2c_client *client = to_i2c_client(dev);
> > +	struct tmp421_data *data = i2c_get_clientdata(client);
> > +	int index = to_sensor_dev_attr(devattr)->index;
> > +	s8 nfactor;
> > +
> > +	mutex_lock(&data->update_lock);
> > +	nfactor = i2c_smbus_read_byte_data(client, TMP421_NFACTOR[index-1]);
> 
> There should be spaces within the array index, [index - 1].
> 
> > +	mutex_unlock(&data->update_lock);
> > +
> > +	return sprintf(buf, "%d\n", nfactor);
> > +}
> 
> I'de prefer implementing the sysfs access methods in a consistent way (see other functions). That means adding the nfactor register to the tmp421_data structure and using tmp421_update_device() to update the structure.
> 
> > +static ssize_t set_nfactor(struct device *dev,
> > +		struct device_attribute *devattr,
> > +		const char *buf, size_t count)
> > +{
> > +	struct i2c_client *client = to_i2c_client(dev);
> > +	struct tmp421_data *data = i2c_get_clientdata(client);
> > +	int index = to_sensor_dev_attr(devattr)->index;
> > +	int nfactor = simple_strtol(buf, NULL, 10);
> > +
> > +	mutex_lock(&data->update_lock);
> > +	i2c_smbus_write_byte_data(client, TMP421_NFACTOR[index-1],
> 
> Missing spaces in array index again.
> 
> > +			SENSORS_LIMIT(nfactor, -128, 127));
> > +	mutex_unlock(&data->update_lock);
> > +
> > +	return count;
> > +}
> > +
> >  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
> >  				int n)
> >  {
> > @@ -177,19 +210,28 @@ static mode_t tmp421_is_visible(struct kobject
> > *kobj, struct attribute *a,
> >  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
> >  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
> >  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> > +static SENSOR_DEVICE_ATTR(temp2_nfactor, S_IRUGO | S_IWUSR,
> > +		show_nfactor, set_nfactor, 1);
> >  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
> >  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> > +static SENSOR_DEVICE_ATTR(temp3_nfactor, S_IRUGO | S_IWUSR,
> > +		show_nfactor, set_nfactor, 2);
> >  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
> >  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> > +static SENSOR_DEVICE_ATTR(temp4_nfactor, S_IRUGO | S_IWUSR,
> > +		show_nfactor, set_nfactor, 3);
> > 
> >  static struct attribute *tmp421_attr[] = {
> >  	&sensor_dev_attr_temp1_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp2_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> > +	&sensor_dev_attr_temp2_nfactor.dev_attr.attr,
> >  	&sensor_dev_attr_temp3_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> > +	&sensor_dev_attr_temp3_nfactor.dev_attr.attr,
> >  	&sensor_dev_attr_temp4_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> > +	&sensor_dev_attr_temp4_nfactor.dev_attr.attr,
> >  	NULL
> >  };

Any hope to standardize on the sysfs attribute files and units? So that
other drivers can implement the same interface.

-- 
Jean Delvare

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support.

[Jeff Angielski]

On 05/11/2010 03:03 PM, Andre Prendel wrote:
> On Mon, May 10, 2010 at 10:43:07AM -0400, Jeff Angielski wrote:
>
> Hi Jeff,
>
> A few comments below.
>
>> Add support for reading and writing the n-factor correction
>> registers.  This is needed to compensate for the characteristics
>> of a particular sensor hanging off of the remote channels.
>>
>> Signed-off-by: Jeff Angielski<jeff@theptrgroup.com>
>> ---
>>   drivers/hwmon/tmp421.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>>   1 files changed, 42 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
>> index 738c472..c9e9855 100644
>> --- a/drivers/hwmon/tmp421.c
>> +++ b/drivers/hwmon/tmp421.c
>> @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
>>
>>   static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
>>   static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
>> +static const u8 TMP421_NFACTOR[3]		= { 0x21, 0x22, 0x23 };
>>
>>   /* Flags */
>>   #define TMP421_CONFIG_SHUTDOWN			0x40
>> @@ -157,6 +158,38 @@ static ssize_t show_fault(struct device *dev,
>>   		return sprintf(buf, "0\n");
>>   }
>>
>> +static ssize_t show_nfactor(struct device *dev,
>> +			  struct device_attribute *devattr, char *buf)
>> +{
>> +	struct i2c_client *client = to_i2c_client(dev);
>> +	struct tmp421_data *data = i2c_get_clientdata(client);
>> +	int index = to_sensor_dev_attr(devattr)->index;
>> +	s8 nfactor;
>> +
>> +	mutex_lock(&data->update_lock);
>> +	nfactor = i2c_smbus_read_byte_data(client, TMP421_NFACTOR[index-1]);
>
> There should be spaces within the array index, [index - 1].

Ok.

>
>> +	mutex_unlock(&data->update_lock);
>> +
>> +	return sprintf(buf, "%d\n", nfactor);
>> +}
>
> I'de prefer implementing the sysfs access methods in a consistent way (see other functions). That means adding the nfactor register to the tmp421_data structure and using tmp421_update_device() to update the structure.

I did this on purpose since the nfactor typically only changes once at 
runtime when you program it for your sensor.  It seemed like a waste of 
processing power and i2c bandwidth to read a "pseudo static" register 
over and over again.

It can easily be changed if that's what will help the community the best.

>> +static ssize_t set_nfactor(struct device *dev,
>> +		struct device_attribute *devattr,
>> +		const char *buf, size_t count)
>> +{
>> +	struct i2c_client *client = to_i2c_client(dev);
>> +	struct tmp421_data *data = i2c_get_clientdata(client);
>> +	int index = to_sensor_dev_attr(devattr)->index;
>> +	int nfactor = simple_strtol(buf, NULL, 10);
>> +
>> +	mutex_lock(&data->update_lock);
>> +	i2c_smbus_write_byte_data(client, TMP421_NFACTOR[index-1],
>
> Missing spaces in array index again.

Ok.




-- 
Jeff Angielski
The PTR Group
www.theptrgroup.com

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [PATCH] hwmon: (tmp421) Add nfactor support.

[Jeff Angielski]

On 05/11/2010 03:03 PM, Andre Prendel wrote:
> On Mon, May 10, 2010 at 10:43:07AM -0400, Jeff Angielski wrote:
>
> Hi Jeff,
>
> A few comments below.
>
>> Add support for reading and writing the n-factor correction
>> registers.  This is needed to compensate for the characteristics
>> of a particular sensor hanging off of the remote channels.
>>
>> Signed-off-by: Jeff Angielski<jeff@theptrgroup.com>
>> ---
>>   drivers/hwmon/tmp421.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>>   1 files changed, 42 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
>> index 738c472..c9e9855 100644
>> --- a/drivers/hwmon/tmp421.c
>> +++ b/drivers/hwmon/tmp421.c
>> @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
>>
>>   static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
>>   static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
>> +static const u8 TMP421_NFACTOR[3]		= { 0x21, 0x22, 0x23 };
>>
>>   /* Flags */
>>   #define TMP421_CONFIG_SHUTDOWN			0x40
>> @@ -157,6 +158,38 @@ static ssize_t show_fault(struct device *dev,
>>   		return sprintf(buf, "0\n");
>>   }
>>
>> +static ssize_t show_nfactor(struct device *dev,
>> +			  struct device_attribute *devattr, char *buf)
>> +{
>> +	struct i2c_client *client = to_i2c_client(dev);
>> +	struct tmp421_data *data = i2c_get_clientdata(client);
>> +	int index = to_sensor_dev_attr(devattr)->index;
>> +	s8 nfactor;
>> +
>> +	mutex_lock(&data->update_lock);
>> +	nfactor = i2c_smbus_read_byte_data(client, TMP421_NFACTOR[index-1]);
>
> There should be spaces within the array index, [index - 1].

Ok.

>
>> +	mutex_unlock(&data->update_lock);
>> +
>> +	return sprintf(buf, "%d\n", nfactor);
>> +}
>
> I'de prefer implementing the sysfs access methods in a consistent way (see other functions). That means adding the nfactor register to the tmp421_data structure and using tmp421_update_device() to update the structure.

I did this on purpose since the nfactor typically only changes once at 
runtime when you program it for your sensor.  It seemed like a waste of 
processing power and i2c bandwidth to read a "pseudo static" register 
over and over again.

It can easily be changed if that's what will help the community the best.

>> +static ssize_t set_nfactor(struct device *dev,
>> +		struct device_attribute *devattr,
>> +		const char *buf, size_t count)
>> +{
>> +	struct i2c_client *client = to_i2c_client(dev);
>> +	struct tmp421_data *data = i2c_get_clientdata(client);
>> +	int index = to_sensor_dev_attr(devattr)->index;
>> +	int nfactor = simple_strtol(buf, NULL, 10);
>> +
>> +	mutex_lock(&data->update_lock);
>> +	i2c_smbus_write_byte_data(client, TMP421_NFACTOR[index-1],
>
> Missing spaces in array index again.

Ok.




-- 
Jeff Angielski
The PTR Group
www.theptrgroup.com

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support.

[Jean Delvare]

Hi Jeff,

On Tue, 11 May 2010 15:34:29 -0400, Jeff Angielski wrote:
> On 05/11/2010 03:03 PM, Andre Prendel wrote:
> > I'de prefer implementing the sysfs access methods in a consistent way (see other functions). That means adding the nfactor register to the tmp421_data structure and using tmp421_update_device() to update the structure.
> 
> I did this on purpose since the nfactor typically only changes once at 
> runtime when you program it for your sensor.  It seemed like a waste of 
> processing power and i2c bandwidth to read a "pseudo static" register 
> over and over again.
> 
> It can easily be changed if that's what will help the community the best.

I get your point and it makes sense. However, we also have to ensure
that no regular user can saturate the I2C link by repeatedly polling
for the same register value. So, any sysfs attribute which triggers an
immediate I2C transaction shouldn't be accessible by regular users
(i.e. change the mode from 0644 to 0640).

An alternative is to slit the cache into short-lived (~ 1 or 2 seconds)
and long-lived (1 to 5 minutes.) A number of drivers do this.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [PATCH] hwmon: (tmp421) Add nfactor support.

[Jean Delvare]

Hi Jeff,

On Tue, 11 May 2010 15:34:29 -0400, Jeff Angielski wrote:
> On 05/11/2010 03:03 PM, Andre Prendel wrote:
> > I'de prefer implementing the sysfs access methods in a consistent way (see other functions). That means adding the nfactor register to the tmp421_data structure and using tmp421_update_device() to update the structure.
> 
> I did this on purpose since the nfactor typically only changes once at 
> runtime when you program it for your sensor.  It seemed like a waste of 
> processing power and i2c bandwidth to read a "pseudo static" register 
> over and over again.
> 
> It can easily be changed if that's what will help the community the best.

I get your point and it makes sense. However, we also have to ensure
that no regular user can saturate the I2C link by repeatedly polling
for the same register value. So, any sysfs attribute which triggers an
immediate I2C transaction shouldn't be accessible by regular users
(i.e. change the mode from 0644 to 0640).

An alternative is to slit the cache into short-lived (~ 1 or 2 seconds)
and long-lived (1 to 5 minutes.) A number of drivers do this.

-- 
Jean Delvare

[lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd

[Jeff Angielski]

Here is a second attempt at a patch to add nfactor support to the tmp421 driver.

This includes the changes as suggested by Andre Prendel, the original driver author.


From 8ebe84174ff6bd294656d77183758044f19d8900 Mon Sep 17 00:00:00 2001
From: Jeff Angielski <jeff@theptrgroup.com>
Date: Mon, 10 May 2010 10:26:34 -0400
Subject: [PATCH] hwmon: (tmp421) Add nfactor support

Add support for reading and writing the n-factor correction
registers.  This is needed to compensate for the characteristics
of a particular sensor hanging off of the remote channels.

Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
---
 drivers/hwmon/tmp421.c |   41 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index 738c472..ce1f6d1 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
 
 static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
 static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
+static const u8 TMP421_NFACTOR[3]		= { 0x21, 0x22, 0x23 };
 
 /* Flags */
 #define TMP421_CONFIG_SHUTDOWN			0x40
@@ -76,6 +77,7 @@ struct tmp421_data {
 	int channels;
 	u8 config;
 	s16 temp[4];
+	s8 nfactor[3];
 };
 
 static int temp_from_s16(s16 reg)
@@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
 			data->temp[i] |= i2c_smbus_read_byte_data(client,
 				TMP421_TEMP_LSB[i]);
 		}
+		for (i = 1; i < data->channels; i++) {
+			data->nfactor[i - 1] = i2c_smbus_read_byte_data(client,
+				TMP421_NFACTOR[i - 1]);
+		}
 		data->last_updated = jiffies;
 		data->valid = 1;
 	}
@@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
 		return sprintf(buf, "0\n");
 }
 
+static ssize_t show_nfactor(struct device *dev,
+			  struct device_attribute *devattr, char *buf)
+{
+	int index = to_sensor_dev_attr(devattr)->index;
+	struct tmp421_data *data = tmp421_update_device(dev);
+
+	return sprintf(buf, "%d\n", data->nfactor[index - 1]);
+}
+
+static ssize_t set_nfactor(struct device *dev,
+		struct device_attribute *devattr,
+		const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct tmp421_data *data = i2c_get_clientdata(client);
+	int index = to_sensor_dev_attr(devattr)->index;
+	int nfactor = simple_strtol(buf, NULL, 10);
+
+	mutex_lock(&data->update_lock);
+	i2c_smbus_write_byte_data(client, TMP421_NFACTOR[index - 1],
+			SENSORS_LIMIT(nfactor, -128, 127));
+	mutex_unlock(&data->update_lock);
+
+	return count;
+}
+
 static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
 				int n)
 {
@@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
 static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
 static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
 static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp2_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
+		show_nfactor, set_nfactor, 1);
 static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
 static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp3_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
+		show_nfactor, set_nfactor, 2);
 static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
 static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
+static SENSOR_DEVICE_ATTR(temp4_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
+		show_nfactor, set_nfactor, 3);
 
 static struct attribute *tmp421_attr[] = {
 	&sensor_dev_attr_temp1_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_fault.dev_attr.attr,
+	&sensor_dev_attr_temp2_nfactor.dev_attr.attr,
 	&sensor_dev_attr_temp3_input.dev_attr.attr,
 	&sensor_dev_attr_temp3_fault.dev_attr.attr,
+	&sensor_dev_attr_temp3_nfactor.dev_attr.attr,
 	&sensor_dev_attr_temp4_input.dev_attr.attr,
 	&sensor_dev_attr_temp4_fault.dev_attr.attr,
+	&sensor_dev_attr_temp4_nfactor.dev_attr.attr,
 	NULL
 };
 
-- 
Jeff Angielski
The PTR Group
www.theptrgroup.com

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

[PATCH] hwmon: (tmp421) Add nfactor support (2nd attempt)

[Jeff Angielski]

Here is a second attempt at a patch to add nfactor support to the tmp421 driver.

This includes the changes as suggested by Andre Prendel, the original driver author.


>From 8ebe84174ff6bd294656d77183758044f19d8900 Mon Sep 17 00:00:00 2001
From: Jeff Angielski <jeff@theptrgroup.com>
Date: Mon, 10 May 2010 10:26:34 -0400
Subject: [PATCH] hwmon: (tmp421) Add nfactor support

Add support for reading and writing the n-factor correction
registers.  This is needed to compensate for the characteristics
of a particular sensor hanging off of the remote channels.

Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
---
 drivers/hwmon/tmp421.c |   41 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index 738c472..ce1f6d1 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
 
 static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
 static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
+static const u8 TMP421_NFACTOR[3]		= { 0x21, 0x22, 0x23 };
 
 /* Flags */
 #define TMP421_CONFIG_SHUTDOWN			0x40
@@ -76,6 +77,7 @@ struct tmp421_data {
 	int channels;
 	u8 config;
 	s16 temp[4];
+	s8 nfactor[3];
 };
 
 static int temp_from_s16(s16 reg)
@@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
 			data->temp[i] |= i2c_smbus_read_byte_data(client,
 				TMP421_TEMP_LSB[i]);
 		}
+		for (i = 1; i < data->channels; i++) {
+			data->nfactor[i - 1] = i2c_smbus_read_byte_data(client,
+				TMP421_NFACTOR[i - 1]);
+		}
 		data->last_updated = jiffies;
 		data->valid = 1;
 	}
@@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
 		return sprintf(buf, "0\n");
 }
 
+static ssize_t show_nfactor(struct device *dev,
+			  struct device_attribute *devattr, char *buf)
+{
+	int index = to_sensor_dev_attr(devattr)->index;
+	struct tmp421_data *data = tmp421_update_device(dev);
+
+	return sprintf(buf, "%d\n", data->nfactor[index - 1]);
+}
+
+static ssize_t set_nfactor(struct device *dev,
+		struct device_attribute *devattr,
+		const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct tmp421_data *data = i2c_get_clientdata(client);
+	int index = to_sensor_dev_attr(devattr)->index;
+	int nfactor = simple_strtol(buf, NULL, 10);
+
+	mutex_lock(&data->update_lock);
+	i2c_smbus_write_byte_data(client, TMP421_NFACTOR[index - 1],
+			SENSORS_LIMIT(nfactor, -128, 127));
+	mutex_unlock(&data->update_lock);
+
+	return count;
+}
+
 static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
 				int n)
 {
@@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
 static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
 static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
 static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp2_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
+		show_nfactor, set_nfactor, 1);
 static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
 static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp3_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
+		show_nfactor, set_nfactor, 2);
 static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
 static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
+static SENSOR_DEVICE_ATTR(temp4_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
+		show_nfactor, set_nfactor, 3);
 
 static struct attribute *tmp421_attr[] = {
 	&sensor_dev_attr_temp1_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_fault.dev_attr.attr,
+	&sensor_dev_attr_temp2_nfactor.dev_attr.attr,
 	&sensor_dev_attr_temp3_input.dev_attr.attr,
 	&sensor_dev_attr_temp3_fault.dev_attr.attr,
+	&sensor_dev_attr_temp3_nfactor.dev_attr.attr,
 	&sensor_dev_attr_temp4_input.dev_attr.attr,
 	&sensor_dev_attr_temp4_fault.dev_attr.attr,
+	&sensor_dev_attr_temp4_nfactor.dev_attr.attr,
 	NULL
 };
 
-- 
Jeff Angielski
The PTR Group
www.theptrgroup.com

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd

[Andre Prendel]

On Mon, May 17, 2010 at 04:30:24PM -0400, Jeff Angielski wrote:
> 
> Here is a second attempt at a patch to add nfactor support to the tmp421 driver.

Hi Jeff,

only a few minor issues, see below.
 
> This includes the changes as suggested by Andre Prendel, the original driver author.
> 
> 
> >From 8ebe84174ff6bd294656d77183758044f19d8900 Mon Sep 17 00:00:00 2001
> From: Jeff Angielski <jeff@theptrgroup.com>
> Date: Mon, 10 May 2010 10:26:34 -0400
> Subject: [PATCH] hwmon: (tmp421) Add nfactor support
> 
> Add support for reading and writing the n-factor correction
> registers.  This is needed to compensate for the characteristics
> of a particular sensor hanging off of the remote channels.
> 
> Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> ---
>  drivers/hwmon/tmp421.c |   41 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 41 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 738c472..ce1f6d1 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
>  
>  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
>  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> +static const u8 TMP421_NFACTOR[3]		= { 0x21, 0x22, 0x23 };
>  
>  /* Flags */
>  #define TMP421_CONFIG_SHUTDOWN			0x40
> @@ -76,6 +77,7 @@ struct tmp421_data {
>  	int channels;
>  	u8 config;
>  	s16 temp[4];
> +	s8 nfactor[3];
>  };
>  
>  static int temp_from_s16(s16 reg)
> @@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
>  			data->temp[i] |= i2c_smbus_read_byte_data(client,
>  				TMP421_TEMP_LSB[i]);
>  		}
> +		for (i = 1; i < data->channels; i++) {
> +			data->nfactor[i - 1] = i2c_smbus_read_byte_data(client,
> +				TMP421_NFACTOR[i - 1]);
> +		}

I'd prefer starting i with 0 and as condition i < data->channels - 1.

for (i = 0; i < data->channels -1; i++) {
	data->nfactor[i] = i2c_smbus_read_byte_data(client,
		TMP421_NFACTOR[i]);
}

What do you think?

>  		data->last_updated = jiffies;
>  		data->valid = 1;
>  	}
> @@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
>  		return sprintf(buf, "0\n");
>  }
>  
> +static ssize_t show_nfactor(struct device *dev,
> +			  struct device_attribute *devattr, char *buf)

Indentation of the arguments is wrong (see other functions).

> +{
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	struct tmp421_data *data = tmp421_update_device(dev);
> +
> +	return sprintf(buf, "%d\n", data->nfactor[index - 1]);
> +}
> +
> +static ssize_t set_nfactor(struct device *dev,
> +		struct device_attribute *devattr,
> +		const char *buf, size_t count)

ditto

> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct tmp421_data *data = i2c_get_clientdata(client);
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	int nfactor = simple_strtol(buf, NULL, 10);
> +
> +	mutex_lock(&data->update_lock);
> +	i2c_smbus_write_byte_data(client, TMP421_NFACTOR[index - 1],
> +			SENSORS_LIMIT(nfactor, -128, 127));

ditto

> +	mutex_unlock(&data->update_lock);
> +
> +	return count;
> +}
> +
>  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
>  				int n)
>  {
> @@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
>  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
>  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
>  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> +static SENSOR_DEVICE_ATTR(temp2_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
> +		show_nfactor, set_nfactor, 1);
>  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
>  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> +static SENSOR_DEVICE_ATTR(temp3_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
> +		show_nfactor, set_nfactor, 2);
>  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
>  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> +static SENSOR_DEVICE_ATTR(temp4_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
> +		show_nfactor, set_nfactor, 3);

And again three times.

>  
>  static struct attribute *tmp421_attr[] = {
>  	&sensor_dev_attr_temp1_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp2_nfactor.dev_attr.attr,
>  	&sensor_dev_attr_temp3_input.dev_attr.attr,
>  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp3_nfactor.dev_attr.attr,
>  	&sensor_dev_attr_temp4_input.dev_attr.attr,
>  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp4_nfactor.dev_attr.attr,
>  	NULL
>  };

All the rest looks good.

Thanks,
Andre

> -- 
> Jeff Angielski
> The PTR Group
> www.theptrgroup.com
> 
> _______________________________________________
> lm-sensors mailing list
> lm-sensors@lm-sensors.org
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd attempt)

[Andre Prendel]

On Mon, May 17, 2010 at 04:30:24PM -0400, Jeff Angielski wrote:
> 
> Here is a second attempt at a patch to add nfactor support to the tmp421 driver.

Hi Jeff,

only a few minor issues, see below.
 
> This includes the changes as suggested by Andre Prendel, the original driver author.
> 
> 
> >From 8ebe84174ff6bd294656d77183758044f19d8900 Mon Sep 17 00:00:00 2001
> From: Jeff Angielski <jeff@theptrgroup.com>
> Date: Mon, 10 May 2010 10:26:34 -0400
> Subject: [PATCH] hwmon: (tmp421) Add nfactor support
> 
> Add support for reading and writing the n-factor correction
> registers.  This is needed to compensate for the characteristics
> of a particular sensor hanging off of the remote channels.
> 
> Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> ---
>  drivers/hwmon/tmp421.c |   41 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 41 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 738c472..ce1f6d1 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
>  
>  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
>  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> +static const u8 TMP421_NFACTOR[3]		= { 0x21, 0x22, 0x23 };
>  
>  /* Flags */
>  #define TMP421_CONFIG_SHUTDOWN			0x40
> @@ -76,6 +77,7 @@ struct tmp421_data {
>  	int channels;
>  	u8 config;
>  	s16 temp[4];
> +	s8 nfactor[3];
>  };
>  
>  static int temp_from_s16(s16 reg)
> @@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
>  			data->temp[i] |= i2c_smbus_read_byte_data(client,
>  				TMP421_TEMP_LSB[i]);
>  		}
> +		for (i = 1; i < data->channels; i++) {
> +			data->nfactor[i - 1] = i2c_smbus_read_byte_data(client,
> +				TMP421_NFACTOR[i - 1]);
> +		}

I'd prefer starting i with 0 and as condition i < data->channels - 1.

for (i = 0; i < data->channels -1; i++) {
	data->nfactor[i] = i2c_smbus_read_byte_data(client,
		TMP421_NFACTOR[i]);
}

What do you think?

>  		data->last_updated = jiffies;
>  		data->valid = 1;
>  	}
> @@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
>  		return sprintf(buf, "0\n");
>  }
>  
> +static ssize_t show_nfactor(struct device *dev,
> +			  struct device_attribute *devattr, char *buf)

Indentation of the arguments is wrong (see other functions).

> +{
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	struct tmp421_data *data = tmp421_update_device(dev);
> +
> +	return sprintf(buf, "%d\n", data->nfactor[index - 1]);
> +}
> +
> +static ssize_t set_nfactor(struct device *dev,
> +		struct device_attribute *devattr,
> +		const char *buf, size_t count)

ditto

> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct tmp421_data *data = i2c_get_clientdata(client);
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	int nfactor = simple_strtol(buf, NULL, 10);
> +
> +	mutex_lock(&data->update_lock);
> +	i2c_smbus_write_byte_data(client, TMP421_NFACTOR[index - 1],
> +			SENSORS_LIMIT(nfactor, -128, 127));

ditto

> +	mutex_unlock(&data->update_lock);
> +
> +	return count;
> +}
> +
>  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
>  				int n)
>  {
> @@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
>  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
>  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
>  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> +static SENSOR_DEVICE_ATTR(temp2_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
> +		show_nfactor, set_nfactor, 1);
>  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
>  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> +static SENSOR_DEVICE_ATTR(temp3_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
> +		show_nfactor, set_nfactor, 2);
>  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
>  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> +static SENSOR_DEVICE_ATTR(temp4_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
> +		show_nfactor, set_nfactor, 3);

And again three times.

>  
>  static struct attribute *tmp421_attr[] = {
>  	&sensor_dev_attr_temp1_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp2_nfactor.dev_attr.attr,
>  	&sensor_dev_attr_temp3_input.dev_attr.attr,
>  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp3_nfactor.dev_attr.attr,
>  	&sensor_dev_attr_temp4_input.dev_attr.attr,
>  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp4_nfactor.dev_attr.attr,
>  	NULL
>  };

All the rest looks good.

Thanks,
Andre

> -- 
> Jeff Angielski
> The PTR Group
> www.theptrgroup.com
> 
> _______________________________________________
> lm-sensors mailing list
> lm-sensors@lm-sensors.org
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd

[Jeff Angielski]

On 05/18/2010 07:38 AM, Andre Prendel wrote:

> I'd prefer starting i with 0 and as condition i < data->channels - 1.
> 
> for (i = 0; i < data->channels -1; i++) {
> 	data->nfactor[i] = i2c_smbus_read_byte_data(client,
> 		TMP421_NFACTOR[i]);
> }
> 
> What do you think?

The first channel is the local channel which does not support nfactor,
so we skip it.  Having the for loop start at 1 illustrates this concept.

> Indentation of the arguments is wrong (see other functions).

Should be fixed in the patch below.  

I am not sure why the patch file itself still shows the function arguments 
off by one.  When I apply it to a raw tmp421.c all of the arguments 
line up correctly.

After rebasing the changes, I am creating the patch as:

$ git format-patch -1 HEAD

Pretty simple stuff...



From 467c74e1d8118e34e84a150f18b5e55c6593c777 Mon Sep 17 00:00:00 2001
From: Jeff Angielski <jeff@theptrgroup.com>
Date: Mon, 10 May 2010 10:26:34 -0400
Subject: [PATCH] hwmon: (tmp421) Add nfactor support

Add support for reading and writing the n-factor correction
registers.  This is needed to compensate for the characteristics
of a particular sensor hanging off of the remote channels.

Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
---
 drivers/hwmon/tmp421.c |   41 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index 738c472..7944627 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
 
 static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
 static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
+static const u8 TMP421_NFACTOR[3]		= { 0x21, 0x22, 0x23 };
 
 /* Flags */
 #define TMP421_CONFIG_SHUTDOWN			0x40
@@ -76,6 +77,7 @@ struct tmp421_data {
 	int channels;
 	u8 config;
 	s16 temp[4];
+	s8 nfactor[3];
 };
 
 static int temp_from_s16(s16 reg)
@@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
 			data->temp[i] |= i2c_smbus_read_byte_data(client,
 				TMP421_TEMP_LSB[i]);
 		}
+		for (i = 1; i < data->channels; i++) {
+			data->nfactor[i - 1] = i2c_smbus_read_byte_data(client,
+				TMP421_NFACTOR[i - 1]);
+		}
 		data->last_updated = jiffies;
 		data->valid = 1;
 	}
@@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
 		return sprintf(buf, "0\n");
 }
 
+static ssize_t show_nfactor(struct device *dev,
+			    struct device_attribute *devattr, char *buf)
+{
+	int index = to_sensor_dev_attr(devattr)->index;
+	struct tmp421_data *data = tmp421_update_device(dev);
+
+	return sprintf(buf, "%d\n", data->nfactor[index - 1]);
+}
+
+static ssize_t set_nfactor(struct device *dev,
+			   struct device_attribute *devattr,
+			   const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct tmp421_data *data = i2c_get_clientdata(client);
+	int index = to_sensor_dev_attr(devattr)->index;
+	int nfactor = simple_strtol(buf, NULL, 10);
+
+	mutex_lock(&data->update_lock);
+	i2c_smbus_write_byte_data(client, TMP421_NFACTOR[index - 1],
+				  SENSORS_LIMIT(nfactor, -128, 127));
+	mutex_unlock(&data->update_lock);
+
+	return count;
+}
+
 static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
 				int n)
 {
@@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
 static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
 static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
 static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp2_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
+			  show_nfactor, set_nfactor, 1);
 static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
 static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp3_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
+			  show_nfactor, set_nfactor, 2);
 static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
 static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
+static SENSOR_DEVICE_ATTR(temp4_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
+			  show_nfactor, set_nfactor, 3);
 
 static struct attribute *tmp421_attr[] = {
 	&sensor_dev_attr_temp1_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_fault.dev_attr.attr,
+	&sensor_dev_attr_temp2_nfactor.dev_attr.attr,
 	&sensor_dev_attr_temp3_input.dev_attr.attr,
 	&sensor_dev_attr_temp3_fault.dev_attr.attr,
+	&sensor_dev_attr_temp3_nfactor.dev_attr.attr,
 	&sensor_dev_attr_temp4_input.dev_attr.attr,
 	&sensor_dev_attr_temp4_fault.dev_attr.attr,
+	&sensor_dev_attr_temp4_nfactor.dev_attr.attr,
 	NULL
 };
 
-- 
1.7.0.4



-- 
Jeff Angielski
The PTR Group
www.theptrgroup.com

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd attempt)

[Jeff Angielski]

On 05/18/2010 07:38 AM, Andre Prendel wrote:

> I'd prefer starting i with 0 and as condition i < data->channels - 1.
> 
> for (i = 0; i < data->channels -1; i++) {
> 	data->nfactor[i] = i2c_smbus_read_byte_data(client,
> 		TMP421_NFACTOR[i]);
> }
> 
> What do you think?

The first channel is the local channel which does not support nfactor,
so we skip it.  Having the for loop start at 1 illustrates this concept.

> Indentation of the arguments is wrong (see other functions).

Should be fixed in the patch below.  

I am not sure why the patch file itself still shows the function arguments 
off by one.  When I apply it to a raw tmp421.c all of the arguments 
line up correctly.

After rebasing the changes, I am creating the patch as:

$ git format-patch -1 HEAD

Pretty simple stuff...



>From 467c74e1d8118e34e84a150f18b5e55c6593c777 Mon Sep 17 00:00:00 2001
From: Jeff Angielski <jeff@theptrgroup.com>
Date: Mon, 10 May 2010 10:26:34 -0400
Subject: [PATCH] hwmon: (tmp421) Add nfactor support

Add support for reading and writing the n-factor correction
registers.  This is needed to compensate for the characteristics
of a particular sensor hanging off of the remote channels.

Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
---
 drivers/hwmon/tmp421.c |   41 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index 738c472..7944627 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
 
 static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
 static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
+static const u8 TMP421_NFACTOR[3]		= { 0x21, 0x22, 0x23 };
 
 /* Flags */
 #define TMP421_CONFIG_SHUTDOWN			0x40
@@ -76,6 +77,7 @@ struct tmp421_data {
 	int channels;
 	u8 config;
 	s16 temp[4];
+	s8 nfactor[3];
 };
 
 static int temp_from_s16(s16 reg)
@@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
 			data->temp[i] |= i2c_smbus_read_byte_data(client,
 				TMP421_TEMP_LSB[i]);
 		}
+		for (i = 1; i < data->channels; i++) {
+			data->nfactor[i - 1] = i2c_smbus_read_byte_data(client,
+				TMP421_NFACTOR[i - 1]);
+		}
 		data->last_updated = jiffies;
 		data->valid = 1;
 	}
@@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
 		return sprintf(buf, "0\n");
 }
 
+static ssize_t show_nfactor(struct device *dev,
+			    struct device_attribute *devattr, char *buf)
+{
+	int index = to_sensor_dev_attr(devattr)->index;
+	struct tmp421_data *data = tmp421_update_device(dev);
+
+	return sprintf(buf, "%d\n", data->nfactor[index - 1]);
+}
+
+static ssize_t set_nfactor(struct device *dev,
+			   struct device_attribute *devattr,
+			   const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct tmp421_data *data = i2c_get_clientdata(client);
+	int index = to_sensor_dev_attr(devattr)->index;
+	int nfactor = simple_strtol(buf, NULL, 10);
+
+	mutex_lock(&data->update_lock);
+	i2c_smbus_write_byte_data(client, TMP421_NFACTOR[index - 1],
+				  SENSORS_LIMIT(nfactor, -128, 127));
+	mutex_unlock(&data->update_lock);
+
+	return count;
+}
+
 static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
 				int n)
 {
@@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
 static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
 static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
 static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp2_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
+			  show_nfactor, set_nfactor, 1);
 static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
 static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp3_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
+			  show_nfactor, set_nfactor, 2);
 static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
 static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
+static SENSOR_DEVICE_ATTR(temp4_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
+			  show_nfactor, set_nfactor, 3);
 
 static struct attribute *tmp421_attr[] = {
 	&sensor_dev_attr_temp1_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_fault.dev_attr.attr,
+	&sensor_dev_attr_temp2_nfactor.dev_attr.attr,
 	&sensor_dev_attr_temp3_input.dev_attr.attr,
 	&sensor_dev_attr_temp3_fault.dev_attr.attr,
+	&sensor_dev_attr_temp3_nfactor.dev_attr.attr,
 	&sensor_dev_attr_temp4_input.dev_attr.attr,
 	&sensor_dev_attr_temp4_fault.dev_attr.attr,
+	&sensor_dev_attr_temp4_nfactor.dev_attr.attr,
 	NULL
 };
 
-- 
1.7.0.4



-- 
Jeff Angielski
The PTR Group
www.theptrgroup.com

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd

[Andre Prendel]

On Tue, May 18, 2010 at 10:35:21AM -0400, Jeff Angielski wrote:
> On 05/18/2010 07:38 AM, Andre Prendel wrote:
> 
> > I'd prefer starting i with 0 and as condition i < data->channels - 1.
> > 
> > for (i = 0; i < data->channels -1; i++) {
> > 	data->nfactor[i] = i2c_smbus_read_byte_data(client,
> > 		TMP421_NFACTOR[i]);
> > }
> > 
> > What do you think?
> 
> The first channel is the local channel which does not support nfactor,
> so we skip it.  Having the for loop start at 1 illustrates this concept.
> 
> > Indentation of the arguments is wrong (see other functions).
> 
> Should be fixed in the patch below.  

Ok, so there is only one remaining task :) Please update the documentation under
Documentation/hwmon/tmp421. Then you will get my Acked-by.

Jean, should we describe this interface in Documentation/hwmon/sysfs-interface
too? For the moment tmp421 is the only driver implementing it. 
 
> I am not sure why the patch file itself still shows the function arguments 
> off by one.  When I apply it to a raw tmp421.c all of the arguments 
> line up correctly.
> 
> After rebasing the changes, I am creating the patch as:
> 
> $ git format-patch -1 HEAD
> 
> Pretty simple stuff...
> 
> 
> 
> >From 467c74e1d8118e34e84a150f18b5e55c6593c777 Mon Sep 17 00:00:00 2001
> From: Jeff Angielski <jeff@theptrgroup.com>
> Date: Mon, 10 May 2010 10:26:34 -0400
> Subject: [PATCH] hwmon: (tmp421) Add nfactor support
> 
> Add support for reading and writing the n-factor correction
> registers.  This is needed to compensate for the characteristics
> of a particular sensor hanging off of the remote channels.
> 
> Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> ---
>  drivers/hwmon/tmp421.c |   41 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 41 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 738c472..7944627 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
>  
>  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
>  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> +static const u8 TMP421_NFACTOR[3]		= { 0x21, 0x22, 0x23 };
>  
>  /* Flags */
>  #define TMP421_CONFIG_SHUTDOWN			0x40
> @@ -76,6 +77,7 @@ struct tmp421_data {
>  	int channels;
>  	u8 config;
>  	s16 temp[4];
> +	s8 nfactor[3];
>  };
>  
>  static int temp_from_s16(s16 reg)
> @@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
>  			data->temp[i] |= i2c_smbus_read_byte_data(client,
>  				TMP421_TEMP_LSB[i]);
>  		}
> +		for (i = 1; i < data->channels; i++) {
> +			data->nfactor[i - 1] = i2c_smbus_read_byte_data(client,
> +				TMP421_NFACTOR[i - 1]);
> +		}
>  		data->last_updated = jiffies;
>  		data->valid = 1;
>  	}
> @@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
>  		return sprintf(buf, "0\n");
>  }
>  
> +static ssize_t show_nfactor(struct device *dev,
> +			    struct device_attribute *devattr, char *buf)
> +{
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	struct tmp421_data *data = tmp421_update_device(dev);
> +
> +	return sprintf(buf, "%d\n", data->nfactor[index - 1]);
> +}
> +
> +static ssize_t set_nfactor(struct device *dev,
> +			   struct device_attribute *devattr,
> +			   const char *buf, size_t count)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct tmp421_data *data = i2c_get_clientdata(client);
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	int nfactor = simple_strtol(buf, NULL, 10);
> +
> +	mutex_lock(&data->update_lock);
> +	i2c_smbus_write_byte_data(client, TMP421_NFACTOR[index - 1],
> +				  SENSORS_LIMIT(nfactor, -128, 127));
> +	mutex_unlock(&data->update_lock);
> +
> +	return count;
> +}
> +
>  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
>  				int n)
>  {
> @@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
>  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
>  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
>  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> +static SENSOR_DEVICE_ATTR(temp2_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
> +			  show_nfactor, set_nfactor, 1);
>  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
>  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> +static SENSOR_DEVICE_ATTR(temp3_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
> +			  show_nfactor, set_nfactor, 2);
>  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
>  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> +static SENSOR_DEVICE_ATTR(temp4_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
> +			  show_nfactor, set_nfactor, 3);
>  
>  static struct attribute *tmp421_attr[] = {
>  	&sensor_dev_attr_temp1_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp2_nfactor.dev_attr.attr,
>  	&sensor_dev_attr_temp3_input.dev_attr.attr,
>  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp3_nfactor.dev_attr.attr,
>  	&sensor_dev_attr_temp4_input.dev_attr.attr,
>  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp4_nfactor.dev_attr.attr,
>  	NULL
>  };
>
Thanks,
Andre

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd attempt)

[Andre Prendel]

On Tue, May 18, 2010 at 10:35:21AM -0400, Jeff Angielski wrote:
> On 05/18/2010 07:38 AM, Andre Prendel wrote:
> 
> > I'd prefer starting i with 0 and as condition i < data->channels - 1.
> > 
> > for (i = 0; i < data->channels -1; i++) {
> > 	data->nfactor[i] = i2c_smbus_read_byte_data(client,
> > 		TMP421_NFACTOR[i]);
> > }
> > 
> > What do you think?
> 
> The first channel is the local channel which does not support nfactor,
> so we skip it.  Having the for loop start at 1 illustrates this concept.
> 
> > Indentation of the arguments is wrong (see other functions).
> 
> Should be fixed in the patch below.  

Ok, so there is only one remaining task :) Please update the documentation under
Documentation/hwmon/tmp421. Then you will get my Acked-by.

Jean, should we describe this interface in Documentation/hwmon/sysfs-interface
too? For the moment tmp421 is the only driver implementing it. 
 
> I am not sure why the patch file itself still shows the function arguments 
> off by one.  When I apply it to a raw tmp421.c all of the arguments 
> line up correctly.
> 
> After rebasing the changes, I am creating the patch as:
> 
> $ git format-patch -1 HEAD
> 
> Pretty simple stuff...
> 
> 
> 
> >From 467c74e1d8118e34e84a150f18b5e55c6593c777 Mon Sep 17 00:00:00 2001
> From: Jeff Angielski <jeff@theptrgroup.com>
> Date: Mon, 10 May 2010 10:26:34 -0400
> Subject: [PATCH] hwmon: (tmp421) Add nfactor support
> 
> Add support for reading and writing the n-factor correction
> registers.  This is needed to compensate for the characteristics
> of a particular sensor hanging off of the remote channels.
> 
> Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> ---
>  drivers/hwmon/tmp421.c |   41 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 41 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 738c472..7944627 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
>  
>  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
>  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> +static const u8 TMP421_NFACTOR[3]		= { 0x21, 0x22, 0x23 };
>  
>  /* Flags */
>  #define TMP421_CONFIG_SHUTDOWN			0x40
> @@ -76,6 +77,7 @@ struct tmp421_data {
>  	int channels;
>  	u8 config;
>  	s16 temp[4];
> +	s8 nfactor[3];
>  };
>  
>  static int temp_from_s16(s16 reg)
> @@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
>  			data->temp[i] |= i2c_smbus_read_byte_data(client,
>  				TMP421_TEMP_LSB[i]);
>  		}
> +		for (i = 1; i < data->channels; i++) {
> +			data->nfactor[i - 1] = i2c_smbus_read_byte_data(client,
> +				TMP421_NFACTOR[i - 1]);
> +		}
>  		data->last_updated = jiffies;
>  		data->valid = 1;
>  	}
> @@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
>  		return sprintf(buf, "0\n");
>  }
>  
> +static ssize_t show_nfactor(struct device *dev,
> +			    struct device_attribute *devattr, char *buf)
> +{
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	struct tmp421_data *data = tmp421_update_device(dev);
> +
> +	return sprintf(buf, "%d\n", data->nfactor[index - 1]);
> +}
> +
> +static ssize_t set_nfactor(struct device *dev,
> +			   struct device_attribute *devattr,
> +			   const char *buf, size_t count)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct tmp421_data *data = i2c_get_clientdata(client);
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	int nfactor = simple_strtol(buf, NULL, 10);
> +
> +	mutex_lock(&data->update_lock);
> +	i2c_smbus_write_byte_data(client, TMP421_NFACTOR[index - 1],
> +				  SENSORS_LIMIT(nfactor, -128, 127));
> +	mutex_unlock(&data->update_lock);
> +
> +	return count;
> +}
> +
>  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
>  				int n)
>  {
> @@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
>  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
>  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
>  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> +static SENSOR_DEVICE_ATTR(temp2_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
> +			  show_nfactor, set_nfactor, 1);
>  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
>  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> +static SENSOR_DEVICE_ATTR(temp3_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
> +			  show_nfactor, set_nfactor, 2);
>  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
>  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> +static SENSOR_DEVICE_ATTR(temp4_nfactor, S_IRUSR | S_IWUSR | S_IRGRP,
> +			  show_nfactor, set_nfactor, 3);
>  
>  static struct attribute *tmp421_attr[] = {
>  	&sensor_dev_attr_temp1_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp2_nfactor.dev_attr.attr,
>  	&sensor_dev_attr_temp3_input.dev_attr.attr,
>  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp3_nfactor.dev_attr.attr,
>  	&sensor_dev_attr_temp4_input.dev_attr.attr,
>  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp4_nfactor.dev_attr.attr,
>  	NULL
>  };
>
Thanks,
Andre

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd

[Jean Delvare]

Hi Andre,

On Wed, 19 May 2010 09:26:59 +0200, Andre Prendel wrote:
> Jean, should we describe this interface in Documentation/hwmon/sysfs-interface
> too? For the moment tmp421 is the only driver implementing it.

If the values are expressed in a standard unit (i.e. not raw register
values) then yes, it would be nice to have a standard interface
described for these files.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd attempt)

[Jean Delvare]

Hi Andre,

On Wed, 19 May 2010 09:26:59 +0200, Andre Prendel wrote:
> Jean, should we describe this interface in Documentation/hwmon/sysfs-interface
> too? For the moment tmp421 is the only driver implementing it.

If the values are expressed in a standard unit (i.e. not raw register
values) then yes, it would be nice to have a standard interface
described for these files.

-- 
Jean Delvare

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd

[Jeff Angielski]

On 05/19/2010 03:26 AM, Andre Prendel wrote:
> Ok, so there is only one remaining task :) Please update the documentation under
> Documentation/hwmon/tmp421. Then you will get my Acked-by.

Documentation is updated in this patch along with the source file.

There is a small cosmetic change in this patch. I changed the name of the 
sysfs entry and internal variable name from nfactor to n_adjust.  This was 
done to make sure the users know we are modifying the N[adjust] parameter 
in the nfactor corrections register and not the actual nfactor itself. I
also changed the reference to the register from NFACTOR to N_CORRECT since
the datasheet uses that name for the register in question.

I think the method of using the nfactor adjustment value is specific to this
chipset.  Consequently, it's not appropriate to go into the general documentation.





From 61f1c203620b06463695b399bae27a884008f169 Mon Sep 17 00:00:00 2001
From: Jeff Angielski <jeff@theptrgroup.com>
Date: Mon, 10 May 2010 10:26:34 -0400
Subject: [PATCH] hwmon: (tmp421) Add nfactor support

Add support for reading and writing the n-factor correction
registers.  This is needed to compensate for the characteristics
of a particular sensor hanging off of the remote channels.

Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
---
 Documentation/hwmon/tmp421 |   19 +++++++++++++++++++
 drivers/hwmon/tmp421.c     |   41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/Documentation/hwmon/tmp421 b/Documentation/hwmon/tmp421
index 0cf07f8..668228a 100644
--- a/Documentation/hwmon/tmp421
+++ b/Documentation/hwmon/tmp421
@@ -17,6 +17,7 @@ Supported chips:
 
 Authors:
 	Andre Prendel <andre.prendel@gmx.de>
+	Jeff Angielski <jeff@theptrgroup.com>
 
 Description
 -----------
@@ -34,3 +35,21 @@ the temperature values via the following sysfs files:
 
 temp[1-4]_input
 temp[2-4]_fault
+
+The chips allow the user to adjust the n-factor value that is used
+when converting the remote channel measurements to temperature. The
+adjustment has a range of -128 to +127 that yields an effective
+n-factor range of 0.706542 to 1.747977.  The power on reset value
+for the adjustment is 0 which results in an n-factor of 1.008.
+
+The effective n-factor is calculated according to the following
+equation:
+
+n_factor = (1.008 * 300) / (300 - nfactor_adjust)
+
+The driver exports the n-factor adjustment value via the following 
+sysfs files:
+
+temp[2-4]_n_adjust
+
+
diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index 738c472..83edbc8 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
 
 static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
 static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
+static const u8 TMP421_N_CORRECT[3]		= { 0x21, 0x22, 0x23 };
 
 /* Flags */
 #define TMP421_CONFIG_SHUTDOWN			0x40
@@ -76,6 +77,7 @@ struct tmp421_data {
 	int channels;
 	u8 config;
 	s16 temp[4];
+	s8 n_adjust[3];
 };
 
 static int temp_from_s16(s16 reg)
@@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
 			data->temp[i] |= i2c_smbus_read_byte_data(client,
 				TMP421_TEMP_LSB[i]);
 		}
+		for (i = 1; i < data->channels; i++) {
+			data->n_adjust[i - 1] = i2c_smbus_read_byte_data(client,
+				TMP421_N_CORRECT[i - 1]);
+		}
 		data->last_updated = jiffies;
 		data->valid = 1;
 	}
@@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
 		return sprintf(buf, "0\n");
 }
 
+static ssize_t show_n_adjust(struct device *dev,
+			     struct device_attribute *devattr, char *buf)
+{
+	int index = to_sensor_dev_attr(devattr)->index;
+	struct tmp421_data *data = tmp421_update_device(dev);
+
+	return sprintf(buf, "%d\n", data->n_adjust[index - 1]);
+}
+
+static ssize_t set_n_adjust(struct device *dev,
+			    struct device_attribute *devattr,
+			    const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct tmp421_data *data = i2c_get_clientdata(client);
+	int index = to_sensor_dev_attr(devattr)->index;
+	int n_adjust= simple_strtol(buf, NULL, 10);
+
+	mutex_lock(&data->update_lock);
+	i2c_smbus_write_byte_data(client, TMP421_N_CORRECT[index - 1],
+				  SENSORS_LIMIT(n_adjust, -128, 127));
+	mutex_unlock(&data->update_lock);
+
+	return count;
+}
+
 static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
 				int n)
 {
@@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
 static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
 static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
 static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp2_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
+			  show_n_adjust, set_n_adjust, 1);
 static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
 static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp3_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
+			  show_n_adjust, set_n_adjust, 2);
 static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
 static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
+static SENSOR_DEVICE_ATTR(temp4_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
+			  show_n_adjust, set_n_adjust, 3);
 
 static struct attribute *tmp421_attr[] = {
 	&sensor_dev_attr_temp1_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_fault.dev_attr.attr,
+	&sensor_dev_attr_temp2_n_adjust.dev_attr.attr,
 	&sensor_dev_attr_temp3_input.dev_attr.attr,
 	&sensor_dev_attr_temp3_fault.dev_attr.attr,
+	&sensor_dev_attr_temp3_n_adjust.dev_attr.attr,
 	&sensor_dev_attr_temp4_input.dev_attr.attr,
 	&sensor_dev_attr_temp4_fault.dev_attr.attr,
+	&sensor_dev_attr_temp4_n_adjust.dev_attr.attr,
 	NULL
 };
 
-- 
1.7.0.4

-- 
Jeff Angielski
The PTR Group
www.theptrgroup.com

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd attempt)

[Jeff Angielski]

On 05/19/2010 03:26 AM, Andre Prendel wrote:
> Ok, so there is only one remaining task :) Please update the documentation under
> Documentation/hwmon/tmp421. Then you will get my Acked-by.

Documentation is updated in this patch along with the source file.

There is a small cosmetic change in this patch. I changed the name of the 
sysfs entry and internal variable name from nfactor to n_adjust.  This was 
done to make sure the users know we are modifying the N[adjust] parameter 
in the nfactor corrections register and not the actual nfactor itself. I
also changed the reference to the register from NFACTOR to N_CORRECT since
the datasheet uses that name for the register in question.

I think the method of using the nfactor adjustment value is specific to this
chipset.  Consequently, it's not appropriate to go into the general documentation.





>From 61f1c203620b06463695b399bae27a884008f169 Mon Sep 17 00:00:00 2001
From: Jeff Angielski <jeff@theptrgroup.com>
Date: Mon, 10 May 2010 10:26:34 -0400
Subject: [PATCH] hwmon: (tmp421) Add nfactor support

Add support for reading and writing the n-factor correction
registers.  This is needed to compensate for the characteristics
of a particular sensor hanging off of the remote channels.

Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
---
 Documentation/hwmon/tmp421 |   19 +++++++++++++++++++
 drivers/hwmon/tmp421.c     |   41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/Documentation/hwmon/tmp421 b/Documentation/hwmon/tmp421
index 0cf07f8..668228a 100644
--- a/Documentation/hwmon/tmp421
+++ b/Documentation/hwmon/tmp421
@@ -17,6 +17,7 @@ Supported chips:
 
 Authors:
 	Andre Prendel <andre.prendel@gmx.de>
+	Jeff Angielski <jeff@theptrgroup.com>
 
 Description
 -----------
@@ -34,3 +35,21 @@ the temperature values via the following sysfs files:
 
 temp[1-4]_input
 temp[2-4]_fault
+
+The chips allow the user to adjust the n-factor value that is used
+when converting the remote channel measurements to temperature. The
+adjustment has a range of -128 to +127 that yields an effective
+n-factor range of 0.706542 to 1.747977.  The power on reset value
+for the adjustment is 0 which results in an n-factor of 1.008.
+
+The effective n-factor is calculated according to the following
+equation:
+
+n_factor = (1.008 * 300) / (300 - nfactor_adjust)
+
+The driver exports the n-factor adjustment value via the following 
+sysfs files:
+
+temp[2-4]_n_adjust
+
+
diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index 738c472..83edbc8 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
 
 static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
 static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
+static const u8 TMP421_N_CORRECT[3]		= { 0x21, 0x22, 0x23 };
 
 /* Flags */
 #define TMP421_CONFIG_SHUTDOWN			0x40
@@ -76,6 +77,7 @@ struct tmp421_data {
 	int channels;
 	u8 config;
 	s16 temp[4];
+	s8 n_adjust[3];
 };
 
 static int temp_from_s16(s16 reg)
@@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
 			data->temp[i] |= i2c_smbus_read_byte_data(client,
 				TMP421_TEMP_LSB[i]);
 		}
+		for (i = 1; i < data->channels; i++) {
+			data->n_adjust[i - 1] = i2c_smbus_read_byte_data(client,
+				TMP421_N_CORRECT[i - 1]);
+		}
 		data->last_updated = jiffies;
 		data->valid = 1;
 	}
@@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
 		return sprintf(buf, "0\n");
 }
 
+static ssize_t show_n_adjust(struct device *dev,
+			     struct device_attribute *devattr, char *buf)
+{
+	int index = to_sensor_dev_attr(devattr)->index;
+	struct tmp421_data *data = tmp421_update_device(dev);
+
+	return sprintf(buf, "%d\n", data->n_adjust[index - 1]);
+}
+
+static ssize_t set_n_adjust(struct device *dev,
+			    struct device_attribute *devattr,
+			    const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct tmp421_data *data = i2c_get_clientdata(client);
+	int index = to_sensor_dev_attr(devattr)->index;
+	int n_adjust= simple_strtol(buf, NULL, 10);
+
+	mutex_lock(&data->update_lock);
+	i2c_smbus_write_byte_data(client, TMP421_N_CORRECT[index - 1],
+				  SENSORS_LIMIT(n_adjust, -128, 127));
+	mutex_unlock(&data->update_lock);
+
+	return count;
+}
+
 static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
 				int n)
 {
@@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
 static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
 static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
 static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp2_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
+			  show_n_adjust, set_n_adjust, 1);
 static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
 static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp3_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
+			  show_n_adjust, set_n_adjust, 2);
 static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
 static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
+static SENSOR_DEVICE_ATTR(temp4_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
+			  show_n_adjust, set_n_adjust, 3);
 
 static struct attribute *tmp421_attr[] = {
 	&sensor_dev_attr_temp1_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_fault.dev_attr.attr,
+	&sensor_dev_attr_temp2_n_adjust.dev_attr.attr,
 	&sensor_dev_attr_temp3_input.dev_attr.attr,
 	&sensor_dev_attr_temp3_fault.dev_attr.attr,
+	&sensor_dev_attr_temp3_n_adjust.dev_attr.attr,
 	&sensor_dev_attr_temp4_input.dev_attr.attr,
 	&sensor_dev_attr_temp4_fault.dev_attr.attr,
+	&sensor_dev_attr_temp4_n_adjust.dev_attr.attr,
 	NULL
 };
 
-- 
1.7.0.4

-- 
Jeff Angielski
The PTR Group
www.theptrgroup.com

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd

[Andre Prendel]

On Wed, May 19, 2010 at 01:13:04PM -0400, Jeff Angielski wrote:
> On 05/19/2010 03:26 AM, Andre Prendel wrote:
> > Ok, so there is only one remaining task :) Please update the documentation under
> > Documentation/hwmon/tmp421. Then you will get my Acked-by.
> 
> Documentation is updated in this patch along with the source file.
> 
> There is a small cosmetic change in this patch. I changed the name of the 
> sysfs entry and internal variable name from nfactor to n_adjust.  This was 
> done to make sure the users know we are modifying the N[adjust] parameter 
> in the nfactor corrections register and not the actual nfactor itself. I
> also changed the reference to the register from NFACTOR to N_CORRECT since
> the datasheet uses that name for the register in question.
> 
> I think the method of using the nfactor adjustment value is specific to this
> chipset.  Consequently, it's not appropriate to go into the general documentation.
> 

You made a careless mistake, see below. Please fix this and resend the patch
again. Then I added my Acked-by and I think Jean will schedule the patch for
2.6.35 (after another review of course :)) 
 
> 
> 
> 
> >From 61f1c203620b06463695b399bae27a884008f169 Mon Sep 17 00:00:00 2001
> From: Jeff Angielski <jeff@theptrgroup.com>
> Date: Mon, 10 May 2010 10:26:34 -0400
> Subject: [PATCH] hwmon: (tmp421) Add nfactor support
> 
> Add support for reading and writing the n-factor correction
> registers.  This is needed to compensate for the characteristics
> of a particular sensor hanging off of the remote channels.
> 
> Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> ---
>  Documentation/hwmon/tmp421 |   19 +++++++++++++++++++
>  drivers/hwmon/tmp421.c     |   41 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 60 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/hwmon/tmp421 b/Documentation/hwmon/tmp421
> index 0cf07f8..668228a 100644
> --- a/Documentation/hwmon/tmp421
> +++ b/Documentation/hwmon/tmp421
> @@ -17,6 +17,7 @@ Supported chips:
>  
>  Authors:
>  	Andre Prendel <andre.prendel@gmx.de>
> +	Jeff Angielski <jeff@theptrgroup.com>
>  
>  Description
>  -----------
> @@ -34,3 +35,21 @@ the temperature values via the following sysfs files:
>  
>  temp[1-4]_input
>  temp[2-4]_fault
> +
> +The chips allow the user to adjust the n-factor value that is used
> +when converting the remote channel measurements to temperature. The
> +adjustment has a range of -128 to +127 that yields an effective
> +n-factor range of 0.706542 to 1.747977.  The power on reset value
> +for the adjustment is 0 which results in an n-factor of 1.008.
> +
> +The effective n-factor is calculated according to the following
> +equation:
> +
> +n_factor = (1.008 * 300) / (300 - nfactor_adjust)
> +
> +The driver exports the n-factor adjustment value via the following 
> +sysfs files:
> +
> +temp[2-4]_n_adjust
> +
> +
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 738c472..83edbc8 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
>  
>  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
>  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> +static const u8 TMP421_N_CORRECT[3]		= { 0x21, 0x22, 0x23 };
>  
>  /* Flags */
>  #define TMP421_CONFIG_SHUTDOWN			0x40
> @@ -76,6 +77,7 @@ struct tmp421_data {
>  	int channels;
>  	u8 config;
>  	s16 temp[4];
> +	s8 n_adjust[3];
>  };
>  
>  static int temp_from_s16(s16 reg)
> @@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
>  			data->temp[i] |= i2c_smbus_read_byte_data(client,
>  				TMP421_TEMP_LSB[i]);
>  		}
> +		for (i = 1; i < data->channels; i++) {
> +			data->n_adjust[i - 1] = i2c_smbus_read_byte_data(client,
> +				TMP421_N_CORRECT[i - 1]);
> +		}
>  		data->last_updated = jiffies;
>  		data->valid = 1;
>  	}
> @@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
>  		return sprintf(buf, "0\n");
>  }
>  
> +static ssize_t show_n_adjust(struct device *dev,
> +			     struct device_attribute *devattr, char *buf)
> +{
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	struct tmp421_data *data = tmp421_update_device(dev);
> +
> +	return sprintf(buf, "%d\n", data->n_adjust[index - 1]);
> +}
> +
> +static ssize_t set_n_adjust(struct device *dev,
> +			    struct device_attribute *devattr,
> +			    const char *buf, size_t count)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct tmp421_data *data = i2c_get_clientdata(client);
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	int n_adjust= simple_strtol(buf, NULL, 10);

Missing space in front of '='.

> +
> +	mutex_lock(&data->update_lock);
> +	i2c_smbus_write_byte_data(client, TMP421_N_CORRECT[index - 1],
> +				  SENSORS_LIMIT(n_adjust, -128, 127));
> +	mutex_unlock(&data->update_lock);
> +
> +	return count;
> +}
> +
>  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
>  				int n)
>  {
> @@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
>  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
>  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
>  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> +static SENSOR_DEVICE_ATTR(temp2_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> +			  show_n_adjust, set_n_adjust, 1);
>  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
>  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> +static SENSOR_DEVICE_ATTR(temp3_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> +			  show_n_adjust, set_n_adjust, 2);
>  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
>  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> +static SENSOR_DEVICE_ATTR(temp4_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> +			  show_n_adjust, set_n_adjust, 3);
>  
>  static struct attribute *tmp421_attr[] = {
>  	&sensor_dev_attr_temp1_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp2_n_adjust.dev_attr.attr,
>  	&sensor_dev_attr_temp3_input.dev_attr.attr,
>  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp3_n_adjust.dev_attr.attr,
>  	&sensor_dev_attr_temp4_input.dev_attr.attr,
>  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp4_n_adjust.dev_attr.attr,
>  	NULL
>  };
>

Thanks,
Andre  

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd attempt)

[Andre Prendel]

On Wed, May 19, 2010 at 01:13:04PM -0400, Jeff Angielski wrote:
> On 05/19/2010 03:26 AM, Andre Prendel wrote:
> > Ok, so there is only one remaining task :) Please update the documentation under
> > Documentation/hwmon/tmp421. Then you will get my Acked-by.
> 
> Documentation is updated in this patch along with the source file.
> 
> There is a small cosmetic change in this patch. I changed the name of the 
> sysfs entry and internal variable name from nfactor to n_adjust.  This was 
> done to make sure the users know we are modifying the N[adjust] parameter 
> in the nfactor corrections register and not the actual nfactor itself. I
> also changed the reference to the register from NFACTOR to N_CORRECT since
> the datasheet uses that name for the register in question.
> 
> I think the method of using the nfactor adjustment value is specific to this
> chipset.  Consequently, it's not appropriate to go into the general documentation.
> 

You made a careless mistake, see below. Please fix this and resend the patch
again. Then I added my Acked-by and I think Jean will schedule the patch for
2.6.35 (after another review of course :)) 
 
> 
> 
> 
> >From 61f1c203620b06463695b399bae27a884008f169 Mon Sep 17 00:00:00 2001
> From: Jeff Angielski <jeff@theptrgroup.com>
> Date: Mon, 10 May 2010 10:26:34 -0400
> Subject: [PATCH] hwmon: (tmp421) Add nfactor support
> 
> Add support for reading and writing the n-factor correction
> registers.  This is needed to compensate for the characteristics
> of a particular sensor hanging off of the remote channels.
> 
> Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> ---
>  Documentation/hwmon/tmp421 |   19 +++++++++++++++++++
>  drivers/hwmon/tmp421.c     |   41 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 60 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/hwmon/tmp421 b/Documentation/hwmon/tmp421
> index 0cf07f8..668228a 100644
> --- a/Documentation/hwmon/tmp421
> +++ b/Documentation/hwmon/tmp421
> @@ -17,6 +17,7 @@ Supported chips:
>  
>  Authors:
>  	Andre Prendel <andre.prendel@gmx.de>
> +	Jeff Angielski <jeff@theptrgroup.com>
>  
>  Description
>  -----------
> @@ -34,3 +35,21 @@ the temperature values via the following sysfs files:
>  
>  temp[1-4]_input
>  temp[2-4]_fault
> +
> +The chips allow the user to adjust the n-factor value that is used
> +when converting the remote channel measurements to temperature. The
> +adjustment has a range of -128 to +127 that yields an effective
> +n-factor range of 0.706542 to 1.747977.  The power on reset value
> +for the adjustment is 0 which results in an n-factor of 1.008.
> +
> +The effective n-factor is calculated according to the following
> +equation:
> +
> +n_factor = (1.008 * 300) / (300 - nfactor_adjust)
> +
> +The driver exports the n-factor adjustment value via the following 
> +sysfs files:
> +
> +temp[2-4]_n_adjust
> +
> +
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 738c472..83edbc8 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
>  
>  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
>  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> +static const u8 TMP421_N_CORRECT[3]		= { 0x21, 0x22, 0x23 };
>  
>  /* Flags */
>  #define TMP421_CONFIG_SHUTDOWN			0x40
> @@ -76,6 +77,7 @@ struct tmp421_data {
>  	int channels;
>  	u8 config;
>  	s16 temp[4];
> +	s8 n_adjust[3];
>  };
>  
>  static int temp_from_s16(s16 reg)
> @@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
>  			data->temp[i] |= i2c_smbus_read_byte_data(client,
>  				TMP421_TEMP_LSB[i]);
>  		}
> +		for (i = 1; i < data->channels; i++) {
> +			data->n_adjust[i - 1] = i2c_smbus_read_byte_data(client,
> +				TMP421_N_CORRECT[i - 1]);
> +		}
>  		data->last_updated = jiffies;
>  		data->valid = 1;
>  	}
> @@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
>  		return sprintf(buf, "0\n");
>  }
>  
> +static ssize_t show_n_adjust(struct device *dev,
> +			     struct device_attribute *devattr, char *buf)
> +{
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	struct tmp421_data *data = tmp421_update_device(dev);
> +
> +	return sprintf(buf, "%d\n", data->n_adjust[index - 1]);
> +}
> +
> +static ssize_t set_n_adjust(struct device *dev,
> +			    struct device_attribute *devattr,
> +			    const char *buf, size_t count)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct tmp421_data *data = i2c_get_clientdata(client);
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	int n_adjust= simple_strtol(buf, NULL, 10);

Missing space in front of '='.

> +
> +	mutex_lock(&data->update_lock);
> +	i2c_smbus_write_byte_data(client, TMP421_N_CORRECT[index - 1],
> +				  SENSORS_LIMIT(n_adjust, -128, 127));
> +	mutex_unlock(&data->update_lock);
> +
> +	return count;
> +}
> +
>  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
>  				int n)
>  {
> @@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
>  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
>  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
>  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> +static SENSOR_DEVICE_ATTR(temp2_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> +			  show_n_adjust, set_n_adjust, 1);
>  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
>  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> +static SENSOR_DEVICE_ATTR(temp3_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> +			  show_n_adjust, set_n_adjust, 2);
>  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
>  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> +static SENSOR_DEVICE_ATTR(temp4_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> +			  show_n_adjust, set_n_adjust, 3);
>  
>  static struct attribute *tmp421_attr[] = {
>  	&sensor_dev_attr_temp1_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp2_n_adjust.dev_attr.attr,
>  	&sensor_dev_attr_temp3_input.dev_attr.attr,
>  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp3_n_adjust.dev_attr.attr,
>  	&sensor_dev_attr_temp4_input.dev_attr.attr,
>  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp4_n_adjust.dev_attr.attr,
>  	NULL
>  };
>

Thanks,
Andre  

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd

[Jeff Angielski]

On 05/20/2010 02:50 PM, Andre Prendel wrote:
> You made a careless mistake, see below. Please fix this and resend the patch
> again. Then I added my Acked-by and I think Jean will schedule the patch for
> 2.6.35 (after another review of course :)) 

You sir, would be correct... *sigh*  I tried using the scripts/Lindent on it to
be sure there were no coding style issues but that messed up even your original 
code.  

In any event, here it is again:

From 9acd29ff48c64e58a7f5cdb888c86e737c56281c Mon Sep 17 00:00:00 2001
From: Jeff Angielski <jeff@theptrgroup.com>
Date: Mon, 10 May 2010 10:26:34 -0400
Subject: [PATCH] hwmon: (tmp421) Add nfactor support

Add support for reading and writing the n-factor correction
registers.  This is needed to compensate for the characteristics
of a particular sensor hanging off of the remote channels.

Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
---
 Documentation/hwmon/tmp421 |   19 +++++++++++++++++++
 drivers/hwmon/tmp421.c     |   41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/Documentation/hwmon/tmp421 b/Documentation/hwmon/tmp421
index 0cf07f8..668228a 100644
--- a/Documentation/hwmon/tmp421
+++ b/Documentation/hwmon/tmp421
@@ -17,6 +17,7 @@ Supported chips:
 
 Authors:
 	Andre Prendel <andre.prendel@gmx.de>
+	Jeff Angielski <jeff@theptrgroup.com>
 
 Description
 -----------
@@ -34,3 +35,21 @@ the temperature values via the following sysfs files:
 
 temp[1-4]_input
 temp[2-4]_fault
+
+The chips allow the user to adjust the n-factor value that is used
+when converting the remote channel measurements to temperature. The
+adjustment has a range of -128 to +127 that yields an effective
+n-factor range of 0.706542 to 1.747977.  The power on reset value
+for the adjustment is 0 which results in an n-factor of 1.008.
+
+The effective n-factor is calculated according to the following
+equation:
+
+n_factor = (1.008 * 300) / (300 - nfactor_adjust)
+
+The driver exports the n-factor adjustment value via the following 
+sysfs files:
+
+temp[2-4]_n_adjust
+
+
diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index 738c472..dfd62be 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
 
 static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
 static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
+static const u8 TMP421_N_CORRECT[3]		= { 0x21, 0x22, 0x23 };
 
 /* Flags */
 #define TMP421_CONFIG_SHUTDOWN			0x40
@@ -76,6 +77,7 @@ struct tmp421_data {
 	int channels;
 	u8 config;
 	s16 temp[4];
+	s8 n_adjust[3];
 };
 
 static int temp_from_s16(s16 reg)
@@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
 			data->temp[i] |= i2c_smbus_read_byte_data(client,
 				TMP421_TEMP_LSB[i]);
 		}
+		for (i = 1; i < data->channels; i++) {
+			data->n_adjust[i - 1] = i2c_smbus_read_byte_data(client,
+				TMP421_N_CORRECT[i - 1]);
+		}
 		data->last_updated = jiffies;
 		data->valid = 1;
 	}
@@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
 		return sprintf(buf, "0\n");
 }
 
+static ssize_t show_n_adjust(struct device *dev,
+			     struct device_attribute *devattr, char *buf)
+{
+	int index = to_sensor_dev_attr(devattr)->index;
+	struct tmp421_data *data = tmp421_update_device(dev);
+
+	return sprintf(buf, "%d\n", data->n_adjust[index - 1]);
+}
+
+static ssize_t set_n_adjust(struct device *dev,
+			    struct device_attribute *devattr,
+			    const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct tmp421_data *data = i2c_get_clientdata(client);
+	int index = to_sensor_dev_attr(devattr)->index;
+	int n_adjust = simple_strtol(buf, NULL, 10);
+
+	mutex_lock(&data->update_lock);
+	i2c_smbus_write_byte_data(client, TMP421_N_CORRECT[index - 1],
+				  SENSORS_LIMIT(n_adjust, -128, 127));
+	mutex_unlock(&data->update_lock);
+
+	return count;
+}
+
 static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
 				int n)
 {
@@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
 static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
 static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
 static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp2_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
+			  show_n_adjust, set_n_adjust, 1);
 static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
 static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp3_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
+			  show_n_adjust, set_n_adjust, 2);
 static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
 static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
+static SENSOR_DEVICE_ATTR(temp4_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
+			  show_n_adjust, set_n_adjust, 3);
 
 static struct attribute *tmp421_attr[] = {
 	&sensor_dev_attr_temp1_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_fault.dev_attr.attr,
+	&sensor_dev_attr_temp2_n_adjust.dev_attr.attr,
 	&sensor_dev_attr_temp3_input.dev_attr.attr,
 	&sensor_dev_attr_temp3_fault.dev_attr.attr,
+	&sensor_dev_attr_temp3_n_adjust.dev_attr.attr,
 	&sensor_dev_attr_temp4_input.dev_attr.attr,
 	&sensor_dev_attr_temp4_fault.dev_attr.attr,
+	&sensor_dev_attr_temp4_n_adjust.dev_attr.attr,
 	NULL
 };
 
-- 
1.7.0.4


-- 
Jeff Angielski
The PTR Group
www.theptrgroup.com

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd attempt)

[Jeff Angielski]

On 05/20/2010 02:50 PM, Andre Prendel wrote:
> You made a careless mistake, see below. Please fix this and resend the patch
> again. Then I added my Acked-by and I think Jean will schedule the patch for
> 2.6.35 (after another review of course :)) 

You sir, would be correct... *sigh*  I tried using the scripts/Lindent on it to
be sure there were no coding style issues but that messed up even your original 
code.  

In any event, here it is again:

>From 9acd29ff48c64e58a7f5cdb888c86e737c56281c Mon Sep 17 00:00:00 2001
From: Jeff Angielski <jeff@theptrgroup.com>
Date: Mon, 10 May 2010 10:26:34 -0400
Subject: [PATCH] hwmon: (tmp421) Add nfactor support

Add support for reading and writing the n-factor correction
registers.  This is needed to compensate for the characteristics
of a particular sensor hanging off of the remote channels.

Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
---
 Documentation/hwmon/tmp421 |   19 +++++++++++++++++++
 drivers/hwmon/tmp421.c     |   41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/Documentation/hwmon/tmp421 b/Documentation/hwmon/tmp421
index 0cf07f8..668228a 100644
--- a/Documentation/hwmon/tmp421
+++ b/Documentation/hwmon/tmp421
@@ -17,6 +17,7 @@ Supported chips:
 
 Authors:
 	Andre Prendel <andre.prendel@gmx.de>
+	Jeff Angielski <jeff@theptrgroup.com>
 
 Description
 -----------
@@ -34,3 +35,21 @@ the temperature values via the following sysfs files:
 
 temp[1-4]_input
 temp[2-4]_fault
+
+The chips allow the user to adjust the n-factor value that is used
+when converting the remote channel measurements to temperature. The
+adjustment has a range of -128 to +127 that yields an effective
+n-factor range of 0.706542 to 1.747977.  The power on reset value
+for the adjustment is 0 which results in an n-factor of 1.008.
+
+The effective n-factor is calculated according to the following
+equation:
+
+n_factor = (1.008 * 300) / (300 - nfactor_adjust)
+
+The driver exports the n-factor adjustment value via the following 
+sysfs files:
+
+temp[2-4]_n_adjust
+
+
diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index 738c472..dfd62be 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
 
 static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
 static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
+static const u8 TMP421_N_CORRECT[3]		= { 0x21, 0x22, 0x23 };
 
 /* Flags */
 #define TMP421_CONFIG_SHUTDOWN			0x40
@@ -76,6 +77,7 @@ struct tmp421_data {
 	int channels;
 	u8 config;
 	s16 temp[4];
+	s8 n_adjust[3];
 };
 
 static int temp_from_s16(s16 reg)
@@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
 			data->temp[i] |= i2c_smbus_read_byte_data(client,
 				TMP421_TEMP_LSB[i]);
 		}
+		for (i = 1; i < data->channels; i++) {
+			data->n_adjust[i - 1] = i2c_smbus_read_byte_data(client,
+				TMP421_N_CORRECT[i - 1]);
+		}
 		data->last_updated = jiffies;
 		data->valid = 1;
 	}
@@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
 		return sprintf(buf, "0\n");
 }
 
+static ssize_t show_n_adjust(struct device *dev,
+			     struct device_attribute *devattr, char *buf)
+{
+	int index = to_sensor_dev_attr(devattr)->index;
+	struct tmp421_data *data = tmp421_update_device(dev);
+
+	return sprintf(buf, "%d\n", data->n_adjust[index - 1]);
+}
+
+static ssize_t set_n_adjust(struct device *dev,
+			    struct device_attribute *devattr,
+			    const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct tmp421_data *data = i2c_get_clientdata(client);
+	int index = to_sensor_dev_attr(devattr)->index;
+	int n_adjust = simple_strtol(buf, NULL, 10);
+
+	mutex_lock(&data->update_lock);
+	i2c_smbus_write_byte_data(client, TMP421_N_CORRECT[index - 1],
+				  SENSORS_LIMIT(n_adjust, -128, 127));
+	mutex_unlock(&data->update_lock);
+
+	return count;
+}
+
 static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
 				int n)
 {
@@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
 static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
 static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
 static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp2_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
+			  show_n_adjust, set_n_adjust, 1);
 static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
 static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp3_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
+			  show_n_adjust, set_n_adjust, 2);
 static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
 static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
+static SENSOR_DEVICE_ATTR(temp4_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
+			  show_n_adjust, set_n_adjust, 3);
 
 static struct attribute *tmp421_attr[] = {
 	&sensor_dev_attr_temp1_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_fault.dev_attr.attr,
+	&sensor_dev_attr_temp2_n_adjust.dev_attr.attr,
 	&sensor_dev_attr_temp3_input.dev_attr.attr,
 	&sensor_dev_attr_temp3_fault.dev_attr.attr,
+	&sensor_dev_attr_temp3_n_adjust.dev_attr.attr,
 	&sensor_dev_attr_temp4_input.dev_attr.attr,
 	&sensor_dev_attr_temp4_fault.dev_attr.attr,
+	&sensor_dev_attr_temp4_n_adjust.dev_attr.attr,
 	NULL
 };
 
-- 
1.7.0.4


-- 
Jeff Angielski
The PTR Group
www.theptrgroup.com

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd

[Andre Prendel]

On Thu, May 20, 2010 at 03:07:05PM -0400, Jeff Angielski wrote:
> On 05/20/2010 02:50 PM, Andre Prendel wrote:
> > You made a careless mistake, see below. Please fix this and resend the patch
> > again. Then I added my Acked-by and I think Jean will schedule the patch for
> > 2.6.35 (after another review of course :)) 
> 
> You sir, would be correct... *sigh*  I tried using the scripts/Lindent on it to
> be sure there were no coding style issues but that messed up even your original 
> code.  

/scripts/checkpatch.pl is the key ;)
 
> In any event, here it is again:

Acked-by: Andre Prendel <andre.prendel@gmx.de>

> 
> >From 9acd29ff48c64e58a7f5cdb888c86e737c56281c Mon Sep 17 00:00:00 2001
> From: Jeff Angielski <jeff@theptrgroup.com>
> Date: Mon, 10 May 2010 10:26:34 -0400
> Subject: [PATCH] hwmon: (tmp421) Add nfactor support
> 
> Add support for reading and writing the n-factor correction
> registers.  This is needed to compensate for the characteristics
> of a particular sensor hanging off of the remote channels.
> 
> Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> ---
>  Documentation/hwmon/tmp421 |   19 +++++++++++++++++++
>  drivers/hwmon/tmp421.c     |   41 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 60 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/hwmon/tmp421 b/Documentation/hwmon/tmp421
> index 0cf07f8..668228a 100644
> --- a/Documentation/hwmon/tmp421
> +++ b/Documentation/hwmon/tmp421
> @@ -17,6 +17,7 @@ Supported chips:
>  
>  Authors:
>  	Andre Prendel <andre.prendel@gmx.de>
> +	Jeff Angielski <jeff@theptrgroup.com>
>  
>  Description
>  -----------
> @@ -34,3 +35,21 @@ the temperature values via the following sysfs files:
>  
>  temp[1-4]_input
>  temp[2-4]_fault
> +
> +The chips allow the user to adjust the n-factor value that is used
> +when converting the remote channel measurements to temperature. The
> +adjustment has a range of -128 to +127 that yields an effective
> +n-factor range of 0.706542 to 1.747977.  The power on reset value
> +for the adjustment is 0 which results in an n-factor of 1.008.
> +
> +The effective n-factor is calculated according to the following
> +equation:
> +
> +n_factor = (1.008 * 300) / (300 - nfactor_adjust)
> +
> +The driver exports the n-factor adjustment value via the following 
> +sysfs files:
> +
> +temp[2-4]_n_adjust
> +
> +
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 738c472..dfd62be 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
>  
>  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
>  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> +static const u8 TMP421_N_CORRECT[3]		= { 0x21, 0x22, 0x23 };
>  
>  /* Flags */
>  #define TMP421_CONFIG_SHUTDOWN			0x40
> @@ -76,6 +77,7 @@ struct tmp421_data {
>  	int channels;
>  	u8 config;
>  	s16 temp[4];
> +	s8 n_adjust[3];
>  };
>  
>  static int temp_from_s16(s16 reg)
> @@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
>  			data->temp[i] |= i2c_smbus_read_byte_data(client,
>  				TMP421_TEMP_LSB[i]);
>  		}
> +		for (i = 1; i < data->channels; i++) {
> +			data->n_adjust[i - 1] = i2c_smbus_read_byte_data(client,
> +				TMP421_N_CORRECT[i - 1]);
> +		}
>  		data->last_updated = jiffies;
>  		data->valid = 1;
>  	}
> @@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
>  		return sprintf(buf, "0\n");
>  }
>  
> +static ssize_t show_n_adjust(struct device *dev,
> +			     struct device_attribute *devattr, char *buf)
> +{
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	struct tmp421_data *data = tmp421_update_device(dev);
> +
> +	return sprintf(buf, "%d\n", data->n_adjust[index - 1]);
> +}
> +
> +static ssize_t set_n_adjust(struct device *dev,
> +			    struct device_attribute *devattr,
> +			    const char *buf, size_t count)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct tmp421_data *data = i2c_get_clientdata(client);
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	int n_adjust = simple_strtol(buf, NULL, 10);
> +
> +	mutex_lock(&data->update_lock);
> +	i2c_smbus_write_byte_data(client, TMP421_N_CORRECT[index - 1],
> +				  SENSORS_LIMIT(n_adjust, -128, 127));
> +	mutex_unlock(&data->update_lock);
> +
> +	return count;
> +}
> +
>  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
>  				int n)
>  {
> @@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
>  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
>  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
>  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> +static SENSOR_DEVICE_ATTR(temp2_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> +			  show_n_adjust, set_n_adjust, 1);
>  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
>  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> +static SENSOR_DEVICE_ATTR(temp3_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> +			  show_n_adjust, set_n_adjust, 2);
>  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
>  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> +static SENSOR_DEVICE_ATTR(temp4_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> +			  show_n_adjust, set_n_adjust, 3);
>  
>  static struct attribute *tmp421_attr[] = {
>  	&sensor_dev_attr_temp1_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp2_n_adjust.dev_attr.attr,
>  	&sensor_dev_attr_temp3_input.dev_attr.attr,
>  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp3_n_adjust.dev_attr.attr,
>  	&sensor_dev_attr_temp4_input.dev_attr.attr,
>  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp4_n_adjust.dev_attr.attr,
>  	NULL
>  };
>  
> -- 
> 1.7.0.4
> 
> 
> -- 
> Jeff Angielski
> The PTR Group
> www.theptrgroup.com
> 
> _______________________________________________
> lm-sensors mailing list
> lm-sensors@lm-sensors.org
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd attempt)

[Andre Prendel]

On Thu, May 20, 2010 at 03:07:05PM -0400, Jeff Angielski wrote:
> On 05/20/2010 02:50 PM, Andre Prendel wrote:
> > You made a careless mistake, see below. Please fix this and resend the patch
> > again. Then I added my Acked-by and I think Jean will schedule the patch for
> > 2.6.35 (after another review of course :)) 
> 
> You sir, would be correct... *sigh*  I tried using the scripts/Lindent on it to
> be sure there were no coding style issues but that messed up even your original 
> code.  

/scripts/checkpatch.pl is the key ;)
 
> In any event, here it is again:

Acked-by: Andre Prendel <andre.prendel@gmx.de>

> 
> >From 9acd29ff48c64e58a7f5cdb888c86e737c56281c Mon Sep 17 00:00:00 2001
> From: Jeff Angielski <jeff@theptrgroup.com>
> Date: Mon, 10 May 2010 10:26:34 -0400
> Subject: [PATCH] hwmon: (tmp421) Add nfactor support
> 
> Add support for reading and writing the n-factor correction
> registers.  This is needed to compensate for the characteristics
> of a particular sensor hanging off of the remote channels.
> 
> Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> ---
>  Documentation/hwmon/tmp421 |   19 +++++++++++++++++++
>  drivers/hwmon/tmp421.c     |   41 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 60 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/hwmon/tmp421 b/Documentation/hwmon/tmp421
> index 0cf07f8..668228a 100644
> --- a/Documentation/hwmon/tmp421
> +++ b/Documentation/hwmon/tmp421
> @@ -17,6 +17,7 @@ Supported chips:
>  
>  Authors:
>  	Andre Prendel <andre.prendel@gmx.de>
> +	Jeff Angielski <jeff@theptrgroup.com>
>  
>  Description
>  -----------
> @@ -34,3 +35,21 @@ the temperature values via the following sysfs files:
>  
>  temp[1-4]_input
>  temp[2-4]_fault
> +
> +The chips allow the user to adjust the n-factor value that is used
> +when converting the remote channel measurements to temperature. The
> +adjustment has a range of -128 to +127 that yields an effective
> +n-factor range of 0.706542 to 1.747977.  The power on reset value
> +for the adjustment is 0 which results in an n-factor of 1.008.
> +
> +The effective n-factor is calculated according to the following
> +equation:
> +
> +n_factor = (1.008 * 300) / (300 - nfactor_adjust)
> +
> +The driver exports the n-factor adjustment value via the following 
> +sysfs files:
> +
> +temp[2-4]_n_adjust
> +
> +
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 738c472..dfd62be 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
>  
>  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
>  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> +static const u8 TMP421_N_CORRECT[3]		= { 0x21, 0x22, 0x23 };
>  
>  /* Flags */
>  #define TMP421_CONFIG_SHUTDOWN			0x40
> @@ -76,6 +77,7 @@ struct tmp421_data {
>  	int channels;
>  	u8 config;
>  	s16 temp[4];
> +	s8 n_adjust[3];
>  };
>  
>  static int temp_from_s16(s16 reg)
> @@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
>  			data->temp[i] |= i2c_smbus_read_byte_data(client,
>  				TMP421_TEMP_LSB[i]);
>  		}
> +		for (i = 1; i < data->channels; i++) {
> +			data->n_adjust[i - 1] = i2c_smbus_read_byte_data(client,
> +				TMP421_N_CORRECT[i - 1]);
> +		}
>  		data->last_updated = jiffies;
>  		data->valid = 1;
>  	}
> @@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
>  		return sprintf(buf, "0\n");
>  }
>  
> +static ssize_t show_n_adjust(struct device *dev,
> +			     struct device_attribute *devattr, char *buf)
> +{
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	struct tmp421_data *data = tmp421_update_device(dev);
> +
> +	return sprintf(buf, "%d\n", data->n_adjust[index - 1]);
> +}
> +
> +static ssize_t set_n_adjust(struct device *dev,
> +			    struct device_attribute *devattr,
> +			    const char *buf, size_t count)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct tmp421_data *data = i2c_get_clientdata(client);
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	int n_adjust = simple_strtol(buf, NULL, 10);
> +
> +	mutex_lock(&data->update_lock);
> +	i2c_smbus_write_byte_data(client, TMP421_N_CORRECT[index - 1],
> +				  SENSORS_LIMIT(n_adjust, -128, 127));
> +	mutex_unlock(&data->update_lock);
> +
> +	return count;
> +}
> +
>  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
>  				int n)
>  {
> @@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
>  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
>  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
>  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> +static SENSOR_DEVICE_ATTR(temp2_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> +			  show_n_adjust, set_n_adjust, 1);
>  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
>  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> +static SENSOR_DEVICE_ATTR(temp3_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> +			  show_n_adjust, set_n_adjust, 2);
>  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
>  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> +static SENSOR_DEVICE_ATTR(temp4_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> +			  show_n_adjust, set_n_adjust, 3);
>  
>  static struct attribute *tmp421_attr[] = {
>  	&sensor_dev_attr_temp1_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp2_n_adjust.dev_attr.attr,
>  	&sensor_dev_attr_temp3_input.dev_attr.attr,
>  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp3_n_adjust.dev_attr.attr,
>  	&sensor_dev_attr_temp4_input.dev_attr.attr,
>  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp4_n_adjust.dev_attr.attr,
>  	NULL
>  };
>  
> -- 
> 1.7.0.4
> 
> 
> -- 
> Jeff Angielski
> The PTR Group
> www.theptrgroup.com
> 
> _______________________________________________
> lm-sensors mailing list
> lm-sensors@lm-sensors.org
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd

[Andre Prendel]

On Thu, May 20, 2010 at 09:35:56PM +0200, Andre Prendel wrote:
> On Thu, May 20, 2010 at 03:07:05PM -0400, Jeff Angielski wrote:
> > On 05/20/2010 02:50 PM, Andre Prendel wrote:
> > > You made a careless mistake, see below. Please fix this and resend the patch
> > > again. Then I added my Acked-by and I think Jean will schedule the patch for
> > > 2.6.35 (after another review of course :)) 
> > 
> > You sir, would be correct... *sigh*  I tried using the scripts/Lindent on it to
> > be sure there were no coding style issues but that messed up even your original 
> > code.  
> 
> /scripts/checkpatch.pl is the key ;)
>  
> > In any event, here it is again:
> 
> Acked-by: Andre Prendel <andre.prendel@gmx.de>

Hi Jean,

Any News on this patch?
 
> > 
> > >From 9acd29ff48c64e58a7f5cdb888c86e737c56281c Mon Sep 17 00:00:00 2001
> > From: Jeff Angielski <jeff@theptrgroup.com>
> > Date: Mon, 10 May 2010 10:26:34 -0400
> > Subject: [PATCH] hwmon: (tmp421) Add nfactor support
> > 
> > Add support for reading and writing the n-factor correction
> > registers.  This is needed to compensate for the characteristics
> > of a particular sensor hanging off of the remote channels.
> > 
> > Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> > ---
> >  Documentation/hwmon/tmp421 |   19 +++++++++++++++++++
> >  drivers/hwmon/tmp421.c     |   41 +++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 60 insertions(+), 0 deletions(-)
> > 
> > diff --git a/Documentation/hwmon/tmp421 b/Documentation/hwmon/tmp421
> > index 0cf07f8..668228a 100644
> > --- a/Documentation/hwmon/tmp421
> > +++ b/Documentation/hwmon/tmp421
> > @@ -17,6 +17,7 @@ Supported chips:
> >  
> >  Authors:
> >  	Andre Prendel <andre.prendel@gmx.de>
> > +	Jeff Angielski <jeff@theptrgroup.com>
> >  
> >  Description
> >  -----------
> > @@ -34,3 +35,21 @@ the temperature values via the following sysfs files:
> >  
> >  temp[1-4]_input
> >  temp[2-4]_fault
> > +
> > +The chips allow the user to adjust the n-factor value that is used
> > +when converting the remote channel measurements to temperature. The
> > +adjustment has a range of -128 to +127 that yields an effective
> > +n-factor range of 0.706542 to 1.747977.  The power on reset value
> > +for the adjustment is 0 which results in an n-factor of 1.008.
> > +
> > +The effective n-factor is calculated according to the following
> > +equation:
> > +
> > +n_factor = (1.008 * 300) / (300 - nfactor_adjust)
> > +
> > +The driver exports the n-factor adjustment value via the following 
> > +sysfs files:
> > +
> > +temp[2-4]_n_adjust
> > +
> > +
> > diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> > index 738c472..dfd62be 100644
> > --- a/drivers/hwmon/tmp421.c
> > +++ b/drivers/hwmon/tmp421.c
> > @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
> >  
> >  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
> >  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> > +static const u8 TMP421_N_CORRECT[3]		= { 0x21, 0x22, 0x23 };
> >  
> >  /* Flags */
> >  #define TMP421_CONFIG_SHUTDOWN			0x40
> > @@ -76,6 +77,7 @@ struct tmp421_data {
> >  	int channels;
> >  	u8 config;
> >  	s16 temp[4];
> > +	s8 n_adjust[3];
> >  };
> >  
> >  static int temp_from_s16(s16 reg)
> > @@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
> >  			data->temp[i] |= i2c_smbus_read_byte_data(client,
> >  				TMP421_TEMP_LSB[i]);
> >  		}
> > +		for (i = 1; i < data->channels; i++) {
> > +			data->n_adjust[i - 1] = i2c_smbus_read_byte_data(client,
> > +				TMP421_N_CORRECT[i - 1]);
> > +		}
> >  		data->last_updated = jiffies;
> >  		data->valid = 1;
> >  	}
> > @@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
> >  		return sprintf(buf, "0\n");
> >  }
> >  
> > +static ssize_t show_n_adjust(struct device *dev,
> > +			     struct device_attribute *devattr, char *buf)
> > +{
> > +	int index = to_sensor_dev_attr(devattr)->index;
> > +	struct tmp421_data *data = tmp421_update_device(dev);
> > +
> > +	return sprintf(buf, "%d\n", data->n_adjust[index - 1]);
> > +}
> > +
> > +static ssize_t set_n_adjust(struct device *dev,
> > +			    struct device_attribute *devattr,
> > +			    const char *buf, size_t count)
> > +{
> > +	struct i2c_client *client = to_i2c_client(dev);
> > +	struct tmp421_data *data = i2c_get_clientdata(client);
> > +	int index = to_sensor_dev_attr(devattr)->index;
> > +	int n_adjust = simple_strtol(buf, NULL, 10);
> > +
> > +	mutex_lock(&data->update_lock);
> > +	i2c_smbus_write_byte_data(client, TMP421_N_CORRECT[index - 1],
> > +				  SENSORS_LIMIT(n_adjust, -128, 127));
> > +	mutex_unlock(&data->update_lock);
> > +
> > +	return count;
> > +}
> > +
> >  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
> >  				int n)
> >  {
> > @@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
> >  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
> >  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
> >  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> > +static SENSOR_DEVICE_ATTR(temp2_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > +			  show_n_adjust, set_n_adjust, 1);
> >  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
> >  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> > +static SENSOR_DEVICE_ATTR(temp3_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > +			  show_n_adjust, set_n_adjust, 2);
> >  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
> >  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> > +static SENSOR_DEVICE_ATTR(temp4_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > +			  show_n_adjust, set_n_adjust, 3);
> >  
> >  static struct attribute *tmp421_attr[] = {
> >  	&sensor_dev_attr_temp1_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp2_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> > +	&sensor_dev_attr_temp2_n_adjust.dev_attr.attr,
> >  	&sensor_dev_attr_temp3_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> > +	&sensor_dev_attr_temp3_n_adjust.dev_attr.attr,
> >  	&sensor_dev_attr_temp4_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> > +	&sensor_dev_attr_temp4_n_adjust.dev_attr.attr,
> >  	NULL
> >  };
> >  
> > -- 
> > 1.7.0.4
> > 
> > 
> > -- 
> > Jeff Angielski
> > The PTR Group
> > www.theptrgroup.com
> > 
> > _______________________________________________
> > lm-sensors mailing list
> > lm-sensors@lm-sensors.org
> > http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
> 
> _______________________________________________
> lm-sensors mailing list
> lm-sensors@lm-sensors.org
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd attempt)

[Andre Prendel]

On Thu, May 20, 2010 at 09:35:56PM +0200, Andre Prendel wrote:
> On Thu, May 20, 2010 at 03:07:05PM -0400, Jeff Angielski wrote:
> > On 05/20/2010 02:50 PM, Andre Prendel wrote:
> > > You made a careless mistake, see below. Please fix this and resend the patch
> > > again. Then I added my Acked-by and I think Jean will schedule the patch for
> > > 2.6.35 (after another review of course :)) 
> > 
> > You sir, would be correct... *sigh*  I tried using the scripts/Lindent on it to
> > be sure there were no coding style issues but that messed up even your original 
> > code.  
> 
> /scripts/checkpatch.pl is the key ;)
>  
> > In any event, here it is again:
> 
> Acked-by: Andre Prendel <andre.prendel@gmx.de>

Hi Jean,

Any News on this patch?
 
> > 
> > >From 9acd29ff48c64e58a7f5cdb888c86e737c56281c Mon Sep 17 00:00:00 2001
> > From: Jeff Angielski <jeff@theptrgroup.com>
> > Date: Mon, 10 May 2010 10:26:34 -0400
> > Subject: [PATCH] hwmon: (tmp421) Add nfactor support
> > 
> > Add support for reading and writing the n-factor correction
> > registers.  This is needed to compensate for the characteristics
> > of a particular sensor hanging off of the remote channels.
> > 
> > Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> > ---
> >  Documentation/hwmon/tmp421 |   19 +++++++++++++++++++
> >  drivers/hwmon/tmp421.c     |   41 +++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 60 insertions(+), 0 deletions(-)
> > 
> > diff --git a/Documentation/hwmon/tmp421 b/Documentation/hwmon/tmp421
> > index 0cf07f8..668228a 100644
> > --- a/Documentation/hwmon/tmp421
> > +++ b/Documentation/hwmon/tmp421
> > @@ -17,6 +17,7 @@ Supported chips:
> >  
> >  Authors:
> >  	Andre Prendel <andre.prendel@gmx.de>
> > +	Jeff Angielski <jeff@theptrgroup.com>
> >  
> >  Description
> >  -----------
> > @@ -34,3 +35,21 @@ the temperature values via the following sysfs files:
> >  
> >  temp[1-4]_input
> >  temp[2-4]_fault
> > +
> > +The chips allow the user to adjust the n-factor value that is used
> > +when converting the remote channel measurements to temperature. The
> > +adjustment has a range of -128 to +127 that yields an effective
> > +n-factor range of 0.706542 to 1.747977.  The power on reset value
> > +for the adjustment is 0 which results in an n-factor of 1.008.
> > +
> > +The effective n-factor is calculated according to the following
> > +equation:
> > +
> > +n_factor = (1.008 * 300) / (300 - nfactor_adjust)
> > +
> > +The driver exports the n-factor adjustment value via the following 
> > +sysfs files:
> > +
> > +temp[2-4]_n_adjust
> > +
> > +
> > diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> > index 738c472..dfd62be 100644
> > --- a/drivers/hwmon/tmp421.c
> > +++ b/drivers/hwmon/tmp421.c
> > @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
> >  
> >  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
> >  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> > +static const u8 TMP421_N_CORRECT[3]		= { 0x21, 0x22, 0x23 };
> >  
> >  /* Flags */
> >  #define TMP421_CONFIG_SHUTDOWN			0x40
> > @@ -76,6 +77,7 @@ struct tmp421_data {
> >  	int channels;
> >  	u8 config;
> >  	s16 temp[4];
> > +	s8 n_adjust[3];
> >  };
> >  
> >  static int temp_from_s16(s16 reg)
> > @@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
> >  			data->temp[i] |= i2c_smbus_read_byte_data(client,
> >  				TMP421_TEMP_LSB[i]);
> >  		}
> > +		for (i = 1; i < data->channels; i++) {
> > +			data->n_adjust[i - 1] = i2c_smbus_read_byte_data(client,
> > +				TMP421_N_CORRECT[i - 1]);
> > +		}
> >  		data->last_updated = jiffies;
> >  		data->valid = 1;
> >  	}
> > @@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
> >  		return sprintf(buf, "0\n");
> >  }
> >  
> > +static ssize_t show_n_adjust(struct device *dev,
> > +			     struct device_attribute *devattr, char *buf)
> > +{
> > +	int index = to_sensor_dev_attr(devattr)->index;
> > +	struct tmp421_data *data = tmp421_update_device(dev);
> > +
> > +	return sprintf(buf, "%d\n", data->n_adjust[index - 1]);
> > +}
> > +
> > +static ssize_t set_n_adjust(struct device *dev,
> > +			    struct device_attribute *devattr,
> > +			    const char *buf, size_t count)
> > +{
> > +	struct i2c_client *client = to_i2c_client(dev);
> > +	struct tmp421_data *data = i2c_get_clientdata(client);
> > +	int index = to_sensor_dev_attr(devattr)->index;
> > +	int n_adjust = simple_strtol(buf, NULL, 10);
> > +
> > +	mutex_lock(&data->update_lock);
> > +	i2c_smbus_write_byte_data(client, TMP421_N_CORRECT[index - 1],
> > +				  SENSORS_LIMIT(n_adjust, -128, 127));
> > +	mutex_unlock(&data->update_lock);
> > +
> > +	return count;
> > +}
> > +
> >  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
> >  				int n)
> >  {
> > @@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
> >  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
> >  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
> >  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> > +static SENSOR_DEVICE_ATTR(temp2_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > +			  show_n_adjust, set_n_adjust, 1);
> >  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
> >  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> > +static SENSOR_DEVICE_ATTR(temp3_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > +			  show_n_adjust, set_n_adjust, 2);
> >  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
> >  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> > +static SENSOR_DEVICE_ATTR(temp4_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > +			  show_n_adjust, set_n_adjust, 3);
> >  
> >  static struct attribute *tmp421_attr[] = {
> >  	&sensor_dev_attr_temp1_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp2_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> > +	&sensor_dev_attr_temp2_n_adjust.dev_attr.attr,
> >  	&sensor_dev_attr_temp3_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> > +	&sensor_dev_attr_temp3_n_adjust.dev_attr.attr,
> >  	&sensor_dev_attr_temp4_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> > +	&sensor_dev_attr_temp4_n_adjust.dev_attr.attr,
> >  	NULL
> >  };
> >  
> > -- 
> > 1.7.0.4
> > 
> > 
> > -- 
> > Jeff Angielski
> > The PTR Group
> > www.theptrgroup.com
> > 
> > _______________________________________________
> > lm-sensors mailing list
> > lm-sensors@lm-sensors.org
> > http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
> 
> _______________________________________________
> lm-sensors mailing list
> lm-sensors@lm-sensors.org
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd

[Andre Prendel]

On Thu, May 20, 2010 at 09:35:56PM +0200, Andre Prendel wrote:
> On Thu, May 20, 2010 at 03:07:05PM -0400, Jeff Angielski wrote:
> > In any event, here it is again:
> 
> Acked-by: Andre Prendel <andre.prendel@gmx.de>

Hi Jeff,

I'de suggest to resend the patch with my acked-by to the lm-sensors list and
Andrew Morton. It looks like Jean is too busy at the moment.

Regards,
Andre
 
> > 
> > >From 9acd29ff48c64e58a7f5cdb888c86e737c56281c Mon Sep 17 00:00:00 2001
> > From: Jeff Angielski <jeff@theptrgroup.com>
> > Date: Mon, 10 May 2010 10:26:34 -0400
> > Subject: [PATCH] hwmon: (tmp421) Add nfactor support
> > 
> > Add support for reading and writing the n-factor correction
> > registers.  This is needed to compensate for the characteristics
> > of a particular sensor hanging off of the remote channels.
> > 
> > Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> > ---
> >  Documentation/hwmon/tmp421 |   19 +++++++++++++++++++
> >  drivers/hwmon/tmp421.c     |   41 +++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 60 insertions(+), 0 deletions(-)
> > 
> > diff --git a/Documentation/hwmon/tmp421 b/Documentation/hwmon/tmp421
> > index 0cf07f8..668228a 100644
> > --- a/Documentation/hwmon/tmp421
> > +++ b/Documentation/hwmon/tmp421
> > @@ -17,6 +17,7 @@ Supported chips:
> >  
> >  Authors:
> >  	Andre Prendel <andre.prendel@gmx.de>
> > +	Jeff Angielski <jeff@theptrgroup.com>
> >  
> >  Description
> >  -----------
> > @@ -34,3 +35,21 @@ the temperature values via the following sysfs files:
> >  
> >  temp[1-4]_input
> >  temp[2-4]_fault
> > +
> > +The chips allow the user to adjust the n-factor value that is used
> > +when converting the remote channel measurements to temperature. The
> > +adjustment has a range of -128 to +127 that yields an effective
> > +n-factor range of 0.706542 to 1.747977.  The power on reset value
> > +for the adjustment is 0 which results in an n-factor of 1.008.
> > +
> > +The effective n-factor is calculated according to the following
> > +equation:
> > +
> > +n_factor = (1.008 * 300) / (300 - nfactor_adjust)
> > +
> > +The driver exports the n-factor adjustment value via the following 
> > +sysfs files:
> > +
> > +temp[2-4]_n_adjust
> > +
> > +
> > diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> > index 738c472..dfd62be 100644
> > --- a/drivers/hwmon/tmp421.c
> > +++ b/drivers/hwmon/tmp421.c
> > @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
> >  
> >  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
> >  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> > +static const u8 TMP421_N_CORRECT[3]		= { 0x21, 0x22, 0x23 };
> >  
> >  /* Flags */
> >  #define TMP421_CONFIG_SHUTDOWN			0x40
> > @@ -76,6 +77,7 @@ struct tmp421_data {
> >  	int channels;
> >  	u8 config;
> >  	s16 temp[4];
> > +	s8 n_adjust[3];
> >  };
> >  
> >  static int temp_from_s16(s16 reg)
> > @@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
> >  			data->temp[i] |= i2c_smbus_read_byte_data(client,
> >  				TMP421_TEMP_LSB[i]);
> >  		}
> > +		for (i = 1; i < data->channels; i++) {
> > +			data->n_adjust[i - 1] = i2c_smbus_read_byte_data(client,
> > +				TMP421_N_CORRECT[i - 1]);
> > +		}
> >  		data->last_updated = jiffies;
> >  		data->valid = 1;
> >  	}
> > @@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
> >  		return sprintf(buf, "0\n");
> >  }
> >  
> > +static ssize_t show_n_adjust(struct device *dev,
> > +			     struct device_attribute *devattr, char *buf)
> > +{
> > +	int index = to_sensor_dev_attr(devattr)->index;
> > +	struct tmp421_data *data = tmp421_update_device(dev);
> > +
> > +	return sprintf(buf, "%d\n", data->n_adjust[index - 1]);
> > +}
> > +
> > +static ssize_t set_n_adjust(struct device *dev,
> > +			    struct device_attribute *devattr,
> > +			    const char *buf, size_t count)
> > +{
> > +	struct i2c_client *client = to_i2c_client(dev);
> > +	struct tmp421_data *data = i2c_get_clientdata(client);
> > +	int index = to_sensor_dev_attr(devattr)->index;
> > +	int n_adjust = simple_strtol(buf, NULL, 10);
> > +
> > +	mutex_lock(&data->update_lock);
> > +	i2c_smbus_write_byte_data(client, TMP421_N_CORRECT[index - 1],
> > +				  SENSORS_LIMIT(n_adjust, -128, 127));
> > +	mutex_unlock(&data->update_lock);
> > +
> > +	return count;
> > +}
> > +
> >  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
> >  				int n)
> >  {
> > @@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
> >  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
> >  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
> >  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> > +static SENSOR_DEVICE_ATTR(temp2_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > +			  show_n_adjust, set_n_adjust, 1);
> >  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
> >  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> > +static SENSOR_DEVICE_ATTR(temp3_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > +			  show_n_adjust, set_n_adjust, 2);
> >  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
> >  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> > +static SENSOR_DEVICE_ATTR(temp4_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > +			  show_n_adjust, set_n_adjust, 3);
> >  
> >  static struct attribute *tmp421_attr[] = {
> >  	&sensor_dev_attr_temp1_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp2_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> > +	&sensor_dev_attr_temp2_n_adjust.dev_attr.attr,
> >  	&sensor_dev_attr_temp3_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> > +	&sensor_dev_attr_temp3_n_adjust.dev_attr.attr,
> >  	&sensor_dev_attr_temp4_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> > +	&sensor_dev_attr_temp4_n_adjust.dev_attr.attr,
> >  	NULL
> >  };
> >  
> > -- 
> > 1.7.0.4
> > 
> > 
> > -- 
> > Jeff Angielski
> > The PTR Group
> > www.theptrgroup.com
> > 
> > _______________________________________________
> > lm-sensors mailing list
> > lm-sensors@lm-sensors.org
> > http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
> 
> _______________________________________________
> lm-sensors mailing list
> lm-sensors@lm-sensors.org
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd attempt)

[Andre Prendel]

On Thu, May 20, 2010 at 09:35:56PM +0200, Andre Prendel wrote:
> On Thu, May 20, 2010 at 03:07:05PM -0400, Jeff Angielski wrote:
> > In any event, here it is again:
> 
> Acked-by: Andre Prendel <andre.prendel@gmx.de>

Hi Jeff,

I'de suggest to resend the patch with my acked-by to the lm-sensors list and
Andrew Morton. It looks like Jean is too busy at the moment.

Regards,
Andre
 
> > 
> > >From 9acd29ff48c64e58a7f5cdb888c86e737c56281c Mon Sep 17 00:00:00 2001
> > From: Jeff Angielski <jeff@theptrgroup.com>
> > Date: Mon, 10 May 2010 10:26:34 -0400
> > Subject: [PATCH] hwmon: (tmp421) Add nfactor support
> > 
> > Add support for reading and writing the n-factor correction
> > registers.  This is needed to compensate for the characteristics
> > of a particular sensor hanging off of the remote channels.
> > 
> > Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> > ---
> >  Documentation/hwmon/tmp421 |   19 +++++++++++++++++++
> >  drivers/hwmon/tmp421.c     |   41 +++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 60 insertions(+), 0 deletions(-)
> > 
> > diff --git a/Documentation/hwmon/tmp421 b/Documentation/hwmon/tmp421
> > index 0cf07f8..668228a 100644
> > --- a/Documentation/hwmon/tmp421
> > +++ b/Documentation/hwmon/tmp421
> > @@ -17,6 +17,7 @@ Supported chips:
> >  
> >  Authors:
> >  	Andre Prendel <andre.prendel@gmx.de>
> > +	Jeff Angielski <jeff@theptrgroup.com>
> >  
> >  Description
> >  -----------
> > @@ -34,3 +35,21 @@ the temperature values via the following sysfs files:
> >  
> >  temp[1-4]_input
> >  temp[2-4]_fault
> > +
> > +The chips allow the user to adjust the n-factor value that is used
> > +when converting the remote channel measurements to temperature. The
> > +adjustment has a range of -128 to +127 that yields an effective
> > +n-factor range of 0.706542 to 1.747977.  The power on reset value
> > +for the adjustment is 0 which results in an n-factor of 1.008.
> > +
> > +The effective n-factor is calculated according to the following
> > +equation:
> > +
> > +n_factor = (1.008 * 300) / (300 - nfactor_adjust)
> > +
> > +The driver exports the n-factor adjustment value via the following 
> > +sysfs files:
> > +
> > +temp[2-4]_n_adjust
> > +
> > +
> > diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> > index 738c472..dfd62be 100644
> > --- a/drivers/hwmon/tmp421.c
> > +++ b/drivers/hwmon/tmp421.c
> > @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
> >  
> >  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
> >  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> > +static const u8 TMP421_N_CORRECT[3]		= { 0x21, 0x22, 0x23 };
> >  
> >  /* Flags */
> >  #define TMP421_CONFIG_SHUTDOWN			0x40
> > @@ -76,6 +77,7 @@ struct tmp421_data {
> >  	int channels;
> >  	u8 config;
> >  	s16 temp[4];
> > +	s8 n_adjust[3];
> >  };
> >  
> >  static int temp_from_s16(s16 reg)
> > @@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
> >  			data->temp[i] |= i2c_smbus_read_byte_data(client,
> >  				TMP421_TEMP_LSB[i]);
> >  		}
> > +		for (i = 1; i < data->channels; i++) {
> > +			data->n_adjust[i - 1] = i2c_smbus_read_byte_data(client,
> > +				TMP421_N_CORRECT[i - 1]);
> > +		}
> >  		data->last_updated = jiffies;
> >  		data->valid = 1;
> >  	}
> > @@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
> >  		return sprintf(buf, "0\n");
> >  }
> >  
> > +static ssize_t show_n_adjust(struct device *dev,
> > +			     struct device_attribute *devattr, char *buf)
> > +{
> > +	int index = to_sensor_dev_attr(devattr)->index;
> > +	struct tmp421_data *data = tmp421_update_device(dev);
> > +
> > +	return sprintf(buf, "%d\n", data->n_adjust[index - 1]);
> > +}
> > +
> > +static ssize_t set_n_adjust(struct device *dev,
> > +			    struct device_attribute *devattr,
> > +			    const char *buf, size_t count)
> > +{
> > +	struct i2c_client *client = to_i2c_client(dev);
> > +	struct tmp421_data *data = i2c_get_clientdata(client);
> > +	int index = to_sensor_dev_attr(devattr)->index;
> > +	int n_adjust = simple_strtol(buf, NULL, 10);
> > +
> > +	mutex_lock(&data->update_lock);
> > +	i2c_smbus_write_byte_data(client, TMP421_N_CORRECT[index - 1],
> > +				  SENSORS_LIMIT(n_adjust, -128, 127));
> > +	mutex_unlock(&data->update_lock);
> > +
> > +	return count;
> > +}
> > +
> >  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
> >  				int n)
> >  {
> > @@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
> >  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
> >  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
> >  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> > +static SENSOR_DEVICE_ATTR(temp2_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > +			  show_n_adjust, set_n_adjust, 1);
> >  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
> >  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> > +static SENSOR_DEVICE_ATTR(temp3_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > +			  show_n_adjust, set_n_adjust, 2);
> >  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
> >  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> > +static SENSOR_DEVICE_ATTR(temp4_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > +			  show_n_adjust, set_n_adjust, 3);
> >  
> >  static struct attribute *tmp421_attr[] = {
> >  	&sensor_dev_attr_temp1_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp2_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> > +	&sensor_dev_attr_temp2_n_adjust.dev_attr.attr,
> >  	&sensor_dev_attr_temp3_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> > +	&sensor_dev_attr_temp3_n_adjust.dev_attr.attr,
> >  	&sensor_dev_attr_temp4_input.dev_attr.attr,
> >  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> > +	&sensor_dev_attr_temp4_n_adjust.dev_attr.attr,
> >  	NULL
> >  };
> >  
> > -- 
> > 1.7.0.4
> > 
> > 
> > -- 
> > Jeff Angielski
> > The PTR Group
> > www.theptrgroup.com
> > 
> > _______________________________________________
> > lm-sensors mailing list
> > lm-sensors@lm-sensors.org
> > http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
> 
> _______________________________________________
> lm-sensors mailing list
> lm-sensors@lm-sensors.org
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd

[Guenter Roeck]

On Tue, Jul 20, 2010 at 11:09:53AM -0400, Andre Prendel wrote:
> On Thu, May 20, 2010 at 09:35:56PM +0200, Andre Prendel wrote:
> > On Thu, May 20, 2010 at 03:07:05PM -0400, Jeff Angielski wrote:
> > > In any event, here it is again:
> > 
> > Acked-by: Andre Prendel <andre.prendel@gmx.de>
> 
> Hi Jeff,
> 
> I'de suggest to resend the patch with my acked-by to the lm-sensors list and
> Andrew Morton. It looks like Jean is too busy at the moment.
> 
> Regards,
> Andre
>  
> > > >From 9acd29ff48c64e58a7f5cdb888c86e737c56281c Mon Sep 17 00:00:00 2001
> > > From: Jeff Angielski <jeff@theptrgroup.com>
> > > Date: Mon, 10 May 2010 10:26:34 -0400
> > > Subject: [PATCH] hwmon: (tmp421) Add nfactor support
> > > 
> > > Add support for reading and writing the n-factor correction
> > > registers.  This is needed to compensate for the characteristics
> > > of a particular sensor hanging off of the remote channels.
> > > 

My concerns with this approach are 

1) It changes the sysfs abi. libsensors won't support it. It opens up
   a can of worms with everyone starting to put chip-specific extensions
   into the ABI. If such an extension has to be made, it should be a) really necessary
   and b) a generic extension which applies to all chips.
2) My understanding is that value adjustments are supposed to be made via sensors.conf,
   and that values reported by the chip should always be 'raw', ie unadjusted
   values.

Instead of exporting n_adjust to the user via sysfs, it might make more sense 
to reset the correction factor to its default (if it was changed), and handle
required adjustments in sensors.conf.

Even if Jean doesn't have time to handle the patch, you should at least get his Ack
for the ABI changes.

Guenter

> > > 
> > > Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> > > ---
> > >  Documentation/hwmon/tmp421 |   19 +++++++++++++++++++
> > >  drivers/hwmon/tmp421.c     |   41 +++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 60 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/Documentation/hwmon/tmp421 b/Documentation/hwmon/tmp421
> > > index 0cf07f8..668228a 100644
> > > --- a/Documentation/hwmon/tmp421
> > > +++ b/Documentation/hwmon/tmp421
> > > @@ -17,6 +17,7 @@ Supported chips:
> > >  
> > >  Authors:
> > >  	Andre Prendel <andre.prendel@gmx.de>
> > > +	Jeff Angielski <jeff@theptrgroup.com>
> > >  
> > >  Description
> > >  -----------
> > > @@ -34,3 +35,21 @@ the temperature values via the following sysfs files:
> > >  
> > >  temp[1-4]_input
> > >  temp[2-4]_fault
> > > +
> > > +The chips allow the user to adjust the n-factor value that is used
> > > +when converting the remote channel measurements to temperature. The
> > > +adjustment has a range of -128 to +127 that yields an effective
> > > +n-factor range of 0.706542 to 1.747977.  The power on reset value
> > > +for the adjustment is 0 which results in an n-factor of 1.008.
> > > +
> > > +The effective n-factor is calculated according to the following
> > > +equation:
> > > +
> > > +n_factor = (1.008 * 300) / (300 - nfactor_adjust)
> > > +
> > > +The driver exports the n-factor adjustment value via the following 
> > > +sysfs files:
> > > +
> > > +temp[2-4]_n_adjust
> > > +
> > > +
> > > diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> > > index 738c472..dfd62be 100644
> > > --- a/drivers/hwmon/tmp421.c
> > > +++ b/drivers/hwmon/tmp421.c
> > > @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
> > >  
> > >  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
> > >  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> > > +static const u8 TMP421_N_CORRECT[3]		= { 0x21, 0x22, 0x23 };
> > >  
> > >  /* Flags */
> > >  #define TMP421_CONFIG_SHUTDOWN			0x40
> > > @@ -76,6 +77,7 @@ struct tmp421_data {
> > >  	int channels;
> > >  	u8 config;
> > >  	s16 temp[4];
> > > +	s8 n_adjust[3];
> > >  };
> > >  
> > >  static int temp_from_s16(s16 reg)
> > > @@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
> > >  			data->temp[i] |= i2c_smbus_read_byte_data(client,
> > >  				TMP421_TEMP_LSB[i]);
> > >  		}
> > > +		for (i = 1; i < data->channels; i++) {
> > > +			data->n_adjust[i - 1] = i2c_smbus_read_byte_data(client,
> > > +				TMP421_N_CORRECT[i - 1]);
> > > +		}
> > >  		data->last_updated = jiffies;
> > >  		data->valid = 1;
> > >  	}
> > > @@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
> > >  		return sprintf(buf, "0\n");
> > >  }
> > >  
> > > +static ssize_t show_n_adjust(struct device *dev,
> > > +			     struct device_attribute *devattr, char *buf)
> > > +{
> > > +	int index = to_sensor_dev_attr(devattr)->index;
> > > +	struct tmp421_data *data = tmp421_update_device(dev);
> > > +
> > > +	return sprintf(buf, "%d\n", data->n_adjust[index - 1]);
> > > +}
> > > +
> > > +static ssize_t set_n_adjust(struct device *dev,
> > > +			    struct device_attribute *devattr,
> > > +			    const char *buf, size_t count)
> > > +{
> > > +	struct i2c_client *client = to_i2c_client(dev);
> > > +	struct tmp421_data *data = i2c_get_clientdata(client);
> > > +	int index = to_sensor_dev_attr(devattr)->index;
> > > +	int n_adjust = simple_strtol(buf, NULL, 10);
> > > +
> > > +	mutex_lock(&data->update_lock);
> > > +	i2c_smbus_write_byte_data(client, TMP421_N_CORRECT[index - 1],
> > > +				  SENSORS_LIMIT(n_adjust, -128, 127));
> > > +	mutex_unlock(&data->update_lock);
> > > +
> > > +	return count;
> > > +}
> > > +
> > >  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
> > >  				int n)
> > >  {
> > > @@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
> > >  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
> > >  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
> > >  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> > > +static SENSOR_DEVICE_ATTR(temp2_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > > +			  show_n_adjust, set_n_adjust, 1);
> > >  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
> > >  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> > > +static SENSOR_DEVICE_ATTR(temp3_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > > +			  show_n_adjust, set_n_adjust, 2);
> > >  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
> > >  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> > > +static SENSOR_DEVICE_ATTR(temp4_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > > +			  show_n_adjust, set_n_adjust, 3);
> > >  
> > >  static struct attribute *tmp421_attr[] = {
> > >  	&sensor_dev_attr_temp1_input.dev_attr.attr,
> > >  	&sensor_dev_attr_temp2_input.dev_attr.attr,
> > >  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> > > +	&sensor_dev_attr_temp2_n_adjust.dev_attr.attr,
> > >  	&sensor_dev_attr_temp3_input.dev_attr.attr,
> > >  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> > > +	&sensor_dev_attr_temp3_n_adjust.dev_attr.attr,
> > >  	&sensor_dev_attr_temp4_input.dev_attr.attr,
> > >  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> > > +	&sensor_dev_attr_temp4_n_adjust.dev_attr.attr,
> > >  	NULL
> > >  };
> > >  
> > > -- 
> > > 1.7.0.4
> > > 
> > > 
> > > -- 
> > > Jeff Angielski
> > > The PTR Group
> > > www.theptrgroup.com
> > > 
> > > _______________________________________________
> > > lm-sensors mailing list
> > > lm-sensors@lm-sensors.org
> > > http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
> > 
> > _______________________________________________
> > lm-sensors mailing list
> > lm-sensors@lm-sensors.org
> > http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
> 
> _______________________________________________
> lm-sensors mailing list
> lm-sensors@lm-sensors.org
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd

[Andre Prendel]

On Tue, Jul 20, 2010 at 08:59:52AM -0700, Guenter Roeck wrote:
> On Tue, Jul 20, 2010 at 11:09:53AM -0400, Andre Prendel wrote:
> > On Thu, May 20, 2010 at 09:35:56PM +0200, Andre Prendel wrote:
> > > On Thu, May 20, 2010 at 03:07:05PM -0400, Jeff Angielski wrote:
> > > > In any event, here it is again:
> > > 
> > > Acked-by: Andre Prendel <andre.prendel@gmx.de>
> > 
> > Hi Jeff,
> > 
> > I'de suggest to resend the patch with my acked-by to the lm-sensors list and
> > Andrew Morton. It looks like Jean is too busy at the moment.
> > 
> > Regards,
> > Andre
> >  
> > > > >From 9acd29ff48c64e58a7f5cdb888c86e737c56281c Mon Sep 17 00:00:00 2001
> > > > From: Jeff Angielski <jeff@theptrgroup.com>
> > > > Date: Mon, 10 May 2010 10:26:34 -0400
> > > > Subject: [PATCH] hwmon: (tmp421) Add nfactor support
> > > > 
> > > > Add support for reading and writing the n-factor correction
> > > > registers.  This is needed to compensate for the characteristics
> > > > of a particular sensor hanging off of the remote channels.
> > > > 
> 
> My concerns with this approach are 
> 
> 1) It changes the sysfs abi. libsensors won't support it. It opens up
>    a can of worms with everyone starting to put chip-specific extensions
>    into the ABI. If such an extension has to be made, it should be a) really necessary
>    and b) a generic extension which applies to all chips.

A chip-specific extension can't be also generic. So we have to decide whether weaccept chip-specific extensions or not.

> 2) My understanding is that value adjustments are supposed to be made via sensors.conf,
>    and that values reported by the chip should always be 'raw', ie unadjusted
>    values.
> 
> Instead of exporting n_adjust to the user via sysfs, it might make more sense 
> to reset the correction factor to its default (if it was changed), and handle
> required adjustments in sensors.conf.

Jeff, what do you think?
 
> Even if Jean doesn't have time to handle the patch, you should at least get his Ack
> for the ABI changes.

That was the intention of resending the patch to the lm-sensors list. It would
be a pity to lose Jeff's effort.
 
> Guenter

Regards,
Andre
 
> > > > 
> > > > Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> > > > ---
> > > >  Documentation/hwmon/tmp421 |   19 +++++++++++++++++++
> > > >  drivers/hwmon/tmp421.c     |   41 +++++++++++++++++++++++++++++++++++++++++
> > > >  2 files changed, 60 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/Documentation/hwmon/tmp421 b/Documentation/hwmon/tmp421
> > > > index 0cf07f8..668228a 100644
> > > > --- a/Documentation/hwmon/tmp421
> > > > +++ b/Documentation/hwmon/tmp421
> > > > @@ -17,6 +17,7 @@ Supported chips:
> > > >  
> > > >  Authors:
> > > >  	Andre Prendel <andre.prendel@gmx.de>
> > > > +	Jeff Angielski <jeff@theptrgroup.com>
> > > >  
> > > >  Description
> > > >  -----------
> > > > @@ -34,3 +35,21 @@ the temperature values via the following sysfs files:
> > > >  
> > > >  temp[1-4]_input
> > > >  temp[2-4]_fault
> > > > +
> > > > +The chips allow the user to adjust the n-factor value that is used
> > > > +when converting the remote channel measurements to temperature. The
> > > > +adjustment has a range of -128 to +127 that yields an effective
> > > > +n-factor range of 0.706542 to 1.747977.  The power on reset value
> > > > +for the adjustment is 0 which results in an n-factor of 1.008.
> > > > +
> > > > +The effective n-factor is calculated according to the following
> > > > +equation:
> > > > +
> > > > +n_factor = (1.008 * 300) / (300 - nfactor_adjust)
> > > > +
> > > > +The driver exports the n-factor adjustment value via the following 
> > > > +sysfs files:
> > > > +
> > > > +temp[2-4]_n_adjust
> > > > +
> > > > +
> > > > diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> > > > index 738c472..dfd62be 100644
> > > > --- a/drivers/hwmon/tmp421.c
> > > > +++ b/drivers/hwmon/tmp421.c
> > > > @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
> > > >  
> > > >  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
> > > >  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> > > > +static const u8 TMP421_N_CORRECT[3]		= { 0x21, 0x22, 0x23 };
> > > >  
> > > >  /* Flags */
> > > >  #define TMP421_CONFIG_SHUTDOWN			0x40
> > > > @@ -76,6 +77,7 @@ struct tmp421_data {
> > > >  	int channels;
> > > >  	u8 config;
> > > >  	s16 temp[4];
> > > > +	s8 n_adjust[3];
> > > >  };
> > > >  
> > > >  static int temp_from_s16(s16 reg)
> > > > @@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
> > > >  			data->temp[i] |= i2c_smbus_read_byte_data(client,
> > > >  				TMP421_TEMP_LSB[i]);
> > > >  		}
> > > > +		for (i = 1; i < data->channels; i++) {
> > > > +			data->n_adjust[i - 1] = i2c_smbus_read_byte_data(client,
> > > > +				TMP421_N_CORRECT[i - 1]);
> > > > +		}
> > > >  		data->last_updated = jiffies;
> > > >  		data->valid = 1;
> > > >  	}
> > > > @@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
> > > >  		return sprintf(buf, "0\n");
> > > >  }
> > > >  
> > > > +static ssize_t show_n_adjust(struct device *dev,
> > > > +			     struct device_attribute *devattr, char *buf)
> > > > +{
> > > > +	int index = to_sensor_dev_attr(devattr)->index;
> > > > +	struct tmp421_data *data = tmp421_update_device(dev);
> > > > +
> > > > +	return sprintf(buf, "%d\n", data->n_adjust[index - 1]);
> > > > +}
> > > > +
> > > > +static ssize_t set_n_adjust(struct device *dev,
> > > > +			    struct device_attribute *devattr,
> > > > +			    const char *buf, size_t count)
> > > > +{
> > > > +	struct i2c_client *client = to_i2c_client(dev);
> > > > +	struct tmp421_data *data = i2c_get_clientdata(client);
> > > > +	int index = to_sensor_dev_attr(devattr)->index;
> > > > +	int n_adjust = simple_strtol(buf, NULL, 10);
> > > > +
> > > > +	mutex_lock(&data->update_lock);
> > > > +	i2c_smbus_write_byte_data(client, TMP421_N_CORRECT[index - 1],
> > > > +				  SENSORS_LIMIT(n_adjust, -128, 127));
> > > > +	mutex_unlock(&data->update_lock);
> > > > +
> > > > +	return count;
> > > > +}
> > > > +
> > > >  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
> > > >  				int n)
> > > >  {
> > > > @@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
> > > >  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
> > > >  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
> > > >  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> > > > +static SENSOR_DEVICE_ATTR(temp2_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > > > +			  show_n_adjust, set_n_adjust, 1);
> > > >  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
> > > >  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> > > > +static SENSOR_DEVICE_ATTR(temp3_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > > > +			  show_n_adjust, set_n_adjust, 2);
> > > >  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
> > > >  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> > > > +static SENSOR_DEVICE_ATTR(temp4_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > > > +			  show_n_adjust, set_n_adjust, 3);
> > > >  
> > > >  static struct attribute *tmp421_attr[] = {
> > > >  	&sensor_dev_attr_temp1_input.dev_attr.attr,
> > > >  	&sensor_dev_attr_temp2_input.dev_attr.attr,
> > > >  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> > > > +	&sensor_dev_attr_temp2_n_adjust.dev_attr.attr,
> > > >  	&sensor_dev_attr_temp3_input.dev_attr.attr,
> > > >  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> > > > +	&sensor_dev_attr_temp3_n_adjust.dev_attr.attr,
> > > >  	&sensor_dev_attr_temp4_input.dev_attr.attr,
> > > >  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> > > > +	&sensor_dev_attr_temp4_n_adjust.dev_attr.attr,
> > > >  	NULL
> > > >  };
> > > >  
> > > > -- 
> > > > 1.7.0.4
> > > > 
> > > > 
> > > > -- 
> > > > Jeff Angielski
> > > > The PTR Group
> > > > www.theptrgroup.com
> > > > 
> > > > _______________________________________________
> > > > lm-sensors mailing list
> > > > lm-sensors@lm-sensors.org
> > > > http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
> > > 
> > > _______________________________________________
> > > lm-sensors mailing list
> > > lm-sensors@lm-sensors.org
> > > http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
> > 
> > _______________________________________________
> > lm-sensors mailing list
> > lm-sensors@lm-sensors.org
> > http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd attempt)

[Andre Prendel]

On Tue, Jul 20, 2010 at 08:59:52AM -0700, Guenter Roeck wrote:
> On Tue, Jul 20, 2010 at 11:09:53AM -0400, Andre Prendel wrote:
> > On Thu, May 20, 2010 at 09:35:56PM +0200, Andre Prendel wrote:
> > > On Thu, May 20, 2010 at 03:07:05PM -0400, Jeff Angielski wrote:
> > > > In any event, here it is again:
> > > 
> > > Acked-by: Andre Prendel <andre.prendel@gmx.de>
> > 
> > Hi Jeff,
> > 
> > I'de suggest to resend the patch with my acked-by to the lm-sensors list and
> > Andrew Morton. It looks like Jean is too busy at the moment.
> > 
> > Regards,
> > Andre
> >  
> > > > >From 9acd29ff48c64e58a7f5cdb888c86e737c56281c Mon Sep 17 00:00:00 2001
> > > > From: Jeff Angielski <jeff@theptrgroup.com>
> > > > Date: Mon, 10 May 2010 10:26:34 -0400
> > > > Subject: [PATCH] hwmon: (tmp421) Add nfactor support
> > > > 
> > > > Add support for reading and writing the n-factor correction
> > > > registers.  This is needed to compensate for the characteristics
> > > > of a particular sensor hanging off of the remote channels.
> > > > 
> 
> My concerns with this approach are 
> 
> 1) It changes the sysfs abi. libsensors won't support it. It opens up
>    a can of worms with everyone starting to put chip-specific extensions
>    into the ABI. If such an extension has to be made, it should be a) really necessary
>    and b) a generic extension which applies to all chips.

A chip-specific extension can't be also generic. So we have to decide whether weaccept chip-specific extensions or not.

> 2) My understanding is that value adjustments are supposed to be made via sensors.conf,
>    and that values reported by the chip should always be 'raw', ie unadjusted
>    values.
> 
> Instead of exporting n_adjust to the user via sysfs, it might make more sense 
> to reset the correction factor to its default (if it was changed), and handle
> required adjustments in sensors.conf.

Jeff, what do you think?
 
> Even if Jean doesn't have time to handle the patch, you should at least get his Ack
> for the ABI changes.

That was the intention of resending the patch to the lm-sensors list. It would
be a pity to lose Jeff's effort.
 
> Guenter

Regards,
Andre
 
> > > > 
> > > > Signed-off-by: Jeff Angielski <jeff@theptrgroup.com>
> > > > ---
> > > >  Documentation/hwmon/tmp421 |   19 +++++++++++++++++++
> > > >  drivers/hwmon/tmp421.c     |   41 +++++++++++++++++++++++++++++++++++++++++
> > > >  2 files changed, 60 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/Documentation/hwmon/tmp421 b/Documentation/hwmon/tmp421
> > > > index 0cf07f8..668228a 100644
> > > > --- a/Documentation/hwmon/tmp421
> > > > +++ b/Documentation/hwmon/tmp421
> > > > @@ -17,6 +17,7 @@ Supported chips:
> > > >  
> > > >  Authors:
> > > >  	Andre Prendel <andre.prendel@gmx.de>
> > > > +	Jeff Angielski <jeff@theptrgroup.com>
> > > >  
> > > >  Description
> > > >  -----------
> > > > @@ -34,3 +35,21 @@ the temperature values via the following sysfs files:
> > > >  
> > > >  temp[1-4]_input
> > > >  temp[2-4]_fault
> > > > +
> > > > +The chips allow the user to adjust the n-factor value that is used
> > > > +when converting the remote channel measurements to temperature. The
> > > > +adjustment has a range of -128 to +127 that yields an effective
> > > > +n-factor range of 0.706542 to 1.747977.  The power on reset value
> > > > +for the adjustment is 0 which results in an n-factor of 1.008.
> > > > +
> > > > +The effective n-factor is calculated according to the following
> > > > +equation:
> > > > +
> > > > +n_factor = (1.008 * 300) / (300 - nfactor_adjust)
> > > > +
> > > > +The driver exports the n-factor adjustment value via the following 
> > > > +sysfs files:
> > > > +
> > > > +temp[2-4]_n_adjust
> > > > +
> > > > +
> > > > diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> > > > index 738c472..dfd62be 100644
> > > > --- a/drivers/hwmon/tmp421.c
> > > > +++ b/drivers/hwmon/tmp421.c
> > > > @@ -49,6 +49,7 @@ enum chips { tmp421, tmp422, tmp423 };
> > > >  
> > > >  static const u8 TMP421_TEMP_MSB[4]		= { 0x00, 0x01, 0x02, 0x03 };
> > > >  static const u8 TMP421_TEMP_LSB[4]		= { 0x10, 0x11, 0x12, 0x13 };
> > > > +static const u8 TMP421_N_CORRECT[3]		= { 0x21, 0x22, 0x23 };
> > > >  
> > > >  /* Flags */
> > > >  #define TMP421_CONFIG_SHUTDOWN			0x40
> > > > @@ -76,6 +77,7 @@ struct tmp421_data {
> > > >  	int channels;
> > > >  	u8 config;
> > > >  	s16 temp[4];
> > > > +	s8 n_adjust[3];
> > > >  };
> > > >  
> > > >  static int temp_from_s16(s16 reg)
> > > > @@ -115,6 +117,10 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
> > > >  			data->temp[i] |= i2c_smbus_read_byte_data(client,
> > > >  				TMP421_TEMP_LSB[i]);
> > > >  		}
> > > > +		for (i = 1; i < data->channels; i++) {
> > > > +			data->n_adjust[i - 1] = i2c_smbus_read_byte_data(client,
> > > > +				TMP421_N_CORRECT[i - 1]);
> > > > +		}
> > > >  		data->last_updated = jiffies;
> > > >  		data->valid = 1;
> > > >  	}
> > > > @@ -157,6 +163,32 @@ static ssize_t show_fault(struct device *dev,
> > > >  		return sprintf(buf, "0\n");
> > > >  }
> > > >  
> > > > +static ssize_t show_n_adjust(struct device *dev,
> > > > +			     struct device_attribute *devattr, char *buf)
> > > > +{
> > > > +	int index = to_sensor_dev_attr(devattr)->index;
> > > > +	struct tmp421_data *data = tmp421_update_device(dev);
> > > > +
> > > > +	return sprintf(buf, "%d\n", data->n_adjust[index - 1]);
> > > > +}
> > > > +
> > > > +static ssize_t set_n_adjust(struct device *dev,
> > > > +			    struct device_attribute *devattr,
> > > > +			    const char *buf, size_t count)
> > > > +{
> > > > +	struct i2c_client *client = to_i2c_client(dev);
> > > > +	struct tmp421_data *data = i2c_get_clientdata(client);
> > > > +	int index = to_sensor_dev_attr(devattr)->index;
> > > > +	int n_adjust = simple_strtol(buf, NULL, 10);
> > > > +
> > > > +	mutex_lock(&data->update_lock);
> > > > +	i2c_smbus_write_byte_data(client, TMP421_N_CORRECT[index - 1],
> > > > +				  SENSORS_LIMIT(n_adjust, -128, 127));
> > > > +	mutex_unlock(&data->update_lock);
> > > > +
> > > > +	return count;
> > > > +}
> > > > +
> > > >  static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
> > > >  				int n)
> > > >  {
> > > > @@ -177,19 +209,28 @@ static mode_t tmp421_is_visible(struct kobject *kobj, struct attribute *a,
> > > >  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
> > > >  static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
> > > >  static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, 1);
> > > > +static SENSOR_DEVICE_ATTR(temp2_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > > > +			  show_n_adjust, set_n_adjust, 1);
> > > >  static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_value, NULL, 2);
> > > >  static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, 2);
> > > > +static SENSOR_DEVICE_ATTR(temp3_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > > > +			  show_n_adjust, set_n_adjust, 2);
> > > >  static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_value, NULL, 3);
> > > >  static SENSOR_DEVICE_ATTR(temp4_fault, S_IRUGO, show_fault, NULL, 3);
> > > > +static SENSOR_DEVICE_ATTR(temp4_n_adjust, S_IRUSR | S_IWUSR | S_IRGRP,
> > > > +			  show_n_adjust, set_n_adjust, 3);
> > > >  
> > > >  static struct attribute *tmp421_attr[] = {
> > > >  	&sensor_dev_attr_temp1_input.dev_attr.attr,
> > > >  	&sensor_dev_attr_temp2_input.dev_attr.attr,
> > > >  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> > > > +	&sensor_dev_attr_temp2_n_adjust.dev_attr.attr,
> > > >  	&sensor_dev_attr_temp3_input.dev_attr.attr,
> > > >  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> > > > +	&sensor_dev_attr_temp3_n_adjust.dev_attr.attr,
> > > >  	&sensor_dev_attr_temp4_input.dev_attr.attr,
> > > >  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> > > > +	&sensor_dev_attr_temp4_n_adjust.dev_attr.attr,
> > > >  	NULL
> > > >  };
> > > >  
> > > > -- 
> > > > 1.7.0.4
> > > > 
> > > > 
> > > > -- 
> > > > Jeff Angielski
> > > > The PTR Group
> > > > www.theptrgroup.com
> > > > 
> > > > _______________________________________________
> > > > lm-sensors mailing list
> > > > lm-sensors@lm-sensors.org
> > > > http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
> > > 
> > > _______________________________________________
> > > lm-sensors mailing list
> > > lm-sensors@lm-sensors.org
> > > http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
> > 
> > _______________________________________________
> > lm-sensors mailing list
> > lm-sensors@lm-sensors.org
> > http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd

[Jean Delvare]

Hi Andre, Guenter, Jeff,

On Wed, 21 Jul 2010 21:46:50 +0200, Andre Prendel wrote:
> On Tue, Jul 20, 2010 at 08:59:52AM -0700, Guenter Roeck wrote:
> > On Tue, Jul 20, 2010 at 11:09:53AM -0400, Andre Prendel wrote:
> > > On Thu, May 20, 2010 at 09:35:56PM +0200, Andre Prendel wrote:
> > > > On Thu, May 20, 2010 at 03:07:05PM -0400, Jeff Angielski wrote:
> > > > > In any event, here it is again:
> > > > 
> > > > Acked-by: Andre Prendel <andre.prendel@gmx.de>
> > > 
> > > Hi Jeff,
> > > 
> > > I'de suggest to resend the patch with my acked-by to the lm-sensors list and
> > > Andrew Morton. It looks like Jean is too busy at the moment.
> > > 
> > > Regards,
> > > Andre
> > >  
> > > > > >From 9acd29ff48c64e58a7f5cdb888c86e737c56281c Mon Sep 17 00:00:00 2001
> > > > > From: Jeff Angielski <jeff@theptrgroup.com>
> > > > > Date: Mon, 10 May 2010 10:26:34 -0400
> > > > > Subject: [PATCH] hwmon: (tmp421) Add nfactor support
> > > > > 
> > > > > Add support for reading and writing the n-factor correction
> > > > > registers.  This is needed to compensate for the characteristics
> > > > > of a particular sensor hanging off of the remote channels.
> > > > > 
> > 
> > My concerns with this approach are 
> > 
> > 1) It changes the sysfs abi. libsensors won't support it. It opens up
> >    a can of worms with everyone starting to put chip-specific extensions
> >    into the ABI. If such an extension has to be made, it should be a) really necessary
> >    and b) a generic extension which applies to all chips.
> 
> A chip-specific extension can't be also generic. So we have to decide whether weaccept chip-specific extensions or not.

libsensors 3 will never support chip-specific extensions. We've been
there with libsensors 2, it was a nightmare, never again please.

This doesn't mean we can't have chip-specific extensions, and actually
some drivers have such extensions already. But as they are often
undocumented and bound to be replaced by standard implementations in
the future, nobody should rely on them. Which makes their usefulness
dubious.

This is the reason why I always insist on trying to define a standard
suitable for all chips before you think of adding a not-yet-supported
feature to a given driver.

> > 2) My understanding is that value adjustments are supposed to be made via sensors.conf,
> >    and that values reported by the chip should always be 'raw', ie unadjusted
> >    values.
> > 
> > Instead of exporting n_adjust to the user via sysfs, it might make more sense 
> > to reset the correction factor to its default (if it was changed), and handle
> > required adjustments in sensors.conf.
> 
> Jeff, what do you think?

I'm not Jeff, but resetting the factors is a bad idea. The
BIOS/firmware might have set them up properly, so the default should be
to leave them untouched (as we do with every other setting.)

> > Even if Jean doesn't have time to handle the patch, you should at least get his Ack
> > for the ABI changes.
> 
> That was the intention of resending the patch to the lm-sensors list. It would
> be a pity to lose Jeff's effort.

I'm not giving my ack for any non-standard feature, sorry.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd attempt)

[Jean Delvare]

Hi Andre, Guenter, Jeff,

On Wed, 21 Jul 2010 21:46:50 +0200, Andre Prendel wrote:
> On Tue, Jul 20, 2010 at 08:59:52AM -0700, Guenter Roeck wrote:
> > On Tue, Jul 20, 2010 at 11:09:53AM -0400, Andre Prendel wrote:
> > > On Thu, May 20, 2010 at 09:35:56PM +0200, Andre Prendel wrote:
> > > > On Thu, May 20, 2010 at 03:07:05PM -0400, Jeff Angielski wrote:
> > > > > In any event, here it is again:
> > > > 
> > > > Acked-by: Andre Prendel <andre.prendel@gmx.de>
> > > 
> > > Hi Jeff,
> > > 
> > > I'de suggest to resend the patch with my acked-by to the lm-sensors list and
> > > Andrew Morton. It looks like Jean is too busy at the moment.
> > > 
> > > Regards,
> > > Andre
> > >  
> > > > > >From 9acd29ff48c64e58a7f5cdb888c86e737c56281c Mon Sep 17 00:00:00 2001
> > > > > From: Jeff Angielski <jeff@theptrgroup.com>
> > > > > Date: Mon, 10 May 2010 10:26:34 -0400
> > > > > Subject: [PATCH] hwmon: (tmp421) Add nfactor support
> > > > > 
> > > > > Add support for reading and writing the n-factor correction
> > > > > registers.  This is needed to compensate for the characteristics
> > > > > of a particular sensor hanging off of the remote channels.
> > > > > 
> > 
> > My concerns with this approach are 
> > 
> > 1) It changes the sysfs abi. libsensors won't support it. It opens up
> >    a can of worms with everyone starting to put chip-specific extensions
> >    into the ABI. If such an extension has to be made, it should be a) really necessary
> >    and b) a generic extension which applies to all chips.
> 
> A chip-specific extension can't be also generic. So we have to decide whether weaccept chip-specific extensions or not.

libsensors 3 will never support chip-specific extensions. We've been
there with libsensors 2, it was a nightmare, never again please.

This doesn't mean we can't have chip-specific extensions, and actually
some drivers have such extensions already. But as they are often
undocumented and bound to be replaced by standard implementations in
the future, nobody should rely on them. Which makes their usefulness
dubious.

This is the reason why I always insist on trying to define a standard
suitable for all chips before you think of adding a not-yet-supported
feature to a given driver.

> > 2) My understanding is that value adjustments are supposed to be made via sensors.conf,
> >    and that values reported by the chip should always be 'raw', ie unadjusted
> >    values.
> > 
> > Instead of exporting n_adjust to the user via sysfs, it might make more sense 
> > to reset the correction factor to its default (if it was changed), and handle
> > required adjustments in sensors.conf.
> 
> Jeff, what do you think?

I'm not Jeff, but resetting the factors is a bad idea. The
BIOS/firmware might have set them up properly, so the default should be
to leave them untouched (as we do with every other setting.)

> > Even if Jean doesn't have time to handle the patch, you should at least get his Ack
> > for the ABI changes.
> 
> That was the intention of resending the patch to the lm-sensors list. It would
> be a pity to lose Jeff's effort.

I'm not giving my ack for any non-standard feature, sorry.

-- 
Jean Delvare