Re: [PATCH v2] cmd: fwu: Dump custom fields from mdata structure

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 21.03.25 11:25, Michal Simek wrote:
> The commit cb9ae40a16f0 ("tools: mkfwumdata: add logic to append vendor
> data to the FWU metadata") added support for adding vendor data to mdata
> structure but it is not visible anywhere that's why extend fwu command to
> dump it.
>
> Tested-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> Reviewed-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> ---
>
> Changes in v2:
> - Extend print message
> - Cover hexdump dependencies
>
> RFC:
> https://lore.kernel.org/r/75c697a4f819bb5e8649ed658c5a559fb8cd1fd9.1717599342.git.michal.simek@amd.com
>
> ---
>   cmd/Kconfig     |  1 +
>   cmd/fwu_mdata.c | 25 +++++++++++++++++++++++++
>   2 files changed, 26 insertions(+)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 642cc1116e87..1f8aa2521a8e 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -185,6 +185,7 @@ config CMD_UFETCH
>   config CMD_FWU_METADATA
>   	bool "fwu metadata read"
>   	depends on FWU_MULTI_BANK_UPDATE
> +	imply HEXDUMP if FWU_MDATA_V2
>   	help
>   	  Command to read the metadata and dump it's contents
>
> diff --git a/cmd/fwu_mdata.c b/cmd/fwu_mdata.c
> index 9c048d69a131..5b5a2e4d1cda 100644
> --- a/cmd/fwu_mdata.c
> +++ b/cmd/fwu_mdata.c
> @@ -7,6 +7,7 @@
>   #include <dm.h>
>   #include <fwu.h>
>   #include <fwu_mdata.h>
> +#include <hexdump.h>
>   #include <log.h>
>   #include <stdio.h>
>   #include <stdlib.h>
> @@ -45,6 +46,30 @@ static void print_mdata(struct fwu_data *data)
>   			       img_info->accepted == 0x1 ? "yes" : "no");
>   		}
>   	}
> +
> +	if (data->version == 2) {

Should this be >= 2 ? Or do we intend to drop custom field support in
future?

> +		struct fwu_mdata *mdata = data->fwu_mdata;
> +		struct fwu_fw_store_desc *desc;
> +		void *end;
> +		u32 diff;
> +
> +		/*
> +		 * fwu_mdata defines only header that's why taking it as array
> +		 * which exactly point to image description location
> +		 */
> +		desc = (struct fwu_fw_store_desc *)&mdata[1];
> +
> +		/* Number of entries is taken from for loop - variable i */
> +		end = &desc->img_entry[i];
> +		debug("mdata %p, desc %p, end %p\n", mdata, desc, end);
> +
> +		diff = data->metadata_size - ((void *)end - (void *)mdata);
> +		if (diff) {
> +			printf("Custom fields covered by CRC len: 0x%x\n", diff);

The print label is a bit hard to understand. Do you mean:

"Length of custom fields in bytes: 0x%x\n"

Wouldn't print_hex_dump_bytes() already provide an address column
conveying that information?

Best regards

Heinrich

> +			print_hex_dump_bytes("CUSTOM ", DUMP_PREFIX_OFFSET,
> +					     end, diff);
> +		}
> +	}
>   }
>
>   int do_fwu_mdata_read(struct cmd_tbl *cmdtp, int flag,