Re: [RFC PATCH 0/3] Fix proposal for the Micron shallow erase issue

[Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>]

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.

> 
> 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.

> 
> Instead we could check a flag in OOB area of first page of the PEB we are about to erase 
> and clear the flag bit/bits when 16th page of the PEB gets programmed. Simmilar to bad 
> block mark.

Well, that doesn't work well because:

1/ programming a page that has already be programmed is not always
   allowed (you must have sub-page write support and the page must have
   been programmed less than the number of subpage writes)
2/ controller side ECCs are in the way, and we don't have a proper
   solution to describe unprotected OOB regions. Even if we had
   that in place, there might not even be such unprotected OOB left
   (some controllers protect/use the whole OOB area)

Note that the writes we do in micron_nand_avoid_shallow_erase() will
corrupt data present in those pages, but that shouldn't be a problem,
because we want to erase them anyway.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/