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:v...@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: > > 1) 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. > > > 2) 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. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot