On Thursday, February 16, 2017, Guenter Roeck wrote:
> On Thu, Feb 16, 2017 at 12:23:18PM -0500, Chris Brandt wrote:
> > Some Renesas SoCs do not have a reset register and the only way to do
> > a SW controlled reset is to use the watchdog timer. Additionally,
> > since all the WDT timeout options are so quick, a system reset is
> > about the only thing it's good for.
> >
> > Signed-off-by: Chris Brandt <chris.bra...@renesas.com>
> > Reviewed-by: Geert Uytterhoeven <geert+rene...@glider.be>
>
> Wondering - why not use the watchdog driver and the infrastructure
> provided by the watchdog core (ie the restart callback) instead ?
That was Geert's first suggestion, but then he said:
On Friday, February 10, 2017, Geert Uytterhoeven wrote:
> On Fri, Feb 10, 2017 at 3:59 PM, Chris Brandt <chris.bra...@renesas.com>
> wrote:
> > 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?
>
> I meant a WDT timer driver. If the watchdog driver provides a .restart
> callback, it will be used for system reset (hmm, renesas_wdt.c no longer
> has the .restart callback, as per arm64 "system reset must be implemented
> using PSCI"-policy).
> > v2:
> > * changed DT bindings from 'wdt-reset' to 'rza-wdt'
> > * changed hard coded register values to defines
> > * added msleep to while(1) loop
>
> Sure you can sleep here ?
As per Geert's review:
On Wednesday, February 15, 2017, Geert Uytterhoeven wrote:
> > +
> > + /* Wait for WDT overflow */
> > + while (1)
> > + ;
>
> This burns CPU, and thus power, albeit for a very short time.
> Perhaps add an msleep() to the loop body?
Note that you only have 7.7us before the restart occurs, so probably
not much sleeping will be going on.
Chris
