diff mbox series

cmd: gpt: add eMMC and GPT support

Message ID 20200505121314.15941-1-rayagonda.kokatanur@broadcom.com
State Superseded
Headers show
Series cmd: gpt: add eMMC and GPT support | expand

Commit Message

Rayagonda Kokatanur May 5, 2020, 12:13 p.m. UTC
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>

Comments

Rayagonda Kokatanur May 5, 2020, 12:16 p.m. UTC | #1
On Tue, May 5, 2020 at 5:43 PM Rayagonda Kokatanur
<rayagonda.kokatanur@broadcom.com> 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>
>
> diff --git a/cmd/gpt.c b/cmd/gpt.c
> index b8d11c167d..c32e272b25 100644
> --- a/cmd/gpt.c
> +++ b/cmd/gpt.c
> @@ -616,6 +616,87 @@ static int gpt_verify(struct blk_desc *blk_dev_desc, const char *str_part)
>         return ret;
>  }
>
> +/*
> + * 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.
> + */
> +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) {
> +                               /* 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);
> +                               env_set("gpt_partition_entry", buf);
> +                               sprintf(buf, "%s", pinfo.name);
> +                               debug("setenv gpt_partition_name %s\n", buf);
> +                               env_set("gpt_partition_name", buf);
> +                               return 0;
> +                       }
> +               }
> +       }
> +       return -1;
> +}
> +
>  static int do_disk_guid(struct blk_desc *dev_desc, char * const namestr)
>  {
>         int ret;
> @@ -822,6 +903,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) {
>                 ret = do_disk_guid(blk_dev_desc, argv[4]);
>  #ifdef CONFIG_CMD_GPT_RENAME
> @@ -852,7 +937,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"
> --
> 2.17.1
>

Please ignore this patch, forgot to add "PATCH v1" in the subject line.
Will send another  patch.
Simon Glass May 5, 2020, 4:02 p.m. UTC | #2
Hi Rayagonda,

On Tue, 5 May 2020 at 06:17, Rayagonda Kokatanur
<rayagonda.kokatanur@broadcom.com> wrote:
>
> On Tue, May 5, 2020 at 5:43 PM Rayagonda Kokatanur
> <rayagonda.kokatanur@broadcom.com> 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>
> >

>
> Please ignore this patch, forgot to add "PATCH v1" in the subject line.
> Will send another  patch.

It's OK, you don't need to have v1 for the first patch.

Regards,
SImon
Simon Glass May 5, 2020, 4:02 p.m. UTC | #3
Hi Rayagonda,

On Tue, 5 May 2020 at 06:13, Rayagonda Kokatanur
<rayagonda.kokatanur@broadcom.com> 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>
>
> diff --git a/cmd/gpt.c b/cmd/gpt.c
> index b8d11c167d..c32e272b25 100644
> --- a/cmd/gpt.c
> +++ b/cmd/gpt.c
> @@ -616,6 +616,87 @@ static int gpt_verify(struct blk_desc *blk_dev_desc, const char *str_part)
>         return ret;
>  }
>
> +/*
> + * Enumerate partition names into environment variable.
> + */

Can you check the style for these comments? You can see examples in
bootcount.h. Please fix for all functions.

It should mention the params and return value.

Also in this case it should explain which environment variable is
updated and the format that is used in the 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);

Can you create functions in part.h to handle this? Something like:

int part_driver_get_count()
struct part_driver *part_driver_get((int n);

Then we don't have lots of places accessing this.

> +       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) {

if (ret)

> +                               /* no more entries in table */
> +                               break;
> +                       }
> +                       strcat(part_list, (const char *)pinfo.name);
> +                       strcat(part_list, " ");

Need to check that you don't go past sizeof(part_list). Can I suggest
that you have a ptr to the end of your string to avoid n^2 behaviour
here?

> +               }
> +       }
> +       if (strlen(part_list) > 0)

if (*part_list)

> +               part_list[strlen(part_list) - 1] = 0;
> +       debug("setenv gpt_partition_list %s\n", part_list);
> +       env_set("gpt_partition_list", part_list);

Please check return value


blank line here

> +       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.
> + */
> +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];

put pinfo and buf inside loop?

