Hi Vladimir,
See comment below.
From: Vladimir Zapolskiy [mailto:vz@mleia.com] Sent: 27-Jul-15 8:22 PM
Hi Sylvain,
On 28.07.2015 00:45, LEMIEUX, SYLVAIN wrote:
Hi Vladimir and Albert,
See comment, questions and test results below;
From: Albert ARIBAUD [mailto:albert.u.boot@aribaud.net] Sent: 18-Jul-15 1:50 AM
Hello Vladimir,
On Sat, 18 Jul 2015 02:10:01 +0300, Vladimir Zapolskiy vz@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@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@gmail.com slemieux.tyco@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:
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/dr...
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?
if you don't object to rebase your changes on top of mine (see my closing comment), I'm fine, if custom NAND ECC Layout for small page chip is added in a separate patch. For your information on my side I won't be able to test it though.
Already done, I will update the change based on the comment bellow and submit a second version of the patch;
The test result, listed below, are with the NXP legacy BSP code on top of your patch.
I will add a patch from the small page change; the change are taken from the legacy NXP BSP and the mapping is matching the kernel driver.
Who will provide feedback on the DMA patch?
- 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.
Great, thank you.
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?I'm not sure, if DMA is really needed in SPL binary, but if you want to add this option, I don't object. To separate code for SPL and normal image please use CONFIG_SPL_BUILD macro.
- Should I separate the NAND SLC /DMA & the USB into 2 separate patch?
I would say DMA driver as a prerequisite should go first, then in arbitrary order NAND SLC updates specific to available/compiled DMA and USB changes.
Test result (full command log below):
Clock configuration: CPU clock: 266MHz / AHB bus clock: 133MHz / Peripheral clock: 13MHz
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
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
Thank you for tests. So, as expected if DMA and hardware ECC is enabled, then the performance is 1.5 (raw read) or 3.5 times (read & ecc) better.
I do recognize and appreciate this work, advocating my change I can repeat my last arguments:
- it has a ready valid user in U-boot, my change is not a dead code,
- it is more functional -- read from NAND in SPL is supported,
- it is extremely simple (180 LoC vs. 500 LoC + ~200 LoC of DMA driver),
- it has been reviewed by Scott, all review comments are fixed and
published in v4, and it is ready for inclusion to the next release IMHO.
If there is no more review comments to my version of the driver, I would kindly ask you to rebase legacy SLC NAND driver on top of my version of the driver, it is a doable and simple task in my opinion, performance optimization with the associated code complexity may be added later on.
-- 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.