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

Miquel Raynal
2023-01-26T08:53:19Z

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