[PATCH] mtd: mxs_nand_spl: fix nand_command protocol violation

[PATCH] mtd: mxs_nand_spl: fix nand_command protocol violation

[Michael Trimarchi]

From: Andrea Scian <andrea.scian@dave.eu>

mxs_nand_command() implementation assume that it's working with a
LP NAND, which is a common case nowadays and thus uses two bytes
for column address.

However this is wrong for NAND_CMD_READID and NAND_CMD_PARAM, which
expects only one byte of column address, even for LP NANDs.
This leads to ONFI detection problem with some NAND manufacturer (like
Winbond) but not with others (like Samsung and Spansion)

We fix this with a simple workaround to avoid the 2nd byte column address
for those two commands.

Also align the code with nand_base to support 16 bit devices.

Tested on an iMX6SX device with:
* Winbond W29N04GVSIAA
* Spansion S34ML04G100TF100
* Samsung K9F4G08U00

Tested on imx8mn device with:
* Windbond W29N04GV

Signed-off-by: Andrea Scian <andrea.scian@dave.eu>
CC: Stefano Babic <sbabic@denx.de>
Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
---
 drivers/mtd/nand/raw/mxs_nand_spl.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/mxs_nand_spl.c b/drivers/mtd/nand/raw/mxs_nand_spl.c
index 2bfb181007..3daacbb330 100644
--- a/drivers/mtd/nand/raw/mxs_nand_spl.c
+++ b/drivers/mtd/nand/raw/mxs_nand_spl.c
@@ -29,8 +29,20 @@ static void mxs_nand_command(struct mtd_info *mtd, unsigned int command,
 
 	/* Serially input address */
 	if (column != -1) {
+		/* Adjust columns for 16 bit buswidth */
+		if (chip->options & NAND_BUSWIDTH_16 &&
+				!nand_opcode_8bits(command))
+			column >>= 1;
 		chip->cmd_ctrl(mtd, column, NAND_ALE);
-		chip->cmd_ctrl(mtd, column >> 8, NAND_ALE);
+
+		/*
+		 * Assume LP NAND here, so use two bytes column address
+		 * but not for CMD_READID and CMD_PARAM, which require
+		 * only one byte column address
+		 */
+		if (command != NAND_CMD_READID &&
+			command != NAND_CMD_PARAM)
+			chip->cmd_ctrl(mtd, column >> 8, NAND_ALE);
 	}
 	if (page_addr != -1) {
 		chip->cmd_ctrl(mtd, page_addr, NAND_ALE);
-- 
2.25.1

Re: [PATCH] mtd: mxs_nand_spl: fix nand_command protocol violation

[Michael Nazzareno Trimarchi]

Hi Tom

On Tue, Jun 21, 2022 at 10:05 PM Michael Trimarchi
<michael@amarulasolutions.com> wrote:
>
> From: Andrea Scian <andrea.scian@dave.eu>
>
> mxs_nand_command() implementation assume that it's working with a
> LP NAND, which is a common case nowadays and thus uses two bytes
> for column address.
>
> However this is wrong for NAND_CMD_READID and NAND_CMD_PARAM, which
> expects only one byte of column address, even for LP NANDs.
> This leads to ONFI detection problem with some NAND manufacturer (like
> Winbond) but not with others (like Samsung and Spansion)
>
> We fix this with a simple workaround to avoid the 2nd byte column address
> for those two commands.
>
> Also align the code with nand_base to support 16 bit devices.
>
> Tested on an iMX6SX device with:
> * Winbond W29N04GVSIAA
> * Spansion S34ML04G100TF100
> * Samsung K9F4G08U00
>
> Tested on imx8mn device with:
> * Windbond W29N04GV
>
> Signed-off-by: Andrea Scian <andrea.scian@dave.eu>
> CC: Stefano Babic <sbabic@denx.de>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> ---
>  drivers/mtd/nand/raw/mxs_nand_spl.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/mxs_nand_spl.c b/drivers/mtd/nand/raw/mxs_nand_spl.c
> index 2bfb181007..3daacbb330 100644
> --- a/drivers/mtd/nand/raw/mxs_nand_spl.c
> +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c
> @@ -29,8 +29,20 @@ static void mxs_nand_command(struct mtd_info *mtd, unsigned int command,
>
>         /* Serially input address */
>         if (column != -1) {
> +               /* Adjust columns for 16 bit buswidth */
> +               if (chip->options & NAND_BUSWIDTH_16 &&
> +                               !nand_opcode_8bits(command))
> +                       column >>= 1;
>                 chip->cmd_ctrl(mtd, column, NAND_ALE);
> -               chip->cmd_ctrl(mtd, column >> 8, NAND_ALE);
> +
> +               /*
> +                * Assume LP NAND here, so use two bytes column address
> +                * but not for CMD_READID and CMD_PARAM, which require
> +                * only one byte column address
> +                */
> +               if (command != NAND_CMD_READID &&
> +                       command != NAND_CMD_PARAM)
> +                       chip->cmd_ctrl(mtd, column >> 8, NAND_ALE);
>         }
>         if (page_addr != -1) {
>                 chip->cmd_ctrl(mtd, page_addr, NAND_ALE);
> --
> 2.25.1
>

It will be nice if you can pick this one too. Fabio can you review it?

Michael

Re: [PATCH] mtd: mxs_nand_spl: fix nand_command protocol violation

[Fabio Estevam]

Hi Michael,

On 29/06/2022 03:49, Michael Nazzareno Trimarchi wrote:

> It will be nice if you can pick this one too. Fabio can you review it?

The change looks good to me:

Reviewed-by: Fabio Estevam <festevam@denx.de>

Re: [PATCH] mtd: mxs_nand_spl: fix nand_command protocol violation

[Tom Rini]

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

On Tue, Jun 21, 2022 at 10:05:10PM +0200, Michael Trimarchi wrote:

> From: Andrea Scian <andrea.scian@dave.eu>
> 
> mxs_nand_command() implementation assume that it's working with a
> LP NAND, which is a common case nowadays and thus uses two bytes
> for column address.
> 
> However this is wrong for NAND_CMD_READID and NAND_CMD_PARAM, which
> expects only one byte of column address, even for LP NANDs.
> This leads to ONFI detection problem with some NAND manufacturer (like
> Winbond) but not with others (like Samsung and Spansion)
> 
> We fix this with a simple workaround to avoid the 2nd byte column address
> for those two commands.
> 
> Also align the code with nand_base to support 16 bit devices.
> 
> Tested on an iMX6SX device with:
> * Winbond W29N04GVSIAA
> * Spansion S34ML04G100TF100
> * Samsung K9F4G08U00
> 
> Tested on imx8mn device with:
> * Windbond W29N04GV
> 
> Signed-off-by: Andrea Scian <andrea.scian@dave.eu>
> CC: Stefano Babic <sbabic@denx.de>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> Reviewed-by: Fabio Estevam <festevam@denx.de>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]