[PATCH] lm90: Support the MAX6648/6692 chips

[PATCH] lm90: Support the MAX6648/6692 chips

[Darrick J. Wong]

The max6648 chip has nearly the same register set as the 6657 and
seems to have a working manufacturer/chip id so we can detect it.
This patch adds support for it.  Tested on a Nvidia Quadro FX 1500
card.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/hwmon/Kconfig |    4 ++--
 drivers/hwmon/lm90.c  |    7 ++++++-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 8c312c6..7da49d3 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -550,8 +550,8 @@ config SENSORS_LM90
 	help
 	  If you say yes here you get support for National Semiconductor LM90,
 	  LM86, LM89 and LM99, Analog Devices ADM1032 and ADT7461, and Maxim
-	  MAX6646, MAX6647, MAX6649, MAX6657, MAX6658, MAX6659, MAX6680 and
-	  MAX6681 sensor chips.
+	  MAX6646, MAX6647, MAX6649, MAX6657, MAX6658, MAX6659, MAX6680,
+	  MAX6648, MAX6692 and MAX6681 sensor chips.
 
 	  This driver can also be built as a module.  If so, the module
 	  will be called lm90.
diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index 96a7018..1802366 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -776,7 +776,12 @@ static int lm90_detect(struct i2c_client *new_client, int kind,
 			 && (reg_config1 & 0x3f) == 0x00
 			 && reg_convrate <= 0x07) {
 				kind = max6646;
-			}
+			} else
+			/* The MAX6648/6692 chips have a working man/chip id
+			 * and the same register set as the 6657.
+			 */
+			if (chip_id == 0x59 && address == 0x4C)
+				kind = max6657;
 		}
 
 		if (kind <= 0) { /* identification failed */

Re: [PATCH] lm90: Support the MAX6648/6692 chips

[Andrew Morton]

On Mon, 2 Mar 2009 13:01:06 -0800
"Darrick J. Wong" <djwong@us.ibm.com> wrote:

> @@ -776,7 +776,12 @@ static int lm90_detect(struct i2c_client *new_client, int kind,
>  			 && (reg_config1 & 0x3f) == 0x00
>  			 && reg_convrate <= 0x07) {
>  				kind = max6646;
> -			}
> +			} else
> +			/* The MAX6648/6692 chips have a working man/chip id
> +			 * and the same register set as the 6657.
> +			 */
> +			if (chip_id == 0x59 && address == 0x4C)
> +				kind = max6657;
>  		}

gack, the indenting and layout there is totally busted.


--- a/drivers/hwmon/lm90.c~lm90-support-the-max6648-6692-chips-fix
+++ a/drivers/hwmon/lm90.c
@@ -776,12 +776,14 @@ static int lm90_detect(struct i2c_client
 			 && (reg_config1 & 0x3f) == 0x00
 			 && reg_convrate <= 0x07) {
 				kind = max6646;
-			} else
-			/* The MAX6648/6692 chips have a working man/chip id
-			 * and the same register set as the 6657.
-			 */
-			if (chip_id == 0x59 && address == 0x4C)
+			} else if (chip_id == 0x59 && address == 0x4C) {
+				/*
+				 * The MAX6648/6692 chips have a working
+				 * man/chip id and the same register set as the
+				 * 6657.
+				 */
 				kind = max6657;
+			}
 		}
 
 		if (kind <= 0) { /* identification failed */
_

Re: [PATCH] lm90: Support the MAX6648/6692 chips

[Jean Delvare]

Hi Andrew,

On Mon, 2 Mar 2009 15:04:26 -0800, Andrew Morton wrote:
> On Mon, 2 Mar 2009 13:01:06 -0800
> "Darrick J. Wong" <djwong@us.ibm.com> wrote:
> 
> > @@ -776,7 +776,12 @@ static int lm90_detect(struct i2c_client *new_client, int kind,
> >  			 && (reg_config1 & 0x3f) == 0x00
> >  			 && reg_convrate <= 0x07) {
> >  				kind = max6646;
> > -			}
> > +			} else
> > +			/* The MAX6648/6692 chips have a working man/chip id
> > +			 * and the same register set as the 6657.
> > +			 */
> > +			if (chip_id == 0x59 && address == 0x4C)
> > +				kind = max6657;
> >  		}
> 
> gack, the indenting and layout there is totally busted.

