| Message ID | 20191119002630.15088-1-vladimir.olovyannikov@broadcom.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot,v1,1/1] cmd: adding malloc, math, and strcmp commands to u-boot | expand |
Hi Vladimir, On Mon, 18 Nov 2019 at 16:27, Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com> wrote: > > cmd: adding malloc, math, and strcmp u-boot commands. U-Boot > - malloc supports allocation of heap memory and free allocated memory > via u-boot command line. U-Boot (please fix globally) > - math provides math commands such as "add", "sub", "mul", "div", > "shift", ability to convert dec->hex. > - strcmp provides string compare command feature for a script. > > All these commands are introduced to be used in u-boot scripts. > > Signed-off-by: Suji Velupiallai <suji.velupillai@broadcom.com> > Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com> > --- > cmd/Kconfig | 21 ++++++++++++++ > cmd/Makefile | 4 +++ > cmd/malloc.c | 54 ++++++++++++++++++++++++++++++++++++ > cmd/math.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > cmd/strcmp.c | 28 +++++++++++++++++++ > 5 files changed, 185 insertions(+) > create mode 100644 cmd/malloc.c > create mode 100644 cmd/math.c > create mode 100644 cmd/strcmp.c > > diff --git a/cmd/Kconfig b/cmd/Kconfig > index cf982ff65e..f11903fe3d 100644 > --- a/cmd/Kconfig > +++ b/cmd/Kconfig > @@ -1286,6 +1286,20 @@ config CMD_ITEST > help > Return true/false on integer compare. > > +config CMD_MALLOC > + bool "malloc" > + default y > + help > + Supports allocation of heap memory and free allocated memory commands. > + These commands are used by u-boot scripts. > + > +config CMD_MATH > + bool "math" > + default y > + help > + Provides math commands such as add, sub, mul, div, shift, > + convert decimal to hex functionalities to be available in the script. > + > config CMD_SOURCE > bool "source" > default y > @@ -1301,6 +1315,13 @@ config CMD_SETEXPR > Also supports loading the value at a memory location into a variable. > If CONFIG_REGEX is enabled, setexpr also supports a gsub function. I think this would be better as three patches. > > +config CMD_STRCMP > + bool "strcmp" > + default y > + help > + Provides string compare command feature to u-boot scripts. U-Boot > + > + > endmenu > > menu "Android support commands" > diff --git a/cmd/Makefile b/cmd/Makefile > index 2d723ea0f0..942d60a0a2 100644 > --- a/cmd/Makefile > +++ b/cmd/Makefile > @@ -164,6 +164,10 @@ obj-$(CONFIG_CMD_GPT) += gpt.o > obj-$(CONFIG_CMD_ETHSW) += ethsw.o > obj-$(CONFIG_CMD_AXI) += axi.o > > +obj-$(CONFIG_CMD_MALLOC) += malloc.o > +obj-$(CONFIG_CMD_MATH) += math.o > +obj-$(CONFIG_CMD_STRCMP) += strcmp.o > + > # Power > obj-$(CONFIG_CMD_PMIC) += pmic.o > obj-$(CONFIG_CMD_REGULATOR) += regulator.o > diff --git a/cmd/malloc.c b/cmd/malloc.c > new file mode 100644 > index 0000000000..e11e030a59 > --- /dev/null > +++ b/cmd/malloc.c > @@ -0,0 +1,54 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright 2019 Broadcom > + */ > + > +#include <common.h> > +#include <command.h> > +#include <environment.h> > +#include <errno.h> > +#include <malloc.h> > + > +static unsigned long get_value(const char *val) Needs a comment > +{ > + char *env = env_get((char *)val); > + > + if (env) > + return simple_strtoul(env, NULL, 16); > + else > + return simple_strtoul(val, NULL, 16); > +} > + > +static int do_malloc(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) > +{ > + char numberbuf[32]; > + void *mem; const? > + > + if (argc < 3) > + return cmd_usage(cmdtp); > + > + mem = memalign(ARCH_DMA_MINALIGN, get_value(argv[2])); > + if (mem) { > + sprintf(numberbuf, "%08x", (unsigned int)mem); I think (ulong) would be better > + env_set(argv[1], numberbuf); blank line before return (please fix globally) > + return 0; > + } > + return -EINVAL; You can't return errors from command functions. Try printing an error and return CMD_RET_FAILURE instead. > +} > + > +static int do_free(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) > +{ > + if (argc < 2) > + return cmd_usage(cmdtp); > + > + free((void *)get_value(argv[1])); > + env_set(argv[1], ""); > + return 0; > +} > + > +U_BOOT_CMD(malloc, 3, 0, do_malloc, > + "Allocate memory from u-boot heap and store pointer in environment variable.", > + "target size\n"); Needs better help - e.g. list arguments > + > +U_BOOT_CMD(free, 2, 0, do_free, > + "Release memory from u-boot heap at target.", "target\n"); I wonder if it would be better to have a 'mem' command, then have 'mem alloc', 'mem free', etc.? > diff --git a/cmd/math.c b/cmd/math.c > new file mode 100644 > index 0000000000..17de5ef70b > --- /dev/null > +++ b/cmd/math.c > @@ -0,0 +1,78 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright 2010-2017 Broadcom > + */ > + > +#include <common.h> > +#include <command.h> > +#include <environment.h> > + > +unsigned long long simple_strtoull(const char *cp, char **endp, > + unsigned int base); Use #include <vsprintf.h> instead > + > +static unsigned long long get_value(const char *val) Function comment > +{ > + char *env = env_get((char *)val); > + > + if (env) > + return simple_strtoull(env, NULL, 16); > + else > + return simple_strtoull(val, NULL, 16); > +} > + > +static unsigned long long get_value_base10(const char *val) > +{ > + char *env = env_get((char *)val); > + > + if (env) > + return simple_strtoull(env, NULL, 10); > + else > + return simple_strtoull(val, NULL, 10); > +} > + > +/* > + * Top level addenv command > + */ > +#define IS_MATH_CMD(cmd, args) ((strcmp(argv[1], cmd) == 0) && \ > + (argc == (args))) Use a function instead of a macro. > + > +static int do_math(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > +{ > + char numberbuf[32]; > + unsigned long long i = 0; > + > + if (IS_MATH_CMD("add", 5)) > + i = get_value(argv[3]) + get_value(argv[4]); It might be better to do something like: const char *left = NULL, *right = NULL; if (argc > 3) left = argv[3]; ... Then you can use 'left' and 'right' below. > + else if (IS_MATH_CMD("sub", 5)) > + i = get_value(argv[3]) - get_value(argv[4]); > + else if (IS_MATH_CMD("mul", 5)) > + i = get_value(argv[3]) * get_value(argv[4]); > + else if (IS_MATH_CMD("div", 5)) > + i = get_value(argv[3]) / get_value(argv[4]); > + else if (IS_MATH_CMD("shl", 5)) > + i = get_value(argv[3]) << get_value(argv[4]); > + else if (IS_MATH_CMD("d2h", 4)) > + i = get_value_base10(argv[3]); > + else > + return cmd_usage(cmdtp); > + > + sprintf(numberbuf, "%llx", i); > + env_set(argv[2], numberbuf); > + return 0; > +} > + > +U_BOOT_CMD(math, 5, 0, do_math, > + "Environment variable math.", > + "add a b c\n" > + " - add b to c and store in a.\n" But also b and c can be numbers, right? You should mention that. > + "math sub a b c\n" > + " - subtract b from c and store in a.\n" > + "math mul a b c\n" > + " - multiply b by c and store in a.\n" > + "math div a b c\n" > + " - divide b by c and store in a.\n" > + "math shl a b c\n" > + " - shift left b by c and store in a.\n" > + "math d2h a b\n" > + " - Convert b from decimal to hex and store in a.\n" > +); > diff --git a/cmd/strcmp.c b/cmd/strcmp.c > new file mode 100644 > index 0000000000..3abd270d40 > --- /dev/null > +++ b/cmd/strcmp.c > @@ -0,0 +1,28 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright 2010-2017 Broadcom > + */ > + > +#include <common.h> > +#include <command.h> > + > +static int do_strcmp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > +{ > + if (argc != 3) > + return cmd_usage(cmdtp); > + > + if (strlen(argv[1]) != strlen(argv[2])) > + return CMD_RET_FAILURE; > + > + /* Compare the temporary string to the given parameter */ > + if (!strncmp(argv[1], argv[2], strlen(argv[2]))) > + return CMD_RET_SUCCESS; > + > + return CMD_RET_FAILURE; > +} > + > +U_BOOT_CMD(strcmp, 3, 0, do_strcmp, > + "String to string comparison.", > + "[str] [str]\n" str1 str2, I think > + " - Returns true if str are same, else returns false.\n" > +); > -- > 2.17.1 > Regards, Simon
Hi Simon, Thank you for reviewing. I will split the patch as you suggested, and resubmit. Vladimir > -----Original Message----- > From: Simon Glass [mailto:sjg@chromium.org] > Sent: Monday, November 18, 2019 5:21 PM > To: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Heinrich Schuchardt > <xypron.glpk@gmx.de>; Suji Velupiallai <suji.velupillai@broadcom.com> > Subject: Re: [PATCH v1 1/1] cmd: adding malloc, math, and strcmp commands > to u-boot > > Hi Vladimir, > > On Mon, 18 Nov 2019 at 16:27, Vladimir Olovyannikov > <vladimir.olovyannikov@broadcom.com> wrote: > > > > cmd: adding malloc, math, and strcmp u-boot commands. > > U-Boot > > > - malloc supports allocation of heap memory and free allocated memory > > via u-boot command line. > > U-Boot (please fix globally) > > > - math provides math commands such as "add", "sub", "mul", "div", > > "shift", ability to convert dec->hex. > > - strcmp provides string compare command feature for a script. > > > > All these commands are introduced to be used in u-boot scripts. > > > > Signed-off-by: Suji Velupiallai <suji.velupillai@broadcom.com> > > Signed-off-by: Vladimir Olovyannikov > > <vladimir.olovyannikov@broadcom.com> > > --- > > cmd/Kconfig | 21 ++++++++++++++ > > cmd/Makefile | 4 +++ > > cmd/malloc.c | 54 ++++++++++++++++++++++++++++++++++++ > > cmd/math.c | 78 > ++++++++++++++++++++++++++++++++++++++++++++++++++++ > > cmd/strcmp.c | 28 +++++++++++++++++++ > > 5 files changed, 185 insertions(+) > > create mode 100644 cmd/malloc.c > > create mode 100644 cmd/math.c > > create mode 100644 cmd/strcmp.c > > > > diff --git a/cmd/Kconfig b/cmd/Kconfig index cf982ff65e..f11903fe3d > > 100644 > > --- a/cmd/Kconfig > > +++ b/cmd/Kconfig > > @@ -1286,6 +1286,20 @@ config CMD_ITEST > > help > > Return true/false on integer compare. > > > > +config CMD_MALLOC > > + bool "malloc" > > + default y > > + help > > + Supports allocation of heap memory and free allocated memory > commands. > > + These commands are used by u-boot scripts. > > + > > +config CMD_MATH > > + bool "math" > > + default y > > + help > > + Provides math commands such as add, sub, mul, div, shift, > > + convert decimal to hex functionalities to be available in the > > script. > > + > > config CMD_SOURCE > > bool "source" > > default y > > @@ -1301,6 +1315,13 @@ config CMD_SETEXPR > > Also supports loading the value at a memory location into a > > variable. > > If CONFIG_REGEX is enabled, setexpr also supports a gsub > > function. > > I think this would be better as three patches. > > > > > +config CMD_STRCMP > > + bool "strcmp" > > + default y > > + help > > + Provides string compare command feature to u-boot scripts. > > U-Boot > > > + > > + > > endmenu > > > > menu "Android support commands" > > diff --git a/cmd/Makefile b/cmd/Makefile index 2d723ea0f0..942d60a0a2 > > 100644 > > --- a/cmd/Makefile > > +++ b/cmd/Makefile > > @@ -164,6 +164,10 @@ obj-$(CONFIG_CMD_GPT) += gpt.o > > obj-$(CONFIG_CMD_ETHSW) += ethsw.o > > obj-$(CONFIG_CMD_AXI) += axi.o > > > > +obj-$(CONFIG_CMD_MALLOC) += malloc.o > > +obj-$(CONFIG_CMD_MATH) += math.o > > +obj-$(CONFIG_CMD_STRCMP) += strcmp.o > > + > > # Power > > obj-$(CONFIG_CMD_PMIC) += pmic.o > > obj-$(CONFIG_CMD_REGULATOR) += regulator.o diff --git a/cmd/malloc.c > > b/cmd/malloc.c new file mode 100644 index 0000000000..e11e030a59 > > --- /dev/null > > +++ b/cmd/malloc.c > > @@ -0,0 +1,54 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright 2019 Broadcom > > + */ > > + > > +#include <common.h> > > +#include <command.h> > > +#include <environment.h> > > +#include <errno.h> > > +#include <malloc.h> > > + > > +static unsigned long get_value(const char *val) > > Needs a comment > > > +{ > > + char *env = env_get((char *)val); > > + > > + if (env) > > + return simple_strtoul(env, NULL, 16); > > + else > > + return simple_strtoul(val, NULL, 16); } > > + > > +static int do_malloc(cmd_tbl_t *cmdtp, int flag, int argc, char > > +*const argv[]) { > > + char numberbuf[32]; > > + void *mem; > > const? > > > + > > + if (argc < 3) > > + return cmd_usage(cmdtp); > > + > > + mem = memalign(ARCH_DMA_MINALIGN, get_value(argv[2])); > > + if (mem) { > > + sprintf(numberbuf, "%08x", (unsigned int)mem); > > I think (ulong) would be better > > > + env_set(argv[1], numberbuf); > > blank line before return (please fix globally) > > > + return 0; > > + } > > + return -EINVAL; > > You can't return errors from command functions. Try printing an error and > return CMD_RET_FAILURE instead. > > > +} > > + > > +static int do_free(cmd_tbl_t *cmdtp, int flag, int argc, char *const > > +argv[]) { > > + if (argc < 2) > > + return cmd_usage(cmdtp); > > + > > + free((void *)get_value(argv[1])); > > + env_set(argv[1], ""); > > + return 0; > > +} > > + > > +U_BOOT_CMD(malloc, 3, 0, do_malloc, > > + "Allocate memory from u-boot heap and store pointer in > environment variable.", > > + "target size\n"); > > Needs better help - e.g. list arguments > > > + > > +U_BOOT_CMD(free, 2, 0, do_free, > > + "Release memory from u-boot heap at target.", "target\n"); > > I wonder if it would be better to have a 'mem' command, then have 'mem > alloc', 'mem free', etc.? > > > diff --git a/cmd/math.c b/cmd/math.c > > new file mode 100644 > > index 0000000000..17de5ef70b > > --- /dev/null > > +++ b/cmd/math.c > > @@ -0,0 +1,78 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright 2010-2017 Broadcom > > + */ > > + > > +#include <common.h> > > +#include <command.h> > > +#include <environment.h> > > + > > +unsigned long long simple_strtoull(const char *cp, char **endp, > > + unsigned int base); > > Use #include <vsprintf.h> instead > > > + > > +static unsigned long long get_value(const char *val) > > Function comment > > > +{ > > + char *env = env_get((char *)val); > > + > > + if (env) > > + return simple_strtoull(env, NULL, 16); > > + else > > + return simple_strtoull(val, NULL, 16); } > > + > > +static unsigned long long get_value_base10(const char *val) { > > + char *env = env_get((char *)val); > > + > > + if (env) > > + return simple_strtoull(env, NULL, 10); > > + else > > + return simple_strtoull(val, NULL, 10); } > > + > > +/* > > + * Top level addenv command > > + */ > > +#define IS_MATH_CMD(cmd, args) ((strcmp(argv[1], cmd) == 0) && \ > > + (argc == (args))) > > Use a function instead of a macro. > > > + > > +static int do_math(cmd_tbl_t *cmdtp, int flag, int argc, char * const > > +argv[]) { > > + char numberbuf[32]; > > + unsigned long long i = 0; > > + > > + if (IS_MATH_CMD("add", 5)) > > + i = get_value(argv[3]) + get_value(argv[4]); > > It might be better to do something like: > > const char *left = NULL, *right = NULL; > > if (argc > 3) > left = argv[3]; > ... > > Then you can use 'left' and 'right' below. > > > + else if (IS_MATH_CMD("sub", 5)) > > + i = get_value(argv[3]) - get_value(argv[4]); > > + else if (IS_MATH_CMD("mul", 5)) > > + i = get_value(argv[3]) * get_value(argv[4]); > > + else if (IS_MATH_CMD("div", 5)) > > + i = get_value(argv[3]) / get_value(argv[4]); > > + else if (IS_MATH_CMD("shl", 5)) > > + i = get_value(argv[3]) << get_value(argv[4]); > > + else if (IS_MATH_CMD("d2h", 4)) > > + i = get_value_base10(argv[3]); > > + else > > + return cmd_usage(cmdtp); > > + > > + sprintf(numberbuf, "%llx", i); > > + env_set(argv[2], numberbuf); > > + return 0; > > +} > > + > > +U_BOOT_CMD(math, 5, 0, do_math, > > + "Environment variable math.", > > + "add a b c\n" > > + " - add b to c and store in a.\n" > > But also b and c can be numbers, right? You should mention that. > > > + "math sub a b c\n" > > + " - subtract b from c and store in a.\n" > > + "math mul a b c\n" > > + " - multiply b by c and store in a.\n" > > + "math div a b c\n" > > + " - divide b by c and store in a.\n" > > + "math shl a b c\n" > > + " - shift left b by c and store in a.\n" > > + "math d2h a b\n" > > + " - Convert b from decimal to hex and store in a.\n" > > +); > > diff --git a/cmd/strcmp.c b/cmd/strcmp.c new file mode 100644 index > > 0000000000..3abd270d40 > > --- /dev/null > > +++ b/cmd/strcmp.c > > @@ -0,0 +1,28 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright 2010-2017 Broadcom > > + */ > > + > > +#include <common.h> > > +#include <command.h> > > + > > +static int do_strcmp(cmd_tbl_t *cmdtp, int flag, int argc, char * > > +const argv[]) { > > + if (argc != 3) > > + return cmd_usage(cmdtp); > > + > > + if (strlen(argv[1]) != strlen(argv[2])) > > + return CMD_RET_FAILURE; > > + > > + /* Compare the temporary string to the given parameter */ > > + if (!strncmp(argv[1], argv[2], strlen(argv[2]))) > > + return CMD_RET_SUCCESS; > > + > > + return CMD_RET_FAILURE; > > +} > > + > > +U_BOOT_CMD(strcmp, 3, 0, do_strcmp, > > + "String to string comparison.", > > + "[str] [str]\n" > > str1 str2, I think > > > + " - Returns true if str are same, else returns false.\n" > > +); > > -- > > 2.17.1 > > > > Regards, > Simon
On Mon, Nov 18, 2019 at 04:26:30PM -0800, Vladimir Olovyannikov wrote: > cmd: adding malloc, math, and strcmp u-boot commands. > - malloc supports allocation of heap memory and free allocated memory > via u-boot command line. Can you expand on how this is used in a script? I'm not sure I see that exactly. Also: > +config CMD_MALLOC > + bool "malloc" > + default y > + help > + Supports allocation of heap memory and free allocated memory commands. > + These commands are used by u-boot scripts. > + > +config CMD_MATH > + bool "math" > + default y > + help > + Provides math commands such as add, sub, mul, div, shift, > + convert decimal to hex functionalities to be available in the script. First, why do we need this, rather than using setexpr ? > + > config CMD_SOURCE > bool "source" > default y > @@ -1301,6 +1315,13 @@ config CMD_SETEXPR > Also supports loading the value at a memory location into a variable. > If CONFIG_REGEX is enabled, setexpr also supports a gsub function. > > +config CMD_STRCMP > + bool "strcmp" > + default y > + help > + Provides string compare command feature to u-boot scripts. Second, new commands must not default to y, but they should be enabled on sandbox and new test.py tests added for them. Thanks!
Hi Tom, > -----Original Message----- > From: Tom Rini [mailto:trini@konsulko.com] > Sent: Tuesday, November 19, 2019 12:38 PM > To: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com> > Cc: u-boot@lists.denx.de; Suji Velupiallai <suji.velupillai@broadcom.com>; > Heinrich Schuchardt <xypron.glpk@gmx.de> > Subject: Re: [U-Boot] [PATCH v1 1/1] cmd: adding malloc, math, and strcmp > commands to u-boot > > On Mon, Nov 18, 2019 at 04:26:30PM -0800, Vladimir Olovyannikov wrote: > > > cmd: adding malloc, math, and strcmp u-boot commands. > > - malloc supports allocation of heap memory and free allocated memory > > via u-boot command line. > > Can you expand on how this is used in a script? I'm not sure I see that > exactly. I am upstreaming the new bcm platform. Here is an excerpt from the script which uses these: #define SD_UPDATE \ "sd_update="\ "if malloc tmp 2000; then "\ "else "\ "echo [sd_update] malloc 2000 bytes ** FAILED **;"\ "exit;"\ "fi;"\ "if fatload mmc ${sd_device_number} ${tmp} "\ "${sd_update_prefix}.sd-update; then "\ "else "\ "echo [sd_update] fatload ${sd_update_prefix}.sd-update "\ "** FAILED **;"\ "exit;"\ "fi;"\ "if source ${tmp}; then "\ "else "\ "echo [sd_update] Executing script ** FAILED **;"\ "exit;"\ "fi;"\ "if free tmp; then "\ "else " \ "echo [sd_update] free 2000 bytes ** FAILED **;"\ "exit;"\ "fi \0" ; "if math add filesize filesize 1FF; then "\ "else "\ "echo [mmc_flash_image_rsa] math add command ** FAILED **;"\ "exit;"\ "fi;"\ "if math div fileblocks filesize 200; then "\ "else "\ "echo [mmc_flash_image_rsa] math div command ** FAILED **;"\ "exit;"\ "fi;"\ > Also: > > +config CMD_MALLOC > > + bool "malloc" > > + default y > > + help > > + Supports allocation of heap memory and free allocated memory > commands. > > + These commands are used by u-boot scripts. > > + > > +config CMD_MATH > > + bool "math" > > + default y > > + help > > + Provides math commands such as add, sub, mul, div, shift, > > + convert decimal to hex functionalities to be available in the script. > > First, why do we need this, rather than using setexpr ? I agree, the platform needs to use setexpr as it contains all math operations. > > > + > > config CMD_SOURCE > > bool "source" > > default y > > @@ -1301,6 +1315,13 @@ config CMD_SETEXPR > > Also supports loading the value at a memory location into a variable. > > If CONFIG_REGEX is enabled, setexpr also supports a gsub function. > > > > +config CMD_STRCMP > > + bool "strcmp" > > + default y > > + help > > + Provides string compare command feature to u-boot scripts. > > Second, new commands must not default to y, but they should be enabled > on sandbox and new test.py tests added for them. Thanks! OK, thank you. Is there an example/doc on how to enable these on sandbox and to provide tests? Vladimir > > -- > Tom
On Wed, Nov 20, 2019 at 02:50:53PM -0800, Vladimir Olovyannikov wrote: > Hi Tom, > > > -----Original Message----- > > From: Tom Rini [mailto:trini@konsulko.com] > > Sent: Tuesday, November 19, 2019 12:38 PM > > To: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com> > > Cc: u-boot@lists.denx.de; Suji Velupiallai > <suji.velupillai@broadcom.com>; > > Heinrich Schuchardt <xypron.glpk@gmx.de> > > Subject: Re: [U-Boot] [PATCH v1 1/1] cmd: adding malloc, math, and > strcmp > > commands to u-boot > > > > On Mon, Nov 18, 2019 at 04:26:30PM -0800, Vladimir Olovyannikov wrote: > > > > > cmd: adding malloc, math, and strcmp u-boot commands. > > > - malloc supports allocation of heap memory and free allocated memory > > > via u-boot command line. > > > > Can you expand on how this is used in a script? I'm not sure I see that > > exactly. > I am upstreaming the new bcm platform. Here is an excerpt from the script > which uses > these: > #define SD_UPDATE \ > "sd_update="\ > "if malloc tmp 2000; then "\ > "else "\ > "echo [sd_update] malloc 2000 bytes ** FAILED **;"\ > "exit;"\ > "fi;"\ > "if fatload mmc ${sd_device_number} ${tmp} "\ > "${sd_update_prefix}.sd-update; then "\ > "else "\ > "echo [sd_update] fatload ${sd_update_prefix}.sd-update "\ > "** FAILED **;"\ > "exit;"\ > "fi;"\ > "if source ${tmp}; then "\ > "else "\ > "echo [sd_update] Executing script ** FAILED **;"\ > "exit;"\ > "fi;"\ > "if free tmp; then "\ > "else " \ > "echo [sd_update] free 2000 bytes ** FAILED **;"\ > "exit;"\ > "fi \0" > ; > "if math add filesize filesize 1FF; then "\ > "else "\ > "echo [mmc_flash_image_rsa] math add command ** FAILED > **;"\ > "exit;"\ > "fi;"\ > "if math div fileblocks filesize 200; then "\ > "else "\ > "echo [mmc_flash_image_rsa] math div command ** FAILED > **;"\ > "exit;"\ > "fi;"\ Ah, interesting. So you're malloc'ing the memory location that you load in to rather than the usual method of using known variables and locations for the SoC. I would discourage this as we generally do not have a large malloc pool available. > > Also: > > > +config CMD_MALLOC > > > + bool "malloc" > > > + default y > > > + help > > > + Supports allocation of heap memory and free allocated memory > > commands. > > > + These commands are used by u-boot scripts. > > > + > > > +config CMD_MATH > > > + bool "math" > > > + default y > > > + help > > > + Provides math commands such as add, sub, mul, div, shift, > > > + convert decimal to hex functionalities to be available in the > script. > > > > First, why do we need this, rather than using setexpr ? > I agree, the platform needs to use setexpr as it contains all math > operations. > > > > > + > > > config CMD_SOURCE > > > bool "source" > > > default y > > > @@ -1301,6 +1315,13 @@ config CMD_SETEXPR > > > Also supports loading the value at a memory location into a > variable. > > > If CONFIG_REGEX is enabled, setexpr also supports a gsub > function. > > > > > > +config CMD_STRCMP > > > + bool "strcmp" > > > + default y > > > + help > > > + Provides string compare command feature to u-boot scripts. > > > > Second, new commands must not default to y, but they should be enabled > > on sandbox and new test.py tests added for them. Thanks! > OK, thank you. > Is there an example/doc on how to enable these on sandbox and to provide > tests? Unfortunately there is not (but would be greatly appreciated). There are a lot of examples under test/py/ however. Thanks!
diff --git a/cmd/Kconfig b/cmd/Kconfig index cf982ff65e..f11903fe3d 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1286,6 +1286,20 @@ config CMD_ITEST help Return true/false on integer compare. +config CMD_MALLOC + bool "malloc" + default y + help + Supports allocation of heap memory and free allocated memory commands. + These commands are used by u-boot scripts. + +config CMD_MATH + bool "math" + default y + help + Provides math commands such as add, sub, mul, div, shift, + convert decimal to hex functionalities to be available in the script. + config CMD_SOURCE bool "source" default y @@ -1301,6 +1315,13 @@ config CMD_SETEXPR Also supports loading the value at a memory location into a variable. If CONFIG_REGEX is enabled, setexpr also supports a gsub function. +config CMD_STRCMP + bool "strcmp" + default y + help + Provides string compare command feature to u-boot scripts. + + endmenu menu "Android support commands" diff --git a/cmd/Makefile b/cmd/Makefile index 2d723ea0f0..942d60a0a2 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -164,6 +164,10 @@ obj-$(CONFIG_CMD_GPT) += gpt.o obj-$(CONFIG_CMD_ETHSW) += ethsw.o obj-$(CONFIG_CMD_AXI) += axi.o +obj-$(CONFIG_CMD_MALLOC) += malloc.o +obj-$(CONFIG_CMD_MATH) += math.o +obj-$(CONFIG_CMD_STRCMP) += strcmp.o + # Power obj-$(CONFIG_CMD_PMIC) += pmic.o obj-$(CONFIG_CMD_REGULATOR) += regulator.o diff --git a/cmd/malloc.c b/cmd/malloc.c new file mode 100644 index 0000000000..e11e030a59 --- /dev/null +++ b/cmd/malloc.c @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2019 Broadcom + */ + +#include <common.h> +#include <command.h> +#include <environment.h> +#include <errno.h> +#include <malloc.h> + +static unsigned long get_value(const char *val) +{ + char *env = env_get((char *)val); + + if (env) + return simple_strtoul(env, NULL, 16); + else + return simple_strtoul(val, NULL, 16); +} + +static int do_malloc(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) +{ + char numberbuf[32]; + void *mem; + + if (argc < 3) + return cmd_usage(cmdtp); + + mem = memalign(ARCH_DMA_MINALIGN, get_value(argv[2])); + if (mem) { + sprintf(numberbuf, "%08x", (unsigned int)mem); + env_set(argv[1], numberbuf); + return 0; + } + return -EINVAL; +} + +static int do_free(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) +{ + if (argc < 2) + return cmd_usage(cmdtp); + + free((void *)get_value(argv[1])); + env_set(argv[1], ""); + return 0; +} + +U_BOOT_CMD(malloc, 3, 0, do_malloc, + "Allocate memory from u-boot heap and store pointer in environment variable.", + "target size\n"); + +U_BOOT_CMD(free, 2, 0, do_free, + "Release memory from u-boot heap at target.", "target\n"); diff --git a/cmd/math.c b/cmd/math.c new file mode 100644 index 0000000000..17de5ef70b --- /dev/null +++ b/cmd/math.c @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2010-2017 Broadcom + */ + +#include <common.h> +#include <command.h> +#include <environment.h> + +unsigned long long simple_strtoull(const char *cp, char **endp, + unsigned int base); + +static unsigned long long get_value(const char *val) +{ + char *env = env_get((char *)val); + + if (env) + return simple_strtoull(env, NULL, 16); + else + return simple_strtoull(val, NULL, 16); +} + +static unsigned long long get_value_base10(const char *val) +{ + char *env = env_get((char *)val); + + if (env) + return simple_strtoull(env, NULL, 10); + else + return simple_strtoull(val, NULL, 10); +} + +/* + * Top level addenv command + */ +#define IS_MATH_CMD(cmd, args) ((strcmp(argv[1], cmd) == 0) && \ + (argc == (args))) + +static int do_math(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + char numberbuf[32]; + unsigned long long i = 0; + + if (IS_MATH_CMD("add", 5)) + i = get_value(argv[3]) + get_value(argv[4]); + else if (IS_MATH_CMD("sub", 5)) + i = get_value(argv[3]) - get_value(argv[4]); + else if (IS_MATH_CMD("mul", 5)) + i = get_value(argv[3]) * get_value(argv[4]); + else if (IS_MATH_CMD("div", 5)) + i = get_value(argv[3]) / get_value(argv[4]); + else if (IS_MATH_CMD("shl", 5)) + i = get_value(argv[3]) << get_value(argv[4]); + else if (IS_MATH_CMD("d2h", 4)) + i = get_value_base10(argv[3]); + else + return cmd_usage(cmdtp); + + sprintf(numberbuf, "%llx", i); + env_set(argv[2], numberbuf); + return 0; +} + +U_BOOT_CMD(math, 5, 0, do_math, + "Environment variable math.", + "add a b c\n" + " - add b to c and store in a.\n" + "math sub a b c\n" + " - subtract b from c and store in a.\n" + "math mul a b c\n" + " - multiply b by c and store in a.\n" + "math div a b c\n" + " - divide b by c and store in a.\n" + "math shl a b c\n" + " - shift left b by c and store in a.\n" + "math d2h a b\n" + " - Convert b from decimal to hex and store in a.\n" +); diff --git a/cmd/strcmp.c b/cmd/strcmp.c new file mode 100644 index 0000000000..3abd270d40 --- /dev/null +++ b/cmd/strcmp.c @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2010-2017 Broadcom + */ + +#include <common.h> +#include <command.h> + +static int do_strcmp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + if (argc != 3) + return cmd_usage(cmdtp); + + if (strlen(argv[1]) != strlen(argv[2])) + return CMD_RET_FAILURE; + + /* Compare the temporary string to the given parameter */ + if (!strncmp(argv[1], argv[2], strlen(argv[2]))) + return CMD_RET_SUCCESS; + + return CMD_RET_FAILURE; +} + +U_BOOT_CMD(strcmp, 3, 0, do_strcmp, + "String to string comparison.", + "[str] [str]\n" + " - Returns true if str are same, else returns false.\n" +);
cmd: adding malloc, math, and strcmp u-boot commands. - malloc supports allocation of heap memory and free allocated memory via u-boot command line. - math provides math commands such as "add", "sub", "mul", "div", "shift", ability to convert dec->hex. - strcmp provides string compare command feature for a script. All these commands are introduced to be used in u-boot scripts. Signed-off-by: Suji Velupiallai <suji.velupillai@broadcom.com> Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com> --- cmd/Kconfig | 21 ++++++++++++++ cmd/Makefile | 4 +++ cmd/malloc.c | 54 ++++++++++++++++++++++++++++++++++++ cmd/math.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++ cmd/strcmp.c | 28 +++++++++++++++++++ 5 files changed, 185 insertions(+) create mode 100644 cmd/malloc.c create mode 100644 cmd/math.c create mode 100644 cmd/strcmp.c