[lm-sensors] [PATCH] hwmon: (lm90) Add support for G781 and G781-1

[lm-sensors] [PATCH] hwmon: (lm90) Add support for G781 and G781-1

[Guenter Roeck]

GMT G781 and G781-1 are ADM1032-compatible temperature sensor chips. Add support
to the LM90 driver.

Cc: Mike Gorchak <lestat@i.com.ua>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
Compile-tested only.

 drivers/hwmon/lm90.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index 2e207de..7694b98 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -107,7 +107,7 @@ static const unsigned short normal_i2c[] = {
 	0x4d, 0x4e, 0x4f, I2C_CLIENT_END };
 
 enum chips { lm90, adm1032, lm99, lm86, max6657, max6659, adt7461, max6680,
-	max6646, w83l771, max6696, sa56004 };
+	max6646, w83l771, max6696, sa56004, g781 };
 
 /*
  * The LM90 registers
@@ -184,6 +184,7 @@ static const struct i2c_device_id lm90_id[] = {
 	{ "adm1032", adm1032 },
 	{ "adt7461", adt7461 },
 	{ "adt7461a", adt7461 },
+	{ "g781", g781 },
 	{ "lm90", lm90 },
 	{ "lm86", lm86 },
 	{ "lm89", lm86 },
@@ -229,6 +230,12 @@ static const struct lm90_params lm90_params[] = {
 		.alert_alarms = 0x7c,
 		.max_convrate = 10,
 	},
+	[g781] = {
+		.flags = LM90_HAVE_OFFSET | LM90_HAVE_REM_LIMIT_EXT
+		  | LM90_HAVE_BROKEN_ALERT,
+		.alert_alarms = 0x7c,
+		.max_convrate = 8,
+	},
 	[lm86] = {
 		.flags = LM90_HAVE_OFFSET | LM90_HAVE_REM_LIMIT_EXT,
 		.alert_alarms = 0x7b,
@@ -1291,6 +1298,13 @@ static int lm90_detect(struct i2c_client *client,
 		 && convrate <= 0x09) {
 			name = "sa56004";
 		}
+	} else
+	if (man_id = 0x47) { /* GMT */
+		if (convrate <= 0x08
+		    && (config1 & 0x3F) = 0x00
+		    && ((address = 0x4C && chip_id = 0x01)      /* G781   */
+			|| (address = 0x4D && chip_id = 0x03))) /* G781-1 */
+			name = "g781";
 	}
 
 	if (!name) { /* identification failed */
-- 
1.7.5.4


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

Re: [lm-sensors] [PATCH] hwmon: (lm90) Add support for G781 and G781-1

[Jean Delvare]

On Sun, 22 Jan 2012 15:43:31 -0800, Guenter Roeck wrote:
> GMT G781 and G781-1 are ADM1032-compatible temperature sensor chips. Add support
> to the LM90 driver.
> 
> Cc: Mike Gorchak <lestat@i.com.ua>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> Compile-tested only.
> 
>  drivers/hwmon/lm90.c |   16 +++++++++++++++-
>  1 files changed, 15 insertions(+), 1 deletions(-)

You'll also have to update Documentation/hwmon/lm90, Kconfig, the
header comment of lm90.c and the address information comment right
before normal_i2c[] in that file.

> 
> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> index 2e207de..7694b98 100644
> --- a/drivers/hwmon/lm90.c
> +++ b/drivers/hwmon/lm90.c
> @@ -107,7 +107,7 @@ static const unsigned short normal_i2c[] = {
>  	0x4d, 0x4e, 0x4f, I2C_CLIENT_END };
>  
>  enum chips { lm90, adm1032, lm99, lm86, max6657, max6659, adt7461, max6680,
> -	max6646, w83l771, max6696, sa56004 };
> +	max6646, w83l771, max6696, sa56004, g781 };
>  
>  /*
>   * The LM90 registers
> @@ -184,6 +184,7 @@ static const struct i2c_device_id lm90_id[] = {
>  	{ "adm1032", adm1032 },
>  	{ "adt7461", adt7461 },
>  	{ "adt7461a", adt7461 },
> +	{ "g781", g781 },
>  	{ "lm90", lm90 },
>  	{ "lm86", lm86 },
>  	{ "lm89", lm86 },
> @@ -229,6 +230,12 @@ static const struct lm90_params lm90_params[] = {
>  		.alert_alarms = 0x7c,
>  		.max_convrate = 10,
>  	},
> +	[g781] = {
> +		.flags = LM90_HAVE_OFFSET | LM90_HAVE_REM_LIMIT_EXT
> +		  | LM90_HAVE_BROKEN_ALERT,

How do you know it has broken alert? Playing it safe, or does anything
in the datasheet suggest so? So far, only Analog Devices chips are
known to need this. I'd rather have Mike test first, and only put this
flag if really needed (it has a cost.)

> +		.alert_alarms = 0x7c,
> +		.max_convrate = 8,
> +	},
>  	[lm86] = {
>  		.flags = LM90_HAVE_OFFSET | LM90_HAVE_REM_LIMIT_EXT,
>  		.alert_alarms = 0x7b,
> @@ -1291,6 +1298,13 @@ static int lm90_detect(struct i2c_client *client,
>  		 && convrate <= 0x09) {
>  			name = "sa56004";
>  		}
> +	} else
> +	if (man_id = 0x47) { /* GMT */
> +		if (convrate <= 0x08
> +		    && (config1 & 0x3F) = 0x00

Would be nice to use the same alignment logic as the rest of the code
in this function, even though it might not be your preferred one.

> +		    && ((address = 0x4C && chip_id = 0x01)      /* G781   */
> +			|| (address = 0x4D && chip_id = 0x03))) /* G781-1 */

