From mboxrd@z Thu Jan 1 00:00:00 1970 From: Miquel Raynal Date: Sun, 29 Sep 2019 21:46:21 +0200 Subject: [U-Boot] [PATCH] cmd: mtd: solve bad block support in erase command In-Reply-To: <28f17168780c42a3bf4766f9e58bb691@SFHDAG6NODE3.st.com> References: <20190920072012.17841-1-patrick.delaunay@st.com> <20190926114231.4b3f0a7b@xps13> <28f17168780c42a3bf4766f9e58bb691@SFHDAG6NODE3.st.com> Message-ID: <20190929214621.359487b9@xps13> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: u-boot@lists.denx.de Hi Patrick, Patrick DELAUNAY wrote on Fri, 27 Sep 2019 11:23:09 +0000: > Hi Miquel >=20 > > From: Miquel Raynal > > Sent: jeudi 26 septembre 2019 11:43 > >=20 > > Hi Patrick, > >=20 > > Patrick DELAUNAY wrote on Thu, 26 Sep 2019 > > 09:31:46 +0000: > > =20 > > > Hi Stefan, > > > =20 > > > > From: Stefan Roese > > > > Sent: vendredi 20 septembre 2019 11:20 > > > > > > > > Hi Patrick, > > > > > > > > On 20.09.19 09:20, Patrick Delaunay wrote: =20 > > > > > This patch modify the loop in mtd erase command to erase one by > > > > > one the blocks in the requested area. > > > > > > > > > > It solves issue on "mtd erase" command on nand with existing bad > > > > > block, the command is interrupted on the first bad block with the= trace: > > > > > "Skipping bad block at 0xffffffffffffffff" > > > > > > > > > > In MTD driver (nand/raw), when a bad block is present on the MTD > > > > > device, the erase_op.fail_addr is not updated and we have the > > > > > initial value MTD_FAIL_ADDR_UNKNOWN =3D (ULL)-1. =20 > > > > > > > > So here is the difference? I remember testing this on a board with > > > > SPI NAND and here it worked correctly. But your test case is with R= AW =20 > > NAND? =20 > > > > > > Yes with RAW nand. > > > > > > it is the difference the U-Boot code, for SPI nan use: > > > int nanddev_mtd_erase() > > > > > > the fail address is always updated =20 > > > =3D> einfo->fail_addr =3D nanddev_pos_to_offs(nand, &pos); =20 > > > > > > =20 > > > > Do you have a change to also test this on a board with SPI NAND? =20 > > > > > > I do the test a SPI-NAND today. > > > > > > The mtd erase command was functional on SPI-ANND before my patch : > > > I create 2 bad block manually and they are correctly skipped. > > > =20 > > > STM32MP> mtd list =20 > > > List of MTD devices: > > > * spi-nand0 > > > - device: spi-nand at 0 > > > - parent: qspi at 58003000 > > > - driver: spi_nand > > > - type: NAND flash > > > - block size: 0x20000 bytes > > > - min I/O: 0x800 bytes > > > - OOB size: 128 bytes > > > - OOB available: 62 bytes > > > - 0x000000000000-0x000010000000 : "spi-nand0" > > > - 0x000000000000-0x000000200000 : "fsbl" > > > - 0x000000200000-0x000000400000 : "ssbl1" > > > - 0x000000400000-0x000000600000 : "ssbl2" > > > - 0x000000600000-0x000010000000 : "UBI" > > > =20 > > > STM32MP> mtd erase spi-nand0 0x00000000 0x10000000 =20 > > > Erasing 0x00000000 ... 0x0fffffff (2048 eraseblock(s)) > > > 0x0fd00000: bad block > > > 0x0fd20000: bad block > > > attempt to erase a bad/reserved block @fd00000 Skipping bad block at > > > 0x0fd00000 attempt to erase a bad/reserved block @fd20000 Skipping bad > > > block at 0x0fd20000 > > > 0x0fd00000: bad block > > > 0x0fd20000: bad block > > > > > > =20 > > > > Thanks, > > > > Stefan > > > > =20 > > > > > > What it is the better solution for you ? > > > > > > update the MTD command (my patch) or allign the behavior of the 2 MTD > > > devices > > > - MTD RAW NAND (nand_base.c:: nand_erase_nand) > > > - MTD SPI NAND (core.c:: nanddev_mtd_erase) =20 > >=20 > > Do you think it is easy to make use of nanddev_mtd_erase() from the raw= NAND > > core? It is probably a little bit more elegant (and efficient) to do al= l in one go than > > iterating over each block (while there is a helper in the core to do th= at). =20 >=20 >=20 > Yes, I agree: it will be more elegant. >=20 > But, I am not comfortable with MTD Raw NAND framework. >=20 > Based on a quick check between Linux Kernel 5.3 and U-Boot, it seems that= U-Boot > Raw NAND framework is aligned with Kernel 4.19 Raw NAND framework. > To be able to use nanddev_mtd_erase API, we should backport Raw NAND fram= ework > from Kernel 5.3 because nanddev_mtd_erase can be used only if memorg stru= cture > is properly set (has been done on Kernel 5.2). >=20 > I have not checked all potential impacts to use this API, but a backport = form Kernel > Raw NAND framework is needed in U-Boot in a first step. >=20 > As I am not comfortable with MTD frameworks, I think that my patch could= be currently > applied to solve this issue, and in a second step, when a MTD expert will= backport the > framework, it could be removed. >=20 > PS: A other solution with minimize the impacts in MTD, it is to change > only nand_base.c:nand_erase_nand(), to update the fail_addr: >=20 > ----------------------- drivers/mtd/nand/raw/nand_base.c ----------------= ------- index aba8ac019d..50542a2b9a 100644 @@ -3554,6 +3554,8 @@ int nand_= erase_nand(struct mtd_info *mtd, struct erase_info *instr, > pr_warn("%s: attempt to erase a bad block at page 0x%08x\n", > __func__, page); > instr->state =3D MTD_ERASE_FAILED; > + instr->fail_addr =3D > + ((loff_t)page << chip->page_shift); > goto erase_exit; > } >=20 > But as it is also a common MTD part with Linux kernel so I continue to pr= efer > my patch on U-Boot only code. I understand, I'm fine with the cmd/mtd.c to be changed only. Thanks, Miqu=C3=A8l