Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v2] mtd: spinand: Add support for Etron EM73D044VCx
To:     Richard Weinberger <richard@nod.at>,
        Miquel Raynal <miquel.raynal@bootlin.com>
Cc:     Vignesh Raghavendra <vigneshr@ti.com>,
        Patrice Chotard <patrice.chotard@foss.st.com>,
        Boris Brezillon <boris.brezillon@collabora.com>,
        Christophe Kerello <christophe.kerello@foss.st.com>,
        Mark Brown <broonie@kernel.org>,
        Alexander Lobakin <alobakin@pm.me>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        linux-mtd <linux-mtd@lists.infradead.org>
References: <20210908201624.237634-1-bert@biot.com>
 <20210914193108.78df5367@xps13>
 <1517789471.73175.1631641788145.JavaMail.zimbra@nod.at>
From:   Bert Vermeulen <bert@biot.com>
Message-ID: <927c7c75-c0d3-b3fb-6b85-13dbc3c6edbe@biot.com>
Date:   Tue, 14 Sep 2021 22:03:49 +0200
MIME-Version: 1.0
In-Reply-To: <1517789471.73175.1631641788145.JavaMail.zimbra@nod.at>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 9/14/21 7:49 PM, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
>> bert@biot.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@biot.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. :-(

Yes, that's all there is according to the datasheet. Can't be _that_
unusual, since that's all the STATUS_ECC_* flags cover.

Incidentally I'm abusing STATUS_ECC_MASK here, since the 0b11 pattern is
missing from those flags.


-- 
Bert Vermeulen
bert@biot.com