All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/14] Uthreads
@ 2025-02-25 16:34 Jerome Forissier
  2025-02-25 16:34 ` [PATCH v2 01/14] arch: introduce symbol HAVE_INITJMP Jerome Forissier
                   ` (13 more replies)
  0 siblings, 14 replies; 47+ messages in thread
From: Jerome Forissier @ 2025-02-25 16:34 UTC (permalink / raw)
  To: u-boot; +Cc: Ilias Apalodimas, Jerome Forissier

This series introduces threads and uses them to improve the performance
of the USB bus scanning code and to implement background jobs in the
shell via two new commands: 'spawn' and 'wait'.

The threading framework is called 'uthread' and is inspired from the
barebox threads [1]. setjmp() and longjmp() are used to save and
restore contexts, as well as a non-standard extension called initjmp().
This new function is added in several patches, one for each
architecture that supports HAVE_SETJMP. A new symbol is defined:
HAVE_INITJMP. Two tests, one for initjmp() and one for the uthread
scheduling, are added to the lib suite. NOTE: the SANDBOX version of
initjmp() appears to have problems and needs to be worked on.

[1] https://github.com/barebox/barebox/blob/master/common/bthread.c

After introducing threads and making schedule() and udelay() a thread
re-scheduling point, the USB stack initialization is modified to benefit
from concurrency when UTHREAD is enabled, where uthreads are used in
usb_init() to initialize and scan multiple busses at the same time.
The code was tested on arm64 and arm QEMU with 4 simulated XHCI buses
and some devices. On this platform the USB scan takes 2.2 s instead of
5.6 s. Tested on i.MX93 EVK with two USB hubs, one ethernet adapter and
one webcam on each, "usb start" takes 2.4 s instead of 4.6 s.

Finally, the spawn and wait commands are introduced, allowing the use of
threads from the shell. Tested on the i.MX93 EVK with a spinning HDD
connected to USB1 and the network connected to ENET1. The USB plus DHCP
init sequence "spawn usb start; spawn dhcp; wait" takes 4.5 seconds
instead of 8 seconds for "usb start; dhcp".

Changes in v2:
- Rewrite the cover letter, do not mention the older coroutines series
- Rebased onto next
- UTHREAD_STACK_SIZE is set to 32768 (32 KiB) instead of 32178
- Remove uthread_free_all() and let threads be freed as they terminate
by uthread_schedule()
- Add function descriptions
- Add documentation (doc/develop/uthread.rst)
- Explain initjmp() in the description of "arch: introduce symbol
HAVE_INITJMP".
- Add thread groups (uthread_grp_new_id() and uthread_grp_done())
- Add the spawn and wait commands

Jerome Forissier (14):
  arch: introduce symbol HAVE_INITJMP
  arm: add initjmp()
  riscv: add initjmp()
  sandbox: add initjmp()
  test: lib: add initjmp() test
  uthread: add cooperative multi-tasking interface
  cyclic: invoke uthread_schedule() from schedule()
  lib: time: hook uthread_schedule() into udelay()
  doc: develop: add documentation for uthreads
  test: lib: add uthread test
  dm: usb: move bus initialization into new static function
    usb_init_bus()
  dm: usb: initialize and scan multiple buses simultaneously with
    uthread
  cmd: add spawn and wait commands
  test: dm: add test for spawn and wait commands

 arch/Kconfig                      |   8 ++
 arch/arm/include/asm/setjmp.h     |   1 +
 arch/arm/lib/setjmp.S             |  11 ++
 arch/arm/lib/setjmp_aarch64.S     |   9 ++
 arch/riscv/include/asm/setjmp.h   |   1 +
 arch/riscv/lib/setjmp.S           |  10 ++
 arch/sandbox/cpu/Makefile         |  11 +-
 arch/sandbox/cpu/initjmp.c        | 172 +++++++++++++++++++++++++++++
 arch/sandbox/include/asm/setjmp.h |   5 +
 cmd/Kconfig                       |  17 +++
 cmd/Makefile                      |   2 +
 cmd/spawn.c                       | 176 +++++++++++++++++++++++++++++
 common/console.c                  |   2 +
 common/cyclic.c                   |   3 +
 doc/develop/index.rst             |   1 +
 doc/develop/uthread.rst           | 136 +++++++++++++++++++++++
 drivers/usb/host/usb-uclass.c     | 168 +++++++++++++++++++---------
 include/u-boot/schedule.h         |   3 +
 include/uthread.h                 |  44 ++++++++
 lib/Kconfig                       |  21 ++++
 lib/Makefile                      |   2 +
 lib/time.c                        |  10 +-
 lib/uthread.c                     | 178 ++++++++++++++++++++++++++++++
 test/boot/bootdev.c               |  14 +--
 test/boot/bootflow.c              |   2 +-
 test/cmd/Makefile                 |   1 +
 test/cmd/spawn.c                  |  33 ++++++
 test/lib/Makefile                 |   2 +
 test/lib/initjmp.c                |  72 ++++++++++++
 test/lib/uthread.c                |  68 ++++++++++++
 30 files changed, 1122 insertions(+), 61 deletions(-)
 create mode 100644 arch/sandbox/cpu/initjmp.c
 create mode 100644 cmd/spawn.c
 create mode 100644 doc/develop/uthread.rst
 create mode 100644 include/uthread.h
 create mode 100644 lib/uthread.c
 create mode 100644 test/cmd/spawn.c
 create mode 100644 test/lib/initjmp.c
 create mode 100644 test/lib/uthread.c

-- 
2.43.0


^ permalink raw reply	[flat|nested] 47+ messages in thread

* [PATCH v2 01/14] arch: introduce symbol HAVE_INITJMP
  2025-02-25 16:34 [PATCH v2 00/14] Uthreads Jerome Forissier
@ 2025-02-25 16:34 ` Jerome Forissier
  2025-02-28 12:01   ` Ilias Apalodimas
  2025-02-28 12:38   ` Heinrich Schuchardt
  2025-02-25 16:34 ` [PATCH v2 02/14] arm: add initjmp() Jerome Forissier
                   ` (12 subsequent siblings)
  13 siblings, 2 replies; 47+ messages in thread
From: Jerome Forissier @ 2025-02-25 16:34 UTC (permalink / raw)
  To: u-boot
  Cc: Ilias Apalodimas, Jerome Forissier, Tom Rini, Heinrich Schuchardt,
	Dan Carpenter, Simon Glass, Leo Yu-Chi Liang, Andrew Goodbody,
	Jiaxun Yang, Yu-Chien Peter Lin

HAVE_INIJMP will be set by architectures that support initjmp(), a
non-standard extension to setjmp()/longjmp() allowing to initialize a
jump buffer with a function pointer and a stack pointer. This will be
useful to later introduce threads. With this new function it becomes
possible to longjmp() to a particular function pointer (rather than
to a point previously reached during program execution as is the case
with setjmp()), and with a custom stack. Both things are needed to
spin off a new thread. Then the usual setjmp()/ longjmp() pair is
enough to save and restore a context, i.e., switch thread.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 arch/Kconfig | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/Kconfig b/arch/Kconfig
index 35b19f9bfdc..8d5b54031b3 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -13,6 +13,12 @@ config HAVE_SETJMP
 	help
 	 The architecture supports setjmp() and longjmp().
 
+config HAVE_INITJMP
+	bool
+	help
+	 The architecture supports initjmp(), a non-standard companion to
+	 setjmp() and longjmp().
+
 config SUPPORT_BIG_ENDIAN
 	bool
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH v2 02/14] arm: add initjmp()
  2025-02-25 16:34 [PATCH v2 00/14] Uthreads Jerome Forissier
  2025-02-25 16:34 ` [PATCH v2 01/14] arch: introduce symbol HAVE_INITJMP Jerome Forissier
@ 2025-02-25 16:34 ` Jerome Forissier
  2025-02-28 12:01   ` Ilias Apalodimas
  2025-02-28 13:05   ` Heinrich Schuchardt
  2025-02-25 16:34 ` [PATCH v2 03/14] riscv: " Jerome Forissier
                   ` (11 subsequent siblings)
  13 siblings, 2 replies; 47+ messages in thread
From: Jerome Forissier @ 2025-02-25 16:34 UTC (permalink / raw)
  To: u-boot
  Cc: Ilias Apalodimas, Jerome Forissier, Tom Rini, Heinrich Schuchardt,
	Simon Glass, Dan Carpenter, Andrew Goodbody, Jiaxun Yang,
	Yu-Chien Peter Lin

Implement initjmp() for Arm.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 arch/Kconfig                  |  1 +
 arch/arm/include/asm/setjmp.h |  1 +
 arch/arm/lib/setjmp.S         | 11 +++++++++++
 arch/arm/lib/setjmp_aarch64.S |  9 +++++++++
 4 files changed, 22 insertions(+)

diff --git a/arch/Kconfig b/arch/Kconfig
index 8d5b54031b3..57695fada8d 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -94,6 +94,7 @@ config ARC
 config ARM
 	bool "ARM architecture"
 	select HAVE_SETJMP
+	select HAVE_INITJMP
 	select ARCH_SUPPORTS_LTO
 	select CREATE_ARCH_SYMLINK
 	select HAVE_PRIVATE_LIBGCC if !ARM64
diff --git a/arch/arm/include/asm/setjmp.h b/arch/arm/include/asm/setjmp.h
index 662bec86321..1ad5b500f2a 100644
--- a/arch/arm/include/asm/setjmp.h
+++ b/arch/arm/include/asm/setjmp.h
@@ -23,5 +23,6 @@ typedef struct jmp_buf_data jmp_buf[1];
 
 int setjmp(jmp_buf jmp);
 void longjmp(jmp_buf jmp, int ret);
+int initjmp(jmp_buf jmp, void __noreturn (*func)(void), void *stack_top);
 
 #endif /* _SETJMP_H_ */
diff --git a/arch/arm/lib/setjmp.S b/arch/arm/lib/setjmp.S
index 2f041aeef01..320ddea85f9 100644
--- a/arch/arm/lib/setjmp.S
+++ b/arch/arm/lib/setjmp.S
@@ -34,3 +34,14 @@ ENTRY(longjmp)
 	ret  lr
 ENDPROC(longjmp)
 .popsection
+
+.pushsection .text.initjmp, "ax"
+ENTRY(initjmp)
+	stm  a1, {v1-v8}
+	/* a2: entry point address, a3: stack top */
+	str  a3, [a1, #32]  /* where setjmp would save sp */
+	str  a2, [a1, #36]  /* where setjmp would save lr */
+	mov  a1, #0
+	ret  lr
+ENDPROC(initjmp)
+.popsection
diff --git a/arch/arm/lib/setjmp_aarch64.S b/arch/arm/lib/setjmp_aarch64.S
index 1b8d000eb48..074320d25fb 100644
--- a/arch/arm/lib/setjmp_aarch64.S
+++ b/arch/arm/lib/setjmp_aarch64.S
@@ -39,3 +39,12 @@ ENTRY(longjmp)
 	ret
 ENDPROC(longjmp)
 .popsection
+
+.pushsection .text.initjmp, "ax"
+ENTRY(initjmp)
+	/* x1: entry point address, x2: stack top */
+	stp x1, x2, [x0,#88]
+	mov  x0, #0
+	ret
+ENDPROC(initjmp)
+.popsection
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH v2 03/14] riscv: add initjmp()
  2025-02-25 16:34 [PATCH v2 00/14] Uthreads Jerome Forissier
  2025-02-25 16:34 ` [PATCH v2 01/14] arch: introduce symbol HAVE_INITJMP Jerome Forissier
  2025-02-25 16:34 ` [PATCH v2 02/14] arm: add initjmp() Jerome Forissier
@ 2025-02-25 16:34 ` Jerome Forissier
  2025-02-25 16:34 ` [PATCH v2 04/14] sandbox: " Jerome Forissier
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 47+ messages in thread
From: Jerome Forissier @ 2025-02-25 16:34 UTC (permalink / raw)
  To: u-boot
  Cc: Ilias Apalodimas, Jerome Forissier, Tom Rini, Rick Chen, Leo,
	Dan Carpenter, Simon Glass, Heinrich Schuchardt,
	Yu-Chien Peter Lin, Jiaxun Yang, Andrew Goodbody

Implement initjmp() for RISC-V.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 arch/Kconfig                    |  1 +
 arch/riscv/include/asm/setjmp.h |  1 +
 arch/riscv/lib/setjmp.S         | 10 ++++++++++
 3 files changed, 12 insertions(+)

diff --git a/arch/Kconfig b/arch/Kconfig
index 57695fada8d..b745222bfbe 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -152,6 +152,7 @@ config RISCV
 	bool "RISC-V architecture"
 	select CREATE_ARCH_SYMLINK
 	select HAVE_SETJMP
+	select HAVE_INITJMP
 	select SUPPORT_ACPI
 	select SUPPORT_LITTLE_ENDIAN
 	select SUPPORT_OF_CONTROL
diff --git a/arch/riscv/include/asm/setjmp.h b/arch/riscv/include/asm/setjmp.h
index 72383d43303..98ea157eb3b 100644
--- a/arch/riscv/include/asm/setjmp.h
+++ b/arch/riscv/include/asm/setjmp.h
@@ -21,5 +21,6 @@ typedef struct jmp_buf_data jmp_buf[1];
 
 int setjmp(jmp_buf jmp);
 void longjmp(jmp_buf jmp, int ret);
+int initjmp(jmp_buf jmp, void __noreturn (*func)(void), void *stack_top);
 
 #endif /* _SETJMP_H_ */
diff --git a/arch/riscv/lib/setjmp.S b/arch/riscv/lib/setjmp.S
index 99d6195827e..6f952a16eee 100644
--- a/arch/riscv/lib/setjmp.S
+++ b/arch/riscv/lib/setjmp.S
@@ -59,3 +59,13 @@ ENTRY(longjmp)
 	ret
 ENDPROC(longjmp)
 .popsection
+
+.pushsection .text.initjmp, "ax"
+ENTRY(initjmp)
+	/* a1: entry point address, a2: stack top */
+	STORE_IDX(a1, 12)
+	STORE_IDX(a2, 13)
+	li  a0, 0
+	ret
+ENDPROC(initjmp)
+.popsection
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH v2 04/14] sandbox: add initjmp()
  2025-02-25 16:34 [PATCH v2 00/14] Uthreads Jerome Forissier
                   ` (2 preceding siblings ...)
  2025-02-25 16:34 ` [PATCH v2 03/14] riscv: " Jerome Forissier
@ 2025-02-25 16:34 ` Jerome Forissier
  2025-02-25 16:34 ` [PATCH v2 05/14] test: lib: add initjmp() test Jerome Forissier
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 47+ messages in thread
From: Jerome Forissier @ 2025-02-25 16:34 UTC (permalink / raw)
  To: u-boot; +Cc: Ilias Apalodimas, Jerome Forissier, Simon Glass, Tom Rini

Add ininijmp() to sandbox. The implementation is taken verbatim from
barebox [1]. It is quite complex because contrary to U-Boot platform
code we don't know how the system's C library implements the jump
buffer, so we can't just write the function and stack pointers into it.

FIXME: this patch should make SANDBOX select HAVE_INITJMP (in
arch/Kconfig). It does not due to the following error detected by CI:

 _________________________ test_ut[ut_lib_lib_initjmp] __________________________
 test/py/tests/test_ut.py:608: in test_ut
     output = u_boot_console.run_command('ut ' + ut_subtest)
 test/py/u_boot_console_base.py:334: in run_command
     m = self.p.expect([self.prompt_compiled] + self.bad_patterns)
 test/py/u_boot_spawn.py:296: in expect
     c = self.receive(1024)
 test/py/u_boot_spawn.py:235: in receive
     raise err
 test/py/u_boot_spawn.py:227: in receive
     c = os.read(self.fd, num_bytes).decode(errors='replace')
 E   OSError: [Errno 5] Input/output error
 ----------------------------- Captured stdout call -----------------------------
 => ut lib lib_initjmp
 Test: lib_initjmp: initjmp.c
 Failures: 0
 common/dlmalloc.c:796: do_check_free_chunk: Assertion `next == top || inuse(next)' failed.common/dlmalloc.c:796: do_check_free_chunk: Assertion `next == top || inuse(next)' failed.
 ---------------- generated xml file: /tmp/sandbox64/results.xml ----------------

On x86 the dmalloc error is not printed but the I/O error is still there.

[1] https://github.com/barebox/barebox/blob/b2a15c383ddc/arch/sandbox/os/setjmp.c

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 arch/sandbox/cpu/Makefile         |  11 +-
 arch/sandbox/cpu/initjmp.c        | 172 ++++++++++++++++++++++++++++++
 arch/sandbox/include/asm/setjmp.h |   5 +
 3 files changed, 187 insertions(+), 1 deletion(-)
 create mode 100644 arch/sandbox/cpu/initjmp.c

diff --git a/arch/sandbox/cpu/Makefile b/arch/sandbox/cpu/Makefile
index bfcdc335d32..038ad78accc 100644
--- a/arch/sandbox/cpu/Makefile
+++ b/arch/sandbox/cpu/Makefile
@@ -5,7 +5,7 @@
 # (C) Copyright 2000-2003
 # Wolfgang Denk, DENX Software Engineering, wd@denx.de.
 
-obj-y	:= cache.o cpu.o state.o
+obj-y	:= cache.o cpu.o state.o initjmp.o
 extra-y	:= start.o os.o
 extra-$(CONFIG_SANDBOX_SDL)    += sdl.o
 obj-$(CONFIG_XPL_BUILD)	+= spl.o
@@ -29,6 +29,15 @@ cmd_cc_eth-raw-os.o = $(CC) $(filter-out -nostdinc, \
 $(obj)/eth-raw-os.o: $(src)/eth-raw-os.c FORCE
 	$(call if_changed_dep,cc_eth-raw-os.o)
 
+# initjmp.c is build in the system environment, so needs standard includes
+# CFLAGS_REMOVE_initjmp.o cannot be used to drop header include path
+quiet_cmd_cc_initjmp.o = CC $(quiet_modtag)  $@
+cmd_cc_initjmp.o = $(CC) $(filter-out -nostdinc, \
+	$(patsubst -I%,-idirafter%,$(c_flags))) -c -o $@ $<
+
+$(obj)/initjmp.o: $(src)/initjmp.c FORCE
+	$(call if_changed_dep,cc_initjmp.o)
+
 # sdl.c fails to build with -fshort-wchar using musl
 cmd_cc_sdl.o = $(CC) $(filter-out -nostdinc -fshort-wchar, \
 	$(patsubst -I%,-idirafter%,$(c_flags))) -fno-lto -c -o $@ $<
diff --git a/arch/sandbox/cpu/initjmp.c b/arch/sandbox/cpu/initjmp.c
new file mode 100644
index 00000000000..c99721423c5
--- /dev/null
+++ b/arch/sandbox/cpu/initjmp.c
@@ -0,0 +1,172 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+/*
+ * An implementation of initjmp() in C, that plays well with the system's
+ * setjmp() and longjmp() functions.
+ * Taken verbatim from arch/sandbox/os/setjmp.c in the barebox project.
+ *
+ * Copyright (C) 2006  Anthony Liguori <anthony@codemonkey.ws>
+ * Copyright (C) 2011  Kevin Wolf <kwolf@redhat.com>
+ * Copyright (C) 2012  Alex Barcelo <abarcelo@ac.upc.edu>
+ * Copyright (C) 2021  Ahmad Fatoum, Pengutronix
+ * This file is partly based on pth_mctx.c, from the GNU Portable Threads
+ *  Copyright (c) 1999-2006 Ralf S. Engelschall <rse@engelschall.com>
+ */
+
+/* XXX Is there a nicer way to disable glibc's stack check for longjmp? */
+#ifdef _FORTIFY_SOURCE
+#undef _FORTIFY_SOURCE
+#endif
+
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <setjmp.h>
+#include <signal.h>
+
+typedef sigjmp_buf _jmp_buf __attribute__((aligned((16))));
+_Static_assert(sizeof(_jmp_buf) <= 512, "sigjmp_buf size exceeds expectation");
+
+/*
+ * Information for the signal handler (trampoline)
+ */
+static struct {
+	_jmp_buf *reenter;
+	void (*entry)(void);
+	volatile sig_atomic_t called;
+} tr_state;
+
+/*
+ * "boot" function
+ * This is what starts the coroutine, is called from the trampoline
+ * (from the signal handler when it is not signal handling, read ahead
+ * for more information).
+ */
+static void __attribute__((noinline, noreturn))
+coroutine_bootstrap(void (*entry)(void))
+{
+	for (;;)
+		entry();
+}
+
+/*
+ * This is used as the signal handler. This is called with the brand new stack
+ * (thanks to sigaltstack). We have to return, given that this is a signal
+ * handler and the sigmask and some other things are changed.
+ */
+static void coroutine_trampoline(int signal)
+{
+	/* Get the thread specific information */
+	tr_state.called = 1;
+
+	/*
+	 * Here we have to do a bit of a ping pong between the caller, given that
+	 * this is a signal handler and we have to do a return "soon". Then the
+	 * caller can reestablish everything and do a siglongjmp here again.
+	 */
+	if (!sigsetjmp(*tr_state.reenter, 0)) {
+		return;
+	}
+
+	/*
+	 * Ok, the caller has siglongjmp'ed back to us, so now prepare
+	 * us for the real machine state switching. We have to jump
+	 * into another function here to get a new stack context for
+	 * the auto variables (which have to be auto-variables
+	 * because the start of the thread happens later). Else with
+	 * PIC (i.e. Position Independent Code which is used when PTH
+	 * is built as a shared library) most platforms would
+	 * horrible core dump as experience showed.
+	 */
+	coroutine_bootstrap(tr_state.entry);
+}
+
+int __attribute__((weak)) initjmp(_jmp_buf jmp, void (*func)(void), void *stack_top)
+{
+	struct sigaction sa;
+	struct sigaction osa;
+	stack_t ss;
+	stack_t oss;
+	sigset_t sigs;
+	sigset_t osigs;
+
+	/* The way to manipulate stack is with the sigaltstack function. We
+	 * prepare a stack, with it delivering a signal to ourselves and then
+	 * put sigsetjmp/siglongjmp where needed.
+	 * This has been done keeping coroutine-ucontext (from the QEMU project)
+	 * as a model and with the pth ideas (GNU Portable Threads).
+	 * See coroutine-ucontext for the basics of the coroutines and see
+	 * pth_mctx.c (from the pth project) for the
+	 * sigaltstack way of manipulating stacks.
+	 */
+
+	tr_state.entry = func;
+	tr_state.reenter = (void *)jmp;
+
+	/*
+	 * Preserve the SIGUSR2 signal state, block SIGUSR2,
+	 * and establish our signal handler. The signal will
+	 * later transfer control onto the signal stack.
+	 */
+	sigemptyset(&sigs);
+	sigaddset(&sigs, SIGUSR2);
+	pthread_sigmask(SIG_BLOCK, &sigs, &osigs);
+	sa.sa_handler = coroutine_trampoline;
+	sigfillset(&sa.sa_mask);
+	sa.sa_flags = SA_ONSTACK;
+	if (sigaction(SIGUSR2, &sa, &osa) != 0) {
+		return -1;
+	}
+
+	/*
+	 * Set the new stack.
+	 */
+	ss.ss_sp = stack_top - CONFIG_STACK_SIZE;
+	ss.ss_size = CONFIG_STACK_SIZE;
+	ss.ss_flags = 0;
+	if (sigaltstack(&ss, &oss) < 0) {
+		return -1;
+	}
+
+	/*
+	 * Now transfer control onto the signal stack and set it up.
+	 * It will return immediately via "return" after the sigsetjmp()
+	 * was performed. Be careful here with race conditions.  The
+	 * signal can be delivered the first time sigsuspend() is
+	 * called.
+	 */
+	tr_state.called = 0;
+	pthread_kill(pthread_self(), SIGUSR2);
+	sigfillset(&sigs);
+	sigdelset(&sigs, SIGUSR2);
+	while (!tr_state.called) {
+		sigsuspend(&sigs);
+	}
+
+	/*
+	 * Inform the system that we are back off the signal stack by
+	 * removing the alternative signal stack. Be careful here: It
+	 * first has to be disabled, before it can be removed.
+	 */
+	sigaltstack(NULL, &ss);
+	ss.ss_flags = SS_DISABLE;
+	if (sigaltstack(&ss, NULL) < 0) {
+		return -1;
+	}
+	sigaltstack(NULL, &ss);
+	if (!(oss.ss_flags & SS_DISABLE)) {
+		sigaltstack(&oss, NULL);
+	}
+
+	/*
+	 * Restore the old SIGUSR2 signal handler and mask
+	 */
+	sigaction(SIGUSR2, &osa, NULL);
+	pthread_sigmask(SIG_SETMASK, &osigs, NULL);
+
+	/*
+	 * jmp can now be used to enter the trampoline again, but not as a
+	 * signal handler. Instead it's longjmp'd to directly.
+	 */
+	return 0;
+}
+
diff --git a/arch/sandbox/include/asm/setjmp.h b/arch/sandbox/include/asm/setjmp.h
index 001c7ea322d..d708e6da3fc 100644
--- a/arch/sandbox/include/asm/setjmp.h
+++ b/arch/sandbox/include/asm/setjmp.h
@@ -31,5 +31,10 @@ typedef struct jmp_buf_data jmp_buf[1];
  */
 int setjmp(jmp_buf jmp);
 __noreturn void longjmp(jmp_buf jmp, int ret);
+/*
+ * initjmp() is non-standard, still it has to play well with the system versions
+ * of setjmp()/longjmp().
+ */
+int initjmp(jmp_buf jmp, void __noreturn (*func)(void), void *stack_top);
 
 #endif /* _SETJMP_H_ */
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH v2 05/14] test: lib: add initjmp() test
  2025-02-25 16:34 [PATCH v2 00/14] Uthreads Jerome Forissier
                   ` (3 preceding siblings ...)
  2025-02-25 16:34 ` [PATCH v2 04/14] sandbox: " Jerome Forissier