As for the sensors-detect patch, 0x01 is the right value for both
variants of the chip.

> +			name = "g781";
>  	}
>  
>  	if (!name) { /* identification failed */


-- 
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: (lm90) Add support for G781 and G781-1

[Guenter Roeck]

Hi Jean,

On Mon, Jan 23, 2012 at 04:26:09AM -0500, Jean Delvare wrote:
> On Sun, 22 Jan 2012 15:43:31 -0800, Guenter Roeck wrote:
> > GMT G781 and G781-1 are ADM1032-compatible temperature sensor chips. Add support
> > to the LM90 driver.
> > 
> > Cc: Mike Gorchak <lestat@i.com.ua>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> > Compile-tested only.
> > 
> >  drivers/hwmon/lm90.c |   16 +++++++++++++++-
> >  1 files changed, 15 insertions(+), 1 deletions(-)
> 
> You'll also have to update Documentation/hwmon/lm90, Kconfig, the
> header comment of lm90.c and the address information comment right
> before normal_i2c[] in that file.
> 
ok

> > 
> > diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> > index 2e207de..7694b98 100644
> > --- a/drivers/hwmon/lm90.c
> > +++ b/drivers/hwmon/lm90.c
> > @@ -107,7 +107,7 @@ static const unsigned short normal_i2c[] = {
> >  	0x4d, 0x4e, 0x4f, I2C_CLIENT_END };
> >  
> >  enum chips { lm90, adm1032, lm99, lm86, max6657, max6659, adt7461, max6680,
> > -	max6646, w83l771, max6696, sa56004 };
> > +	max6646, w83l771, max6696, sa56004, g781 };
> >  
> >  /*
> >   * The LM90 registers
> > @@ -184,6 +184,7 @@ static const struct i2c_device_id lm90_id[] = {
> >  	{ "adm1032", adm1032 },
> >  	{ "adt7461", adt7461 },
> >  	{ "adt7461a", adt7461 },
> > +	{ "g781", g781 },
> >  	{ "lm90", lm90 },
> >  	{ "lm86", lm86 },
> >  	{ "lm89", lm86 },
> > @@ -229,6 +230,12 @@ static const struct lm90_params lm90_params[] = {
> >  		.alert_alarms = 0x7c,
> >  		.max_convrate = 10,
> >  	},
> > +	[g781] = {
> > +		.flags = LM90_HAVE_OFFSET | LM90_HAVE_REM_LIMIT_EXT
> > +		  | LM90_HAVE_BROKEN_ALERT,
> 
> How do you know it has broken alert? Playing it safe, or does anything
> in the datasheet suggest so? So far, only Analog Devices chips are
> known to need this. I'd rather have Mike test first, and only put this
> flag if really needed (it has a cost.)
> 
Datasheet says "Note that the ALERT interrupt latch is not automatically
cleared when the status flag bit is cleared." Maybe I misunderstood that ?
No problem, though, I'll take it out.

> > +		.alert_alarms = 0x7c,
> > +		.max_convrate = 8,
> > +	},
> >  	[lm86] = {
> >  		.flags = LM90_HAVE_OFFSET | LM90_HAVE_REM_LIMIT_EXT,
> >  		.alert_alarms = 0x7b,
> > @@ -1291,6 +1298,13 @@ static int lm90_detect(struct i2c_client *client,
> >  		 && convrate <= 0x09) {
> >  			name = "sa56004";
> >  		}
> > +	} else
> > +	if (man_id = 0x47) { /* GMT */
> > +		if (convrate <= 0x08
> > +		    && (config1 & 0x3F) = 0x00
> 
> Would be nice to use the same alignment logic as the rest of the code
> in this function, even though it might not be your preferred one.
> 
Sure.

> > +		    && ((address = 0x4C && chip_id = 0x01)      /* G781   */
> > +			|| (address = 0x4D && chip_id = 0x03))) /* G781-1 */
> 
> As for the sensors-detect patch, 0x01 is the right value for both
> variants of the chip.
> 
Datasheet says:

