u-boot.git/common/cyclic.c, branch next

cyclic: make cyclic_register safe to call on already-registered info

2025-05-16T11:44:19+00:00

Now that cyclic_unregister() is safe to call on a not-registered
cyclic_info, we can make cyclic_register() behave like the mod_timer()
and hrtimer_start() APIs in linux, in that they don't distinguish
between whether the timer was already enabled or not; from the point
of the call it is, with whatever timeout/period is set in that most
recent call.

This avoids users of the cyclic API from separately keeping track of
whether their callback is already registered or not, and even if they
know it is, can be used for changing the period (and/or the callback
function) without first doing unregister().

See also this recent'ish message from kernel maintainer Thomas
Gleixner on that API design for timer frameworks:

  https://lore.kernel.org/lkml/87ikn6sibi.ffs@tglx/

  First of all the question is whether add() and mod() are really
  valuable distinctions. I'm not convinced at all. Back then, when we
  introduced hrtimers, we came to the conclusion that hrtimer_start()
  is sufficient.

Signed-off-by: Rasmus Villemoes 
Reviewed-by: Stefan Roese

cyclic: make cyclic_unregister() idempotent

2025-05-16T11:44:19+00:00

Make cyclic_unregister() safe to call with an already unregistered, or
possibly never registered, struct cyclic_info. This is similar to how
the various timer APIs in the linux kernel work (they all allow
calling delete/cancel/... on an inactive timer object).

This means callers don't have to separately keep track of whether
their cyclic callback is registered or not, and avoids them trying to
peek into the struct cyclic_info for that information - which leads to
somewhat ugly code as it would have to be guarded by ifdef
CONFIG_CYCLIC.

Signed-off-by: Rasmus Villemoes 
Reviewed-by: Stefan Roese

cyclic: invoke uthread_schedule() from schedule()

2025-04-23T19:19:44+00:00

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 
Reviewed-by: Ilias Apalodimas 
Reviewed-by: Stefan Roese

cyclic: Fix rollover every 72 min on 32 bits platforms

2025-01-22T09:22:34+00:00

On 32 bits platforms, timer_get_us() returns an unsigned long which
is a 32 bits. timer_get_us() wraps around every 72 minutes
(2 ^ 32 / 1000000 =~ 4295 sec =~ 72 min).

So the test "if time_after_eq64(now, cyclic->next_call)" is no more
true when cyclic->next_call becomes above 32 bits max value (4294967295).

At this point after 72 min, no more cyclic function are
executed included watchdog one.

Instead of using timer_get_us(), use get_timer_us() which returns a
uint64_t, this allows a rollover every 584942 years.

Signed-off-by: Patrice Chotard 
Reviewed-by: Stefan Roese

cyclic: make cyclic_run static

2024-10-23T04:52:38+00:00

The only caller left is schedule(); everybody outside cyclic.c now
calls or references schedule().

Signed-off-by: Rasmus Villemoes 
Reviewed-by: Simon Glass 
Reviewed-by: Stefan Roese

cyclic: introduce u-boot/schedule.h

2024-10-23T04:52:38+00:00

I noticed an "unnecessary" include of  in
global_data.h, in the sense that nothing in cyclic.h is needed in
order to define 'struct global_data'.

Well, it's not unnecessary, as it implicitly ensures that everybody
gets a declaration of schedule(), and schedule() is (obviously) called
all over the tree. Almost none of those places directly include
, but for historical reasons, many do include
 (most schedule() instances are replacements of
WATCHDOG_RESET()).

However, very few TUs actually need the declarations of the
cyclic_register() and struct cyclic_info, and they also don't really
need anything from the watchdog.h header.

So introduce a new header which just contains a declaration of
schedule(), which can then be included from all the places that do
call schedule(). I removed the direct reference to cyclic_run(),
because we shouldn't have two public functions for doing roughly the
same without being very explicit about when one should call one or the
other.

Testing of later patches that explicitly include  when
schedule() is used revealed a problem with host tool build on win32,
which apparently picked up a host . To avoid that problem,
put the new header in include/u-boot/ and hence make the include
statements say .

Signed-off-by: Rasmus Villemoes 
Reviewed-by: Simon Glass 
Reviewed-by: Stefan Roese

cyclic: make clients embed a struct cyclic_info in their own data structure

2024-06-16T10:13:44+00:00

There are of course not a whole lot of examples in-tree yet, but
before they appear, let's make this API change: Instead of separately
allocating a 'struct cyclic_info', make the users embed such an
instance in their own structure, and make the convention that the
callback simply receives the 'struct cyclic_info *', from which the
clients can get their own data using the container_of() macro.

This has a number of advantages.

