Re: [PATCH v3 07/15] fdtdump: Handle unknown tags

[Herve Codina <herve.codina@bootlin.com>]

On Thu, 10 Sep 2026 15:25:06 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Wed, Aug 26, 2026 at 10:31:38AM +0200, Herve Codina wrote:
> > 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.
> > 
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  fdtdump.c                              |  45 ++++++-
> >  tests/Makefile.tests                   |   3 +-
> >  tests/meson.build                      |   2 +
> >  tests/run_tests.sh                     |  41 +++++++
> >  tests/treegen.c                        | 162 +++++++++++++++++++++++++
> >  tests/unknown_tags_can_skip.dtb.expect |  29 +++++
> >  6 files changed, 278 insertions(+), 4 deletions(-)
> >  create mode 100644 tests/unknown_tags_can_skip.dtb.expect
> > 
> > diff --git a/fdtdump.c b/fdtdump.c
> > index 0e7a2659..7a8b2784 100644
> > --- 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_VARLEN:
> > +				/* Get the length */
> > +				sz = fdt32_to_cpu(GET_CELL(p));
> > +				break;
> > +			}
> > +
> > +			if (dump_unknown) {  
> 
> I'd do this unconditionally: fdtdump is specifically for low-level and
> debug dumping of a tree.  If you want pretty printing, dtc -Odts is
> the thing to use.

Ok, will do it unconditionally.

> 
> > +				printf("%*s// Unknown tag ignored: 0x%08"PRIx32", data len %d",
> > +				       depth * shift, "", tag, sz);  
> 
> "Unknown" seems redundant.  "ignored" is not really true - you're
> dumping its contents, which is all that fdtdump could be asked to do
> with it.  So I'd just say:
> 
> 	// Tag 0x<tag>: <data>
> 
> Or maybe "Metadata tag" / "skippable tag" if we go with one of those
> terms as I suggested on an earlier patch.

I am fine with 
  // Tag 0x<tag>: <data>

I will use that.

Best regards,
Hervé