All of lore.kernel.org
 help / color / mirror / Atom feed
* [lm-sensors] [PATCH] SPI lm70: Code streamlining and cleanup
@ 2008-11-12  7:38 Kaiwan N Billimoria
  2008-11-12  7:59 ` David Brownell
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Kaiwan N Billimoria @ 2008-11-12  7:38 UTC (permalink / raw)
  To: lm-sensors

spi_lm70llp driver:
-changed the bits_per_word setting.
-removed the convoluted if (first_time) ... else ... logic from the
bit-banging routine;
it now invokes a single bitbang_txrx_* call as required.
-better control over the parport D9 line in (de)assert inline routines
(see schematic for details).

lm70 driver:
-parse and save raw temperature in 'correct' byte-order.

Documentation/ updates as appropriate.

Signed-off-by: Kaiwan N Billimoria <kaiwan@designergraphix.com>

---
diff -uprN -X a/Documentation/dontdiff a/Documentation/hwmon/lm70
b/Documentation/hwmon/lm70
--- a/Documentation/hwmon/lm70	2008-10-10 03:43:53.000000000 +0530
+++ b/Documentation/hwmon/lm70	2008-11-12 12:17:21.000000000 +0530
@@ -25,6 +25,10 @@ complement digital temperature (sent via
 driver for interpretation. This driver makes use of the kernel's
in-core
 SPI support.
 
+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.
+
 Thanks to
 ---------
 Jean Delvare <khali@linux-fr.org> for mentoring the hwmon-side driver
diff -uprN -X a/Documentation/dontdiff a/Documentation/spi/spi-lm70llp
b/Documentation/spi/spi-lm70llp
--- a/Documentation/spi/spi-lm70llp	2008-10-10 03:43:53.000000000 +0530
+++ b/Documentation/spi/spi-lm70llp	2008-11-12 12:13:50.000000000 +0530
@@ -13,10 +13,19 @@ Description
 This driver provides glue code connecting a National Semiconductor LM70
LLP
 temperature sensor evaluation board to the kernel's SPI core subsystem.
 
+This is an SPI master controller driver. It can be used in conjunction
with 
+(layered under) the LM70 logical driver (an "SPI protocol driver").
 In effect, this driver turns the parallel port interface on the eval
board
 into a SPI bus with a single device, which will be driven by the
generic
 LM70 driver (drivers/hwmon/lm70.c).
 
+
+Hardware Interfacing
+--------------------
+The schematic for this particular board (the LM70LLP eval board) is 
+available (on page 4) here:
+http://www.designergraphix.com/pull/spi_lm70/LM70LLPEVALmanual.pdf
+
 The hardware interfacing on the LM70 LLP eval board is as follows:
 
    Parallel                 LM70 LLP
@@ -67,3 +76,4 @@ Thanks to
 o David Brownell for mentoring the SPI-side driver development.
 o Dr.Craig Hollabaugh for the (early) "manual" bitbanging driver
version.
 o Nadir Billimoria for help interpreting the circuit schematic.
+
diff -uprN -X a/Documentation/dontdiff a/drivers/hwmon/lm70.c
b/drivers/hwmon/lm70.c
--- a/drivers/hwmon/lm70.c	2008-10-10 03:43:53.000000000 +0530
+++ b/drivers/hwmon/lm70.c	2008-11-12 12:18:34.000000000 +0530
@@ -65,10 +65,9 @@ static ssize_t lm70_sense_temp(struct de
 		"spi_write_then_read failed with status %d\n", status);
 		goto out;
 	}
-	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);
+	raw = (rxbuf[0] << 8) + rxbuf[1];
+	dev_dbg(dev, "rxbuf[0] : 0x%x rxbuf[1] : 0x%x raw=0x%08x\n", 
+		rxbuf[0], rxbuf[1], raw);
 
 	/*
 	 * The "raw" temperature read into rxbuf[] is a 16-bit signed 2's
diff -uprN -X a/Documentation/dontdiff a/drivers/spi/spi_lm70llp.c
b/drivers/spi/spi_lm70llp.c
--- a/drivers/spi/spi_lm70llp.c	2008-10-10 03:43:53.000000000 +0530
+++ b/drivers/spi/spi_lm70llp.c	2008-11-12 12:12:16.000000000 +0530
@@ -1,5 +1,5 @@
 /*
- * spi_lm70llp.c - driver for lm70llp eval board for the LM70 sensor
+ * spi_lm70llp.c - driver for LM70LLP eval board for the LM70 sensor
  *
  * Copyright (C) 2006 Kaiwan N Billimoria <kaiwan@designergraphix.com>
  *
@@ -27,11 +27,9 @@
 #include <linux/sysfs.h>
 #include <linux/workqueue.h>
 
-
 #include <linux/spi/spi.h>
 #include <linux/spi/spi_bitbang.h>
 
-
 /*
  * The LM70 communicates with a host processor using a 3-wire variant
of
  * the SPI/Microwire bus interface. This driver specifically supports
an
@@ -40,8 +38,14 @@
  * master controller driver.  The hwmon/lm70 driver is a "SPI protocol
  * driver", layered on top of this one and usable without the lm70llp.
  *
+ * Datasheet and Schematic:
  * The LM70 is a temperature sensor chip from National Semiconductor;
its
  * datasheet is available at http://www.national.com/pf/LM/LM70.html
+ * You can also find it here: 
+ *  http://www.designergraphix.com/pull/spi_lm70/LM70.pdf
+ * The schematic for this particular board (the LM70LLP eval board) is 
+ * available (on page 4) here:
+ *  http://www.designergraphix.com/pull/spi_lm70/LM70LLPEVALmanual.pdf
  *
  * Also see Documentation/spi/spi-lm70llp.  The SPI<->parport code here
is
  * (heavily) based on spi-butterfly by David Brownell.
@@ -64,7 +68,7 @@
  *
  * Note that parport pin 13 actually gets inverted by the transistor
  * arrangement which lets either the parport or the LM70 drive the
- * SI/SO signal.
+ * SI/SO signal (see the schematic for details).
  */
 
 #define DRVNAME		"spi-lm70llp"
@@ -106,12 +110,14 @@ static inline struct spi_lm70llp *spidev
 static inline void deassertCS(struct spi_lm70llp *pp)
 {
 	u8 data = parport_read_data(pp->port);
+	data &= ~0x80; /* keep D9 low while de-asserted */
 	parport_write_data(pp->port, data | nCS);
 }
 
 static inline void assertCS(struct spi_lm70llp *pp)
 {
 	u8 data = parport_read_data(pp->port);
+	data |= 0x80; /* keep D9 high while asserted */
 	parport_write_data(pp->port, data & ~nCS);
 }
 
@@ -184,22 +190,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)
@@ -293,10 +284,9 @@ static void spi_lm70llp_attach(struct pa
 		status = -ENODEV;
 		goto out_bitbang_stop;
 	}
-	pp->spidev_lm70->bits_per_word = 16;
+	pp->spidev_lm70->bits_per_word = 8;
 
 	lm70llp = pp;
-
 	return;
 
 out_bitbang_stop:
@@ -326,7 +316,6 @@ static void spi_lm70llp_detach(struct pa
 
 	/* power down */
 	parport_write_data(pp->port, 0);
-	msleep(10);
 
 	parport_release(pp->pd);
 	parport_unregister_device(pp->pd);


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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [lm-sensors] [PATCH] SPI lm70: Code streamlining and cleanup
  2008-11-12  7:38 [lm-sensors] [PATCH] SPI lm70: Code streamlining and cleanup Kaiwan N Billimoria
@ 2008-11-12  7:59 ` David Brownell
  2008-11-12  8:30 ` Kaiwan N Billimoria
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: David Brownell @ 2008-11-12  7:59 UTC (permalink / raw)
  To: lm-sensors

On Tuesday 11 November 2008, Kaiwan N Billimoria wrote:
> +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.

This patch arrived badly word wrapped.  Could you resend?
Attachment is OK.


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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [lm-sensors] [PATCH] SPI lm70: Code streamlining and cleanup
  2008-11-12  7:38 [lm-sensors] [PATCH] SPI lm70: Code streamlining and cleanup Kaiwan N Billimoria
  2008-11-12  7:59 ` David Brownell
