[RFC 4/5] ARM: shmobile: r8a7790: let rst module allow watchdog resets if desired

[RFC 4/5] ARM: shmobile: r8a7790: let rst module allow watchdog resets if desired

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm/mach-shmobile/setup-rcar-gen2.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c b/arch/arm/mach-shmobile/setup-rcar-gen2.c
index e2b9bb46393d..0d408ac5b96a 100644
--- a/arch/arm/mach-shmobile/setup-rcar-gen2.c
+++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c
@@ -28,7 +28,18 @@
 #include "common.h"
 #include "rcar-gen2.h"
 
-#define MODEMR 0xe6160060
+#define WDTRSTCR	0xe6160054
+#define MODEMR		0xe6160060
+
+void __init rcar_gen2_wdt_rst_init(void)
+{
+#if defined(CONFIG_WATCHDOG)
+	void __iomem *p = ioremap_nocache(WDTRSTCR, 4);
+	BUG_ON(!p);
+	iowrite32(0xa55a0000, p);
+	iounmap(p);
+#endif
+}
 
 u32 rcar_gen2_read_mode_pins(void)
 {
@@ -132,6 +143,9 @@ void __init rcar_gen2_timer_init(void)
 #ifdef CONFIG_COMMON_CLK
 	rcar_gen2_clocks_init(mode);
 #endif
+	/* allow watchdog timers to trigger reset */
+	rcar_gen2_wdt_rst_init();
+
 	clocksource_of_init();
 }
 
-- 
2.1.4

Re: [RFC 4/5] ARM: shmobile: r8a7790: let rst module allow watchdog resets if desired

[Geert Uytterhoeven]

Hi Wolfram,

On Sun, Feb 1, 2015 at 3:47 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  arch/arm/mach-shmobile/setup-rcar-gen2.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c b/arch/arm/mach-shmobile/setup-rcar-gen2.c
> index e2b9bb46393d..0d408ac5b96a 100644
> --- a/arch/arm/mach-shmobile/setup-rcar-gen2.c
> +++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c
> @@ -28,7 +28,18 @@
>  #include "common.h"
>  #include "rcar-gen2.h"
>
> -#define MODEMR 0xe6160060
> +#define WDTRSTCR       0xe6160054
> +#define MODEMR         0xe6160060
> +
> +void __init rcar_gen2_wdt_rst_init(void)
> +{
> +#if defined(CONFIG_WATCHDOG)
> +       void __iomem *p = ioremap_nocache(WDTRSTCR, 4);
> +       BUG_ON(!p);
> +       iowrite32(0xa55a0000, p);

This is dangerous. If the xWDT was left running, the system will be
restarted soon.

I think clearing the xWDT reset mask should be handled in the WDT driver
itself, when enabling the watchdog.

As the wiring between WDT and reset controller differs on the various SoCs
(R-Car Gen2 uses the RST module, R-Mobile APE6 uses the SYSC module),
I was thinking about representing this into DT, e.g. adding a node for the RST:

        rst: reset-controller@e6160000 {
                compatible = "renesas,rst-r8a7791", "syscon";
                reg = <0 0xe6160000 0 0x64>;
        };

and adding a link to the RST to the wdt node:

        syscon = <&rst 0>; /* 0 for RWDT, 1 for SWDT */

On APE6 it can become a link to the SYSC instead.
It's slightly more complicated there, as we already have a driver for the
SYSC on R-Mobile (rmobile-reset), and a device node can't be bound
by both the syscon and the rmobile-reset driver.

On older SH-Mobile / R-Mobile, the link can be left out, as the reset is
not configurable on these SoCs, I think.

Do you have a better idea?

> +       iounmap(p);
> +#endif
> +}
>
>  u32 rcar_gen2_read_mode_pins(void)
>  {
> @@ -132,6 +143,9 @@ void __init rcar_gen2_timer_init(void)
>  #ifdef CONFIG_COMMON_CLK
>         rcar_gen2_clocks_init(mode);
>  #endif
> +       /* allow watchdog timers to trigger reset */
> +       rcar_gen2_wdt_rst_init();
> +
>         clocksource_of_init();
>  }

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

Re: [RFC 4/5] ARM: shmobile: r8a7790: let rst module allow watchdog resets if desired

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 982 bytes --]


> > +void __init rcar_gen2_wdt_rst_init(void)
> > +{
> > +#if defined(CONFIG_WATCHDOG)
> > +       void __iomem *p = ioremap_nocache(WDTRSTCR, 4);
> > +       BUG_ON(!p);
> > +       iowrite32(0xa55a0000, p);
> 
> This is dangerous. If the xWDT was left running, the system will be
> restarted soon.

Agreed, that might be the case.

> I think clearing the xWDT reset mask should be handled in the WDT driver
> itself, when enabling the watchdog.

Yes, and masked again if WDT is disabled.

> It's slightly more complicated there, as we already have a driver for the
> SYSC on R-Mobile (rmobile-reset), and a device node can't be bound
> by both the syscon and the rmobile-reset driver.

Is it just complicated or is it an open issue how to handle that?

> Do you have a better idea?

Nope. I also thought that something like syscon must be used if the
initialization in arch-code won't work. Still, I tried first with the
least intrusive version :)


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC 4/5] ARM: shmobile: r8a7790: let rst module allow watchdog resets if desired

[Geert Uytterhoeven]

Hi Wolfram,

On Mon, Feb 2, 2015 at 12:13 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> It's slightly more complicated there, as we already have a driver for the
>> SYSC on R-Mobile (rmobile-reset), and a device node can't be bound
>> by both the syscon and the rmobile-reset driver.
>
> Is it just complicated or is it an open issue how to handle that?

If the syscon drivers binds against the device, rmobile-reset can no
longer bind against it. One solution is to convert rmobile-reset to use
syscon, so both the rwdt and the rmobile-reset drivers would access it
through syscon.
But currently syscon provides access to the first register bank of a device
only, while the RESCNT register is part of the second bank.

>> Do you have a better idea?
>
> Nope. I also thought that something like syscon must be used if the
> initialization in arch-code won't work. Still, I tried first with the
> least intrusive version :)

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

Re: [RFC 4/5] ARM: shmobile: r8a7790: let rst module allow watchdog resets if desired

[Laurent Pinchart]

On Tuesday 03 February 2015 13:12:31 Geert Uytterhoeven wrote:
> Hi Wolfram,
> 
> On Mon, Feb 2, 2015 at 12:13 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> >> It's slightly more complicated there, as we already have a driver for the
> >> SYSC on R-Mobile (rmobile-reset), and a device node can't be bound
> >> by both the syscon and the rmobile-reset driver.
> > 
> > Is it just complicated or is it an open issue how to handle that?
> 
> If the syscon drivers binds against the device, rmobile-reset can no
> longer bind against it. One solution is to convert rmobile-reset to use
> syscon, so both the rwdt and the rmobile-reset drivers would access it
> through syscon.
> But currently syscon provides access to the first register bank of a device
> only, while the RESCNT register is part of the second bank.

Shouldn't syscon be split into an API that individual drivers can implement 
and a default driver for syscon devices that just need to expose one register 
bank ?

> >> Do you have a better idea?
> > 
> > Nope. I also thought that something like syscon must be used if the
> > initialization in arch-code won't work. Still, I tried first with the
> > least intrusive version :)

-- 
Regards,

Laurent Pinchart