@ 2025-02-25 16:34 ` Jerome Forissier
  2025-02-28 12:38   ` Ilias Apalodimas
  2025-02-28 13:04   ` Heinrich Schuchardt
  2025-02-25 16:34 ` [PATCH v2 06/14] uthread: add cooperative multi-tasking interface Jerome Forissier
                   ` (8 subsequent siblings)
  13 siblings, 2 replies; 47+ messages in thread
From: Jerome Forissier @ 2025-02-25 16:34 UTC (permalink / raw)
  To: u-boot
  Cc: Ilias Apalodimas, Jerome Forissier, Tom Rini, Simon Glass,
	Heinrich Schuchardt, Raymond Mao, Philippe Reynes

Test the initjmp() function when HAVE_INITJMP is set.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 test/lib/Makefile  |  1 +
 test/lib/initjmp.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)
 create mode 100644 test/lib/initjmp.c

diff --git a/test/lib/Makefile b/test/lib/Makefile
index 0e4cb8e3dfd..bf04685dae1 100644
--- a/test/lib/Makefile
+++ b/test/lib/Makefile
@@ -14,6 +14,7 @@ obj-y += hexdump.o
 obj-$(CONFIG_SANDBOX) += kconfig.o
 obj-y += lmb.o
 obj-$(CONFIG_HAVE_SETJMP) += longjmp.o
+obj-$(CONFIG_HAVE_INITJMP) += initjmp.o
 obj-$(CONFIG_CONSOLE_RECORD) += test_print.o
 obj-$(CONFIG_SSCANF) += sscanf.o
 obj-$(CONFIG_$(PHASE_)STRTO) += str.o
diff --git a/test/lib/initjmp.c b/test/lib/initjmp.c
new file mode 100644
index 00000000000..52bbda8cc60
--- /dev/null
+++ b/test/lib/initjmp.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2025 Linaro Limited
+ *
+ * Unit test for initjmp()
+ */
+
+#include <compiler.h>
+#include <asm/setjmp.h>
+#include <stdbool.h>
+#include <test/lib.h>
+#include <test/ut.h>
+#include <vsprintf.h>
+
+static bool ep_entered;
+static jmp_buf return_buf;
+
+static void __noreturn entrypoint(void)
+{
+	ep_entered = true;
+
+	/* Jump back to the main routine */
+	longjmp(return_buf, 1);
+
+	/* Not reached */
+	panic("longjmp failed\n");
+}
+
+static int lib_initjmp(struct unit_test_state *uts)
+{
+	int ret;
+	void *stack;
+	jmp_buf buf;
+	int stack_sz = 1024;
+
+	ep_entered = false;
+
+	stack = malloc(stack_sz);
+	ut_assertnonnull(stack);
+
+	ut_assert(!ep_entered);
+
+	/*
+	 * Prepare return_buf so that entrypoint may jump back just after the
+	 * if()
+	 */
+	if (!setjmp(return_buf)) {
+		/* return_buf initialized, entrypoint not yet called */
+
+		/*
+		 * Prepare another jump buffer to jump into entrypoint with the
+		 * given stack
+		 */
+		ret = initjmp(buf, entrypoint, stack + stack_sz);
+		ut_assertok(ret);
+
+		/* Jump into entrypoint */
+		longjmp(buf, 1);
+		/*
+		 * Not reached since entrypoint is expected to branch after
+		 * the if()
+		 */
+		ut_assert(false);
+	}
+
+	ut_assert(ep_entered);
+
+	free(stack);
+
+	return 0;
+}
+LIB_TEST(lib_initjmp, 0);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH v2 06/14] uthread: add cooperative multi-tasking interface
  2025-02-25 16:34 [PATCH v2 00/14] Uthreads Jerome Forissier
                   ` (4 preceding siblings ...)
  2025-02-25 16:34 ` [PATCH v2 05/14] test: lib: add initjmp() test Jerome Forissier
@ 2025-02-25 16:34 ` Jerome Forissier
  2025-02-28 13:09   ` Ilias Apalodimas
  2025-02-28 13:21   ` Heinrich Schuchardt
  2025-02-25 16:34 ` [PATCH v2 07/14] cyclic: invoke uthread_schedule() from schedule() Jerome Forissier
                   ` (7 subsequent siblings)
  13 siblings, 2 replies; 47+ messages in thread
From: Jerome Forissier @ 2025-02-25 16:34 UTC (permalink / raw)
  To: u-boot
  Cc: Ilias Apalodimas, Jerome Forissier, Tom Rini, Simon Glass,
	Sughosh Ganu, Raymond Mao, Patrick Rudolph, Michal Simek,
	Heinrich Schuchardt

Add an new internal API called uthread (Kconfig symbol: UTHREAD) which
provides cooperative multi-tasking. The goal is to be able to improve
the performance of some parts of U-Boot by overlapping lengthy
operations, and also implement background jobs in the U-Boot shell.
Each uthread has its own stack allocated on the heap. The default stack
size is defined by the UTHREAD_STACK_SIZE symbol and is used when
uthread_create() receives zero for the stack_sz argument.

The implementation is based on context-switching via initjmp()/setjmp()/
longjmp() and is inspired from barebox threads [1]. A notion of thread
group helps with dependencies, such as when a thread needs to block
until a number of other threads have returned.

The name "uthread" comes from "user-space threads" because the
scheduling happens with no help from a higher privileged mode, contrary
to more complex models where kernel threads are defined. But the 'u'
may as well stand for 'U-Boot' since the bootloader may actually be
running at any privilege level and the notion of user vs. kernel may
not make much sense in this context.

[1] https://github.com/barebox/barebox/blob/master/common/bthread.c

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 include/uthread.h |  44 ++++++++++++
 lib/Kconfig       |  21 ++++++
 lib/Makefile      |   2 +
 lib/uthread.c     | 178 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 245 insertions(+)
 create mode 100644 include/uthread.h
 create mode 100644 lib/uthread.c

diff --git a/include/uthread.h b/include/uthread.h
new file mode 100644
index 00000000000..f1f86d210d5
--- /dev/null
+++ b/include/uthread.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2025 Linaro Limited
+ */
+
+#include <linux/types.h>
+
+#ifndef _UTHREAD_H_
+#define _UTHREAD_H_
+
+#ifdef CONFIG_UTHREAD
+
+int uthread_create(void (*fn)(void *), void *arg, size_t stack_sz,
+		   unsigned int grp_id);
+bool uthread_schedule(void);
+unsigned int uthread_grp_new_id(void);
+bool uthread_grp_done(unsigned int grp_id);
+
+#else
+
+static inline int uthread_create(void (*fn)(void *), void *arg, size_t stack_sz,
+				 unsigned int grp_id)
+{
+	fn(arg);
+	return 0;
+}
+
+static inline bool uthread_schedule(void)
+{
+	return false;
+}
+
+static inline unsigned int uthread_grp_new_id(void)
+{
+	return 0;
+}
+
+static inline bool uthread_grp_done(unsigned int grp_id)
+{
+	return true;
+}
+
+#endif /* CONFIG_UTHREAD */
+#endif /* _UTHREAD_H_ */
diff --git a/lib/Kconfig b/lib/Kconfig
index 1a683dea670..b32740ecbcc 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -1255,6 +1255,27 @@ config PHANDLE_CHECK_SEQ
 	  enable this config option to distinguish them using
 	  phandles in fdtdec_get_alias_seq() function.
 
+config UTHREAD
+	bool "Enable thread support"
+	depends on HAVE_INITJMP
+	help
+	  Implement a simple form of cooperative multi-tasking based on
+	  context-switching via initjmp(), setjmp() and longjmp(). The
+	  uthread_ interface enables the main thread of execution to create
+	  one or more secondary threads and schedule them until they all have
+	  returned. At any point a thread may suspend its execution and
+	  schedule another thread, which allows for the efficient multiplexing
+	  of leghthy operations.
+
+config UTHREAD_STACK_SIZE
+	int "Default uthread stack size"
+	depends on UTHREAD
+	default 32768
+	help
+	  The default stak size for uthreads. Each uthread has its own stack.
+	  When the stack_sz argument to uthread_create() is zero then this
+	  value is used.
+
 endmenu
 
 source "lib/fwu_updates/Kconfig"
diff --git a/lib/Makefile b/lib/Makefile
index a7bc2f3134a..3610694de7a 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -164,6 +164,8 @@ obj-$(CONFIG_LIB_ELF) += elf.o
 
 obj-$(CONFIG_$(PHASE_)SEMIHOSTING) += semihosting.o
 
+obj-$(CONFIG_UTHREAD) += uthread.o
+
 #
 # Build a fast OID lookup registry from include/linux/oid_registry.h
 #
diff --git a/lib/uthread.c b/lib/uthread.c
new file mode 100644
index 00000000000..430d1c0de32
--- /dev/null
+++ b/lib/uthread.c
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2021 Ahmad Fatoum, Pengutronix
+ * Copyright (C) 2025 Linaro Limited
+ *
+ * An implementation of cooperative multi-tasking inspired from barebox threads
+ * https://github.com/barebox/barebox/blob/master/common/bthread.c
+ */
+
+#include <compiler.h>
+#include <asm/setjmp.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <malloc.h>
+#include <stdint.h>
+#include <uthread.h>
+
+static struct uthread {
+	void (*fn)(void *);
+	void *arg;
+	jmp_buf ctx;
+	void *stack;
+	bool done;
+	unsigned int grp_id;
+	struct list_head list;
+} main_thread = {
+	.list = LIST_HEAD_INIT(main_thread.list),
+};
+
+static struct uthread *current = &main_thread;
+
+/**
+ * uthread_trampoline() - Call the current thread's entry point then resume the
+ * main thread.
+ *
+ * This is a helper function which is used as the @func argument to the inijmp()
+ * function, and ultimately invoked via setjmp(). It does not return, but
+ * instead longjmp()'s back to the main thread.
+ */
+static void __noreturn uthread_trampoline(void)
+{
+	struct uthread *curr = current;
+
+	curr->fn(curr->arg);
+	curr->done = true;
+	current = &main_thread;
+	longjmp(current->ctx, 1);
+	/* Not reached */
+	while (true)
+		;
+}
+
+/**
+ * uthread_free() - Free memory used by a uthread object.
+ */
+static void uthread_free(struct uthread *uthread)
+{
+	if (!uthread)
+		return;
+	free(uthread->stack);
+	free(uthread);
+}
+
+/**
+ * uthread_create() - Create a uthread object and make it ready for execution
+ *
+ * Threads are automatically deleted when then return from their entry point.
+ *
+ * @fn: the thread's entry point
+ * @arg: argument passed to the thread's entry point
+ * @stack_sz: stack size for the new thread (in bytes). The stack is allocated
+ * on the heap.
+ * @grp_id: an optional thread group ID that the new thread should belong to
+ * (zero for no group)
+ */
+int uthread_create(void (*fn)(void *), void *arg, size_t stack_sz,
+		   unsigned int grp_id)
+{
+	struct uthread *uthread;
+
+	if (!stack_sz)
+		stack_sz = CONFIG_UTHREAD_STACK_SIZE;
+
+	uthread = calloc(1, sizeof(*uthread));
+	if (!uthread)
+		return -1;
+
+	uthread->stack = memalign(16, stack_sz);
+	if (!uthread->stack)
+		goto err;
+
+	uthread->fn = fn;
+	uthread->arg = arg;
+	uthread->grp_id = grp_id;
+
+	list_add_tail(&uthread->list, &current->list);
+
+	initjmp(uthread->ctx, uthread_trampoline, uthread->stack + stack_sz);
+
+	return 0;
+err:
+	uthread_free(uthread);
+	return -1;
+}
+
+/**
+ * uthread_resume() - switch execution to a given thread
+ *
+ * @uthread: the thread object that should be resumed
+ */
+static void uthread_resume(struct uthread *uthread)
+{
+	if (!setjmp(current->ctx)) {
+		current = uthread;
+		longjmp(uthread->ctx, 1);
+	}
+}
+
+/**
+ * uthread_schedule() - yield the CPU to the next runnable thread
+ *
+ * This function is called either by the main thread or any secondary thread
+ * (that is, any thread created via uthread_create()) to switch execution to
+ * the next runnable thread.
+ *
+ * Return: true if a thread was scheduled, false if no runnable thread was found
+ */
+bool uthread_schedule(void)
+{
+	struct uthread *next;
+	struct uthread *tmp;
+
+	if (list_empty(&current->list))
+	    return false;
+
+	list_for_each_entry_safe(next, tmp, &current->list, list) {
+		if (!next->done) {
+			uthread_resume(next);
+			return true;
+		} else {
+			/* Found a 'done' thread, free its resources */
+			list_del(&next->list);
+			uthread_free(next);
+		}
+	}
+	return false;
+}
+
+/**
+ * uthread_grp_new_id() - return a new ID for a thread group
+ *
+ * Return: the new thread group ID
+ */
+unsigned int uthread_grp_new_id(void)
+{
+	static unsigned int id = 0;
+
+	return ++id;
+}
+
+/**
+ * uthread_grp_done() - test if all threads in a group are done
+ *
+ * @grp: the ID of the thread group that should be considered
+ * Return: false if the group contains at least one runnable thread (i.e., one
+ * thread which entry point has not returned yet), true otherwise
+ */
+bool uthread_grp_done(unsigned int grp_id)
+{
+	struct uthread *next;
+
+	list_for_each_entry(next, &main_thread.list, list) {
+		if (next->grp_id == grp_id && !next->done)
+			return false;
+	}
+
+	return true;
+}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH v2 07/14] cyclic: invoke uthread_schedule() from schedule()
  2025-02-25 16:34 [PATCH v2 00/14] Uthreads Jerome Forissier
                   ` (5 preceding siblings ...)
  2025-02-25 16:34 ` [PATCH v2 06/14] uthread: add cooperative multi-tasking interface Jerome Forissier
@ 2025-02-25 16:34 ` Jerome Forissier
  2025-02-27 12:30   ` Stefan Roese
  2025-02-28 13:22   ` Ilias Apalodimas
  2025-02-25 16:34 ` [PATCH v2 08/14] lib: time: hook uthread_schedule() into udelay() Jerome Forissier
                   ` (6 subsequent siblings)
  13 siblings, 2 replies; 47+ messages in thread
From: Jerome Forissier @ 2025-02-25 16:34 UTC (permalink / raw)
  To: u-boot
  Cc: Ilias Apalodimas, Jerome Forissier, Stefan Roese, Tom Rini,
	Rasmus Villemoes, Simon Glass, Patrice Chotard

Make the schedule() call from the CYCLIC framework a uthread scheduling
point too. This makes sense since schedule() is called from a lot of
places where uthread_schedule() needs to be called.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 common/cyclic.c           | 3 +++
 include/u-boot/schedule.h | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/common/cyclic.c b/common/cyclic.c
index fad071a39c6..b695f092f52 100644
--- a/common/cyclic.c
+++ b/common/cyclic.c
@@ -16,6 +16,7 @@
 #include <linux/list.h>
 #include <asm/global_data.h>
 #include <u-boot/schedule.h>
+#include <uthread.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -100,6 +101,8 @@ void schedule(void)
 	 */
 	if (gd)
 		cyclic_run();
+
+	uthread_schedule();
 }
 
 int cyclic_unregister_all(void)
diff --git a/include/u-boot/schedule.h b/include/u-boot/schedule.h
index 4fd34c41229..4605971fdcb 100644
--- a/include/u-boot/schedule.h
+++ b/include/u-boot/schedule.h
@@ -3,6 +3,8 @@
 #ifndef _U_BOOT_SCHEDULE_H
 #define _U_BOOT_SCHEDULE_H
 
+#include <uthread.h>
+
 #if CONFIG_IS_ENABLED(CYCLIC)
 /**
  * schedule() - Schedule all potentially waiting tasks
@@ -17,6 +19,7 @@ void schedule(void);
 
 static inline void schedule(void)
 {
+	uthread_schedule();
 }
 
 #endif
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH v2 08/14] lib: time: hook uthread_schedule() into udelay()
  2025-02-25 16:34 [PATCH v2 00/14] Uthreads Jerome Forissier
                   ` (6 preceding siblings ...)
  2025-02-25 16:34 ` [PATCH v2 07/14] cyclic: invoke uthread_schedule() from schedule() Jerome Forissier
@ 2025-02-25 16:34 ` Jerome Forissier
  2025-02-28 13:38   ` Ilias Apalodimas
  2025-02-25 16:34 ` [PATCH v2 09/14] doc: develop: add documentation for uthreads Jerome Forissier
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 47+ messages in thread
From: Jerome Forissier @ 2025-02-25 16:34 UTC (permalink / raw)
  To: u-boot; +Cc: Ilias Apalodimas, Jerome Forissier, Tom Rini, Simon Glass

Introduce a uthread scheduling loop into udelay() when CONFIG_UTHREAD
is enabled. This means that any uthread calling into udelay() may yield
to uthread and be scheduled again later.

While not strictly necessary since uthread_schedule() is already called
by schedule(), tests show that it is desirable to call it in a tight
loop instead of calling __usleep(). It gives more opportunities for
other threads to make progress and results in better performances.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 lib/time.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/time.c b/lib/time.c
index d88edafb196..d1a1a66f301 100644
--- a/lib/time.c
+++ b/lib/time.c
@@ -17,6 +17,7 @@
 #include <asm/global_data.h>
 #include <asm/io.h>
 #include <linux/delay.h>
+#include <uthread.h>
 
 #ifndef CFG_WD_PERIOD
 # define CFG_WD_PERIOD	(10 * 1000 * 1000)	/* 10 seconds default */
@@ -197,7 +198,14 @@ void udelay(unsigned long usec)
 	do {
 		schedule();
 		kv = usec > CFG_WD_PERIOD ? CFG_WD_PERIOD : usec;
-		__udelay(kv);
+		if (CONFIG_IS_ENABLED(UTHREAD)) {
+			ulong t0 = timer_get_us();
+			while (timer_get_us() < t0 + kv)
+				uthread_schedule();
+		} else {
+			__udelay(kv);
+		}
 		usec -= kv;
 	} while(usec);
+
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH v2 09/14] doc: develop: add documentation for uthreads
  2025-02-25 16:34 [PATCH v2 00/14] Uthreads Jerome Forissier
                   ` (7 preceding siblings ...)
  2025-02-25 16:34 ` [PATCH v2 08/14] lib: time: hook uthread_schedule() into udelay() Jerome Forissier
@ 2025-02-25 16:34 ` Jerome Forissier
  2025-02-25 16:34 ` [PATCH v2 10/14] test: lib: add uthread test Jerome Forissier
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 47+ messages in thread
From: Jerome Forissier @ 2025-02-25 16:34 UTC (permalink / raw)
  To: u-boot
  Cc: Ilias Apalodimas, Jerome Forissier, Tom Rini, Simon Glass,
	Heinrich Schuchardt, Mattijs Korpershoek, Alexander Dahl,
	Quentin Schulz

Add documentation for the uthread framework.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 doc/develop/index.rst   |   1 +
 doc/develop/uthread.rst | 136 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 137 insertions(+)
 create mode 100644 doc/develop/uthread.rst

diff --git a/doc/develop/index.rst b/doc/develop/index.rst
index d9f2a838207..89c171c2089 100644
--- a/doc/develop/index.rst
+++ b/doc/develop/index.rst
@@ -53,6 +53,7 @@ Implementation
    spl
    falcon
    uefi/index
+   uthread
    vbe
    version
 
diff --git a/doc/develop/uthread.rst b/doc/develop/uthread.rst
new file mode 100644
index 00000000000..a7dc48ebc9c
--- /dev/null
+++ b/doc/develop/uthread.rst
@@ -0,0 +1,136 @@
+.. SPDX-License-Identifier: GPL-2.0+
+.. (C) Copyright 2025 Linaro Limited
+
+Uthread Framework
+=================
+
+Introduction
+------------
+
+The uthread framework is a basic task scheduler that allows to run functions
+"in parallel" on a single CPU core. The scheduling is cooperative, not
+preemptive -- meaning that context switches from one task to another task is
+voluntary, via a call to uthread_schedule(). This characteristic makes thread
+synchronization much easier, because a thread cannot be interrupted in the
+middle of a critical section (reading from or writing to shared state, for
+instance).
+
+`CONFIG_UTHREAD` in lib/Kconfig enables the uthread framework. When disabled,
+the uthread_create()  and uthread_schedule() functions may still be used so
+that code differences between uthreads enabled and disabled can be reduced to
+a minimum. See details below.
+
+Function description
+--------------------
+
+See `lib/uthread.c`.
+
+Usage
+-----
+
+This section shows how uthreads may be used to convert sequential code
+into parallel code. Error handling is omitted for brevity.
+Consider the following:
+
+.. code-block:: C
+
+   static void init_foo(void)
+   {
+        start_foo();
+        while (!foo_is_ready())
+            udelay(10);
+   }
+
+   static void init_bar(void)
+   {
+        start_bar();
+        while (!bar_is_ready())
+            udelay(10);
+   }
+
+   void init_foo_bar(void)
+   {
+        init_foo();
+        init_bar();
+   }
+
+This example is a simplified version of typical device initialization, where
+some commands are sent to a device and the CPU needs to wait for the device
+to reply or change state after wich the device is known to be ready.
+Assuming devices 'foo' and 'bar' are independant, and assuming they both take
+some significant amount of time to initialize, then the above code is clearly
+suboptimal because device 'bar' is started only after 'foo' is ready, although
+it could have been started at the same time. Therefore a better version would
+be:
+
+.. code-block:: C
+
+   void init_foo_bar(void)
+   {
+        start_foo();
+        start_bar();
+        while (!foo_is_ready() || !bar_is_ready())
+            udelay(10);
+   }
+
+
+Unfortunately, refactoring the code like that is rarely so easy because
+init_foo() and init_bar() would in reality involve dozens of functions
+and result in deep call stacks. This is where uthreads are helpful. Here is
+how.
+
+.. code-block:: C
+
+   /* Unchanged */
+   static void init_foo(void)
+   {
+        start_foo();
+        while (!foo_is_ready())
+            udelay(10);
+   }
+
+   /* Unchanged */
+   static void init_bar(void)
+   {
+        start_bar();
+        while (!bar_is_ready())
+            udelay(10);
+   }
+
+   /* Added only because init_foo() does not take a (void *) */
+   static void do_init_foo(void *arg)
+   {
+        init_foo();
+   }
+
+   /* Added only because init_bar() does not take a (void *) */
+   static void do_init_bar(void *arg)
+   {
+        init_bar();
+   }
+
+   void init_foo_bar(void)
+   {
+       int id;
+
+       /* Allocate a thread group ID (optional) */
+       id = uthread_grp_new_id();
+       /* Create and start two threads */
+       uthread_create(do_init_foo, NULL, 0, id);
+       uthread_create(do_init_bar, NULL, 0, id);
+       /* Wait until both threads are done */
+       while (!uthread_grp_done(id))
+           uthread_schedule();
+   }
+
+When `CONFIG_UTHREAD` is enabled, do_init_foo() is started and quickly yields
+the CPU back to the main thread due to udelay() calling uthread_schedule().
+Then do_init_bar() is started and it also calls udelay(), which in turn calls
+uthread_schedule(). With the main thread entering the scheduling loop, we
+effectively have three tasks scheduled in a round-robin fashion until
+do_init_foo() and do_init_bar() are both done.
+
+when `CONFIG_UTHREAD` is disabled, uthread_grp_new_id() always returns 0,
+uthread_create() simply calls its first argument, uthread_grp_done() always
+returns true and uthread_schedule() does nothing. In this case, the code is
+functionally equivalent to the sequential version.
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH v2 10/14] test: lib: add uthread test
  2025-02-25 16:34 [PATCH v2 00/14] Uthreads Jerome Forissier
                   ` (8 preceding siblings ...)
  2025-02-25 16:34 ` [PATCH v2 09/14] doc: develop: add documentation for uthreads Jerome Forissier
@ 2025-02-25 16:34 ` Jerome Forissier
  2025-02-28 13:26   ` Ilias Apalodimas
  2025-02-25 16:34 ` [PATCH v2 11/14] dm: usb: move bus initialization into new static function usb_init_bus() Jerome Forissier
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 47+ messages in thread
From: Jerome Forissier @ 2025-02-25 16:34 UTC (permalink / raw)
  To: u-boot
  Cc: Ilias Apalodimas, Jerome Forissier, Tom Rini, Simon Glass,
	Heinrich Schuchardt, Raymond Mao, Philippe Reynes

Add a uhread framework test to the lib tests.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 test/lib/Makefile  |  1 +
 test/lib/uthread.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+)
 create mode 100644 test/lib/uthread.c

diff --git a/test/lib/Makefile b/test/lib/Makefile
index bf04685dae1..c991dff1c63 100644
--- a/test/lib/Makefile
+++ b/test/lib/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_CRC8) += test_crc8.o
 obj-$(CONFIG_UT_LIB_CRYPT) += test_crypt.o
 obj-$(CONFIG_UT_TIME) += time.o
 obj-$(CONFIG_$(XPL_)UT_UNICODE) += unicode.o
+obj-$(CONFIG_UTHREAD) += uthread.o
 obj-$(CONFIG_LIB_UUID) += uuid.o
 else
 obj-$(CONFIG_SANDBOX) += kconfig_spl.o
diff --git a/test/lib/uthread.c b/test/lib/uthread.c
new file mode 100644
index 00000000000..84c41d35875
--- /dev/null
+++ b/test/lib/uthread.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2025 Linaro Limited
+ *
+ * Unit test for uthread
+ */
+
+#include <stdbool.h>
+#include <test/lib.h>
+#include <test/ut.h>
+#include <uthread.h>
+
+static int count;
+
+static void worker(void *arg)
+{
+	int loops = (int)(unsigned long)arg;
+	int i;
+
+	for (i = 0; i < loops; i++) {
+		count++;
+		uthread_schedule();
+	}
+}
+
+static int lib_uthread(struct unit_test_state *uts)
+{
+	int i;
+	int id1, id2;
+
+	count = 0;
+	id1 = uthread_grp_new_id();
+	ut_assert(id1 != 0);
+	id2 = uthread_grp_new_id();
+	ut_assert(id2 != 0);
+	ut_assert(id1 != id2);
+	ut_assertok(uthread_create(worker, (void *)5, 0, id1));
+	ut_assertok(uthread_create(worker, (void *)10, 0, 0));
+	/*
+	 * The first call is expected to schedule the first worker, which will
+	 * schedule the second one, which will schedule back to the main thread
+	 * (here). Therefore count should be 2.
+	 */
+	ut_assert(uthread_schedule());
+	ut_asserteq(2, count);
+	ut_assert(!uthread_grp_done(id1));
+	/* Four more calls should bring the count to 10 */
+	for (i = 0; i < 4; i++) {
+		ut_assert(!uthread_grp_done(id1));
+		ut_assert(uthread_schedule());
+	}
+	ut_asserteq(10, count);
+	/* This one allows the first worker to exit */
+	ut_assert(uthread_schedule());
+	/* At this point there should be no runnable thread in group 'id1' */
+	ut_assert(uthread_grp_done(id1));
+	/* Five more calls for the second worker to finish incrementing  */
+	for (i = 0; i < 5; i++)
+		ut_assert(uthread_schedule());
+	ut_asserteq(15, count);
+	/* Plus one call to let the second worker return from its entry point */
+	ut_assert(uthread_schedule());
+	/* Now both tasks should be done, schedule should return false */
+	ut_assert(!uthread_schedule());
+
+	return 0;
+}
+LIB_TEST(lib_uthread, 0);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH v2 11/14] dm: usb: move bus initialization into new static function usb_init_bus()
  2025-02-25 16:34 [PATCH v2 00/14] Uthreads Jerome Forissier
                   ` (9 preceding siblings ...)
  2025-02-25 16:34 ` [PATCH v2 10/14] test: lib: add uthread test Jerome Forissier
@ 2025-02-25 16:34 ` Jerome Forissier
  2025-02-28 13:27   ` Ilias Apalodimas
  2025-02-25 16:34 ` [PATCH v2 12/14] dm: usb: initialize and scan multiple buses simultaneously with uthread Jerome Forissier
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 47+ messages in thread
From: Jerome Forissier @ 2025-02-25 16:34 UTC (permalink / raw)
  To: u-boot
  Cc: Ilias Apalodimas, Jerome Forissier, Marek Vasut, Tom Rini,
	Mattijs Korpershoek, Heinrich Schuchardt, Caleb Connolly,
	Dragan Simic

To prepare for the introduction of coroutines in the USB initialization
sequence, move code out of usb_init() into a new helper function:
usb_init_bus(). No functional change.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 drivers/usb/host/usb-uclass.c | 88 +++++++++++++++++++----------------
 1 file changed, 48 insertions(+), 40 deletions(-)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index bfec303e7af..cc803241461 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -287,9 +287,55 @@ static int usb_probe_companion(struct udevice *bus)
 	return 0;
 }
 
