Rather than do another review on [1] I thought it better to add a patch showing changes. This is intended to be squashed in, if acceptable.

With the patch as is, various bootstd tests fail. This is because the test adds a new mmc6 device. There is nothing really wrong with adding new devices, but it does require quite a few tests to be updated. It is simpler to have mmc6 disabled by default, then just enable it for the one test that uses it.

This patch does that and undoes the changes to blk.c and bootdev.c

[1] https://patchwork.ozlabs.org/project/uboot/patch/ZSNWVGTjPEt2jXn5@alg-gentoo...

Signed-off-by: Simon Glass sjg@chromium.org ---

arch/Kconfig | 2 ++ arch/sandbox/dts/test.dts | 1 + configs/sandbox_defconfig | 2 -- configs/sandbox_flattree_defconfig | 2 ++ test/boot/bootdev.c | 2 +- test/cmd/mbr.c | 13 ++++++++- test/dm/blk.c | 44 ++++++++++-------------------- 7 files changed, 32 insertions(+), 34 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig index 19f2891ba1c5..4f5b75129f34 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -208,6 +208,8 @@ config SANDBOX imply PHYSMEM imply GENERATE_ACPI_TABLE imply BINMAN + imply CMD_MBR + imply CMD_MMC

config SH bool "SuperH architecture" diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 3a8c2456d401..9fe2d63dcc50 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -1101,6 +1101,7 @@

/* This is used for mbr tests */ mmc6 { + status = "disabled"; compatible = "sandbox,mmc"; filename = "mmc6.img"; }; diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 01830c7bd255..a1c1a45a08a3 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -73,8 +73,6 @@ CONFIG_CMD_IDE=y CONFIG_CMD_I2C=y CONFIG_CMD_LOADM=y CONFIG_CMD_LSBLK=y -CONFIG_CMD_MBR=y -CONFIG_CMD_MMC=y CONFIG_CMD_MUX=y CONFIG_CMD_OSD=y CONFIG_CMD_PCI=y diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index 29ae4532c508..44ab80725fc7 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -47,10 +47,12 @@ CONFIG_CMD_MBR=y CONFIG_CMD_MMC=y CONFIG_CMD_OSD=y CONFIG_CMD_PCI=y +CONFIG_CMD_READ=y CONFIG_CMD_REMOTEPROC=y CONFIG_CMD_SPI=y CONFIG_CMD_TEMPERATURE=y CONFIG_CMD_USB=y +CONFIG_CMD_WRITE=y CONFIG_BOOTP_DNS2=y CONFIG_CMD_TFTPPUT=y CONFIG_CMD_TFTPSRV=y diff --git a/test/boot/bootdev.c b/test/boot/bootdev.c index 66c151131edb..c5f14a7a1323 100644 --- a/test/boot/bootdev.c +++ b/test/boot/bootdev.c @@ -128,7 +128,7 @@ static int bootdev_test_labels(struct unit_test_state *uts) bootdev_find_by_label("fred0", &dev, &mflags));

/* Check unknown sequence number */ - ut_asserteq(-ENOENT, bootdev_find_by_label("mmc7", &dev, &mflags)); + ut_asserteq(-ENOENT, bootdev_find_by_label("mmc6", &dev, &mflags));

return 0; } diff --git a/test/cmd/mbr.c b/test/cmd/mbr.c index 4091ea687fc5..751b16d605b7 100644 --- a/test/cmd/mbr.c +++ b/test/cmd/mbr.c @@ -7,11 +7,14 @@ */

#include <common.h> -#include <asm/global_data.h> +#include <dm.h> #include <console.h> #include <dm/test.h> #include <mapmem.h> #include <part.h> +#include <asm/global_data.h> +#include <dm/device-internal.h> +#include <dm/lists.h> #include <test/suites.h> #include <test/ut.h>

@@ -231,6 +234,14 @@ static int mbr_test_run(struct unit_test_state *uts) unsigned char mbr_wbuf[512], ebr_wbuf[512], rbuf[512]; char mbr_parts_buf[256]; ulong mbr_wa, ebr_wa, ra, ebr_blk, mbr_parts_max; + struct udevice *dev; + ofnode root, node; + + /* Enable the mmc6 node for this test */ + root = oftree_root(oftree_default()); + node = ofnode_find_subnode(root, "mmc6"); + ut_assert(ofnode_valid(node)); + ut_assertok(lists_bind_fdt(gd->dm_root, node, &dev, NULL, false));

mbr_parts_max = sizeof('\0') + 2 + strlen(mbr_parts_header) + diff --git a/test/dm/blk.c b/test/dm/blk.c index 993c617832a8..799f1e4dc75b 100644 --- a/test/dm/blk.c +++ b/test/dm/blk.c @@ -83,12 +83,12 @@ static int dm_test_blk_usb(struct unit_test_state *uts) ut_asserteq_ptr(usb_dev, dev_get_parent(dev));

/* Check we have one block device for each mass storage device */ - ut_asserteq(7, count_blk_devices()); + ut_asserteq(6, count_blk_devices());

/* Now go around again, making sure the old devices were unbound */ ut_assertok(usb_stop()); ut_assertok(usb_init()); - ut_asserteq(7, count_blk_devices()); + ut_asserteq(6, count_blk_devices()); ut_assertok(usb_stop());