@ 2008-11-12  8:30 ` Kaiwan N Billimoria
  2008-11-12 13:25 ` Jean Delvare
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Kaiwan N Billimoria @ 2008-11-12  8:30 UTC (permalink / raw)
  To: lm-sensors

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

On Tue, 2008-11-11 at 23:59 -0800, David Brownell wrote:
> On Tuesday 11 November 2008, Kaiwan N Billimoria wrote:
> > +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.
> 
> This patch arrived badly word wrapped.  Could you resend?
> Attachment is OK.

Sigh. sorry..

Pl find the patch attached.

Regards,
-kaiwan.


[-- Attachment #2: spi_lm70.nov08.patch --]
[-- Type: text/x-patch, Size: 6012 bytes --]

diff -uprN -X a/Documentation/dontdiff a/Documentation/hwmon/lm70 b/Documentation/hwmon/lm70
--- a/Documentation/hwmon/lm70	2008-10-10 03:43:53.000000000 +0530
+++ b/Documentation/hwmon/lm70	2008-11-12 12:17:21.000000000 +0530
@@ -25,6 +25,10 @@ complement digital temperature (sent via
 driver for interpretation. This driver makes use of the kernel's in-core
 SPI support.
 
+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.
+
 Thanks to
 ---------
 Jean Delvare <khali@linux-fr.org> for mentoring the hwmon-side driver
diff -uprN -X a/Documentation/dontdiff a/Documentation/spi/spi-lm70llp b/Documentation/spi/spi-lm70llp
--- a/Documentation/spi/spi-lm70llp	2008-10-10 03:43:53.000000000 +0530
+++ b/Documentation/spi/spi-lm70llp	2008-11-12 12:13:50.000000000 +0530
@@ -13,10 +13,19 @@ Description
 This driver provides glue code connecting a National Semiconductor LM70 LLP
 temperature sensor evaluation board to the kernel's SPI core subsystem.
 
+This is an SPI master controller driver. It can be used in conjunction with 
+(layered under) the LM70 logical driver (an "SPI protocol driver").
 In effect, this driver turns the parallel port interface on the eval board
 into a SPI bus with a single device, which will be driven by the generic
 LM70 driver (drivers/hwmon/lm70.c).
 
+
+Hardware Interfacing
+--------------------
+The schematic for this particular board (the LM70LLP eval board) is 
+available (on page 4) here:
+http://www.designergraphix.com/pull/spi_lm70/LM70LLPEVALmanual.pdf
+
 The hardware interfacing on the LM70 LLP eval board is as follows:
 
    Parallel                 LM70 LLP
@@ -67,3 +76,4 @@ Thanks to
 o David Brownell for mentoring the SPI-side driver development.
 o Dr.Craig Hollabaugh for the (early) "manual" bitbanging driver version.
 o Nadir Billimoria for help interpreting the circuit schematic.
+
diff -uprN -X a/Documentation/dontdiff a/drivers/hwmon/lm70.c b/drivers/hwmon/lm70.c
--- a/drivers/hwmon/lm70.c	2008-10-10 03:43:53.000000000 +0530
+++ b/drivers/hwmon/lm70.c	2008-11-12 12:18:34.000000000 +0530
@@ -65,10 +65,9 @@ static ssize_t lm70_sense_temp(struct de
 		"spi_write_then_read failed with status %d\n", status);
 		goto out;
 	}
-	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);
+	raw = (rxbuf[0] << 8) + rxbuf[1];
+	dev_dbg(dev, "rxbuf[0] : 0x%x rxbuf[1] : 0x%x raw=0x%08x\n", 
+		rxbuf[0], rxbuf[1], raw);
 
 	/*
 	 * The "raw" temperature read into rxbuf[] is a 16-bit signed 2's
diff -uprN -X a/Documentation/dontdiff a/drivers/spi/spi_lm70llp.c b/drivers/spi/spi_lm70llp.c
--- a/drivers/spi/spi_lm70llp.c	2008-10-10 03:43:53.000000000 +0530
+++ b/drivers/spi/spi_lm70llp.c	2008-11-12 12:12:16.000000000 +0530
@@ -1,5 +1,5 @@
 /*
- * spi_lm70llp.c - driver for lm70llp eval board for the LM70 sensor
+ * spi_lm70llp.c - driver for LM70LLP eval board for the LM70 sensor
  *
  * Copyright (C) 2006 Kaiwan N Billimoria <kaiwan@designergraphix.com>
  *
@@ -27,11 +27,9 @@
 #include <linux/sysfs.h>
 #include <linux/workqueue.h>
 
-
 #include <linux/spi/spi.h>
 #include <linux/spi/spi_bitbang.h>
 
-
 /*
  * The LM70 communicates with a host processor using a 3-wire variant of
  * the SPI/Microwire bus interface. This driver specifically supports an
@@ -40,8 +38,14 @@
  * master controller driver.  The hwmon/lm70 driver is a "SPI protocol
  * driver", layered on top of this one and usable without the lm70llp.
  *
+ * Datasheet and Schematic:
  * The LM70 is a temperature sensor chip from National Semiconductor; its
  * datasheet is available at http://www.national.com/pf/LM/LM70.html
+ * You can also find it here: 
+ *  http://www.designergraphix.com/pull/spi_lm70/LM70.pdf
+ * The schematic for this particular board (the LM70LLP eval board) is 
+ * available (on page 4) here:
+ *  http://www.designergraphix.com/pull/spi_lm70/LM70LLPEVALmanual.pdf
  *
  * Also see Documentation/spi/spi-lm70llp.  The SPI<->parport code here is
  * (heavily) based on spi-butterfly by David Brownell.
@@ -64,7 +68,7 @@
  *
  * Note that parport pin 13 actually gets inverted by the transistor
  * arrangement which lets either the parport or the LM70 drive the
- * SI/SO signal.
+ * SI/SO signal (see the schematic for details).
  */
 
 #define DRVNAME		"spi-lm70llp"
