[PATCH 0/9] This series adds support for flashing eMMC boot partitions, and for

[PATCH 0/9] This series adds support for flashing eMMC boot partitions, and for

[Sean Anderson]

flashing whole partitions. Specifically, it does this by using the
existing U-Boot naming scheme to specify partitions, and not by adding
new KConfig options.

I have added tests for partition naming, but not for the whole flash
process (though that could be a future project). I have tested this on
one board, but I have *not* tested the mt85-specific KConfigs. Hopefully
this series can be a way to phase out those options.


Sean Anderson (9):
  mmc: sandbox: Add support for writing
  test: dm: Add test for fastboot mmc partition naming
  part: Give several functions more useful return values
  part: Support getting whole disk from
    part_get_info_by_dev_and_name_or_num
  part: Support string block devices in part_get_info_by_dev_and_name
  fastboot: Remove mmcpart argument from raw_part_get_info_by_name
  fastboot: Move part_get_info_by_name_or_alias after
    raw_part_get_info_by_name
  fastboot: Allow u-boot-style partitions
  fastboot: Document alternate partition names

 cmd/ab_select.c             |   3 +-
 configs/sandbox64_defconfig |   2 +
 configs/sandbox_defconfig   |   2 +
 disk/part.c                 |  81 +++++++-------
 doc/android/fastboot.rst    |  28 +++++
 drivers/fastboot/fb_mmc.c   | 203 +++++++++++++++++++++---------------
 drivers/mmc/sandbox_mmc.c   |  41 +++++++-
 include/part.h              |   6 +-
 test/dm/Makefile            |   3 +
 test/dm/fastboot.c          |  95 +++++++++++++++++
 test/dm/mmc.c               |  19 +++-
 11 files changed, 349 insertions(+), 134 deletions(-)
 create mode 100644 test/dm/fastboot.c

-- 
2.25.1

[PATCH 1/9] mmc: sandbox: Add support for writing

[Sean Anderson]

This adds support writing to the sandbox mmc backed by an in-memory buffer.
The unit test has been updated to test reading, writing, and erasing. I'm
not sure what MMC erase to; I picked 0, but if it's 0xFF then that can be
easily changed.

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

 drivers/mmc/sandbox_mmc.c | 41 ++++++++++++++++++++++++++++++++++-----
 test/dm/mmc.c             | 19 +++++++++++++-----
 2 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/sandbox_mmc.c b/drivers/mmc/sandbox_mmc.c
index e86ea8fe09..3daf708451 100644
--- a/drivers/mmc/sandbox_mmc.c
+++ b/drivers/mmc/sandbox_mmc.c
@@ -17,6 +17,17 @@ struct sandbox_mmc_plat {
 	struct mmc mmc;
 };
 
+#define MMC_CSIZE 0
+#define MMC_CMULT 8 /* 8 because the card is high-capacity */
+#define MMC_BL_LEN_SHIFT 10
+#define MMC_BL_LEN BIT(MMC_BL_LEN_SHIFT)
+#define MMC_CAPACITY (((MMC_CSIZE + 1) << (MMC_CMULT + 2)) \
+		      * MMC_BL_LEN) /* 1 MiB */
+
+struct sandbox_mmc_priv {
+	u8 buf[MMC_CAPACITY];
+};
+
 /**
  * sandbox_mmc_send_cmd() - Emulate SD commands
  *
@@ -26,6 +37,10 @@ struct sandbox_mmc_plat {
 static int sandbox_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
 				struct mmc_data *data)
 {
+	struct sandbox_mmc_priv *priv = dev_get_priv(dev);
+	struct mmc *mmc = mmc_get_mmc_dev(dev);
+	static ulong erase_start, erase_end;
+
 	switch (cmd->cmdidx) {
 	case MMC_CMD_ALL_SEND_CID:
 		memset(cmd->response, '\0', sizeof(cmd->response));
@@ -44,8 +59,9 @@ static int sandbox_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
 		break;
 	case MMC_CMD_SEND_CSD:
 		cmd->response[0] = 0;
-		cmd->response[1] = 10 << 16;	/* 1 << block_len */
-		cmd->response[2] = 0;
+		cmd->response[1] = (MMC_BL_LEN_SHIFT << 16) |
+				   ((MMC_CSIZE >> 16) & 0x3f);
+		cmd->response[2] = (MMC_CSIZE & 0xffff) << 16;
 		cmd->response[3] = 0;
 		break;
 	case SD_CMD_SWITCH_FUNC: {
@@ -59,13 +75,27 @@ static int sandbox_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
 		break;
 	}
 	case MMC_CMD_READ_SINGLE_BLOCK:
-		memset(data->dest, '\0', data->blocksize);
-		break;
 	case MMC_CMD_READ_MULTIPLE_BLOCK:
-		strcpy(data->dest, "this is a test");
+		memcpy(data->dest, &priv->buf[cmd->cmdarg * data->blocksize],
+		       data->blocks * data->blocksize);
+		break;
+	case MMC_CMD_WRITE_SINGLE_BLOCK:
+	case MMC_CMD_WRITE_MULTIPLE_BLOCK:
+		memcpy(&priv->buf[cmd->cmdarg * data->blocksize], data->src,
+		       data->blocks * data->blocksize);
 		break;
 	case MMC_CMD_STOP_TRANSMISSION:
 		break;
+	case SD_CMD_ERASE_WR_BLK_START:
+		erase_start = cmd->cmdarg;
+		break;
+	case SD_CMD_ERASE_WR_BLK_END:
+		erase_end = cmd->cmdarg;
+		break;
+	case MMC_CMD_ERASE:
+		memset(&priv->buf[erase_start * mmc->write_bl_len], '\0',
+		       (erase_end - erase_start + 1) * mmc->write_bl_len);
+		break;
 	case SD_CMD_APP_SEND_OP_COND:
 		cmd->response[0] = OCR_BUSY | OCR_HCS;
 		cmd->response[1] = 0;
@@ -148,5 +178,6 @@ U_BOOT_DRIVER(mmc_sandbox) = {
 	.bind		= sandbox_mmc_bind,
 	.unbind		= sandbox_mmc_unbind,
 	.probe		= sandbox_mmc_probe,
+	.priv_auto_alloc_size = sizeof(struct sandbox_mmc_priv),
 	.platdata_auto_alloc_size = sizeof(struct sandbox_mmc_plat),
 };
diff --git a/test/dm/mmc.c b/test/dm/mmc.c
index 4e5136c850..f744452ff2 100644
--- a/test/dm/mmc.c
+++ b/test/dm/mmc.c
@@ -29,16 +29,25 @@ static int dm_test_mmc_blk(struct unit_test_state *uts)
 {
 	struct udevice *dev;
 	struct blk_desc *dev_desc;
-	char cmp[1024];
+	int i;
+	char write[1024], read[1024];
 
 	ut_assertok(uclass_get_device(UCLASS_MMC, 0, &dev));
 	ut_assertok(blk_get_device_by_str("mmc", "0", &dev_desc));
 
-	/* Read a few blocks and look for the string we expect */
+	/* Write a few blocks and verify that we get the same data back */
 	ut_asserteq(512, dev_desc->blksz);
-	memset(cmp, '\0', sizeof(cmp));
-	ut_asserteq(2, blk_dread(dev_desc, 0, 2, cmp));
-	ut_assertok(strcmp(cmp, "this is a test"));
+	for (i = 0; i < sizeof(write); i++)
+		write[i] = i;
+	ut_asserteq(2, blk_dwrite(dev_desc, 0, 2, write));
+	ut_asserteq(2, blk_dread(dev_desc, 0, 2, read));
+	ut_asserteq_mem(write, read, sizeof(write));
+
+	/* Now erase them */
+	memset(write, '\0', sizeof(write));
+	ut_asserteq(2, blk_derase(dev_desc, 0, 2));
+	ut_asserteq(2, blk_dread(dev_desc, 0, 2, read));
+	ut_asserteq_mem(write, read, sizeof(write));
 
 	return 0;
 }
-- 
2.25.1

[PATCH 2/9] test: dm: Add test for fastboot mmc partition naming

[Sean Anderson]

This test verifies the mapping between fastboot partitions and partitions
as understood by U-Boot. It also tests the creation of GPT partitions,
though that is not the primary goal.

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

 configs/sandbox64_defconfig |  2 ++
 configs/sandbox_defconfig   |  2 ++
 test/dm/Makefile            |  3 ++
 test/dm/fastboot.c          | 64 +++++++++++++++++++++++++++++++++++++
 4 files changed, 71 insertions(+)
 create mode 100644 test/dm/fastboot.c

diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 5fbbfd7236..e61aa71fbc 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -109,6 +109,8 @@ CONFIG_CPU=y
 CONFIG_DM_DEMO=y
 CONFIG_DM_DEMO_SIMPLE=y
 CONFIG_DM_DEMO_SHAPE=y
+CONFIG_FASTBOOT_FLASH=y
+CONFIG_FASTBOOT_FLASH_MMC_DEV=0
 CONFIG_GPIO_HOG=y
 CONFIG_DM_GPIO_LOOKUP_LABEL=y
 CONFIG_PM8916_GPIO=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index f1ec701a9f..ba53426df4 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -134,6 +134,8 @@ CONFIG_DM_DEMO_SHAPE=y
 CONFIG_DMA=y
 CONFIG_DMA_CHANNELS=y
 CONFIG_SANDBOX_DMA=y
+CONFIG_FASTBOOT_FLASH=y
+CONFIG_FASTBOOT_FLASH_MMC_DEV=0
 CONFIG_GPIO_HOG=y
 CONFIG_DM_GPIO_LOOKUP_LABEL=y
 CONFIG_PM8916_GPIO=y
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 46e076ed09..be4385c709 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -91,5 +91,8 @@ obj-$(CONFIG_SCMI_FIRMWARE) += scmi.o
 ifneq ($(CONFIG_PINMUX),)
 obj-$(CONFIG_PINCONF) += pinmux.o
 endif
+ifneq ($(CONFIG_EFI_PARTITION)$(CONFIG_FASTBOOT_FLASH_MMC),)
+obj-y += fastboot.o
+endif
 endif
 endif # !SPL
