Hi Boris, Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx> wrote on Wed, 15 Jan 2020 08:58:06 +0100: > On Tue, 14 Jan 2020 20:46:17 +0000 > "Wojtaszczyk, Piotr" <WojtaszczykP@xxxxxxxxxxxxxxxxxx> wrote: > > > On 1/14/20 3:12 AM, Miquel Raynal wrote: > > > Crap. I'll not resend immediately as this is an RFC, I expect > > > feedback on this proposal before sending an actual patch. > > > > > > > > > Thanks, > > > Miquèl > > > > > > > Hi Miquèl, here are some my comments: > > > > +static int micron_nand_avoid_shallow_erase(struct nand_chip *chip, > > + unsigned int eb) > > +{ > > + struct micron_nand *micron = nand_get_manufacturer_data(chip); > > + unsigned int page = eb * nanddev_pages_per_eraseblock(&chip->base); > > + u8 *databuf = nand_get_data_buf(chip); > > + int ret, i; > > + > > + memset(databuf, 0xFF, nanddev_page_size(&chip->base)); > > + > > + /* Micron advises to only write odd pages */ > > + for (i = 0; i < MICRON_SHALLOW_ERASE_MIN_PAGE; i += 2, page += 2) { > > + if (!(micron->writtenp[eb] & BIT(i))) { > > + ret = nand_write_page_raw(chip, databuf, false, page); > > + if (ret) > > + return ret; > > + } > > + } > > + > > + return 0; > > +} > > > > Shouldn't we program only the OOB area of the pages to 0'es? Programming pages to 0xFF > > which are already 0xFF takes more time and doesn't make any difference. > > Hm, I'm pretty sure we should set in-band data to 0, not 0xff. > Programming only the OOB portion might not be enough for the internal > "is page written?" logic to return true. Absolutely, this is a mistake, the idea is to program all cells (to 0). > > Also after power loss all flags in micron->writtenp are gone so the > > micron_nand_avoid_shallow_erase will perform on all PEBs causing performance loss. > > Yes, that's a performance hit we'll have to accept for now. > This is quite severe issue, this is the best idea we came with to limit performance hits. Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/