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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 91D02C2BB41 for ; Tue, 16 Aug 2022 10:10:34 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 352F0848B7; Tue, 16 Aug 2022 12:10:32 +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=1660644632; bh=9Ol6Cn4e57ure7aFLFhZRPs+vbRZ5vQgvrFmg9tolaE=; h=Date:Subject:To:Cc:References:From:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=H2huO8eax25Tj1z+WvviZ1BH/wyiOcEC24X+s88iss7bdmFoF4qx/5LoaXtzrVXvI rtf1i8RyGApl72jbOlYcDrvuvZTPJlB0l+ZhxM7sJKvPSki7svkiwDxG9I8ZWqVPv0 1ClcZ1TyKOMrbLq97qLoarbPA8zPDWpjhxsfRcF+TTNgMbuhKT6Rl47D1nLf26cPvv 3t5/C+x/4TaJKtBCv5XQOjLlGzpNbCGyGZQNeIc0N24KJZhWnX7cFI3hXWuLMX+tH/ YaRC6Nn9SK+Y4n6L1E4M32SEyxQ7+kUnukkbF6OvQvwcciTpDuRayWyz4XNs3IlIKf yF/zHbU45g2DQ== Received: by phobos.denx.de (Postfix, from userid 109) id 73E05848B7; Tue, 16 Aug 2022 12:10:30 +0200 (CEST) Received: from mout-u-107.mailbox.org (mout-u-107.mailbox.org [IPv6:2001:67c:2050:101:465::107]) (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 0AF968412C for ; Tue, 16 Aug 2022 12:10:28 +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 smtp202.mailbox.org (smtp202.mailbox.org [10.196.197.202]) (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-107.mailbox.org (Postfix) with ESMTPS id 4M6RjJ6bHMz9sNY; Tue, 16 Aug 2022 12:10:12 +0200 (CEST) Message-ID: <02dde535-d628-2d56-4b7e-b9551b432ae6@denx.de> Date: Tue, 16 Aug 2022 12:10:11 +0200 MIME-Version: 1.0 Subject: Re: [PATCH v3 3/8] cyclic: Integrate cyclic infrastructure into WATCHDOG_RESET Content-Language: en-US To: Simon Glass Cc: U-Boot Mailing List , Tom Rini , Chandrakala Chavva , Aaron Williams References: <20220805142610.375427-1-sr@denx.de> <20220805142610.375427-4-sr@denx.de> From: Stefan Roese In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 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.6 at phobos.denx.de X-Virus-Status: Clean Hi Simon, On 15.08.22 19:37, Simon Glass wrote: > Hi Stefan, > > On Mon, 15 Aug 2022 at 10:16, Stefan Roese wrote: >> >> Hi Simon, >> >> On 05.08.22 18:48, Simon Glass wrote: >>> Hi Stefan, >>> >>> On Fri, 5 Aug 2022 at 08:26, Stefan Roese wrote: >>>> >>>> This patch 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. >>>> >>>> If CONFIG_WATCHDOG is not enabled, only cyclic_run() without calling >>>> watchdog_reset(). This guarantees that the cyclic functionality does not >>>> rely on CONFIG_WATCHDOG being enabled. >>>> >>>> Signed-off-by: Stefan Roese >>>> --- >>>> v3: >>>> - No change >>>> v2: >>>> - No change >>>> >>>> fs/cramfs/uncompress.c | 2 +- >>>> include/watchdog.h | 23 ++++++++++++++++++++--- >>>> 2 files changed, 21 insertions(+), 4 deletions(-) >>> >>> Reviewed-by: Simon Glass >>> >>>> >>>> diff --git a/fs/cramfs/uncompress.c b/fs/cramfs/uncompress.c >>>> index f431cc46c1f7..38e10e2e4422 100644 >>>> --- a/fs/cramfs/uncompress.c >>>> +++ b/fs/cramfs/uncompress.c >>>> @@ -62,7 +62,7 @@ int cramfs_uncompress_init (void) >>>> stream.avail_in = 0; >>>> >>>> #if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) >>>> - stream.outcb = (cb_func) WATCHDOG_RESET; >>>> + stream.outcb = (cb_func)watchdog_reset_func; >>>> #else >>>> stream.outcb = Z_NULL; >>>> #endif /* CONFIG_HW_WATCHDOG */ >>>> diff --git a/include/watchdog.h b/include/watchdog.h >>>> index 813cc8f2a5d3..0a9777edcbad 100644 >>>> --- a/include/watchdog.h >>>> +++ b/include/watchdog.h >>>> @@ -11,6 +11,8 @@ >>>> #define _WATCHDOG_H_ >>>> >>>> #if !defined(__ASSEMBLY__) >>>> +#include >>>> + >>>> /* >>>> * Reset the watchdog timer, always returns 0 >>>> * >>>> @@ -60,11 +62,16 @@ int init_func_watchdog_reset(void); >>>> /* Don't require the watchdog to be enabled in SPL */ >>>> #if defined(CONFIG_SPL_BUILD) && \ >>>> !defined(CONFIG_SPL_WATCHDOG) >>>> - #define WATCHDOG_RESET() {} >>>> + #define WATCHDOG_RESET() { \ >>>> + cyclic_run(); \ >>>> + } >>>> #else >>>> extern void watchdog_reset(void); >>>> >>>> - #define WATCHDOG_RESET watchdog_reset >>>> + #define WATCHDOG_RESET() { \ >>>> + watchdog_reset(); \ >>>> + cyclic_run(); \ >>> >>> Doesn't this create two function calls from every reset site? I worry >>> about code size. >> >> Good point. Even though the world build did not trigger any new problems >> with oversized images. >> >>> I suggest creating a new function like >>> check_watchdog() which you can define (in the C file) with >>> IS_ENABLED() as either empty, calling watchdog_reset() and/or calling >>> cyclic_run(). LTO should help here. >> >> I tried a bit to get clean up this ugly #ifdef construct. And move this >> as you suggested into a C file. Just now I noticed, that we don't have >> a matching C file, which is compiled in all cases. wdt-uclass.c and >> cyclic.c are both only compiled when actually enabled in Kconfig. >> >> So do you have some other / better idea on how to improve this? >> >> BTW: Moving the watchdog integration into the cyclic infrastructure in >> some follow-up patches will make all this much cleaner AFAICT. > > If you are going to do this soon, then I suggest not working about it too much. > > But you could create wdt_common.c or similar. It should be OK to > always compile it since the code will be dropped if nothing calls it. Yes, this would be one potential intermediate "solution". I'll try to work on the WDT migration to cyclic IF soon and will keep the code in this header as-is for now. Thanks, Stefan