> -----Original Message-----
> From: Michael Trimarchi <mich...@amarulasolutions.com>
> Sent: Wednesday, April 27, 2022 12:50 AM
> To: Han Xu <han...@nxp.com>; U-Boot-Denx <u-boot@lists.denx.de>
> Cc: Ye Li <ye...@nxp.com>; Stefano Babic <sba...@denx.de>; Miquel Raynal
> <miquel.ray...@bootlin.com>; Fabio Estevam <feste...@denx.de>; Dario
> Binacchi <dario.binac...@amarulasolutions.com>; Sean Anderson
> <sean.ander...@seco.com>; linux-ker...@amarulasolutions.com; Jagan Teki
> <ja...@amarulasolutions.com>; Ariel D'Alessandro
> <ariel.dalessan...@collabora.com>; Fabio Estevam <feste...@gmail.com>
> Subject: [PATCH V2 2/4] mtd: nand: mxs_nand_spl: Fix bad block skipping
> 
> The specific implementation was having bug. Those bugs are since the beginning
> of the implementation. Some manufactures can receive this bug in their SPL 
> code.
> This bug start to be more visible on architecture that has complicated boot
> process like imx8mn. Older version of uboot has the same problem only if the
> bad block appear in correspoding of befine of u-boot image. In order to adjust
> the function we scan from the first block. The logic is not changed to have a
> simple way to fix without get regression.
> 
> 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.

Could you please describe more details about your test? Thanks.

> The code was tested on an imx8mn where the boot rom api was not able to skip
> it. Apart of that other architecure are using this code and all boards that 
> has
> nand as boot device can be affected
> 
> Cc: Han Xu <han...@nxp.com>
> Cc: Fabio Estevam <feste...@gmail.com>
> Signed-off-by: Michael Trimarchi <mich...@amarulasolutions.com>
> ---
> 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;
>  }
> --
> 2.25.1

Reply via email to