Hi Sean, > 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. >
Interesting as https://dev.azure.com/lukma633/U-Boot/_build/results?buildId=22&view=logs&j=425537f6-562f-5a62-0856-6c59be70cbe4&t=62b0f1ee-aadf-504c-f243-098b7c952989 Still shows errors after applying patch v5 version. I'm using this series applied on top of Marek's usb tree: https://github.com/lmajewski/u-boot-dfu/commits/testing Could you look on this issue? > --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.ander...@seco.com> > >> Reviewed-by: Simon Glass <s...@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: > > lu...@denx.de 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: lu...@denx.de
pgp9sGkiH3s5_.pgp
Description: OpenPGP digital signature