Skip to contentSkip to conversation

spi: bitbang bugfix in message setup

PATCH New David Brownell 1 participant 1 message Jun 24, 2009 SPI

lore
Share & tools
lore

David Brownell

author
#0 lore raw

PATCH 1/1 spi: bitbang bugfix in message setup

From: David Brownell <[email protected]>

Bugfix to spi_bitbang infrastructure: make sure to always set transfer parameters on the first pass through the message's per-transfer loop. This can matter with drivers that replace the per-word or per-buffer transfer primitives, on busses with multiple SPI devices.

Previously, this could have started messages using the settings left after previous messages. The problem was observed when a high speed chip (m25p80 type flash) was running very slowly because a low speed device (avr8 microcontroller) had previously used the bus. Similar faults could have driven the low speed device too fast, or used an unexpected word size.

Acked-by Steven A. Falco <sfalco-7/[email protected]>
Signed-off-by David Brownell <[email protected]>

---
For 2.6.31 ... bug is in older kernels too, but doesn't appear to have hurt anyone before now; this particular driver config isn't common.

 drivers/spi/spi_bitbang.c |   24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)
drivers/spi/spi_bitbang.c +14 -10
@@ -258,6 +258,11 @@ static void bitbang_work(struct work_str
 	struct spi_bitbang	*bitbang =
 		container_of(work, struct spi_bitbang, work);
 	unsigned long		flags;
+	int			do_setup = -1;
+	int			(*setup_transfer)(struct spi_device *,
+					struct spi_transfer *);
+
+	setup_transfer = bitbang->setup_transfer;
 
 	spin_lock_irqsave(&bitbang->lock, flags);
 	bitbang->busy = 1;

@@ -269,8 +274,6 @@ static void bitbang_work(struct work_str
 		unsigned		tmp;
 		unsigned		cs_change;
 		int			status;
-		int			(*setup_transfer)(struct spi_device *,
-						struct spi_transfer *);
 
 		m = container_of(bitbang->queue.next, struct spi_message,
 				queue);

@@ -287,19 +290,19 @@ static void bitbang_work(struct work_str
 		tmp = 0;
 		cs_change = 1;
 		status = 0;
-		setup_transfer = NULL;
 
 		list_for_each_entry (t, &m->transfers, transfer_list) {
 
-			/* override or restore speed and wordsize */
-			if (t->speed_hz || t->bits_per_word) {
-				setup_transfer = bitbang->setup_transfer;
+			/* override speed or wordsize? */
+			if (t->speed_hz || t->bits_per_word)
+				do_setup = 1;
+
+			/* init (-1) or override (1) transfer params */
+			if (do_setup != 0) {
 				if (!setup_transfer) {
 					status = -ENOPROTOOPT;
 					break;
 				}
-			}
-			if (setup_transfer) {
 				status = setup_transfer(spi, t);
 				if (status < 0)
 					break;

@@ -363,9 +366,10 @@ static void bitbang_work(struct work_str
 		m->status = status;
 		m->complete(m->context);
 
-		/* restore speed and wordsize */
-		if (setup_transfer)
+		/* restore speed and wordsize if it was overridden */
+		if (do_setup == 1)
 			setup_transfer(spi, NULL);
+		do_setup = 0;
 
 		/* normally deactivate chipselect ... unless no error and
 		 * cs_change has hinted that the next message will probably

------------------------------------------------------------------------------