This specific layout is consistently used through the whole function,
and checkpatch.pl doesn't complain about it. While unconventional, it
has its advantages, in particular it avoids extra indentation that
would make some lines too long. At any rate it doesn't make sense to
change this last chunk without changing all the rest if this layout is
deemed unacceptable.

> --- a/drivers/hwmon/lm90.c~lm90-support-the-max6648-6692-chips-fix
> +++ a/drivers/hwmon/lm90.c
> @@ -776,12 +776,14 @@ static int lm90_detect(struct i2c_client
>  			 && (reg_config1 & 0x3f) == 0x00
>  			 && reg_convrate <= 0x07) {
>  				kind = max6646;
> -			} else
> -			/* The MAX6648/6692 chips have a working man/chip id
> -			 * and the same register set as the 6657.
> -			 */
> -			if (chip_id == 0x59 && address == 0x4C)
> +			} else if (chip_id == 0x59 && address == 0x4C) {
> +				/*
> +				 * The MAX6648/6692 chips have a working
> +				 * man/chip id and the same register set as the
> +				 * 6657.
> +				 */
>  				kind = max6657;
> +			}
>  		}
>  
>  		if (kind <= 0) { /* identification failed */

I thus nack this change of yours.

-- 
Jean Delvare

Re: [PATCH] lm90: Support the MAX6648/6692 chips

[Andrew Morton]

On Tue, 3 Mar 2009 08:47:46 +0100 Jean Delvare <khali@linux-fr.org> wrote:

> Hi Andrew,
> 
> On Mon, 2 Mar 2009 15:04:26 -0800, Andrew Morton wrote:
> > On Mon, 2 Mar 2009 13:01:06 -0800
> > "Darrick J. Wong" <djwong@us.ibm.com> wrote:
> > 
> > > @@ -776,7 +776,12 @@ static int lm90_detect(struct i2c_client *new_client, int kind,
> > >  			 && (reg_config1 & 0x3f) == 0x00
> > >  			 && reg_convrate <= 0x07) {
> > >  				kind = max6646;
> > > -			}
> > > +			} else
> > > +			/* The MAX6648/6692 chips have a working man/chip id
> > > +			 * and the same register set as the 6657.
> > > +			 */
> > > +			if (chip_id == 0x59 && address == 0x4C)
> > > +				kind = max6657;
> > >  		}
> > 
> > gack, the indenting and layout there is totally busted.
> 
> This specific layout is consistently used through the whole function,
> and checkpatch.pl doesn't complain about it. While unconventional, it
> has its advantages, in particular it avoids extra indentation that
> would make some lines too long. At any rate it doesn't make sense to
> change this last chunk without changing all the rest if this layout is
> deemed unacceptable.

lol, be serious.

> > --- a/drivers/hwmon/lm90.c~lm90-support-the-max6648-6692-chips-fix
> > +++ a/drivers/hwmon/lm90.c
> > @@ -776,12 +776,14 @@ static int lm90_detect(struct i2c_client
> >  			 && (reg_config1 & 0x3f) == 0x00
> >  			 && reg_convrate <= 0x07) {
> >  				kind = max6646;
> > -			} else
> > -			/* The MAX6648/6692 chips have a working man/chip id
> > -			 * and the same register set as the 6657.
> > -			 */
> > -			if (chip_id == 0x59 && address == 0x4C)
> > +			} else if (chip_id == 0x59 && address == 0x4C) {
> > +				/*
> > +				 * The MAX6648/6692 chips have a working
> > +				 * man/chip id and the same register set as the
> > +				 * 6657.
> > +				 */
> >  				kind = max6657;
> > +			}
> >  		}
> >  
> >  		if (kind <= 0) { /* identification failed */
> 
> I thus nack this change of yours.

Something like this...

diff -puN drivers/hwmon/lm90.c~drivers-hwmon-lm90c-fix-coding-style drivers/hwmon/lm90.c
--- a/drivers/hwmon/lm90.c~drivers-hwmon-lm90c-fix-coding-style
+++ a/drivers/hwmon/lm90.c
@@ -694,22 +694,22 @@ static int lm90_detect(struct i2c_client
 						LM90_REG_R_CONVRATE)) < 0)
 			return -ENODEV;
 		
