From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 68424C433FE for ; Fri, 6 May 2022 08:22:00 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 93DA3811D8; Fri, 6 May 2022 10:21:57 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=amarulasolutions.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=amarulasolutions.com header.i=@amarulasolutions.com header.b="KH76q7PF"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 2E81F80391; Fri, 6 May 2022 10:21:56 +0200 (CEST) Received: from mail-pj1-x102a.google.com (mail-pj1-x102a.google.com [IPv6:2607:f8b0:4864:20::102a]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 9449B81DB4 for ; Fri, 6 May 2022 10:21:46 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=amarulasolutions.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=michael@amarulasolutions.com Received: by mail-pj1-x102a.google.com with SMTP id fv2so6403419pjb.4 for ; Fri, 06 May 2022 01:21:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amarulasolutions.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=ettBPOuDvHnvwLyq70U8v7Fx6eyipmlpar7O9hXNcY8=; b=KH76q7PFzGh7XmONm9DA2BR7S4Ms84bE7hu16lPKNxaKGwHtfAZb+Yjg/4/AQiZMj2 sRGJLUeErzdpTnjzGRLeRbtEraygaW7J1FX44pRxLChvTkIKgg8C4fBceAq5GJeBdcV8 fgRtsYzunB2qBHVCrFvZa5LMpwKiBT0foH9sM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=ettBPOuDvHnvwLyq70U8v7Fx6eyipmlpar7O9hXNcY8=; b=nWCftSEbtLItquhsyn+IcdvToW385+U0xOz0RjUD6naACADzvIRNvsA+a+bVILzqGM k3f/RUeZDqAutdLyXQtnEOCsCMicyW3IarGrJoZb57PxnXiaceWvYJfHEnqA9ylyPFRQ aFcqqFnbn3bHh/TaFDixNALj97Zpgsh5wCm9YMF8hThEWYhnQBG/18dd2jF9svTHXbe5 oVt5UUX1u63ry0pB9fyYPsk5JFjmwgcGSubgbMNCCrKc3Qi1mWLgxg86/XFjsjo1APKf ok86H3xXkmvuoemRx4ZoHoPSKo7uJfjyKnspt7TbX3IlfgiMaCXEISqdpHCtCLvb+OkJ np8w== X-Gm-Message-State: AOAM533Iv+mxnDDqS9Xmp/8QaMotISgaAbf0UY7c2jAB6Q7dskhhZgif bBkek5OrmeU52uxdt788piQOqSgZSwBmubcpLBjtgw== X-Google-Smtp-Source: ABdhPJzpVoqbtSyD/vkJEhL6PQ0ki4Qheg8B66HUFkDxUsHx0FC6fJGzMO/71VQ0F1P3PFCXkaRZMPgRKHLlBnw6nL8= X-Received: by 2002:a17:90b:4a90:b0:1dc:aec3:c04 with SMTP id lp16-20020a17090b4a9000b001dcaec30c04mr10910378pjb.118.1651825304561; Fri, 06 May 2022 01:21:44 -0700 (PDT) MIME-Version: 1.0 References: <20220427055025.231586-1-michael@amarulasolutions.com> <20220427055025.231586-3-michael@amarulasolutions.com> <20220502213215.piy7bwyavagovbdk@umbrella> In-Reply-To: From: Michael Nazzareno Trimarchi Date: Fri, 6 May 2022 10:21:32 +0200 Message-ID: Subject: Re: [PATCH V2 2/4] mtd: nand: mxs_nand_spl: Fix bad block skipping To: Han Xu Cc: U-Boot-Denx , Ye Li , Stefano Babic , Miquel Raynal , Fabio Estevam , Dario Binacchi , Sean Anderson , "linux-kernel@amarulasolutions.com" , Jagan Teki , "Ariel D'Alessandro" , Fabio Estevam Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean Hi Han Any update? Michael On Tue, May 3, 2022 at 7:14 AM Michael Nazzareno Trimarchi wrote: > > Hi Han > > On Mon, May 2, 2022 at 11:32 PM Han Xu wrote: > > > > On 22/05/01 08:36AM, Michael Nazzareno Trimarchi wrote: > > > Dear Han and Fabio > > > > > > On Thu, Apr 28, 2022 at 7:01 AM Michael Nazzareno Trimarchi > > > wrote: > > > > > > > > Hi > > > > > > > > On Thu, Apr 28, 2022 at 2:27 AM Han Xu wrote: > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > From: Michael Trimarchi > > > > > > Sent: Wednesday, April 27, 2022 12:50 AM > > > > > > To: Han Xu ; U-Boot-Denx > > > > > > Cc: Ye Li ; Stefano Babic ; Miqu= el Raynal > > > > > > ; Fabio Estevam ; = Dario > > > > > > Binacchi ; Sean Anderson > > > > > > ; linux-kernel@amarulasolutions.com; Ja= gan Teki > > > > > > ; Ariel D'Alessandro > > > > > > ; Fabio Estevam > > > > > > Subject: [PATCH V2 2/4] mtd: nand: mxs_nand_spl: Fix bad block = skipping > > > > > > > > > > > > The specific implementation was having bug. Those bugs are sinc= e the beginning > > > > > > of the implementation. Some manufactures can receive this bug i= n their SPL code. > > > > > > This bug start to be more visible on architecture that has comp= licated boot > > > > > > process like imx8mn. Older version of uboot has the same proble= m 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 cha= nged 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 =3D page + nand_page_per_block; > > > > > > /* Check i we've reached the end of flash. */ > > > > > > if (page >=3D mtd->size >> chip->page_shift) { > > > > > > free(page_buf); > > > > > > return -ENOMEM; > > > > > > } > > > > > > } > > > > > > > > > > > > Even we fix it adding increment of the offset of one erase bloc= k size we don't fix > > > > > > the problem, because the first erase block where the image star= t 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 cov= er 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 writ= e > > > > 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 pa= tch > > > > try to fix the above behavior > > > > > > > > Case 3) can be any combination > > > > > > > > > > Do I need to resend v3? > > > > > > Michael > > > > It's not about the v3. I need to discuss some details with ROM team but > > unfortunately they are all in vacation till May 5th. I will respond aft= er the > > discussion. > > > > No problem I will wait, but the code is used by other nxp > architectures that don't use the > rom api. > > Michael > > Definit > > > > > > > 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 > > > > > > Cc: Fabio Estevam > > > > > > Signed-off-by: Michael Trimarchi > > > > > > --- > > > > > > V1->V2: > > > > > > - Adjust the commit message > > > > > > - Add Cc Han Xu and Fabio > > > > > > - fix size >=3D 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 =3D 0; > > > > > > + struct nand_chip *chip; > > > > > > u8 *page_buf =3D NULL; > > > > > > - u32 page_off; > > > > > > > > > > > > chip =3D mtd_to_nand(mtd); > > > > > > if (!chip->numchips) > > > > > > @@ -235,47 +235,42 @@ int nand_spl_load_image(uint32_t offs, un= signed int > > > > > > size, void *buf) > > > > > > if (!page_buf) > > > > > > return -ENOMEM; > > > > > > > > > > > > - page =3D offs >> chip->page_shift; > > > > > > - page_off =3D offs & (mtd->writesize - 1); > > > > > > + /* offs has to be aligned to a page address! */ > > > > > > + block =3D offs / mtd->erasesize; > > > > > > + lastblock =3D (offs + size - 1) / mtd->erasesize; > > > > > > + page =3D (offs % mtd->erasesize) / mtd->writesize; > > > > > > + page_offset =3D offs % mtd->writesize; > > > > > > nand_page_per_block =3D 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 =3D (mtd->writesize - page_off); > > > > > > - else > > > > > > - sz =3D size; > > > > > > - > > > > > > - memcpy(buf, page_buf + page_off, sz); > > > > > > - > > > > > > - offs +=3D mtd->writesize; > > > > > > - page++; > > > > > > - buf +=3D (mtd->writesize - page_off); > > > > > > - page_off =3D 0; > > > > > > - size -=3D 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 g= ood. If not, > > > > > > - * loop until we find a good block. > > > > > > - */ > > > > > > - while (is_badblock(mtd, offs, 1)) { > > > > > > - page =3D page + nand_page_per_blo= ck; > > > > > > - /* Check i we've reached the end = of flash. */ > > > > > > - if (page >=3D mtd->size >> chip->= page_shift) { > > > > > > + while (block <=3D lastblock && size > 0) { > > > > > > + if (!is_badblock(mtd, mtd->erasesize * block, 1))= { > > > > > > + /* Skip bad blocks */ > > > > > > + while (page < nand_page_per_block) { > > > > > > + int curr_page =3D nand_page_per_b= lock * block + > > > > > > page; > > > > > > + > > > > > > + if (mxs_read_page_ecc(mtd, page_b= uf, curr_page) > > > > > > < 0) { > > > > > > free(page_buf); > > > > > > - return -ENOMEM; > > > > > > + return -EIO; > > > > > > } > > > > > > + > > > > > > + if (size > (mtd->writesize - page= _offset)) > > > > > > + sz =3D (mtd->writesize - = page_offset); > > > > > > + else > > > > > > + sz =3D size; > > > > > > + > > > > > > + memcpy(dst, page_buf + page_offse= t, sz); > > > > > > + dst +=3D sz; > > > > > > + size -=3D sz; > > > > > > + page_offset =3D 0; > > > > > > + page++; > > > > > > } > > > > > > + > > > > > > + page =3D 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 =3D sector / mtd->erasesize; > > > > > > + lastblock =3D (sector + offs) / mtd->erasesize; > > > > > > + > > > > > > + while (block <=3D lastblock) { > > > > > > + if (is_badblock(mtd, block * mtd->erasesize, 1)) = { > > > > > > + offs +=3D mtd->erasesize; > > > > > > + lastblock++; > > > > > > + } > > > > > > + > > > > > > + block++; > > > > > > + } > > > > > > + > > > > > > return offs; > > > > > > } > > > > > > -- > > > > > > 2.25.1 > > > > > > > > > > > > > > > > > -- > > > > Michael Nazzareno Trimarchi > > > > Co-Founder & Chief Executive Officer > > > > M. +39 347 913 2170 > > > > michael@amarulasolutions.com > > > > __________________________________ > > > > > > > > Amarula Solutions BV > > > > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > > > > T. +31 (0)85 111 9172 > > > > info@amarulasolutions.com > > > > https://eur01.safelinks.protection.outlook.com/?url=3Dhttp%3A%2F%2F= www.amarulasolutions.com%2F&data=3D05%7C01%7Chan.xu%40nxp.com%7C81091e4= 443d745ce460708da2b3cec6a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6378= 69837927785927%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIi= LCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3DI5jvEx5qmQLzO8Qr= QT9aMYttDPZhdvbGi0Z2lk77bio%3D&reserved=3D0 > > > > > > > > > > > > -- > > > Michael Nazzareno Trimarchi > > > Co-Founder & Chief Executive Officer > > > M. +39 347 913 2170 > > > michael@amarulasolutions.com > > > __________________________________ > > > > > > Amarula Solutions BV > > > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > > > T. +31 (0)85 111 9172 > > > info@amarulasolutions.com > > > https://eur01.safelinks.protection.outlook.com/?url=3Dhttp%3A%2F%2Fww= w.amarulasolutions.com%2F&data=3D05%7C01%7Chan.xu%40nxp.com%7C81091e444= 3d745ce460708da2b3cec6a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637869= 837927785927%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLC= JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3DI5jvEx5qmQLzO8QrQT= 9aMYttDPZhdvbGi0Z2lk77bio%3D&reserved=3D0 > > > > -- > Michael Nazzareno Trimarchi > Co-Founder & Chief Executive Officer > M. +39 347 913 2170 > michael@amarulasolutions.com > __________________________________ > > Amarula Solutions BV > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > T. +31 (0)85 111 9172 > info@amarulasolutions.com > www.amarulasolutions.com --=20 Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 michael@amarulasolutions.com __________________________________ Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 info@amarulasolutions.com www.amarulasolutions.com