Re: [PATCH v3 1/1] cmd: gpt: add eMMC and GPT support

On 15.05.20 07:52, Rayagonda Kokatanur wrote:

Hi Heinrich,

On Fri, May 15, 2020 at 5:27 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

On 5/13/20 5:27 PM, Rayagonda Kokatanur wrote:

...

From: Corneliu Doban cdoban@broadcom.com

Add eMMC and GPT support.

• GPT partition list and command to create the GPT added to u-boot environment
• eMMC boot commands added to u-boot environment
• new gpt commands (enumarate and setenv) that are used by broadcom update scripts and boot commands
• eMMC specific u-boot configurations with environment saved in eMMC and GPT support

Signed-off-by: Corneliu Doban cdoban@broadcom.com Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com

---

Changes from v2: -Address review comments from Simon Glass, Check for return value of part_driver_get_count(), Don't check return value of part_driver_get(), Rewrite part_driver_get() and rename to part_driver_get_first(), Use env_set_ulong() whereever applicable,

-Address review comments from Michael Nazzareno Trimarchi, Add new function to set all env vriables,

Changes from v1: -Address review comments from Simon Glass, Correct function comments, Check for return value, Add helper function in part.h

cmd/gpt.c | 160 +++++++++++++++++++++++++++++++++++++++++++++++++ include/part.h | 29 +++++++++ 2 files changed, 189 insertions(+)

diff --git a/cmd/gpt.c b/cmd/gpt.c index b8d11c167d..bba79aca64 100644 --- a/cmd/gpt.c +++ b/cmd/gpt.c @@ -15,6 +15,7 @@ #include <malloc.h> #include <command.h> #include <part_efi.h> +#include <part.h> #include <exports.h> #include <linux/ctype.h> #include <div64.h> @@ -616,6 +617,151 @@ static int gpt_verify(struct blk_desc *blk_dev_desc, const char *str_part) return ret; }

+/**

• • gpt_enumerate() - Enumerate partition names into environment variable.
• • Enumerate partition names. Partition names are stored in gpt_partition_list
• • environment variable. Each partition name is delimited by space.
• • @blk_dev_desc: block device descriptor
• • @Return: '0' on success and 1 on failure
• */

+static int gpt_enumerate(struct blk_desc *blk_dev_desc) +{

• struct part_driver *first_drv, *part_drv;
  

• int str_len = 0, tmp_len;
  

• char part_list[2048];
  

• int n_drvs;
  

• char *ptr;
  

• part_list[0] = 0;
  

• n_drvs = part_driver_get_count();
  

• if (!n_drvs) {
  

•         printf("Failed to get partition driver count\n");
  

•         return 1;
  

• }
  

• first_drv = part_driver_get_first();
  

• for (part_drv = first_drv; part_drv != first_drv + n_drvs; part_drv++) {
  

•         disk_partition_t pinfo;
  

•         int ret;
  

•         int i;
  

•         for (i = 1; i < part_drv->max_entries; i++) {
  

•                 ret = part_drv->get_info(blk_dev_desc, i, &pinfo);
  

•                 if (ret) {
  

•                         /* no more entries in table */
  

•                         break;
  

•                 }
  

•                 ptr = &part_list[str_len];
  

•                 tmp_len = strlen((const char *)pinfo.name);
  

•                 str_len += tmp_len;
  

•                 if (str_len > sizeof(part_list)) {
  

•                         printf("Error insufficient memory\n");
  

•                         return -ENOMEM;
  

•                 }
  

•                 strncpy(ptr, (const char *)pinfo.name, tmp_len);
  

•                 /* One byte for space(" ") delimiter */
  

•                 strncpy(&ptr[tmp_len], " ", 1);
  

•                 str_len++;
  

•         }
  

• }
  

• if (*part_list)
  

•         part_list[strlen(part_list) - 1] = 0;
  

• debug("setenv gpt_partition_list %s\n", part_list);
  

• return env_set("gpt_partition_list", part_list);
  

+}

+/**

• • gpt_setenv_part_variables() - setup partition environmental variables
• • Setup the gpt_partition_name, gpt_partition_entry, gpt_partition_addr
• • and gpt_partition_size environment variables.
• • @pinfo: pointer to disk partition
• • @i: partition entry
• • @Return: '0' on success and -ENOENT on failure
• */

+static int gpt_setenv_part_variables(disk_partition_t *pinfo, int i) +{

• int ret;
  

• ret = env_set_ulong("gpt_partition_addr", pinfo->start);
  

• if (ret)
  

•         goto fail;
  

• ret = env_set_ulong("gpt_partition_size", pinfo->size);
  

• if (ret)
  

•         goto fail;
  

• ret = env_set_ulong("gpt_partition_entry", i);
  

• if (ret)
  

•         goto fail;
  

• ret = env_set("gpt_partition_name", pinfo->name);
  

• if (ret)
  

•         goto fail;
  

• return 0;
  

+fail:

• return -ENOENT;
  

+}

