On Fri, 25 Aug 2023 at 10:00, Marek Vasut <marek.va...@mailbox.org> wrote:
>
> On 8/25/23 12:52, Sughosh Ganu wrote:
> > On Wed, 23 Aug 2023 at 05:47, Marek Vasut
> > <marek.vasut+rene...@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+rene...@mailbox.org>
> >> ---
> >> Cc: Etienne Carriere <etienne.carri...@linaro.org>
> >> Cc: Ilias Apalodimas <ilias.apalodi...@linaro.org>
> >> Cc: Jassi Brar <jaswinder.si...@linaro.org>
> >> Cc: Masahisa Kojima <masahisa.koj...@linaro.org>
> >> Cc: Michal Simek <michal.si...@amd.com>
> >> Cc: Patrick Delaunay <patrick.delau...@foss.st.com>
> >> Cc: Simon Glass <s...@chromium.org>
> >> Cc: Sughosh Ganu <sughosh.g...@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 <s...@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.g...@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);
