[PATCH 0/3] mtd: nand: ONFI probing for x16 devices on i.MX25 (and others)

[PATCH 0/3] mtd: nand: ONFI probing for x16 devices on i.MX25 (and others)

[Uwe Kleine-König]

Hello,

On an i.MX25 based machine barebox issued a warning during startup:

	nand: Trying ONFI probe in 16 bits mode, aborting !

without much side effects as the NAND chip was detected correctly nevertheless.

On the quest to fix this warning I noticed that there isn't much justification
to not probe via ONFI even in x16 mode in the presence of the read_byte
callback that is correctly used in the ONFI detection code. But removing this
early exit in the generic probe routines brought to light that the imx nand
driver didn't implement this read_byte callback correctly.

This series first fixes the imx nand driver to implement the read_byte callback
correctly and then "fixes" the ONFI code to try detection even in x16 mode.
This made the above mentioned warning go away and print

	nand: ONFI flash detected

instead \o/. The generic code change was mostly cherry-picked from Linux, so I
kept the original author annotation here.

As an added bonus there is a little optimisation at the end of the series.

Best regards
Uwe

Brian Norris (1):
  mtd: nand: cleanup ONFI printed errors, warnings

Uwe Kleine-König (2):
  mtd: nand-imx: fix byte reading in x16 mode
  mtd: nand-imx: don't copy more bytes than read from hardware

 drivers/mtd/nand/nand_base.c | 15 ++++++---------
 drivers/mtd/nand/nand_imx.c  | 39 ++++++++++++++++++++-------------------
 2 files changed, 26 insertions(+), 28 deletions(-)

-- 
2.1.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/3] mtd: nand-imx: fix byte reading in x16 mode

[Uwe Kleine-König]

There are a few NAND commands that make the chip only respond on I/O
lines 0 to 7 even for x16 devices. READID (90h) is one such command
which was already handled fine in the driver (at least for NFC v1 and
v2). Other commands (like PARAM (ECh) to read out ONFI parameters)
however were not handled properly. Instead of adding another callback
make the read_byte callback handle the holes added by the NFC and depend
on the nand-base support to call read_byte when necessary instead of
read_buf.

This fixes reading the ONFI parameter page on an i.MX25 with an x16 NAND
and probably[1] also the result of READID on i.MX51/i.MX53 with x16
NAND.

[1] untested because no matching machine available

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/mtd/nand/nand_imx.c | 33 +++++++++++++++------------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/drivers/mtd/nand/nand_imx.c b/drivers/mtd/nand/nand_imx.c
index e2aaa153eb18..685750991389 100644
--- a/drivers/mtd/nand/nand_imx.c
+++ b/drivers/mtd/nand/nand_imx.c
@@ -377,8 +377,6 @@ static void send_read_param_v3(struct imx_nand_host *host)
 
 static void send_read_id_v1_v2(struct imx_nand_host *host)
 {
-	struct nand_chip *this = &host->nand;
-
 	/* NANDFC buffer 0 is used for device ID output */
 	writew(0x0, host->regs + NFC_V1_V2_BUF_ADDR);
 
@@ -387,20 +385,11 @@ static void send_read_id_v1_v2(struct imx_nand_host *host)
 	/* Wait for operation to complete */
 	wait_op_done(host);
 
-	if (this->options & NAND_BUSWIDTH_16) {
-		volatile u16 *mainbuf = host->main_area0;
-
-		/*
-		 * Pack the every-other-byte result for 16-bit ID reads
-		 * into every-byte as the generic code expects and various
-		 * chips implement.
-		 */
-
-		mainbuf[0] = (mainbuf[0] & 0xff) | ((mainbuf[1] & 0xff) << 8);
-		mainbuf[1] = (mainbuf[2] & 0xff) | ((mainbuf[3] & 0xff) << 8);
-		mainbuf[2] = (mainbuf[4] & 0xff) | ((mainbuf[5] & 0xff) << 8);
-	}
-	memcpy32(host->data_buf, host->main_area0, 16);
+	/*
+	 * NFC_ID results in reading 6 bytes or words (depending on data width),
+	 * so copying 3 32-bit values is just fine.
+	 */
+	memcpy32(host->data_buf, host->main_area0, 12);
 }
 
 static void send_read_param_v1_v2(struct imx_nand_host *host)
@@ -572,8 +561,16 @@ static u_char imx_nand_read_byte(struct mtd_info *mtd)
 	if (host->status_request)
 		return host->get_dev_status(host) & 0xFF;
 
-	ret = *(uint8_t *)(host->data_buf + host->buf_start);
-	host->buf_start++;
+	if (nand_chip->options & NAND_BUSWIDTH_16) {
+		/* only take the lower byte of each word */
+		BUG_ON(host->buf_start & 1);
+		ret = *(uint16_t *)(host->data_buf + host->buf_start);
+
+		host->buf_start += 2;
+	} else {
+		ret = *(uint8_t *)(host->data_buf + host->buf_start);
+		host->buf_start++;
+	}
 
 	return ret;
 }
