[U-Boot] [PATCH 06/11] MX6: add struct for sharing data between SPL and uboot

[Igor Grinberg <grinberg@compulab.co.il>]

Hi Stefano, Tim,

On 04/17/14 12:30, Stefano Babic wrote:
> Hi Tim,
> 
> On 17/04/2014 08:07, Tim Harvey wrote:
>> On Mon, Apr 14, 2014 at 5:35 AM, Stefano Babic <sbabic@denx.de> wrote:
>>> Hi Tim,
>>>
>>> I see checking your patch that the MXS uses the same concept. And as far
>>> as I can see, boot_mode_idx is used only to print the boot devioce
>>
>> Stefano,
>>
>> yes, that is where I got the concept from.
> 
> Well, it slipped into mainline, but adding such a structure only to have
> a better print is not very useful...
> 
>> I've been told this before, but I've found that get_ram_size() will
>> hang on an i.MX6 if you pass it a maxsize larger than the memory in
>> your system. Perhaps you can verify you see the same behavior?
> 
> get_ram_size() works if the memory is accessible, that means that no
> exception is generated when it tries to access. If the RAM controller is
> programmed for a larger amount of memory, because you can have board
> with different capacity of RAM, get_ram_size() finds dynamically the
> soldered value.
> 
> However, if the RAM controller is programmed for an amount of RAM
> smaller as what get_ram_size() tries to identify, an exception is
> raised. In get_ram_size(start, size) we pass the maximum amount of
> memory that the function is allowed to check. I can imagine we are
> running in this case. Checking on i.MX6, we have both boards checking
> the size with get_ram_size() as mx6sabresd, as well as boards fixing at
> compile time the value of the RAM (nitrogen6x).

get_ram_size() works on cm-fx6 all DRAM configurations.

> 
>>> SPL is thought to generally load an image (of course, in most cases it
>>> is u-boot). In Falcon mode, the kernel is started without running
>>> u-boot, making this structure useless. Do we really need such a way (but
>>> then, it must be generalized as SPL API), or can we get rid of it ?
>>
>> As we have an EEPROM on the board that tells us the physical ram size,
>> I use that to avoid the lockup.
> 
> It seems weird. There is several boards in mainline (I am sure about
> Freescale's PowerPC) discovering the mounted RAM. Of course, this is
> simpler if many parameters (row/columns/...) are the same, I am not sure
> about this.

Even if the parameters are different, this should not result in lockup...
AFAICS, lockup of that type has nothing to do with the DRAM itself, but
the controller configuration.

> Are we sure it is not possible here ? What does it happen
> with an inconsistent value in EEprom ?
> 
>> Eventually I would like to read and
>> validate the entire EEPROM once in SPL and pass this to u-boot.img to
>> avoid reading and validating it again. I think this is a good example
>> of why sharing data between SPL and u-boot.img could be useful.
> 
> I am not sure, and I doubt it is a good idea to use persistent data to
> store that. It is potentially dangerous, and if some reasons the EEprom
> is changed, the board could not boot at all.

This is more a question of design and definition of what purpose the eeprom
should serve, so I would let the vendor decide if he wants to depend on eeprom
or not.

> 
> On the other side, I like the current concept that the SPL is simply a
> loader and can load images of different types, and fixing an API between
> SPL and U-Boot goes against this concept.

I agree on this one, there is no real need (at least currently) to
introduce such an API.

Also, there are cases (e.g. cm-fx6) where the DRAM size can be determined by
just reading the MMDC configuration and thus spare additional get_ram_size()
call from U-Boot.


-- 
Regards,
Igor.