The G781 and G781-1 have the following SMBus
slave address:
	A6 A5 A4 A3 A2 A1 A0
G781	1  0  0  1  1  0  0
G781-1	1  0  0  1  1  0  1

and:

MFGIO	FEh		0100 0111	Manufacturer ID
DEVID	FFh	G781	0000 0001	Device ID
		G781-1	0000 0011

Obviously G781 can be on both addresses, but what to do with devid 0x03 ?

Guenter

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

Re: [lm-sensors] [PATCH] hwmon: (lm90) Add support for G781 and G781-1

[Jean Delvare]

On Mon, 23 Jan 2012 06:22:00 -0800, Guenter Roeck wrote:
> On Mon, Jan 23, 2012 at 04:26:09AM -0500, Jean Delvare wrote:
> > On Sun, 22 Jan 2012 15:43:31 -0800, Guenter Roeck wrote:
> > > diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> > > (...)
> > > @@ -229,6 +230,12 @@ static const struct lm90_params lm90_params[] = {
> > >  		.alert_alarms = 0x7c,
> > >  		.max_convrate = 10,
> > >  	},
> > > +	[g781] = {
> > > +		.flags = LM90_HAVE_OFFSET | LM90_HAVE_REM_LIMIT_EXT
> > > +		  | LM90_HAVE_BROKEN_ALERT,
> > 
> > How do you know it has broken alert? Playing it safe, or does anything
> > in the datasheet suggest so? So far, only Analog Devices chips are
> > known to need this. I'd rather have Mike test first, and only put this
> > flag if really needed (it has a cost.)
> > 
> Datasheet says "Note that the ALERT interrupt latch is not automatically
> cleared when the status flag bit is cleared." Maybe I misunderstood that ?
> No problem, though, I'll take it out.

Keep it in then :) I was suspecting a careless copy-n-paste, but
apparently I was wrong, sorry about that. I had not read the datasheet
carefully, while obviously you did exactly that.

> > > (...)
> > > +		    && ((address = 0x4C && chip_id = 0x01)      /* G781   */
> > > +			|| (address = 0x4D && chip_id = 0x03))) /* G781-1 */
> > 
> > As for the sensors-detect patch, 0x01 is the right value for both
> > variants of the chip.
> > 
> Datasheet says:
> 
> The G781 and G781-1 have the following SMBus
> slave address:
> 	A6 A5 A4 A3 A2 A1 A0
> G781	1  0  0  1  1  0  0
> G781-1	1  0  0  1  1  0  1
> 
> and:
> 
> MFGIO	FEh		0100 0111	Manufacturer ID
> DEVID	FFh	G781	0000 0001	Device ID
> 		G781-1	0000 0011
> 
> Obviously G781 can be on both addresses, but what to do with devid 0x03 ?

I read it the other way around. Obviously G781-1 has address 0x4D and
the same device ID as the G781. I.e. the datasheet is wrong. That would
be consistent with what the other manufacturers did (e.g. LM89 and
LM89-1 have the same device ID.)

I'd stick to what Mike has, at least we know this exists. If we get
reports of unsupported devices, we can add the new IDs later.

-- 
Jean Delvare

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