@@ -106,12 +110,14 @@ static inline struct spi_lm70llp *spidev
 static inline void deassertCS(struct spi_lm70llp *pp)
 {
 	u8 data = parport_read_data(pp->port);
+	data &= ~0x80; /* keep D9 low while de-asserted */
 	parport_write_data(pp->port, data | nCS);
 }
 
 static inline void assertCS(struct spi_lm70llp *pp)
 {
 	u8 data = parport_read_data(pp->port);
+	data |= 0x80; /* keep D9 high while asserted */
 	parport_write_data(pp->port, data & ~nCS);
 }
 
@@ -184,22 +190,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)
@@ -293,10 +284,9 @@ static void spi_lm70llp_attach(struct pa
 		status = -ENODEV;
 		goto out_bitbang_stop;
 	}
-	pp->spidev_lm70->bits_per_word = 16;
+	pp->spidev_lm70->bits_per_word = 8;
 
 	lm70llp = pp;
-
 	return;
 
 out_bitbang_stop:
@@ -326,7 +316,6 @@ static void spi_lm70llp_detach(struct pa
 
 	/* power down */
 	parport_write_data(pp->port, 0);
-	msleep(10);
 
 	parport_release(pp->pd);
 	parport_unregister_device(pp->pd);

[-- Attachment #3: Type: text/plain, Size: 153 bytes --]

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [lm-sensors] [PATCH] SPI lm70: Code streamlining and cleanup
  2008-11-12  7:38 [lm-sensors] [PATCH] SPI lm70: Code streamlining and cleanup Kaiwan N Billimoria
  2008-11-12  7:59 ` David Brownell
  2008-11-12  8:30 ` Kaiwan N Billimoria
@ 2008-11-12 13:25 ` Jean Delvare
  2008-11-13  5:35 ` Kaiwan N Billimoria
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Jean Delvare @ 2008-11-12 13:25 UTC (permalink / raw)
  To: lm-sensors

On Wed, 12 Nov 2008 13:48:30 +0530, Kaiwan N Billimoria wrote:
> On Tue, 2008-11-11 at 23:59 -0800, David Brownell wrote:
> > On Tuesday 11 November 2008, Kaiwan N Billimoria wrote:
> > > +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.
> > 
> > This patch arrived badly word wrapped.  Could you resend?
> > Attachment is OK.
> 
> Sigh. sorry..
> 
> Pl find the patch attached.

David, can we get this patch reviewed and pushed upstream quickly? So
that I can finally take Manuel's patch adding support for the TMP121.

Alternatively, I can take this patch in my hwmon tree after it has been
reviewed.

My own review, for the parts I can comment on:

> diff -uprN -X a/Documentation/dontdiff a/Documentation/hwmon/lm70 b/Documentation/hwmon/lm70
> --- a/Documentation/hwmon/lm70	2008-10-10 03:43:53.000000000 +0530
> +++ b/Documentation/hwmon/lm70	2008-11-12 12:17:21.000000000 +0530
> @@ -25,6 +25,10 @@ complement digital temperature (sent via
>  driver for interpretation. This driver makes use of the kernel's in-core
>  SPI support.
>  
> +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.

Spelling: its.

> +
>  Thanks to
>  ---------
>  Jean Delvare <khali@linux-fr.org> for mentoring the hwmon-side driver
> diff -uprN -X a/Documentation/dontdiff a/Documentation/spi/spi-lm70llp b/Documentation/spi/spi-lm70llp
> --- a/Documentation/spi/spi-lm70llp	2008-10-10 03:43:53.000000000 +0530
> +++ b/Documentation/spi/spi-lm70llp	2008-11-12 12:13:50.000000000 +0530
> @@ -13,10 +13,19 @@ Description
>  This driver provides glue code connecting a National Semiconductor LM70 LLP
>  temperature sensor evaluation board to the kernel's SPI core subsystem.
>  
> +This is an SPI master controller driver. It can be used in conjunction with 
> +(layered under) the LM70 logical driver (an "SPI protocol driver").
>  In effect, this driver turns the parallel port interface on the eval board
>  into a SPI bus with a single device, which will be driven by the generic
>  LM70 driver (drivers/hwmon/lm70.c).
>  
> +
> +Hardware Interfacing
> +--------------------
> +The schematic for this particular board (the LM70LLP eval board) is 
> +available (on page 4) here:
> +http://www.designergraphix.com/pull/spi_lm70/LM70LLPEVALmanual.pdf
> +
>  The hardware interfacing on the LM70 LLP eval board is as follows:
>  
>     Parallel                 LM70 LLP
> @@ -67,3 +76,4 @@ Thanks to
>  o David Brownell for mentoring the SPI-side driver development.
>  o Dr.Craig Hollabaugh for the (early) "manual" bitbanging driver version.
>  o Nadir Billimoria for help interpreting the circuit schematic.
> +

Pointless change, should be reverted.

> diff -uprN -X a/Documentation/dontdiff a/drivers/hwmon/lm70.c b/drivers/hwmon/lm70.c
> --- a/drivers/hwmon/lm70.c	2008-10-10 03:43:53.000000000 +0530
> +++ b/drivers/hwmon/lm70.c	2008-11-12 12:18:34.000000000 +0530
> @@ -65,10 +65,9 @@ static ssize_t lm70_sense_temp(struct de
>  		"spi_write_then_read failed with status %d\n", status);
>  		goto out;
>  	}
> -	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);
> +	raw = (rxbuf[0] << 8) + rxbuf[1];
> +	dev_dbg(dev, "rxbuf[0] : 0x%x rxbuf[1] : 0x%x raw=0x%08x\n", 

raw is a s16, it makes little sense to print 8 digits. Should be:
0x%04x.

> +		rxbuf[0], rxbuf[1], raw);
>  
>  	/*
>  	 * The "raw" temperature read into rxbuf[] is a 16-bit signed 2's

-- 
Jean Delvare

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [lm-sensors] [PATCH] SPI lm70: Code streamlining and cleanup
  2008-11-12  7:38 [lm-sensors] [PATCH] SPI lm70: Code streamlining and cleanup Kaiwan N Billimoria
                   ` (2 preceding siblings ...)
  2008-11-12 13:25 ` Jean Delvare
@ 2008-11-13  5:35 ` Kaiwan N Billimoria
  2008-11-13  8:53 ` David Brownell
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Kaiwan N Billimoria @ 2008-11-13  5:35 UTC (permalink / raw)
  To: lm-sensors

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

Hi Jean,

Thanks for your comments; pl see below..

On Wed, 2008-11-12 at 14:25 +0100, Jean Delvare wrote:

> My own review, for the parts I can comment on:
> 
> > diff -uprN -X a/Documentation/dontdiff a/Documentation/hwmon/lm70 b/Documentation/hwmon/lm70
> > --- a/Documentation/hwmon/lm70	2008-10-10 03:43:53.000000000 +0530
> > +++ b/Documentation/hwmon/lm70	2008-11-12 12:17:21.000000000 +0530
> > @@ -25,6 +25,10 @@ complement digital temperature (sent via
> >  driver for interpretation. This driver makes use of the kernel's in-core
> >  SPI support.
> >  
> > +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.
> 
> Spelling: its.

Done.

> > +
> >  Thanks to
> >  ---------
> >  Jean Delvare <khali@linux-fr.org> for mentoring the hwmon-side driver
> > diff -uprN -X a/Documentation/dontdiff a/Documentation/spi/spi-lm70llp b/Documentation/spi/spi-lm70llp
> > --- a/Documentation/spi/spi-lm70llp	2008-10-10 03:43:53.000000000 +0530
> > +++ b/Documentation/spi/spi-lm70llp	2008-11-12 12:13:50.000000000 +0530
> > @@ -13,10 +13,19 @@ Description
> >  This driver provides glue code connecting a National Semiconductor LM70 LLP
> >  temperature sensor evaluation board to the kernel's SPI core subsystem.
> >  
> > +This is an SPI master controller driver. It can be used in conjunction with 
> > +(layered under) the LM70 logical driver (an "SPI protocol driver").
> >  In effect, this driver turns the parallel port interface on the eval board
> >  into a SPI bus with a single device, which will be driven by the generic
> >  LM70 driver (drivers/hwmon/lm70.c).
> >  
> > +
> > +Hardware Interfacing
> > +--------------------
> > +The schematic for this particular board (the LM70LLP eval board) is 
> > +available (on page 4) here:
> > +http://www.designergraphix.com/pull/spi_lm70/LM70LLPEVALmanual.pdf
> > +
> >  The hardware interfacing on the LM70 LLP eval board is as follows:
> >  
> >     Parallel                 LM70 LLP
> > @@ -67,3 +76,4 @@ Thanks to
> >  o David Brownell for mentoring the SPI-side driver development.
> >  o Dr.Craig Hollabaugh for the (early) "manual" bitbanging driver version.
> >  o Nadir Billimoria for help interpreting the circuit schematic.
> > +
> 
> Pointless change, should be reverted.

Sorry I do not follow you..which part are you referring to as pointless?
IMHO, the schematic PDF download is important for anyone trying to
understand the 'physical' driver.

> 
> > diff -uprN -X a/Documentation/dontdiff a/drivers/hwmon/lm70.c b/drivers/hwmon/lm70.c
> > --- a/drivers/hwmon/lm70.c	2008-10-10 03:43:53.000000000 +0530
> > +++ b/drivers/hwmon/lm70.c	2008-11-12 12:18:34.000000000 +0530
> > @@ -65,10 +65,9 @@ static ssize_t lm70_sense_temp(struct de
> >  		"spi_write_then_read failed with status %d\n", status);
> >  		goto out;
> >  	}
> > -	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);
> > +	raw = (rxbuf[0] << 8) + rxbuf[1];
> > +	dev_dbg(dev, "rxbuf[0] : 0x%x rxbuf[1] : 0x%x raw=0x%08x\n", 
> 
> raw is a s16, it makes little sense to print 8 digits. Should be:
> 0x%04x.
> 
Done.

Pl find the revised patch attached.

David, is the patch okay with you?
Pl let me know,

Regards,
Kaiwan.

[-- Attachment #2: spi_lm70.13nov08.patch --]
[-- Type: text/x-patch, Size: 6011 bytes --]

diff -uprN -X a/Documentation/dontdiff a/Documentation/hwmon/lm70 b/Documentation/hwmon/lm70
--- a/Documentation/hwmon/lm70	2008-10-10 03:43:53.000000000 +0530
+++ b/Documentation/hwmon/lm70	2008-11-13 10:38:27.000000000 +0530
@@ -25,6 +25,10 @@ complement digital temperature (sent via
 driver for interpretation. This driver makes use of the kernel's in-core
 SPI support.
 
+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 its associated documentation.
+
 Thanks to
 ---------
 Jean Delvare <khali@linux-fr.org> for mentoring the hwmon-side driver
diff -uprN -X a/Documentation/dontdiff a/Documentation/spi/spi-lm70llp b/Documentation/spi/spi-lm70llp
--- a/Documentation/spi/spi-lm70llp	2008-10-10 03:43:53.000000000 +0530
+++ b/Documentation/spi/spi-lm70llp	2008-11-12 12:13:50.000000000 +0530
@@ -13,10 +13,19 @@ Description
 This driver provides glue code connecting a National Semiconductor LM70 LLP
 temperature sensor evaluation board to the kernel's SPI core subsystem.
 
+This is an SPI master controller driver. It can be used in conjunction with 
+(layered under) the LM70 logical driver (an "SPI protocol driver").
 In effect, this driver turns the parallel port interface on the eval board
 into a SPI bus with a single device, which will be driven by the generic
 LM70 driver (drivers/hwmon/lm70.c).
 
+
+Hardware Interfacing
+--------------------
+The schematic for this particular board (the LM70LLP eval board) is 
+available (on page 4) here:
+http://www.designergraphix.com/pull/spi_lm70/LM70LLPEVALmanual.pdf
+
 The hardware interfacing on the LM70 LLP eval board is as follows:
 
    Parallel                 LM70 LLP
@@ -67,3 +76,4 @@ Thanks to
 o David Brownell for mentoring the SPI-side driver development.
 o Dr.Craig Hollabaugh for the (early) "manual" bitbanging driver version.
 o Nadir Billimoria for help interpreting the circuit schematic.
+
diff -uprN -X a/Documentation/dontdiff a/drivers/hwmon/lm70.c b/drivers/hwmon/lm70.c
--- a/drivers/hwmon/lm70.c	2008-10-10 03:43:53.000000000 +0530
+++ b/drivers/hwmon/lm70.c	2008-11-13 10:40:53.000000000 +0530
@@ -65,10 +65,9 @@ static ssize_t lm70_sense_temp(struct de
 		"spi_write_then_read failed with status %d\n", status);
 		goto out;
 	}
-	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);
+	raw = (rxbuf[0] << 8) + rxbuf[1];
+	dev_dbg(dev, "rxbuf[0] : 0x%x rxbuf[1] : 0x%x raw=0x%04x\n", 
+		rxbuf[0], rxbuf[1], raw);
 
 	/*
 	 * The "raw" temperature read into rxbuf[] is a 16-bit signed 2's
diff -uprN -X a/Documentation/dontdiff a/drivers/spi/spi_lm70llp.c b/drivers/spi/spi_lm70llp.c
--- a/drivers/spi/spi_lm70llp.c	2008-10-10 03:43:53.000000000 +0530
+++ b/drivers/spi/spi_lm70llp.c	2008-11-12 12:12:16.000000000 +0530
@@ -1,5 +1,5 @@
 /*
- * spi_lm70llp.c - driver for lm70llp eval board for the LM70 sensor
+ * spi_lm70llp.c - driver for LM70LLP eval board for the LM70 sensor
  *
  * Copyright (C) 2006 Kaiwan N Billimoria <kaiwan@designergraphix.com>
  *
@@ -27,11 +27,9 @@
 #include <linux/sysfs.h>
 #include <linux/workqueue.h>
 
-
 #include <linux/spi/spi.h>
 #include <linux/spi/spi_bitbang.h>
 
-
 /*
  * The LM70 communicates with a host processor using a 3-wire variant of
  * the SPI/Microwire bus interface. This driver specifically supports an
@@ -40,8 +38,14 @@
  * master controller driver.  The hwmon/lm70 driver is a "SPI protocol
  * driver", layered on top of this one and usable without the lm70llp.
  *
+ * Datasheet and Schematic:
  * The LM70 is a temperature sensor chip from National Semiconductor; its
  * datasheet is available at http://www.national.com/pf/LM/LM70.html
+ * You can also find it here: 
+ *  http://www.designergraphix.com/pull/spi_lm70/LM70.pdf
+ * The schematic for this particular board (the LM70LLP eval board) is 
+ * available (on page 4) here:
+ *  http://www.designergraphix.com/pull/spi_lm70/LM70LLPEVALmanual.pdf
  *
  * Also see Documentation/spi/spi-lm70llp.  The SPI<->parport code here is
  * (heavily) based on spi-butterfly by David Brownell.
@@ -64,7 +68,7 @@
  *
  * Note that parport pin 13 actually gets inverted by the transistor
  * arrangement which lets either the parport or the LM70 drive the
- * SI/SO signal.
+ * SI/SO signal (see the schematic for details).
  */
 
 #define DRVNAME		"spi-lm70llp"
@@ -106,12 +110,14 @@ static inline struct spi_lm70llp *spidev
 static inline void deassertCS(struct spi_lm70llp *pp)
 {
 	u8 data = parport_read_data(pp->port);
+	data &= ~0x80; /* keep D9 low while de-asserted */
 	parport_write_data(pp->port, data | nCS);
 }
 
 static inline void assertCS(struct spi_lm70llp *pp)
 {
 	u8 data = parport_read_data(pp->port);
+	data |= 0x80; /* keep D9 high while asserted */
 	parport_write_data(pp->port, data & ~nCS);
 }
 
@@ -184,22 +190,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)
@@ -293,10 +284,9 @@ static void spi_lm70llp_attach(struct pa
 		status = -ENODEV;
 		goto out_bitbang_stop;
 	}
