| Message ID | 20220609123010.1017463-11-sughosh.ganu@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | FWU: Add FWU Multi Bank Update feature support | expand |
On Thu, Jun 09, 2022 at 05:59:57PM +0530, Sughosh Ganu wrote: > Add a command to read the metadata as specified in the FWU > specification and print the fields of the metadata. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > cmd/Kconfig | 7 +++++ > cmd/Makefile | 1 + > cmd/fwu_mdata.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 82 insertions(+) > create mode 100644 cmd/fwu_mdata.c > > diff --git a/cmd/Kconfig b/cmd/Kconfig > index 09193b61b9..275becd837 100644 > --- a/cmd/Kconfig > +++ b/cmd/Kconfig > @@ -144,6 +144,13 @@ config CMD_CPU > internal name) and clock frequency. Other information may be > available depending on the CPU driver. > > +config CMD_FWU_METADATA > + bool "fwu metadata read" > + depends on FWU_MULTI_BANK_UPDATE > + default y if FWU_MULTI_BANK_UPDATE > + help > + Command to read the metadata and dump it's contents > + > config CMD_LICENSE > bool "license" > select BUILD_BIN2C > diff --git a/cmd/Makefile b/cmd/Makefile > index 5e43a1e022..259a93bc65 100644 > --- a/cmd/Makefile > +++ b/cmd/Makefile > @@ -76,6 +76,7 @@ obj-$(CONFIG_CMD_FPGA) += fpga.o > obj-$(CONFIG_CMD_FPGAD) += fpgad.o > obj-$(CONFIG_CMD_FS_GENERIC) += fs.o > obj-$(CONFIG_CMD_FUSE) += fuse.o > +obj-$(CONFIG_CMD_FWU_METADATA) += fwu_mdata.o > obj-$(CONFIG_CMD_GETTIME) += gettime.o > obj-$(CONFIG_CMD_GPIO) += gpio.o > obj-$(CONFIG_CMD_HVC) += smccc.o > diff --git a/cmd/fwu_mdata.c b/cmd/fwu_mdata.c > new file mode 100644 > index 0000000000..bc20ca26a3 > --- /dev/null > +++ b/cmd/fwu_mdata.c > @@ -0,0 +1,74 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * Copyright (c) 2022, Linaro Limited > + */ > + > +#include <command.h> > +#include <dm.h> > +#include <fwu.h> > +#include <fwu_mdata.h> > +#include <log.h> > +#include <stdio.h> > +#include <stdlib.h> > + > +#include <linux/types.h> > + > +static void print_mdata(struct fwu_mdata *mdata) > +{ > + int i, j; > + struct fwu_image_entry *img_entry; > + struct fwu_image_bank_info *img_info; > + u32 nimages, nbanks; nit but we don't really need those two. Just use the define. > + > + printf("\tFWU Metadata\n"); > + printf("crc32: %#x\n", mdata->crc32); > + printf("version: %#x\n", mdata->version); > + printf("active_index: %#x\n", mdata->active_index); > + printf("previous_active_index: %#x\n", mdata->previous_active_index); > + > + nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK; > + nbanks = CONFIG_FWU_NUM_BANKS; > + printf("\tImage Info\n"); > + for (i = 0; i < nimages; i++) { > + img_entry = &mdata->img_entry[i]; > + printf("\nImage Type Guid: %pUL\n", &img_entry->image_type_uuid); > + printf("Location Guid: %pUL\n", &img_entry->location_uuid); > + for (j = 0; j < nbanks; j++) { > + img_info = &img_entry->img_bank_info[j]; > + printf("Image Guid: %pUL\n", &img_info->image_uuid); > + printf("Image Acceptance: %#x\n", img_info->accepted); Can we do 'yes/no' on the image acceptance please? > + } > + } > +} > + > +int do_fwu_mdata_read(struct cmd_tbl *cmdtp, int flag, > + int argc, char * const argv[]) > +{ > + struct udevice *dev; > + int ret = CMD_RET_SUCCESS; > + struct fwu_mdata *mdata = NULL; > + > + if (uclass_get_device(UCLASS_FWU_MDATA, 0, &dev) || !dev) { > + log_err("Unable to get FWU metadata device\n"); > + return CMD_RET_FAILURE; > + } > + > + ret = fwu_get_mdata(&mdata); > + if (ret < 0) { > + log_err("Unable to get valid FWU metadata\n"); > + ret = CMD_RET_FAILURE; > + goto out; > + } > + > + print_mdata(mdata); > + > +out: > + free(mdata); > + return ret; > +} > + > +U_BOOT_CMD( > + fwu_mdata_read, 1, 1, do_fwu_mdata_read, > + "Read and print FWU metadata", > + "" > +); > -- > 2.25.1 > Regards /Ilias
hi Ilias, On Fri, 10 Jun 2022 at 17:37, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > On Thu, Jun 09, 2022 at 05:59:57PM +0530, Sughosh Ganu wrote: > > Add a command to read the metadata as specified in the FWU > > specification and print the fields of the metadata. > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > --- > > cmd/Kconfig | 7 +++++ > > cmd/Makefile | 1 + > > cmd/fwu_mdata.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 82 insertions(+) > > create mode 100644 cmd/fwu_mdata.c > > > > diff --git a/cmd/Kconfig b/cmd/Kconfig > > index 09193b61b9..275becd837 100644 > > --- a/cmd/Kconfig > > +++ b/cmd/Kconfig > > @@ -144,6 +144,13 @@ config CMD_CPU > > internal name) and clock frequency. Other information may be > > available depending on the CPU driver. > > > > +config CMD_FWU_METADATA > > + bool "fwu metadata read" > > + depends on FWU_MULTI_BANK_UPDATE > > + default y if FWU_MULTI_BANK_UPDATE > > + help > > + Command to read the metadata and dump it's contents > > + > > config CMD_LICENSE > > bool "license" > > select BUILD_BIN2C > > diff --git a/cmd/Makefile b/cmd/Makefile > > index 5e43a1e022..259a93bc65 100644 > > --- a/cmd/Makefile > > +++ b/cmd/Makefile > > @@ -76,6 +76,7 @@ obj-$(CONFIG_CMD_FPGA) += fpga.o > > obj-$(CONFIG_CMD_FPGAD) += fpgad.o > > obj-$(CONFIG_CMD_FS_GENERIC) += fs.o > > obj-$(CONFIG_CMD_FUSE) += fuse.o > > +obj-$(CONFIG_CMD_FWU_METADATA) += fwu_mdata.o > > obj-$(CONFIG_CMD_GETTIME) += gettime.o > > obj-$(CONFIG_CMD_GPIO) += gpio.o > > obj-$(CONFIG_CMD_HVC) += smccc.o > > diff --git a/cmd/fwu_mdata.c b/cmd/fwu_mdata.c > > new file mode 100644 > > index 0000000000..bc20ca26a3 > > --- /dev/null > > +++ b/cmd/fwu_mdata.c > > @@ -0,0 +1,74 @@ > > +/* SPDX-License-Identifier: GPL-2.0+ */ > > +/* > > + * Copyright (c) 2022, Linaro Limited > > + */ > > + > > +#include <command.h> > > +#include <dm.h> > > +#include <fwu.h> > > +#include <fwu_mdata.h> > > +#include <log.h> > > +#include <stdio.h> > > +#include <stdlib.h> > > + > > +#include <linux/types.h> > > + > > +static void print_mdata(struct fwu_mdata *mdata) > > +{ > > + int i, j; > > + struct fwu_image_entry *img_entry; > > + struct fwu_image_bank_info *img_info; > > + u32 nimages, nbanks; > > nit but we don't really need those two. Just use the define. Okay. Will change. > > > + > > + printf("\tFWU Metadata\n"); > > + printf("crc32: %#x\n", mdata->crc32); > > + printf("version: %#x\n", mdata->version); > > + printf("active_index: %#x\n", mdata->active_index); > > + printf("previous_active_index: %#x\n", mdata->previous_active_index); > > + > > + nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK; > > + nbanks = CONFIG_FWU_NUM_BANKS; > > + printf("\tImage Info\n"); > > + for (i = 0; i < nimages; i++) { > > + img_entry = &mdata->img_entry[i]; > > + printf("\nImage Type Guid: %pUL\n", &img_entry->image_type_uuid); > > + printf("Location Guid: %pUL\n", &img_entry->location_uuid); > > + for (j = 0; j < nbanks; j++) { > > + img_info = &img_entry->img_bank_info[j]; > > + printf("Image Guid: %pUL\n", &img_info->image_uuid); > > + printf("Image Acceptance: %#x\n", img_info->accepted); > > Can we do 'yes/no' on the image acceptance please? Okay. Will change. -sughosh > > > + } > > + } > > +} > > + > > +int do_fwu_mdata_read(struct cmd_tbl *cmdtp, int flag, > > + int argc, char * const argv[]) > > +{ > > + struct udevice *dev; > > + int ret = CMD_RET_SUCCESS; > > + struct fwu_mdata *mdata = NULL; > > + > > + if (uclass_get_device(UCLASS_FWU_MDATA, 0, &dev) || !dev) { > > + log_err("Unable to get FWU metadata device\n"); > > + return CMD_RET_FAILURE; > > + } > > + > > + ret = fwu_get_mdata(&mdata); > > + if (ret < 0) { > > + log_err("Unable to get valid FWU metadata\n"); > > + ret = CMD_RET_FAILURE; > > + goto out; > > + } > > + > > + print_mdata(mdata); > > + > > +out: > > + free(mdata); > > + return ret; > > +} > > + > > +U_BOOT_CMD( > > + fwu_mdata_read, 1, 1, do_fwu_mdata_read, > > + "Read and print FWU metadata", > > + "" > > +); > > -- > > 2.25.1 > > > > Regards > /Ilias
On 6/9/22 14:29, Sughosh Ganu wrote: > Add a command to read the metadata as specified in the FWU > specification and print the fields of the metadata. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > cmd/Kconfig | 7 +++++ > cmd/Makefile | 1 + > cmd/fwu_mdata.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 82 insertions(+) > create mode 100644 cmd/fwu_mdata.c > > diff --git a/cmd/Kconfig b/cmd/Kconfig > index 09193b61b9..275becd837 100644 > --- a/cmd/Kconfig > +++ b/cmd/Kconfig > @@ -144,6 +144,13 @@ config CMD_CPU > internal name) and clock frequency. Other information may be > available depending on the CPU driver. > > +config CMD_FWU_METADATA > + bool "fwu metadata read" > + depends on FWU_MULTI_BANK_UPDATE > + default y if FWU_MULTI_BANK_UPDATE > + help > + Command to read the metadata and dump it's contents > + > config CMD_LICENSE > bool "license" > select BUILD_BIN2C > diff --git a/cmd/Makefile b/cmd/Makefile > index 5e43a1e022..259a93bc65 100644 > --- a/cmd/Makefile > +++ b/cmd/Makefile > @@ -76,6 +76,7 @@ obj-$(CONFIG_CMD_FPGA) += fpga.o > obj-$(CONFIG_CMD_FPGAD) += fpgad.o > obj-$(CONFIG_CMD_FS_GENERIC) += fs.o > obj-$(CONFIG_CMD_FUSE) += fuse.o > +obj-$(CONFIG_CMD_FWU_METADATA) += fwu_mdata.o > obj-$(CONFIG_CMD_GETTIME) += gettime.o > obj-$(CONFIG_CMD_GPIO) += gpio.o > obj-$(CONFIG_CMD_HVC) += smccc.o > diff --git a/cmd/fwu_mdata.c b/cmd/fwu_mdata.c > new file mode 100644 > index 0000000000..bc20ca26a3 > --- /dev/null > +++ b/cmd/fwu_mdata.c > @@ -0,0 +1,74 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * Copyright (c) 2022, Linaro Limited > + */ > + > +#include <command.h> > +#include <dm.h> > +#include <fwu.h> > +#include <fwu_mdata.h> > +#include <log.h> > +#include <stdio.h> > +#include <stdlib.h> > + > +#include <linux/types.h> > + > +static void print_mdata(struct fwu_mdata *mdata) > +{ > + int i, j; > + struct fwu_image_entry *img_entry; > + struct fwu_image_bank_info *img_info; > + u32 nimages, nbanks; > + > + printf("\tFWU Metadata\n"); > + printf("crc32: %#x\n", mdata->crc32); > + printf("version: %#x\n", mdata->version); > + printf("active_index: %#x\n", mdata->active_index); > + printf("previous_active_index: %#x\n", mdata->previous_active_index); > + > + nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK; > + nbanks = CONFIG_FWU_NUM_BANKS; > + printf("\tImage Info\n"); > + for (i = 0; i < nimages; i++) { > + img_entry = &mdata->img_entry[i]; > + printf("\nImage Type Guid: %pUL\n", &img_entry->image_type_uuid); > + printf("Location Guid: %pUL\n", &img_entry->location_uuid); > + for (j = 0; j < nbanks; j++) { > + img_info = &img_entry->img_bank_info[j]; > + printf("Image Guid: %pUL\n", &img_info->image_uuid); > + printf("Image Acceptance: %#x\n", img_info->accepted); > + } > + } > +} > + > +int do_fwu_mdata_read(struct cmd_tbl *cmdtp, int flag, > + int argc, char * const argv[]) > +{ > + struct udevice *dev; > + int ret = CMD_RET_SUCCESS; > + struct fwu_mdata *mdata = NULL; > + > + if (uclass_get_device(UCLASS_FWU_MDATA, 0, &dev) || !dev) { > + log_err("Unable to get FWU metadata device\n"); > + return CMD_RET_FAILURE; > + } > + > + ret = fwu_get_mdata(&mdata); > + if (ret < 0) { > + log_err("Unable to get valid FWU metadata\n"); > + ret = CMD_RET_FAILURE; > + goto out; > + } I think here would be a need to also check data you get back. I am playing with it and if I rewrite one copy and second copy is different I don't get any error message and I think it is wrong. You should IMHO use mdata_check hooks to check that what you get is correct and aligned. Thanks, Michal
On 6/9/22 14:29, Sughosh Ganu wrote: > Add a command to read the metadata as specified in the FWU > specification and print the fields of the metadata. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > cmd/Kconfig | 7 +++++ > cmd/Makefile | 1 + > cmd/fwu_mdata.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 82 insertions(+) > create mode 100644 cmd/fwu_mdata.c > > diff --git a/cmd/Kconfig b/cmd/Kconfig > index 09193b61b9..275becd837 100644 > --- a/cmd/Kconfig > +++ b/cmd/Kconfig > @@ -144,6 +144,13 @@ config CMD_CPU > internal name) and clock frequency. Other information may be > available depending on the CPU driver. > > +config CMD_FWU_METADATA > + bool "fwu metadata read" > + depends on FWU_MULTI_BANK_UPDATE > + default y if FWU_MULTI_BANK_UPDATE + default y it is enough with the depends on > + help > + Command to read the metadata and dump it's contents > + > config CMD_LICENSE > bool "license" > select BUILD_BIN2C > diff --git a/cmd/Makefile b/cmd/Makefile > index 5e43a1e022..259a93bc65 100644 > --- a/cmd/Makefile > +++ b/cmd/Makefile > @@ -76,6 +76,7 @@ obj-$(CONFIG_CMD_FPGA) += fpga.o > obj-$(CONFIG_CMD_FPGAD) += fpgad.o > obj-$(CONFIG_CMD_FS_GENERIC) += fs.o > obj-$(CONFIG_CMD_FUSE) += fuse.o > +obj-$(CONFIG_CMD_FWU_METADATA) += fwu_mdata.o > obj-$(CONFIG_CMD_GETTIME) += gettime.o > obj-$(CONFIG_CMD_GPIO) += gpio.o > obj-$(CONFIG_CMD_HVC) += smccc.o > diff --git a/cmd/fwu_mdata.c b/cmd/fwu_mdata.c > new file mode 100644 > index 0000000000..bc20ca26a3 > --- /dev/null > +++ b/cmd/fwu_mdata.c > @@ -0,0 +1,74 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * Copyright (c) 2022, Linaro Limited > + */ > + > +#include <command.h> > +#include <dm.h> > +#include <fwu.h> > +#include <fwu_mdata.h> > +#include <log.h> > +#include <stdio.h> > +#include <stdlib.h> > + > +#include <linux/types.h> > + > +static void print_mdata(struct fwu_mdata *mdata) > +{ > + int i, j; > + struct fwu_image_entry *img_entry; > + struct fwu_image_bank_info *img_info; > + u32 nimages, nbanks; > + > + printf("\tFWU Metadata\n"); > + printf("crc32: %#x\n", mdata->crc32); > + printf("version: %#x\n", mdata->version); > + printf("active_index: %#x\n", mdata->active_index); > + printf("previous_active_index: %#x\n", mdata->previous_active_index); > + > + nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK; > + nbanks = CONFIG_FWU_NUM_BANKS; > + printf("\tImage Info\n"); > + for (i = 0; i < nimages; i++) { > + img_entry = &mdata->img_entry[i]; > + printf("\nImage Type Guid: %pUL\n", &img_entry->image_type_uuid); > + printf("Location Guid: %pUL\n", &img_entry->location_uuid); > + for (j = 0; j < nbanks; j++) { > + img_info = &img_entry->img_bank_info[j]; > + printf("Image Guid: %pUL\n", &img_info->image_uuid); > + printf("Image Acceptance: %#x\n", img_info->accepted); > + } > + } > +} > + > +int do_fwu_mdata_read(struct cmd_tbl *cmdtp, int flag, > + int argc, char * const argv[]) > +{ > + struct udevice *dev; > + int ret = CMD_RET_SUCCESS; > + struct fwu_mdata *mdata = NULL; > + > + if (uclass_get_device(UCLASS_FWU_MDATA, 0, &dev) || !dev) { > + log_err("Unable to get FWU metadata device\n"); > + return CMD_RET_FAILURE; > + } > + ret is reused here ? so the default value is overwrite I think it is better to have 2 variables ret => cmd result res => result of fwu_get_mdata() ? > + ret = fwu_get_mdata(&mdata); > + if (ret < 0) { > + log_err("Unable to get valid FWU metadata\n"); > + ret = CMD_RET_FAILURE; > + goto out; > + } + res = fwu_get_mdata(&mdata); + if (res < 0) { + log_err("Unable to get valid FWU metadata\n"); + ret = CMD_RET_FAILURE; + goto out; + } > + > + print_mdata(mdata); > + > +out: > + free(mdata); > + return ret; > +} > + > +U_BOOT_CMD( > + fwu_mdata_read, 1, 1, do_fwu_mdata_read, > + "Read and print FWU metadata", > + "" > +); regards Patrick
diff --git a/cmd/Kconfig b/cmd/Kconfig index 09193b61b9..275becd837 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -144,6 +144,13 @@ config CMD_CPU internal name) and clock frequency. Other information may be available depending on the CPU driver. +config CMD_FWU_METADATA + bool "fwu metadata read" + depends on FWU_MULTI_BANK_UPDATE + default y if FWU_MULTI_BANK_UPDATE + help + Command to read the metadata and dump it's contents + config CMD_LICENSE bool "license" select BUILD_BIN2C diff --git a/cmd/Makefile b/cmd/Makefile index 5e43a1e022..259a93bc65 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -76,6 +76,7 @@ obj-$(CONFIG_CMD_FPGA) += fpga.o obj-$(CONFIG_CMD_FPGAD) += fpgad.o obj-$(CONFIG_CMD_FS_GENERIC) += fs.o obj-$(CONFIG_CMD_FUSE) += fuse.o +obj-$(CONFIG_CMD_FWU_METADATA) += fwu_mdata.o obj-$(CONFIG_CMD_GETTIME) += gettime.o obj-$(CONFIG_CMD_GPIO) += gpio.o obj-$(CONFIG_CMD_HVC) += smccc.o diff --git a/cmd/fwu_mdata.c b/cmd/fwu_mdata.c new file mode 100644 index 0000000000..bc20ca26a3 --- /dev/null +++ b/cmd/fwu_mdata.c @@ -0,0 +1,74 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (c) 2022, Linaro Limited + */ + +#include <command.h> +#include <dm.h> +#include <fwu.h> +#include <fwu_mdata.h> +#include <log.h> +#include <stdio.h> +#include <stdlib.h> + +#include <linux/types.h> + +static void print_mdata(struct fwu_mdata *mdata) +{ + int i, j; + struct fwu_image_entry *img_entry; + struct fwu_image_bank_info *img_info; + u32 nimages, nbanks; + + printf("\tFWU Metadata\n"); + printf("crc32: %#x\n", mdata->crc32); + printf("version: %#x\n", mdata->version); + printf("active_index: %#x\n", mdata->active_index); + printf("previous_active_index: %#x\n", mdata->previous_active_index); + + nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK; + nbanks = CONFIG_FWU_NUM_BANKS; + printf("\tImage Info\n"); + for (i = 0; i < nimages; i++) { + img_entry = &mdata->img_entry[i]; + printf("\nImage Type Guid: %pUL\n", &img_entry->image_type_uuid); + printf("Location Guid: %pUL\n", &img_entry->location_uuid); + for (j = 0; j < nbanks; j++) { + img_info = &img_entry->img_bank_info[j]; + printf("Image Guid: %pUL\n", &img_info->image_uuid); + printf("Image Acceptance: %#x\n", img_info->accepted); + } + } +} + +int do_fwu_mdata_read(struct cmd_tbl *cmdtp, int flag, + int argc, char * const argv[]) +{ + struct udevice *dev; + int ret = CMD_RET_SUCCESS; + struct fwu_mdata *mdata = NULL; + + if (uclass_get_device(UCLASS_FWU_MDATA, 0, &dev) || !dev) { + log_err("Unable to get FWU metadata device\n"); + return CMD_RET_FAILURE; + } + + ret = fwu_get_mdata(&mdata); + if (ret < 0) { + log_err("Unable to get valid FWU metadata\n"); + ret = CMD_RET_FAILURE; + goto out; + } + + print_mdata(mdata); + +out: + free(mdata); + return ret; +} + +U_BOOT_CMD( + fwu_mdata_read, 1, 1, do_fwu_mdata_read, + "Read and print FWU metadata", + "" +);
Add a command to read the metadata as specified in the FWU specification and print the fields of the metadata. Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> --- cmd/Kconfig | 7 +++++ cmd/Makefile | 1 + cmd/fwu_mdata.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+) create mode 100644 cmd/fwu_mdata.c