[PATCH] hwmon/lm70: adding support for NS LM74 chip

[PATCH] hwmon/lm70: adding support for NS LM74 chip

[Christophe Leroy]

Hello,

This Patch adds support for the LM74 chip from National Semiconductor (NS) 
in the lm70 driver

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

diff -u linux-3.5-vanilla/drivers/hwmon/Kconfig linux-3.5/drivers/hwmon/Kconfig
--- linux-3.5-vanilla/drivers/hwmon/Kconfig	2012-07-21 22:58:29.000000000 +0200
+++ linux-3.5/drivers/hwmon/Kconfig	2012-08-21 13:46:47.000000000 +0200
@@ -530,11 +530,11 @@
 	  will be called lm63.
 
 config SENSORS_LM70
-	tristate "National Semiconductor LM70 / Texas Instruments TMP121"
+	tristate "National Semiconductor LM70&LM74 / Texas Instruments TMP121"
 	depends on SPI_MASTER
 	help
 	  If you say yes here you get support for the National Semiconductor
-	  LM70 and Texas Instruments TMP121/TMP123 digital temperature
+	  LM70 and LM74 and Texas Instruments TMP121/TMP123 digital temperature
 	  sensor chips.
 
 	  This driver can also be built as a module.  If so, the module
diff -u linux-3.5-vanilla/drivers/hwmon/lm70.c linux-3.5/drivers/hwmon/lm70.c
--- linux-3.5-vanilla/drivers/hwmon/lm70.c	2012-07-21 22:58:29.000000000 +0200
+++ linux-3.5/drivers/hwmon/lm70.c	2012-08-21 13:46:31.000000000 +0200
@@ -3,11 +3,12 @@
  *
  * The LM70 is a temperature sensor chip from National Semiconductor (NS).
  * Copyright (C) 2006 Kaiwan N Billimoria <kaiwan@designergraphix.com>
+ * Copyright (C) 2012 Christophe Leroy <christophe.leroy@c-s.fr>
  *
  * The LM70 communicates with a host processor via an SPI/Microwire Bus
  * interface. The complete datasheet is available at National's website
  * here:
- * http://www.national.com/pf/LM/LM70.html
+ * http://www.ti.com/product/LM70
  *
  * 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
@@ -43,6 +44,7 @@
 
 #define LM70_CHIP_LM70		0	/* original NS LM70 */
 #define LM70_CHIP_TMP121	1	/* TI TMP121/TMP123 */
+#define LM70_CHIP_LM74		2	/* original NS LM74 */
 
 struct lm70 {
 	struct device *hwmon_dev;
@@ -98,6 +100,7 @@
 		break;
 
 	case LM70_CHIP_TMP121:
+	case LM70_CHIP_LM74:
 		val = ((int)raw / 8) * 625 / 10;
 		break;
 	}
@@ -123,6 +126,9 @@
 	case LM70_CHIP_TMP121:
 		ret = sprintf(buf, "tmp121\n");
 		break;
+	case LM70_CHIP_LM74:
+		ret = sprintf(buf, "lm74\n");
+		break;
 	default:
 		ret = -EINVAL;
 	}
@@ -139,12 +145,13 @@
 	struct lm70 *p_lm70;
 	int status;
 
-	/* signaling is SPI_MODE_0 for both LM70 and TMP121 */
+	/* signaling is SPI_MODE_0 for LM70, LM74 and TMP121 */
 	if (spi->mode & (SPI_CPOL | SPI_CPHA))
 		return -EINVAL;
 
-	/* 3-wire link (shared SI/SO) for LM70 */
-	if (chip == LM70_CHIP_LM70 && !(spi->mode & SPI_3WIRE))
+	/* 3-wire link (shared SI/SO) for LM70 and LM74 */
+	if ((chip == LM70_CHIP_LM70 || chip == LM70_CHIP_LM74)
+			&& !(spi->mode & SPI_3WIRE))
 		return -EINVAL;
 
 	/* NOTE:  we assume 8-bit words, and convert to 16 bits manually */
@@ -202,6 +209,7 @@
 static const struct spi_device_id lm70_ids[] = {
 	{ "lm70",   LM70_CHIP_LM70 },
 	{ "tmp121", LM70_CHIP_TMP121 },
+	{ "lm74",   LM70_CHIP_LM74 },
 	{ },
 };
 MODULE_DEVICE_TABLE(spi, lm70_ids);
