Hi Fabrizio, On Wed, Jan 31, 2018 at 11:47 AM, Fabrizio Castro <fabrizio.cas...@bp.renesas.com> wrote: >> Subject: Re: [RFC v3 11/25] watchdog: renesas_wdt: Add R-Car Gen2 support >> On Tue, Jan 30, 2018 at 08:22:44PM +0000, Fabrizio Castro wrote: >> > On R-Car Gen2 and RZ/G1 the rwdt clock needs to be always ON, therefore >> > when suspending to RAM we need to explicitly disable the counting by >> > clearing TME from RWTCSRA. >> > Also, on some systems RWDT is the only piece of HW that allows the SoC >> > to be restarted, therefore this patch implements the restart callback. >> > >> > Signed-off-by: Fabrizio Castro <fabrizio.cas...@bp.renesas.com> >> > Signed-off-by: Ramesh Shanmugasundaram >> > <ramesh.shanmugasunda...@bp.renesas.com> >> > --- >> > Wolfram, was the restart callback implementation missing for a reason? >> > Is its implementation going to break any Gen3 platform? >> >> The changes clearly are way more than claimed in the subject. Adding >> restart handler and PM support may be prerequisites for Gen2, but the >> changes apply to Gen3 as well. What happened to "one patch per logical >> change" ? > > The PM implementation should be ok for Gen3 too, without this patch series > the kernel > would disable the rwdt clock when suspending to RAM, this would "pause" the > counting. > This patch series declares the rwdt clock as critical for Gen2 and RZ/G1, > which means we > need to explicitly disable the counting to keep the same behaviour in place.
Note that if the kernel crashes after rwdt_suspend(), the watchdog won't restart the system. But that's an issue common to many watchdog driver, right? > With respect to the restart callback implementation, that may well have > consequences on > Gen3, hopefully Wolfram will give us a feedback on this. > In particular I would like to know if we should be installing the restart > callback only when > we use "renesas,rcar-gen2-wdt" as compatible string, or it's ok to make it > available for > Gen3 as well. IIRC, the reason we don't have the restart callback on R-Car Gen3 is the arm64 maintainers insisting on using PSCI, and vetoing other means, to restart the system. Which leaves us with a few boards where we have to use the watchdog, and wait until the timeout triggers... > The only way to reboot iWave's boards iwg20d and iwg22d is to use the > internal watchdog, > that's why the restart implementation was merged into this patch. > I can split this patch in two, one for PM support and one for restart support > for the next > version, what do you think about this? Splitting in individual logical changes sounds like a good idea to me. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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