Re: [PATCH v5 10/23] FWU: cmd: Add a command to read FWU metadata
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
---
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 00..bc20ca26a3
--- /dev/null
+++ b/cmd/fwu_mdata.c
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2022, Linaro Limited
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+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
Re: [PATCH v5 10/23] FWU: cmd: Add a command to read FWU metadata
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
---
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 00..bc20ca26a3
--- /dev/null
+++ b/cmd/fwu_mdata.c
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2022, Linaro Limited
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+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
Re: [PATCH v5 10/23] FWU: cmd: Add a command to read FWU metadata
hi Ilias,
On Fri, 10 Jun 2022 at 17:37, Ilias Apalodimas
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
> > ---
> > 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 00..bc20ca26a3
> > --- /dev/null
> > +++ b/cmd/fwu_mdata.c
> > @@ -0,0 +1,74 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright (c) 2022, Linaro Limited
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +
> > +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
Re: [PATCH v5 10/23] FWU: cmd: Add a command to read FWU metadata
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
> ---
> 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 00..bc20ca26a3
> --- /dev/null
> +++ b/cmd/fwu_mdata.c
> @@ -0,0 +1,74 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (c) 2022, Linaro Limited
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +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
[PATCH v5 10/23] FWU: cmd: Add a command to read 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
---
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 00..bc20ca26a3
--- /dev/null
+++ b/cmd/fwu_mdata.c
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2022, Linaro Limited
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+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",
+ ""
+);
--
2.25.1