First, it means cyclic_register() simply cannot fail, simplifying the
code. The necessary storage will simply be allocated automatically
when the client's own structure is allocated (often via
uclass_priv_auto or similar).

Second, code for which CONFIG_CYCLIC is just an option can more easily
be written without #ifdefs, if we just provide an empty struct
cyclic_info {}. For example, the nested CONFIG_IS_ENABLED()s in
https://lore.kernel.org/u-boot/20240316201416.211480-1-marek.vasut+renesas@mailbox.org/
are mostly due to the existence of the 'struct cyclic_info *' member
being guarded by #ifdef CONFIG_CYCLIC.

And we do probably want to avoid the extra memory overhead of that
member when !CONFIG_CYCLIC. But that is automatic if, instead of a
'struct cyclic_info *', one simply embeds a 'struct cyclic_info',
which will have size 0 when !CONFIG_CYCLIC. Also, the no-op
cyclic_register() function can just unconditionally be called, and the
compiler will see that (1) the callback is referenced, so not emit a
warning for a maybe-unused function and (2) see that it can actually
never be reached, so not emit any code for it.

Reviewed-by: Stefan Roese 
Signed-off-by: Rasmus Villemoes

cyclic: stop strdup'ing name in cyclic_register()

2024-06-16T10:13:38+00:00

We are not checking the return value of strdup(), nor
freeing the string in cyclic_unregister().

However, all current users either pass a string literal or the
dev->name of the client device. So in all cases the name string will
live at least as long as the cyclic_info is registered, so just make
that a requirement.

Reviewed-by: Stefan Roese 
Signed-off-by: Rasmus Villemoes

cyclic: get rid of cyclic_init()

2022-11-02T07:42:03+00:00

Currently, we must call cyclic_init() at some point before
cyclic_register() becomes possible. That turns out to be somewhat
awkward, especially with SPL, and has resulted in a watchdog callback
not being registered, thus causing the board to prematurely reset.

We already rely on gd->cyclic reliably being set to NULL by the asm
code that clears all of gd. Now that the cyclic list is a hlist, and
thus an empty list is represented by a NULL head pointer, and struct
cyclic_drv has no other members, we can just as well drop a level of
indirection and put the hlist_head directly in struct
global_data. This doesn't increase the size of struct global_data,
gets rid of an early malloc(), and generates slightly smaller code.

But primarily, this avoids having to call cyclic_init() early; the cyclic
infrastructure is simply ready to register callbacks as soon as we
enter C code.

We can still end up with schedule() being called from asm very early,
so we still need to check that gd itself has been properly initialized
[*], but once it has, gd->cyclic_list is perfectly fine to access, and
will just be an empty list.

As for cyclic_uninit(), it was never really the opposite of
cyclic_init() since it didn't free the struct cyclic_drv nor set
gd->cyclic to NULL. Rename it to cyclic_unregister_all() and use that
in test/, and also insert a call at the end of the board_init_f
sequence so that gd->cyclic_list is a fresh empty list before we enter
board_init_r().

A small piece of ugliness is that I had to add a cast in
cyclic_get_list() to silence a "discards 'volatile' qualifier"
warning, but that is completely equivalent to the existing handling of
the uclass_root_s list_head member.

[*] I'm not really sure where we guarantee that the register used for
gd contains 0 until it gets explicitly initialized, but that must be
the case, otherwise testing gd for being NULL would not make much sense.

Signed-off-by: Rasmus Villemoes 
Reviewed-by: Stefan Roese 
Tested-by: Stefan Roese 
Tested-by: Tim Harvey  # imx8mm-venice-*

cyclic: switch to using hlist instead of list

2022-11-02T07:41:55+00:00

A hlist is headed by just a single pointer, so can only be traversed
forwards, and insertions can only happen at the head (or before/after
an existing list member). But each list node still consists of two
pointers, so arbitrary elements can still be removed in O(1).

This is precisely what we need for the cyclic_list - we never need to
traverse it backwards, and the order the callbacks appear in the list
should really not matter.

One advantage, and the main reason for doing this switch, is that an
empty list is represented by a NULL head pointer, so unlike a
list_head, it does not need separate C code to initialize - a
memset(,0,) of the containing structure is sufficient.

This is mostly mechanical:

- The iterators are updated with an h prefix, and the type of the
  temporary variable changed to struct hlist_node*.

- Adding/removing is now just hlist_add_head (and not tail) and
  hlist_del().

- struct members and function return values updated.

Signed-off-by: Rasmus Villemoes 
Reviewed-by: Stefan Roese 
Tested-by: Stefan Roese 
Tested-by: Tim Harvey  # imx8mm-venice-*