Re: [PATCH] libfdt: place new nodes & properties after the parent's ones

From: Marek Szyprowski
Date: Mon Feb 10 2020 - 06:40:25 EST


Hi David,

On 05.02.2020 06:45, David Gibson wrote:
> On Tue, Feb 04, 2020 at 01:58:44PM +0100, Marek Szyprowski wrote:
>> While applying dt-overlays using libfdt code, the order of the applied
>> properties and sub-nodes is reversed. This should not be a problem in
>> ideal world (mainline), but this matters for some vendor specific/custom
>> dtb files. This can be easily fixed by the little change to libfdt code:
>> any new properties and sub-nodes should be added after the parent's node
>> properties and subnodes.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> I'm not convinced this is a good idea.
>
> First, anything that relies on the order of properties or subnodes in
> a dtb is deeply, fundamentally broken. That can't even really be a
> problem with a dtb file itself, only with the code processing it.

I agree about the properties, but generally the order of nodes usually
implies the order of creation of some devices or objects. This sometimes
has some side-effects.

For comparison, the other lib used for fdt manipulation (libufdt)
applies overlays in a such way, that the order of properties and nodes
is not reversed.

> I'm also concerned this could have a negative performance impact,
> since it has to skip over a bunch of existing things before adding the
> new one. On the other hand, that may be offset by the fact that it
> will reduce the amount of stuff that needs to be memmove()ed later on.

This code is already slow (especially in the way the uboot's use it for
'fdt apply' command), but in practice I've didn't observe negative
impact on the performance of applying large overlays at all.

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland