Re: [PATCH] atmel_spi: fix dma addr calculation for len >BUFFER_SIZE

[Ben Nizette]

On Mon, 2009-09-28 at 09:12 +0200, Haavard Skinnemoen wrote:

> Wow, that is subtle. I had to stare at it for a long while before I
> understood what's going on, but I believe you're right.

Prolly just means my changelog was crappy ;-)

> 
> Acked-by: Haavard Skinnemoen <haavard.skinnemoen@xxxxxxxxx>
> 
> While you're at it, could you send another patch renaming 'len' to
> 'next_len'? I think that would make it a bit more obvious why your
> patch is correct and prevent similar mistakes in the future.

Good plan, below.

Thx,
	--Ben.
> 
> Haavard

---8<---

From: Ben Nizette <bn@xxxxxxxxxxxxxxx>
Subject: [PATCH] atmel_spi: make "len" variable name less ambiguous in dma addr calculation

"[PATCH] atmel_spi: fix dma addr calculation for len > BUFFER_SIZE"
fixed a bug where the "len" variable in atmel_spi_next_xfer_data() was
taken to be the total number of bytes remaining in the transfer but it
actually represents the number of bytes which will be sent this dma
chunk.  While the 2 will be the same if there is less than 1 chunk to go
(or if we aren't using a scratch buffer and therefore aren't breaking
the transfers in to chunks), they won't be the same in general.

s/len/next_len to reflect the true nature and usage of this variable.

Signed-off-by: Ben Nizette <bn@xxxxxxxxxxxxxxx>
---
 drivers/spi/atmel_spi.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c
index 8ce70cb..5d94fca 100644
--- a/drivers/spi/atmel_spi.c
+++ b/drivers/spi/atmel_spi.c
@@ -185,28 +185,28 @@ static void atmel_spi_next_xfer_data(struct spi_master *master,
 				u32 *plen)
 {
 	struct atmel_spi	*as = spi_master_get_devdata(master);
-	u32			len = *plen;
+	u32			next_len = *plen;
 
 	/* use scratch buffer only when rx or tx data is unspecified */
 	if (xfer->rx_buf)
 		*rx_dma = xfer->rx_dma + xfer->len - *plen;
 	else {
 		*rx_dma = as->buffer_dma;
-		if (len > BUFFER_SIZE)
-			len = BUFFER_SIZE;
+		if (next_len > BUFFER_SIZE)
+			next_len = BUFFER_SIZE;
 	}
 	if (xfer->tx_buf)
 		*tx_dma = xfer->tx_dma + xfer->len - *plen;
 	else {
 		*tx_dma = as->buffer_dma;
-		if (len > BUFFER_SIZE)
-			len = BUFFER_SIZE;
-		memset(as->buffer, 0, len);
+		if (next_len > BUFFER_SIZE)
+			next_len = BUFFER_SIZE;
+		memset(as->buffer, 0, next_len);
 		dma_sync_single_for_device(&as->pdev->dev,
-				as->buffer_dma, len, DMA_TO_DEVICE);
+				as->buffer_dma, next_len, DMA_TO_DEVICE);
 	}
 
-	*plen = len;
+	*plen = next_len;
 }
 
 /*
-- 
1.6.0.4



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/