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 7E2C0C433FE for ; Wed, 26 Oct 2022 06:02:54 +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:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=oOYVU8aqZCrOfZisVmnjXEObDmSbtd/jAVkMhz714d8=; b=mZ/KOsrRa+vN3S ESIoC7bLL3IQpIiE5AnoP3dNXW9comU0xqWjKwq6tOOyTbe6iE6rH8yXgyJyFVaowVyhAYOFCVlA8 NvdYV5nR0Fvnswn37tGaygLmo/JzA2kAfocDAd/dyU6zY4JMblY+1umiip6qAKhxtSiB/jOw/dFPb f4cBFiLJZhWGCEd+wwJnyxKwfQwUfNdpq0gbfxCnRFLfOEdfrGFWfF/XXbA7Hs79E2tjvP1E6DBtP RfNcFNWE/Sogw0drAR/OHIhHzkX010/G/a3Ys/Hz9L5Bv6ldZRlO+UI/19vr7bmAvszB2p9EpU06g w1PV5X/F9gFF3l/XASdA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1onZU6-008C5r-Fl; Wed, 26 Oct 2022 06:01:50 +0000 Received: from mx1.tq-group.com ([93.104.207.81]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1onZU2-008C3H-28 for linux-arm-kernel@lists.infradead.org; Wed, 26 Oct 2022 06:01:48 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1666764106; x=1698300106; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=zmD0kxt0R4vUUSaUB+bUhlFkoSbFxMeO9O8HddzYL1Y=; b=gPWzsoAeXU3vMovfGI0vTHN28+4fqhCPbmFzK3AmZOKfpz0sNF3JPB4O 3shJ3fmiIsYso8ATtzDtvVAPF61k/rHGvZ9BYt+nZdayImJ/ZJ810Yi0Y Cbqbhj0BmIS6evdfRfZ0JeXX8NgHzlWCOVZjipTzhtaMf3/+xVkbCeBIe q7lXx58vodFv3GLy9Wl4K3fMyn7jo2Ic5lXWBMparDs9rDfHOuU5kfxZh tJiSRE6pBLQR+CNlvBKiY79+vbuArRNqdOHngvNxeflC4k3egKhaKi0yT aZsN9xzLvr8YmJifzHLnwK+A87BPbbjuudFMpLkl5DfYhOfzriwp2pyvE w==; X-IronPort-AV: E=Sophos;i="5.95,213,1661810400"; d="scan'208";a="26973016" Received: from unknown (HELO tq-pgp-pr1.tq-net.de) ([192.168.6.15]) by mx1-pgp.tq-group.com with ESMTP; 26 Oct 2022 08:01:43 +0200 Received: from mx1.tq-group.com ([192.168.6.7]) by tq-pgp-pr1.tq-net.de (PGP Universal service); Wed, 26 Oct 2022 08:01:43 +0200 X-PGP-Universal: processed; by tq-pgp-pr1.tq-net.de on Wed, 26 Oct 2022 08:01:43 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1666764103; x=1698300103; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=zmD0kxt0R4vUUSaUB+bUhlFkoSbFxMeO9O8HddzYL1Y=; b=TzmdZSgIPfOeQuO/SZqfLWNI/h7Ti6dHSu4W/HoptizZ7B0yGLX3lKhu nUrJneTU/kST3pO7BFfBQvpU3CbGyu8v+1DDu8UHimj1u3ZSMB5RybVDl gcFHZZ+8Ep3fwalPw7UTsXi23GbecI2hb432yLgdpCPEN58IQ13uFPG6O qvMiiv5hbdA5/JDb4qkseUK8dEhp22Ochg1KQ/BDXaaE2Vc/DSFvKx6km qocLSACtK5uoKtec6xtcqu1kcua+TNY8G4x98R3UrLexp/ddnMm1yeCG9 WKlcMao/0dR9uvQeLk+P2/zYNqOlrLcQ6KvDoUP/tKYVDoKFULQXL07Ig Q==; X-IronPort-AV: E=Sophos;i="5.95,213,1661810400"; d="scan'208";a="26973015" Received: from vtuxmail01.tq-net.de ([10.115.0.20]) by mx1.tq-group.com with ESMTP; 26 Oct 2022 08:01:42 +0200 Received: from steina-w.localnet (unknown [10.123.53.21]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by vtuxmail01.tq-net.de (Postfix) with ESMTPSA id A0287280056; Wed, 26 Oct 2022 08:01:42 +0200 (CEST) From: Alexander Stein To: Andrej Picej Cc: linux-watchdog@vger.kernel.org, linux-arm-kernel@lists.infradead.org, wim@linux-watchdog.org, linux@roeck-us.net, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, shawnguo@kernel.org, s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com, linux-imx@nxp.com, Anson.Huang@nxp.com, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/3] watchdog: imx2_wdg: suspend watchdog in WAIT mode Date: Wed, 26 Oct 2022 08:01:38 +0200 Message-ID: <2201746.iZASKD2KPV@steina-w> Organization: TQ-Systems GmbH In-Reply-To: <56af1cc3-c10e-5694-d25f-252304732568@norik.com> References: <20221025072533.2980154-1-andrej.picej@norik.com> <13126397.uLZWGnKmhe@steina-w> <56af1cc3-c10e-5694-d25f-252304732568@norik.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221025_230146_519955_8B03D6B0 X-CRM114-Status: GOOD ( 40.83 ) 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 Hello Andrej, Am Dienstag, 25. Oktober 2022, 13:21:18 CEST schrieb Andrej Picej: > Hi Alexander, > > On 25. 10. 22 11:38, Alexander Stein wrote: > > Am Dienstag, 25. Oktober 2022, 09:25:31 CEST schrieb Andrej Picej: > >> Putting device into the "Suspend-To-Idle" mode causes watchdog to > >> trigger and reset the board after set watchdog timeout period elapses. > >> > >> Introduce new device-tree property "fsl,suspend-in-wait" which suspends > >> watchdog in WAIT mode. This is done by setting WDW bit in WCR > >> (Watchdog Control Register) Watchdog operation is restored after exiting > >> WAIT mode as expected. WAIT mode coresponds with Linux's > >> "Suspend-To-Idle". > >> > >> Signed-off-by: Andrej Picej > >> Reviewed-by: Fabio Estevam > >> --- > >> > >> Changes in v2: > >> - validate the property with compatible string, as this functionality > >> > >> is not supported by all devices. > >> > >> --- > >> > >> drivers/watchdog/imx2_wdt.c | 37 +++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 37 insertions(+) > >> > >> diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c > >> index d0c5d47ddede..dd9866c6f1e5 100644 > >> --- a/drivers/watchdog/imx2_wdt.c > >> +++ b/drivers/watchdog/imx2_wdt.c > >> @@ -35,6 +35,7 @@ > >> > >> #define IMX2_WDT_WCR 0x00 /* Control > > > > Register */ > > > >> #define IMX2_WDT_WCR_WT (0xFF << 8) /* -> > > > > Watchdog Timeout Field */ > > > >> +#define IMX2_WDT_WCR_WDW BIT(7) /* -> Watchdog disable > > > > for WAIT */ > > > >> #define IMX2_WDT_WCR_WDA BIT(5) /* -> External Reset > > > > WDOG_B */ > > > >> #define IMX2_WDT_WCR_SRS BIT(4) /* -> Software Reset > > > > Signal */ > > > >> #define IMX2_WDT_WCR_WRE BIT(3) /* -> WDOG Reset Enable > > > > */ > > > >> @@ -67,6 +68,27 @@ struct imx2_wdt_device { > >> > >> bool ext_reset; > >> bool clk_is_on; > >> bool no_ping; > >> > >> + bool sleep_wait; > >> +}; > >> + > >> +static const char * const wdw_boards[] __initconst = { > >> + "fsl,imx25-wdt", > >> + "fsl,imx35-wdt", > >> + "fsl,imx50-wdt", > >> + "fsl,imx51-wdt", > >> + "fsl,imx53-wdt", > >> + "fsl,imx6q-wdt", > >> + "fsl,imx6sl-wdt", > >> + "fsl,imx6sll-wdt", > >> + "fsl,imx6sx-wdt", > >> + "fsl,imx6ul-wdt", > >> + "fsl,imx7d-wdt", > >> + "fsl,imx8mm-wdt", > >> + "fsl,imx8mn-wdt", > >> + "fsl,imx8mp-wdt", > >> + "fsl,imx8mq-wdt", > >> + "fsl,vf610-wdt", > >> + NULL > >> > >> }; > > > > So the models listed in > > Documentation/devicetree/bindings/watchdog/fsl-imx- > > wdt.yaml not supporting this feature are > > * fsl,imx21-wdt > > * fsl,imx27-wdt > > * fsl,imx31-wdt > > * fsl,ls1012a-wdt > > * fsl,ls1043a-wdt > > ? > > yes, you are correct. > > > But all models are listed as compatible to fsl,imx21-wdt. So there is > > something wrong here. IMHO this sounds like the compatible list has to be > > split and updated. Depending on that this feature can be detected. > > Maintaining another list seems error prone to me. > > So basically the compatible lists would be split into two groups, one > for the devices which support this WDW bit and the rest which don't > support this? This was my idea, so only one set has to be maintained. > You got a point here, but...this means that every processors > device-tree, which has two compatible strings (with "fsl,imx21-wdt") > should be updated, right? That sounds like quite a lot of changes, which > I'd like to avoid if possible. Well, the compatible list right now doesn't reflect the hardware features/ compatibility correctly, so IMHO it should be fixed. But apparently Krzysztof is okay having the special property only applicable for a specific set of devices. But in this case you will have to maintain two sets of device models (bindings + driver) to which WDW applies/does not apply to. Best regards, Alexander > Best regards, > Andrej > > > Best regards, > > Alexander > > > >> static bool nowayout = WATCHDOG_NOWAYOUT; > >> > >> @@ -129,6 +151,9 @@ static inline void imx2_wdt_setup(struct > >> watchdog_device *wdog) > >> > >> /* Suspend timer in low power mode, write once-only */ > >> val |= IMX2_WDT_WCR_WDZST; > >> > >> + /* Suspend timer in low power WAIT mode, write once-only */ > >> + if (wdev->sleep_wait) > >> + val |= IMX2_WDT_WCR_WDW; > >> > >> /* Strip the old watchdog Time-Out value */ > >> val &= ~IMX2_WDT_WCR_WT; > >> /* Generate internal chip-level reset if WDOG times out */ > >> > >> @@ -313,6 +338,18 @@ static int __init imx2_wdt_probe(struct > >> platform_device *pdev) > >> > >> wdev->ext_reset = of_property_read_bool(dev->of_node, > >> > >> "fsl,ext- > > > > reset-output"); > > > >> + > >> + if (of_property_read_bool(dev->of_node, "fsl,suspend-in-wait")) > >> + if (of_device_compatible_match(dev->of_node, > > > > wdw_boards)) > > > >> + wdev->sleep_wait = 1; > >> + else { > >> + dev_warn(dev, "Warning: Suspending watchdog > > > > during " \ > > > >> + "WAIT mode is not supported for > > > > this device.\n"); > > > >> + wdev->sleep_wait = 0; > >> + } > >> + else > >> + wdev->sleep_wait = 0; > >> + > >> > >> /* > >> > >> * The i.MX7D doesn't support low power mode, so we need to ping > > > > the > > > >> watchdog * during suspend. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel