cyclic: make clients embed a struct cyclic_info in their own data structure
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 <sr@denx.de>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 1285001..e2e7f9a 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -17,12 +17,15 @@
#include <asm/global_data.h>
#include <dm/device-internal.h>
#include <dm/lists.h>
+#include <linux/kernel.h>
DECLARE_GLOBAL_DATA_PTR;
#define WATCHDOG_TIMEOUT_SECS (CONFIG_WATCHDOG_TIMEOUT_MSECS / 1000)
struct wdt_priv {
+ /* The udevice owning this wdt_priv. */
+ struct udevice *dev;
/* Timeout, in seconds, to configure this device to. */
u32 timeout;
/*
@@ -40,18 +43,17 @@
/* autostart */
bool autostart;
- struct cyclic_info *cyclic;
+ struct cyclic_info cyclic;
};
-static void wdt_cyclic(void *ctx)
+static void wdt_cyclic(struct cyclic_info *c)
{
- struct udevice *dev = ctx;
- struct wdt_priv *priv;
+ struct wdt_priv *priv = container_of(c, struct wdt_priv, cyclic);
+ struct udevice *dev = priv->dev;
if (!device_active(dev))
return;
- priv = dev_get_uclass_priv(dev);
if (!priv->running)
return;
@@ -124,20 +126,14 @@
memset(str, 0, 16);
if (IS_ENABLED(CONFIG_WATCHDOG)) {
if (priv->running)
- cyclic_unregister(priv->cyclic);
+ cyclic_unregister(&priv->cyclic);
/* Register the watchdog driver as a cyclic function */
- priv->cyclic = cyclic_register(wdt_cyclic,
- priv->reset_period * 1000,
- dev->name, dev);
- if (!priv->cyclic) {
- printf("cyclic_register for %s failed\n",
- dev->name);
- return -ENODEV;
- } else {
- snprintf(str, 16, "every %ldms",
- priv->reset_period);
- }
+ cyclic_register(&priv->cyclic, wdt_cyclic,
+ priv->reset_period * 1000,
+ dev->name);
+
+ snprintf(str, 16, "every %ldms", priv->reset_period);
}
priv->running = true;
@@ -162,7 +158,7 @@
struct wdt_priv *priv = dev_get_uclass_priv(dev);
if (IS_ENABLED(CONFIG_WATCHDOG) && priv->running)
- cyclic_unregister(priv->cyclic);
+ cyclic_unregister(&priv->cyclic);
priv->running = false;
}
@@ -262,6 +258,7 @@
autostart = true;
}
priv = dev_get_uclass_priv(dev);
+ priv->dev = dev;
priv->timeout = timeout;
priv->reset_period = reset_period;
priv->autostart = autostart;