+static int controllers_initialized;
+
+static void usb_init_bus(struct udevice *bus)
+{
+	int ret;
+
+	/* init low_level USB */
+	printf("Bus %s: ", bus->name);
+
+	/*
+	 * For Sandbox, we need scan the device tree each time when we
+	 * start the USB stack, in order to re-create the emulated USB
+	 * devices and bind drivers for them before we actually do the
+	 * driver probe.
+	 *
+	 * For USB onboard HUB, we need to do some non-trivial init
+	 * like enabling a power regulator, before enumeration.
+	 */
+	if (IS_ENABLED(CONFIG_SANDBOX) ||
+	    IS_ENABLED(CONFIG_USB_ONBOARD_HUB)) {
+		ret = dm_scan_fdt_dev(bus);
+		if (ret) {
+			printf("USB device scan from fdt failed (%d)", ret);
+			return;
+		}
+	}
+
+	ret = device_probe(bus);
+	if (ret == -ENODEV) {	/* No such device. */
+		puts("Port not available.\n");
+		controllers_initialized++;
+		return;
+	}
+
+	if (ret) {		/* Other error. */
+		printf("probe failed, error %d\n", ret);
+		return;
+	}
+
+	ret = usb_probe_companion(bus);
+	if (ret)
+		return;
+
+	controllers_initialized++;
+	usb_started = true;
+}
+
 int usb_init(void)
 {
-	int controllers_initialized = 0;
 	struct usb_uclass_priv *uc_priv;
 	struct usb_bus_priv *priv;
 	struct udevice *bus;
@@ -305,45 +351,7 @@ int usb_init(void)
 	uc_priv = uclass_get_priv(uc);
 
 	uclass_foreach_dev(bus, uc) {
-		/* init low_level USB */
-		printf("Bus %s: ", bus->name);
-
-		/*
-		 * For Sandbox, we need scan the device tree each time when we
-		 * start the USB stack, in order to re-create the emulated USB
-		 * devices and bind drivers for them before we actually do the
-		 * driver probe.
-		 *
-		 * For USB onboard HUB, we need to do some non-trivial init
-		 * like enabling a power regulator, before enumeration.
-		 */
-		if (IS_ENABLED(CONFIG_SANDBOX) ||
-		    IS_ENABLED(CONFIG_USB_ONBOARD_HUB)) {
-			ret = dm_scan_fdt_dev(bus);
-			if (ret) {
-				printf("USB device scan from fdt failed (%d)", ret);
-				continue;
-			}
-		}
-
-		ret = device_probe(bus);
-		if (ret == -ENODEV) {	/* No such device. */
-			puts("Port not available.\n");
-			controllers_initialized++;
-			continue;
-		}
-
-		if (ret) {		/* Other error. */
-			printf("probe failed, error %d\n", ret);
-			continue;
-		}
-
-		ret = usb_probe_companion(bus);
-		if (ret)
-			continue;
-
-		controllers_initialized++;
-		usb_started = true;
+		usb_init_bus(bus);
 	}
 
 	/*
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH v2 12/14] dm: usb: initialize and scan multiple buses simultaneously with uthread
  2025-02-25 16:34 [PATCH v2 00/14] Uthreads Jerome Forissier
                   ` (10 preceding siblings ...)
  2025-02-25 16:34 ` [PATCH v2 11/14] dm: usb: move bus initialization into new static function usb_init_bus() Jerome Forissier
@ 2025-02-25 16:34 ` Jerome Forissier
  2025-02-27 16:25   ` Simon Glass
  2025-02-25 16:34 ` [PATCH v2 13/14] cmd: add spawn and wait commands Jerome Forissier
  2025-02-25 16:34 ` [PATCH v2 14/14] test: dm: add test for " Jerome Forissier
  13 siblings, 1 reply; 47+ messages in thread
From: Jerome Forissier @ 2025-02-25 16:34 UTC (permalink / raw)
  To: u-boot
  Cc: Ilias Apalodimas, Jerome Forissier, Marek Vasut, Tom Rini,
	Simon Glass, Mattijs Korpershoek, Heinrich Schuchardt,
	Caleb Connolly, Julius Lehmann, Guillaume La Roque

Use the uthread framework to initialize and scan USB buses in parallel
for better performance. The console output is slightly modified with a
final per-bus report of the number of devices found, common to UTHREAD
and !UTHREAD. The USB tests are updated accordingly.

Tested on two platforms:

1. arm64 QEMU on a somewhat contrived example (4 USB buses, each with
one audio device, one keyboard, one mouse and one tablet)

 $ make qemu_arm64_defconfig
 $ make -j$(nproc) CROSS_COMPILE="ccache aarch64-linux-gnu-"
 $ qemu-system-aarch64 -M virt -nographic -cpu max -bios u-boot.bin \
     $(for i in {1..4}; do echo -device qemu-xhci,id=xhci$i \
         -device\ usb-{audio,kbd,mouse,tablet},bus=xhci$i.0; \
     done)

2. i.MX93 EVK (imx93_11x11_evk_defconfig) with two USB hubs, each with
one webcam and one ethernet adapter, resulting in the following device
tree:

 USB device tree:
   1  Hub (480 Mb/s, 0mA)
   |  u-boot EHCI Host Controller
   |
   +-2  Hub (480 Mb/s, 100mA)
     |  GenesysLogic USB2.1 Hub
     |
     +-3  Vendor specific (480 Mb/s, 350mA)
     |    Realtek USB 10/100/1000 LAN 001000001
     |
     +-4   (480 Mb/s, 500mA)
           HD Pro Webcam C920 8F7CD51F

   1  Hub (480 Mb/s, 0mA)
   |  u-boot EHCI Host Controller
   |
   +-2  Hub (480 Mb/s, 100mA)
     |   USB 2.0 Hub
     |
     +-3  Vendor specific (480 Mb/s, 200mA)
     |    Realtek USB 10/100/1000 LAN 000001
     |
     +-4   (480 Mb/s, 500mA)
          Generic OnLan-CS30 201801010008

Note that i.MX was tested on top of the downstream repository [1] since
USB doesn't work in the upstream master branch.

[1] https://github.com/nxp-imx/uboot-imx/tree/lf-6.6.52-2.2.0
    commit 6c4545203d12 ("LF-13928 update key for capsule")

The time spent in usb_init() ("usb start" command) is reported on
the console. Here are the results:

        | CONFIG_UTHREAD=n | CONFIG_UTHREAD=y
--------+------------------+-----------------
QEMU    |          5628 ms |          2212 ms
i.MX93  |          4591 ms |          2441 ms

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 drivers/usb/host/usb-uclass.c | 92 ++++++++++++++++++++++++++++-------
 test/boot/bootdev.c           | 14 +++---
 test/boot/bootflow.c          |  2 +-
 3 files changed, 83 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index cc803241461..27c7aaee2b4 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -9,6 +9,7 @@
 #define LOG_CATEGORY UCLASS_USB
 
 #include <bootdev.h>
+#include <uthread.h>
 #include <dm.h>
 #include <errno.h>
 #include <log.h>
@@ -17,6 +18,7 @@
 #include <dm/device-internal.h>
 #include <dm/lists.h>
 #include <dm/uclass-internal.h>
+#include <time.h>
 
 static bool asynch_allowed;
 
@@ -221,25 +223,18 @@ int usb_stop(void)
 	return err;
 }
 
-static void usb_scan_bus(struct udevice *bus, bool recurse)
+static void _usb_scan_bus(void *arg)
 {
+	struct udevice *bus = (struct udevice *)arg;
 	struct usb_bus_priv *priv;
 	struct udevice *dev;
 	int ret;
 
 	priv = dev_get_uclass_priv(bus);
 
-	assert(recurse);	/* TODO: Support non-recusive */
-
-	printf("scanning bus %s for devices... ", bus->name);
-	debug("\n");
 	ret = usb_scan_device(bus, 0, USB_SPEED_FULL, &dev);
 	if (ret)
-		printf("failed, error %d\n", ret);
-	else if (priv->next_addr == 0)
-		printf("No USB Device found\n");
-	else
-		printf("%d USB Device(s) found\n", priv->next_addr);
+		printf("Scanning bus %s failed, error %d\n", bus->name, ret);
 }
 
 static void remove_inactive_children(struct uclass *uc, struct udevice *bus)
@@ -289,12 +284,12 @@ static int usb_probe_companion(struct udevice *bus)
 
 static int controllers_initialized;
 
-static void usb_init_bus(struct udevice *bus)
+static void _usb_init_bus(void *arg)
 {
+	struct udevice *bus = (struct udevice *)arg;
 	int ret;
 
 	/* init low_level USB */
-	printf("Bus %s: ", bus->name);
 
 	/*
 	 * For Sandbox, we need scan the device tree each time when we
@@ -309,33 +304,84 @@ static void usb_init_bus(struct udevice *bus)
 	    IS_ENABLED(CONFIG_USB_ONBOARD_HUB)) {
 		ret = dm_scan_fdt_dev(bus);
 		if (ret) {
-			printf("USB device scan from fdt failed (%d)", ret);
+			printf("Bus %s: USB device scan from fdt failed (%d)\n",
+			       bus->name, ret);
 			return;
 		}
 	}
 
 	ret = device_probe(bus);
 	if (ret == -ENODEV) {	/* No such device. */
-		puts("Port not available.\n");
+		printf("Bus %s: Port not available.\n", bus->name);
 		controllers_initialized++;
 		return;
 	}
 
 	if (ret) {		/* Other error. */
-		printf("probe failed, error %d\n", ret);
+		printf("Bus %s: probe failed, error %d\n", bus->name, ret);
 		return;
 	}
 
 	ret = usb_probe_companion(bus);
-	if (ret)
+	if (ret) {
 		return;
+	}
 
 	controllers_initialized++;
 	usb_started = true;
 }
 
+static int nthr;
+static int grp_id;
+
+static void usb_init_bus(struct udevice *bus)
+{
+	if (!grp_id)
+		grp_id = uthread_grp_new_id();
+	if (!uthread_create(_usb_init_bus, (void *)bus, 0, grp_id))
+		nthr++;
+}
+
+static void usb_scan_bus(struct udevice *bus, bool recurse)
+{
+	if (!grp_id)
+		grp_id = uthread_grp_new_id();
+	if (!uthread_create(_usb_scan_bus, (void *)bus, 0, grp_id))
+		nthr++;
+}
+
+static void usb_report_devices(struct uclass *uc)
+{
+	struct usb_bus_priv *priv;
+	struct udevice *bus;
+
+	uclass_foreach_dev(bus, uc) {
+		if (!device_active(bus))
+			continue;
+		priv = dev_get_uclass_priv(bus);
+		printf("Bus %s: ", bus->name);
+		if (priv->next_addr == 0)
+			printf("No USB Device found\n");
+		else
+			printf("%d USB Device(s) found\n", priv->next_addr);
+	}
+}
+
+static void run_threads(void)
+{
+#if CONFIG_IS_ENABLED(UTHREAD)
+	if (!nthr)
+		return;
+	while (!uthread_grp_done(grp_id))
+		uthread_schedule();
+	nthr = 0;
+	grp_id = 0;
+#endif
+}
+
 int usb_init(void)
 {
+	unsigned long t0 = timer_get_us();
 	struct usb_uclass_priv *uc_priv;
 	struct usb_bus_priv *priv;
 	struct udevice *bus;
@@ -354,6 +400,9 @@ int usb_init(void)
 		usb_init_bus(bus);
 	}
 
+	if (CONFIG_IS_ENABLED(UTHREAD))
+		run_threads();
+
 	/*
 	 * lowlevel init done, now scan the bus for devices i.e. search HUBs
 	 * and configure them, first scan primary controllers.
@@ -367,6 +416,9 @@ int usb_init(void)
 			usb_scan_bus(bus, true);
 	}
 
+	if (CONFIG_IS_ENABLED(UTHREAD))
+		run_threads();
+
 	/*
 	 * Now that the primary controllers have been scanned and have handed
 	 * over any devices they do not understand to their companions, scan
@@ -383,7 +435,10 @@ int usb_init(void)
 		}
 	}
 
-	debug("scan end\n");
+	if (CONFIG_IS_ENABLED(UTHREAD))
+		run_threads();
+
+	usb_report_devices(uc);
 
 	/* Remove any devices that were not found on this scan */
 	remove_inactive_children(uc, bus);
@@ -397,6 +452,9 @@ int usb_init(void)
 	if (controllers_initialized == 0)
 		printf("No USB controllers found\n");
 
+	debug("USB initialized in %ld ms\n",
+	       (timer_get_us() - t0) / 1000);
+
 	return usb_started ? 0 : -ENOENT;
 }
 
diff --git a/test/boot/bootdev.c b/test/boot/bootdev.c
index 5f07430714e..70a1d868de8 100644
--- a/test/boot/bootdev.c
+++ b/test/boot/bootdev.c
@@ -392,8 +392,8 @@ static int bootdev_test_hunter(struct unit_test_state *uts)
 	ut_assert_console_end();
 
 	ut_assertok(bootdev_hunt("usb1", false));
-	ut_assert_nextline(
-		"Bus usb@1: scanning bus usb@1 for devices... 5 USB Device(s) found");
+	ut_assert_skip_to_line(
+		"Bus usb@1: 5 USB Device(s) found");
 	ut_assert_console_end();
 
 	/* USB is 7th in the list, so bit 8 */
@@ -448,8 +448,8 @@ static int bootdev_test_cmd_hunt(struct unit_test_state *uts)
 	ut_assert_nextline("scanning bus for devices...");
 	ut_assert_skip_to_line("Hunting with: spi_flash");
 	ut_assert_nextline("Hunting with: usb");
-	ut_assert_nextline(
-		"Bus usb@1: scanning bus usb@1 for devices... 5 USB Device(s) found");
+	ut_assert_skip_to_line(
+		"Bus usb@1: 5 USB Device(s) found");
 	ut_assert_nextline("Hunting with: virtio");
 	ut_assert_console_end();
 
@@ -550,8 +550,8 @@ static int bootdev_test_hunt_prio(struct unit_test_state *uts)
 	ut_assertok(bootdev_hunt_prio(BOOTDEVP_5_SCAN_SLOW, true));
 	ut_assert_nextline("Hunting with: ide");
 	ut_assert_nextline("Hunting with: usb");
-	ut_assert_nextline(
-		"Bus usb@1: scanning bus usb@1 for devices... 5 USB Device(s) found");
+	ut_assert_skip_to_line(
+		"Bus usb@1: 5 USB Device(s) found");
 	ut_assert_console_end();
 
 	return 0;
@@ -603,7 +603,7 @@ static int bootdev_test_hunt_label(struct unit_test_state *uts)
 	ut_assertnonnull(dev);
 	ut_asserteq_str("usb_mass_storage.lun0.bootdev", dev->name);
 	ut_asserteq(BOOTFLOW_METHF_SINGLE_UCLASS, mflags);
-	ut_assert_nextlinen("Bus usb@1: scanning bus usb@1");
+	ut_assert_nextline("Bus usb@1: 5 USB Device(s) found");
 	ut_assert_console_end();
 
 	return 0;
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index eb7f00af39a..699ba0edaaf 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -1289,7 +1289,7 @@ static int bootflow_efi(struct unit_test_state *uts)
 
 	ut_assertok(run_command("bootflow scan", 0));
 	ut_assert_skip_to_line(
-		"Bus usb@1: scanning bus usb@1 for devices... 5 USB Device(s) found");
+		"Bus usb@1: 5 USB Device(s) found");
 
 	ut_assertok(run_command("bootflow list", 0));
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH v2 13/14] cmd: add spawn and wait commands
  2025-02-25 16:34 [PATCH v2 00/14] Uthreads Jerome Forissier
                   ` (11 preceding siblings ...)
  2025-02-25 16:34 ` [PATCH v2 12/14] dm: usb: initialize and scan multiple buses simultaneously with uthread Jerome Forissier
@ 2025-02-25 16:34 ` Jerome Forissier
  2025-02-25 16:34 ` [PATCH v2 14/14] test: dm: add test for " Jerome Forissier
  13 siblings, 0 replies; 47+ messages in thread
From: Jerome Forissier @ 2025-02-25 16:34 UTC (permalink / raw)
  To: u-boot
  Cc: Ilias Apalodimas, Jerome Forissier, Tom Rini, Simon Glass,
	Heinrich Schuchardt, Mattijs Korpershoek, Ibai Erkiaga,
	Michal Simek, Venkatesh Yadav Abbarapu, Sebastian Reichel,
	Dmitry Rokosov, Alexander Sverdlin, Kever Yang

Add a spawn command which runs another command in the background, as
well as a wait command to suspend the shell until one or more background
jobs have completed. The job_id environment variable is set by spawn and
wait accepts optional job ids, so that one can selectively wait on any
job.

Example:

 => date; spawn sleep 5; spawn sleep 3; date; echo "waiting..."; wait; date
 Date: 2025-02-21 (Friday)    Time: 17:04:52
 Date: 2025-02-21 (Friday)    Time: 17:04:52
 waiting...
 Date: 2025-02-21 (Friday)    Time: 17:04:57
 =>

Another example showing how background jobs can make initlizations
faster. The board is i.MX93 EVK, with one spinning HDD connected to
USB1 via a hub, and a network cable plugged into ENET1.

 # From power up / reset
 u-boot=> setenv autoload 0
 u-boot=> setenv ud "usb start; dhcp"
 u-boot=> time run ud
 [...]
 time: 8.058 seconds

 # From power up / reset
 u-boot=> setenv autoload 0
 u-boot=> setenv ud "spawn usb start; spawn dhcp; wait"
 u-boot=> time run ud
 [...]
 time: 4.475 seconds

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 cmd/Kconfig      |  17 +++++
 cmd/Makefile     |   2 +
 cmd/spawn.c      | 176 +++++++++++++++++++++++++++++++++++++++++++++++
 common/console.c |   2 +
 4 files changed, 197 insertions(+)
 create mode 100644 cmd/spawn.c

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 8dd42571abc..209ca365f4c 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -3058,4 +3058,21 @@ config CMD_MESON
 	help
 	  Enable useful commands for the Meson Soc family developed by Amlogic Inc.
 
+config CMD_SPAWN
+	bool "spawn and wait commands"
+	depends on UTHREAD
+	help
+	  spawn runs a command in the background and sets the job_id environment
+	  variable. wait is used to suspend the shell execution until one or more
+	  jobs are complete.
+
+config CMD_SPAWN_NUM_JOBS
+	int "Maximum number of simultaneous jobs for spawn"
+	default 16
+	help
+	  Job identifiers are in the range 1..CMD_SPAWN_NUM_JOBS. In other words
+	  there can be no more that CMD_SPAWN_NUM_JOBS running simultaneously.
+	  When a jobs exits, its identifier is available to be re-used by the next
+	  spawn command.
+
 endif
diff --git a/cmd/Makefile b/cmd/Makefile
index 8410be576bb..eeda40855bf 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -240,6 +240,8 @@ obj-$(CONFIG_CMD_SCP03) += scp03.o
 
 obj-$(CONFIG_HUSH_SELECTABLE) += cli.o
 
+obj-$(CONFIG_CMD_SPAWN) += spawn.o
+
 obj-$(CONFIG_ARM) += arm/
 obj-$(CONFIG_RISCV) += riscv/
 obj-$(CONFIG_SANDBOX) += sandbox/
diff --git a/cmd/spawn.c b/cmd/spawn.c
new file mode 100644
index 00000000000..26532a30087
--- /dev/null
+++ b/cmd/spawn.c
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2011 The Chromium OS Authors.
+ */
+
+#include <command.h>
+#include <malloc.h>
+#include <vsprintf.h>
+#include <uthread.h>
+
+/* Spawn arguments and job index  */
+struct spa {
+	int argc;
+	char **argv;
+	unsigned int job_idx;
+};
+
+/*
+ * uthread group identifiers for each running job
+ * 0: job slot available, != 0: uthread group id
+ * Note that job[0] is job_id 1, job[1] is job_id 2 etc.
+ */
+static unsigned int job[CONFIG_CMD_SPAWN_NUM_JOBS];
+/*
+ * Return values of the commands run as jobs */
+static enum command_ret_t job_ret[CONFIG_CMD_SPAWN_NUM_JOBS];
+
+static void spa_free(struct spa *spa)
+{
+	int i;
+
+	if (!spa)
+		return;
+
+	for (i = 0; i < spa->argc; i++)
+		free(spa->argv[i]);
+	free(spa->argv);
+	free(spa);
+}
+
+static struct spa *spa_create(int argc, char *const argv[])
+{
+	struct spa *spa;
+	int i;
+
+	spa = calloc(1, sizeof(*spa));
+	if (!spa)
+		return NULL;
+	spa->argc = argc;
+	spa->argv = malloc(argc * sizeof(char *));
+	if (!spa->argv)
+		goto err;
+	for (i = 0; i < argc; i++) {
+		spa->argv[i] = strdup(argv[i]);
+		if (!spa->argv[i])
+			goto err;
+	}
+	return spa;
+err:
+	spa_free(spa);
+	return NULL;
+}
+
+static void spawn_thread(void *arg)
+{
+	struct spa *spa = (struct spa *)arg;
+	ulong cycles = 0;
+	int repeatable = 0;
+
+	job_ret[spa->job_idx] = cmd_process(0, spa->argc, spa->argv,
+					    &repeatable, &cycles);
+	spa_free(spa);
+}
+
+static unsigned int next_job_id(void)
+{
+	int i;
+
+	for (i = 0; i < CONFIG_CMD_SPAWN_NUM_JOBS; i++)
+		if (!job[i])
+			return i + 1;
+
+	/* No job available */
+	return 0;
+}
+
+static void refresh_jobs(void)
+{
+	int i;
+
+	for (i = 0; i < CONFIG_CMD_SPAWN_NUM_JOBS; i++)
+		if (job[i] && uthread_grp_done(job[i]))
+			job[i] = 0;
+
+}
+
+static int do_spawn(struct cmd_tbl *cmdtp, int flag, int argc,
+		    char *const argv[])
+{
+	unsigned int id;
+	unsigned int idx;
+	struct spa *spa;
+	int ret;
+
+	if (argc == 1)
+		return CMD_RET_USAGE;
+
+	spa = spa_create(argc - 1, argv + 1);
+	if (!spa)
+		return CMD_RET_FAILURE;
+
+	refresh_jobs();
+
+	id = next_job_id();
+	if (!id)
+		return CMD_RET_FAILURE;
+	idx = id - 1;
+
+	job[idx] = uthread_grp_new_id();
+
+	ret = uthread_create(spawn_thread, spa, 0, job[idx]);
+	if (ret) {
+		job[idx] = 0;
+		return CMD_RET_FAILURE;
+	}
+
+	ret = env_set_ulong("job_id", id);
+	if (ret)
+		return CMD_RET_FAILURE;
+
+	return CMD_RET_SUCCESS;
+}
+
+U_BOOT_CMD(spawn, CONFIG_SYS_MAXARGS, 0, do_spawn,
+	   "run commands and summarize execution time",
+	   "command [args...]\n");
+
+static enum command_ret_t wait_job(unsigned int idx)
+{
+	while (!uthread_grp_done(job[idx]))
+		uthread_schedule();
+	job[idx] = 0;
+	return job_ret[idx];
+}
+
+static int do_wait(struct cmd_tbl *cmdtp, int flag, int argc,
+		   char *const argv[])
+{
+	enum command_ret_t ret = CMD_RET_SUCCESS;
+	unsigned long id;
+	unsigned int idx;
+	int i;
+
+	if (argc == 1) {
+		for (i = 0; i < CONFIG_CMD_SPAWN_NUM_JOBS; i++)
+			if (job[idx])
+				ret = wait_job(i);
+	} else {
+		for (i = 1; i < argc; i++) {
+			id = dectoul(argv[i], NULL);
+			if (id < 0 || id > CONFIG_CMD_SPAWN_NUM_JOBS)
+				return CMD_RET_USAGE;
+			idx = (int)id - 1;
+			ret = wait_job(idx);
+		}
+	}
+
+	return ret;
+}
+
+U_BOOT_CMD(wait, CONFIG_SYS_MAXARGS, 0, do_wait,
+	   "wait for one or more jobs to complete",
+	   "[job_id ...]\n"
+	   "    - Wait until all specified jobs have exited and return the\n"
+	   "      exit status of the last job waited for. When no job_id is\n"
+	   "      given, wait for all the background jobs.\n");
diff --git a/common/console.c b/common/console.c
index 863ac6aa9dc..2c8eaa37ad7 100644
--- a/common/console.c
+++ b/common/console.c
@@ -24,6 +24,7 @@
 #include <watchdog.h>
 #include <asm/global_data.h>
 #include <linux/delay.h>
+#include <uthread.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -508,6 +509,7 @@ int fgetc(int file)
 		 */
 		for (;;) {
 			schedule();
+			uthread_schedule();
 			if (CONFIG_IS_ENABLED(CONSOLE_MUX)) {
 				/*
 				 * Upper layer may have already called tstc() so
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* [PATCH v2 14/14] test: dm: add test for spawn and wait commands
  2025-02-25 16:34 [PATCH v2 00/14] Uthreads Jerome Forissier
                   ` (12 preceding siblings ...)
  2025-02-25 16:34 ` [PATCH v2 13/14] cmd: add spawn and wait commands Jerome Forissier
@ 2025-02-25 16:34 ` Jerome Forissier
  2025-02-27 16:25   ` Simon Glass
  13 siblings, 1 reply; 47+ messages in thread
From: Jerome Forissier @ 2025-02-25 16:34 UTC (permalink / raw)
  To: u-boot
  Cc: Ilias Apalodimas, Jerome Forissier, Tom Rini, Simon Glass,
	Heinrich Schuchardt

Test the spawn and wait commands.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 test/cmd/Makefile |  1 +
 test/cmd/spawn.c  | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)
 create mode 100644 test/cmd/spawn.c

diff --git a/test/cmd/Makefile b/test/cmd/Makefile
index d8a5e77402d..cf47f04851c 100644
--- a/test/cmd/Makefile
+++ b/test/cmd/Makefile
@@ -39,3 +39,4 @@ obj-$(CONFIG_CMD_WGET) += wget.o
 endif
 obj-$(CONFIG_ARM_FFA_TRANSPORT) += armffa.o
 endif
+obj-$(CONFIG_CMD_SPAWN) += spawn.o
diff --git a/test/cmd/spawn.c b/test/cmd/spawn.c
new file mode 100644
index 00000000000..5e0770858c0
--- /dev/null
+++ b/test/cmd/spawn.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Tests for spawn and wait commands
+ *
+ * Copyright 2025, Linaro Ltd.
+ */
+
+#include <command.h>
+#include <dm.h>
+#include <dm/test.h>
+#include <test/test.h>
+#include <test/ut.h>
+
+static int dm_test_cmd_spawn(struct unit_test_state *uts)
+{
+	ut_assertok(run_command("wait; spawn sleep 2; setenv j ${job_id}; "
+				"spawn setenv spawned true; "
+				"setenv jj ${job_id}; wait; "
+				"echo ${j} ${jj} ${spawned}", 0));
+	console_record_readline(uts->actual_str, sizeof(uts->actual_str));
+	ut_asserteq_ptr(uts->actual_str,
+			strstr(uts->actual_str, "1 2 true"));
+
+	ut_assertok(run_command("spawn true; wait; setenv t $?; spawn false; "
+				"wait; setenv f $?; wait; echo $t $f $?", 0));
+	console_record_readline(uts->actual_str, sizeof(uts->actual_str));
+	ut_asserteq_ptr(uts->actual_str,
+			strstr(uts->actual_str, "0 1 0"));
+	ut_assert_console_end();
+
+	return 0;
+}
+DM_TEST(dm_test_cmd_spawn, UTF_CONSOLE);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 07/14] cyclic: invoke uthread_schedule() from schedule()
  2025-02-25 16:34 ` [PATCH v2 07/14] cyclic: invoke uthread_schedule() from schedule() Jerome Forissier
@ 2025-02-27 12:30   ` Stefan Roese
  2025-02-27 17:05     ` Jerome Forissier
  2025-02-28 13:22   ` Ilias Apalodimas
  1 sibling, 1 reply; 47+ messages in thread
From: Stefan Roese @ 2025-02-27 12:30 UTC (permalink / raw)
  To: Jerome Forissier, u-boot
  Cc: Ilias Apalodimas, Tom Rini, Rasmus Villemoes, Simon Glass,
	Patrice Chotard

Hi Jerome,

On 25.02.25 17:34, Jerome Forissier wrote:
> Make the schedule() call from the CYCLIC framework a uthread scheduling
> point too. This makes sense since schedule() is called from a lot of
> places where uthread_schedule() needs to be called.
> 
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>

Frankly, at first I was wondering a bit, if and why another framework
for "multitasking" is needed in U-Boot, additionally to the cyclic
framework that I introduced a few years ago. Which was greatly enhanced
by Rasmus over the time. But looking at your "uthread" implementation
it makes sense to add such a probably more intuitive interface as well.

In general I'm really happy seeing activity in this "multitaking" area
in U-Boot. As it brings a lot of new possibilities and, as you've also
shown in your patchset, may greatly help reducing boot time in the
USB example. :)