diff --git a/test/dm/fastboot.c b/test/dm/fastboot.c
new file mode 100644
index 0000000000..8f905d8fa8
--- /dev/null
+++ b/test/dm/fastboot.c
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2015 Google, Inc
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <fastboot.h>
+#include <fb_mmc.h>
+#include <mmc.h>
+#include <part.h>
+#include <part_efi.h>
+#include <dm/test.h>
+#include <test/ut.h>
+#include <linux/stringify.h>
+
+#define FB_ALIAS_PREFIX "fastboot_partition_alias_"
+
+static int dm_test_fastboot_mmc_part(struct unit_test_state *uts)
+{
+	char response[FASTBOOT_RESPONSE_LEN] = {0};
+	char str_disk_guid[UUID_STR_LEN + 1];
+	struct blk_desc *mmc_dev_desc, *fb_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_assertok(blk_get_device_by_str("mmc",
+					  __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV),
+					  &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)));
+
+	/* "Classic" partition labels */
+	ut_asserteq(1, fastboot_mmc_get_part_info("test1", &fb_dev_desc,
+						  &part_info, response));
+	ut_asserteq(2, fastboot_mmc_get_part_info("test2", &fb_dev_desc,
+						  &part_info, response));
+
+	/* Test aliases */
+	ut_assertnull(env_get(FB_ALIAS_PREFIX "test3"));
+	ut_assertok(env_set(FB_ALIAS_PREFIX "test3", "test1"));
+	ut_asserteq(1, fastboot_mmc_get_part_info("test3", &fb_dev_desc,
+						  &part_info, response));
+	ut_assertok(env_set(FB_ALIAS_PREFIX "test3", NULL));
+
+	return 0;
+}
+DM_TEST(dm_test_fastboot_mmc_part, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
-- 
2.25.1

[PATCH 3/9] part: Give several functions more useful return values

[Sean Anderson]

Several functions in disk/part.c just return -1 on error. This makes them
return different errnos for different failures. This helps callers
differentiate between failures, even if they cannot read stdout.

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

 disk/part.c | 50 ++++++++++++++++++++++++++++----------------------
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/disk/part.c b/disk/part.c
index b69fd345f3..5e354e256f 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -354,7 +354,7 @@ int part_get_info(struct blk_desc *dev_desc, int part,
 	}
 #endif /* CONFIG_HAVE_BLOCK_DEVICE */
 
-	return -1;
+	return -ENOENT;
 }
 
 int part_get_info_whole_disk(struct blk_desc *dev_desc,
@@ -416,7 +416,7 @@ int blk_get_device_by_str(const char *ifname, const char *dev_hwpart_str,
 	*dev_desc = get_dev_hwpart(ifname, dev, hwpart);
 	if (!(*dev_desc) || ((*dev_desc)->type == DEV_TYPE_UNKNOWN)) {
 		debug("** Bad device %s %s **\n", ifname, dev_hwpart_str);
-		dev = -ENOENT;
+		dev = -ENODEV;
 		goto cleanup;
 	}
 
@@ -440,7 +440,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
 			     struct blk_desc **dev_desc,
 			     struct disk_partition *info, int allow_whole_dev)
 {
-	int ret = -1;
+	int ret;
 	const char *part_str;
 	char *dup_str = NULL;
 	const char *dev_str;
@@ -482,7 +482,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
 	if (0 == strcmp(ifname, "ubi")) {
 		if (!ubifs_is_mounted()) {
 			printf("UBIFS not mounted, use ubifsmount to mount volume first!\n");
-			return -1;
+			return -EINVAL;
 		}
 
 		*dev_desc = NULL;
@@ -504,6 +504,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
 	/* If still no dev_part_str, it's an error */
 	if (!dev_part_str) {
 		printf("** No device specified **\n");
+		ret = -ENODEV;
 		goto cleanup;
 	}
 
@@ -520,8 +521,10 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
 
 	/* Look up the device */
 	dev = blk_get_device_by_str(ifname, dev_str, dev_desc);
-	if (dev < 0)
+	if (dev < 0) {
+		ret = dev;
 		goto cleanup;
+	}
 
 	/* Convert partition ID string to number */
 	if (!part_str || !*part_str) {
@@ -538,6 +541,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
 		if (*ep || (part == 0 && !allow_whole_dev)) {
 			printf("** Bad partition specification %s %s **\n",
 			    ifname, dev_part_str);
+			ret = -ENOENT;
 			goto cleanup;
 		}
 	}
@@ -551,6 +555,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
 		if (!(*dev_desc)->lba) {
 			printf("** Bad device size - %s %s **\n", ifname,
 			       dev_str);
+			ret = -EINVAL;
 			goto cleanup;
 		}
 
@@ -562,6 +567,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
 		if ((part > 0) || (!allow_whole_dev)) {
 			printf("** No partition table - %s %s **\n", ifname,
 			       dev_str);
+			ret = -EPROTONOSUPPORT;
 			goto cleanup;
 		}
 
@@ -630,7 +636,6 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
 				*info = tmpinfo;
 		} else {
 			printf("** No valid partitions found **\n");
-			ret = -1;
 			goto cleanup;
 		}
 	}
@@ -638,7 +643,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
 		printf("** Invalid partition type \"%.32s\""
 			" (expect \"" BOOT_PART_TYPE "\")\n",
 			info->type);
-		ret  = -1;
+		ret  = -EINVAL;
 		goto cleanup;
 	}
 
@@ -674,7 +679,7 @@ int part_get_info_by_name_type(struct blk_desc *dev_desc, const char *name,
 		}
 	}
 
