Re: [PATCH v2] mtd: spinand: Add support for Etron EM73D044VCx

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Miquel Raynal" <miquel.raynal@xxxxxxxxxxx>
> An: "richard" <richard@xxxxxx>
> CC: "Bert Vermeulen" <bert@xxxxxxxx>, "Vignesh Raghavendra" <vigneshr@xxxxxx>, "Patrice Chotard"
> <patrice.chotard@xxxxxxxxxxx>, "Boris Brezillon" <boris.brezillon@xxxxxxxxxxxxx>, "Christophe Kerello"
> <christophe.kerello@xxxxxxxxxxx>, "Mark Brown" <broonie@xxxxxxxxxx>, "Alexander Lobakin" <alobakin@xxxxx>,
> "linux-kernel" <linux-kernel@xxxxxxxxxxxxxxx>, "linux-mtd" <linux-mtd@xxxxxxxxxxxxxxxxxxx>
> Gesendet: Dienstag, 14. September 2021 19:31:08
> Betreff: Re: [PATCH v2] mtd: spinand: Add support for Etron EM73D044VCx

> Hi Bert,
> 
> Richard, a question for you below!

:)
 
> bert@xxxxxxxx wrote on Wed,  8 Sep 2021 22:16:19 +0200:
> 
>> This adds a new vendor Etron, and support for a 2Gb chip.
>> 
>> The datasheet is available at
>> https://www.etron.com/cn/products/EM73%5B8%5DC%5BD_E_F%5DVC%20SPI%20NAND%20Flash_Promotion_Rev%201_00A.pdf
>> 
>> Signed-off-by: Bert Vermeulen <bert@xxxxxxxx>
>> ---
>> v2:
>> - Made ooblayout_free/_ecc depend on chip-specific parameters, instead of
>>   hardcoded to this 2Gb chip only
>> - Fixed manufacturer ordering
>> - Fixed minor formatting issues as reported
>> - Removed debug comment
>> 
>>  drivers/mtd/nand/spi/Makefile |   2 +-
>>  drivers/mtd/nand/spi/core.c   |   1 +
>>  drivers/mtd/nand/spi/etron.c  | 104 ++++++++++++++++++++++++++++++++++
>>  include/linux/mtd/spinand.h   |   1 +
>>  4 files changed, 107 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/mtd/nand/spi/etron.c
> 
> [...]
> 
>> +static int etron_ecc_get_status(struct spinand_device *spinand, u8 status)
>> +{
>> +	switch (status & STATUS_ECC_MASK) {
>> +	case STATUS_ECC_NO_BITFLIPS:
>> +		return 0;
>> +
>> +	case STATUS_ECC_HAS_BITFLIPS:
>> +		/* Between 1-7 bitflips were corrected */
>> +		return 7;
> 
> Mmmh this is a bit problematic, having no intermediate value means a
> single bitflip will trigger UBI to move the data around as its
> threshold will be reached. Richard, any feedback on this?

So, the NAND controller can only report "no bitflips", "some bitflips", "maximum biflips" and "no way to fix"?
If so, yes, this is problematic for UBI because it will trigger wear-leveling way too often.
On a medium aged NAND I'd expect to see STATUS_ECC_HAS_BITFLIPS almost always set. :-(

Thanks,
//richard