-	pp->spidev_lm70->bits_per_word = 16;
+	pp->spidev_lm70->bits_per_word = 8;
 
 	lm70llp = pp;
-
 	return;
 
 out_bitbang_stop:
@@ -326,7 +316,6 @@ static void spi_lm70llp_detach(struct pa
 
 	/* power down */
 	parport_write_data(pp->port, 0);
-	msleep(10);
 
 	parport_release(pp->pd);
 	parport_unregister_device(pp->pd);

[-- Attachment #3: Type: text/plain, Size: 153 bytes --]

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [lm-sensors] [PATCH] SPI lm70: Code streamlining and cleanup
  2008-11-12  7:38 [lm-sensors] [PATCH] SPI lm70: Code streamlining and cleanup Kaiwan N Billimoria
                   ` (3 preceding siblings ...)
  2008-11-13  5:35 ` Kaiwan N Billimoria
@ 2008-11-13  8:53 ` David Brownell
  2008-11-13  9:34 ` Jean Delvare
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: David Brownell @ 2008-11-13  8:53 UTC (permalink / raw)
  To: lm-sensors

On Wednesday 12 November 2008, Jean Delvare wrote:
> David, can we get this patch reviewed and pushed upstream quickly? So
> that I can finally take Manuel's patch adding support for the TMP121.

Yeah, appended.  I made some minor updates, notably:

 - using URLs from National's website
 - listing the real name of the eval board (!)
 - there is no "D9" bit in any byte; explain that change better
 - restore a comment of mine :)

And different patch comments -- sorry, I lost the first set.

Jean, I think it's best if you merge this through your tree
along with Manuel's patch.

- Dave


====== CUT HERE
From: Kaiwan N Billimoria <kaiwan@designergraphix.com>

This fixes a byteswap bug in the LM70 temperature sensor driver,
which was previously covered up by a converse bug in the driver
for the LM70EVAL-LLP board (which is also fixed).

Other fixes:  doc updates, remove an annoying msleep(), and improve
three-wire protocol handling.

Signed-off-by: Kaiwan N Billimoria <kaiwan@designergraphix.com>
[ dbrownell@users.sourceforge.net: doc and whitespace tweaks ]
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
 Documentation/hwmon/lm70      |    4 ++++
 Documentation/spi/spi-lm70llp |   10 ++++++++++
 drivers/hwmon/lm70.c          |    9 +++++----
 drivers/spi/spi_lm70llp.c     |   33 ++++++++++++---------------------
 4 files changed, 31 insertions(+), 25 deletions(-)

--- a/Documentation/hwmon/lm70
+++ b/Documentation/hwmon/lm70
@@ -25,6 +25,10 @@ complement digital temperature (sent via
 driver for interpretation. This driver makes use of the kernel's in-core
 SPI support.
 
+As a real (in-tree) example of this "SPI protocol driver" interfacing
+with a "SPI master controller driver", see drivers/spi/spi_lm70llp.c
+and its associated documentation.
+
 Thanks to
 ---------
 Jean Delvare <khali@linux-fr.org> for mentoring the hwmon-side driver
--- a/Documentation/spi/spi-lm70llp
+++ b/Documentation/spi/spi-lm70llp
@@ -13,10 +13,20 @@ Description
 This driver provides glue code connecting a National Semiconductor LM70 LLP
 temperature sensor evaluation board to the kernel's SPI core subsystem.
 
+This is a SPI master controller driver. It can be used in conjunction with
+(layered under) the LM70 logical driver (a "SPI protocol driver").
 In effect, this driver turns the parallel port interface on the eval board
 into a SPI bus with a single device, which will be driven by the generic
 LM70 driver (drivers/hwmon/lm70.c).
 
+
+Hardware Interfacing
+--------------------
+The schematic for this particular board (the LM70EVAL-LLP) is
+available (on page 4) here:
+
+  http://www.national.com/appinfo/tempsensors/files/LM70LLPEVALmanual.pdf
+
 The hardware interfacing on the LM70 LLP eval board is as follows:
 
    Parallel                 LM70 LLP
--- a/drivers/hwmon/lm70.c
+++ b/drivers/hwmon/lm70.c
@@ -65,10 +65,9 @@ static ssize_t lm70_sense_temp(struct de
 		"spi_write_then_read failed with status %d\n", status);
 		goto out;
 	}
-	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);
+	raw = (rxbuf[0] << 8) + rxbuf[1];
+	dev_dbg(dev, "rxbuf[0] : 0x%02x rxbuf[1] : 0x%02x raw=0x%04x\n",
+		rxbuf[0], rxbuf[1], raw);
 
 	/*
 	 * The "raw" temperature read into rxbuf[] is a 16-bit signed 2's
@@ -109,6 +108,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
@@ -1,5 +1,5 @@
 /*
- * spi_lm70llp.c - driver for lm70llp eval board for the LM70 sensor
+ * spi_lm70llp.c - driver for LM70EVAL-LLP board for the LM70 sensor
  *
  * Copyright (C) 2006 Kaiwan N Billimoria <kaiwan@designergraphix.com>
  *
@@ -40,8 +40,12 @@
  * master controller driver.  The hwmon/lm70 driver is a "SPI protocol
  * driver", layered on top of this one and usable without the lm70llp.
  *
+ * Datasheet and Schematic:
  * The LM70 is a temperature sensor chip from National Semiconductor; its
  * datasheet is available at http://www.national.com/pf/LM/LM70.html
+ * The schematic for this particular board (the LM70EVAL-LLP) is
+ * available (on page 4) here:
+ *  http://www.national.com/appinfo/tempsensors/files/LM70LLPEVALmanual.pdf
  *
  * Also see Documentation/spi/spi-lm70llp.  The SPI<->parport code here is
  * (heavily) based on spi-butterfly by David Brownell.
@@ -64,7 +68,7 @@
  *
  * Note that parport pin 13 actually gets inverted by the transistor
  * arrangement which lets either the parport or the LM70 drive the
- * SI/SO signal.
+ * SI/SO signal (see the schematic for details).
  */
 
 #define DRVNAME		"spi-lm70llp"
