[lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[Manuel Lauss]

The Texas Instruments TMP121 is a SPI temperature sensor very similar
to the LM70, with slightly higher resolution.  This patch extends the
LM70 driver to support the TMP121.  Chip type is determined through
SPI platform data.

Signed-off-by: Manuel Lauss <mano@roarinelk.homelinux.net>
---
 Documentation/hwmon/lm70 |    7 +++++-
 drivers/hwmon/Kconfig    |    5 ++-
 drivers/hwmon/lm70.c     |   56 ++++++++++++++++++++++++++++++++++++++++-----
 include/linux/spi/lm70.h |   15 ++++++++++++
 4 files changed, 73 insertions(+), 10 deletions(-)
 create mode 100644 include/linux/spi/lm70.h

diff --git a/Documentation/hwmon/lm70 b/Documentation/hwmon/lm70
index 2bdd3fe..88f4322 100644
--- a/Documentation/hwmon/lm70
+++ b/Documentation/hwmon/lm70
@@ -1,9 +1,11 @@
 Kernel driver lm70
 =========
 
-Supported chip:
+Supported chips:
   * National Semiconductor LM70
     Datasheet: http://www.national.com/pf/LM/LM70.html
+  * Texas Instruments TMP121/TMP123
+    Datasheet: http://www.ti.com/lit/gpn/tmp121
 
 Author:
         Kaiwan N Billimoria <kaiwan@designergraphix.com>
@@ -25,6 +27,9 @@ complement digital temperature (sent via the SIO line), is available in the
 driver for interpretation. This driver makes use of the kernel's in-core
 SPI support.
 
+The TMP121 is very similar; main differences are 4 wire SPI interface,
+and 13bit temperature data (0.0625 celsius resolution).
+
 Thanks to
 ---------
 Jean Delvare <khali@linux-fr.org> for mentoring the hwmon-side driver
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 6de1e0f..f86923d 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -407,11 +407,12 @@ config SENSORS_LM63
 	  will be called lm63.
 
 config SENSORS_LM70
-	tristate "National Semiconductor LM70"
+	tristate "National Semiconductor LM70 / Texas Instruments TMP121"
 	depends on SPI_MASTER && EXPERIMENTAL
 	help
 	  If you say yes here you get support for the National Semiconductor
-	  LM70 digital temperature sensor chip.
+	  LM70 and Texas Instruments TMP121/TMP123 digital temperature
+	  sensor chips.
 
 	  This driver can also be built as a module.  If so, the module
 	  will be called lm70.
diff --git a/drivers/hwmon/lm70.c b/drivers/hwmon/lm70.c
index d435f00..9ab0479 100644
--- a/drivers/hwmon/lm70.c
+++ b/drivers/hwmon/lm70.c
@@ -33,13 +33,14 @@
 #include <linux/hwmon.h>
 #include <linux/mutex.h>
 #include <linux/spi/spi.h>
-
+#include <linux/spi/lm70.h>
 
 #define DRVNAME		"lm70"
 
 struct lm70 {
 	struct device *hwmon_dev;
 	struct mutex lock;
+	unsigned int chip;
 };
 
 /* sysfs hook function */
@@ -67,10 +68,8 @@ static ssize_t lm70_sense_temp(struct device *dev,
 	}
 	dev_dbg(dev, "rxbuf[1] : 0x%x rxbuf[0] : 0x%x\n", rxbuf[1], rxbuf[0]);
 
-	raw = (rxbuf[1] << 8) + rxbuf[0];
-	dev_dbg(dev, "raw=0x%x\n", raw);
-
 	/*
+	 * LM70:
 	 * The "raw" temperature read into rxbuf[] is a 16-bit signed 2's
 	 * complement value. Only the MSB 11 bits (1 sign + 10 temperature
 	 * bits) are meaningful; the LSB 5 bits are to be discarded.
@@ -80,8 +79,27 @@ static ssize_t lm70_sense_temp(struct device *dev,
 	 * by 0.25. Also multiply by 1000 to represent in millidegrees
 	 * Celsius.
 	 * So it's equivalent to multiplying by 0.25 * 1000 = 250.
+	 *
+	 * TMP121:
+	 * 13bits of 2's complement data, discard lower 3 bits.  Chip
+	 * transmits high byte first.  Resolution 0.0625 celsius.
 	 */
-	val = ((int)raw/32) * 250;
+	switch (p_lm70->chip) {
+	case LM70_CHIP_LM70:
+		raw = (rxbuf[1] << 8) + rxbuf[0];
+		dev_dbg(dev, "raw=0x%x\n", raw);
+		val = ((int)raw/32) * 250;
+		break;
+
+	case LM70_CHIP_TMP121:
+		raw = (rxbuf[0] << 8) + rxbuf[1];
+		dev_dbg(dev, "raw=0x%x\n", raw);
+		val = (raw / 8) * 625 / 10;
+		break;
+
+	default:
+		raw = 0;
+	}
 	status = sprintf(buf, "%d\n", val); /* millidegrees Celsius */
 out:
 	mutex_unlock(&p_lm70->lock);
@@ -93,7 +111,21 @@ static DEVICE_ATTR(temp1_input, S_IRUGO, lm70_sense_temp, NULL);
 static ssize_t lm70_show_name(struct device *dev, struct device_attribute
 			      *devattr, char *buf)
 {
-	return sprintf(buf, "lm70\n");
+	struct spi_device *spi = to_spi_device(dev);
+	struct lm70 *p_lm70 = dev_get_drvdata(&spi->dev);
+	int ret;
+
+	switch (p_lm70->chip) {
+	case LM70_CHIP_LM70:
+		ret = sprintf(buf, "lm70\n");
+		break;
+	case LM70_CHIP_TMP121:
+		ret = sprintf(buf, "tmp121\n");
+		break;
+	default:
+		ret = -EINVAL;
+	}
+	return ret;
 }
 
 static DEVICE_ATTR(name, S_IRUGO, lm70_show_name, NULL);
@@ -102,11 +134,20 @@ static DEVICE_ATTR(name, S_IRUGO, lm70_show_name, NULL);
 
 static int __devinit lm70_probe(struct spi_device *spi)
 {
+	struct lm70_platform_data *pdata;
 	struct lm70 *p_lm70;
 	int status;
+	unsigned int chip;
+
+	pdata = spi->dev.platform_data;
+	if (pdata)
+		chip = pdata->chip;
+	else
+		chip = LM70_CHIP_LM70;
 
 	/* signaling is SPI_MODE_0 on a 3-wire link (shared SI/SO) */
-	if ((spi->mode & (SPI_CPOL|SPI_CPHA)) || !(spi->mode & SPI_3WIRE))
+	if ((chip = LM70_CHIP_LM70) &&
+	    ((spi->mode & (SPI_CPOL|SPI_CPHA)) || !(spi->mode & SPI_3WIRE)))
 		return -EINVAL;
 
 	p_lm70 = kzalloc(sizeof *p_lm70, GFP_KERNEL);
@@ -114,6 +155,7 @@ static int __devinit lm70_probe(struct spi_device *spi)
 		return -ENOMEM;
 
 	mutex_init(&p_lm70->lock);
+	p_lm70->chip = chip;
 
 	/* sysfs hook */
 	p_lm70->hwmon_dev = hwmon_device_register(&spi->dev);
diff --git a/include/linux/spi/lm70.h b/include/linux/spi/lm70.h
new file mode 100644
index 0000000..986c35a
--- /dev/null
+++ b/include/linux/spi/lm70.h
@@ -0,0 +1,15 @@
+/*
+ * LM70 platform data
+ */
+
+#ifndef _LM70_H_
+#define _LM70_H_
+
+#define LM70_CHIP_LM70		0
+#define LM70_CHIP_TMP121	1
+
+struct lm70_platform_data {
+	unsigned int chip;
+};
+
+#endif
-- 
1.6.0.2


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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[Jean Delvare]

Hi Manuel,

On Wed, 22 Oct 2008 10:04:23 +0200, Manuel Lauss wrote:
> The Texas Instruments TMP121 is a SPI temperature sensor very similar
> to the LM70, with slightly higher resolution.  This patch extends the
> LM70 driver to support the TMP121.  Chip type is determined through
> SPI platform data.

Review:

> 
> Signed-off-by: Manuel Lauss <mano@roarinelk.homelinux.net>
> ---
>  Documentation/hwmon/lm70 |    7 +++++-
>  drivers/hwmon/Kconfig    |    5 ++-
>  drivers/hwmon/lm70.c     |   56 ++++++++++++++++++++++++++++++++++++++++-----
>  include/linux/spi/lm70.h |   15 ++++++++++++
>  4 files changed, 73 insertions(+), 10 deletions(-)
>  create mode 100644 include/linux/spi/lm70.h
> 
> diff --git a/Documentation/hwmon/lm70 b/Documentation/hwmon/lm70
> index 2bdd3fe..88f4322 100644
> --- a/Documentation/hwmon/lm70
> +++ b/Documentation/hwmon/lm70
> @@ -1,9 +1,11 @@
>  Kernel driver lm70
>  =========
>  
> -Supported chip:
> +Supported chips:
>    * National Semiconductor LM70
>      Datasheet: http://www.national.com/pf/LM/LM70.html
> +  * Texas Instruments TMP121/TMP123
> +    Datasheet: http://www.ti.com/lit/gpn/tmp121

I'd rather link to:
http://focus.ti.com/docs/prod/folders/print/tmp121.html
Where people can see a lot of useful information about the chip, and
then download the datasheet.

>  
>  Author:
>          Kaiwan N Billimoria <kaiwan@designergraphix.com>
> @@ -25,6 +27,9 @@ complement digital temperature (sent via the SIO line), is available in the
>  driver for interpretation. This driver makes use of the kernel's in-core
>  SPI support.
>  
> +The TMP121 is very similar; main differences are 4 wire SPI interface,
> +and 13bit temperature data (0.0625 celsius resolution).

13-bit, degree Celsius

> +
>  Thanks to
>  ---------
>  Jean Delvare <khali@linux-fr.org> for mentoring the hwmon-side driver
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 6de1e0f..f86923d 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -407,11 +407,12 @@ config SENSORS_LM63
>  	  will be called lm63.
>  
>  config SENSORS_LM70
> -	tristate "National Semiconductor LM70"
> +	tristate "National Semiconductor LM70 / Texas Instruments TMP121"
>  	depends on SPI_MASTER && EXPERIMENTAL
>  	help
>  	  If you say yes here you get support for the National Semiconductor
> -	  LM70 digital temperature sensor chip.
> +	  LM70 and Texas Instruments TMP121/TMP123 digital temperature
> +	  sensor chips.
>  
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called lm70.
> diff --git a/drivers/hwmon/lm70.c b/drivers/hwmon/lm70.c
> index d435f00..9ab0479 100644
> --- a/drivers/hwmon/lm70.c
> +++ b/drivers/hwmon/lm70.c
> @@ -33,13 +33,14 @@
>  #include <linux/hwmon.h>
>  #include <linux/mutex.h>
>  #include <linux/spi/spi.h>
> -
> +#include <linux/spi/lm70.h>
>  
>  #define DRVNAME		"lm70"
>  
>  struct lm70 {
>  	struct device *hwmon_dev;
>  	struct mutex lock;
> +	unsigned int chip;
>  };
>  
>  /* sysfs hook function */
> @@ -67,10 +68,8 @@ static ssize_t lm70_sense_temp(struct device *dev,
>  	}
>  	dev_dbg(dev, "rxbuf[1] : 0x%x rxbuf[0] : 0x%x\n", rxbuf[1], rxbuf[0]);
>  
> -	raw = (rxbuf[1] << 8) + rxbuf[0];
> -	dev_dbg(dev, "raw=0x%x\n", raw);
> -
>  	/*
> +	 * LM70:
>  	 * The "raw" temperature read into rxbuf[] is a 16-bit signed 2's
>  	 * complement value. Only the MSB 11 bits (1 sign + 10 temperature
>  	 * bits) are meaningful; the LSB 5 bits are to be discarded.
> @@ -80,8 +79,27 @@ static ssize_t lm70_sense_temp(struct device *dev,
>  	 * by 0.25. Also multiply by 1000 to represent in millidegrees
>  	 * Celsius.
>  	 * So it's equivalent to multiplying by 0.25 * 1000 = 250.
> +	 *
> +	 * TMP121:
> +	 * 13bits of 2's complement data, discard lower 3 bits.  Chip
> +	 * transmits high byte first.  Resolution 0.0625 celsius.
>  	 */
> -	val = ((int)raw/32) * 250;
> +	switch (p_lm70->chip) {
> +	case LM70_CHIP_LM70:
> +		raw = (rxbuf[1] << 8) + rxbuf[0];
> +		dev_dbg(dev, "raw=0x%x\n", raw);

This debug statement is common to all cases so it could be moved
outside of the switch block.

> +		val = ((int)raw/32) * 250;
> +		break;
> +
> +	case LM70_CHIP_TMP121:
> +		raw = (rxbuf[0] << 8) + rxbuf[1];
> +		dev_dbg(dev, "raw=0x%x\n", raw);
> +		val = (raw / 8) * 625 / 10;
> +		break;
> +
> +	default:
> +		raw = 0;

This default case is broken, it sets raw but not val, while what the
following code uses is val. Anyway, I don't think you really need a
default here, the chip type should have already been checked as you
reach this portion of the driver code.

> +	}
>  	status = sprintf(buf, "%d\n", val); /* millidegrees Celsius */
>  out:
>  	mutex_unlock(&p_lm70->lock);
> @@ -93,7 +111,21 @@ static DEVICE_ATTR(temp1_input, S_IRUGO, lm70_sense_temp, NULL);
>  static ssize_t lm70_show_name(struct device *dev, struct device_attribute
>  			      *devattr, char *buf)
>  {
> -	return sprintf(buf, "lm70\n");
> +	struct spi_device *spi = to_spi_device(dev);
> +	struct lm70 *p_lm70 = dev_get_drvdata(&spi->dev);

Err, what about just:
	struct lm70 *p_lm70 = dev_get_drvdata(dev);
? You don't use spi anywhere else.

> +	int ret;
> +
> +	switch (p_lm70->chip) {
> +	case LM70_CHIP_LM70:
> +		ret = sprintf(buf, "lm70\n");
> +		break;
> +	case LM70_CHIP_TMP121:
> +		ret = sprintf(buf, "tmp121\n");
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +	return ret;
>  }
>  
>  static DEVICE_ATTR(name, S_IRUGO, lm70_show_name, NULL);
> @@ -102,11 +134,20 @@ static DEVICE_ATTR(name, S_IRUGO, lm70_show_name, NULL);
>  
>  static int __devinit lm70_probe(struct spi_device *spi)
>  {
> +	struct lm70_platform_data *pdata;
>  	struct lm70 *p_lm70;
>  	int status;
> +	unsigned int chip;
> +
> +	pdata = spi->dev.platform_data;
> +	if (pdata)
> +		chip = pdata->chip;
> +	else
> +		chip = LM70_CHIP_LM70;
>  
>  	/* signaling is SPI_MODE_0 on a 3-wire link (shared SI/SO) */
> -	if ((spi->mode & (SPI_CPOL|SPI_CPHA)) || !(spi->mode & SPI_3WIRE))
> +	if ((chip = LM70_CHIP_LM70) &&
> +	    ((spi->mode & (SPI_CPOL|SPI_CPHA)) || !(spi->mode & SPI_3WIRE)))
>  		return -EINVAL;
>  
>  	p_lm70 = kzalloc(sizeof *p_lm70, GFP_KERNEL);
> @@ -114,6 +155,7 @@ static int __devinit lm70_probe(struct spi_device *spi)
>  		return -ENOMEM;
>  
>  	mutex_init(&p_lm70->lock);
> +	p_lm70->chip = chip;
>  
>  	/* sysfs hook */
>  	p_lm70->hwmon_dev = hwmon_device_register(&spi->dev);
> diff --git a/include/linux/spi/lm70.h b/include/linux/spi/lm70.h
> new file mode 100644
> index 0000000..986c35a
> --- /dev/null
> +++ b/include/linux/spi/lm70.h
> @@ -0,0 +1,15 @@
> +/*
> + * LM70 platform data
> + */
> +
> +#ifndef _LM70_H_
> +#define _LM70_H_
> +
> +#define LM70_CHIP_LM70		0
> +#define LM70_CHIP_TMP121	1
> +
> +struct lm70_platform_data {
> +	unsigned int chip;
> +};
> +
> +#endif

I don't know much about the SPI side of things so I would like someone
else to comment on that aspect of the patch (in particular the way
different device types are supported by the same driver.) David, can
you please take a look and let us know if you have any objection?

Thanks,
-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[Manuel Lauss]

Hello Jean,

On Thu, Oct 23, 2008 at 02:57:10PM +0200, Jean Delvare wrote:
> > +  * Texas Instruments TMP121/TMP123
> > +    Datasheet: http://www.ti.com/lit/gpn/tmp121
> 
> I'd rather link to:
> http://focus.ti.com/docs/prod/folders/print/tmp121.html
> Where people can see a lot of useful information about the chip, and
> then download the datasheet.

done; (the chip isn't _that_ interesting or special ;-) )


> > +The TMP121 is very similar; main differences are 4 wire SPI interface,
> > +and 13bit temperature data (0.0625 celsius resolution).
> 
> 13-bit, degree Celsius

> > +	switch (p_lm70->chip) {
> > +	case LM70_CHIP_LM70:
> > +		raw = (rxbuf[1] << 8) + rxbuf[0];
> > +		dev_dbg(dev, "raw=0x%x\n", raw);
> 
> This debug statement is common to all cases so it could be moved
> outside of the switch block.

> > +	case LM70_CHIP_TMP121:
> > +		raw = (rxbuf[0] << 8) + rxbuf[1];
> > +		dev_dbg(dev, "raw=0x%x\n", raw);
> > +		val = (raw / 8) * 625 / 10;
> > +		break;
> > +
> > +	default:
> > +		raw = 0;
> 
> This default case is broken, it sets raw but not val, while what the
> following code uses is val. Anyway, I don't think you really need a
> default here, the chip type should have already been checked as you
> reach this portion of the driver code.
 
> > -	return sprintf(buf, "lm70\n");
> > +	struct spi_device *spi = to_spi_device(dev);
> > +	struct lm70 *p_lm70 = dev_get_drvdata(&spi->dev);
> 
> Err, what about just:
> 	struct lm70 *p_lm70 = dev_get_drvdata(dev);
> ? You don't use spi anywhere else.

I fixed everything you pointed out above;


> > --- /dev/null
> > +++ b/include/linux/spi/lm70.h
> > @@ -0,0 +1,15 @@
> > +/*
> > + * LM70 platform data
> > + */
> > +
> > +#ifndef _LM70_H_
> > +#define _LM70_H_
> > +
> > +#define LM70_CHIP_LM70		0
> > +#define LM70_CHIP_TMP121	1
> > +
> > +struct lm70_platform_data {
> > +	unsigned int chip;
> > +};
> > +
> > +#endif
> 
> I don't know much about the SPI side of things so I would like someone
> else to comment on that aspect of the patch (in particular the way
> different device types are supported by the same driver.) David, can
> you please take a look and let us know if you have any objection?

I'd be more than happy to just use a number cast to void * for 
spi platform_data and get rid of this header.  I'm not sure however
whether this is acceptable style for kernel code.

Thank you!
	Manuel Lauss

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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[Jean Delvare]

On Thu, 23 Oct 2008 15:13:23 +0200, Manuel Lauss wrote:
> On Thu, Oct 23, 2008 at 02:57:10PM +0200, Jean Delvare wrote:
> > I don't know much about the SPI side of things so I would like someone
> > else to comment on that aspect of the patch (in particular the way
> > different device types are supported by the same driver.) David, can
> > you please take a look and let us know if you have any objection?
> 
> I'd be more than happy to just use a number cast to void * for 
> spi platform_data and get rid of this header.  I'm not sure however
> whether this is acceptable style for kernel code.

Err, no, that's not what I was suggesting. Abusing pointers to store
integers is bad, please don't do that.

I was more curious about the need to have such arbitrary numbers to
differentiate between chip types. For i2c, we can just tell a driver to
support several chip names. But maybe the spi subsystem doesn't support
that.

-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[Manuel Lauss]

On Thu, Oct 23, 2008 at 03:20:30PM +0200, Jean Delvare wrote:
> On Thu, 23 Oct 2008 15:13:23 +0200, Manuel Lauss wrote:
> > On Thu, Oct 23, 2008 at 02:57:10PM +0200, Jean Delvare wrote:
> > > I don't know much about the SPI side of things so I would like someone
> > > else to comment on that aspect of the patch (in particular the way
> > > different device types are supported by the same driver.) David, can
> > > you please take a look and let us know if you have any objection?
> > 
> > I'd be more than happy to just use a number cast to void * for 
> > spi platform_data and get rid of this header.  I'm not sure however
> > whether this is acceptable style for kernel code.
> 
> Err, no, that's not what I was suggesting. Abusing pointers to store
> integers is bad, please don't do that.
> 
> I was more curious about the need to have such arbitrary numbers to
> differentiate between chip types. For i2c, we can just tell a driver to
> support several chip names. But maybe the spi subsystem doesn't support
> that.

Ah, yes, the lack of an i2c_driver.id_table parameter was why I originally
wrote a separate driver.  I suppose I could add another struct spi_driver
for the TMP121 and sort out chip types internally.

Thanks!
	Manuel Lauss

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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[Manuel Lauss]

Hello again,

I wrote:
> On Thu, Oct 23, 2008 at 03:20:30PM +0200, Jean Delvare wrote:
> > On Thu, 23 Oct 2008 15:13:23 +0200, Manuel Lauss wrote:
> > > On Thu, Oct 23, 2008 at 02:57:10PM +0200, Jean Delvare wrote:
> > > > I don't know much about the SPI side of things so I would like someone
> > > > else to comment on that aspect of the patch (in particular the way
> > > > different device types are supported by the same driver.) David, can
> > > > you please take a look and let us know if you have any objection?
> > > 
> > > I'd be more than happy to just use a number cast to void * for 
> > > spi platform_data and get rid of this header.  I'm not sure however
> > > whether this is acceptable style for kernel code.
> > 
> > Err, no, that's not what I was suggesting. Abusing pointers to store
> > integers is bad, please don't do that.
> > 
> > I was more curious about the need to have such arbitrary numbers to
> > differentiate between chip types. For i2c, we can just tell a driver to
> > support several chip names. But maybe the spi subsystem doesn't support
> > that.
> 
> Ah, yes, the lack of an i2c_driver.id_table parameter was why I originally
> wrote a separate driver.  I suppose I could add another struct spi_driver
> for the TMP121 and sort out chip types internally.

Does this look acceptable?

--- 

From: Manuel Lauss <mano@roarinelk.homelinux.net>
Subject: [PATCH] hwmon: Extend LM70 with TMP121/TMP123 support.

The Texas Instruments TMP121 is a SPI temperature sensor very similar
to the LM70, with slightly higher resolution.  This patch extends the
LM70 driver to support the TMP121.

Signed-off-by: Manuel Lauss <mano@roarinelk.homelinux.net>
---
 Documentation/hwmon/lm70 |    7 +++-
 drivers/hwmon/Kconfig    |    5 ++-
 drivers/hwmon/lm70.c     |   85 +++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 81 insertions(+), 16 deletions(-)

diff --git a/Documentation/hwmon/lm70 b/Documentation/hwmon/lm70
index 2bdd3fe..80716d2 100644
--- a/Documentation/hwmon/lm70
+++ b/Documentation/hwmon/lm70
@@ -1,9 +1,11 @@
 Kernel driver lm70
 =========
 
-Supported chip:
+Supported chips:
   * National Semiconductor LM70
     Datasheet: http://www.national.com/pf/LM/LM70.html
+  * Texas Instruments TMP121/TMP123
+    Information: http://focus.ti.com/docs/prod/folders/print/tmp121.html
 
 Author:
         Kaiwan N Billimoria <kaiwan@designergraphix.com>
@@ -25,6 +27,9 @@ complement digital temperature (sent via the SIO line), is available in the
 driver for interpretation. This driver makes use of the kernel's in-core
 SPI support.
 
+The TMP121 is very similar; main differences are 4 wire SPI interface
+and 13-bit temperature data (0.0625 degrees celsius resolution).
+
 Thanks to
 ---------
 Jean Delvare <khali@linux-fr.org> for mentoring the hwmon-side driver
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 6de1e0f..f86923d 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -407,11 +407,12 @@ config SENSORS_LM63
 	  will be called lm63.
 
 config SENSORS_LM70
-	tristate "National Semiconductor LM70"
+	tristate "National Semiconductor LM70 / Texas Instruments TMP121"
 	depends on SPI_MASTER && EXPERIMENTAL
 	help
 	  If you say yes here you get support for the National Semiconductor
-	  LM70 digital temperature sensor chip.
+	  LM70 and Texas Instruments TMP121/TMP123 digital temperature
+	  sensor chips.
 
 	  This driver can also be built as a module.  If so, the module
 	  will be called lm70.
diff --git a/drivers/hwmon/lm70.c b/drivers/hwmon/lm70.c
index d435f00..8c7651b 100644
--- a/drivers/hwmon/lm70.c
+++ b/drivers/hwmon/lm70.c
@@ -37,9 +37,13 @@
 
 #define DRVNAME		"lm70"
 
+#define LM70_CHIP_LM70		0
+#define LM70_CHIP_TMP121	1
+
 struct lm70 {
 	struct device *hwmon_dev;
 	struct mutex lock;
+	unsigned int chip;
 };
 
 /* sysfs hook function */
@@ -47,7 +51,7 @@ static ssize_t lm70_sense_temp(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
 	struct spi_device *spi = to_spi_device(dev);
-	int status, val;
+	int status, val = 0;
 	u8 rxbuf[2];
 	s16 raw=0;
 	struct lm70 *p_lm70 = dev_get_drvdata(&spi->dev);
@@ -67,10 +71,8 @@ static ssize_t lm70_sense_temp(struct device *dev,
 	}
 	dev_dbg(dev, "rxbuf[1] : 0x%x rxbuf[0] : 0x%x\n", rxbuf[1], rxbuf[0]);
 
-	raw = (rxbuf[1] << 8) + rxbuf[0];
-	dev_dbg(dev, "raw=0x%x\n", raw);
-
 	/*
+	 * LM70:
 	 * The "raw" temperature read into rxbuf[] is a 16-bit signed 2's
 	 * complement value. Only the MSB 11 bits (1 sign + 10 temperature
 	 * bits) are meaningful; the LSB 5 bits are to be discarded.
@@ -80,8 +82,23 @@ static ssize_t lm70_sense_temp(struct device *dev,
 	 * by 0.25. Also multiply by 1000 to represent in millidegrees
 	 * Celsius.
 	 * So it's equivalent to multiplying by 0.25 * 1000 = 250.
+	 *
+	 * TMP121:
+	 * 13 bits of 2's complement data, discard LSB 3 bits.  Chip
+	 * transmits high byte first.  Resolution 0.0625 degrees celsius.
 	 */
-	val = ((int)raw/32) * 250;
+	switch (p_lm70->chip) {
+	case LM70_CHIP_LM70:
+		raw = (rxbuf[1] << 8) + rxbuf[0];
+		val = ((int)raw/32) * 250;
+		break;
+
+	case LM70_CHIP_TMP121:
+		raw = (rxbuf[0] << 8) + rxbuf[1];
+		val = (raw / 8) * 625 / 10;
+		break;
+	}
+	dev_dbg(dev, "raw=0x%x\n", raw);
 	status = sprintf(buf, "%d\n", val); /* millidegrees Celsius */
 out:
 	mutex_unlock(&p_lm70->lock);
@@ -93,27 +110,37 @@ static DEVICE_ATTR(temp1_input, S_IRUGO, lm70_sense_temp, NULL);
 static ssize_t lm70_show_name(struct device *dev, struct device_attribute
 			      *devattr, char *buf)
 {
-	return sprintf(buf, "lm70\n");
+	struct lm70 *p_lm70 = dev_get_drvdata(dev);
+	int ret;
+
+	switch (p_lm70->chip) {
+	case LM70_CHIP_LM70:
+		ret = sprintf(buf, "lm70\n");
+		break;
+	case LM70_CHIP_TMP121:
+		ret = sprintf(buf, "tmp121\n");
+		break;
+	default:
+		ret = -EINVAL;
+	}
+	return ret;
 }
 
 static DEVICE_ATTR(name, S_IRUGO, lm70_show_name, NULL);
 
 /*----------------------------------------------------------------------*/
 
-static int __devinit lm70_probe(struct spi_device *spi)
+static int __devinit common_probe(struct spi_device *spi, int chip)
 {
 	struct lm70 *p_lm70;
 	int status;
 
-	/* signaling is SPI_MODE_0 on a 3-wire link (shared SI/SO) */
-	if ((spi->mode & (SPI_CPOL|SPI_CPHA)) || !(spi->mode & SPI_3WIRE))
-		return -EINVAL;
-
 	p_lm70 = kzalloc(sizeof *p_lm70, GFP_KERNEL);
 	if (!p_lm70)
 		return -ENOMEM;
 
 	mutex_init(&p_lm70->lock);
+	p_lm70->chip = chip;
 
 	/* sysfs hook */
 	p_lm70->hwmon_dev = hwmon_device_register(&spi->dev);
@@ -141,6 +168,20 @@ out_dev_reg_failed:
 	return status;
 }
 
+static int __devinit lm70_probe(struct spi_device *spi)
+{
+	/* signaling is SPI_MODE_0 on a 3-wire link (shared SI/SO) */
+	if ((spi->mode & (SPI_CPOL|SPI_CPHA)) || !(spi->mode & SPI_3WIRE))
+		return -EINVAL;
+
+	return common_probe(spi, LM70_CHIP_LM70);
+}
+
+static int __devinit tmp121_probe(struct spi_device *spi)
+{
+	return common_probe(spi, LM70_CHIP_TMP121);
+}
+
 static int __devexit lm70_remove(struct spi_device *spi)
 {
 	struct lm70 *p_lm70 = dev_get_drvdata(&spi->dev);
@@ -154,6 +195,15 @@ static int __devexit lm70_remove(struct spi_device *spi)
 	return 0;
 }
 
+static struct spi_driver tmp121_driver = {
+	.driver = {
+		.name	= "tmp121",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= tmp121_probe,
+	.remove	= __devexit_p(lm70_remove),
+};
+
 static struct spi_driver lm70_driver = {
 	.driver = {
 		.name	= "lm70",
@@ -165,17 +215,26 @@ static struct spi_driver lm70_driver = {
 
 static int __init init_lm70(void)
 {
-	return spi_register_driver(&lm70_driver);
+	int ret = spi_register_driver(&lm70_driver);
+	if (ret)
+		return ret;
+
+	ret = spi_register_driver(&tmp121_driver);
+	if (ret)
+		spi_unregister_driver(&lm70_driver);
+
+	return ret;
 }
 
 static void __exit cleanup_lm70(void)
 {
 	spi_unregister_driver(&lm70_driver);
+	spi_unregister_driver(&tmp121_driver);
 }
 
 module_init(init_lm70);
 module_exit(cleanup_lm70);
 
 MODULE_AUTHOR("Kaiwan N Billimoria");
-MODULE_DESCRIPTION("National Semiconductor LM70 Linux driver");
+MODULE_DESCRIPTION("National Semiconductor LM70 / TI TMP121 Linux driver");
 MODULE_LICENSE("GPL");
-- 
1.6.0.2


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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[David Brownell]

On Thursday 23 October 2008, Manuel Lauss wrote:
> > Ah, yes, the lack of an i2c_driver.id_table parameter was why I originally
> > wrote a separate driver.  I suppose I could add another struct spi_driver
> > for the TMP121 and sort out chip types internally.
> 
> Does this look acceptable?

Much better, thanks.  The SPI framework doesn't currently
identify chips other than as a driver name.  Hasn't needed to.


> --- 
> 
> From: Manuel Lauss <mano@roarinelk.homelinux.net>
> Subject: [PATCH] hwmon: Extend LM70 with TMP121/TMP123 support.
> 
> The Texas Instruments TMP121 is a SPI temperature sensor very similar
> to the LM70, with slightly higher resolution.  This patch extends the
> LM70 driver to support the TMP121.
> 
> Signed-off-by: Manuel Lauss <mano@roarinelk.homelinux.net>
> ---
>  Documentation/hwmon/lm70 |    7 +++-
>  drivers/hwmon/Kconfig    |    5 ++-
>  drivers/hwmon/lm70.c     |   85 +++++++++++++++++++++++++++++++++++++++-------
>  3 files changed, 81 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/hwmon/lm70 b/Documentation/hwmon/lm70
> index 2bdd3fe..80716d2 100644
> --- a/Documentation/hwmon/lm70
> +++ b/Documentation/hwmon/lm70
> @@ -1,9 +1,11 @@
>  Kernel driver lm70
>  =========
>  
> -Supported chip:
> +Supported chips:
>    * National Semiconductor LM70
>      Datasheet: http://www.national.com/pf/LM/LM70.html
> +  * Texas Instruments TMP121/TMP123
> +    Information: http://focus.ti.com/docs/prod/folders/print/tmp121.html
>  
>  Author:
>          Kaiwan N Billimoria <kaiwan@designergraphix.com>
> @@ -25,6 +27,9 @@ complement digital temperature (sent via the SIO line), is available in the
>  driver for interpretation. This driver makes use of the kernel's in-core
>  SPI support.
>  
> +The TMP121 is very similar; main differences are 4 wire SPI interface
> +and 13-bit temperature data (0.0625 degrees celsius resolution).

Instead of saying "4 wire SPI" I'd say "output-only SPI" ... since
in fact it only uses three wires.  An unlike the LM70, it's not
using the data wire for half duplex  transfers, it just spits out
the temperature reading.


> +
>  Thanks to
>  ---------
>  Jean Delvare <khali@linux-fr.org> for mentoring the hwmon-side driver
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 6de1e0f..f86923d 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -407,11 +407,12 @@ config SENSORS_LM63
>  	  will be called lm63.
>  
>  config SENSORS_LM70
> -	tristate "National Semiconductor LM70"
> +	tristate "National Semiconductor LM70 / Texas Instruments TMP121"

TMP121 and TMP123 ...

>  	depends on SPI_MASTER && EXPERIMENTAL
>  	help
>  	  If you say yes here you get support for the National Semiconductor
> -	  LM70 digital temperature sensor chip.
> +	  LM70 and Texas Instruments TMP121/TMP123 digital temperature
> +	  sensor chips.
>  
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called lm70.
> diff --git a/drivers/hwmon/lm70.c b/drivers/hwmon/lm70.c
> index d435f00..8c7651b 100644
> --- a/drivers/hwmon/lm70.c
> +++ b/drivers/hwmon/lm70.c
> @@ -37,9 +37,13 @@
>  
>  #define DRVNAME		"lm70"
>  
> +#define LM70_CHIP_LM70		0
> +#define LM70_CHIP_TMP121	1

Obviously TMP123 doesn't need a different identifier, since
the difference is only in the electrical connections used
for pins 1 & 2.  But a comment to that effect might be wise.


> +
>  struct lm70 {
>  	struct device *hwmon_dev;
>  	struct mutex lock;
> +	unsigned int chip;
>  };
>  
>  /* sysfs hook function */
> @@ -47,7 +51,7 @@ static ssize_t lm70_sense_temp(struct device *dev,
>  		struct device_attribute *attr, char *buf)
>  {
>  	struct spi_device *spi = to_spi_device(dev);
> -	int status, val;
> +	int status, val = 0;
>  	u8 rxbuf[2];
>  	s16 raw=0;
>  	struct lm70 *p_lm70 = dev_get_drvdata(&spi->dev);
> @@ -67,10 +71,8 @@ static ssize_t lm70_sense_temp(struct device *dev,
>  	}
>  	dev_dbg(dev, "rxbuf[1] : 0x%x rxbuf[0] : 0x%x\n", rxbuf[1], rxbuf[0]);
>  
> -	raw = (rxbuf[1] << 8) + rxbuf[0];
> -	dev_dbg(dev, "raw=0x%x\n", raw);
> -
>  	/*
> +	 * LM70:
>  	 * The "raw" temperature read into rxbuf[] is a 16-bit signed 2's
>  	 * complement value. Only the MSB 11 bits (1 sign + 10 temperature
>  	 * bits) are meaningful; the LSB 5 bits are to be discarded.
> @@ -80,8 +82,23 @@ static ssize_t lm70_sense_temp(struct device *dev,
>  	 * by 0.25. Also multiply by 1000 to represent in millidegrees
>  	 * Celsius.
>  	 * So it's equivalent to multiplying by 0.25 * 1000 = 250.
> +	 *
> +	 * TMP121:
> +	 * 13 bits of 2's complement data, discard LSB 3 bits.  Chip
> +	 * transmits high byte first.  Resolution 0.0625 degrees celsius.

For SPI, everything is MSB first ... else insist (spi->mode & SPI_LSB_FIRST)
is true.  Please strike the comment implying it might not be MSB first.


>  	 */
> -	val = ((int)raw/32) * 250;
> +	switch (p_lm70->chip) {
> +	case LM70_CHIP_LM70:
> +		raw = (rxbuf[1] << 8) + rxbuf[0];
> +		val = ((int)raw/32) * 250;
> +		break;
> +
> +	case LM70_CHIP_TMP121:
> +		raw = (rxbuf[0] << 8) + rxbuf[1];
> +		val = (raw / 8) * 625 / 10;
> +		break;
> +	}
> +	dev_dbg(dev, "raw=0x%x\n", raw);
>  	status = sprintf(buf, "%d\n", val); /* millidegrees Celsius */
>  out:
>  	mutex_unlock(&p_lm70->lock);
> @@ -93,27 +110,37 @@ static DEVICE_ATTR(temp1_input, S_IRUGO, lm70_sense_temp, NULL);
>  static ssize_t lm70_show_name(struct device *dev, struct device_attribute
>  			      *devattr, char *buf)
>  {
> -	return sprintf(buf, "lm70\n");
> +	struct lm70 *p_lm70 = dev_get_drvdata(dev);
> +	int ret;
> +
> +	switch (p_lm70->chip) {
> +	case LM70_CHIP_LM70:
> +		ret = sprintf(buf, "lm70\n");
> +		break;
> +	case LM70_CHIP_TMP121:
> +		ret = sprintf(buf, "tmp121\n");
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +	return ret;
>  }
>  
>  static DEVICE_ATTR(name, S_IRUGO, lm70_show_name, NULL);
>  
>  /*----------------------------------------------------------------------*/
>  
> -static int __devinit lm70_probe(struct spi_device *spi)
> +static int __devinit common_probe(struct spi_device *spi, int chip)
>  {
>  	struct lm70 *p_lm70;
>  	int status;
>  
> -	/* signaling is SPI_MODE_0 on a 3-wire link (shared SI/SO) */
> -	if ((spi->mode & (SPI_CPOL|SPI_CPHA)) || !(spi->mode & SPI_3WIRE))
> -		return -EINVAL;
> -
>  	p_lm70 = kzalloc(sizeof *p_lm70, GFP_KERNEL);
>  	if (!p_lm70)
>  		return -ENOMEM;
>  
>  	mutex_init(&p_lm70->lock);
> +	p_lm70->chip = chip;
>  
>  	/* sysfs hook */
>  	p_lm70->hwmon_dev = hwmon_device_register(&spi->dev);
> @@ -141,6 +168,20 @@ out_dev_reg_failed:
>  	return status;
>  }
>  
> +static int __devinit lm70_probe(struct spi_device *spi)
> +{
> +	/* signaling is SPI_MODE_0 on a 3-wire link (shared SI/SO) */
> +	if ((spi->mode & (SPI_CPOL|SPI_CPHA)) || !(spi->mode & SPI_3WIRE))
> +		return -EINVAL;
> +
> +	return common_probe(spi, LM70_CHIP_LM70);
> +}
> +
> +static int __devinit tmp121_probe(struct spi_device *spi)
> +{

For symmetry you might verify the CPOL and CPHA settings are right;
that'd mostly help catch user configuration bugs.

> +	return common_probe(spi, LM70_CHIP_TMP121);
> +}
> +
>  static int __devexit lm70_remove(struct spi_device *spi)
>  {
>  	struct lm70 *p_lm70 = dev_get_drvdata(&spi->dev);
> @@ -154,6 +195,15 @@ static int __devexit lm70_remove(struct spi_device *spi)
>  	return 0;
>  }
>  
> +static struct spi_driver tmp121_driver = {
> +	.driver = {
> +		.name	= "tmp121",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe	= tmp121_probe,
> +	.remove	= __devexit_p(lm70_remove),
> +};
> +
>  static struct spi_driver lm70_driver = {
>  	.driver = {
>  		.name	= "lm70",
> @@ -165,17 +215,26 @@ static struct spi_driver lm70_driver = {
>  
>  static int __init init_lm70(void)
>  {
> -	return spi_register_driver(&lm70_driver);
> +	int ret = spi_register_driver(&lm70_driver);
> +	if (ret)
> +		return ret;
> +
> +	ret = spi_register_driver(&tmp121_driver);
> +	if (ret)
> +		spi_unregister_driver(&lm70_driver);
> +
> +	return ret;
>  }
>  
>  static void __exit cleanup_lm70(void)
>  {
>  	spi_unregister_driver(&lm70_driver);
> +	spi_unregister_driver(&tmp121_driver);
>  }
>  
>  module_init(init_lm70);
>  module_exit(cleanup_lm70);
>  
>  MODULE_AUTHOR("Kaiwan N Billimoria");
> -MODULE_DESCRIPTION("National Semiconductor LM70 Linux driver");
> +MODULE_DESCRIPTION("National Semiconductor LM70 / TI TMP121 Linux driver");
>  MODULE_LICENSE("GPL");
> -- 
> 1.6.0.2
> 
> 



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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[Jean Delvare]

Hi David,

Thanks for your comments.

On Thu, 23 Oct 2008 09:53:00 -0700, David Brownell wrote:
> On Thursday 23 October 2008, Manuel Lauss wrote:
> >  	/*
> > +	 * LM70:
> >  	 * The "raw" temperature read into rxbuf[] is a 16-bit signed 2's
> >  	 * complement value. Only the MSB 11 bits (1 sign + 10 temperature
> >  	 * bits) are meaningful; the LSB 5 bits are to be discarded.
> > @@ -80,8 +82,23 @@ static ssize_t lm70_sense_temp(struct device *dev,
> >  	 * by 0.25. Also multiply by 1000 to represent in millidegrees
> >  	 * Celsius.
> >  	 * So it's equivalent to multiplying by 0.25 * 1000 = 250.
> > +	 *
> > +	 * TMP121:
> > +	 * 13 bits of 2's complement data, discard LSB 3 bits.  Chip
> > +	 * transmits high byte first.  Resolution 0.0625 degrees celsius.
> 
> For SPI, everything is MSB first ... else insist (spi->mode & SPI_LSB_FIRST)
> is true.  Please strike the comment implying it might not be MSB first.

Except that the LM70 transmits the LSB first, so it seems valuable to
underline that the TMP121 behaves differently.

> >  	 */
> > -	val = ((int)raw/32) * 250;
> > +	switch (p_lm70->chip) {
> > +	case LM70_CHIP_LM70:
> > +		raw = (rxbuf[1] << 8) + rxbuf[0];
> > +		val = ((int)raw/32) * 250;
> > +		break;
> > +
> > +	case LM70_CHIP_TMP121:
> > +		raw = (rxbuf[0] << 8) + rxbuf[1];
> > +		val = (raw / 8) * 625 / 10;
> > +		break;
> > +	}
> > +	dev_dbg(dev, "raw=0x%x\n", raw);
> >  	status = sprintf(buf, "%d\n", val); /* millidegrees Celsius */
> >  out:
> >  	mutex_unlock(&p_lm70->lock);

-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[David Brownell]

On Thursday 23 October 2008, Jean Delvare wrote:
> 
> > For SPI, everything is MSB first ... else insist (spi->mode & SPI_LSB_FIRST)
> > is true.  Please strike the comment implying it might not be MSB first.
> 
> Except that the LM70 transmits the LSB first, so it seems valuable to
> underline that the TMP121 behaves differently.

If true, then this driver has a bug ... it should require
SPI_LSB_FIRST as well as SPI_3WIRE in spi->mode, for lm70.

But no, I checked the data sheet and it shows MSB first.
See the table in section 1.5.2 (MSB = D15) and the figures
in section 2 (D15 first).


See what confusion comes from implying things which are
contrary-to-fact?  :)

- Dave

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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[Jean Delvare]

On Thu, 23 Oct 2008 10:46:29 -0700, David Brownell wrote:
> On Thursday 23 October 2008, Jean Delvare wrote:
> > 
> > > For SPI, everything is MSB first ... else insist (spi->mode & SPI_LSB_FIRST)
> > > is true.  Please strike the comment implying it might not be MSB first.
> > 
> > Except that the LM70 transmits the LSB first, so it seems valuable to
> > underline that the TMP121 behaves differently.
> 
> If true, then this driver has a bug ... it should require
> SPI_LSB_FIRST as well as SPI_3WIRE in spi->mode, for lm70.
> 
> But no, I checked the data sheet and it shows MSB first.
> See the table in section 1.5.2 (MSB = D15) and the figures
> in section 2 (D15 first).
> 
> 
> See what confusion comes from implying things which are
> contrary-to-fact?  :)

I didn't assume anything, I simply read the code:

+	switch (p_lm70->chip) {
+	case LM70_CHIP_LM70:
+		raw = (rxbuf[1] << 8) + rxbuf[0];
+		val = ((int)raw/32) * 250;
+		break;
+
+	case LM70_CHIP_TMP121:
+		raw = (rxbuf[0] << 8) + rxbuf[1];
+		val = (raw / 8) * 625 / 10;
+		break;
+	}

Does it look to you like both chips expect the same byte order? Me not.

-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[David Brownell]

> +	switch (p_lm70->chip) {
> +	case LM70_CHIP_LM70:
> +		raw = (rxbuf[1] << 8) + rxbuf[0];
> +		val = ((int)raw/32) * 250;
> +		break;

This would seem to be masking a bug in the
drivers/spi/spi_lm70llp.c code ... observe
how lm70_txrx() interchanges bytes.  Sigh.


> +
> +	case LM70_CHIP_TMP121:
> +		raw = (rxbuf[0] << 8) + rxbuf[1];
> +		val = (raw / 8) * 625 / 10;
> +		break;

Which is correct for a 16-bit word being
encoded MSB-first in two bytes.


Arguably spi->bits_per_word should be set
to 16 and the driver should just perform
the I/O into a u16, so it only needs to
cope with the garbage LSBs.



> +	}
> 
> Does it look to you like both chips expect the same byte order? Me not.

Right, but afaik the lm70 code was only
tested with drivers/spi/spi_lm70llp.c and
that has a byte order handling bug.

Kaiwan Billimoria is the only person I
know who could test fixes to the LM70
and LM70LLP support ... Kaiwan, could
you give that a try?

- Dave



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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[Kaiwan N Billimoria]

On Thu, 2008-10-23 at 11:37 -0700, David Brownell wrote:
> Right, but afaik the lm70 code was only
> tested with drivers/spi/spi_lm70llp.c and
> that has a byte order handling bug.
> 
> Kaiwan Billimoria is the only person I
> know who could test fixes to the LM70
> and LM70LLP support ... Kaiwan, could
> you give that a try?
> 
> - Dave
> 
Sure Dave.
Pl point me to the latest patch(es) and help in clarifying what 
exactly the bug is; been a while...  :-)

And of course, if you already have a fix i'll test it on the lm70llp
board.

- kaiwan.




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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[David Brownell]

On Friday 24 October 2008, Kaiwan N Billimoria wrote:
> On Thu, 2008-10-23 at 11:37 -0700, David Brownell wrote:
> > Right, but afaik the lm70 code was only
> > tested with drivers/spi/spi_lm70llp.c and
> > that has a byte order handling bug.
> > 
> > Kaiwan Billimoria is the only person I
> > know who could test fixes to the LM70
> > and LM70LLP support ... Kaiwan, could
> > you give that a try?
> > 
> > - Dave
> > 
> Sure Dave.
> Pl point me to the latest patch(es) and help in clarifying what 
> exactly the bug is; been a while...  :-)
> 
> And of course, if you already have a fix i'll test it on the lm70llp
> board.

I suspect the following should do the job ... it has
the parport based adapter leave the data in MSB-first
byte order, with the lm70 driver converting to CPU
byte order.  (In a way that will match the TMP121/123
support.)

- Dave

---
 drivers/hwmon/lm70.c      |    4 +++-
 drivers/spi/spi_lm70llp.c |   19 +++----------------
 2 files changed, 6 insertions(+), 17 deletions(-)

--- a/drivers/hwmon/lm70.c
+++ b/drivers/hwmon/lm70.c
@@ -67,7 +67,7 @@ static ssize_t lm70_sense_temp(struct de
 	}
 	dev_dbg(dev, "rxbuf[1] : 0x%x rxbuf[0] : 0x%x\n", rxbuf[1], rxbuf[0]);
 
-	raw = (rxbuf[1] << 8) + rxbuf[0];
+	raw = (rxbuf[0] << 8) + rxbuf[1];
 	dev_dbg(dev, "raw=0x%x\n", raw);
 
 	/*
@@ -109,6 +109,8 @@ static int __devinit lm70_probe(struct s
 	if ((spi->mode & (SPI_CPOL|SPI_CPHA)) || !(spi->mode & SPI_3WIRE))
 		return -EINVAL;
 
+	/* NOTE:  we assume 8-bit words, and convert to 16 bits manually */
+
 	p_lm70 = kzalloc(sizeof *p_lm70, GFP_KERNEL);
 	if (!p_lm70)
 		return -ENOMEM;
--- a/drivers/spi/spi_lm70llp.c
+++ b/drivers/spi/spi_lm70llp.c
@@ -173,6 +173,8 @@ static void lm70_chipselect(struct spi_d
 {
 	struct spi_lm70llp *pp = spidev_to_pp(spi);
 
+	/* REVISIT initialize the clock polarity ... */
+
 	if (value)
 		assertCS(pp);
 	else
@@ -184,22 +186,7 @@ static void lm70_chipselect(struct spi_d
  */
 static u32 lm70_txrx(struct spi_device *spi, unsigned nsecs, u32 word, u8 bits)
 {
-	static u32 sio=0;
-	static int first_time=1;
-
-	/* First time: perform SPI bitbang and return the LSB of
-	 * the result of the SPI call.
-	 */
-	if (first_time) {
-		sio = bitbang_txrx_be_cpha0(spi, nsecs, 0, word, bits);
-		first_time=0;
-		return (sio & 0x00ff);
-	}
-	/* Return the MSB of the result of the SPI call */
-	else {
-		first_time=1;
-		return (sio >> 8);
-	}
+	return bitbang_txrx_be_cpha0(spi, nsecs, 0, word, bits);
 }
 
 static void spi_lm70llp_attach(struct parport *p)

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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[Kaiwan N Billimoria]

On Fri, 2008-10-24 at 02:21 -0700, David Brownell wrote:
> > And of course, if you already have a fix i'll test it on the lm70llp
> > board.
> 
> I suspect the following should do the job ... it has
> the parport based adapter leave the data in MSB-first
> byte order, with the lm70 driver converting to CPU
> byte order.  (In a way that will match the TMP121/123
> support.)
> 
> - Dave
> 
> ---
>  drivers/hwmon/lm70.c      |    4 +++-
>  drivers/spi/spi_lm70llp.c |   19 +++----------------
>  2 files changed, 6 insertions(+), 17 deletions(-)
> 
> --- a/drivers/hwmon/lm70.c
> +++ b/drivers/hwmon/lm70.c
> @@ -67,7 +67,7 @@ static ssize_t lm70_sense_temp(struct de
>  	}
>  	dev_dbg(dev, "rxbuf[1] : 0x%x rxbuf[0] : 0x%x\n", rxbuf[1], rxbuf[0]);
>  
> -	raw = (rxbuf[1] << 8) + rxbuf[0];
> +	raw = (rxbuf[0] << 8) + rxbuf[1];
>  	dev_dbg(dev, "raw=0x%x\n", raw);
>  
>  	/*
> @@ -109,6 +109,8 @@ static int __devinit lm70_probe(struct s
>  	if ((spi->mode & (SPI_CPOL|SPI_CPHA)) || !(spi->mode & SPI_3WIRE))
>  		return -EINVAL;
>  
> +	/* NOTE:  we assume 8-bit words, and convert to 16 bits manually */
> +
>  	p_lm70 = kzalloc(sizeof *p_lm70, GFP_KERNEL);
>  	if (!p_lm70)
>  		return -ENOMEM;
> --- a/drivers/spi/spi_lm70llp.c
> +++ b/drivers/spi/spi_lm70llp.c
> @@ -173,6 +173,8 @@ static void lm70_chipselect(struct spi_d
>  {
>  	struct spi_lm70llp *pp = spidev_to_pp(spi);
>  
> +	/* REVISIT initialize the clock polarity ... */
> +
>  	if (value)
>  		assertCS(pp);
>  	else
> @@ -184,22 +186,7 @@ static void lm70_chipselect(struct spi_d
>   */
>  static u32 lm70_txrx(struct spi_device *spi, unsigned nsecs, u32 word, u8 bits)
>  {
> -	static u32 sio=0;
> -	static int first_time=1;
> -
> -	/* First time: perform SPI bitbang and return the LSB of
> -	 * the result of the SPI call.
> -	 */
> -	if (first_time) {
> -		sio = bitbang_txrx_be_cpha0(spi, nsecs, 0, word, bits);
> -		first_time=0;
> -		return (sio & 0x00ff);
> -	}
> -	/* Return the MSB of the result of the SPI call */
> -	else {
> -		first_time=1;
> -		return (sio >> 8);
> -	}
> +	return bitbang_txrx_be_cpha0(spi, nsecs, 0, word, bits);
>  }
>  
>  static void spi_lm70llp_attach(struct parport *p)

Ok, will give it a hot...
unfortunately i won't be able to actually tell you'll anything until
Mon/Tue as i'll gain access only then to the PC and lm70.

Later,
kaiwan.



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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[Kaiwan N Billimoria]

Hi,

Got a chance to test this patch today..
short answer: no it did not seem to work.

I got readings with largish negative values (from /sys/.../temp1_input);
have not really had the time to debug this..

[Sorry have not pasted o/p here becoz i do the work on an office PC,
where i built the 2.6.28-rc2 kernel & tried this; a bit misconfigured so
graphics did not come up (hence no clipboard stuff)...
Also, am writing this email off my notebook where i can't test the
device as it has no parport.]

Of course, in the first place, that's why the logic is the way it is.
I can (re)confirm though that for the LM70 chip the temperature register
is sent MSB first...

Let me know,

-kaiwan.

On Fri, 2008-10-24 at 02:21 -0700, David Brownell wrote:

> I suspect the following should do the job ... it has
> the parport based adapter leave the data in MSB-first
> byte order, with the lm70 driver converting to CPU
> byte order.  (In a way that will match the TMP121/123
> support.)
> 
> - Dave
> 
> ---
>  drivers/hwmon/lm70.c      |    4 +++-
>  drivers/spi/spi_lm70llp.c |   19 +++----------------
>  2 files changed, 6 insertions(+), 17 deletions(-)
> 
> --- a/drivers/hwmon/lm70.c
> +++ b/drivers/hwmon/lm70.c
> @@ -67,7 +67,7 @@ static ssize_t lm70_sense_temp(struct de
>  	}
>  	dev_dbg(dev, "rxbuf[1] : 0x%x rxbuf[0] : 0x%x\n", rxbuf[1], rxbuf[0]);
>  
> -	raw = (rxbuf[1] << 8) + rxbuf[0];
> +	raw = (rxbuf[0] << 8) + rxbuf[1];
>  	dev_dbg(dev, "raw=0x%x\n", raw);
>  
>  	/*
> @@ -109,6 +109,8 @@ static int __devinit lm70_probe(struct s
>  	if ((spi->mode & (SPI_CPOL|SPI_CPHA)) || !(spi->mode & SPI_3WIRE))
>  		return -EINVAL;
>  
> +	/* NOTE:  we assume 8-bit words, and convert to 16 bits manually */
> +
>  	p_lm70 = kzalloc(sizeof *p_lm70, GFP_KERNEL);
>  	if (!p_lm70)
>  		return -ENOMEM;
> --- a/drivers/spi/spi_lm70llp.c
> +++ b/drivers/spi/spi_lm70llp.c
> @@ -173,6 +173,8 @@ static void lm70_chipselect(struct spi_d
>  {
>  	struct spi_lm70llp *pp = spidev_to_pp(spi);
>  
> +	/* REVISIT initialize the clock polarity ... */
> +
>  	if (value)
>  		assertCS(pp);
>  	else
> @@ -184,22 +186,7 @@ static void lm70_chipselect(struct spi_d
>   */
>  static u32 lm70_txrx(struct spi_device *spi, unsigned nsecs, u32 word, u8 bits)
>  {
> -	static u32 sio=0;
> -	static int first_time=1;
> -
> -	/* First time: perform SPI bitbang and return the LSB of
> -	 * the result of the SPI call.
> -	 */
> -	if (first_time) {
> -		sio = bitbang_txrx_be_cpha0(spi, nsecs, 0, word, bits);
> -		first_time=0;
> -		return (sio & 0x00ff);
> -	}
> -	/* Return the MSB of the result of the SPI call */
> -	else {
> -		first_time=1;
> -		return (sio >> 8);
> -	}
> +	return bitbang_txrx_be_cpha0(spi, nsecs, 0, word, bits);
>  }
>  
>  static void spi_lm70llp_attach(struct parport *p)



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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[David Brownell]

On Tuesday 28 October 2008, Kaiwan N Billimoria wrote:
> Hi,
> 
> Got a chance to test this patch today..
> short answer: no it did not seem to work.

Well, then can you see what the issue is then?
The current mainline code is clearly buggy...


> I got readings with largish negative values (from /sys/.../temp1_input);
> have not really had the time to debug this..
> 
> [Sorry have not pasted o/p here becoz i do the work on an office PC,
> where i built the 2.6.28-rc2 kernel & tried this; a bit misconfigured so
> graphics did not come up (hence no clipboard stuff)...
> Also, am writing this email off my notebook where i can't test the
> device as it has no parport.]
> 
> Of course, in the first place, that's why the logic is the way it is.

I think there must be bugs covering up for each other...


> I can (re)confirm though that for the LM70 chip the temperature register
> is sent MSB first...

Right, which is why the lm70.c code should use the

> -     raw = (rxbuf[1] << 8) + rxbuf[0];
> +     raw = (rxbuf[0] << 8) + rxbuf[1];

So the question is then what *else* is going on (buggy)
that the previous code seemed to work despite that bug...

- Dave


> Let me know,
> 
> -kaiwan.
> 
> On Fri, 2008-10-24 at 02:21 -0700, David Brownell wrote:
> 
> > I suspect the following should do the job ... it has
> > the parport based adapter leave the data in MSB-first
> > byte order, with the lm70 driver converting to CPU
> > byte order.  (In a way that will match the TMP121/123
> > support.)
> > 
> > - Dave
> > 
> > ---
> >  drivers/hwmon/lm70.c      |    4 +++-
> >  drivers/spi/spi_lm70llp.c |   19 +++----------------
> >  2 files changed, 6 insertions(+), 17 deletions(-)
> > 
> > --- a/drivers/hwmon/lm70.c
> > +++ b/drivers/hwmon/lm70.c
> > @@ -67,7 +67,7 @@ static ssize_t lm70_sense_temp(struct de
> >  	}
> >  	dev_dbg(dev, "rxbuf[1] : 0x%x rxbuf[0] : 0x%x\n", rxbuf[1], rxbuf[0]);
> >  
> > -	raw = (rxbuf[1] << 8) + rxbuf[0];
> > +	raw = (rxbuf[0] << 8) + rxbuf[1];
> >  	dev_dbg(dev, "raw=0x%x\n", raw);
> >  
> >  	/*
> > @@ -109,6 +109,8 @@ static int __devinit lm70_probe(struct s
> >  	if ((spi->mode & (SPI_CPOL|SPI_CPHA)) || !(spi->mode & SPI_3WIRE))
> >  		return -EINVAL;
> >  
> > +	/* NOTE:  we assume 8-bit words, and convert to 16 bits manually */
> > +
> >  	p_lm70 = kzalloc(sizeof *p_lm70, GFP_KERNEL);
> >  	if (!p_lm70)
> >  		return -ENOMEM;
> > --- a/drivers/spi/spi_lm70llp.c
> > +++ b/drivers/spi/spi_lm70llp.c
> > @@ -173,6 +173,8 @@ static void lm70_chipselect(struct spi_d
> >  {
> >  	struct spi_lm70llp *pp = spidev_to_pp(spi);
> >  
> > +	/* REVISIT initialize the clock polarity ... */
> > +
> >  	if (value)
> >  		assertCS(pp);
> >  	else
> > @@ -184,22 +186,7 @@ static void lm70_chipselect(struct spi_d
> >   */
> >  static u32 lm70_txrx(struct spi_device *spi, unsigned nsecs, u32 word, u8 bits)
> >  {
> > -	static u32 sio=0;
> > -	static int first_time=1;
> > -
> > -	/* First time: perform SPI bitbang and return the LSB of
> > -	 * the result of the SPI call.
> > -	 */
> > -	if (first_time) {
> > -		sio = bitbang_txrx_be_cpha0(spi, nsecs, 0, word, bits);
> > -		first_time=0;
> > -		return (sio & 0x00ff);
> > -	}
> > -	/* Return the MSB of the result of the SPI call */
> > -	else {
> > -		first_time=1;
> > -		return (sio >> 8);
> > -	}
> > +	return bitbang_txrx_be_cpha0(spi, nsecs, 0, word, bits);
> >  }
> >  
> >  static void spi_lm70llp_attach(struct parport *p)
> 
> 
> 



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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[Kaiwan N Billimoria]

On Tue, 2008-10-28 at 03:43 -0700, David Brownell wrote:
> On Tuesday 28 October 2008, Kaiwan N Billimoria wrote:
> > Hi,
> > 
> > Got a chance to test this patch today..
> > short answer: no it did not seem to work.
> 
> Well, then can you see what the issue is then?
> The current mainline code is clearly buggy...
> 

A quick qs: (perhaps i'm missing something obvious/silly here);
rxbuf[1] is the MSB part of the rxbuf buffer right.
So the code is moving in the bits from there first into the variable
raw, then adding in the LSB part of rxbuf. Why is that wrong?

> I think there must be bugs covering up for each other...
> 
> 
> > I can (re)confirm though that for the LM70 chip the temperature
> register
> > is sent MSB first...
> 
> Right, which is why the lm70.c code should use the
> 
> > -     raw = (rxbuf[1] << 8) + rxbuf[0];
> > +     raw = (rxbuf[0] << 8) + rxbuf[1];
> 
> So the question is then what *else* is going on (buggy)
> that the previous code seemed to work despite that bug...
> 
> - Dave

Okay, though am unable to work on this right now.
It's in my queue & i'll get back when i have a chance...

Later,
Kaiwan.



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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[David Brownell]

On Wednesday 29 October 2008, Kaiwan N Billimoria wrote:
> On Tue, 2008-10-28 at 03:43 -0700, David Brownell wrote:
> > On Tuesday 28 October 2008, Kaiwan N Billimoria wrote:
> > > Hi,
> > > 
> > > Got a chance to test this patch today..
> > > short answer: no it did not seem to work.
> > 
> > Well, then can you see what the issue is then?

Maybe the lm70llp code shouldn't set spi->bits_per_word to 16;
that seems to be another problem.


> > The current mainline code is clearly buggy...
> > 
> 
> A quick qs: (perhaps i'm missing something obvious/silly here);
> rxbuf[1] is the MSB part of the rxbuf buffer right.

No.  Since the sensor sends an 11-bit number (sign then 10 data bits)
in MSB format, when the lm70llp (parport adapter) returns two bytes
it will return first rxbuf[0] with the 8 MSBs, then rxbuf[1] with
three data bits and five irrelevant bits.  (Although code at that
level has been known to goof up and shift a bit in the wrong direction,
for example because it's sampling the wrong edge...)

rxbuf[0] holds the MSBs ... rxbuf[1] holds the LSBs.

Recall that the whole reason we noticed this bug is that the $SUBJECT
patch was -- correctly!! -- getting its MSBs from rxbuf[0], but Jean
noted the lm70 code was different.


> So the code is moving in the bits from there first into the variable
> raw, then adding in the LSB part of rxbuf. Why is that wrong?

See above.  If spi->bits_per_word were 16 AND you were hard-wiring
the lm70.c code to work only on little-endian hardware, THEN it would
be right to "know" that rxbuf[1] has the MSBs. 

But it's not OK to hard-wire drivers to work only on little endian
systems, which is why I suspect the missing part of the fix is to
strike the lm70llp line setting bits_per_word to nonzero.


> > I think there must be bugs covering up for each other...
> > 
> > 
> > > I can (re)confirm though that for the LM70 chip the temperature
> > register
> > > is sent MSB first...
> > 
> > Right, which is why the lm70.c code should use the
> > 
> > > -     raw = (rxbuf[1] << 8) + rxbuf[0];
> > > +     raw = (rxbuf[0] << 8) + rxbuf[1];
> > 
> > So the question is then what *else* is going on (buggy)
> > that the previous code seemed to work despite that bug...
> > 
> > - Dave
> 
> Okay, though am unable to work on this right now.
> It's in my queue & i'll get back when i have a chance...
> 
> Later,
> Kaiwan.
> 
> 
> 



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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[Kaiwan N Billimoria]

Hey,

It's working now as expected..
will send in the patch soon,

On Wed, 2008-10-29 at 23:49 -0700, David Brownell wrote:
> On Wednesday 29 October 2008, Kaiwan N Billimoria wrote:
> > On Tue, 2008-10-28 at 03:43 -0700, David Brownell wrote:
> > > On Tuesday 28 October 2008, Kaiwan N Billimoria wrote:
> > > > Hi,
> > > > 
> > > > Got a chance to test this patch today..
> > > > short answer: no it did not seem to work.
> > > 
> > > Well, then can you see what the issue is then?
> 
> Maybe the lm70llp code shouldn't set spi->bits_per_word to 16;
> that seems to be another problem.

Yes, that's one of the issues..in fact the main one.
I keep it at zero now.

> Recall that the whole reason we noticed this bug is that the $SUBJECT
> patch was -- correctly!! -- getting its MSBs from rxbuf[0], but Jean
> noted the lm70 code was different.
Yup neat catch!

Thanks guys,
Later,
Kaiwan.



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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[Kaiwan N Billimoria]

I've just sent in the patch (as a separate email). Pl take a look..

On Wed, 2008-10-29 at 23:49 -0700, David Brownell wrote:

> Maybe the lm70llp code shouldn't set spi->bits_per_word to 16;
> that seems to be another problem.
Well, kind of pecuiliar behaviour here..

When i set bits_per_word to 0, the chip does not sense temperature
correctly, returning wrong values.

When set to 8, it works just fine.

However: 
drivers/spi/spi_bitbang.c:spi_bitbang_setup
--snip--
      if (!spi->bits_per_word)
                 spi->bits_per_word = 8;
--snip--


- Kaiwan.



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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[Jean Delvare]

Hi Manuel,

On Thu, 23 Oct 2008 16:00:32 +0200, Manuel Lauss wrote:
> > Ah, yes, the lack of an i2c_driver.id_table parameter was why I originally
> > wrote a separate driver.  I suppose I could add another struct spi_driver
> > for the TMP121 and sort out chip types internally.
> 
> Does this look acceptable?
> 
> --- 
> 
> From: Manuel Lauss <mano@roarinelk.homelinux.net>
> Subject: [PATCH] hwmon: Extend LM70 with TMP121/TMP123 support.
> 
> The Texas Instruments TMP121 is a SPI temperature sensor very similar
> to the LM70, with slightly higher resolution.  This patch extends the
> LM70 driver to support the TMP121.
> 
> Signed-off-by: Manuel Lauss <mano@roarinelk.homelinux.net>
> ---
>  Documentation/hwmon/lm70 |    7 +++-
>  drivers/hwmon/Kconfig    |    5 ++-
>  drivers/hwmon/lm70.c     |   85 +++++++++++++++++++++++++++++++++++++++-------
>  3 files changed, 81 insertions(+), 16 deletions(-)

Yes, this version of the patch looks all OK to me. It simply needed
some fixes to apply properly on top of Kaiwan's lm70 fixes.

Can you please test the following two patches:
http://khali.linux-fr.org/devel/linux-2.6/jdelvare-hwmon/hwmon-lm70-01-fix-byte-order.patch
http://khali.linux-fr.org/devel/linux-2.6/jdelvare-hwmon/hwmon-lm70-02-ti-tmp121-support.patch

and confirm that things work fine for you with them applied?

-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[Manuel Lauss]

Good evening,

On Wed, Nov 12, 2008 at 09:39:49PM +0100, Jean Delvare wrote:
> Hi Manuel,
> 
> On Thu, 23 Oct 2008 16:00:32 +0200, Manuel Lauss wrote:
> > > Ah, yes, the lack of an i2c_driver.id_table parameter was why I originally
> > > wrote a separate driver.  I suppose I could add another struct spi_driver
> > > for the TMP121 and sort out chip types internally.
> > 
> > Does this look acceptable?
> > 
> > --- 
> > 
> > From: Manuel Lauss <mano@roarinelk.homelinux.net>
> > Subject: [PATCH] hwmon: Extend LM70 with TMP121/TMP123 support.
> > 
> > The Texas Instruments TMP121 is a SPI temperature sensor very similar
> > to the LM70, with slightly higher resolution.  This patch extends the
> > LM70 driver to support the TMP121.
> > 
> > Signed-off-by: Manuel Lauss <mano@roarinelk.homelinux.net>
> > ---
> >  Documentation/hwmon/lm70 |    7 +++-
> >  drivers/hwmon/Kconfig    |    5 ++-
> >  drivers/hwmon/lm70.c     |   85 +++++++++++++++++++++++++++++++++++++++-------
> >  3 files changed, 81 insertions(+), 16 deletions(-)
> 
> Yes, this version of the patch looks all OK to me. It simply needed
> some fixes to apply properly on top of Kaiwan's lm70 fixes.
> 
> Can you please test the following two patches:
> http://khali.linux-fr.org/devel/linux-2.6/jdelvare-hwmon/hwmon-lm70-01-fix-byte-order.patch
> http://khali.linux-fr.org/devel/linux-2.6/jdelvare-hwmon/hwmon-lm70-02-ti-tmp121-support.patch
> 
> and confirm that things work fine for you with them applied?

Actually I made a new one fixing a few things David pointed out and rebased
on top of Kaiwan's patch.  And, it's run-tested too ;-)

Thanks!
	Manuel Lauss

--- 

From 1a28a0df02deff221004ea5e8b48984a122968fe Mon Sep 17 00:00:00 2001
From: Manuel Lauss <mano@roarinelk.homelinux.net>
Date: Mon, 27 Oct 2008 19:00:04 +0100
Subject: [PATCH] hwmon: Extend LM70 with TMP121/TMP123 support.

The Texas Instruments TMP121 is a SPI temperature sensor very similar
to the LM70, with slightly higher resolution.  This patch extends the
LM70 driver to support the TMP121.  The TMP123 differs in pin assign-
ment.

Signed-off-by: Manuel Lauss <mano@roarinelk.homelinux.net>
---
 Documentation/hwmon/lm70 |    8 ++++-
 drivers/hwmon/Kconfig    |    5 ++-
 drivers/hwmon/lm70.c     |   84 ++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 84 insertions(+), 13 deletions(-)

diff --git a/Documentation/hwmon/lm70 b/Documentation/hwmon/lm70
index dc5c975..4fb18fd 100644
--- a/Documentation/hwmon/lm70
+++ b/Documentation/hwmon/lm70
@@ -1,9 +1,11 @@
 Kernel driver lm70
 =========
 
-Supported chip:
+Supported chips:
   * National Semiconductor LM70
     Datasheet: http://www.national.com/pf/LM/LM70.html
+  * Texas Instruments TMP121/TMP123
+    Information: http://focus.ti.com/docs/prod/folders/print/tmp121.html
 
 Author:
         Kaiwan N Billimoria <kaiwan@designergraphix.com>
@@ -29,6 +31,10 @@ As a real (in-tree) example of this "logical SPI protocol driver" interfacing
 with a "physical SPI master controller" driver, see drivers/spi/spi_lm70llp
 and it's 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).
+
 Thanks to
 ---------
 Jean Delvare <khali@linux-fr.org> for mentoring the hwmon-side driver
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 6de1e0f..f86923d 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -407,11 +407,12 @@ config SENSORS_LM63
 	  will be called lm63.
 
 config SENSORS_LM70
-	tristate "National Semiconductor LM70"
+	tristate "National Semiconductor LM70 / Texas Instruments TMP121"
 	depends on SPI_MASTER && EXPERIMENTAL
 	help
 	  If you say yes here you get support for the National Semiconductor
-	  LM70 digital temperature sensor chip.
+	  LM70 and Texas Instruments TMP121/TMP123 digital temperature
+	  sensor chips.
 
 	  This driver can also be built as a module.  If so, the module
 	  will be called lm70.
diff --git a/drivers/hwmon/lm70.c b/drivers/hwmon/lm70.c
index 8618014..d31522b 100644
--- a/drivers/hwmon/lm70.c
+++ b/drivers/hwmon/lm70.c
@@ -37,9 +37,13 @@
 
 #define DRVNAME		"lm70"
 
+#define LM70_CHIP_LM70		0	/* original NS LM70 */
+#define LM70_CHIP_TMP121	1	/* TI TMP121/TMP123 */
+
 struct lm70 {
 	struct device *hwmon_dev;
 	struct mutex lock;
+	unsigned int chip;
 };
 
 /* sysfs hook function */
@@ -47,7 +51,7 @@ static ssize_t lm70_sense_temp(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
 	struct spi_device *spi = to_spi_device(dev);
-	int status, val;
+	int status, val = 0;
 	u8 rxbuf[2];
 	s16 raw=0;
 	struct lm70 *p_lm70 = dev_get_drvdata(&spi->dev);
@@ -70,6 +74,7 @@ static ssize_t lm70_sense_temp(struct device *dev,
 		rxbuf[0], rxbuf[1], raw);
 
 	/*
+	 * LM70:
 	 * The "raw" temperature read into rxbuf[] is a 16-bit signed 2's
 	 * complement value. Only the MSB 11 bits (1 sign + 10 temperature
 	 * bits) are meaningful; the LSB 5 bits are to be discarded.
@@ -79,8 +84,21 @@ static ssize_t lm70_sense_temp(struct device *dev,
 	 * by 0.25. Also multiply by 1000 to represent in millidegrees
 	 * Celsius.
 	 * So it's equivalent to multiplying by 0.25 * 1000 = 250.
+	 *
+	 * TMP121/TMP123:
+	 * 13 bits of 2's complement data, discard LSB 3 bits,
+	 * resolution 0.0625 degrees celsius.
 	 */
-	val = ((int)raw/32) * 250;
+	switch (p_lm70->chip) {
+	case LM70_CHIP_LM70:
+		val = ((int)raw / 32) * 250;
+		break;
+
+	case LM70_CHIP_TMP121:
+		val = ((int)raw / 8) * 625 / 10;
+		break;
+	}
+
 	status = sprintf(buf, "%d\n", val); /* millidegrees Celsius */
 out:
 	mutex_unlock(&p_lm70->lock);
@@ -92,27 +110,37 @@ static DEVICE_ATTR(temp1_input, S_IRUGO, lm70_sense_temp, NULL);
 static ssize_t lm70_show_name(struct device *dev, struct device_attribute
 			      *devattr, char *buf)
 {
-	return sprintf(buf, "lm70\n");
+	struct lm70 *p_lm70 = dev_get_drvdata(dev);
+	int ret;
+
+	switch (p_lm70->chip) {
+	case LM70_CHIP_LM70:
+		ret = sprintf(buf, "lm70\n");
+		break;
+	case LM70_CHIP_TMP121:
+		ret = sprintf(buf, "tmp121\n");
+		break;
+	default:
+		ret = -EINVAL;
+	}
+	return ret;
 }
 
 static DEVICE_ATTR(name, S_IRUGO, lm70_show_name, NULL);
 
 /*----------------------------------------------------------------------*/
 
-static int __devinit lm70_probe(struct spi_device *spi)
+static int __devinit common_probe(struct spi_device *spi, int chip)
 {
 	struct lm70 *p_lm70;
 	int status;
 
-	/* signaling is SPI_MODE_0 on a 3-wire link (shared SI/SO) */
-	if ((spi->mode & (SPI_CPOL|SPI_CPHA)) || !(spi->mode & SPI_3WIRE))
-		return -EINVAL;
-
 	p_lm70 = kzalloc(sizeof *p_lm70, GFP_KERNEL);
 	if (!p_lm70)
 		return -ENOMEM;
 
 	mutex_init(&p_lm70->lock);
+	p_lm70->chip = chip;
 
 	/* sysfs hook */
 	p_lm70->hwmon_dev = hwmon_device_register(&spi->dev);
@@ -140,6 +168,24 @@ out_dev_reg_failed:
 	return status;
 }
 
+static int __devinit lm70_probe(struct spi_device *spi)
+{
+	/* signaling is SPI_MODE_0 on a 3-wire link (shared SI/SO) */
+	if ((spi->mode & (SPI_CPOL | SPI_CPHA)) || !(spi->mode & SPI_3WIRE))
+		return -EINVAL;
+
+	return common_probe(spi, LM70_CHIP_LM70);
+}
+
+static int __devinit tmp121_probe(struct spi_device *spi)
+{
+	/* signaling is SPI_MODE_0 with only MISO connected */
+	if (spi->mode & (SPI_CPOL | SPI_CPHA))
+		return -EINVAL;
+
+	return common_probe(spi, LM70_CHIP_TMP121);
+}
+
 static int __devexit lm70_remove(struct spi_device *spi)
 {
 	struct lm70 *p_lm70 = dev_get_drvdata(&spi->dev);
@@ -153,6 +199,15 @@ static int __devexit lm70_remove(struct spi_device *spi)
 	return 0;
 }
 
+static struct spi_driver tmp121_driver = {
+	.driver = {
+		.name	= "tmp121",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= tmp121_probe,
+	.remove	= __devexit_p(lm70_remove),
+};
+
 static struct spi_driver lm70_driver = {
 	.driver = {
 		.name	= "lm70",
@@ -164,17 +219,26 @@ static struct spi_driver lm70_driver = {
 
 static int __init init_lm70(void)
 {
-	return spi_register_driver(&lm70_driver);
+	int ret = spi_register_driver(&lm70_driver);
+	if (ret)
+		return ret;
+
+	ret = spi_register_driver(&tmp121_driver);
+	if (ret)
+		spi_unregister_driver(&lm70_driver);
+
+	return ret;
 }
 
 static void __exit cleanup_lm70(void)
 {
 	spi_unregister_driver(&lm70_driver);
+	spi_unregister_driver(&tmp121_driver);
 }
 
 module_init(init_lm70);
 module_exit(cleanup_lm70);
 
 MODULE_AUTHOR("Kaiwan N Billimoria");
-MODULE_DESCRIPTION("National Semiconductor LM70 Linux driver");
+MODULE_DESCRIPTION("NS LM70 / TI TMP121/TMP123 Linux driver");
 MODULE_LICENSE("GPL");
-- 
1.6.0.3


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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[Jean Delvare]

Hi Manuel,

On Wed, 12 Nov 2008 23:06:03 +0100, Manuel Lauss wrote:
> Good evening,
> 
> On Wed, Nov 12, 2008 at 09:39:49PM +0100, Jean Delvare wrote:
> > Yes, this version of the patch looks all OK to me. It simply needed
> > some fixes to apply properly on top of Kaiwan's lm70 fixes.
> > 
> > Can you please test the following two patches:
> > http://khali.linux-fr.org/devel/linux-2.6/jdelvare-hwmon/hwmon-lm70-01-fix-byte-order.patch
> > http://khali.linux-fr.org/devel/linux-2.6/jdelvare-hwmon/hwmon-lm70-02-ti-tmp121-support.patch
> > 
> > and confirm that things work fine for you with them applied?
> 
> Actually I made a new one fixing a few things David pointed out and rebased
> on top of Kaiwan's patch.  And, it's run-tested too ;-)
> 
> Thanks!
> 	Manuel Lauss
> 
> --- 
> 
> From 1a28a0df02deff221004ea5e8b48984a122968fe Mon Sep 17 00:00:00 2001
> From: Manuel Lauss <mano@roarinelk.homelinux.net>
> Date: Mon, 27 Oct 2008 19:00:04 +0100
> Subject: [PATCH] hwmon: Extend LM70 with TMP121/TMP123 support.
> 
> The Texas Instruments TMP121 is a SPI temperature sensor very similar
> to the LM70, with slightly higher resolution.  This patch extends the
> LM70 driver to support the TMP121.  The TMP123 differs in pin assign-
> ment.
> 
> Signed-off-by: Manuel Lauss <mano@roarinelk.homelinux.net>
> ---
>  Documentation/hwmon/lm70 |    8 ++++-
>  drivers/hwmon/Kconfig    |    5 ++-
>  drivers/hwmon/lm70.c     |   84 ++++++++++++++++++++++++++++++++++++++++-----
>  3 files changed, 84 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/hwmon/lm70 b/Documentation/hwmon/lm70
> index dc5c975..4fb18fd 100644
> --- a/Documentation/hwmon/lm70
> +++ b/Documentation/hwmon/lm70
> @@ -1,9 +1,11 @@
>  Kernel driver lm70
>  =========
>  
> -Supported chip:
> +Supported chips:
>    * National Semiconductor LM70
>      Datasheet: http://www.national.com/pf/LM/LM70.html
> +  * Texas Instruments TMP121/TMP123
> +    Information: http://focus.ti.com/docs/prod/folders/print/tmp121.html
>  
>  Author:
>          Kaiwan N Billimoria <kaiwan@designergraphix.com>
> @@ -29,6 +31,10 @@ As a real (in-tree) example of this "logical SPI protocol driver" interfacing
>  with a "physical SPI master controller" driver, see drivers/spi/spi_lm70llp
>  and it's 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).
> +
>  Thanks to
>  ---------
>  Jean Delvare <khali@linux-fr.org> for mentoring the hwmon-side driver
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 6de1e0f..f86923d 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -407,11 +407,12 @@ config SENSORS_LM63
>  	  will be called lm63.
>  
>  config SENSORS_LM70
> -	tristate "National Semiconductor LM70"
> +	tristate "National Semiconductor LM70 / Texas Instruments TMP121"
>  	depends on SPI_MASTER && EXPERIMENTAL
>  	help
>  	  If you say yes here you get support for the National Semiconductor
> -	  LM70 digital temperature sensor chip.
> +	  LM70 and Texas Instruments TMP121/TMP123 digital temperature
> +	  sensor chips.
>  
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called lm70.
> diff --git a/drivers/hwmon/lm70.c b/drivers/hwmon/lm70.c
> index 8618014..d31522b 100644
> --- a/drivers/hwmon/lm70.c
> +++ b/drivers/hwmon/lm70.c
> @@ -37,9 +37,13 @@
>  
>  #define DRVNAME		"lm70"
>  
> +#define LM70_CHIP_LM70		0	/* original NS LM70 */
> +#define LM70_CHIP_TMP121	1	/* TI TMP121/TMP123 */
> +
>  struct lm70 {
>  	struct device *hwmon_dev;
>  	struct mutex lock;
> +	unsigned int chip;
>  };
>  
>  /* sysfs hook function */
> @@ -47,7 +51,7 @@ static ssize_t lm70_sense_temp(struct device *dev,
>  		struct device_attribute *attr, char *buf)
>  {
>  	struct spi_device *spi = to_spi_device(dev);
> -	int status, val;
> +	int status, val = 0;
>  	u8 rxbuf[2];
>  	s16 raw=0;
>  	struct lm70 *p_lm70 = dev_get_drvdata(&spi->dev);
> @@ -70,6 +74,7 @@ static ssize_t lm70_sense_temp(struct device *dev,
>  		rxbuf[0], rxbuf[1], raw);
>  
>  	/*
> +	 * LM70:
>  	 * The "raw" temperature read into rxbuf[] is a 16-bit signed 2's
>  	 * complement value. Only the MSB 11 bits (1 sign + 10 temperature
>  	 * bits) are meaningful; the LSB 5 bits are to be discarded.
> @@ -79,8 +84,21 @@ static ssize_t lm70_sense_temp(struct device *dev,
>  	 * by 0.25. Also multiply by 1000 to represent in millidegrees
>  	 * Celsius.
>  	 * So it's equivalent to multiplying by 0.25 * 1000 = 250.
> +	 *
> +	 * TMP121/TMP123:
> +	 * 13 bits of 2's complement data, discard LSB 3 bits,
> +	 * resolution 0.0625 degrees celsius.
>  	 */
> -	val = ((int)raw/32) * 250;
> +	switch (p_lm70->chip) {
> +	case LM70_CHIP_LM70:
> +		val = ((int)raw / 32) * 250;
> +		break;
> +
> +	case LM70_CHIP_TMP121:
> +		val = ((int)raw / 8) * 625 / 10;
> +		break;
> +	}
> +
>  	status = sprintf(buf, "%d\n", val); /* millidegrees Celsius */
>  out:
>  	mutex_unlock(&p_lm70->lock);
> @@ -92,27 +110,37 @@ static DEVICE_ATTR(temp1_input, S_IRUGO, lm70_sense_temp, NULL);
>  static ssize_t lm70_show_name(struct device *dev, struct device_attribute
>  			      *devattr, char *buf)
>  {
> -	return sprintf(buf, "lm70\n");
> +	struct lm70 *p_lm70 = dev_get_drvdata(dev);
> +	int ret;
> +
> +	switch (p_lm70->chip) {
> +	case LM70_CHIP_LM70:
> +		ret = sprintf(buf, "lm70\n");
> +		break;
> +	case LM70_CHIP_TMP121:
> +		ret = sprintf(buf, "tmp121\n");
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +	return ret;
>  }
>  
>  static DEVICE_ATTR(name, S_IRUGO, lm70_show_name, NULL);
>  
>  /*----------------------------------------------------------------------*/
>  
> -static int __devinit lm70_probe(struct spi_device *spi)
> +static int __devinit common_probe(struct spi_device *spi, int chip)
>  {
>  	struct lm70 *p_lm70;
>  	int status;
>  
> -	/* signaling is SPI_MODE_0 on a 3-wire link (shared SI/SO) */
> -	if ((spi->mode & (SPI_CPOL|SPI_CPHA)) || !(spi->mode & SPI_3WIRE))
> -		return -EINVAL;
> -
>  	p_lm70 = kzalloc(sizeof *p_lm70, GFP_KERNEL);
>  	if (!p_lm70)
>  		return -ENOMEM;
>  
>  	mutex_init(&p_lm70->lock);
> +	p_lm70->chip = chip;
>  
>  	/* sysfs hook */
>  	p_lm70->hwmon_dev = hwmon_device_register(&spi->dev);
> @@ -140,6 +168,24 @@ out_dev_reg_failed:
>  	return status;
>  }
>  
> +static int __devinit lm70_probe(struct spi_device *spi)
> +{
> +	/* signaling is SPI_MODE_0 on a 3-wire link (shared SI/SO) */
> +	if ((spi->mode & (SPI_CPOL | SPI_CPHA)) || !(spi->mode & SPI_3WIRE))
> +		return -EINVAL;
> +
> +	return common_probe(spi, LM70_CHIP_LM70);
> +}
> +
> +static int __devinit tmp121_probe(struct spi_device *spi)
> +{
> +	/* signaling is SPI_MODE_0 with only MISO connected */
> +	if (spi->mode & (SPI_CPOL | SPI_CPHA))
> +		return -EINVAL;
> +
> +	return common_probe(spi, LM70_CHIP_TMP121);
> +}
> +
>  static int __devexit lm70_remove(struct spi_device *spi)
>  {
>  	struct lm70 *p_lm70 = dev_get_drvdata(&spi->dev);
> @@ -153,6 +199,15 @@ static int __devexit lm70_remove(struct spi_device *spi)
>  	return 0;
>  }
>  
> +static struct spi_driver tmp121_driver = {
> +	.driver = {
> +		.name	= "tmp121",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe	= tmp121_probe,
> +	.remove	= __devexit_p(lm70_remove),
> +};
> +
>  static struct spi_driver lm70_driver = {
>  	.driver = {
>  		.name	= "lm70",
> @@ -164,17 +219,26 @@ static struct spi_driver lm70_driver = {
>  
>  static int __init init_lm70(void)
>  {
> -	return spi_register_driver(&lm70_driver);
> +	int ret = spi_register_driver(&lm70_driver);
> +	if (ret)
> +		return ret;
> +
> +	ret = spi_register_driver(&tmp121_driver);
> +	if (ret)
> +		spi_unregister_driver(&lm70_driver);
> +
> +	return ret;
>  }
>  
>  static void __exit cleanup_lm70(void)
>  {
>  	spi_unregister_driver(&lm70_driver);
> +	spi_unregister_driver(&tmp121_driver);
>  }
>  
>  module_init(init_lm70);
>  module_exit(cleanup_lm70);
>  
>  MODULE_AUTHOR("Kaiwan N Billimoria");
> -MODULE_DESCRIPTION("National Semiconductor LM70 Linux driver");
> +MODULE_DESCRIPTION("NS LM70 / TI TMP121/TMP123 Linux driver");
>  MODULE_LICENSE("GPL");

Applied, thanks. I had to fix one hunk manually to apply after David's
changes, hopefully I got it right. At least it builds ;)

Updated patches at:
http://khali.linux-fr.org/devel/linux-2.6/jdelvare-hwmon/hwmon-lm70-01-fix-byte-order.patch
http://khali.linux-fr.org/devel/linux-2.6/jdelvare-hwmon/hwmon-lm70-02-ti-tmp121-support.patch

-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH RESEND] hwmon: lm70: TI TMP121 support.

[Manuel Lauss]

Howdy Jean,

On Thu, Nov 13, 2008 at 11:05:22AM +0100, Jean Delvare wrote:
> Hi Manuel,
> 
> On Wed, 12 Nov 2008 23:06:03 +0100, Manuel Lauss wrote:
> > Good evening,
> > 
> > On Wed, Nov 12, 2008 at 09:39:49PM +0100, Jean Delvare wrote:
> > > Yes, this version of the patch looks all OK to me. It simply needed
> > > some fixes to apply properly on top of Kaiwan's lm70 fixes.
> > > 
> > > Can you please test the following two patches:
> > > http://khali.linux-fr.org/devel/linux-2.6/jdelvare-hwmon/hwmon-lm70-01-fix-byte-order.patch
> > > http://khali.linux-fr.org/devel/linux-2.6/jdelvare-hwmon/hwmon-lm70-02-ti-tmp121-support.patch
> > > 
> > > and confirm that things work fine for you with them applied?
> > 
> > Actually I made a new one fixing a few things David pointed out and rebased
> > on top of Kaiwan's patch.  And, it's run-tested too ;-)

[patch snipped]
 
> Applied, thanks. I had to fix one hunk manually to apply after David's
> changes, hopefully I got it right. At least it builds ;)
> 
> Updated patches at:
> http://khali.linux-fr.org/devel/linux-2.6/jdelvare-hwmon/hwmon-lm70-01-fix-byte-order.patch
> http://khali.linux-fr.org/devel/linux-2.6/jdelvare-hwmon/hwmon-lm70-02-ti-tmp121-support.patch

Works fine.

Thank you!
	Manuel Lauss

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