Hi Vladimir,
On Mon, 18 Nov 2019 at 16:27, Vladimir Olovyannikov
<vladimir.olovyanni...@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.velupil...@broadcom.com>
> Signed-off-by: Vladimir Olovyannikov <vladimir.olovyanni...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
