[PATCH 1/1] cmd: gpt: Enumerate partitions and save info into an U-Boot variable

[Simon Glass]

Hi Vladimir,

On Fri, 22 Nov 2019 at 14:48, Vladimir Olovyannikov
<vladimir.olovyannikov at broadcom.com> wrote:
>
> From: Corneliu Doban <corneliu.doban at broadcom.com>
>
> Add enumeration of gpt partitions and saving this information into
> U-Boot variables.
>
> Signed-off-by: Corneliu Doban <corneliu.doban at broadcom.com>
> Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov at broadcom.com>
> ---
>  cmd/gpt.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 95 insertions(+)
>
> diff --git a/cmd/gpt.c b/cmd/gpt.c
> index 0c4349f4b2..061ffaa757 100644
> --- a/cmd/gpt.c
> +++ b/cmd/gpt.c
> @@ -2,6 +2,8 @@
>  /*
>   * cmd_gpt.c -- GPT (GUID Partition Table) handling command
>   *
> + * Copyright (C) 2019 Broadcom
> + * author: Corneliu Doban <corneliu.doban at broadcom.com>
>   * Copyright (C) 2015
>   * Lukasz Majewski <l.majewski at majess.pl>
>   *
> @@ -804,6 +806,88 @@ static int do_rename_gpt_parts(struct blk_desc *dev_desc, char *subcomm,
>  }
>  #endif
>
> +/*
> + * Enumerate partition names into environment variable.
> + */
> +static int gpt_enumerate(struct blk_desc *blk_dev_desc)
> +{
> +       disk_partition_t pinfo;
> +       struct part_driver *first_drv =
> +               ll_entry_start(struct part_driver, part_driver);
> +       const int n_drvs = ll_entry_count(struct part_driver, part_driver);
> +       struct part_driver *part_drv;
> +       char part_list[2048];
> +
> +       part_list[0] = 0;
> +
> +       for (part_drv = first_drv; part_drv != first_drv + n_drvs; part_drv++) {
> +               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 != 0) {
> +                               /* no more entries in table */
> +                               break;
> +                       }
> +                       strcat(part_list, (const char *)pinfo.name);
> +                       strcat(part_list, " ");
> +               }
> +       }
> +       if (strlen(part_list) > 0)
> +               part_list[strlen(part_list) - 1] = 0;
> +       debug("setenv gpt_partition_list %s\n", part_list);
> +       env_set("gpt_partition_list", part_list);
> +       return 0;
> +}
> +
> +/*
> + * Dynamically setup environment variables for name, index, offset and size
> + * for partition in GPT table after running "gpt setenv" for a partition name.
> + * gpt_partition_name, gpt_partition_entry, gpt_partition_addr and
> + * gpt_partition_size environment variables will be set.

comment arguments and return value

> + */
> +static int gpt_setenv(struct blk_desc *blk_dev_desc, const char *name)
> +{
> +       disk_partition_t pinfo;
> +       struct part_driver *first_drv =
> +               ll_entry_start(struct part_driver, part_driver);
> +       const int n_drvs = ll_entry_count(struct part_driver, part_driver);
> +       struct part_driver *part_drv;
> +       char buf[32];
> +
> +       for (part_drv = first_drv; part_drv != first_drv + n_drvs; part_drv++) {
> +               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 != 0) {
> +                               /* no more entries in table */
> +                               break;
> +                       }
> +                       if (strcmp(name, (const char *)pinfo.name) == 0) {

if (!strcmp...

> +                               /* match found, setup environment variables */
> +                               sprintf(buf, LBAF, pinfo.start);
> +                               debug("setenv gpt_partition_addr %s\n", buf);
> +                               env_set("gpt_partition_addr", buf);
> +                               sprintf(buf, LBAF, pinfo.size);
> +                               debug("setenv gpt_partition_size %s\n", buf);
> +                               env_set("gpt_partition_size", buf);
> +                               sprintf(buf, "%d", i);
> +                               debug("setenv gpt_partition_entry %s\n", buf);

See env_set_ulong()

> +                               env_set("gpt_partition_entry", buf);
> +                               sprintf(buf, "%s", pinfo.name);

Why not just write pinfo.name directly?

> +                               debug("setenv gpt_partition_name %s\n", buf);
> +                               env_set("gpt_partition_name", buf);
> +                               return 0;
> +                       }
> +               }
> +       }
> +       return -1;

If that is an error it should be a -E... error code, perhaps -ENOENT -
see errno.h

> +}
> +
>  /**
>   * do_gpt(): Perform GPT operations
>   *
> @@ -855,6 +939,10 @@ static int do_gpt(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>                    (strcmp(argv[1], "rename") == 0)) {
>                 ret = do_rename_gpt_parts(blk_dev_desc, argv[1], argv[4], argv[5]);
>  #endif
> +       } 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 {
>                 return CMD_RET_USAGE;
>         }
> @@ -897,4 +985,11 @@ U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt,
>         " gpt swap mmc 0 foo bar\n"
>         " gpt rename mmc 0 3 foo\n"
>  #endif
> +       " 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"
> +
>  );
> --
> 2.17.1
>

Can you please write a sandbox test for this command? See cmd_ut.c for this.

Regards,
Simon