From b41d4b83f0203be79a3bfa1e4bde316355c7f2a0 Mon Sep 17 00:00:00 2001 From: Sean Anderson Date: Sun, 2 Feb 2020 13:15:17 -0500 Subject: serial: Set baudrate on boot Currently, the baud rate is never set on boot. This works ok when a previous bootloader has configured the baudrate properly, or when the baudrate is set to a reasonable default in the serial driver's probe(). However, when this is not the case, we could be using a different baud rate than what was configured. Signed-off-by: Sean Anderson --- drivers/serial/serial-uclass.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index 30f9b8c..7703c67 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -162,6 +162,7 @@ int serial_init(void) #if CONFIG_IS_ENABLED(SERIAL_PRESENT) serial_find_console_or_panic(); gd->flags |= GD_FLG_SERIAL_READY; + serial_setbrg(); #endif return 0; -- cgit v1.1 From 8a770f9eb7be58439361e0ad30a0c122cd46dc7b Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 8 Feb 2020 07:53:10 -0700 Subject: sandbox: p2sb: Silence compiler warning Some compilers produce a warning about 'child' being used before init. Silence this by setting to NULL at the start. Signed-off-by: Simon Glass Reviewed-by: Bin Meng --- drivers/misc/p2sb_emul.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/misc/p2sb_emul.c b/drivers/misc/p2sb_emul.c index a6ee9a2..02f7a7e 100644 --- a/drivers/misc/p2sb_emul.c +++ b/drivers/misc/p2sb_emul.c @@ -215,7 +215,7 @@ static int sandbox_p2sb_emul_map_physmem(struct udevice *dev, void **ptrp) { struct p2sb_emul_priv *priv = dev_get_priv(dev); - struct udevice *child; + struct udevice *child = NULL; /* Silence compiler warning */ unsigned int offset; int barnum; int ret; -- cgit v1.1 From 42c64d1bc9d5d3c68f14b872ab71ec7d5fe97cbc Mon Sep 17 00:00:00 2001 From: Tom Rini Date: Tue, 11 Feb 2020 12:41:23 -0500 Subject: sandbox: Update PCI nodes in dts files The way the PCI nodes are written today causes a number of warnings if we stop disabling some of the warnings we pass to DTC. As these warnings aren't disabled in current Linux Kernel builds, we should aim to not disable them here either, so rewrite these slightly. Update the driver model doc as well. Cc: Simon Glass Signed-off-by: Tom Rini --- arch/sandbox/dts/sandbox.dts | 5 +++-- arch/sandbox/dts/sandbox.dtsi | 2 +- arch/sandbox/dts/sandbox64.dts | 5 +++-- arch/sandbox/dts/test.dts | 9 ++++++--- doc/driver-model/pci-info.rst | 10 +++++----- 5 files changed, 18 insertions(+), 13 deletions(-) diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts index 4dd82f6..2d7db02 100644 --- a/arch/sandbox/dts/sandbox.dts +++ b/arch/sandbox/dts/sandbox.dts @@ -10,7 +10,7 @@ aliases { i2c0 = &i2c_0; - pci0 = &pci; + pci0 = &pcic; rtc0 = &rtc_0; axi0 = &axi; spi0 = &spi; @@ -52,9 +52,10 @@ pinctrl-0 = <&pinctrl_i2c0>; }; - pci: pci-controller { + pcic: pci@0 { compatible = "sandbox,pci"; device_type = "pci"; + bus-range = <0x00 0xff>; #address-cells = <3>; #size-cells = <2>; ranges = <0x02000000 0 0x10000000 0x10000000 0 0x2000 diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi index 7cd56c1..e1f68cd 100644 --- a/arch/sandbox/dts/sandbox.dtsi +++ b/arch/sandbox/dts/sandbox.dtsi @@ -100,7 +100,7 @@ }; }; - pci-controller { + pci@0 { pci@1e,0 { compatible = "sandbox,pmc"; reg = <0xf000 0 0 0 0>; diff --git a/arch/sandbox/dts/sandbox64.dts b/arch/sandbox/dts/sandbox64.dts index 5c95cee..97e33f1 100644 --- a/arch/sandbox/dts/sandbox64.dts +++ b/arch/sandbox/dts/sandbox64.dts @@ -10,7 +10,7 @@ aliases { i2c0 = &i2c_0; - pci0 = &pci; + pci0 = &pcic; rtc0 = &rtc_0; axi0 = &axi; spi0 = &spi; @@ -47,9 +47,10 @@ pinctrl-0 = <&pinctrl_i2c0>; }; - pci: pci-controller { + pcic: pci@0 { compatible = "sandbox,pci"; device_type = "pci"; + bus-range = <0x00 0xff>; #address-cells = <3>; #size-cells = <2>; ranges = <0x02000000 0 0x10000000 0 0x10000000 0 0x2000 diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 4a27793..2338064 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -463,9 +463,10 @@ compatible = "sandbox,pch"; }; - pci0: pci-controller0 { + pci0: pci@0 { compatible = "sandbox,pci"; device_type = "pci"; + bus-range = <0x00 0xff>; #address-cells = <3>; #size-cells = <2>; ranges = <0x02000000 0 0x10000000 0x10000000 0 0x2000000 @@ -531,9 +532,10 @@ }; }; - pci1: pci-controller1 { + pci1: pci@1 { compatible = "sandbox,pci"; device_type = "pci"; + bus-range = <0x00 0xff>; #address-cells = <3>; #size-cells = <2>; ranges = <0x02000000 0 0x30000000 0x30000000 0 0x2000 @@ -546,9 +548,10 @@ }; }; - pci2: pci-controller2 { + pci2: pci@2 { compatible = "sandbox,pci"; device_type = "pci"; + bus-range = <0x00 0xff>; #address-cells = <3>; #size-cells = <2>; ranges = <0x02000000 0 0x50000000 0x50000000 0 0x2000 diff --git a/doc/driver-model/pci-info.rst b/doc/driver-model/pci-info.rst index 3c1b1ad..8b9faa1 100644 --- a/doc/driver-model/pci-info.rst +++ b/doc/driver-model/pci-info.rst @@ -12,10 +12,10 @@ Bus number 0 will need to be requested first, and the alias in the device tree file will point to the correct device:: aliases { - pci0 = &pci; + pci0 = &pcic; }; - pci: pci-controller { + pcic: pci@0 { compatible = "sandbox,pci"; ... }; @@ -138,7 +138,7 @@ be scanned as a PCI device, causing confusion. When this bus is scanned we will end up with something like this:: - `- * pci-controller @ 05c660c8, 0 + `- * pci@0 @ 05c660c8, 0 `- pci@1f,0 @ 05c661c8, 63488 `- emul@1f,0 @ 05c662c8 @@ -152,7 +152,7 @@ host controller node for this functionality to work. .. code-block:: none - pci1: pci-controller1 { + pci1: pci@1 { compatible = "sandbox,pci"; ... sandbox,dev-info = <0x08 0x00 0x1234 0x5678 @@ -166,6 +166,6 @@ fourth cells are PCI vendor ID and device ID respectively. When this bus is scanned we will end up with something like this:: - pci [ + ] pci_sandbo |-- pci-controller1 + pci [ + ] pci_sandbo |-- pci1 pci_emul [ ] sandbox_sw | |-- sandbox_swap_case_emul pci_emul [ ] sandbox_sw | `-- sandbox_swap_case_emul -- cgit v1.1 From 2960107a22b32f6e17794f5e56db718ab82c896f Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Fri, 14 Feb 2020 10:58:37 +0000 Subject: sandbox: also restore terminal settings when killed by SIGINT Hitting Ctrl-C is a documented way to exit the sandbox, but it is not actually equivalent to the reset command. The latter, since it follows normal process exit, takes care to reset terminal settings and restoring the O_NONBLOCK behaviour of stdin (and, in a terminal, that is usually the same file description as stdout and stderr, i.e. some /dev/pts/NN). Failure to restore (remove) O_NONBLOCK from stdout/stderr can cause very surprising and hard to debug problems back in the terminal. For example, I had "make -j8" consistently failing without much information about just exactly what went wrong, but sometimes I did get a "echo: write error". I was at first afraid my disk was getting bad, but then a simple "dmesg" _also_ failed with write error - so it was writing to the terminal that was buggered. And both "make -j8" and dmesg in another terminal window worked just fine. So install a SIGINT handler so that if the chosen terminal mode (cooked or raw-with-sigs) means Ctrl-C sends a SIGINT, we will still call os_fd_restore(), then reraise the signal and die as usual from SIGINT. Before: $ grep flags /proc/$$/fdinfo/1 flags: 0102002 $ ./u-boot # hit Ctrl-C $ grep flags /proc/$$/fdinfo/1 flags: 0106002 After: $ grep flags /proc/$$/fdinfo/1 flags: 0102002 $ ./u-boot # hit Ctrl-C $ grep flags /proc/$$/fdinfo/1 flags: 0102002 Signed-off-by: Rasmus Villemoes Reviewed-by: Simon Glass --- arch/sandbox/cpu/os.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index f7c73e3..e7ec892 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -175,6 +176,13 @@ void os_fd_restore(void) } } +static void os_sigint_handler(int sig) +{ + os_fd_restore(); + signal(SIGINT, SIG_DFL); + raise(SIGINT); +} + /* Put tty into raw mode so and work */ void os_tty_raw(int fd, bool allow_sigs) { @@ -205,6 +213,7 @@ void os_tty_raw(int fd, bool allow_sigs) term_setup = true; atexit(os_fd_restore); + signal(SIGINT, os_sigint_handler); } void *os_malloc(size_t length) -- cgit v1.1 From f93a07dd4f2e9096208a3230b0eca669d9760397 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 15 Feb 2020 21:38:48 +0100 Subject: dm: core: remove redundant if statement The value of parent is not changed in the first if statement. So we can merge the two if statements depending on parent. Indicated by cppcheck. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- drivers/core/device.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/core/device.c b/drivers/core/device.c index 89ea820..534cfa7 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -143,11 +143,9 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv, goto fail_alloc3; } } - } - - /* put dev into parent's successor list */ - if (parent) + /* put dev into parent's successor list */ list_add_tail(&dev->sibling_node, &parent->child_head); + } ret = uclass_bind_device(dev); if (ret) -- cgit v1.1 From 67817b3b7a885b86b02b222675e0e29b1a2f25bf Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 15 Feb 2020 21:46:04 +0100 Subject: dm: core: remove redundant assignment Variable count is initialized at the start of every round of the while loop and it is not used after the while loop. So there is no need to initialize it beforehand. Identified by cppcheck. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- drivers/core/of_access.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c index acd745c..29e705e 100644 --- a/drivers/core/of_access.c +++ b/drivers/core/of_access.c @@ -577,7 +577,7 @@ static int __of_parse_phandle_with_args(const struct device_node *np, { const __be32 *list, *list_end; int rc = 0, cur_index = 0; - uint32_t count = 0; + uint32_t count; struct device_node *node = NULL; phandle phandle; int size; -- cgit v1.1 From 0544ecbfe92e909dd8ac6795d4399a65d6727d9b Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Tue, 18 Feb 2020 15:43:46 +0100 Subject: dm: core: Move "/chosen" and "/firmware" node scan Use the new function dm_scan_fdt_ofnode_path() to scan all the nodes which aren't devices themselves but may contain some: - "/chosen" - "/clocks" - "/firmware" The patch removes the strcmp call in recursive function dm_scan_fdt_live() and also corrects a conflict with the 2 applied patches in the commit 1712ca21924b ("dm: core: Scan /firmware node by default") and in the commit 747558d01457 ("dm: fdt: scan for devices under /firmware too"): the subnodes of "/firmware" (optee for example) are bound 2 times. For example the dm tree command result on STM32MP1 is: STM32MP> dm tree Class Index Probed Driver Name ----------------------------------------------------------- root 0 [ + ] root_driver root_driver firmware 0 [ ] psci |-- psci sysreset 0 [ ] psci-sysreset | `-- psci-sysreset simple_bus 0 [ + ] generic_simple_bus |-- soc ... tee 0 [ + ] optee |-- optee ... tee 1 [ ] optee `-- optee Signed-off-by: Patrick Delaunay Tested-by: Michal Simek Reviewed-by: Simon Glass --- drivers/core/root.c | 52 +++++++++++++++++----------------------------------- test/dm/test-fdt.c | 2 +- 2 files changed, 18 insertions(+), 36 deletions(-) diff --git a/drivers/core/root.c b/drivers/core/root.c index e856438..14df16c 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -203,15 +203,6 @@ static int dm_scan_fdt_live(struct udevice *parent, int ret = 0, err; for (np = node_parent->child; np; np = np->sibling) { - /* "chosen" node isn't a device itself but may contain some: */ - if (!strcmp(np->name, "chosen")) { - pr_debug("parsing subnodes of \"chosen\"\n"); - - err = dm_scan_fdt_live(parent, np, pre_reloc_only); - if (err && !ret) - ret = err; - continue; - } if (!of_device_is_available(np)) { pr_debug(" - ignoring disabled device\n"); @@ -256,21 +247,6 @@ static int dm_scan_fdt_node(struct udevice *parent, const void *blob, offset = fdt_next_subnode(blob, offset)) { const char *node_name = fdt_get_name(blob, offset, NULL); - /* - * The "chosen" and "firmware" nodes aren't devices - * themselves but may contain some: - */ - if (!strcmp(node_name, "chosen") || - !strcmp(node_name, "firmware")) { - pr_debug("parsing subnodes of \"%s\"\n", node_name); - - err = dm_scan_fdt_node(parent, blob, offset, - pre_reloc_only); - if (err && !ret) - ret = err; - continue; - } - if (!fdtdec_get_is_enabled(blob, offset)) { pr_debug(" - ignoring disabled device\n"); continue; @@ -315,7 +291,8 @@ int dm_scan_fdt(const void *blob, bool pre_reloc_only) return dm_scan_fdt_node(gd->dm_root, blob, 0, pre_reloc_only); } -static int dm_scan_fdt_ofnode_path(const char *path, bool pre_reloc_only) +static int dm_scan_fdt_ofnode_path(const void *blob, const char *path, + bool pre_reloc_only) { ofnode node; @@ -327,13 +304,18 @@ static int dm_scan_fdt_ofnode_path(const char *path, bool pre_reloc_only) if (of_live_active()) return dm_scan_fdt_live(gd->dm_root, node.np, pre_reloc_only); #endif - return dm_scan_fdt_node(gd->dm_root, gd->fdt_blob, node.of_offset, + return dm_scan_fdt_node(gd->dm_root, blob, node.of_offset, pre_reloc_only); } int dm_extended_scan_fdt(const void *blob, bool pre_reloc_only) { - int ret; + int ret, i; + const char * const nodes[] = { + "/chosen", + "/clocks", + "/firmware" + }; ret = dm_scan_fdt(blob, pre_reloc_only); if (ret) { @@ -341,16 +323,16 @@ int dm_extended_scan_fdt(const void *blob, bool pre_reloc_only) return ret; } - ret = dm_scan_fdt_ofnode_path("/clocks", pre_reloc_only); - if (ret) { - debug("scan for /clocks failed: %d\n", ret); - return ret; + /* Some nodes aren't devices themselves but may contain some */ + for (i = 0; i < ARRAY_SIZE(nodes); i++) { + ret = dm_scan_fdt_ofnode_path(blob, nodes[i], pre_reloc_only); + if (ret) { + debug("dm_scan_fdt() scan for %s failed: %d\n", + nodes[i], ret); + return ret; + } } - ret = dm_scan_fdt_ofnode_path("/firmware", pre_reloc_only); - if (ret) - debug("scan for /firmware failed: %d\n", ret); - return ret; } #endif diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 75ae080..3500ab1 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -255,7 +255,7 @@ static int dm_test_fdt(struct unit_test_state *uts) int ret; int i; - ret = dm_scan_fdt(gd->fdt_blob, false); + ret = dm_extended_scan_fdt(gd->fdt_blob, false); ut_assert(!ret); ret = uclass_get(UCLASS_TEST_FDT, &uc); -- cgit v1.1 From d1a02f53b3f24dfce488ba244ba4f8809bebe596 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 26 Feb 2020 21:48:15 +0100 Subject: log: correct CONFIG_LOG_TEST prerequisites An error undefined reference to `do_log_test' occurs for CONFIG_CMD_LOG=y, CONFIG_LOG_TEST=y, CONGIG_UNIT_TEST=n Make CONFIG_UNIT_TEST a prerequisite. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- common/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/Kconfig b/common/Kconfig index 3072651..40da8fa 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -777,7 +777,7 @@ config TPL_LOG_CONSOLE config LOG_TEST bool "Provide a test for logging" - depends on LOG + depends on LOG && UNIT_TEST default y if SANDBOX help This enables a 'log test' command to test logging. It is normally -- cgit v1.1 From befadde0a24c3a726689745d5a00c8586adc9c84 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 26 Feb 2020 21:48:16 +0100 Subject: log: syslog driver Provide a log driver that broadcasts RFC 3164 messages to syslog servers. rsyslog is one implementation of such a server. The messages are sent to the local broadcast address 255.255.255.255 on port 514. The environment variable log_hostname can be used to provide the HOSTNAME field for the messages. The optional TIMESTAMP field of RFC 3164 is not provided. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- MAINTAINERS | 2 +- common/Kconfig | 7 ++++ common/Makefile | 1 + common/log_syslog.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++++ doc/README.log | 3 ++ 5 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 common/log_syslog.c diff --git a/MAINTAINERS b/MAINTAINERS index d8d420f..0bafe5c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -654,7 +654,7 @@ LOGGING M: Simon Glass S: Maintained T: git https://gitlab.denx.de/u-boot/u-boot.git -F: common/log.c +F: common/log* F: cmd/log.c F: test/log/log_test.c F: test/py/tests/test_log.py diff --git a/common/Kconfig b/common/Kconfig index 40da8fa..ee4f748 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -775,6 +775,13 @@ config TPL_LOG_CONSOLE log message is shown - other details like level, category, file and line number are omitted. +config LOG_SYSLOG + bool "Log output to syslog server" + depends on LOG && NET + help + Enables a log driver which broadcasts log records via UDP port 514 + to syslog servers. + config LOG_TEST bool "Provide a test for logging" depends on LOG && UNIT_TEST diff --git a/common/Makefile b/common/Makefile index 702f239..d84e10b 100644 --- a/common/Makefile +++ b/common/Makefile @@ -132,6 +132,7 @@ obj-$(CONFIG_DFU_OVER_USB) += dfu.o obj-y += command.o obj-$(CONFIG_$(SPL_TPL_)LOG) += log.o obj-$(CONFIG_$(SPL_TPL_)LOG_CONSOLE) += log_console.o +obj-$(CONFIG_$(SPL_TPL_)LOG_SYSLOG) += log_syslog.o obj-y += s_record.o obj-$(CONFIG_CMD_LOADB) += xyzModem.o obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o diff --git a/common/log_syslog.c b/common/log_syslog.c new file mode 100644 index 0000000..5e3e20e --- /dev/null +++ b/common/log_syslog.c @@ -0,0 +1,117 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Log to syslog. + * + * Copyright (c) 2020, Heinrich Schuchardt + */ + +#include +#include + +DECLARE_GLOBAL_DATA_PTR; + +#define BUFFER_SIZE 480 + +static void append(char **buf, char *buf_end, const char *fmt, ...) +{ + va_list args; + size_t size = buf_end - *buf; + + va_start(args, fmt); + vsnprintf(*buf, size, fmt, args); + va_end(args); + *buf += strlen(*buf); +} + +static int log_syslog_emit(struct log_device *ldev, struct log_rec *rec) +{ + int ret; + int fmt = gd->log_fmt; + char msg[BUFFER_SIZE]; + char *msg_end = msg + BUFFER_SIZE; + char *ptr = msg; + char *iphdr; + char *log_msg; + int eth_hdr_size; + struct in_addr bcast_ip; + static int processing_msg; + unsigned int log_level; + char *log_hostname; + + /* Fend off messages from the network stack while writing a message */ + if (processing_msg) + return 0; + + processing_msg = 1; + + /* Setup packet buffers */ + net_init(); + /* Disable hardware and put it into the reset state */ + eth_halt(); + /* Set current device according to environment variables */ + eth_set_current(); + /* Get hardware ready for send and receive operations */ + ret = eth_init(); + if (ret < 0) { + eth_halt(); + goto out; + } + + memset(msg, 0, BUFFER_SIZE); + + /* Set ethernet header */ + eth_hdr_size = net_set_ether((uchar *)ptr, net_bcast_ethaddr, PROT_IP); + ptr += eth_hdr_size; + iphdr = ptr; + ptr += IP_UDP_HDR_SIZE; + log_msg = ptr; + + /* + * The syslog log levels defined in RFC 5424 match the U-Boot ones up to + * level 7 (debug). + */ + log_level = rec->level; + if (log_level > 7) + log_level = 7; + /* Leave high bits as 0 to write a 'kernel message' */ + + /* Write log message to buffer */ + append(&ptr, msg_end, "<%u>", log_level); + log_hostname = env_get("log_hostname"); + if (log_hostname) + append(&ptr, msg_end, "%s ", log_hostname); + append(&ptr, msg_end, "uboot: "); + if (fmt & (1 << LOGF_LEVEL)) + append(&ptr, msg_end, "%s.", + log_get_level_name(rec->level)); + if (fmt & (1 << LOGF_CAT)) + append(&ptr, msg_end, "%s,", + log_get_cat_name(rec->cat)); + if (fmt & (1 << LOGF_FILE)) + append(&ptr, msg_end, "%s:", rec->file); + if (fmt & (1 << LOGF_LINE)) + append(&ptr, msg_end, "%d-", rec->line); + if (fmt & (1 << LOGF_FUNC)) + append(&ptr, msg_end, "%s()", rec->func); + if (fmt & (1 << LOGF_MSG)) + append(&ptr, msg_end, "%s%s", + fmt != (1 << LOGF_MSG) ? " " : "", rec->msg); + /* Consider trailing 0x00 */ + ptr++; + + debug("log message: '%s'\n", log_msg); + + /* Broadcast message */ + bcast_ip.s_addr = 0xFFFFFFFFL; + net_set_udp_header((uchar *)iphdr, bcast_ip, 514, 514, ptr - log_msg); + net_send_packet((uchar *)msg, ptr - msg); + +out: + processing_msg = 0; + return ret; +} + +LOG_DRIVER(syslog) = { + .name = "syslog", + .emit = log_syslog_emit, +}; diff --git a/doc/README.log b/doc/README.log index 19856d4..1057981 100644 --- a/doc/README.log +++ b/doc/README.log @@ -147,7 +147,10 @@ several possible determinations for logging information, all of which can be enabled or disabled independently: console - goes to stdout + syslog - broadcast RFC 3164 messages to syslog servers on UDP port 514 +The syslog driver sends the value of environmental variable 'log_hostname' as +HOSTNAME if available. Log format ---------- -- cgit v1.1 From 20fd256deb055479c9c0c9f0b1a9f9098c96f310 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 26 Feb 2020 21:48:17 +0100 Subject: log: output for CONFIG_LOG=n If CONFIG_LOG=n, we should still output errors, warnings, notices, infos, and for DEBUG=1 also debug messages. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- include/log.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/log.h b/include/log.h index 62fb8af..0453876 100644 --- a/include/log.h +++ b/include/log.h @@ -115,11 +115,11 @@ static inline int _log_nop(enum log_category_t cat, enum log_level_t level, #define log_io(_fmt...) log(LOG_CATEGORY, LOGL_DEBUG_IO, ##_fmt) #else #define _LOG_MAX_LEVEL LOGL_INFO -#define log_err(_fmt...) log_nop(LOG_CATEGORY, LOGL_ERR, ##_fmt) -#define log_warning(_fmt...) log_nop(LOG_CATEGORY, LOGL_WARNING, ##_fmt) -#define log_notice(_fmt...) log_nop(LOG_CATEGORY, LOGL_NOTICE, ##_fmt) -#define log_info(_fmt...) log_nop(LOG_CATEGORY, LOGL_INFO, ##_fmt) -#define log_debug(_fmt...) log_nop(LOG_CATEGORY, LOGL_DEBUG, ##_fmt) +#define log_err(_fmt, ...) printf(_fmt, ##__VA_ARGS__) +#define log_warning(_fmt, ...) printf(_fmt, ##__VA_ARGS__) +#define log_notice(_fmt, ...) printf(_fmt, ##__VA_ARGS__) +#define log_info(_fmt, ...) printf(_fmt, ##__VA_ARGS__) +#define log_debug(_fmt, ...) debug(_fmt, ##__VA_ARGS__) #define log_content(_fmt...) log_nop(LOG_CATEGORY, \ LOGL_DEBUG_CONTENT, ##_fmt) #define log_io(_fmt...) log_nop(LOG_CATEGORY, LOGL_DEBUG_IO, ##_fmt) -- cgit v1.1 From 395041b2fd2f1cb2c84127acb18d87c27c29448c Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 26 Feb 2020 21:48:18 +0100 Subject: test: log functions with CONFIG_LOG=n If CONFIG_LOG=n, we still expect output for log_err(), log_warning(), log_notice(), log_info() and in case of DEBUG=1 also for log_debug(). Provide unit tests verifying this. The tests depend on: CONFIG_CONSOLE_RECORD=y CONFIG_LOG=n CONFIG_UT_LOG=y It may be necessary to increase the value of CONFIG_SYS_MALLOC_F_LEN to accommodate CONFIG_CONSOLE_RECORD=y. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- MAINTAINERS | 2 +- include/test/log.h | 16 ++++++ include/test/suites.h | 1 + test/Kconfig | 9 ++++ test/Makefile | 2 +- test/cmd_ut.c | 6 +++ test/log/Makefile | 10 ++++ test/log/nolog_test.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++++++ test/log/test-main.c | 20 ++++++++ 9 files changed, 199 insertions(+), 2 deletions(-) create mode 100644 include/test/log.h create mode 100644 test/log/nolog_test.c create mode 100644 test/log/test-main.c diff --git a/MAINTAINERS b/MAINTAINERS index 0bafe5c..7ac7e21 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -656,7 +656,7 @@ S: Maintained T: git https://gitlab.denx.de/u-boot/u-boot.git F: common/log* F: cmd/log.c -F: test/log/log_test.c +F: test/log/ F: test/py/tests/test_log.py MALI DISPLAY PROCESSORS diff --git a/include/test/log.h b/include/test/log.h new file mode 100644 index 0000000..c661cde --- /dev/null +++ b/include/test/log.h @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (c) 2020, Heinrich Schuchardt + * + * Tests for logging functions + */ + +#ifndef __TEST_LOG_H__ +#define __TEST_LOG_H__ + +#include + +/* Declare a new logging test */ +#define LOG_TEST(_name) UNIT_TEST(_name, 0, log_test) + +#endif /* __TEST_LOG_H__ */ diff --git a/include/test/suites.h b/include/test/suites.h index 0748185..39ad81a 100644 --- a/include/test/suites.h +++ b/include/test/suites.h @@ -30,6 +30,7 @@ int do_ut_compression(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]); int do_ut_dm(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); int do_ut_env(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); int do_ut_lib(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); +int do_ut_log(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); int do_ut_optee(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); int do_ut_overlay(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); int do_ut_time(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); diff --git a/test/Kconfig b/test/Kconfig index 0157e0b..28704a2 100644 --- a/test/Kconfig +++ b/test/Kconfig @@ -40,6 +40,15 @@ config UT_LIB_RSA endif +config UT_LOG + bool "Unit tests for logging functions" + depends on UNIT_TEST + default y + help + Enables the 'ut log' command which tests logging functions like + log_err(). + See also CONFIG_LOG_TEST which provides the 'log test' command. + config UT_TIME bool "Unit tests for time functions" depends on UNIT_TEST diff --git a/test/Makefile b/test/Makefile index 2fe41f4..2971d0d 100644 --- a/test/Makefile +++ b/test/Makefile @@ -10,5 +10,5 @@ obj-$(CONFIG_SANDBOX) += compression.o obj-$(CONFIG_SANDBOX) += print_ut.o obj-$(CONFIG_UT_TIME) += time_ut.o obj-$(CONFIG_UT_UNICODE) += unicode_ut.o -obj-$(CONFIG_$(SPL_)LOG) += log/ +obj-y += log/ obj-$(CONFIG_UNIT_TEST) += lib/ diff --git a/test/cmd_ut.c b/test/cmd_ut.c index a3a9d49..7fdcdbb 100644 --- a/test/cmd_ut.c +++ b/test/cmd_ut.c @@ -60,6 +60,9 @@ static cmd_tbl_t cmd_ut_sub[] = { #ifdef CONFIG_UT_LIB U_BOOT_CMD_MKENT(lib, CONFIG_SYS_MAXARGS, 1, do_ut_lib, "", ""), #endif +#ifdef CONFIG_UT_LOG + U_BOOT_CMD_MKENT(log, CONFIG_SYS_MAXARGS, 1, do_ut_log, "", ""), +#endif #ifdef CONFIG_UT_TIME U_BOOT_CMD_MKENT(time, CONFIG_SYS_MAXARGS, 1, do_ut_time, "", ""), #endif @@ -125,6 +128,9 @@ static char ut_help_text[] = #ifdef CONFIG_UT_LIB "ut lib [test-name] - test library functions\n" #endif +#ifdef CONFIG_UT_LOG + "ut log [test-name] - test logging functions\n" +#endif #ifdef CONFIG_UT_OPTEE "ut optee [test-name]\n" #endif diff --git a/test/log/Makefile b/test/log/Makefile index e0d0a47..98178f5 100644 --- a/test/log/Makefile +++ b/test/log/Makefile @@ -3,3 +3,13 @@ # Copyright (c) 2017 Google, Inc obj-$(CONFIG_LOG_TEST) += log_test.o + +ifdef CONFIG_UT_LOG + +obj-y += test-main.o + +ifndef CONFIG_LOG +obj-$(CONFIG_CONSOLE_RECORD) += nolog_test.o +endif + +endif # CONFIG_UT_LOG diff --git a/test/log/nolog_test.c b/test/log/nolog_test.c new file mode 100644 index 0000000..8461952 --- /dev/null +++ b/test/log/nolog_test.c @@ -0,0 +1,135 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2020, Heinrich Schuchardt + * + * Logging function tests for CONFIG_LOG=n. + */ + +/* Needed for testing log_debug() */ +#define DEBUG 1 + +#include +#include +#include +#include +#include +#include + +DECLARE_GLOBAL_DATA_PTR; + +#define BUFFSIZE 32 + +static int nolog_test_log_err(struct unit_test_state *uts) +{ + char buf[BUFFSIZE]; + + memset(buf, 0, BUFFSIZE); + console_record_reset_enable(); + log_err("testing %s\n", "log_err"); + gd->flags &= ~GD_FLG_RECORD; + ut_assertok(ut_check_console_line(uts, "testing log_err")); + ut_assertok(ut_check_console_end(uts)); + return 0; +} +LOG_TEST(nolog_test_log_err); + +static int nolog_test_log_warning(struct unit_test_state *uts) +{ + char buf[BUFFSIZE]; + + memset(buf, 0, BUFFSIZE); + console_record_reset_enable(); + log_warning("testing %s\n", "log_warning"); + gd->flags &= ~GD_FLG_RECORD; + ut_assertok(ut_check_console_line(uts, "testing log_warning")); + ut_assertok(ut_check_console_end(uts)); + return 0; +} +LOG_TEST(nolog_test_log_warning); + +static int nolog_test_log_notice(struct unit_test_state *uts) +{ + char buf[BUFFSIZE]; + + memset(buf, 0, BUFFSIZE); + console_record_reset_enable(); + log_notice("testing %s\n", "log_notice"); + gd->flags &= ~GD_FLG_RECORD; + ut_assertok(ut_check_console_line(uts, "testing log_notice")); + ut_assertok(ut_check_console_end(uts)); + return 0; +} +LOG_TEST(nolog_test_log_notice); + +static int nolog_test_log_info(struct unit_test_state *uts) +{ + char buf[BUFFSIZE]; + + memset(buf, 0, BUFFSIZE); + console_record_reset_enable(); + log_err("testing %s\n", "log_info"); + gd->flags &= ~GD_FLG_RECORD; + ut_assertok(ut_check_console_line(uts, "testing log_info")); + ut_assertok(ut_check_console_end(uts)); + return 0; +} +LOG_TEST(nolog_test_log_info); + +#undef _DEBUG +#define _DEBUG 0 +static int nolog_test_nodebug(struct unit_test_state *uts) +{ + char buf[BUFFSIZE]; + + memset(buf, 0, BUFFSIZE); + console_record_reset_enable(); + debug("testing %s\n", "debug"); + gd->flags &= ~GD_FLG_RECORD; + ut_assertok(ut_check_console_end(uts)); + return 0; +} +LOG_TEST(nolog_test_nodebug); + +static int nolog_test_log_nodebug(struct unit_test_state *uts) +{ + char buf[BUFFSIZE]; + + memset(buf, 0, BUFFSIZE); + console_record_reset_enable(); + log_debug("testing %s\n", "log_debug"); + gd->flags &= ~GD_FLG_RECORD; + ut_assert(!strcmp(buf, "")); + ut_assertok(ut_check_console_end(uts)); + return 0; +} +LOG_TEST(nolog_test_log_nodebug); + +#undef _DEBUG +#define _DEBUG 1 +static int nolog_test_debug(struct unit_test_state *uts) +{ + char buf[BUFFSIZE]; + + memset(buf, 0, BUFFSIZE); + console_record_reset_enable(); + debug("testing %s\n", "debug"); + gd->flags &= ~GD_FLG_RECORD; + ut_assertok(ut_check_console_line(uts, "testing debug")); + ut_assertok(ut_check_console_end(uts)); + return 0; +} +LOG_TEST(nolog_test_debug); + +static int nolog_test_log_debug(struct unit_test_state *uts) +{ + char buf[BUFFSIZE]; + + memset(buf, 0, BUFFSIZE); + console_record_reset_enable(); + log_debug("testing %s\n", "log_debug"); + gd->flags &= ~GD_FLG_RECORD; + ut_assertok(ut_check_console_line(uts, "testing log_debug")); + ut_assertok(ut_check_console_end(uts)); + return 0; +} +LOG_TEST(nolog_test_log_debug); diff --git a/test/log/test-main.c b/test/log/test-main.c new file mode 100644 index 0000000..855de47 --- /dev/null +++ b/test/log/test-main.c @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2020, Heinrich Schuchardt + * + * Logging function tests. + */ + +#include +#include +#include +#include + +int do_ut_log(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + struct unit_test *tests = ll_entry_start(struct unit_test, log_test); + const int n_ents = ll_entry_count(struct unit_test, log_test); + + return cmd_ut_category("log", "log_test_", + tests, n_ents, argc, argv); +} -- cgit v1.1 From 7b912edcf66e3fe9555d5666b40a6fe480694b11 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 26 Feb 2020 21:48:19 +0100 Subject: test: log: test syslog logging driver Provide unit tests for the syslog logging driver. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- test/log/Makefile | 4 + test/log/syslog_test.c | 280 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 284 insertions(+) create mode 100644 test/log/syslog_test.c diff --git a/test/log/Makefile b/test/log/Makefile index 98178f5..4c92550 100644 --- a/test/log/Makefile +++ b/test/log/Makefile @@ -8,6 +8,10 @@ ifdef CONFIG_UT_LOG obj-y += test-main.o +ifdef CONFIG_SANDBOX +obj-$(CONFIG_LOG_SYSLOG) += syslog_test.o +endif + ifndef CONFIG_LOG obj-$(CONFIG_CONSOLE_RECORD) += nolog_test.o endif diff --git a/test/log/syslog_test.c b/test/log/syslog_test.c new file mode 100644 index 0000000..6ca5760 --- /dev/null +++ b/test/log/syslog_test.c @@ -0,0 +1,280 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2020, Heinrich Schuchardt + * + * Logging function tests for CONFIG_LOG_SYSLOG=y. + * + * Invoke the test with: ./u-boot -d arch/sandbox/dts/test.dtb + */ + +/* Override CONFIG_LOG_MAX_LEVEL */ +#define LOG_DEBUG + +#include +#include +#include +#include +#include +#include +#include +#include + +DECLARE_GLOBAL_DATA_PTR; + +/** + * struct sb_log_env - private data for sandbox ethernet driver + * + * This structure is used for the private data of the sandbox ethernet + * driver. + * + * @expected: string expected to be written by the syslog driver + * @uts: unit test state + */ +struct sb_log_env { + const char *expected; + struct unit_test_state *uts; +}; + +/** + * sb_log_tx_handler() - transmit callback function + * + * This callback function is invoked when a network package is sent using the + * sandbox Ethernet driver. The private data of the driver holds a sb_log_env + * structure with the unit test state and the expected UDP payload. + * + * The following checks are executed: + * + * * the Ethernet packet indicates a IP broadcast message + * * the IP header is for a local UDP broadcast message to port 514 + * * the UDP payload matches the expected string + * + * After testing the pointer to the expected string is set to NULL to signal + * that the callback function has been called. + * + * @dev: sandbox ethernet device + * @packet: Ethernet packet + * @len: length of Ethernet packet + * Return: 0 = success + */ +static int sb_log_tx_handler(struct udevice *dev, void *packet, + unsigned int len) +{ + struct eth_sandbox_priv *priv = dev_get_priv(dev); + struct sb_log_env *env = priv->priv; + /* uts is updated by the ut_assert* macros */ + struct unit_test_state *uts = env->uts; + char *buf = packet; + struct ethernet_hdr *eth_hdr = packet; + struct ip_udp_hdr *ip_udp_hdr; + + /* Check Ethernet header */ + ut_asserteq_mem(ð_hdr->et_dest, net_bcast_ethaddr, ARP_HLEN); + ut_asserteq(ntohs(eth_hdr->et_protlen), PROT_IP); + + /* Check IP header */ + buf += sizeof(struct ethernet_hdr); + ip_udp_hdr = (struct ip_udp_hdr *)buf; + ut_asserteq(ip_udp_hdr->ip_p, IPPROTO_UDP); + ut_asserteq(ip_udp_hdr->ip_dst.s_addr, 0xffffffff); + ut_asserteq(ntohs(ip_udp_hdr->udp_dst), 514); + ut_asserteq(UDP_HDR_SIZE + strlen(env->expected) + 1, + ntohs(ip_udp_hdr->udp_len)); + + /* Check payload */ + buf += sizeof(struct ip_udp_hdr); + ut_asserteq_mem(env->expected, buf, + ntohs(ip_udp_hdr->udp_len) - UDP_HDR_SIZE); + + /* Signal that the callback function has been executed */ + env->expected = NULL; + + return 0; +} + +/** + * syslog_test_log_err() - test log_err() function + * + * @uts: unit test state + * Return: 0 = success + */ +static int syslog_test_log_err(struct unit_test_state *uts) +{ + int old_log_level = gd->default_log_level; + struct sb_log_env env; + + gd->log_fmt = LOGF_DEFAULT; + gd->default_log_level = LOGL_INFO; + env_set("ethact", "eth@10002000"); + env_set("log_hostname", "sandbox"); + env.expected = "<3>sandbox uboot: syslog_test_log_err() " + "testing log_err\n"; + env.uts = uts; + sandbox_eth_set_tx_handler(0, sb_log_tx_handler); + /* Used by ut_assert macros in the tx_handler */ + sandbox_eth_set_priv(0, &env); + log_err("testing %s\n", "log_err"); + /* Check that the callback function was called */ + sandbox_eth_set_tx_handler(0, NULL); + gd->default_log_level = old_log_level; + + return 0; +} +LOG_TEST(syslog_test_log_err); + +/** + * syslog_test_log_warning() - test log_warning() function + * + * @uts: unit test state + * Return: 0 = success + */ +static int syslog_test_log_warning(struct unit_test_state *uts) +{ + int old_log_level = gd->default_log_level; + struct sb_log_env env; + + gd->log_fmt = LOGF_DEFAULT; + gd->default_log_level = LOGL_INFO; + env_set("ethact", "eth@10002000"); + env_set("log_hostname", "sandbox"); + env.expected = "<4>sandbox uboot: syslog_test_log_warning() " + "testing log_warning\n"; + env.uts = uts; + sandbox_eth_set_tx_handler(0, sb_log_tx_handler); + /* Used by ut_assert macros in the tx_handler */ + sandbox_eth_set_priv(0, &env); + log_warning("testing %s\n", "log_warning"); + sandbox_eth_set_tx_handler(0, NULL); + /* Check that the callback function was called */ + ut_assertnull(env.expected); + gd->default_log_level = old_log_level; + + return 0; +} +LOG_TEST(syslog_test_log_warning); + +/** + * syslog_test_log_notice() - test log_notice() function + * + * @uts: unit test state + * Return: 0 = success + */ +static int syslog_test_log_notice(struct unit_test_state *uts) +{ + int old_log_level = gd->default_log_level; + struct sb_log_env env; + + gd->log_fmt = LOGF_DEFAULT; + gd->default_log_level = LOGL_INFO; + env_set("ethact", "eth@10002000"); + env_set("log_hostname", "sandbox"); + env.expected = "<5>sandbox uboot: syslog_test_log_notice() " + "testing log_notice\n"; + env.uts = uts; + sandbox_eth_set_tx_handler(0, sb_log_tx_handler); + /* Used by ut_assert macros in the tx_handler */ + sandbox_eth_set_priv(0, &env); + log_notice("testing %s\n", "log_notice"); + sandbox_eth_set_tx_handler(0, NULL); + /* Check that the callback function was called */ + ut_assertnull(env.expected); + gd->default_log_level = old_log_level; + + return 0; +} +LOG_TEST(syslog_test_log_notice); + +/** + * syslog_test_log_info() - test log_info() function + * + * @uts: unit test state + * Return: 0 = success + */ +static int syslog_test_log_info(struct unit_test_state *uts) +{ + int old_log_level = gd->default_log_level; + struct sb_log_env env; + + gd->log_fmt = LOGF_DEFAULT; + gd->default_log_level = LOGL_INFO; + env_set("ethact", "eth@10002000"); + env_set("log_hostname", "sandbox"); + env.expected = "<6>sandbox uboot: syslog_test_log_info() " + "testing log_info\n"; + env.uts = uts; + sandbox_eth_set_tx_handler(0, sb_log_tx_handler); + /* Used by ut_assert macros in the tx_handler */ + sandbox_eth_set_priv(0, &env); + log_info("testing %s\n", "log_info"); + sandbox_eth_set_tx_handler(0, NULL); + /* Check that the callback function was called */ + ut_assertnull(env.expected); + gd->default_log_level = old_log_level; + + return 0; +} +LOG_TEST(syslog_test_log_info); + +/** + * syslog_test_log_debug() - test log_debug() function + * + * @uts: unit test state + * Return: 0 = success + */ +static int syslog_test_log_debug(struct unit_test_state *uts) +{ + int old_log_level = gd->default_log_level; + struct sb_log_env env; + + gd->log_fmt = LOGF_DEFAULT; + gd->default_log_level = LOGL_DEBUG; + env_set("ethact", "eth@10002000"); + env_set("log_hostname", "sandbox"); + env.expected = "<7>sandbox uboot: syslog_test_log_debug() " + "testing log_debug\n"; + env.uts = uts; + sandbox_eth_set_tx_handler(0, sb_log_tx_handler); + /* Used by ut_assert macros in the tx_handler */ + sandbox_eth_set_priv(0, &env); + log_debug("testing %s\n", "log_debug"); + sandbox_eth_set_tx_handler(0, NULL); + /* Check that the callback function was called */ + ut_assertnull(env.expected); + gd->default_log_level = old_log_level; + + return 0; +} +LOG_TEST(syslog_test_log_debug); + +/** + * syslog_test_log_nodebug() - test logging level filter + * + * Verify that log_debug() does not lead to a log message if the logging level + * is set to LOGL_INFO. + * + * @uts: unit test state + * Return: 0 = success + */ +static int syslog_test_log_nodebug(struct unit_test_state *uts) +{ + int old_log_level = gd->default_log_level; + struct sb_log_env env; + + gd->log_fmt = LOGF_DEFAULT; + gd->default_log_level = LOGL_INFO; + env_set("ethact", "eth@10002000"); + env_set("log_hostname", "sandbox"); + env.expected = "<7>sandbox uboot: syslog_test_log_nodebug() " + "testing log_debug\n"; + env.uts = uts; + sandbox_eth_set_tx_handler(0, sb_log_tx_handler); + /* Used by ut_assert macros in the tx_handler */ + sandbox_eth_set_priv(0, &env); + log_debug("testing %s\n", "log_debug"); + sandbox_eth_set_tx_handler(0, NULL); + /* Check that the callback function was not called */ + ut_assertnonnull(env.expected); + gd->default_log_level = old_log_level; + + return 0; +} +LOG_TEST(syslog_test_log_nodebug); -- cgit v1.1 From 292defdd2b9e7dbd2ace5a93568f078ca3d4bc11 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 26 Feb 2020 21:48:20 +0100 Subject: configs: sandbox: enable LOG_SYSLOG For testing purposes enable the syslog logging driver. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- configs/sandbox64_defconfig | 1 + configs/sandbox_defconfig | 1 + configs/sandbox_flattree_defconfig | 1 + 3 files changed, 3 insertions(+) diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index 8ca17d6..0d4cf74 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -18,6 +18,7 @@ CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000 CONFIG_SILENT_CONSOLE=y CONFIG_PRE_CONSOLE_BUFFER=y CONFIG_LOG_MAX_LEVEL=6 +CONFIG_LOG_SYSLOG=y CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_CMD_CPU=y CONFIG_CMD_LICENSE=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index cc90f00..a43b48d 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -19,6 +19,7 @@ CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000 CONFIG_SILENT_CONSOLE=y CONFIG_PRE_CONSOLE_BUFFER=y CONFIG_LOG_MAX_LEVEL=6 +CONFIG_LOG_SYSLOG=y CONFIG_LOG_ERROR_RETURN=y CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_ANDROID_AB=y diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index 59d34cb..346ef58 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -15,6 +15,7 @@ CONFIG_CONSOLE_RECORD=y CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000 CONFIG_SILENT_CONSOLE=y CONFIG_LOG_MAX_LEVEL=6 +CONFIG_LOG_SYSLOG=y CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_CMD_CPU=y CONFIG_CMD_LICENSE=y -- cgit v1.1 From da2fa6d86a1f7aea590e7cbcd234718bcfd435b6 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 26 Feb 2020 20:18:54 +0100 Subject: doc: driver-model: there is no UCLASS_ETHERNET %s/UCLASS_ETHERNET/UCLASS_ETH/g Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- doc/driver-model/design.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/driver-model/design.rst b/doc/driver-model/design.rst index 5247ecc..e37b51b 100644 --- a/doc/driver-model/design.rst +++ b/doc/driver-model/design.rst @@ -579,7 +579,7 @@ a USB bus with several devices attached to it, each from a different (made up) uclass:: xhci_usb (UCLASS_USB) - eth (UCLASS_ETHERNET) + eth (UCLASS_ETH) camera (UCLASS_CAMERA) flash (UCLASS_FLASH_STORAGE) -- cgit v1.1 From cf0ef9317cdb8a9b5be433f9c9da4c96ae7433d1 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 27 Feb 2020 18:49:23 -0700 Subject: patman: Apply the cc limit to the cover letter also Quite often on a series that has clean-up patches, the individual patches may fit within the cc limit but the cover letter does not. Apply the same limit to the cover letter. Signed-off-by: Simon Glass Reviewed-by: Chris Packham --- tools/patman/series.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/patman/series.py b/tools/patman/series.py index a15f762..6d9d48b 100644 --- a/tools/patman/series.py +++ b/tools/patman/series.py @@ -249,8 +249,10 @@ class Series(dict): if cover_fname: cover_cc = gitutil.BuildEmailList(self.get('cover_cc', '')) cover_cc = [tools.FromUnicode(m) for m in cover_cc] - cc_list = '\0'.join([tools.ToUnicode(x) - for x in sorted(set(cover_cc + all_ccs))]) + cover_cc = list(set(cover_cc + all_ccs)) + if limit is not None: + cover_cc = cover_cc[:limit] + cc_list = '\0'.join([tools.ToUnicode(x) for x in sorted(cover_cc)]) print(cover_fname, cc_list, file=fd) fd.close() -- cgit v1.1 From 1ecea74e2e3f42185bb018fa64f70f43d2096d2f Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 14 Mar 2020 12:13:39 +0100 Subject: sandbox: add reserved-memory node in device tree For testing the handling of memory reservations create a reserved-memory node in sandbox.dts and sandbox64.dts. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- arch/sandbox/dts/sandbox.dts | 19 +++++++++++++++++++ arch/sandbox/dts/sandbox64.dts | 20 ++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts index 2d7db02..20f6893 100644 --- a/arch/sandbox/dts/sandbox.dts +++ b/arch/sandbox/dts/sandbox.dts @@ -20,6 +20,25 @@ reg = <0 CONFIG_SYS_SDRAM_SIZE>; }; + reserved-memory { + #address-cells = <1>; + #size-cells = <1>; + ranges; + + reservation_test0 { + size = <0x4000>; + alignment = <0x2000>; + }; + + reservation_test1: restest@a000 { + reg = <0x00d0a000 0x2000>; + }; + + reservation_test2: restest@7000 { + reg = <0x00d07000 0x1000>; + }; + }; + cros_ec: cros-ec { reg = <0 0>; u-boot,dm-pre-reloc; diff --git a/arch/sandbox/dts/sandbox64.dts b/arch/sandbox/dts/sandbox64.dts index 97e33f1..a39f94f 100644 --- a/arch/sandbox/dts/sandbox64.dts +++ b/arch/sandbox/dts/sandbox64.dts @@ -20,6 +20,26 @@ reg = /bits/ 64 <0 CONFIG_SYS_SDRAM_SIZE>; }; + reserved-memory { + #address-cells = <2>; + #size-cells = <2>; + ranges; + + reservation_test_size { + size = <0 0x4000>; + alignment = <0 0x2000>; + }; + + reservation_test@a000 { + reg = <0 0x00d0a000 0 0x2000>; + }; + + reservation_test@7000 { + reg = <0 0x00d07000 0 0x1000>; + }; + }; + + /* ... */ cros_ec: cros-ec { reg = <0 0 0 0>; u-boot,dm-pre-reloc; -- cgit v1.1 From 1c0bc80ae106a363770d38d633a40b4c3f583566 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 14 Mar 2020 12:13:40 +0100 Subject: sandbox: implement ft_board_setup() Currently we are not able to test reservations created by ft_board_setup(). Implement ft_board_setup() to create an arbitrary reservation and enable OF_BOARD_SETUP. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- arch/Kconfig | 1 + board/sandbox/sandbox.c | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/arch/Kconfig b/arch/Kconfig index ae9c93e..91e049b 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -96,6 +96,7 @@ config SANDBOX select DM_SPI_FLASH select HAVE_BLOCK_DEVICE select LZO + select OF_BOARD_SETUP select PCI_ENDPOINT select SPI select SUPPORT_OF_CONTROL diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c index 0c3d245..1372003 100644 --- a/board/sandbox/sandbox.c +++ b/board/sandbox/sandbox.c @@ -58,6 +58,12 @@ int board_init(void) return 0; } +int ft_board_setup(void *fdt, bd_t *bd) +{ + /* Create an arbitrary reservation to allow testing OF_BOARD_SETUP.*/ + return fdt_add_mem_rsv(fdt, 0x00d02000, 0x4000); +} + #ifdef CONFIG_BOARD_LATE_INIT int board_late_init(void) { -- cgit v1.1 From 48e4288aed7d05399a4c0d3ff908319d2793e3f5 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 14 Mar 2020 12:27:02 +0100 Subject: sandbox: enable CMD_BOOTEFI_HELLO and CMD_EFIDEBUG 'bootefi hello' is used in one of the Python tests. efidebug can be used to verify the correct initialization of the UEFI sub-system. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- configs/sandbox64_defconfig | 2 ++ configs/sandbox_defconfig | 1 + configs/sandbox_flattree_defconfig | 2 ++ configs/sandbox_spl_defconfig | 2 ++ 4 files changed, 7 insertions(+) diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index 0d4cf74..a6183d9 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -23,6 +23,7 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_CMD_CPU=y CONFIG_CMD_LICENSE=y CONFIG_CMD_BOOTZ=y +CONFIG_CMD_BOOTEFI_HELLO=y # CONFIG_CMD_ELF is not set CONFIG_CMD_ASKENV=y CONFIG_CMD_GREPENV=y @@ -54,6 +55,7 @@ CONFIG_CMD_DNS=y CONFIG_CMD_LINK_LOCAL=y CONFIG_CMD_ETHSW=y CONFIG_CMD_BMP=y +CONFIG_CMD_EFIDEBUG=y CONFIG_CMD_TIME=y CONFIG_CMD_TIMER=y CONFIG_CMD_SOUND=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index a43b48d..fa72c66 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -26,6 +26,7 @@ CONFIG_ANDROID_AB=y CONFIG_CMD_CPU=y CONFIG_CMD_LICENSE=y CONFIG_CMD_BOOTZ=y +CONFIG_CMD_BOOTEFI_HELLO=y CONFIG_CMD_ABOOTIMG=y # CONFIG_CMD_ELF is not set CONFIG_CMD_ASKENV=y diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index 346ef58..00d9359 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -20,6 +20,7 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_CMD_CPU=y CONFIG_CMD_LICENSE=y CONFIG_CMD_BOOTZ=y +CONFIG_CMD_BOOTEFI_HELLO=y # CONFIG_CMD_ELF is not set CONFIG_CMD_ASKENV=y CONFIG_CMD_GREPENV=y @@ -44,6 +45,7 @@ CONFIG_CMD_CDP=y CONFIG_CMD_SNTP=y CONFIG_CMD_DNS=y CONFIG_CMD_LINK_LOCAL=y +CONFIG_CMD_EFIDEBUG=y CONFIG_CMD_TIME=y CONFIG_CMD_TIMER=y CONFIG_CMD_SOUND=y diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index 53c5bd8..18c6a47 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -28,6 +28,7 @@ CONFIG_SPL_ENV_SUPPORT=y CONFIG_CMD_CPU=y CONFIG_CMD_LICENSE=y CONFIG_CMD_BOOTZ=y +CONFIG_CMD_BOOTEFI_HELLO=y # CONFIG_CMD_ELF is not set CONFIG_CMD_ASKENV=y CONFIG_CMD_GREPENV=y @@ -56,6 +57,7 @@ CONFIG_CMD_SNTP=y CONFIG_CMD_DNS=y CONFIG_CMD_LINK_LOCAL=y CONFIG_CMD_BMP=y +CONFIG_CMD_EFIDEBUG=y CONFIG_CMD_TIME=y CONFIG_CMD_TIMER=y CONFIG_CMD_SOUND=y -- cgit v1.1 From 0688b758a2c2d3c56dd46ceb6c78ebf29867cc57 Mon Sep 17 00:00:00 2001 From: Tom Warren Date: Thu, 26 Mar 2020 15:20:44 -0700 Subject: fdt: Fix 'system' command 'fdt systemsetup' wasn't working, due to the fact that the 'set' command was being parsed in do_fdt() by only testing for the leading 's' instead of "se", which kept the "sys" test further down from executing. Changed to test for "se" instead, now 'fdt systemsetup' works (to test the ft_system_setup proc w/o having to boot a kernel). Signed-off-by: Tom Warren --- cmd/fdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/fdt.c b/cmd/fdt.c index 25a6ed4..36cc726 100644 --- a/cmd/fdt.c +++ b/cmd/fdt.c @@ -286,7 +286,7 @@ static int do_fdt(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) /* * Set the value of a property in the working_fdt. */ - } else if (argv[1][0] == 's') { + } else if (strncmp(argv[1], "se", 2) == 0) { char *pathp; /* path */ char *prop; /* property */ int nodeoffset; /* node offset from libfdt */ -- cgit v1.1 From 8474da946f58a127b2006c526ae6f9b5a968d422 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 28 Mar 2020 14:03:47 -0600 Subject: dm: core: Add logging on unbind failure This failure path is tricky to debug since it continues after failure and there are a lot of error paths. Add logging to help. Signed-off-by: Simon Glass --- drivers/core/device-remove.c | 20 ++++++++++++-------- drivers/core/uclass.c | 4 ++-- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c index ff5b28c..8736fd9 100644 --- a/drivers/core/device-remove.c +++ b/drivers/core/device-remove.c @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -30,11 +31,14 @@ int device_chld_unbind(struct udevice *dev, struct driver *drv) continue; ret = device_unbind(pos); - if (ret && !saved_ret) + if (ret && !saved_ret) { + log_warning("device '%s' failed to unbind\n", + pos->name); saved_ret = ret; + } } - return saved_ret; + return log_ret(saved_ret); } int device_chld_remove(struct udevice *dev, struct driver *drv, @@ -63,13 +67,13 @@ int device_unbind(struct udevice *dev) int ret; if (!dev) - return -EINVAL; + return log_msg_ret("dev", -EINVAL); if (dev->flags & DM_FLAG_ACTIVATED) - return -EINVAL; + return log_msg_ret("active", -EINVAL); if (!(dev->flags & DM_FLAG_BOUND)) - return -EINVAL; + return log_msg_ret("not-bound", -EINVAL); drv = dev->driver; assert(drv); @@ -77,12 +81,12 @@ int device_unbind(struct udevice *dev) if (drv->unbind) { ret = drv->unbind(dev); if (ret) - return ret; + return log_msg_ret("unbind", ret); } ret = device_chld_unbind(dev, NULL); if (ret) - return ret; + return log_msg_ret("child unbind", ret); if (dev->flags & DM_FLAG_ALLOC_PDATA) { free(dev->platdata); @@ -98,7 +102,7 @@ int device_unbind(struct udevice *dev) } ret = uclass_unbind_device(dev); if (ret) - return ret; + return log_msg_ret("uc", ret); if (dev->parent) list_del(&dev->sibling_node); diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index 58b19a4..b24b677 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -120,10 +120,10 @@ int uclass_destroy(struct uclass *uc) uclass_node); ret = device_remove(dev, DM_REMOVE_NORMAL); if (ret) - return ret; + return log_msg_ret("remove", ret); ret = device_unbind(dev); if (ret) - return ret; + return log_msg_ret("unbind", ret); } uc_drv = uc->uc_drv; -- cgit v1.1 From ced1080489077ab9943c319a38c2d89adb215f1f Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 28 Mar 2020 14:03:48 -0600 Subject: dm: core: Add a way to skip powering down power domains When removing a device the power domains it uses are generally powered off. But when we are trying to unbind all devices (e.g. for running tests) we don't want to probe a device in the 'remove' path. Add a new flag to skip this power-down step. Signed-off-by: Simon Glass --- drivers/core/device-remove.c | 3 ++- drivers/core/uclass.c | 2 +- include/dm/device.h | 11 +++++++---- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c index 8736fd9..efdb0f2 100644 --- a/drivers/core/device-remove.c +++ b/drivers/core/device-remove.c @@ -198,7 +198,8 @@ int device_remove(struct udevice *dev, uint flags) } } - if (!(drv->flags & + if (!(flags & DM_REMOVE_NO_PD) && + !(drv->flags & (DM_FLAG_DEFAULT_PD_CTRL_OFF | DM_FLAG_REMOVE_WITH_PD_ON)) && dev != gd->cur_serial_dev) dev_power_domain_off(dev); diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index b24b677..6849302 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -118,7 +118,7 @@ int uclass_destroy(struct uclass *uc) while (!list_empty(&uc->dev_head)) { dev = list_first_entry(&uc->dev_head, struct udevice, uclass_node); - ret = device_remove(dev, DM_REMOVE_NORMAL); + ret = device_remove(dev, DM_REMOVE_NORMAL | DM_REMOVE_NO_PD); if (ret) return log_msg_ret("remove", ret); ret = device_unbind(dev); diff --git a/include/dm/device.h b/include/dm/device.h index a56164b..17e57bf 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -80,18 +80,21 @@ struct driver_info; */ enum { /* Normal remove, remove all devices */ - DM_REMOVE_NORMAL = 1 << 0, + DM_REMOVE_NORMAL = 1 << 0, /* Remove devices with active DMA */ - DM_REMOVE_ACTIVE_DMA = DM_FLAG_ACTIVE_DMA, + DM_REMOVE_ACTIVE_DMA = DM_FLAG_ACTIVE_DMA, /* Remove devices which need some final OS preparation steps */ - DM_REMOVE_OS_PREPARE = DM_FLAG_OS_PREPARE, + DM_REMOVE_OS_PREPARE = DM_FLAG_OS_PREPARE, /* Add more use cases here */ /* Remove devices with any active flag */ - DM_REMOVE_ACTIVE_ALL = DM_REMOVE_ACTIVE_DMA | DM_REMOVE_OS_PREPARE, + DM_REMOVE_ACTIVE_ALL = DM_REMOVE_ACTIVE_DMA | DM_REMOVE_OS_PREPARE, + + /* Don't power down any attached power domains */ + DM_REMOVE_NO_PD = 1 << 1, }; /** -- cgit v1.1 From 70573c6c46be96d2e60497d8484b9afb119da8c1 Mon Sep 17 00:00:00 2001 From: Dario Binacchi Date: Sun, 29 Mar 2020 18:04:40 +0200 Subject: dm: test: add test case for dev_read_u64 function Add test case to cover dev_read_u64 and dev_read_u64_default functions. Signed-off-by: Dario Binacchi Reviewed-by: Simon Glass --- arch/sandbox/dts/test.dts | 1 + include/test/ut.h | 16 ++++++++++++++++ test/dm/test-fdt.c | 10 ++++++++++ 3 files changed, 27 insertions(+) diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 2338064..8c6a48d 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -93,6 +93,7 @@ <&gpio_b 9 0xc 3 2 1>; int-value = <1234>; uint-value = <(-1234)>; + int64-value = /bits/ 64 <0x1111222233334444>; interrupts-extended = <&irq 3 0>; }; diff --git a/include/test/ut.h b/include/test/ut.h index 04df8ba..ab86158 100644 --- a/include/test/ut.h +++ b/include/test/ut.h @@ -104,6 +104,22 @@ int ut_check_console_dump(struct unit_test_state *uts, int total_bytes); } \ } +/* Assert that two 64 int expressions are equal */ +#define ut_asserteq_64(expr1, expr2) { \ + u64 _val1 = (expr1), _val2 = (expr2); \ + \ + if (_val1 != _val2) { \ + ut_failf(uts, __FILE__, __LINE__, __func__, \ + #expr1 " == " #expr2, \ + "Expected %#llx (%lld), got %#llx (%lld)", \ + (unsigned long long)_val1, \ + (unsigned long long)_val1, \ + (unsigned long long)_val2, \ + (unsigned long long)_val2); \ + return CMD_RET_FAILURE; \ + } \ +} + /* Assert that two string expressions are equal */ #define ut_asserteq_str(expr1, expr2) { \ const char *_val1 = (expr1), *_val2 = (expr2); \ diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 3500ab1..b39777f 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -867,6 +867,7 @@ static int dm_test_read_int(struct unit_test_state *uts) u32 val32; s32 sval; uint val; + u64 val64; ut_assertok(uclass_first_device_err(UCLASS_TEST_FDT, &dev)); ut_asserteq_str("a-test", dev->name); @@ -891,6 +892,15 @@ static int dm_test_read_int(struct unit_test_state *uts) ut_assertok(dev_read_u32u(dev, "uint-value", &val)); ut_asserteq(-1234, val); + ut_assertok(dev_read_u64(dev, "int64-value", &val64)); + ut_asserteq_64(0x1111222233334444, val64); + + ut_asserteq_64(-EINVAL, dev_read_u64(dev, "missing", &val64)); + ut_asserteq_64(6, dev_read_u64_default(dev, "missing", 6)); + + ut_asserteq_64(0x1111222233334444, + dev_read_u64_default(dev, "int64-value", 6)); + return 0; } DM_TEST(dm_test_read_int, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); -- cgit v1.1 From 4bb7075c830c6f4e4512fe0277ff1f08c5a9e084 Mon Sep 17 00:00:00 2001 From: Dario Binacchi Date: Sun, 29 Mar 2020 18:04:41 +0200 Subject: dm: core: support reading a single indexed u32 value The patch adds helper functions to allow reading a single indexed u32 value from a device-tree property containing multiple u32 values, that is an array of integers. Signed-off-by: Dario Binacchi Reviewed-by: Simon Glass --- arch/sandbox/dts/test.dts | 1 + drivers/core/of_access.c | 22 ++++++++++++++++++++++ drivers/core/ofnode.c | 40 ++++++++++++++++++++++++++++++++++++++++ drivers/core/read.c | 13 +++++++++++++ include/dm/of_access.h | 19 +++++++++++++++++++ include/dm/ofnode.h | 25 +++++++++++++++++++++++++ include/dm/read.h | 40 ++++++++++++++++++++++++++++++++++++++++ test/dm/test-fdt.c | 29 +++++++++++++++++++++++++++++ 8 files changed, 189 insertions(+) diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 8c6a48d..1973623 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -94,6 +94,7 @@ int-value = <1234>; uint-value = <(-1234)>; int64-value = /bits/ 64 <0x1111222233334444>; + int-array = <5678 9123 4567>; interrupts-extended = <&irq 3 0>; }; diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c index 29e705e..8b2ce7a 100644 --- a/drivers/core/of_access.c +++ b/drivers/core/of_access.c @@ -485,6 +485,28 @@ int of_read_u32_array(const struct device_node *np, const char *propname, return 0; } +int of_read_u32_index(const struct device_node *np, const char *propname, + int index, u32 *outp) +{ + const __be32 *val; + + debug("%s: %s: ", __func__, propname); + if (!np) + return -EINVAL; + + val = of_find_property_value_of_size(np, propname, + sizeof(*outp) * (index + 1)); + if (IS_ERR(val)) { + debug("(not found)\n"); + return PTR_ERR(val); + } + + *outp = be32_to_cpup(val + index); + debug("%#x (%d)\n", *outp, *outp); + + return 0; +} + int of_read_u64(const struct device_node *np, const char *propname, u64 *outp) { const __be64 *val; diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c index 96a5dd2..5bc3b02 100644 --- a/drivers/core/ofnode.c +++ b/drivers/core/ofnode.c @@ -48,6 +48,46 @@ u32 ofnode_read_u32_default(ofnode node, const char *propname, u32 def) return def; } +int ofnode_read_u32_index(ofnode node, const char *propname, int index, + u32 *outp) +{ + const fdt32_t *cell; + int len; + + assert(ofnode_valid(node)); + debug("%s: %s: ", __func__, propname); + + if (ofnode_is_np(node)) + return of_read_u32_index(ofnode_to_np(node), propname, index, + outp); + + cell = fdt_getprop(gd->fdt_blob, ofnode_to_offset(node), propname, + &len); + if (!cell) { + debug("(not found)\n"); + return -EINVAL; + } + + if (len < (sizeof(int) * (index + 1))) { + debug("(not large enough)\n"); + return -EOVERFLOW; + } + + *outp = fdt32_to_cpu(cell[index]); + debug("%#x (%d)\n", *outp, *outp); + + return 0; +} + +u32 ofnode_read_u32_index_default(ofnode node, const char *propname, int index, + u32 def) +{ + assert(ofnode_valid(node)); + ofnode_read_u32_index(node, propname, index, &def); + + return def; +} + int ofnode_read_s32_default(ofnode node, const char *propname, s32 def) { assert(ofnode_valid(node)); diff --git a/drivers/core/read.c b/drivers/core/read.c index 1f999b1..ce78f09 100644 --- a/drivers/core/read.c +++ b/drivers/core/read.c @@ -22,6 +22,19 @@ int dev_read_u32_default(const struct udevice *dev, const char *propname, return ofnode_read_u32_default(dev_ofnode(dev), propname, def); } +int dev_read_u32_index(struct udevice *dev, const char *propname, int index, + u32 *outp) +{ + return ofnode_read_u32_index(dev_ofnode(dev), propname, index, outp); +} + +u32 dev_read_u32_index_default(struct udevice *dev, const char *propname, + int index, u32 def) +{ + return ofnode_read_u32_index_default(dev_ofnode(dev), propname, index, + def); +} + int dev_read_s32(const struct udevice *dev, const char *propname, s32 *outp) { return ofnode_read_u32(dev_ofnode(dev), propname, (u32 *)outp); diff --git a/include/dm/of_access.h b/include/dm/of_access.h index 13fedb7..92876b3 100644 --- a/include/dm/of_access.h +++ b/include/dm/of_access.h @@ -235,6 +235,25 @@ struct device_node *of_find_node_by_phandle(phandle handle); int of_read_u32(const struct device_node *np, const char *propname, u32 *outp); /** + * of_read_u32_index() - Find and read a 32-bit value from a multi-value + * property + * + * Search for a property in a device node and read a 32-bit value from + * it. + * + * @np: device node from which the property value is to be read. + * @propname: name of the property to be searched. + * @index: index of the u32 in the list of values + * @outp: pointer to return value, modified only if return value is 0. + * + * @return 0 on success, -EINVAL if the property does not exist, + * -ENODATA if property does not have a value, and -EOVERFLOW if the + * property data isn't large enough. + */ +int of_read_u32_index(const struct device_node *np, const char *propname, + int index, u32 *outp); + +/** * of_read_u64() - Find and read a 64-bit integer from a property * * Search for a property in a device node and read a 64-bit value from diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h index b5a50e8..ce5e366 100644 --- a/include/dm/ofnode.h +++ b/include/dm/ofnode.h @@ -203,6 +203,18 @@ static inline ofnode ofnode_null(void) int ofnode_read_u32(ofnode node, const char *propname, u32 *outp); /** + * ofnode_read_u32_index() - Read a 32-bit integer from a multi-value property + * + * @ref: valid node reference to read property from + * @propname: name of the property to read from + * @index: index of the integer to return + * @outp: place to put value (if found) + * @return 0 if OK, -ve on error + */ +int ofnode_read_u32_index(ofnode node, const char *propname, int index, + u32 *outp); + +/** * ofnode_read_s32() - Read a 32-bit integer from a property * * @ref: valid node reference to read property from @@ -227,6 +239,19 @@ static inline int ofnode_read_s32(ofnode node, const char *propname, u32 ofnode_read_u32_default(ofnode ref, const char *propname, u32 def); /** + * ofnode_read_u32_index_default() - Read a 32-bit integer from a multi-value + * property + * + * @ref: valid node reference to read property from + * @propname: name of the property to read from + * @index: index of the integer to return + * @def: default value to return if the property has no value + * @return property value, or @def if not found + */ +u32 ofnode_read_u32_index_default(ofnode ref, const char *propname, int index, + u32 def); + +/** * ofnode_read_s32_default() - Read a 32-bit integer from a property * * @ref: valid node reference to read property from diff --git a/include/dm/read.h b/include/dm/read.h index da8c7f2..77d3bc8 100644 --- a/include/dm/read.h +++ b/include/dm/read.h @@ -67,6 +67,32 @@ int dev_read_u32_default(const struct udevice *dev, const char *propname, int def); /** + * dev_read_u32_index() - read an indexed 32-bit integer from a device's DT + * property + * + * @dev: device to read DT property from + * @propname: name of the property to read from + * @index: index of the integer to return + * @outp: place to put value (if found) + * @return 0 if OK, -ve on error + */ +int dev_read_u32_index(struct udevice *dev, const char *propname, int index, + u32 *outp); + +/** + * dev_read_u32_index_default() - read an indexed 32-bit integer from a device's + * DT property + * + * @dev: device to read DT property from + * @propname: name of the property to read from + * @index: index of the integer to return + * @def: default value to return if the property has no value + * @return property value, or @def if not found + */ +u32 dev_read_u32_index_default(struct udevice *dev, const char *propname, + int index, u32 def); + +/** * dev_read_s32() - read a signed 32-bit integer from a device's DT property * * @dev: device to read DT property from @@ -621,6 +647,20 @@ static inline int dev_read_u32_default(const struct udevice *dev, return ofnode_read_u32_default(dev_ofnode(dev), propname, def); } +static inline int dev_read_u32_index(struct udevice *dev, + const char *propname, int index, u32 *outp) +{ + return ofnode_read_u32_index(dev_ofnode(dev), propname, index, outp); +} + +static inline u32 dev_read_u32_index_default(struct udevice *dev, + const char *propname, int index, + u32 def) +{ + return ofnode_read_u32_index_default(dev_ofnode(dev), propname, index, + def); +} + static inline int dev_read_s32(const struct udevice *dev, const char *propname, s32 *outp) { diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index b39777f..7c9472a 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -905,6 +905,35 @@ static int dm_test_read_int(struct unit_test_state *uts) } DM_TEST(dm_test_read_int, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); +static int dm_test_read_int_index(struct unit_test_state *uts) +{ + struct udevice *dev; + u32 val32; + + ut_assertok(uclass_first_device_err(UCLASS_TEST_FDT, &dev)); + ut_asserteq_str("a-test", dev->name); + + ut_asserteq(-EINVAL, dev_read_u32_index(dev, "missing", 0, &val32)); + ut_asserteq(19, dev_read_u32_index_default(dev, "missing", 0, 19)); + + ut_assertok(dev_read_u32_index(dev, "int-array", 0, &val32)); + ut_asserteq(5678, val32); + ut_assertok(dev_read_u32_index(dev, "int-array", 1, &val32)); + ut_asserteq(9123, val32); + ut_assertok(dev_read_u32_index(dev, "int-array", 2, &val32)); + ut_asserteq(4567, val32); + ut_asserteq(-EOVERFLOW, dev_read_u32_index(dev, "int-array", 3, + &val32)); + + ut_asserteq(5678, dev_read_u32_index_default(dev, "int-array", 0, 2)); + ut_asserteq(9123, dev_read_u32_index_default(dev, "int-array", 1, 2)); + ut_asserteq(4567, dev_read_u32_index_default(dev, "int-array", 2, 2)); + ut_asserteq(2, dev_read_u32_index_default(dev, "int-array", 3, 2)); + + return 0; +} +DM_TEST(dm_test_read_int_index, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); + /* Test iteration through devices by drvdata */ static int dm_test_uclass_drvdata(struct unit_test_state *uts) { -- cgit v1.1 From 59006608d65b242b19176f1a1fdeeb99391654d2 Mon Sep 17 00:00:00 2001 From: Dario Binacchi Date: Sun, 29 Mar 2020 18:04:42 +0200 Subject: dm: core: refactor functions reading an u32 from dt Now reading a 32 bit value from a device-tree property can be expressed as reading the first element of an array with a single value. Signed-off-by: Dario Binacchi Reviewed-by: Simon Glass --- drivers/core/of_access.c | 16 +--------------- drivers/core/ofnode.c | 23 ++--------------------- 2 files changed, 3 insertions(+), 36 deletions(-) diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c index 8b2ce7a..c54baa1 100644 --- a/drivers/core/of_access.c +++ b/drivers/core/of_access.c @@ -449,21 +449,7 @@ static void *of_find_property_value_of_size(const struct device_node *np, int of_read_u32(const struct device_node *np, const char *propname, u32 *outp) { - const __be32 *val; - - debug("%s: %s: ", __func__, propname); - if (!np) - return -EINVAL; - val = of_find_property_value_of_size(np, propname, sizeof(*outp)); - if (IS_ERR(val)) { - debug("(not found)\n"); - return PTR_ERR(val); - } - - *outp = be32_to_cpup(val); - debug("%#x (%d)\n", *outp, *outp); - - return 0; + return of_read_u32_index(np, propname, 0, outp); } int of_read_u32_array(const struct device_node *np, const char *propname, diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c index 5bc3b02..b0be7cb 100644 --- a/drivers/core/ofnode.c +++ b/drivers/core/ofnode.c @@ -18,32 +18,13 @@ int ofnode_read_u32(ofnode node, const char *propname, u32 *outp) { - assert(ofnode_valid(node)); - debug("%s: %s: ", __func__, propname); - - if (ofnode_is_np(node)) { - return of_read_u32(ofnode_to_np(node), propname, outp); - } else { - const fdt32_t *cell; - int len; - - cell = fdt_getprop(gd->fdt_blob, ofnode_to_offset(node), - propname, &len); - if (!cell || len < sizeof(int)) { - debug("(not found)\n"); - return -EINVAL; - } - *outp = fdt32_to_cpu(cell[0]); - } - debug("%#x (%d)\n", *outp, *outp); - - return 0; + return ofnode_read_u32_index(node, propname, 0, outp); } u32 ofnode_read_u32_default(ofnode node, const char *propname, u32 def) { assert(ofnode_valid(node)); - ofnode_read_u32(node, propname, &def); + ofnode_read_u32_index(node, propname, 0, &def); return def; } -- cgit v1.1 From 5c9c9bc9571f23d4c8bab18fc50fc1238da0c6c1 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Fri, 3 Apr 2020 11:39:18 +0200 Subject: dm: core: remove the duplicated function dm_ofnode_pre_reloc The content dm_ofnode_pre_reloc() is identical with ofnode_pre_reloc() defined in drivers/core/ofnode.c and used only three times: - drivers/core/lists.c:lists_bind_fdt() - drivers/clk/at91/pmc.c::at91_clk_sub_device_bind - drivers/clk/altera/clk-arria10.c::socfpga_a10_clk_bind So this function dm_ofnode_pre_reloc can be removed and replaced by these function calls by ofnode_pre_reloc(). Signed-off-by: Patrick Delaunay Acked-by: Simon Glass --- drivers/clk/altera/clk-arria10.c | 2 +- drivers/clk/at91/pmc.c | 2 +- drivers/core/lists.c | 2 +- drivers/core/util.c | 28 ---------------------------- include/dm/util.h | 27 --------------------------- 5 files changed, 3 insertions(+), 58 deletions(-) diff --git a/drivers/clk/altera/clk-arria10.c b/drivers/clk/altera/clk-arria10.c index b7eed94..694a942 100644 --- a/drivers/clk/altera/clk-arria10.c +++ b/drivers/clk/altera/clk-arria10.c @@ -258,7 +258,7 @@ static int socfpga_a10_clk_bind(struct udevice *dev) continue; if (pre_reloc_only && - !dm_ofnode_pre_reloc(offset_to_ofnode(offset))) + !ofnode_pre_reloc(offset_to_ofnode(offset))) continue; ret = device_bind_driver_to_node(dev, "clk-a10", name, diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c index 6b55ec5..f580844 100644 --- a/drivers/clk/at91/pmc.c +++ b/drivers/clk/at91/pmc.c @@ -61,7 +61,7 @@ int at91_clk_sub_device_bind(struct udevice *dev, const char *drv_name) offset > 0; offset = fdt_next_subnode(fdt, offset)) { if (pre_reloc_only && - !dm_ofnode_pre_reloc(offset_to_ofnode(offset))) + !ofnode_pre_reloc(offset_to_ofnode(offset))) continue; /* * If this node has "compatible" property, this is not diff --git a/drivers/core/lists.c b/drivers/core/lists.c index 68204c3..c7db14e 100644 --- a/drivers/core/lists.c +++ b/drivers/core/lists.c @@ -175,7 +175,7 @@ int lists_bind_fdt(struct udevice *parent, ofnode node, struct udevice **devp, continue; if (pre_reloc_only) { - if (!dm_ofnode_pre_reloc(node) && + if (!ofnode_pre_reloc(node) && !(entry->flags & DM_FLAG_PRE_RELOC)) { log_debug("Skipping device pre-relocation\n"); return 0; diff --git a/drivers/core/util.c b/drivers/core/util.c index 69f8375..25b0d76 100644 --- a/drivers/core/util.c +++ b/drivers/core/util.c @@ -33,34 +33,6 @@ int list_count_items(struct list_head *head) return count; } -#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) -bool dm_ofnode_pre_reloc(ofnode node) -{ -#if defined(CONFIG_SPL_BUILD) || defined(CONFIG_TPL_BUILD) - /* for SPL and TPL the remaining nodes after the fdtgrep 1st pass - * had property dm-pre-reloc or u-boot,dm-spl/tpl. - * They are removed in final dtb (fdtgrep 2nd pass) - */ - return true; -#else - if (ofnode_read_bool(node, "u-boot,dm-pre-reloc")) - return true; - if (ofnode_read_bool(node, "u-boot,dm-pre-proper")) - return true; - - /* - * In regular builds individual spl and tpl handling both - * count as handled pre-relocation for later second init. - */ - if (ofnode_read_bool(node, "u-boot,dm-spl") || - ofnode_read_bool(node, "u-boot,dm-tpl")) - return true; - - return false; -#endif -} -#endif - #if !CONFIG_IS_ENABLED(OF_PLATDATA) int pci_get_devfn(struct udevice *dev) { diff --git a/include/dm/util.h b/include/dm/util.h index 0ccb3fb..23f8deb 100644 --- a/include/dm/util.h +++ b/include/dm/util.h @@ -42,31 +42,4 @@ static inline void dm_dump_devres(void) /* Dump out a list of drivers */ void dm_dump_drivers(void); -/** - * Check if an of node should be or was bound before relocation. - * - * Devicetree nodes can be marked as needed to be bound - * in the loader stages via special devicetree properties. - * - * Before relocation this function can be used to check if nodes - * are required in either SPL or TPL stages. - * - * After relocation and jumping into the real U-Boot binary - * it is possible to determine if a node was bound in one of - * SPL/TPL stages. - * - * There are 4 settings currently in use - * - u-boot,dm-pre-proper: U-Boot proper pre-relocation only - * - u-boot,dm-pre-reloc: legacy and indicates any of TPL or SPL - * Existing platforms only use it to indicate nodes needed in - * SPL. Should probably be replaced by u-boot,dm-spl for - * existing platforms. - * - u-boot,dm-spl: SPL and U-Boot pre-relocation - * - u-boot,dm-tpl: TPL and U-Boot pre-relocation - * @node: of node - * - * Returns true if node is needed in SPL/TL, false otherwise. - */ -bool dm_ofnode_pre_reloc(ofnode node); - #endif -- cgit v1.1 From b9200b191f62254036cd0d7818bfb24125b7585b Mon Sep 17 00:00:00 2001 From: Laurentiu Tudor Date: Fri, 3 Apr 2020 13:43:03 +0300 Subject: fdtdec: support multiple phandles in memory carveout fdtdec_set_carveout() is limited to only one phandle. Fix this limitation by adding support for multiple phandles. Signed-off-by: Laurentiu Tudor Reviewed-by: Simon Glass --- lib/fdtdec.c | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/lib/fdtdec.c b/lib/fdtdec.c index eb11fc8..9ecfa2a 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1433,14 +1433,9 @@ int fdtdec_set_carveout(void *blob, const char *node, const char *prop_name, const struct fdt_memory *carveout) { uint32_t phandle; - int err, offset; + int err, offset, len; fdt32_t value; - - /* XXX implement support for multiple phandles */ - if (index > 0) { - debug("invalid index %u\n", index); - return -FDT_ERR_BADOFFSET; - } + void *prop; err = fdtdec_add_reserved_memory(blob, name, carveout, &phandle); if (err < 0) { @@ -1456,10 +1451,31 @@ int fdtdec_set_carveout(void *blob, const char *node, const char *prop_name, value = cpu_to_fdt32(phandle); - err = fdt_setprop(blob, offset, prop_name, &value, sizeof(value)); + if (!fdt_getprop(blob, offset, prop_name, &len)) { + if (len == -FDT_ERR_NOTFOUND) + len = 0; + else + return len; + } + + if ((index + 1) * sizeof(value) > len) { + err = fdt_setprop_placeholder(blob, offset, prop_name, + (index + 1) * sizeof(value), + &prop); + if (err < 0) { + debug("failed to resize reserved memory property: %s\n", + fdt_strerror(err)); + return err; + } + } + + err = fdt_setprop_inplace_namelen_partial(blob, offset, prop_name, + strlen(prop_name), + index * sizeof(value), + &value, sizeof(value)); if (err < 0) { - debug("failed to set %s property for node %s: %d\n", prop_name, - node, err); + debug("failed to update %s property for node %s: %s\n", + prop_name, node, fdt_strerror(err)); return err; } -- cgit v1.1 From 528d6b37ae81a6111e53fb8717a95b802c72a476 Mon Sep 17 00:00:00 2001 From: Laurentiu Tudor Date: Fri, 3 Apr 2020 13:43:04 +0300 Subject: test: fdtdec: test fdtdec_set_carveout() Add a new test for fdtdec_set_carveout(). Signed-off-by: Laurentiu Tudor Reviewed-by: Simon Glass Drop blank line at EFO: Signed-off-by: Simon Glass --- test/dm/Makefile | 1 + test/dm/fdtdec.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 test/dm/fdtdec.c diff --git a/test/dm/Makefile b/test/dm/Makefile index dd1ceff..53caa29 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -31,6 +31,7 @@ obj-$(CONFIG_LED) += led.o obj-$(CONFIG_DM_MAILBOX) += mailbox.o obj-$(CONFIG_DM_MMC) += mmc.o obj-y += ofnode.o +obj-y += fdtdec.o obj-$(CONFIG_OSD) += osd.o obj-$(CONFIG_DM_VIDEO) += panel.o obj-$(CONFIG_DM_PCI) += pci.o diff --git a/test/dm/fdtdec.c b/test/dm/fdtdec.c new file mode 100644 index 0000000..b2f75b5 --- /dev/null +++ b/test/dm/fdtdec.c @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2020 NXP + */ + +#include +#include +#include +#include +#include + +static int dm_test_fdtdec_set_carveout(struct unit_test_state *uts) +{ + struct fdt_memory resv; + void *blob; + const fdt32_t *prop; + int blob_sz, len, offset; + + blob_sz = fdt_totalsize(gd->fdt_blob) + 4096; + blob = malloc(blob_sz); + ut_assertnonnull(blob); + + /* Make a writtable copy of the fdt blob */ + ut_assertok(fdt_open_into(gd->fdt_blob, blob, blob_sz)); + + resv.start = 0x1000; + resv.end = 0x2000; + ut_assertok(fdtdec_set_carveout(blob, "/a-test", + "memory-region", 2, "test_resv1", + &resv)); + + resv.start = 0x10000; + resv.end = 0x20000; + ut_assertok(fdtdec_set_carveout(blob, "/a-test", + "memory-region", 1, "test_resv2", + &resv)); + + resv.start = 0x100000; + resv.end = 0x200000; + ut_assertok(fdtdec_set_carveout(blob, "/a-test", + "memory-region", 0, "test_resv3", + &resv)); + + offset = fdt_path_offset(blob, "/a-test"); + ut_assert(offset > 0); + prop = fdt_getprop(blob, offset, "memory-region", &len); + ut_assertnonnull(prop); + + ut_asserteq(len, 12); + ut_assert(fdt_node_offset_by_phandle(blob, fdt32_to_cpu(prop[0])) > 0); + ut_assert(fdt_node_offset_by_phandle(blob, fdt32_to_cpu(prop[1])) > 0); + ut_assert(fdt_node_offset_by_phandle(blob, fdt32_to_cpu(prop[2])) > 0); + + free(blob); + + return 0; +} +DM_TEST(dm_test_fdtdec_set_carveout, + DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT | DM_TESTF_FLAT_TREE); -- cgit v1.1 From b0dcc87106464c3fc019e3771378a092fd32ebdb Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 5 Apr 2020 15:38:19 -0600 Subject: dm: core: Read parent ofdata before children At present a device can read its ofdata before its parent has done the same. This can cause problems in the case where the parent has a 'ranges' property, thus affecting the operation of dev_read_addr(), for example. We already probe parent devices before children so it does not seem to be a large step to do the same with ofdata. Make the change and update the documentation in this area. Signed-off-by: Simon Glass Tested-by: Ley Foon Tan --- doc/driver-model/design.rst | 94 +++++++++++++++++++++++++++++++++------------ drivers/core/device.c | 16 ++++++++ test/dm/test-fdt.c | 25 ++++++++++++ 3 files changed, 111 insertions(+), 24 deletions(-) diff --git a/doc/driver-model/design.rst b/doc/driver-model/design.rst index e37b51b..635effc 100644 --- a/doc/driver-model/design.rst +++ b/doc/driver-model/design.rst @@ -683,11 +683,17 @@ probe/remove which is independent of bind/unbind. This is partly because in U-Boot it may be expensive to probe devices and we don't want to do it until they are needed, or perhaps until after relocation. -Activation/probe -^^^^^^^^^^^^^^^^ +Reading ofdata +^^^^^^^^^^^^^^ + +Most devices have data in the device tree which they can read to find out the +base address of hardware registers and parameters relating to driver +operation. This is called 'ofdata' (Open-Firmware data). -When a device needs to be used, U-Boot activates it, by following these -steps (see device_probe()): +The device's_ofdata_to_platdata() implemnents allocation and reading of +platdata. A parent's ofdata is always read before a child. + +The steps are: 1. If priv_auto_alloc_size is non-zero, then the device-private space is allocated for the device and zeroed. It will be accessible as @@ -713,32 +719,72 @@ steps (see device_probe()): space. The controller can hold information about the USB state of each of its children. - 5. All parent devices are probed. It is not possible to activate a device + 5. If the driver provides an ofdata_to_platdata() method, then this is + called to convert the device tree data into platform data. This should + do various calls like dev_read_u32(dev, ...) to access the node and store + the resulting information into dev->platdata. After this point, the device + works the same way whether it was bound using a device tree node or + U_BOOT_DEVICE() structure. In either case, the platform data is now stored + in the platdata structure. Typically you will use the + platdata_auto_alloc_size feature to specify the size of the platform data + structure, and U-Boot will automatically allocate and zero it for you before + entry to ofdata_to_platdata(). But if not, you can allocate it yourself in + ofdata_to_platdata(). Note that it is preferable to do all the device tree + decoding in ofdata_to_platdata() rather than in probe(). (Apart from the + ugliness of mixing configuration and run-time data, one day it is possible + that U-Boot will cache platform data for devices which are regularly + de/activated). + + 5. The device is marked 'platdata valid'. + +Note that ofdata reading is always done (for a child and all its parents) +before probing starts. Thus devices go through two distinct states when +probing: reading platform data and actually touching the hardware to bring +the device up. + +Having probing separate from ofdata-reading helps deal with of-platdata, where +the probe() method is common to both DT/of-platdata operation, but the +ofdata_to_platdata() method is implemented differently. + +Another case has come up where this separate is useful. Generation of ACPI +tables uses the of-platdata but does not want to probe the device. Probing +would cause U-Boot to violate one of its design principles, viz that it +should only probe devices that are used. For ACPI we want to generate a +table for each device, even if U-Boot does not use it. In fact it may not +even be possible to probe the device - e.g. an SD card which is not +present will cause an error on probe, yet we still must tell Linux about +the SD card connector in case it is used while Linux is running. + +It is important that the ofdata_to_platdata() method does not actually probe +the device itself. However there are cases where other devices must be probed +in the ofdata_to_platdata() method. An example is where a device requires a +GPIO for it to operate. To select a GPIO obviously requires that the GPIO +device is probed. This is OK when used by common, core devices such as GPIO, +clock, interrupts, reset and the like. + +If your device relies on its parent setting up a suitable address space, so +that dev_read_addr() works correctly, then make sure that the parent device +has its setup code in ofdata_to_platdata(). If it has it in the probe method, +then you cannot call dev_read_addr() from the child device's +ofdata_to_platdata() method. Move it to probe() instead. Buses like PCI can +fall afoul of this rule. + +Activation/probe +^^^^^^^^^^^^^^^^ + +When a device needs to be used, U-Boot activates it, by first reading ofdata +as above and then following these steps (see device_probe()): + + 1. All parent devices are probed. It is not possible to activate a device unless its predecessors (all the way up to the root device) are activated. This means (for example) that an I2C driver will require that its bus be activated. - 6. The device's sequence number is assigned, either the requested one + 2. The device's sequence number is assigned, either the requested one (assuming no conflicts) or the next available one if there is a conflict or nothing particular is requested. - 7. If the driver provides an ofdata_to_platdata() method, then this is - called to convert the device tree data into platform data. This should - do various calls like fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), ...) - to access the node and store the resulting information into dev->platdata. - After this point, the device works the same way whether it was bound - using a device tree node or U_BOOT_DEVICE() structure. In either case, - the platform data is now stored in the platdata structure. Typically you - will use the platdata_auto_alloc_size feature to specify the size of the - platform data structure, and U-Boot will automatically allocate and zero - it for you before entry to ofdata_to_platdata(). But if not, you can - allocate it yourself in ofdata_to_platdata(). Note that it is preferable - to do all the device tree decoding in ofdata_to_platdata() rather than - in probe(). (Apart from the ugliness of mixing configuration and run-time - data, one day it is possible that U-Boot will cache platform data for - devices which are regularly de/activated). - - 8. The device's probe() method is called. This should do anything that + 4. The device's probe() method is called. This should do anything that is required by the device to get it going. This could include checking that the hardware is actually present, setting up clocks for the hardware and setting up hardware registers to initial values. The code @@ -753,7 +799,7 @@ steps (see device_probe()): allocate the priv space here yourself. The same applies also to platdata_auto_alloc_size. Remember to free them in the remove() method. - 9. The device is marked 'activated' + 5. The device is marked 'activated' 10. The uclass's post_probe() method is called, if one exists. This may cause the uclass to do some housekeeping to record the device as diff --git a/drivers/core/device.c b/drivers/core/device.c index 534cfa7..0157bb1 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -321,6 +321,22 @@ int device_ofdata_to_platdata(struct udevice *dev) if (dev->flags & DM_FLAG_PLATDATA_VALID) return 0; + /* Ensure all parents have ofdata */ + if (dev->parent) { + ret = device_ofdata_to_platdata(dev->parent); + if (ret) + goto fail; + + /* + * The device might have already been probed during + * the call to device_probe() on its parent device + * (e.g. PCI bridge devices). Test the flags again + * so that we don't mess up the device. + */ + if (dev->flags & DM_FLAG_PLATDATA_VALID) + return 0; + } + drv = dev->driver; assert(drv); diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 7c9472a..a56275a 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -992,3 +992,28 @@ static int dm_test_first_child_probe(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_first_child_probe, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); + +/* Test that ofdata is read for parents before children */ +static int dm_test_ofdata_order(struct unit_test_state *uts) +{ + struct udevice *bus, *dev; + + ut_assertok(uclass_find_first_device(UCLASS_I2C, &bus)); + ut_assertnonnull(bus); + ut_assert(!(bus->flags & DM_FLAG_PLATDATA_VALID)); + + ut_assertok(device_find_first_child(bus, &dev)); + ut_assertnonnull(dev); + ut_assert(!(dev->flags & DM_FLAG_PLATDATA_VALID)); + + /* read the child's ofdata which should cause the parent's to be read */ + ut_assertok(device_ofdata_to_platdata(dev)); + ut_assert(dev->flags & DM_FLAG_PLATDATA_VALID); + ut_assert(bus->flags & DM_FLAG_PLATDATA_VALID); + + ut_assert(!(dev->flags & DM_FLAG_ACTIVATED)); + ut_assert(!(bus->flags & DM_FLAG_ACTIVATED)); + + return 0; +} +DM_TEST(dm_test_ofdata_order, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); -- cgit v1.1