On Thu, Feb 16, 2017 at 06:40:05PM +0000, Chris Brandt wrote:
> 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.brandt@xxxxxxxxxxx> Reviewed-by: Geert
> > > Uytterhoeven <geert+renesas@xxxxxxxxx>
> >
> > 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.Brandt@xxxxxxxxxxx>
> > 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).
>
Hmm, ok. Guess I don't have to understand that you can not use the watchdog
driver because of the above, but implementing exactly the same functionality
in a separate driver is ok.
[ I am sure I am missing something here, so just ignore what I am saying ]
>
>
> > > 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.
>
Let me rephrase my question. Is it guaranteed that you _can_ sleep in this
function, or in other words that it is guaranteed that interrupts are enabled ?
Thanks,
Guenter
