All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
To: u-boot@lists.denx.de
Cc: Simon Glass <sjg@chromium.org>, Stefan Roese <sr@denx.de>,
	Tom Rini <trini@konsulko.com>,
	Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Subject: [PATCH v3 07/10] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset()
Date: Fri,  2 Jul 2021 14:45:07 +0200	[thread overview]
Message-ID: <20210702124510.124401-8-rasmus.villemoes@prevas.dk> (raw)
In-Reply-To: <20210702124510.124401-1-rasmus.villemoes@prevas.dk>

A board can have and make use of more than one watchdog device, say
one built into the SOC and an external gpio-petted one. Having
wdt-uclass only handle the first is both a little arbitrary and
unexpected.

So change initr_watchdog() so we visit (probe) all DM watchdog
devices. This essentially boils down to making the init_watchdog_dev
function into a .post_probe method.

Similarly let watchdog_reset() loop over the whole uclass - each
having their own ratelimiting metadata, and a separate "is this device
running" flag.

This gets rid of the watchdog_dev member of struct global_data.  We
do, however, still need the GD_FLG_WDT_READY set in
initr_watchdog(). This is because watchdog_reset() can get called
before DM is ready, and I don't think we can call uclass_get() that
early.

The current code just returns 0 if "getting" the first device fails -
that can of course happen because there are no devices, but it could
also happen if its ->probe call failed. In keeping with that, continue
with the handling of the remaining devices even if one fails to
probe. This is also why we cannot use uclass_probe_all().

If desired, it's possible to later add a per-device "u-boot,autostart"
boolean property, so that one can do CONFIG_WATCHDOG_AUTOSTART
per-device.

Reviewed-by: Stefan Roese <sr@denx.de>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/watchdog/wdt-uclass.c     | 96 ++++++++++++++++++-------------
 include/asm-generic/global_data.h |  6 --
 2 files changed, 56 insertions(+), 46 deletions(-)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index ca20c124d4..0972541148 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -35,44 +35,23 @@ struct wdt_priv {
 	bool running;
 };
 
