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 359B2C433EF for ; Thu, 28 Apr 2022 05:01:35 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 7E6FA81B5B; Thu, 28 Apr 2022 07:01:32 +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="O7CO575b"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 33A1480F92; Thu, 28 Apr 2022 07:01:31 +0200 (CEST) Received: from mail-pl1-x636.google.com (mail-pl1-x636.google.com [IPv6:2607:f8b0:4864:20::636]) (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 6B5FA81B5B for ; Thu, 28 Apr 2022 07:01:27 +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-pl1-x636.google.com with SMTP id q8so3360967plx.3 for ; Wed, 27 Apr 2022 22:01:27 -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; bh=aA/trEKXzvO2Se16ViowXmzdr5T3oLlc3HKy+jxrac4=; b=O7CO575bTcucbkBtbPY2HHh85BBCvvsmM6w8GPDGU0rGC9Iz5D7FpM0Q8ekKrBNjz6 S7/TvEiG5jJj1PXWMrTdn79DsFg2Mce/1AOMVVJORqZNcU+UuAP+mZthNh62+K7y7zPd fg1p5wdXyBw4tw4cC1pu/Jq5h14YB60pIwFPk= 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; bh=aA/trEKXzvO2Se16ViowXmzdr5T3oLlc3HKy+jxrac4=; b=uH6h1SjCONo5cyzivkyxbZWp9jOiINm44gL04MT6zR/tmEfQtMA0grytTjVRPLH8tI z3L1jnfwnqb63OD9OYFmY/PD8vVd3Vbdu76diOLx+FFitP/bUgK3dQukyIeNI87+68SV xLxAZ3K4FImvRKEXExl04NErw7HBoVVxKLYZiaHM3PYABoeH2G84ApZVINgXRNUL5TAC I9Mvqzp7pGkEc1OEMAMPnTARc2HBDxlOYMyfeecVsj73ZdUJvJfi3NtPCHQbHf0F1Tu1 1U6c02jxEQblJL8pzueAh2kHBuvgR4MPlZfwdU0FTn/DV3P379/0GsqzEKym78KpW1v9 Wxvg== X-Gm-Message-State: AOAM533jXMwVRry2QSHqbei7hp8WcApMzfQj2sGYh2F4m2xna/rKUCTG 2hVXfWhs73XM/In6ef7C7LWcozJCM9hjPDHq5ddWJg== X-Google-Smtp-Source: ABdhPJxtPyD2s/nJy77pbfStJOSqoHCrPIlQPFlz0mExhMctbYgs0cPTcZm9taAVI/zP1tHBGWVUcbEFZDJIxGy2gVE= X-Received: by 2002:a17:902:e74c:b0:15e:60a0:c6d2 with SMTP id p12-20020a170902e74c00b0015e60a0c6d2mr1583046plf.130.1651122085716; Wed, 27 Apr 2022 22:01:25 -0700 (PDT) MIME-Version: 1.0 References: <20220427055025.231586-1-michael@amarulasolutions.com> <20220427055025.231586-3-michael@amarulasolutions.com> In-Reply-To: From: Michael Nazzareno Trimarchi Date: Thu, 28 Apr 2022 07:01:14 +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" 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 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 ; Miquel Raynal > > ; Fabio Estevam ; Dario > > Binacchi ; Sean Anderson > > ; linux-kernel@amarulasolutions.com; Jagan 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 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 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 >= 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 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