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

[Sughosh Ganu]

On Wed, 5 Jun 2024 at 20:25, Michal Simek <michal.simek at amd.com> 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.
>
> Signed-off-by: Michal Simek <michal.simek at amd.com>
> ---

Looks good. Just a couple of nits.

>
> I am using this for some time to check various configurations that's why it
> can be useful for others too.
> Sughosh: Maybe there is better way how to dump it.
> ---
>  cmd/fwu_mdata.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/cmd/fwu_mdata.c b/cmd/fwu_mdata.c
> index 9c048d69a131..ff6435505167 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) {
> +               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 0x%x\n", diff);

It would be better to mention that the value being printed is the
length of the vendor data. Or not print the value.

> +                       print_hex_dump_bytes("CUSTOM ", DUMP_PREFIX_OFFSET,
> +                                            end, diff);
> +               }

Would be better to do a 'select HEXDUMP if FWU_MDATA_V2' under the
CMD_FWU_METADATA config.

With the above changes

Tested-by: Sughosh Ganu <sughosh.ganu at linaro.org>
Reviewed by: Sughosh Ganu <sughosh.ganu at linaro.org>

-sughosh

> +       }
>  }
>
>  int do_fwu_mdata_read(struct cmd_tbl *cmdtp, int flag,
> --
> 2.40.1
>