Hi Stefan, On Mon, 1 Aug 2022 at 08:09, Stefan Roese <s...@denx.de> wrote: > > 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...
It depends on whether cyclic is a small adder to events or something larger. Actually events are not too big, but having 'dynamic' ones (allocated at runtime) does add a bit more. > > >> ? > >> > >> 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. OK, sounds good. You are writing the code :-) Regards, Simon
