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


Reply via email to