* [PATCH v3 1/1] cmd: gpt: add eMMC and GPT support
@ 2020-05-13 15:27 Rayagonda Kokatanur
2020-05-14 23:57 ` Heinrich Schuchardt
2020-05-15 21:02 ` Simon Glass
0 siblings, 2 replies; 6+ messages in thread
From: Rayagonda Kokatanur @ 2020-05-13 15:27 UTC (permalink / raw)
To: u-boot
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) {
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 */
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 1/1] cmd: gpt: add eMMC and GPT support
2020-05-13 15:27 [PATCH v3 1/1] cmd: gpt: add eMMC and GPT support Rayagonda Kokatanur
@ 2020-05-14 23:57 ` Heinrich Schuchardt
2020-05-15 5:52 ` Rayagonda Kokatanur
2020-05-15 21:02 ` Simon Glass
1 sibling, 1 reply; 6+ messages in thread
From: Heinrich Schuchardt @ 2020-05-14 23:57 UTC (permalink / raw)
To: u-boot
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.
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 */
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/1] cmd: gpt: add eMMC and GPT support
2020-05-14 23:57 ` Heinrich Schuchardt
@ 2020-05-15 5:52 ` Rayagonda Kokatanur
2020-05-15 6:51 ` Heinrich Schuchardt
0 siblings, 1 reply; 6+ messages in thread
From: Rayagonda Kokatanur @ 2020-05-15 5:52 UTC (permalink / raw)
To: u-boot
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
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 */
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/1] cmd: gpt: add eMMC and GPT support
2020-05-15 5:52 ` Rayagonda Kokatanur
@ 2020-05-15 6:51 ` Heinrich Schuchardt
0 siblings, 0 replies; 6+ messages in thread
From: Heinrich Schuchardt @ 2020-05-15 6:51 UTC (permalink / raw)
To: u-boot
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 */
>>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/1] cmd: gpt: add eMMC and GPT support
2020-05-13 15:27 [PATCH v3 1/1] cmd: gpt: add eMMC and GPT support Rayagonda Kokatanur
2020-05-14 23:57 ` Heinrich Schuchardt
@ 2020-05-15 21:02 ` Simon Glass
2020-07-23 11:59 ` Rayagonda Kokatanur
1 sibling, 1 reply; 6+ messages in thread
From: Simon Glass @ 2020-05-15 21:02 UTC (permalink / raw)
To: u-boot
Hi Rayagonda,
On Wed, 13 May 2020 at 09:28, 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>
> ---
> 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(+)
This looks good, but can you add a test?
A few nits below
>
> 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
Should return a -ve error code. You return -ENOMEM for example.
> + */
> +static int gpt_enumerate(struct blk_desc *blk_dev_desc)
Please use 'desc' for arg as it is shorter
> +{
> + 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;
How about -ENOENT?
> + }
> +
> + 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;
+1 for space (below). Otherwise you can overfllow
> + if (str_len > sizeof(part_list)) {
> + printf("Error insufficient memory\n");
> + return -ENOMEM;
> + }
> + strncpy(ptr, (const char *)pinfo.name, tmp_len);
But you know ptr has enough space, so just use strcpy
> + /* One byte for space(" ") delimiter */
> + strncpy(&ptr[tmp_len], " ", 1);
ptr[tmp_len] = ' '
> + 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) {
!strcmp()
> + /* match found, setup environment variables */
> + ret = gpt_setenv_part_variables(&pinfo, i);
> + if (ret)
> + goto fail;
> +
> + return 0;
> + }
> + }
> + }
> +
> +fail:
> + return -ENOENT;
return ret
> +}
> +
> 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) {
> 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 */
> --
> 2.17.1
>
Regards,
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/1] cmd: gpt: add eMMC and GPT support
2020-05-15 21:02 ` Simon Glass
@ 2020-07-23 11:59 ` Rayagonda Kokatanur
0 siblings, 0 replies; 6+ messages in thread
From: Rayagonda Kokatanur @ 2020-07-23 11:59 UTC (permalink / raw)
To: u-boot
Hi Simon,
On Sat, May 16, 2020 at 2:33 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Rayagonda,
>
> On Wed, 13 May 2020 at 09:28, 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>
> > ---
> > 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(+)
>
> This looks good, but can you add a test?
I am finding it a little difficult in adding tests and running existing tests.
I will add a test late for this.
I have fixed all other review comments, requesting you to review patch v4.
Best regards,
Rayagonda
>
> A few nits below
>
> >
> > 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
>
> Should return a -ve error code. You return -ENOMEM for example.
>
> > + */
> > +static int gpt_enumerate(struct blk_desc *blk_dev_desc)
>
> Please use 'desc' for arg as it is shorter
>
> > +{
> > + 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;
>
> How about -ENOENT?
>
> > + }
> > +
> > + 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;
>
> +1 for space (below). Otherwise you can overfllow
>
> > + if (str_len > sizeof(part_list)) {
> > + printf("Error insufficient memory\n");
> > + return -ENOMEM;
> > + }
> > + strncpy(ptr, (const char *)pinfo.name, tmp_len);
>
> But you know ptr has enough space, so just use strcpy
>
> > + /* One byte for space(" ") delimiter */
> > + strncpy(&ptr[tmp_len], " ", 1);
>
> ptr[tmp_len] = ' '
>
> > + 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) {
>
> !strcmp()
>
> > + /* match found, setup environment variables */
> > + ret = gpt_setenv_part_variables(&pinfo, i);
> > + if (ret)
> > + goto fail;
> > +
> > + return 0;
> > + }
> > + }
> > + }
> > +
> > +fail:
> > + return -ENOENT;
>
> return ret
>
> > +}
> > +
> > 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) {
> > 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 */
> > --
> > 2.17.1
> >
>
> Regards,
> Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-23 11:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-13 15:27 [PATCH v3 1/1] cmd: gpt: add eMMC and GPT support Rayagonda Kokatanur
2020-05-14 23:57 ` Heinrich Schuchardt
2020-05-15 5:52 ` Rayagonda Kokatanur
2020-05-15 6:51 ` Heinrich Schuchardt
2020-05-15 21:02 ` Simon Glass
2020-07-23 11:59 ` Rayagonda Kokatanur
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.