Re: [PATCH v2] cmd: mbr: Allow 4 MBR partitions without need for extended

[Tom Rini]

On Mon, Oct 09, 2023 at 01:24:36AM +0000, Alexander Gendin wrote:

> Current code allows up to 3 MBR partitions without extended one.
> If more than 3 partitions are required, then extended partition(s)
> must be used.
> This commit allows up to 4 primary MBR partitions without the
> need for extended partition.
> 
> Add mbr test unit. In order to run the test manually, mmc6.img file
> of size 12 MiB or greater is required in the same directory as u-boot.
> Test also runs automatically via ./test/py/test.py tool.
> Running mbr test is only supported in sandbox mode.
> 
> Signed-off-by: Alex Gendin <agen...@matrox.com>
> ---
> Changes for v2:
> - Cleanup coding style
> - Adjust commit message to the changes in v2
> - Add mmc6 device to sandbox device tree
> - Adjust boot/bootdev.c and dm/blk.c tests to include mmc6 in tests
> - Auto-create mmc6.img in test_ut_dm_init()
> - Add mbr test to the list of tests run by test.py. Test can be run manually
>   too via 'ut mbr'.
> - Change mbr test to use mmc6.img dedicated to the test, instead of mmc1.img.
> - Clear read buffer before reading data from test file.
> 
>  arch/sandbox/dts/test.dts |   7 +
>  disk/part_dos.c           |   2 +-
>  include/test/suites.h     |   1 +
>  test/boot/bootdev.c       |   2 +-
>  test/cmd/Makefile         |   1 +
>  test/cmd/mbr.c            | 471 ++++++++++++++++++++++++++++++++++++++
>  test/cmd_ut.c             |   4 +
>  test/dm/blk.c             |  44 ++--

So, in addition to the changes to test/dm/blk.c I had gotten as far as:
diff --git a/test/boot/bootdev.c b/test/boot/bootdev.c
index 66c151131edb..33babdcf699c 100644
--- a/test/boot/bootdev.c
+++ b/test/boot/bootdev.c
@@ -43,8 +43,10 @@ static int bootdev_test_cmd_list(struct unit_test_state *uts)
                                   "mmc", "mmc1.bootdev");
                ut_assert_nextline("%3x   [ %c ]  %6s  %-8s  %s", 2, probe_ch, 
"OK",
                                   "mmc", "mmc0.bootdev");
+               ut_assert_nextline("%3x   [ %c ]  %6s  %-8s  %s", 3, probe_ch, 
"OK",
+                                  "mmc", "mmc6.bootdev");
                ut_assert_nextlinen("---");
-               ut_assert_nextline("(3 bootdevs)");
+               ut_assert_nextline("(4 bootdevs)");
                ut_assert_console_end();
        }
 
@@ -225,10 +227,11 @@ static int bootdev_test_order(struct unit_test_state *uts)
        ut_assertok(env_set("boot_targets", "mmc spi"));
        ut_asserteq(0, bootflow_scan_first(NULL, NULL, &iter, 0, &bflow));
        ut_asserteq(-ENODEV, bootflow_scan_next(&iter, &bflow));
-       ut_asserteq(3, iter.num_devs);
+       ut_asserteq(4, iter.num_devs);
        ut_asserteq_str("mmc2.bootdev", iter.dev_used[0]->name);
        ut_asserteq_str("mmc1.bootdev", iter.dev_used[1]->name);
        ut_asserteq_str("mmc0.bootdev", iter.dev_used[2]->name);
+       ut_asserteq_str("mmc6.bootdev", iter.dev_used[3]->name);
        bootflow_iter_uninit(&iter);
 
        return 0;
@@ -254,7 +257,7 @@ static int bootdev_test_order_default(struct 
unit_test_state *uts)
        ut_asserteq_str("mmc1.bootdev", iter.dev_used[1]->name);
 
        ut_asserteq(-ENODEV, bootflow_scan_next(&iter, &bflow));
-       ut_asserteq(3, iter.num_devs);
+       ut_asserteq(4, iter.num_devs);
        ut_asserteq_str("mmc0.bootdev", iter.dev_used[2]->name);
        bootflow_iter_uninit(&iter);
 
@@ -693,6 +696,10 @@ static int bootdev_test_next_prio(struct unit_test_state 
*uts)
        ut_asserteq_str("mmc0.bootdev", dev->name);
        ut_assert_console_end();
 
+       ut_assertok(bootdev_next_prio(&iter, &dev));
+       ut_asserteq_str("mmc6.bootdev", dev->name);
+       ut_assert_console_end();
+
        ut_assertok(bootdev_next_prio(&iter, &dev));
        ut_asserteq_str("spi.bin@0.bootdev", dev->name);
        ut_assert_skip_to_line("Hunting with: spi_flash");

To make the bootdev test pass (and they still don't with the above)
before deciding that it seems like there's a bigger problem here, and
maybe Simon can help explain or figure out what to do?  Is there no way
to drop mmc6 once we've completed the new test here?

-- 
Tom

signature.asc
Description: PGP signature