Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Tue, 17 May 2022 16:02:37 +0200
From:   Jonathan =?utf-8?Q?Neusch=C3=A4fer?= <j.neuschaefer@gmx.net>
To:     Guenter Roeck <linux@roeck-us.net>
Cc:     Jonathan =?utf-8?Q?Neusch=C3=A4fer?= <j.neuschaefer@gmx.net>,
        linux-clk@vger.kernel.org, openbmc@lists.ozlabs.org,
        linux-kernel@vger.kernel.org, linux-watchdog@vger.kernel.org,
        devicetree@vger.kernel.org,
        Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzk+dt@kernel.org>,
        Avi Fishman <avifishman70@gmail.com>,
        Tomer Maimon <tmaimon77@gmail.com>,
        Tali Perry <tali.perry1@gmail.com>,
        Patrick Venture <venture@google.com>,
        Nancy Yuen <yuenn@google.com>,
        Benjamin Fair <benjaminfair@google.com>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Wim Van Sebroeck <wim@linux-watchdog.org>
Subject: Re: [PATCH v3 3/7] watchdog: npcm: Enable clock if provided
Message-ID: <YoOq/enI6YSplnS5@latitude>
References: <20220508194333.2170161-1-j.neuschaefer@gmx.net>
 <20220508194333.2170161-4-j.neuschaefer@gmx.net>
 <6c22fe1c-03d9-3a7e-26e8-463b989a02b7@roeck-us.net>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="vQZf1NsHbsdMxgvC"
Content-Disposition: inline
In-Reply-To: <6c22fe1c-03d9-3a7e-26e8-463b989a02b7@roeck-us.net>
Precedence: bulk


--vQZf1NsHbsdMxgvC
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, May 09, 2022 at 07:31:31PM -0700, Guenter Roeck wrote:
> On 5/8/22 12:43, Jonathan Neusch=C3=A4fer wrote:
> > On the Nuvoton WPCM450 SoC, with its upcoming clock driver, peripheral
> > clocks are individually gated and ungated. Therefore, the watchdog
> > driver must be able to ungate the watchdog clock.
> >=20
> > Signed-off-by: Jonathan Neusch=C3=A4fer <j.neuschaefer@gmx.net>
> > ---
> >=20
> > v3:
> > - Add enable/disable calls to npcm_wdt_restart handler
> > - Not applied due to the above change:  Acked-by: Guenter Roeck <linux@=
roeck-us.net>
> >=20
> > v2:
> > - https://lore.kernel.org/lkml/20220429172030.398011-4-j.neuschaefer@gm=
x.net/
> > - Add clk_disable_unprepare call, suggested by Guenter Roeck
> >=20
> > v1:
> > - https://lore.kernel.org/lkml/20220422183012.444674-4-j.neuschaefer@gm=
x.net/
> > ---
[...]
> > @@ -147,9 +155,15 @@ static int npcm_wdt_restart(struct watchdog_device=
 *wdd,
> >   {
> >   	struct npcm_wdt *wdt =3D to_npcm_wdt(wdd);
> >=20
> > +	if (wdt->clk)
> > +		clk_prepare_enable(wdt->clk);
> > +
> >   	writel(NPCM_WTR | NPCM_WTRE | NPCM_WTE, wdt->reg);
> >   	udelay(1000);
> >=20
> > +	if (wdt->clk)
> > +		clk_disable_unprepare(wdt->clk);
> > +
>=20
> I am trying to understand why you stop the clock here.
> If the watchdog didn't reset the system by now, for whatever reason,
> you explicitly don't want it to reset the system ? If so, please add
> a comment describing the reason for stopping the clock here.

It was for symmetry with starting the clock, and in the hope that
udelay(1000) will be enough to reach timer expiration. (In practice it
does appear to work, although 1ms is not much.)

Upon reconsideration, I agree it's better to leave the clock running for
reset (with a comment pointing out the asymmetry).


Best regards,
Jonathan

--vQZf1NsHbsdMxgvC
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEvHAHGBBjQPVy+qvDCDBEmo7zX9sFAmKDqtwACgkQCDBEmo7z
X9uEnQ/+PieUjR1LsgZzUOsXNS+aAubr0M4s9fyJdH14/eol2H39zy5/Y8ylCJIq
zzFWjUFI6PVXPKiZ/GkOMMNp25cD2a+vEfFzSaaN/yaeNGv8vkU6q1ABm5PVihpK
+44o0bPpuy0SHbIxKCy7pGMvBR3UrjVA5Q/1GSxQd5fLQxOMwoziLNzhfXqI3dJb
s5S1hx8pezpRR9WwAqIX4R6xiyqrwdL8KacjoAiNYQhLLrD/OOQcIlSijCLLnriM
au59vZK1FzFaYsTI8w5vRgq8C6LtiXQCZsXUT9TJ17hC4OQnzNIlxvls+0O9GHlL
e6MTmskpf/BLMbPiYUarLVvgFGqUE3FyVvuha3GUNK4nfBM5bjjyhtbDElvAs1j1
cgBsIyWKbMB2zH5XrTsdTPB3CfQx8+ZZ98sHhzZEYg4C3oynFDNf1Y8g735SZiAq
7bEqMy+nyrLXxPF55eEuh6AKm6FhZaVjZj9n5YqjxfAfeezDN4K8rCg9xWRy0Oyw
C16jjVdbEufJWQIWopPzw5zM2uvu3ZTMjYr3kfIULs3wIYwoa2+fW6tiK8yHWB2B
LFweECTpYkfBYkKGuxfnft4lgzTmSsSK7ggOnkkBjA0GiA0EgJN0JQ0SchBbzlhb
nOPyAKh9/4LFosqLlGiLyDdZJGX3yaqw3xGK7up7tgfWUiWL+F8=
=24UD
-----END PGP SIGNATURE-----

--vQZf1NsHbsdMxgvC--