| Message ID | 20220817124323.375968-11-sughosh.ganu@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | FWU: Add FWU Multi Bank Update feature support | expand |
Hi Sugosh, On Wed, 17 Aug 2022 at 06:44, Sughosh Ganu <sughosh.ganu@linaro.org> 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> > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > Changes since V7: None > > cmd/Kconfig | 7 +++++ > cmd/Makefile | 1 + > cmd/fwu_mdata.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 88 insertions(+) > create mode 100644 cmd/fwu_mdata.c This needs docs and a test. BTW I forgot to mention that the uclass needs a simple test of some sort. https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html Regards, SImon
hi Simon, On Thu, 18 Aug 2022 at 08:51, Simon Glass <sjg@chromium.org> wrote: > > Hi Sugosh, > > On Wed, 17 Aug 2022 at 06:44, Sughosh Ganu <sughosh.ganu@linaro.org> 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> > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > --- > > Changes since V7: None > > > > cmd/Kconfig | 7 +++++ > > cmd/Makefile | 1 + > > cmd/fwu_mdata.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 88 insertions(+) > > create mode 100644 cmd/fwu_mdata.c > > This needs docs and a test. > > BTW I forgot to mention that the uclass needs a simple test of some sort. > > https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html Okay. I will check how this can be done. Btw, there are plans to add a test for the feature once support for the feature has been added on the Synquacer platform. That test will exercise the above command as well as the driver code. Do we still need a standalone test for the uclass? -sughosh
Hi Sughosh, On Thu, 18 Aug 2022 at 05:53, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > hi Simon, > > On Thu, 18 Aug 2022 at 08:51, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Sugosh, > > > > On Wed, 17 Aug 2022 at 06:44, Sughosh Ganu <sughosh.ganu@linaro.org> 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> > > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > --- > > > Changes since V7: None > > > > > > cmd/Kconfig | 7 +++++ > > > cmd/Makefile | 1 + > > > cmd/fwu_mdata.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 88 insertions(+) > > > create mode 100644 cmd/fwu_mdata.c > > > > This needs docs and a test. > > > > BTW I forgot to mention that the uclass needs a simple test of some sort. > > > > https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html > > Okay. I will check how this can be done. Btw, there are plans to add a > test for the feature once support for the feature has been added on > the Synquacer platform. That test will exercise the above command as > well as the driver code. Do we still need a standalone test for the > uclass? Yes, testing on real hardware has nothing to do with the uclass test, which runs on sandbox. It should be a small unit test like others in test/dm/... Regards, Simon
hi Simon, On Thu, 18 Aug 2022 at 23:19, Simon Glass <sjg@chromium.org> wrote: > > Hi Sughosh, > > On Thu, 18 Aug 2022 at 05:53, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > hi Simon, > > > > On Thu, 18 Aug 2022 at 08:51, Simon Glass <sjg@chromium.org> wrote: > > > > > > Hi Sugosh, > > > > > > On Wed, 17 Aug 2022 at 06:44, Sughosh Ganu <sughosh.ganu@linaro.org> 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> > > > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > > --- > > > > Changes since V7: None > > > > > > > > cmd/Kconfig | 7 +++++ > > > > cmd/Makefile | 1 + > > > > cmd/fwu_mdata.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++ > > > > 3 files changed, 88 insertions(+) > > > > create mode 100644 cmd/fwu_mdata.c > > > > > > This needs docs and a test. > > > > > > BTW I forgot to mention that the uclass needs a simple test of some sort. > > > > > > https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html > > > > Okay. I will check how this can be done. Btw, there are plans to add a > > test for the feature once support for the feature has been added on > > the Synquacer platform. That test will exercise the above command as > > well as the driver code. Do we still need a standalone test for the > > uclass? > > Yes, testing on real hardware has nothing to do with the uclass test, which runs on sandbox. It should be a small unit test like others in test/dm/... I am not talking about testing on real hardware, but a test to be run on sandbox. I had posted the relevant patch [1] in an earlier version of the patch series. But this test relies on support being added for the feature on the Synquacer platform. Once those patches get in, I will be adding the test for the feature as well. And this test exercises both the fwu_mdata_read command as well as the driver code. Which is why I was asking if it is necessary to add additional tests for the command and dm code. -sughosh [1] - https://lists.denx.de/pipermail/u-boot/2022-June/485992.html > > Regards, > Simon
Hi Sughosh, On Fri, 19 Aug 2022 at 01:41, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > hi Simon, > > On Thu, 18 Aug 2022 at 23:19, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Sughosh, > > > > On Thu, 18 Aug 2022 at 05:53, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > hi Simon, > > > > > > On Thu, 18 Aug 2022 at 08:51, Simon Glass <sjg@chromium.org> wrote: > > > > > > > > Hi Sugosh, > > > > > > > > On Wed, 17 Aug 2022 at 06:44, Sughosh Ganu <sughosh.ganu@linaro.org> 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> > > > > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > > > --- > > > > > Changes since V7: None > > > > > > > > > > cmd/Kconfig | 7 +++++ > > > > > cmd/Makefile | 1 + > > > > > cmd/fwu_mdata.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++ > > > > > 3 files changed, 88 insertions(+) > > > > > create mode 100644 cmd/fwu_mdata.c > > > > > > > > This needs docs and a test. > > > > > > > > BTW I forgot to mention that the uclass needs a simple test of some sort. > > > > > > > > https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html > > > > > > Okay. I will check how this can be done. Btw, there are plans to add a > > > test for the feature once support for the feature has been added on > > > the Synquacer platform. That test will exercise the above command as > > > well as the driver code. Do we still need a standalone test for the > > > uclass? > > > > Yes, testing on real hardware has nothing to do with the uclass test, which runs on sandbox. It should be a small unit test like others in test/dm/... > > I am not talking about testing on real hardware, but a test to be run > on sandbox. I had posted the relevant patch [1] in an earlier version > of the patch series. But this test relies on support being added for > the feature on the Synquacer platform. Once those patches get in, I > will be adding the test for the feature as well. And this test > exercises both the fwu_mdata_read command as well as the driver code. > Which is why I was asking if it is necessary to add additional tests > for the command and dm code. It looks like that 'functional' test should be split into several unit tests that check particular things. See how this works with bootstd, in test/boot - the Python sets things up and the unit tests cover particular areas. The test seems to rely on things happening at reboot, so create a command to do those things, to provide for testability. So for example 'ut boot fwu_prepare_update' could set up the update and 'ut boot fwu_apply_update' could apply it. Regards, Simon > > -sughosh > > [1] - https://lists.denx.de/pipermail/u-boot/2022-June/485992.html > > > > > Regards, > > Simon
hi Simon, On Fri, 19 Aug 2022 at 20:55, Simon Glass <sjg@chromium.org> wrote: > > Hi Sughosh, > > On Fri, 19 Aug 2022 at 01:41, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > hi Simon, > > > > On Thu, 18 Aug 2022 at 23:19, Simon Glass <sjg@chromium.org> wrote: > > > > > > Hi Sughosh, > > > > > > On Thu, 18 Aug 2022 at 05:53, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > > > hi Simon, > > > > > > > > On Thu, 18 Aug 2022 at 08:51, Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > Hi Sugosh, > > > > > > > > > > On Wed, 17 Aug 2022 at 06:44, Sughosh Ganu <sughosh.ganu@linaro.org> 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> > > > > > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > > > > --- > > > > > > Changes since V7: None > > > > > > > > > > > > cmd/Kconfig | 7 +++++ > > > > > > cmd/Makefile | 1 + > > > > > > cmd/fwu_mdata.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++ > > > > > > 3 files changed, 88 insertions(+) > > > > > > create mode 100644 cmd/fwu_mdata.c > > > > > > > > > > This needs docs and a test. > > > > > > > > > > BTW I forgot to mention that the uclass needs a simple test of some sort. > > > > > > > > > > https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html > > > > > > > > Okay. I will check how this can be done. Btw, there are plans to add a > > > > test for the feature once support for the feature has been added on > > > > the Synquacer platform. That test will exercise the above command as > > > > well as the driver code. Do we still need a standalone test for the > > > > uclass? > > > > > > Yes, testing on real hardware has nothing to do with the uclass test, which runs on sandbox. It should be a small unit test like others in test/dm/... > > > > I am not talking about testing on real hardware, but a test to be run > > on sandbox. I had posted the relevant patch [1] in an earlier version > > of the patch series. But this test relies on support being added for > > the feature on the Synquacer platform. Once those patches get in, I > > will be adding the test for the feature as well. And this test > > exercises both the fwu_mdata_read command as well as the driver code. > > Which is why I was asking if it is necessary to add additional tests > > for the command and dm code. > > It looks like that 'functional' test should be split into several unit > tests that check particular things. See how this works with bootstd, > in test/boot - the Python sets things up and the unit tests cover > particular areas. The test seems to rely on things happening at > reboot, so create a command to do those things, to provide for > testability. The testing of the feature is being done on similar lines to how the capsule update feature is being tested -- in fact, the FWU feature relies on the capsule update for the underlying update functionality. I think it would be good to have a script to test the feature. If you want, I can see how I can add a test for the command, although I think it would be superfluous given that the command will be tested as part of the feature testing. -sughosh > > So for example 'ut boot fwu_prepare_update' could set up the update > and 'ut boot fwu_apply_update' could apply it. > > Regards, > Simon > > > > > > -sughosh > > > > [1] - https://lists.denx.de/pipermail/u-boot/2022-June/485992.html > > > > > > > > Regards, > > > Simon
Hi Sughosh, On Sun, 21 Aug 2022 at 22:59, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > hi Simon, > > On Fri, 19 Aug 2022 at 20:55, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Sughosh, > > > > On Fri, 19 Aug 2022 at 01:41, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > hi Simon, > > > > > > On Thu, 18 Aug 2022 at 23:19, Simon Glass <sjg@chromium.org> wrote: > > > > > > > > Hi Sughosh, > > > > > > > > On Thu, 18 Aug 2022 at 05:53, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > > > > > hi Simon, > > > > > > > > > > On Thu, 18 Aug 2022 at 08:51, Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > Hi Sugosh, > > > > > > > > > > > > On Wed, 17 Aug 2022 at 06:44, Sughosh Ganu <sughosh.ganu@linaro.org> 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> > > > > > > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > > > > > --- > > > > > > > Changes since V7: None > > > > > > > > > > > > > > cmd/Kconfig | 7 +++++ > > > > > > > cmd/Makefile | 1 + > > > > > > > cmd/fwu_mdata.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++ > > > > > > > 3 files changed, 88 insertions(+) > > > > > > > create mode 100644 cmd/fwu_mdata.c > > > > > > > > > > > > This needs docs and a test. > > > > > > > > > > > > BTW I forgot to mention that the uclass needs a simple test of some sort. > > > > > > > > > > > > https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html > > > > > > > > > > Okay. I will check how this can be done. Btw, there are plans to add a > > > > > test for the feature once support for the feature has been added on > > > > > the Synquacer platform. That test will exercise the above command as > > > > > well as the driver code. Do we still need a standalone test for the > > > > > uclass? > > > > > > > > Yes, testing on real hardware has nothing to do with the uclass test, which runs on sandbox. It should be a small unit test like others in test/dm/... > > > > > > I am not talking about testing on real hardware, but a test to be run > > > on sandbox. I had posted the relevant patch [1] in an earlier version > > > of the patch series. But this test relies on support being added for > > > the feature on the Synquacer platform. Once those patches get in, I > > > will be adding the test for the feature as well. And this test > > > exercises both the fwu_mdata_read command as well as the driver code. > > > Which is why I was asking if it is necessary to add additional tests > > > for the command and dm code. > > > > It looks like that 'functional' test should be split into several unit > > tests that check particular things. See how this works with bootstd, > > in test/boot - the Python sets things up and the unit tests cover > > particular areas. The test seems to rely on things happening at > > reboot, so create a command to do those things, to provide for > > testability. > > The testing of the feature is being done on similar lines to how the > capsule update feature is being tested -- in fact, the FWU feature > relies on the capsule update for the underlying update functionality. > I think it would be good to have a script to test the feature. If you > want, I can see how I can add a test for the command, although I think > it would be superfluous given that the command will be tested as part > of the feature testing. The uclass needs a sandbox test. The capsule-update thing seems to rely on restarting the executable which is not a good thing for debugging a test in gdb, as you can imagine. Please read through the testing docs if you haven't already. We need a simple test that checks that the uclass and sandbox driver does what it should. Regards, Simon > > -sughosh > > > > > So for example 'ut boot fwu_prepare_update' could set up the update > > and 'ut boot fwu_apply_update' could apply it. > > > > Regards, > > Simon > > > > > > > > > > -sughosh > > > > > > [1] - https://lists.denx.de/pipermail/u-boot/2022-June/485992.html > > > > > > > > > > > Regards, > > > > Simon
diff --git a/cmd/Kconfig b/cmd/Kconfig index 211ebe9c87..95e762d21c 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -161,6 +161,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 + 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 6e87522b62..ff6e160f4a 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..ee9d035374 --- /dev/null +++ b/cmd/fwu_mdata.c @@ -0,0 +1,80 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * 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; + + 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); + + printf("\tImage Info\n"); + for (i = 0; i < CONFIG_FWU_NUM_IMAGES_PER_BANK; 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 < CONFIG_FWU_NUM_BANKS; j++) { + img_info = &img_entry->img_bank_info[j]; + printf("Image Guid: %pUL\n", &img_info->image_uuid); + printf("Image Acceptance: %s\n", + img_info->accepted == 0x1 ? "yes" : "no"); + } + } +} + +int do_fwu_mdata_read(struct cmd_tbl *cmdtp, int flag, + int argc, char * const argv[]) +{ + struct udevice *dev; + int ret = CMD_RET_SUCCESS, res; + 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; + } + + res = fwu_mdata_check(); + if (res < 0) { + log_err("FWU Metadata check failed\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", + "" +);