Hi Simon,
On 01.08.22 15:00, Simon Glass wrote:
Hi Stefan,
On Mon, 1 Aug 2022 at 06:40, Stefan Roese <s...@denx.de> wrote:
Hi Simon,
On 01.08.22 14:22, Simon Glass wrote:
Hi Stefan,
On Mon, 1 Aug 2022 at 01:17, Stefan Roese <s...@denx.de> wrote:
Hi Simon,
On 31.07.22 03:27, Simon Glass wrote:
Hi Stefan,
On Thu, 28 Jul 2022 at 01:09, Stefan Roese <s...@denx.de> wrote:
This patchset adds the basic infrastructure to periodically execute
code, e.g. all 100ms. Examples for such functions might be LED blinking
etc. The functions that are hooked into this cyclic list should be
small timewise as otherwise the execution of the other code that relies
on a high frequent polling (e.g. UART rx char ready check) might be
delayed too much. This patch also adds the Kconfig option
CONFIG_CYCLIC_MAX_CPU_TIME_US, which configures the max allowed time
for such a cyclic function. If it's execution time exceeds this time,
this cyclic function will get removed from the cyclic list.
How is this cyclic functionality executed?
This patchset integrates the main function responsible for calling all
registered cyclic functions cyclic_run() into the common WATCHDOG_RESET
macro. This guarantees that cyclic_run() is executed very often, which
is necessary for the cyclic functions to get scheduled and executed at
their configured periods.
This cyclic infrastructure will be used by a board specific function on
the NIC23 MIPS Octeon board, which needs to check periodically, if a
PCIe FLR has occurred.
Ideas how to continue:
One idea is to rename WATCHDOG_RESET to something like SCHEDULE and
move the watchdog_reset call into this cyclic infrastructure as well.
Or to perhaps move the shell UART RX ready polling to a cyclic
function.
It's also possible to extend the "cyclic" command, to support the
creation of periodically executed shell commands (for testing etc).
Here the Azure build, without any issues:
https://dev.azure.com/sr0718/u-boot/_build/results?buildId=219&view=results
Thanks,
Stefan
Aaron Williams (1):
mips: octeon_nic23: Add PCIe FLR fixup via cyclic infrastructure
Stefan Roese (6):
time: Import time_after64() and friends from Linux
cyclic: Add basic support for cyclic function execution infrastruture
cyclic: Integrate cyclic infrastructure into WATCHDOG_RESET
cyclic: Integrate cyclic functionality at bootup in board_r/f
cyclic: Add 'cyclic list' command
sandbox: Add cyclic demo function
MAINTAINERS | 7 +
board/Marvell/octeon_nic23/board.c | 197 +++++++++++++++++++++++++++++
board/sandbox/sandbox.c | 15 +++
cmd/Kconfig | 7 +
cmd/Makefile | 1 +
cmd/cyclic.c | 40 ++++++
common/Kconfig | 20 +++
common/Makefile | 1 +
common/board_f.c | 2 +
common/board_r.c | 2 +
common/cyclic.c | 112 ++++++++++++++++
configs/octeon_nic23_defconfig | 3 +
configs/sandbox_defconfig | 3 +
fs/cramfs/uncompress.c | 2 +-
include/cyclic.h | 97 ++++++++++++++
include/time.h | 19 +++
include/watchdog.h | 23 +++-
17 files changed, 547 insertions(+), 4 deletions(-)
create mode 100644 cmd/cyclic.c
create mode 100644 common/cyclic.c
create mode 100644 include/cyclic.h
--
2.37.1
This looks interesting. I like the idea of integrating the watchdog
stuff too, since you are making use of it.
Would it make sense to make use of the new event system, since it has
static and dynamic handlers?
IIRC, I tried to make use of this infrastructure but it did not work
out. Or do you see some way to integrate this cyclic IF into the
event system? FWICT, the only way to get this done is to make use of
the (ugly) WATCHDOG_RESET calls.
Yes that makes sense. I don't see another way.
But within that, one option would be to send an event every time
WATCHDOG_RESET is used, and have things register as an event spy, thus
making use of that existing system. The event feature is not enabled
by default, but it could be enabled when this functionality is needed.
I still need to double-check, sorry: You are thinking about this call-
trace:
WATCHDOG_RESET -> event IF -> cylic IF -> cylic user
More this, i.e. I am wondering if the cyclic functionality can be done
using events.
WATCHDOG_RESET -> sends watchdog event -> event spy receives event
This would mean in the end, that all U-Boot targets would need to
enable the event subsystem as well. This might lead to problems with
image sizes on some of those especially old/ancient targets. Hmmm...
?
If yes, what would be the advantage(s) against implementing this
separately?
It would need handling of max CPU time and perhaps some other tweaks,
but it would result in less code (?) and one less thing for people to
learn. The cyclic command could still be provided if that is useful,
by showing only cyclic certain event handlers.
Frankly, I'm not 100% convinced that this would really result in less
code. Or even worse, it could (?) result in more complex code, as both
interfaces are in the same area but still handle different tasks,
e.g. synchronous vs. asynchronous.
So my preference would be to continue on my current path. I'll integrate
some documentation in my next patchset version and will resubmit
hopefully sometime later this week.
Thanks,
Stefan
