[PATCH] cmd: gpt: add eMMC and GPT support
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"
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.
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
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
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-doc...
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
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-doc...
as reference.
Thank you. I've added that to the wiki page.
https://www.denx.de/wiki/U-Boot/CodingStyle
Regards, Simon [..]
participants (3)
-
Heinrich Schuchardt -
Rayagonda Kokatanur -
Simon Glass