> +
> +       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) {
> +                               /* 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);
> +                               env_set("gpt_partition_entry", buf);
> +                               sprintf(buf, "%s", pinfo.name);
> +                               debug("setenv gpt_partition_name %s\n", buf);
> +                               env_set("gpt_partition_name", buf);

Need to check return values of env_set()

> +                               return 0;
> +                       }
> +               }
> +       }
> +       return -1;

That is -EPERM. Perhaps -ENOENT?

> +}
> +
>  static int do_disk_guid(struct blk_desc *dev_desc, char * const namestr)
>  {
>         int ret;
> @@ -822,6 +903,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) {
>                 ret = do_disk_guid(blk_dev_desc, argv[4]);
>  #ifdef CONFIG_CMD_GPT_RENAME
> @@ -852,7 +937,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"
> --
> 2.17.1
>

It would be good to have a test for this, but I don't think anything
exists at present.

+Heinrich Schuchardt might know

Regards,
SImon
Heinrich Schuchardt May 5, 2020, 4:45 p.m. UTC | #4
On 05.05.20 18:02, Simon Glass wrote:
> Hi Rayagonda,
>
> On Tue, 5 May 2020 at 06:13, Rayagonda Kokatanur
> <rayagonda.kokatanur@broadcom.com> 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>
>>
>> diff --git a/cmd/gpt.c b/cmd/gpt.c
>> index b8d11c167d..c32e272b25 100644
>> --- a/cmd/gpt.c
>> +++ b/cmd/gpt.c
>> @@ -616,6 +616,87 @@ static int gpt_verify(struct blk_desc *blk_dev_desc, const char *str_part)
>>         return ret;
>>  }
>>
>> +/*
>> + * Enumerate partition names into environment variable.
>> + */
>
> Can you check the style for these comments? You can see examples in
> bootcount.h. Please fix for all functions.


Unfortunately bootcount.h itself is in bad shape. Please, use

https://www.kernel.org/doc/html/v4.10/doc-guide/kernel-doc.html#function-documentation

as reference.

Best regards

Heinrich

>
> It should mention the params and return value.
>
> Also in this case it should explain which environment variable is
> updated and the format that is used in the 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);
>
> Can you create functions in part.h to handle this? Something like:
>
> int part_driver_get_count()
> struct part_driver *part_driver_get((int n);
>
> Then we don't have lots of places accessing this.
>
>> +       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) {
>
> if (ret)
>
>> +                               /* no more entries in table */
>> +                               break;
>> +                       }
>> +                       strcat(part_list, (const char *)pinfo.name);
>> +                       strcat(part_list, " ");
>
> Need to check that you don't go past sizeof(part_list). Can I suggest
> that you have a ptr to the end of your string to avoid n^2 behaviour
> here?
>
>> +               }
>> +       }
>> +       if (strlen(part_list) > 0)
>
> if (*part_list)
>
>> +               part_list[strlen(part_list) - 1] = 0;
>> +       debug("setenv gpt_partition_list %s\n", part_list);
>> +       env_set("gpt_partition_list", part_list);
>
> Please check return value
>
>
> blank line here
>
>> +       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.
>> + */
>> +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];
>
> put pinfo and buf inside loop?
>
>> +
>> +       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) {
>> +                               /* 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);
>> +                               env_set("gpt_partition_entry", buf);
>> +                               sprintf(buf, "%s", pinfo.name);
>> +                               debug("setenv gpt_partition_name %s\n", buf);
>> +                               env_set("gpt_partition_name", buf);
>
> Need to check return values of env_set()
>
>> +                               return 0;
>> +                       }
>> +               }
>> +       }
>> +       return -1;
>
> That is -EPERM. Perhaps -ENOENT?
>
>> +}
>> +
>>  static int do_disk_guid(struct blk_desc *dev_desc, char * const namestr)
>>  {
>>         int ret;
>> @@ -822,6 +903,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) {
>>                 ret = do_disk_guid(blk_dev_desc, argv[4]);
>>  #ifdef CONFIG_CMD_GPT_RENAME
>> @@ -852,7 +937,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"
>> --
>> 2.17.1
>>
>
> It would be good to have a test for this, but I don't think anything
> exists at present.
>
> +Heinrich Schuchardt might know
>
> Regards,
> SImon
>
Simon Glass May 6, 2020, 2:47 p.m. UTC | #5
Hi Heinrich,

On Tue, 5 May 2020 at 10:45, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 05.05.20 18:02, Simon Glass wrote:
> > Hi Rayagonda,
> >
> > On Tue, 5 May 2020 at 06:13, Rayagonda Kokatanur
> > <rayagonda.kokatanur@broadcom.com> 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>
> >>
> >> diff --git a/cmd/gpt.c b/cmd/gpt.c
> >> index b8d11c167d..c32e272b25 100644
> >> --- a/cmd/gpt.c
> >> +++ b/cmd/gpt.c
> >> @@ -616,6 +616,87 @@ static int gpt_verify(struct blk_desc *blk_dev_desc, const char *str_part)
> >>         return ret;
> >>  }
> >>
> >> +/*
> >> + * Enumerate partition names into environment variable.
> >> + */
> >
> > Can you check the style for these comments? You can see examples in
> > bootcount.h. Please fix for all functions.
>
>
> Unfortunately bootcount.h itself is in bad shape. Please, use
>
> https://www.kernel.org/doc/html/v4.10/doc-guide/kernel-doc.html#function-documentation
>
> as reference.

Thank you. I've added that to the wiki page.

https://www.denx.de/wiki/U-Boot/CodingStyle

Regards,
Simon
[..]
diff mbox series

Patch

diff --git a/cmd/gpt.c b/cmd/gpt.c
index b8d11c167d..c32e272b25 100644
--- a/cmd/gpt.c
+++ b/cmd/gpt.c
@@ -616,6 +616,87 @@  static int gpt_verify(struct blk_desc *blk_dev_desc, const char *str_part)
 	return ret;
 }
 
+/*
+ * 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.
+ */
+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) {
+				/* 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);
+				env_set("gpt_partition_entry", buf);
+				sprintf(buf, "%s", pinfo.name);
+				debug("setenv gpt_partition_name %s\n", buf);
+				env_set("gpt_partition_name", buf);
+				return 0;
+			}
+		}
+	}
+	return -1;
+}
+
 static int do_disk_guid(struct blk_desc *dev_desc, char * const namestr)
 {
 	int ret;
@@ -822,6 +903,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) {
 		ret = do_disk_guid(blk_dev_desc, argv[4]);
 #ifdef CONFIG_CMD_GPT_RENAME
@@ -852,7 +937,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"