[PATCH 4/4] mtd: nand: raw: atmel: Introduce optional debug commands

[Alexander Dahl]

Hello Mihai,

Am Mon, Mar 18, 2024 at 11:28:39AM +0000 schrieb Mihai.Sain at microchip.com:
> Hi Alexander,
> 
> I'm sorry for quoting because the email is sent from Outlook ☹
> 
> > > U-Boot> nand info
> > >
> > > Device 0: nand0, sector size 256 KiB
> > >   Manufacturer  MACRONIX
> > >   Model         MX30LF4G28AD
> > >   Device size        512 MiB
> > >   Page size         4096 b
> > >   OOB size           256 b
> > >   Erase size      262144 b
> > >   ecc strength         8 bits
> > >   ecc step size      512 b
> > >   subpagesize       4096 b
> > >   options       0x00004200
> > >   bbt options   0x00028000
> >
> > This seems to be the same NAND chip as on the sam9x60 curiosity, but your output has three additional lines, see mine:
> > Do you have some additional patches printing manufacturer, model, and 
> > device size?  I can't see those lines printed in
> > nand_print_and_set_info() here.
> >
> > Yes. I have 😊
> > +     printf("  Manufacturer  %s \n", chip->onfi_params.manufacturer);
> > +     printf("  Model         %s \n", chip->onfi_params.model);
> > +     printf("  Device size   %8d MiB\n", (int)(chip->chipsize >> 20));
> 
> This is nice, and I think it would be valuable to have upstream.
> Maybe you could send a patch for that?
> 	Sure. I will send the patch !
> 
> > > U-Boot> hsmc decode
> > >
> > > mck clock rate: 200000000
> > >
> > > HSMC_SETUP3:    0x00000001
> > > HSMC_PULSE3:    0x07040804
> > > HSMC_CYCLE3:    0x00070008
> > > HSMC_TIMINGS3:  0x880402f2
> > > HSMC_MODE3:     0x001f0003
> > > NCS_RD: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
> > >    NRD: setup: 0 (0 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 7
> > > (35 ns)
> > > NCS_WR: setup: 0 (0 ns), pulse: 8 (40 ns), hold: 0 (0 ns), cycle: 8 (40 ns)
> > >    NWE: setup: 1 (5 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 8
> > > (40 ns) TDF optimization enabled TDF cycles: 15 (75 ns) Data Bus
> > > Width: 8-bit bus NWAIT Mode: 0 Write operation controlled by NWE 
> > > signal Read operation controlled by NRD signal
> >
> > This is also interesting.  Given the mck clock rate is the same as on 
> > sam9x60, I would have guessed the timings set by
> > atmel_smc_nand_prepare_smcconf() should give the same results, both for ONFI timiming mode 3, which is the fastest mode the (H)SMC supports according to comments in the driver.  This is the output with the patch in question applied on next for sam9x60:
> >
> >     U-Boot> hsmc decode
> >
> >     mck clock rate: 200000000
> >
> >     SMC_SETUP3:     0x00000002
> >     SMC_PULSE3:     0x06030703
> >     SMC_CYCLE3:     0x00060007
> >     SMC_MODE3:      0x001f0003
> >     NCS_RD: setup: 0 (0 ns), pulse: 6 (30 ns), hold: 0 (0 ns), cycle: 6 (30 ns)
> >        NRD: setup: 0 (0 ns), pulse: 3 (15 ns), hold: 3 (15 ns), cycle: 6 (30 ns)
> >     NCS_WR: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
> >        NWE: setup: 2 (10 ns), pulse: 3 (15 ns), hold: 2 (10 ns), cycle: 7 (35 ns)
> >     Standard read is applied.
> >     TDF optimization enabled
> >     TDF cycles: 15 (75 ns)
> >     Data Bus Width: 8-bit bus
> >     NWAIT Mode: 0
> >     Write operation controlled by NWE signal
> >     Read operation controlled by NRD signal
> >
> > Notice the pulse times for read are one clock cycle smaller than in your output, and the timings for write are also different.  Do you have changes for atmel_smc_nand_prepare_smcconf() applied which are not upstream yet?  Or is the HSMC on sama7g54 somehow different than on older SoCs?
> >
> > Yes. I force timing mode 2 in nand-controller.c:
> > +     if (conf->timings.sdr.tRC_min < 30001) // force timing mode 2, 
> > + 35ns for read/write cycle
> >
> > This will pass the nand torture test 😊
> >
> > U-Boot> nand torture 0x800000 0x1000000
> >
> > NAND torture: device 0 offset 0x800000 size 0x1000000 (block size 
> > 0x40000)
> >  Passed: 64, failed: 0
> 
> Ah okay.  I have another patch here for manually setting the ONFI timing mode from commandline.  This is probably too late for some scenarios, but it helped me when testing.  If you're interested I could send it to the public.
> 	Yes. I'm very interested !

Okay, I'll add it to v2 of the series then.

> 	Maybe we should add an automatic fallback for timing mode in nand-controller.c 😊
> 	As of now the driver is forcing tRC_min to 30ns (mode 3), which is not reliable for sama7 nfc controller ☹
> 	https://github.com/u-boot/u-boot/blob/master/drivers/mtd/nand/raw/nand_timings.c#L167
> 	The nand torture command helped me to manually force tRC_min to 35ns (mode 2).

This sounds like the same problem encountered in
https://github.com/linux4sam/at91bootstrap/issues/174 and the fix
proposed there works in Linux and U-Boot as well.  I consider the
original commit message of the patch attached to that ticket not easy
to understand however, so I wrote what I think is the problem.  Could
you please test the patch attached to this mail which does the same
thing and should apply to U-Boot cleanly?  I tested that on sam9x60
and sama5, but have no other boards/socs to test with.  If that works
on sama7, I will propose it on U-Boot, too.

(I hope it is okay to attach it as an attachment for now, it's not
ready for submission anyways.)

Greets
Alex

> 	Thanks.
> 
> Greets
> Alex
> 
> >
> > Note: I'm currently testing a patch changing the computation of the read pulse cycles based on a patch for at91bootstrap [1], but that is not applied here for the output quoted above.
> >
> > Greets
> > Alex
> >
> > [1] 
> > https://github.com/linux4sam/at91bootstrap/issues/174#issuecomment-197
> > 0698527
> >
> > >
> > > Best regards,
> > > Mihai Sain
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-mtd-nand-raw-atmel-Port-timing-fix-from-at91bootstra.patch
Type: text/x-diff
Size: 2289 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20240318/a8ad11fb/attachment.patch>