One question though:
Do you have some means in your uthread framework, measuring and
perhaps limiting the time spent in these uthreads? If and how is a
preemption of a uthread possible? So that it does not consume too
much time resulting in e.g. things like dropping input chars on
the prompt? Sorry, I did not thoroughly go through all your code
to get the internals from there. It would be great if you could
elaborate a bit on this.

For this patch:

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   common/cyclic.c           | 3 +++
>   include/u-boot/schedule.h | 3 +++
>   2 files changed, 6 insertions(+)
> 
> diff --git a/common/cyclic.c b/common/cyclic.c
> index fad071a39c6..b695f092f52 100644
> --- a/common/cyclic.c
> +++ b/common/cyclic.c
> @@ -16,6 +16,7 @@
>   #include <linux/list.h>
>   #include <asm/global_data.h>
>   #include <u-boot/schedule.h>
> +#include <uthread.h>
>   
>   DECLARE_GLOBAL_DATA_PTR;
>   
> @@ -100,6 +101,8 @@ void schedule(void)
>   	 */
>   	if (gd)
>   		cyclic_run();
> +
> +	uthread_schedule();
>   }
>   
>   int cyclic_unregister_all(void)
> diff --git a/include/u-boot/schedule.h b/include/u-boot/schedule.h
> index 4fd34c41229..4605971fdcb 100644
> --- a/include/u-boot/schedule.h
> +++ b/include/u-boot/schedule.h
> @@ -3,6 +3,8 @@
>   #ifndef _U_BOOT_SCHEDULE_H
>   #define _U_BOOT_SCHEDULE_H
>   
> +#include <uthread.h>
> +
>   #if CONFIG_IS_ENABLED(CYCLIC)
>   /**
>    * schedule() - Schedule all potentially waiting tasks
> @@ -17,6 +19,7 @@ void schedule(void);
>   
>   static inline void schedule(void)
>   {
> +	uthread_schedule();
>   }
>   
>   #endif

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de


^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 14/14] test: dm: add test for spawn and wait commands
  2025-02-25 16:34 ` [PATCH v2 14/14] test: dm: add test for " Jerome Forissier
@ 2025-02-27 16:25   ` Simon Glass
  2025-02-27 17:12     ` Jerome Forissier
  0 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2025-02-27 16:25 UTC (permalink / raw)
  To: Jerome Forissier; +Cc: u-boot, Ilias Apalodimas, Tom Rini, Heinrich Schuchardt

Hi Jerome,

On Tue, 25 Feb 2025 at 09:35, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> Test the spawn and wait commands.
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>  test/cmd/Makefile |  1 +
>  test/cmd/spawn.c  | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
>  create mode 100644 test/cmd/spawn.c
>
> diff --git a/test/cmd/Makefile b/test/cmd/Makefile
> index d8a5e77402d..cf47f04851c 100644
> --- a/test/cmd/Makefile
> +++ b/test/cmd/Makefile
> @@ -39,3 +39,4 @@ obj-$(CONFIG_CMD_WGET) += wget.o
>  endif
>  obj-$(CONFIG_ARM_FFA_TRANSPORT) += armffa.o
>  endif
> +obj-$(CONFIG_CMD_SPAWN) += spawn.o
> diff --git a/test/cmd/spawn.c b/test/cmd/spawn.c
> new file mode 100644
> index 00000000000..5e0770858c0
> --- /dev/null
> +++ b/test/cmd/spawn.c
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Tests for spawn and wait commands
> + *
> + * Copyright 2025, Linaro Ltd.
> + */
> +
> +#include <command.h>
> +#include <dm.h>
> +#include <dm/test.h>
> +#include <test/test.h>
> +#include <test/ut.h>
> +
> +static int dm_test_cmd_spawn(struct unit_test_state *uts)
> +{
> +       ut_assertok(run_command("wait; spawn sleep 2; setenv j ${job_id}; "
> +                               "spawn setenv spawned true; "
> +                               "setenv jj ${job_id}; wait; "
> +                               "echo ${j} ${jj} ${spawned}", 0));
> +       console_record_readline(uts->actual_str, sizeof(uts->actual_str));
> +       ut_asserteq_ptr(uts->actual_str,
> +                       strstr(uts->actual_str, "1 2 true"));
> +
> +       ut_assertok(run_command("spawn true; wait; setenv t $?; spawn false; "
> +                               "wait; setenv f $?; wait; echo $t $f $?", 0));
> +       console_record_readline(uts->actual_str, sizeof(uts->actual_str));
> +       ut_asserteq_ptr(uts->actual_str,
> +                       strstr(uts->actual_str, "0 1 0"));
> +       ut_assert_console_end();
> +
> +       return 0;
> +}
> +DM_TEST(dm_test_cmd_spawn, UTF_CONSOLE);
> --
> 2.43.0
>

This doesn't look like a DM test as it doesn't use devices. Could it
be a CMD test?

Regards,
Simon

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 12/14] dm: usb: initialize and scan multiple buses simultaneously with uthread
  2025-02-25 16:34 ` [PATCH v2 12/14] dm: usb: initialize and scan multiple buses simultaneously with uthread Jerome Forissier
@ 2025-02-27 16:25   ` Simon Glass
  2025-02-27 17:30     ` Jerome Forissier
  0 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2025-02-27 16:25 UTC (permalink / raw)
  To: Jerome Forissier
  Cc: u-boot, Ilias Apalodimas, Marek Vasut, Tom Rini,
	Mattijs Korpershoek, Heinrich Schuchardt, Caleb Connolly,
	Julius Lehmann, Guillaume La Roque

Hi Jerome,

On Tue, 25 Feb 2025 at 09:35, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> Use the uthread framework to initialize and scan USB buses in parallel
> for better performance. The console output is slightly modified with a
> final per-bus report of the number of devices found, common to UTHREAD
> and !UTHREAD. The USB tests are updated accordingly.
>
> Tested on two platforms:
>
> 1. arm64 QEMU on a somewhat contrived example (4 USB buses, each with
> one audio device, one keyboard, one mouse and one tablet)
>
>  $ make qemu_arm64_defconfig
>  $ make -j$(nproc) CROSS_COMPILE="ccache aarch64-linux-gnu-"
>  $ qemu-system-aarch64 -M virt -nographic -cpu max -bios u-boot.bin \
>      $(for i in {1..4}; do echo -device qemu-xhci,id=xhci$i \
>          -device\ usb-{audio,kbd,mouse,tablet},bus=xhci$i.0; \
>      done)
>
> 2. i.MX93 EVK (imx93_11x11_evk_defconfig) with two USB hubs, each with
> one webcam and one ethernet adapter, resulting in the following device
> tree:
>
>  USB device tree:
>    1  Hub (480 Mb/s, 0mA)
>    |  u-boot EHCI Host Controller
>    |
>    +-2  Hub (480 Mb/s, 100mA)
>      |  GenesysLogic USB2.1 Hub
>      |
>      +-3  Vendor specific (480 Mb/s, 350mA)
>      |    Realtek USB 10/100/1000 LAN 001000001
>      |
>      +-4   (480 Mb/s, 500mA)
>            HD Pro Webcam C920 8F7CD51F
>
>    1  Hub (480 Mb/s, 0mA)
>    |  u-boot EHCI Host Controller
>    |
>    +-2  Hub (480 Mb/s, 100mA)
>      |   USB 2.0 Hub
>      |
>      +-3  Vendor specific (480 Mb/s, 200mA)
>      |    Realtek USB 10/100/1000 LAN 000001
>      |
>      +-4   (480 Mb/s, 500mA)
>           Generic OnLan-CS30 201801010008
>
> Note that i.MX was tested on top of the downstream repository [1] since
> USB doesn't work in the upstream master branch.
>
> [1] https://github.com/nxp-imx/uboot-imx/tree/lf-6.6.52-2.2.0
>     commit 6c4545203d12 ("LF-13928 update key for capsule")
>
> The time spent in usb_init() ("usb start" command) is reported on
> the console. Here are the results:
>
>         | CONFIG_UTHREAD=n | CONFIG_UTHREAD=y
> --------+------------------+-----------------
> QEMU    |          5628 ms |          2212 ms
> i.MX93  |          4591 ms |          2441 ms
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>  drivers/usb/host/usb-uclass.c | 92 ++++++++++++++++++++++++++++-------
>  test/boot/bootdev.c           | 14 +++---
>  test/boot/bootflow.c          |  2 +-
>  3 files changed, 83 insertions(+), 25 deletions(-)

What happens to output produced by a thread? Does it get stored
somewhere and written when the thread completes, or do the threads
intermingle their output?

I'm not sure if you saw my email about using a state machine for USB.
If so, could you please point me to your reply?

Regards,
Simon

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 07/14] cyclic: invoke uthread_schedule() from schedule()
  2025-02-27 12:30   ` Stefan Roese
@ 2025-02-27 17:05     ` Jerome Forissier
  2025-02-28 15:43       ` Stefan Roese
  0 siblings, 1 reply; 47+ messages in thread
From: Jerome Forissier @ 2025-02-27 17:05 UTC (permalink / raw)
  To: Stefan Roese, u-boot
  Cc: Ilias Apalodimas, Tom Rini, Rasmus Villemoes, Simon Glass,
	Patrice Chotard

Hi Stefan,

On 2/27/25 13:30, Stefan Roese wrote:
> Hi Jerome,
> 
> On 25.02.25 17:34, Jerome Forissier wrote:
>> Make the schedule() call from the CYCLIC framework a uthread scheduling
>> point too. This makes sense since schedule() is called from a lot of
>> places where uthread_schedule() needs to be called.
>>
>> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> 
> Frankly, at first I was wondering a bit, if and why another framework
> for "multitasking" is needed in U-Boot, additionally to the cyclic
> framework that I introduced a few years ago. Which was greatly enhanced
> by Rasmus over the time. But looking at your "uthread" implementation
> it makes sense to add such a probably more intuitive interface as well.

cyclic is clean and simple and certainly well suited when introducing new
code. But when reworking older code I find it somewhat difficult to use
due to the need to keep a context and pass it everywhere. This can lead
to lots of changes when call stacks are deep.

> In general I'm really happy seeing activity in this "multitaking" area
> in U-Boot. As it brings a lot of new possibilities and, as you've also
> shown in your patchset, may greatly help reducing boot time in the
> USB example. :)

Thank you.
 
> One question though:
> Do you have some means in your uthread framework, measuring and
> perhaps limiting the time spent in these uthreads? If and how is a
> preemption of a uthread possible? So that it does not consume too
> much time resulting in e.g. things like dropping input chars on
> the prompt? Sorry, I did not thoroughly go through all your code
> to get the internals from there. It would be great if you could
> elaborate a bit on this.

That's a very valid point. The short answer is no, there is no control
over how long a thread keeps the CPU busy. uthread is similar to cyclic
in that respect. That said, I occasionally noted the issue you mentioned
about the console dropping characters, and yes it is annoying. I believe
there is a simple solution though. If we can somehow make sure we're
always scheduling the main thread every other time, we're much less
likely to starve it from the CPU, especially when many threads are
active. That is, assume we have 2 threads in addition to the main thread.
The thread list is main -> thread1 -> thread2 and uthread_schedule() will
iterate in that order. So back to main only after thread1 *and* thread2
have run and called uthread_schedule(). The idea is to schedule in a
different order: main -> thread1 -> main -> thread2 -> main etc.,
effectively giving a higher priority to the main thread (which would be
the console parsing thread).

I feel that introducing preemption would be opening a can of worms...
Because in this case we would likely need locking everywhere. Without
preemption, we still do need locking in theory, it's just that I have
not yet identified critical sections where locks would be mandatory in
the code that I have "parallelized". BTW I believe a uthread lock would
be trivial to implement like so:

struct uthread_lock {
	bool locked;
};

void uthread_lock(struct uthread_lock *l)
{
	while (l->locked)
		uthread_schedule();
	l->locked = true;
}
void uthread_unlock(struct uthread_lock *l)
{
	l->locked = false;
}

> 
> For this patch:
> 
> Reviewed-by: Stefan Roese <sr@denx.de>
> 
> Thanks,
> Stefan

Thanks,
-- 
Jerome

> 
>> ---
>>   common/cyclic.c           | 3 +++
>>   include/u-boot/schedule.h | 3 +++
>>   2 files changed, 6 insertions(+)
>>
>> diff --git a/common/cyclic.c b/common/cyclic.c
>> index fad071a39c6..b695f092f52 100644
>> --- a/common/cyclic.c
>> +++ b/common/cyclic.c
>> @@ -16,6 +16,7 @@
>>   #include <linux/list.h>
>>   #include <asm/global_data.h>
>>   #include <u-boot/schedule.h>
>> +#include <uthread.h>
>>     DECLARE_GLOBAL_DATA_PTR;
>>   @@ -100,6 +101,8 @@ void schedule(void)
>>        */
>>       if (gd)
>>           cyclic_run();
>> +
>> +    uthread_schedule();
>>   }
>>     int cyclic_unregister_all(void)
>> diff --git a/include/u-boot/schedule.h b/include/u-boot/schedule.h
>> index 4fd34c41229..4605971fdcb 100644
>> --- a/include/u-boot/schedule.h
>> +++ b/include/u-boot/schedule.h
>> @@ -3,6 +3,8 @@
>>   #ifndef _U_BOOT_SCHEDULE_H
>>   #define _U_BOOT_SCHEDULE_H
>>   +#include <uthread.h>
>> +
>>   #if CONFIG_IS_ENABLED(CYCLIC)
>>   /**
>>    * schedule() - Schedule all potentially waiting tasks
>> @@ -17,6 +19,7 @@ void schedule(void);
>>     static inline void schedule(void)
>>   {
>> +    uthread_schedule();
>>   }
>>     #endif
> 
> Viele Grüße,
> Stefan Roese
> 

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 14/14] test: dm: add test for spawn and wait commands
  2025-02-27 16:25   ` Simon Glass
@ 2025-02-27 17:12     ` Jerome Forissier
  2025-03-04 13:14       ` Simon Glass
  0 siblings, 1 reply; 47+ messages in thread
From: Jerome Forissier @ 2025-02-27 17:12 UTC (permalink / raw)
  To: Simon Glass; +Cc: u-boot, Ilias Apalodimas, Tom Rini, Heinrich Schuchardt

Hi Simon,

On 2/27/25 17:25, Simon Glass wrote:
> Hi Jerome,
> 
> On Tue, 25 Feb 2025 at 09:35, Jerome Forissier
> <jerome.forissier@linaro.org> wrote:
>>
>> Test the spawn and wait commands.
>>
>> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
>> ---
>>  test/cmd/Makefile |  1 +
>>  test/cmd/spawn.c  | 33 +++++++++++++++++++++++++++++++++
>>  2 files changed, 34 insertions(+)
>>  create mode 100644 test/cmd/spawn.c
>>
>> diff --git a/test/cmd/Makefile b/test/cmd/Makefile
>> index d8a5e77402d..cf47f04851c 100644
>> --- a/test/cmd/Makefile
>> +++ b/test/cmd/Makefile
>> @@ -39,3 +39,4 @@ obj-$(CONFIG_CMD_WGET) += wget.o
>>  endif
>>  obj-$(CONFIG_ARM_FFA_TRANSPORT) += armffa.o
>>  endif
>> +obj-$(CONFIG_CMD_SPAWN) += spawn.o
>> diff --git a/test/cmd/spawn.c b/test/cmd/spawn.c
>> new file mode 100644
>> index 00000000000..5e0770858c0
>> --- /dev/null
>> +++ b/test/cmd/spawn.c
>> @@ -0,0 +1,33 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Tests for spawn and wait commands
>> + *
>> + * Copyright 2025, Linaro Ltd.
>> + */
>> +
>> +#include <command.h>
>> +#include <dm.h>
>> +#include <dm/test.h>
>> +#include <test/test.h>
>> +#include <test/ut.h>
>> +
>> +static int dm_test_cmd_spawn(struct unit_test_state *uts)
>> +{
>> +       ut_assertok(run_command("wait; spawn sleep 2; setenv j ${job_id}; "
>> +                               "spawn setenv spawned true; "
>> +                               "setenv jj ${job_id}; wait; "
>> +                               "echo ${j} ${jj} ${spawned}", 0));
>> +       console_record_readline(uts->actual_str, sizeof(uts->actual_str));
>> +       ut_asserteq_ptr(uts->actual_str,
>> +                       strstr(uts->actual_str, "1 2 true"));
>> +
>> +       ut_assertok(run_command("spawn true; wait; setenv t $?; spawn false; "
>> +                               "wait; setenv f $?; wait; echo $t $f $?", 0));
>> +       console_record_readline(uts->actual_str, sizeof(uts->actual_str));
>> +       ut_asserteq_ptr(uts->actual_str,
>> +                       strstr(uts->actual_str, "0 1 0"));
>> +       ut_assert_console_end();
>> +
>> +       return 0;
>> +}
>> +DM_TEST(dm_test_cmd_spawn, UTF_CONSOLE);
>> --
>> 2.43.0
>>
> 
> This doesn't look like a DM test as it doesn't use devices. Could it
> be a CMD test?

Absolutely. In fact I started from a copy of test/cmd/hash.c, which is
not a DM test either is it?

Regards,
-- 
Jerome

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 12/14] dm: usb: initialize and scan multiple buses simultaneously with uthread
  2025-02-27 16:25   ` Simon Glass
@ 2025-02-27 17:30     ` Jerome Forissier
  2025-03-04 13:13       ` Simon Glass
  0 siblings, 1 reply; 47+ messages in thread
From: Jerome Forissier @ 2025-02-27 17:30 UTC (permalink / raw)
  To: Simon Glass
  Cc: u-boot, Ilias Apalodimas, Marek Vasut, Tom Rini,
	Mattijs Korpershoek, Heinrich Schuchardt, Caleb Connolly,
	Julius Lehmann, Guillaume La Roque



On 2/27/25 17:25, Simon Glass wrote:
> Hi Jerome,
> 
> On Tue, 25 Feb 2025 at 09:35, Jerome Forissier
> <jerome.forissier@linaro.org> wrote:
>>
>> Use the uthread framework to initialize and scan USB buses in parallel
>> for better performance. The console output is slightly modified with a
>> final per-bus report of the number of devices found, common to UTHREAD
>> and !UTHREAD. The USB tests are updated accordingly.
>>
>> Tested on two platforms:
>>
>> 1. arm64 QEMU on a somewhat contrived example (4 USB buses, each with
>> one audio device, one keyboard, one mouse and one tablet)
>>
>>  $ make qemu_arm64_defconfig
>>  $ make -j$(nproc) CROSS_COMPILE="ccache aarch64-linux-gnu-"
>>  $ qemu-system-aarch64 -M virt -nographic -cpu max -bios u-boot.bin \
>>      $(for i in {1..4}; do echo -device qemu-xhci,id=xhci$i \
>>          -device\ usb-{audio,kbd,mouse,tablet},bus=xhci$i.0; \
>>      done)
>>
>> 2. i.MX93 EVK (imx93_11x11_evk_defconfig) with two USB hubs, each with
>> one webcam and one ethernet adapter, resulting in the following device
>> tree:
>>
>>  USB device tree:
>>    1  Hub (480 Mb/s, 0mA)
>>    |  u-boot EHCI Host Controller
>>    |
>>    +-2  Hub (480 Mb/s, 100mA)
>>      |  GenesysLogic USB2.1 Hub
>>      |
>>      +-3  Vendor specific (480 Mb/s, 350mA)
>>      |    Realtek USB 10/100/1000 LAN 001000001
>>      |
>>      +-4   (480 Mb/s, 500mA)
>>            HD Pro Webcam C920 8F7CD51F
>>
>>    1  Hub (480 Mb/s, 0mA)
>>    |  u-boot EHCI Host Controller
>>    |
>>    +-2  Hub (480 Mb/s, 100mA)
>>      |   USB 2.0 Hub
>>      |
>>      +-3  Vendor specific (480 Mb/s, 200mA)
>>      |    Realtek USB 10/100/1000 LAN 000001
>>      |
>>      +-4   (480 Mb/s, 500mA)
>>           Generic OnLan-CS30 201801010008
>>
>> Note that i.MX was tested on top of the downstream repository [1] since
>> USB doesn't work in the upstream master branch.
>>
>> [1] https://github.com/nxp-imx/uboot-imx/tree/lf-6.6.52-2.2.0
>>     commit 6c4545203d12 ("LF-13928 update key for capsule")
>>
>> The time spent in usb_init() ("usb start" command) is reported on
>> the console. Here are the results:
>>
>>         | CONFIG_UTHREAD=n | CONFIG_UTHREAD=y
>> --------+------------------+-----------------
>> QEMU    |          5628 ms |          2212 ms
>> i.MX93  |          4591 ms |          2441 ms
>>
>> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
>> ---
>>  drivers/usb/host/usb-uclass.c | 92 ++++++++++++++++++++++++++++-------
>>  test/boot/bootdev.c           | 14 +++---
>>  test/boot/bootflow.c          |  2 +-
>>  3 files changed, 83 insertions(+), 25 deletions(-)
> 
> What happens to output produced by a thread? Does it get stored
> somewhere and written when the thread completes, or do the threads
> intermingle their output?

The latter. That is why I slightly reworked the USB initialization
output to print a status once the threads are done, and I also updated
the related tests as you can see in the patch. For instance the tests
were expecting:
"Bus usb@1: scanning bus usb@1 for devices... 5 USB Device(s) found"
The "scanning bus usb@1 for devices..." part is printed by the threads,
therefore in any order. I decided to move the text
"Bus usb@1: 5 USB Device(s) found" to after the threads are complete,
iterating over the devices in a deterministic order.

> 
> I'm not sure if you saw my email about using a state machine for USB.
> If so, could you please point me to your reply?

I did, but I did not reply because I did not try. I have a feeling that
the change would be more intrusive than what I did, but above all I am
not doing the uthread thing to address only USB but as a general
technique to make things parallel without too much trouble (at least
that's my hope). So kind of a different goal.

Regards,
-- 
Jerome

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 01/14] arch: introduce symbol HAVE_INITJMP
  2025-02-25 16:34 ` [PATCH v2 01/14] arch: introduce symbol HAVE_INITJMP Jerome Forissier
@ 2025-02-28 12:01   ` Ilias Apalodimas
  2025-02-28 12:38   ` Heinrich Schuchardt
  1 sibling, 0 replies; 47+ messages in thread
From: Ilias Apalodimas @ 2025-02-28 12:01 UTC (permalink / raw)
  To: Jerome Forissier
  Cc: u-boot, Tom Rini, Heinrich Schuchardt, Dan Carpenter, Simon Glass,
	Leo Yu-Chi Liang, Andrew Goodbody, Jiaxun Yang,
	Yu-Chien Peter Lin

Hi Jerome,


On Tue, 25 Feb 2025 at 18:35, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> HAVE_INIJMP will be set by architectures that support initjmp(), a
> non-standard extension to setjmp()/longjmp() allowing to initialize a
> jump buffer with a function pointer and a stack pointer. This will be
> useful to later introduce threads. With this new function it becomes
> possible to longjmp() to a particular function pointer (rather than
> to a point previously reached during program execution as is the case
> with setjmp()), and with a custom stack. Both things are needed to
> spin off a new thread. Then the usual setjmp()/ longjmp() pair is
> enough to save and restore a context, i.e., switch thread.
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>  arch/Kconfig | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 35b19f9bfdc..8d5b54031b3 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -13,6 +13,12 @@ config HAVE_SETJMP
>         help
>          The architecture supports setjmp() and longjmp().
>
> +config HAVE_INITJMP
> +       bool
> +       help
> +        The architecture supports initjmp(), a non-standard companion to
> +        setjmp() and longjmp().
> +
>  config SUPPORT_BIG_ENDIAN
>         bool
>
> --
> 2.43.0
>\


Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 02/14] arm: add initjmp()
  2025-02-25 16:34 ` [PATCH v2 02/14] arm: add initjmp() Jerome Forissier
@ 2025-02-28 12:01   ` Ilias Apalodimas
  2025-02-28 13:05   ` Heinrich Schuchardt
  1 sibling, 0 replies; 47+ messages in thread
From: Ilias Apalodimas @ 2025-02-28 12:01 UTC (permalink / raw)
  To: Jerome Forissier
  Cc: u-boot, Tom Rini, Heinrich Schuchardt, Simon Glass, Dan Carpenter,
	Andrew Goodbody, Jiaxun Yang, Yu-Chien Peter Lin

On Tue, 25 Feb 2025 at 18:35, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> Implement initjmp() for Arm.
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>  arch/Kconfig                  |  1 +
>  arch/arm/include/asm/setjmp.h |  1 +
>  arch/arm/lib/setjmp.S         | 11 +++++++++++
>  arch/arm/lib/setjmp_aarch64.S |  9 +++++++++
>  4 files changed, 22 insertions(+)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 8d5b54031b3..57695fada8d 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -94,6 +94,7 @@ config ARC
>  config ARM
>         bool "ARM architecture"
>         select HAVE_SETJMP
> +       select HAVE_INITJMP
>         select ARCH_SUPPORTS_LTO
>         select CREATE_ARCH_SYMLINK
>         select HAVE_PRIVATE_LIBGCC if !ARM64
> diff --git a/arch/arm/include/asm/setjmp.h b/arch/arm/include/asm/setjmp.h
> index 662bec86321..1ad5b500f2a 100644
> --- a/arch/arm/include/asm/setjmp.h
> +++ b/arch/arm/include/asm/setjmp.h
> @@ -23,5 +23,6 @@ typedef struct jmp_buf_data jmp_buf[1];
>
>  int setjmp(jmp_buf jmp);
>  void longjmp(jmp_buf jmp, int ret);
> +int initjmp(jmp_buf jmp, void __noreturn (*func)(void), void *stack_top);
>
>  #endif /* _SETJMP_H_ */
> diff --git a/arch/arm/lib/setjmp.S b/arch/arm/lib/setjmp.S
> index 2f041aeef01..320ddea85f9 100644
> --- a/arch/arm/lib/setjmp.S
> +++ b/arch/arm/lib/setjmp.S
> @@ -34,3 +34,14 @@ ENTRY(longjmp)
>         ret  lr
>  ENDPROC(longjmp)
>  .popsection
> +
> +.pushsection .text.initjmp, "ax"
> +ENTRY(initjmp)
> +       stm  a1, {v1-v8}
> +       /* a2: entry point address, a3: stack top */
> +       str  a3, [a1, #32]  /* where setjmp would save sp */
> +       str  a2, [a1, #36]  /* where setjmp would save lr */
> +       mov  a1, #0
> +       ret  lr
> +ENDPROC(initjmp)
> +.popsection
> diff --git a/arch/arm/lib/setjmp_aarch64.S b/arch/arm/lib/setjmp_aarch64.S
> index 1b8d000eb48..074320d25fb 100644
> --- a/arch/arm/lib/setjmp_aarch64.S
> +++ b/arch/arm/lib/setjmp_aarch64.S
> @@ -39,3 +39,12 @@ ENTRY(longjmp)
>         ret
>  ENDPROC(longjmp)
>  .popsection
> +
> +.pushsection .text.initjmp, "ax"
> +ENTRY(initjmp)
> +       /* x1: entry point address, x2: stack top */
> +       stp x1, x2, [x0,#88]
> +       mov  x0, #0
> +       ret
> +ENDPROC(initjmp)
> +.popsection
> --
> 2.43.0
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 01/14] arch: introduce symbol HAVE_INITJMP
  2025-02-25 16:34 ` [PATCH v2 01/14] arch: introduce symbol HAVE_INITJMP Jerome Forissier
  2025-02-28 12:01   ` Ilias Apalodimas
@ 2025-02-28 12:38   ` Heinrich Schuchardt
  2025-02-28 12:57     ` Jerome Forissier
  1 sibling, 1 reply; 47+ messages in thread
From: Heinrich Schuchardt @ 2025-02-28 12:38 UTC (permalink / raw)
  To: Jerome Forissier
  Cc: Ilias Apalodimas, Tom Rini, Dan Carpenter, Simon Glass,
	Leo Yu-Chi Liang, Andrew Goodbody, Jiaxun Yang, u-boot,
	Yu-Chien Peter Lin

On 25.02.25 17:34, Jerome Forissier wrote:
> HAVE_INIJMP will be set by architectures that support initjmp(), a
> non-standard extension to setjmp()/longjmp() allowing to initialize a
> jump buffer with a function pointer and a stack pointer. This will be
> useful to later introduce threads. With this new function it becomes
> possible to longjmp() to a particular function pointer (rather than
> to a point previously reached during program execution as is the case
> with setjmp()), and with a custom stack. Both things are needed to
> spin off a new thread. Then the usual setjmp()/ longjmp() pair is
> enough to save and restore a context, i.e., switch thread.
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>   arch/Kconfig | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 35b19f9bfdc..8d5b54031b3 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -13,6 +13,12 @@ config HAVE_SETJMP
>   	help
>   	 The architecture supports setjmp() and longjmp().
>
> +config HAVE_INITJMP
> +	bool

HAVE_SETJMP controls if /arch/<arch>/lib/setjmp.S is compiled.

Please, add the missing dependency.

	depends HAVE_SETJMP

Best regards

Heinrich

> +	help
> +	 The architecture supports initjmp(), a non-standard companion to
> +	 setjmp() and longjmp().
> +
>   config SUPPORT_BIG_ENDIAN
>   	bool
>


^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 05/14] test: lib: add initjmp() test
  2025-02-25 16:34 ` [PATCH v2 05/14] test: lib: add initjmp() test Jerome Forissier