return 0; @@ -191,8 +191,6 @@ static int dm_test_blk_iter(struct unit_test_state *uts) ut_asserteq_str("mmc1.blk", dev->name); ut_assertok(blk_next_device_err(BLKF_REMOVABLE, &dev)); ut_asserteq_str("mmc0.blk", dev->name); - ut_assertok(blk_next_device_err(BLKF_REMOVABLE, &dev)); - ut_asserteq_str("mmc6.blk", dev->name); ut_asserteq(-ENODEV, blk_next_device_err(BLKF_REMOVABLE, &dev));

ut_assertok(blk_first_device_err(BLKF_BOTH, &dev)); @@ -204,8 +202,8 @@ static int dm_test_blk_iter(struct unit_test_state *uts) ut_asserteq(-ENODEV, blk_next_device_err(BLKF_FIXED, &dev));

ut_asserteq(1, blk_count_devices(BLKF_FIXED)); - ut_asserteq(3, blk_count_devices(BLKF_REMOVABLE)); - ut_asserteq(4, blk_count_devices(BLKF_BOTH)); + ut_asserteq(2, blk_count_devices(BLKF_REMOVABLE)); + ut_asserteq(3, blk_count_devices(BLKF_BOTH));

i = 0; blk_foreach_probe(BLKF_FIXED, dev) @@ -214,16 +212,14 @@ static int dm_test_blk_iter(struct unit_test_state *uts)

i = 0; blk_foreach_probe(BLKF_REMOVABLE, dev) - ut_asserteq_str((++i == 1 ? "mmc1.blk" : i == 2 ? - "mmc0.blk" : "mmc6.blk"), dev->name); - ut_asserteq(3, i); + ut_asserteq_str(i++ ? "mmc0.blk" : "mmc1.blk", dev->name); + ut_asserteq(2, i);

i = 0; blk_foreach_probe(BLKF_BOTH, dev) ut_asserteq_str((++i == 1 ? "mmc2.blk" : i == 2 ? - "mmc1.blk" : i == 3 ? "mmc0.blk" : "mmc6.blk"), - dev->name); - ut_asserteq(4, i); + "mmc1.blk" : "mmc0.blk"), dev->name); + ut_asserteq(3, i);

return 0; } @@ -247,10 +243,6 @@ static int dm_test_blk_flags(struct unit_test_state *uts) ut_assertnonnull(dev); ut_asserteq_str("mmc0.blk", dev->name);

- ut_assertok(blk_find_next(BLKF_BOTH, &dev)); - ut_assertnonnull(dev); - ut_asserteq_str("mmc6.blk", dev->name); - ut_asserteq(-ENODEV, blk_find_next(BLKF_BOTH, &dev)); ut_assertnull(dev);

@@ -274,10 +266,6 @@ static int dm_test_blk_flags(struct unit_test_state *uts) ut_assertnonnull(dev); ut_asserteq_str("mmc0.blk", dev->name);

- ut_assertok(blk_next_device_err(BLKF_BOTH, &dev)); - ut_assertnonnull(dev); - ut_asserteq_str("mmc6.blk", dev->name); - ut_asserteq(-ENODEV, blk_next_device_err(BLKF_BOTH, &dev));

/* Look only for fixed devices */ @@ -296,10 +284,6 @@ static int dm_test_blk_flags(struct unit_test_state *uts) ut_assertnonnull(dev); ut_asserteq_str("mmc0.blk", dev->name);

- ut_assertok(blk_next_device_err(BLKF_REMOVABLE, &dev)); - ut_assertnonnull(dev); - ut_asserteq_str("mmc6.blk", dev->name); - ut_asserteq(-ENODEV, blk_next_device_err(BLKF_REMOVABLE, &dev));

return 0; @@ -316,7 +300,7 @@ static int dm_test_blk_foreach(struct unit_test_state *uts) found = 0; blk_foreach(BLKF_BOTH, dev) found |= 1 << dectoul(&dev->name[3], NULL); - ut_asserteq(0x47, found); + ut_asserteq(7, found);

/* All devices are removable until probed */ found = 0; @@ -327,14 +311,14 @@ static int dm_test_blk_foreach(struct unit_test_state *uts) found = 0; blk_foreach(BLKF_REMOVABLE, dev) found |= 1 << dectoul(&dev->name[3], NULL); - ut_asserteq(0x47, found); + ut_asserteq(7, found);

/* Now try again with the probing functions */ found = 0; blk_foreach_probe(BLKF_BOTH, dev) found |= 1 << dectoul(&dev->name[3], NULL); - ut_asserteq(0x47, found); - ut_asserteq(4, blk_count_devices(BLKF_BOTH)); + ut_asserteq(7, found); + ut_asserteq(3, blk_count_devices(BLKF_BOTH));

found = 0; blk_foreach_probe(BLKF_FIXED, dev) @@ -345,8 +329,8 @@ static int dm_test_blk_foreach(struct unit_test_state *uts) found = 0; blk_foreach_probe(BLKF_REMOVABLE, dev) found |= 1 << dectoul(&dev->name[3], NULL); - ut_asserteq(0x43, found); - ut_asserteq(3, blk_count_devices(BLKF_REMOVABLE)); + ut_asserteq(3, found); + ut_asserteq(2, blk_count_devices(BLKF_REMOVABLE));

return 0; }