Re: [U-Boot] [PATCH 1/8] fdtgrep: Improve error handling with invalid device tree

Hi Masahiro,

On 6 March 2016 at 20:07, Masahiro Yamada yamada.masahiro@socionext.com wrote:

Hi Simon,

2016-03-07 11:45 GMT+09:00 Simon Glass sjg@chromium.org:

...

This tool requires that the aliases node be the first node in the tree. But when it is not, it does not handle things gracefully. In fact it crashes.

Fix this, and add a more helpful error message.

Signed-off-by: Simon Glass sjg@chromium.org Reported-by: Masahiro Yamada yamada.masahiro@socionext.com

I think this patch would diplay a clearer hint, but would not solve the root cause.

fdtgrep still crashes with the following DTS:

Crashes?

/ { model = "UniPhier PH1-LD20 Reference Board"; compatible = "socionext,ph1-ld20-ref", "socionext,ph1-ld20";

memory {
    device_type = "memory";
    reg = <0 0x80000000 0 0xc0000000>;
};

chosen {
    stdout-path = "serial0:115200n8";
};

aliases {
    serial0 = &serial0;
    serial1 = &serial1;
    serial2 = &serial2;
    serial3 = &serial3;
    i2c0 = &i2c0;
    i2c1 = &i2c1;
    i2c2 = &i2c2;
    i2c3 = &i2c3;
    i2c4 = &i2c4;
    i2c5 = &i2c5;
    i2c6 = &i2c6;
};

};

Error at 'fdt_find_regions': FDT_ERR_BADLAYOUT /aliases node must come before all other nodes Error: FDT_ERR_BADMAGIC

You mean it gives an error?

But, fdtgrep can accept the following:

/ { model = "UniPhier PH1-LD20 Reference Board"; compatible = "socionext,ph1-ld20-ref", "socionext,ph1-ld20";

aliases {
    serial0 = &serial0;
    serial1 = &serial1;
    serial2 = &serial2;
    serial3 = &serial3;
    i2c0 = &i2c0;
    i2c1 = &i2c1;
    i2c2 = &i2c2;
    i2c3 = &i2c3;
    i2c4 = &i2c4;
    i2c5 = &i2c5;
    i2c6 = &i2c6;
};

memory {
    device_type = "memory";
    reg = <0 0x80000000 0 0xc0000000>;
};

chosen {
    stdout-path = "serial0:115200n8";
};

};

Should I really move the "aliases" node? I do not think this restriction is reasonable.

Agreed. Given that it now sorts the regions I'm not sure why that restriction is still there, actually.

Looking a bit closer, it is really checking that the aliases node isn't last. It could probably work around it by using fdt_size_dt_structs() when node_end is -ve.

Regards, Simon