@ 2025-02-28 12:38   ` Ilias Apalodimas
  2025-02-28 13:04   ` Heinrich Schuchardt
  1 sibling, 0 replies; 47+ messages in thread
From: Ilias Apalodimas @ 2025-02-28 12:38 UTC (permalink / raw)
  To: Jerome Forissier
  Cc: u-boot, Tom Rini, Simon Glass, Heinrich Schuchardt, Raymond Mao,
	Philippe Reynes

On Tue, 25 Feb 2025 at 18:35, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> Test the initjmp() function when HAVE_INITJMP is set.
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>  test/lib/Makefile  |  1 +
>  test/lib/initjmp.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 73 insertions(+)
>  create mode 100644 test/lib/initjmp.c
>
> diff --git a/test/lib/Makefile b/test/lib/Makefile
> index 0e4cb8e3dfd..bf04685dae1 100644
> --- a/test/lib/Makefile
> +++ b/test/lib/Makefile
> @@ -14,6 +14,7 @@ obj-y += hexdump.o
>  obj-$(CONFIG_SANDBOX) += kconfig.o
>  obj-y += lmb.o
>  obj-$(CONFIG_HAVE_SETJMP) += longjmp.o
> +obj-$(CONFIG_HAVE_INITJMP) += initjmp.o
>  obj-$(CONFIG_CONSOLE_RECORD) += test_print.o
>  obj-$(CONFIG_SSCANF) += sscanf.o
>  obj-$(CONFIG_$(PHASE_)STRTO) += str.o
> diff --git a/test/lib/initjmp.c b/test/lib/initjmp.c
> new file mode 100644
> index 00000000000..52bbda8cc60
> --- /dev/null
> +++ b/test/lib/initjmp.c
> @@ -0,0 +1,72 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2025 Linaro Limited
> + *
> + * Unit test for initjmp()
> + */
> +
> +#include <compiler.h>
> +#include <asm/setjmp.h>
> +#include <stdbool.h>
> +#include <test/lib.h>
> +#include <test/ut.h>
> +#include <vsprintf.h>
> +
> +static bool ep_entered;
> +static jmp_buf return_buf;
> +
> +static void __noreturn entrypoint(void)
> +{
> +       ep_entered = true;
> +
> +       /* Jump back to the main routine */
> +       longjmp(return_buf, 1);
> +
> +       /* Not reached */
> +       panic("longjmp failed\n");
> +}
> +
> +static int lib_initjmp(struct unit_test_state *uts)
> +{
> +       int ret;
> +       void *stack;
> +       jmp_buf buf;
> +       int stack_sz = 1024;
> +
> +       ep_entered = false;
> +
> +       stack = malloc(stack_sz);
> +       ut_assertnonnull(stack);
> +
> +       ut_assert(!ep_entered);
> +
> +       /*
> +        * Prepare return_buf so that entrypoint may jump back just after the
> +        * if()
> +        */
> +       if (!setjmp(return_buf)) {
> +               /* return_buf initialized, entrypoint not yet called */
> +
> +               /*
> +                * Prepare another jump buffer to jump into entrypoint with the
> +                * given stack
> +                */
> +               ret = initjmp(buf, entrypoint, stack + stack_sz);
> +               ut_assertok(ret);
> +
> +               /* Jump into entrypoint */
> +               longjmp(buf, 1);
> +               /*
> +                * Not reached since entrypoint is expected to branch after
> +                * the if()
> +                */
> +               ut_assert(false);
> +       }
> +
> +       ut_assert(ep_entered);
> +
> +       free(stack);
> +
> +       return 0;
> +}
> +LIB_TEST(lib_initjmp, 0);
> --
> 2.43.0
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 01/14] arch: introduce symbol HAVE_INITJMP
  2025-02-28 12:38   ` Heinrich Schuchardt
@ 2025-02-28 12:57     ` Jerome Forissier
  0 siblings, 0 replies; 47+ messages in thread
From: Jerome Forissier @ 2025-02-28 12:57 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: Ilias Apalodimas, Tom Rini, Dan Carpenter, Simon Glass,
	Leo Yu-Chi Liang, Andrew Goodbody, Jiaxun Yang, u-boot,
	Yu-Chien Peter Lin

On 2/28/25 13:38, Heinrich Schuchardt wrote:
> On 25.02.25 17:34, Jerome Forissier wrote:
>> HAVE_INIJMP will be set by architectures that support initjmp(), a
>> non-standard extension to setjmp()/longjmp() allowing to initialize a
>> jump buffer with a function pointer and a stack pointer. This will be
>> useful to later introduce threads. With this new function it becomes
>> possible to longjmp() to a particular function pointer (rather than
>> to a point previously reached during program execution as is the case
>> with setjmp()), and with a custom stack. Both things are needed to
>> spin off a new thread. Then the usual setjmp()/ longjmp() pair is
>> enough to save and restore a context, i.e., switch thread.
>>
>> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
>> ---
>>   arch/Kconfig | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/Kconfig b/arch/Kconfig
>> index 35b19f9bfdc..8d5b54031b3 100644
>> --- a/arch/Kconfig
>> +++ b/arch/Kconfig
>> @@ -13,6 +13,12 @@ config HAVE_SETJMP
>>       help
>>        The architecture supports setjmp() and longjmp().
>>
>> +config HAVE_INITJMP
>> +    bool
> 
> HAVE_SETJMP controls if /arch/<arch>/lib/setjmp.S is compiled.
> 
> Please, add the missing dependency.
> 
>     depends HAVE_SETJMP

Ack.

Thanks,
-- 
Jerome

> Best regards
> 
> Heinrich
> 
>> +    help
>> +     The architecture supports initjmp(), a non-standard companion to
>> +     setjmp() and longjmp().
>> +
>>   config SUPPORT_BIG_ENDIAN
>>       bool
>>
> 

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 05/14] test: lib: add initjmp() test
  2025-02-25 16:34 ` [PATCH v2 05/14] test: lib: add initjmp() test Jerome Forissier
  2025-02-28 12:38   ` Ilias Apalodimas
@ 2025-02-28 13:04   ` Heinrich Schuchardt
  2025-02-28 13:09     ` Jerome Forissier
  1 sibling, 1 reply; 47+ messages in thread
From: Heinrich Schuchardt @ 2025-02-28 13:04 UTC (permalink / raw)
  To: Jerome Forissier
  Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Raymond Mao,
	Philippe Reynes, u-boot

On 25.02.25 17:34, Jerome Forissier wrote:
> Test the initjmp() function when HAVE_INITJMP is set.
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>   test/lib/Makefile  |  1 +
>   test/lib/initjmp.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 73 insertions(+)
>   create mode 100644 test/lib/initjmp.c
>
> diff --git a/test/lib/Makefile b/test/lib/Makefile
> index 0e4cb8e3dfd..bf04685dae1 100644
> --- a/test/lib/Makefile
> +++ b/test/lib/Makefile
> @@ -14,6 +14,7 @@ obj-y += hexdump.o
>   obj-$(CONFIG_SANDBOX) += kconfig.o
>   obj-y += lmb.o
>   obj-$(CONFIG_HAVE_SETJMP) += longjmp.o
> +obj-$(CONFIG_HAVE_INITJMP) += initjmp.o
>   obj-$(CONFIG_CONSOLE_RECORD) += test_print.o
>   obj-$(CONFIG_SSCANF) += sscanf.o
>   obj-$(CONFIG_$(PHASE_)STRTO) += str.o
> diff --git a/test/lib/initjmp.c b/test/lib/initjmp.c
> new file mode 100644
> index 00000000000..52bbda8cc60
> --- /dev/null
> +++ b/test/lib/initjmp.c
> @@ -0,0 +1,72 @@
> +// SPDX-License-Identifier: GPL-2.0+

GPL-2.0-or-later

Best regards

Heinrich

> +/*
> + * Copyright 2025 Linaro Limited
> + *
> + * Unit test for initjmp()
> + */
> +
> +#include <compiler.h>
> +#include <asm/setjmp.h>
> +#include <stdbool.h>
> +#include <test/lib.h>
> +#include <test/ut.h>
> +#include <vsprintf.h>
> +
> +static bool ep_entered;
> +static jmp_buf return_buf;
> +
> +static void __noreturn entrypoint(void)
> +{
> +	ep_entered = true;
> +
> +	/* Jump back to the main routine */
> +	longjmp(return_buf, 1);
> +
> +	/* Not reached */
> +	panic("longjmp failed\n");
> +}
> +
> +static int lib_initjmp(struct unit_test_state *uts)
> +{
> +	int ret;
> +	void *stack;
> +	jmp_buf buf;
> +	int stack_sz = 1024;
> +
> +	ep_entered = false;
> +
> +	stack = malloc(stack_sz);
> +	ut_assertnonnull(stack);
> +
> +	ut_assert(!ep_entered);
> +
> +	/*
> +	 * Prepare return_buf so that entrypoint may jump back just after the
> +	 * if()
> +	 */
> +	if (!setjmp(return_buf)) {
> +		/* return_buf initialized, entrypoint not yet called */
> +
> +		/*
> +		 * Prepare another jump buffer to jump into entrypoint with the
> +		 * given stack
> +		 */
> +		ret = initjmp(buf, entrypoint, stack + stack_sz);
> +		ut_assertok(ret);
> +
> +		/* Jump into entrypoint */
> +		longjmp(buf, 1);
> +		/*
> +		 * Not reached since entrypoint is expected to branch after
> +		 * the if()
> +		 */
> +		ut_assert(false);
> +	}
> +
> +	ut_assert(ep_entered);
> +
> +	free(stack);
> +
> +	return 0;
> +}
> +LIB_TEST(lib_initjmp, 0);


^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 02/14] arm: add initjmp()
  2025-02-25 16:34 ` [PATCH v2 02/14] arm: add initjmp() Jerome Forissier
  2025-02-28 12:01   ` Ilias Apalodimas
@ 2025-02-28 13:05   ` Heinrich Schuchardt
  2025-02-28 13:21     ` Jerome Forissier
  1 sibling, 1 reply; 47+ messages in thread
From: Heinrich Schuchardt @ 2025-02-28 13:05 UTC (permalink / raw)
  To: Jerome Forissier
  Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Dan Carpenter,
	Andrew Goodbody, Jiaxun Yang, Yu-Chien Peter Lin, u-boot

On 25.02.25 17:34, Jerome Forissier wrote:
> Implement initjmp() for Arm.
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>   arch/Kconfig                  |  1 +
>   arch/arm/include/asm/setjmp.h |  1 +
>   arch/arm/lib/setjmp.S         | 11 +++++++++++
>   arch/arm/lib/setjmp_aarch64.S |  9 +++++++++
>   4 files changed, 22 insertions(+)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 8d5b54031b3..57695fada8d 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -94,6 +94,7 @@ config ARC
>   config ARM
>   	bool "ARM architecture"
>   	select HAVE_SETJMP
> +	select HAVE_INITJMP
>   	select ARCH_SUPPORTS_LTO
>   	select CREATE_ARCH_SYMLINK
>   	select HAVE_PRIVATE_LIBGCC if !ARM64
> diff --git a/arch/arm/include/asm/setjmp.h b/arch/arm/include/asm/setjmp.h
> index 662bec86321..1ad5b500f2a 100644
> --- a/arch/arm/include/asm/setjmp.h
> +++ b/arch/arm/include/asm/setjmp.h
> @@ -23,5 +23,6 @@ typedef struct jmp_buf_data jmp_buf[1];
>
>   int setjmp(jmp_buf jmp);
>   void longjmp(jmp_buf jmp, int ret);
> +int initjmp(jmp_buf jmp, void __noreturn (*func)(void), void *stack_top);

As this is not a standard function we cannot expect developers to know
what it does and how to use it.

Please, provide Sphinx style documentation with the necessary information.

Best regards

Heinrich

>
>   #endif /* _SETJMP_H_ */
> diff --git a/arch/arm/lib/setjmp.S b/arch/arm/lib/setjmp.S
> index 2f041aeef01..320ddea85f9 100644
> --- a/arch/arm/lib/setjmp.S
> +++ b/arch/arm/lib/setjmp.S
> @@ -34,3 +34,14 @@ ENTRY(longjmp)
>   	ret  lr
>   ENDPROC(longjmp)
>   .popsection
> +
> +.pushsection .text.initjmp, "ax"
> +ENTRY(initjmp)
> +	stm  a1, {v1-v8}
> +	/* a2: entry point address, a3: stack top */
> +	str  a3, [a1, #32]  /* where setjmp would save sp */
> +	str  a2, [a1, #36]  /* where setjmp would save lr */
> +	mov  a1, #0
> +	ret  lr
> +ENDPROC(initjmp)
> +.popsection
> diff --git a/arch/arm/lib/setjmp_aarch64.S b/arch/arm/lib/setjmp_aarch64.S
> index 1b8d000eb48..074320d25fb 100644
> --- a/arch/arm/lib/setjmp_aarch64.S
> +++ b/arch/arm/lib/setjmp_aarch64.S
> @@ -39,3 +39,12 @@ ENTRY(longjmp)
>   	ret
>   ENDPROC(longjmp)
>   .popsection
> +
> +.pushsection .text.initjmp, "ax"
> +ENTRY(initjmp)
> +	/* x1: entry point address, x2: stack top */
> +	stp x1, x2, [x0,#88]
> +	mov  x0, #0
> +	ret
> +ENDPROC(initjmp)
> +.popsection


^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 05/14] test: lib: add initjmp() test
  2025-02-28 13:04   ` Heinrich Schuchardt
@ 2025-02-28 13:09     ` Jerome Forissier
  0 siblings, 0 replies; 47+ messages in thread
From: Jerome Forissier @ 2025-02-28 13:09 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Raymond Mao,
	Philippe Reynes, u-boot



On 2/28/25 14:04, Heinrich Schuchardt wrote:
> On 25.02.25 17:34, Jerome Forissier wrote:
>> Test the initjmp() function when HAVE_INITJMP is set.
>>
>> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
>> ---
>>   test/lib/Makefile  |  1 +
>>   test/lib/initjmp.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 73 insertions(+)
>>   create mode 100644 test/lib/initjmp.c
>>
>> diff --git a/test/lib/Makefile b/test/lib/Makefile
>> index 0e4cb8e3dfd..bf04685dae1 100644
>> --- a/test/lib/Makefile
>> +++ b/test/lib/Makefile
>> @@ -14,6 +14,7 @@ obj-y += hexdump.o
>>   obj-$(CONFIG_SANDBOX) += kconfig.o
>>   obj-y += lmb.o
>>   obj-$(CONFIG_HAVE_SETJMP) += longjmp.o
>> +obj-$(CONFIG_HAVE_INITJMP) += initjmp.o
>>   obj-$(CONFIG_CONSOLE_RECORD) += test_print.o
>>   obj-$(CONFIG_SSCANF) += sscanf.o
>>   obj-$(CONFIG_$(PHASE_)STRTO) += str.o
>> diff --git a/test/lib/initjmp.c b/test/lib/initjmp.c
>> new file mode 100644
>> index 00000000000..52bbda8cc60
>> --- /dev/null
>> +++ b/test/lib/initjmp.c
>> @@ -0,0 +1,72 @@
>> +// SPDX-License-Identifier: GPL-2.0+
> 
> GPL-2.0-or-later

Ack.

Thanks,
-- 
Jerome

> 
> Best regards
> 
> Heinrich
> 
>> +/*
>> + * Copyright 2025 Linaro Limited
>> + *
>> + * Unit test for initjmp()
>> + */
>> +
>> +#include <compiler.h>
>> +#include <asm/setjmp.h>
>> +#include <stdbool.h>
>> +#include <test/lib.h>
>> +#include <test/ut.h>
>> +#include <vsprintf.h>
>> +
>> +static bool ep_entered;
>> +static jmp_buf return_buf;
>> +
>> +static void __noreturn entrypoint(void)
>> +{
>> +    ep_entered = true;
>> +
>> +    /* Jump back to the main routine */
>> +    longjmp(return_buf, 1);
>> +
>> +    /* Not reached */
>> +    panic("longjmp failed\n");
>> +}
>> +
>> +static int lib_initjmp(struct unit_test_state *uts)
>> +{
>> +    int ret;
>> +    void *stack;
>> +    jmp_buf buf;
>> +    int stack_sz = 1024;
>> +
>> +    ep_entered = false;
>> +
>> +    stack = malloc(stack_sz);
>> +    ut_assertnonnull(stack);
>> +
>> +    ut_assert(!ep_entered);
>> +
>> +    /*
>> +     * Prepare return_buf so that entrypoint may jump back just after the
>> +     * if()
>> +     */
>> +    if (!setjmp(return_buf)) {
>> +        /* return_buf initialized, entrypoint not yet called */
>> +
>> +        /*
>> +         * Prepare another jump buffer to jump into entrypoint with the
>> +         * given stack
>> +         */
>> +        ret = initjmp(buf, entrypoint, stack + stack_sz);
>> +        ut_assertok(ret);
>> +
>> +        /* Jump into entrypoint */
>> +        longjmp(buf, 1);
>> +        /*
>> +         * Not reached since entrypoint is expected to branch after
>> +         * the if()
>> +         */
>> +        ut_assert(false);
>> +    }
>> +
>> +    ut_assert(ep_entered);
>> +
>> +    free(stack);
>> +
>> +    return 0;
>> +}
>> +LIB_TEST(lib_initjmp, 0);
> 

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 06/14] uthread: add cooperative multi-tasking interface
  2025-02-25 16:34 ` [PATCH v2 06/14] uthread: add cooperative multi-tasking interface Jerome Forissier
@ 2025-02-28 13:09   ` Ilias Apalodimas
  2025-02-28 14:30     ` Jerome Forissier
  2025-02-28 13:21   ` Heinrich Schuchardt
  1 sibling, 1 reply; 47+ messages in thread
From: Ilias Apalodimas @ 2025-02-28 13:09 UTC (permalink / raw)
  To: Jerome Forissier
  Cc: u-boot, Tom Rini, Simon Glass, Sughosh Ganu, Raymond Mao,
	Patrick Rudolph, Michal Simek, Heinrich Schuchardt

Hi Jerome,

On Tue, 25 Feb 2025 at 18:35, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> Add an new

'a'

>  internal API called uthread (Kconfig symbol: UTHREAD) which
> provides cooperative multi-tasking. The goal is to be able to improve
> the performance of some parts of U-Boot by overlapping lengthy
> operations, and also implement background jobs in the U-Boot shell.
> Each uthread has its own stack allocated on the heap. The default stack
> size is defined by the UTHREAD_STACK_SIZE symbol and is used when
> uthread_create() receives zero for the stack_sz argument.
>
> The implementation is based on context-switching via initjmp()/setjmp()/
> longjmp() and is inspired from barebox threads [1]. A notion of thread
> group helps with dependencies, such as when a thread needs to block
> until a number of other threads have returned.
>
> The name "uthread" comes from "user-space threads" because the
> scheduling happens with no help from a higher privileged mode, contrary
> to more complex models where kernel threads are defined. But the 'u'
> may as well stand for 'U-Boot' since the bootloader may actually be
> running at any privilege level and the notion of user vs. kernel may
> not make much sense in this context.
>
> [1] https://github.com/barebox/barebox/blob/master/common/bthread.c
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>  include/uthread.h |  44 ++++++++++++
>  lib/Kconfig       |  21 ++++++
>  lib/Makefile      |   2 +
>  lib/uthread.c     | 178 ++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 245 insertions(+)
>  create mode 100644 include/uthread.h
>  create mode 100644 lib/uthread.c
>
> diff --git a/include/uthread.h b/include/uthread.h
> new file mode 100644
> index 00000000000..f1f86d210d5
> --- /dev/null
> +++ b/include/uthread.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright 2025 Linaro Limited
> + */
> +
> +#include <linux/types.h>
> +
> +#ifndef _UTHREAD_H_
> +#define _UTHREAD_H_
> +
> +#ifdef CONFIG_UTHREAD
> +
> +int uthread_create(void (*fn)(void *), void *arg, size_t stack_sz,
> +                  unsigned int grp_id);
> +bool uthread_schedule(void);
> +unsigned int uthread_grp_new_id(void);
> +bool uthread_grp_done(unsigned int grp_id);
> +
> +#else
> +
> +static inline int uthread_create(void (*fn)(void *), void *arg, size_t stack_sz,
> +                                unsigned int grp_id)
> +{
> +       fn(arg);
> +       return 0;
> +}
> +
> +static inline bool uthread_schedule(void)
> +{
> +       return false;
> +}
> +
> +static inline unsigned int uthread_grp_new_id(void)
> +{
> +       return 0;
> +}
> +
> +static inline bool uthread_grp_done(unsigned int grp_id)
> +{
> +       return true;
> +}
> +
> +#endif /* CONFIG_UTHREAD */
> +#endif /* _UTHREAD_H_ */
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 1a683dea670..b32740ecbcc 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -1255,6 +1255,27 @@ config PHANDLE_CHECK_SEQ
>           enable this config option to distinguish them using
>           phandles in fdtdec_get_alias_seq() function.
>
> +config UTHREAD
> +       bool "Enable thread support"
> +       depends on HAVE_INITJMP
> +       help
> +         Implement a simple form of cooperative multi-tasking based on
> +         context-switching via initjmp(), setjmp() and longjmp(). The
> +         uthread_ interface enables the main thread of execution to create
> +         one or more secondary threads and schedule them until they all have
> +         returned. At any point a thread may suspend its execution and
> +         schedule another thread, which allows for the efficient multiplexing
> +         of leghthy operations.
> +
> +config UTHREAD_STACK_SIZE
> +       int "Default uthread stack size"
> +       depends on UTHREAD
> +       default 32768
> +       help
> +         The default stak size for uthreads. Each uthread has its own stack.

stack size

> +         When the stack_sz argument to uthread_create() is zero then this
> +         value is used.
> +
>  endmenu
>
>  source "lib/fwu_updates/Kconfig"
> diff --git a/lib/Makefile b/lib/Makefile
> index a7bc2f3134a..3610694de7a 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -164,6 +164,8 @@ obj-$(CONFIG_LIB_ELF) += elf.o
>
>  obj-$(CONFIG_$(PHASE_)SEMIHOSTING) += semihosting.o
>
> +obj-$(CONFIG_UTHREAD) += uthread.o
> +
>  #
>  # Build a fast OID lookup registry from include/linux/oid_registry.h
>  #
> diff --git a/lib/uthread.c b/lib/uthread.c
> new file mode 100644
> index 00000000000..430d1c0de32
> --- /dev/null
> +++ b/lib/uthread.c
> @@ -0,0 +1,178 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2021 Ahmad Fatoum, Pengutronix
> + * Copyright (C) 2025 Linaro Limited
> + *
> + * An implementation of cooperative multi-tasking inspired from barebox threads
> + * https://github.com/barebox/barebox/blob/master/common/bthread.c
> + */
> +
> +#include <compiler.h>
> +#include <asm/setjmp.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <malloc.h>
> +#include <stdint.h>
> +#include <uthread.h>
> +
> +static struct uthread {
> +       void (*fn)(void *);
> +       void *arg;
> +       jmp_buf ctx;
> +       void *stack;
> +       bool done;
> +       unsigned int grp_id;
> +       struct list_head list;
> +} main_thread = {
> +       .list = LIST_HEAD_INIT(main_thread.list),
> +};
> +
> +static struct uthread *current = &main_thread;
> +
> +/**
> + * uthread_trampoline() - Call the current thread's entry point then resume the
> + * main thread.
> + *
> + * This is a helper function which is used as the @func argument to the inijmp()

initjump

> + * function, and ultimately invoked via setjmp(). It does not return, but
> + * instead longjmp()'s back to the main thread.
> + */
> +static void __noreturn uthread_trampoline(void)
> +{
> +       struct uthread *curr = current;
> +
> +       curr->fn(curr->arg);
> +       curr->done = true;
> +       current = &main_thread;
> +       longjmp(current->ctx, 1);
> +       /* Not reached */
> +       while (true)
> +               ;
> +}
> +
> +/**
> + * uthread_free() - Free memory used by a uthread object.
> + */
> +static void uthread_free(struct uthread *uthread)
> +{
> +       if (!uthread)
> +               return;
> +       free(uthread->stack);
> +       free(uthread);
> +}
> +
> +/**
> + * uthread_create() - Create a uthread object and make it ready for execution
> + *
> + * Threads are automatically deleted when then return from their entry point.

when they

[...]
> +/**
> + * uthread_grp_new_id() - return a new ID for a thread group
> + *
> + * Return: the new thread group ID
> + */
> +unsigned int uthread_grp_new_id(void)
> +{
> +       static unsigned int id = 0;
> +
> +       return ++id;
> +}

This seems a bit weird. Why do we need this function?

> +
> +/**
> + * uthread_grp_done() - test if all threads in a group are done
> + *
> + * @grp: the ID of the thread group that should be considered
> + * Return: false if the group contains at least one runnable thread (i.e., one
> + * thread which entry point has not returned yet), true otherwise
> + */
> +bool uthread_grp_done(unsigned int grp_id)
> +{
> +       struct uthread *next;
> +
> +       list_for_each_entry(next, &main_thread.list, list) {
> +               if (next->grp_id == grp_id && !next->done)
> +                       return false;
> +       }
> +
> +       return true;
> +}
> --
> 2.43.0
>


Apart from the minor typos and the function that I can't figure out
why we need this look pretty clean

Thnaks
/Ilias

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 06/14] uthread: add cooperative multi-tasking interface
  2025-02-25 16:34 ` [PATCH v2 06/14] uthread: add cooperative multi-tasking interface Jerome Forissier
  2025-02-28 13:09   ` Ilias Apalodimas
@ 2025-02-28 13:21   ` Heinrich Schuchardt
  2025-02-28 14:39     ` Jerome Forissier
  1 sibling, 1 reply; 47+ messages in thread
From: Heinrich Schuchardt @ 2025-02-28 13:21 UTC (permalink / raw)
  To: Jerome Forissier
  Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Sughosh Ganu,
	Raymond Mao, Patrick Rudolph, Michal Simek, u-boot

On 25.02.25 17:34, Jerome Forissier wrote:
> Add an new internal API called uthread (Kconfig symbol: UTHREAD) which

nits:

%s/an/a/

