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

[Heinrich Schuchardt]

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.

Best regards

Heinrich