Re: [PATCH] libfdt: Add fdt_add_subnode_tail()

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

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

On Wed, Nov 11, 2026 at 06:24:15PM +0000, Simon Glass wrote:
> Hi David,
> 
> On Tue, 10 Nov 2026 at 19:50, David Gibson <david@gibson.dropbear.id.au> wrote:
> >
> > 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.
> 
> Right. We are only really talking about libfdt here, i.e. what it
> happens to implement. I believe this is a case to be made for keeping
> the current behaviour. But in any case perhaps we should document
> libfdt's intended node-ordering semantics, separate from its actual
> behaviour. For example we could say that when adding a node, libfdt
> may put it in any order, not necessarily at the start or end.

Right, certainly the intention was that add_subnode() makes no
guarantee about how it orders the new node.  Nothing that explicitly
might be worthwhile.

> I actually like the way it works now, but perhaps I've just got used to it.
> 
> > > 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.
> 
> Another option would be to add a sequence numbers as a property to
> each node. Depending on where this goes I can update Binman.

The other option you could consider is having an explicit (ordered)
list of the things to consider as a different property somewhere.  Or,
having /aliases entries which give the numbers, along the lines of
	'hd0'	=> "/pci@XYZZY/scsi@PQRS/lun@3/..."
	'hd1'	=> "/pci@PLUGH/usb@ABCD/mass-storage@4,3/..."

Fwiw, sometime in the distant past that was suggested as a method that
the device tree could use to advertise a stable device naming/order to
Linux.  Not sure if that was ever implemented, though.

-- 
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 --]