From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Anderson Date: Sat, 6 Feb 2021 18:49:38 -0500 Subject: [PATCH v4 8/9] fastboot: Allow u-boot-style partitions In-Reply-To: <20210206183749.139d3e85@jawa> References: <20210202152352.33245-1-sean.anderson@seco.com> <20210202152352.33245-9-sean.anderson@seco.com> <20210205154633.659cf4c6@jawa> <6f1e203d-94ec-5eb6-fe74-a652a33d455a@seco.com> <20210206183749.139d3e85@jawa> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 2/6/21 12:37 PM, Lukasz Majewski wrote: > 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. It looks like it's due to the following construct in fastboot.h #if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_BOOTBUS) FASTBOOT_COMMAND_OEM_BOOTBUS, #if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT) FASTBOOT_COMMAND_ACMD, FASTBOOT_COMMAND_UCMD, #endif Which was added in 427f563 ("fastboot: add UUU command UCmd and ACmd support") It seems like Heiko made an error while rebasing, CCing him. --Sean > > 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 >> >> Reviewed-by: Simon Glass >> >> --- >> >> >> >> 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 > > > > > 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 >