-		if ((address == 0x4C || address == 0x4D)
-		 && man_id == 0x01) { /* National Semiconductor */
+		if ((address == 0x4C || address == 0x4D) && man_id == 0x01) {
+			/* National Semiconductor */
 			int reg_config2;
 
 			if ((reg_config2 = i2c_smbus_read_byte_data(new_client,
 						LM90_REG_R_CONFIG2)) < 0)
 				return -ENODEV;
 
-			if ((reg_config1 & 0x2A) == 0x00
-			 && (reg_config2 & 0xF8) == 0x00
-			 && reg_convrate <= 0x09) {
-				if (address == 0x4C
-				 && (chip_id & 0xF0) == 0x20) { /* LM90 */
+			if ((reg_config1 & 0x2A) == 0x00 &&
+			    (reg_config2 & 0xF8) == 0x00 &&
+			    reg_convrate <= 0x09) {
+				if (address == 0x4C &&
+				    (chip_id & 0xF0) == 0x20) { /* LM90 */
 					kind = lm90;
-				} else
-				if ((chip_id & 0xF0) == 0x30) { /* LM89/LM99 */
+				} else if ((chip_id & 0xF0) == 0x30) {
+					/* LM89/LM99 */
 					kind = lm99;
 					dev_info(&adapter->dev,
 						 "Assuming LM99 chip at "
@@ -720,27 +720,24 @@ static int lm90_detect(struct i2c_client
 						 "loading the lm90 driver\n",
 						 i2c_adapter_id(adapter),
 						 address);
-				} else
-				if (address == 0x4C
-				 && (chip_id & 0xF0) == 0x10) { /* LM86 */
+				} else if (address == 0x4C &&
+					(chip_id & 0xF0) == 0x10) { /* LM86 */
 					kind = lm86;
 				}
 			}
-		} else
-		if ((address == 0x4C || address == 0x4D)
-		 && man_id == 0x41) { /* Analog Devices */
-			if ((chip_id & 0xF0) == 0x40 /* ADM1032 */
-			 && (reg_config1 & 0x3F) == 0x00
-			 && reg_convrate <= 0x0A) {
+		} else if ((address == 0x4C || address == 0x4D) &&
+				man_id == 0x41) {
+			/* Analog Devices */
+			if ((chip_id & 0xF0) == 0x40  && /* ADM1032 */
+			    (reg_config1 & 0x3F) == 0x00 &&
+			    reg_convrate <= 0x0A) {
 				kind = adm1032;
-			} else
-			if (chip_id == 0x51 /* ADT7461 */
-			 && (reg_config1 & 0x1B) == 0x00
-			 && reg_convrate <= 0x0A) {
+			} else if (chip_id == 0x51 && /* ADT7461 */
+				 (reg_config1 & 0x1B) == 0x00 &&
+				 reg_convrate <= 0x0A) {
 				kind = adt7461;
 			}
-		} else
-		if (man_id == 0x4D) { /* Maxim */
+		} else if (man_id == 0x4D) { /* Maxim */
 			/*
 			 * The MAX6657, MAX6658 and MAX6659 do NOT have a
 			 * chip_id register. Reading from that address will
@@ -750,31 +747,32 @@ static int lm90_detect(struct i2c_client
 			 * will be those of the previous read, so in our case
 			 * those of the man_id register.
 			 */
-			if (chip_id == man_id
-			 && (address == 0x4C || address == 0x4D)
-			 && (reg_config1 & 0x1F) == (man_id & 0x0F)
-			 && reg_convrate <= 0x09) {
+			if (chip_id == man_id &&
+			    (address == 0x4C || address == 0x4D) &&
+			    (reg_config1 & 0x1F) == (man_id & 0x0F) &&
+			    reg_convrate <= 0x09) {
 			 	kind = max6657;
-			} else
-			/* The chip_id register of the MAX6680 and MAX6681
-			 * holds the revision of the chip.
-			 * the lowest bit of the config1 register is unused
-			 * and should return zero when read, so should the
-			 * second to last bit of config1 (software reset)
-			 */
-			if (chip_id == 0x01
-			 && (reg_config1 & 0x03) == 0x00
-			 && reg_convrate <= 0x07) {
+			} else if (chip_id == 0x01 &&
+					(reg_config1 & 0x03) == 0x00 &&
+				 	reg_convrate <= 0x07) {
+				/*
+				 * The chip_id register of the MAX6680 and
+				 * MAX6681 holds the revision of the chip.
+				 * the lowest bit of the config1 register is
+				 * unused and should return zero when read, so
+				 * should the second to last bit of config1
+				 * (software reset)
+				 */
 			 	kind = max6680;
-			} else
-			/* The chip_id register of the MAX6646/6647/6649
-			 * holds the revision of the chip.
-			 * The lowest 6 bits of the config1 register are
-			 * unused and should return zero when read.
-			 */
-			if (chip_id == 0x59
-			 && (reg_config1 & 0x3f) == 0x00
-			 && reg_convrate <= 0x07) {
+			} else if (chip_id == 0x59 &&
+					(reg_config1 & 0x3f) == 0x00 &&
+					 reg_convrate <= 0x07) {
+				/*
+				 * The chip_id register of the MAX6646/6647/6649
+				 * holds the revision of the chip.
+				 * The lowest 6 bits of the config1 register are
+				 * unused and should return zero when read.
+				 */
 				kind = max6646;
 			} else if (chip_id == 0x59 && address == 0x4C) {
 				/*
_

Re: [PATCH] lm90: Support the MAX6648/6692 chips

[Jean Delvare]

On Tue, 3 Mar 2009 00:04:12 -0800, Andrew Morton wrote:
> On Tue, 3 Mar 2009 08:47:46 +0100 Jean Delvare <khali@linux-fr.org> wrote:
> 
> > Hi Andrew,
> > 
> > On Mon, 2 Mar 2009 15:04:26 -0800, Andrew Morton wrote:
> > > On Mon, 2 Mar 2009 13:01:06 -0800
> > > "Darrick J. Wong" <djwong@us.ibm.com> wrote:
> > > 
> > > > @@ -776,7 +776,12 @@ static int lm90_detect(struct i2c_client *new_client, int kind,
> > > >  			 && (reg_config1 & 0x3f) == 0x00
> > > >  			 && reg_convrate <= 0x07) {
> > > >  				kind = max6646;
> > > > -			}
> > > > +			} else
> > > > +			/* The MAX6648/6692 chips have a working man/chip id
> > > > +			 * and the same register set as the 6657.
> > > > +			 */
> > > > +			if (chip_id == 0x59 && address == 0x4C)
> > > > +				kind = max6657;
> > > >  		}
> > > 
> > > gack, the indenting and layout there is totally busted.
> > 
> > This specific layout is consistently used through the whole function,
> > and checkpatch.pl doesn't complain about it. While unconventional, it
> > has its advantages, in particular it avoids extra indentation that
> > would make some lines too long. At any rate it doesn't make sense to
> > change this last chunk without changing all the rest if this layout is
> > deemed unacceptable.
> 
> lol, be serious.
> 
> > > --- a/drivers/hwmon/lm90.c~lm90-support-the-max6648-6692-chips-fix
> > > +++ a/drivers/hwmon/lm90.c
> > > @@ -776,12 +776,14 @@ static int lm90_detect(struct i2c_client
> > >  			 && (reg_config1 & 0x3f) == 0x00
> > >  			 && reg_convrate <= 0x07) {
> > >  				kind = max6646;
> > > -			} else
> > > -			/* The MAX6648/6692 chips have a working man/chip id
> > > -			 * and the same register set as the 6657.
> > > -			 */
> > > -			if (chip_id == 0x59 && address == 0x4C)
> > > +			} else if (chip_id == 0x59 && address == 0x4C) {
> > > +				/*
> > > +				 * The MAX6648/6692 chips have a working
> > > +				 * man/chip id and the same register set as the
> > > +				 * 6657.
> > > +				 */
> > >  				kind = max6657;
> > > +			}
> > >  		}
> > >  
> > >  		if (kind <= 0) { /* identification failed */
> > 
> > I thus nack this change of yours.
> 
> Something like this...
> 
> diff -puN drivers/hwmon/lm90.c~drivers-hwmon-lm90c-fix-coding-style drivers/hwmon/lm90.c
> --- a/drivers/hwmon/lm90.c~drivers-hwmon-lm90c-fix-coding-style
> +++ a/drivers/hwmon/lm90.c
> @@ -694,22 +694,22 @@ static int lm90_detect(struct i2c_client
>  						LM90_REG_R_CONVRATE)) < 0)
>  			return -ENODEV;
>  		
> -		if ((address == 0x4C || address == 0x4D)
> -		 && man_id == 0x01) { /* National Semiconductor */
> +		if ((address == 0x4C || address == 0x4D) && man_id == 0x01) {
> +			/* National Semiconductor */
>  			int reg_config2;
>  
>  			if ((reg_config2 = i2c_smbus_read_byte_data(new_client,
>  						LM90_REG_R_CONFIG2)) < 0)
>  				return -ENODEV;
>  
> -			if ((reg_config1 & 0x2A) == 0x00
> -			 && (reg_config2 & 0xF8) == 0x00
> -			 && reg_convrate <= 0x09) {
> -				if (address == 0x4C
> -				 && (chip_id & 0xF0) == 0x20) { /* LM90 */
> +			if ((reg_config1 & 0x2A) == 0x00 &&
> +			    (reg_config2 & 0xF8) == 0x00 &&
> +			    reg_convrate <= 0x09) {
> +				if (address == 0x4C &&
> +				    (chip_id & 0xF0) == 0x20) { /* LM90 */
>  					kind = lm90;
> -				} else
> -				if ((chip_id & 0xF0) == 0x30) { /* LM89/LM99 */
> +				} else if ((chip_id & 0xF0) == 0x30) {
> +					/* LM89/LM99 */
>  					kind = lm99;
>  					dev_info(&adapter->dev,
>  						 "Assuming LM99 chip at "
> @@ -720,27 +720,24 @@ static int lm90_detect(struct i2c_client
>  						 "loading the lm90 driver\n",
>  						 i2c_adapter_id(adapter),
>  						 address);
> -				} else
> -				if (address == 0x4C
> -				 && (chip_id & 0xF0) == 0x10) { /* LM86 */
> +				} else if (address == 0x4C &&
> +					(chip_id & 0xF0) == 0x10) { /* LM86 */
>  					kind = lm86;
>  				}
>  			}
> -		} else
> -		if ((address == 0x4C || address == 0x4D)
> -		 && man_id == 0x41) { /* Analog Devices */
> -			if ((chip_id & 0xF0) == 0x40 /* ADM1032 */
> -			 && (reg_config1 & 0x3F) == 0x00
> -			 && reg_convrate <= 0x0A) {
> +		} else if ((address == 0x4C || address == 0x4D) &&
> +				man_id == 0x41) {
> +			/* Analog Devices */
> +			if ((chip_id & 0xF0) == 0x40  && /* ADM1032 */
> +			    (reg_config1 & 0x3F) == 0x00 &&
> +			    reg_convrate <= 0x0A) {
>  				kind = adm1032;
> -			} else
> -			if (chip_id == 0x51 /* ADT7461 */
> -			 && (reg_config1 & 0x1B) == 0x00
> -			 && reg_convrate <= 0x0A) {
> +			} else if (chip_id == 0x51 && /* ADT7461 */
> +				 (reg_config1 & 0x1B) == 0x00 &&
> +				 reg_convrate <= 0x0A) {
>  				kind = adt7461;
>  			}
> -		} else
> -		if (man_id == 0x4D) { /* Maxim */
> +		} else if (man_id == 0x4D) { /* Maxim */
>  			/*
>  			 * The MAX6657, MAX6658 and MAX6659 do NOT have a
>  			 * chip_id register. Reading from that address will
> @@ -750,31 +747,32 @@ static int lm90_detect(struct i2c_client
>  			 * will be those of the previous read, so in our case
>  			 * those of the man_id register.
>  			 */
> -			if (chip_id == man_id
> -			 && (address == 0x4C || address == 0x4D)
> -			 && (reg_config1 & 0x1F) == (man_id & 0x0F)
> -			 && reg_convrate <= 0x09) {
> +			if (chip_id == man_id &&
> +			    (address == 0x4C || address == 0x4D) &&
> +			    (reg_config1 & 0x1F) == (man_id & 0x0F) &&
> +			    reg_convrate <= 0x09) {
>  			 	kind = max6657;
> -			} else
> -			/* The chip_id register of the MAX6680 and MAX6681
> -			 * holds the revision of the chip.
> -			 * the lowest bit of the config1 register is unused
> -			 * and should return zero when read, so should the
> -			 * second to last bit of config1 (software reset)
> -			 */
> -			if (chip_id == 0x01
> -			 && (reg_config1 & 0x03) == 0x00
> -			 && reg_convrate <= 0x07) {
> +			} else if (chip_id == 0x01 &&
> +					(reg_config1 & 0x03) == 0x00 &&
> +				 	reg_convrate <= 0x07) {
> +				/*
> +				 * The chip_id register of the MAX6680 and
> +				 * MAX6681 holds the revision of the chip.
> +				 * the lowest bit of the config1 register is
> +				 * unused and should return zero when read, so
> +				 * should the second to last bit of config1
> +				 * (software reset)
> +				 */
>  			 	kind = max6680;
> -			} else
> -			/* The chip_id register of the MAX6646/6647/6649
> -			 * holds the revision of the chip.
> -			 * The lowest 6 bits of the config1 register are
> -			 * unused and should return zero when read.
> -			 */
> -			if (chip_id == 0x59
> -			 && (reg_config1 & 0x3f) == 0x00
> -			 && reg_convrate <= 0x07) {
> +			} else if (chip_id == 0x59 &&
> +					(reg_config1 & 0x3f) == 0x00 &&
> +					 reg_convrate <= 0x07) {
> +				/*
> +				 * The chip_id register of the MAX6646/6647/6649
> +				 * holds the revision of the chip.
> +				 * The lowest 6 bits of the config1 register are
> +				 * unused and should return zero when read.
> +				 */
>  				kind = max6646;
>  			} else if (chip_id == 0x59 && address == 0x4C) {
>  				/*

Yes, something like this. But you know, the lm90 driver has a dedicated
maintainer, so you don't need to take care about this driver personally.

-- 
Jean Delvare

Re: [PATCH] lm90: Support the MAX6648/6692 chips

[Andrew Morton]

These three patches look like 2.6.29 material to me.  If none of
them pop up in linux-next I'll plan to merge them a few days from now.

OK?

Re: [PATCH] lm90: Support the MAX6648/6692 chips

[Jean Delvare]

Hi Andrew,

On Mon, 2 Mar 2009 15:07:42 -0800, Andrew Morton wrote:
> These three patches look like 2.6.29 material to me.  If none of
> them pop up in linux-next I'll plan to merge them a few days from now.

As the lm90 driver maintainer, I will take care of the lm90 patch. Feel
free to handle the lm85 patches if you want.

Thanks,
-- 
Jean Delvare

Re: [PATCH] lm90: Support the MAX6648/6692 chips

[Jean Delvare]

Hi Darrick

On Mon, 2 Mar 2009 13:01:06 -0800, Darrick J. Wong wrote:
> The max6648 chip has nearly the same register set as the 6657 and
> seems to have a working manufacturer/chip id so we can detect it.
> This patch adds support for it.  Tested on a Nvidia Quadro FX 1500
> card.
> 
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> ---
> 
>  drivers/hwmon/Kconfig |    4 ++--
>  drivers/hwmon/lm90.c  |    7 ++++++-
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 8c312c6..7da49d3 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -550,8 +550,8 @@ config SENSORS_LM90
>  	help
>  	  If you say yes here you get support for National Semiconductor LM90,
>  	  LM86, LM89 and LM99, Analog Devices ADM1032 and ADT7461, and Maxim
> -	  MAX6646, MAX6647, MAX6649, MAX6657, MAX6658, MAX6659, MAX6680 and
> -	  MAX6681 sensor chips.
> +	  MAX6646, MAX6647, MAX6649, MAX6657, MAX6658, MAX6659, MAX6680,
> +	  MAX6648, MAX6692 and MAX6681 sensor chips.
>  
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called lm90.
> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> index 96a7018..1802366 100644
> --- a/drivers/hwmon/lm90.c
> +++ b/drivers/hwmon/lm90.c
> @@ -776,7 +776,12 @@ static int lm90_detect(struct i2c_client *new_client, int kind,
>  			 && (reg_config1 & 0x3f) == 0x00
>  			 && reg_convrate <= 0x07) {
>  				kind = max6646;
> -			}
> +			} else
> +			/* The MAX6648/6692 chips have a working man/chip id
> +			 * and the same register set as the 6657.
> +			 */
> +			if (chip_id == 0x59 && address == 0x4C)
> +				kind = max6657;
>  		}
>  
>  		if (kind <= 0) { /* identification failed */

I am confused. According to my notes, the MAX6648/MAX6692 is the same
chip as the MAX6646/MAX6647/MAX6649 (same chip ID of 0x59), the only
difference being the I2C address (0x4c for the MAX6646, 0x4e for the
MAX6647 and 0x4d for the MAX6648/MAX6649/MAX6692). So the current code
should _already_ detect your MAX6648 or MAX6692 as kind = max6646.

Can you please test the latest version of the sensors-detect script [1]
and let me know if your chip is properly detected? If not, please
provide a dump of your chip.

[1] http://www.lm-sensors.org/svn/lm-sensors/trunk/prog/detect/sensors-detect

-- 
Jean Delvare

Re: [PATCH] lm90: Support the MAX6648/6692 chips

[Darrick J. Wong]

On Thu, Mar 05, 2009 at 03:25:17PM +0100, Jean Delvare wrote:
> Hi Darrick
> I am confused. According to my notes, the MAX6648/MAX6692 is the same
> chip as the MAX6646/MAX6647/MAX6649 (same chip ID of 0x59), the only
> difference being the I2C address (0x4c for the MAX6646, 0x4e for the
> MAX6647 and 0x4d for the MAX6648/MAX6649/MAX6692). So the current code
> should _already_ detect your MAX6648 or MAX6692 as kind = max6646.

Heh, yep, it does.  I guess this patch has been sitting around long
enough to become obsolete, sorry for the unnecessary mail traffic.  I
guess we can drop this one.

> Can you please test the latest version of the sensors-detect script [1]
> and let me know if your chip is properly detected? If not, please
> provide a dump of your chip.

Just for fun, here's a dump of that chip (6648):

No size specified (using byte-data access)
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
00: 15 1d 00 00 05 69 00 6e 05 05 05 05 05 05 05 05    ??..?i.n????????
10: e0 c0 c0 c0 c0 c0 c0 c0 00 91 91 91 91 91 91 91    ????????.???????
20: 91 0a 86 86 86 86 86 86 86 86 86 86 86 86 86 86    ????????????????
30: 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86    ????????????????
40: 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86    ????????????????
50: 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86    ????????????????
60: 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86    ????????????????
70: 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86    ????????????????
80: 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86    ????????????????
90: 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86    ????????????????
a0: 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86    ????????????????
b0: 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86    ????????????????
c0: 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86    ????????????????
d0: 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86    ????????????????
e0: 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86 86    ????????????????
f0: 86 86 86 86 86 86 86 86 86 86 86 86 86 86 4d 59    ??????????????MY

> [1] http://www.lm-sensors.org/svn/lm-sensors/trunk/prog/detect/sensors-detect

This also detects it properly.

--D

Re: [PATCH] lm90: Support the MAX6648/6692 chips

[Jean Delvare]

Hi Darrick,

On Thu, 5 Mar 2009 07:47:30 -0800, Darrick J. Wong wrote:
> On Thu, Mar 05, 2009 at 03:25:17PM +0100, Jean Delvare wrote:
> > I am confused. According to my notes, the MAX6648/MAX6692 is the same
> > chip as the MAX6646/MAX6647/MAX6649 (same chip ID of 0x59), the only
> > difference being the I2C address (0x4c for the MAX6646, 0x4e for the
> > MAX6647 and 0x4d for the MAX6648/MAX6649/MAX6692). So the current code
> > should _already_ detect your MAX6648 or MAX6692 as kind = max6646.
> 
> Heh, yep, it does.  I guess this patch has been sitting around long
> enough to become obsolete, sorry for the unnecessary mail traffic.  I
> guess we can drop this one.

Well, confusion probably would have been avoided if it had been
properly documented that the MAX6648 and MAX6692 were already
supported. So I think we should still apply the Kconfig part of your
patch. Documentation/hwmon/lm90 would need a small update as well.
Could you please send an updated patch doing this? I would be happy to
apply that.

Thanks,
-- 
Jean Delvare

[PATCH] lm90: Document support for the MAX6648/6692 chips

[Darrick J. Wong]

Update documentation to prevent further confusion/duplication.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/hwmon/Kconfig |    4 ++--
 drivers/hwmon/lm90.c  |    8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 8c312c6..7da49d3 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -550,8 +550,8 @@ config SENSORS_LM90
 	help
 	  If you say yes here you get support for National Semiconductor LM90,
 	  LM86, LM89 and LM99, Analog Devices ADM1032 and ADT7461, and Maxim
-	  MAX6646, MAX6647, MAX6649, MAX6657, MAX6658, MAX6659, MAX6680 and
-	  MAX6681 sensor chips.
+	  MAX6646, MAX6647, MAX6649, MAX6657, MAX6658, MAX6659, MAX6680,
+	  MAX6648, MAX6692 and MAX6681 sensor chips.
 
 	  This driver can also be built as a module.  If so, the module
 	  will be called lm90.
diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index 96a7018..58012b7 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -32,10 +32,10 @@
  * supported by this driver. These chips lack the remote temperature
  * offset feature.
  *
- * This driver also supports the MAX6646, MAX6647 and MAX6649 chips
- * made by Maxim.  These are again similar to the LM86, but they use
- * unsigned temperature values and can report temperatures from 0 to
- * 145 degrees.
+ * This driver also supports the MAX6646, MAX6647, MAX6648, MAX6649, and
+ * MAX6692 chips made by Maxim.  These are again similar to the LM86,
+ * but they use unsigned temperature values and can report temperatures
+ * from 0 to 145 degrees.
  *
  * This driver also supports the MAX6680 and MAX6681, two other sensor
  * chips made by Maxim. These are quite similar to the other Maxim

Re: [PATCH] lm90: Document support for the MAX6648/6692 chips

[Jean Delvare]

On Thu, 5 Mar 2009 09:37:33 -0800, Darrick J. Wong wrote:
> Update documentation to prevent further confusion/duplication.
> 
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> ---
> 
>  drivers/hwmon/Kconfig |    4 ++--
>  drivers/hwmon/lm90.c  |    8 ++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 8c312c6..7da49d3 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -550,8 +550,8 @@ config SENSORS_LM90
>  	help
>  	  If you say yes here you get support for National Semiconductor LM90,
>  	  LM86, LM89 and LM99, Analog Devices ADM1032 and ADT7461, and Maxim
> -	  MAX6646, MAX6647, MAX6649, MAX6657, MAX6658, MAX6659, MAX6680 and
> -	  MAX6681 sensor chips.
> +	  MAX6646, MAX6647, MAX6649, MAX6657, MAX6658, MAX6659, MAX6680,
> +	  MAX6648, MAX6692 and MAX6681 sensor chips.
>  
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called lm90.
> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> index 96a7018..58012b7 100644
> --- a/drivers/hwmon/lm90.c
> +++ b/drivers/hwmon/lm90.c
> @@ -32,10 +32,10 @@
>   * supported by this driver. These chips lack the remote temperature
>   * offset feature.
>   *
> - * This driver also supports the MAX6646, MAX6647 and MAX6649 chips
> - * made by Maxim.  These are again similar to the LM86, but they use
> - * unsigned temperature values and can report temperatures from 0 to
> - * 145 degrees.
> + * This driver also supports the MAX6646, MAX6647, MAX6648, MAX6649, and
> + * MAX6692 chips made by Maxim.  These are again similar to the LM86,
> + * but they use unsigned temperature values and can report temperatures
> + * from 0 to 145 degrees.
>   *
>   * This driver also supports the MAX6680 and MAX6681, two other sensor
>   * chips made by Maxim. These are quite similar to the other Maxim

Applied, thanks!

-- 
Jean Delvare

[PATCH] lm90: Update Documentation/hwmon/lm90 to reflect max6648/92 support

[Darrick J. Wong]

Oops, forgot this piece.
---
Update Documentation/hwmon/lm90 to add max6648/92 support.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 Documentation/hwmon/lm90 |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/Documentation/hwmon/lm90 b/Documentation/hwmon/lm90
index 0e84117..ab36eb0 100644
--- a/Documentation/hwmon/lm90
+++ b/Documentation/hwmon/lm90
@@ -42,6 +42,11 @@ Supported chips:
     Addresses scanned: I2C 0x4e
     Datasheet: Publicly available at the Maxim website
                http://www.maxim-ic.com/quick_view2.cfm/qv_pk/3497
+  * Maxim MAX6648/MAX6692
+    Prefix: 'max6646'
+    Addresses scanned: I2C 0x4c
+    Datasheet: Publicly available at the Maxim website
+               http://www.maxim-ic.com/getds.cfm/qv_pk/3500
   * Maxim MAX6649
     Prefix: 'max6646'
     Addresses scanned: I2C 0x4c