@@ -219,5 +227,5 @@
 module_spi_driver(lm70_driver);
 
 MODULE_AUTHOR("Kaiwan N Billimoria");
-MODULE_DESCRIPTION("NS LM70 / TI TMP121/TMP123 Linux driver");
+MODULE_DESCRIPTION("NS LM70 & LM74 / TI TMP121/TMP123 Linux driver");
 MODULE_LICENSE("GPL");
diff -u linux-3.5-vanilla/Documentation/hwmon/lm70 linux-3.5/Documentation/hwmon/lm70
--- linux-3.5-vanilla/Documentation/hwmon/lm70	2012-07-21 22:58:29.000000000 +0200
+++ linux-3.5/Documentation/hwmon/lm70	2012-08-21 13:38:24.000000000 +0200
@@ -3,7 +3,9 @@
 
 Supported chips:
   * National Semiconductor LM70
-    Datasheet: http://www.national.com/pf/LM/LM70.html
+    Datasheet: http://www.ti.com/product/LM70
+  * National Semiconductor LM74
+    Datasheet: http://www.ti.com/product/LM74
   * Texas Instruments TMP121/TMP123
     Information: http://focus.ti.com/docs/prod/folders/print/tmp121.html
 
@@ -31,9 +33,11 @@
 with a "SPI master controller driver", see drivers/spi/spi_lm70llp.c
 and its associated documentation.
 
-The TMP121/TMP123 are very similar; main differences are 4 wire SPI inter-
-face (read only) and 13-bit temperature data (0.0625 degrees celsius reso-
-lution).
+The LM74 is very similar; main differences is 13-bit temperature data 
+(0.0625 degrees celsius resolution).
+
+The TMP121/TMP123 are very similar to the LM74; main difference is 4 wire
+SPI interface (read only).
 
 Thanks to
 ---------

Re: [PATCH] hwmon/lm70: adding support for NS LM74 chip

[Guenter Roeck]

On Thu, Aug 23, 2012 at 05:32:12PM +0200, Christophe Leroy wrote:
> Hello,
> 
Hi Christophe,

Just a nitpick, but as written the Hello is added to the commit log.

> This Patch adds support for the LM74 chip from National Semiconductor (NS) 
> in the lm70 driver
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> 
> diff -u linux-3.5-vanilla/drivers/hwmon/Kconfig linux-3.5/drivers/hwmon/Kconfig
> --- linux-3.5-vanilla/drivers/hwmon/Kconfig	2012-07-21 22:58:29.000000000 +0200
> +++ linux-3.5/drivers/hwmon/Kconfig	2012-08-21 13:46:47.000000000 +0200
> @@ -530,11 +530,11 @@
>  	  will be called lm63.
>  
>  config SENSORS_LM70
> -	tristate "National Semiconductor LM70 / Texas Instruments TMP121"
> +	tristate "National Semiconductor LM70&LM74 / Texas Instruments TMP121"

Make it "LM70 and compatibles". There are other chips which should work as well,
such as LM71 and TMP122/TMP124.

>  	depends on SPI_MASTER
>  	help
>  	  If you say yes here you get support for the National Semiconductor
> -	  LM70 and Texas Instruments TMP121/TMP123 digital temperature
> +	  LM70 and LM74 and Texas Instruments TMP121/TMP123 digital temperature
>  	  sensor chips.
>  
>  	  This driver can also be built as a module.  If so, the module
> diff -u linux-3.5-vanilla/drivers/hwmon/lm70.c linux-3.5/drivers/hwmon/lm70.c
> --- linux-3.5-vanilla/drivers/hwmon/lm70.c	2012-07-21 22:58:29.000000000 +0200
> +++ linux-3.5/drivers/hwmon/lm70.c	2012-08-21 13:46:31.000000000 +0200
> @@ -3,11 +3,12 @@
>   *
>   * The LM70 is a temperature sensor chip from National Semiconductor (NS).
>   * Copyright (C) 2006 Kaiwan N Billimoria <kaiwan@designergraphix.com>
> + * Copyright (C) 2012 Christophe Leroy <christophe.leroy@c-s.fr>

Not for a couple of lines of code.

>   *
>   * The LM70 communicates with a host processor via an SPI/Microwire Bus
>   * interface. The complete datasheet is available at National's website
>   * here:
> - * http://www.national.com/pf/LM/LM70.html
> + * http://www.ti.com/product/LM70

Updating the data sheet link for LM70 would be a second patch. Only one change
per patch, please (even more so since the old link still works).

