Message-ID: <aW3IjNpBnnFE7-r7@zatzit>
Date: Mon, 19 Jan 2026 17:00:44 +1100
From: David Gibson <david@...son.dropbear.id.au>
To: Herve Codina <herve.codina@...tlin.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Ayush Singh <ayush@...gleboard.org>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	devicetree-compiler@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, devicetree-spec@...r.kernel.org,
	Hui Pu <hui.pu@...ealthcare.com>,
	Ian Ray <ian.ray@...ealthcare.com>,
	Luca Ceresoli <luca.ceresoli@...tlin.com>,
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
	Saravana Kannan <saravanak@...nel.org>
Subject: Re: [RFC PATCH 00/77] Add support for dtb metadata and addon
 device-trees

On Wed, Jan 14, 2026 at 05:18:22PM +0100, Herve Codina wrote:
> Hi Rob,
> 
> On Tue, 13 Jan 2026 12:44:07 -0600
> Rob Herring <robh@...nel.org> wrote:
> 
> > On Mon, Jan 12, 2026 at 8:20 AM Herve Codina <herve.codina@...tlin.com> wrote:
[snip]
> > >   - Patches 13..17: Introduce addons device-tree
> > >
> > >     This part introduce the new /addon/ dts keyword
> > >
> > >   - Patches 18..30: Introduce /export/ keyword and related dtb tags
> > >
> > >     This part introduces the new /export/ dts keyword (details in patch
> > >     20) and the related FDT_EXPORT_SYM and FDT_EXPORT_SYM_REF dtb tags.
> > >
> > >     FDT_EXPORT_SYM (details in patch 25) is used when the exported
> > >     symbol involved is a local node and FDT_EXPORT_SYM_REF (details in
> > >     patch 29) is used when the node involved is a non local node.  
> > 
> > More generally, would these just be "node metadata" tags?
> > 
> 
> I think we can have metadata at 3 differents levels:
> - Property
> - Node
> - Global dtb

This is a really minor point, but I don't especially like the term
"metadata" for the symbol / fixup information.  Although it's
technically accurate that it's metadata for the property bytestrings,
in most contexts "metadata" makes me think only of tree global
metadata.  By analogy, symbols and fixup information in a .so or .a
could be seen as metadata to the raw code / data bytes, but I wouldn't
normally use that term for it (whereas I might for, say, the soname or
certain .note sections).

> With the suggestion you did on patch 6 related to FDT_REF_LOCAL and if I
> understood correctly, you expect to have a kind of "container" tag to group
> metadata on each level.
> 
> Also you expect to have the ability to handle all 'for now unknown' tag
> smoothly and so, I agree, the length of the data related to a tag are
> needed to be present with the tag itself. I see to kind of tag, some with
> the length of data available in the u32 following the tag and other without
> the length encoded.
> 
> Tags without length encoded are followed by one u32 field containing data
> related to the tag. This allow to avoid a lot of 'TAG_XXX 0x04 u32_data'
> Indeed, I have the feeling that quite a lot of tags will have only one u32
> field as data part and so, having 0x04 encoded (cell aligned) each time.
> 
> A tag value is on 32bits. We can define the structure of this value.
>   - bit 31 (msb):
>      - 0: This is not a new kind to tag and so it doesn't follow this definition.
>           All existing tags are in this categorie
>      - 1: New kind of tag adopting this definition
> 
>   - bits 30..28:
>      tag data length encoding
>      0b000: No data related to the tag
>      0b001: 1 data cell (u32) directly follows the tag
>      0b010: 2 data cells (2 u32) directly follow the tag
>      ...
>      0b110: 6 data cells (6 u32) directly follow the tag
>      0b111: Tag is followed by a cell (u32) indicating the size (in bytes)
>             of data available just after this cell (including any padding
>             if needed).
> 	    Because this size include some possible padding, its value is a
>             multiple of 4 bytes.
>             The offset of the tag + 4 + size points to the next tag.
>           
> 
>   - bit 27..0
>      tag specific identifier

