diff mbox

mtd: nand: atmel: probe of Spansion S34ML02G1 fails

Message ID 20170728211301.10d25227@bbrezillon
State Not Applicable
Headers show

Commit Message

Boris Brezillon July 28, 2017, 7:13 p.m. UTC 
Le Fri, 28 Jul 2017 15:15:25 +0200,
Alexander Dahl <ada@thorsis.com> a écrit :

> Hello Boris,
> 
> Am Mittwoch, 26. Juli 2017, 20:33:26 schrieb Boris Brezillon:
> > Okay, first thing to test: revert f9ce2eddf176 ("mtd: nand: atmel: Add  
> > ->setup_data_interface() hooks"). You may have to revert other  
> > commits to compile the driver.
> > 
> > If it works, I'd like you to drop SMC timings in both situations (with
> > and without this commit).
> > 
> > I'd also like to to dump NAND timings (those passed to  
> > ->setup_data_interface()).  
> 
> So based on those three atmel-ebi patches from earlier this week I used 
> the attached patch to dump the nand timings and smc settings before the 
> get applied. clk mck runs at 132096000 Hz (slow_xtal 32768, main_xtal 
> 18432000). Previously clock settings for NAND were set in (very old) U-
> Boot to this (double checked, Linux didn't touch it):
> 
> 	U-Boot> md.l 0xffffef1c 1
> 	ffffef1c: 0001000a    ....
> 	U-Boot> md.l 0xffffec30 4
> 	ffffec30: 00010001 03030303 00050005 00020003    ................
> 
> So setup, pulse, cycle, tdf all to quite low number of clock cycles, but 
> as far as I can tell this worked with all flash chips we had connected 
> over the years.
> 
> Now with the old Hynix flash chip, I get the following:
> 
> 	atmel-nand-controller 10000000.ebi:nand-controller: atmel_smc_nand_setup_data_interface(csline: 0, type: 0)
> 	atmel-nand-controller 10000000.ebi:nand-controller: tBERS_max: 0, tCCS_min: 500000, tPROG_max: 0, tR_max: 200000000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tALH_min: 20000, tADL_min: 400000, tALS_min: 50000, tAR_min: 25000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tCEA_max: 100000, tCEH_min: 20000, tCH_min: 20000, tCHZ_max: 100000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tCLH_min: 20000, tCLR_min: 20000, tCLS_min: 50000, tCOH_min: 0
> 	atmel-nand-controller 10000000.ebi:nand-controller: tCS_min: 70000, tDH_min: 20000, tDS_min: 40000, tFEAT_max: 1000000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tIR_min: 10000, tITC_max: 1000000, tRC_min: 100000, tREA_max: 40000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tREH_min: 30000, tRHOH_min: 0, tRHW_min: 200000, tRHZ_max: 200000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tRLOH_min: 0, tRP_min: 50000, tRR_min: 40000, tRST_max: 250000000000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tWB_max: 200000, tWC_min: 100000, tWH_min: 30000, tWHR_min: 120000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tWP_min: 50000, tWW_min: 100000
> 	atmel-nand-controller 10000000.ebi:nand-controller: smcconf: setup: 0x00000002, pulse: 0x0f080f08, cycle: 0x000f000f, timings: 0x88060483, mode: 0x001f0003
> 	nand: Could not find valid ONFI parameter page; aborting
> 	nand: device found, Manufacturer ID: 0xad, Chip ID: 0xda
> 	nand: Hynix NAND 256MiB 3,3V 8-bit
> 	nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
> 
> 	$ devmem 0xffffef1c
> 	0x0001000A
> 	$ devmem 0xffffec30
> 	0x00000002
> 	$ devmem 0xffffec34
> 	0x0F080F08
> 	$ devmem 0xffffec38
> 	0x000F000F
> 	$ devmem 0xffffec3c
> 	0x001F0003
> 
> To me those values look like some kind of defaults every nand chip should run with?
> 
> The failing Spansion chip here:
> 
> 	atmel-nand-controller 10000000.ebi:nand-controller: atmel_smc_nand_setup_data_interface(csline: 0, type: 0)
> 	atmel-nand-controller 10000000.ebi:nand-controller: tBERS_max: 0, tCCS_min: 500000, tPROG_max: 0, tR_max: 200000000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tALH_min: 20000, tADL_min: 400000, tALS_min: 50000, tAR_min: 25000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tCEA_max: 100000, tCEH_min: 20000, tCH_min: 20000, tCHZ_max: 100000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tCLH_min: 20000, tCLR_min: 20000, tCLS_min: 50000, tCOH_min: 0
> 	atmel-nand-controller 10000000.ebi:nand-controller: tCS_min: 70000, tDH_min: 20000, tDS_min: 40000, tFEAT_max: 1000000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tIR_min: 10000, tITC_max: 1000000, tRC_min: 100000, tREA_max: 40000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tREH_min: 30000, tRHOH_min: 0, tRHW_min: 200000, tRHZ_max: 200000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tRLOH_min: 0, tRP_min: 50000, tRR_min: 40000, tRST_max: 250000000000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tWB_max: 200000, tWC_min: 100000, tWH_min: 30000, tWHR_min: 120000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tWP_min: 50000, tWW_min: 100000
> 	atmel-nand-controller 10000000.ebi:nand-controller: smcconf: setup: 0x00000002, pulse: 0x0f080f08, cycle: 0x000f000f, timings: 0x88060483, mode: 0x001f0003
> 	nand: device found, Manufacturer ID: 0x01, Chip ID: 0xda
> 	nand: AMD/Spansion S34ML02G1
> 	nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
> 	atmel-nand-controller 10000000.ebi:nand-controller: atmel_smc_nand_setup_data_interface(csline: -1, type: 0)
> 	atmel-nand-controller 10000000.ebi:nand-controller: tBERS_max: 1410065408, tCCS_min: 100000, tPROG_max: 700000000, tR_max: 25000000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tALH_min: 5000, tADL_min: 400000, tALS_min: 10000, tAR_min: 10000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tCEA_max: 25000, tCEH_min: 20000, tCH_min: 5000, tCHZ_max: 30000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tCLH_min: 5000, tCLR_min: 10000, tCLS_min: 10000, tCOH_min: 15000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tCS_min: 20000, tDH_min: 5000, tDS_min: 10000, tFEAT_max: 1000000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tIR_min: 0, tITC_max: 1000000, tRC_min: 25000, tREA_max: 20000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tREH_min: 10000, tRHOH_min: 15000, tRHW_min: 100000, tRHZ_max: 100000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tRLOH_min: 5000, tRP_min: 12000, tRR_min: 20000, tRST_max: 500000000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tWB_max: 100000, tWC_min: 25000, tWH_min: 10000, tWHR_min: 80000
> 	atmel-nand-controller 10000000.ebi:nand-controller: tWP_min: 12000, tWW_min: 100000
> 	atmel-nand-controller 10000000.ebi:nand-controller: smcconf: setup: 0x00000001, pulse: 0x05020502, cycle: 0x00050005, timings: 0x88030282, mode: 0x001b0003


Hm, ok. It seems that we're accepting mode 4 even though the controller
is not supporting EDO mode which mandatory for mode 4 and 5.

Can you try with the following patch? Timings are expressed in picoseconds
and this test was testing against a value expressed in nanoseconds.  

BTW, did you try to comment the line I pointed out yesterday? I'd like
to be sure this is a timing issue.

--->8---

Comments

Alexander Dahl July 31, 2017, 6:29 a.m. UTC | #1 
Hello Boris,

Am Freitag, 28. Juli 2017, 21:13:01 schrieb Boris Brezillon:
> Can you try with the following patch? Timings are expressed in
> picoseconds and this test was testing against a value expressed in
> nanoseconds.
> 
> BTW, did you try to comment the line I pointed out yesterday? I'd like
> to be sure this is a timing issue.

Now I did, see my other mail.

> --->8---
> diff --git a/drivers/mtd/nand/atmel/nand-controller.c
> b/drivers/mtd/nand/atmel/nand-controller.c index
> d922a88e407f..2c8baa0c2c4e 100644
> --- a/drivers/mtd/nand/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/atmel/nand-controller.c
> @@ -1201,7 +1201,7 @@ static int atmel_smc_nand_prepare_smcconf(struct
> atmel_nand *nand, * tRC < 30ns implies EDO mode. This controller does
> not support this * mode.
>          */
> -       if (conf->timings.sdr.tRC_min < 30)
> +       if (conf->timings.sdr.tRC_min < 30000)
>                 return -ENOTSUPP;
> 
>         atmel_smc_cs_conf_init(smcconf);

This gives me the following debug output (with my own debug messages 
from the patch in one mail from last week, with an additional dump of 
the values read from onfi page):

atmel-nand-controller 10000000.ebi:nand-controller: atmel_smc_nand_setup_data_interface(csline: 0, type: 0)
atmel-nand-controller 10000000.ebi:nand-controller: tBERS_max: 0, tCCS_min: 500000, tPROG_max: 0, tR_max: 200000000
atmel-nand-controller 10000000.ebi:nand-controller: tALH_min: 20000, tADL_min: 400000, tALS_min: 50000, tAR_min: 25000
atmel-nand-controller 10000000.ebi:nand-controller: tCEA_max: 100000, tCEH_min: 20000, tCH_min: 20000, tCHZ_max: 100000
atmel-nand-controller 10000000.ebi:nand-controller: tCLH_min: 20000, tCLR_min: 20000, tCLS_min: 50000, tCOH_min: 0
atmel-nand-controller 10000000.ebi:nand-controller: tCS_min: 70000, tDH_min: 20000, tDS_min: 40000, tFEAT_max: 1000000
atmel-nand-controller 10000000.ebi:nand-controller: tIR_min: 10000, tITC_max: 1000000, tRC_min: 100000, tREA_max: 40000
atmel-nand-controller 10000000.ebi:nand-controller: tREH_min: 30000, tRHOH_min: 0, tRHW_min: 200000, tRHZ_max: 200000
atmel-nand-controller 10000000.ebi:nand-controller: tRLOH_min: 0, tRP_min: 50000, tRR_min: 40000, tRST_max: 250000000000
atmel-nand-controller 10000000.ebi:nand-controller: tWB_max: 200000, tWC_min: 100000, tWH_min: 30000, tWHR_min: 120000
atmel-nand-controller 10000000.ebi:nand-controller: tWP_min: 50000, tWW_min: 100000
atmel-nand-controller 10000000.ebi:nand-controller: smcconf: setup: 0x00000002, pulse: 0x0f080f08, cycle: 0x000f000f, timings: 0x88060483, mode: 0x001f0003
nand: device found, Manufacturer ID: 0x01, Chip ID: 0xda
nand: AMD/Spansion S34ML02G1
nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
onfi timings: t_prog: 0x02BC, t_bers: 0x2710, t_r: 0x0019, t_ccs: 0x0064
atmel-nand-controller 10000000.ebi:nand-controller: atmel_smc_nand_setup_data_interface(csline: -1, type: 0)
atmel-nand-controller 10000000.ebi:nand-controller: tBERS_max: 1410065408, tCCS_min: 100000, tPROG_max: 700000000, tR_max: 25000000
atmel-nand-controller 10000000.ebi:nand-controller: tALH_min: 5000, tADL_min: 400000, tALS_min: 10000, tAR_min: 10000
atmel-nand-controller 10000000.ebi:nand-controller: tCEA_max: 25000, tCEH_min: 20000, tCH_min: 5000, tCHZ_max: 30000
atmel-nand-controller 10000000.ebi:nand-controller: tCLH_min: 5000, tCLR_min: 10000, tCLS_min: 10000, tCOH_min: 15000
atmel-nand-controller 10000000.ebi:nand-controller: tCS_min: 20000, tDH_min: 5000, tDS_min: 10000, tFEAT_max: 1000000
atmel-nand-controller 10000000.ebi:nand-controller: tIR_min: 0, tITC_max: 1000000, tRC_min: 25000, tREA_max: 20000
atmel-nand-controller 10000000.ebi:nand-controller: tREH_min: 10000, tRHOH_min: 15000, tRHW_min: 100000, tRHZ_max: 100000
atmel-nand-controller 10000000.ebi:nand-controller: tRLOH_min: 5000, tRP_min: 12000, tRR_min: 20000, tRST_max: 500000000
atmel-nand-controller 10000000.ebi:nand-controller: tWB_max: 100000, tWC_min: 25000, tWH_min: 10000, tWHR_min: 80000
atmel-nand-controller 10000000.ebi:nand-controller: tWP_min: 12000, tWW_min: 100000
atmel-nand-controller 10000000.ebi:nand-controller: atmel_smc_nand_prepare_smcconf() failed: -524
onfi timings: t_prog: 0x02BC, t_bers: 0x2710, t_r: 0x0019, t_ccs: 0x0064
atmel-nand-controller 10000000.ebi:nand-controller: atmel_smc_nand_setup_data_interface(csline: -1, type: 0)
atmel-nand-controller 10000000.ebi:nand-controller: tBERS_max: 1410065408, tCCS_min: 100000, tPROG_max: 700000000, tR_max: 25000000
atmel-nand-controller 10000000.ebi:nand-controller: tALH_min: 5000, tADL_min: 400000, tALS_min: 10000, tAR_min: 10000
atmel-nand-controller 10000000.ebi:nand-controller: tCEA_max: 25000, tCEH_min: 20000, tCH_min: 5000, tCHZ_max: 50000
atmel-nand-controller 10000000.ebi:nand-controller: tCLH_min: 5000, tCLR_min: 10000, tCLS_min: 10000, tCOH_min: 15000
atmel-nand-controller 10000000.ebi:nand-controller: tCS_min: 25000, tDH_min: 5000, tDS_min: 10000, tFEAT_max: 1000000
atmel-nand-controller 10000000.ebi:nand-controller: tIR_min: 0, tITC_max: 1000000, tRC_min: 30000, tREA_max: 20000
atmel-nand-controller 10000000.ebi:nand-controller: tREH_min: 10000, tRHOH_min: 15000, tRHW_min: 100000, tRHZ_max: 100000
atmel-nand-controller 10000000.ebi:nand-controller: tRLOH_min: 0, tRP_min: 15000, tRR_min: 20000, tRST_max: 500000000
atmel-nand-controller 10000000.ebi:nand-controller: tWB_max: 100000, tWC_min: 30000, tWH_min: 10000, tWHR_min: 80000
atmel-nand-controller 10000000.ebi:nand-controller: tWP_min: 15000, tWW_min: 100000
atmel-nand-controller 10000000.ebi:nand-controller: smcconf: setup: 0x00000001, pulse: 0x06030603, cycle: 0x00060006, timings: 0x88030282, mode: 0x001b0003
atmel-nand-controller 10000000.ebi:nand-controller: atmel_smc_nand_setup_data_interface(csline: 0, type: 0)
atmel-nand-controller 10000000.ebi:nand-controller: tBERS_max: 1410065408, tCCS_min: 100000, tPROG_max: 700000000, tR_max: 25000000
atmel-nand-controller 10000000.ebi:nand-controller: tALH_min: 5000, tADL_min: 400000, tALS_min: 10000, tAR_min: 10000
atmel-nand-controller 10000000.ebi:nand-controller: tCEA_max: 25000, tCEH_min: 20000, tCH_min: 5000, tCHZ_max: 50000
atmel-nand-controller 10000000.ebi:nand-controller: tCLH_min: 5000, tCLR_min: 10000, tCLS_min: 10000, tCOH_min: 15000
atmel-nand-controller 10000000.ebi:nand-controller: tCS_min: 25000, tDH_min: 5000, tDS_min: 10000, tFEAT_max: 1000000
atmel-nand-controller 10000000.ebi:nand-controller: tIR_min: 0, tITC_max: 1000000, tRC_min: 30000, tREA_max: 20000
atmel-nand-controller 10000000.ebi:nand-controller: tREH_min: 10000, tRHOH_min: 15000, tRHW_min: 100000, tRHZ_max: 100000
atmel-nand-controller 10000000.ebi:nand-controller: tRLOH_min: 0, tRP_min: 15000, tRR_min: 20000, tRST_max: 500000000
atmel-nand-controller 10000000.ebi:nand-controller: tWB_max: 100000, tWC_min: 30000, tWH_min: 10000, tWHR_min: 80000
atmel-nand-controller 10000000.ebi:nand-controller: tWP_min: 15000, tWW_min: 100000
atmel-nand-controller 10000000.ebi:nand-controller: smcconf: setup: 0x00000001, pulse: 0x06030603, cycle: 0x00060006, timings: 0x88030282, mode: 0x001b0003

So, the system boots again, meaning it can successfully read from the 
ubifs. The resulting timings are the same as in debug output:

$ devmem 0xffffec30
0x00000001
$ devmem 0xffffec34
0x06030603
$ devmem 0xffffec38
0x00060006
$ devmem 0xffffec3c
0x001B0003

One thing I noticed is the tBERS_max value. From the 0x2710 in 
the onfi page I would assume 10000 µs, so 10.000.000 ns, so 
10.000.000.000 ps, which does not fit in an u32 anymore, which gives 
those 1410065408 due to integer overflow. I didn't check the other 
timings for possible overflow, maybe you can do this?

Greets
Alex
Boris Brezillon July 31, 2017, 7:44 a.m. UTC | #2 
Hi Alexander,

On Mon, 31 Jul 2017 08:29:35 +0200
Alexander Dahl <ada@thorsis.com> wrote:

> Hello Boris,
> 
> Am Freitag, 28. Juli 2017, 21:13:01 schrieb Boris Brezillon:
> > Can you try with the following patch? Timings are expressed in
> > picoseconds and this test was testing against a value expressed in
> > nanoseconds.
> > 
> > BTW, did you try to comment the line I pointed out yesterday? I'd like
> > to be sure this is a timing issue.  
> 
> Now I did, see my other mail.
> 
> > --->8---  
> > diff --git a/drivers/mtd/nand/atmel/nand-controller.c
> > b/drivers/mtd/nand/atmel/nand-controller.c index
> > d922a88e407f..2c8baa0c2c4e 100644
> > --- a/drivers/mtd/nand/atmel/nand-controller.c
> > +++ b/drivers/mtd/nand/atmel/nand-controller.c
> > @@ -1201,7 +1201,7 @@ static int atmel_smc_nand_prepare_smcconf(struct
> > atmel_nand *nand, * tRC < 30ns implies EDO mode. This controller does
> > not support this * mode.
> >          */
> > -       if (conf->timings.sdr.tRC_min < 30)
> > +       if (conf->timings.sdr.tRC_min < 30000)
> >                 return -ENOTSUPP;
> > 
> >         atmel_smc_cs_conf_init(smcconf);  
> 
> This gives me the following debug output (with my own debug messages 
> from the patch in one mail from last week, with an additional dump of 
> the values read from onfi page):
> 
> atmel-nand-controller 10000000.ebi:nand-controller: atmel_smc_nand_setup_data_interface(csline: 0, type: 0)
> atmel-nand-controller 10000000.ebi:nand-controller: tBERS_max: 0, tCCS_min: 500000, tPROG_max: 0, tR_max: 200000000
> atmel-nand-controller 10000000.ebi:nand-controller: tALH_min: 20000, tADL_min: 400000, tALS_min: 50000, tAR_min: 25000
> atmel-nand-controller 10000000.ebi:nand-controller: tCEA_max: 100000, tCEH_min: 20000, tCH_min: 20000, tCHZ_max: 100000
> atmel-nand-controller 10000000.ebi:nand-controller: tCLH_min: 20000, tCLR_min: 20000, tCLS_min: 50000, tCOH_min: 0
> atmel-nand-controller 10000000.ebi:nand-controller: tCS_min: 70000, tDH_min: 20000, tDS_min: 40000, tFEAT_max: 1000000
> atmel-nand-controller 10000000.ebi:nand-controller: tIR_min: 10000, tITC_max: 1000000, tRC_min: 100000, tREA_max: 40000
> atmel-nand-controller 10000000.ebi:nand-controller: tREH_min: 30000, tRHOH_min: 0, tRHW_min: 200000, tRHZ_max: 200000
> atmel-nand-controller 10000000.ebi:nand-controller: tRLOH_min: 0, tRP_min: 50000, tRR_min: 40000, tRST_max: 250000000000
> atmel-nand-controller 10000000.ebi:nand-controller: tWB_max: 200000, tWC_min: 100000, tWH_min: 30000, tWHR_min: 120000
> atmel-nand-controller 10000000.ebi:nand-controller: tWP_min: 50000, tWW_min: 100000
> atmel-nand-controller 10000000.ebi:nand-controller: smcconf: setup: 0x00000002, pulse: 0x0f080f08, cycle: 0x000f000f, timings: 0x88060483, mode: 0x001f0003
> nand: device found, Manufacturer ID: 0x01, Chip ID: 0xda
> nand: AMD/Spansion S34ML02G1
> nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
> onfi timings: t_prog: 0x02BC, t_bers: 0x2710, t_r: 0x0019, t_ccs: 0x0064
> atmel-nand-controller 10000000.ebi:nand-controller: atmel_smc_nand_setup_data_interface(csline: -1, type: 0)
> atmel-nand-controller 10000000.ebi:nand-controller: tBERS_max: 1410065408, tCCS_min: 100000, tPROG_max: 700000000, tR_max: 25000000
> atmel-nand-controller 10000000.ebi:nand-controller: tALH_min: 5000, tADL_min: 400000, tALS_min: 10000, tAR_min: 10000
> atmel-nand-controller 10000000.ebi:nand-controller: tCEA_max: 25000, tCEH_min: 20000, tCH_min: 5000, tCHZ_max: 30000
> atmel-nand-controller 10000000.ebi:nand-controller: tCLH_min: 5000, tCLR_min: 10000, tCLS_min: 10000, tCOH_min: 15000
> atmel-nand-controller 10000000.ebi:nand-controller: tCS_min: 20000, tDH_min: 5000, tDS_min: 10000, tFEAT_max: 1000000
> atmel-nand-controller 10000000.ebi:nand-controller: tIR_min: 0, tITC_max: 1000000, tRC_min: 25000, tREA_max: 20000
> atmel-nand-controller 10000000.ebi:nand-controller: tREH_min: 10000, tRHOH_min: 15000, tRHW_min: 100000, tRHZ_max: 100000
> atmel-nand-controller 10000000.ebi:nand-controller: tRLOH_min: 5000, tRP_min: 12000, tRR_min: 20000, tRST_max: 500000000
> atmel-nand-controller 10000000.ebi:nand-controller: tWB_max: 100000, tWC_min: 25000, tWH_min: 10000, tWHR_min: 80000
> atmel-nand-controller 10000000.ebi:nand-controller: tWP_min: 12000, tWW_min: 100000
> atmel-nand-controller 10000000.ebi:nand-controller: atmel_smc_nand_prepare_smcconf() failed: -524
> onfi timings: t_prog: 0x02BC, t_bers: 0x2710, t_r: 0x0019, t_ccs: 0x0064
> atmel-nand-controller 10000000.ebi:nand-controller: atmel_smc_nand_setup_data_interface(csline: -1, type: 0)
> atmel-nand-controller 10000000.ebi:nand-controller: tBERS_max: 1410065408, tCCS_min: 100000, tPROG_max: 700000000, tR_max: 25000000
> atmel-nand-controller 10000000.ebi:nand-controller: tALH_min: 5000, tADL_min: 400000, tALS_min: 10000, tAR_min: 10000
> atmel-nand-controller 10000000.ebi:nand-controller: tCEA_max: 25000, tCEH_min: 20000, tCH_min: 5000, tCHZ_max: 50000
> atmel-nand-controller 10000000.ebi:nand-controller: tCLH_min: 5000, tCLR_min: 10000, tCLS_min: 10000, tCOH_min: 15000
> atmel-nand-controller 10000000.ebi:nand-controller: tCS_min: 25000, tDH_min: 5000, tDS_min: 10000, tFEAT_max: 1000000
> atmel-nand-controller 10000000.ebi:nand-controller: tIR_min: 0, tITC_max: 1000000, tRC_min: 30000, tREA_max: 20000
> atmel-nand-controller 10000000.ebi:nand-controller: tREH_min: 10000, tRHOH_min: 15000, tRHW_min: 100000, tRHZ_max: 100000
> atmel-nand-controller 10000000.ebi:nand-controller: tRLOH_min: 0, tRP_min: 15000, tRR_min: 20000, tRST_max: 500000000
> atmel-nand-controller 10000000.ebi:nand-controller: tWB_max: 100000, tWC_min: 30000, tWH_min: 10000, tWHR_min: 80000
> atmel-nand-controller 10000000.ebi:nand-controller: tWP_min: 15000, tWW_min: 100000
> atmel-nand-controller 10000000.ebi:nand-controller: smcconf: setup: 0x00000001, pulse: 0x06030603, cycle: 0x00060006, timings: 0x88030282, mode: 0x001b0003
> atmel-nand-controller 10000000.ebi:nand-controller: atmel_smc_nand_setup_data_interface(csline: 0, type: 0)
> atmel-nand-controller 10000000.ebi:nand-controller: tBERS_max: 1410065408, tCCS_min: 100000, tPROG_max: 700000000, tR_max: 25000000
> atmel-nand-controller 10000000.ebi:nand-controller: tALH_min: 5000, tADL_min: 400000, tALS_min: 10000, tAR_min: 10000
> atmel-nand-controller 10000000.ebi:nand-controller: tCEA_max: 25000, tCEH_min: 20000, tCH_min: 5000, tCHZ_max: 50000
> atmel-nand-controller 10000000.ebi:nand-controller: tCLH_min: 5000, tCLR_min: 10000, tCLS_min: 10000, tCOH_min: 15000
> atmel-nand-controller 10000000.ebi:nand-controller: tCS_min: 25000, tDH_min: 5000, tDS_min: 10000, tFEAT_max: 1000000
> atmel-nand-controller 10000000.ebi:nand-controller: tIR_min: 0, tITC_max: 1000000, tRC_min: 30000, tREA_max: 20000
> atmel-nand-controller 10000000.ebi:nand-controller: tREH_min: 10000, tRHOH_min: 15000, tRHW_min: 100000, tRHZ_max: 100000
> atmel-nand-controller 10000000.ebi:nand-controller: tRLOH_min: 0, tRP_min: 15000, tRR_min: 20000, tRST_max: 500000000
> atmel-nand-controller 10000000.ebi:nand-controller: tWB_max: 100000, tWC_min: 30000, tWH_min: 10000, tWHR_min: 80000
> atmel-nand-controller 10000000.ebi:nand-controller: tWP_min: 15000, tWW_min: 100000
> atmel-nand-controller 10000000.ebi:nand-controller: smcconf: setup: 0x00000001, pulse: 0x06030603, cycle: 0x00060006, timings: 0x88030282, mode: 0x001b0003
> 
> So, the system boots again, meaning it can successfully read from the 
> ubifs.

Cool. I'll send a patch to fix that.

> The resulting timings are the same as in debug output:
> 
> $ devmem 0xffffec30
> 0x00000001
> $ devmem 0xffffec34
> 0x06030603
> $ devmem 0xffffec38
> 0x00060006
> $ devmem 0xffffec3c
> 0x001B0003
> 
> One thing I noticed is the tBERS_max value. From the 0x2710 in 
> the onfi page I would assume 10000 µs, so 10.000.000 ns, so 
> 10.000.000.000 ps, which does not fit in an u32 anymore, which gives 
> those 1410065408 due to integer overflow.

Indeed, it should be an u64 not an u32.

> I didn't check the other 
> timings for possible overflow, maybe you can do this?

Yep, we have the same problem with tR_max and tPROG_max.

I'll send a patch to fix that.

Thanks,

Boris
Boris Brezillon July 31, 2017, 8:41 a.m. UTC | #3 
On Mon, 31 Jul 2017 09:44:43 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> Hi Alexander,
> 
> On Mon, 31 Jul 2017 08:29:35 +0200
> Alexander Dahl <ada@thorsis.com> wrote:
> 
> > Hello Boris,
> > 
> > Am Freitag, 28. Juli 2017, 21:13:01 schrieb Boris Brezillon:  
> > > Can you try with the following patch? Timings are expressed in
> > > picoseconds and this test was testing against a value expressed in
> > > nanoseconds.
> > > 
> > > BTW, did you try to comment the line I pointed out yesterday? I'd like
> > > to be sure this is a timing issue.    
> > 
> > Now I did, see my other mail.
> >   
> > > --->8---    
> > > diff --git a/drivers/mtd/nand/atmel/nand-controller.c
> > > b/drivers/mtd/nand/atmel/nand-controller.c index
> > > d922a88e407f..2c8baa0c2c4e 100644
> > > --- a/drivers/mtd/nand/atmel/nand-controller.c
> > > +++ b/drivers/mtd/nand/atmel/nand-controller.c
> > > @@ -1201,7 +1201,7 @@ static int atmel_smc_nand_prepare_smcconf(struct
> > > atmel_nand *nand, * tRC < 30ns implies EDO mode. This controller does
> > > not support this * mode.
> > >          */
> > > -       if (conf->timings.sdr.tRC_min < 30)
> > > +       if (conf->timings.sdr.tRC_min < 30000)
> > >                 return -ENOTSUPP;
> > > 
> > >         atmel_smc_cs_conf_init(smcconf);    
> > 
> > This gives me the following debug output (with my own debug messages 
> > from the patch in one mail from last week, with an additional dump of 
> > the values read from onfi page):
> > 
> > atmel-nand-controller 10000000.ebi:nand-controller: atmel_smc_nand_setup_data_interface(csline: 0, type: 0)
> > atmel-nand-controller 10000000.ebi:nand-controller: tBERS_max: 0, tCCS_min: 500000, tPROG_max: 0, tR_max: 200000000
> > atmel-nand-controller 10000000.ebi:nand-controller: tALH_min: 20000, tADL_min: 400000, tALS_min: 50000, tAR_min: 25000
> > atmel-nand-controller 10000000.ebi:nand-controller: tCEA_max: 100000, tCEH_min: 20000, tCH_min: 20000, tCHZ_max: 100000
> > atmel-nand-controller 10000000.ebi:nand-controller: tCLH_min: 20000, tCLR_min: 20000, tCLS_min: 50000, tCOH_min: 0
> > atmel-nand-controller 10000000.ebi:nand-controller: tCS_min: 70000, tDH_min: 20000, tDS_min: 40000, tFEAT_max: 1000000
> > atmel-nand-controller 10000000.ebi:nand-controller: tIR_min: 10000, tITC_max: 1000000, tRC_min: 100000, tREA_max: 40000
> > atmel-nand-controller 10000000.ebi:nand-controller: tREH_min: 30000, tRHOH_min: 0, tRHW_min: 200000, tRHZ_max: 200000
> > atmel-nand-controller 10000000.ebi:nand-controller: tRLOH_min: 0, tRP_min: 50000, tRR_min: 40000, tRST_max: 250000000000
> > atmel-nand-controller 10000000.ebi:nand-controller: tWB_max: 200000, tWC_min: 100000, tWH_min: 30000, tWHR_min: 120000
> > atmel-nand-controller 10000000.ebi:nand-controller: tWP_min: 50000, tWW_min: 100000
> > atmel-nand-controller 10000000.ebi:nand-controller: smcconf: setup: 0x00000002, pulse: 0x0f080f08, cycle: 0x000f000f, timings: 0x88060483, mode: 0x001f0003
> > nand: device found, Manufacturer ID: 0x01, Chip ID: 0xda
> > nand: AMD/Spansion S34ML02G1
> > nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
> > onfi timings: t_prog: 0x02BC, t_bers: 0x2710, t_r: 0x0019, t_ccs: 0x0064
> > atmel-nand-controller 10000000.ebi:nand-controller: atmel_smc_nand_setup_data_interface(csline: -1, type: 0)
> > atmel-nand-controller 10000000.ebi:nand-controller: tBERS_max: 1410065408, tCCS_min: 100000, tPROG_max: 700000000, tR_max: 25000000
> > atmel-nand-controller 10000000.ebi:nand-controller: tALH_min: 5000, tADL_min: 400000, tALS_min: 10000, tAR_min: 10000
> > atmel-nand-controller 10000000.ebi:nand-controller: tCEA_max: 25000, tCEH_min: 20000, tCH_min: 5000, tCHZ_max: 30000
> > atmel-nand-controller 10000000.ebi:nand-controller: tCLH_min: 5000, tCLR_min: 10000, tCLS_min: 10000, tCOH_min: 15000
> > atmel-nand-controller 10000000.ebi:nand-controller: tCS_min: 20000, tDH_min: 5000, tDS_min: 10000, tFEAT_max: 1000000
> > atmel-nand-controller 10000000.ebi:nand-controller: tIR_min: 0, tITC_max: 1000000, tRC_min: 25000, tREA_max: 20000
> > atmel-nand-controller 10000000.ebi:nand-controller: tREH_min: 10000, tRHOH_min: 15000, tRHW_min: 100000, tRHZ_max: 100000
> > atmel-nand-controller 10000000.ebi:nand-controller: tRLOH_min: 5000, tRP_min: 12000, tRR_min: 20000, tRST_max: 500000000
> > atmel-nand-controller 10000000.ebi:nand-controller: tWB_max: 100000, tWC_min: 25000, tWH_min: 10000, tWHR_min: 80000
> > atmel-nand-controller 10000000.ebi:nand-controller: tWP_min: 12000, tWW_min: 100000
> > atmel-nand-controller 10000000.ebi:nand-controller: atmel_smc_nand_prepare_smcconf() failed: -524
> > onfi timings: t_prog: 0x02BC, t_bers: 0x2710, t_r: 0x0019, t_ccs: 0x0064
> > atmel-nand-controller 10000000.ebi:nand-controller: atmel_smc_nand_setup_data_interface(csline: -1, type: 0)
> > atmel-nand-controller 10000000.ebi:nand-controller: tBERS_max: 1410065408, tCCS_min: 100000, tPROG_max: 700000000, tR_max: 25000000
> > atmel-nand-controller 10000000.ebi:nand-controller: tALH_min: 5000, tADL_min: 400000, tALS_min: 10000, tAR_min: 10000
> > atmel-nand-controller 10000000.ebi:nand-controller: tCEA_max: 25000, tCEH_min: 20000, tCH_min: 5000, tCHZ_max: 50000
> > atmel-nand-controller 10000000.ebi:nand-controller: tCLH_min: 5000, tCLR_min: 10000, tCLS_min: 10000, tCOH_min: 15000
> > atmel-nand-controller 10000000.ebi:nand-controller: tCS_min: 25000, tDH_min: 5000, tDS_min: 10000, tFEAT_max: 1000000
> > atmel-nand-controller 10000000.ebi:nand-controller: tIR_min: 0, tITC_max: 1000000, tRC_min: 30000, tREA_max: 20000
> > atmel-nand-controller 10000000.ebi:nand-controller: tREH_min: 10000, tRHOH_min: 15000, tRHW_min: 100000, tRHZ_max: 100000
> > atmel-nand-controller 10000000.ebi:nand-controller: tRLOH_min: 0, tRP_min: 15000, tRR_min: 20000, tRST_max: 500000000
> > atmel-nand-controller 10000000.ebi:nand-controller: tWB_max: 100000, tWC_min: 30000, tWH_min: 10000, tWHR_min: 80000
> > atmel-nand-controller 10000000.ebi:nand-controller: tWP_min: 15000, tWW_min: 100000
> > atmel-nand-controller 10000000.ebi:nand-controller: smcconf: setup: 0x00000001, pulse: 0x06030603, cycle: 0x00060006, timings: 0x88030282, mode: 0x001b0003
> > atmel-nand-controller 10000000.ebi:nand-controller: atmel_smc_nand_setup_data_interface(csline: 0, type: 0)
> > atmel-nand-controller 10000000.ebi:nand-controller: tBERS_max: 1410065408, tCCS_min: 100000, tPROG_max: 700000000, tR_max: 25000000
> > atmel-nand-controller 10000000.ebi:nand-controller: tALH_min: 5000, tADL_min: 400000, tALS_min: 10000, tAR_min: 10000
> > atmel-nand-controller 10000000.ebi:nand-controller: tCEA_max: 25000, tCEH_min: 20000, tCH_min: 5000, tCHZ_max: 50000
> > atmel-nand-controller 10000000.ebi:nand-controller: tCLH_min: 5000, tCLR_min: 10000, tCLS_min: 10000, tCOH_min: 15000
> > atmel-nand-controller 10000000.ebi:nand-controller: tCS_min: 25000, tDH_min: 5000, tDS_min: 10000, tFEAT_max: 1000000
> > atmel-nand-controller 10000000.ebi:nand-controller: tIR_min: 0, tITC_max: 1000000, tRC_min: 30000, tREA_max: 20000
> > atmel-nand-controller 10000000.ebi:nand-controller: tREH_min: 10000, tRHOH_min: 15000, tRHW_min: 100000, tRHZ_max: 100000
> > atmel-nand-controller 10000000.ebi:nand-controller: tRLOH_min: 0, tRP_min: 15000, tRR_min: 20000, tRST_max: 500000000
> > atmel-nand-controller 10000000.ebi:nand-controller: tWB_max: 100000, tWC_min: 30000, tWH_min: 10000, tWHR_min: 80000
> > atmel-nand-controller 10000000.ebi:nand-controller: tWP_min: 15000, tWW_min: 100000
> > atmel-nand-controller 10000000.ebi:nand-controller: smcconf: setup: 0x00000001, pulse: 0x06030603, cycle: 0x00060006, timings: 0x88030282, mode: 0x001b0003
> > 
> > So, the system boots again, meaning it can successfully read from the 
> > ubifs.  
> 
> Cool. I'll send a patch to fix that.
> 
> > The resulting timings are the same as in debug output:
> > 
> > $ devmem 0xffffec30
> > 0x00000001
> > $ devmem 0xffffec34
> > 0x06030603
> > $ devmem 0xffffec38
> > 0x00060006
> > $ devmem 0xffffec3c
> > 0x001B0003
> > 
> > One thing I noticed is the tBERS_max value. From the 0x2710 in 
> > the onfi page I would assume 10000 µs, so 10.000.000 ns, so 
> > 10.000.000.000 ps, which does not fit in an u32 anymore, which gives 
> > those 1410065408 due to integer overflow.  
> 
> Indeed, it should be an u64 not an u32.
> 
> > I didn't check the other 
> > timings for possible overflow, maybe you can do this?  
> 
> Yep, we have the same problem with tR_max and tPROG_max.
> 
> I'll send a patch to fix that.

Just sent 3 patches to fix the bugs you reported. Can you test them and
add your Reviewed-by/Tested-by? I'll try to prepare a PR this week and
ask Brian to queue those fixes for 4.13-rc4.

Thanks,

Boris
diff mbox

Patch

diff --git a/drivers/mtd/nand/atmel/nand-controller.c b/drivers/mtd/nand/atmel/nand-controller.c
index d922a88e407f..2c8baa0c2c4e 100644
--- a/drivers/mtd/nand/atmel/nand-controller.c
+++ b/drivers/mtd/nand/atmel/nand-controller.c
@@ -1201,7 +1201,7 @@  static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand,
         * tRC < 30ns implies EDO mode. This controller does not support this
         * mode.
         */
-       if (conf->timings.sdr.tRC_min < 30)
+       if (conf->timings.sdr.tRC_min < 30000)
                return -ENOTSUPP;
 
        atmel_smc_cs_conf_init(smcconf);