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

Sean Anderson sean.anderson at seco.com
Fri Feb 5 15:52:20 CET 2021



On 2/5/21 9:46 AM, Lukasz Majewski wrote:
 > Hi Sean,
 >
 >> 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.
 >
 > This patch series causes following build errors:
 > 
https://dev.azure.com/lukma633/U-Boot/_build/results?buildId=20&view=results

Yes, I saw those errors; they should be addressed in v5.

 >
 > I saw the v5 of this patch series. Could you check if it pass with
 > green the CI tests?

It is building at [1].

[1] 
https://dev.azure.com/u-boot/u-boot/_build/results?buildId=1748&view=results.

--Sean

 >
 > Thanks in advance,
 >
 >>
 >> 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 at seco.com>
 >> Reviewed-by: Simon Glass <sjg at chromium.org>
 >> ---
 >>
 >> Changes in v4:
 >> - Fix missing closing brace
 >>
 >>   drivers/fastboot/fb_mmc.c | 157
 >> ++++++++++++++++++++++++-------------- test/dm/fastboot.c        |
 >> 37 ++++++++- 2 files changed, 132 insertions(+), 62 deletions(-)
 >>
 >> diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
 >> index 71eeb02c8f..8e74e50e91 100644
 >> --- a/drivers/fastboot/fb_mmc.c
 >> +++ b/drivers/fastboot/fb_mmc.c
 >> @@ -76,12 +76,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;
 >>
 >> -	ret = part_get_info_by_name(dev_desc, name, info);
 >> +	/* 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 = 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 +117,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;
 >>   }
 >> @@ -430,27 +455,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;
 >>   }
 >>
 >>   /**
 >> @@ -467,22 +514,19 @@ 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_BOOT_SUPPORT
 >>   	if (strcmp(cmd, CONFIG_FASTBOOT_MMC_BOOT1_NAME) == 0) {
 >> -		fb_mmc_boot_ops(dev_desc, download_buffer, 1,
 >> -				download_bytes, response);
 >> +		dev_desc = fastboot_mmc_get_dev(response);
 >> +		if (dev_desc)
 >> +			fb_mmc_boot_ops(dev_desc, download_buffer, 1,
 >> +					download_bytes, response);
 >>   		return;
 >>   	}
 >>   	if (strcmp(cmd, CONFIG_FASTBOOT_MMC_BOOT2_NAME) == 0) {
 >> -		fb_mmc_boot_ops(dev_desc, download_buffer, 2,
 >> -				download_bytes, response);
 >> +		dev_desc = fastboot_mmc_get_dev(response);
 >> +		if (dev_desc)
 >> +			fb_mmc_boot_ops(dev_desc, download_buffer, 1,
 >> +					download_bytes, response);
 >>   		return;
 >>   	}
 >>   #endif
 >> @@ -494,6 +538,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)) {
 >> @@ -517,6 +565,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", @@ -539,19 +591,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;
 >> @@ -594,28 +643,19 @@ 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_BOOT_SUPPORT
 >>   	if (strcmp(cmd, CONFIG_FASTBOOT_MMC_BOOT1_NAME) == 0) {
 >>   		/* erase EMMC boot1 */
 >> -		fb_mmc_boot_ops(dev_desc, NULL, 1, 0, response);
 >> +		dev_desc = fastboot_mmc_get_dev(response);
 >> +		if (dev_desc)
 >> +			fb_mmc_boot_ops(dev_desc, NULL, 1, 0,
 >> response); return;
 >>   	}
 >>   	if (strcmp(cmd, CONFIG_FASTBOOT_MMC_BOOT2_NAME) == 0) {
 >>   		/* erase EMMC boot2 */
 >> -		fb_mmc_boot_ops(dev_desc, NULL, 2, 0, response);
 >> +		dev_desc = fastboot_mmc_get_dev(response);
 >> +		if (dev_desc)
 >> +			fb_mmc_boot_ops(dev_desc, NULL, 1, 0,
 >> response); return;
 >>   	}
 >>   #endif
 >> @@ -623,6 +663,10 @@ void fastboot_mmc_erase(const char *cmd, char
 >> *response) #ifdef CONFIG_FASTBOOT_MMC_USER_SUPPORT
 >>   	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
 >> @@ -631,13 +675,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);
 >
 >
 >
 >
 > Best regards,
 >
 > Lukasz Majewski
 >
 > --
 >
 > DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
 > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
 > Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
 >


More information about the U-Boot mailing list