From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
Geert Uytterhoeven <geert+renesas@glider.be>,
Catalin Marinas <catalin.marinas@arm.com>,
Michael Turquette <mturquette@baylibre.com>,
Will Deacon <will.deacon@arm.com>,
Wolfram Sang <wsa+renesas@sang-engineering.com>,
Wim Van Sebroeck <wim@iguana.be>,
linux-clk <linux-clk@vger.kernel.org>,
Magnus Damm <magnus.damm@gmail.com>,
Russell King <linux@armlinux.org.uk>,
Guenter Roeck <linux@roeck-us.net>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@vger.kernel.org>,
Chris Paterson <Chris.Paterson2@renesas.com>,
Linux Watchdog Mailing List <linux-watchdog@vger.kernel.org>,
Biju Das <biju.das@bp.renesas.com>,
Simon Horman <horms@verge.net.au>,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
Stephen Boyd <sboyd@codeaurora.org>,
Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
Rob Herring <robh+dt@kernel.org>
Subject: Re: [PATCH v5 12/26] watchdog: renesas_wdt: Add R-Car Gen2 support
Date: Wed, 28 Feb 2018 20:24:27 +0100 [thread overview]
Message-ID: <CAMuHMdXKHzpi9evmTOt58DaZzA3dHmMJsAqPNi0+j9Bv1QBbdQ@mail.gmail.com> (raw)
In-Reply-To: <1518457475-4480-13-git-send-email-fabrizio.castro@bp.renesas.com>
Hi Fabrizio,
On Mon, Feb 12, 2018 at 6:44 PM, Fabrizio Castro
<fabrizio.castro@bp.renesas.com> wrote:
> Due to commits:
> * "ARM: shmobile: Add watchdog support",
> * "ARM: shmobile: rcar-gen2: Add watchdog support", and
> * "soc: renesas: rcar-rst: Enable watchdog as reset trigger for Gen2",
> we now have everything we needed for the watchdog to work on Gen2 and
> RZ/G1.
>
> This commit adds "renesas,rcar-gen2-wdt" as compatible string for R-Car
> Gen2 and RZ/G1, and since on those platforms the rwdt clock needs to be
> always ON, when suspending to RAM we need to explicitly disable the
> counting by clearing TME from RWTCSRA.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
Thanks for your patch!
I verified this works on R-Car Gen2, so
Reviewed-and-Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Still, more comments below...
> --- a/drivers/watchdog/renesas_wdt.c
> +++ b/drivers/watchdog/renesas_wdt.c
> @@ -203,13 +203,29 @@ static int rwdt_remove(struct platform_device *pdev)
> return 0;
> }
>
> -/*
> - * This driver does also fit for R-Car Gen2 (r8a779[0-4]) WDT. However, for SMP
> - * to work there, one also needs a RESET (RST) driver which does not exist yet
> - * due to HW issues. This needs to be solved before adding compatibles here.
> - */
> +static int __maybe_unused rwdt_suspend(struct device *dev)
> +{
> + struct rwdt_priv *priv = dev_get_drvdata(dev);
> +
> + if (watchdog_active(&priv->wdev))
> + rwdt_write(priv, priv->cks, RWTCSRA);
> + return 0;
> +}
> +
> +static int __maybe_unused rwdt_resume(struct device *dev)
> +{
> + struct rwdt_priv *priv = dev_get_drvdata(dev);
> +
> + if (watchdog_active(&priv->wdev))
> + rwdt_write(priv, priv->cks | RWTCSRA_TME, RWTCSRA);
Writing to this register is not sufficient on R-Car Gen3, where PSCI
suspend powers down the whole SoC. Hence all WDT register content is lost,
causing the watchdog timeout never to trigger.
Note that this issue is pre-existing, and not caused by your patch.
This can be fixed by replacing the RWTCSRA register writes in the
suspend/resume handlers by calls to rwdt_stop() resp. rwdt_start(), like is
done in the BSP in commit e406980763f18f38 ("watchdog: renesas-wdt: Support
the suspend/resume"). Note that this would cause a small change in behavior
on R-Car Gen2, where the timeout would be reset on resume, instead of
continuing where stopped before. I don't think that hurts, though.
Since I was always a bit uncomfortable with this patch doing two things at
once (1. suspend/resume, 2. "renesas,rcar-gen2-wdt" matching), I think it
would be better to take the patch from the BSP first, and add support for
"renesas,rcar-gen2-wdt" in a subsequent patch.
Does the above make sense?
Do you agree?
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
next prev parent reply other threads:[~2018-02-28 19:24 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-12 17:44 [PATCH v5 00/26] Fix watchdog on Renesas R-Car Gen2 and RZ/G1 Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 01/26] ARM: shmobile: Add watchdog support Fabrizio Castro
2018-02-28 12:57 ` Geert Uytterhoeven
2018-02-28 17:37 ` Fabrizio Castro
2018-02-28 17:40 ` [PATCH v6 " Fabrizio Castro
2018-03-01 9:44 ` Simon Horman
2018-03-13 13:43 ` Geert Uytterhoeven
2018-03-14 11:02 ` Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 02/26] ARM: dts: r8a7743: Adjust SMP routine size Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 03/26] ARM: dts: r8a7745: " Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 04/26] ARM: dts: r8a7790: " Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 07/26] ARM: dts: r8a7793: " Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 14/26] ARM: shmobile: defconfig: Enable RENESAS_WDT_GEN Fabrizio Castro
[not found] ` <1518457475-4480-15-git-send-email-fabrizio.castro-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>
2018-02-13 13:08 ` Fabrizio Castro
[not found] ` <TY1PR06MB0895970E6725F954D1A97E4CC0F60-/PRLmSCtZ16EeHdvShrxA20DtJ1/0DrXvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2018-02-15 16:20 ` Simon Horman
2018-05-02 9:38 ` Simon Horman
2018-02-12 17:44 ` [PATCH v5 15/26] clk: renesas: r8a7743: Add rwdt clock Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 16/26] clk: renesas: r8a7745: " Fabrizio Castro
[not found] ` <1518457475-4480-1-git-send-email-fabrizio.castro-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>
2018-02-12 17:44 ` [PATCH v5 05/26] ARM: dts: r8a7791: Adjust SMP routine size Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 06/26] ARM: dts: r8a7792: " Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 08/26] ARM: dts: r8a7794: " Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 09/26] soc: renesas: rcar-rst: Enable watchdog as reset trigger for Gen2 Fabrizio Castro
2018-02-13 8:19 ` Simon Horman
2018-02-13 12:57 ` Fabrizio Castro
2018-02-13 13:02 ` [PATCH v6 " Fabrizio Castro
2018-02-15 16:20 ` Simon Horman
2018-02-12 17:44 ` [PATCH v5 10/26] ARM: shmobile: rcar-gen2: Add watchdog support Fabrizio Castro
[not found] ` <1518457475-4480-11-git-send-email-fabrizio.castro-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>
2018-02-13 8:21 ` Simon Horman
2018-02-12 17:44 ` [PATCH v5 11/26] dt-bindings: watchdog: renesas-wdt: Add R-Car Gen2 support Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 12/26] watchdog: renesas_wdt: " Fabrizio Castro
[not found] ` <1518457475-4480-13-git-send-email-fabrizio.castro-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>
2018-02-12 19:24 ` Guenter Roeck
2018-02-12 20:58 ` Wolfram Sang
2018-02-28 19:24 ` Geert Uytterhoeven [this message]
2018-03-01 15:34 ` Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 13/26] watchdog: renesas_wdt: Add restart handler Fabrizio Castro
2018-02-12 20:59 ` Wolfram Sang
2018-02-12 17:44 ` [PATCH v5 17/26] clk: renesas: r8a7790: Add rwdt clock Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 18/26] clk: renesas: r8a7791/r8a7793: " Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 22/26] ARM: dts: r8a7790: Add watchdog support to SoC dtsi Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 25/26] ARM: dts: iwg20m: Add watchdog support to SoM dtsi Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 19/26] clk: renesas: r8a7794: Add rwdt clock Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 20/26] ARM: dts: r8a7743: Add watchdog support to SoC dtsi Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 21/26] ARM: dts: r8a7745: " Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 23/26] ARM: dts: r8a7791: " Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 24/26] ARM: dts: r8a7794: " Fabrizio Castro
2018-02-12 17:44 ` [PATCH v5 26/26] ARM: dts: iwg22m: Add watchdog support to SoM dtsi Fabrizio Castro
2018-02-13 8:05 ` [PATCH v5 00/26] Fix watchdog on Renesas R-Car Gen2 and RZ/G1 Simon Horman
2018-02-20 12:51 ` Geert Uytterhoeven
2018-02-21 16:13 ` Simon Horman
2018-02-21 16:30 ` Geert Uytterhoeven
2018-02-21 18:32 ` Simon Horman
2018-02-22 8:38 ` Geert Uytterhoeven
2018-02-23 8:14 ` Simon Horman
2018-03-01 10:20 ` Geert Uytterhoeven
2018-03-13 20:05 ` Simon Horman
2018-03-14 8:17 ` Geert Uytterhoeven
2018-04-18 13:37 ` Geert Uytterhoeven
2018-04-24 9:09 ` Simon Horman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAMuHMdXKHzpi9evmTOt58DaZzA3dHmMJsAqPNi0+j9Bv1QBbdQ@mail.gmail.com \
--to=geert@linux-m68k.org \
--cc=Chris.Paterson2@renesas.com \
--cc=biju.das@bp.renesas.com \
--cc=catalin.marinas@arm.com \
--cc=devicetree@vger.kernel.org \
--cc=fabrizio.castro@bp.renesas.com \
--cc=geert+renesas@glider.be \
--cc=horms@verge.net.au \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=linux-watchdog@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=linux@roeck-us.net \
--cc=magnus.damm@gmail.com \
--cc=mark.rutland@arm.com \
--cc=mturquette@baylibre.com \
--cc=robh+dt@kernel.org \
--cc=sboyd@codeaurora.org \
--cc=will.deacon@arm.com \
--cc=wim@iguana.be \
--cc=wsa+renesas@sang-engineering.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).