> provides cooperative multi-tasking. The goal is to be able to improve
> the performance of some parts of U-Boot by overlapping lengthy
> operations, and also implement background jobs in the U-Boot shell.
> Each uthread has its own stack allocated on the heap. The default stack
> size is defined by the UTHREAD_STACK_SIZE symbol and is used when
> uthread_create() receives zero for the stack_sz argument.
>
> The implementation is based on context-switching via initjmp()/setjmp()/
> longjmp() and is inspired from barebox threads [1]. A notion of thread
> group helps with dependencies, such as when a thread needs to block
> until a number of other threads have returned.
>
> The name "uthread" comes from "user-space threads" because the
> scheduling happens with no help from a higher privileged mode, contrary
> to more complex models where kernel threads are defined. But the 'u'
> may as well stand for 'U-Boot' since the bootloader may actually be
> running at any privilege level and the notion of user vs. kernel may
> not make much sense in this context.
>
> [1] https://github.com/barebox/barebox/blob/master/common/bthread.c
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>   include/uthread.h |  44 ++++++++++++
>   lib/Kconfig       |  21 ++++++
>   lib/Makefile      |   2 +
>   lib/uthread.c     | 178 ++++++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 245 insertions(+)
>   create mode 100644 include/uthread.h
>   create mode 100644 lib/uthread.c
>
> diff --git a/include/uthread.h b/include/uthread.h
> new file mode 100644
> index 00000000000..f1f86d210d5
> --- /dev/null
> +++ b/include/uthread.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright 2025 Linaro Limited
> + */
> +
> +#include <linux/types.h>
> +
> +#ifndef _UTHREAD_H_
> +#define _UTHREAD_H_
> +
> +#ifdef CONFIG_UTHREAD
> +
> +int uthread_create(void (*fn)(void *), void *arg, size_t stack_sz,
> +		   unsigned int grp_id);

Every function needs a Shinx style documentation.

> +bool uthread_schedule(void);
> +unsigned int uthread_grp_new_id(void);
> +bool uthread_grp_done(unsigned int grp_id);
> +
> +#else
> +
> +static inline int uthread_create(void (*fn)(void *), void *arg, size_t stack_sz,
> +				 unsigned int grp_id)
> +{
> +	fn(arg);
> +	return 0;
> +}
> +
> +static inline bool uthread_schedule(void)
> +{
> +	return false;
> +}
> +
> +static inline unsigned int uthread_grp_new_id(void)
> +{
> +	return 0;
> +}
> +
> +static inline bool uthread_grp_done(unsigned int grp_id)
> +{
> +	return true;
> +}
> +
> +#endif /* CONFIG_UTHREAD */
> +#endif /* _UTHREAD_H_ */
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 1a683dea670..b32740ecbcc 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -1255,6 +1255,27 @@ config PHANDLE_CHECK_SEQ
>   	  enable this config option to distinguish them using
>   	  phandles in fdtdec_get_alias_seq() function.
>
> +config UTHREAD
> +	bool "Enable thread support"
> +	depends on HAVE_INITJMP
> +	help
> +	  Implement a simple form of cooperative multi-tasking based on
> +	  context-switching via initjmp(), setjmp() and longjmp(). The
> +	  uthread_ interface enables the main thread of execution to create
> +	  one or more secondary threads and schedule them until they all have
> +	  returned. At any point a thread may suspend its execution and
> +	  schedule another thread, which allows for the efficient multiplexing
> +	  of leghthy operations.
> +
> +config UTHREAD_STACK_SIZE
> +	int "Default uthread stack size"
> +	depends on UTHREAD
> +	default 32768
> +	help
> +	  The default stak size for uthreads. Each uthread has its own stack.
> +	  When the stack_sz argument to uthread_create() is zero then this
> +	  value is used.
> +
>   endmenu
>
>   source "lib/fwu_updates/Kconfig"
> diff --git a/lib/Makefile b/lib/Makefile
> index a7bc2f3134a..3610694de7a 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -164,6 +164,8 @@ obj-$(CONFIG_LIB_ELF) += elf.o
>
>   obj-$(CONFIG_$(PHASE_)SEMIHOSTING) += semihosting.o
>
> +obj-$(CONFIG_UTHREAD) += uthread.o
> +
>   #
>   # Build a fast OID lookup registry from include/linux/oid_registry.h
>   #
> diff --git a/lib/uthread.c b/lib/uthread.c
> new file mode 100644
> index 00000000000..430d1c0de32
> --- /dev/null
> +++ b/lib/uthread.c
> @@ -0,0 +1,178 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2021 Ahmad Fatoum, Pengutronix
> + * Copyright (C) 2025 Linaro Limited
> + *
> + * An implementation of cooperative multi-tasking inspired from barebox threads
> + * https://github.com/barebox/barebox/blob/master/common/bthread.c
> + */
> +
> +#include <compiler.h>
> +#include <asm/setjmp.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <malloc.h>
> +#include <stdint.h>
> +#include <uthread.h>
> +
> +static struct uthread {
> +	void (*fn)(void *);
> +	void *arg;
> +	jmp_buf ctx;
> +	void *stack;
> +	bool done;
> +	unsigned int grp_id;
> +	struct list_head list;

The structure and all its components need to be documented.

> +} main_thread = {
> +	.list = LIST_HEAD_INIT(main_thread.list),
> +};
> +
> +static struct uthread *current = &main_thread;
> +
> +/**
> + * uthread_trampoline() - Call the current thread's entry point then resume the
> + * main thread.
> + *
> + * This is a helper function which is used as the @func argument to the inijmp()
> + * function, and ultimately invoked via setjmp(). It does not return, but
> + * instead longjmp()'s back to the main thread.
> + */
> +static void __noreturn uthread_trampoline(void)
> +{
> +	struct uthread *curr = current;
> +
> +	curr->fn(curr->arg);
> +	curr->done = true;
> +	current = &main_thread;
> +	longjmp(current->ctx, 1);
> +	/* Not reached */
> +	while (true)
> +		;
> +}
> +
> +/**
> + * uthread_free() - Free memory used by a uthread object.
> + */
> +static void uthread_free(struct uthread *uthread)
> +{
> +	if (!uthread)
> +		return;
> +	free(uthread->stack);
> +	free(uthread);
> +}
> +
> +/**
> + * uthread_create() - Create a uthread object and make it ready for execution
> + *
> + * Threads are automatically deleted when then return from their entry point.
> + *
> + * @fn: the thread's entry point
> + * @arg: argument passed to the thread's entry point
> + * @stack_sz: stack size for the new thread (in bytes). The stack is allocated
> + * on the heap.
> + * @grp_id: an optional thread group ID that the new thread should belong to
> + * (zero for no group)
> + */

This documentation should be move to the include and the include needs
to be added to doc/api/.

> +int uthread_create(void (*fn)(void *), void *arg, size_t stack_sz,
> +		   unsigned int grp_id)

We should model the functions more like pthreads.

Let the caller provide struct uthread(). This will allow us to implement
pthread_cancel() and phthread_join().

Best regards

Heinrich

> +{
> +	struct uthread *uthread;
> +
> +	if (!stack_sz)
> +		stack_sz = CONFIG_UTHREAD_STACK_SIZE;
> +
> +	uthread = calloc(1, sizeof(*uthread));
> +	if (!uthread)
> +		return -1;
> +
> +	uthread->stack = memalign(16, stack_sz);
> +	if (!uthread->stack)
> +		goto err;
> +
> +	uthread->fn = fn;
> +	uthread->arg = arg;
> +	uthread->grp_id = grp_id;
> +
> +	list_add_tail(&uthread->list, &current->list);
> +
> +	initjmp(uthread->ctx, uthread_trampoline, uthread->stack + stack_sz);
> +
> +	return 0;
> +err:
> +	uthread_free(uthread);
> +	return -1;
> +}
> +
> +/**
> + * uthread_resume() - switch execution to a given thread
> + *
> + * @uthread: the thread object that should be resumed
> + */
> +static void uthread_resume(struct uthread *uthread)
> +{
> +	if (!setjmp(current->ctx)) {
> +		current = uthread;
> +		longjmp(uthread->ctx, 1);
> +	}
> +}
> +
> +/**
> + * uthread_schedule() - yield the CPU to the next runnable thread
> + *
> + * This function is called either by the main thread or any secondary thread
> + * (that is, any thread created via uthread_create()) to switch execution to
> + * the next runnable thread.
> + *
> + * Return: true if a thread was scheduled, false if no runnable thread was found
> + */
> +bool uthread_schedule(void)
> +{
> +	struct uthread *next;
> +	struct uthread *tmp;
> +
> +	if (list_empty(&current->list))
> +	    return false;
> +
> +	list_for_each_entry_safe(next, tmp, &current->list, list) {
> +		if (!next->done) {
> +			uthread_resume(next);
> +			return true;
> +		} else {
> +			/* Found a 'done' thread, free its resources */
> +			list_del(&next->list);
> +			uthread_free(next);
> +		}
> +	}
> +	return false;
> +}
> +
> +/**
> + * uthread_grp_new_id() - return a new ID for a thread group
> + *
> + * Return: the new thread group ID
> + */
> +unsigned int uthread_grp_new_id(void)
> +{
> +	static unsigned int id = 0;
> +
> +	return ++id;
> +}
> +
> +/**
> + * uthread_grp_done() - test if all threads in a group are done
> + *
> + * @grp: the ID of the thread group that should be considered
> + * Return: false if the group contains at least one runnable thread (i.e., one
> + * thread which entry point has not returned yet), true otherwise
> + */
> +bool uthread_grp_done(unsigned int grp_id)
> +{
> +	struct uthread *next;
> +
> +	list_for_each_entry(next, &main_thread.list, list) {
> +		if (next->grp_id == grp_id && !next->done)
> +			return false;
> +	}
> +
> +	return true;
> +}


^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 02/14] arm: add initjmp()
  2025-02-28 13:05   ` Heinrich Schuchardt
@ 2025-02-28 13:21     ` Jerome Forissier
  2025-02-28 13:28       ` Heinrich Schuchardt
  0 siblings, 1 reply; 47+ messages in thread
From: Jerome Forissier @ 2025-02-28 13:21 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Dan Carpenter,
	Andrew Goodbody, Jiaxun Yang, Yu-Chien Peter Lin, u-boot



On 2/28/25 14:05, Heinrich Schuchardt wrote:
> On 25.02.25 17:34, Jerome Forissier wrote:
>> Implement initjmp() for Arm.
>>
>> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
>> ---
>>   arch/Kconfig                  |  1 +
>>   arch/arm/include/asm/setjmp.h |  1 +
>>   arch/arm/lib/setjmp.S         | 11 +++++++++++
>>   arch/arm/lib/setjmp_aarch64.S |  9 +++++++++
>>   4 files changed, 22 insertions(+)
>>
>> diff --git a/arch/Kconfig b/arch/Kconfig
>> index 8d5b54031b3..57695fada8d 100644
>> --- a/arch/Kconfig
>> +++ b/arch/Kconfig
>> @@ -94,6 +94,7 @@ config ARC
>>   config ARM
>>       bool "ARM architecture"
>>       select HAVE_SETJMP
>> +    select HAVE_INITJMP
>>       select ARCH_SUPPORTS_LTO
>>       select CREATE_ARCH_SYMLINK
>>       select HAVE_PRIVATE_LIBGCC if !ARM64
>> diff --git a/arch/arm/include/asm/setjmp.h b/arch/arm/include/asm/setjmp.h
>> index 662bec86321..1ad5b500f2a 100644
>> --- a/arch/arm/include/asm/setjmp.h
>> +++ b/arch/arm/include/asm/setjmp.h
>> @@ -23,5 +23,6 @@ typedef struct jmp_buf_data jmp_buf[1];
>>
>>   int setjmp(jmp_buf jmp);
>>   void longjmp(jmp_buf jmp, int ret);
>> +int initjmp(jmp_buf jmp, void __noreturn (*func)(void), void *stack_top);
> 
> As this is not a standard function we cannot expect developers to know
> what it does and how to use it.
> 
> Please, provide Sphinx style documentation with the necessary information.

I agree on principle, but if I add documentation here then I should add the
same for all architectures. That being said the prototypes are already
duplicated so maybe consolidation is a question for another time.  I will
add Shpinx style doc to all 3 commits "{arm,riscv,sandbox}: add initjmp()".

As for usage, I think the best documentation is test/lib/initjmp.c added
by "test: lib: add initjmp() test".

Thanks,
-- 
Jerome

> 
> Best regards
> 
> Heinrich
> 
>>
>>   #endif /* _SETJMP_H_ */
>> diff --git a/arch/arm/lib/setjmp.S b/arch/arm/lib/setjmp.S
>> index 2f041aeef01..320ddea85f9 100644
>> --- a/arch/arm/lib/setjmp.S
>> +++ b/arch/arm/lib/setjmp.S
>> @@ -34,3 +34,14 @@ ENTRY(longjmp)
>>       ret  lr
>>   ENDPROC(longjmp)
>>   .popsection
>> +
>> +.pushsection .text.initjmp, "ax"
>> +ENTRY(initjmp)
>> +    stm  a1, {v1-v8}
>> +    /* a2: entry point address, a3: stack top */
>> +    str  a3, [a1, #32]  /* where setjmp would save sp */
>> +    str  a2, [a1, #36]  /* where setjmp would save lr */
>> +    mov  a1, #0
>> +    ret  lr
>> +ENDPROC(initjmp)
>> +.popsection
>> diff --git a/arch/arm/lib/setjmp_aarch64.S b/arch/arm/lib/setjmp_aarch64.S
>> index 1b8d000eb48..074320d25fb 100644
>> --- a/arch/arm/lib/setjmp_aarch64.S
>> +++ b/arch/arm/lib/setjmp_aarch64.S
>> @@ -39,3 +39,12 @@ ENTRY(longjmp)
>>       ret
>>   ENDPROC(longjmp)
>>   .popsection
>> +
>> +.pushsection .text.initjmp, "ax"
>> +ENTRY(initjmp)
>> +    /* x1: entry point address, x2: stack top */
>> +    stp x1, x2, [x0,#88]
>> +    mov  x0, #0
>> +    ret
>> +ENDPROC(initjmp)
>> +.popsection
> 

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 07/14] cyclic: invoke uthread_schedule() from schedule()
  2025-02-25 16:34 ` [PATCH v2 07/14] cyclic: invoke uthread_schedule() from schedule() Jerome Forissier
  2025-02-27 12:30   ` Stefan Roese
@ 2025-02-28 13:22   ` Ilias Apalodimas
  1 sibling, 0 replies; 47+ messages in thread
From: Ilias Apalodimas @ 2025-02-28 13:22 UTC (permalink / raw)
  To: Jerome Forissier
  Cc: u-boot, Stefan Roese, Tom Rini, Rasmus Villemoes, Simon Glass,
	Patrice Chotard

On Tue, 25 Feb 2025 at 18:35, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> Make the schedule() call from the CYCLIC framework a uthread scheduling
> point too. This makes sense since schedule() is called from a lot of
> places where uthread_schedule() needs to be called.
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>

[...]

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 10/14] test: lib: add uthread test
  2025-02-25 16:34 ` [PATCH v2 10/14] test: lib: add uthread test Jerome Forissier
@ 2025-02-28 13:26   ` Ilias Apalodimas
  0 siblings, 0 replies; 47+ messages in thread
From: Ilias Apalodimas @ 2025-02-28 13:26 UTC (permalink / raw)
  To: Jerome Forissier
  Cc: u-boot, Tom Rini, Simon Glass, Heinrich Schuchardt, Raymond Mao,
	Philippe Reynes

On Tue, 25 Feb 2025 at 18:35, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> Add a uhread framework test to the lib tests.
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>  test/lib/Makefile  |  1 +
>  test/lib/uthread.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 69 insertions(+)
>  create mode 100644 test/lib/uthread.c
>
> diff --git a/test/lib/Makefile b/test/lib/Makefile
> index bf04685dae1..c991dff1c63 100644
> --- a/test/lib/Makefile
> +++ b/test/lib/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_CRC8) += test_crc8.o
>  obj-$(CONFIG_UT_LIB_CRYPT) += test_crypt.o
>  obj-$(CONFIG_UT_TIME) += time.o
>  obj-$(CONFIG_$(XPL_)UT_UNICODE) += unicode.o
> +obj-$(CONFIG_UTHREAD) += uthread.o
>  obj-$(CONFIG_LIB_UUID) += uuid.o
>  else
>  obj-$(CONFIG_SANDBOX) += kconfig_spl.o
> diff --git a/test/lib/uthread.c b/test/lib/uthread.c
> new file mode 100644
> index 00000000000..84c41d35875
> --- /dev/null
> +++ b/test/lib/uthread.c
> @@ -0,0 +1,68 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2025 Linaro Limited
> + *
> + * Unit test for uthread
> + */
> +
> +#include <stdbool.h>
> +#include <test/lib.h>
> +#include <test/ut.h>
> +#include <uthread.h>
> +
> +static int count;
> +
> +static void worker(void *arg)
> +{
> +       int loops = (int)(unsigned long)arg;
> +       int i;
> +
> +       for (i = 0; i < loops; i++) {
> +               count++;
> +               uthread_schedule();
> +       }
> +}
> +
> +static int lib_uthread(struct unit_test_state *uts)
> +{
> +       int i;
> +       int id1, id2;
> +
> +       count = 0;
> +       id1 = uthread_grp_new_id();
> +       ut_assert(id1 != 0);
> +       id2 = uthread_grp_new_id();
> +       ut_assert(id2 != 0);
> +       ut_assert(id1 != id2);
> +       ut_assertok(uthread_create(worker, (void *)5, 0, id1));
> +       ut_assertok(uthread_create(worker, (void *)10, 0, 0));
> +       /*
> +        * The first call is expected to schedule the first worker, which will
> +        * schedule the second one, which will schedule back to the main thread
> +        * (here). Therefore count should be 2.
> +        */
> +       ut_assert(uthread_schedule());
> +       ut_asserteq(2, count);
> +       ut_assert(!uthread_grp_done(id1));
> +       /* Four more calls should bring the count to 10 */
> +       for (i = 0; i < 4; i++) {
> +               ut_assert(!uthread_grp_done(id1));
> +               ut_assert(uthread_schedule());
> +       }
> +       ut_asserteq(10, count);
> +       /* This one allows the first worker to exit */
> +       ut_assert(uthread_schedule());
> +       /* At this point there should be no runnable thread in group 'id1' */
> +       ut_assert(uthread_grp_done(id1));
> +       /* Five more calls for the second worker to finish incrementing  */
> +       for (i = 0; i < 5; i++)
> +               ut_assert(uthread_schedule());
> +       ut_asserteq(15, count);
> +       /* Plus one call to let the second worker return from its entry point */
> +       ut_assert(uthread_schedule());
> +       /* Now both tasks should be done, schedule should return false */
> +       ut_assert(!uthread_schedule());
> +
> +       return 0;
> +}
> +LIB_TEST(lib_uthread, 0);
> --
> 2.43.0
>
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 11/14] dm: usb: move bus initialization into new static function usb_init_bus()
  2025-02-25 16:34 ` [PATCH v2 11/14] dm: usb: move bus initialization into new static function usb_init_bus() Jerome Forissier
@ 2025-02-28 13:27   ` Ilias Apalodimas
  0 siblings, 0 replies; 47+ messages in thread
From: Ilias Apalodimas @ 2025-02-28 13:27 UTC (permalink / raw)
  To: Jerome Forissier
  Cc: u-boot, Marek Vasut, Tom Rini, Mattijs Korpershoek,
	Heinrich Schuchardt, Caleb Connolly, Dragan Simic

On Tue, 25 Feb 2025 at 18:35, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> To prepare for the introduction of coroutines in the USB initialization
> sequence, move code out of usb_init() into a new helper function:
> usb_init_bus(). No functional change.
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>  drivers/usb/host/usb-uclass.c | 88 +++++++++++++++++++----------------
>  1 file changed, 48 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
> index bfec303e7af..cc803241461 100644
> --- a/drivers/usb/host/usb-uclass.c
> +++ b/drivers/usb/host/usb-uclass.c
> @@ -287,9 +287,55 @@ static int usb_probe_companion(struct udevice *bus)
>         return 0;
>  }
>
> +static int controllers_initialized;
> +
> +static void usb_init_bus(struct udevice *bus)
> +{
> +       int ret;
> +
> +       /* init low_level USB */
> +       printf("Bus %s: ", bus->name);
> +
> +       /*
> +        * For Sandbox, we need scan the device tree each time when we
> +        * start the USB stack, in order to re-create the emulated USB
> +        * devices and bind drivers for them before we actually do the
> +        * driver probe.
> +        *
> +        * For USB onboard HUB, we need to do some non-trivial init
> +        * like enabling a power regulator, before enumeration.
> +        */
> +       if (IS_ENABLED(CONFIG_SANDBOX) ||
> +           IS_ENABLED(CONFIG_USB_ONBOARD_HUB)) {
> +               ret = dm_scan_fdt_dev(bus);
> +               if (ret) {
> +                       printf("USB device scan from fdt failed (%d)", ret);
> +                       return;
> +               }
> +       }
> +
> +       ret = device_probe(bus);
> +       if (ret == -ENODEV) {   /* No such device. */
> +               puts("Port not available.\n");
> +               controllers_initialized++;
> +               return;
> +       }
> +
> +       if (ret) {              /* Other error. */
> +               printf("probe failed, error %d\n", ret);
> +               return;
> +       }
> +
> +       ret = usb_probe_companion(bus);
> +       if (ret)
> +               return;
> +
> +       controllers_initialized++;
> +       usb_started = true;
> +}
> +
>  int usb_init(void)
>  {
> -       int controllers_initialized = 0;
>         struct usb_uclass_priv *uc_priv;
>         struct usb_bus_priv *priv;
>         struct udevice *bus;
> @@ -305,45 +351,7 @@ int usb_init(void)
>         uc_priv = uclass_get_priv(uc);
>
>         uclass_foreach_dev(bus, uc) {
> -               /* init low_level USB */
> -               printf("Bus %s: ", bus->name);
> -
> -               /*
> -                * For Sandbox, we need scan the device tree each time when we
> -                * start the USB stack, in order to re-create the emulated USB
> -                * devices and bind drivers for them before we actually do the
> -                * driver probe.
> -                *
> -                * For USB onboard HUB, we need to do some non-trivial init
> -                * like enabling a power regulator, before enumeration.
> -                */
> -               if (IS_ENABLED(CONFIG_SANDBOX) ||
> -                   IS_ENABLED(CONFIG_USB_ONBOARD_HUB)) {
> -                       ret = dm_scan_fdt_dev(bus);
> -                       if (ret) {
> -                               printf("USB device scan from fdt failed (%d)", ret);
> -                               continue;
> -                       }
> -               }
> -
> -               ret = device_probe(bus);
> -               if (ret == -ENODEV) {   /* No such device. */
> -                       puts("Port not available.\n");
> -                       controllers_initialized++;
> -                       continue;
> -               }
> -
> -               if (ret) {              /* Other error. */
> -                       printf("probe failed, error %d\n", ret);
> -                       continue;
> -               }
> -
> -               ret = usb_probe_companion(bus);
> -               if (ret)
> -                       continue;
> -
> -               controllers_initialized++;
> -               usb_started = true;
> +               usb_init_bus(bus);
>         }
>
>         /*
> --
> 2.43.0
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 02/14] arm: add initjmp()
  2025-02-28 13:21     ` Jerome Forissier
@ 2025-02-28 13:28       ` Heinrich Schuchardt
  2025-02-28 14:16         ` Jerome Forissier
  0 siblings, 1 reply; 47+ messages in thread
From: Heinrich Schuchardt @ 2025-02-28 13:28 UTC (permalink / raw)
  To: Jerome Forissier
  Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Dan Carpenter,
	Andrew Goodbody, Jiaxun Yang, Yu-Chien Peter Lin, u-boot

On 28.02.25 14:21, Jerome Forissier wrote:
>
>
> On 2/28/25 14:05, Heinrich Schuchardt wrote:
>> On 25.02.25 17:34, Jerome Forissier wrote:
>>> Implement initjmp() for Arm.
>>>
>>> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
>>> ---
>>>    arch/Kconfig                  |  1 +
>>>    arch/arm/include/asm/setjmp.h |  1 +
>>>    arch/arm/lib/setjmp.S         | 11 +++++++++++
>>>    arch/arm/lib/setjmp_aarch64.S |  9 +++++++++
>>>    4 files changed, 22 insertions(+)
>>>
>>> diff --git a/arch/Kconfig b/arch/Kconfig
>>> index 8d5b54031b3..57695fada8d 100644
>>> --- a/arch/Kconfig
>>> +++ b/arch/Kconfig
>>> @@ -94,6 +94,7 @@ config ARC
>>>    config ARM
>>>        bool "ARM architecture"
>>>        select HAVE_SETJMP
>>> +    select HAVE_INITJMP
>>>        select ARCH_SUPPORTS_LTO
>>>        select CREATE_ARCH_SYMLINK
>>>        select HAVE_PRIVATE_LIBGCC if !ARM64
>>> diff --git a/arch/arm/include/asm/setjmp.h b/arch/arm/include/asm/setjmp.h
>>> index 662bec86321..1ad5b500f2a 100644
>>> --- a/arch/arm/include/asm/setjmp.h
>>> +++ b/arch/arm/include/asm/setjmp.h
>>> @@ -23,5 +23,6 @@ typedef struct jmp_buf_data jmp_buf[1];
>>>
>>>    int setjmp(jmp_buf jmp);
>>>    void longjmp(jmp_buf jmp, int ret);
>>> +int initjmp(jmp_buf jmp, void __noreturn (*func)(void), void *stack_top);
>>
>> As this is not a standard function we cannot expect developers to know
>> what it does and how to use it.
>>
>> Please, provide Sphinx style documentation with the necessary information.
>
> I agree on principle, but if I add documentation here then I should add the
> same for all architectures. That being said the prototypes are already
> duplicated so maybe consolidation is a question for another time.  I will
> add Shpinx style doc to all 3 commits "{arm,riscv,sandbox}: add initjmp()".

Yes, this was not designed correctly:

We should move setjmp.h to include/asm-generic and include an
architecture specific setjmp_bits.h from there.

setjmp_bits.h would provide the struct jmp_buf_data definition.

>
> As for usage, I think the best documentation is test/lib/initjmp.c added
> by "test: lib: add initjmp() test".

Example code is valuable but does not replace documentation.

Best regards

Heinrich

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 08/14] lib: time: hook uthread_schedule() into udelay()
  2025-02-25 16:34 ` [PATCH v2 08/14] lib: time: hook uthread_schedule() into udelay() Jerome Forissier
@ 2025-02-28 13:38   ` Ilias Apalodimas
  2025-02-28 14:16     ` Yao Zi
  0 siblings, 1 reply; 47+ messages in thread
From: Ilias Apalodimas @ 2025-02-28 13:38 UTC (permalink / raw)
  To: Jerome Forissier; +Cc: u-boot, Tom Rini, Simon Glass

Hi Jerome,

On Tue, 25 Feb 2025 at 18:35, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> Introduce a uthread scheduling loop into udelay() when CONFIG_UTHREAD
> is enabled. This means that any uthread calling into udelay() may yield
> to uthread and be scheduled again later.
>
> While not strictly necessary since uthread_schedule() is already called
> by schedule(),
> tests show that it is desirable to call it in a tight
> loop instead of calling __usleep(). It gives more opportunities for
> other threads to make progress and results in better performances.

Some examples of timing gains would be nice.

>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>  lib/time.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/lib/time.c b/lib/time.c
> index d88edafb196..d1a1a66f301 100644
> --- a/lib/time.c
> +++ b/lib/time.c
> @@ -17,6 +17,7 @@
>  #include <asm/global_data.h>
>  #include <asm/io.h>
>  #include <linux/delay.h>
> +#include <uthread.h>
>
>  #ifndef CFG_WD_PERIOD
>  # define CFG_WD_PERIOD (10 * 1000 * 1000)      /* 10 seconds default */
> @@ -197,7 +198,14 @@ void udelay(unsigned long usec)
>         do {
>                 schedule();
>                 kv = usec > CFG_WD_PERIOD ? CFG_WD_PERIOD : usec;
> -               __udelay(kv);
> +               if (CONFIG_IS_ENABLED(UTHREAD)) {
> +                       ulong t0 = timer_get_us();
> +                       while (timer_get_us() < t0 + kv)

Do we make progress by constantly scheduling new tasks? Perhaps we
should at least leave the task running for some time?

Thanks
/Ilias

> +                               uthread_schedule();
> +               } else {
> +                       __udelay(kv);
> +               }
>                 usec -= kv;
>         } while(usec);
> +
>  }
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 02/14] arm: add initjmp()
  2025-02-28 13:28       ` Heinrich Schuchardt
@ 2025-02-28 14:16         ` Jerome Forissier
  2025-02-28 17:54           ` Tom Rini
  0 siblings, 1 reply; 47+ messages in thread
From: Jerome Forissier @ 2025-02-28 14:16 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Dan Carpenter,
	Andrew Goodbody, Jiaxun Yang, Yu-Chien Peter Lin, u-boot



On 2/28/25 14:28, Heinrich Schuchardt wrote:
> On 28.02.25 14:21, Jerome Forissier wrote:
>>
>>
>> On 2/28/25 14:05, Heinrich Schuchardt wrote:
>>> On 25.02.25 17:34, Jerome Forissier wrote:
>>>> Implement initjmp() for Arm.
>>>>
>>>> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
>>>> ---
>>>>    arch/Kconfig                  |  1 +
>>>>    arch/arm/include/asm/setjmp.h |  1 +
>>>>    arch/arm/lib/setjmp.S         | 11 +++++++++++
>>>>    arch/arm/lib/setjmp_aarch64.S |  9 +++++++++
>>>>    4 files changed, 22 insertions(+)
>>>>
>>>> diff --git a/arch/Kconfig b/arch/Kconfig
>>>> index 8d5b54031b3..57695fada8d 100644
>>>> --- a/arch/Kconfig
>>>> +++ b/arch/Kconfig
>>>> @@ -94,6 +94,7 @@ config ARC
>>>>    config ARM
>>>>        bool "ARM architecture"
>>>>        select HAVE_SETJMP
>>>> +    select HAVE_INITJMP
>>>>        select ARCH_SUPPORTS_LTO
>>>>        select CREATE_ARCH_SYMLINK
>>>>        select HAVE_PRIVATE_LIBGCC if !ARM64
>>>> diff --git a/arch/arm/include/asm/setjmp.h b/arch/arm/include/asm/setjmp.h
>>>> index 662bec86321..1ad5b500f2a 100644
>>>> --- a/arch/arm/include/asm/setjmp.h
>>>> +++ b/arch/arm/include/asm/setjmp.h
>>>> @@ -23,5 +23,6 @@ typedef struct jmp_buf_data jmp_buf[1];
>>>>
>>>>    int setjmp(jmp_buf jmp);
>>>>    void longjmp(jmp_buf jmp, int ret);
>>>> +int initjmp(jmp_buf jmp, void __noreturn (*func)(void), void *stack_top);
>>>
>>> As this is not a standard function we cannot expect developers to know
>>> what it does and how to use it.
>>>
>>> Please, provide Sphinx style documentation with the necessary information.
>>
>> I agree on principle, but if I add documentation here then I should add the
>> same for all architectures. That being said the prototypes are already
>> duplicated so maybe consolidation is a question for another time.  I will
>> add Shpinx style doc to all 3 commits "{arm,riscv,sandbox}: add initjmp()".
> 
> Yes, this was not designed correctly:
> 
> We should move setjmp.h to include/asm-generic and include an
> architecture specific setjmp_bits.h from there.
> 
> setjmp_bits.h would provide the struct jmp_buf_data definition.

Makes sense. I'll rework in v3.

> 
>>
>> As for usage, I think the best documentation is test/lib/initjmp.c added
>> by "test: lib: add initjmp() test".
> 
> Example code is valuable but does not replace documentation.

Should I provide a stripped-down example in the Sphinx doc? Or do you have
another location in mind? (doc/develop/initjmp.rst?).

Thanks,
-- 
Jerome

> 
> Best regards
> 
> Heinrich

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 08/14] lib: time: hook uthread_schedule() into udelay()
  2025-02-28 13:38   ` Ilias Apalodimas
