diff mbox

mtd: atmel_nand: fix access to 16 bit NAND devices

Message ID 1328184370.28171.175.camel@sauron.fi.intel.com
State Accepted
Commit 500823195d0c9eec2a4637484f30cc93ec633d4a
Headers show

Commit Message

Artem Bityutskiy Feb. 2, 2012, 12:06 p.m. UTC 
On Mon, 2012-01-30 at 09:57 +0100, Nicolas Ferre wrote:
> Artem, do you want me to prepare a patch for reverting initial commit or
> you just need my "Acked-by" (feel free to add though)?

OK, thanks, just pushed this patch to l2-mtd.git and add Cc to -stable,
please, validate.

I'll ask David to merge it to 3.3, but no guarantees - you should ping
him directly if you want this to happen.

URL: http://git.infradead.org/users/dedekind/l2-mtd.git/commit/21c7726c98628016c868d803a2a8e6f2d5702519

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Date: Thu, 2 Feb 2012 13:54:25 +0200
Subject: [PATCH] Revert "mtd: atmel_nand: optimize read/write buffer functions"

This reverts commit fb5427508abbd635e877fabdf55795488119c2d6.

The reason is that it breaks 16 bits NAND flash as it was reported by
Nikolaus Voss and confirmed by Eric Bénard.

Nicolas Ferre <nicolas.ferre@atmel.com> alco confirmed:
"After double checking with designers, I must admit that I misunderstood
the way of optimizing accesses to SMC. 16 bit nand is not so common
those days..."

Reported-by: Nikolaus Voss <n.voss@weinmann.de>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Cc: stable@kernel.org [3.1+]
---
 drivers/mtd/nand/atmel_nand.c |   45 +++++++++++++++++++++++++++++++++++++---
 1 files changed, 41 insertions(+), 4 deletions(-)

Comments

Venu Byravarasu Feb. 3, 2012, 3:35 a.m. UTC | #1 
I see a problem with atmel_read_buf16 & atmel_write_buf16 functions.
As they are calling __raw_readsw & __raw_writesw respectively, which 
would cause panic in cases of 16 bit unaligned buffers.
However, the buf passed to these functions is of type u8* from 
atmel_read_buf & atmel_write_buf functions.

[Venu] 
> -	/* if no DMA operation possible, use PIO */
> -	memcpy_fromio(buf, chip->IO_ADDR_R, len);
> +	if (host->board->bus_width_16)
> +		atmel_read_buf16(mtd, buf, len);

> +	else
> +		atmel_read_buf8(mtd, buf, len);
>  }
Artem Bityutskiy Feb. 3, 2012, 5:32 a.m. UTC | #2 
On Fri, 2012-02-03 at 09:05 +0530, Venu Byravarasu wrote:
> I see a problem with atmel_read_buf16 & atmel_write_buf16 functions.
> As they are calling __raw_readsw & __raw_writesw respectively, which 
> would cause panic in cases of 16 bit unaligned buffers.
> However, the buf passed to these functions is of type u8* from 
> atmel_read_buf & atmel_write_buf functions.

I think this is a valid concern. In theory, the NAND infrustructure must
guarantee that the buffer is always of even length, and most probably it
does, but we could add a WARN_ON(len & 1) statements in
'atmel_read_buf()' and 'atmel_write_buf()'.

And probably the type could indeed be changed.

But these concerns should be addressed separately.
diff mbox

Patch

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 4dd056e..35b4fb5 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -161,6 +161,37 @@  static int atmel_nand_device_ready(struct mtd_info *mtd)
                 !!host->board->rdy_pin_active_low;
 }
 
+/*
+ * Minimal-overhead PIO for data access.
+ */
+static void atmel_read_buf8(struct mtd_info *mtd, u8 *buf, int len)
+{
+	struct nand_chip	*nand_chip = mtd->priv;
+
+	__raw_readsb(nand_chip->IO_ADDR_R, buf, len);
+}
+
+static void atmel_read_buf16(struct mtd_info *mtd, u8 *buf, int len)
+{
+	struct nand_chip	*nand_chip = mtd->priv;
+
+	__raw_readsw(nand_chip->IO_ADDR_R, buf, len / 2);
+}
+
+static void atmel_write_buf8(struct mtd_info *mtd, const u8 *buf, int len)
+{
+	struct nand_chip	*nand_chip = mtd->priv;
+
+	__raw_writesb(nand_chip->IO_ADDR_W, buf, len);
+}
+
+static void atmel_write_buf16(struct mtd_info *mtd, const u8 *buf, int len)
+{
+	struct nand_chip	*nand_chip = mtd->priv;
+
+	__raw_writesw(nand_chip->IO_ADDR_W, buf, len / 2);
+}
+
 static void dma_complete_func(void *completion)
 {
 	complete(completion);
@@ -235,27 +266,33 @@  err_buf:
 static void atmel_read_buf(struct mtd_info *mtd, u8 *buf, int len)
 {
 	struct nand_chip *chip = mtd->priv;
+	struct atmel_nand_host *host = chip->priv;
 
 	if (use_dma && len > mtd->oobsize)
 		/* only use DMA for bigger than oob size: better performances */
 		if (atmel_nand_dma_op(mtd, buf, len, 1) == 0)
 			return;
 
-	/* if no DMA operation possible, use PIO */
-	memcpy_fromio(buf, chip->IO_ADDR_R, len);
+	if (host->board->bus_width_16)
+		atmel_read_buf16(mtd, buf, len);
+	else
+		atmel_read_buf8(mtd, buf, len);
 }
 
 static void atmel_write_buf(struct mtd_info *mtd, const u8 *buf, int len)
 {
 	struct nand_chip *chip = mtd->priv;
+	struct atmel_nand_host *host = chip->priv;
 
 	if (use_dma && len > mtd->oobsize)
 		/* only use DMA for bigger than oob size: better performances */
 		if (atmel_nand_dma_op(mtd, (void *)buf, len, 0) == 0)
 			return;
 
-	/* if no DMA operation possible, use PIO */
-	memcpy_toio(chip->IO_ADDR_W, buf, len);
+	if (host->board->bus_width_16)
+		atmel_write_buf16(mtd, buf, len);
+	else
+		atmel_write_buf8(mtd, buf, len);
 }
 
 /*