[U-Boot] [linux-sunxi] Re: [PATCH v2 5/6] sunxi: add code for recalculating the DRAM size in U-Boot

[André Przywara]

On 03/04/18 15:13, Siarhei Siamashka wrote:

Hi Siarhei,

thanks for chiming in!

> On Tue, 3 Apr 2018 13:43:43 +0100
> Andre Przywara <andre.przywara at arm.com> wrote:
>> On 03/04/18 12:51, Icenowy Zheng wrote:

....

>>>> I guess we'd need to find another way (put some information in an
>>>> SRAM somewhere?) to try to do that for all variants.  
>>>
>>> Extend the SPL header again? If we found SPL v3+, use
>>> the DRAM size encoded and bypass ram_get_size,
>>> otherwise fallback to ram_get_size?  
>>
>> Yes, that would be a possibility as well. Though I believe at the moment
>> we don't access the SPL header from U-Boot proper, do we?
> 
> We do. The boot device and also the boot.scr location (in the case of
> FEL boot) is read from the SPL header by the main U-Boot part.

Ah, true, forgot about that. Even better then.

>> Not a real show-stopper, but we probably need to document that the SPL
>> header would need to stay around.
> 
> Maybe.
> 
>> But if we have a fallback anyway ...
> 
> Which fallback? Do you mean calling things like ram_get_size()
> from U-Boot?

Yes, that was what Icenowy suggested: If SPL version > 2, we use the
information from there, otherwise we fall back to the current behaviour,
which is to ride through the DRAM and hope for the best.
Though I am not sure this is really needed, as I don't see a strong
reason to combine different versions of SPL and U-Boot proper (apart
from FEL, maybe).

> We should never do this because this is very wrong.

I mostly agree, though it's not too bad, since we have quite a
controlled environment. But if we can get rid of it: Yes, we should.

> The D-Cache may be already enabled, causing all kinds of weird
> effects. Also modifying data in DRAM (even temporarily) while
> our code is already running from DRAM is dangerous.

I don't see immediately how the D$ could get nasty here, but as I said
above, we should do it.

>>> (Although it will lead to some days of mess on sunxi-tools,
>>> this is a reasonable choice.)  
>>
>> True, but actually I wonder why we have SPL_MAX_VERSION in there in the
>> first place. Can't we just postulate that every new SPL version stays
>> backwards compatible? So if sunxi-tools can deal with v2, a v3 SPL would
>> still be fine, you would just loose the v3 features (if at all)?
>> We can just put a warning in there, to ask users to upgrade.
>> That would have worked already with the v1/v2 transition, I believe.
> 
> Yes, that's more or less how this was supposed to work in sunxi-tools
> from the very beginning. Except that we unfortunately got a bug in
> this code, which has been reported back in July 2017 and is still not
> resolved due to various reasons:

Well, that sounds more like a design issue to me: Defining the latest
currently supported version as the maximum.

>    https://github.com/linux-sunxi/sunxi-tools/issues/104
> 
> But hopefully it can be fixed sooner or later.

I think we can do it now, see below.

>> Probably worth a separate discussion with some sunxi-tools stakeholders.
> 
> On the U-Boot side we can just increase the version number as long as
> the new header is a backwards compatible superset of the old one.
> 
> In the unlikely case if we suddenly have to introduce a compatibility
> breaking change to the SPL header format, we can always change the SPL
> header signature from 'SPL' to something else.

Or we could introduce some major/minor scheme, with a major change
breaking compatibility, whereas a minor change does not.
Then we split the uint8_t version into 2 bits of major and 6 bits of
minor, for instance.
So we just document this and bump SPL_MAX_VERSION now to 0x3f and are
good for a while.

Thoughts?

Cheers,
Andre.