[U-Boot] [PATCH 1/6] arm: socfpga: fix SPL on gen5 after moving to DM serial

[Marek Vasut]

On 08/10/2018 02:55 PM, Simon Goldschmidt wrote:
> On 10.08.2018 14:41, Marek Vasut wrote:
>> On 08/10/2018 02:39 PM, Simon Goldschmidt wrote:
>>> On 09.08.2018 23:42, Marek Vasut wrote:
>>>> On 08/09/2018 09:04 PM, Simon Goldschmidt wrote:
>>>>> There were NULL pointers dereferenced because DM was used
>>>>> too early without correct initialization:
>>>>> - malloc_simple returned NULL when called from
>>>>> preloader_console_init()
>>>>>     because gd->malloc_limit was 0
>>>>> - uclass_add dereferenced gd->uclass_root members which were NULL
>>>>> because
>>>>>     dm_init (or one of its relatives) has not been called.
>>>>>
>>>>> All this is fixed by calling spl_early_init before calling
>>>>> preloader_console_init.
>>>>>
>>>>> This fixes commit 73172753f4f3 ("ARM: socfpga: Convert to DM serial")
>>>>>
>>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
>>>>> ---
>>>>> v2:
>>>>> - Don't remove gd->malloc_base assignment at the end of board_init_f()
>>>>>     (moved to an extra patch)
>>>>>
>>>>>    arch/arm/mach-socfpga/spl_gen5.c | 7 +++++++
>>>>>    1 file changed, 7 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c
>>>>> b/arch/arm/mach-socfpga/spl_gen5.c
>>>>> index d6fe7d35af..9bdfaa3c1e 100644
>>>>> --- a/arch/arm/mach-socfpga/spl_gen5.c
>>>>> +++ b/arch/arm/mach-socfpga/spl_gen5.c
>>>>> @@ -86,6 +86,7 @@ void board_init_f(ulong dummy)
>>>>>        const struct cm_config *cm_default_cfg =
>>>>> cm_get_default_config();
>>>>>        unsigned long sdram_size;
>>>>>        unsigned long reg;
>>>>> +    int ret;
>>>>>          /*
>>>>>         * First C code to run. Clear fake OCRAM ECC first as SBE
>>>>> @@ -152,6 +153,12 @@ void board_init_f(ulong dummy)
>>>>>        /* unfreeze / thaw all IO banks */
>>>>>        sys_mgr_frzctrl_thaw_req();
>>>>>    +    ret = spl_early_init();
>>>> Uh, but isn't this called from common/spl/spl.c ? I suspect the SoCFPGA
>>>> SPL is a bit weird.
>>> Ehrm, I copied this from spl_s10.c, but other boards seem to do this,
>>> too. Honestly, I don't know how any SPL can use DM serial without this
>>> being called. Maybe other SPLs initialize the serial port later (not in
>>> board_init_f).
>> I mean, spl_early_init() is called in common/spl/spl.c , which is common
>> code. Maybe the socfpga SPL is structured in a really weird way (I think
>> it is).
> 
> Not exactly: common/spl/spl.c calls spl_common_init(), just like
> spl_early_init() does. Given the names, I think spl_early_init() is
> meant to be called early, e.g. from board_init_f() ;-)
> 
> Oh, I just saw spl_common_init() emits a debug print "spl_early_init()",
> so that might have tricked you...?

Ah, yes, I think so.

>>> common/spl/spl.c calls spl_init(), which also calls the part that
>>> spl_early_init() calls.
>>>
>>> I can only take other SPLs as reference and from reading all the code, I
>>> think this should be good.
>> Right
> 
> So is this change OK for v2018.09 once I fix the dts thing? Given that
> v2018.07 is broken for socfpga gen5, it would be good to merge it
> before. I can prepare a v3 of the series with only minimal changes in
> the socfpga files and resend the rest as detached patches.
Looks good, yes.

-- 
Best regards,
Marek Vasut