RE: [PATCH] ARM: shmobile: r7s72100: add restart handler

[Chris Brandt <Chris.Brandt@renesas.com>]

Hi Geert,

On Friday, February 10, 2017, Geert Uytterhoeven wrote:
> >  static const char *const r7s72100_boards_compat_dt[] __initconst = {
> >         "renesas,r7s72100",
> >         NULL,
> > @@ -29,4 +58,5 @@ DT_MACHINE_START(R7S72100_DT, "Generic R7S72100
> (Flattened Device Tree)")
> >         .init_early     = shmobile_init_delay,
> >         .init_late      = shmobile_init_late,
> >         .dt_compat      = r7s72100_boards_compat_dt,
> > +       .restart        = r7s72100_restart,
> >  MACHINE_END
> 
> Perhaps unsurprisingly, I'd recommend writing a watchdog driver instead.
> drivers/watchdog/renesas_wdt.c (currently supporting R-Car Gen3 only) may
> serve as an example.

Why do you guys always make things more difficult for me... ;)


To be clear, you are recommending I make a WDT timer driver (and not
use .restart) and that will reset the system?

Or, make a WDT driver just so I can steal the base address?

Note that the WDT timeout for the RZ/A1 is too short in my opinion, so
it's really only good for resetting the system.


> From an earlier discussion during development of that driver:
> 
> | The RWDT exists on various Renesas SoCs.
> | From digging into the datasheets, I had discovered two variants a while
> go:
> |   1. 32-bit registers
> |        a. R-Car Gen2: using RST for restarting
> |        b. R-Mobile APE6: using SYSC for restarting
> |   2. 8-bit registers (SH-Mobile AP4/AG5, R-Mobile A1)
> |
> | The differences are small: the variant with 8-bit registers has a
> | smaller maximum timeout, and no magic value to be stored in the upper
> bits.
> |
> | Wolfram discovered the third variant in RZ/A1H, which differs in
> | register layout.
> 
> IIRC, apart from the different register layout, actual operation on RZ/A1H
> is similar to other Renesas SoCs.  Depending on the differences, you may
> decide to write a new driver instead, though.

More or less they all do the same thing (all only have 3 registers).
I guess the decision comes down to since the file name is already
"renesas_wdt.c", do we just make the 1 file work for all Renesas SoC
and not add yet another file.
It is only 3 registers we're talking about...it's not like it's going
to turn into another sh_pfc.c file.

Question: R-Car Gen 3 has 3 watchdog timers:
 1. RCLK watchdog timer (RWDT)
 2. Window Watchdog Timer (WWDT)
 3. System Watchdog

#1 and #3 look like they are the same thing (except #3 is enabled on power
on reset). The renesas_wdt.c uses the register names from #1.
Is the idea that you only use #3 to make sure your systems boots and get into
Linux, then from there you use #1 and stop #3 (hence no driver is needed)?

I don’t see the point of having an "overflow interrupt" enabled if the system
is going to reset regardless.


Chris