From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EBB41C4338F for ; Tue, 3 Aug 2021 08:28:52 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B2C9760249 for ; Tue, 3 Aug 2021 08:28:51 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org B2C9760249 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 5340682D6F; Tue, 3 Aug 2021 10:28:49 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1627979329; bh=Hv6yJWECZFxTV7947SEf0/vydex5bA4taGCK6j9Ihdw=; h=Subject:To:Cc:References:From:Date:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=EbqlHtfWbPLdc0ufce51+NDTHIfC0UyyoSP+97j3ktu3FMsqCxD72ojmiZricIH2m 1R4Q696ASYIYdZlHg0wTDFhClct/JalCn+okTsnXEGR2ZjHunku98vIn80ImdVC9j+ 1Lspd8nboxsL1gwpwvTZLffaT96e+gsn1Kw//cBBuruvGhblcgxvY1aJ1JLZiDjg3Z uNySs1lVBO7PHLG9vo0RvPQBwQIZYitCbgVHZu4eOBpqWso4Nd/cyp3gv9hGxy21m1 6ArQVC7lhY+b+3TnnjlNgr0aNndZYcBpT7wlSY/vRN93YjuEFdtAAb13rjjFuqOF+i Xz5EKRTxltWNQ== Received: by phobos.denx.de (Postfix, from userid 109) id 8DA2782D9C; Tue, 3 Aug 2021 10:28:47 +0200 (CEST) Received: from mout-u-205.mailbox.org (mout-u-205.mailbox.org [IPv6:2001:67c:2050:1::465:205]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 8437782D52 for ; Tue, 3 Aug 2021 10:28:44 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=fail smtp.mailfrom=sr@denx.de Received: from smtp1.mailbox.org (smtp1.mailbox.org [IPv6:2001:67c:2050:105:465:1:1:0]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-u-205.mailbox.org (Postfix) with ESMTPS id 4Gf7Lh1Sn8zQkFj; Tue, 3 Aug 2021 10:28:44 +0200 (CEST) Received: from smtp1.mailbox.org ([80.241.60.240]) by spamfilter05.heinlein-hosting.de (spamfilter05.heinlein-hosting.de [80.241.56.123]) (amavisd-new, port 10030) with ESMTP id 5I6CmbtbD6yb; Tue, 3 Aug 2021 10:28:41 +0200 (CEST) Subject: Re: [PATCH v4 07/10] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset() To: Rasmus Villemoes , u-boot@lists.denx.de Cc: Simon Glass , Tom Rini References: <20210802150016.588750-1-rasmus.villemoes@prevas.dk> <20210802150016.588750-8-rasmus.villemoes@prevas.dk> From: Stefan Roese Message-ID: Date: Tue, 3 Aug 2021 10:28:40 +0200 MIME-Version: 1.0 In-Reply-To: <20210802150016.588750-8-rasmus.villemoes@prevas.dk> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: de-DE Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 1387F18BD X-Rspamd-UID: 631f9d X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Hi Rasmus, On 02.08.21 17:00, Rasmus Villemoes wrote: > 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, and call the init_watchdog_dev helper for each. > > 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. > > Signed-off-by: Rasmus Villemoes > --- > drivers/watchdog/wdt-uclass.c | 56 ++++++++++++++++++++----------- > include/asm-generic/global_data.h | 6 ---- > 2 files changed, 36 insertions(+), 26 deletions(-) > > diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c > index 358fc68e27..0ce8b3a425 100644 > --- a/drivers/watchdog/wdt-uclass.c > +++ b/drivers/watchdog/wdt-uclass.c > @@ -61,20 +61,24 @@ static void init_watchdog_dev(struct udevice *dev) > > 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; > + struct udevice *dev; > + struct uclass *uc; > + int ret; > + > + ret = uclass_get(UCLASS_WDT, &uc); > + if (ret) { > + log_debug("Error getting UCLASS_WDT: %d\n", ret); > + return 0; > + } > + > + uclass_foreach_dev(dev, uc) { > + ret = device_probe(dev); > + if (ret) { > + log_debug("Error probing %s: %d\n", dev->name, ret); > + continue; > } > + init_watchdog_dev(dev); > } > - init_watchdog_dev(gd->watchdog_dev); > > gd->flags |= GD_FLG_WDT_READY; > return 0; > @@ -157,22 +161,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 > diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h > index e55070303f..28d749538c 100644 > --- a/include/asm-generic/global_data.h > +++ b/include/asm-generic/global_data.h > @@ -447,12 +447,6 @@ struct global_data { > */ > fdt_addr_t translation_offset; > #endif > -#if CONFIG_IS_ENABLED(WDT) > - /** > - * @watchdog_dev: watchdog device > - */ > - struct udevice *watchdog_dev; > -#endif After applying the patchset v4 to current master, I see this error when building for "x530": board/alliedtelesis/x530/x530.c: In function 'arch_preboot_os': board/alliedtelesis/x530/x530.c:125:13: error: 'gd_t' {aka 'volatile struct global_data'} has no member named 'watchdog_dev' 125 | wdt_stop(gd->watchdog_dev); | ^~ make[1]: *** [scripts/Makefile.build:254: board/alliedtelesis/x530/x530.o] Error 1 Perhaps we need a common function now to stop all watchdogs, which can be called from such places? Thanks, Stefan