Hi Simon, On ven., nov. 22, 2024 at 06:37, Simon Glass <[email protected]> wrote:
> Hi Mattijs, > > On Thu, 21 Nov 2024 at 08:03, Mattijs Korpershoek > <[email protected]> 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 <[email protected]> >> --- >> 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/[email protected]/ >> --- >> 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 <[email protected]> >> > > 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
