[PATCH v2 1/3] cmd: fs: Use part_get_info_by_dev_and_name_or_num to parse partitions

[PATCH v2 1/3] cmd: fs: Use part_get_info_by_dev_and_name_or_num to parse partitions

[Sean Anderson]

This allows using dev#partlabel syntax.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

Changes in v2:
- Add stub for when CONFIG_PARTITIONS=n

 fs/fs.c        |  4 ++--
 include/part.h | 12 +++++++++++-
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/fs/fs.c b/fs/fs.c
index 900928c394..b7936fd4cf 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -385,8 +385,8 @@ int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype)
 	}
 #endif
 
-	part = blk_get_device_part_str(ifname, dev_part_str, &fs_dev_desc,
-					&fs_partition, 1);
+	part = part_get_info_by_dev_and_name_or_num(ifname, dev_part_str, &fs_dev_desc,
+						    &fs_partition, 1);
 	if (part < 0)
 		return -1;
 
diff --git a/include/part.h b/include/part.h
index 7f78271a98..419c859708 100644
--- a/include/part.h
+++ b/include/part.h
@@ -230,7 +230,7 @@ int part_get_info_by_name(struct blk_desc *dev_desc,
  * @param[in] allow_whole_dev true to allow the user to select partition 0
  *		(which means the whole device), false to require a valid
  *		partition number >= 1
- * @return 0 on success, or a negative on error
+ * @return the partition number on success, or negative errno on error
  */
 int part_get_info_by_dev_and_name_or_num(const char *dev_iface,
 					 const char *dev_part_str,
@@ -275,6 +275,16 @@ static inline int blk_get_device_part_str(const char *ifname,
 					  struct disk_partition *info,
 					  int allow_whole_dev)
 { *dev_desc = NULL; return -1; }
+static inline int
+part_get_info_by_dev_and_name_or_num(const char *dev_iface,
+				     const char *dev_part_str,
+				     struct blk_desc **dev_desc,
+				     struct disk_partition *part_info,
+				     int allow_whole_dev)
+{
+	*dev_desc = NULL;
+	return -ENOSYS;
+}
 #endif
 
 /*
-- 
2.25.1

[PATCH v2 2/3] part: Fix bogus return from part_get_info_by_dev_and_name

[Sean Anderson]

blk_get_device_by_str returns the device number on success. So we must
check if the return was negative to determine an error.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

Changes in v2:
- New

 disk/part.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/disk/part.c b/disk/part.c
index 80ced2ba88..5e7e59cf25 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -725,7 +725,7 @@ static int part_get_info_by_dev_and_name(const char *dev_iface,
 	}
 
 	ret = blk_get_device_by_str(dev_iface, dev_str, dev_desc);
-	if (ret)
+	if (ret < 0)
 		goto cleanup;
 
 	ret = part_get_info_by_name(*dev_desc, part_str, part_info);
-- 
2.25.1

[PATCH v2 3/3] test: Add test for partitions

[Sean Anderson]

This is technically a library function, but we use MMCs for testing, so
it is easier to do it with DM. At the moment, the only block devices in
sandbox are MMCs (AFAIK) so we just test with those.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

Changes in v2:
- New

 test/dm/Makefile |  1 +
 test/dm/part.c   | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)
 create mode 100644 test/dm/part.c

diff --git a/test/dm/Makefile b/test/dm/Makefile
index f5cc5540e8..7d017f8750 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -98,5 +98,6 @@ endif
 ifneq ($(CONFIG_EFI_PARTITION),)
 obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fastboot.o
 endif
+obj-$(CONFIG_EFI_PARTITION) += part.o
 endif
 endif # !SPL
diff --git a/test/dm/part.c b/test/dm/part.c
new file mode 100644
index 0000000000..051e9010b6
--- /dev/null
+++ b/test/dm/part.c
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2020 Sean Anderson <sean.anderson@seco.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <mmc.h>
+#include <part.h>
+#include <part_efi.h>
+#include <dm/test.h>
+#include <test/ut.h>
+
+static int dm_test_part(struct unit_test_state *uts)
+{
+	char str_disk_guid[UUID_STR_LEN + 1];
+	struct blk_desc *mmc_dev_desc;
+	struct disk_partition part_info;
+	struct disk_partition parts[2] = {
+		{
+			.start = 48, /* GPT data takes up the first 34 blocks or so */
+			.size = 1,
+			.name = "test1",
+		},
+		{
+			.start = 49,
+			.size = 1,
+			.name = "test2",
+		},
+	};
+
+	ut_asserteq(1, blk_get_device_by_str("mmc", "1", &mmc_dev_desc));
+	if (CONFIG_IS_ENABLED(RANDOM_UUID)) {
+		gen_rand_uuid_str(parts[0].uuid, UUID_STR_FORMAT_STD);
+		gen_rand_uuid_str(parts[1].uuid, UUID_STR_FORMAT_STD);
+		gen_rand_uuid_str(str_disk_guid, UUID_STR_FORMAT_STD);
+	}
+	ut_assertok(gpt_restore(mmc_dev_desc, str_disk_guid, parts,
+				ARRAY_SIZE(parts)));
+
+#define test(expected, part_str, whole) \
+	ut_asserteq(expected, \
+		    part_get_info_by_dev_and_name_or_num("mmc", part_str, \
+							 &mmc_dev_desc, \
+							 &part_info, whole))
+
+	test(-ENODEV, "", true);
+	env_set("bootdevice", "0");
+	test(0, "", true);
+	env_set("bootdevice", "1");
+	test(1, "", false);
+	test(1, "-", false);
+	env_set("bootdevice", "");
+	test(-EPROTONOSUPPORT, "0", false);
+	test(0, "0", true);
+	test(0, ":0", true);
+	test(0, ".0", true);
+	test(0, ".0:0", true);
+	test(-EINVAL, "#test1", true);
+	test(1, "1", false);
+	test(1, "1", true);
+	test(-ENOENT, "1:0", false);
+	test(0, "1:0", true);
+	test(1, "1:1", false);
+	test(2, "1:2", false);
+	test(1, "1.0", false);
+	test(0, "1.0:0", true);
+	test(1, "1.0:1", false);
+	test(2, "1.0:2", false);
+	test(-EINVAL, "1#bogus", false);
+	test(1, "1#test1", false);
+	test(2, "1#test2", false);
+
+	return 0;
+}
+DM_TEST(dm_test_part, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
-- 
2.25.1

[PATCH v2 2/3] part: Fix bogus return from part_get_info_by_dev_and_name

[Simon Glass]

On Mon, 12 Apr 2021 at 23:53, Sean Anderson <sean.anderson@seco.com> wrote:
>
> blk_get_device_by_str returns the device number on success. So we must
> check if the return was negative to determine an error.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
>
> Changes in v2:
> - New
>
>  disk/part.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 3/3] test: Add test for partitions

