[U-Boot] [PATCH 1/3] nand: lpc32xx: add SLC NAND driver

[LEMIEUX, SYLVAIN]

Hi Vladimir and Albert,

See comment, questions and test results below;

> From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
> Sent: 18-Jul-15 1:50 AM
>
> Hello Vladimir,
>
> On Sat, 18 Jul 2015 02:10:01 +0300, Vladimir Zapolskiy <vz at mleia.com>
> wrote:
> > Hi Sylvain, Albert,
> >
> > On 18.07.2015 01:24, LEMIEUX, SYLVAIN wrote:
> > > Hi Albert,
> > >
> > > Thanks for the feedback.
> > >
> > >> From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert ARIBAUD
> > >> Sent: 17-Jul-15 5:20 PM
> > >>
> > >> Hello Sylvain,
> > >>
> > >> On Fri, 17 Jul 2015 16:48:52 -0400, slemieux.tyco at gmail.com
> > >> <slemieux.tyco at gmail.com> wrote:
> > >>
> > >>>

 ....

>
> > * What about functional support in SPL? Is it correct, that if I want to
> > have this code in SPL, then I have to pull in DMA driver as a mandatory
> > dependency to tiny SPL?
>
> You have to ensure DMA, but whether you pull the whole BSP driver or
> hard-code the necessary parts into a single self-contained SLC NAND +
> DMA file is a design decision. That said, I would personally go for
> pulling the driver *and* adding preprocessing out any part of it not
> necessary for SPL -- not so much for size (the compiler and linker
> should optimize useless parts away on their own anyway) than for clarity
> and maintenance (readers of the driver code would know which part is
> needed for SPL and which is not).
>

I am in the process of putting a patch together to add the hardware ECC
support on top of the SLC NAND driver patch
(https://patchwork.ozlabs.org/patch/497308/).

A have a few questions:
1) The LPC32xx SLC NAND driver is using a custom NAND ECC Layout for small page;
    You can refer to the Kernel driver:
     https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/drivers/mtd/nand/lpc32xx_slc.c?id=refs/tags/v4.1.3

    Are you planning to update your driver with the change or should I submit a separate
    patch for it before adding the support for DMA and hardware ECC?

2) As suggested by Albert, I will add a conditional compile option to ensure the
     original version of the driver (no DMA) can be used for SPL.

    I was planning to do it using the configuration option use to select the LPC32xx
    DMA driver (CONFIG_DMA_LPC32XX). Are you OK with this approach?

3) Should I separate the NAND SLC /DMA & the USB into 2 separate patch?


Test result (full command log below):

Clock configuration:
CPU clock: 266MHz / AHB bus clock: 133MHz / Peripheral clock: 13MHz

1) NAND SLC driver: https://patchwork.ozlabs.org/patch/497308/
     NAND read:  51904512 bytes
     read.raw: 1.444 MiB per second / read: 0.625 MiB per second

2) NAND SLC driver: https://patchwork.ozlabs.org/patch/497308/
     & Legacy BSP porting (hardware ECC & DMA)
     NAND read:  51904512 bytes
     read.raw: 2.272 MiB per second / read: 2.139 MiB per second

------------------
Full log:

NAND SLC driver: https://patchwork.ozlabs.org/patch/497308/

==> gettime; nand read.raw 0x80000000 0xd00000 0x6000; gettime
Timer val: 259607
Seconds : 259
Remainder : 607
sys_hz = 1000

NAND read:  51904512 bytes read: OK
Timer val: 293885
Seconds : 293
Remainder : 885
sys_hz = 1000
--> 1.444 MiB per second

==> gettime; nand read.e 0x80000000 0xd00000 0x3180000; gettime
Timer val: 11304
Seconds : 11
Remainder : 304
sys_hz = 1000

NAND read: device 0 offset 0xd00000, size 0x3180000
 51904512 bytes read: OK
Timer val: 90495
Seconds : 90
Remainder : 495
sys_hz = 1000
--> 0.625 MiB per second

NAND SLC driver: https://patchwork.ozlabs.org/patch/497308/
& Legacy BSP porting (hardware ECC & DMA)

==> gettime; nand read.raw 0x80000000 0xd00000 0x6000; gettime
Timer val: 49705
Seconds : 49
Remainder : 705
sys_hz = 1000

NAND read:  51904512 bytes read: OK
Timer val: 71483
Seconds : 71
Remainder : 483
sys_hz = 1000
--> 2.272 MiB per second

Timer val: 280282
Seconds : 280
Remainder : 282
sys_hz = 1000

NAND read: device 0 offset 0xd00000, size 0x3180000
 51904512 bytes read: OK
Timer val: 303423
Seconds : 303
Remainder : 423
sys_hz = 1000
--> 2.139 MiB per second

Sylvain

> Amicalement,
> --
> Albert.

________________________________

This e-mail contains privileged and confidential information intended for the use of the addressees named above. If you are not the intended recipient of this e-mail, you are hereby notified that you must not disseminate, copy or take any action in respect of any information contained in it. If you have received this e-mail in error, please notify the sender immediately by e-mail and immediately destroy this e-mail and its attachments.