+/**

• • gpt_setenv() - Dynamically setup environment variables.
• • Dynamically setup environment variables for name, index, offset and size
• • for partition in GPT table after running "gpt setenv" for a partition name.
• • @blk_dev_desc: block device descriptor
• • @name: partition name
• • @Return: '0' on success and -ENOENT on failure
• */

+static int gpt_setenv(struct blk_desc *blk_dev_desc, const char *name) +{

• struct part_driver *first_drv, *part_drv;
  

• int n_drvs;
  

• n_drvs = part_driver_get_count();
  

• if (!n_drvs) {
  

•         printf("Failed to get partition driver count\n");
  

•         goto fail;
  

• }
  

• first_drv = part_driver_get_first();
  

• for (part_drv = first_drv; part_drv != first_drv + n_drvs; part_drv++) {
  

•         disk_partition_t pinfo;
  

•         int ret;
  

•         int i;
  

•         for (i = 1; i < part_drv->max_entries; i++) {
  

•                 ret = part_drv->get_info(blk_dev_desc, i, &pinfo);
  

•                 if (ret) {
  

•                         /* no more entries in table */
  

•                         break;
  

•                 }
  

•                 if (strcmp(name, (const char *)pinfo.name) == 0) {
  

•                         /* match found, setup environment variables */
  

•                         ret = gpt_setenv_part_variables(&pinfo, i);
  

•                         if (ret)
  

•                                 goto fail;
  

•                         return 0;
  

•                 }
  

•         }
  

• }
  

+fail:

• return -ENOENT;
  

+}

static int do_disk_guid(struct blk_desc *dev_desc, char * const namestr) { int ret; @@ -822,6 +968,10 @@ static int do_gpt(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } else if ((strcmp(argv[1], "verify") == 0)) { ret = gpt_verify(blk_dev_desc, argv[4]); printf("Verify GPT: ");

• } else if ((strcmp(argv[1], "setenv") == 0)) {
  

•         ret = gpt_setenv(blk_dev_desc, argv[4]);
  

• } else if ((strcmp(argv[1], "enumerate") == 0)) {
  

•         ret = gpt_enumerate(blk_dev_desc);
  } else if (strcmp(argv[1], "guid") == 0) {
  

The way sub-commands are implemented here does not conform to doc/README.commands. A TODO for a later patch.

Thank you for your review. Request you to tell what is not confirm with doc/REDME.commands

Sub-chapter "Sub-command definition" describes how to use macro U_BOOT_CMD_MKENT().

Best regards

Heinrich

Best regards, Rayagonda

...

Best regards

Heinrich

...

          ret = do_disk_guid(blk_dev_desc, argv[4]);

#ifdef CONFIG_CMD_GPT_RENAME @@ -852,7 +1002,17 @@ U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt, " to interface\n" " Example usage:\n" " gpt write mmc 0 $partitions\n"

• "    - write the GPT to device\n"
  " gpt verify mmc 0 $partitions\n"
  

• "    - verify the GPT on device against $partitions\n"
  

• " gpt setenv mmc 0 $name\n"
  

• "    - setup environment variables for partition $name:\n"
  

• "      gpt_partition_addr, gpt_partition_size,\n"
  

• "      gpt_partition_name, gpt_partition_entry\n"
  

• " gpt enumerate mmc 0\n"
  

• "    - store list of partitions to gpt_partition_list environment variable\n"
  

• " read <interface> <dev>\n"
  

• "    - read GPT into a data structure for manipulation\n"
  " gpt guid <interface> <dev>\n"
  "    - print disk GUID\n"
  " gpt guid <interface> <dev> <varname>\n"
  

diff --git a/include/part.h b/include/part.h index 3693527397..bf45c0497b 100644 --- a/include/part.h +++ b/include/part.h @@ -9,6 +9,7 @@ #include <blk.h> #include <ide.h> #include <uuid.h> +#include <linker_lists.h> #include <linux/list.h>

struct block_drvr { @@ -474,5 +475,33 @@ int write_mbr_partition(struct blk_desc *dev_desc, void *buf);

#endif

+#ifdef CONFIG_PARTITIONS +/**

• • part_driver_get_count() - get partition driver count
• • @return - number of partition drivers
• */

+static inline int part_driver_get_count(void) +{

• return ll_entry_count(struct part_driver, part_driver);
  

+}

+/**

• • part_driver_get_first() - get first partition driver
• • @return - pointer to first partition driver on success, otherwise NULL
• */

+static inline struct part_driver *part_driver_get_first(void) +{

• return ll_entry_start(struct part_driver, part_driver);
  

+}

+#else +static inline int part_driver_get_count(void) +{ return 0; }

+static inline struct part_driver *part_driver_get_first(void) +{ return NULL; } +#endif /* CONFIG_PARTITIONS */

#endif /* _PART_H */