[Simon Glass]

Hi Sean,

On Mon, 12 Apr 2021 at 23:53, Sean Anderson <sean.anderson@seco.com> wrote:
>
> This is technically a library function, but we use MMCs for testing, so
> it is easier to do it with DM. At the moment, the only block devices in
> sandbox are MMCs (AFAIK) so we just test with those.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
>
> Changes in v2:
> - New
>
>  test/dm/Makefile |  1 +
>  test/dm/part.c   | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 77 insertions(+)
>  create mode 100644 test/dm/part.c
>
> diff --git a/test/dm/Makefile b/test/dm/Makefile
> index f5cc5540e8..7d017f8750 100644
> --- a/test/dm/Makefile
> +++ b/test/dm/Makefile
> @@ -98,5 +98,6 @@ endif
>  ifneq ($(CONFIG_EFI_PARTITION),)
>  obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fastboot.o
>  endif
> +obj-$(CONFIG_EFI_PARTITION) += part.o
>  endif
>  endif # !SPL
> diff --git a/test/dm/part.c b/test/dm/part.c
> new file mode 100644
> index 0000000000..051e9010b6
> --- /dev/null
> +++ b/test/dm/part.c
> @@ -0,0 +1,76 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2020 Sean Anderson <sean.anderson@seco.com>
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <mmc.h>
> +#include <part.h>
> +#include <part_efi.h>
> +#include <dm/test.h>
> +#include <test/ut.h>
> +
> +static int dm_test_part(struct unit_test_state *uts)
> +{
> +       char str_disk_guid[UUID_STR_LEN + 1];
> +       struct blk_desc *mmc_dev_desc;
> +       struct disk_partition part_info;
> +       struct disk_partition parts[2] = {
> +               {
> +                       .start = 48, /* GPT data takes up the first 34 blocks or so */
> +                       .size = 1,
> +                       .name = "test1",
> +               },
> +               {
> +                       .start = 49,
> +                       .size = 1,
> +                       .name = "test2",
> +               },
> +       };
> +
> +       ut_asserteq(1, blk_get_device_by_str("mmc", "1", &mmc_dev_desc));
> +       if (CONFIG_IS_ENABLED(RANDOM_UUID)) {
> +               gen_rand_uuid_str(parts[0].uuid, UUID_STR_FORMAT_STD);
> +               gen_rand_uuid_str(parts[1].uuid, UUID_STR_FORMAT_STD);
> +               gen_rand_uuid_str(str_disk_guid, UUID_STR_FORMAT_STD);
> +       }
> +       ut_assertok(gpt_restore(mmc_dev_desc, str_disk_guid, parts,
> +                               ARRAY_SIZE(parts)));
> +
> +#define test(expected, part_str, whole) \

Can this be a function instead of a macro?

> +       ut_asserteq(expected, \
> +                   part_get_info_by_dev_and_name_or_num("mmc", part_str, \
> +                                                        &mmc_dev_desc, \
> +                                                        &part_info, whole))
> +
> +       test(-ENODEV, "", true);
> +       env_set("bootdevice", "0");
> +       test(0, "", true);
> +       env_set("bootdevice", "1");
> +       test(1, "", false);
> +       test(1, "-", false);
> +       env_set("bootdevice", "");
> +       test(-EPROTONOSUPPORT, "0", false);
> +       test(0, "0", true);
> +       test(0, ":0", true);
> +       test(0, ".0", true);
> +       test(0, ".0:0", true);
> +       test(-EINVAL, "#test1", true);
> +       test(1, "1", false);
> +       test(1, "1", true);
> +       test(-ENOENT, "1:0", false);
> +       test(0, "1:0", true);
> +       test(1, "1:1", false);
> +       test(2, "1:2", false);
> +       test(1, "1.0", false);
> +       test(0, "1.0:0", true);
> +       test(1, "1.0:1", false);
> +       test(2, "1.0:2", false);
> +       test(-EINVAL, "1#bogus", false);
> +       test(1, "1#test1", false);
> +       test(2, "1#test2", false);
> +
> +       return 0;
> +}
> +DM_TEST(dm_test_part, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
> --
> 2.25.1
>

Regards,
Simon

[PATCH v2 3/3] test: Add test for partitions

[Sean Anderson]

On 4/14/21 3:37 PM, Simon Glass wrote:
 > Hi Sean,
 >
 > On Mon, 12 Apr 2021 at 23:53, Sean Anderson <sean.anderson@seco.com> wrote:
 >>
 >> This is technically a library function, but we use MMCs for testing, so
 >> it is easier to do it with DM. At the moment, the only block devices in
 >> sandbox are MMCs (AFAIK) so we just test with those.
 >>
 >> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
 >> ---
 >>
 >> Changes in v2:
 >> - New
 >>
 >>   test/dm/Makefile |  1 +
 >>   test/dm/part.c   | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
 >>   2 files changed, 77 insertions(+)
 >>   create mode 100644 test/dm/part.c
 >>
 >> diff --git a/test/dm/Makefile b/test/dm/Makefile
 >> index f5cc5540e8..7d017f8750 100644
 >> --- a/test/dm/Makefile
 >> +++ b/test/dm/Makefile
 >> @@ -98,5 +98,6 @@ endif
 >>   ifneq ($(CONFIG_EFI_PARTITION),)
 >>   obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fastboot.o
 >>   endif
 >> +obj-$(CONFIG_EFI_PARTITION) += part.o
 >>   endif
 >>   endif # !SPL
 >> diff --git a/test/dm/part.c b/test/dm/part.c
 >> new file mode 100644
 >> index 0000000000..051e9010b6
 >> --- /dev/null
 >> +++ b/test/dm/part.c
 >> @@ -0,0 +1,76 @@
 >> +// SPDX-License-Identifier: GPL-2.0+
 >> +/*
 >> + * Copyright (C) 2020 Sean Anderson <sean.anderson@seco.com>
 >> + */
 >> +
 >> +#include <common.h>
 >> +#include <dm.h>
 >> +#include <mmc.h>
 >> +#include <part.h>
 >> +#include <part_efi.h>
 >> +#include <dm/test.h>
 >> +#include <test/ut.h>
 >> +
 >> +static int dm_test_part(struct unit_test_state *uts)
 >> +{
 >> +       char str_disk_guid[UUID_STR_LEN + 1];
 >> +       struct blk_desc *mmc_dev_desc;
 >> +       struct disk_partition part_info;
 >> +       struct disk_partition parts[2] = {
 >> +               {
 >> +                       .start = 48, /* GPT data takes up the first 34 blocks or so */
 >> +                       .size = 1,
 >> +                       .name = "test1",
 >> +               },
 >> +               {
 >> +                       .start = 49,
 >> +                       .size = 1,
 >> +                       .name = "test2",
 >> +               },
 >> +       };
 >> +
 >> +       ut_asserteq(1, blk_get_device_by_str("mmc", "1", &mmc_dev_desc));
 >> +       if (CONFIG_IS_ENABLED(RANDOM_UUID)) {
 >> +               gen_rand_uuid_str(parts[0].uuid, UUID_STR_FORMAT_STD);
 >> +               gen_rand_uuid_str(parts[1].uuid, UUID_STR_FORMAT_STD);
 >> +               gen_rand_uuid_str(str_disk_guid, UUID_STR_FORMAT_STD);
 >> +       }
 >> +       ut_assertok(gpt_restore(mmc_dev_desc, str_disk_guid, parts,
 >> +                               ARRAY_SIZE(parts)));
 >> +
 >> +#define test(expected, part_str, whole) \
 >
 > Can this be a function instead of a macro?

Not one-to-one because ut-asserteq returns on error. This could be
changed to

	ut_asserteq(-ENODEV, test("", true));

but I think a macro is the simplest option.

--Sean

 >
 >> +       ut_asserteq(expected, \
 >> +                   part_get_info_by_dev_and_name_or_num("mmc", part_str, \
 >> +                                                        &mmc_dev_desc, \
 >> +                                                        &part_info, whole))
 >> +
 >> +       test(-ENODEV, "", true);
 >> +       env_set("bootdevice", "0");
 >> +       test(0, "", true);
 >> +       env_set("bootdevice", "1");
 >> +       test(1, "", false);
 >> +       test(1, "-", false);
 >> +       env_set("bootdevice", "");
 >> +       test(-EPROTONOSUPPORT, "0", false);
 >> +       test(0, "0", true);
 >> +       test(0, ":0", true);
 >> +       test(0, ".0", true);
 >> +       test(0, ".0:0", true);
 >> +       test(-EINVAL, "#test1", true);
 >> +       test(1, "1", false);
 >> +       test(1, "1", true);
 >> +       test(-ENOENT, "1:0", false);
 >> +       test(0, "1:0", true);
 >> +       test(1, "1:1", false);
 >> +       test(2, "1:2", false);
 >> +       test(1, "1.0", false);
 >> +       test(0, "1.0:0", true);
 >> +       test(1, "1.0:1", false);
 >> +       test(2, "1.0:2", false);
 >> +       test(-EINVAL, "1#bogus", false);
 >> +       test(1, "1#test1", false);
 >> +       test(2, "1#test2", false);
 >> +
 >> +       return 0;
 >> +}
 >> +DM_TEST(dm_test_part, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
 >> --
 >> 2.25.1
 >>
 >
 > Regards,
 > Simon
 >

[PATCH v2 3/3] test: Add test for partitions

[Simon Glass]

Hi Sean,

On Fri, 16 Apr 2021 at 02:36, Sean Anderson <sean.anderson@seco.com> wrote:
>
>
>
> On 4/14/21 3:37 PM, Simon Glass wrote:
>  > Hi Sean,
>  >
>  > On Mon, 12 Apr 2021 at 23:53, Sean Anderson <sean.anderson@seco.com> wrote:
>  >>
>  >> This is technically a library function, but we use MMCs for testing, so
>  >> it is easier to do it with DM. At the moment, the only block devices in
>  >> sandbox are MMCs (AFAIK) so we just test with those.
>  >>
>  >> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
>  >> ---
>  >>
>  >> Changes in v2:
>  >> - New
>  >>
>  >>   test/dm/Makefile |  1 +
>  >>   test/dm/part.c   | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
>  >>   2 files changed, 77 insertions(+)
>  >>   create mode 100644 test/dm/part.c
>  >>
>  >> diff --git a/test/dm/Makefile b/test/dm/Makefile
>  >> index f5cc5540e8..7d017f8750 100644
>  >> --- a/test/dm/Makefile
>  >> +++ b/test/dm/Makefile
>  >> @@ -98,5 +98,6 @@ endif
>  >>   ifneq ($(CONFIG_EFI_PARTITION),)
>  >>   obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fastboot.o
>  >>   endif
>  >> +obj-$(CONFIG_EFI_PARTITION) += part.o
>  >>   endif
>  >>   endif # !SPL
>  >> diff --git a/test/dm/part.c b/test/dm/part.c
>  >> new file mode 100644
>  >> index 0000000000..051e9010b6
>  >> --- /dev/null
>  >> +++ b/test/dm/part.c
>  >> @@ -0,0 +1,76 @@
>  >> +// SPDX-License-Identifier: GPL-2.0+
>  >> +/*
>  >> + * Copyright (C) 2020 Sean Anderson <sean.anderson@seco.com>
>  >> + */
>  >> +
>  >> +#include <common.h>
>  >> +#include <dm.h>
>  >> +#include <mmc.h>
>  >> +#include <part.h>
>  >> +#include <part_efi.h>
>  >> +#include <dm/test.h>
>  >> +#include <test/ut.h>
>  >> +
>  >> +static int dm_test_part(struct unit_test_state *uts)
>  >> +{
>  >> +       char str_disk_guid[UUID_STR_LEN + 1];
>  >> +       struct blk_desc *mmc_dev_desc;
>  >> +       struct disk_partition part_info;
>  >> +       struct disk_partition parts[2] = {
>  >> +               {
>  >> +                       .start = 48, /* GPT data takes up the first 34 blocks or so */
>  >> +                       .size = 1,
>  >> +                       .name = "test1",
>  >> +               },
>  >> +               {
>  >> +                       .start = 49,
>  >> +                       .size = 1,
>  >> +                       .name = "test2",
>  >> +               },
>  >> +       };
>  >> +
>  >> +       ut_asserteq(1, blk_get_device_by_str("mmc", "1", &mmc_dev_desc));
>  >> +       if (CONFIG_IS_ENABLED(RANDOM_UUID)) {
>  >> +               gen_rand_uuid_str(parts[0].uuid, UUID_STR_FORMAT_STD);
>  >> +               gen_rand_uuid_str(parts[1].uuid, UUID_STR_FORMAT_STD);
>  >> +               gen_rand_uuid_str(str_disk_guid, UUID_STR_FORMAT_STD);
>  >> +       }
>  >> +       ut_assertok(gpt_restore(mmc_dev_desc, str_disk_guid, parts,
>  >> +                               ARRAY_SIZE(parts)));
>  >> +
>  >> +#define test(expected, part_str, whole) \
>  >
>  > Can this be a function instead of a macro?
>
> Not one-to-one because ut-asserteq returns on error. This could be
> changed to
>
>         ut_asserteq(-ENODEV, test("", true));
>
> but I think a macro is the simplest option.

Well you are using ut_asserteq() in the macro so I don't see why the
macro is better than what you have above, with the code in a function?
That is what we normally do.


>
> --Sean
>
>  >
>  >> +       ut_asserteq(expected, \
>  >> +                   part_get_info_by_dev_and_name_or_num("mmc", part_str, \
>  >> +                                                        &mmc_dev_desc, \
>  >> +                                                        &part_info, whole))
>  >> +
>  >> +       test(-ENODEV, "", true);
>  >> +       env_set("bootdevice", "0");
>  >> +       test(0, "", true);
>  >> +       env_set("bootdevice", "1");
>  >> +       test(1, "", false);
>  >> +       test(1, "-", false);
>  >> +       env_set("bootdevice", "");
>  >> +       test(-EPROTONOSUPPORT, "0", false);
>  >> +       test(0, "0", true);
>  >> +       test(0, ":0", true);
>  >> +       test(0, ".0", true);
>  >> +       test(0, ".0:0", true);
>  >> +       test(-EINVAL, "#test1", true);
>  >> +       test(1, "1", false);
>  >> +       test(1, "1", true);
>  >> +       test(-ENOENT, "1:0", false);
>  >> +       test(0, "1:0", true);
>  >> +       test(1, "1:1", false);
>  >> +       test(2, "1:2", false);
>  >> +       test(1, "1.0", false);
>  >> +       test(0, "1.0:0", true);
>  >> +       test(1, "1.0:1", false);
>  >> +       test(2, "1.0:2", false);
>  >> +       test(-EINVAL, "1#bogus", false);
>  >> +       test(1, "1#test1", false);
>  >> +       test(2, "1#test2", false);
>  >> +
>  >> +       return 0;
>  >> +}
>  >> +DM_TEST(dm_test_part, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
>  >> --
>  >> 2.25.1
>  >>

Regards,
Simon

[PATCH v2 3/3] test: Add test for partitions

[Sean Anderson]

On 4/21/21 3:14 AM, Simon Glass wrote:
 > Hi Sean,
 >
 > On Fri, 16 Apr 2021 at 02:36, Sean Anderson <sean.anderson@seco.com> wrote:
 >>
 >>
 >>
 >> On 4/14/21 3:37 PM, Simon Glass wrote:
 >>   > Hi Sean,
 >>   >
 >>   > On Mon, 12 Apr 2021 at 23:53, Sean Anderson <sean.anderson@seco.com> wrote:
 >>   >>
 >>   >> This is technically a library function, but we use MMCs for testing, so
 >>   >> it is easier to do it with DM. At the moment, the only block devices in
 >>   >> sandbox are MMCs (AFAIK) so we just test with those.
 >>   >>
 >>   >> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
 >>   >> ---
 >>   >>
 >>   >> Changes in v2:
 >>   >> - New
 >>   >>
 >>   >>   test/dm/Makefile |  1 +
 >>   >>   test/dm/part.c   | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
 >>   >>   2 files changed, 77 insertions(+)
 >>   >>   create mode 100644 test/dm/part.c
 >>   >>
 >>   >> diff --git a/test/dm/Makefile b/test/dm/Makefile
 >>   >> index f5cc5540e8..7d017f8750 100644
 >>   >> --- a/test/dm/Makefile
 >>   >> +++ b/test/dm/Makefile
 >>   >> @@ -98,5 +98,6 @@ endif
 >>   >>   ifneq ($(CONFIG_EFI_PARTITION),)
 >>   >>   obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fastboot.o
 >>   >>   endif
 >>   >> +obj-$(CONFIG_EFI_PARTITION) += part.o
 >>   >>   endif
 >>   >>   endif # !SPL
 >>   >> diff --git a/test/dm/part.c b/test/dm/part.c
 >>   >> new file mode 100644
 >>   >> index 0000000000..051e9010b6
 >>   >> --- /dev/null
 >>   >> +++ b/test/dm/part.c
 >>   >> @@ -0,0 +1,76 @@
 >>   >> +// SPDX-License-Identifier: GPL-2.0+
 >>   >> +/*
 >>   >> + * Copyright (C) 2020 Sean Anderson <sean.anderson@seco.com>
 >>   >> + */
 >>   >> +
 >>   >> +#include <common.h>
 >>   >> +#include <dm.h>
 >>   >> +#include <mmc.h>
 >>   >> +#include <part.h>
 >>   >> +#include <part_efi.h>
 >>   >> +#include <dm/test.h>
 >>   >> +#include <test/ut.h>
 >>   >> +
 >>   >> +static int dm_test_part(struct unit_test_state *uts)
 >>   >> +{
 >>   >> +       char str_disk_guid[UUID_STR_LEN + 1];
 >>   >> +       struct blk_desc *mmc_dev_desc;
 >>   >> +       struct disk_partition part_info;
 >>   >> +       struct disk_partition parts[2] = {
 >>   >> +               {
 >>   >> +                       .start = 48, /* GPT data takes up the first 34 blocks or so */
 >>   >> +                       .size = 1,
 >>   >> +                       .name = "test1",
 >>   >> +               },
 >>   >> +               {
 >>   >> +                       .start = 49,
 >>   >> +                       .size = 1,
 >>   >> +                       .name = "test2",
 >>   >> +               },
 >>   >> +       };
 >>   >> +
 >>   >> +       ut_asserteq(1, blk_get_device_by_str("mmc", "1", &mmc_dev_desc));
 >>   >> +       if (CONFIG_IS_ENABLED(RANDOM_UUID)) {
 >>   >> +               gen_rand_uuid_str(parts[0].uuid, UUID_STR_FORMAT_STD);
 >>   >> +               gen_rand_uuid_str(parts[1].uuid, UUID_STR_FORMAT_STD);
 >>   >> +               gen_rand_uuid_str(str_disk_guid, UUID_STR_FORMAT_STD);
 >>   >> +       }
 >>   >> +       ut_assertok(gpt_restore(mmc_dev_desc, str_disk_guid, parts,
 >>   >> +                               ARRAY_SIZE(parts)));
 >>   >> +
 >>   >> +#define test(expected, part_str, whole) \
 >>   >
 >>   > Can this be a function instead of a macro?
 >>
 >> Not one-to-one because ut-asserteq returns on error. This could be
 >> changed to
 >>
 >>          ut_asserteq(-ENODEV, test("", true));
 >>
 >> but I think a macro is the simplest option.
 >
 > Well you are using ut_asserteq() in the macro so I don't see why the
 > macro is better than what you have above, with the code in a function?
 > That is what we normally do.

I suppose. It doesn't really matter too much IMO, but I will change it
for v3.

--Sean

 >
 >
 >>
 >> --Sean
 >>
 >>   >
 >>   >> +       ut_asserteq(expected, \
 >>   >> +                   part_get_info_by_dev_and_name_or_num("mmc", part_str, \
 >>   >> +                                                        &mmc_dev_desc, \
 >>   >> +                                                        &part_info, whole))
 >>   >> +
 >>   >> +       test(-ENODEV, "", true);
 >>   >> +       env_set("bootdevice", "0");
 >>   >> +       test(0, "", true);
 >>   >> +       env_set("bootdevice", "1");
 >>   >> +       test(1, "", false);
 >>   >> +       test(1, "-", false);
 >>   >> +       env_set("bootdevice", "");
 >>   >> +       test(-EPROTONOSUPPORT, "0", false);
 >>   >> +       test(0, "0", true);
 >>   >> +       test(0, ":0", true);
 >>   >> +       test(0, ".0", true);
 >>   >> +       test(0, ".0:0", true);
 >>   >> +       test(-EINVAL, "#test1", true);
 >>   >> +       test(1, "1", false);
 >>   >> +       test(1, "1", true);
 >>   >> +       test(-ENOENT, "1:0", false);
 >>   >> +       test(0, "1:0", true);
 >>   >> +       test(1, "1:1", false);
 >>   >> +       test(2, "1:2", false);
 >>   >> +       test(1, "1.0", false);
 >>   >> +       test(0, "1.0:0", true);
 >>   >> +       test(1, "1.0:1", false);
 >>   >> +       test(2, "1.0:2", false);
 >>   >> +       test(-EINVAL, "1#bogus", false);
 >>   >> +       test(1, "1#test1", false);
 >>   >> +       test(2, "1#test2", false);
 >>   >> +
 >>   >> +       return 0;
 >>   >> +}
 >>   >> +DM_TEST(dm_test_part, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
 >>   >> --
 >>   >> 2.25.1
 >>   >>
 >
 > Regards,
 > Simon
 >

[PATCH v2 1/3] cmd: fs: Use part_get_info_by_dev_and_name_or_num to parse partitions

[Tom Rini]

On Mon, Apr 12, 2021 at 06:53:05PM -0400, Sean Anderson wrote:

> This allows using dev#partlabel syntax.
> 
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210423/085e7365/attachment.sig>

[PATCH v2 2/3] part: Fix bogus return from part_get_info_by_dev_and_name

[Tom Rini]

On Mon, Apr 12, 2021 at 06:53:06PM -0400, Sean Anderson wrote:

> blk_get_device_by_str returns the device number on success. So we must
> check if the return was negative to determine an error.
> 
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210423/adb296da/attachment.sig>

[PATCH v2 3/3] test: Add test for partitions

[Tom Rini]

On Mon, Apr 12, 2021 at 06:53:07PM -0400, Sean Anderson wrote:

> This is technically a library function, but we use MMCs for testing, so
> it is easier to do it with DM. At the moment, the only block devices in
> sandbox are MMCs (AFAIK) so we just test with those.
> 
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210423/3f10fd20/attachment.sig>

[BUG] [PATCH v2 1/3] cmd: fs: Use part_get_info_by_dev_and_name_or_num to parse partitions

[Heinrich Schuchardt]

On 4/13/21 12:53 AM, Sean Anderson wrote:
> This allows using dev#partlabel syntax.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Since this patch was merged as commit 7194527b the command 'ls host'
leads to a segmentation violation:

=> host bind 0 ../sandbox.img
=> setenv bootdevice 0:1
=> ls host

Segmentation violation
pc = 0x55a5b66b7ec1, pc_reloc = 0x55a5a66b7ec1

Writing sandbox state

Before the patch:

=> host bind 0 ../sandbox.img
=> setenv bootdevice 0:1
=> ls host
       148   boot.scr
             Test/

1 file(s), 1 dir(s)

Please, provide a fix for your patch and a unit test using 'setenv
bootdevice' to avoid future mischief.

Best regards

Heinrich