>   *
>   * 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
> @@ -43,6 +44,7 @@
>  
>  #define LM70_CHIP_LM70		0	/* original NS LM70 */
>  #define LM70_CHIP_TMP121	1	/* TI TMP121/TMP123 */
> +#define LM70_CHIP_LM74		2	/* original NS LM74 */
>  
Drop the "original" here. All chips are "original". The "original LM70"
statement above probably refers to the first chip supported by this driver.

>  struct lm70 {
>  	struct device *hwmon_dev;
> @@ -98,6 +100,7 @@
>  		break;
>  
>  	case LM70_CHIP_TMP121:
> +	case LM70_CHIP_LM74:
>  		val = ((int)raw / 8) * 625 / 10;
>  		break;
>  	}
> @@ -123,6 +126,9 @@
>  	case LM70_CHIP_TMP121:
>  		ret = sprintf(buf, "tmp121\n");
>  		break;
> +	case LM70_CHIP_LM74:
> +		ret = sprintf(buf, "lm74\n");
> +		break;
>  	default:
>  		ret = -EINVAL;
>  	}
> @@ -139,12 +145,13 @@
>  	struct lm70 *p_lm70;
>  	int status;
>  
> -	/* signaling is SPI_MODE_0 for both LM70 and TMP121 */
> +	/* signaling is SPI_MODE_0 for LM70, LM74 and TMP121 */
>  	if (spi->mode & (SPI_CPOL | SPI_CPHA))
>  		return -EINVAL;
>  
> -	/* 3-wire link (shared SI/SO) for LM70 */
> -	if (chip == LM70_CHIP_LM70 && !(spi->mode & SPI_3WIRE))
> +	/* 3-wire link (shared SI/SO) for LM70 and LM74 */
> +	if ((chip == LM70_CHIP_LM70 || chip == LM70_CHIP_LM74)
> +			&& !(spi->mode & SPI_3WIRE))

Ok for now, but I'll have to check this. The driver does not really write
anything to the chip, so it should be irrelevant which mode the SPI master
controller supports (MOSI does not have to be connected). Besides, from the
chip specification it looks like it is possible to connect the chip to a
standard SPI interface by adding a 10k resistor between MOSI and MISO.

