[PATCH V2 2/4] mtd: nand: mxs_nand_spl: Fix bad block skipping

Michael Nazzareno Trimarchi michael at amarulasolutions.com
Fri May 6 16:44:03 CEST 2022


Hi

Thank you. What is the reply from bootrom team?

Michael

On Fri, May 6, 2022 at 4:41 PM Han Xu <han.xu at nxp.com> wrote:
>
> On 22/04/27 07:50AM, Michael Trimarchi wrote:
> > 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. 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.xu at nxp.com>
> > Cc: Fabio Estevam <festevam at gmail.com>
> > Signed-off-by: Michael Trimarchi <michael at amarulasolutions.com>
>
> Acked-by: Han Xu <han.xu at nxp.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
> >



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael at amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info at amarulasolutions.com
www.amarulasolutions.com


More information about the U-Boot mailing list