[U-Boot] [PATCH v2 14/23] sunxi: H3: add DRAM controller single bit delay support

[André Przywara]

On 07/12/16 03:48, Simon Glass wrote:
> Hi Andre,
> 
> [...]
> 
>>>> I wonder if there is value in moving this to device tree with of-platdata?
>>
>> While I kind of like the idea of using the DT for this, there are some
>> issues:
>>
>> 1) There is no binding so far for representing the DRAM data. Given the
>> lacking documentation for the DRAM controller it sounds very hard to
>> come up with a good binding anyway. Also we can't push this through the
>> Linux DT binding review, since this is of no interest to the kernel. And
>> I'd rather avoid making up some dodgy binding just for this.
>>
>> There is work underway to improve the DRAM init code and make it more
>> robust and flexible. Ideally we can use some autodetection and
>> calibration feature the controller offers to get rid of arbitrary magic
>> numbers. But this is quite some work ahead and shouldn't block the much
>> sought after A64 SPL support for now.
>>
>> 2) If there is need, we can detect the SoC easily by reading the ID
>> register and differentiate at runtime. This is probably less code than
>> pulling in DT bits, also more robust.
>>
>>> I think device tree support is unlikely to fit in SPL for sunxi.
>>> IIRC Andre already mentions the space constraints in his cover letter.
>>
>> 3) Yes, adding DT support for the SPL makes it rather big. I think it
>> breaks the 28K limit that the mksunxiboot tool currently has. This can
>> (and will) be fixed later, but just for this exercise I'd rather keep it
>> small, especially as we would use it only for the DRAM code and not for
>> the device drivers.
> 
> Take a look at rk3288-firefly if you like. It has an ad-hoc device
> tree binding (no one has the energy to try to get this into Linux :-).

I found some lpddr2 binding in Linux, I guess we can use these as a
template. But ....

> With of-platdata, DT support doesn't actually add any space (or at
> least very little). There is no libfdt and the only code is that
> needed to copy data from the of-platdata struct to the normal one.
> 
> That said, there has to be a benefit, and it's much more desirable to
> spend the time on this IMO:

I think there is some benefit, but as you hinted it takes more time. My
understanding is that these parameters are actually board specific,
although a) nobody cared so much before and just went with the same
Allwinner provided values for every board and b) many vendors copy the
DRAM trace layout and thus share the same values here.

So:
1) We would need to work out what parameters we actually need.
2) Also which are a SoC property, which are board specific and which are
DRAM chip dependent. For instance we often see the same chips used on
different boards, also similar layouts across boards.
Technically the amount of DRAM also matters, as that means different
chips in possibly different configurations (Pine64 1GB with 2x16 bits
vs. the 2GB version with 4x8 bits)
3) We need to learn how much we can actually auto detect. The DRAM
controller seem to have some facilities, it may be worth to explore this.

There are some patches to rework and improve the DRAM setup, so I guess
we need to revisit this anyway. I tried to merge them in here, but gave
up because I think they need more love.
So for the sake of getting good-enough support for the Pine64 board now
I'd rather go with these fixed values for now and postpone this discussion.

Cheers,
Andre.

>>
>> Actually I have a plan to make better use of DT, but not for the SPL. To
>> a good degree the SPL code mimics the on-SoC boot ROM operation
>> (accessing storage devices to load code), which has to work with every
>> board already and thus does not need a board specific DT.
>> I can elaborate on that if there is interest.
>