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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 14 Sep 2021 19:49:48 +0200 (CEST)
From:   Richard Weinberger <richard@....at>
To:     Miquel Raynal <miquel.raynal@...tlin.com>
Cc:     Bert Vermeulen <bert@...t.com>,
        Vignesh Raghavendra <vigneshr@...com>,
        Patrice Chotard <patrice.chotard@...s.st.com>,
        Boris Brezillon <boris.brezillon@...labora.com>,
        Christophe Kerello <christophe.kerello@...s.st.com>,
        Mark Brown <broonie@...nel.org>,
        Alexander Lobakin <alobakin@...me>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-mtd <linux-mtd@...ts.infradead.org>
Subject: Re: [PATCH v2] mtd: spinand: Add support for Etron EM73D044VCx

----- Ursprüngliche Mail -----
> Von: "Miquel Raynal" <miquel.raynal@...tlin.com>
> An: "richard" <richard@....at>
> CC: "Bert Vermeulen" <bert@...t.com>, "Vignesh Raghavendra" <vigneshr@...com>, "Patrice Chotard"
> <patrice.chotard@...s.st.com>, "Boris Brezillon" <boris.brezillon@...labora.com>, "Christophe Kerello"
> <christophe.kerello@...s.st.com>, "Mark Brown" <broonie@...nel.org>, "Alexander Lobakin" <alobakin@...me>,
> "linux-kernel" <linux-kernel@...r.kernel.org>, "linux-mtd" <linux-mtd@...ts.infradead.org>
> 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@...t.com 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@...t.com>
>> ---
>> 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