Hi Simon,

On ven., nov. 22, 2024 at 06:37, Simon Glass sjg@chromium.org wrote:

Hi Mattijs,

On Thu, 21 Nov 2024 at 08:03, Mattijs Korpershoek mkorpershoek@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@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@baylibre.com

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

UTF_DM | UTF_SCAN_FDT

That works as well, and seems like a cleaner approach. Thanks a lot for the suggestion.

Will send a v2.

Regards, Simon