Re: [PATCH v2] mtd: spinand: add support for Foresee FS35ND01G-S1Y2

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Daniel,

Daniel Palmer <daniel@0x0f.com> wrote on Fri, 26 Mar 2021 23:09:28
+0900:

> Hi Miquel,
> 
> Sorry for the constant pestering on this..
> 
> On Tue, 23 Mar 2021 at 23:06, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > # nandbiterrs -i /dev/mtd1
> > > incremental biterrors test
> > > Successfully corrected 0 bit errors per subpage
> > > Inserted biterror @ 0/5
> > > Read reported 4 corrected bit errors
> > > ECC failure, invalid data despite read success  
> >
> > This is not a valid behavior. There is something wrong with the way ECC
> > status is read/retrieved. The read should indeed report 4 corrected bit
> > errors, but then the data should be valid. Here it means that the
> > introduced error appears corrected but in fact is not.
> > We need to understand what status are available and write the
> > appropriate vendor code.  
> 
> I looked at the datasheet again and the encoding for ecc status seems
> a bit funky.
> The chip seems to have only two bits for ECC status with this encoding:
> 0 - Read success with 0-3 flipped bits.
> 1 - Read success with 4 flipped bits.
> 2 - Uncorrectable.
> 3 - Reserved.
> 

Very nice information.

> This looks almost the same as the Winbond chips with 0 and 1 being successes
> but with no totally error free status.
> 
> Anyhow, if 4 bits were corrected is returned for 1 then nandbiterrs
> reports "ECC failure, invalid data despite read success".
> If I return 3 bit errors for 0 and -EBADMSG for anything else
> nandbiterrs doesn't complain anymore but is this right?:

You may look at micron_8_ecc_get_status() helper to guide you. But
IMHO, if there are 0-3 bf, you should probably assume there were 3 bf
and return 3, if there were 4, return 4, if it's uncorrectable return
-EBADMSG otherwise -EINVAL.

We should verify that this does not mess with UBI wear leveling
though. Please check that returning 3-bit errors no matter the
actual number of flipped bits does not lead UBI to move the data away
(I think it's fine but we need to be sure otherwise the implementation
proposal is not valid).

Thanks,
Miquèl