[U-Boot] [PATCH] ARM: tegra: Use mem size from MC in combination with get_ram_size()

[Stephen Warren]

On 10/02/2014 06:18 PM, Marcel Ziswiler wrote:
> On Thu, 2014-10-02 at 17:29 -0600, Stephen Warren wrote:
>> Please mention the commit description too, so people can still find the
>> commit if it's been cherry-picked to a different branch with a different
>> commit ID:
>>
>> aeb3fcb35956 ARM: tegra: Use mem size from MC rather than ODMDATA
>
> Will sure do.
>
>> I still think calling get_ram_size() is completely pointless given how
>> Tegra works and that this code is already running from RAM. Equally,
>> there doesn't appear to be any protection in the implementation against
>> trashing the code that implements get_ram_size() with the RAM writes the
>> code performs. I'd rather not call get_ram_size() at all.
>
> I really don't think it is that pointless given most all other ports are
> currently using it (see doc/README.memory-test which I quoted) but I

That could due to different use-cases (see next paragraph) or simply 
cargo-cult programming.

Recall that on many systems, U-Boot executes from ROM initially and is 
tasked with initializing the RAM. On all current Tegra ports, U-Boot 
always executes from RAM, and the boot ROM has already fully initialized 
RAM. That's quite a different semantic.

> agree to a certain extend to the claim you are making. Let me add
> Wolfgang CC to the discussion as the original author thereof.
>
>> That's *extremely* unlikely. Perhaps it will catch a very tiny
>> percentage of HW problems,
>
> It would catch stuck address lines which is the most like manufacturing
> issue to ever happen on the RAM side of things. But agreed for that to
> happen is very unlikely. In the last 10 thousand Tegra modules we
> produced we probably only saw one or two such failures.

If there were any stuck address lines, the chances of U-Boot actually 
executing the code far enough for the RAM size test to succeed vs fail 
is pretty minimal, since U-Boot is executing from RAM.

>>   and some number of SW configuration problems
>> (i.e. BCT EmemCfg field mismatch with the board's actual RAM size).
>
> Exactly, which is BTW how we are currently still doing things on our
> Apalis T30 modules which exist in a 1 GB and a 2 GB flavour both running
> off the exact same BCT. Downsteam so far we used custom code to detect
> the mirroring happening on the 1 GB variant:
>
> http://git.toradex.com/gitweb/u-boot-toradex.git/blob/refs/heads/colibri:/arch/arm/cpu/armv7/tegra-common/board.c#l100
>
> Now using get_ram_size() automatically takes care of this plus it also
> takes care of cases where customers for some reason flash the wrong BCT
> onto say some Colibri T20 modules which unfortunately may still boot
> even with the wrong BCT (see above given link some 22 source lines
> further up)!

To be honest, I would rather that U-Boot explicitly *not* second guess 
what the user puts into the BCT. That will make debugging other problems 
much harder.

If the user puts the wrong BCT into flash, I want something to fail hard 
so they realize there's a problem and fix it early. If U-Boot just 
ignores what the user programmed, and silently fixes it, the user may 
well not know about the problem, and hence leave it unfixed for longer, 
which will only make it far more expensive to fix.