Re: Question about Davinci NAND using 4 bit ECC

Re: Question about Davinci NAND using 4 bit ECC

[Miquel Raynal]

Hi Christopher,

christophercordahi@nanometrics.ca wrote on Mon, 17 Jul 2023 21:46:34
-0400:

> Hello Linux TI NAND experts,
> 
> An ex-colleague discovered a discrepancy between the nand_davinci_correct_4bit()
> function [1] and the sequence of steps recommended by TI.  I'm using a TI OMAP
> L138 processor, based on the DA-850 EVM board.  Ideally the code should match
> the sequence of steps in the OMAP-L138 Technical Reference Manual section
> 20.2.5.6 NAND Flash Mode [2].  This very old driver was initially written for
> the DM355 processor on a DM-355 EVM board.  The documentation in the older
> TMS320DM35x Digital Media System-on-Chip (DMSoC) Asynchronous External Memory
> Interface (EMIF) [3] is similar to the more recent documentation for the OMAP
> L138, but not exactly the same.  I suspect both processors have the same EMIF
> peripheral, and the L138 procedure is simply more up-to-date, but I can't be
> sure.  And the code doesn't match either one.
> 
> The first difference is that the code contains the following
> 
> /*
> * Clear any previous address calculation by doing a dummy read of an
> * error address register.
> */
> davinci_nand_readl(info, NAND_ERR_ADD1_OFFSET);
> 
> /* Start address calculation, and wait for it to complete.
> * We _could_ start reading more data while this is working,
> * to speed up the overall page read.
> */
> davinci_nand_writel(info, NANDFCR_OFFSET,
>         davinci_nand_readl(info, NANDFCR_OFFSET) | BIT(13));
> 
> The dummy read is not mentioned in the DM-355 documentation.
> The f12a9473283e68ae708e9ada37cb352ea2652397 commit [4] in which it was added
> doesn't provide any details.  This dummy read is mentioned in step 10 of the
> newer L138 documentation, but it should be after the "Start address calculation"
> write in step 9 and it specifically indicates to not use that error address
> register.
> 
> > 10. Perform a dummy read to any EMIFA registers except the NAND Flash error
> > address 1-2 registers (NANDERRADD[2:1]) or the NAND Flash error value 1-2
> > registers (NANDERRVAL[2:1]).  
> 
> The code then continues with an extra block introduced in commit
> 1c3275b656045aff9a75bb2c9f3251af1043ebb3 [5] to work-around unexpected behaviour
> before continuing with what seems to be the correct procedure.
> 
> /*
> * ECC_STATE field reads 0x3 (Error correction complete) immediately
> * after setting the 4BITECC_ADD_CALC_START bit. So if you immediately
> * begin trying to poll for the state, you may fall right out of your
> * loop without any of the correction calculations having taken place.
> * The recommendation from the hardware team is to initially delay as
> * long as ECC_STATE reads less than 4. After that, ECC HW has entered
> * correction state.
> */
> timeo = jiffies + usecs_to_jiffies(100);
> do {
>     ecc_state = (davinci_nand_readl(info,
>             NANDFSR_OFFSET) >> 8) & 0x0f;
>     cpu_relax();
> } while ((ecc_state < 4) && time_before(jiffies, timeo));
> 
> Can anyone rationalize these discrepancies?
> Do these two discrepancies offset each other resulting in correct behaviour?
> If this procedure is in fact incorrect, what would the symptoms be?
> Does this mean that anything read from NAND Flash could be corrupted?
> 
> Any clarification would be greatly appreciated.

It is hard to tell whether these changes are legitimate or not without
properly testing. If you benchmark your changes and never see any
error, it does not mean you fall into the pits during your tests. If
however, by removing one of the two hacks, you observe a wrong
behavior, then you could perhaps tell that the latter offsets the
former. But TBH I don't think they are related, on one side we want to
reset the outcome of the former calculations, on the other side we need
to wait a bit before a status register can be trusted.

Observing bitflips in userspace and detecting them can be achieved
easily with the nandbiterrs -i tool from the mtd-utils test suite. This
tool manually introduces bitflips and expects them to be corrected
(until proper failure) by the ECC engine. Be aware, it will smash your
data.

