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

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

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

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.

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


The rest of this LGTM, except for things that will need to change
based on suggestions about the earlier patches in the series.

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