On Fri, 21 Mar 2025 at 14:09, Michal Simek <[email protected]> wrote: > > > > On 3/21/25 08:40, Sughosh Ganu wrote: > > On Wed, 5 Jun 2024 at 20:25, Michal Simek <[email protected]> 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 <[email protected]> > >> --- > > > > 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. > > > printf("Custom fields covered by CRC len: 0x%x\n", diff); > > > > >> + 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. > > I think select is too hard here because hexdump is implementing empty function > if HEXDUMP is not enabled. > > imply HEXDUMP if FWU_MDATA_V2
Fine with me. -sughosh