> [1] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/mtd/nand/raw/davinci_nand.c
> [2] https://www.ti.com/lit/ug/spruh77c/spruh77c.pdf#page=884
> [3] https://www.ti.com/lit/ug/sprued1b/sprued1b.pdf?ts#page=29
> [4] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/mtd/nand/davinci_nand.c?id=f12a9473283e68ae708e9ada37cb352ea2652397
> [5] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/mtd/nand/davinci_nand.c?id=1c3275b656045aff9a75bb2c9f3251af1043ebb3
> 

Good luck,
Miquèl

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

Re: Question about Davinci NAND using 4 bit ECC

[Christopher Cordahi]

On Tue, 18 Jul 2023 at 04:02, Miquel Raynal <miquel.raynal@bootlin.com> wrote:

Thanks Miquel for the quick response

And apologies for sending you the question 3 times.  My first attempt included
an HTML copy which was rejected by the mailing list, and it took me an extra
attempt to force my email client to use plain text.  And the third email is
still awaiting approval by the mailing list moderator, I suspect due to how I
forwarded and reformatted the original email back to the mailing list.

> It is hard to tell whether these changes are legitimate or not without
> properly testing. If you benchmark your changes and never see any
> error, it does not mean you fall into the pits during your tests.

OK, I was hoping to avoid my own testing because as you write, no errors
doesn't mean it's better.  And I was overly optimistic in hoping for an
explanation as to why this driver mostly written by TI 15 years ago doesn't
match their documented procedure.

> Observing bitflips in userspace and detecting them can be achieved
> easily with the nandbiterrs -i tool from the mtd-utils test suite.

Thanks for this tip, I'll look into extending the Yocto mtd-utils recipe [1]
to install it.  I see the kernel also has a nandbiterrs mtd test driver[2].
Can it be used? Is there any documentation on how to use this kernel test?

[1] https://git.yoctoproject.org/poky/plain/meta/recipes-devtools/mtd/mtd-utils_git.bb
[2] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/mtd/tests/nandbiterrs.c

--
Chris

-- 
This message is intended exclusively for the individual or entity to which 
it is addressed. This communication may contain information that is 
proprietary, privileged, confidential or otherwise legally exempt from 
disclosure. If you are not the named addressee, or have been inadvertently 
and erroneously referenced in the address line, you are not authorized to 
read, print, retain, copy or disseminate this message or any part of it. If 
you have received this message in error, please notify the sender 
immediately by e-mail and delete all copies of the message.

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

Re: Question about Davinci NAND using 4 bit ECC

[Miquel Raynal]

Hi Christopher,

christophercordahi@nanometrics.ca wrote on Wed, 19 Jul 2023 12:34:36
-0400:

> On Tue, 18 Jul 2023 at 04:02, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> Thanks Miquel for the quick response
> 
> And apologies for sending you the question 3 times.  My first attempt included
> an HTML copy which was rejected by the mailing list, and it took me an extra
> attempt to force my email client to use plain text.  And the third email is
> still awaiting approval by the mailing list moderator, I suspect due to how I
> forwarded and reformatted the original email back to the mailing list.

No problem :)

> > It is hard to tell whether these changes are legitimate or not without
> > properly testing. If you benchmark your changes and never see any
> > error, it does not mean you fall into the pits during your tests.  
> 
> OK, I was hoping to avoid my own testing because as you write, no errors
> doesn't mean it's better.  And I was overly optimistic in hoping for an
> explanation as to why this driver mostly written by TI 15 years ago doesn't
> match their documented procedure.

Probably :-)

> > Observing bitflips in userspace and detecting them can be achieved
> > easily with the nandbiterrs -i tool from the mtd-utils test suite.  
> 
> Thanks for this tip, I'll look into extending the Yocto mtd-utils recipe [1]
> to install it.  I see the kernel also has a nandbiterrs mtd test driver[2].
> Can it be used? Is there any documentation on how to use this kernel test?

The kernel tests are deprecated. You can play with them, just insmod
them in your kernel. They have been reimplemented in userspace to check
the whole mtd stack, and that's the mtd-utils test suite. Prefer using
these.

Yes, adding the debug tools in Yocto might be a good idea. I already
did it for the package in Buildroot.

> [1] https://git.yoctoproject.org/poky/plain/meta/recipes-devtools/mtd/mtd-utils_git.bb
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/mtd/tests/nandbiterrs.c

Thanks,
Miquèl

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