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 35F4DC00144 for ; Mon, 1 Aug 2022 13:01:10 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id D8F5784129; Mon, 1 Aug 2022 15:01:08 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="B/jUveog"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 6D35B8444D; Mon, 1 Aug 2022 15:01:07 +0200 (CEST) Received: from mail-ua1-x935.google.com (mail-ua1-x935.google.com [IPv6:2607:f8b0:4864:20::935]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 48BC68401B for ; Mon, 1 Aug 2022 15:01:04 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-ua1-x935.google.com with SMTP id s18so1932567uac.10 for ; Mon, 01 Aug 2022 06:01:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Iki3Z2nZZ38JCglK1B7nPkbwMZYHhl56KCLa4Ss1avI=; b=B/jUveogi1EFPrPA+L84ONRQAqBxylXcPnbkNwutBlHXXF6MvSUgUceDBswqpe3h/6 6hqSFSiZpi1DYwwrXwQdsH+VlSlthlMGeQrZh0FsHZx/6ggYoDfYcyypCzrGO8R8/dU7 UEwPXbqaFNkMLJBcpPLSfdRaxfrMJVa0UOs4I= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Iki3Z2nZZ38JCglK1B7nPkbwMZYHhl56KCLa4Ss1avI=; b=CQBGWN22X8t36iIWFiddZYsxlHTciUXJjHa6+xcflYHLuC9r/cZErVLrFs6R0X2Fp2 D19Zmk6n0Moj9MrPlK+VaDaKp3H1nT1eqwZ/8dUXPOqGWH1fjYuPtBbbsBs1TnTgMtR8 6J3rRj2psTGqskG3Eg/Dr4x7XG1SlRDMQL6wLdo/NPUXH38DN3n1j2G+ut4SHb24gvFh b7PYjPbq/Orf+GbUH8Ra6R3mWyUVwssOVYtuLQnL6CB/W2DHGGMWKGV8wCF0/W6hEKwl bmAgM/szdUnGV1pjHrmOagjXBsEHyCzIXd4ErgDdBb0G1xHZn7eI7By3iQ7iBp7lYGay rIIw== X-Gm-Message-State: ACgBeo3SbBlj6Svz5th+PT1tS8LFBCMWofpy7poyKDG9ZX+rUBoY0nA8 6+6JAJqvs/SABboCO6ra+sZT7LMwCOj0/nmVT2gpYA== X-Google-Smtp-Source: AA6agR5nRPXF4tdPfrMSCGI0ifozrAe/wkyTVmws+52XTyYgSoRsViYZYHGu/z8vzoPjWLRyLu7QNqHBJrJWCbNurNo= X-Received: by 2002:ab0:2714:0:b0:383:63cc:70e7 with SMTP id s20-20020ab02714000000b0038363cc70e7mr6077349uao.97.1659358861207; Mon, 01 Aug 2022 06:01:01 -0700 (PDT) MIME-Version: 1.0 References: <20220728070934.641674-1-sr@denx.de> <62a9c9c6-baac-87f1-a080-f99262fa2d87@denx.de> In-Reply-To: From: Simon Glass Date: Mon, 1 Aug 2022 07:00:49 -0600 Message-ID: Subject: Re: [PATCH v2 0/7] Add support for cyclic function execution infrastruture To: Stefan Roese Cc: U-Boot Mailing List , Tom Rini , Chandrakala Chavva , Aaron Williams Content-Type: text/plain; charset="UTF-8" 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 Stefan, On Mon, 1 Aug 2022 at 06:40, Stefan Roese wrote: > > Hi Simon, > > On 01.08.22 14:22, Simon Glass wrote: > > Hi Stefan, > > > > On Mon, 1 Aug 2022 at 01:17, Stefan Roese wrote: > >> > >> Hi Simon, > >> > >> On 31.07.22 03:27, Simon Glass wrote: > >>> Hi Stefan, > >>> > >>> On Thu, 28 Jul 2022 at 01:09, Stefan Roese wrote: > >>>> > >>>> This patchset adds the basic infrastructure to periodically execute > >>>> code, e.g. all 100ms. Examples for such functions might be LED blinking > >>>> etc. The functions that are hooked into this cyclic list should be > >>>> small timewise as otherwise the execution of the other code that relies > >>>> on a high frequent polling (e.g. UART rx char ready check) might be > >>>> delayed too much. This patch also adds the Kconfig option > >>>> CONFIG_CYCLIC_MAX_CPU_TIME_US, which configures the max allowed time > >>>> for such a cyclic function. If it's execution time exceeds this time, > >>>> this cyclic function will get removed from the cyclic list. > >>>> > >>>> How is this cyclic functionality executed? > >>>> This patchset 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. > >>>> > >>>> This cyclic infrastructure will be used by a board specific function on > >>>> the NIC23 MIPS Octeon board, which needs to check periodically, if a > >>>> PCIe FLR has occurred. > >>>> > >>>> Ideas how to continue: > >>>> One idea is to rename WATCHDOG_RESET to something like SCHEDULE and > >>>> move the watchdog_reset call into this cyclic infrastructure as well. > >>>> Or to perhaps move the shell UART RX ready polling to a cyclic > >>>> function. > >>>> > >>>> It's also possible to extend the "cyclic" command, to support the > >>>> creation of periodically executed shell commands (for testing etc). > >>>> > >>>> Here the Azure build, without any issues: > >>>> https://dev.azure.com/sr0718/u-boot/_build/results?buildId=219&view=results > >>>> > >>>> Thanks, > >>>> Stefan > >>>> > >>>> Aaron Williams (1): > >>>> mips: octeon_nic23: Add PCIe FLR fixup via cyclic infrastructure > >>>> > >>>> Stefan Roese (6): > >>>> time: Import time_after64() and friends from Linux > >>>> cyclic: Add basic support for cyclic function execution infrastruture > >>>> cyclic: Integrate cyclic infrastructure into WATCHDOG_RESET > >>>> cyclic: Integrate cyclic functionality at bootup in board_r/f > >>>> cyclic: Add 'cyclic list' command > >>>> sandbox: Add cyclic demo function > >>>> > >>>> MAINTAINERS | 7 + > >>>> board/Marvell/octeon_nic23/board.c | 197 +++++++++++++++++++++++++++++ > >>>> board/sandbox/sandbox.c | 15 +++ > >>>> cmd/Kconfig | 7 + > >>>> cmd/Makefile | 1 + > >>>> cmd/cyclic.c | 40 ++++++ > >>>> common/Kconfig | 20 +++ > >>>> common/Makefile | 1 + > >>>> common/board_f.c | 2 + > >>>> common/board_r.c | 2 + > >>>> common/cyclic.c | 112 ++++++++++++++++ > >>>> configs/octeon_nic23_defconfig | 3 + > >>>> configs/sandbox_defconfig | 3 + > >>>> fs/cramfs/uncompress.c | 2 +- > >>>> include/cyclic.h | 97 ++++++++++++++ > >>>> include/time.h | 19 +++ > >>>> include/watchdog.h | 23 +++- > >>>> 17 files changed, 547 insertions(+), 4 deletions(-) > >>>> create mode 100644 cmd/cyclic.c > >>>> create mode 100644 common/cyclic.c > >>>> create mode 100644 include/cyclic.h > >>>> > >>>> -- > >>>> 2.37.1 > >>>> > >>> > >>> This looks interesting. I like the idea of integrating the watchdog > >>> stuff too, since you are making use of it. > >>> > >>> Would it make sense to make use of the new event system, since it has > >>> static and dynamic handlers? > >> > >> IIRC, I tried to make use of this infrastructure but it did not work > >> out. Or do you see some way to integrate this cyclic IF into the > >> event system? FWICT, the only way to get this done is to make use of > >> the (ugly) WATCHDOG_RESET calls. > > > > Yes that makes sense. I don't see another way. > > > > But within that, one option would be to send an event every time > > WATCHDOG_RESET is used, and have things register as an event spy, thus > > making use of that existing system. The event feature is not enabled > > by default, but it could be enabled when this functionality is needed. > > I still need to double-check, sorry: You are thinking about this call- > trace: > > WATCHDOG_RESET -> event IF -> cylic IF -> cylic user > More this, i.e. I am wondering if the cyclic functionality can be done using events. WATCHDOG_RESET -> sends watchdog event -> event spy receives event > ? > > If yes, what would be the advantage(s) against implementing this > separately? It would need handling of max CPU time and perhaps some other tweaks, but it would result in less code (?) and one less thing for people to learn. The cyclic command could still be provided if that is useful, by showing only cyclic certain event handlers. Regards, Simon