>  		return -EINVAL;
>  
>  	/* NOTE:  we assume 8-bit words, and convert to 16 bits manually */
> @@ -202,6 +209,7 @@
>  static const struct spi_device_id lm70_ids[] = {
>  	{ "lm70",   LM70_CHIP_LM70 },
>  	{ "tmp121", LM70_CHIP_TMP121 },
> +	{ "lm74",   LM70_CHIP_LM74 },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(spi, lm70_ids);
> @@ -219,5 +227,5 @@
>  module_spi_driver(lm70_driver);
>  
>  MODULE_AUTHOR("Kaiwan N Billimoria");
> -MODULE_DESCRIPTION("NS LM70 / TI TMP121/TMP123 Linux driver");
> +MODULE_DESCRIPTION("NS LM70 & LM74 / TI TMP121/TMP123 Linux driver");
>  MODULE_LICENSE("GPL");
> diff -u linux-3.5-vanilla/Documentation/hwmon/lm70 linux-3.5/Documentation/hwmon/lm70
> --- linux-3.5-vanilla/Documentation/hwmon/lm70	2012-07-21 22:58:29.000000000 +0200
> +++ linux-3.5/Documentation/hwmon/lm70	2012-08-21 13:38:24.000000000 +0200
> @@ -3,7 +3,9 @@
>  
>  Supported chips:
>    * National Semiconductor LM70
> -    Datasheet: http://www.national.com/pf/LM/LM70.html
> +    Datasheet: http://www.ti.com/product/LM70

Again, separate patch

> +  * National Semiconductor LM74
> +    Datasheet: http://www.ti.com/product/LM74
>    * Texas Instruments TMP121/TMP123
>      Information: http://focus.ti.com/docs/prod/folders/print/tmp121.html
>  
> @@ -31,9 +33,11 @@
>  with a "SPI master controller driver", see drivers/spi/spi_lm70llp.c
>  and its associated documentation.
>  
> -The TMP121/TMP123 are very similar; main differences are 4 wire SPI inter-
> -face (read only) and 13-bit temperature data (0.0625 degrees celsius reso-
> -lution).
> +The LM74 is very similar; main differences is 13-bit temperature data 

ERROR: trailing whitespace
#131: FILE: Documentation/hwmon/lm70:36:
+The LM74 is very similar; main differences is 13-bit temperature data $

> +(0.0625 degrees celsius resolution).
> +
> +The TMP121/TMP123 are very similar to the LM74; main difference is 4 wire
> +SPI interface (read only).
>  
>  Thanks to
>  ---------
> 

Re: [PATCH] hwmon/lm70: adding support for NS LM74 chip

[Guenter Roeck]

On Thu, Aug 23, 2012 at 11:13:17AM -0700, Guenter Roeck wrote:
> On Thu, Aug 23, 2012 at 05:32:12PM +0200, Christophe Leroy wrote:
> > Hello,
> > 
> Hi Christophe,
> 
Hi again,

[ ... ]

> >  
> > -	/* 3-wire link (shared SI/SO) for LM70 */
> > -	if (chip == LM70_CHIP_LM70 && !(spi->mode & SPI_3WIRE))
> > +	/* 3-wire link (shared SI/SO) for LM70 and LM74 */
> > +	if ((chip == LM70_CHIP_LM70 || chip == LM70_CHIP_LM74)
> > +			&& !(spi->mode & SPI_3WIRE))
> 
> Ok for now, but I'll have to check this. The driver does not really write
> anything to the chip, so it should be irrelevant which mode the SPI master
> controller supports (MOSI does not have to be connected). Besides, from the
> chip specification it looks like it is possible to connect the chip to a
> standard SPI interface by adding a 10k resistor between MOSI and MISO.
> 
After looking into the above, the proper change or fix is really to remove the
above check for SPI_3WIRE entirely. The driver works perfectly fine if a LM70 is
connected to a controller which does not explicitly support SPI_3WIRE; all that
needs to be done is to connect MOSI through a resistor or not at all. This is,
however, a board design decision, not a SPI master chip limitation.

With that, we don't need to explicitly add any LM74 specific code to the driver,
since the TMP121 code does the trick. All we need to do is to update the
documentation, adding a note that LM74 is supported as well, and that it can be
selected with 'tmp121'.

Thanks,
Guenter

Re: [PATCH] hwmon/lm70: adding support for NS LM74 chip

[leroy christophe]

Le 26/08/2012 19:47, Guenter Roeck a écrit :
> On Thu, Aug 23, 2012 at 11:13:17AM -0700, Guenter Roeck wrote:
>> On Thu, Aug 23, 2012 at 05:32:12PM +0200, Christophe Leroy wrote:
>>> Hello,
>>>
>> Hi Christophe,
>>
> Hi again,
>
> [ ... ]
Hi Guenter,

>>>   
>>> -	/* 3-wire link (shared SI/SO) for LM70 */
>>> -	if (chip == LM70_CHIP_LM70 && !(spi->mode & SPI_3WIRE))
>>> +	/* 3-wire link (shared SI/SO) for LM70 and LM74 */
>>> +	if ((chip == LM70_CHIP_LM70 || chip == LM70_CHIP_LM74)
>>> +			&& !(spi->mode & SPI_3WIRE))
>> Ok for now, but I'll have to check this. The driver does not really write
>> anything to the chip, so it should be irrelevant which mode the SPI master
>> controller supports (MOSI does not have to be connected). Besides, from the
>> chip specification it looks like it is possible to connect the chip to a
>> standard SPI interface by adding a 10k resistor between MOSI and MISO.
>>
> After looking into the above, the proper change or fix is really to remove the
> above check for SPI_3WIRE entirely. The driver works perfectly fine if a LM70 is
> connected to a controller which does not explicitly support SPI_3WIRE; all that
> needs to be done is to connect MOSI through a resistor or not at all. This is,
> however, a board design decision, not a SPI master chip limitation.
Ok, then I'll prepare a patch for this.
>
> With that, we don't need to explicitly add any LM74 specific code to the driver,
> since the TMP121 code does the trick. All we need to do is to update the
> documentation, adding a note that LM74 is supported as well, and that it can be
> selected with 'tmp121'.
Yes, could be the solution, but I'm however a bit puzzled with doing 
this. The driver is originaly for NS LM70 component. Later on an 
extension has been added for TMP121 which is from a different family, 
even if compatible. Now we are talking about the LM74 which is in the 
same family as the original LM70. Therefore I would expect to be able to 
handle the LM74 directly from this driver, not through a tricky 
compatibility with the 'tmp121'. I would expect to handle it the other 
way round, ie consider the 'tmp121' as compatible with the LM74.
One of the reasons for that is that the driver reports the component ID 
through /sys/class/hwmon/hwmonX/name

As the 'tmp121' is already in the driver, it is not a good idea to 
replace it by LM74, however, I would prefer having the LM74 explicitly 
addressable by the driver, in additional to the 'tmp121'

I'm preparing a patch to include support for both LM71 and LM74 in the 
LM70 driver, in order to have a driver handling the complete family. Do 
you see a problem for that ? It would look strange that the LM70 driver 
handles LM70 and LM71, but LM74 through the compatible 'tmp121'

Nb, refer to your first answer, implementation for 'tmp122/124' is 
another story because those have a programmable precision, not a fixed one.
>
> Thanks,
> Guenter
>
Thanks
Christophe

Re: [PATCH] hwmon/lm70: adding support for NS LM74 chip

[Guenter Roeck]

On Tue, Aug 28, 2012 at 02:09:00PM +0200, leroy christophe wrote:
> 
> Le 26/08/2012 19:47, Guenter Roeck a écrit :
> >On Thu, Aug 23, 2012 at 11:13:17AM -0700, Guenter Roeck wrote:
> >>On Thu, Aug 23, 2012 at 05:32:12PM +0200, Christophe Leroy wrote:
> >>>Hello,
> >>>
> >>Hi Christophe,
> >>
> >Hi again,
> >
> >[ ... ]
> Hi Guenter,
> 
> >>>-	/* 3-wire link (shared SI/SO) for LM70 */
> >>>-	if (chip == LM70_CHIP_LM70 && !(spi->mode & SPI_3WIRE))
> >>>+	/* 3-wire link (shared SI/SO) for LM70 and LM74 */
> >>>+	if ((chip == LM70_CHIP_LM70 || chip == LM70_CHIP_LM74)
> >>>+			&& !(spi->mode & SPI_3WIRE))
> >>Ok for now, but I'll have to check this. The driver does not really write
> >>anything to the chip, so it should be irrelevant which mode the SPI master
> >>controller supports (MOSI does not have to be connected). Besides, from the
> >>chip specification it looks like it is possible to connect the chip to a
> >>standard SPI interface by adding a 10k resistor between MOSI and MISO.
> >>
> >After looking into the above, the proper change or fix is really to remove the
> >above check for SPI_3WIRE entirely. The driver works perfectly fine if a LM70 is
> >connected to a controller which does not explicitly support SPI_3WIRE; all that
> >needs to be done is to connect MOSI through a resistor or not at all. This is,
> >however, a board design decision, not a SPI master chip limitation.
> Ok, then I'll prepare a patch for this.
> >
> >With that, we don't need to explicitly add any LM74 specific code to the driver,
> >since the TMP121 code does the trick. All we need to do is to update the
> >documentation, adding a note that LM74 is supported as well, and that it can be
> >selected with 'tmp121'.
> Yes, could be the solution, but I'm however a bit puzzled with doing
> this. The driver is originaly for NS LM70 component. Later on an
> extension has been added for TMP121 which is from a different
> family, even if compatible. Now we are talking about the LM74 which
> is in the same family as the original LM70. Therefore I would expect
> to be able to handle the LM74 directly from this driver, not through
> a tricky compatibility with the 'tmp121'. I would expect to handle
> it the other way round, ie consider the 'tmp121' as compatible with
> the LM74.
> One of the reasons for that is that the driver reports the component
> ID through /sys/class/hwmon/hwmonX/name
> 
> As the 'tmp121' is already in the driver, it is not a good idea to
> replace it by LM74, however, I would prefer having the LM74
> explicitly addressable by the driver, in additional to the 'tmp121'
> 
> I'm preparing a patch to include support for both LM71 and LM74 in
> the LM70 driver, in order to have a driver handling the complete
> family. Do you see a problem for that ? It would look strange that
> the LM70 driver handles LM70 and LM71, but LM74 through the
> compatible 'tmp121'
> 
Ok, I'll accept that.

> Nb, refer to your first answer, implementation for 'tmp122/124' is
> another story because those have a programmable precision, not a
> fixed one.

That isn't really a reason - we could just assume the highest precision, since
the chip returns 0 in the lower bits if it is not configured for it. But then
we can as well wait until someone asks for it, which will probably never happen.
The more intersting aspect of the tmp122/124 is that it has limit registers,
and supporting those would be more interesting anyway.

Thanks,
Guenter