[U-Boot] Some discussion about Allwinner DesignWare DRAM controller SPL code cleanup
Hi Jens, Andre, Chen-Yu and everyone, As we know, allwinner start to refuse to provide code about DRAM initialization after H3. For A64, H5, V3s we have already did some work on reusing H3 code and it gives good result. For R40 the result is also usable (although half of the memory is lost) I think we will need some refactor, which will makes the H3 driver finally a SUNXI_DW_DRAM driver ;-) The first thing I want to say is that the DDR timing parameters part needs a lot refactor, for V3s' integrated DDR2, for SoPine's LPDDR3, etc... What I think is to create some header files included by the DRAM driver which defines the timing parameters, and make dram_sun8i_h3.c include them by the choice in .config . The second thing is that, for some alike controller from AW, we have at least source files: dram_sun8i_{a33,h3}.c (A33 uses a similar DW DRAM). Should we merge A33 into the H3 source code? Opinions wanted ;-)
Thanks, Icenowy
Hi,
On 10/01/17 10:25, Icenowy Zheng wrote:
Hi Jens, Andre, Chen-Yu and everyone, As we know, allwinner start to refuse to provide code about DRAM initialization after H3. For A64, H5, V3s we have already did some work on reusing H3 code and it gives good result. For R40 the result is also usable (although half of the memory is lost) I think we will need some refactor, which will makes the H3 driver finally a SUNXI_DW_DRAM driver ;-)
Great, thanks for picking this up!
The first thing I want to say is that the DDR timing parameters part needs a lot refactor, for V3s' integrated DDR2, for SoPine's LPDDR3, etc... What I think is to create some header files included by the DRAM driver which defines the timing parameters, and make dram_sun8i_h3.c include them by the choice in .config .
Yes, that would definitely be a good step. Please consider using Philipp's branch[1] as a template, I believe he did this parameter inclusion from a header file already. Also he fixed the actual timing calculation, which suffers from rounding errors apparently (8 ns vs. 7.5 ns). As a bonus I wonder if you could make it in a way which would also allow to apply different parameters at runtime, either by autodetection or by putting the right parameters somewhere in the SPL, similar to what AW does with their boot0. But that's optional, really.
And just as a heads up: you might be asked to go even further: On my A64 SPL series I got comments ([2] + follow-ups) which asked for stuff actually being pulled in from a DT. The SPL has functionality to convert DT nodes into C code upon build, so we don't need to pull in a DT and libfdt code into the SPL. Check doc/driver-model/of-plat.txt for more information. There is some DDR2 DT binding with timing bits in Linux [3] already, you might want to take a look there.
[1] https://github.com/ptomsich/u-boot/commits/a64uQ7-spl-wip [2] http://lists.denx.de/pipermail/u-boot/2016-December/275337.html [3] /src/linux/Documentation/devicetree/bindings/lpddr2/lpddr2.txt
The second thing is that, for some alike controller from AW, we have at least source files: dram_sun8i_{a33,h3}.c (A33 uses a similar DW DRAM). Should we merge A33 into the H3 source code?
Give it a try it you like, sounds useful at least.
Cheers, Andre
Opinions wanted ;-)
Thanks, Icenowy
participants (2)
-
Andre Przywara -
Icenowy Zheng