-static void init_watchdog_dev(struct udevice *dev)
+int initr_watchdog(void)
 {
-	struct wdt_priv *priv;
+	struct udevice *dev;
+	struct uclass *uc;
 	int ret;
 
-	priv = dev_get_uclass_priv(dev);
-
-	if (!CONFIG_IS_ENABLED(WATCHDOG_AUTOSTART)) {
-		printf("WDT:   Not starting %s\n", dev->name);
-		return;
-	}
-
-	ret = wdt_start(dev, priv->timeout * 1000, 0);
-	if (ret != 0) {
-		printf("WDT:   Failed to start %s\n", dev->name);
-		return;
+	ret = uclass_get(UCLASS_WDT, &uc);
+	if (ret) {
+		log_debug("Error getting UCLASS_WDT: %d\n", ret);
+		return 0;
 	}
 
-	printf("WDT:   Started %s with%s servicing (%ds timeout)\n", dev->name,
-	       IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", priv->timeout);
-}
-
-int initr_watchdog(void)
-{
-	/*
-	 * Init watchdog: This will call the probe function of the
-	 * watchdog driver, enabling the use of the device
-	 */
-	if (uclass_get_device_by_seq(UCLASS_WDT, 0,
-				     (struct udevice **)&gd->watchdog_dev)) {
-		debug("WDT:   Not found by seq!\n");
-		if (uclass_get_device(UCLASS_WDT, 0,
-				      (struct udevice **)&gd->watchdog_dev)) {
-			printf("WDT:   Not found!\n");
-			return 0;
-		}
+	uclass_foreach_dev(dev, uc) {
+		ret = device_probe(dev);
+		if (ret)
+			log_debug("Error probing %s: %d\n", dev->name, ret);
 	}
-	init_watchdog_dev(gd->watchdog_dev);
 
 	gd->flags |= GD_FLG_WDT_READY;
 	return 0;
@@ -155,22 +134,34 @@ void watchdog_reset(void)
 {
 	struct wdt_priv *priv;
 	struct udevice *dev;
+	struct uclass *uc;
 	ulong now;
 
 	/* Exit if GD is not ready or watchdog is not initialized yet */
 	if (!gd || !(gd->flags & GD_FLG_WDT_READY))
 		return;
 
-	dev = gd->watchdog_dev;
-	priv = dev_get_uclass_priv(dev);
-	if (!priv->running)
+	if (uclass_get(UCLASS_WDT, &uc))
 		return;
 
-	/* Do not reset the watchdog too often */
-	now = get_timer(0);
-	if (time_after_eq(now, priv->next_reset)) {
-		priv->next_reset = now + priv->reset_period;
-		wdt_reset(dev);
+	/*
+	 * All devices bound to the wdt uclass should have been probed
+	 * in initr_watchdog(). But just in case something went wrong,
+	 * check device_active() before accessing the uclass private
+	 * data.
+	 */
+	uclass_foreach_dev(dev, uc) {
+		if (!device_active(dev))
+			continue;
+		priv = dev_get_uclass_priv(dev);
+		if (!priv->running)
+			continue;
+		/* Do not reset the watchdog too often */
+		now = get_timer(0);
+		if (time_after_eq(now, priv->next_reset)) {
+			priv->next_reset = now + priv->reset_period;
+			wdt_reset(dev);
+		}
 	}
 }
 #endif
@@ -224,11 +215,36 @@ static int wdt_pre_probe(struct udevice *dev)
 	return 0;
 }
 
+static int wdt_post_probe(struct udevice *dev)
+{
+	struct wdt_priv *priv;
+	int ret;
+
+	priv = dev_get_uclass_priv(dev);
+
+	if (!CONFIG_IS_ENABLED(WATCHDOG_AUTOSTART)) {
+		printf("WDT:   Not starting %s\n", dev->name);
+		return 0;
+	}
+
+	ret = wdt_start(dev, priv->timeout * 1000, 0);
+	if (ret != 0) {
+		printf("WDT:   Failed to start %s\n", dev->name);
+		return 0;
+	}
+
+	printf("WDT:   Started %s with%s servicing (%ds timeout)\n", dev->name,
+	       IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", priv->timeout);
+
+	return 0;
+}
+
 UCLASS_DRIVER(wdt) = {
 	.id			= UCLASS_WDT,
 	.name			= "watchdog",
 	.flags			= DM_UC_FLAG_SEQ_ALIAS,
 	.post_bind		= wdt_post_bind,
 	.pre_probe		= wdt_pre_probe,
+	.post_probe		= wdt_post_probe,
 	.per_device_auto	= sizeof(struct wdt_priv),
 };
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index 47921d27b1..b554016628 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -445,12 +445,6 @@ struct global_data {
 	 */
 	fdt_addr_t translation_offset;
 #endif
-#if CONFIG_IS_ENABLED(WDT)
-	/**
-	 * @watchdog_dev: watchdog device
-	 */
-	struct udevice *watchdog_dev;
-#endif
 #ifdef CONFIG_GENERATE_ACPI_TABLE
 	/**
 	 * @acpi_ctx: ACPI context pointer
-- 
2.31.1


  parent reply	other threads:[~2021-07-02 12:46 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-02 12:45 [PATCH v3 00/10] handling all DM watchdogs in watchdog_reset() Rasmus Villemoes
2021-07-02 12:45 ` [PATCH v3 01/10] watchdog: wdt-uclass.c: use wdt_start() in wdt_expire_now() Rasmus Villemoes
2021-07-02 12:45 ` [PATCH v3 02/10] watchdog: wdt-uclass.c: introduce struct wdt_priv Rasmus Villemoes
2021-07-05  7:17   ` Stefan Roese
2021-07-02 12:45 ` [PATCH v3 03/10] watchdog: wdt-uclass.c: neaten UCLASS_DRIVER definition Rasmus Villemoes
2021-07-02 12:45 ` [PATCH v3 04/10] watchdog: wdt-uclass.c: refactor initr_watchdog() Rasmus Villemoes
2021-07-02 12:45 ` [PATCH v3 05/10] watchdog: wdt-uclass.c: keep track of each device's running state Rasmus Villemoes
2021-07-02 12:45 ` [PATCH v3 06/10] sandbox: disable CONFIG_WATCHDOG_AUTOSTART Rasmus Villemoes
2021-07-02 12:45 ` Rasmus Villemoes [this message]
2021-07-05 15:30   ` [PATCH v3 07/10] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset() Simon Glass
2021-07-15  8:15     ` Stefan Roese
2021-07-31 10:06       ` Stefan Roese
2021-08-02  9:18         ` Rasmus Villemoes
2021-08-02 10:39           ` Stefan Roese
2021-08-02 19:20           ` Simon Glass
2021-08-03  6:48             ` Rasmus Villemoes
2021-08-04 14:35               ` Simon Glass
2021-07-02 12:45 ` [PATCH v3 08/10] watchdog: add gpio watchdog driver Rasmus Villemoes
2021-07-02 12:45 ` [PATCH v3 09/10] sandbox: add test of wdt_gpio driver Rasmus Villemoes
2021-07-02 12:45 ` [PATCH v3 10/10] sandbox: add test of wdt-uclass' watchdog_reset() Rasmus Villemoes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210702124510.124401-8-rasmus.villemoes@prevas.dk \
    --to=rasmus.villemoes@prevas.dk \
    --cc=sjg@chromium.org \
    --cc=sr@denx.de \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.