Hi Vladimir,
Thanks for taking time to read my feedback. You can see my comments and my answer below.
Sylvain
-----Original Message----- From: Vladimir Zapolskiy [mailto:vz@mleia.com] Sent: 15-Jul-15 8:20 PM To: LEMIEUX, SYLVAIN; Albert ARIBAUD Cc: Scott Wood; u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] nand: lpc32xx: add SLC NAND controller support
Hi Sylvain,
On 15.07.2015 22:23, LEMIEUX, SYLVAIN wrote:
Hi Vladimir and Albert,
During this merge window (once our issues with our exchange server are
resolve), we were planning on submitting a few patches for the LPC32xx.
great, feel free to add me to Cc.
Will do;
Some of the patches are the porting of the legacy NXP BSP (u-boot) drivers
into the latest version; the drivers are the DMA, the SLC NAND and the USB.
If DMA and USB are added, I'll gratefully reuse this on my board :)
I will submit the LPC32xx patches using an alternate e-mail for now, until the problem with our e-mail infrastructure is resolve.
First, I need to do some rework (matching the naming convention of your NAND SLC patch and update our porting effort based on the feedback from Albert).
This original NXP implementation of the SLC NAND was using the DMA. I am
also planning on testing this patch to compare the flashing time, with and without the DMA.
Sounds good. Also since DMA is going to be supported it would be nice to add HW ECC calculation to the SLC NAND driver.
Hardware ECC was already supported in the legacy BSP; it will be part of the patches I will submit.
FYI here are performance test results of my PIO version:
=> gettime; nand read.raw 0x80000000 0x0 0x6000; gettime Timer val: 63952 Seconds : 63 Remainder : 952 sys_hz = 1000
NAND read: 51904512 bytes read: OK Timer val: 113352 Seconds : 113 Remainder : 352 sys_hz = 1000
1.002 MiB per second, quite slow, but not drastically slow.
FYI, I did the same testing on my side using the legacy NXP BSP implementation; the test was done with the CPU clock at 208MHz and 266MHz.
For those test, we have no timing optimization for the SLC NAND.
Clock configuration: CPU clock: 266MHz / AHB bus clock: 133MHz / Peripheral clock: 13MHz ==> gettime; nand read.raw 0x80000000 0xd00000 0x6000; gettime Timer val: 22949 Seconds : 22 Remainder : 949 sys_hz = 1000
NAND read: 51904512 bytes read: OK Timer val: 44803 Seconds : 44 Remainder : 803 sys_hz = 1000 --> 2.265 MiB per second
==> gettime; nand read.e 0x80000000 0xd00000 0x3180000; gettime Timer val: 66054 Seconds : 66 Remainder : 54 sys_hz = 1000
NAND read: device 0 offset 0xd00000, size 0x3180000 51904512 bytes read: OK Timer val: 89214 Seconds : 89 Remainder : 214 sys_hz = 1000 --> 2.137 MiB per second
Clock configuration: CPU clock: 208MHz / AHB bus clock: 104MHz / Peripheral clock: 13MHz
==> gettime; nand read.raw 0x80000000 0xd00000 0x6000; gettime Timer val: 24605 Seconds : 24 Remainder : 605 sys_hz = 1000
NAND read: 51904512 bytes read: OK Timer val: 52458 Seconds : 52 Remainder : 458 sys_hz = 1000 --> 1.777 MiB per second
==> gettime; nand read.e 0x80000000 0x00d00000 0x3180000; gettime Timer val: 134819 Seconds : 134 Remainder : 819 sys_hz = 1000
NAND read: device 0 offset 0xd00000, size 0x3180000 51904512 bytes read: OK Timer val: 164465 Seconds : 164 Remainder : 465 sys_hz = 1000 --> 1.669 MiB per second
I have two questions:
- How do you suggest to approach this, as some patches may be similar or
conflicting with what Vladimir is planning on submitting?
I presume the only conflicting place is SLC NAND driver.
Yes, this will be the only conflicting patch.
Here I see some benefits of my version:
- the driver is very tiny, practically it is read_buf()/write_buf() and timing
configuration, all the rest I managed to offload to existing mtd/nand and spl/nand frameworks at the price of more added CONFIG_* defines in a board header file,
- not sure what OOB layout is coming from NXP BSP (I don't have this BSP to
check, unfortunately), but I would prefer to see the same OOB layout in U- boot and in vanilla Linux --- this is done in my version,
- the driver can be included to SPL binary,
- the driver is well tested on my environment,
- the code has been published for review.
This is the benefits (I am thinking we get) from the legacy NXP BSP porting: * The driver went through multiple iteration (the latest version of the legacy patch was 1.07). * The BSP, from LPC Linux, was most likely review and tested by multiple users; it was the initial u-boot reference for the LPC32xx development boards. * The SLC NAND implementation is integrated with the DMA, and already support hardware ECC. * The OOB layout from the legacy BSP is matching the LPC32xx NAND SLC Linux driver.
The only two missing things from the driver I see at the moment are based on working DMA driver:
- data transfer by means of DMA,
- HW ECC calculation (data correction is always done by software).
Also my driver has not been tested with small page NAND chips, not sure, if it is relevant for you.
The legacy BSP driver was only tested with large page NAND on our side.
If DMA works, I hope it should be easy to add some lpc32xx_chip.ecc.* callbacks to my version of the driver.
- For submitting legacy NXP BSP driver porting patch, would you like to see
a 3 patches series (original driver, checkpatch script fix and the update for latest u-boot) to have history of the change or a single patch with the final result?
If it were related to Linux kernel project, I know the clear answer, but please let me leave U-boot maintenance specifics to be explained by Albert.
-- With best wishes, Vladimir
________________________________
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.