Re: [RFC PATCH] mtd: nand: add support for ts72xx

[Miquel Raynal]

Hi Nikita,

> ```
> # modprobe technologic-nand-controller
> nand: device found, Manufacturer ID: 0xec, Chip ID: 0xf1
> nand: Samsung NAND 128MiB 3,3V 8-bit
> nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
> Scanning device for bad blocks
> Bad eraseblock 137 at 0x000001120000
> Bad eraseblock 310 at 0x0000026c0000
> 3 fixed-partitions partitions found on MTD device 60000000.nand-
> controller
> Creating 3 MTD partitions on "60000000.nand-controller":
> 0x000000000000-0x000000020000 : "TS-BOOTROM"
> 0x000000020000-0x000007d20000 : "Linux"
> 0x000007d20000-0x000008000000 : "RedBoot"
> ```
> 
> This looks like Samsung K9F1G08U0D or K9F1G08U0B.
> 
> And the patch above totally solves my issues with READCACHE - thank
> you!
> 

Great!

> > If you are using a Micron chip that specifies not supporting ECC
> > correction together with sequential cache reads, then it's a problem
> > that we will solve soon (you can hardcode
> > chip->controller->supported_op.cont_read to 0);
> > 
> > Otherwise we can discuss it.
> >   
> > > - legacy wait implementation had no delays - only timeout and if i
> > > set 
> > >   readb_poll_timeout() to something reasonable i experience speed
> > > degradation 
> > >   on mtd_speedtest, can it be left as 0 ?  
> > 
> > Looks like the legacy implementation used cond_resched(). The delay
> > needs to be observed before first checking for the status, it's a
> > delay
> > that is described in the spec, if you get the status before you might
> > just not see a consistent value. I believe that is worth taking into
> > account in your implementation below (don't wait then delay, it's
> > not relevant). Can you share the values you've tried and the
> > performances you've got?  
> 
> The numbers are pretty stable, so indeed legacy is a bit faster,
> followed up by new one with zero interval.

...

> Legacy speedtest (old version of nand controller):

> mtd_speedtest: eraseblock write speed is 3793 KiB/s
> mtd_speedtest: eraseblock read speed is 3567 KiB/s
> mtd_speedtest: page write speed is 3682 KiB/s
> mtd_speedtest: page read speed is 3488 KiB/s
> mtd_speedtest: 2 page write speed is 3724 KiB/s
> mtd_speedtest: 2 page read speed is 3521 KiB/s
> mtd_speedtest: erase speed is 198709 KiB/s

...
 
> New version with interval zero:

> mtd_speedtest: eraseblock write speed is 3685 KiB/s
> mtd_speedtest: eraseblock read speed is 3517 KiB/s
> mtd_speedtest: page write speed is 3592 KiB/s
> mtd_speedtest: page read speed is 3444 KiB/s
> mtd_speedtest: 2 page write speed is 3608 KiB/s
> mtd_speedtest: 2 page read speed is 3475 KiB/s
> mtd_speedtest: erase speed is 194499 KiB/s

Looks almost the same as above, I believe the difference is just noise
between measurements.

...

> New version with interval 100:

> mtd_speedtest: eraseblock write speed is 2722 KiB/s
> mtd_speedtest: eraseblock read speed is 2175 KiB/s
> mtd_speedtest: page write speed is 2598 KiB/s
> mtd_speedtest: page read speed is 2070 KiB/s
> mtd_speedtest: 2 page write speed is 2627 KiB/s
> mtd_speedtest: 2 page read speed is 2106 KiB/s
> mtd_speedtest: erase speed is 175851 KiB/s

...

> Also providing version with zero interval and "if (instr->delay_ns)"
> dropped - it's optional as far i understood:

It's not optional, sorry for the wrong comment, please keep it as it
is, knowing that it will be null after a wait_rdy.

> mtd_speedtest: eraseblock write speed is 3695 KiB/s
> mtd_speedtest: eraseblock read speed is 3532 KiB/s
> mtd_speedtest: page write speed is 3593 KiB/s
> mtd_speedtest: page read speed is 3457 KiB/s
> mtd_speedtest: 2 page write speed is 3640 KiB/s
> mtd_speedtest: 2 page read speed is 3488 KiB/s
> mtd_speedtest: erase speed is 195451 KiB/s

Thanks,
Miquèl