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

[Simon Glass]

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.

Regards,
Simon