Re: [RFC PATCH 05/15] libfdt: Introduce fdt_first_node()

Re: [RFC PATCH 05/15] libfdt: Introduce fdt_first_node()

[Luca Ceresoli]

Hi Hervé, David,

I'm trying to review the patches that have no feedback so far.

Being new to the dtc codebase I'm mostly pointing out things that are not
clear from a newcomer point of view. I hope this helps anyway.

On Tue Feb 10, 2026 at 6:33 PM CET, Herve Codina wrote:
> In several places, libfdt assumes that a FDT_BEGIN_NODE tag is present
> at the offset 0 of the structure block.
>
> This assumption is not correct. Indeed, a FDT_NOP can be present at the
> offset 0 and this is a legit case.

I wonder whether this can be proven by showing an example, or the specs, or
whatever use case that makes sense.

> Introduce fdt_first_node() in order to get the offset of the first node
> (first FDT_BEGIN_NODE tag) available in a fdt blob.
>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  libfdt/fdt.c             | 25 +++++++++++++++++++++++++
>  libfdt/libfdt_internal.h |  1 +
>  2 files changed, 26 insertions(+)
>
> diff --git a/libfdt/fdt.c b/libfdt/fdt.c
> index 56d4dcb..676c7d7 100644
> --- a/libfdt/fdt.c
> +++ b/libfdt/fdt.c
> @@ -252,6 +252,31 @@ int fdt_check_prop_offset_(const void *fdt, int offset)
>  	return offset;
>  }
>

Even though this seems to be quite uncommon in this repository, I think
documenting new functions would be helpful, especially preconditions,
postconditions and parameter values when not obvious.

What about:

  Find the initial node with content (FDT_BEGIN_NODE) in a fdt, skipping
  FDT_NOP [and <other tags> is applicable].

  *return: pointer to the first node into the fdt or e negative error value

> +int fdt_first_node(const void *fdt)
> +{
> +	int nextoffset = 0;
> +	int offset;
> +	uint32_t tag;
> +
> +	do {
> +		offset = nextoffset;
> +		tag = fdt_next_tag(fdt, offset, &nextoffset);
> +		switch (tag) {
> +		case FDT_END_NODE:
> +		case FDT_PROP:
> +			return -FDT_ERR_BADSTRUCTURE;
> +
> +		case FDT_BEGIN_NODE:
> +			return offset;
> +
> +		default:
> +			break;
> +		}
> +	} while (tag != FDT_END);
> +
> +	return (nextoffset < 0) ? nextoffset : -FDT_ERR_NOTFOUND;
> +}
> +

Luca


--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [RFC PATCH 09/15] Introduce structured tag value definition

[Luca Ceresoli]

On Tue Feb 10, 2026 at 6:33 PM CET, Herve Codina wrote:
> The goal of structured tag values is to ease the introduction of new
> tags in future releases with the capability for an already existing
> release to ignore those structured tags. In order to do that data length
> related to the unknown tag needs to be identify.
                                         ^
					 identified

> Also a flag is present
 "Also add a flag"

> to tell an old release if this tag can be simply skipped or must lead to
> an error.
>
> Structured tag value is defined on 32bit and is defined as follow:
>
> Bits  | 31 | 30       | 29             28 | 27    0|
> ------+----+----------+-------------------+--------+
> Fields| 1  | CAN_SKIP | DATA_LNG_ENCODING | TAG_ID |
> ------+----+----------+-------------------+--------+
>
> Bit 31 is always set to 1 to identified a structured tag value.
                               ^
			       identify

> Bit 30 (CAN_SKIP) is set to 1 if the tag can be safely ignore when its
                                                         ^
							 ignored


> TAG_ID value is not a known value (unknown tag). If the CAN_SKIP bit is
> set to 0 this tag must not be ignored and an error should be reported
> when its TAG_ID value is not a known value (unknown tag).
>
> Bits 29..28 (DATA_LNG_ENCODING) indicates the length of the data related

I think "LEN" is more common than "LNG".

