From db3667413d99b8ec1b1e7e9b34c20a8883ec413d Mon Sep 17 00:00:00 2001 From: Anton Leontiev Date: Tue, 3 Sep 2019 10:52:24 +0300 Subject: [PATCH 01/23] cmd: pxe: Use internal FDT if retrieving from FDTDIR fails As FDTDIR label doesn't specify exact file to be loaded, it should not fail if no file exists in the directory. In this case try to boot with internal FDT if it exists. Signed-off-by: Anton Leontiev --- cmd/pxe_utils.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/cmd/pxe_utils.c b/cmd/pxe_utils.c index 8716e782f6..235522f4bb 100644 --- a/cmd/pxe_utils.c +++ b/cmd/pxe_utils.c @@ -451,11 +451,14 @@ static int label_boot(struct cmd_tbl *cmdtp, struct pxe_label *label) /* * fdt usage is optional: - * It handles the following scenarios. All scenarios are exclusive + * It handles the following scenarios. * - * Scenario 1: If fdt_addr_r specified and "fdt" label is defined in - * pxe file, retrieve fdt blob from server. Pass fdt_addr_r to bootm, - * and adjust argc appropriately. + * Scenario 1: If fdt_addr_r specified and "fdt" or "fdtdir" label is + * defined in pxe file, retrieve fdt blob from server. Pass fdt_addr_r to + * bootm, and adjust argc appropriately. + * + * If retrieve fails and no exact fdt blob is specified in pxe file with + * "fdt" label, try Scenario 2. * * Scenario 2: If there is an fdt_addr specified, pass it along to * bootm, and adjust argc appropriately. @@ -521,9 +524,13 @@ static int label_boot(struct cmd_tbl *cmdtp, struct pxe_label *label) free(fdtfilefree); if (err < 0) { - printf("Skipping %s for failure retrieving fdt\n", - label->name); - goto cleanup; + bootm_argv[3] = NULL; + + if (label->fdt) { + printf("Skipping %s for failure retrieving FDT\n", + label->name); + goto cleanup; + } } } else { bootm_argv[3] = NULL; From c8e251f82af97562741e51110ed8828a5c5744c6 Mon Sep 17 00:00:00 2001 From: Lyle Franklin Date: Mon, 5 Aug 2019 06:23:42 -0400 Subject: [PATCH 02/23] Adds basic support for ProxyDHCP - ProxyDHCP allows a second DHCP server to exist alongside your main DHCP server and supply additional BOOTP related options - When u-boot sends out a DHCP request, the real DHCP server will respond with a normal response containing the new client IP address while simultaneously the ProxyDHCP server will respond with a blank client IP address and a `bootfile` option - This patch adds CONFIG_SERVERIP_FROM_PROXYDHCP (default false) to enable this behavior and CONFIG_SERVERIP_FROM_PROXYDHCP_DELAY_MS (default 100) which tells u-boot to wait additional time after receiving the main DHCP response to give the ProxyDHCP response time to arrive - The PXE spec for ProxyDHCP is more complicated than the solution added here as diagramed on page 16: http://www.pix.net/software/pxeboot/archive/pxespec.pdf: ``` DHCP Discover will be retried four times. The four timeouts are 4, 8, 16 and 32 seconds respectively. If a DHCPOFFER is received without an Option timeouts in an attempt to receive a PXE response. ``` - Adding a simple delay worked for my purposes but let me know if a more robust solution is required Signed-off-by: Lyle Franklin --- net/Kconfig | 14 ++++++++++++++ net/bootp.c | 27 ++++++++++++++++++++++----- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/net/Kconfig b/net/Kconfig index 1b3e420d0d..c4b4dae064 100644 --- a/net/Kconfig +++ b/net/Kconfig @@ -74,4 +74,18 @@ config TFTP_WINDOWSIZE before an ack response is required. The default TFTP implementation implies a window size of 1. +config SERVERIP_FROM_PROXYDHCP + bool "Get serverip value from Proxy DHCP response" + help + Allows bootfile config to be fetched from Proxy DHCP server + while IP is obtained from main DHCP server. + +config SERVERIP_FROM_PROXYDHCP_DELAY_MS + int "# of additional milliseconds to wait for ProxyDHCP response" + default 100 + help + Amount of additional time to wait for ProxyDHCP response after + receiving response from main DHCP server. Has no effect if + SERVERIP_FROM_PROXYDHCP is false. + endif # if NET diff --git a/net/bootp.c b/net/bootp.c index de3dab4114..163af41e92 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -146,10 +146,7 @@ static int check_reply_packet(uchar *pkt, unsigned dest, unsigned src, return retval; } -/* - * Copy parameters of interest from BOOTP_REPLY/DHCP_OFFER packet - */ -static void store_net_params(struct bootp_hdr *bp) +static void store_bootp_params(struct bootp_hdr *bp) { #if !defined(CONFIG_BOOTP_SERVERIP) struct in_addr tmp_ip; @@ -182,6 +179,16 @@ static void store_net_params(struct bootp_hdr *bp) */ if (*net_boot_file_name) env_set("bootfile", net_boot_file_name); +#endif +} + +/* + * Copy parameters of interest from BOOTP_REPLY/DHCP_OFFER packet + */ +static void store_net_params(struct bootp_hdr *bp) +{ +#if !defined(CONFIG_SERVERIP_FROM_PROXYDHCP) + store_bootp_params(bp); #endif net_copy_ip(&net_ip, &bp->bp_yiaddr); } @@ -1055,8 +1062,12 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip, debug("DHCPHandler: got DHCP packet: (src=%d, dst=%d, len=%d) state: " "%d\n", src, dest, len, dhcp_state); - if (net_read_ip(&bp->bp_yiaddr).s_addr == 0) + if (net_read_ip(&bp->bp_yiaddr).s_addr == 0) { +#if defined(CONFIG_SERVERIP_FROM_PROXYDHCP) + store_bootp_params(bp); +#endif return; + } switch (dhcp_state) { case SELECTING: @@ -1075,6 +1086,12 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip, dhcp_packet_process_options(bp); efi_net_set_dhcp_ack(pkt, len); +#if defined(CONFIG_SERVERIP_FROM_PROXYDHCP) + if (!net_server_ip.s_addr) + udelay(CONFIG_SERVERIP_FROM_PROXYDHCP_DELAY_MS * + 1000); +#endif /* CONFIG_SERVERIP_FROM_PROXYDHCP */ + debug("TRANSITIONING TO REQUESTING STATE\n"); dhcp_state = REQUESTING; From 8558217153df685992573e067bd9f95dd4d2ef3a Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Wed, 9 Sep 2020 18:26:16 +0200 Subject: [PATCH 03/23] gpio: Convert to use APIs which support live DT Use ofnode_ or dev_ APIs instead of fdt_ and fdtdec_ APIs so that the driver can support live DT. Signed-off-by: Patrick Delaunay Reviewed-by: Simon Glass Reviewed-by: Heiko Schocher --- drivers/gpio/gpio-uclass.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 0c01413b58..4785b6b34a 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -1100,9 +1100,8 @@ int gpio_get_list_count(struct udevice *dev, const char *list_name) { int ret; - ret = fdtdec_parse_phandle_with_args(gd->fdt_blob, dev_of_offset(dev), - list_name, "#gpio-cells", 0, -1, - NULL); + ret = dev_read_phandle_with_args(dev, list_name, "#gpio-cells", 0, -1, + NULL); if (ret) { debug("%s: Node '%s', property '%s', GPIO count failed: %d\n", __func__, dev->name, list_name, ret); From 4532bf411e56ef32984e2e98cf1aab6755fd77e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 26 Oct 2020 14:10:49 +0100 Subject: [PATCH 04/23] Makefile: Correctly propagate failure when removing target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On more places is used pattern 'command > $@ || rm -f $@'. But it does not propagate failure from 'command' as 'rm -f' returns success. Fix it by calling 'false' to correctly propagate failure after 'rm -f'. Signed-off-by: Pali Rohár Reviewed-by: Simon Glass --- Makefile | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 41446d80cb..c94cc39e78 100644 --- a/Makefile +++ b/Makefile @@ -1005,7 +1005,7 @@ cmd_cat = cat $(filter-out $(PHONY), $^) > $@ append = cat $(filter-out $< $(PHONY), $^) >> $@ quiet_cmd_pad_cat = CAT $@ -cmd_pad_cat = $(cmd_objcopy) && $(append) || rm -f $@ +cmd_pad_cat = $(cmd_objcopy) && $(append) || { rm -f $@; false; } quiet_cmd_lzma = LZMA $@ cmd_lzma = lzma -c -z -k -9 $< > $@ @@ -1312,7 +1312,7 @@ endif shell_cmd = { $(call echo-cmd,$(1)) $(cmd_$(1)); } quiet_cmd_objcopy_uboot = OBJCOPY $@ -cmd_objcopy_uboot = $(cmd_objcopy) && $(call shell_cmd,static_rela,$<,$@,$(CONFIG_SYS_TEXT_BASE)) || rm -f $@ +cmd_objcopy_uboot = $(cmd_objcopy) && $(call shell_cmd,static_rela,$<,$@,$(CONFIG_SYS_TEXT_BASE)) || { rm -f $@; false; } u-boot-nodtb.bin: u-boot FORCE $(call if_changed,objcopy_uboot) @@ -1584,12 +1584,12 @@ u-boot.spr: spl/u-boot-spl.img u-boot.img FORCE ifneq ($(CONFIG_ARCH_SOCFPGA),) quiet_cmd_gensplx4 = GENSPLX4 $@ cmd_gensplx4 = cat spl/u-boot-spl.sfp spl/u-boot-spl.sfp \ - spl/u-boot-spl.sfp spl/u-boot-spl.sfp > $@ || rm -f $@ + spl/u-boot-spl.sfp spl/u-boot-spl.sfp > $@ || { rm -f $@; false; } spl/u-boot-splx4.sfp: spl/u-boot-spl.sfp FORCE $(call if_changed,gensplx4) quiet_cmd_socboot = SOCBOOT $@ -cmd_socboot = cat spl/u-boot-splx4.sfp u-boot.img > $@ || rm -f $@ +cmd_socboot = cat spl/u-boot-splx4.sfp u-boot.img > $@ || { rm -f $@; false; } u-boot-with-spl.sfp: spl/u-boot-splx4.sfp u-boot.img FORCE $(call if_changed,socboot) @@ -1599,12 +1599,12 @@ cmd_gensplpadx4 = dd if=/dev/zero of=spl/u-boot-spl.pad bs=64 count=1024 ; \ spl/u-boot-spl.sfp spl/u-boot-spl.pad \ spl/u-boot-spl.sfp spl/u-boot-spl.pad \ spl/u-boot-spl.sfp spl/u-boot-spl.pad > $@ || \ - rm -f $@ spl/u-boot-spl.pad + { rm -f $@ spl/u-boot-spl.pad; false; } u-boot-spl-padx4.sfp: spl/u-boot-spl.sfp FORCE $(call if_changed,gensplpadx4) quiet_cmd_socnandboot = SOCNANDBOOT $@ -cmd_socnandboot = cat u-boot-spl-padx4.sfp u-boot.img > $@ || rm -f $@ +cmd_socnandboot = cat u-boot-spl-padx4.sfp u-boot.img > $@ || { rm -f $@; false; } u-boot-with-nand-spl.sfp: u-boot-spl-padx4.sfp u-boot.img FORCE $(call if_changed,socnandboot) From 90a9901764d71308192c96ecd7d735531fe9cc30 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 1 Nov 2020 14:15:35 -0700 Subject: [PATCH 05/23] test: Add some tests for setexpr This command currently has no tests. Add some for basic assignment and the integer operations. Note that the default size for setexpr is ulong, which varies depending on the build machine. So for sandbox on a 64-bit host, this means that the default size is 64 bits. Signed-off-by: Simon Glass --- include/test/suites.h | 2 + test/cmd/Makefile | 1 + test/cmd/setexpr.c | 162 ++++++++++++++++++++++++++++++++++++++++++ test/cmd_ut.c | 3 + 4 files changed, 168 insertions(+) create mode 100644 test/cmd/setexpr.c diff --git a/include/test/suites.h b/include/test/suites.h index ab7b3bd9ca..5c97846e7f 100644 --- a/include/test/suites.h +++ b/include/test/suites.h @@ -38,6 +38,8 @@ int do_ut_mem(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_ut_optee(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_ut_overlay(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); +int do_ut_setexpr(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]); int do_ut_str(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_ut_time(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_ut_unicode(struct cmd_tbl *cmdtp, int flag, int argc, diff --git a/test/cmd/Makefile b/test/cmd/Makefile index 859dcda239..b2f71af847 100644 --- a/test/cmd/Makefile +++ b/test/cmd/Makefile @@ -4,3 +4,4 @@ obj-y += mem.o obj-$(CONFIG_CMD_MEM_SEARCH) += mem_search.o +obj-y += setexpr.o diff --git a/test/cmd/setexpr.c b/test/cmd/setexpr.c new file mode 100644 index 0000000000..cab6fdf4b8 --- /dev/null +++ b/test/cmd/setexpr.c @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Tests for setexpr command + * + * Copyright 2020 Google LLC + * Written by Simon Glass + */ + +#include +#include +#include +#include +#include +#include + +#define BUF_SIZE 0x100 + +/* Declare a new mem test */ +#define SETEXPR_TEST(_name, _flags) UNIT_TEST(_name, _flags, setexpr_test) + +/* Test 'setexpr' command with simply setting integers */ +static int setexpr_test_int(struct unit_test_state *uts) +{ + u8 *buf; + + buf = map_sysmem(0, BUF_SIZE); + memset(buf, '\xff', BUF_SIZE); + + /* byte */ + buf[0x0] = 0x12; + ut_assertok(run_command("setexpr.b fred 0", 0)); + ut_asserteq_str("0", env_get("fred")); + ut_assertok(run_command("setexpr.b fred *0", 0)); + ut_asserteq_str("12", env_get("fred")); + + /* 16-bit */ + *(short *)buf = 0x2345; + ut_assertok(run_command("setexpr.w fred 0", 0)); + ut_asserteq_str("0", env_get("fred")); + ut_assertok(run_command("setexpr.w fred *0", 0)); + ut_asserteq_str("2345", env_get("fred")); + + /* 32-bit */ + *(u32 *)buf = 0x3456789a; + ut_assertok(run_command("setexpr.l fred 0", 0)); + ut_asserteq_str("0", env_get("fred")); + ut_assertok(run_command("setexpr.l fred *0", 0)); + ut_asserteq_str("ffffffff3456789a", env_get("fred")); + + /* 64-bit */ + *(u64 *)buf = 0x456789abcdef0123; + ut_assertok(run_command("setexpr.q fred 0", 0)); + ut_asserteq_str("0", env_get("fred")); + ut_assertok(run_command("setexpr.q fred *0", 0)); + ut_asserteq_str("456789abcdef0123", env_get("fred")); + + /* default */ + ut_assertok(run_command("setexpr fred 0", 0)); + ut_asserteq_str("0", env_get("fred")); + ut_assertok(run_command("setexpr fred *0", 0)); + ut_asserteq_str("456789abcdef0123", env_get("fred")); + + unmap_sysmem(buf); + + return 0; +} +SETEXPR_TEST(setexpr_test_int, UT_TESTF_CONSOLE_REC); + +/* Test 'setexpr' command with + operator */ +static int setexpr_test_plus(struct unit_test_state *uts) +{ + char *buf; + + buf = map_sysmem(0, BUF_SIZE); + memset(buf, '\xff', BUF_SIZE); + + /* byte */ + buf[0x0] = 0x12; + buf[0x10] = 0x34; + ut_assertok(run_command("setexpr.b fred *0 + *10", 0)); + ut_asserteq_str("46", env_get("fred")); + + /* 16-bit */ + *(short *)buf = 0x2345; + *(short *)(buf + 0x10) = 0xf012; + ut_assertok(run_command("setexpr.w fred *0 + *10", 0)); + ut_asserteq_str("11357", env_get("fred")); + + /* 32-bit */ + *(u32 *)buf = 0x3456789a; + *(u32 *)(buf + 0x10) = 0xc3384235; + ut_assertok(run_command("setexpr.l fred *0 + *10", 0)); + ut_asserteq_str("fffffffef78ebacf", env_get("fred")); + + /* 64-bit */ + *(u64 *)buf = 0x456789abcdef0123; + *(u64 *)(buf + 0x10) = 0x4987328372849283; + ut_assertok(run_command("setexpr.q fred *0 + *10", 0)); + ut_asserteq_str("8eeebc2f407393a6", env_get("fred")); + + /* default */ + ut_assertok(run_command("setexpr fred *0 + *10", 0)); + ut_asserteq_str("8eeebc2f407393a6", env_get("fred")); + + unmap_sysmem(buf); + + return 0; +} +SETEXPR_TEST(setexpr_test_plus, UT_TESTF_CONSOLE_REC); + +/* Test 'setexpr' command with other operators */ +static int setexpr_test_oper(struct unit_test_state *uts) +{ + char *buf; + + buf = map_sysmem(0, BUF_SIZE); + memset(buf, '\xff', BUF_SIZE); + + *(u32 *)buf = 0x1234; + *(u32 *)(buf + 0x10) = 0x560000; + + /* Quote | to avoid confusing hush */ + ut_assertok(run_command("setexpr fred *0 \"|\" *10", 0)); + ut_asserteq_str("ffffffff00561234", env_get("fred")); + + *(u32 *)buf = 0x561200; + *(u32 *)(buf + 0x10) = 0x1234; + + /* Quote & to avoid confusing hush */ + ut_assertok(run_command("setexpr.l fred *0 \"&\" *10", 0)); + ut_asserteq_str("ffffffff00001200", env_get("fred")); + + ut_assertok(run_command("setexpr.l fred *0 ^ *10", 0)); + ut_asserteq_str("560034", env_get("fred")); + + ut_assertok(run_command("setexpr.l fred *0 - *10", 0)); + ut_asserteq_str("55ffcc", env_get("fred")); + + ut_assertok(run_command("setexpr.l fred *0 * *10", 0)); + ut_asserteq_str("ffa9dbd21ebfa800", env_get("fred")); + + ut_assertok(run_command("setexpr.l fred *0 / *10", 0)); + ut_asserteq_str("1", env_get("fred")); + + ut_assertok(run_command("setexpr.l fred *0 % *10", 0)); + ut_asserteq_str("55ffcc", env_get("fred")); + + unmap_sysmem(buf); + + return 0; +} +SETEXPR_TEST(setexpr_test_oper, UT_TESTF_CONSOLE_REC); + +int do_ut_setexpr(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) +{ + struct unit_test *tests = ll_entry_start(struct unit_test, + setexpr_test); + const int n_ents = ll_entry_count(struct unit_test, setexpr_test); + + return cmd_ut_category("cmd_setexpr", "cmd_mem_", tests, n_ents, argc, + argv); +} diff --git a/test/cmd_ut.c b/test/cmd_ut.c index 8f0bc688a2..f79109e6f8 100644 --- a/test/cmd_ut.c +++ b/test/cmd_ut.c @@ -75,6 +75,8 @@ static struct cmd_tbl cmd_ut_sub[] = { U_BOOT_CMD_MKENT(log, CONFIG_SYS_MAXARGS, 1, do_ut_log, "", ""), #endif U_BOOT_CMD_MKENT(mem, CONFIG_SYS_MAXARGS, 1, do_ut_mem, "", ""), + U_BOOT_CMD_MKENT(setexpr, CONFIG_SYS_MAXARGS, 1, do_ut_setexpr, "", + ""), #ifdef CONFIG_UT_TIME U_BOOT_CMD_MKENT(time, CONFIG_SYS_MAXARGS, 1, do_ut_time, "", ""), #endif @@ -153,6 +155,7 @@ static char ut_help_text[] = #ifdef CONFIG_UT_OVERLAY "ut overlay [test-name]\n" #endif + "ut setexpr [test-name] - test setexpr command\n" #ifdef CONFIG_SANDBOX "ut str - Basic test of string functions\n" #endif From 7526deec7e52da92dee25aaf3a47514e639c2bb0 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 1 Nov 2020 14:15:36 -0700 Subject: [PATCH 06/23] command: Add constants for cmd_get_data_size string / error At present these values are open-coded in a few places. Add constants so the meaning is clear. Also add a comment to cmd_get_data_size() Signed-off-by: Simon Glass --- cmd/itest.c | 4 ++-- cmd/mem.c | 2 +- common/command.c | 4 ++-- include/command.h | 26 +++++++++++++++++++++++++- 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/cmd/itest.c b/cmd/itest.c index a0cf4bee04..9a441ce9b8 100644 --- a/cmd/itest.c +++ b/cmd/itest.c @@ -197,10 +197,10 @@ static int do_itest(struct cmd_tbl *cmdtp, int flag, int argc, #endif value = binary_test (argv[2], argv[1], argv[3], w); break; - case -2: + case CMD_DATA_SIZE_STR: value = binary_test (argv[2], argv[1], argv[3], 0); break; - case -1: + case CMD_DATA_SIZE_ERR: default: puts("Invalid data width specifier\n"); value = 0; diff --git a/cmd/mem.c b/cmd/mem.c index 56e1d0755b..1d4f2bab2f 100644 --- a/cmd/mem.c +++ b/cmd/mem.c @@ -393,7 +393,7 @@ static int do_mem_search(struct cmd_tbl *cmdtp, int flag, int argc, * Defaults to long if no or incorrect specification. */ size = cmd_get_data_size(argv[0], 4); - if (size < 0 && size != -2 /* string */) + if (size < 0 && size != CMD_DATA_SIZE_STR) return 1; argc--; diff --git a/common/command.c b/common/command.c index 2c491e20a7..068cb55b4c 100644 --- a/common/command.c +++ b/common/command.c @@ -475,13 +475,13 @@ int cmd_get_data_size(char* arg, int default_size) case 'l': return 4; case 's': - return -2; + return CMD_DATA_SIZE_STR; case 'q': if (MEM_SUPPORT_64BIT_DATA) return 8; /* no break */ default: - return -1; + return CMD_DATA_SIZE_ERR; } } return default_size; diff --git a/include/command.h b/include/command.h index b9b5ec1afa..e900f97df3 100644 --- a/include/command.h +++ b/include/command.h @@ -117,7 +117,31 @@ int cmd_process_error(struct cmd_tbl *cmdtp, int err); defined(CONFIG_CMD_PCI) || \ defined(CONFIG_CMD_SETEXPR) #define CMD_DATA_SIZE -extern int cmd_get_data_size(char* arg, int default_size); +#define CMD_DATA_SIZE_ERR (-1) +#define CMD_DATA_SIZE_STR (-2) + +/** + * cmd_get_data_size() - Get the data-size specifier from a command + * + * This reads a '.x' size specifier appended to a command. For example 'md.b' + * is the 'md' command with a '.b' specifier, meaning that the command should + * use bytes. + * + * Valid characters are: + * + * b - byte + * w - word (16 bits) + * l - long (32 bits) + * q - quad (64 bits) + * s - string + * + * @arg: Pointers to the command to check. If a valid specifier is present it + * will be the last character of the string, following a '.' + * @default_size: Default size to return if there is no specifier + * @return data size in bytes (1, 2, 4, 8) or CMD_DATA_SIZE_ERR for an invalid + * character, or CMD_DATA_SIZE_STR for a string + */ +int cmd_get_data_size(char *arg, int default_size); #endif #ifdef CONFIG_CMD_BOOTD From 25a43ac84a9509b8ccd093808bb39b71e4438cf5 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 1 Nov 2020 14:15:37 -0700 Subject: [PATCH 07/23] setexpr: Add explicit support for 32- and 64-bit ints At present this function assumes that a size of 4 refers to a ulong. This is true on 32-bit machines but not commonly on 64-bit machines. This means that the 'l' specify does not work correctly with setexpr. Add an explicit case for 32-bit values so that 64-bit machines can still use the 'l' specifier. On 32-bit machines, 64-bit is still not supported. This corrects the operation of the default size (which is 4 for setexpr), so update the tests accordingly. The original code for reading from memory was included in 47ab5ad1457 ("cmd_setexpr: allow memory addresses in expressions") but I am not adding a Fixes: tag since that code was not written with 64-bit machines in mind. Signed-off-by: Simon Glass --- cmd/setexpr.c | 4 ++++ test/cmd/setexpr.c | 18 +++++++++--------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/cmd/setexpr.c b/cmd/setexpr.c index 770dc24d2b..dd9c2574fd 100644 --- a/cmd/setexpr.c +++ b/cmd/setexpr.c @@ -39,6 +39,10 @@ static ulong get_arg(char *s, int w) unmap_sysmem(p); return val; case 4: + p = map_sysmem(addr, sizeof(u32)); + val = *(u32 *)p; + unmap_sysmem(p); + return val; default: p = map_sysmem(addr, sizeof(ulong)); val = *p; diff --git a/test/cmd/setexpr.c b/test/cmd/setexpr.c index cab6fdf4b8..e950c380ce 100644 --- a/test/cmd/setexpr.c +++ b/test/cmd/setexpr.c @@ -45,7 +45,7 @@ static int setexpr_test_int(struct unit_test_state *uts) ut_assertok(run_command("setexpr.l fred 0", 0)); ut_asserteq_str("0", env_get("fred")); ut_assertok(run_command("setexpr.l fred *0", 0)); - ut_asserteq_str("ffffffff3456789a", env_get("fred")); + ut_asserteq_str("3456789a", env_get("fred")); /* 64-bit */ *(u64 *)buf = 0x456789abcdef0123; @@ -58,7 +58,7 @@ static int setexpr_test_int(struct unit_test_state *uts) ut_assertok(run_command("setexpr fred 0", 0)); ut_asserteq_str("0", env_get("fred")); ut_assertok(run_command("setexpr fred *0", 0)); - ut_asserteq_str("456789abcdef0123", env_get("fred")); + ut_asserteq_str("cdef0123", env_get("fred")); unmap_sysmem(buf); @@ -90,7 +90,7 @@ static int setexpr_test_plus(struct unit_test_state *uts) *(u32 *)buf = 0x3456789a; *(u32 *)(buf + 0x10) = 0xc3384235; ut_assertok(run_command("setexpr.l fred *0 + *10", 0)); - ut_asserteq_str("fffffffef78ebacf", env_get("fred")); + ut_asserteq_str("f78ebacf", env_get("fred")); /* 64-bit */ *(u64 *)buf = 0x456789abcdef0123; @@ -100,7 +100,7 @@ static int setexpr_test_plus(struct unit_test_state *uts) /* default */ ut_assertok(run_command("setexpr fred *0 + *10", 0)); - ut_asserteq_str("8eeebc2f407393a6", env_get("fred")); + ut_asserteq_str("1407393a6", env_get("fred")); unmap_sysmem(buf); @@ -121,14 +121,14 @@ static int setexpr_test_oper(struct unit_test_state *uts) /* Quote | to avoid confusing hush */ ut_assertok(run_command("setexpr fred *0 \"|\" *10", 0)); - ut_asserteq_str("ffffffff00561234", env_get("fred")); + ut_asserteq_str("561234", env_get("fred")); *(u32 *)buf = 0x561200; *(u32 *)(buf + 0x10) = 0x1234; /* Quote & to avoid confusing hush */ ut_assertok(run_command("setexpr.l fred *0 \"&\" *10", 0)); - ut_asserteq_str("ffffffff00001200", env_get("fred")); + ut_asserteq_str("1200", env_get("fred")); ut_assertok(run_command("setexpr.l fred *0 ^ *10", 0)); ut_asserteq_str("560034", env_get("fred")); @@ -137,13 +137,13 @@ static int setexpr_test_oper(struct unit_test_state *uts) ut_asserteq_str("55ffcc", env_get("fred")); ut_assertok(run_command("setexpr.l fred *0 * *10", 0)); - ut_asserteq_str("ffa9dbd21ebfa800", env_get("fred")); + ut_asserteq_str("61ebfa800", env_get("fred")); ut_assertok(run_command("setexpr.l fred *0 / *10", 0)); - ut_asserteq_str("1", env_get("fred")); + ut_asserteq_str("4ba", env_get("fred")); ut_assertok(run_command("setexpr.l fred *0 % *10", 0)); - ut_asserteq_str("55ffcc", env_get("fred")); + ut_asserteq_str("838", env_get("fred")); unmap_sysmem(buf); From e713124e352efe3141af6a4ffc83f6dba449a177 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 1 Nov 2020 14:15:38 -0700 Subject: [PATCH 08/23] test: Add some setexpr regex tests Add tests for the setexpr regex commands. Note that these tests currently crash on sandbox due to an existing bug in the setexpr implementation, so two of the tests are commented out. Signed-off-by: Simon Glass --- test/cmd/setexpr.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/test/cmd/setexpr.c b/test/cmd/setexpr.c index e950c380ce..de54561917 100644 --- a/test/cmd/setexpr.c +++ b/test/cmd/setexpr.c @@ -151,6 +151,64 @@ static int setexpr_test_oper(struct unit_test_state *uts) } SETEXPR_TEST(setexpr_test_oper, UT_TESTF_CONSOLE_REC); +/* Test 'setexpr' command with regex */ +static int setexpr_test_regex(struct unit_test_state *uts) +{ + char *buf, *val; + + buf = map_sysmem(0, BUF_SIZE); + + /* Single substitution */ + ut_assertok(run_command("setenv fred 'this is a test'", 0)); + ut_assertok(run_command("setexpr fred sub is us", 0)); + val = env_get("fred"); + ut_asserteq_str("thus is a test", val); + + /* Global substitution */ + ut_assertok(run_command("setenv fred 'this is a test'", 0)); + if (0) { + /* Causes a crash at present due to a bug in setexpr */ + ut_assertok(run_command("setexpr fred gsub is us", 0)); + val = env_get("fred"); + ut_asserteq_str("thus us a test", val); + } + /* Global substitution */ + ut_assertok(run_command("setenv fred 'this is a test'", 0)); + ut_assertok(run_command("setenv mary 'this is a test'", 0)); + ut_assertok(run_command("setexpr fred gsub is us \"${mary}\"", 0)); + val = env_get("fred"); + ut_asserteq_str("thus us a test", val); + val = env_get("mary"); + ut_asserteq_str("this is a test", val); + + unmap_sysmem(buf); + + return 0; +} +SETEXPR_TEST(setexpr_test_regex, UT_TESTF_CONSOLE_REC); + +/* Test 'setexpr' command with regex replacement that expands the string */ +static int setexpr_test_regex_inc(struct unit_test_state *uts) +{ + char *buf, *val; + + buf = map_sysmem(0, BUF_SIZE); + + ut_assertok(run_command("setenv fred 'this is a test'", 0)); + if (0) { + /* Causes a crash at present due to a bug in setexpr */ + ut_assertok(run_command("setexpr fred gsub is much_longer_string", + 0)); + val = env_get("fred"); + ut_asserteq_str("thmuch_longer_string much_longer_string a test", + val); + } + unmap_sysmem(buf); + + return 0; +} +SETEXPR_TEST(setexpr_test_regex_inc, UT_TESTF_CONSOLE_REC); + int do_ut_setexpr(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct unit_test *tests = ll_entry_start(struct unit_test, From 56331b2680d9fef7e60a88fa50d3e167f236c4a0 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 1 Nov 2020 14:15:39 -0700 Subject: [PATCH 09/23] setexpr: Split the core logic into its own function At present this function always allocates a large amount of stack, and selects its own size for buffers. This makes it hard to test the code for buffer overflow. Separate out the inner logic of the substitution so that tests can call this directly. This will allow checking that the algorithm does not overflow the buffer. Fix up one of the error lines at the same time, since it should be printing nbuf_size, not data_size. Signed-off-by: Simon Glass --- cmd/setexpr.c | 156 +++++++++++++++++++++++++++++++------------------- 1 file changed, 98 insertions(+), 58 deletions(-) diff --git a/cmd/setexpr.c b/cmd/setexpr.c index dd9c2574fd..fe3435b4d9 100644 --- a/cmd/setexpr.c +++ b/cmd/setexpr.c @@ -58,9 +58,6 @@ static ulong get_arg(char *s, int w) #include -#define SLRE_BUFSZ 16384 -#define SLRE_PATSZ 4096 - /* * memstr - Find the first substring in memory * @s1: The string to be searched @@ -83,13 +80,24 @@ static char *memstr(const char *s1, int l1, const char *s2, int l2) return NULL; } -static char *substitute(char *string, /* string buffer */ - int *slen, /* current string length */ - int ssize, /* string bufer size */ - const char *old,/* old (replaced) string */ - int olen, /* length of old string */ - const char *new,/* new (replacement) string */ - int nlen) /* length of new string */ +/** + * substitute() - Substitute part of one string with another + * + * This updates @string so that the first occurrence of @old is replaced with + * @new + * + * @string: String buffer containing string to update at the start + * @slen: Pointer to current string length, updated on success + * @ssize: Size of string buffer + * @old: Old string to find in the buffer (no terminator needed) + * @olen: Length of @old excluding terminator + * @new: New string to replace @old with + * @nlen: Length of @new excluding terminator + * @return pointer to immediately after the copied @new in @string, or NULL if + * no replacement took place + */ +static char *substitute(char *string, int *slen, int ssize, + const char *old, int olen, const char *new, int nlen) { char *p = memstr(string, *slen, old, olen); @@ -118,7 +126,7 @@ static char *substitute(char *string, /* string buffer */ memmove(p + nlen, p + olen, tail); } - /* insert substitue */ + /* insert substitute */ memcpy(p, new, nlen); *slen += nlen - olen; @@ -126,52 +134,33 @@ static char *substitute(char *string, /* string buffer */ return p + nlen; } -/* - * Perform regex operations on a environment variable +/** + * regex_sub() - Replace a regex pattern with a string * - * Returns 0 if OK, 1 in case of errors. + * @data: Buffer containing the string to update + * @data_size: Size of buffer (must be large enough for the new string) + * @nbuf: Back-reference buffer + * @nbuf_size: Size of back-reference buffer (must be larger enough for @s plus + * all back-reference expansions) + * @r: Regular expression to find + * @s: String to replace with + * @global: true to replace all matches in @data, false to replace just the + * first + * @return 0 if OK, 1 on error */ -static int regex_sub(const char *name, - const char *r, const char *s, const char *t, - int global) +static int regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size, + const char *r, const char *s, bool global) { struct slre slre; - char data[SLRE_BUFSZ]; char *datap = data; - const char *value; int res, len, nlen, loop; - if (name == NULL) - return 1; - if (slre_compile(&slre, r) == 0) { printf("Error compiling regex: %s\n", slre.err_str); return 1; } - if (t == NULL) { - value = env_get(name); - - if (value == NULL) { - printf("## Error: variable \"%s\" not defined\n", name); - return 1; - } - t = value; - } - - debug("REGEX on %s=%s\n", name, t); - debug("REGEX=\"%s\", SUBST=\"%s\", GLOBAL=%d\n", - r, s ? s : "", global); - - len = strlen(t); - if (len + 1 > SLRE_BUFSZ) { - printf("## error: subst buffer overflow: have %d, need %d\n", - SLRE_BUFSZ, len + 1); - return 1; - } - - strcpy(data, t); - + len = strlen(data); if (s == NULL) nlen = 0; else @@ -179,7 +168,6 @@ static int regex_sub(const char *name, for (loop = 0;; loop++) { struct cap caps[slre.num_caps + 2]; - char nbuf[SLRE_PATSZ]; const char *old; char *np; int i, olen; @@ -199,7 +187,7 @@ static int regex_sub(const char *name, if (res == 0) { if (loop == 0) { - printf("%s: No match\n", t); + printf("%s: No match\n", data); return 1; } else { break; @@ -208,17 +196,15 @@ static int regex_sub(const char *name, debug("## MATCH ## %s\n", data); - if (s == NULL) { - printf("%s=%s\n", name, t); + if (!s) return 1; - } old = caps[0].ptr; olen = caps[0].len; - if (nlen + 1 >= SLRE_PATSZ) { + if (nlen + 1 >= nbuf_size) { printf("## error: pattern buffer overflow: have %d, need %d\n", - SLRE_BUFSZ, nlen + 1); + nbuf_size, nlen + 1); return 1; } strcpy(nbuf, s); @@ -263,7 +249,7 @@ static int regex_sub(const char *name, break; np = substitute(np, &nlen, - SLRE_PATSZ, + nbuf_size, backref, 2, caps[i].ptr, caps[i].len); @@ -273,9 +259,8 @@ static int regex_sub(const char *name, } debug("## SUBST(2) ## %s\n", nbuf); - datap = substitute(datap, &len, SLRE_BUFSZ, - old, olen, - nbuf, nlen); + datap = substitute(datap, &len, data_size, old, olen, + nbuf, nlen); if (datap == NULL) return 1; @@ -289,6 +274,61 @@ static int regex_sub(const char *name, } debug("## FINAL (now env_set()) : %s\n", data); + return 0; +} + +#define SLRE_BUFSZ 16384 +#define SLRE_PATSZ 4096 + +/* + * Perform regex operations on a environment variable + * + * Returns 0 if OK, 1 in case of errors. + */ +static int regex_sub_var(const char *name, const char *r, const char *s, + const char *t, int global) +{ + struct slre slre; + char data[SLRE_BUFSZ]; + char nbuf[SLRE_PATSZ]; + const char *value; + int len; + int ret; + + if (!name) + return 1; + + if (slre_compile(&slre, r) == 0) { + printf("Error compiling regex: %s\n", slre.err_str); + return 1; + } + + if (!t) { + value = env_get(name); + if (!value) { + printf("## Error: variable \"%s\" not defined\n", name); + return 1; + } + t = value; + } + + debug("REGEX on %s=%s\n", name, t); + debug("REGEX=\"%s\", SUBST=\"%s\", GLOBAL=%d\n", r, s ? s : "", + global); + + len = strlen(t); + if (len + 1 > SLRE_BUFSZ) { + printf("## error: subst buffer overflow: have %d, need %d\n", + SLRE_BUFSZ, len + 1); + return 1; + } + + strcpy(data, t); + + ret = regex_sub(data, SLRE_BUFSZ, nbuf, SLRE_PATSZ, r, s, global); + if (ret) + return 1; + printf("%s=%s\n", name, data); return env_set(name, data); @@ -331,10 +371,10 @@ static int do_setexpr(struct cmd_tbl *cmdtp, int flag, int argc, * with 5 args, "t" will be NULL */ if (strcmp(argv[2], "gsub") == 0) - return regex_sub(argv[1], argv[3], argv[4], argv[5], 1); + return regex_sub_var(argv[1], argv[3], argv[4], argv[5], 1); if (strcmp(argv[2], "sub") == 0) - return regex_sub(argv[1], argv[3], argv[4], argv[5], 0); + return regex_sub_var(argv[1], argv[3], argv[4], argv[5], 0); #endif /* standard operators: "setexpr name val1 op val2" */ From d422c77ae8e0cb1211b34eb4af442600b0da8d5b Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 1 Nov 2020 14:15:40 -0700 Subject: [PATCH 10/23] setexpr: Add some tests for buffer overflow and backref Add tests to check for buffer overflow using simple replacement as well as back references. At present these don't fully pass. Signed-off-by: Simon Glass --- cmd/setexpr.c | 21 +++-------- include/command.h | 17 +++++++++ test/cmd/setexpr.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 17 deletions(-) diff --git a/cmd/setexpr.c b/cmd/setexpr.c index fe3435b4d9..dbb43b3be2 100644 --- a/cmd/setexpr.c +++ b/cmd/setexpr.c @@ -134,22 +134,8 @@ static char *substitute(char *string, int *slen, int ssize, return p + nlen; } -/** - * regex_sub() - Replace a regex pattern with a string - * - * @data: Buffer containing the string to update - * @data_size: Size of buffer (must be large enough for the new string) - * @nbuf: Back-reference buffer - * @nbuf_size: Size of back-reference buffer (must be larger enough for @s plus - * all back-reference expansions) - * @r: Regular expression to find - * @s: String to replace with - * @global: true to replace all matches in @data, false to replace just the - * first - * @return 0 if OK, 1 on error - */ -static int regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size, - const char *r, const char *s, bool global) +int setexpr_regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size, + const char *r, const char *s, bool global) { struct slre slre; char *datap = data; @@ -325,7 +311,8 @@ static int regex_sub_var(const char *name, const char *r, const char *s, strcpy(data, t); - ret = regex_sub(data, SLRE_BUFSZ, nbuf, SLRE_PATSZ, r, s, global); + ret = setexpr_regex_sub(data, SLRE_BUFSZ, nbuf, SLRE_PATSZ, r, s, + global); if (ret) return 1; diff --git a/include/command.h b/include/command.h index e900f97df3..e229bf2825 100644 --- a/include/command.h +++ b/include/command.h @@ -183,6 +183,23 @@ extern int do_env_set_efi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); #endif +/** + * setexpr_regex_sub() - Replace a regex pattern with a string + * + * @data: Buffer containing the string to update + * @data_size: Size of buffer (must be large enough for the new string) + * @nbuf: Back-reference buffer + * @nbuf_size: Size of back-reference buffer (must be larger enough for @s plus + * all back-reference expansions) + * @r: Regular expression to find + * @s: String to replace with + * @global: true to replace all matches in @data, false to replace just the + * first + * @return 0 if OK, 1 on error + */ +int setexpr_regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size, + const char *r, const char *s, bool global); + /* * Error codes that commands return to cmd_process(). We use the standard 0 * and 1 for success and failure, but add one more case - failure with a diff --git a/test/cmd/setexpr.c b/test/cmd/setexpr.c index de54561917..a6940fd82d 100644 --- a/test/cmd/setexpr.c +++ b/test/cmd/setexpr.c @@ -209,6 +209,95 @@ static int setexpr_test_regex_inc(struct unit_test_state *uts) } SETEXPR_TEST(setexpr_test_regex_inc, UT_TESTF_CONSOLE_REC); +/* Test setexpr_regex_sub() directly to check buffer usage */ +static int setexpr_test_sub(struct unit_test_state *uts) +{ + char *buf, *nbuf; + int i; + + buf = map_sysmem(0, BUF_SIZE); + nbuf = map_sysmem(0x1000, BUF_SIZE); + + /* Add a pattern so we can check the buffer limits */ + memset(buf, '\xff', BUF_SIZE); + memset(nbuf, '\xff', BUF_SIZE); + for (i = BUF_SIZE; i < 0x1000; i++) { + buf[i] = i & 0xff; + nbuf[i] = i & 0xff; + } + strcpy(buf, "this is a test"); + + /* + * This is a regression test, since a bug was found in the use of + * memmove() in setexpr + */ + ut_assertok(setexpr_regex_sub(buf, BUF_SIZE, nbuf, BUF_SIZE, "is", + "us it is longer", true)); + ut_asserteq_str("thus it is longer us it is longer a test", buf); + + /* The following checks fail at present due to a bug in setexpr */ + return 0; + for (i = BUF_SIZE; i < 0x1000; i++) { + ut_assertf(buf[i] == (char)i, + "buf byte at %x should be %02x, got %02x)\n", + i, i & 0xff, (u8)buf[i]); + ut_assertf(nbuf[i] == (char)i, + "nbuf byte at %x should be %02x, got %02x)\n", + i, i & 0xff, (u8)nbuf[i]); + } + + unmap_sysmem(buf); + + return 0; +} +SETEXPR_TEST(setexpr_test_sub, UT_TESTF_CONSOLE_REC); + +/* Test setexpr_regex_sub() with back references */ +static int setexpr_test_backref(struct unit_test_state *uts) +{ + char *buf, *nbuf; + int i; + + buf = map_sysmem(0, BUF_SIZE); + nbuf = map_sysmem(0x1000, BUF_SIZE); + + /* Add a pattern so we can check the buffer limits */ + memset(buf, '\xff', BUF_SIZE); + memset(nbuf, '\xff', BUF_SIZE); + for (i = BUF_SIZE; i < 0x1000; i++) { + buf[i] = i & 0xff; + nbuf[i] = i & 0xff; + } + strcpy(buf, "this is surely a test is it? yes this is indeed a test"); + + /* + * This is a regression test, since a bug was found in the use of + * memmove() in setexpr + */ + ut_assertok(setexpr_regex_sub(buf, BUF_SIZE, nbuf, BUF_SIZE, + "(this) (is) (surely|indeed)", + "us \\1 \\2 \\3!", true)); + + /* The following checks fail at present due to bugs in setexpr */ + return 0; + ut_asserteq_str("us this is surely! a test is it? yes us this is indeed! a test", + buf); + + for (i = BUF_SIZE; i < 0x1000; i++) { + ut_assertf(buf[i] == (char)i, + "buf byte at %x should be %02x, got %02x)\n", + i, i & 0xff, (u8)buf[i]); + ut_assertf(nbuf[i] == (char)i, + "nbuf byte at %x should be %02x, got %02x)\n", + i, i & 0xff, (u8)nbuf[i]); + } + + unmap_sysmem(buf); + + return 0; +} +SETEXPR_TEST(setexpr_test_backref, UT_TESTF_CONSOLE_REC); + int do_ut_setexpr(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct unit_test *tests = ll_entry_start(struct unit_test, From 9528229f22b590cc4b5cf8bf0d3212d2ab08ffd5 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 1 Nov 2020 14:15:41 -0700 Subject: [PATCH 11/23] setexpr: Correct dropping of final unmatched string At present the 'nlen' variable increases with each loop. If the previous loop had back references, then subsequent loops without back references use the wrong value of nlen. The value is larger, meaning that the string terminator from nbuf is copied along to the main buffer, thus terminating the string prematurely. This leads to the final result being truncated, e.g. missing the last (unmatched) part of the string. So "match match tail" become "replaced replaced" instead of "replaced replaced tail". Fix this by resetting nlen to the correct value each time around the lop. Fixes: 855f18ea0e6 ("setexpr: add regex substring matching and substitution") Signed-off-by: Simon Glass --- cmd/setexpr.c | 6 +----- test/cmd/setexpr.c | 5 ++--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/cmd/setexpr.c b/cmd/setexpr.c index dbb43b3be2..0cc7cf15bd 100644 --- a/cmd/setexpr.c +++ b/cmd/setexpr.c @@ -147,11 +147,6 @@ int setexpr_regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size, } len = strlen(data); - if (s == NULL) - nlen = 0; - else - nlen = strlen(s); - for (loop = 0;; loop++) { struct cap caps[slre.num_caps + 2]; const char *old; @@ -187,6 +182,7 @@ int setexpr_regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size, old = caps[0].ptr; olen = caps[0].len; + nlen = strlen(s); if (nlen + 1 >= nbuf_size) { printf("## error: pattern buffer overflow: have %d, need %d\n", diff --git a/test/cmd/setexpr.c b/test/cmd/setexpr.c index a6940fd82d..d06dda260e 100644 --- a/test/cmd/setexpr.c +++ b/test/cmd/setexpr.c @@ -277,12 +277,11 @@ static int setexpr_test_backref(struct unit_test_state *uts) ut_assertok(setexpr_regex_sub(buf, BUF_SIZE, nbuf, BUF_SIZE, "(this) (is) (surely|indeed)", "us \\1 \\2 \\3!", true)); - - /* The following checks fail at present due to bugs in setexpr */ - return 0; ut_asserteq_str("us this is surely! a test is it? yes us this is indeed! a test", buf); + /* The following checks fail at present due to a bug in setexpr */ + return 0; for (i = BUF_SIZE; i < 0x1000; i++) { ut_assertf(buf[i] == (char)i, "buf byte at %x should be %02x, got %02x)\n", From 8f4aa7ddb908369db971d4c31850ca1eef2e3687 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 1 Nov 2020 14:15:42 -0700 Subject: [PATCH 12/23] setexpr: Correct buffer overflow bug and enable tests At present when more than one substitution is made this function overwrites its buffers. Fix this bug and update the tests now that they can pass. Also update the debug code to show all substrings, since at present it omits the final one. Fixes: 855f18ea0e6 ("setexpr: add regex substring matching and substitution") Signed-off-by: Simon Glass --- cmd/setexpr.c | 10 +++++----- test/cmd/setexpr.c | 24 +++++++----------------- 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/cmd/setexpr.c b/cmd/setexpr.c index 0cc7cf15bd..d364dbc2bc 100644 --- a/cmd/setexpr.c +++ b/cmd/setexpr.c @@ -155,11 +155,11 @@ int setexpr_regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size, (void) memset(caps, 0, sizeof(caps)); - res = slre_match(&slre, datap, len, caps); + res = slre_match(&slre, datap, len - (datap - data), caps); debug("Result: %d\n", res); - for (i = 0; i < slre.num_caps; i++) { + for (i = 0; i <= slre.num_caps; i++) { if (caps[i].len > 0) { debug("Substring %d: [%.*s]\n", i, caps[i].len, caps[i].ptr); @@ -231,7 +231,7 @@ int setexpr_regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size, break; np = substitute(np, &nlen, - nbuf_size, + nbuf_size - (np - nbuf), backref, 2, caps[i].ptr, caps[i].len); @@ -241,8 +241,8 @@ int setexpr_regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size, } debug("## SUBST(2) ## %s\n", nbuf); - datap = substitute(datap, &len, data_size, old, olen, - nbuf, nlen); + datap = substitute(datap, &len, data_size - (datap - data), + old, olen, nbuf, nlen); if (datap == NULL) return 1; diff --git a/test/cmd/setexpr.c b/test/cmd/setexpr.c index d06dda260e..2a897efd9b 100644 --- a/test/cmd/setexpr.c +++ b/test/cmd/setexpr.c @@ -166,12 +166,10 @@ static int setexpr_test_regex(struct unit_test_state *uts) /* Global substitution */ ut_assertok(run_command("setenv fred 'this is a test'", 0)); - if (0) { - /* Causes a crash at present due to a bug in setexpr */ - ut_assertok(run_command("setexpr fred gsub is us", 0)); - val = env_get("fred"); - ut_asserteq_str("thus us a test", val); - } + ut_assertok(run_command("setexpr fred gsub is us", 0)); + val = env_get("fred"); + ut_asserteq_str("thus us a test", val); + /* Global substitution */ ut_assertok(run_command("setenv fred 'this is a test'", 0)); ut_assertok(run_command("setenv mary 'this is a test'", 0)); @@ -195,14 +193,9 @@ static int setexpr_test_regex_inc(struct unit_test_state *uts) buf = map_sysmem(0, BUF_SIZE); ut_assertok(run_command("setenv fred 'this is a test'", 0)); - if (0) { - /* Causes a crash at present due to a bug in setexpr */ - ut_assertok(run_command("setexpr fred gsub is much_longer_string", - 0)); - val = env_get("fred"); - ut_asserteq_str("thmuch_longer_string much_longer_string a test", - val); - } + ut_assertok(run_command("setexpr fred gsub is much_longer_string", 0)); + val = env_get("fred"); + ut_asserteq_str("thmuch_longer_string much_longer_string a test", val); unmap_sysmem(buf); return 0; @@ -234,9 +227,6 @@ static int setexpr_test_sub(struct unit_test_state *uts) ut_assertok(setexpr_regex_sub(buf, BUF_SIZE, nbuf, BUF_SIZE, "is", "us it is longer", true)); ut_asserteq_str("thus it is longer us it is longer a test", buf); - - /* The following checks fail at present due to a bug in setexpr */ - return 0; for (i = BUF_SIZE; i < 0x1000; i++) { ut_assertf(buf[i] == (char)i, "buf byte at %x should be %02x, got %02x)\n", From f66bee41abe8288e5fd6aa6b61ff23c5f8a2c851 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 1 Nov 2020 14:15:43 -0700 Subject: [PATCH 13/23] setexpr: Convert to use a struct for values At present a ulong is used to hold operand values. This means that strings cannot be used. While most operations are not useful for strings, concatenation is. As a starting point to supporting strings, convert the code to use a struct instead of a ulong for operands. Signed-off-by: Simon Glass --- cmd/setexpr.c | 109 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 66 insertions(+), 43 deletions(-) diff --git a/cmd/setexpr.c b/cmd/setexpr.c index d364dbc2bc..8a3654505d 100644 --- a/cmd/setexpr.c +++ b/cmd/setexpr.c @@ -15,8 +15,19 @@ #include #include -static ulong get_arg(char *s, int w) +/** + * struct expr_arg: Holds an argument to an expression + * + * @ival: Integer value (if width is not CMD_DATA_SIZE_STR) + */ +struct expr_arg { + ulong ival; +}; + +static int get_arg(char *s, int w, struct expr_arg *argp) { + struct expr_arg arg; + /* * If the parameter starts with a '*' then assume it is a pointer to * the value we want. @@ -32,26 +43,33 @@ static ulong get_arg(char *s, int w) p = map_sysmem(addr, sizeof(uchar)); val = (ulong)*(uchar *)p; unmap_sysmem(p); - return val; + arg.ival = val; + break; case 2: p = map_sysmem(addr, sizeof(ushort)); val = (ulong)*(ushort *)p; unmap_sysmem(p); - return val; + arg.ival = val; + break; case 4: p = map_sysmem(addr, sizeof(u32)); val = *(u32 *)p; unmap_sysmem(p); - return val; + arg.ival = val; + break; default: p = map_sysmem(addr, sizeof(ulong)); val = *p; unmap_sysmem(p); - return val; + arg.ival = val; + break; } } else { - return simple_strtoul(s, NULL, 16); + arg.ival = simple_strtoul(s, NULL, 16); } + *argp = arg; + + return 0; } #ifdef CONFIG_REGEX @@ -321,7 +339,7 @@ static int regex_sub_var(const char *name, const char *r, const char *s, static int do_setexpr(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { - ulong a, b; + struct expr_arg aval, bval; ulong value; int w; @@ -339,13 +357,12 @@ static int do_setexpr(struct cmd_tbl *cmdtp, int flag, int argc, w = cmd_get_data_size(argv[0], 4); - a = get_arg(argv[2], w); + if (get_arg(argv[2], w, &aval)) + return CMD_RET_FAILURE; /* plain assignment: "setexpr name value" */ - if (argc == 3) { - env_set_hex(argv[1], a); - return 0; - } + if (argc == 3) + return env_set_hex(argv[1], aval.ival); /* 5 or 6 args (6 args only with [g]sub) */ #ifdef CONFIG_REGEX @@ -367,40 +384,46 @@ static int do_setexpr(struct cmd_tbl *cmdtp, int flag, int argc, if (strlen(argv[3]) != 1) return CMD_RET_USAGE; - b = get_arg(argv[4], w); + if (get_arg(argv[4], w, &bval)) + return CMD_RET_FAILURE; - switch (argv[3][0]) { - case '|': - value = a | b; - break; - case '&': - value = a & b; - break; - case '+': - value = a + b; - break; - case '^': - value = a ^ b; - break; - case '-': - value = a - b; - break; - case '*': - value = a * b; - break; - case '/': - value = a / b; - break; - case '%': - value = a % b; - break; - default: - printf("invalid op\n"); - return 1; + if (w != CMD_DATA_SIZE_STR) { + ulong a = aval.ival; + ulong b = bval.ival; + + switch (argv[3][0]) { + case '|': + value = a | b; + break; + case '&': + value = a & b; + break; + case '+': + value = a + b; + break; + case '^': + value = a ^ b; + break; + case '-': + value = a - b; + break; + case '*': + value = a * b; + break; + case '/': + value = a / b; + break; + case '%': + value = a % b; + break; + default: + printf("invalid op\n"); + return 1; + } + + env_set_hex(argv[1], value); } - env_set_hex(argv[1], value); - return 0; } From 2c02152a8e2d640fe1d93177fbf523d25cd3e8f9 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 1 Nov 2020 14:15:44 -0700 Subject: [PATCH 14/23] setexpr: Add support for strings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add support for dealing with string operands, including reading a string from memory into an environment variable and concatenating two strings. Signed-off-by: Simon Glass Acked-by: Marek Behún --- cmd/setexpr.c | 82 +++++++++++++++++++++++++++++++++++++++---- test/cmd/setexpr.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 161 insertions(+), 7 deletions(-) diff --git a/cmd/setexpr.c b/cmd/setexpr.c index 8a3654505d..e828be3970 100644 --- a/cmd/setexpr.c +++ b/cmd/setexpr.c @@ -13,15 +13,21 @@ #include #include #include +#include #include +#include /** * struct expr_arg: Holds an argument to an expression * * @ival: Integer value (if width is not CMD_DATA_SIZE_STR) + * @sval: String value (if width is CMD_DATA_SIZE_STR) */ struct expr_arg { - ulong ival; + union { + ulong ival; + char *sval; + }; }; static int get_arg(char *s, int w, struct expr_arg *argp) @@ -36,6 +42,8 @@ static int get_arg(char *s, int w, struct expr_arg *argp) ulong *p; ulong addr; ulong val; + int len; + char *str; addr = simple_strtoul(&s[1], NULL, 16); switch (w) { @@ -51,6 +59,21 @@ static int get_arg(char *s, int w, struct expr_arg *argp) unmap_sysmem(p); arg.ival = val; break; + case CMD_DATA_SIZE_STR: + p = map_sysmem(addr, SZ_64K); + + /* Maximum string length of 64KB plus terminator */ + len = strnlen((char *)p, SZ_64K) + 1; + str = malloc(len); + if (!str) { + printf("Out of memory\n"); + return -ENOMEM; + } + memcpy(str, p, len); + str[len - 1] = '\0'; + unmap_sysmem(p); + arg.sval = str; + break; case 4: p = map_sysmem(addr, sizeof(u32)); val = *(u32 *)p; @@ -65,6 +88,8 @@ static int get_arg(char *s, int w, struct expr_arg *argp) break; } } else { + if (w == CMD_DATA_SIZE_STR) + return -EINVAL; arg.ival = simple_strtoul(s, NULL, 16); } *argp = arg; @@ -341,6 +366,7 @@ static int do_setexpr(struct cmd_tbl *cmdtp, int flag, int argc, { struct expr_arg aval, bval; ulong value; + int ret = 0; int w; /* @@ -361,8 +387,16 @@ static int do_setexpr(struct cmd_tbl *cmdtp, int flag, int argc, return CMD_RET_FAILURE; /* plain assignment: "setexpr name value" */ - if (argc == 3) - return env_set_hex(argv[1], aval.ival); + if (argc == 3) { + if (w == CMD_DATA_SIZE_STR) { + ret = env_set(argv[1], aval.sval); + free(aval.sval); + } else { + ret = env_set_hex(argv[1], aval.ival); + } + + return ret; + } /* 5 or 6 args (6 args only with [g]sub) */ #ifdef CONFIG_REGEX @@ -384,10 +418,38 @@ static int do_setexpr(struct cmd_tbl *cmdtp, int flag, int argc, if (strlen(argv[3]) != 1) return CMD_RET_USAGE; - if (get_arg(argv[4], w, &bval)) + if (get_arg(argv[4], w, &bval)) { + if (w == CMD_DATA_SIZE_STR) + free(aval.sval); return CMD_RET_FAILURE; + } - if (w != CMD_DATA_SIZE_STR) { + if (w == CMD_DATA_SIZE_STR) { + int len; + char *str; + + switch (argv[3][0]) { + case '+': + len = strlen(aval.sval) + strlen(bval.sval) + 1; + str = malloc(len); + if (!str) { + printf("Out of memory\n"); + ret = CMD_RET_FAILURE; + } else { + /* These were copied out and checked earlier */ + strcpy(str, aval.sval); + strcat(str, bval.sval); + ret = env_set(argv[1], str); + if (ret) + printf("Could not set var\n"); + free(str); + } + break; + default: + printf("invalid op\n"); + ret = 1; + } + } else { ulong a = aval.ival; ulong b = bval.ival; @@ -424,15 +486,21 @@ static int do_setexpr(struct cmd_tbl *cmdtp, int flag, int argc, env_set_hex(argv[1], value); } - return 0; + if (w == CMD_DATA_SIZE_STR) { + free(aval.sval); + free(bval.sval); + } + + return ret; } U_BOOT_CMD( setexpr, 6, 0, do_setexpr, "set environment variable as the result of eval expression", - "[.b, .w, .l] name [*]value1 [*]value2\n" + "[.b, .w, .l, .s] name [*]value1 [*]value2\n" " - set environment variable 'name' to the result of the evaluated\n" " expression specified by . can be &, |, ^, +, -, *, /, %\n" + " (for strings only + is supported)\n" " size argument is only meaningful if value1 and/or value2 are\n" " memory addresses (*)\n" "setexpr[.b, .w, .l] name [*]value\n" diff --git a/test/cmd/setexpr.c b/test/cmd/setexpr.c index 2a897efd9b..fd6d869c0e 100644 --- a/test/cmd/setexpr.c +++ b/test/cmd/setexpr.c @@ -287,6 +287,92 @@ static int setexpr_test_backref(struct unit_test_state *uts) } SETEXPR_TEST(setexpr_test_backref, UT_TESTF_CONSOLE_REC); +/* Test 'setexpr' command with setting strings */ +static int setexpr_test_str(struct unit_test_state *uts) +{ + ulong start_mem; + char *buf; + + buf = map_sysmem(0, BUF_SIZE); + memset(buf, '\xff', BUF_SIZE); + + /* + * Set 'fred' to the same length as we expect to get below, to avoid a + * new allocation in 'setexpr'. That way we can check for memory leaks. + */ + ut_assertok(env_set("fred", "x")); + start_mem = ut_check_free(); + strcpy(buf, "hello"); + ut_asserteq(1, run_command("setexpr.s fred 0", 0)); + ut_assertok(ut_check_delta(start_mem)); + + start_mem = ut_check_free(); + ut_assertok(env_set("fred", "12345")); + ut_assertok(run_command("setexpr.s fred *0", 0)); + ut_asserteq_str("hello", env_get("fred")); + ut_assertok(ut_check_delta(start_mem)); + + unmap_sysmem(buf); + + return 0; +} +SETEXPR_TEST(setexpr_test_str, UT_TESTF_CONSOLE_REC); + + +/* Test 'setexpr' command with concatenating strings */ +static int setexpr_test_str_oper(struct unit_test_state *uts) +{ + ulong start_mem; + char *buf; + + buf = map_sysmem(0, BUF_SIZE); + memset(buf, '\xff', BUF_SIZE); + strcpy(buf, "hello"); + strcpy(buf + 0x10, " there"); + + ut_assertok(console_record_reset_enable()); + start_mem = ut_check_free(); + ut_asserteq(1, run_command("setexpr.s fred *0 * *10", 0)); + ut_assertok(ut_check_delta(start_mem)); + ut_assert_nextline("invalid op"); + ut_assert_console_end(); + + /* + * Set 'fred' to the same length as we expect to get below, to avoid a + * new allocation in 'setexpr'. That way we can check for memory leaks. + */ + ut_assertok(env_set("fred", "12345012345")); + start_mem = ut_check_free(); + ut_assertok(run_command("setexpr.s fred *0 + *10", 0)); + ut_asserteq_str("hello there", env_get("fred")); + ut_assertok(ut_check_delta(start_mem)); + + unmap_sysmem(buf); + + return 0; +} +SETEXPR_TEST(setexpr_test_str_oper, UT_TESTF_CONSOLE_REC); + +/* Test 'setexpr' command with a string that is too long */ +static int setexpr_test_str_long(struct unit_test_state *uts) +{ + const int size = 128 << 10; /* setexpr strings are a max of 64KB */ + char *buf, *val; + + buf = map_sysmem(0, size); + memset(buf, 'a', size); + + /* String should be truncated to 64KB */ + ut_assertok(run_command("setexpr.s fred *0", 0)); + val = env_get("fred"); + ut_asserteq(64 << 10, strlen(val)); + + unmap_sysmem(buf); + + return 0; +} +SETEXPR_TEST(setexpr_test_str_long, UT_TESTF_CONSOLE_REC); + int do_ut_setexpr(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct unit_test *tests = ll_entry_start(struct unit_test, From e1864db60d3a4281f440ca4230b4e810bde0fb99 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Fri, 23 Oct 2020 12:58:27 +0200 Subject: [PATCH 15/23] net: sntp: remove CONFIG_TIMESTAMP constraint CONFIG_TIMESTAMP is not related to the RTC drivers. It does not make any sense to let the updating of the RTC by the sntp command depend on it. Drop the CONFIG_TIMESTAMP checks. Furthermore function dm_rtc_set() is enabled by CONFIG_DM_RTC. There is no reason to require CONFIG_CMD_DATE when using a driver model RTC. The UEFI sub-system can consume the RTC functions even if there is not date command. Only check CONFIG_CMD_DATE when using a non-driver model RTC. Signed-off-by: Heinrich Schuchardt --- net/sntp.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/net/sntp.c b/net/sntp.c index d5d5671933..dac0f8ceea 100644 --- a/net/sntp.c +++ b/net/sntp.c @@ -57,18 +57,15 @@ static void sntp_timeout_handler(void) static void sntp_handler(uchar *pkt, unsigned dest, struct in_addr sip, unsigned src, unsigned len) { -#ifdef CONFIG_TIMESTAMP struct sntp_pkt_t *rpktp = (struct sntp_pkt_t *)pkt; struct rtc_time tm; ulong seconds; -#endif debug("%s\n", __func__); if (dest != sntp_our_port) return; -#ifdef CONFIG_TIMESTAMP /* * As the RTC's used in U-Boot support second resolution only * we simply ignore the sub-second field. @@ -76,8 +73,7 @@ static void sntp_handler(uchar *pkt, unsigned dest, struct in_addr sip, memcpy(&seconds, &rpktp->transmit_timestamp, sizeof(ulong)); rtc_to_tm(ntohl(seconds) - 2208988800UL + net_ntp_time_offset, &tm); -#if defined(CONFIG_CMD_DATE) -# ifdef CONFIG_DM_RTC +#ifdef CONFIG_DM_RTC struct udevice *dev; int ret; @@ -86,14 +82,12 @@ static void sntp_handler(uchar *pkt, unsigned dest, struct in_addr sip, printf("SNTP: cannot find RTC: err=%d\n", ret); else dm_rtc_set(dev, &tm); -# else +#elif defined(CONFIG_CMD_DATE) rtc_set(&tm); -# endif #endif printf("Date: %4d-%02d-%02d Time: %2d:%02d:%02d\n", tm.tm_year, tm.tm_mon, tm.tm_mday, tm.tm_hour, tm.tm_min, tm.tm_sec); -#endif net_set_state(NETLOOP_SUCCESS); } From 310f285e788dbc77fe804261ff5d016cd56ad523 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Mon, 26 Oct 2020 09:31:41 +0100 Subject: [PATCH 16/23] hush: Remove default CONFIG_SYS_PROMPT_HUSH_PS2 setting from board files There is no reason to define default option for this macro which is already done in common/cli_hush.c. 87 #ifndef CONFIG_SYS_PROMPT_HUSH_PS2 88 #define CONFIG_SYS_PROMPT_HUSH_PS2 "> " 89 #endif Signed-off-by: Patrick Delaunay Acked-by: Oleksandr Andrushchenko --- include/configs/apalis-imx8.h | 1 - include/configs/colibri-imx8x.h | 1 - include/configs/imx8mm_beacon.h | 1 - include/configs/imx8mm_evk.h | 1 - include/configs/imx8mn_evk.h | 1 - include/configs/imx8mp_evk.h | 1 - include/configs/imx8mq_evk.h | 1 - include/configs/imx8mq_phanbell.h | 1 - include/configs/s5p4418_nanopi2.h | 4 ---- include/configs/verdin-imx8mm.h | 1 - include/configs/xenguest_arm64.h | 1 - 11 files changed, 14 deletions(-) diff --git a/include/configs/apalis-imx8.h b/include/configs/apalis-imx8.h index db4e9011c0..b474b2f522 100644 --- a/include/configs/apalis-imx8.h +++ b/include/configs/apalis-imx8.h @@ -98,7 +98,6 @@ #define PHYS_SDRAM_2_SIZE SZ_2G /* 2 GB */ /* Monitor Command Prompt */ -#define CONFIG_SYS_PROMPT_HUSH_PS2 "> " #define CONFIG_SYS_CBSIZE SZ_2K #define CONFIG_SYS_MAXARGS 64 #define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE diff --git a/include/configs/colibri-imx8x.h b/include/configs/colibri-imx8x.h index 29a37ed44f..fc2c191594 100644 --- a/include/configs/colibri-imx8x.h +++ b/include/configs/colibri-imx8x.h @@ -132,7 +132,6 @@ #define PHYS_SDRAM_2_SIZE 0x00000000 /* 0 GB */ /* Monitor Command Prompt */ -#define CONFIG_SYS_PROMPT_HUSH_PS2 "> " #define CONFIG_SYS_CBSIZE SZ_2K #define CONFIG_SYS_MAXARGS 64 #define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE diff --git a/include/configs/imx8mm_beacon.h b/include/configs/imx8mm_beacon.h index 3c9541187f..9a93dba1c5 100644 --- a/include/configs/imx8mm_beacon.h +++ b/include/configs/imx8mm_beacon.h @@ -119,7 +119,6 @@ #define CONFIG_MXC_UART_BASE UART2_BASE_ADDR /* Monitor Command Prompt */ -#define CONFIG_SYS_PROMPT_HUSH_PS2 "> " #define CONFIG_SYS_CBSIZE 2048 #define CONFIG_SYS_MAXARGS 64 #define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE diff --git a/include/configs/imx8mm_evk.h b/include/configs/imx8mm_evk.h index 83521ad401..92eb85553e 100644 --- a/include/configs/imx8mm_evk.h +++ b/include/configs/imx8mm_evk.h @@ -120,7 +120,6 @@ #define CONFIG_MXC_UART_BASE UART2_BASE_ADDR /* Monitor Command Prompt */ -#define CONFIG_SYS_PROMPT_HUSH_PS2 "> " #define CONFIG_SYS_CBSIZE 2048 #define CONFIG_SYS_MAXARGS 64 #define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE diff --git a/include/configs/imx8mn_evk.h b/include/configs/imx8mn_evk.h index a6333085fe..cda8fc2ef7 100644 --- a/include/configs/imx8mn_evk.h +++ b/include/configs/imx8mn_evk.h @@ -124,7 +124,6 @@ #define CONFIG_MXC_UART_BASE UART2_BASE_ADDR /* Monitor Command Prompt */ -#define CONFIG_SYS_PROMPT_HUSH_PS2 "> " #define CONFIG_SYS_CBSIZE 2048 #define CONFIG_SYS_MAXARGS 64 #define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE diff --git a/include/configs/imx8mp_evk.h b/include/configs/imx8mp_evk.h index 8253c6aa2f..92091dfd6b 100644 --- a/include/configs/imx8mp_evk.h +++ b/include/configs/imx8mp_evk.h @@ -135,7 +135,6 @@ #define CONFIG_MXC_UART_BASE UART2_BASE_ADDR /* Monitor Command Prompt */ -#define CONFIG_SYS_PROMPT_HUSH_PS2 "> " #define CONFIG_SYS_CBSIZE 2048 #define CONFIG_SYS_MAXARGS 64 #define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE diff --git a/include/configs/imx8mq_evk.h b/include/configs/imx8mq_evk.h index 3f9a3bc100..96bfff749c 100644 --- a/include/configs/imx8mq_evk.h +++ b/include/configs/imx8mq_evk.h @@ -175,7 +175,6 @@ /* Monitor Command Prompt */ #undef CONFIG_SYS_PROMPT #define CONFIG_SYS_PROMPT "u-boot=> " -#define CONFIG_SYS_PROMPT_HUSH_PS2 "> " #define CONFIG_SYS_CBSIZE 1024 #define CONFIG_SYS_MAXARGS 64 #define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE diff --git a/include/configs/imx8mq_phanbell.h b/include/configs/imx8mq_phanbell.h index e8b65a4ba5..66c2c3a8d8 100644 --- a/include/configs/imx8mq_phanbell.h +++ b/include/configs/imx8mq_phanbell.h @@ -169,7 +169,6 @@ #define CONFIG_MXC_UART_BASE UART1_BASE_ADDR /* Monitor Command Prompt */ -#define CONFIG_SYS_PROMPT_HUSH_PS2 "> " #define CONFIG_SYS_CBSIZE 1024 #define CONFIG_SYS_MAXARGS 64 #define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE diff --git a/include/configs/s5p4418_nanopi2.h b/include/configs/s5p4418_nanopi2.h index 6dd1f3bc04..1e2180b970 100644 --- a/include/configs/s5p4418_nanopi2.h +++ b/include/configs/s5p4418_nanopi2.h @@ -102,10 +102,6 @@ /* Boot Argument Buffer Size */ #define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE -#ifdef CONFIG_HUSH_PARSER -#define CONFIG_SYS_PROMPT_HUSH_PS2 "> " -#endif - /*----------------------------------------------------------------------- * Etc Command definition */ diff --git a/include/configs/verdin-imx8mm.h b/include/configs/verdin-imx8mm.h index fd8405433d..4751bf5a5a 100644 --- a/include/configs/verdin-imx8mm.h +++ b/include/configs/verdin-imx8mm.h @@ -98,7 +98,6 @@ #define CONFIG_MXC_UART_BASE UART1_BASE_ADDR /* Monitor Command Prompt */ -#define CONFIG_SYS_PROMPT_HUSH_PS2 "> " #define CONFIG_SYS_CBSIZE SZ_2K #define CONFIG_SYS_MAXARGS 64 #define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE diff --git a/include/configs/xenguest_arm64.h b/include/configs/xenguest_arm64.h index c44381e966..d76ce13d14 100644 --- a/include/configs/xenguest_arm64.h +++ b/include/configs/xenguest_arm64.h @@ -27,7 +27,6 @@ #define CONFIG_SYS_MALLOC_LEN (32 * 1024 * 1024) /* Monitor Command Prompt */ -#define CONFIG_SYS_PROMPT_HUSH_PS2 "> " #define CONFIG_SYS_CBSIZE 1024 #define CONFIG_SYS_MAXARGS 64 #define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE From 48aee0afb62ee4ce11dd882600cfbcda60efb563 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Mon, 26 Oct 2020 09:31:42 +0100 Subject: [PATCH 17/23] cmd: Kconfig: migrate CONFIG_SYS_PROMPT_HUSH_PS2 Move CONFIG_SYS_PROMPT_HUSH_PS2 in Kconfig, depending on CONFIG_HUSH_PARSER, and remove the default value defined in cli_hush.c under __U_BOOT__. Signed-off-by: Patrick Delaunay Reviewed-by: Simon Glass --- README | 7 ------- cmd/Kconfig | 9 +++++++++ common/cli_hush.c | 3 --- scripts/config_whitelist.txt | 1 - 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/README b/README index 7b73a1c973..5ca11f25ee 100644 --- a/README +++ b/README @@ -1925,13 +1925,6 @@ The following options need to be configured: try longer timeout such as #define CONFIG_NFS_TIMEOUT 10000UL -- Command Interpreter: - CONFIG_SYS_PROMPT_HUSH_PS2 - - This defines the secondary prompt string, which is - printed when the command interpreter needs more input - to complete a command. Usually "> ". - Note: In the current implementation, the local variables diff --git a/cmd/Kconfig b/cmd/Kconfig index 1595de999b..98cf543ead 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -55,6 +55,15 @@ config SYS_PROMPT This string is displayed in the command line to the left of the cursor. +config SYS_PROMPT_HUSH_PS2 + string "Hush shell secondary prompt" + depends on HUSH_PARSER + default "> " + help + This defines the secondary prompt string, which is + printed when the command interpreter needs more input + to complete a command. Usually "> ". + config SYS_XTRACE string "Command execution tracer" depends on CMDLINE diff --git a/common/cli_hush.c b/common/cli_hush.c index 66995c255b..b7f0f0ff41 100644 --- a/common/cli_hush.c +++ b/common/cli_hush.c @@ -84,9 +84,6 @@ #include #include #include /* find_cmd */ -#ifndef CONFIG_SYS_PROMPT_HUSH_PS2 -#define CONFIG_SYS_PROMPT_HUSH_PS2 "> " -#endif #endif #ifndef __U_BOOT__ #include /* isalpha, isdigit */ diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt index 8b4fcba395..31acc36d4c 100644 --- a/scripts/config_whitelist.txt +++ b/scripts/config_whitelist.txt @@ -3508,7 +3508,6 @@ CONFIG_SYS_POST_WATCHDOG CONFIG_SYS_POST_WORD_ADDR CONFIG_SYS_PPC_DDR_WIMGE CONFIG_SYS_PQSPAR -CONFIG_SYS_PROMPT_HUSH_PS2 CONFIG_SYS_PSDPAR CONFIG_SYS_PSSR_VAL CONFIG_SYS_PTCPAR From 90534536a3a13672305ae4ff4ffcd4df9d7a4187 Mon Sep 17 00:00:00 2001 From: Robert Marko Date: Wed, 28 Oct 2020 13:56:23 +0100 Subject: [PATCH 18/23] IPQ40xx: clk: use dev_read_addr() Lets convert the driver to use dev_read_addr() instead of the devfdt_get_addr(). While we are here, lets also alphabetise the includes. Signed-off-by: Robert Marko Cc: Luka Perkov --- arch/arm/mach-ipq40xx/clock-ipq4019.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-ipq40xx/clock-ipq4019.c b/arch/arm/mach-ipq40xx/clock-ipq4019.c index 31ae9719e8..1f385665cc 100644 --- a/arch/arm/mach-ipq40xx/clock-ipq4019.c +++ b/arch/arm/mach-ipq40xx/clock-ipq4019.c @@ -8,8 +8,8 @@ * */ -#include #include +#include #include #include @@ -35,7 +35,7 @@ static int msm_clk_probe(struct udevice *dev) { struct msm_clk_priv *priv = dev_get_priv(dev); - priv->base = devfdt_get_addr(dev); + priv->base = dev_read_addr(dev); if (priv->base == FDT_ADDR_T_NONE) return -EINVAL; From 164cc98a7b9a8962a69cf4ae43bacb35eb98f51d Mon Sep 17 00:00:00 2001 From: Robert Marko Date: Wed, 28 Oct 2020 13:56:24 +0100 Subject: [PATCH 19/23] IPQ40xx: clk: drop breaks in switch case There is no point in having break statements in the switch case as there is already a return before break. So lets drop them from the driver. Signed-off-by: Robert Marko Cc: Luka Perkov --- arch/arm/mach-ipq40xx/clock-ipq4019.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/arch/arm/mach-ipq40xx/clock-ipq4019.c b/arch/arm/mach-ipq40xx/clock-ipq4019.c index 1f385665cc..ac2b830353 100644 --- a/arch/arm/mach-ipq40xx/clock-ipq4019.c +++ b/arch/arm/mach-ipq40xx/clock-ipq4019.c @@ -25,7 +25,6 @@ ulong msm_set_rate(struct clk *clk, ulong rate) case GCC_BLSP1_UART1_APPS_CLK: /*UART1*/ /* This clock is already initialized by SBL1 */ return 0; - break; default: return 0; } @@ -53,11 +52,9 @@ static int msm_enable(struct clk *clk) case GCC_BLSP1_QUP1_SPI_APPS_CLK: /*SPI1*/ /* This clock is already initialized by SBL1 */ return 0; - break; case GCC_PRNG_AHB_CLK: /*PRNG*/ /* This clock is already initialized by SBL1 */ return 0; - break; default: return 0; } From 20476b51aa4d77c774f14671a185b5ac94f86c49 Mon Sep 17 00:00:00 2001 From: Robert Marko Date: Wed, 28 Oct 2020 13:56:25 +0100 Subject: [PATCH 20/23] IPQ40xx: clk: dont always return 0 Currently the driver will go through the clock ID-s and set/enable them as needed. But if the ID is unknown it will fall through the switch case to the default case which will always return 0. This is not correct and default cases should return a error code since clock ID is unknown. So lets return -EINVAL instead. Signed-off-by: Robert Marko Cc: Luka Perkov --- arch/arm/mach-ipq40xx/clock-ipq4019.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-ipq40xx/clock-ipq4019.c b/arch/arm/mach-ipq40xx/clock-ipq4019.c index ac2b830353..7308563ad1 100644 --- a/arch/arm/mach-ipq40xx/clock-ipq4019.c +++ b/arch/arm/mach-ipq40xx/clock-ipq4019.c @@ -26,7 +26,7 @@ ulong msm_set_rate(struct clk *clk, ulong rate) /* This clock is already initialized by SBL1 */ return 0; default: - return 0; + return -EINVAL; } } @@ -56,7 +56,7 @@ static int msm_enable(struct clk *clk) /* This clock is already initialized by SBL1 */ return 0; default: - return 0; + return -EINVAL; } } From a282ada1987ca85826e6f47bb6e3c48a17e11098 Mon Sep 17 00:00:00 2001 From: Robert Marko Date: Wed, 28 Oct 2020 13:56:26 +0100 Subject: [PATCH 21/23] IPQ40xx: clk: add USB clock handling USB clocks were completely forgotten as driver would always return 0 even if clock ID was unknown. This behaviour changed with "IPQ40xx: clk: dont always return 0" and this will now causes the USB-s to fail probing as clock enable will return -EINVAL. So to fix that lets add all of the USB clocks to the driver. Fixes: 430e1dcf ("IPQ40xx: Add USB nodes") Signed-off-by: Robert Marko Cc: Luka Perkov --- arch/arm/mach-ipq40xx/clock-ipq4019.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/arch/arm/mach-ipq40xx/clock-ipq4019.c b/arch/arm/mach-ipq40xx/clock-ipq4019.c index 7308563ad1..a3f872947d 100644 --- a/arch/arm/mach-ipq40xx/clock-ipq4019.c +++ b/arch/arm/mach-ipq40xx/clock-ipq4019.c @@ -55,6 +55,14 @@ static int msm_enable(struct clk *clk) case GCC_PRNG_AHB_CLK: /*PRNG*/ /* This clock is already initialized by SBL1 */ return 0; + case GCC_USB3_MASTER_CLK: + case GCC_USB3_SLEEP_CLK: + case GCC_USB3_MOCK_UTMI_CLK: + case GCC_USB2_MASTER_CLK: + case GCC_USB2_SLEEP_CLK: + case GCC_USB2_MOCK_UTMI_CLK: + /* These clocks is already initialized by SBL1 */ + return 0; default: return -EINVAL; } From 7d5b1bf6b84489cbbafd6dc679fdac2eda876c74 Mon Sep 17 00:00:00 2001 From: Philippe Reynes Date: Thu, 29 Oct 2020 18:50:29 +0100 Subject: [PATCH 22/23] spl: spl_fit.c: enable check of signature for config node in spl/tpl This commit add the support of signature check for config node in spl/tpl when the function spl_load_simple_fit is used. Signed-off-by: Philippe Reynes Reviewed-by: Simon Glass --- common/spl/spl_fit.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index 6418062b93..2fbee4f19f 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -558,6 +558,16 @@ int spl_load_simple_fit(struct spl_image_info *spl_image, if (spl_load_simple_fit_skip_processing()) return 0; + if (IS_ENABLED(CONFIG_SPL_FIT_SIGNATURE)) { + int conf_offset = fit_find_config_node(fit); + + printf("## Checking hash(es) for config %s ... ", + fit_get_name(fit, conf_offset, NULL)); + if (fit_config_verify(fit, conf_offset)) + return -EPERM; + puts("OK\n"); + } + /* find the node holding the images information */ images = fdt_path_offset(fit, FIT_IMAGES_PATH); if (images < 0) { From 7292a18256252e41970eceacd5ccd3ed79ea64f0 Mon Sep 17 00:00:00 2001 From: Holger Brunck Date: Fri, 30 Oct 2020 12:45:49 +0100 Subject: [PATCH 23/23] km/arm: coding style clean up Address most of the checkpatch issues we found in km_arm and common km code. CC: Stefan Roese CC: Valentin Longchamp Signed-off-by: Holger Brunck Reviewed-by: Stefan Roese --- board/keymile/common/common.c | 44 ++++++++------- board/keymile/common/ivm.c | 84 ++++++++++++++--------------- board/keymile/km_arm/fpga_config.c | 16 +++--- board/keymile/km_arm/km_arm.c | 41 +++++++------- include/configs/km/keymile-common.h | 4 +- include/configs/km/km_arm.h | 2 +- 6 files changed, 96 insertions(+), 95 deletions(-) diff --git a/board/keymile/common/common.c b/board/keymile/common/common.c index 03c7ce9da7..df507e2790 100644 --- a/board/keymile/common/common.c +++ b/board/keymile/common/common.c @@ -56,7 +56,7 @@ int set_km_env(void) /* try to read rootfssize (ram image) from environment */ p = env_get("rootfssize"); - if (p != NULL) + if (p) strict_strtoul(p, 16, &rootfssize); pram = (rootfssize + CONFIG_KM_RESERVED_PRAM + CONFIG_KM_PHRAM + CONFIG_KM_PNVRAM) / 0x400; @@ -165,7 +165,7 @@ static int do_setboardid(struct cmd_tbl *cmdtp, int flag, int argc, char *p; p = get_local_var("IVM_BoardId"); - if (p == NULL) { + if (!p) { printf("can't get the IVM_Boardid\n"); return 1; } @@ -174,7 +174,7 @@ static int do_setboardid(struct cmd_tbl *cmdtp, int flag, int argc, printf("set boardid=%s\n", buf); p = get_local_var("IVM_HWKey"); - if (p == NULL) { + if (!p) { printf("can't get the IVM_HWKey\n"); return 1; } @@ -186,8 +186,8 @@ static int do_setboardid(struct cmd_tbl *cmdtp, int flag, int argc, return 0; } -U_BOOT_CMD(km_setboardid, 1, 0, do_setboardid, "setboardid", "read out bid and " - "hwkey from IVM and set in environment"); +U_BOOT_CMD(km_setboardid, 1, 0, do_setboardid, "setboardid", + "read out bid and hwkey from IVM and set in environment"); /* * command km_checkbidhwk @@ -218,14 +218,14 @@ static int do_checkboardidhwk(struct cmd_tbl *cmdtp, int flag, int argc, * already stored in the local hush variables */ p = get_local_var("IVM_BoardId"); - if (p == NULL) { + if (!p) { printf("can't get the IVM_Boardid\n"); return 1; } rc = strict_strtoul(p, 16, &ivmbid); p = get_local_var("IVM_HWKey"); - if (p == NULL) { + if (!p) { printf("can't get the IVM_HWKey\n"); return 1; } @@ -238,10 +238,10 @@ static int do_checkboardidhwk(struct cmd_tbl *cmdtp, int flag, int argc, /* now try to read values from environment if available */ p = env_get("boardid"); - if (p != NULL) + if (p) rc = strict_strtoul(p, 16, &envbid); p = env_get("hwkey"); - if (p != NULL) + if (p) rc = strict_strtoul(p, 16, &envhwkey); if (rc != 0) { @@ -263,9 +263,8 @@ static int do_checkboardidhwk(struct cmd_tbl *cmdtp, int flag, int argc, if (verbose) { printf("IVM_BoardId: %ld, IVM_HWKey=%ld\n", - ivmbid, ivmhwkey); - printf("boardIdHwKeyList: %s\n", - bidhwklist); + ivmbid, ivmhwkey); + printf("boardIdHwKeyList: %s\n", bidhwklist); } while (!found) { /* loop over each bid/hwkey pair in the list */ @@ -291,13 +290,13 @@ static int do_checkboardidhwk(struct cmd_tbl *cmdtp, int flag, int argc, while (*rest && !isxdigit(*rest)) rest++; } - if ((!bid) || (!hwkey)) { + if (!bid || !hwkey) { /* end of list */ break; } if (verbose) { printf("trying bid=0x%lX, hwkey=%ld\n", - bid, hwkey); + bid, hwkey); } /* * Compare the values of the found entry in the @@ -305,7 +304,7 @@ static int do_checkboardidhwk(struct cmd_tbl *cmdtp, int flag, int argc, * in the inventory eeprom. If they are equal * set the values in environment variables. */ - if ((bid == ivmbid) && (hwkey == ivmhwkey)) { + if (bid == ivmbid && hwkey == ivmhwkey) { char buf[10]; found = 1; @@ -321,12 +320,12 @@ static int do_checkboardidhwk(struct cmd_tbl *cmdtp, int flag, int argc, } /* compare now the values */ - if ((ivmbid == envbid) && (ivmhwkey == envhwkey)) { + if (ivmbid == envbid && ivmhwkey == envhwkey) { printf("boardid=0x%3lX, hwkey=%ld\n", envbid, envhwkey); rc = 0; /* match */ } else { printf("Error: env boardid=0x%3lX, hwkey=%ld\n", envbid, - envhwkey); + envhwkey); printf(" IVM bId=0x%3lX, hwKey=%ld\n", ivmbid, ivmhwkey); rc = 1; /* don't match */ } @@ -334,10 +333,8 @@ static int do_checkboardidhwk(struct cmd_tbl *cmdtp, int flag, int argc, } U_BOOT_CMD(km_checkbidhwk, 2, 0, do_checkboardidhwk, - "check boardid and hwkey", - "[v]\n - check environment parameter "\ - "\"boardIdListHex\" against stored boardid and hwkey "\ - "from the IVM\n v: verbose output" + "check boardid and hwkey", + "[v]\n - check environment parameter \"boardIdListHex\" against stored boardid and hwkey from the IVM\n v: verbose output" ); /* @@ -356,6 +353,7 @@ static int do_checktestboot(struct cmd_tbl *cmdtp, int flag, int argc, #if defined(CONFIG_POST) testpin = post_hotkeys_pressed(); #endif + s = env_get("test_bank"); /* when test_bank is not set, act as if testpin is not asserted */ testboot = (testpin != 0) && (s); @@ -370,6 +368,6 @@ static int do_checktestboot(struct cmd_tbl *cmdtp, int flag, int argc, } U_BOOT_CMD(km_checktestboot, 2, 0, do_checktestboot, - "check if testpin is asserted", - "[v]\n v - verbose output" + "check if testpin is asserted", + "[v]\n v - verbose output" ); diff --git a/board/keymile/common/ivm.c b/board/keymile/common/ivm.c index 60b89fe348..e989bf609f 100644 --- a/board/keymile/common/ivm.c +++ b/board/keymile/common/ivm.c @@ -46,28 +46,27 @@ static int ivm_set_value(char *name, char *value) { char tempbuf[256]; - if (value != NULL) { + if (value) { sprintf(tempbuf, "%s=%s", name, value); return set_local_var(tempbuf, 0); - } else { - unset_local_var(name); } + unset_local_var(name); return 0; } static int ivm_get_value(unsigned char *buf, int len, char *name, int off, - int check) + int check) { unsigned short val; unsigned char valbuf[30]; - if ((buf[off + 0] != buf[off + 2]) && - (buf[off + 2] != buf[off + 4])) { + if (buf[off + 0] != buf[off + 2] && + buf[off + 2] != buf[off + 4]) { printf("%s Error corrupted %s\n", __func__, name); val = -1; } else { val = buf[off + 0] + (buf[off + 1] << 8); - if ((val == 0) && (check == 1)) + if (val == 0 && check == 1) val = -1; } sprintf((char *)valbuf, "%x", val); @@ -98,9 +97,9 @@ static char convert_char(char c) } static int ivm_findinventorystring(int type, - unsigned char *const string, - unsigned long maxlen, - unsigned char *buf) + unsigned char *const string, + unsigned long maxlen, + unsigned char *buf) { int xcode = 0; unsigned long cr = 0; @@ -133,12 +132,12 @@ static int ivm_findinventorystring(int type, */ if (addr < INVENTORYDATASIZE) { /* Copy the IVM string in the corresponding string */ - for (; (buf[addr] != '\r') && - ((buf[addr] != ';') || (!stop)) && - (size < (maxlen - 1) && - (addr < INVENTORYDATASIZE)); addr++) { + for (; (buf[addr] != '\r') && + ((buf[addr] != ';') || (!stop)) && + (size < (maxlen - 1) && + (addr < INVENTORYDATASIZE)); addr++) { size += sprintf((char *)string + size, "%c", - convert_char (buf[addr])); + convert_char (buf[addr])); } /* @@ -176,12 +175,12 @@ static int ivm_check_crc(unsigned char *buf, int block) unsigned long crceeprom; crc = ivm_calc_crc(buf, CONFIG_SYS_IVM_EEPROM_PAGE_LEN - 2); - crceeprom = (buf[CONFIG_SYS_IVM_EEPROM_PAGE_LEN - 1] + \ + crceeprom = (buf[CONFIG_SYS_IVM_EEPROM_PAGE_LEN - 1] + buf[CONFIG_SYS_IVM_EEPROM_PAGE_LEN - 2] * 256); if (crc != crceeprom) { if (block == 0) - printf("Error CRC Block: %d EEprom: calculated: \ - %lx EEprom: %lx\n", block, crc, crceeprom); + printf("Error CRC Block: %d EEprom: calculated: %lx EEprom: %lx\n", + block, crc, crceeprom); return -1; } return 0; @@ -189,7 +188,7 @@ static int ivm_check_crc(unsigned char *buf, int block) /* take care of the possible MAC address offset and the IVM content offset */ static int process_mac(unsigned char *valbuf, unsigned char *buf, - int offset, bool unique) + int offset, bool unique) { unsigned char mac[6]; unsigned long val = (buf[4] << 16) + (buf[5] << 8) + buf[6]; @@ -197,9 +196,9 @@ static int process_mac(unsigned char *valbuf, unsigned char *buf, /* use an intermediate buffer, to not change IVM content * MAC address is at offset 1 */ - memcpy(mac, buf+1, 6); + memcpy(mac, buf + 1, 6); - /* MAC adress can be set to locally administred, this is only allowed + /* MAC address can be set to locally administred, this is only allowed * for interfaces which have now connection to the outside. For these * addresses we need to set the second bit in the first byte. */ @@ -222,7 +221,7 @@ static int ivm_analyze_block2(unsigned char *buf, int len) unsigned char valbuf[MAC_STR_SZ]; unsigned long count; - /* IVM_MAC Adress begins at offset 1 */ + /* IVM_MAC Address begins at offset 1 */ sprintf((char *)valbuf, "%pM", buf + 1); ivm_set_value("IVM_MacAddress", (char *)valbuf); /* IVM_MacCount */ @@ -247,9 +246,9 @@ int ivm_analyze_eeprom(unsigned char *buf, int len) return -1; ivm_get_value(buf, CONFIG_SYS_IVM_EEPROM_PAGE_LEN, - "IVM_BoardId", 0, 1); + "IVM_BoardId", 0, 1); val = ivm_get_value(buf, CONFIG_SYS_IVM_EEPROM_PAGE_LEN, - "IVM_HWKey", 6, 1); + "IVM_HWKey", 6, 1); if (val != 0xffff) { sprintf((char *)valbuf, "%x", ((val / 100) % 10)); ivm_set_value("IVM_HWVariant", (char *)valbuf); @@ -257,7 +256,7 @@ int ivm_analyze_eeprom(unsigned char *buf, int len) ivm_set_value("IVM_HWVersion", (char *)valbuf); } ivm_get_value(buf, CONFIG_SYS_IVM_EEPROM_PAGE_LEN, - "IVM_Functions", 12, 0); + "IVM_Functions", 12, 0); GET_STRING("IVM_Symbol", IVM_POS_SYMBOL_ONLY, 8) GET_STRING("IVM_DeviceName", IVM_POS_SHORT_TEXT, 64) @@ -269,7 +268,7 @@ int ivm_analyze_eeprom(unsigned char *buf, int len) while (i < len) { if (tmp[i] == ';') { ivm_set_value("IVM_ShortText", - (char *)&tmp[i + 1]); + (char *)&tmp[i + 1]); break; } i++; @@ -292,7 +291,7 @@ int ivm_analyze_eeprom(unsigned char *buf, int len) if (ivm_check_crc(&buf[CONFIG_SYS_IVM_EEPROM_PAGE_LEN * 2], 2) != 0) return 0; ivm_analyze_block2(&buf[CONFIG_SYS_IVM_EEPROM_PAGE_LEN * 2], - CONFIG_SYS_IVM_EEPROM_PAGE_LEN); + CONFIG_SYS_IVM_EEPROM_PAGE_LEN); return 0; } @@ -305,22 +304,23 @@ static int ivm_populate_env(unsigned char *buf, int len, int mac_address_offset) /* do we have the page 2 filled ? if not return */ if (ivm_check_crc(buf, 2)) return 0; - page2 = &buf[CONFIG_SYS_IVM_EEPROM_PAGE_LEN*2]; + page2 = &buf[CONFIG_SYS_IVM_EEPROM_PAGE_LEN * 2]; -#ifndef CONFIG_KMTEGR1 - /* if an offset is defined, add it */ - process_mac(valbuf, page2, mac_address_offset, true); - env_set((char *)"ethaddr", (char *)valbuf); -#else -/* KMTEGR1 has a special setup. eth0 has no connection to the outside and - * gets an locally administred MAC address, eth1 is the debug interface and - * gets the official MAC address from the IVM - */ - process_mac(valbuf, page2, mac_address_offset, false); - env_set((char *)"ethaddr", (char *)valbuf); - process_mac(valbuf, page2, mac_address_offset, true); - env_set((char *)"eth1addr", (char *)valbuf); -#endif + if (!IS_ENABLED(CONFIG_KMTEGR1)) { + /* if an offset is defined, add it */ + process_mac(valbuf, page2, mac_address_offset, true); + env_set((char *)"ethaddr", (char *)valbuf); + } else { + /* KMTEGR1 has a special setup. eth0 has no connection to the + * outside and gets an locally administred MAC address, eth1 is + * the debug interface and gets the official MAC address from + * the IVM + */ + process_mac(valbuf, page2, mac_address_offset, false); + env_set((char *)"ethaddr", (char *)valbuf); + process_mac(valbuf, page2, mac_address_offset, true); + env_set((char *)"eth1addr", (char *)valbuf); + } return 0; } diff --git a/board/keymile/km_arm/fpga_config.c b/board/keymile/km_arm/fpga_config.c index abb5b7d60d..839b162eea 100644 --- a/board/keymile/km_arm/fpga_config.c +++ b/board/keymile/km_arm/fpga_config.c @@ -40,14 +40,14 @@ static int boco_clear_bits(u8 reg, u8 flags) ret = i2c_read(BOCO_ADDR, reg, 1, ®val, 1); if (ret) { printf("%s: error reading the BOCO @%#x !!\n", - __func__, reg); + __func__, reg); return ret; } regval &= ~flags; ret = i2c_write(BOCO_ADDR, reg, 1, ®val, 1); if (ret) { printf("%s: error writing the BOCO @%#x !!\n", - __func__, reg); + __func__, reg); return ret; } @@ -63,14 +63,14 @@ static int boco_set_bits(u8 reg, u8 flags) ret = i2c_read(BOCO_ADDR, reg, 1, ®val, 1); if (ret) { printf("%s: error reading the BOCO @%#x !!\n", - __func__, reg); + __func__, reg); return ret; } regval |= flags; ret = i2c_write(BOCO_ADDR, reg, 1, ®val, 1); if (ret) { printf("%s: error writing the BOCO @%#x !!\n", - __func__, reg); + __func__, reg); return ret; } @@ -113,7 +113,8 @@ int trigger_fpga_config(void) skip = 0; #ifndef CONFIG_KM_FPGA_FORCE_CONFIG /* if the FPGA is already configured, we do not want to - * reconfigure it */ + * reconfigure it + */ skip = 0; if (fpga_done()) { printf("PCIe FPGA config: skipped\n"); @@ -179,7 +180,7 @@ int wait_for_fpga_config(void) ret = i2c_read(BOCO_ADDR, SPI_REG, 1, &spictrl, 1); if (ret) { printf("%s: error reading the BOCO spictrl !!\n", - __func__); + __func__); return ret; } if (timeout-- == 0) { @@ -235,7 +236,8 @@ int fpga_reset(void) #endif /* the FPGA was configured, we configure the BOCO2 so that the EEPROM - * is available from the Bobcat SPI bus */ + * is available from the Bobcat SPI bus + */ int toggle_eeprom_spi_bus(void) { int ret = 0; diff --git a/board/keymile/km_arm/km_arm.c b/board/keymile/km_arm/km_arm.c index 7d191ab860..60187bd8d2 100644 --- a/board/keymile/km_arm/km_arm.c +++ b/board/keymile/km_arm/km_arm.c @@ -53,9 +53,9 @@ DECLARE_GLOBAL_DATA_PTR; #define PHY_MARVELL_88E1118R_LED_CTRL_REG 0x0010 #define PHY_MARVELL_88E1118R_LED_CTRL_RESERVED 0x1000 -#define PHY_MARVELL_88E1118R_LED_CTRL_LED0_1000MB (0x7<<0) -#define PHY_MARVELL_88E1118R_LED_CTRL_LED1_ACT (0x3<<4) -#define PHY_MARVELL_88E1118R_LED_CTRL_LED2_LINK (0x0<<8) +#define PHY_MARVELL_88E1118R_LED_CTRL_LED0_1000MB (0x7 << 0) +#define PHY_MARVELL_88E1118R_LED_CTRL_LED1_ACT (0x3 << 4) +#define PHY_MARVELL_88E1118R_LED_CTRL_LED2_LINK (0x0 << 8) /* I/O pin to erase flash RGPP09 = MPP43 */ #define KM_FLASH_ERASE_ENABLE 43 @@ -169,6 +169,7 @@ static void set_bootcount_addr(void) { uchar buf[32]; unsigned int bootcountaddr; + bootcountaddr = gd->ram_size - BOOTCOUNT_ADDR; sprintf((char *)buf, "0x%x", bootcountaddr); env_set("bootcountaddr", (char *)buf); @@ -192,7 +193,7 @@ int board_early_init_f(void) /* set the 2 bitbang i2c pins as output gpios */ tmp = readl(MVEBU_GPIO0_BASE + 4); - writel(tmp & (~KM_KIRKWOOD_SOFT_I2C_GPIOS) , MVEBU_GPIO0_BASE + 4); + writel(tmp & (~KM_KIRKWOOD_SOFT_I2C_GPIOS), MVEBU_GPIO0_BASE + 4); #endif /* adjust SDRAM size for bank 0 */ mvebu_sdram_size_adjust(0); @@ -292,11 +293,11 @@ int mvebu_board_spi_release_bus(struct udevice *dev) #define PHY_LED_SEL_REG 0x18 #define PHY_LED0_LINK (0x5) -#define PHY_LED1_ACT (0x8<<4) -#define PHY_LED2_INT (0xe<<8) +#define PHY_LED1_ACT (0x8 << 4) +#define PHY_LED2_INT (0xe << 8) #define PHY_SPEC_CTRL_REG 0x1c -#define PHY_RGMII_CLK_STABLE (0x1<<10) -#define PHY_CLSA (0x1<<1) +#define PHY_RGMII_CLK_STABLE (0x1 << 10) +#define PHY_CLSA (0x1 << 1) /* Configure and enable MV88E3018 PHY */ void reset_phy(void) @@ -407,8 +408,8 @@ void reset_phy(void) return; /* check for Marvell 88E1118R Gigabit PHY (PIGGY3) */ - if ((oui == PHY_MARVELL_OUI) && - (model == PHY_MARVELL_88E1118R_MODEL)) { + if (oui == PHY_MARVELL_OUI && + model == PHY_MARVELL_88E1118R_MODEL) { /* set page register to 3 */ if (miiphy_write(name, CONFIG_PHY_BASE_ADR, PHY_MARVELL_PAGE_REG, @@ -438,7 +439,6 @@ void reset_phy(void) } #endif - #if defined(CONFIG_HUSH_INIT_VAR) int hush_init_var(void) { @@ -478,22 +478,23 @@ int get_scl(void) int post_hotkeys_pressed(void) { -#if defined(CONFIG_KM_COGE5UN) - return kw_gpio_get_value(KM_POST_EN_L); -#else - return !kw_gpio_get_value(KM_POST_EN_L); -#endif + if (IS_ENABLED(CONFIG_KM_COGE5UN)) + return kw_gpio_get_value(KM_POST_EN_L); + else + return !kw_gpio_get_value(KM_POST_EN_L); } ulong post_word_load(void) { - void* addr = (void *) (gd->ram_size - BOOTCOUNT_ADDR + POST_WORD_OFF); + void *addr = (void *)(gd->ram_size - BOOTCOUNT_ADDR + POST_WORD_OFF); + return in_le32(addr); } void post_word_store(ulong value) { - void* addr = (void *) (gd->ram_size - BOOTCOUNT_ADDR + POST_WORD_OFF); + void *addr = (void *)(gd->ram_size - BOOTCOUNT_ADDR + POST_WORD_OFF); + out_le32(addr, value); } @@ -502,14 +503,14 @@ int arch_memory_test_prepare(u32 *vstart, u32 *size, phys_addr_t *phys_offset) *vstart = CONFIG_SYS_SDRAM_BASE; /* we go up to relocation plus a 1 MB margin */ - *size = CONFIG_SYS_TEXT_BASE - (1<<20); + *size = CONFIG_SYS_TEXT_BASE - (1 << 20); return 0; } #endif #if defined(CONFIG_SYS_EEPROM_WREN) -int eeprom_write_enable(unsigned dev_addr, int state) +int eeprom_write_enable(unsigned int dev_addr, int state) { kw_gpio_set_value(KM_KIRKWOOD_ENV_WP, !state); diff --git a/include/configs/km/keymile-common.h b/include/configs/km/keymile-common.h index 851b13e063..e084862518 100644 --- a/include/configs/km/keymile-common.h +++ b/include/configs/km/keymile-common.h @@ -184,9 +184,9 @@ "cramfsloadfdt=" \ "cramfsload ${fdt_addr_r} " \ "fdt_0x${IVM_BoardId}_0x${IVM_HWKey}.dtb\0" \ - "fdt_addr_r="__stringify(CONFIG_KM_FDT_ADDR) "\0" \ + "fdt_addr_r=" __stringify(CONFIG_KM_FDT_ADDR) "\0" \ "init=/sbin/init-overlay.sh\0" \ - "load_addr_r="__stringify(CONFIG_KM_KERNEL_ADDR) "\0" \ + "load_addr_r=" __stringify(CONFIG_KM_KERNEL_ADDR) "\0" \ "load=tftpboot ${load_addr_r} ${u-boot}\0" \ "mtdids=" CONFIG_MTDIDS_DEFAULT "\0" \ "mtdparts=" CONFIG_MTDPARTS_DEFAULT "\0" \ diff --git a/include/configs/km/km_arm.h b/include/configs/km/km_arm.h index 98e0ce1c24..29060fa96b 100644 --- a/include/configs/km/km_arm.h +++ b/include/configs/km/km_arm.h @@ -48,7 +48,7 @@ " boardid=0x${IVM_BoardId} hwkey=0x${IVM_HWKey}" #define CONFIG_KM_DEF_ENV_CPU \ - "u-boot="CONFIG_HOSTNAME "/u-boot.kwb\0" \ + "u-boot=" CONFIG_HOSTNAME "/u-boot.kwb\0" \ CONFIG_KM_UPDATE_UBOOT \ "set_fdthigh=setenv fdt_high ${kernelmem}\0" \ "checkfdt=" \