@ 2025-02-28 14:16     ` Yao Zi
  2025-02-28 14:45       ` Jerome Forissier
  2025-02-28 18:16       ` Ilias Apalodimas
  0 siblings, 2 replies; 47+ messages in thread
From: Yao Zi @ 2025-02-28 14:16 UTC (permalink / raw)
  To: Ilias Apalodimas, Jerome Forissier; +Cc: u-boot, Tom Rini, Simon Glass

On Fri, Feb 28, 2025 at 03:38:22PM +0200, Ilias Apalodimas wrote:
> Hi Jerome,
> 
> On Tue, 25 Feb 2025 at 18:35, Jerome Forissier
> <jerome.forissier@linaro.org> wrote:
> >
> > Introduce a uthread scheduling loop into udelay() when CONFIG_UTHREAD
> > is enabled. This means that any uthread calling into udelay() may yield
> > to uthread and be scheduled again later.
> >
> > While not strictly necessary since uthread_schedule() is already called
> > by schedule(),
> > tests show that it is desirable to call it in a tight
> > loop instead of calling __usleep(). It gives more opportunities for
> > other threads to make progress and results in better performances.
> 
> Some examples of timing gains would be nice.
> 
> >
> > Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> > ---
> >  lib/time.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/time.c b/lib/time.c
> > index d88edafb196..d1a1a66f301 100644
> > --- a/lib/time.c
> > +++ b/lib/time.c
> > @@ -17,6 +17,7 @@
> >  #include <asm/global_data.h>
> >  #include <asm/io.h>
> >  #include <linux/delay.h>
> > +#include <uthread.h>
> >
> >  #ifndef CFG_WD_PERIOD
> >  # define CFG_WD_PERIOD (10 * 1000 * 1000)      /* 10 seconds default */
> > @@ -197,7 +198,14 @@ void udelay(unsigned long usec)
> >         do {
> >                 schedule();
> >                 kv = usec > CFG_WD_PERIOD ? CFG_WD_PERIOD : usec;
> > -               __udelay(kv);
> > +               if (CONFIG_IS_ENABLED(UTHREAD)) {
> > +                       ulong t0 = timer_get_us();
> > +                       while (timer_get_us() < t0 + kv)
> 
> Do we make progress by constantly scheduling new tasks? Perhaps we
> should at least leave the task running for some time?

If I get the point, the UTHREAD is a cooperative framework, which means
a task yields the control flow only when it considers nothing else could
be done. And there's no preemption (at least in this revision). Thus I
don't think it's a problem.

> Thanks
> /Ilias

Best regards,
Yao Zi

> > +                               uthread_schedule();
> > +               } else {
> > +                       __udelay(kv);
> > +               }
> >                 usec -= kv;
> >         } while(usec);
> > +
> >  }
> > --
> > 2.43.0
> >

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 06/14] uthread: add cooperative multi-tasking interface
  2025-02-28 13:09   ` Ilias Apalodimas
@ 2025-02-28 14:30     ` Jerome Forissier
  0 siblings, 0 replies; 47+ messages in thread
From: Jerome Forissier @ 2025-02-28 14:30 UTC (permalink / raw)
  To: Ilias Apalodimas
  Cc: u-boot, Tom Rini, Simon Glass, Sughosh Ganu, Raymond Mao,
	Patrick Rudolph, Michal Simek, Heinrich Schuchardt



On 2/28/25 14:09, Ilias Apalodimas wrote:
> Hi Jerome,
> 
> On Tue, 25 Feb 2025 at 18:35, Jerome Forissier
> <jerome.forissier@linaro.org> wrote:
>>
>> Add an new
> 
> 'a'

Fixed in v3

> 
>>  internal API called uthread (Kconfig symbol: UTHREAD) which
>> provides cooperative multi-tasking. The goal is to be able to improve
>> the performance of some parts of U-Boot by overlapping lengthy
>> operations, and also implement background jobs in the U-Boot shell.
>> Each uthread has its own stack allocated on the heap. The default stack
>> size is defined by the UTHREAD_STACK_SIZE symbol and is used when
>> uthread_create() receives zero for the stack_sz argument.
>>
>> The implementation is based on context-switching via initjmp()/setjmp()/
>> longjmp() and is inspired from barebox threads [1]. A notion of thread
>> group helps with dependencies, such as when a thread needs to block
>> until a number of other threads have returned.
>>
>> The name "uthread" comes from "user-space threads" because the
>> scheduling happens with no help from a higher privileged mode, contrary
>> to more complex models where kernel threads are defined. But the 'u'
>> may as well stand for 'U-Boot' since the bootloader may actually be
>> running at any privilege level and the notion of user vs. kernel may
>> not make much sense in this context.
>>
>> [1] https://github.com/barebox/barebox/blob/master/common/bthread.c
>>
>> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
>> ---
>>  include/uthread.h |  44 ++++++++++++
>>  lib/Kconfig       |  21 ++++++
>>  lib/Makefile      |   2 +
>>  lib/uthread.c     | 178 ++++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 245 insertions(+)
>>  create mode 100644 include/uthread.h
>>  create mode 100644 lib/uthread.c
>>
>> diff --git a/include/uthread.h b/include/uthread.h
>> new file mode 100644
>> index 00000000000..f1f86d210d5
>> --- /dev/null
>> +++ b/include/uthread.h
>> @@ -0,0 +1,44 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/*
>> + * Copyright 2025 Linaro Limited
>> + */
>> +
>> +#include <linux/types.h>
>> +
>> +#ifndef _UTHREAD_H_
>> +#define _UTHREAD_H_
>> +
>> +#ifdef CONFIG_UTHREAD
>> +
>> +int uthread_create(void (*fn)(void *), void *arg, size_t stack_sz,
>> +                  unsigned int grp_id);
>> +bool uthread_schedule(void);
>> +unsigned int uthread_grp_new_id(void);
>> +bool uthread_grp_done(unsigned int grp_id);
>> +
>> +#else
>> +
>> +static inline int uthread_create(void (*fn)(void *), void *arg, size_t stack_sz,
>> +                                unsigned int grp_id)
>> +{
>> +       fn(arg);
>> +       return 0;
>> +}
>> +
>> +static inline bool uthread_schedule(void)
>> +{
>> +       return false;
>> +}
>> +
>> +static inline unsigned int uthread_grp_new_id(void)
>> +{
>> +       return 0;
>> +}
>> +
>> +static inline bool uthread_grp_done(unsigned int grp_id)
>> +{
>> +       return true;
>> +}
>> +
>> +#endif /* CONFIG_UTHREAD */
>> +#endif /* _UTHREAD_H_ */
>> diff --git a/lib/Kconfig b/lib/Kconfig
>> index 1a683dea670..b32740ecbcc 100644
>> --- a/lib/Kconfig
>> +++ b/lib/Kconfig
>> @@ -1255,6 +1255,27 @@ config PHANDLE_CHECK_SEQ
>>           enable this config option to distinguish them using
>>           phandles in fdtdec_get_alias_seq() function.
>>
>> +config UTHREAD
>> +       bool "Enable thread support"
>> +       depends on HAVE_INITJMP
>> +       help
>> +         Implement a simple form of cooperative multi-tasking based on
>> +         context-switching via initjmp(), setjmp() and longjmp(). The
>> +         uthread_ interface enables the main thread of execution to create
>> +         one or more secondary threads and schedule them until they all have
>> +         returned. At any point a thread may suspend its execution and
>> +         schedule another thread, which allows for the efficient multiplexing
>> +         of leghthy operations.
>> +
>> +config UTHREAD_STACK_SIZE
>> +       int "Default uthread stack size"
>> +       depends on UTHREAD
>> +       default 32768
>> +       help
>> +         The default stak size for uthreads. Each uthread has its own stack.
> 
> stack size

Fixed in v3

> 
>> +         When the stack_sz argument to uthread_create() is zero then this
>> +         value is used.
>> +
>>  endmenu
>>
>>  source "lib/fwu_updates/Kconfig"
>> diff --git a/lib/Makefile b/lib/Makefile
>> index a7bc2f3134a..3610694de7a 100644
>> --- a/lib/Makefile
>> +++ b/lib/Makefile
>> @@ -164,6 +164,8 @@ obj-$(CONFIG_LIB_ELF) += elf.o
>>
>>  obj-$(CONFIG_$(PHASE_)SEMIHOSTING) += semihosting.o
>>
>> +obj-$(CONFIG_UTHREAD) += uthread.o
>> +
>>  #
>>  # Build a fast OID lookup registry from include/linux/oid_registry.h
>>  #
>> diff --git a/lib/uthread.c b/lib/uthread.c
>> new file mode 100644
>> index 00000000000..430d1c0de32
>> --- /dev/null
>> +++ b/lib/uthread.c
>> @@ -0,0 +1,178 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2021 Ahmad Fatoum, Pengutronix
>> + * Copyright (C) 2025 Linaro Limited
>> + *
>> + * An implementation of cooperative multi-tasking inspired from barebox threads
>> + * https://github.com/barebox/barebox/blob/master/common/bthread.c
>> + */
>> +
>> +#include <compiler.h>
>> +#include <asm/setjmp.h>
>> +#include <linux/kernel.h>
>> +#include <linux/list.h>
>> +#include <malloc.h>
>> +#include <stdint.h>
>> +#include <uthread.h>
>> +
>> +static struct uthread {
>> +       void (*fn)(void *);
>> +       void *arg;
>> +       jmp_buf ctx;
>> +       void *stack;
>> +       bool done;
>> +       unsigned int grp_id;
>> +       struct list_head list;
>> +} main_thread = {
>> +       .list = LIST_HEAD_INIT(main_thread.list),
>> +};
>> +
>> +static struct uthread *current = &main_thread;
>> +
>> +/**
>> + * uthread_trampoline() - Call the current thread's entry point then resume the
>> + * main thread.
>> + *
>> + * This is a helper function which is used as the @func argument to the inijmp()
> 
> initjump
> 
>> + * function, and ultimately invoked via setjmp(). It does not return, but
>> + * instead longjmp()'s back to the main thread.
>> + */
>> +static void __noreturn uthread_trampoline(void)
>> +{
>> +       struct uthread *curr = current;
>> +
>> +       curr->fn(curr->arg);
>> +       curr->done = true;
>> +       current = &main_thread;
>> +       longjmp(current->ctx, 1);
>> +       /* Not reached */
>> +       while (true)
>> +               ;
>> +}
>> +
>> +/**
>> + * uthread_free() - Free memory used by a uthread object.
>> + */
>> +static void uthread_free(struct uthread *uthread)
>> +{
>> +       if (!uthread)
>> +               return;
>> +       free(uthread->stack);
>> +       free(uthread);
>> +}
>> +
>> +/**
>> + * uthread_create() - Create a uthread object and make it ready for execution
>> + *
>> + * Threads are automatically deleted when then return from their entry point.
> 
> when they

Fixed in v3

> 
> [...]
>> +/**
>> + * uthread_grp_new_id() - return a new ID for a thread group
>> + *
>> + * Return: the new thread group ID
>> + */
>> +unsigned int uthread_grp_new_id(void)
>> +{
>> +       static unsigned int id = 0;
>> +
>> +       return ++id;
>> +}
> 
> This seems a bit weird. Why do we need this function?

As it should appear clearly in the subsequent patches, it is to help with
dependencies. Suppose the main thread creates a thread to perform the
"usb start" command. At this point we have two threads: let's call them
main and usb_start. The latter enumerates the busses and creates one
thread per bus to do the actual device scan. Let's assume we have two
busses, then usb_start creates scan1 and scan2.

The main thread needs to do:

while (!done(usb_start))
    uthread_schedule();

The usb_start on the other hand is only interested in is own two threads,
scan1 and scan2. It has to wait until both are done, and only those two:

while (!done(scan1) || !done(scan2))
    uthread_schedule();

This can easily be written if threads can be assigned group IDs:

/* Main */
usb_start_id = uthread_new_grp_id();
uthread_create(..., usb_start, NULL, usb_start_id);
while (!uthread_grp_done(usb_start_id))
    uthread_schedule();

/* usb_start() */
scan_id = uthread_new_grp_id();
uthread_create(..., usb_start, bus1, scan_id);
uthread_create(..., usb_start, bus2, scan_id);
while (!uthread_grp_done(scan_id))
    uthread_schedule();

> 
>> +
>> +/**
>> + * uthread_grp_done() - test if all threads in a group are done
>> + *
>> + * @grp: the ID of the thread group that should be considered
>> + * Return: false if the group contains at least one runnable thread (i.e., one
>> + * thread which entry point has not returned yet), true otherwise
>> + */
>> +bool uthread_grp_done(unsigned int grp_id)
>> +{
>> +       struct uthread *next;
>> +
>> +       list_for_each_entry(next, &main_thread.list, list) {
>> +               if (next->grp_id == grp_id && !next->done)
>> +                       return false;
>> +       }
>> +
>> +       return true;
>> +}
>> --
>> 2.43.0
>>
> 
> 
> Apart from the minor typos and the function that I can't figure out
> why we need this look pretty clean

Thanks!

Cheers,
-- 
Jerome

> 
> Thnaks
> /Ilias

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 06/14] uthread: add cooperative multi-tasking interface
  2025-02-28 13:21   ` Heinrich Schuchardt
@ 2025-02-28 14:39     ` Jerome Forissier
  0 siblings, 0 replies; 47+ messages in thread
From: Jerome Forissier @ 2025-02-28 14:39 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Sughosh Ganu,
	Raymond Mao, Patrick Rudolph, Michal Simek, u-boot



On 2/28/25 14:21, Heinrich Schuchardt wrote:
> On 25.02.25 17:34, Jerome Forissier wrote:
>> Add an new internal API called uthread (Kconfig symbol: UTHREAD) which
> 
> nits:
> 
> %s/an/a/

Fixed in v3

> 
>> provides cooperative multi-tasking. The goal is to be able to improve
>> the performance of some parts of U-Boot by overlapping lengthy
>> operations, and also implement background jobs in the U-Boot shell.
>> Each uthread has its own stack allocated on the heap. The default stack
>> size is defined by the UTHREAD_STACK_SIZE symbol and is used when
>> uthread_create() receives zero for the stack_sz argument.
>>
>> The implementation is based on context-switching via initjmp()/setjmp()/
>> longjmp() and is inspired from barebox threads [1]. A notion of thread
>> group helps with dependencies, such as when a thread needs to block
>> until a number of other threads have returned.
>>
>> The name "uthread" comes from "user-space threads" because the
>> scheduling happens with no help from a higher privileged mode, contrary
>> to more complex models where kernel threads are defined. But the 'u'
>> may as well stand for 'U-Boot' since the bootloader may actually be
>> running at any privilege level and the notion of user vs. kernel may
>> not make much sense in this context.
>>
>> [1] https://github.com/barebox/barebox/blob/master/common/bthread.c
>>
>> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
>> ---
>>   include/uthread.h |  44 ++++++++++++
>>   lib/Kconfig       |  21 ++++++
>>   lib/Makefile      |   2 +
>>   lib/uthread.c     | 178 ++++++++++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 245 insertions(+)
>>   create mode 100644 include/uthread.h
>>   create mode 100644 lib/uthread.c
>>
>> diff --git a/include/uthread.h b/include/uthread.h
>> new file mode 100644
>> index 00000000000..f1f86d210d5
>> --- /dev/null
>> +++ b/include/uthread.h
>> @@ -0,0 +1,44 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/*
>> + * Copyright 2025 Linaro Limited
>> + */
>> +
>> +#include <linux/types.h>
>> +
>> +#ifndef _UTHREAD_H_
>> +#define _UTHREAD_H_
>> +
>> +#ifdef CONFIG_UTHREAD
>> +
>> +int uthread_create(void (*fn)(void *), void *arg, size_t stack_sz,
>> +           unsigned int grp_id);
> 
> Every function needs a Shinx style documentation.

Ack.

> 
>> +bool uthread_schedule(void);
>> +unsigned int uthread_grp_new_id(void);
>> +bool uthread_grp_done(unsigned int grp_id);
>> +
>> +#else
>> +
>> +static inline int uthread_create(void (*fn)(void *), void *arg, size_t stack_sz,
>> +                 unsigned int grp_id)
>> +{
>> +    fn(arg);
>> +    return 0;
>> +}
>> +
>> +static inline bool uthread_schedule(void)
>> +{
>> +    return false;
>> +}
>> +
>> +static inline unsigned int uthread_grp_new_id(void)
>> +{
>> +    return 0;
>> +}
>> +
>> +static inline bool uthread_grp_done(unsigned int grp_id)
>> +{
>> +    return true;
>> +}
>> +
>> +#endif /* CONFIG_UTHREAD */
>> +#endif /* _UTHREAD_H_ */
>> diff --git a/lib/Kconfig b/lib/Kconfig
>> index 1a683dea670..b32740ecbcc 100644
>> --- a/lib/Kconfig
>> +++ b/lib/Kconfig
>> @@ -1255,6 +1255,27 @@ config PHANDLE_CHECK_SEQ
>>         enable this config option to distinguish them using
>>         phandles in fdtdec_get_alias_seq() function.
>>
>> +config UTHREAD
>> +    bool "Enable thread support"
>> +    depends on HAVE_INITJMP
>> +    help
>> +      Implement a simple form of cooperative multi-tasking based on
>> +      context-switching via initjmp(), setjmp() and longjmp(). The
>> +      uthread_ interface enables the main thread of execution to create
>> +      one or more secondary threads and schedule them until they all have
>> +      returned. At any point a thread may suspend its execution and
>> +      schedule another thread, which allows for the efficient multiplexing
>> +      of leghthy operations.
>> +
>> +config UTHREAD_STACK_SIZE
>> +    int "Default uthread stack size"
>> +    depends on UTHREAD
>> +    default 32768
>> +    help
>> +      The default stak size for uthreads. Each uthread has its own stack.
>> +      When the stack_sz argument to uthread_create() is zero then this
>> +      value is used.
>> +
>>   endmenu
>>
>>   source "lib/fwu_updates/Kconfig"
>> diff --git a/lib/Makefile b/lib/Makefile
>> index a7bc2f3134a..3610694de7a 100644
>> --- a/lib/Makefile
>> +++ b/lib/Makefile
>> @@ -164,6 +164,8 @@ obj-$(CONFIG_LIB_ELF) += elf.o
>>
>>   obj-$(CONFIG_$(PHASE_)SEMIHOSTING) += semihosting.o
>>
>> +obj-$(CONFIG_UTHREAD) += uthread.o
>> +
>>   #
>>   # Build a fast OID lookup registry from include/linux/oid_registry.h
>>   #
>> diff --git a/lib/uthread.c b/lib/uthread.c
>> new file mode 100644
>> index 00000000000..430d1c0de32
>> --- /dev/null
>> +++ b/lib/uthread.c
>> @@ -0,0 +1,178 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2021 Ahmad Fatoum, Pengutronix
>> + * Copyright (C) 2025 Linaro Limited
>> + *
>> + * An implementation of cooperative multi-tasking inspired from barebox threads
>> + * https://github.com/barebox/barebox/blob/master/common/bthread.c
>> + */
>> +
>> +#include <compiler.h>
>> +#include <asm/setjmp.h>
>> +#include <linux/kernel.h>
>> +#include <linux/list.h>
>> +#include <malloc.h>
>> +#include <stdint.h>
>> +#include <uthread.h>
>> +
>> +static struct uthread {
>> +    void (*fn)(void *);
>> +    void *arg;
>> +    jmp_buf ctx;
>> +    void *stack;
>> +    bool done;
>> +    unsigned int grp_id;
>> +    struct list_head list;
> 
> The structure and all its components need to be documented.

Ack.

> 
>> +} main_thread = {
>> +    .list = LIST_HEAD_INIT(main_thread.list),
>> +};
>> +
>> +static struct uthread *current = &main_thread;
>> +
>> +/**
>> + * uthread_trampoline() - Call the current thread's entry point then resume the
>> + * main thread.
>> + *
>> + * This is a helper function which is used as the @func argument to the inijmp()
>> + * function, and ultimately invoked via setjmp(). It does not return, but
>> + * instead longjmp()'s back to the main thread.
>> + */
>> +static void __noreturn uthread_trampoline(void)
>> +{
>> +    struct uthread *curr = current;
>> +
>> +    curr->fn(curr->arg);
>> +    curr->done = true;
>> +    current = &main_thread;
>> +    longjmp(current->ctx, 1);
>> +    /* Not reached */
>> +    while (true)
>> +        ;
>> +}
>> +
>> +/**
>> + * uthread_free() - Free memory used by a uthread object.
>> + */
>> +static void uthread_free(struct uthread *uthread)
>> +{
>> +    if (!uthread)
>> +        return;
>> +    free(uthread->stack);
>> +    free(uthread);
>> +}
>> +
>> +/**
>> + * uthread_create() - Create a uthread object and make it ready for execution
>> + *
>> + * Threads are automatically deleted when then return from their entry point.
>> + *
>> + * @fn: the thread's entry point
>> + * @arg: argument passed to the thread's entry point
>> + * @stack_sz: stack size for the new thread (in bytes). The stack is allocated
>> + * on the heap.
>> + * @grp_id: an optional thread group ID that the new thread should belong to
>> + * (zero for no group)
>> + */
> 
> This documentation should be move to the include and the include needs
> to be added to doc/api/.

OK.

> 
>> +int uthread_create(void (*fn)(void *), void *arg, size_t stack_sz,
>> +           unsigned int grp_id)
> 
> We should model the functions more like pthreads.
> 
> Let the caller provide struct uthread(). This will allow us to implement
> pthread_cancel() and phthread_join().

I'd like to keep the ability to let the framework do as much as possible
and the notion of grp_id already allows to wait and could be used to cancel
too. That said, the uthread struct would be helpful to get the return status.
I can do:

int uthread_create(struct uthread *uthread, void (*fn)(void *), void *arg,
                   size_t stack_sz, unsigned int grp_id);

...with uthread being optional (NULL allowed). Would that work for you?

Regards,
-- 
Jerome

> 
> Best regards
> 
> Heinrich
> 
>> +{
>> +    struct uthread *uthread;
>> +
>> +    if (!stack_sz)
>> +        stack_sz = CONFIG_UTHREAD_STACK_SIZE;
>> +
>> +    uthread = calloc(1, sizeof(*uthread));
>> +    if (!uthread)
>> +        return -1;
>> +
>> +    uthread->stack = memalign(16, stack_sz);
>> +    if (!uthread->stack)
>> +        goto err;
>> +
>> +    uthread->fn = fn;
>> +    uthread->arg = arg;
>> +    uthread->grp_id = grp_id;
>> +
>> +    list_add_tail(&uthread->list, &current->list);
>> +
>> +    initjmp(uthread->ctx, uthread_trampoline, uthread->stack + stack_sz);
>> +
>> +    return 0;
>> +err:
>> +    uthread_free(uthread);
>> +    return -1;
>> +}
>> +
>> +/**
>> + * uthread_resume() - switch execution to a given thread
>> + *
>> + * @uthread: the thread object that should be resumed
>> + */
>> +static void uthread_resume(struct uthread *uthread)
>> +{
>> +    if (!setjmp(current->ctx)) {
>> +        current = uthread;
>> +        longjmp(uthread->ctx, 1);
>> +    }
>> +}
>> +
>> +/**
>> + * uthread_schedule() - yield the CPU to the next runnable thread
>> + *
>> + * This function is called either by the main thread or any secondary thread
>> + * (that is, any thread created via uthread_create()) to switch execution to
>> + * the next runnable thread.
>> + *
>> + * Return: true if a thread was scheduled, false if no runnable thread was found
>> + */
>> +bool uthread_schedule(void)
>> +{
>> +    struct uthread *next;
>> +    struct uthread *tmp;
>> +
>> +    if (list_empty(&current->list))
>> +        return false;
>> +
>> +    list_for_each_entry_safe(next, tmp, &current->list, list) {
>> +        if (!next->done) {
>> +            uthread_resume(next);
>> +            return true;
>> +        } else {
>> +            /* Found a 'done' thread, free its resources */
>> +            list_del(&next->list);
>> +            uthread_free(next);
>> +        }
>> +    }
>> +    return false;
>> +}
>> +
>> +/**
>> + * uthread_grp_new_id() - return a new ID for a thread group
>> + *
>> + * Return: the new thread group ID
>> + */
>> +unsigned int uthread_grp_new_id(void)
>> +{
>> +    static unsigned int id = 0;
>> +
>> +    return ++id;
>> +}
>> +
>> +/**
>> + * uthread_grp_done() - test if all threads in a group are done
>> + *
>> + * @grp: the ID of the thread group that should be considered
>> + * Return: false if the group contains at least one runnable thread (i.e., one
>> + * thread which entry point has not returned yet), true otherwise
>> + */
>> +bool uthread_grp_done(unsigned int grp_id)
>> +{
>> +    struct uthread *next;
>> +
>> +    list_for_each_entry(next, &main_thread.list, list) {
>> +        if (next->grp_id == grp_id && !next->done)
>> +            return false;
>> +    }
>> +
>> +    return true;
>> +}
> 

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 08/14] lib: time: hook uthread_schedule() into udelay()
  2025-02-28 14:16     ` Yao Zi