> to the tag. Following values are possible:
>   - 0b00: No data.
>           The tag is followed by the next tag value.
>
>   - 0b01: 1 cell data
>           The tag is followed by a 1 cell (u32) data. The next tag is
>           available after this cell.
>
>   - 0b10: 2 cells data
>           The tag is followed by a 2 cells (2 * u32) data. The next tag
>           is available after those two cells.
>
>   - 0b11: Data length encoding
>           The tag is followed by a cell (u32) indicating the size of the
>           data. This size is given in bytes. Data are available right
>           after this cell.
>
>           The next tag is available after the data. Padding is present
>           after the data in order to have the next tag aligned on 32bits.
>           This padding is not included in the size of the data.
>
> Bits 27..0 (TAG_ID) is the tag identifier defining a specific tag.
>
> Introduce the structured tag values definition and some specific tags
> reserved for tests based on this structure definition.
>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  libfdt/fdt.h | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/libfdt/fdt.h b/libfdt/fdt.h
> index a07abfc..2e07599 100644
> --- a/libfdt/fdt.h
> +++ b/libfdt/fdt.h
> @@ -49,6 +49,7 @@ struct fdt_property {
>
>  #define FDT_MAGIC	0xd00dfeed	/* 4: version, 4: total size */
>  #define FDT_TAGSIZE	sizeof(fdt32_t)
> +#define FDT_CELLSIZE	sizeof(fdt32_t)
>
>  #define FDT_BEGIN_NODE	0x1		/* Start node: full name */
>  #define FDT_END_NODE	0x2		/* End node */
> @@ -57,6 +58,28 @@ struct fdt_property {
>  #define FDT_NOP		0x4		/* nop */
>  #define FDT_END		0x9
>
> +/* Tag values flags */
> +#define FDT_TAG_STRUCTURED	(1<<31)
> +#define FDT_TAG_SKIP_SAFE	(1<<30)

This is called CAN_SKIP in the commit message and SKIP_SAFE here. Using a
consistent name would be better IMO.

> +#define FDT_TAG_DATA_MASK	(3<<28)
> +#define FDT_TAG_DATA_NONE	(0<<28)
> +#define FDT_TAG_DATA_1CELL	(1<<28)
> +#define FDT_TAG_DATA_2CELLS	(2<<28)
> +#define FDT_TAG_DATA_LNG	(3<<28)

I find _LNG (or _LEN) misleading: this is not the length, but rather an
enum value telling you the length is stored in the next cell. What about
FDT_TAG_DATA_VARLEN?

Otherwise looks good.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [RFC PATCH 06/15] libfdt: Don't assume that a FDT_BEGIN_NODE tag is available at offset 0

[Luca Ceresoli]

On Tue Feb 10, 2026 at 6:33 PM CET, Herve Codina wrote:
> In several places, libfdt assumes that a FDT_BEGIN_NODE tag is present
> at the offset 0 of the structure block.
>
> This assumption is not correct. Indeed, a FDT_NOP can be present at the
> offset 0 and this is a legit case.
>
> fdt_first_node() has been introduce recently to get the offset of the
                            ^
			    introduced

> first node (first FDT_BEGIN_NODE) in a fdt blob.
>
> Use this function to get the first node offset instead of looking for
> this node at offset 0.
>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  libfdt/fdt.c    | 10 ++++++++--
>  libfdt/fdt_ro.c | 16 +++++++++++++---
>  libfdt/fdt_rw.c |  6 ++++++
>  3 files changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/libfdt/fdt.c b/libfdt/fdt.c
> index 676c7d7..ff2fa6c 100644
> --- a/libfdt/fdt.c
> +++ b/libfdt/fdt.c
> @@ -279,11 +279,17 @@ int fdt_first_node(const void *fdt)
>
>  int fdt_next_node(const void *fdt, int offset, int *depth)
>  {
> -	int nextoffset = 0;
> +	int nextoffset = offset;
>  	uint32_t tag;
>
> +	if (offset <= 0) {

What is the difference between 0 and a engative value?

This is where the parameter value is not obvious to the newcomer and I'd
love seeing it concisely documented.

Otherwise this patch looks good to me.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [RFC PATCH 10/15] fdtdump: Handle unknown tags

[Luca Ceresoli]

On Tue Feb 10, 2026 at 6:33 PM CET, Herve Codina wrote:
> The structured tag value definition introduced recently gives the
> ability to ignore unknown tags without any error when they are read.
>
> Handle those structured tag.

How? This sentence is vague, what about:

  Allow dumping the unknown tags or not based on a command line flag.

> --- a/fdtdump.c
> +++ b/fdtdump.c
> @@ -44,7 +44,7 @@ static const char *tagname(uint32_t tag)
>  #define dumpf(fmt, args...) \
>  	do { if (debug) printf("// " fmt, ## args); } while (0)
>
> -static void dump_blob(void *blob, bool debug)
> +static void dump_blob(void *blob, bool debug, int dump_unknown)
>  {
>  	uintptr_t blob_off = (uintptr_t)blob;
>  	struct fdt_header *bph = blob;
> @@ -146,20 +146,55 @@ static void dump_blob(void *blob, bool debug)
>  			continue;
>  		}
>
> +		if ((tag & FDT_TAG_STRUCTURED) && (tag & FDT_TAG_SKIP_SAFE)) {
> +			sz = 0;
> +			switch (tag & FDT_TAG_DATA_MASK) {
> +			case FDT_TAG_DATA_NONE:
> +				break;
> +			case FDT_TAG_DATA_1CELL:
> +				sz = FDT_CELLSIZE;
> +				break;
> +			case FDT_TAG_DATA_2CELLS:
> +				sz = 2 * FDT_CELLSIZE;
> +				break;
> +			case FDT_TAG_DATA_LNG:
> +				/* Get the length */
> +				sz = fdt32_to_cpu(GET_CELL(p));
> +				break;
> +			}
> +
> +			if (dump_unknown) {
> +				printf("%*s// Unknown tag ignored: 0x%08"PRIx32", data lng %d",

As before, I'd use "len" instead of "lng".

> +				       depth * shift, "", tag, sz);
> +				if (dump_unknown > 1 && sz != 0) {
> +					printf(" ");
> +					for (i = 0; i < sz; i++)
> +						printf("%02hhx", *(p + i));
> +				}
> +				printf("\n");
> +			}
> +
> +			/* Skip the data bytes */
> +			p = PALIGN(p + sz, 4);
> +			continue;
> +		}
> +
>  		die("** Unknown tag 0x%08"PRIx32"\n", tag);
>  	}
>  }
>
>  /* Usage related data. */
>  static const char usage_synopsis[] = "fdtdump [options] <file>";
> -static const char usage_short_opts[] = "ds" USAGE_COMMON_SHORT_OPTS;
> +static const char usage_short_opts[] = "dus" USAGE_COMMON_SHORT_OPTS;
>  static struct option const usage_long_opts[] = {
>  	{"debug",            no_argument, NULL, 'd'},
> +	{"unknown",          no_argument, NULL, 'u'},
>  	{"scan",             no_argument, NULL, 's'},
>  	USAGE_COMMON_LONG_OPTS
>  };
>  static const char * const usage_opts_help[] = {
>  	"Dump debug information while decoding the file",
> +	"Dump unknown tags information while decoding the file (-uu to have data)",
                                                                       ^
								       dump

> --- a/tests/trees.S
> +++ b/tests/trees.S
> @@ -328,3 +328,113 @@ named_root_strings:
>  named_root_strings_end:
>
>  named_root_end:
> +
> +
> +	/* Tree with "unknown" tags that can be skipped
> +	 * Use a really future dtb version to check version downgrade on
> +	 * modification.
> +	 */
> +	treehdr_vers	unknown_tags_can_skip 0xffffffff 0x10
> +	empty_rsvmap	unknown_tags_can_skip
> +
> +unknown_tags_can_skip_struct:
> +	fdtlong	FDT_TEST_1CELL_CAN_SKIP
> +	fdtlong	0x1
> +
> +	beginn	""
> +		fdtlong	FDT_TEST_NONE_CAN_SKIP
> +
> +		propu32	unknown_tags_can_skip, prop_int, 1
> +
> +		fdtlong	FDT_TEST_1CELL_CAN_SKIP
> +		fdtlong	0x11
> +
> +		propstr	unknown_tags_can_skip, prop_str, "abcd"
> +
> +		fdtlong	FDT_TEST_2CELLS_CAN_SKIP
> +		fdtlong	0x12
> +		fdtlong	0x12

Can you use different values here, just to make the test slightly more
robust? Just in case parsing ends up on the wrong cell, as unlikely as it
can be.

Same in various places below.

> +
> +		fdtlong	FDT_TEST_LNG_CAN_SKIP
> +		fdtlong	3
> +		.byte 0x13
> +		.byte 0x13
> +		.byte 0x13
> +		.byte 0 /* padding */
> +
> +		beginn	"subnode1"
> +			propu64	unknown_tags_can_skip, prop_int, 1, 2
> +			fdtlong	FDT_TEST_NONE_CAN_SKIP
> +		endn
> +
> +		beginn	"subnode2"
> +			fdtlong	FDT_TEST_1CELL_CAN_SKIP
> +			fdtlong	0x121
> +			propu64	unknown_tags_can_skip, prop_int1, 1, 2
> +			fdtlong	FDT_TEST_1CELL_CAN_SKIP
> +			fdtlong	0x122
> +			propu64	unknown_tags_can_skip, prop_int2, 1, 2
> +			beginn	"subsubnode"
> +				fdtlong	FDT_TEST_1CELL_CAN_SKIP
> +				fdtlong	0x123
> +				propu64	unknown_tags_can_skip, prop_int, 1, 2

As before, you are using values 1 and 2 for all the properties, I'd use
different values, and possibly even with different amounts of cells in
properties.

Other than these two minor nits, this patch looks very good to me.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [RFC PATCH 11/15] flattree: Handle unknown tags

[Luca Ceresoli]

On Tue Feb 10, 2026 at 6:33 PM CET, Herve Codina wrote:
> The structured tag value definition introduced recently gives the
> ability to ignore unknown tags without any error when they are read.
>
> Handle those structured tag.
>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

> +++ b/tests/unknown_tags_can_skip.dtb.dts.expect
> @@ -0,0 +1,19 @@
> +/dts-v1/;
> +
> +/ {
> +	prop-int = <0x01>;
> +	prop-str = "abcd";
> +
> +	subnode1 {
> +		prop-int = <0x01 0x02>;
> +	};
> +
> +	subnode2 {
> +		prop-int1 = <0x01 0x02>;
> +		prop-int2 = <0x01 0x02>;
> +
> +		subsubnode {
> +			prop-int = <0x01 0x02>;
> +		};
> +	};
> +};

Of course this will need updating in case you adopt the changes to the
tests I suggested in patch 10.

With that done:
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [RFC PATCH 12/15] libfdt: Handle unknown tags in fdt_get_next()

[Luca Ceresoli]

On Tue Feb 10, 2026 at 6:33 PM CET, Herve Codina wrote:
> The structured tag value definition introduced recently gives the
> ability to ignore unknown tags without any error when they are read.
>
> libfdt uses fdt_get_next() to get a tag.

I think you mean fdt_next_tag(), here and elsewhere in the commit message.

>
> Filtering out tags that should be ignored in fdt_get_next() allows to
> have the filtering done globally and allows, in future release, to have
                                                         ^
							 releases

> a central place to add new known tags that should not be filtered out.
>
> An already known tag exists with the meaning of "just ignore". This tag
> is FDT_NOP. fdt_get_next() callers already handle the FDT_NOP tag.
>
> Avoid unneeded modification at callers side and use a fake FDT_NOP tag
> when an unknown tag that should be ignored is encountered.
>
> Add also fdt_get_next_() internal function for callers who need to know

And here fdt_next_tag_()?

> if the FDT_NOP tag returned is a real FDT_NOP or a fake FDT_NOP due to
> an unknown tag.
>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Looks good otherwise, so with those fixed:
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [RFC PATCH 13/15] libfdt: Introduce fdt_ptr_offset_

[Luca Ceresoli]

On Tue Feb 10, 2026 at 6:33 PM CET, Herve Codina wrote:
> libfdt provides internal used helpers to convert an offset to a pointer
                  ^^^^^^^^^^^^^^^^^^^^^

Just "internal helpers"


> but nothing to do the reverse operation.
>
> Fill this lack and introduce the fdt_ptr_offset_() internal helper to

"Fill this gap"? Or just "Introduce the..." would be enough.

With those fixed:
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [RFC PATCH 14/15] libfdt: Handle unknown tags on dtb modifications

[Luca Ceresoli]

On Tue Feb 10, 2026 at 6:33 PM CET, Herve Codina wrote:
> The structured tag value definition introduced recently gives the
> ability to ignore unknown tags without any error.
>
> When the dtb is modified those unknown tags have to be taken into
> account.
>
> First, depending on the unknown tag location, the item associated with
> the tag is identified:
>   - An unknown tag located just after a FDT_BEGIN_NODE is related to the
>     node.
>
>   - An unknown tag located just after a FDT_PROP is related to the
>     property.
>
>   - An unknown tag out of any node (i.e located before the first
>     FDT_BEGIN_NODE or after the last FDT_END_NODE is a global tag
                                                   ^
						   missing ')'
>     related to the dtb itself.

Out of curiosity, is there a real use case for global tags after
FDT_END_NODE?

> Then, if we are allowed to write a dtb containing unknown tags, the
> following rules are used:
>   - When a property is modified, tags related to this property are
>     removed and the dtb version is downgraded.
>
>   - When a property is removed, tags related to this property are
>     obviously removed. The dtb version is kept unchanged.
>
>   - When a property or a node is added, obviously no unknown tags are
>     added and the dtb version is kept unchanged.
>
>   - When a node is removed, tags related to this node are obviously
>     removed. The dtb version is kept unchanged.
>
>   - Adding, removing or modifying a property is not considered as a node
>     modification and so, those operations have no impacts on unknown
>     tags related to the node. Those node related tags are kept unchanged.
>
>   - The only modification considered as a node modification is setting
>     its name. We consider that this operation has no impact on tags
>     related to the node. Here also, those node related tags and the
>     dtb version are kept unchanged.
>
>   - Global (dtb related) unknown tags are kept unchanged regardless the
>     modification done.
>
> Implement those rules when a dtb is modified.
>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

With the tests updated as per comments in patch 10:
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [RFC PATCH 15/15] Introduce v18 dtb version

[Luca Ceresoli]

On Tue Feb 10, 2026 at 6:33 PM CET, Herve Codina wrote:
> This v18 version will add support for
>   - Structured tags.
>     Those tags value definition will allow old libfdt, dtc and other
>     tools to skip unknown tags if encountered in future dtb version.

"old" seems to imply that versions released before today will be able to
wkip unknown tags. I think this should be clarified along the lines of:

  libfdt, dtc and other tools implementing version v18 will be able to wkip
  unknown tags in dtbs generated with later versions of dtc

>   - dt_flags header field.
>     For now this flag field is set to 0. It is a placeholder for future
>     dtb version and could be used to store some dtb related information
>     such as the kind of dtb.

Is this intended for DT addons?

You may mention a realistiv use case here.

>
>   - last_comp_version_w header field.
>     This field is similar to last_comp_version but for writing.
>     It contains the lowest version of the devicetree data structure with
>     which the version used can safely perform modifications (taking into
>     account following rules related to unknown tags).
>     If this lowest version is greater than the last known supported
>     version, modification are simply forbidden and lead to a
>     FDT_ERR_BADVERSION error.
>
> For modification, when an unknown tag that can be skipped is involved
> and last_comp_version_w allows modifications, the following rules
> applies:
  ^
  apply

>   - When a property is modified, tags related to this property are
>     removed and the dtb version is downgraded.
>
>   - When a property is removed, tags related to this property are
>     obviously removed. The dtb version is kept unchanged.
>
>   - When a property or a node is added, obviously no unknown tags are
>     added and the dtb version is kept unchanged.
>
>   - When a node is removed, tags related to this node are obviously
>     removed. The dtb version is kept unchanged.
>
>   - Adding, removing or modifying a property is not considered as a node
>     modification and so, those operations have no impacts on unknown
>     tags related to the node. Those node related tags are kept
>     unchanged.
>
>   - The only modification considered as a node modification is setting
>     its name. We consider that this operation has no impact on tags
>     related to the node. Here also, those node related tags and the dtb
>     version are kept unchanged.
>
>   - Global (dtb related) unknown tags are kept unchanged regardless the
>     modification done.
>
> In all cases, if unknown tags are not involved in a modification, the
> dtb version is not downgraded when the modification is node.
                                                         ^
							 made?


>
> Compared to previous version, it is worth noting that the dtb is not
                                                               ^
				                        "dtb version"

> downgrade for all modification but only when unknown tags are removed
  ^
  downgraded for any


> due a property modification.
  ^
  "due to a ..."

I'm not sure I got what you mean by the initial "Compared to previous
version". Version(s) of what?

If I just remove those 4 words the sentence seems OK to me BTW.

> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

I am unable to fully understand the implications of this patch, but the
code seems OK to me as far as I can tell.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [RFC PATCH 05/15] libfdt: Introduce fdt_first_node()

[Herve Codina]

Hi Luca, David,

On Wed, 01 Apr 2026 17:11:15 +0200
"Luca Ceresoli" <luca.ceresoli@bootlin.com> wrote:

> Hi Hervé, David,
> 
> I'm trying to review the patches that have no feedback so far.
> 
> Being new to the dtc codebase I'm mostly pointing out things that are not
> clear from a newcomer point of view. I hope this helps anyway.
> 
> On Tue Feb 10, 2026 at 6:33 PM CET, Herve Codina wrote:
> > In several places, libfdt assumes that a FDT_BEGIN_NODE tag is present
> > at the offset 0 of the structure block.
> >
> > This assumption is not correct. Indeed, a FDT_NOP can be present at the
> > offset 0 and this is a legit case.  
> 
> I wonder whether this can be proven by showing an example, or the specs, or
> whatever use case that makes sense.

I can point the device-tree specification and add the following:
--- 8< ---
Indeed, the FDT_NOP definition according to the device-tree specification [0]
is the following:
   The FDT_NOP token will be ignored by any program parsing the device
   tree. This token has no extra data; so it is followed immediately by
   the next token, which can be any valid token. A property or node
   definition in the tree can be overwritten with FDT_NOP tokens to
   remove it from the tree without needing to move other sections of
   the tree’s representation in the devicetree blob.

Nothing refers to any location for this tag and it has to be simply ignored.
Having this tag at offset 0 doesn't make an exception, the tag has to be
ignored.

[0] https://github.com/devicetree-org/devicetree-specification/blob/main/source/chapter5-flattened-format.rst?plain=1#L317
--- 8< ---

> 
> > Introduce fdt_first_node() in order to get the offset of the first node
> > (first FDT_BEGIN_NODE tag) available in a fdt blob.
> >
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > ---
> >  libfdt/fdt.c             | 25 +++++++++++++++++++++++++
> >  libfdt/libfdt_internal.h |  1 +
> >  2 files changed, 26 insertions(+)
> >
> > diff --git a/libfdt/fdt.c b/libfdt/fdt.c
> > index 56d4dcb..676c7d7 100644
> > --- a/libfdt/fdt.c
> > +++ b/libfdt/fdt.c
> > @@ -252,6 +252,31 @@ int fdt_check_prop_offset_(const void *fdt, int offset)
> >  	return offset;
> >  }
> >  
> 
> Even though this seems to be quite uncommon in this repository, I think
> documenting new functions would be helpful, especially preconditions,
> postconditions and parameter values when not obvious.
> 
> What about:
> 
>   Find the initial node with content (FDT_BEGIN_NODE) in a fdt, skipping
>   FDT_NOP [and <other tags> is applicable].
> 
>   *return: pointer to the first node into the fdt or e negative error value

As you already said, documentation is quite uncommon here.

I would prefer to keep the code consistent and avoid adding documentation for
some functions and not others.

On the other side, adding some documentation could be beneficial.

David, what do you prefer?

Should I document new functions I introduce?

Best regards,
Hervé

Re: [RFC PATCH 06/15] libfdt: Don't assume that a FDT_BEGIN_NODE tag is available at offset 0

[Herve Codina]

Hi Luca, David,

On Wed, 01 Apr 2026 17:11:42 +0200
"Luca Ceresoli" <luca.ceresoli@bootlin.com> wrote:

> On Tue Feb 10, 2026 at 6:33 PM CET, Herve Codina wrote:
> > In several places, libfdt assumes that a FDT_BEGIN_NODE tag is present
> > at the offset 0 of the structure block.
> >
> > This assumption is not correct. Indeed, a FDT_NOP can be present at the
> > offset 0 and this is a legit case.
> >
> > fdt_first_node() has been introduce recently to get the offset of the  
>                             ^
> 			    introduced

Will be fixed in the next iteration.

...
> >  int fdt_next_node(const void *fdt, int offset, int *depth)
> >  {
> > -	int nextoffset = 0;
> > +	int nextoffset = offset;
> >  	uint32_t tag;
> >
> > +	if (offset <= 0) {  
> 
> What is the difference between 0 and a engative value?

I will add a comment in the code.

0 means that we want the next node from the first node.

The negative value is an invalid value. In that case we start from the
first node. This was the behavior of the code without my modification and
I kept the same behavior in that case.

> 
> This is where the parameter value is not obvious to the newcomer and I'd
> love seeing it concisely documented.

David, is functions documentation expected ?

I can add to this documentation if you want it.

Best regards,
Hervé

Re: [RFC PATCH 09/15] Introduce structured tag value definition

[Herve Codina]

Hi Luca,

On Wed, 01 Apr 2026 17:11:35 +0200
"Luca Ceresoli" <luca.ceresoli@bootlin.com> wrote:

> On Tue Feb 10, 2026 at 6:33 PM CET, Herve Codina wrote:
> > The goal of structured tag values is to ease the introduction of new
> > tags in future releases with the capability for an already existing
> > release to ignore those structured tags. In order to do that data length
> > related to the unknown tag needs to be identify.  
>                                          ^
> 					 identified

Will be fixed in the next iteration.

> 
> > Also a flag is present  
>  "Also add a flag"

Will be updated in the next iteration.

> 
> > to tell an old release if this tag can be simply skipped or must lead to
> > an error.
> >
> > Structured tag value is defined on 32bit and is defined as follow:
> >
> > Bits  | 31 | 30       | 29             28 | 27    0|
> > ------+----+----------+-------------------+--------+
> > Fields| 1  | CAN_SKIP | DATA_LNG_ENCODING | TAG_ID |
> > ------+----+----------+-------------------+--------+
> >
> > Bit 31 is always set to 1 to identified a structured tag value.  
>                                ^
> 			       identify
> 
> > Bit 30 (CAN_SKIP) is set to 1 if the tag can be safely ignore when its  
>                                                          ^
> 							 ignored

Both will be fixed in the next iteration.

> 
> 
> > TAG_ID value is not a known value (unknown tag). If the CAN_SKIP bit is
> > set to 0 this tag must not be ignored and an error should be reported
> > when its TAG_ID value is not a known value (unknown tag).
> >
> > Bits 29..28 (DATA_LNG_ENCODING) indicates the length of the data related  
> 
> I think "LEN" is more common than "LNG".

Agree, will be changed.

...
> >
> > +/* Tag values flags */
> > +#define FDT_TAG_STRUCTURED	(1<<31)
> > +#define FDT_TAG_SKIP_SAFE	(1<<30)  
> 
> This is called CAN_SKIP in the commit message and SKIP_SAFE here. Using a
> consistent name would be better IMO.

I will use SKIP_SAFE and so update the commit message accordingly in the next
iteration.

> 
> > +#define FDT_TAG_DATA_MASK	(3<<28)
> > +#define FDT_TAG_DATA_NONE	(0<<28)
> > +#define FDT_TAG_DATA_1CELL	(1<<28)
> > +#define FDT_TAG_DATA_2CELLS	(2<<28)
> > +#define FDT_TAG_DATA_LNG	(3<<28)  
> 
> I find _LNG (or _LEN) misleading: this is not the length, but rather an
> enum value telling you the length is stored in the next cell. What about
> FDT_TAG_DATA_VARLEN?

Yes indeed, VARLEN is better.
I will use FDT_TAG_DATA_VARLEN in the next iteration.

Best regards,
Hervé

Re: [RFC PATCH 10/15] fdtdump: Handle unknown tags

[Herve Codina]

Hi Luca,

On Wed, 01 Apr 2026 17:15:09 +0200
"Luca Ceresoli" <luca.ceresoli@bootlin.com> wrote:

> On Tue Feb 10, 2026 at 6:33 PM CET, Herve Codina wrote:
> > The structured tag value definition introduced recently gives the
> > ability to ignore unknown tags without any error when they are read.
> >
> > Handle those structured tag.  
> 
> How? This sentence is vague, what about:
> 
>   Allow dumping the unknown tags or not based on a command line flag.

Hum indeed but I don't fully agree with your proposal.

The patch adds support for structured tag in fdtdump and introduce the '-u'
option to dump unknown tags which can be safely ignored.

What do you think about:

    The structured tag value definition introduced recently gives the
    ability to ignore unknown tags without any error when they are read.

    Add support for those structured tags in fdtdump and introduce a
    command line option to dump unknown tags that should be ignored.

> 
> > --- a/fdtdump.c
> > +++ b/fdtdump.c
> > @@ -44,7 +44,7 @@ static const char *tagname(uint32_t tag)
> >  #define dumpf(fmt, args...) \
> >  	do { if (debug) printf("// " fmt, ## args); } while (0)
> >
> > -static void dump_blob(void *blob, bool debug)
> > +static void dump_blob(void *blob, bool debug, int dump_unknown)
> >  {
> >  	uintptr_t blob_off = (uintptr_t)blob;
> >  	struct fdt_header *bph = blob;
> > @@ -146,20 +146,55 @@ static void dump_blob(void *blob, bool debug)
> >  			continue;
> >  		}
> >
> > +		if ((tag & FDT_TAG_STRUCTURED) && (tag & FDT_TAG_SKIP_SAFE)) {
> > +			sz = 0;
> > +			switch (tag & FDT_TAG_DATA_MASK) {
> > +			case FDT_TAG_DATA_NONE:
> > +				break;
> > +			case FDT_TAG_DATA_1CELL:
> > +				sz = FDT_CELLSIZE;
> > +				break;
> > +			case FDT_TAG_DATA_2CELLS:
> > +				sz = 2 * FDT_CELLSIZE;
> > +				break;
> > +			case FDT_TAG_DATA_LNG:
> > +				/* Get the length */
> > +				sz = fdt32_to_cpu(GET_CELL(p));
> > +				break;
> > +			}
> > +
> > +			if (dump_unknown) {
> > +				printf("%*s// Unknown tag ignored: 0x%08"PRIx32", data lng %d",  
> 
> As before, I'd use "len" instead of "lng".

I will change in the next iteration.

> 
> > +				       depth * shift, "", tag, sz);
> > +				if (dump_unknown > 1 && sz != 0) {
> > +					printf(" ");
> > +					for (i = 0; i < sz; i++)
> > +						printf("%02hhx", *(p + i));
> > +				}
> > +				printf("\n");
> > +			}
> > +
> > +			/* Skip the data bytes */
> > +			p = PALIGN(p + sz, 4);
> > +			continue;
> > +		}
> > +
> >  		die("** Unknown tag 0x%08"PRIx32"\n", tag);
> >  	}
> >  }
> >
> >  /* Usage related data. */
> >  static const char usage_synopsis[] = "fdtdump [options] <file>";
> > -static const char usage_short_opts[] = "ds" USAGE_COMMON_SHORT_OPTS;
> > +static const char usage_short_opts[] = "dus" USAGE_COMMON_SHORT_OPTS;
> >  static struct option const usage_long_opts[] = {
> >  	{"debug",            no_argument, NULL, 'd'},
> > +	{"unknown",          no_argument, NULL, 'u'},
> >  	{"scan",             no_argument, NULL, 's'},
> >  	USAGE_COMMON_LONG_OPTS
> >  };
> >  static const char * const usage_opts_help[] = {
> >  	"Dump debug information while decoding the file",
> > +	"Dump unknown tags information while decoding the file (-uu to have data)",  
>                                                                        ^
> 								       dump

Will be fixed in the next iteration.

> 
> > --- a/tests/trees.S
> > +++ b/tests/trees.S
> > @@ -328,3 +328,113 @@ named_root_strings:
> >  named_root_strings_end:
> >
> >  named_root_end:
> > +
> > +
> > +	/* Tree with "unknown" tags that can be skipped
> > +	 * Use a really future dtb version to check version downgrade on
> > +	 * modification.
> > +	 */
> > +	treehdr_vers	unknown_tags_can_skip 0xffffffff 0x10
> > +	empty_rsvmap	unknown_tags_can_skip
> > +
> > +unknown_tags_can_skip_struct:
> > +	fdtlong	FDT_TEST_1CELL_CAN_SKIP
> > +	fdtlong	0x1
> > +
> > +	beginn	""
> > +		fdtlong	FDT_TEST_NONE_CAN_SKIP
> > +
> > +		propu32	unknown_tags_can_skip, prop_int, 1
> > +
> > +		fdtlong	FDT_TEST_1CELL_CAN_SKIP
> > +		fdtlong	0x11
> > +
> > +		propstr	unknown_tags_can_skip, prop_str, "abcd"
> > +
> > +		fdtlong	FDT_TEST_2CELLS_CAN_SKIP
> > +		fdtlong	0x12
> > +		fdtlong	0x12  
> 
> Can you use different values here, just to make the test slightly more
> robust? Just in case parsing ends up on the wrong cell, as unlikely as it
> can be.

Yes indeed, good proposal. I will update patterns.

> 
> Same in various places below.
> 
> > +
> > +		fdtlong	FDT_TEST_LNG_CAN_SKIP
> > +		fdtlong	3
> > +		.byte 0x13
> > +		.byte 0x13
> > +		.byte 0x13
> > +		.byte 0 /* padding */
> > +
> > +		beginn	"subnode1"
> > +			propu64	unknown_tags_can_skip, prop_int, 1, 2
> > +			fdtlong	FDT_TEST_NONE_CAN_SKIP
> > +		endn
> > +
> > +		beginn	"subnode2"
> > +			fdtlong	FDT_TEST_1CELL_CAN_SKIP
> > +			fdtlong	0x121
> > +			propu64	unknown_tags_can_skip, prop_int1, 1, 2
> > +			fdtlong	FDT_TEST_1CELL_CAN_SKIP
> > +			fdtlong	0x122
> > +			propu64	unknown_tags_can_skip, prop_int2, 1, 2
> > +			beginn	"subsubnode"
> > +				fdtlong	FDT_TEST_1CELL_CAN_SKIP
> > +				fdtlong	0x123
> > +				propu64	unknown_tags_can_skip, prop_int, 1, 2  
> 
> As before, you are using values 1 and 2 for all the properties, I'd use
> different values, and possibly even with different amounts of cells in
> properties.

I will update too.

Best regards,
Hervé

Re: [RFC PATCH 12/15] libfdt: Handle unknown tags in fdt_get_next()

[Herve Codina]

Hi Luca,

On Wed, 01 Apr 2026 17:17:46 +0200
"Luca Ceresoli" <luca.ceresoli@bootlin.com> wrote:

> On Tue Feb 10, 2026 at 6:33 PM CET, Herve Codina wrote:
> > The structured tag value definition introduced recently gives the
> > ability to ignore unknown tags without any error when they are read.
> >
> > libfdt uses fdt_get_next() to get a tag.  
> 
> I think you mean fdt_next_tag(), here and elsewhere in the commit message.
> 
> >
> > Filtering out tags that should be ignored in fdt_get_next() allows to
> > have the filtering done globally and allows, in future release, to have  
>                                                          ^
> 							 releases
> 
> > a central place to add new known tags that should not be filtered out.
> >
> > An already known tag exists with the meaning of "just ignore". This tag
> > is FDT_NOP. fdt_get_next() callers already handle the FDT_NOP tag.
> >
> > Avoid unneeded modification at callers side and use a fake FDT_NOP tag
> > when an unknown tag that should be ignored is encountered.
> >
> > Add also fdt_get_next_() internal function for callers who need to know  
> 
> And here fdt_next_tag_()?
> 

You're perfectly right. fdt_next_tag() and fdt_next_tag_() are the correct
functions.

This will be fixed in the next iteration as well as the commit title (same
issue) and the 'release' typo.

Best regards,
Hervé

Re: [RFC PATCH 14/15] libfdt: Handle unknown tags on dtb modifications

[Herve Codina]

Hi Luca,

On Wed, 01 Apr 2026 17:18:54 +0200
"Luca Ceresoli" <luca.ceresoli@bootlin.com> wrote:

...
> >   - An unknown tag out of any node (i.e located before the first
> >     FDT_BEGIN_NODE or after the last FDT_END_NODE is a global tag  
>                                                    ^
> 						   missing ')'
> >     related to the dtb itself.  
> 
> Out of curiosity, is there a real use case for global tags after
> FDT_END_NODE?

Well what could be use cases in the future?

We talk about unknown tag and nothing prevent an unknown tag to be after
the last FDT_END_NODE tag in the future.

In my RFC series adding support for addons, I added FDT_IMPORT_SYM tags at
the end of the addon dtb and so a global tags were available after a
FDT_END_NODE tag.

In the end of the commit log introducing FDT_IMPORT_SYM tags [0], the
location of those tags is mentioned:
--- 8< ---
   If FDT_IMPORT_SYM tags are present in the dtb, they are present after
   the root node definition (i.e. after the FDT_END_NODE related to the
   first FDT_BEGIN_NODE).
--- 8< ---

Also in tests related to import symbols [0], you can have a look look at
the tests/metadata_importsyms.dtb.expect file and you will find:
--- 8< ---
    --- /dev/null
    +++ b/tests/metadata_importsyms.dtb.expect
    @@ -0,0 +1,8 @@
    +/dts-v1/;
    +/addon/;
    +
    +/ {
    +    prop = <0x00000001>;
    +};
    +// [FDT_IMPORT_SYM] 'base_a' (foo,bar)
    +// [FDT_IMPORT_SYM] 'base_b' (foo,baz)
--- 8< ---

This is the expected result when the metadata_importsyms.dtb is dumped using
fdtdump.

fdtdump dumps a dtb in a linear way starting from the beginning to the end
of file.

The FDT_END_NODE tag is represented by the '};' sequence (end of node).
FDT_IMPORT_SYM tags are present after the end of node and so between the
FDT_END_NODE tag and the FDT_END tag.

Not sure I will keep those tags at the end of dtb when I rework the series
on top of "structured tags" but well, this was a real use case.

[0] https://lore.kernel.org/devicetree-compiler/20260112142009.1006236-36-herve.codina@bootlin.com/
[1] https://lore.kernel.org/devicetree-compiler/20260112142009.1006236-37-herve.codina@bootlin.com/

Best regards,
Hervé

Re: [RFC PATCH 10/15] fdtdump: Handle unknown tags

[Luca Ceresoli]

On Tue Apr 7, 2026 at 4:03 PM CEST, Herve Codina wrote:
> Hi Luca,
>
> On Wed, 01 Apr 2026 17:15:09 +0200
> "Luca Ceresoli" <luca.ceresoli@bootlin.com> wrote:
>
>> On Tue Feb 10, 2026 at 6:33 PM CET, Herve Codina wrote:
>> > The structured tag value definition introduced recently gives the
>> > ability to ignore unknown tags without any error when they are read.
>> >
>> > Handle those structured tag.
>>
>> How? This sentence is vague, what about:
>>
>>   Allow dumping the unknown tags or not based on a command line flag.
>
> Hum indeed but I don't fully agree with your proposal.
>
> The patch adds support for structured tag in fdtdump and introduce the '-u'
> option to dump unknown tags which can be safely ignored.
>
> What do you think about:
>
>     The structured tag value definition introduced recently gives the
>     ability to ignore unknown tags without any error when they are read.
>
>     Add support for those structured tags in fdtdump and introduce a
>     command line option to dump unknown tags that should be ignored.

Looks way better now, thanks!

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [RFC PATCH 15/15] Introduce v18 dtb version

[Herve Codina]

Hi Luca,

On Wed, 01 Apr 2026 17:19:09 +0200
"Luca Ceresoli" <luca.ceresoli@bootlin.com> wrote:

> On Tue Feb 10, 2026 at 6:33 PM CET, Herve Codina wrote:
> > This v18 version will add support for
> >   - Structured tags.
> >     Those tags value definition will allow old libfdt, dtc and other
> >     tools to skip unknown tags if encountered in future dtb version.  
> 
> "old" seems to imply that versions released before today will be able to
> wkip unknown tags. I think this should be clarified along the lines of:
> 
>   libfdt, dtc and other tools implementing version v18 will be able to wkip
>   unknown tags in dtbs generated with later versions of dtc

Yes, I will add this clarification in the next iteration.

> 
> >   - dt_flags header field.
> >     For now this flag field is set to 0. It is a placeholder for future
> >     dtb version and could be used to store some dtb related information
> >     such as the kind of dtb.  
> 
> Is this intended for DT addons?
> 
> You may mention a realistiv use case here.

Intended, maybe not. Used by addons, yes, for sure.

What do you think if I add the following:
    For instance, the future addons format will use this field to
    clearly identify that the dtb is an addon dtb.

...
> >
> > Compared to previous version, it is worth noting that the dtb is not  
>                                                                ^
> 				                        "dtb version"
> 
> > downgrade for all modification but only when unknown tags are removed  
>   ^
>   downgraded for any
> 
> 
> > due a property modification.  
>   ^
>   "due to a ..."
> 
> I'm not sure I got what you mean by the initial "Compared to previous
> version". Version(s) of what?
> 
> If I just remove those 4 words the sentence seems OK to me BTW.

Is the following clearer?

    It is worth noting that with this v18 version, the dtb version is not
    downgraded for any modification but only when unknown tags are removed
    due to a property modification. In v17 or older version any modification
    led to a dtb version downgrade.

Best regards,
Hervé

Re: [RFC PATCH 15/15] Introduce v18 dtb version

[Luca Ceresoli]

On Tue Apr 7, 2026 at 6:44 PM CEST, Herve Codina wrote:
> Hi Luca,
>
> On Wed, 01 Apr 2026 17:19:09 +0200
> "Luca Ceresoli" <luca.ceresoli@bootlin.com> wrote:
>
>> On Tue Feb 10, 2026 at 6:33 PM CET, Herve Codina wrote:
>> > This v18 version will add support for
>> >   - Structured tags.
>> >     Those tags value definition will allow old libfdt, dtc and other
>> >     tools to skip unknown tags if encountered in future dtb version.
>>
>> "old" seems to imply that versions released before today will be able to
>> wkip unknown tags. I think this should be clarified along the lines of:
>>
>>   libfdt, dtc and other tools implementing version v18 will be able to wkip
>>   unknown tags in dtbs generated with later versions of dtc
>
> Yes, I will add this clarification in the next iteration.
>
>>
>> >   - dt_flags header field.
>> >     For now this flag field is set to 0. It is a placeholder for future
>> >     dtb version and could be used to store some dtb related information
>> >     such as the kind of dtb.
>>
>> Is this intended for DT addons?
>>
>> You may mention a realistiv use case here.
>
> Intended, maybe not. Used by addons, yes, for sure.
>
> What do you think if I add the following:
>     For instance, the future addons format will use this field to
>     clearly identify that the dtb is an addon dtb.

This clarifies a lot to me, so I think it can be added to the commit
message.

>> > Compared to previous version, it is worth noting that the dtb is not
>>                                                                ^
>> 				                        "dtb version"
>>
>> > downgrade for all modification but only when unknown tags are removed
>>   ^
>>   downgraded for any
>>
>>
>> > due a property modification.
>>   ^
>>   "due to a ..."
>>
>> I'm not sure I got what you mean by the initial "Compared to previous
>> version". Version(s) of what?
>>
>> If I just remove those 4 words the sentence seems OK to me BTW.
>
> Is the following clearer?
>
>     It is worth noting that with this v18 version, the dtb version is not
>     downgraded for any modification but only when unknown tags are removed
>     due to a property modification. In v17 or older version any modification
>     led to a dtb version downgrade.

Yes, totally clear now, thanks!

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com