@@ -106,12 +110,16 @@ static inline struct spi_lm70llp *spidev
 static inline void deassertCS(struct spi_lm70llp *pp)
 {
 	u8 data = parport_read_data(pp->port);
+
+	data &= ~0x80;		/* pull D7/SI-out low while de-asserted */
 	parport_write_data(pp->port, data | nCS);
 }
 
 static inline void assertCS(struct spi_lm70llp *pp)
 {
 	u8 data = parport_read_data(pp->port);
+
+	data |= 0x80;		/* pull D7/SI-out high so lm70 drives SO-in */
 	parport_write_data(pp->port, data & ~nCS);
 }
 
@@ -184,22 +192,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)
@@ -293,10 +286,9 @@ static void spi_lm70llp_attach(struct pa
 		status = -ENODEV;
 		goto out_bitbang_stop;
 	}
-	pp->spidev_lm70->bits_per_word = 16;
+	pp->spidev_lm70->bits_per_word = 8;
 
 	lm70llp = pp;
-
 	return;
 
 out_bitbang_stop:
@@ -326,7 +318,6 @@ static void spi_lm70llp_detach(struct pa
 
 	/* power down */
 	parport_write_data(pp->port, 0);
-	msleep(10);
 
 	parport_release(pp->pd);
 	parport_unregister_device(pp->pd);

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [lm-sensors] [PATCH] SPI lm70: Code streamlining and cleanup
  2008-11-12  7:38 [lm-sensors] [PATCH] SPI lm70: Code streamlining and cleanup Kaiwan N Billimoria
                   ` (4 preceding siblings ...)
  2008-11-13  8:53 ` David Brownell
@ 2008-11-13  9:34 ` Jean Delvare
  2008-11-13  9:55 ` Jean Delvare
  2008-11-13 11:52 ` Kaiwan N Billimoria
  7 siblings, 0 replies; 9+ messages in thread
From: Jean Delvare @ 2008-11-13  9:34 UTC (permalink / raw)
  To: lm-sensors

Hi Kaiwan,

On Thu, 13 Nov 2008 10:53:58 +0530, Kaiwan N Billimoria wrote:
> On Wed, 2008-11-12 at 14:25 +0100, Jean Delvare wrote:
> > > --- a/Documentation/spi/spi-lm70llp	2008-10-10 03:43:53.000000000 +0530
> > > +++ b/Documentation/spi/spi-lm70llp	2008-11-12 12:13:50.000000000 +0530
> > > @@ -13,10 +13,19 @@ Description
> > >  This driver provides glue code connecting a National Semiconductor LM70 LLP
> > >  temperature sensor evaluation board to the kernel's SPI core subsystem.
> > >  
> > > +This is an SPI master controller driver. It can be used in conjunction with 
> > > +(layered under) the LM70 logical driver (an "SPI protocol driver").
> > >  In effect, this driver turns the parallel port interface on the eval board
> > >  into a SPI bus with a single device, which will be driven by the generic
> > >  LM70 driver (drivers/hwmon/lm70.c).
> > >  
> > > +
> > > +Hardware Interfacing
> > > +--------------------
> > > +The schematic for this particular board (the LM70LLP eval board) is 
> > > +available (on page 4) here:
> > > +http://www.designergraphix.com/pull/spi_lm70/LM70LLPEVALmanual.pdf
> > > +
> > >  The hardware interfacing on the LM70 LLP eval board is as follows:
> > >  
> > >     Parallel                 LM70 LLP
> > > @@ -67,3 +76,4 @@ Thanks to
> > >  o David Brownell for mentoring the SPI-side driver development.
> > >  o Dr.Craig Hollabaugh for the (early) "manual" bitbanging driver version.
> > >  o Nadir Billimoria for help interpreting the circuit schematic.
> > > +
> > 
> > Pointless change, should be reverted.
> 
> Sorry I do not follow you..which part are you referring to as pointless?
> IMHO, the schematic PDF download is important for anyone trying to
> understand the 'physical' driver.

The change I was referring to is: adding a blank line at the end of the
file. I have no objection to the other ("real") changes!

-- 
Jean Delvare

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [lm-sensors] [PATCH] SPI lm70: Code streamlining and cleanup
  2008-11-12  7:38 [lm-sensors] [PATCH] SPI lm70: Code streamlining and cleanup Kaiwan N Billimoria
                   ` (5 preceding siblings ...)
  2008-11-13  9:34 ` Jean Delvare
@ 2008-11-13  9:55 ` Jean Delvare
  2008-11-13 11:52 ` Kaiwan N Billimoria
  7 siblings, 0 replies; 9+ messages in thread
From: Jean Delvare @ 2008-11-13  9:55 UTC (permalink / raw)
  To: lm-sensors

Hi David,

On Thu, 13 Nov 2008 00:53:05 -0800, David Brownell wrote:
> On Wednesday 12 November 2008, Jean Delvare wrote:
> > David, can we get this patch reviewed and pushed upstream quickly? So
> > that I can finally take Manuel's patch adding support for the TMP121.
> 
> Yeah, appended.  I made some minor updates, notably:
> 
>  - using URLs from National's website
>  - listing the real name of the eval board (!)
>  - there is no "D9" bit in any byte; explain that change better
>  - restore a comment of mine :)
> 
> And different patch comments -- sorry, I lost the first set.
> 
> Jean, I think it's best if you merge this through your tree
> along with Manuel's patch.

Fine with me, I'll do that right now.

> 
> - Dave
> 
> 
> ====== CUT HERE
> From: Kaiwan N Billimoria <kaiwan@designergraphix.com>
> 
> This fixes a byteswap bug in the LM70 temperature sensor driver,
> which was previously covered up by a converse bug in the driver
> for the LM70EVAL-LLP board (which is also fixed).
> 
> Other fixes:  doc updates, remove an annoying msleep(), and improve
> three-wire protocol handling.
> 
> Signed-off-by: Kaiwan N Billimoria <kaiwan@designergraphix.com>
> [ dbrownell@users.sourceforge.net: doc and whitespace tweaks ]
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> ---
>  Documentation/hwmon/lm70      |    4 ++++
>  Documentation/spi/spi-lm70llp |   10 ++++++++++
>  drivers/hwmon/lm70.c          |    9 +++++----
>  drivers/spi/spi_lm70llp.c     |   33 ++++++++++++---------------------
>  4 files changed, 31 insertions(+), 25 deletions(-)
> 
> --- a/Documentation/hwmon/lm70
> +++ b/Documentation/hwmon/lm70
> @@ -25,6 +25,10 @@ complement digital temperature (sent via
>  driver for interpretation. This driver makes use of the kernel's in-core
>  SPI support.
>  
> +As a real (in-tree) example of this "SPI protocol driver" interfacing
> +with a "SPI master controller driver", see drivers/spi/spi_lm70llp.c
> +and its associated documentation.
> +
>  Thanks to
>  ---------
>  Jean Delvare <khali@linux-fr.org> for mentoring the hwmon-side driver
> --- a/Documentation/spi/spi-lm70llp
> +++ b/Documentation/spi/spi-lm70llp
> @@ -13,10 +13,20 @@ Description
>  This driver provides glue code connecting a National Semiconductor LM70 LLP
>  temperature sensor evaluation board to the kernel's SPI core subsystem.
>  
> +This is a SPI master controller driver. It can be used in conjunction with
> +(layered under) the LM70 logical driver (a "SPI protocol driver").
>  In effect, this driver turns the parallel port interface on the eval board
>  into a SPI bus with a single device, which will be driven by the generic
>  LM70 driver (drivers/hwmon/lm70.c).
>  
> +
> +Hardware Interfacing
> +--------------------
> +The schematic for this particular board (the LM70EVAL-LLP) is
> +available (on page 4) here:
> +
> +  http://www.national.com/appinfo/tempsensors/files/LM70LLPEVALmanual.pdf
> +
>  The hardware interfacing on the LM70 LLP eval board is as follows:
>  
>     Parallel                 LM70 LLP
> --- a/drivers/hwmon/lm70.c
> +++ b/drivers/hwmon/lm70.c
> @@ -65,10 +65,9 @@ static ssize_t lm70_sense_temp(struct de
>  		"spi_write_then_read failed with status %d\n", status);
>  		goto out;
>  	}
> -	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);
> +	raw = (rxbuf[0] << 8) + rxbuf[1];
> +	dev_dbg(dev, "rxbuf[0] : 0x%02x rxbuf[1] : 0x%02x raw=0x%04x\n",
> +		rxbuf[0], rxbuf[1], raw);
>  
>  	/*
>  	 * The "raw" temperature read into rxbuf[] is a 16-bit signed 2's
> @@ -109,6 +108,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
> @@ -1,5 +1,5 @@
>  /*
> - * spi_lm70llp.c - driver for lm70llp eval board for the LM70 sensor
> + * spi_lm70llp.c - driver for LM70EVAL-LLP board for the LM70 sensor
>   *
>   * Copyright (C) 2006 Kaiwan N Billimoria <kaiwan@designergraphix.com>
>   *
> @@ -40,8 +40,12 @@
>   * master controller driver.  The hwmon/lm70 driver is a "SPI protocol
>   * driver", layered on top of this one and usable without the lm70llp.
>   *
> + * Datasheet and Schematic:
>   * The LM70 is a temperature sensor chip from National Semiconductor; its
>   * datasheet is available at http://www.national.com/pf/LM/LM70.html
> + * The schematic for this particular board (the LM70EVAL-LLP) is
> + * available (on page 4) here:
> + *  http://www.national.com/appinfo/tempsensors/files/LM70LLPEVALmanual.pdf
>   *
>   * Also see Documentation/spi/spi-lm70llp.  The SPI<->parport code here is
>   * (heavily) based on spi-butterfly by David Brownell.
> @@ -64,7 +68,7 @@
>   *
>   * Note that parport pin 13 actually gets inverted by the transistor
>   * arrangement which lets either the parport or the LM70 drive the
> - * SI/SO signal.
> + * SI/SO signal (see the schematic for details).
>   */
>  
>  #define DRVNAME		"spi-lm70llp"
> @@ -106,12 +110,16 @@ static inline struct spi_lm70llp *spidev
>  static inline void deassertCS(struct spi_lm70llp *pp)
>  {
>  	u8 data = parport_read_data(pp->port);
> +
> +	data &= ~0x80;		/* pull D7/SI-out low while de-asserted */
>  	parport_write_data(pp->port, data | nCS);
>  }
>  
>  static inline void assertCS(struct spi_lm70llp *pp)
>  {
>  	u8 data = parport_read_data(pp->port);
> +
> +	data |= 0x80;		/* pull D7/SI-out high so lm70 drives SO-in */
>  	parport_write_data(pp->port, data & ~nCS);
>  }
>  
> @@ -184,22 +192,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)
> @@ -293,10 +286,9 @@ static void spi_lm70llp_attach(struct pa
>  		status = -ENODEV;
>  		goto out_bitbang_stop;
>  	}
> -	pp->spidev_lm70->bits_per_word = 16;
> +	pp->spidev_lm70->bits_per_word = 8;
>  
>  	lm70llp = pp;
> -
>  	return;
>  
>  out_bitbang_stop:
> @@ -326,7 +318,6 @@ static void spi_lm70llp_detach(struct pa
>  
>  	/* power down */
>  	parport_write_data(pp->port, 0);
> -	msleep(10);
>  
>  	parport_release(pp->pd);
>  	parport_unregister_device(pp->pd);

-- 
Jean Delvare

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [lm-sensors] [PATCH] SPI lm70: Code streamlining and cleanup
  2008-11-12  7:38 [lm-sensors] [PATCH] SPI lm70: Code streamlining and cleanup Kaiwan N Billimoria
                   ` (6 preceding siblings ...)
  2008-11-13  9:55 ` Jean Delvare
@ 2008-11-13 11:52 ` Kaiwan N Billimoria
  7 siblings, 0 replies; 9+ messages in thread
From: Kaiwan N Billimoria @ 2008-11-13 11:52 UTC (permalink / raw)
  To: lm-sensors

On Thu, 2008-11-13 at 00:53 -0800, David Brownell wrote:
> On Wednesday 12 November 2008, Jean Delvare wrote:
> > David, can we get this patch reviewed and pushed upstream quickly? So
> > that I can finally take Manuel's patch adding support for the TMP121.
> 
> Yeah, appended.  I made some minor updates, notably:
> 
>  - using URLs from National's website
>  - listing the real name of the eval board (!)
>  - there is no "D9" bit in any byte; explain that change better
>  - restore a comment of mine :)
> 
Yes, better, thanks David.
Reg the D9 bit, "bit" is my mistake - I really was referring to the D9
pin on the data register of the parport.

