Re: [PATCH v2] mtd: spinand: Add support for AllianceMemory AS5F34G04SND

[Mario Kicherer <dev@kicherer.org>]

Hello Miquèl,

I made all requested changes and added additional code to make the 
ooblayout
functions compatible with all flashes of this series.

I also ran nandbiterrs with the following result:

# nandbiterrs -i /dev/mtd11
incremental biterrors test
Successfully corrected 0 bit errors per subpage
Inserted biterror @ 0/5
Read reported 7 corrected bit errors
Successfully corrected 1 bit errors per subpage
Inserted biterror @ 0/2
Read reported 7 corrected bit errors
Successfully corrected 2 bit errors per subpage
Inserted biterror @ 0/0
Read reported 7 corrected bit errors
Successfully corrected 3 bit errors per subpage
Inserted biterror @ 1/7
Read reported 7 corrected bit errors
Successfully corrected 4 bit errors per subpage
Inserted biterror @ 1/5
Read reported 7 corrected bit errors
Successfully corrected 5 bit errors per subpage
Inserted biterror @ 1/2
Read reported 7 corrected bit errors
Successfully corrected 6 bit errors per subpage
Inserted biterror @ 1/0
Read reported 7 corrected bit errors
Successfully corrected 7 bit errors per subpage
Inserted biterror @ 2/6
Read reported 8 corrected bit errors
Successfully corrected 8 bit errors per subpage
Inserted biterror @ 2/5
Failed to recover 1 bitflips
Read error after 9 bit errors per page

I would say this looks good?

I will send the new patch in a few seconds.

Thank you!
Mario

On 2023-01-26 09:51, Miquel Raynal wrote:
> Hi Mario,
> 
> dev@kicherer.org wrote on Wed, 25 Jan 2023 16:03:58 +0100:
> 
>> On 2023-01-25 15:27, Miquel Raynal wrote:
>> >> >> +static int am_ecc_get_status(struct spinand_device *spinand, u8 >> status)
>> >> >> +{
>> >> >> +	switch (status & AM_STATUS_ECC_BITMASK) {
>> >> >> +	case AM_STATUS_ECC_NONE_DETECTED:
>> >> >> +		return 0;
>> >> >> +
>> >> >> +	case AM_STATUS_ECC_1_CORRECTED:
>> >> >> +		return 1;
>> >> >> +
>> >> >> +	case AM_STATUS_ECC_MAX_CORRECTED:
>> >> >> +		return 8;
>> >> >> +
>> >> >> +	case AM_STATUS_ECC_1_DETECTED:
>> >> >
>> >> > What does this mean "1 detected"?
>> >> >> According to the manual, the chip can report that a bit error was >> detected
>> >> but could not be corrected.
>> >
>> > That does not make a lot of sense. Either you use a Hamming algorithm,
>> > you will be able to correct 1 bit error and to detect 2 bit errors, or
>> > you use another algorithm (BCH, usually) with a strength of X (X > 1)
>> > and you'll be able to correct up to X errors and detect X+1 errors.
>> >
>> > But only being able to detect a single bit flip without being able to
>> > correct it is strange (even useless?). So I don't understand how it
>> > should be used.
>> 
>> Unfortunately, I do not have much experience with NAND flashes. The
>> manual does not say much more - page 41 in [1] in case you want to
>> see for yourself. I thought returning -EBADMSG in this case would be
>> the right thing to do as an uncorrectable error happened.
> 
> 00b = No bit errors were detected
> 01b = bit error was detected and corrected
> 10b = bit error was detected and not corrected
> 11b = bit error was detected and corrected, error bit number = ECC max 
> which
> is according to extended register.
> 
> I guess it means that if you have a strength of X:
> - from 1 bf to X-1 the status will be set to 0x1
> - if you have X bf it will be set to 0x3
> - if you have more than X bf, it will be set to 0x2.
> 
> Please update the code and use mtd-utils (like the bit error test) to
> verify what the tools says and by tracing the register values extracted
> with a simple printk to follow if the above logic works.
> 
>> I have not used the flash much yet, so I cannot say that I have much
>> confidence into the chip's error reporting. So far the flash worked
>> without issues.
>> 
>> Should I change something in the patch?
>> 
>> Best regards,
>> Mario
>> 
>> [1] 
>> https://www.alliancememory.com/wp-content/uploads/pdf/flash/AllianceMemory_SPI_NAND_Flash_July2020_Rev1.0.pdf
> 
> 
> Thanks,
> Miquèl

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