[PATCH v3 3/3] test: bootdev: scan with a broken high-priority device

[Simon Glass]

Hi Denis,

On 2026-06-21T05:14:42, None <dmukhin at ford.com> wrote:
> test: bootdev: scan with a broken high-priority device
>
> Add bootdev_hunt_fallthrough() test to verify that 'bootflow scan -l'
> falls back to a lower-priority bootdev when a higher-priority hunter
> fails.
>
> Introduce a simple 'sandbox-bootdev' device for the test. The new
> bootdev can be configured to produce an error at the hunting stage.
>
> Adjust boot{dev,flow} tests which depend on bootdev hunters.
>
> Signed-off-by: Denis Mukhin <dmukhin at ford.com>
>
> drivers/block/Makefile          |  2 +-
>  drivers/block/host-uclass.c     | 17 +++++++++++
>  drivers/block/sandbox-bootdev.c | 62 +++++++++++++++++++++++++++++++++++++++++
>  include/sandbox_host.h          | 18 ++++++++++++
>  test/boot/bootdev.c             | 25 +++++++++--------
>  test/boot/bootflow.c            | 51 +++++++++++++++++++++++++++++++++
>  test/boot/bootstd_common.h      |  2 +-
>  7 files changed, 164 insertions(+), 13 deletions(-)

Thanks for digging into this....sorry quite a bit of minor feedback.

> diff --git a/drivers/block/host-uclass.c b/drivers/block/host-uclass.c
> @@ -150,6 +150,23 @@ struct udevice *host_find_by_label(const char *label)
> +int host_set_flags_by_label(const char *label, unsigned int flags)
> +{
> +     struct udevice *dev;
> +     struct uclass *uc;
> +
> +     uclass_id_foreach_dev(UCLASS_HOST, dev, uc) {
> +             struct host_sb_plat *plat = dev_get_plat(dev);
> +
> +             if (plat->label && !strcmp(label, plat->label)) {
> +                     plat->flags = flags;
> +                     return 0;
> +             }
> +     }
> +
> +     return -ENODEV;
> +}

Please reuse host_find_by_label() rather than duplicating the loop:

   dev = host_find_by_label(label);
   if (!dev)
           return -ENODEV;
   plat = dev_get_plat(dev);
   plat->flags = flags;

   return 0;

> diff --git a/drivers/block/sandbox-bootdev.c b/drivers/block/sandbox-bootdev.c
> @@ -0,0 +1,62 @@
> +static int sandbox_bootdev_hunt(struct bootdev_hunter *info, bool show)
> +{
> +     struct udevice *dev;
> +     struct uclass *uc;
> +     int ret;
> +
> +     uclass_id_foreach_dev(UCLASS_HOST, dev, uc) {
> +             struct host_sb_plat *plat = dev_get_plat(dev);
> +
> +             log_debug("hunting %s\n", plat->label);
> +
> +             if (plat->flags & BLK_HOST_BROKEN) {
> +                     ret = -ETIME;
> +                     log_err("cannot hunt sandbox device '%s': %d\n",
> +                            plat->label, ret);
> +                     return ret;
> +             }
> +     }
> +
> +     return 0;
> +}

This hunter never binds a bootdev - it only exists to fail. Please add
a comment at the top of the file making that clear, so nobody copies
it as an example of a real hunter.

I'd also prefer to drop the log_err() - a hunter failing is a legitimate
outcome that patch 2 makes the scan loop tolerate, and the
caller is the right place to decide whether that warrants
user-visible noise. At least for here, log_debug() is sufficient.

> diff --git a/drivers/block/sandbox-bootdev.c b/drivers/block/sandbox-bootdev.c
> @@ -0,0 +1,62 @@
> +struct bootdev_ops sandbox_bootdev_ops = {
> +};

Please make this static const - it is only referenced from the
U_BOOT_DRIVER() in this file.

> diff --git a/include/sandbox_host.h b/include/sandbox_host.h
> @@ -8,17 +8,26 @@
> +/**
> + * Device flags.
> + */
> +enum {
> +     BLK_HOST_BROKEN                 = BIT(0), /* Simulate broken device */
> +};

The BLK_ prefix reads as a block-layer flag, but this lives on
host_sb_plat and is consumed by the sandbox bootdev hunter. Please
rename to HOST_FLAG_BROKEN or similar and give the enum a tag so
it can be used as a type. Also, kerneldoc on an unnamed enum does
not generate useful output - either name it or drop the /** */.

> diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
> @@ -1532,3 +1534,52 @@ static int bootstd_images(struct unit_test_state *uts)
> +     /* Reset bootstd state */
> +     ut_assertok(bootstd_get_priv(&std));
> +     old_order = std->bootdev_order;
> +     std->bootdev_order = NULL;
> +     std->hunters_used = 0;

Please use bootstd_test_drop_bootdev_order(uts) rather than
open-coding this. As written, any ut_assertok() failure before the
'Clean up' block leaks the modified bootdev_order into subsequent
tests.

> diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
> @@ -1532,3 +1534,52 @@ static int bootstd_images(struct unit_test_state *uts)
> +     ut_assertok(run_command("bootflow scan -l", 0));
> +
> +     /* USB was hunted despite the sandbox hunter failure */
> +     ut_assert_skip_to_line("Bus usb at 1: 5 USB Device(s) found");

Good test of the symptom, but please also assert that the sandbox
hunter's failure was reached - e.g. check std->hunters_used has the
sandbox hunter bit set, or ut_assert_skip_to_line() on its error
output before the USB line. Otherwise a regression that simply
skips priority 4 entirely would still pass.

> diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
> @@ -1532,3 +1534,52 @@ static int bootstd_images(struct unit_test_state *uts)
> +BOOTSTD_TEST(bootdev_hunt_fallthrough,
> +             UTF_DM | UTF_SCAN_FDT | UTF_SF_BOOTDEV | UTF_CONSOLE);
> +#endif /* CONFIG_SANDBOX */

Continuation should use a tab, not spaces. Also, test/boot/ is
already only built for sandbox in practice, so please drop the
#ifdef CONFIG_SANDBOX guard (or gate it the same way other
sandbox-only bits in this file are gated).

> commit 86288522b3f41eb776f9b0c7dc2d342a2ba078d9
>     Adjust boot{dev,flow} tests which depend on bootdev hunters.

Please mention in the commit message that this also adds a new
host_set_flags_by_label() API and a flags field to host_sb_plat -
that is not merely  an 'adjustment' to existing tests.

Regards,
Simon