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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id BBF68E6FE31 for ; Tue, 26 Sep 2023 00:36:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=NBrlHnkHacH4vslZRUYwX5OBvgKHWuge3g/UwRWeOj8=; b=Zc3ZZXUDnIvW+3 ujCTpMSyBc0FGGvwv0Dcmhead5x1++vunzLq58ShDRz7GoHIN1q2dBmuiQ6jTC7w932Jvz9yZxaFT K6GH53bhUrTK6P660ZKCLaSBwk6cV2K3q1drr3P1HQag7c8gdMj93CVsuOxpNl3MK7zJJPMm81SGJ soqIDRcIl58VKf+FpJjRIe0IWfligd21J4md416VMJxL6YYy0gh4HJrMjskg0c3dkRKtubIBvkrOP yvj4PGf0CKrL2OnSyAbHtsCDrTPKAAHuP1H3AMuNnaH7P3IYq8urQUZu42o8udPSxHxw7p8+6W4Lt DU9YK+moqwfAJ+kU9WJg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qkw38-00FIUx-0y; Tue, 26 Sep 2023 00:35:38 +0000 Received: from mail-oi1-x22c.google.com ([2607:f8b0:4864:20::22c]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qkw34-00FIU3-1W for linux-arm-kernel@lists.infradead.org; Tue, 26 Sep 2023 00:35:36 +0000 Received: by mail-oi1-x22c.google.com with SMTP id 5614622812f47-3aca0934e74so3885093b6e.0 for ; Mon, 25 Sep 2023 17:35:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1695688531; x=1696293331; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:from:to:cc:subject:date:message-id :reply-to; bh=yDfwvoV6juhNVfFYjv+kFk+nrGJP0YOG7IEbzTcdy7c=; b=JNxBwcowv8PdtUCa60uU/Pma+ugOfaNz/Bl+PLi0r7Y146bhz5XTnAy0/EpjfP+XZw hV255eIzEAsuJw4+xMKhC+OWXHtdTcnF4YHuJqRW8xHfJZZsxuYOdQcxA7RWFGp8dDhN ZG4Oz8ehUMgz+GZQzsUltATY8IJlIuYuaWZG4nS/u1YK/ZyU2Kyk1eCWOOwu6ZI8zCgX 1l7GEeoe3GMS6XcPSE1480+veS5H12fi5x32+unRNWzEeha3BcFSTooJ7NiQfwM26WUA y0672VNN61IXRIxEzlmpAZpPM+2kyYeD2avSJ3aEqdspcFRpACNmESV3CVqM/TVW2Dpw CBiw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695688531; x=1696293331; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=yDfwvoV6juhNVfFYjv+kFk+nrGJP0YOG7IEbzTcdy7c=; b=kW0HH94vdpcXfiKAQDU5ldLkF4sPDhgmLEdKBW0KSAcUiWmapEqU+T2eVHF1sRTGjH lSaZ6vqSbNWVuDInOIeNr06bllVBt4FRNFfo6cQZz2bSbLvJZRjQiyEWNC3k7Ut7H+Lc GyfxGBTjA2kBS86erryz3xbRgiHK53ySreCNEkSqfTvAgBLpQS+N0ivbmtvFA4jcGizG nn7WNm2/2Cf6927g6YcR55KaszXeTXHUMaNm7TP6XDlnxw7ahwpUVw+nZBE2MMKLMNUV +aWDjpDKnXDjy+/Eg4il4E3PDM85gl897ds6uQBAmpZbIp4VsAqt5XqglVlqzrbS/H3c dCcg== X-Gm-Message-State: AOJu0YxyTW6HGv1JFjcfXC39iO4I+Vrcm+lfM1eYTtyPyTBhzdTlk9rD LsyvOxkaZI8bLqYsmCGu/RE= X-Google-Smtp-Source: AGHT+IFZCmNJnJdBBgZG8mrF9VaaQIQQKbIAtmGU3RLgAWG+zAccdSVW4KNX9t2EvpjwBsxfUTs40w== X-Received: by 2002:a05:6808:19a2:b0:3a8:5fd6:f4d0 with SMTP id bj34-20020a05680819a200b003a85fd6f4d0mr9300136oib.49.1695688531176; Mon, 25 Sep 2023 17:35:31 -0700 (PDT) Received: from server.roeck-us.net ([2600:1700:e321:62f0:329c:23ff:fee3:9d7c]) by smtp.gmail.com with ESMTPSA id z10-20020aa791ca000000b0068fbad48817sm8638053pfa.123.2023.09.25.17.35.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 25 Sep 2023 17:35:30 -0700 (PDT) Date: Mon, 25 Sep 2023 17:35:28 -0700 From: Guenter Roeck To: Zev Weiss Cc: Andrew Jeffery , g@hatter.bewilderbeest.net, Conor Dooley , Joel Stanley , Krzysztof Kozlowski , "Milton D. Miller II" , Rob Herring , Wim Van Sebroeck , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org, linux-watchdog@vger.kernel.org, openbmc@lists.ozlabs.org, Eddie James , Ivan Mikhaylov , Thomas =?iso-8859-1?Q?Wei=DFschuh?= Subject: Re: [PATCH 1/2] dt-bindings: watchdog: aspeed-wdt: Add aspeed,reset-mask property Message-ID: References: <20230922104231.1434-4-zev@bewilderbeest.net> <20230922104231.1434-5-zev@bewilderbeest.net> <6df088a6-75ab-42f2-ba39-3f155714ed2d@app.fastmail.com> <6b0d4901-d543-4a06-a1e4-7f1558f5361f@hatter.bewilderbeest.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <6b0d4901-d543-4a06-a1e4-7f1558f5361f@hatter.bewilderbeest.net> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230925_173534_536684_7CC8D4D7 X-CRM114-Status: GOOD ( 53.94 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Sep 25, 2023 at 05:04:10PM -0700, Zev Weiss wrote: > On Sun, Sep 24, 2023 at 07:42:45PM PDT, Andrew Jeffery wrote: > > > > > > On Fri, 22 Sep 2023, at 20:12, Zev Weiss wrote: > > > This property configures the Aspeed watchdog timer's reset mask, which > > > controls which peripherals are reset when the watchdog timer expires. > > > Some platforms require that certain devices be left untouched across a > > > reboot; aspeed,reset-mask can now be used to express such constraints. > > > > > > Signed-off-by: Zev Weiss > > > --- > > > .../bindings/watchdog/aspeed-wdt.txt | 18 +++- > > > include/dt-bindings/watchdog/aspeed-wdt.h | 92 +++++++++++++++++++ > > > 2 files changed, 109 insertions(+), 1 deletion(-) > > > create mode 100644 include/dt-bindings/watchdog/aspeed-wdt.h > > > > > > diff --git a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt > > > b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt > > > index a8197632d6d2..3208adb3e52e 100644 > > > --- a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt > > > +++ b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt > > > @@ -47,7 +47,15 @@ Optional properties for AST2500-compatible watchdogs: > > > is configured as push-pull, then set the pulse > > > polarity to active-high. The default is active-low. > > > > > > -Example: > > > +Optional properties for AST2500- and AST2600-compatible watchdogs: > > > + - aspeed,reset-mask: A bitmask indicating which peripherals will be reset if > > > + the watchdog timer expires. On AST2500 this should be a > > > + single word defined using the AST2500_WDT_RESET_* macros; > > > + on AST2600 this should be a two-word array with the first > > > + word defined using the AST2600_WDT_RESET1_* macros and the > > > + second word defined using the AST2600_WDT_RESET2_* macros. > > > + > > > +Examples: > > > > > > wdt1: watchdog@1e785000 { > > > compatible = "aspeed,ast2400-wdt"; > > > @@ -55,3 +63,11 @@ Example: > > > aspeed,reset-type = "system"; > > > aspeed,external-signal; > > > }; > > > + > > > + #include > > > + wdt2: watchdog@1e785040 { > > > + compatible = "aspeed,ast2600-wdt"; > > > + reg = <0x1e785040 0x40>; > > > + aspeed,reset-mask = > > + (AST2600_WDT_RESET2_DEFAULT & ~AST2600_WDT_RESET2_LPC)>; > > > + }; > > > > Rob has acked your current approach already, but I do wonder about an > > alternative that aligns more with the clock/reset/interrupt properties. > > Essentially, define a new generic watchdog property that is specified on > > the controllers to be reset by the watchdog (or even on just the > > watchdog node itself, emulating what you've proposed here): > > > > watchdog-resets = ; > > > > The phandle links to the watchdog of interest, and the index specifies > > the controller associated with the configuration. It might even be > > useful to do: > > > > watchdog-resets = ; > > > > "enable" could provide explicit control over whether somethings should > > be reset or not (as a way to prevent reset if the controller targeted by > > the provided index would otherwise be reset in accordance with the > > default reset value in the watchdog controller). > > > > The macros from the dt-bindings header can then use macros to name the > > indexes rather than define a mask tied to the register layout. The index > > may still in some way represent the mask position. This has the benefit > > of hiding the issue of one vs two configuration registers between the > > AST2500 and AST2600 while also allowing other controllers to exploit the > > binding (Nuvoton BMCs? Though maybe it's generalising too early?). > > > > Sorry, I'm having a bit of a hard time picturing exactly what you're > suggesting here...to start with: > > > property that is specified on the controllers to be reset by the > > watchdog > > and > > > or even on just the watchdog node itself > > seem on the face of it like two fairly different approaches to me. The > former sounds more like existing clock/reset/etc. stuff, where the > peripheral has a property describing its relationship to the "central" > subsystem, and various peripheral drivers are all individually responsible > for observing that property and calling in to the central subsystem to > configure things for that peripheral appropriately; if I'm understanding you > correctly, it might look something like: > > &spi1 { > watchdog-resets = <&wdt1 WDT_INDEX_SPI1 0>; > }; > > Or maybe something more like how pinctrl works, via phandles to subnodes of > the central device? > > &wdt1 { > wdt1_spi1_reset: spi1_reset { > reg = <0x1c>; > bit = <24>; > }; > }; > > &spi1 { > watchdog-resets = <&wdt1_spi1_reset 0>; > }; > > Either way, it seems like it'd be complicated by any insufficient > granularity in the watchdog w.r.t. having independent control over the > individual devices represented by separate DT nodes (such as how the AST2500 > watchdog has a single SPI controller reset bit instead of one per SPI > interface, or its "misc SOC controller" bit governing all sorts of odds and > ends). > > In the latter case (property on the wdt node), would it essentially just be > kind of an indirection layer mapping hardware-independent device indices to > specific registers/bits? It's not obvious to me what purpose a phandle to > the peripheral device node would serve (would the wdt driver have a good way > of identifying what specific peripheral it's pointing to to know what bit to > twiddle?), but maybe I'm misunderstanding what you're suggesting... > > > I guess my other uncertainty is the balance between generalization and > applicability -- how many other watchdog devices have sufficient comparable > configurability to make use of it? I haven't pored over all of them, but > from a random sampling of 20 so of the other existing wdt drivers I don't > see any obvious candidates -- the closest I saw were cpwd.c, which > apparently can distinguish between a CPU reset and a CPU/backplane/board > reset, and realtek_otto_wdt.c, which can do a CPU or a SOC reset (though I > don't have any of the hardware docs to know what capabilities other devices > might provide that the drivers don't use). Do the Nuvoton BMCs have > watchdogs with peripheral-granularity reset configuration? > Quite frankly, I don't like where this is going. It is getting way too complicated. And when something is becoming way too complicated, I tend to put it on my backburner list. The length of that list quickly approaches maxint. Guenter _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel