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.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
>