@ 2025-02-28 14:45       ` Jerome Forissier
  2025-02-28 18:16       ` Ilias Apalodimas
  1 sibling, 0 replies; 47+ messages in thread
From: Jerome Forissier @ 2025-02-28 14:45 UTC (permalink / raw)
  To: Yao Zi, Ilias Apalodimas; +Cc: u-boot, Tom Rini, Simon Glass



On 2/28/25 15:16, Yao Zi wrote:
> On Fri, Feb 28, 2025 at 03:38:22PM +0200, Ilias Apalodimas wrote:
>> Hi Jerome,
>>
>> On Tue, 25 Feb 2025 at 18:35, Jerome Forissier
>> <jerome.forissier@linaro.org> wrote:
>>>
>>> Introduce a uthread scheduling loop into udelay() when CONFIG_UTHREAD
>>> is enabled. This means that any uthread calling into udelay() may yield
>>> to uthread and be scheduled again later.
>>>
>>> While not strictly necessary since uthread_schedule() is already called
>>> by schedule(),
>>> tests show that it is desirable to call it in a tight
>>> loop instead of calling __usleep(). It gives more opportunities for
>>> other threads to make progress and results in better performances.
>>
>> Some examples of timing gains would be nice.

I'll try to provide numbers in v3.

>>
>>>
>>> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
>>> ---
>>>  lib/time.c | 10 +++++++++-
>>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/time.c b/lib/time.c
>>> index d88edafb196..d1a1a66f301 100644
>>> --- a/lib/time.c
>>> +++ b/lib/time.c
>>> @@ -17,6 +17,7 @@
>>>  #include <asm/global_data.h>
>>>  #include <asm/io.h>
>>>  #include <linux/delay.h>
>>> +#include <uthread.h>
>>>
>>>  #ifndef CFG_WD_PERIOD
>>>  # define CFG_WD_PERIOD (10 * 1000 * 1000)      /* 10 seconds default */
>>> @@ -197,7 +198,14 @@ void udelay(unsigned long usec)
>>>         do {
>>>                 schedule();
>>>                 kv = usec > CFG_WD_PERIOD ? CFG_WD_PERIOD : usec;
>>> -               __udelay(kv);
>>> +               if (CONFIG_IS_ENABLED(UTHREAD)) {
>>> +                       ulong t0 = timer_get_us();
>>> +                       while (timer_get_us() < t0 + kv)
>>
>> Do we make progress by constantly scheduling new tasks? Perhaps we
>> should at least leave the task running for some time?
> 
> If I get the point, the UTHREAD is a cooperative framework, which means
> a task yields the control flow only when it considers nothing else could
> be done. And there's no preemption (at least in this revision). Thus I
> don't think it's a problem.

That's correct. Code always executes uninterrupted until it reaches
uthread_schedule().

Thanks,
-- 
Jerome

> 
>> Thanks
>> /Ilias
> 
> Best regards,
> Yao Zi
> 
>>> +                               uthread_schedule();
>>> +               } else {
>>> +                       __udelay(kv);
>>> +               }
>>>                 usec -= kv;
>>>         } while(usec);
>>> +
>>>  }
>>> --
>>> 2.43.0
>>>

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 07/14] cyclic: invoke uthread_schedule() from schedule()
  2025-02-27 17:05     ` Jerome Forissier
@ 2025-02-28 15:43       ` Stefan Roese
  0 siblings, 0 replies; 47+ messages in thread
From: Stefan Roese @ 2025-02-28 15:43 UTC (permalink / raw)
  To: Jerome Forissier, u-boot
  Cc: Ilias Apalodimas, Tom Rini, Rasmus Villemoes, Simon Glass,
	Patrice Chotard

Hi Jerome,

On 27.02.25 18:05, Jerome Forissier wrote:
> Hi Stefan,
> 
> On 2/27/25 13:30, Stefan Roese wrote:
>> Hi Jerome,
>>
>> On 25.02.25 17:34, Jerome Forissier wrote:
>>> Make the schedule() call from the CYCLIC framework a uthread scheduling
>>> point too. This makes sense since schedule() is called from a lot of
>>> places where uthread_schedule() needs to be called.
>>>
>>> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
>>
>> Frankly, at first I was wondering a bit, if and why another framework
>> for "multitasking" is needed in U-Boot, additionally to the cyclic
>> framework that I introduced a few years ago. Which was greatly enhanced
>> by Rasmus over the time. But looking at your "uthread" implementation
>> it makes sense to add such a probably more intuitive interface as well.
> 
> cyclic is clean and simple and certainly well suited when introducing new
> code. But when reworking older code I find it somewhat difficult to use
> due to the need to keep a context and pass it everywhere. This can lead
> to lots of changes when call stacks are deep.
> 
>> In general I'm really happy seeing activity in this "multitaking" area
>> in U-Boot. As it brings a lot of new possibilities and, as you've also
>> shown in your patchset, may greatly help reducing boot time in the
>> USB example. :)
> 
> Thank you.
>   
>> One question though:
>> Do you have some means in your uthread framework, measuring and
>> perhaps limiting the time spent in these uthreads? If and how is a
>> preemption of a uthread possible? So that it does not consume too
>> much time resulting in e.g. things like dropping input chars on
>> the prompt? Sorry, I did not thoroughly go through all your code
>> to get the internals from there. It would be great if you could
>> elaborate a bit on this.
> 
> That's a very valid point. The short answer is no, there is no control
> over how long a thread keeps the CPU busy. uthread is similar to cyclic
> in that respect. That said, I occasionally noted the issue you mentioned
> about the console dropping characters, and yes it is annoying. I believe
> there is a simple solution though. If we can somehow make sure we're
> always scheduling the main thread every other time, we're much less
> likely to starve it from the CPU, especially when many threads are
> active. That is, assume we have 2 threads in addition to the main thread.
> The thread list is main -> thread1 -> thread2 and uthread_schedule() will
> iterate in that order. So back to main only after thread1 *and* thread2
> have run and called uthread_schedule(). The idea is to schedule in a
> different order: main -> thread1 -> main -> thread2 -> main etc.,
> effectively giving a higher priority to the main thread (which would be
> the console parsing thread).

Sounds like a good idea / improvement to me.

Thanks,
Stefan

> I feel that introducing preemption would be opening a can of worms...
> Because in this case we would likely need locking everywhere. Without
> preemption, we still do need locking in theory, it's just that I have
> not yet identified critical sections where locks would be mandatory in
> the code that I have "parallelized". BTW I believe a uthread lock would
> be trivial to implement like so:
> 
> struct uthread_lock {
> 	bool locked;
> };
> 
> void uthread_lock(struct uthread_lock *l)
> {
> 	while (l->locked)
> 		uthread_schedule();
> 	l->locked = true;
> }
> void uthread_unlock(struct uthread_lock *l)
> {
> 	l->locked = false;
> }
> 
>>
>> For this patch:
>>
>> Reviewed-by: Stefan Roese <sr@denx.de>
>>
>> Thanks,
>> Stefan
> 
> Thanks,


^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 02/14] arm: add initjmp()
  2025-02-28 14:16         ` Jerome Forissier
@ 2025-02-28 17:54           ` Tom Rini
  0 siblings, 0 replies; 47+ messages in thread
From: Tom Rini @ 2025-02-28 17:54 UTC (permalink / raw)
  To: Jerome Forissier
  Cc: Heinrich Schuchardt, Ilias Apalodimas, Simon Glass, Dan Carpenter,
	Andrew Goodbody, Jiaxun Yang, Yu-Chien Peter Lin, u-boot

[-- Attachment #1: Type: text/plain, Size: 2952 bytes --]

On Fri, Feb 28, 2025 at 03:16:37PM +0100, Jerome Forissier wrote:
> 
> 
> On 2/28/25 14:28, Heinrich Schuchardt wrote:
> > On 28.02.25 14:21, Jerome Forissier wrote:
> >>
> >>
> >> On 2/28/25 14:05, Heinrich Schuchardt wrote:
> >>> On 25.02.25 17:34, Jerome Forissier wrote:
> >>>> Implement initjmp() for Arm.
> >>>>
> >>>> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> >>>> ---
> >>>>    arch/Kconfig                  |  1 +
> >>>>    arch/arm/include/asm/setjmp.h |  1 +
> >>>>    arch/arm/lib/setjmp.S         | 11 +++++++++++
> >>>>    arch/arm/lib/setjmp_aarch64.S |  9 +++++++++
> >>>>    4 files changed, 22 insertions(+)
> >>>>
> >>>> diff --git a/arch/Kconfig b/arch/Kconfig
> >>>> index 8d5b54031b3..57695fada8d 100644
> >>>> --- a/arch/Kconfig
> >>>> +++ b/arch/Kconfig
> >>>> @@ -94,6 +94,7 @@ config ARC
> >>>>    config ARM
> >>>>        bool "ARM architecture"
> >>>>        select HAVE_SETJMP
> >>>> +    select HAVE_INITJMP
> >>>>        select ARCH_SUPPORTS_LTO
> >>>>        select CREATE_ARCH_SYMLINK
> >>>>        select HAVE_PRIVATE_LIBGCC if !ARM64
> >>>> diff --git a/arch/arm/include/asm/setjmp.h b/arch/arm/include/asm/setjmp.h
> >>>> index 662bec86321..1ad5b500f2a 100644
> >>>> --- a/arch/arm/include/asm/setjmp.h
> >>>> +++ b/arch/arm/include/asm/setjmp.h
> >>>> @@ -23,5 +23,6 @@ typedef struct jmp_buf_data jmp_buf[1];
> >>>>
> >>>>    int setjmp(jmp_buf jmp);
> >>>>    void longjmp(jmp_buf jmp, int ret);
> >>>> +int initjmp(jmp_buf jmp, void __noreturn (*func)(void), void *stack_top);
> >>>
> >>> As this is not a standard function we cannot expect developers to know
> >>> what it does and how to use it.
> >>>
> >>> Please, provide Sphinx style documentation with the necessary information.
> >>
> >> I agree on principle, but if I add documentation here then I should add the
> >> same for all architectures. That being said the prototypes are already
> >> duplicated so maybe consolidation is a question for another time.  I will
> >> add Shpinx style doc to all 3 commits "{arm,riscv,sandbox}: add initjmp()".
> > 
> > Yes, this was not designed correctly:
> > 
> > We should move setjmp.h to include/asm-generic and include an
> > architecture specific setjmp_bits.h from there.
> > 
> > setjmp_bits.h would provide the struct jmp_buf_data definition.
> 
> Makes sense. I'll rework in v3.
> 
> > 
> >>
> >> As for usage, I think the best documentation is test/lib/initjmp.c added
> >> by "test: lib: add initjmp() test".
> > 
> > Example code is valuable but does not replace documentation.
> 
> Should I provide a stripped-down example in the Sphinx doc? Or do you have
> another location in mind? (doc/develop/initjmp.rst?).

We should have doc/api/initjmp.rst and if test/lib/initjmp.c is
commented correctly / well enough it can be referenced in the rST via
'.. kernel-doc::test/lib/initjmp.c' I think.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 08/14] lib: time: hook uthread_schedule() into udelay()
  2025-02-28 14:16     ` Yao Zi
  2025-02-28 14:45       ` Jerome Forissier
@ 2025-02-28 18:16       ` Ilias Apalodimas
  1 sibling, 0 replies; 47+ messages in thread
From: Ilias Apalodimas @ 2025-02-28 18:16 UTC (permalink / raw)
  To: Yao Zi; +Cc: Jerome Forissier, u-boot, Tom Rini, Simon Glass

Hi Yao

On Fri, 28 Feb 2025 at 16:16, Yao Zi <ziyao@disroot.org> wrote:
>
> On Fri, Feb 28, 2025 at 03:38:22PM +0200, Ilias Apalodimas wrote:
> > Hi Jerome,
> >
> > On Tue, 25 Feb 2025 at 18:35, Jerome Forissier
> > <jerome.forissier@linaro.org> wrote:
> > >
> > > Introduce a uthread scheduling loop into udelay() when CONFIG_UTHREAD
> > > is enabled. This means that any uthread calling into udelay() may yield
> > > to uthread and be scheduled again later.
> > >
> > > While not strictly necessary since uthread_schedule() is already called
> > > by schedule(),
> > > tests show that it is desirable to call it in a tight
> > > loop instead of calling __usleep(). It gives more opportunities for
> > > other threads to make progress and results in better performances.
> >
> > Some examples of timing gains would be nice.
> >
> > >
> > > Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> > > ---
> > >  lib/time.c | 10 +++++++++-
> > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/time.c b/lib/time.c
> > > index d88edafb196..d1a1a66f301 100644
> > > --- a/lib/time.c
> > > +++ b/lib/time.c
> > > @@ -17,6 +17,7 @@
> > >  #include <asm/global_data.h>
> > >  #include <asm/io.h>
> > >  #include <linux/delay.h>
> > > +#include <uthread.h>
> > >
> > >  #ifndef CFG_WD_PERIOD
> > >  # define CFG_WD_PERIOD (10 * 1000 * 1000)      /* 10 seconds default */
> > > @@ -197,7 +198,14 @@ void udelay(unsigned long usec)
> > >         do {
> > >                 schedule();
> > >                 kv = usec > CFG_WD_PERIOD ? CFG_WD_PERIOD : usec;
> > > -               __udelay(kv);
> > > +               if (CONFIG_IS_ENABLED(UTHREAD)) {
> > > +                       ulong t0 = timer_get_us();
> > > +                       while (timer_get_us() < t0 + kv)

I think it's better to rewrite this as timer_get_us() - t0 < k.

> >
> > Do we make progress by constantly scheduling new tasks? Perhaps we
> > should at least leave the task running for some time?
>
> If I get the point, the UTHREAD is a cooperative framework, which means
> a task yields the control flow only when it considers nothing else could
> be done.

> And there's no preemption (at least in this revision). Thus I
> don't think it's a problem.

I was implying it would cause a problem, I was just wondering if we
could optimize it easily somehow. But i think it's fine as is

thanks
/Ilias


>
> > Thanks
> > /Ilias
>
> Best regards,
> Yao Zi
>
> > > +                               uthread_schedule();
> > > +               } else {
> > > +                       __udelay(kv);
> > > +               }
> > >                 usec -= kv;
> > >         } while(usec);
> > > +
> > >  }
> > > --
> > > 2.43.0
> > >

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 12/14] dm: usb: initialize and scan multiple buses simultaneously with uthread
  2025-02-27 17:30     ` Jerome Forissier
@ 2025-03-04 13:13       ` Simon Glass
  0 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2025-03-04 13:13 UTC (permalink / raw)
  To: Jerome Forissier
  Cc: u-boot, Ilias Apalodimas, Marek Vasut, Tom Rini,
	Mattijs Korpershoek, Heinrich Schuchardt, Caleb Connolly,
	Julius Lehmann, Guillaume La Roque

Hi Jerome,

On Thu, 27 Feb 2025 at 10:30, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
>
>
> On 2/27/25 17:25, Simon Glass wrote:
> > Hi Jerome,
> >
> > On Tue, 25 Feb 2025 at 09:35, Jerome Forissier
> > <jerome.forissier@linaro.org> wrote:
> >>
> >> Use the uthread framework to initialize and scan USB buses in parallel
> >> for better performance. The console output is slightly modified with a
> >> final per-bus report of the number of devices found, common to UTHREAD
> >> and !UTHREAD. The USB tests are updated accordingly.
> >>
> >> Tested on two platforms:
> >>
> >> 1. arm64 QEMU on a somewhat contrived example (4 USB buses, each with
> >> one audio device, one keyboard, one mouse and one tablet)
> >>
> >>  $ make qemu_arm64_defconfig
> >>  $ make -j$(nproc) CROSS_COMPILE="ccache aarch64-linux-gnu-"
> >>  $ qemu-system-aarch64 -M virt -nographic -cpu max -bios u-boot.bin \
> >>      $(for i in {1..4}; do echo -device qemu-xhci,id=xhci$i \
> >>          -device\ usb-{audio,kbd,mouse,tablet},bus=xhci$i.0; \
> >>      done)
> >>
> >> 2. i.MX93 EVK (imx93_11x11_evk_defconfig) with two USB hubs, each with
> >> one webcam and one ethernet adapter, resulting in the following device
> >> tree:
> >>
> >>  USB device tree:
> >>    1  Hub (480 Mb/s, 0mA)
> >>    |  u-boot EHCI Host Controller
> >>    |
> >>    +-2  Hub (480 Mb/s, 100mA)
> >>      |  GenesysLogic USB2.1 Hub
> >>      |
> >>      +-3  Vendor specific (480 Mb/s, 350mA)
> >>      |    Realtek USB 10/100/1000 LAN 001000001
> >>      |
> >>      +-4   (480 Mb/s, 500mA)
> >>            HD Pro Webcam C920 8F7CD51F
> >>
> >>    1  Hub (480 Mb/s, 0mA)
> >>    |  u-boot EHCI Host Controller
> >>    |
> >>    +-2  Hub (480 Mb/s, 100mA)
> >>      |   USB 2.0 Hub
> >>      |
> >>      +-3  Vendor specific (480 Mb/s, 200mA)
> >>      |    Realtek USB 10/100/1000 LAN 000001
> >>      |
> >>      +-4   (480 Mb/s, 500mA)
> >>           Generic OnLan-CS30 201801010008
> >>
> >> Note that i.MX was tested on top of the downstream repository [1] since
> >> USB doesn't work in the upstream master branch.
> >>
> >> [1] https://github.com/nxp-imx/uboot-imx/tree/lf-6.6.52-2.2.0
> >>     commit 6c4545203d12 ("LF-13928 update key for capsule")
> >>
> >> The time spent in usb_init() ("usb start" command) is reported on
> >> the console. Here are the results:
> >>
> >>         | CONFIG_UTHREAD=n | CONFIG_UTHREAD=y
> >> --------+------------------+-----------------
> >> QEMU    |          5628 ms |          2212 ms
> >> i.MX93  |          4591 ms |          2441 ms
> >>
> >> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> >> ---
> >>  drivers/usb/host/usb-uclass.c | 92 ++++++++++++++++++++++++++++-------
> >>  test/boot/bootdev.c           | 14 +++---
> >>  test/boot/bootflow.c          |  2 +-
> >>  3 files changed, 83 insertions(+), 25 deletions(-)
> >
> > What happens to output produced by a thread? Does it get stored
> > somewhere and written when the thread completes, or do the threads
> > intermingle their output?
>
> The latter. That is why I slightly reworked the USB initialization
> output to print a status once the threads are done, and I also updated
> the related tests as you can see in the patch. For instance the tests
> were expecting:
> "Bus usb@1: scanning bus usb@1 for devices... 5 USB Device(s) found"
> The "scanning bus usb@1 for devices..." part is printed by the threads,
> therefore in any order. I decided to move the text
> "Bus usb@1: 5 USB Device(s) found" to after the threads are complete,
> iterating over the devices in a deterministic order.

OK. I think at some point we would want to collected the output and
only show it when at a prompt or when all processing is done.

>
> >
> > I'm not sure if you saw my email about using a state machine for USB.
> > If so, could you please point me to your reply?
>
> I did, but I did not reply because I did not try. I have a feeling that
> the change would be more intrusive than what I did, but above all I am
> not doing the uthread thing to address only USB but as a general
> technique to make things parallel without too much trouble (at least
> that's my hope). So kind of a different goal.

They are separate approaches. I believe that having the usb-scanning
state in one place as an iterator would benefit U-Boot. It is
currently a bit messy.

Regards,
Simon

^ permalink raw reply	[flat|nested] 47+ messages in thread

* Re: [PATCH v2 14/14] test: dm: add test for spawn and wait commands
  2025-02-27 17:12     ` Jerome Forissier
@ 2025-03-04 13:14       ` Simon Glass
  0 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2025-03-04 13:14 UTC (permalink / raw)
  To: Jerome Forissier; +Cc: u-boot, Ilias Apalodimas, Tom Rini, Heinrich Schuchardt

Hi Jerome,

On Thu, 27 Feb 2025 at 10:12, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> Hi Simon,
>
> On 2/27/25 17:25, Simon Glass wrote:
> > Hi Jerome,
> >
> > On Tue, 25 Feb 2025 at 09:35, Jerome Forissier
> > <jerome.forissier@linaro.org> wrote:
> >>
> >> Test the spawn and wait commands.
> >>
> >> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> >> ---
> >>  test/cmd/Makefile |  1 +
> >>  test/cmd/spawn.c  | 33 +++++++++++++++++++++++++++++++++
> >>  2 files changed, 34 insertions(+)
> >>  create mode 100644 test/cmd/spawn.c
> >>
> >> diff --git a/test/cmd/Makefile b/test/cmd/Makefile
> >> index d8a5e77402d..cf47f04851c 100644
> >> --- a/test/cmd/Makefile
> >> +++ b/test/cmd/Makefile
> >> @@ -39,3 +39,4 @@ obj-$(CONFIG_CMD_WGET) += wget.o
> >>  endif
> >>  obj-$(CONFIG_ARM_FFA_TRANSPORT) += armffa.o
> >>  endif
> >> +obj-$(CONFIG_CMD_SPAWN) += spawn.o
> >> diff --git a/test/cmd/spawn.c b/test/cmd/spawn.c
> >> new file mode 100644
> >> index 00000000000..5e0770858c0
> >> --- /dev/null
> >> +++ b/test/cmd/spawn.c
> >> @@ -0,0 +1,33 @@
> >> +// SPDX-License-Identifier: GPL-2.0-or-later
> >> +/*
> >> + * Tests for spawn and wait commands
> >> + *
> >> + * Copyright 2025, Linaro Ltd.
> >> + */
> >> +
> >> +#include <command.h>
> >> +#include <dm.h>
> >> +#include <dm/test.h>
> >> +#include <test/test.h>
> >> +#include <test/ut.h>
> >> +
> >> +static int dm_test_cmd_spawn(struct unit_test_state *uts)
> >> +{
> >> +       ut_assertok(run_command("wait; spawn sleep 2; setenv j ${job_id}; "
> >> +                               "spawn setenv spawned true; "
> >> +                               "setenv jj ${job_id}; wait; "
> >> +                               "echo ${j} ${jj} ${spawned}", 0));
> >> +       console_record_readline(uts->actual_str, sizeof(uts->actual_str));
> >> +       ut_asserteq_ptr(uts->actual_str,
> >> +                       strstr(uts->actual_str, "1 2 true"));
> >> +
> >> +       ut_assertok(run_command("spawn true; wait; setenv t $?; spawn false; "
> >> +                               "wait; setenv f $?; wait; echo $t $f $?", 0));
> >> +       console_record_readline(uts->actual_str, sizeof(uts->actual_str));
> >> +       ut_asserteq_ptr(uts->actual_str,
> >> +                       strstr(uts->actual_str, "0 1 0"));
> >> +       ut_assert_console_end();
> >> +
> >> +       return 0;
> >> +}
> >> +DM_TEST(dm_test_cmd_spawn, UTF_CONSOLE);
> >> --
> >> 2.43.0
> >>
> >
> > This doesn't look like a DM test as it doesn't use devices. Could it
> > be a CMD test?
>
> Absolutely. In fact I started from a copy of test/cmd/hash.c, which is
> not a DM test either is it?

That's right. Since DM had the first tests nearly everything started
out as a DM test, so you could clean those up if you have time.

Regards,
SImon

^ permalink raw reply	[flat|nested] 47+ messages in thread

end of thread, other threads:[~2025-03-04 13:14 UTC | newest]

Thread overview: 47+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-25 16:34 [PATCH v2 00/14] Uthreads Jerome Forissier
2025-02-25 16:34 ` [PATCH v2 01/14] arch: introduce symbol HAVE_INITJMP Jerome Forissier
2025-02-28 12:01   ` Ilias Apalodimas
2025-02-28 12:38   ` Heinrich Schuchardt
2025-02-28 12:57     ` Jerome Forissier
2025-02-25 16:34 ` [PATCH v2 02/14] arm: add initjmp() Jerome Forissier
2025-02-28 12:01   ` Ilias Apalodimas
2025-02-28 13:05   ` Heinrich Schuchardt
2025-02-28 13:21     ` Jerome Forissier
2025-02-28 13:28       ` Heinrich Schuchardt
2025-02-28 14:16         ` Jerome Forissier
2025-02-28 17:54           ` Tom Rini
2025-02-25 16:34 ` [PATCH v2 03/14] riscv: " Jerome Forissier
2025-02-25 16:34 ` [PATCH v2 04/14] sandbox: " Jerome Forissier
2025-02-25 16:34 ` [PATCH v2 05/14] test: lib: add initjmp() test Jerome Forissier
2025-02-28 12:38   ` Ilias Apalodimas
2025-02-28 13:04   ` Heinrich Schuchardt
2025-02-28 13:09     ` Jerome Forissier
2025-02-25 16:34 ` [PATCH v2 06/14] uthread: add cooperative multi-tasking interface Jerome Forissier
2025-02-28 13:09   ` Ilias Apalodimas
2025-02-28 14:30     ` Jerome Forissier
2025-02-28 13:21   ` Heinrich Schuchardt
2025-02-28 14:39     ` Jerome Forissier
2025-02-25 16:34 ` [PATCH v2 07/14] cyclic: invoke uthread_schedule() from schedule() Jerome Forissier
2025-02-27 12:30   ` Stefan Roese
2025-02-27 17:05     ` Jerome Forissier
2025-02-28 15:43       ` Stefan Roese
2025-02-28 13:22   ` Ilias Apalodimas
2025-02-25 16:34 ` [PATCH v2 08/14] lib: time: hook uthread_schedule() into udelay() Jerome Forissier
2025-02-28 13:38   ` Ilias Apalodimas
2025-02-28 14:16     ` Yao Zi
2025-02-28 14:45       ` Jerome Forissier
2025-02-28 18:16       ` Ilias Apalodimas
2025-02-25 16:34 ` [PATCH v2 09/14] doc: develop: add documentation for uthreads Jerome Forissier
2025-02-25 16:34 ` [PATCH v2 10/14] test: lib: add uthread test Jerome Forissier
2025-02-28 13:26   ` Ilias Apalodimas
2025-02-25 16:34 ` [PATCH v2 11/14] dm: usb: move bus initialization into new static function usb_init_bus() Jerome Forissier
2025-02-28 13:27   ` Ilias Apalodimas
2025-02-25 16:34 ` [PATCH v2 12/14] dm: usb: initialize and scan multiple buses simultaneously with uthread Jerome Forissier
2025-02-27 16:25   ` Simon Glass
2025-02-27 17:30     ` Jerome Forissier
2025-03-04 13:13       ` Simon Glass
2025-02-25 16:34 ` [PATCH v2 13/14] cmd: add spawn and wait commands Jerome Forissier
2025-02-25 16:34 ` [PATCH v2 14/14] test: dm: add test for " Jerome Forissier
2025-02-27 16:25   ` Simon Glass
2025-02-27 17:12     ` Jerome Forissier
2025-03-04 13:14       ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.