Re: [PATCH] mtd: spinand: Add support for GigaDevice GD5F1GQ4UC

[Boris Brezillon <bbrezillon@xxxxxxxxxx>]

On Wed, 23 Jan 2019 13:34:06 +0100
Stefan Roese <sr@xxxxxxx> wrote:

> On 23.01.19 13:22, Boris Brezillon wrote:
> 
> <snip>
> 
> > Okay. There's something interesting in section "10.1 Page Program" of
> > the datasheet:
> > 
> > "
> > Note:
> > 1. The contents of Cache Register doesn’t reset when Program Load (02h)
> > command, Program Random Load (84h)
> > command and RESET (FFh) command.
> > 2. When Program Execute (10h) command was issued just after Program Load
> > (02h) command, SPI-NAND controller
> > outputs 0xFF data to the NAND for the address that data was not loaded
> > by Program Load (02h) command.
> > 3. When Program Execute (10h) command was issued just after Program Load
> > Random Data (84h) command,
> > SPI-NAND controller outputs contents of Cache Register to the NAND.
> > "
> > 
> > Until now, I assumed that a "Program Load" would reset the page cache
> > content to 0xff (as is done on the NANDs I had tested on), but it seems
> > some vendors decided to implement it differently (keep the cache in its
> > previous state and send 0xff at execute time if the previous command
> > was a Program Load and some bytes were left uninitialized in the cache).
> > 
> > This forces us to fill the whole cache if we want the logic to work on
> > all NANDs otherwise we might corrupt things in the OOB area. It might
> > also explain while nandbiterrs does not work properly. Can you try to
> > apply the following diff and run nandbiterrs -i again?  
> 
> Definitely something different this time:
> 
> root@mt7688:~# ./nandbiterrs /dev/mtd5 -i
> incremental biterrors test
> [   24.467652] gd5f1gq4u_ecc_get_status (124): status=0x00 status2=0x00
> Successfully corrected 0 bit errors per subpage
> Inserted biterr[   24.478946] gd5f1gq4u_ecc_get_status (124): status=0x10 status2=0x00
> or @ 1/7
> [   24.486718] gd5f1gq4u_ecc_get_status (138): status2=0x00
> Read reported 4 corrected bit errors
> Successfully corrected 1 b[   24.497760] gd5f1gq4u_ecc_get_status (124): status=0x10 status2=0x00
> it errors per su[   24.505708] gd5f1gq4u_ecc_get_status (138): status2=0x00
> bpage
> Inserted biterror @ 3/7
> Read reported 4 corrected bit er[   24.517400] gd5f1gq4u_ecc_get_status (124): status=0x10 status2=0x00
> rors
> Successful[   24.524623] gd5f1gq4u_ecc_get_status (138): status2=0x00
> ly corrected 2 bit errors per subpage
> Inserted biterror @ 5/7
> [   24.536350] gd5f1gq4u_ecc_get_status (124): status=0x10 status2=0x00
> Read reported 4 [   24.543391] gd5f1gq4u_ecc_get_status (138): status2=0x00
> corrected bit errors
> Successfully corrected 3 bit errors per su[   24.555245] gd5f1gq4u_ecc_get_status (124): status=0x10 status2=0x10
> bpage
> Inserted [   24.562436] gd5f1gq4u_ecc_get_status (138): status2=0x10
> biterror @ 7/7
> Read reported 4 corrected bit errors
> Successful[   24.574014] gd5f1gq4u_ecc_get_status (124): status=0x10 status2=0x20
> ly corrected 4 b[   24.581360] gd5f1gq4u_ecc_get_status (138): status2=0x20
> it errors per subpage
> Inserted biterror @ 8/7
> Read reported 5 [   24.593042] gd5f1gq4u_ecc_get_status (124): status=0x10 status2=0x30
> corrected bit er[   24.600284] gd5f1gq4u_ecc_get_status (138): status2=0x30
> rors
> Successfully corrected 5 bit errors per subpage
> Inserted [   24.612127] gd5f1gq4u_ecc_get_status (124): status=0x30 status2=0x00
> biterror @ 10/7
> Read reported 6 corrected bit errors
> Successfu[   24.623870] gd5f1gq4u_ecc_get_status (124): status=0x20 status2=0x00
> lly corrected 6 bit errors per subpage
> Inserted biterror @ 12/7
> Read reported 7 corrected bit errors
> Successfully corrected 7 bit errors per subpage
> Inserted biterror @ 14/7
> Read reported 8 corrected bit errors
> Successfully corrected 8 bit errors per subpage
> Inserted biterror @ 17/7
> Failed to recover 1 bitflips
> Read error after 9 bit errors per page
> 
> 
> I removed the debug output from the chip ecc_get_status function for
> a clearer output. Here the same without the debug output:
> 
> root@mt7688:~# ./nandbiterrs /dev/mtd5 -i
> incremental biterrors test
> Successfully corrected 0 bit errors per subpage
> Inserted biterror @ 1/7
> Read reported 4 corrected bit errors
> Successfully corrected 1 bit errors per subpage
> Inserted biterror @ 3/7
> Read reported 4 corrected bit errors
> Successfully corrected 2 bit errors per subpage
> Inserted biterror @ 5/7
> Read reported 4 corrected bit errors
> Successfully corrected 3 bit errors per subpage
> Inserted biterror @ 7/7
> Read reported 4 corrected bit errors
> Successfully corrected 4 bit errors per subpage
> Inserted biterror @ 8/7
> Read reported 5 corrected bit errors
> Successfully corrected 5 bit errors per subpage
> Inserted biterror @ 10/7
> Read reported 6 corrected bit errors
> Successfully corrected 6 bit errors per subpage
> Inserted biterror @ 12/7
> Read reported 7 corrected bit errors
> Successfully corrected 7 bit errors per subpage
> Inserted biterror @ 14/7
> Read reported 8 corrected bit errors
> Successfully corrected 8 bit errors per subpage
> Inserted biterror @ 17/7
> Failed to recover 1 bitflips
> Read error after 9 bit errors per page
> 
> This definitely does look better. I assume that we are we on the
> right track now?

Yep, and it confirms the ECC caps => 8bits/512bytes. Will send a proper
commit for the fix I did and Cc you so you can add your
Tested-by/Reviewed-by.

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