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.goldschm...@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...?

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.

Simon

+    if (ret) {
+        debug("spl_early_init() failed: %d\n", ret);
+        hang();
+    }
+
       /* enable console uart printing */
       preloader_console_init();


_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to