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

[Simon Glass]

Hi Masahiro,

On 6 March 2016 at 20:07, Masahiro Yamada <yamada.masahiro at socionext.com> wrote:
> Hi Simon,
>
>
> 2016-03-07 11:45 GMT+09:00 Simon Glass <sjg at 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 at chromium.org>
>> Reported-by: Masahiro Yamada <yamada.masahiro at 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