As noted elsewhere, I'm not necessarily opposed to having a general
length encoding.  However, for each new tag I think we need to think
carefully about whether it really is safe for older software that
doesn't understand it to just skip it.

> With that definition, the following tags can be defined:
>   - FDT_INFO_PROPERTY (new tag, length encoding): 0xf0000001
>      This tag is available after a property.
>      It is followed by a cell for the length of data, the data part is a
>      sequence of tags (and related data) giving information related to the
>      last property available before the tag.

I'd prefer to avoid an additional layer of nesting here - I'd rather
just have multiple top level tags.

>   - FDT_REF_LOCAL (new tag, 1 cell data): 0x90000002:
>      The cell after this tag is the offset in the property where a local
>      phandle is available
> 
>   - FDT_REF_PHANDLE (new tag, length encoding): 0xf0000003
>      Cf. patch 11 for definition
>      It is followed by a cell for the length of data. The data part is
>      composed of:
>        - offset (u32)
>        - label (string including \0)
>        - padding if needed to have next item aligned on 32bits
> 
> 
> With that defined, supposing the following dts example:
>   --- 8< ---
>   /* 'foo' is a reference to local node,
>    * 'bar' is a reference to an external node
>    */
>   prop = <1 2 &foo &bar1>;
>   --- 8< ---
> 
> The dtb will see the following structure:
> FDT_PROP ...
> FDT_INFO_PROPERTY (0xf0000001)
>   28 (length = (4+4)+(4+4+12) bytes)
>   FDT_REF_LOCAL (0x90000002)
>     0x8                             <--- offset of &foo
>   FDT_REF_PHANDLE (0xf0000003)
>     12 (length = 4+4+1+3 bytes)
>     0xc                             <--- offset of &bar
>     "bar1" + its \0                 <-- reference to resolve
>     0x00 0x00 0x00                  <-- 3 bytes padding
> 
> Adding FDT_TYPE_U32 later will consist in defining
> its value, probably a 0x9 family (1 cell after the tag for the
> offset value)
> 
> At any point, only looking at the higher part of the tag (i.e. 0xN.......), we
> can skip the tag and its data if don't know about the tag.
>  - 0x0: Old tag format
>     -> Error if unknown
> 
>  - 0x8 to 0xe: New format followed by 0 (0x8) to 6 cells of data
>     -> Ignore if unknown and skip the N cells of data to look at the next
> 
>  - 0xf: New format followed by 1 cell giving the size of following data.
>     -> Ignore if unknown and read the length available in the cell after the
>        tag, skip length byte of data to look at the next.
>        If the length read is not a multiple of 4: Error, invalid tag.
> 
> 
> For this series we need the container tags:
> - FDT_INFO_PROPERTY for information related to a property
>   Among known tags defined in this series, only FDT_REF_LOCAL and
>   FDT_REF_PHANDLE can be grouped into a FDT_INFO_PROPERTY.
> 
> - FDT_INFO_NODE for information related to a node
>   Among known tags defined in this series, only FDT_EXPORT_SYM_LOCAL
>   and FDT_EXPORT_SYM_REF can be grouped into a FDT_INFO_NODE.
> 
> - FDT_INFO_DTB for information related to the dtb
>   Among known tags defined in this series, only FDT_IMPORT_SYM can
>   be present into a FDT_INFO_DTB.
> 
> IMHO, the new tag FDT_BEGIN_NODE_REF related to orphan nodes doesn't
> have to be in one of those containers. Indeed, FDT_BEGIN_NODE_REF
> is more a node definition than a metadata.

That's a perfect example of a new tag that absolutely cannot be just
skipped if not understood.  Software *must* hard error if they
encounter this and don't understand it.

> Rob, does this could fit with what you expect?
> 
> If it does, is it relevant to keep the length cell available in 0xf
> family to be in bytes. It should be a multiple of 4 in all cases and
> so it can be given in the number of 32bit words instead of bytes.
> 
> Best regards,
> Hervé
> 
> 

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

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists