[EXT] Re: [PATCH] imx8m,imx9: Fix DRAM size calculation in SPL/dram_init_banksize()

[Marek Vasut]

On 8/7/23 18:43, Elena Popa wrote:
> 
>> Subject tags should be (per git log arch/arm/mach-imx/imx8m/soc.c):
>>
>> arm: imx: imx8m: imx9: Fix DRAM .....
> 
> Will do!
> 
>>> If dram_init_banksize() is called from SPL, the rom_pointer, at that
>>> point, is not correctly initialized. This causes wrong calculation of
>>> DRAM start and size in dram_init_banksize(). The issue became apparent
>>> only in Falcon Mode. Added an extra condition to prevent using
>>> rom_pointer in SPL.
>>
>> +CC Stefano, Fabio, Peng.
>>
>>> Signed-off-by: Elena Popa <elena.popa at nxp.com>
>>> ---
>>>    arch/arm/mach-imx/imx8m/soc.c | 2 +-
>>>    arch/arm/mach-imx/imx9/soc.c  | 2 +-
>>>    2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-imx/imx8m/soc.c
>>> b/arch/arm/mach-imx/imx8m/soc.c index d5254886be..dbb2154b08
>> 100644
>>> --- a/arch/arm/mach-imx/imx8m/soc.c
>>> +++ b/arch/arm/mach-imx/imx8m/soc.c
>>> @@ -273,7 +273,7 @@ int dram_init_banksize(void)
>>>        }
>>>
>>>        gd->bd->bi_dram[bank].start = PHYS_SDRAM;
>>> -     if (!IS_ENABLED(CONFIG_ARMV8_PSCI) && rom_pointer[1]) {
>>> +     if (!IS_ENABLED(CONFIG_ARMV8_PSCI) &&
>>> + !IS_ENABLED(CONFIG_SPL_BUILD) && rom_pointer[1]) {
>>
>> Use ./scripts/checkpatch.pl , it should indicate line too long here.
> 
> I did that, but got no warning. For text, the script limit is 75 and code limit is 100.

I wasn't aware of that, thanks for pointing it out.

> Should I break the line?

No

>> There seem to be other places which likely also need similar fix:
>>
>> $ git grep CONFIG_ARMV8_PSCI.*rom_pointer arch/arm/mach-imx/
>> arch/arm/mach-imx/imx8m/soc.c:  if (!IS_ENABLED(CONFIG_ARMV8_PSCI) &&
>> rom_pointer[1])
>> arch/arm/mach-imx/imx8m/soc.c:  if (!IS_ENABLED(CONFIG_ARMV8_PSCI) &&
>> rom_pointer[1]) {
>> arch/arm/mach-imx/imx8m/soc.c:          if
>> (!IS_ENABLED(CONFIG_ARMV8_PSCI) && rom_pointer[1]) {
>> arch/arm/mach-imx/imx8m/soc.c:  if (!IS_ENABLED(CONFIG_ARMV8_PSCI) &&
>> rom_pointer[0] &&
> 
> The other 2 places with similar conditions are in get_effective_memsize() and dram_init().
> At least for i.MX8M and i.MX93 are not called from within SPL. Should we add nonetheless
> the extra condition in those places as well? (for some of the other architectures,
> dram_init() is called in SPL).

Better be consistent and fix them all, yes.