-	return -1;
+	return -ENOENT;
 }
 
 int part_get_info_by_name(struct blk_desc *dev_desc, const char *name,
@@ -704,7 +709,7 @@ static int part_get_info_by_dev_and_name(const char *dev_iface,
 {
 	char *ep;
 	const char *part_str;
-	int dev_num;
+	int dev_num, ret;
 
 	part_str = strchr(dev_part_str, '#');
 	if (!part_str || part_str == dev_part_str)
@@ -720,13 +725,12 @@ static int part_get_info_by_dev_and_name(const char *dev_iface,
 	*dev_desc = blk_get_dev(dev_iface, dev_num);
 	if (!*dev_desc) {
 		printf("Could not find %s %d\n", dev_iface, dev_num);
-		return -EINVAL;
+		return -ENODEV;
 	}
-	if (part_get_info_by_name(*dev_desc, part_str, part_info) < 0) {
+	ret = part_get_info_by_name(*dev_desc, part_str, part_info);
+	if (ret < 0)
 		printf("Could not find \"%s\" partition\n", part_str);
-		return -EINVAL;
-	}
-	return 0;
+	return ret;
 }
 
 int part_get_info_by_dev_and_name_or_num(const char *dev_iface,
@@ -734,21 +738,23 @@ int part_get_info_by_dev_and_name_or_num(const char *dev_iface,
 					 struct blk_desc **dev_desc,
 					 struct disk_partition *part_info)
 {
+	int ret;
+
 	/* Split the part_name if passed as "$dev_num#part_name". */
-	if (!part_get_info_by_dev_and_name(dev_iface, dev_part_str,
-					   dev_desc, part_info))
-		return 0;
+	ret = part_get_info_by_dev_and_name(dev_iface, dev_part_str,
+					    dev_desc, part_info);
+	if (ret >= 0)
+		return ret;
 	/*
 	 * Couldn't lookup by name, try looking up the partition description
 	 * directly.
 	 */
-	if (blk_get_device_part_str(dev_iface, dev_part_str,
-				    dev_desc, part_info, 1) < 0) {
+	ret = blk_get_device_part_str(dev_iface, dev_part_str,
+				      dev_desc, part_info, 1);
+	if (ret < 0)
 		printf("Couldn't find partition %s %s\n",
 		       dev_iface, dev_part_str);
-		return -EINVAL;
-	}
-	return 0;
+	return ret;
 }
 
 void part_set_generic_name(const struct blk_desc *dev_desc,
-- 
2.25.1

[PATCH 4/9] part: Support getting whole disk from part_get_info_by_dev_and_name_or_num

[Sean Anderson]

This adds an option to part_get_info_by_dev_and_name_or_num to allow
callers to specify whether whole-disk partitions are fine.

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

 cmd/ab_select.c | 3 ++-
 disk/part.c     | 5 +++--
 include/part.h  | 6 +++++-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/cmd/ab_select.c b/cmd/ab_select.c
index 6298fcfb60..3e46663d6e 100644
--- a/cmd/ab_select.c
+++ b/cmd/ab_select.c
@@ -22,7 +22,8 @@ static int do_ab_select(struct cmd_tbl *cmdtp, int flag, int argc,
 
 	/* Lookup the "misc" partition from argv[2] and argv[3] */
 	if (part_get_info_by_dev_and_name_or_num(argv[2], argv[3],
-						 &dev_desc, &part_info) < 0) {
+						 &dev_desc, &part_info,
+						 false) < 0) {
 		return CMD_RET_FAILURE;
 	}
 
diff --git a/disk/part.c b/disk/part.c
index 5e354e256f..39c6b00a59 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -736,7 +736,8 @@ static int part_get_info_by_dev_and_name(const char *dev_iface,
 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)
+					 struct disk_partition *part_info,
+					 int allow_whole_dev)
 {
 	int ret;
 
@@ -750,7 +751,7 @@ int part_get_info_by_dev_and_name_or_num(const char *dev_iface,
 	 * directly.
 	 */
 	ret = blk_get_device_part_str(dev_iface, dev_part_str,
-				      dev_desc, part_info, 1);
+				      dev_desc, part_info, allow_whole_dev);
 	if (ret < 0)
 		printf("Couldn't find partition %s %s\n",
 		       dev_iface, dev_part_str);
diff --git a/include/part.h b/include/part.h
index 55be724d20..778cb36199 100644
--- a/include/part.h
+++ b/include/part.h
@@ -226,12 +226,16 @@ int part_get_info_by_name(struct blk_desc *dev_desc,
  * @param[in] dev_part_str Input partition description, like "0#misc" or "0:1"
  * @param[out] dev_desc	Place to store the device description pointer
  * @param[out] part_info Place to store the partition information
+ * @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
  */
 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);
+					 struct disk_partition *part_info,
+					 int allow_whole_dev);
 
 /**
  * part_set_generic_name() - create generic partition like hda1 or sdb2
-- 
2.25.1

[PATCH 5/9] part: Support string block devices in part_get_info_by_dev_and_name

[Sean Anderson]

This adds support for things like "#partname" and "0.1#partname". The block
device parsing is done like in blk_get_device_part_str.

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

 disk/part.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/disk/part.c b/disk/part.c
index 39c6b00a59..e07dd67fd9 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -707,29 +707,31 @@ static int part_get_info_by_dev_and_name(const char *dev_iface,
 					 struct blk_desc **dev_desc,
 					 struct disk_partition *part_info)
 {
-	char *ep;
-	const char *part_str;
-	int dev_num, ret;
+	char *dup_str = NULL;
+	const char *dev_str, *part_str;
+	int ret;
 
+	/* Separate device and partition name specification */
 	part_str = strchr(dev_part_str, '#');
-	if (!part_str || part_str == dev_part_str)
-		return -EINVAL;
-
-	dev_num = simple_strtoul(dev_part_str, &ep, 16);
-	if (ep != part_str) {
-		/* Not all the first part before the # was parsed. */
+	if (part_str) {
+		dup_str = strdup(dev_part_str);
+		dup_str[part_str - dev_part_str] = 0;
+		dev_str = dup_str;
+		part_str++;
+	} else {
 		return -EINVAL;
 	}
-	part_str++;
 
-	*dev_desc = blk_get_dev(dev_iface, dev_num);
-	if (!*dev_desc) {
-		printf("Could not find %s %d\n", dev_iface, dev_num);
-		return -ENODEV;
-	}
+	ret = blk_get_device_by_str(dev_iface, dev_str, dev_desc);
+	if (ret)
+		goto cleanup;
+
 	ret = part_get_info_by_name(*dev_desc, part_str, part_info);
 	if (ret < 0)
 		printf("Could not find \"%s\" partition\n", part_str);
+
+cleanup:
+	free(dup_str);
 	return ret;
 }
 
-- 
2.25.1

[PATCH 6/9] fastboot: Remove mmcpart argument from raw_part_get_info_by_name

[Sean Anderson]

The only thing mmcpart was used for was to pass to blk_dselect_hwpart.
This calls blk_dselect_hwpart directly from raw_part_get_info_by_name. The
error handling is dropped, but it is reintroduced in the next commit
(albeit less specificly).

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

 drivers/fastboot/fb_mmc.c | 41 +++++++++++++++++----------------------
 1 file changed, 18 insertions(+), 23 deletions(-)

diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
index ae8e8e512f..f6cacd711b 100644
--- a/drivers/fastboot/fb_mmc.c
+++ b/drivers/fastboot/fb_mmc.c
@@ -51,7 +51,8 @@ static int part_get_info_by_name_or_alias(struct blk_desc *dev_desc,
 }
 
 static int raw_part_get_info_by_name(struct blk_desc *dev_desc,
-		const char *name, struct disk_partition *info, int *mmcpart)
+				     const char *name,
+				     struct disk_partition *info)
 {
 	/* strlen("fastboot_raw_partition_") + PART_NAME_LEN + 1 */
 	char env_desc_name[23 + PART_NAME_LEN + 1];
@@ -85,8 +86,13 @@ static int raw_part_get_info_by_name(struct blk_desc *dev_desc,
 	strncpy((char *)info->name, name, PART_NAME_LEN);
 
 	if (raw_part_desc) {
-		if (strcmp(strsep(&raw_part_desc, " "), "mmcpart") == 0)
-			*mmcpart = simple_strtoul(raw_part_desc, NULL, 0);
+		if (strcmp(strsep(&raw_part_desc, " "), "mmcpart") == 0) {
+			ulong mmcpart = simple_strtoul(raw_part_desc, NULL, 0);
+			int ret = blk_dselect_hwpart(dev_desc, mmcpart);
+
+			if (ret)
+				return ret;
+		}
 	}
 
 	return 0;
@@ -419,7 +425,6 @@ int fastboot_mmc_get_part_info(const char *part_name,
 			       struct disk_partition *part_info, char *response)
 {
 	int r = 0;
-	int mmcpart;
 
 	*dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
 	if (!*dev_desc) {
@@ -431,7 +436,7 @@ int fastboot_mmc_get_part_info(const char *part_name,
 		return -ENOENT;
 	}
 
-	if (raw_part_get_info_by_name(*dev_desc, part_name, part_info, &mmcpart) < 0) {
+	if (raw_part_get_info_by_name(*dev_desc, part_name, part_info) < 0) {
 		r = part_get_info_by_name_or_alias(*dev_desc, part_name, part_info);
 		if (r < 0) {
 			fastboot_fail("partition not found", response);
@@ -455,7 +460,6 @@ void fastboot_mmc_flash_write(const char *cmd, void *download_buffer,
 {
 	struct blk_desc *dev_desc;
 	struct disk_partition info;
-	int mmcpart = 0;
 
 	dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
 	if (!dev_desc || dev_desc->type == DEV_TYPE_UNKNOWN) {
@@ -528,16 +532,12 @@ void fastboot_mmc_flash_write(const char *cmd, void *download_buffer,
 	}
 #endif
 
-	if (raw_part_get_info_by_name(dev_desc, cmd, &info, &mmcpart) == 0) {
-		if (blk_dselect_hwpart(dev_desc, mmcpart)) {
-			pr_err("Failed to select hwpart\n");
-			fastboot_fail("Failed to select hwpart", response);
+	if (raw_part_get_info_by_name(dev_desc, cmd, &info) != 0) {
+		if (part_get_info_by_name_or_alias(dev_desc, cmd, &info) < 0) {
+			pr_err("cannot find partition: '%s'\n", cmd);
+			fastboot_fail("cannot find partition", response);
 			return;
 		}
-	} else if (part_get_info_by_name_or_alias(dev_desc, cmd, &info) < 0) {
-		pr_err("cannot find partition: '%s'\n", cmd);
-		fastboot_fail("cannot find partition", response);
-		return;
 	}
 
 	if (is_sparse_image(download_buffer)) {
@@ -580,7 +580,6 @@ void fastboot_mmc_erase(const char *cmd, char *response)
 	struct disk_partition info;
 	lbaint_t blks, blks_start, blks_size, grp_size;
 	struct mmc *mmc = find_mmc_device(CONFIG_FASTBOOT_FLASH_MMC_DEV);
-	int mmcpart = 0;
 
 	if (mmc == NULL) {
 		pr_err("invalid mmc device\n");
@@ -614,16 +613,12 @@ void fastboot_mmc_erase(const char *cmd, char *response)
 	}
 #endif
 
-	if (raw_part_get_info_by_name(dev_desc, cmd, &info, &mmcpart) == 0) {
-		if (blk_dselect_hwpart(dev_desc, mmcpart)) {
-			pr_err("Failed to select hwpart\n");
-			fastboot_fail("Failed to select hwpart", response);
+	if (raw_part_get_info_by_name(dev_desc, cmd, &info) != 0) {
+		if (part_get_info_by_name_or_alias(dev_desc, cmd, &info) < 0) {
+			pr_err("cannot find partition: '%s'\n", cmd);
+			fastboot_fail("cannot find partition", response);
 			return;
 		}
-	} else if (part_get_info_by_name_or_alias(dev_desc, cmd, &info) < 0) {
-		pr_err("cannot find partition: '%s'\n", cmd);
-		fastboot_fail("cannot find partition", response);
-		return;
 	}
 
 	/* Align blocks to erase group size to avoid erasing other partitions */
-- 
2.25.1

[PATCH 7/9] fastboot: Move part_get_info_by_name_or_alias after raw_part_get_info_by_name

[Sean Anderson]

This makes the next commit more readable by doing the move now.

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

 drivers/fastboot/fb_mmc.c | 44 +++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
index f6cacd711b..b0610d3151 100644
--- a/drivers/fastboot/fb_mmc.c
+++ b/drivers/fastboot/fb_mmc.c
@@ -28,28 +28,6 @@ struct fb_mmc_sparse {
 	struct blk_desc	*dev_desc;
 };
 
-static int part_get_info_by_name_or_alias(struct blk_desc *dev_desc,
-		const char *name, struct disk_partition *info)
-{
-	int ret;
-
-	ret = part_get_info_by_name(dev_desc, name, info);
-	if (ret < 0) {
-		/* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */
-		char env_alias_name[25 + PART_NAME_LEN + 1];
-		char *aliased_part_name;
-
-		/* check for alias */
-		strcpy(env_alias_name, "fastboot_partition_alias_");
-		strncat(env_alias_name, name, PART_NAME_LEN);
-		aliased_part_name = env_get(env_alias_name);
-		if (aliased_part_name != NULL)
-			ret = part_get_info_by_name(dev_desc,
-					aliased_part_name, info);
-	}
-	return ret;
-}
-
 static int raw_part_get_info_by_name(struct blk_desc *dev_desc,
 				     const char *name,
 				     struct disk_partition *info)
@@ -98,6 +76,28 @@ static int raw_part_get_info_by_name(struct blk_desc *dev_desc,
 	return 0;
 }
 
+static int part_get_info_by_name_or_alias(struct blk_desc *dev_desc,
+		const char *name, struct disk_partition *info)
+{
+	int ret;
+
+	ret = part_get_info_by_name(dev_desc, name, info);
+	if (ret < 0) {
+		/* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */
+		char env_alias_name[25 + PART_NAME_LEN + 1];
+		char *aliased_part_name;
+
+		/* check for alias */
+		strcpy(env_alias_name, "fastboot_partition_alias_");
+		strncat(env_alias_name, name, PART_NAME_LEN);
+		aliased_part_name = env_get(env_alias_name);
+		if (aliased_part_name != NULL)
+			ret = part_get_info_by_name(dev_desc,
+					aliased_part_name, info);
+	}
+	return ret;
+}
+
 /**
  * fb_mmc_blk_write() - Write/erase MMC in chunks of FASTBOOT_MAX_BLK_WRITE
  *
-- 
2.25.1

[PATCH 8/9] fastboot: Allow u-boot-style partitions

[Sean Anderson]

This adds support for partitions of the form "dev.hwpart:part" and
"dev#partname". This allows one to flash to eMMC boot partitions without
having to use CONFIG_FASTBOOT_MMC_BOOT1_SUPPORT. It also allows one to
flash to an entire device without needing CONFIG_FASTBOOT_MMC_USER_NAME.
Lastly, one can also flash MMC devices other than
CONFIG_FASTBOOT_FLASH_MMC_DEV.

Because devices can be specified explicitly, CONFIG_FASTBOOT_FLASH_MMC_DEV
is used only when necessary for existing functionality. For those cases,
fastboot_mmc_get_dev has been added as a helper function. This allows

There should be no conflicts with the existing system, but just in case, I
have ordered detection of these names after all existing names.

The fastboot_mmc_part test has been updated for these new names.

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

 drivers/fastboot/fb_mmc.c | 150 +++++++++++++++++++++++---------------
 test/dm/fastboot.c        |  37 +++++++++-
 2 files changed, 127 insertions(+), 60 deletions(-)

diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
index b0610d3151..a52b1e3ed6 100644
--- a/drivers/fastboot/fb_mmc.c
+++ b/drivers/fastboot/fb_mmc.c
@@ -37,6 +37,7 @@ static int raw_part_get_info_by_name(struct blk_desc *dev_desc,
 	char *raw_part_desc;
 	const char *argv[2];
 	const char **parg = argv;
+	int ret;
 
 	/* check for raw partition descriptor */
 	strcpy(env_desc_name, "fastboot_raw_partition_");
@@ -60,7 +61,7 @@ static int raw_part_get_info_by_name(struct blk_desc *dev_desc,
 
 	info->start = simple_strtoul(argv[0], NULL, 0);
 	info->size = simple_strtoul(argv[1], NULL, 0);
-	info->blksz = dev_desc->blksz;
+	info->blksz = *dev_desc->blksz;
 	strncpy((char *)info->name, name, PART_NAME_LEN);
 
 	if (raw_part_desc) {
@@ -76,12 +77,37 @@ static int raw_part_get_info_by_name(struct blk_desc *dev_desc,
 	return 0;
 }
 
-static int part_get_info_by_name_or_alias(struct blk_desc *dev_desc,
-		const char *name, struct disk_partition *info)
+static int do_get_part_info(struct blk_desc **dev_desc, const char *name,
+			    struct disk_partition *info)
+{
+	int ret;
+
+	/* First try partition names on the default device */
+	*dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
+	if (*dev_desc) {
+		ret = part_get_info_by_name(*dev_desc, name, info);
+		if (ret >= 0)
+			return ret;
+
+		/* Then try raw partitions */
+		ret = raw_part_get_info_by_name(*dev_desc, name, info);
+		if (ret >= 0)
+			return ret;
+	}
+
+	/* Then try dev.hwpart:part */
+	ret = part_get_info_by_dev_and_name_or_num("mmc", name, dev_desc,
+						   info, true);
+	return ret;
+}
+
+static int part_get_info_by_name_or_alias(struct blk_desc **dev_desc,
+					  const char *name,
+					  struct disk_partition *info)
 {
 	int ret;
 
-	ret = part_get_info_by_name(dev_desc, name, info);
+	ret = do_get_part_info(dev_desc, name, info);
 	if (ret < 0) {
 		/* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */
 		char env_alias_name[25 + PART_NAME_LEN + 1];
@@ -92,8 +118,8 @@ static int part_get_info_by_name_or_alias(struct blk_desc *dev_desc,
 		strncat(env_alias_name, name, PART_NAME_LEN);
 		aliased_part_name = env_get(env_alias_name);
 		if (aliased_part_name != NULL)
-			ret = part_get_info_by_name(dev_desc,
-					aliased_part_name, info);
+			ret = do_get_part_info(dev_desc, aliased_part_name,
+					       info);
 	}
 	return ret;
 }
@@ -424,27 +450,49 @@ int fastboot_mmc_get_part_info(const char *part_name,
 			       struct blk_desc **dev_desc,
 			       struct disk_partition *part_info, char *response)
 {
-	int r = 0;
+	int ret;
 
-	*dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
-	if (!*dev_desc) {
-		fastboot_fail("block device not found", response);
-		return -ENOENT;
-	}
 	if (!part_name || !strcmp(part_name, "")) {
 		fastboot_fail("partition not given", response);
 		return -ENOENT;
 	}
 
-	if (raw_part_get_info_by_name(*dev_desc, part_name, part_info) < 0) {
-		r = part_get_info_by_name_or_alias(*dev_desc, part_name, part_info);
-		if (r < 0) {
-			fastboot_fail("partition not found", response);
-			return r;
+	ret = part_get_info_by_name_or_alias(dev_desc, part_name, part_info);
+	if (ret < 0) {
+		switch (ret) {
+		case -ENOSYS:
+		case -EINVAL:
+			fastboot_fail("invalid partition or device", response);
+			break;
+		case -ENODEV:
+			fastboot_fail("no such device", response);
+			break;
+		case -ENOENT:
+			fastboot_fail("no such partition", response);
+			break;
+		case -EPROTONOSUPPORT:
+			fastboot_fail("unknown partition table type", response);
+			break;
+		default:
+			fastboot_fail("unanticipated error", response);
+			break;
 		}
 	}
 
-	return r;
+	return ret;
+}
+
+static struct blk_desc *fastboot_mmc_get_dev(char *response)
+{
+	struct blk_desc *ret = blk_get_dev("mmc",
+					   CONFIG_FASTBOOT_FLASH_MMC_DEV);
+
+	if (!ret || ret->type == DEV_TYPE_UNKNOWN) {
+		pr_err("invalid mmc device\n");
+		fastboot_fail("invalid mmc device", response);
+		return NULL;
+	}
+	return ret;
 }
 
 /**
@@ -461,17 +509,12 @@ void fastboot_mmc_flash_write(const char *cmd, void *download_buffer,
 	struct blk_desc *dev_desc;
 	struct disk_partition info;
 
-	dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
-	if (!dev_desc || dev_desc->type == DEV_TYPE_UNKNOWN) {
-		pr_err("invalid mmc device\n");
-		fastboot_fail("invalid mmc device", response);
-		return;
-	}
-
 #ifdef CONFIG_FASTBOOT_MMC_BOOT1_SUPPORT
 	if (strcmp(cmd, CONFIG_FASTBOOT_MMC_BOOT1_NAME) == 0) {
-		fb_mmc_boot1_ops(dev_desc, download_buffer,
-				 download_bytes, response);
+		dev_desc = fastboot_mmc_get_dev(response);
+		if (dev_desc)
+			fb_mmc_boot1_ops(dev_desc, download_buffer,
+					 download_bytes, response);
 		return;
 	}
 #endif
@@ -483,6 +526,10 @@ void fastboot_mmc_flash_write(const char *cmd, void *download_buffer,
 	if (strcmp(cmd, CONFIG_FASTBOOT_GPT_NAME) == 0 ||
 	    strcmp(cmd, CONFIG_FASTBOOT_MMC_USER_NAME) == 0) {
 #endif
+		dev_desc = fastboot_mmc_get_dev(response);
+		if (!dev_desc)
+			return;
+
 		printf("%s: updating MBR, Primary and Backup GPT(s)\n",
 		       __func__);
 		if (is_valid_gpt_buf(dev_desc, download_buffer)) {
@@ -505,6 +552,10 @@ void fastboot_mmc_flash_write(const char *cmd, void *download_buffer,
 
 #if CONFIG_IS_ENABLED(DOS_PARTITION)
 	if (strcmp(cmd, CONFIG_FASTBOOT_MBR_NAME) == 0) {
+		dev_desc = fastboot_mmc_get_dev(response);
+		if (!dev_desc)
+			return;
+
 		printf("%s: updating MBR\n", __func__);
 		if (is_valid_dos_buf(download_buffer)) {
 			printf("%s: invalid MBR - refusing to write to flash\n",
@@ -526,19 +577,16 @@ void fastboot_mmc_flash_write(const char *cmd, void *download_buffer,
 
 #ifdef CONFIG_ANDROID_BOOT_IMAGE
 	if (strncasecmp(cmd, "zimage", 6) == 0) {
-		fb_mmc_update_zimage(dev_desc, download_buffer,
-				     download_bytes, response);
+		dev_desc = fastboot_mmc_get_dev(response);
+		if (dev_desc)
+			fb_mmc_update_zimage(dev_desc, download_buffer,
+					     download_bytes, response);
 		return;
 	}
 #endif
 
-	if (raw_part_get_info_by_name(dev_desc, cmd, &info) != 0) {
-		if (part_get_info_by_name_or_alias(dev_desc, cmd, &info) < 0) {
-			pr_err("cannot find partition: '%s'\n", cmd);
-			fastboot_fail("cannot find partition", response);
-			return;
-		}
-	}
+	if (fastboot_mmc_get_part_info(cmd, &dev_desc, &info, response) < 0)
+		return;
 
 	if (is_sparse_image(download_buffer)) {
 		struct fb_mmc_sparse sparse_priv;
@@ -581,23 +629,12 @@ void fastboot_mmc_erase(const char *cmd, char *response)
 	lbaint_t blks, blks_start, blks_size, grp_size;
 	struct mmc *mmc = find_mmc_device(CONFIG_FASTBOOT_FLASH_MMC_DEV);
 
-	if (mmc == NULL) {
-		pr_err("invalid mmc device\n");
-		fastboot_fail("invalid mmc device", response);
-		return;
-	}
-
-	dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
-	if (!dev_desc || dev_desc->type == DEV_TYPE_UNKNOWN) {
-		pr_err("invalid mmc device\n");
-		fastboot_fail("invalid mmc device", response);
-		return;
-	}
-
 #ifdef CONFIG_FASTBOOT_MMC_BOOT1_SUPPORT
 	if (strcmp(cmd, CONFIG_FASTBOOT_MMC_BOOT1_NAME) == 0) {
 		/* erase EMMC boot1 */
-		fb_mmc_boot1_ops(dev_desc, NULL, 0, response);
+		dev_desc = fastboot_mmc_get_dev(response);
+		if (dev_desc)
+			fb_mmc_boot1_ops(dev_desc, NULL, 0, response);
 		return;
 	}
 #endif
@@ -605,6 +642,10 @@ void fastboot_mmc_erase(const char *cmd, char *response)
 #ifdef CONFIG_FASTBOOT_MMC_USER_NAME
 	if (strcmp(cmd, CONFIG_FASTBOOT_MMC_USER_NAME) == 0) {
 		/* erase EMMC userdata */
+		dev_desc = fastboot_mmc_get_dev(response);
+		if (!dev_desc)
+			return;
+
 		if (fb_mmc_erase_mmc_hwpart(dev_desc))
 			fastboot_fail("Failed to erase EMMC_USER", response);
 		else
@@ -613,13 +654,8 @@ void fastboot_mmc_erase(const char *cmd, char *response)
 	}
 #endif
 
-	if (raw_part_get_info_by_name(dev_desc, cmd, &info) != 0) {
-		if (part_get_info_by_name_or_alias(dev_desc, cmd, &info) < 0) {
-			pr_err("cannot find partition: '%s'\n", cmd);
-			fastboot_fail("cannot find partition", response);
-			return;
-		}
-	}
+	if (fastboot_mmc_get_part_info(cmd, &dev_desc, &info, response) < 0)
+		return;
 
 	/* Align blocks to erase group size to avoid erasing other partitions */
 	grp_size = mmc->erase_grp_size;
diff --git a/test/dm/fastboot.c b/test/dm/fastboot.c
index 8f905d8fa8..e7f8c362b8 100644
--- a/test/dm/fastboot.c
+++ b/test/dm/fastboot.c
@@ -35,9 +35,12 @@ static int dm_test_fastboot_mmc_part(struct unit_test_state *uts)
 		},
 	};
 
-	ut_assertok(blk_get_device_by_str("mmc",
-					  __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV),
-					  &mmc_dev_desc));
+	/*
+	 * There are a lot of literal 0s I don't want to have to construct from
+	 * MMC_DEV.
+	 */
+	ut_asserteq(0, CONFIG_FASTBOOT_FLASH_MMC_DEV);
+	ut_assertok(blk_get_device_by_str("mmc", "0", &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);
@@ -59,6 +62,34 @@ static int dm_test_fastboot_mmc_part(struct unit_test_state *uts)
 						  &part_info, response));
 	ut_assertok(env_set(FB_ALIAS_PREFIX "test3", NULL));
 
+	/* "New" partition labels */
+	ut_asserteq(1, fastboot_mmc_get_part_info("#test1", &fb_dev_desc,
+						  &part_info, response));
+	ut_asserteq(1, fastboot_mmc_get_part_info("0#test1", &fb_dev_desc,
+						  &part_info, response));
+	ut_asserteq(1, fastboot_mmc_get_part_info("0.0#test1", &fb_dev_desc,
+						  &part_info, response));
+	ut_asserteq(1, fastboot_mmc_get_part_info("0:1", &fb_dev_desc,
+						  &part_info, response));
+	ut_asserteq(1, fastboot_mmc_get_part_info("0.0:1", &fb_dev_desc,
+						  &part_info, response));
+	ut_asserteq(1, fastboot_mmc_get_part_info("0", &fb_dev_desc,
+						  &part_info, response));
+	ut_asserteq(1, fastboot_mmc_get_part_info("0.0", &fb_dev_desc,
+						  &part_info, response));
+	ut_asserteq(0, fastboot_mmc_get_part_info("0:0", &fb_dev_desc,
+						  &part_info, response));
+	ut_asserteq(0, fastboot_mmc_get_part_info("0.0:0", &fb_dev_desc,
+						  &part_info, response));
+	ut_asserteq(0, fastboot_mmc_get_part_info("1", &fb_dev_desc,
+						  &part_info, response));
+	ut_asserteq(0, fastboot_mmc_get_part_info("1.0", &fb_dev_desc,
+						  &part_info, response));
+	ut_asserteq(1, fastboot_mmc_get_part_info(":1", &fb_dev_desc,
+						  &part_info, response));
+	ut_asserteq(0, fastboot_mmc_get_part_info(":0", &fb_dev_desc,
+						  &part_info, response));
+
 	return 0;
 }
 DM_TEST(dm_test_fastboot_mmc_part, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
-- 
2.25.1

[PATCH 9/9] fastboot: Document alternate partition names

[Sean Anderson]

This documents the new partition names added in the previous commit.

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

 doc/android/fastboot.rst | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst
index 2877c3cbaa..2ca80ae844 100644
--- a/doc/android/fastboot.rst
+++ b/doc/android/fastboot.rst
@@ -151,6 +151,34 @@ The device index starts from ``a`` and refers to the interface (e.g. USB
 controller, SD/MMC controller) or disk index. The partition index starts
 from ``1`` and describes the partition number on the particular device.
 
+
+Alternate Partition Names
+^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Partitions may also be specified like::
+
+    devnum.hwpartnum#partname
+
+or like::
+
+    devnum.hwpartnum:partnum
+
+Where
+
+  * ``devnum`` is the MMC device number. This defaults to 0.
+  * ``hwpartnum`` is the hardware partition number. This defaults to 0 (the user
+    partition on eMMC devices).
+  * ``partname`` is the partition name on GPT devices. Partitions do not have
+    names on MBR devices.
+  * ``partnum`` is the partition number, starting from 1. The partition number 0
+    is special, and specifies that the whole device is to be used as one
+    "partition."
+
+If neither ``partname`` nor ``partnum`` is specified and there is a partition
+table, then partition 1 is used. If there is no partition table, then the whole
+device is used as one "partion." Examples of alternate partition names include
+``0.1``, ``0#boot``, and ``:3``.
+
 Writing Partition Table
 -----------------------
 
-- 
2.25.1

[PATCH 9/9] fastboot: Document alternate partition names

[Heinrich Schuchardt]

On 12/31/20 11:48 PM, Sean Anderson wrote:
> This documents the new partition names added in the previous commit.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
>
>   doc/android/fastboot.rst | 28 ++++++++++++++++++++++++++++
>   1 file changed, 28 insertions(+)
>
> diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst
> index 2877c3cbaa..2ca80ae844 100644
> --- a/doc/android/fastboot.rst
> +++ b/doc/android/fastboot.rst
> @@ -151,6 +151,34 @@ The device index starts from ``a`` and refers to the interface (e.g. USB
>   controller, SD/MMC controller) or disk index. The partition index starts
>   from ``1`` and describes the partition number on the particular device.
>
> +
> +Alternate Partition Names
> +^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Partitions may also be specified like::
> +
> +    devnum.hwpartnum#partname

Thank you for getting this all documented.

Don't we need the interface (mmc), too?

> +
> +or like::
> +
> +    devnum.hwpartnum:partnum

I think this is the wrong place to document how partitions are addressed
as it is not Android specific. I would prefer a sub-chapter of doc/usage/.

Cf.
https://u-boot.readthedocs.io/en/latest/usage/index.html

Best regards

Heinrich

> +
> +Where
> +
> +  * ``devnum`` is the MMC device number. This defaults to 0.
> +  * ``hwpartnum`` is the hardware partition number. This defaults to 0 (the user
> +    partition on eMMC devices).
> +  * ``partname`` is the partition name on GPT devices. Partitions do not have
> +    names on MBR devices.
> +  * ``partnum`` is the partition number, starting from 1. The partition number 0
> +    is special, and specifies that the whole device is to be used as one
> +    "partition."
> +
> +If neither ``partname`` nor ``partnum`` is specified and there is a partition
> +table, then partition 1 is used. If there is no partition table, then the whole
> +device is used as one "partion." Examples of alternate partition names include
> +``0.1``, ``0#boot``, and ``:3``.
> +
>   Writing Partition Table
>   -----------------------
>
>

[PATCH 9/9] fastboot: Document alternate partition names

[Sean Anderson]

On 12/31/20 7:36 PM, Heinrich Schuchardt wrote:
> On 12/31/20 11:48 PM, Sean Anderson wrote:
>> This documents the new partition names added in the previous commit.
>>
>> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
>> ---
>>
>>   doc/android/fastboot.rst | 28 ++++++++++++++++++++++++++++
>>   1 file changed, 28 insertions(+)
>>
>> diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst
>> index 2877c3cbaa..2ca80ae844 100644
>> --- a/doc/android/fastboot.rst
>> +++ b/doc/android/fastboot.rst
>> @@ -151,6 +151,34 @@ The device index starts from ``a`` and refers to the interface (e.g. USB
>>   controller, SD/MMC controller) or disk index. The partition index starts
>>   from ``1`` and describes the partition number on the particular device.
>>
>> +
>> +Alternate Partition Names
>> +^^^^^^^^^^^^^^^^^^^^^^^^^
>> +
>> +Partitions may also be specified like::
>> +
>> +    devnum.hwpartnum#partname
> 
> Thank you for getting this all documented.
> 
> Don't we need the interface (mmc), too?

No. At the moment only mmc is supported. I suppose I could build in some
forward-compatibility and have something like

     iface,devnum.hwpartnum#partname

It should be relatively easy to extend the MMC code to be block-generic,
but it's out of scope for this series.

> 
>> +
>> +or like::
>> +
>> +    devnum.hwpartnum:partnum
> 
> I think this is the wrong place to document how partitions are addressed
> as it is not Android specific. I would prefer a sub-chapter of doc/usage/.

Sure. Though atm only fastboot and android a/b uses the #partname syntax
as far as I can tell.

--Sean

> 
> Cf.
> https://u-boot.readthedocs.io/en/latest/usage/index.html
> 
> Best regards
> 
> Heinrich
> 
>> +
>> +Where
>> +
>> +  * ``devnum`` is the MMC device number. This defaults to 0.
>> +  * ``hwpartnum`` is the hardware partition number. This defaults to 0 (the user
>> +    partition on eMMC devices).
>> +  * ``partname`` is the partition name on GPT devices. Partitions do not have
>> +    names on MBR devices.
>> +  * ``partnum`` is the partition number, starting from 1. The partition number 0
>> +    is special, and specifies that the whole device is to be used as one
>> +    "partition."
>> +
>> +If neither ``partname`` nor ``partnum`` is specified and there is a partition
>> +table, then partition 1 is used. If there is no partition table, then the whole
>> +device is used as one "partion." Examples of alternate partition names include
>> +``0.1``, ``0#boot``, and ``:3``.
>> +
>>   Writing Partition Table
>>   -----------------------
>>
>>
> 

[PATCH 8/9] fastboot: Allow u-boot-style partitions

[Sean Anderson]

On 12/31/20 5:48 PM, Sean Anderson wrote:
 > This adds support for partitions of the form "dev.hwpart:part" and
 > "dev#partname". This allows one to flash to eMMC boot partitions without
 > having to use CONFIG_FASTBOOT_MMC_BOOT1_SUPPORT. It also allows one to
 > flash to an entire device without needing CONFIG_FASTBOOT_MMC_USER_NAME.
 > Lastly, one can also flash MMC devices other than
 > CONFIG_FASTBOOT_FLASH_MMC_DEV.
 >
 > Because devices can be specified explicitly, 
CONFIG_FASTBOOT_FLASH_MMC_DEV
 > is used only when necessary for existing functionality. For those cases,
 > fastboot_mmc_get_dev has been added as a helper function. This allows
 >
 > There should be no conflicts with the existing system, but just in 
case, I
 > have ordered detection of these names after all existing names.
 >
 > The fastboot_mmc_part test has been updated for these new names.
 >
 > Signed-off-by: Sean Anderson <sean.anderson@seco.com>
 > ---
 >
 >   drivers/fastboot/fb_mmc.c | 150 +++++++++++++++++++++++---------------
 >   test/dm/fastboot.c        |  37 +++++++++-
 >   2 files changed, 127 insertions(+), 60 deletions(-)
 >
 > diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
 > index b0610d3151..a52b1e3ed6 100644
 > --- a/drivers/fastboot/fb_mmc.c
 > +++ b/drivers/fastboot/fb_mmc.c
 > @@ -37,6 +37,7 @@ static int raw_part_get_info_by_name(struct 
blk_desc *dev_desc,
 >   	char *raw_part_desc;
 >   	const char *argv[2];
 >   	const char **parg = argv;
 > +	int ret;
 >
 >   	/* check for raw partition descriptor */
 >   	strcpy(env_desc_name, "fastboot_raw_partition_");
 > @@ -60,7 +61,7 @@ static int raw_part_get_info_by_name(struct 
blk_desc *dev_desc,
 >
 >   	info->start = simple_strtoul(argv[0], NULL, 0);
 >   	info->size = simple_strtoul(argv[1], NULL, 0);
 > -	info->blksz = dev_desc->blksz;
 > +	info->blksz = *dev_desc->blksz;
 >   	strncpy((char *)info->name, name, PART_NAME_LEN);

Looks like this slipped through while rebasing. The above two hunks
shouldn't have been included; will be fixed in v2.

--Sean

 >
 >   	if (raw_part_desc) {
 > @@ -76,12 +77,37 @@ static int raw_part_get_info_by_name(struct 
blk_desc *dev_desc,
 >   	return 0;
 >   }
 >
 > -static int part_get_info_by_name_or_alias(struct blk_desc *dev_desc,
 > -		const char *name, struct disk_partition *info)
 > +static int do_get_part_info(struct blk_desc **dev_desc, const char 
*name,
 > +			    struct disk_partition *info)
 > +{
 > +	int ret;
 > +
 > +	/* First try partition names on the default device */
 > +	*dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
 > +	if (*dev_desc) {
 > +		ret = part_get_info_by_name(*dev_desc, name, info);
 > +		if (ret >= 0)
 > +			return ret;
 > +
 > +		/* Then try raw partitions */
 > +		ret = raw_part_get_info_by_name(*dev_desc, name, info);
 > +		if (ret >= 0)
 > +			return ret;
 > +	}
 > +
 > +	/* Then try dev.hwpart:part */
 > +	ret = part_get_info_by_dev_and_name_or_num("mmc", name, dev_desc,
 > +						   info, true);
 > +	return ret;
 > +}
 > +
 > +static int part_get_info_by_name_or_alias(struct blk_desc **dev_desc,
 > +					  const char *name,
 > +					  struct disk_partition *info)
 >   {
 >   	int ret;
 >
 > -	ret = part_get_info_by_name(dev_desc, name, info);
 > +	ret = do_get_part_info(dev_desc, name, info);
 >   	if (ret < 0) {
 >   		/* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */
 >   		char env_alias_name[25 + PART_NAME_LEN + 1];
 > @@ -92,8 +118,8 @@ static int part_get_info_by_name_or_alias(struct 
blk_desc *dev_desc,
 >   		strncat(env_alias_name, name, PART_NAME_LEN);
 >   		aliased_part_name = env_get(env_alias_name);
 >   		if (aliased_part_name != NULL)
 > -			ret = part_get_info_by_name(dev_desc,
 > -					aliased_part_name, info);
 > +			ret = do_get_part_info(dev_desc, aliased_part_name,
 > +					       info);
 >   	}
 >   	return ret;
 >   }
 > @@ -424,27 +450,49 @@ int fastboot_mmc_get_part_info(const char 
*part_name,
 >   			       struct blk_desc **dev_desc,
 >   			       struct disk_partition *part_info, char *response)
 >   {
 > -	int r = 0;
 > +	int ret;
 >
 > -	*dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
 > -	if (!*dev_desc) {
 > -		fastboot_fail("block device not found", response);
 > -		return -ENOENT;
 > -	}
 >   	if (!part_name || !strcmp(part_name, "")) {
 >   		fastboot_fail("partition not given", response);
 >   		return -ENOENT;
 >   	}
 >
 > -	if (raw_part_get_info_by_name(*dev_desc, part_name, part_info) < 0) {
 > -		r = part_get_info_by_name_or_alias(*dev_desc, part_name, part_info);
 > -		if (r < 0) {
 > -			fastboot_fail("partition not found", response);
 > -			return r;
 > +	ret = part_get_info_by_name_or_alias(dev_desc, part_name, part_info);
 > +	if (ret < 0) {
 > +		switch (ret) {
 > +		case -ENOSYS:
 > +		case -EINVAL:
 > +			fastboot_fail("invalid partition or device", response);
 > +			break;
 > +		case -ENODEV:
 > +			fastboot_fail("no such device", response);
 > +			break;
 > +		case -ENOENT:
 > +			fastboot_fail("no such partition", response);
 > +			break;
 > +		case -EPROTONOSUPPORT:
 > +			fastboot_fail("unknown partition table type", response);
 > +			break;
 > +		default:
 > +			fastboot_fail("unanticipated error", response);
 > +			break;
 >   		}
 >   	}
 >
 > -	return r;
 > +	return ret;
 > +}
 > +
 > +static struct blk_desc *fastboot_mmc_get_dev(char *response)
 > +{
 > +	struct blk_desc *ret = blk_get_dev("mmc",
 > +					   CONFIG_FASTBOOT_FLASH_MMC_DEV);
 > +
 > +	if (!ret || ret->type == DEV_TYPE_UNKNOWN) {
 > +		pr_err("invalid mmc device\n");
 > +		fastboot_fail("invalid mmc device", response);
 > +		return NULL;
 > +	}
 > +	return ret;
 >   }
 >
 >   /**
 > @@ -461,17 +509,12 @@ void fastboot_mmc_flash_write(const char *cmd, 
void *download_buffer,
 >   	struct blk_desc *dev_desc;
 >   	struct disk_partition info;
 >
 > -	dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
 > -	if (!dev_desc || dev_desc->type == DEV_TYPE_UNKNOWN) {
 > -		pr_err("invalid mmc device\n");
 > -		fastboot_fail("invalid mmc device", response);
 > -		return;
 > -	}
 > -
 >   #ifdef CONFIG_FASTBOOT_MMC_BOOT1_SUPPORT
 >   	if (strcmp(cmd, CONFIG_FASTBOOT_MMC_BOOT1_NAME) == 0) {
 > -		fb_mmc_boot1_ops(dev_desc, download_buffer,
 > -				 download_bytes, response);
 > +		dev_desc = fastboot_mmc_get_dev(response);
 > +		if (dev_desc)
 > +			fb_mmc_boot1_ops(dev_desc, download_buffer,
 > +					 download_bytes, response);
 >   		return;
 >   	}
 >   #endif
 > @@ -483,6 +526,10 @@ void fastboot_mmc_flash_write(const char *cmd, 
void *download_buffer,
 >   	if (strcmp(cmd, CONFIG_FASTBOOT_GPT_NAME) == 0 ||
 >   	    strcmp(cmd, CONFIG_FASTBOOT_MMC_USER_NAME) == 0) {
 >   #endif
 > +		dev_desc = fastboot_mmc_get_dev(response);
 > +		if (!dev_desc)
 > +			return;
 > +
 >   		printf("%s: updating MBR, Primary and Backup GPT(s)\n",
 >   		       __func__);
 >   		if (is_valid_gpt_buf(dev_desc, download_buffer)) {
 > @@ -505,6 +552,10 @@ void fastboot_mmc_flash_write(const char *cmd, 
void *download_buffer,
 >
 >   #if CONFIG_IS_ENABLED(DOS_PARTITION)
 >   	if (strcmp(cmd, CONFIG_FASTBOOT_MBR_NAME) == 0) {
 > +		dev_desc = fastboot_mmc_get_dev(response);
 > +		if (!dev_desc)
 > +			return;
 > +
 >   		printf("%s: updating MBR\n", __func__);
 >   		if (is_valid_dos_buf(download_buffer)) {
 >   			printf("%s: invalid MBR - refusing to write to flash\n",
 > @@ -526,19 +577,16 @@ void fastboot_mmc_flash_write(const char *cmd, 
void *download_buffer,
 >
 >   #ifdef CONFIG_ANDROID_BOOT_IMAGE
 >   	if (strncasecmp(cmd, "zimage", 6) == 0) {
 > -		fb_mmc_update_zimage(dev_desc, download_buffer,
 > -				     download_bytes, response);
 > +		dev_desc = fastboot_mmc_get_dev(response);
 > +		if (dev_desc)
 > +			fb_mmc_update_zimage(dev_desc, download_buffer,
 > +					     download_bytes, response);
 >   		return;
 >   	}
 >   #endif
 >
 > -	if (raw_part_get_info_by_name(dev_desc, cmd, &info) != 0) {
 > -		if (part_get_info_by_name_or_alias(dev_desc, cmd, &info) < 0) {
 > -			pr_err("cannot find partition: '%s'\n", cmd);
 > -			fastboot_fail("cannot find partition", response);
 > -			return;
 > -		}
 > -	}
 > +	if (fastboot_mmc_get_part_info(cmd, &dev_desc, &info, response) < 0)
 > +		return;
 >
 >   	if (is_sparse_image(download_buffer)) {
 >   		struct fb_mmc_sparse sparse_priv;
 > @@ -581,23 +629,12 @@ void fastboot_mmc_erase(const char *cmd, char 
*response)
 >   	lbaint_t blks, blks_start, blks_size, grp_size;
 >   	struct mmc *mmc = find_mmc_device(CONFIG_FASTBOOT_FLASH_MMC_DEV);
 >
 > -	if (mmc == NULL) {
 > -		pr_err("invalid mmc device\n");
 > -		fastboot_fail("invalid mmc device", response);
 > -		return;
 > -	}
 > -
 > -	dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
 > -	if (!dev_desc || dev_desc->type == DEV_TYPE_UNKNOWN) {
 > -		pr_err("invalid mmc device\n");
 > -		fastboot_fail("invalid mmc device", response);
 > -		return;
 > -	}
 > -
 >   #ifdef CONFIG_FASTBOOT_MMC_BOOT1_SUPPORT
 >   	if (strcmp(cmd, CONFIG_FASTBOOT_MMC_BOOT1_NAME) == 0) {
 >   		/* erase EMMC boot1 */
 > -		fb_mmc_boot1_ops(dev_desc, NULL, 0, response);
 > +		dev_desc = fastboot_mmc_get_dev(response);
 > +		if (dev_desc)
 > +			fb_mmc_boot1_ops(dev_desc, NULL, 0, response);
 >   		return;
 >   	}
 >   #endif
 > @@ -605,6 +642,10 @@ void fastboot_mmc_erase(const char *cmd, char 
*response)
 >   #ifdef CONFIG_FASTBOOT_MMC_USER_NAME
 >   	if (strcmp(cmd, CONFIG_FASTBOOT_MMC_USER_NAME) == 0) {
 >   		/* erase EMMC userdata */
 > +		dev_desc = fastboot_mmc_get_dev(response);
 > +		if (!dev_desc)
 > +			return;
 > +
 >   		if (fb_mmc_erase_mmc_hwpart(dev_desc))
 >   			fastboot_fail("Failed to erase EMMC_USER", response);
 >   		else
 > @@ -613,13 +654,8 @@ void fastboot_mmc_erase(const char *cmd, char 
*response)
 >   	}
 >   #endif
 >
 > -	if (raw_part_get_info_by_name(dev_desc, cmd, &info) != 0) {
 > -		if (part_get_info_by_name_or_alias(dev_desc, cmd, &info) < 0) {
 > -			pr_err("cannot find partition: '%s'\n", cmd);
 > -			fastboot_fail("cannot find partition", response);
 > -			return;
 > -		}
 > -	}
 > +	if (fastboot_mmc_get_part_info(cmd, &dev_desc, &info, response) < 0)
 > +		return;
 >
 >   	/* Align blocks to erase group size to avoid erasing other 
partitions */
 >   	grp_size = mmc->erase_grp_size;
 > diff --git a/test/dm/fastboot.c b/test/dm/fastboot.c
 > index 8f905d8fa8..e7f8c362b8 100644
 > --- a/test/dm/fastboot.c
 > +++ b/test/dm/fastboot.c
 > @@ -35,9 +35,12 @@ static int dm_test_fastboot_mmc_part(struct 
unit_test_state *uts)
 >   		},
 >   	};
 >
 > -	ut_assertok(blk_get_device_by_str("mmc",
 > -					  __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV),
 > -					  &mmc_dev_desc));
 > +	/*
 > +	 * There are a lot of literal 0s I don't want to have to construct from
 > +	 * MMC_DEV.
 > +	 */
 > +	ut_asserteq(0, CONFIG_FASTBOOT_FLASH_MMC_DEV);
 > +	ut_assertok(blk_get_device_by_str("mmc", "0", &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);
 > @@ -59,6 +62,34 @@ static int dm_test_fastboot_mmc_part(struct 
unit_test_state *uts)
 >   						  &part_info, response));
 >   	ut_assertok(env_set(FB_ALIAS_PREFIX "test3", NULL));
 >
 > +	/* "New" partition labels */
 > +	ut_asserteq(1, fastboot_mmc_get_part_info("#test1", &fb_dev_desc,
 > +						  &part_info, response));
 > +	ut_asserteq(1, fastboot_mmc_get_part_info("0#test1", &fb_dev_desc,
 > +						  &part_info, response));
 > +	ut_asserteq(1, fastboot_mmc_get_part_info("0.0#test1", &fb_dev_desc,
 > +						  &part_info, response));
 > +	ut_asserteq(1, fastboot_mmc_get_part_info("0:1", &fb_dev_desc,
 > +						  &part_info, response));
 > +	ut_asserteq(1, fastboot_mmc_get_part_info("0.0:1", &fb_dev_desc,
 > +						  &part_info, response));
 > +	ut_asserteq(1, fastboot_mmc_get_part_info("0", &fb_dev_desc,
 > +						  &part_info, response));
 > +	ut_asserteq(1, fastboot_mmc_get_part_info("0.0", &fb_dev_desc,
 > +						  &part_info, response));
 > +	ut_asserteq(0, fastboot_mmc_get_part_info("0:0", &fb_dev_desc,
 > +						  &part_info, response));
 > +	ut_asserteq(0, fastboot_mmc_get_part_info("0.0:0", &fb_dev_desc,
 > +						  &part_info, response));
 > +	ut_asserteq(0, fastboot_mmc_get_part_info("1", &fb_dev_desc,
 > +						  &part_info, response));
 > +	ut_asserteq(0, fastboot_mmc_get_part_info("1.0", &fb_dev_desc,
 > +						  &part_info, response));
 > +	ut_asserteq(1, fastboot_mmc_get_part_info(":1", &fb_dev_desc,
 > +						  &part_info, response));
 > +	ut_asserteq(0, fastboot_mmc_get_part_info(":0", &fb_dev_desc,
 > +						  &part_info, response));
 > +
 >   	return 0;
 >   }
 >   DM_TEST(dm_test_fastboot_mmc_part, UT_TESTF_SCAN_PDATA | 
UT_TESTF_SCAN_FDT);
 >

[PATCH 1/9] mmc: sandbox: Add support for writing

[Simon Glass]

On Thu, 31 Dec 2020 at 15:49, Sean Anderson <sean.anderson@seco.com> wrote:
>
> This adds support writing to the sandbox mmc backed by an in-memory buffer.
> The unit test has been updated to test reading, writing, and erasing. I'm
> not sure what MMC erase to; I picked 0, but if it's 0xFF then that can be
> easily changed.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
>
>  drivers/mmc/sandbox_mmc.c | 41 ++++++++++++++++++++++++++++++++++-----
>  test/dm/mmc.c             | 19 +++++++++++++-----
>  2 files changed, 50 insertions(+), 10 deletions(-)

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

[PATCH 2/9] test: dm: Add test for fastboot mmc partition naming

[Simon Glass]

On Thu, 31 Dec 2020 at 15:49, Sean Anderson <sean.anderson@seco.com> wrote:
>
> This test verifies the mapping between fastboot partitions and partitions
> as understood by U-Boot. It also tests the creation of GPT partitions,
> though that is not the primary goal.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
>
>  configs/sandbox64_defconfig |  2 ++
>  configs/sandbox_defconfig   |  2 ++
>  test/dm/Makefile            |  3 ++
>  test/dm/fastboot.c          | 64 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 71 insertions(+)
>  create mode 100644 test/dm/fastboot.c

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

[PATCH 3/9] part: Give several functions more useful return values

[Simon Glass]

On Thu, 31 Dec 2020 at 15:49, Sean Anderson <sean.anderson@seco.com> wrote:
>
> Several functions in disk/part.c just return -1 on error. This makes them
> return different errnos for different failures. This helps callers
> differentiate between failures, even if they cannot read stdout.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
>
>  disk/part.c | 50 ++++++++++++++++++++++++++++----------------------
>  1 file changed, 28 insertions(+), 22 deletions(-)
>

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

[PATCH 4/9] part: Support getting whole disk from part_get_info_by_dev_and_name_or_num

[Simon Glass]

On Thu, 31 Dec 2020 at 15:49, Sean Anderson <sean.anderson@seco.com> wrote:
>
> This adds an option to part_get_info_by_dev_and_name_or_num to allow
> callers to specify whether whole-disk partitions are fine.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
>
>  cmd/ab_select.c | 3 ++-
>  disk/part.c     | 5 +++--
>  include/part.h  | 6 +++++-
>  3 files changed, 10 insertions(+), 4 deletions(-)
>

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

> diff --git a/cmd/ab_select.c b/cmd/ab_select.c
> index 6298fcfb60..3e46663d6e 100644
> --- a/cmd/ab_select.c
> +++ b/cmd/ab_select.c
> @@ -22,7 +22,8 @@ static int do_ab_select(struct cmd_tbl *cmdtp, int flag, int argc,
>
>         /* Lookup the "misc" partition from argv[2] and argv[3] */
>         if (part_get_info_by_dev_and_name_or_num(argv[2], argv[3],
> -                                                &dev_desc, &part_info) < 0) {
> +                                                &dev_desc, &part_info,
> +                                                false) < 0) {
>                 return CMD_RET_FAILURE;
>         }
>
> diff --git a/disk/part.c b/disk/part.c
> index 5e354e256f..39c6b00a59 100644
> --- a/disk/part.c
> +++ b/disk/part.c
> @@ -736,7 +736,8 @@ static int part_get_info_by_dev_and_name(const char *dev_iface,
>  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)
> +                                        struct disk_partition *part_info,
> +                                        int allow_whole_dev)

bool?

>  {
>         int ret;
>
> @@ -750,7 +751,7 @@ int part_get_info_by_dev_and_name_or_num(const char *dev_iface,
>          * directly.
>          */
>         ret = blk_get_device_part_str(dev_iface, dev_part_str,
> -                                     dev_desc, part_info, 1);
> +                                     dev_desc, part_info, allow_whole_dev);
>         if (ret < 0)
>                 printf("Couldn't find partition %s %s\n",
>                        dev_iface, dev_part_str);
> diff --git a/include/part.h b/include/part.h
> index 55be724d20..778cb36199 100644
> --- a/include/part.h
> +++ b/include/part.h
> @@ -226,12 +226,16 @@ int part_get_info_by_name(struct blk_desc *dev_desc,
>   * @param[in] dev_part_str Input partition description, like "0#misc" or "0:1"
>   * @param[out] dev_desc        Place to store the device description pointer
>   * @param[out] part_info Place to store the partition information
> + * @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
>   */
>  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);
> +                                        struct disk_partition *part_info,
> +                                        int allow_whole_dev);
>
>  /**
>   * part_set_generic_name() - create generic partition like hda1 or sdb2
> --
> 2.25.1
>

Regards,
Simon

[PATCH 5/9] part: Support string block devices in part_get_info_by_dev_and_name

[Simon Glass]

On Thu, 31 Dec 2020 at 15:49, Sean Anderson <sean.anderson@seco.com> wrote:
>
> This adds support for things like "#partname" and "0.1#partname". The block
> device parsing is done like in blk_get_device_part_str.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
>
>  disk/part.c | 32 +++++++++++++++++---------------
>  1 file changed, 17 insertions(+), 15 deletions(-)

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

Does the function comment need updating?


>
> diff --git a/disk/part.c b/disk/part.c
> index 39c6b00a59..e07dd67fd9 100644
> --- a/disk/part.c
> +++ b/disk/part.c
> @@ -707,29 +707,31 @@ static int part_get_info_by_dev_and_name(const char *dev_iface,
>                                          struct blk_desc **dev_desc,
>                                          struct disk_partition *part_info)
>  {
> -       char *ep;
> -       const char *part_str;
> -       int dev_num, ret;
> +       char *dup_str = NULL;
> +       const char *dev_str, *part_str;
> +       int ret;
>
> +       /* Separate device and partition name specification */
>         part_str = strchr(dev_part_str, '#');
> -       if (!part_str || part_str == dev_part_str)
> -               return -EINVAL;
> -
> -       dev_num = simple_strtoul(dev_part_str, &ep, 16);
> -       if (ep != part_str) {
> -               /* Not all the first part before the # was parsed. */
> +       if (part_str) {
> +               dup_str = strdup(dev_part_str);
> +               dup_str[part_str - dev_part_str] = 0;
> +               dev_str = dup_str;
> +               part_str++;
> +       } else {
>                 return -EINVAL;
>         }
> -       part_str++;
>
> -       *dev_desc = blk_get_dev(dev_iface, dev_num);
> -       if (!*dev_desc) {
> -               printf("Could not find %s %d\n", dev_iface, dev_num);
> -               return -ENODEV;
> -       }
> +       ret = blk_get_device_by_str(dev_iface, dev_str, dev_desc);
> +       if (ret)
> +               goto cleanup;
> +
>         ret = part_get_info_by_name(*dev_desc, part_str, part_info);
>         if (ret < 0)
>                 printf("Could not find \"%s\" partition\n", part_str);
> +
> +cleanup:
> +       free(dup_str);
>         return ret;
>  }
>
> --
> 2.25.1
>

[PATCH 6/9] fastboot: Remove mmcpart argument from raw_part_get_info_by_name

[Simon Glass]

On Thu, 31 Dec 2020 at 15:49, Sean Anderson <sean.anderson@seco.com> wrote:
>
> The only thing mmcpart was used for was to pass to blk_dselect_hwpart.
> This calls blk_dselect_hwpart directly from raw_part_get_info_by_name. The
> error handling is dropped, but it is reintroduced in the next commit
> (albeit less specificly).
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
>
>  drivers/fastboot/fb_mmc.c | 41 +++++++++++++++++----------------------
>  1 file changed, 18 insertions(+), 23 deletions(-)

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

[PATCH 7/9] fastboot: Move part_get_info_by_name_or_alias after raw_part_get_info_by_name

[Simon Glass]

On Thu, 31 Dec 2020 at 15:49, Sean Anderson <sean.anderson@seco.com> wrote:
>
> This makes the next commit more readable by doing the move now.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
>
>  drivers/fastboot/fb_mmc.c | 44 +++++++++++++++++++--------------------
>  1 file changed, 22 insertions(+), 22 deletions(-)

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

[PATCH 9/9] fastboot: Document alternate partition names

[Simon Glass]

On Thu, 31 Dec 2020 at 15:49, Sean Anderson <sean.anderson@seco.com> wrote:
>
> This documents the new partition names added in the previous commit.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
>
>  doc/android/fastboot.rst | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>

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

(for content, not location)

[PATCH 8/9] fastboot: Allow u-boot-style partitions

[Simon Glass]

On Mon, 4 Jan 2021 at 09:53, Sean Anderson <sean.anderson@seco.com> wrote:
>
>
>
> On 12/31/20 5:48 PM, Sean Anderson wrote:
>  > This adds support for partitions of the form "dev.hwpart:part" and
>  > "dev#partname". This allows one to flash to eMMC boot partitions without
>  > having to use CONFIG_FASTBOOT_MMC_BOOT1_SUPPORT. It also allows one to
>  > flash to an entire device without needing CONFIG_FASTBOOT_MMC_USER_NAME.
>  > Lastly, one can also flash MMC devices other than
>  > CONFIG_FASTBOOT_FLASH_MMC_DEV.
>  >
>  > Because devices can be specified explicitly,
> CONFIG_FASTBOOT_FLASH_MMC_DEV
>  > is used only when necessary for existing functionality. For those cases,
>  > fastboot_mmc_get_dev has been added as a helper function. This allows
>  >
>  > There should be no conflicts with the existing system, but just in
> case, I
>  > have ordered detection of these names after all existing names.
>  >
>  > The fastboot_mmc_part test has been updated for these new names.
>  >
>  > Signed-off-by: Sean Anderson <sean.anderson@seco.com>
>  > ---
>  >
>  >   drivers/fastboot/fb_mmc.c | 150 +++++++++++++++++++++++---------------
>  >   test/dm/fastboot.c        |  37 +++++++++-
>  >   2 files changed, 127 insertions(+), 60 deletions(-)
>  >
>  > diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
>  > index b0610d3151..a52b1e3ed6 100644
>  > --- a/drivers/fastboot/fb_mmc.c
>  > +++ b/drivers/fastboot/fb_mmc.c
>  > @@ -37,6 +37,7 @@ static int raw_part_get_info_by_name(struct
> blk_desc *dev_desc,
>  >      char *raw_part_desc;
>  >      const char *argv[2];
>  >      const char **parg = argv;
>  > +    int ret;
>  >
>  >      /* check for raw partition descriptor */
>  >      strcpy(env_desc_name, "fastboot_raw_partition_");
>  > @@ -60,7 +61,7 @@ static int raw_part_get_info_by_name(struct
> blk_desc *dev_desc,
>  >
>  >      info->start = simple_strtoul(argv[0], NULL, 0);
>  >      info->size = simple_strtoul(argv[1], NULL, 0);
>  > -    info->blksz = dev_desc->blksz;
>  > +    info->blksz = *dev_desc->blksz;
>  >      strncpy((char *)info->name, name, PART_NAME_LEN);
>
> Looks like this slipped through while rebasing. The above two hunks
> shouldn't have been included; will be fixed in v2.

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

[PATCH 4/9] part: Support getting whole disk from part_get_info_by_dev_and_name_or_num

[Sean Anderson]

On 1/7/21 7:35 AM, Simon Glass wrote:
 > On Thu, 31 Dec 2020 at 15:49, Sean Anderson <sean.anderson@seco.com> 
wrote:
 >>
 >> This adds an option to part_get_info_by_dev_and_name_or_num to allow
 >> callers to specify whether whole-disk partitions are fine.
 >>
 >> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
 >> ---
 >>
 >>   cmd/ab_select.c | 3 ++-
 >>   disk/part.c     | 5 +++--
 >>   include/part.h  | 6 +++++-
 >>   3 files changed, 10 insertions(+), 4 deletions(-)
 >>
 >
 > Reviewed-by: Simon Glass <sjg@chromium.org>
 >
 >> diff --git a/cmd/ab_select.c b/cmd/ab_select.c
 >> index 6298fcfb60..3e46663d6e 100644
 >> --- a/cmd/ab_select.c
 >> +++ b/cmd/ab_select.c
 >> @@ -22,7 +22,8 @@ static int do_ab_select(struct cmd_tbl *cmdtp, int 
flag, int argc,
 >>
 >>          /* Lookup the "misc" partition from argv[2] and argv[3] */
 >>          if (part_get_info_by_dev_and_name_or_num(argv[2], argv[3],
 >> -                                                &dev_desc, 
&part_info) < 0) {
 >> +                                                &dev_desc, &part_info,
 >> +                                                false) < 0) {
 >>                  return CMD_RET_FAILURE;
 >>          }
 >>
 >> diff --git a/disk/part.c b/disk/part.c
 >> index 5e354e256f..39c6b00a59 100644
 >> --- a/disk/part.c
 >> +++ b/disk/part.c
 >> @@ -736,7 +736,8 @@ static int part_get_info_by_dev_and_name(const 
char *dev_iface,
 >>   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)
 >> +                                        struct disk_partition 
*part_info,
 >> +                                        int allow_whole_dev)
 >
 > bool?

It's int to match the allow_whole_dev argument of
blk_get_device_part_str.

 >
 >>   {
 >>          int ret;
 >>
 >> @@ -750,7 +751,7 @@ int part_get_info_by_dev_and_name_or_num(const 
char *dev_iface,
 >>           * directly.
 >>           */
 >>          ret = blk_get_device_part_str(dev_iface, dev_part_str,
 >> -                                     dev_desc, part_info, 1);
 >> +                                     dev_desc, part_info, 
allow_whole_dev);
 >>          if (ret < 0)
 >>                  printf("Couldn't find partition %s %s\n",
 >>                         dev_iface, dev_part_str);
 >> diff --git a/include/part.h b/include/part.h
 >> index 55be724d20..778cb36199 100644
 >> --- a/include/part.h
 >> +++ b/include/part.h
 >> @@ -226,12 +226,16 @@ int part_get_info_by_name(struct blk_desc 
*dev_desc,
 >>    * @param[in] dev_part_str Input partition description, like 
"0#misc" or "0:1"
 >>    * @param[out] dev_desc        Place to store the device 
description pointer
 >>    * @param[out] part_info Place to store the partition information
 >> + * @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
 >>    */
 >>   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);
 >> +                                        struct disk_partition 
*part_info,
 >> +                                        int allow_whole_dev);
 >>
 >>   /**
 >>    * part_set_generic_name() - create generic partition like hda1 or 
sdb2
 >> --
 >> 2.25.1
 >>
 >
 > Regards,
 > Simon
 >

[PATCH 5/9] part: Support string block devices in part_get_info_by_dev_and_name

[Sean Anderson]

On 1/7/21 7:35 AM, Simon Glass wrote:
> On Thu, 31 Dec 2020 at 15:49, Sean Anderson <sean.anderson@seco.com> wrote:
>>
>> This adds support for things like "#partname" and "0.1#partname". The block
>> device parsing is done like in blk_get_device_part_str.
>>
>> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
>> ---
>>
>>   disk/part.c | 32 +++++++++++++++++---------------
>>   1 file changed, 17 insertions(+), 15 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> Does the function comment need updating?

Yes. Will be updated.

--Sean

> 
> 
>>
>> diff --git a/disk/part.c b/disk/part.c
>> index 39c6b00a59..e07dd67fd9 100644
>> --- a/disk/part.c
>> +++ b/disk/part.c
>> @@ -707,29 +707,31 @@ static int part_get_info_by_dev_and_name(const char *dev_iface,
>>                                           struct blk_desc **dev_desc,
>>                                           struct disk_partition *part_info)
>>   {
>> -       char *ep;
>> -       const char *part_str;
>> -       int dev_num, ret;
>> +       char *dup_str = NULL;
>> +       const char *dev_str, *part_str;
>> +       int ret;
>>
>> +       /* Separate device and partition name specification */
>>          part_str = strchr(dev_part_str, '#');
>> -       if (!part_str || part_str == dev_part_str)
>> -               return -EINVAL;
>> -
>> -       dev_num = simple_strtoul(dev_part_str, &ep, 16);
>> -       if (ep != part_str) {
>> -               /* Not all the first part before the # was parsed. */
>> +       if (part_str) {
>> +               dup_str = strdup(dev_part_str);
>> +               dup_str[part_str - dev_part_str] = 0;
>> +               dev_str = dup_str;
>> +               part_str++;
>> +       } else {
>>                  return -EINVAL;
>>          }
>> -       part_str++;
>>
>> -       *dev_desc = blk_get_dev(dev_iface, dev_num);
>> -       if (!*dev_desc) {
>> -               printf("Could not find %s %d\n", dev_iface, dev_num);
>> -               return -ENODEV;
>> -       }
>> +       ret = blk_get_device_by_str(dev_iface, dev_str, dev_desc);
>> +       if (ret)
>> +               goto cleanup;
>> +
>>          ret = part_get_info_by_name(*dev_desc, part_str, part_info);
>>          if (ret < 0)
>>                  printf("Could not find \"%s\" partition\n", part_str);
>> +
>> +cleanup:
>> +       free(dup_str);
>>          return ret;
>>   }
>>
>> --
>> 2.25.1
>>