[PATCH] mtd: nand: fix byte addressing on 16-bit wide devices

[PATCH] mtd: nand: fix byte addressing on 16-bit wide devices

[David Mosberger]

[-- Attachment #1: Type: text/plain, Size: 205 bytes --]

This is a revised version of a patch that I submitted a while ago.
It's relative to linux-mtd.
Please apply.

Thanks!

  --david
-- 
eGauge Systems LLC, http://egauge.net/, 1.877-EGAUGE1, fax 720.545.9768

[-- Attachment #2: nand-byte-addr.diff --]
[-- Type: text/plain, Size: 3518 bytes --]

mtd: nand: fix byte adressing on 16-bit wide devices

Some commands (READID, GET_FEATURES, and SET_FEATURES) require
byte-addresses even when the databus is 16 bits wide.  Without this
patch, 16-bit wide devices are unable to detect ONFI.

Signed-off-by: David Mosberger <davidm@egauge.net>

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 9715a7b..132866c 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -103,6 +103,20 @@ static int nand_do_write_oob(struct mtd_info *mtd, loff_t to,
  */
 DEFINE_LED_TRIGGER(nand_led_trigger);
 
+/*
+ * Some commands (READID, GET_FEATURES, and SET_FEATURES) are
+ * byte-oriented even when the data-bus is 16 bits wide.  To preserve
+ * the byte-address, we need to encode it properly depending on
+ * whether or not the device has a 16 bit wide bus.
+ */
+static inline int byte_addr(struct nand_chip *chip, int addr)
+{
+	if (chip->options & NAND_BUSWIDTH_16)
+		return addr << 1;
+	else
+		return addr;
+}
+
 static int check_offs_len(struct mtd_info *mtd,
 					loff_t ofs, uint64_t len)
 {
@@ -2821,7 +2835,7 @@ static int nand_onfi_set_features(struct mtd_info *mtd, struct nand_chip *chip,
 	      & ONFI_OPT_CMD_SET_GET_FEATURES))
 		return -EINVAL;
 
-	chip->cmdfunc(mtd, NAND_CMD_SET_FEATURES, addr, -1);
+	chip->cmdfunc(mtd, NAND_CMD_SET_FEATURES, byte_addr(chip, addr), -1);
 	for (i = 0; i < ONFI_SUBFEATURE_PARAM_LEN; ++i)
 		chip->write_byte(mtd, subfeature_param[i]);
 
@@ -2851,7 +2865,7 @@ static int nand_onfi_get_features(struct mtd_info *mtd, struct nand_chip *chip,
 	/* clear the sub feature parameters */
 	memset(subfeature_param, 0, ONFI_SUBFEATURE_PARAM_LEN);
 
-	chip->cmdfunc(mtd, NAND_CMD_GET_FEATURES, addr, -1);
+	chip->cmdfunc(mtd, NAND_CMD_GET_FEATURES, byte_addr(chip, addr), -1);
 	for (i = 0; i < ONFI_SUBFEATURE_PARAM_LEN; ++i)
 		*subfeature_param++ = chip->read_byte(mtd);
 	return 0;
@@ -3067,7 +3081,7 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
 	int val;
 
 	/* Try ONFI for unknown chip or LP */
-	chip->cmdfunc(mtd, NAND_CMD_READID, 0x20, -1);
+	chip->cmdfunc(mtd, NAND_CMD_READID, byte_addr(chip, 0x20), -1);
 	if (chip->read_byte(mtd) != 'O' || chip->read_byte(mtd) != 'N' ||
 		chip->read_byte(mtd) != 'F' || chip->read_byte(mtd) != 'I')
 		return 0;
@@ -3491,7 +3505,7 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 	chip->cmdfunc(mtd, NAND_CMD_RESET, -1, -1);
 
 	/* Send the command for reading device ID */
-	chip->cmdfunc(mtd, NAND_CMD_READID, 0x00, -1);
+	chip->cmdfunc(mtd, NAND_CMD_READID, byte_addr(chip, 0x00), -1);
 
 	/* Read manufacturer and device IDs */
 	*maf_id = chip->read_byte(mtd);
@@ -3504,7 +3518,7 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 	 * not match, ignore the device completely.
 	 */
 
-	chip->cmdfunc(mtd, NAND_CMD_READID, 0x00, -1);
+	chip->cmdfunc(mtd, NAND_CMD_READID, byte_addr(chip, 0x00), -1);
 
 	/* Read entire ID string */
 	for (i = 0; i < 8; i++)
@@ -3662,7 +3676,7 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips,
 		/* See comment in nand_get_flash_type for reset */
 		chip->cmdfunc(mtd, NAND_CMD_RESET, -1, -1);
 		/* Send the command for reading device ID */
-		chip->cmdfunc(mtd, NAND_CMD_READID, 0x00, -1);
+		chip->cmdfunc(mtd, NAND_CMD_READID, byte_addr(chip, 0x00), -1);
 		/* Read manufacturer and device IDs */
 		if (nand_maf_id != chip->read_byte(mtd) ||
 		    nand_dev_id != chip->read_byte(mtd)) {

Re: [PATCH] mtd: nand: fix byte addressing on 16-bit wide devices

[Ezequiel Garcia]

Hi David,

On Mar 21, David Mosberger wrote:
> This is a revised version of a patch that I submitted a while ago.
> It's relative to linux-mtd.
> Please apply.
> 
> Thanks!
> 
>   --david
> -- 
> eGauge Systems LLC, http://egauge.net/, 1.877-EGAUGE1, fax 720.545.9768

> mtd: nand: fix byte adressing on 16-bit wide devices
> 
> Some commands (READID, GET_FEATURES, and SET_FEATURES) require
> byte-addresses even when the databus is 16 bits wide.  Without this
> patch, 16-bit wide devices are unable to detect ONFI.
> 

I haven't looked at your patch, but I think this is already fixed.
You can take a look at these commits:

bd9c6e99b58255b9de1982711ac9487c9a2f18be
3dad2344e92c6e1aeae42df1c4824f307c51bcc7

AFAIK, the READID command data is now issued as 8-bit, disregarding of
NAND_BUSWIDTH_16.

Maybe you can re-test this building a kernel from linux-next.git and
report if your issues are now fixed?

Thanks!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Fwd: [PATCH] mtd: nand: fix byte addressing on 16-bit wide devices

[David Mosberger-Tang]

Resend in plain text...

---------- Forwarded message ----------
From: David Mosberger-Tang <davidm@egauge.net>
Date: Fri, Mar 21, 2014 at 12:18 PM
Subject: Re: [PATCH] mtd: nand: fix byte addressing on 16-bit wide devices
To: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Cc: David Mosberger <davidm@egauge.net>,
"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>


Ezequiel,

On Fri, Mar 21, 2014 at 12:05 PM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
>
>
> You can take a look at these commits:
>
> 3dad2344e92c6e1aeae42df1c4824f307c51bcc7
>
> AFAIK, the READID command data is now issued as 8-bit, disregarding of
> NAND_BUSWIDTH_16.
>
> Maybe you can re-test this building a kernel from linux-next.git and
> report if your issues are now fixed?


That's not sufficient.  GET_FEATURES and SET_FEATURES  also need
byte-addressing.

No offense, but my patch seems both simpler and more complete?

  --david

Re: [PATCH] mtd: nand: fix byte addressing on 16-bit wide devices

[Ezequiel Garcia]

(Ccing Brian and Uwe)

On Mar 21, David Mosberger-Tang wrote:
> On Fri, Mar 21, 2014 at 12:05 PM, Ezequiel Garcia <
> ezequiel.garcia@free-electrons.com> wrote:
> >
> > You can take a look at these commits:
> >
> > 3dad2344e92c6e1aeae42df1c4824f307c51bcc7
> >
> > AFAIK, the READID command data is now issued as 8-bit, disregarding of
> > NAND_BUSWIDTH_16.
> >
> > Maybe you can re-test this building a kernel from linux-next.git and
> > report if your issues are now fixed?
> >
> 
> That's not sufficient.  GET_FEATURES and SET_FEATURES  also need
> byte-addressing.
> 

AFAIK, the issue for GET_FEATURES and SET_FEATURES was also fixed.
IIRC, it was done here:

commit 05f7835975dad6b3b517f9e23415985e648fb875
Author: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Date:   Thu Dec 5 22:22:04 2013 +0100

    mtd: nand: don't use {read,write}_buf for 8-bit transfers

> No offense, but my patch seems both simpler and more complete?
> 

Could be.

Have you actually tested linux-next? It would be good to check if we still
have issues with 16-bit devices on the latest kernel, or if this is
already fixed. Last time I checked, my 16-bit device was ONFI-detected.

Brian: Related to this, the current code for fixing READID on 16-bit devices
seem to apply *only* for drivers that don't override cmdfunc(), right?
So each of the drivers that have their cmdfunc() currently need some
tweak to support 16-bit devices.

Or am I missing anything?
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH] mtd: nand: fix byte addressing on 16-bit wide devices

[David Mosberger]

On Fri, Mar 21, 2014 at 12:54 PM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> (Ccing Brian and Uwe)

> AFAIK, the issue for GET_FEATURES and SET_FEATURES was also fixed.
> IIRC, it was done here:
>
> commit 05f7835975dad6b3b517f9e23415985e648fb875

This commit fixes the transfer, but not the addressing.

Let me switch to linux-next and take it from there.

  --david
-- 
eGauge Systems LLC, http://egauge.net/, 1.877-EGAUGE1, fax 720.545.9768

Re: [PATCH] mtd: nand: fix byte addressing on 16-bit wide devices

[Brian Norris]

On Fri, Mar 21, 2014 at 03:54:46PM -0300, Ezequiel Garcia wrote:
> (Ccing Brian and Uwe)

Thanks for the heads up. Probably would have read it anyway, but you
never know :)

> Brian: Related to this, the current code for fixing READID on 16-bit devices
> seem to apply *only* for drivers that don't override cmdfunc(), right?
> So each of the drivers that have their cmdfunc() currently need some
> tweak to support 16-bit devices.

I think I fixed up all current in-tree drivers that override cmdfunc()
(and David's newer patch fixes the remaining opcodes). Whether they need
a tweak or not depends on how their hardware handles addressing for x16.
For some, they need to selectively convert some byte addresses to word
addresses. But for others, the hardware might take care of the
addressing.

Brian

Re: [PATCH] mtd: nand: fix byte addressing on 16-bit wide devices

[Brian Norris]

On Fri, Mar 21, 2014 at 01:43:41PM -0600, David Mosberger wrote:
> On Fri, Mar 21, 2014 at 12:54 PM, Ezequiel Garcia
> <ezequiel.garcia@free-electrons.com> wrote:
> > (Ccing Brian and Uwe)
> 
> > AFAIK, the issue for GET_FEATURES and SET_FEATURES was also fixed.
> > IIRC, it was done here:
> >
> > commit 05f7835975dad6b3b517f9e23415985e648fb875
> 
> This commit fixes the transfer, but not the addressing.

And the $SUBJECT patch fixes the addressing and not the transfer :)

> Let me switch to linux-next and take it from there.

I see that you did already.

Regards,
Brian