Re: [PATCH] libfdt: Add fdt_add_subnode_tail()

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4049 bytes --]

On Wed, Nov 04, 2026 at 05:50:45PM +0000, Simon Glass wrote:
> Hi,
> 
> On Tue, 3 Nov 2026 at 22:55, David Gibson <david@gibson.dropbear.id.au> wrote:
> >
> > On Tue, Nov 03, 2026 at 09:44:25PM +0530, Vishal Chourasia wrote:
> > > fdt_add_subnode() always inserts the new node as the first child of its
> > > parent, after the parent's properties. Code that builds a list of nodes
> > > by adding them one at a time therefore ends up with the nodes in reverse
> > > order. That matters when the order is significant, e.g. when a blob is
> > > rebuilt from a live tree.
> > >
> > > Add fdt_add_subnode_tail() and fdt_add_subnode_namelen_tail(), which
> > > insert the new node right before the closing tag of the parent, so that
> > > nodes added in sequence keep that sequence. Like fdt_add_subnode(), they
> > > use fdt_splice_struct_() and so leave the blob unmodified if there is
> > > not enough room.
> > >
> > > Also add an Fdt.add_subnode_tail() method to pylibfdt.
> > >
> > > Add tests for both: a C testcase covering ordering, parents with and
> > > without existing children, the root node, duplicate names and running
> > > out of space, and a short pylibfdt test of the wrapper.
> > >
> > > Signed-off-by: Vishal Chourasia <vishalc@linux.ibm.com>
> >
> > I'd need a stronger and more specific rationale to consider applying
> > this.  In general node order should not matter to consumers of the
> > device tree, which is why we make no attempt to preserve it the
> > fdt_rw.c interface,
> 
> Yes 'when a blob is rebuilt from a live tree' needs more detail. What
> is the use case that drove this?
> 
> The spec doesn’t say that sibling order is significant, but it doesn’t
> say that it isn’t either. It only requires properties to come before
> subnodes.

I guess not, because the modern(ish) spec is specifically describing
the flattened format, which necessarily has some sort of order.
Non-significance of the order might be implied by compatibility of the
device tree contents with IEEE 1275.  In IEEE 1275 the tree is not
flattened (access is via Forth firmware methods), and I don't think it
precludes an implementation which uses a hash table to store
properties and subnodes.

> rom my side, one use for this new function could be with Binman[1]
> which builds firmware images. The nodes describe the ordering of
> binaries within the image - example at [2].

I would strongly advise you not to design something which relies on
property or node order within the fdt format.

> Binman knows the current libfdt behaviour and is careful about
> ordering when it is generating nodes / processing templates.
> 
> Specifically, the code at [3] (a Python interface on pylibfdt which I
> am looking forward to replacing with Doug's one) has to add nodes in
> reverse order and then rotate its own node list to match what libfdt
> produced. Inserting a node anywhere but first means deleting all its
> siblings and recreating them ([4]), and template expansion in binman
> ([5], called from [6]) has to walk its sources backwards.
> 
> An fdt_add_subnode_tail() would let this code add nodes in the natural
> order, without the reversals or the delete-and-re-add step.
> 
> Regards,
> Simon
> 
> [1] https://github.com/deinde-dev/binman
> [2] https://binman.readthedocs.io/en/latest/binman.html#a-first-image
> [3] https://github.com/deinde-dev/dtoc/blob/1e8b69d7e95970f7c226d1a4d6cb90b3e067ee61/dtoc/fdt.py#L684-L696
> [4] https://github.com/deinde-dev/dtoc/blob/1e8b69d7e95970f7c226d1a4d6cb90b3e067ee61/dtoc/fdt.py#L577-L597
> [5] https://github.com/deinde-dev/dtoc/blob/1e8b69d7e95970f7c226d1a4d6cb90b3e067ee61/dtoc/fdt.py#L735-L786
> [6] https://github.com/deinde-dev/binman/blob/7a9cfb456ddeaba09f0cefc3dfd6c801978c6509/binman/control.py#L498-L523
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]