[PATCH] test: boot: Bind bootmeth_{cros, android} only once per run

[Simon Glass]

Hi Mattijs,

On Thu, 21 Nov 2024 at 08:03, Mattijs Korpershoek
<mkorpershoek at baylibre.com> wrote:
>
> We make fewer calls to dm_test_restore() since
> commit fbdac8155c89 ("test: Expand implementation of ut_list_has_dm_tests()")
>
> Because of this some valid test combinations are now broken:
>
> $ ./test/py/test.py --bd sandbox --build -k test_ut
> $ ./test/py/test.py --bd sandbox --build -k "bootflow_android or bootflow_cros"
>
> Shows:
>
>   Expected '  2  cros         ready   mmc          4 mmc5.bootdev.part_4       ',
>   got '  2  cros         ready   mmc          2 mmc5.bootdev.part_2       '
>
> Here prep_mmc_bootdev() is called twice and it will bind bootmeth_cros twice.
>
> Since bootmeth_cros is bound twice, 'bootflow scan' will find 2x the
> expected bootflows.
>
> Before
> commit fbdac8155c89 ("test: Expand implementation of ut_list_has_dm_tests()")
> this did not happen because a cleanup was called each time.
>
> Check if the bootstd already has a "cros" or "android" driver and only bind it
> if it does not exist.
>
> Fixes: fbdac8155c89 ("test: Expand implementation of ut_list_has_dm_tests()")
> Signed-off-by: Mattijs Korpershoek <mkorpershoek at baylibre.com>
> ---
> Initially, this was found when merging some Android bootflow patches
> from [1].
>
> However, this can be reproduced on 'next' as well.
>
> Here is the test output that's broken (we have 4 cros bootmethods instead of 2)
>
> => ut bootstd bootflow_cros
> Test: bootflow_cros: bootflow.c
> Showing all bootflows
> Seq  Method       State   Uclass    Part  Name                      Filename
> ---  -----------  ------  --------  ----  ------------------------ ----------------
> 0  extlinux     ready   mmc          1  mmc1.bootdev.part_1 /extlinux/extlinux.conf
> 1  cros         ready   mmc          2  mmc5.bootdev.part_2
> 2  cros         ready   mmc          2  mmc5.bootdev.part_2
> 3  cros         ready   mmc          4  mmc5.bootdev.part_4
> 4  cros         ready   mmc          4  mmc5.bootdev.part_4
> ---  -----------  ------  --------  ----  ------------------------ ----------------
> (5 bootflows, 5 valid)
> test/boot/bootflow.c:1192, bootflow_cros(): console:
> Expected '  2  cros         ready   mmc          4 mmc5.bootdev.part_4       ',
> got '  2  cros         ready   mmc          2 mmc5.bootdev.part_2       '
> Test bootflow_cros failed 1 times
>
> [1] https://lore.kernel.org/r/all/87mshvxhq3.fsf@baylibre.com/
> ---
>  test/boot/bootflow.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
> index 9397328609d0..835d2e6e35a6 100644
> --- a/test/boot/bootflow.c
> +++ b/test/boot/bootflow.c
> @@ -552,15 +552,21 @@ static int prep_mmc_bootdev(struct unit_test_state *uts, const char *mmc_dev,
>         /* Enable the cros bootmeth if needed */
>         if (IS_ENABLED(CONFIG_BOOTMETH_CROS) && bind_cros_android) {
>                 ut_assertok(uclass_first_device_err(UCLASS_BOOTSTD, &bootstd));
> -               ut_assertok(device_bind(bootstd, DM_DRIVER_REF(bootmeth_cros),
> -                                       "cros", 0, ofnode_null(), &dev));
> +               /* Only bind the bootmeth once to avoid duplicate scan results */
> +               if (device_find_child_by_name(bootstd, "cros", &dev) == -ENODEV) {
> +                       ut_assertok(device_bind(bootstd, DM_DRIVER_REF(bootmeth_cros),
> +                                               "cros", 0, ofnode_null(), &dev));
> +               }
>         }
>
>         /* Enable the android bootmeths if needed */
>         if (IS_ENABLED(CONFIG_BOOTMETH_ANDROID) && bind_cros_android) {
>                 ut_assertok(uclass_first_device_err(UCLASS_BOOTSTD, &bootstd));
> -               ut_assertok(device_bind(bootstd, DM_DRIVER_REF(bootmeth_android),
> -                                       "android", 0, ofnode_null(), &dev));
> +               /* Only bind the bootmeth once to avoid duplicate scan results */
> +               if (device_find_child_by_name(bootstd, "android", &dev) == -ENODEV) {
> +                       ut_assertok(device_bind(bootstd, DM_DRIVER_REF(bootmeth_android),
> +                                               "android", 0, ofnode_null(), &dev));
> +               }
>         }
>
>         /* Change the order to include the device */
>
> ---
> base-commit: dc1859f8d2ac3faaa5e2e1d465ec4bd8980520a5
> change-id: 20241121-bootstd-test-fix-multiple-bind-159bd2523414
>
> Best regards,
> --
> Mattijs Korpershoek <mkorpershoek at baylibre.com>
>

Instead of this, could you add flags to the test to tell it to reset
driver mode?

UTF_DM | UTF_SCAN_FDT

Regards,
Simon