[PATCH] fwu: Initialize global fwu library state during CI test

[Simon Glass]

On Fri, 25 Aug 2023 at 10:00, Marek Vasut <marek.vasut at mailbox.org> wrote:
>
> On 8/25/23 12:52, Sughosh Ganu wrote:
> > On Wed, 23 Aug 2023 at 05:47, Marek Vasut
> > <marek.vasut+renesas at mailbox.org> wrote:
> >>
> >> The current CI test worked by sheer luck, the g_dev global pointer
> >> in the fwu library was never initialized and the test equally well
> >> failed on sandbox64. Trigger the main loop in sandbox tests too to
> >> initialize that global state, and move the sandbox specific exit
> >> from fwu_boottime_checks after g_dev is initialized.
> >>
> >> Signed-off-by: Marek Vasut <marek.vasut+renesas at mailbox.org>
> >> ---
> >> Cc: Etienne Carriere <etienne.carriere at linaro.org>
> >> Cc: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> >> Cc: Jassi Brar <jaswinder.singh at linaro.org>
> >> Cc: Masahisa Kojima <masahisa.kojima at linaro.org>
> >> Cc: Michal Simek <michal.simek at amd.com>
> >> Cc: Patrick Delaunay <patrick.delaunay at foss.st.com>
> >> Cc: Simon Glass <sjg at chromium.org>
> >> Cc: Sughosh Ganu <sughosh.ganu at linaro.org>
> >> ---
> >>   lib/fwu_updates/fwu.c | 12 ++++++------
> >>   test/dm/fwu_mdata.c   | 12 ++++++++++++
> >>   2 files changed, 18 insertions(+), 6 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

BTW, Sughosh this state should really be attached to the uclass, not
sitting out on its own. As it is, it won't reinit properly in tests.


> >
> > Acked-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> >
> > But I have a question below.
> >
> >>
> >> diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
> >> index 4d0c8b84b9d..22bdc78df59 100644
> >> --- a/lib/fwu_updates/fwu.c
> >> +++ b/lib/fwu_updates/fwu.c
> >> @@ -623,18 +623,18 @@ static int fwu_boottime_checks(void *ctx, struct event *event)
> >>          int ret;
> >>          u32 boot_idx, active_idx;
> >>
> >> -       /* Don't have boot time checks on sandbox */
> >> -       if (IS_ENABLED(CONFIG_SANDBOX)) {
> >> -               boottime_check = 1;
> >> -               return 0;
> >> -       }
> >> -
> >>          ret = uclass_first_device_err(UCLASS_FWU_MDATA, &g_dev);
> >>          if (ret) {
> >>                  log_debug("Cannot find fwu device\n");
> >>                  return ret;
> >>          }
> >>
> >> +       /* Don't have boot time checks on sandbox */
> >> +       if (IS_ENABLED(CONFIG_SANDBOX)) {
> >> +               boottime_check = 1;
> >> +               return 0;
> >> +       }
> >> +
> >>          ret = fwu_get_mdata(NULL);
> >>          if (ret) {
> >>                  log_debug("Unable to read meta-data\n");
> >> diff --git a/test/dm/fwu_mdata.c b/test/dm/fwu_mdata.c
> >> index 8b5c83ef4e2..52018f610fe 100644
> >> --- a/test/dm/fwu_mdata.c
> >> +++ b/test/dm/fwu_mdata.c
> >> @@ -93,6 +93,12 @@ static int dm_test_fwu_mdata_read(struct unit_test_state *uts)
> >>          struct udevice *dev;
> >>          struct fwu_mdata mdata = { 0 };
> >>
> >> +       /*
> >> +        * Trigger lib/fwu_updates/fwu.c fwu_boottime_checks()
> >> +        * to populate g_dev global pointer in that library.
> >> +        */
> >> +       event_notify_null(EVT_MAIN_LOOP);
> >> +
> >
> > Are these explicit invocations required? Should fwu_boottime_checks
> > not get called through run_main_loop() during boot?
>
> board_r.c event_notify_null(EVT_MAIN_LOOP); is not reached in sandbox:
>
> 571 static int run_main_loop(void)
> 572 {
> 573 #ifdef CONFIG_SANDBOX <----------- because of this
> 574         sandbox_main_loop_init();
> 575 #endif
> 576
> 577         event_notify_null(EVT_MAIN_LOOP);