[PATCH v8 10/13] FWU: cmd: Add a command to read FWU metadata

[Simon Glass]

Hi Sughosh,

On Sun, 21 Aug 2022 at 22:59, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> hi Simon,
>
> On Fri, 19 Aug 2022 at 20:55, Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Fri, 19 Aug 2022 at 01:41, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > >
> > > hi Simon,
> > >
> > > On Thu, 18 Aug 2022 at 23:19, Simon Glass <sjg at chromium.org> wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Thu, 18 Aug 2022 at 05:53, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > > > >
> > > > > hi Simon,
> > > > >
> > > > > On Thu, 18 Aug 2022 at 08:51, Simon Glass <sjg at chromium.org> wrote:
> > > > > >
> > > > > > Hi Sugosh,
> > > > > >
> > > > > > On Wed, 17 Aug 2022 at 06:44, Sughosh Ganu <sughosh.ganu at 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 at linaro.org>
> > > > > > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas at 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