-- 
2.1.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/3] mtd: nand: cleanup ONFI printed errors, warnings

[Uwe Kleine-König]

From: Brian Norris <computersforpeace@gmail.com>

This corresponds to commit c7f23a70635895b5125aeb5593aaf8cb44d3a088 in
the Linux kernel.

One notable difference is however that ONFI detection is allowed here
even in 16-bit mode. This is proved to work fine on an i.MX25 based
machine with a x16 NAND.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/mtd/nand/nand_base.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 54d8ba34ff38..2b3f9a9942d8 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2768,14 +2768,9 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
 					int *busw)
 {
 	struct nand_onfi_params *p = &chip->onfi_params;
-	int i;
+	int i, j;
 	int val;
 
-	/* ONFI need to be probed in 8 bits mode, and 16 bits should be selected with NAND_BUSWIDTH_AUTO */
-	if (chip->options & NAND_BUSWIDTH_16) {
-		pr_err("Trying ONFI probe in 16 bits mode, aborting !\n");
-		return 0;
-	}
 	/* Try ONFI for unknown chip or LP */
 	chip->cmdfunc(mtd, NAND_CMD_READID, 0x20, -1);
 	if (chip->read_byte(mtd) != 'O' || chip->read_byte(mtd) != 'N' ||
@@ -2784,16 +2779,18 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
 
 	chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);
 	for (i = 0; i < 3; i++) {
-		chip->read_buf(mtd, (uint8_t *)p, sizeof(*p));
+		for (j = 0; j < sizeof(*p); j++)
+			((uint8_t *)p)[j] = chip->read_byte(mtd);
 		if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) ==
 				le16_to_cpu(p->crc)) {
-			pr_info("ONFI param page %d valid\n", i);
 			break;
 		}
 	}
 
-	if (i == 3)
+	if (i == 3) {
+		pr_err("Could not find valid ONFI parameter page; aborting\n");
 		return 0;
+	}
 
 	/* Check version */
 	val = le16_to_cpu(p->revision);
-- 
2.1.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/3] mtd: nand-imx: don't copy more bytes than read from hardware

[Uwe Kleine-König]

The NFC command used for reading the result of a READID command to the NAND chip
reads 6 bytes (in x8 mode) or 6 words (in x16 mode with the upper bytes all
being 0). So there is no need to safe 16 bytes for later consumption.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/mtd/nand/nand_imx.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/nand_imx.c b/drivers/mtd/nand/nand_imx.c
index 685750991389..00c05d00afe3 100644
--- a/drivers/mtd/nand/nand_imx.c
+++ b/drivers/mtd/nand/nand_imx.c
@@ -362,7 +362,11 @@ static void send_read_id_v3(struct imx_nand_host *host)
 
 	wait_op_done(host);
 
-	memcpy(host->data_buf, host->main_area0, 16);
+	/*
+	 * NFC_ID results in reading 6 bytes or words (depending on data width),
+	 * so copying 3 32-bit values is just fine.
+	 */
+	memcpy(host->data_buf, host->main_area0, 12);
 }
 
 static void send_read_param_v3(struct imx_nand_host *host)
-- 
2.1.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/3] mtd: nand: ONFI probing for x16 devices on i.MX25 (and others)

[Sascha Hauer]

On Thu, Feb 05, 2015 at 10:21:29PM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> On an i.MX25 based machine barebox issued a warning during startup:
> 
> 	nand: Trying ONFI probe in 16 bits mode, aborting !
> 
> without much side effects as the NAND chip was detected correctly nevertheless.
> 
> On the quest to fix this warning I noticed that there isn't much justification
> to not probe via ONFI even in x16 mode in the presence of the read_byte
> callback that is correctly used in the ONFI detection code. But removing this
> early exit in the generic probe routines brought to light that the imx nand
> driver didn't implement this read_byte callback correctly.
> 
> This series first fixes the imx nand driver to implement the read_byte callback
> correctly and then "fixes" the ONFI code to try detection even in x16 mode.
> This made the above mentioned warning go away and print
> 
> 	nand: ONFI flash detected
> 
> instead \o/. The generic code change was mostly cherry-picked from Linux, so I
> kept the original author annotation here.
> 
> As an added bonus there is a little optimisation at the end of the series.
> 
> Best regards
> Uwe
> 
> Brian Norris (1):
>   mtd: nand: cleanup ONFI printed errors, warnings
> 
> Uwe Kleine-König (2):
>   mtd: nand-imx: fix byte reading in x16 mode
>   mtd: nand-imx: don't copy more bytes than read from hardware

Applied, thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox