[U-Boot] [PATCH 0/2] iMX6 SPL NAND flash detection fixes
While working on different iMX6 platforms, I've encountered some issue with a couple of NAND device while booting from SPL.
It seems that there's a NAND protocol violation in ONFI detection: I try a naive fix, which works only for LP NAND. However, AFAIK, all the mxs_nand_spl code is minimalistic and assume it's working with a LP NAND. A more generic approach will have a bigger impact and requires more tests.
I've also changed mxs_flash_ident() code so, when legacy full-ident detection is enabled, this one is used only as fall back when ONFI detection fails.
This is closer to standard MTD implementation than the previous solution, where ONFI and full-ident detection where mutually exclusive.
This has been originally developed on old v2017.03 but ported on current master.
Kind Regards,
Andrea Scian
Andrea Scian (2): mtd: mxs_nand_spl: fix nand_command protocol violation mtd: mxs_nand_spl: use legacy probe only if ONFI fails
drivers/mtd/nand/raw/mxs_nand_spl.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)
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
Signed-off-by: Andrea Scian andrea.scian@dave.eu CC: Stefano Babic sbabic@denx.de --- 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 2d7bbe83cc..ad3b7ade64 100644 --- a/drivers/mtd/nand/raw/mxs_nand_spl.c +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c @@ -22,8 +22,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);
Hi all
On Tue, Jan 29, 2019 at 3:40 PM Andrea Scian andrea.scian@dave.eu wrote:
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
Signed-off-by: Andrea Scian andrea.scian@dave.eu CC: Stefano Babic sbabic@denx.de
Is this somenthing that is already addressed?
Michael
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 2d7bbe83cc..ad3b7ade64 100644 --- a/drivers/mtd/nand/raw/mxs_nand_spl.c +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c @@ -22,8 +22,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.19.2
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Hi Michael, On 14/05/19 20:44, Michael Nazzareno Trimarchi wrote:
Hi all
On Tue, Jan 29, 2019 at 3:40 PM Andrea Scian andrea.scian@dave.eu wrote:
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
Signed-off-by: Andrea Scian andrea.scian@dave.eu CC: Stefano Babic sbabic@denx.de
Is this somenthing that is already addressed?
AFAIK, this is still an open problem on current master This patch has not been integrated and the code is the same as the one that have the original issue
Andrea SCIAN
*DAVE Embedded Systems*
via Talponedo 29/A 33080 Porcia (PN) - Italy Telephone: +39.0434.921215 Telefax: +39.0434.1994030 web: www.dave.eu http://www.dave.eu
Michael
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 2d7bbe83cc..ad3b7ade64 100644 --- a/drivers/mtd/nand/raw/mxs_nand_spl.c +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c @@ -22,8 +22,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.19.2
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
On 14/05/19 22:56, Andrea Scian - DAVE Embedded Systems wrote:
Hi Michael, On 14/05/19 20:44, Michael Nazzareno Trimarchi wrote:
Hi all
On Tue, Jan 29, 2019 at 3:40 PM Andrea Scian andrea.scian@dave.eu wrote:
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
Signed-off-by: Andrea Scian andrea.scian@dave.eu CC: Stefano Babic sbabic@denx.de
Is this somenthing that is already addressed?
AFAIK, this is still an open problem on current master This patch has not been integrated and the code is the same as the one that have the original issue
As far as I see on patchwork, patch was not assigned to any maintainer - I have now assigned to Scott (as NAND maintainer).
Stefano
Andrea SCIAN
*DAVE Embedded Systems*
via Talponedo 29/A 33080 Porcia (PN) - Italy Telephone: +39.0434.921215 Telefax: +39.0434.1994030 web: www.dave.eu http://www.dave.eu
Michael
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 2d7bbe83cc..ad3b7ade64 100644 --- a/drivers/mtd/nand/raw/mxs_nand_spl.c +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c @@ -22,8 +22,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.19.2
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Hi Stefano
On Fri, May 17, 2019 at 4:52 PM Stefano Babic sbabic@denx.de wrote:
On 14/05/19 22:56, Andrea Scian - DAVE Embedded Systems wrote:
Hi Michael, On 14/05/19 20:44, Michael Nazzareno Trimarchi wrote:
Hi all
On Tue, Jan 29, 2019 at 3:40 PM Andrea Scian andrea.scian@dave.eu wrote:
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.
We have partially tested on imx28 but we have other problem on it. I will give test-by when everything is working correctly
Michael
Also align the code with nand_base to support 16 bit devices.
Tested on an iMX6SX device with:
- Winbond W29N04GVSIAA
- Spansion S34ML04G100TF100
- Samsung K9F4G08U00
Signed-off-by: Andrea Scian andrea.scian@dave.eu CC: Stefano Babic sbabic@denx.de
Is this somenthing that is already addressed?
AFAIK, this is still an open problem on current master This patch has not been integrated and the code is the same as the one that have the original issue
As far as I see on patchwork, patch was not assigned to any maintainer - I have now assigned to Scott (as NAND maintainer).
Stefano
Andrea SCIAN
*DAVE Embedded Systems*
via Talponedo 29/A 33080 Porcia (PN) - Italy Telephone: +39.0434.921215 Telefax: +39.0434.1994030 web: www.dave.eu http://www.dave.eu
Michael
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 2d7bbe83cc..ad3b7ade64 100644 --- a/drivers/mtd/nand/raw/mxs_nand_spl.c +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c @@ -22,8 +22,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.19.2
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de =====================================================================
This updates mxs_flash_ident() to always use ONFI NAND detection and, if enabled using CONFIG_SPL_NAND_IDENT, fallback into legacy full NAND ids detection if ONFI fails.
This allows to have a single binary to support ONFI and non-ONFI NAND, without the need to always use legacy full NAND ids detection.
Signed-off-by: Andrea Scian andrea.scian@dave.eu CC: Stefano Babic sbabic@denx.de --- drivers/mtd/nand/raw/mxs_nand_spl.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/mtd/nand/raw/mxs_nand_spl.c b/drivers/mtd/nand/raw/mxs_nand_spl.c index ad3b7ade64..41b3e0d9d5 100644 --- a/drivers/mtd/nand/raw/mxs_nand_spl.c +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c @@ -80,7 +80,7 @@ static int mxs_flash_full_ident(struct mtd_info *mtd) return 0; }
-#else +#endif /* CONFIG_SPL_NAND_IDENT */
/* Trying to detect the NAND flash using ONFi only */ static int mxs_flash_onfi_ident(struct mtd_info *mtd) @@ -143,15 +143,15 @@ static int mxs_flash_onfi_ident(struct mtd_info *mtd) return 0; }
-#endif /* CONFIG_SPL_NAND_IDENT */ - static int mxs_flash_ident(struct mtd_info *mtd) { int ret; -#if defined (CONFIG_SPL_NAND_IDENT) - ret = mxs_flash_full_ident(mtd); -#else ret = mxs_flash_onfi_ident(mtd); +#if defined (CONFIG_SPL_NAND_IDENT) + if (ret) { + puts("NAND: ONFI probe failed, trying legacy ident\n"); + ret = mxs_flash_full_ident(mtd); + } #endif return ret; }
participants (4)
-
Andrea Scian -
Andrea Scian - DAVE Embedded Systems
-
Michael Nazzareno Trimarchi -
Stefano Babic