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

[Simon Glass]

Hi,

On Mon, 16 Oct 2023 at 10:14, Tom Rini <trini at konsulko.com> wrote:
>
> 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 <agendin at 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 at 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?

I did start looking at this. See u-boot-dm/mbr-working

In short you set status = "disabled" for mmc6 and then manually bind
it in the test.

But it still has one failure which I was trying to get to.

Regards,
Simon