[PATCH V3 0/5] MXS nand fixes in SPL
Those patches come after some testing of failing in factory on some unit. We found out that the bootrom imx loader was not able to handling badblock. This can be a limit of the implementation right now in imx8mn. Anyway not all the imx platform has the support of this loader. I found some problems on the implementation so I have fixed it up according the experience of Sitara (coming from Dario). I tested only using a Fit Image as a flash container. This version add in the series the fix of cmd_nandbcb and the fix of spl_nand load. I can imagine that a lot of boards and users are affected. I have started to backport this changes in some older uboot and adapt it. Move BSH board using the spl loading instead the romapi
Michael Trimarchi (5): nand: raw: mxs_nand: Fix specific hook registration mtd: nand: mxs_nand_spl: Fix bad block skipping arm: mach-imx: cmd_nandbcb fix bad block handling spl: spl_nand: Fix bad block handling in fitImage board: bsh: Switch to nand spl load instead of romapi
arch/arm/mach-imx/cmd_nandbcb.c | 21 +++---- board/bsh/imx8mn_smm_s2/spl.c | 3 + common/spl/spl_nand.c | 5 +- drivers/mtd/nand/raw/mxs_nand.c | 32 +++++----- drivers/mtd/nand/raw/mxs_nand_spl.c | 90 ++++++++++++++++------------- 5 files changed, 76 insertions(+), 75 deletions(-)
Move the hook after nand_scan_tail is called. The hook must be replaced to the mxs specific one but those must to be assignment later in the probe function.
With this fix markbad is working again. Before this change:
nand markbad 0xDEC00 NXS NAND: Writing OOB isn't supported NXS NAND: Writing OOB isn't supported block 0x000dec00 NOT marked as bad! ERROR 0
Cc: Han Xu han.xu@nxp.com Cc: Fabio Estevam festevam@gmail.com Acked-by: Han Xu han.xu@nxp.com Tested-By: Tim Harvey tharvey@gateworks.com Signed-off-by: Michael Trimarchi michael@amarulasolutions.com --- V2->V3: - Add tested-by from Tim - Add ack from Han Xu V1->V2: - Adjust the commit message - Add Cc Han Xu and Fabio --- drivers/mtd/nand/raw/mxs_nand.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/mtd/nand/raw/mxs_nand.c b/drivers/mtd/nand/raw/mxs_nand.c index ee5d7fde9c..53f24b9c4b 100644 --- a/drivers/mtd/nand/raw/mxs_nand.c +++ b/drivers/mtd/nand/raw/mxs_nand.c @@ -1246,22 +1246,6 @@ int mxs_nand_setup_ecc(struct mtd_info *mtd) /* Enable BCH complete interrupt */ writel(BCH_CTRL_COMPLETE_IRQ_EN, &bch_regs->hw_bch_ctrl_set);
- /* Hook some operations at the MTD level. */ - if (mtd->_read_oob != mxs_nand_hook_read_oob) { - nand_info->hooked_read_oob = mtd->_read_oob; - mtd->_read_oob = mxs_nand_hook_read_oob; - } - - if (mtd->_write_oob != mxs_nand_hook_write_oob) { - nand_info->hooked_write_oob = mtd->_write_oob; - mtd->_write_oob = mxs_nand_hook_write_oob; - } - - if (mtd->_block_markbad != mxs_nand_hook_block_markbad) { - nand_info->hooked_block_markbad = mtd->_block_markbad; - mtd->_block_markbad = mxs_nand_hook_block_markbad; - } - return 0; }
@@ -1467,6 +1451,22 @@ int mxs_nand_init_ctrl(struct mxs_nand_info *nand_info) if (err) goto err_free_buffers;
+ /* Hook some operations at the MTD level. */ + if (mtd->_read_oob != mxs_nand_hook_read_oob) { + nand_info->hooked_read_oob = mtd->_read_oob; + mtd->_read_oob = mxs_nand_hook_read_oob; + } + + if (mtd->_write_oob != mxs_nand_hook_write_oob) { + nand_info->hooked_write_oob = mtd->_write_oob; + mtd->_write_oob = mxs_nand_hook_write_oob; + } + + if (mtd->_block_markbad != mxs_nand_hook_block_markbad) { + nand_info->hooked_block_markbad = mtd->_block_markbad; + mtd->_block_markbad = mxs_nand_hook_block_markbad; + } + err = nand_register(0, mtd); if (err) goto err_free_buffers;
The specific implementation was having bug. Those bugs are since the beginning of the implementation. Some manufactures can already experience this bug in their SPL code. This bug can be more visible on architecture that has complicated boot process like imx8mn. Older version of uboot can be affected if the bad block appear in correspoding of the beginning of u-boot image. In order to adjust the function we scan from the first erase block.
The problematic part of old code was in this part:
while (is_badblock(mtd, offs, 1)) { page = page + nand_page_per_block; /* Check i we've reached the end of flash. */ if (page >= mtd->size >> chip->page_shift) { free(page_buf); return -ENOMEM; } }
Even we fix it adding increment of the offset of one erase block size , we don't fix the problem, because the first erase block where the image start is not checked. The code was tested on an imx8mn where the boot rom api was not able to skip it. This code is used by other architecures like imx6 and imx8mm
Cc: Han Xu han.xu@nxp.com Cc: Fabio Estevam festevam@gmail.com Acked-by: Han Xu han.xu@nxp.com Tested-By: Tim Harvey tharvey@gateworks.com Signed-off-by: Michael Trimarchi michael@amarulasolutions.com --- V2->V3: - Add tested-by from Tim - Add ack from Han Xu - Rework english of commit message V1->V2: - Adjust the commit message - Add Cc Han Xu and Fabio - fix size >= 0 to > 0 --- drivers/mtd/nand/raw/mxs_nand_spl.c | 90 ++++++++++++++++------------- 1 file changed, 49 insertions(+), 41 deletions(-)
diff --git a/drivers/mtd/nand/raw/mxs_nand_spl.c b/drivers/mtd/nand/raw/mxs_nand_spl.c index 59a67ee414..2bfb181007 100644 --- a/drivers/mtd/nand/raw/mxs_nand_spl.c +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c @@ -218,14 +218,14 @@ void nand_init(void) mxs_nand_setup_ecc(mtd); }
-int nand_spl_load_image(uint32_t offs, unsigned int size, void *buf) +int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst) { - struct nand_chip *chip; - unsigned int page; + unsigned int sz; + unsigned int block, lastblock; + unsigned int page, page_offset; unsigned int nand_page_per_block; - unsigned int sz = 0; + struct nand_chip *chip; u8 *page_buf = NULL; - u32 page_off;
chip = mtd_to_nand(mtd); if (!chip->numchips) @@ -235,47 +235,42 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *buf) if (!page_buf) return -ENOMEM;
- page = offs >> chip->page_shift; - page_off = offs & (mtd->writesize - 1); + /* offs has to be aligned to a page address! */ + block = offs / mtd->erasesize; + lastblock = (offs + size - 1) / mtd->erasesize; + page = (offs % mtd->erasesize) / mtd->writesize; + page_offset = offs % mtd->writesize; nand_page_per_block = mtd->erasesize / mtd->writesize;
- debug("%s offset:0x%08x len:%d page:%x\n", __func__, offs, size, page); - - while (size) { - if (mxs_read_page_ecc(mtd, page_buf, page) < 0) - return -1; - - if (size > (mtd->writesize - page_off)) - sz = (mtd->writesize - page_off); - else - sz = size; - - memcpy(buf, page_buf + page_off, sz); - - offs += mtd->writesize; - page++; - buf += (mtd->writesize - page_off); - page_off = 0; - size -= sz; - - /* - * Check if we have crossed a block boundary, and if so - * check for bad block. - */ - if (!(page % nand_page_per_block)) { - /* - * Yes, new block. See if this block is good. If not, - * loop until we find a good block. - */ - while (is_badblock(mtd, offs, 1)) { - page = page + nand_page_per_block; - /* Check i we've reached the end of flash. */ - if (page >= mtd->size >> chip->page_shift) { + while (block <= lastblock && size > 0) { + if (!is_badblock(mtd, mtd->erasesize * block, 1)) { + /* Skip bad blocks */ + while (page < nand_page_per_block) { + int curr_page = nand_page_per_block * block + page; + + if (mxs_read_page_ecc(mtd, page_buf, curr_page) < 0) { free(page_buf); - return -ENOMEM; + return -EIO; } + + if (size > (mtd->writesize - page_offset)) + sz = (mtd->writesize - page_offset); + else + sz = size; + + memcpy(dst, page_buf + page_offset, sz); + dst += sz; + size -= sz; + page_offset = 0; + page++; } + + page = 0; + } else { + lastblock++; } + + block++; }
free(page_buf); @@ -294,6 +289,19 @@ void nand_deselect(void)
u32 nand_spl_adjust_offset(u32 sector, u32 offs) { - /* Handle the offset adjust in nand_spl_load_image,*/ + unsigned int block, lastblock; + + block = sector / mtd->erasesize; + lastblock = (sector + offs) / mtd->erasesize; + + while (block <= lastblock) { + if (is_badblock(mtd, block * mtd->erasesize, 1)) { + offs += mtd->erasesize; + lastblock++; + } + + block++; + } + return offs; }
The badblock should be skipped properly in reading and writing. Fix the logic. The bcb struct is written, skipping the bad block, so we need to read using the same logic. This was tested create bad block in the area and then flash it and read it back.
Acked-by: Han Xu han.xu@nxp.com Tested-By: Tim Harvey tharvey@gateworks.com Signed-off-by: Michael Trimarchi michael@amarulasolutions.com --- V2->V3: - Add tested-by from Tim - Add ack from Han Xu V1->V2: - Adjust the commit message - Add Cc Han Xu and Fabio - move out from RFC --- arch/arm/mach-imx/cmd_nandbcb.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mach-imx/cmd_nandbcb.c b/arch/arm/mach-imx/cmd_nandbcb.c index f119e9f88d..c54f52b343 100644 --- a/arch/arm/mach-imx/cmd_nandbcb.c +++ b/arch/arm/mach-imx/cmd_nandbcb.c @@ -506,10 +506,6 @@ static int read_fcb(struct boot_config *boot_cfg, struct fcb_block *fcb, int ret = 0;
mtd = boot_cfg->mtd; - if (mtd_block_isbad(mtd, off)) { - printf("Block %d is bad, skipped\n", (int)CONV_TO_BLOCKS(off)); - return 1; - }
fcb_raw_page = kzalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL); if (!fcb_raw_page) { @@ -530,7 +526,7 @@ static int read_fcb(struct boot_config *boot_cfg, struct fcb_block *fcb, else if (plat_config.misc_flags & FCB_ENCODE_BCH_40b) mxs_nand_mode_fcb_40bit(mtd);
- ret = nand_read(mtd, off, &size, (u_char *)fcb); + ret = nand_read_skip_bad(mtd, off, &size, NULL, mtd->size, (u_char *)fcb);
/* switch BCH back */ mxs_nand_mode_normal(mtd); @@ -617,6 +613,7 @@ static int write_fcb(struct boot_config *boot_cfg, struct fcb_block *fcb) for (i = 0; i < g_boot_search_count; i++) { if (mtd_block_isbad(mtd, off)) { printf("Block %d is bad, skipped\n", i); + off += mtd->erasesize; continue; }
@@ -676,20 +673,15 @@ static int read_dbbt(struct boot_config *boot_cfg, struct dbbt_block *dbbt, void *dbbt_data_page, loff_t off) { size_t size; + size_t actual_size; struct mtd_info *mtd; loff_t to; int ret;
mtd = boot_cfg->mtd;
- if (mtd_block_isbad(mtd, off)) { - printf("Block %d is bad, skipped\n", - (int)CONV_TO_BLOCKS(off)); - return 1; - } - size = sizeof(struct dbbt_block); - ret = nand_read(mtd, off, &size, (u_char *)dbbt); + ret = nand_read_skip_bad(mtd, off, &size, &actual_size, mtd->size, (u_char *)dbbt); printf("NAND DBBT read from 0x%llx offset 0x%zx read: %s\n", off, size, ret ? "ERROR" : "OK"); if (ret) @@ -697,9 +689,9 @@ static int read_dbbt(struct boot_config *boot_cfg, struct dbbt_block *dbbt,
/* dbbtpages == 0 if no bad blocks */ if (dbbt->dbbtpages > 0) { - to = off + 4 * mtd->writesize; + to = off + 4 * mtd->writesize + actual_size - size; size = mtd->writesize; - ret = nand_read(mtd, to, &size, dbbt_data_page); + ret = nand_read_skip_bad(mtd, to, &size, NULL, mtd->size, dbbt_data_page); printf("DBBT data read from 0x%llx offset 0x%zx read: %s\n", to, size, ret ? "ERROR" : "OK");
@@ -729,6 +721,7 @@ static int write_dbbt(struct boot_config *boot_cfg, struct dbbt_block *dbbt, if (mtd_block_isbad(mtd, off)) { printf("Block %d is bad, skipped\n", (int)(i + CONV_TO_BLOCKS(off))); + off += mtd->erasesize; continue; }
If the fitImage has some bad block in fit image area, the offset must be recalulcated. This should be done always. After implementing it in mxs now is possible to call the function even for that platform.
Cc: Fabio Estevam festevam@gmail.com Tested-By: Tim Harvey tharvey@gateworks.com Reviewed-by: Tom Rini trini@konsulko.com Signed-off-by: Michael Trimarchi michael@amarulasolutions.com --- V2->V3: - Add tested-by from Tim - Add ack from Han Xu V1->V2: - move out from RFC --- common/spl/spl_nand.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/common/spl/spl_nand.c b/common/spl/spl_nand.c index fc61b447a5..82a10ffa63 100644 --- a/common/spl/spl_nand.c +++ b/common/spl/spl_nand.c @@ -43,15 +43,12 @@ static ulong spl_nand_fit_read(struct spl_load_info *load, ulong offs, ulong size, void *dst) { int err; -#ifdef CONFIG_SYS_NAND_BLOCK_SIZE ulong sector;
sector = *(int *)load->priv; - offs = sector + nand_spl_adjust_offset(sector, offs - sector); -#else offs *= load->bl_len; size *= load->bl_len; -#endif + offs = sector + nand_spl_adjust_offset(sector, offs - sector); err = nand_spl_load_image(offs, size, dst); if (err) return 0;
romapi is not eble to skip bad block so we need to workaround using the spl
Signed-off-by: Michael Trimarchi michael@amarulasolutions.com --- board/bsh/imx8mn_smm_s2/spl.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/board/bsh/imx8mn_smm_s2/spl.c b/board/bsh/imx8mn_smm_s2/spl.c index 0f61acc630..e778cd16c4 100644 --- a/board/bsh/imx8mn_smm_s2/spl.c +++ b/board/bsh/imx8mn_smm_s2/spl.c @@ -18,6 +18,9 @@
int spl_board_boot_device(enum boot_device boot_dev_spl) { + if (IS_ENABLED(CONFIG_NAND_MXS) && get_boot_device() == USB_BOOT) + return BOOT_DEVICE_NAND; + return BOOT_DEVICE_BOOTROM; }
participants (1)
-
Michael Trimarchi