Patchwork mtd: atmel_nand: fix access to 16 bit NAND devices

login
register
mail settings
Submitter Artem Bityutskiy
Date Feb. 2, 2012, 12:06 p.m.
Message ID <1328184370.28171.175.camel@sauron.fi.intel.com>
Download mbox | patch
Permalink /patch/139124/
State Accepted
Commit 500823195d0c9eec2a4637484f30cc93ec633d4a
Headers show

Comments

Artem Bityutskiy - Feb. 2, 2012, 12:06 p.m.
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(-)
Venu Byravarasu - Feb. 3, 2012, 3:35 a.m.
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.
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.

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);
 }
 
 /*