[PATCH v2 06/39] bootstd: Create a function to reset USB

[Simon Glass]

Hi Heinrich,

On Thu, 8 Aug 2024 at 15:12, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 07.08.24 16:36, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Tue, 6 Aug 2024 at 19:56, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >>
> >> On 06.08.24 14:58, Simon Glass wrote:
> >>> Set up a function for this, since it needs to be used from multiple test
> >>> files.
> >>>
> >>> Signed-off-by: Simon Glass <sjg at chromium.org>
> >>> ---
> >>>
> >>> (no changes since v1)
> >>>
> >>>    test/boot/bootdev.c        | 19 ++++++-------------
> >>>    test/boot/bootstd_common.c | 12 ++++++++++++
> >>>    test/boot/bootstd_common.h |  8 ++++++++
> >>>    3 files changed, 26 insertions(+), 13 deletions(-)
> >>>
> >>> diff --git a/test/boot/bootdev.c b/test/boot/bootdev.c
> >>> index 1bf5929c396..de16a51956d 100644
> >>> --- a/test/boot/bootdev.c
> >>> +++ b/test/boot/bootdev.c
> >>> @@ -16,13 +16,6 @@
> >>>    #include <test/ut.h>
> >>>    #include "bootstd_common.h"
> >>>
> >>> -/* Allow reseting the USB-started flag */
> >>> -#if defined(CONFIG_USB_HOST) || defined(CONFIG_USB_GADGET)
> >>> -extern bool usb_started;
> >>> -#else
> >>> -#include <usb.h>
> >>> -#endif
> >>> -
> >>>    /* Check 'bootdev list' command */
> >>>    static int bootdev_test_cmd_list(struct unit_test_state *uts)
> >>>    {
> >>> @@ -201,7 +194,7 @@ static int bootdev_test_order(struct unit_test_state *uts)
> >>>        test_set_skip_delays(true);
> >>>
> >>>        /* Start up USB which gives us three additional bootdevs */
> >>> -     usb_started = false;
> >>> +     bootstd_reset_usb();
> >>>        ut_assertok(run_command("usb start", 0));
> >>>
> >>>        /*
> >>> @@ -317,7 +310,7 @@ static int bootdev_test_prio(struct unit_test_state *uts)
> >>>        test_set_eth_enable(false);
> >>>
> >>>        /* Start up USB which gives us three additional bootdevs */
> >>> -     usb_started = false;
> >>> +     bootstd_reset_usb();
> >>>        ut_assertok(run_command("usb start", 0));
> >>>
> >>>        ut_assertok(bootstd_test_drop_bootdev_order(uts));
> >>> @@ -357,7 +350,7 @@ static int bootdev_test_hunter(struct unit_test_state *uts)
> >>>    {
> >>>        struct bootstd_priv *std;
> >>>
> >>> -     usb_started = false;
> >>> +     bootstd_reset_usb();
> >>>        test_set_skip_delays(true);
> >>>
> >>>        /* get access to the used hunters */
> >>> @@ -398,7 +391,7 @@ static int bootdev_test_cmd_hunt(struct unit_test_state *uts)
> >>>        struct bootstd_priv *std;
> >>>
> >>>        test_set_skip_delays(true);
> >>> -     usb_started = false;
> >>> +     bootstd_reset_usb();
> >>>
> >>>        /* get access to the used hunters */
> >>>        ut_assertok(bootstd_get_priv(&std));
> >>> @@ -527,7 +520,7 @@ BOOTSTD_TEST(bootdev_test_bootable, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
> >>>    /* Check hunting for bootdev of a particular priority */
> >>>    static int bootdev_test_hunt_prio(struct unit_test_state *uts)
> >>>    {
> >>> -     usb_started = false;
> >>> +     bootstd_reset_usb();
> >>>        test_set_skip_delays(true);
> >>>
> >>>        console_record_reset_enable();
> >>> @@ -556,7 +549,7 @@ static int bootdev_test_hunt_label(struct unit_test_state *uts)
> >>>        struct bootstd_priv *std;
> >>>        int mflags;
> >>>
> >>> -     usb_started = false;
> >>> +     bootstd_reset_usb();
> >>>
> >>>        /* get access to the used hunters */
> >>>        ut_assertok(bootstd_get_priv(&std));
> >>> diff --git a/test/boot/bootstd_common.c b/test/boot/bootstd_common.c
> >>> index e50539500a0..ff0aff4bbe7 100644
> >>> --- a/test/boot/bootstd_common.c
> >>> +++ b/test/boot/bootstd_common.c
> >>> @@ -20,6 +20,13 @@
> >>>    /* tracks whether bootstd_setup_for_tests() has been run yet */
> >>>    bool vbe_setup_done;
> >>>
> >>> +/* Allow resetting the USB-started flag */
> >>> +#if defined(CONFIG_USB_HOST) || defined(CONFIG_USB_GADGET)
> >>> +extern bool usb_started;
> >>> +#else
> >>> +#include <usb.h>
> >>> +#endif
> >>
> >> Why can't you always include usb.h which defines usb_started?
> >> Is there a bug in usb.h?
> >
> > I can, but even in tests it is not great to access globals. Having it
> > in a function lets us call out these particular settings better, which
> > has aided debugging, at least for me. These tests are special in that
> > they are not DM tests, but do stop USB.
>
> I was just wondering if we can can rid of the #if here. That should not
> stop you from creating a function to access the variable.

Ah, I see. Yes, it is an artifact of it being built for boards without
USB, but now we only build for sandbox, so I can drop that.

Regards,
Simon