> And different patch comments -- sorry, I lost the first set.
> 
> Jean, I think it's best if you merge this through your tree
> along with Manuel's patch.
> 
Seen that Jean's merged it and sent to Manuel already; great!

Thanks v much David & Jean,
Kaiwan.

> - Dave
> 
> 
> ====== CUT HERE
> From: Kaiwan N Billimoria <kaiwan@designergraphix.com>
> 
> This fixes a byteswap bug in the LM70 temperature sensor driver,
> which was previously covered up by a converse bug in the driver
> for the LM70EVAL-LLP board (which is also fixed).
> 
> Other fixes:  doc updates, remove an annoying msleep(), and improve
> three-wire protocol handling.
> 
> Signed-off-by: Kaiwan N Billimoria <kaiwan@designergraphix.com>
> [ dbrownell@users.sourceforge.net: doc and whitespace tweaks ]
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> ---
>  Documentation/hwmon/lm70      |    4 ++++
>  Documentation/spi/spi-lm70llp |   10 ++++++++++
>  drivers/hwmon/lm70.c          |    9 +++++----
>  drivers/spi/spi_lm70llp.c     |   33 ++++++++++++---------------------
>  4 files changed, 31 insertions(+), 25 deletions(-)
> 
> --- a/Documentation/hwmon/lm70
> +++ b/Documentation/hwmon/lm70
> @@ -25,6 +25,10 @@ complement digital temperature (sent via
>  driver for interpretation. This driver makes use of the kernel's in-core
>  SPI support.
>  
> +As a real (in-tree) example of this "SPI protocol driver" interfacing
> +with a "SPI master controller driver", see drivers/spi/spi_lm70llp.c
> +and its associated documentation.
> +
>  Thanks to
>  ---------
>  Jean Delvare <khali@linux-fr.org> for mentoring the hwmon-side driver
> --- a/Documentation/spi/spi-lm70llp
> +++ b/Documentation/spi/spi-lm70llp
> @@ -13,10 +13,20 @@ Description
>  This driver provides glue code connecting a National Semiconductor LM70 LLP
>  temperature sensor evaluation board to the kernel's SPI core subsystem.
>  
> +This is a SPI master controller driver. It can be used in conjunction with
> +(layered under) the LM70 logical driver (a "SPI protocol driver").
>  In effect, this driver turns the parallel port interface on the eval board
>  into a SPI bus with a single device, which will be driven by the generic
>  LM70 driver (drivers/hwmon/lm70.c).
>  
> +
> +Hardware Interfacing
> +--------------------
> +The schematic for this particular board (the LM70EVAL-LLP) is
> +available (on page 4) here:
> +
> +  http://www.national.com/appinfo/tempsensors/files/LM70LLPEVALmanual.pdf
> +
>  The hardware interfacing on the LM70 LLP eval board is as follows:
>  
>     Parallel                 LM70 LLP
> --- a/drivers/hwmon/lm70.c
> +++ b/drivers/hwmon/lm70.c
> @@ -65,10 +65,9 @@ static ssize_t lm70_sense_temp(struct de
>  		"spi_write_then_read failed with status %d\n", status);
>  		goto out;
>  	}
> -	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);
> +	raw = (rxbuf[0] << 8) + rxbuf[1];
> +	dev_dbg(dev, "rxbuf[0] : 0x%02x rxbuf[1] : 0x%02x raw=0x%04x\n",
> +		rxbuf[0], rxbuf[1], raw);
>  
>  	/*
>  	 * The "raw" temperature read into rxbuf[] is a 16-bit signed 2's
> @@ -109,6 +108,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
> @@ -1,5 +1,5 @@
>  /*
> - * spi_lm70llp.c - driver for lm70llp eval board for the LM70 sensor
> + * spi_lm70llp.c - driver for LM70EVAL-LLP board for the LM70 sensor
>   *
>   * Copyright (C) 2006 Kaiwan N Billimoria <kaiwan@designergraphix.com>
>   *
> @@ -40,8 +40,12 @@
>   * master controller driver.  The hwmon/lm70 driver is a "SPI protocol
>   * driver", layered on top of this one and usable without the lm70llp.
>   *
> + * Datasheet and Schematic:
>   * The LM70 is a temperature sensor chip from National Semiconductor; its
>   * datasheet is available at http://www.national.com/pf/LM/LM70.html
> + * The schematic for this particular board (the LM70EVAL-LLP) is
> + * available (on page 4) here:
> + *  http://www.national.com/appinfo/tempsensors/files/LM70LLPEVALmanual.pdf
>   *
>   * Also see Documentation/spi/spi-lm70llp.  The SPI<->parport code here is
>   * (heavily) based on spi-butterfly by David Brownell.
> @@ -64,7 +68,7 @@
>   *
>   * Note that parport pin 13 actually gets inverted by the transistor
>   * arrangement which lets either the parport or the LM70 drive the
> - * SI/SO signal.
> + * SI/SO signal (see the schematic for details).
>   */
>  
>  #define DRVNAME		"spi-lm70llp"
> @@ -106,12 +110,16 @@ static inline struct spi_lm70llp *spidev
>  static inline void deassertCS(struct spi_lm70llp *pp)
>  {
>  	u8 data = parport_read_data(pp->port);
> +
> +	data &= ~0x80;		/* pull D7/SI-out low while de-asserted */
>  	parport_write_data(pp->port, data | nCS);
>  }
>  
>  static inline void assertCS(struct spi_lm70llp *pp)
>  {
>  	u8 data = parport_read_data(pp->port);
> +
> +	data |= 0x80;		/* pull D7/SI-out high so lm70 drives SO-in */
>  	parport_write_data(pp->port, data & ~nCS);
>  }
>  
> @@ -184,22 +192,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)
> @@ -293,10 +286,9 @@ static void spi_lm70llp_attach(struct pa
>  		status = -ENODEV;
>  		goto out_bitbang_stop;
>  	}
> -	pp->spidev_lm70->bits_per_word = 16;
> +	pp->spidev_lm70->bits_per_word = 8;
>  
>  	lm70llp = pp;
> -
>  	return;
>  
>  out_bitbang_stop:
> @@ -326,7 +318,6 @@ static void spi_lm70llp_detach(struct pa
>  
>  	/* power down */
>  	parport_write_data(pp->port, 0);
> -	msleep(10);
>  
>  	parport_release(pp->pd);
>  	parport_unregister_device(pp->pd);
--
Kaiwan N Billimoria <kaiwan@designergraphix.com>
Designer Graphix
4923, 11th Floor, Highpoint IV, 45 Palace Road, Bangalore 560 001.
+91.98450.16788 (M) | TeleFax: +91.80.22389396 | Alt. Tel:
+91.80.64500257

Do visit our enhanced website: http://www.designergraphix.com 

"Keep away from people who try to belittle your ambitions. Small people
always do that, but the
really great make you feel that you, too, can become great." --Mark
Twain.


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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2008-11-13 11:52 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-11-12  7:38 [lm-sensors] [PATCH] SPI lm70: Code streamlining and cleanup Kaiwan N Billimoria
2008-11-12  7:59 ` David Brownell
2008-11-12  8:30 ` Kaiwan N Billimoria
2008-11-12 13:25 ` Jean Delvare
2008-11-13  5:35 ` Kaiwan N Billimoria
2008-11-13  8:53 ` David Brownell
2008-11-13  9:34 ` Jean Delvare
2008-11-13  9:55 ` Jean Delvare
2008-11-13 11:52 ` Kaiwan N Billimoria

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.