[HWMON] Add LM82 support

[HWMON] Add LM82 support

[Jordan Crouse]

[-- Attachment #1: Type: text/plain, Size: 625 bytes --]

This patch adds support for the LM82 temperature sensor from National
Semiconductor.  The chip is very much like the LM83 with less temperature
sensors.  The really only goofy thing here, is that the registers for the
2nd temperature sensor on the LM82 are marked as the *3rd* sensor on the
LM83, so I play a little magic to keep things all nice and linear.

For all you Geode users, the LM82 is the temperature sensor of choice
on most of our platforms, so this should be valuable for you.

Regards,
Jordan
-- 
Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<www.amd.com/embeddedprocessors>

[-- Attachment #2: hwmon-lm83.patch --]
[-- Type: text/plain, Size: 4623 bytes --]

[HWMON] Add LM82 support

Add LM82 thermal detector support (similar to the LM83, but less featureful).

Signed-off-by: Jordan Crouse <jordan.crouse@amd.com>
---

 drivers/hwmon/lm83.c |   63 +++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/drivers/hwmon/lm83.c b/drivers/hwmon/lm83.c
index 26dfa9e..aa8b74d 100644
--- a/drivers/hwmon/lm83.c
+++ b/drivers/hwmon/lm83.c
@@ -12,6 +12,11 @@
  * Since the datasheet omits to give the chip stepping code, I give it
  * here: 0x03 (at register 0xff).
  *
+ * <jordan.crouse@amd.com>: Added LM82 support
+ * http://www.national.com/pf/LM/LM82.html - basically a stripped down
+ * model of the LM83, with only two temperatures reported.  The stepping
+ * is 0x01 (at least according to the datasheet).
+ *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License, or
@@ -51,7 +56,7 @@ static unsigned short normal_i2c[] = { 0
  * Insmod parameters
  */
 
-I2C_CLIENT_INSMOD_1(lm83);
+I2C_CLIENT_INSMOD_2(lm83, lm82);
 
 /*
  * The LM83 registers
@@ -142,6 +147,7 @@ struct lm83_data {
 	struct semaphore update_lock;
 	char valid; /* zero until following fields are valid */
 	unsigned long last_updated; /* in jiffies */
+	int type;  /* Remember the type of chip */
 
 	/* registers values */
 	s8 temp[9];	/* 0..3: input 1-4,
@@ -159,7 +165,14 @@ static ssize_t show_temp(struct device *
 {
 	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
 	struct lm83_data *data = lm83_update_device(dev);
-	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp[attr->index]));
+	int index = attr->index;
+
+	if (data->type == lm82) {
+		if (index == 1 || index == 5)
+			index++;
+	}
+
+	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp[index]));
 }
 
 static ssize_t set_temp(struct device *dev, struct device_attribute *devattr,
@@ -172,6 +185,12 @@ static ssize_t set_temp(struct device *d
 	int nr = attr->index;
 
 	down(&data->update_lock);
+
+	if (data->type == lm82) {
+		if (nr == 1 || nr == 5)
+			nr++;
+	}
+
 	data->temp[nr] = TEMP_TO_REG(val);
 	i2c_smbus_write_byte_data(client, LM83_REG_W_HIGH[nr - 4],
 				  data->temp[nr]);
@@ -283,6 +302,9 @@ static int lm83_detect(struct i2c_adapte
 			if (chip_id == 0x03) {
 				kind = lm83;
 			}
+			else if (chip_id == 0x01) {
+				kind = lm82;
+			}
 		}
 
 		if (kind <= 0) { /* identification failed */
@@ -296,10 +318,15 @@ static int lm83_detect(struct i2c_adapte
 	if (kind == lm83) {
 		name = "lm83";
 	}
+	else if (kind = lm82) {
+		name = "lm82";
+	}
 
 	/* We can fill in the remaining client fields */
 	strlcpy(new_client->name, name, I2C_NAME_SIZE);
 	data->valid = 0;
+	data->type = kind;
+
 	init_MUTEX(&data->update_lock);
 
 	/* Tell the I2C layer a new client has arrived */
@@ -322,28 +349,36 @@ static int lm83_detect(struct i2c_adapte
 			   &sensor_dev_attr_temp1_input.dev_attr);
 	device_create_file(&new_client->dev,
 			   &sensor_dev_attr_temp2_input.dev_attr);
-	device_create_file(&new_client->dev,
-			   &sensor_dev_attr_temp3_input.dev_attr);
-	device_create_file(&new_client->dev,
-			   &sensor_dev_attr_temp4_input.dev_attr);
+
 	device_create_file(&new_client->dev,
 			   &sensor_dev_attr_temp1_max.dev_attr);
 	device_create_file(&new_client->dev,
 			   &sensor_dev_attr_temp2_max.dev_attr);
-	device_create_file(&new_client->dev,
-			   &sensor_dev_attr_temp3_max.dev_attr);
-	device_create_file(&new_client->dev,
-			   &sensor_dev_attr_temp4_max.dev_attr);
+
 	device_create_file(&new_client->dev,
 			   &sensor_dev_attr_temp1_crit.dev_attr);
 	device_create_file(&new_client->dev,
 			   &sensor_dev_attr_temp2_crit.dev_attr);
-	device_create_file(&new_client->dev,
-			   &sensor_dev_attr_temp3_crit.dev_attr);
-	device_create_file(&new_client->dev,
-			   &sensor_dev_attr_temp4_crit.dev_attr);
+
 	device_create_file(&new_client->dev, &dev_attr_alarms);
 
+	if (kind == lm83) {
+		device_create_file(&new_client->dev,
+				   &sensor_dev_attr_temp3_input.dev_attr);
+		device_create_file(&new_client->dev,
+				   &sensor_dev_attr_temp4_input.dev_attr);
+
+		device_create_file(&new_client->dev,
+				   &sensor_dev_attr_temp3_max.dev_attr);
+		device_create_file(&new_client->dev,
+				   &sensor_dev_attr_temp4_max.dev_attr);
+
+		device_create_file(&new_client->dev,
+				   &sensor_dev_attr_temp3_crit.dev_attr);
+		device_create_file(&new_client->dev,
+				   &sensor_dev_attr_temp4_crit.dev_attr);
+	}
+
 	return 0;
 
 exit_detach:

Re: [lm-sensors] [HWMON] Add LM82 support

[Jean Delvare]

Hi Jordan,

It's nice to see someone from AMD working with us :)

On 2006-02-16, Jordan Crouse wrote:
> This patch adds support for the LM82 temperature sensor from National
> Semiconductor.  The chip is very much like the LM83 with less temperature
> sensors.  The really only goofy thing here, is that the registers for the
> 2nd temperature sensor on the LM82 are marked as the *3rd* sensor on the
> LM83, so I play a little magic to keep things all nice and linear.

It is really common for hardware monitoring drivers to just skip some
inputs when one driver handles several chips with minor differences. For
example, the w83627hf driver skips in5 and in6 for the W83627THF chip.
So I'd prefer that you don't attempt to renumber the inputs for the
LM82, but simply create temp1* and temp3* and omit temp2* and temp4*.

One reason why this is important is that there seem to be two variants of
the LM82. First is the one you have with a chip ID of 0x01. Second is a
stripped down version of the LM83, with chip ID of 0x03, which is
totally unrecognizable from the LM83. It will make things much easier if
the two variants can be handled the same way from user-space, which
implies that we don't renumber the inputs.

My comments on your code now:

> --- a/drivers/hwmon/lm83.c
> +++ b/drivers/hwmon/lm83.c
> @@ -12,6 +12,11 @@
>   * Since the datasheet omits to give the chip stepping code, I give it
>   * here: 0x03 (at register 0xff).
>   *
> + * <jordan.crouse@amd.com>: Added LM82 support
> + * http://www.national.com/pf/LM/LM82.html - basically a stripped down
> + * model of the LM83, with only two temperatures reported.  The stepping
> + * is 0x01 (at least according to the datasheet).
> + *

History doesn't belong to the source code, as we have GIT for this. So
this paragraph should be written just as if the driver had always
supported the LM82 device. See lm90.c for an example.

You should also include changes to Documentation/hwmon/lm83 in your
patch. Again you may use lm90 as an example. And an update to
drivers/hwmon/Kconfig would be welcome as well, to mention the LM82 in
the lm83 driver help text if no even label.

> @@ -142,6 +147,7 @@ struct lm83_data {
>  	struct semaphore update_lock;
>  	char valid; /* zero until following fields are valid */
>  	unsigned long last_updated; /* in jiffies */
> +	int type;  /* Remember the type of chip */

You should no more need this if you don't renumber the inputs, right?

All the rest is just fine with me. Great work!

Would you be kind enough to also provide a patch against lm_sensors
2.10.0 or CVS for user-space support? It should be quite simple.

Thanks,
--
Jean Delvare

Re: [HWMON] Add LM82 support

[Nick Warne]

'
>                 if (kind <= 0) { /* identification failed */
> @@ -296,10 +318,15 @@ static int lm83_detect(struct i2c_adapte
>         if (kind == lm83) {
>                 name = "lm83";
>         }
> +       else if (kind = lm82) {
> +               name = "lm82";
> +       }

Is that a '=' typo in the 'else if' there?

Nick

Re: Add LM82 support

[Jordan Crouse]

On 17/02/06 13:09 +0000, Nick Warne wrote:
> '
> >                 if (kind <= 0) { /* identification failed */
> > @@ -296,10 +318,15 @@ static int lm83_detect(struct i2c_adapte
> >         if (kind == lm83) {
> >                 name = "lm83";
> >         }
> > +       else if (kind = lm82) {
> > +               name = "lm82";
> > +       }
> 
> Is that a '=' typo in the 'else if' there?

Yep - thats what that would be.  That must be my typo of the week, since
I just had it in another driver I'm writing.  I'll post a fixup here
very shortly with all the comments.

Jordan 

-- 
Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<www.amd.com/embeddedprocessors>

Re: Add LM82 support

[Jean Delvare]

Hi Jordan,

> On 17/02/06 13:09 +0000, Nick Warne wrote:
> > > +       else if (kind = lm82) {
> > > +               name = "lm82";
> > > +       }
> > 
> > Is that a '=' typo in the 'else if' there?
> 
> Yep - thats what that would be.  That must be my typo of the week, since
> I just had it in another driver I'm writing.

Given that gcc clearly warns on this kind of typo, how could it go
unnoticed?

-- 
Jean Delvare

Re: Add LM82 support

[Jean Delvare]

Hi Jordan,

> Yep - thats what that would be.  That must be my typo of the week, since
> I just had it in another driver I'm writing.  I'll post a fixup here
> very shortly with all the comments.

Where's that update? I'd like to merge it upstream.

Remember that a patch for user-space support would be welcome too.

Thanks,
-- 
Jean Delvare