[U-Boot,v1,1/1] cmd: adding malloc, math, and strcmp commands to u-boot
diff mbox series

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
Related show

Commit Message

Vladimir Olovyannikov Nov. 19, 2019, 12:26 a.m. UTC
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

Comments

Simon Glass Nov. 19, 2019, 1:21 a.m. UTC | #1
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
Vladimir Olovyannikov Nov. 19, 2019, 8:02 p.m. UTC | #2
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
Tom Rini Nov. 19, 2019, 8:37 p.m. UTC | #3
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!
Vladimir Olovyannikov Nov. 20, 2019, 10:50 p.m. UTC | #4
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
Tom Rini Nov. 20, 2019, 11:26 p.m. UTC | #5
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!

Patch
diff mbox series

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"
+);