Dear Han and Fabio On Thu, Apr 28, 2022 at 7:01 AM Michael Nazzareno Trimarchi <mich...@amarulasolutions.com> wrote: > > Hi > > On Thu, Apr 28, 2022 at 2:27 AM Han Xu <han...@nxp.com> wrote: > > > > > > > > > -----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. > > Suppose you have a badblock on 5 or 6. Let's start to have only 6 > and you write uboot from 5 and let's the uboot be enough big to cover 5, 6, > 7, 8 > > > Case 1) > When you write the block 6 the code will skip it as bad during > programming. THe image of uboot (or flash.bin) will > be on 5 7 8 9, because the 6 is skipped. The while loop on spl will > read (from raw offset the 5) and then he will found the > bad block on next erase block in the while loop and will exists at the > end of the flash because the test > is done on the offset and not on the page that is not incremented > > Case 2) > > Now same example but let's suppose to have block 5 bad. So you write > your image and it will start > from a raw offset 5 but it will be written starting from 6. The spl > loader will fail because it will not skip > the first block and then will fail anyway to read the image. The patch > try to fix the above behavior > > Case 3) can be any combination >
Do I need to resend v3? Michael > Michael > > > > > > > > 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->e > Copy Link > MA > Copy Link > NA > Copy Link > rasesize / 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 > mich...@amarulasolutions.com > __________________________________ > > Amarula Solutions BV > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > T. +31 (0)85 111 9172 > i...@amarulasolutions.com > www.amarulasolutions.com -- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 mich...@amarulasolutions.com __________________________________ Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 i...@amarulasolutions.com www.amarulasolutions.com