[U-Boot] Some discussion about Allwinner DesignWare DRAM controller SPL code cleanup

[U-Boot] Some discussion about Allwinner DesignWare DRAM controller SPL code cleanup

[Icenowy Zheng]

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

[U-Boot] Some discussion about Allwinner DesignWare DRAM controller SPL code cleanup

[Andre Przywara]

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
>