| Message ID | 1517343778-27902-12-git-send-email-fabrizio.castro@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
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.castro@bp.renesas.com> > Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@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" ? > v1->v3: > * unified Gen2 and Gen3 drivers. > > drivers/watchdog/renesas_wdt.c | 61 ++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 56 insertions(+), 5 deletions(-) > > diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c > index 831ef83..eedb016 100644 > --- a/drivers/watchdog/renesas_wdt.c > +++ b/drivers/watchdog/renesas_wdt.c > @@ -107,6 +107,24 @@ static unsigned int rwdt_get_timeleft(struct watchdog_device *wdev) > return DIV_BY_CLKS_PER_SEC(priv, 65536 - val); > } > > +static int rwdt_restart(struct watchdog_device *wdev, unsigned long action, > + void *data) > +{ > + struct rwdt_priv *priv = watchdog_get_drvdata(wdev); > + > + pm_runtime_get_sync(wdev->parent); > + > + rwdt_write(priv, 0x00, RWTCSRB); > + rwdt_write(priv, 0x00, RWTCSRA); > + rwdt_write(priv, 0xffff, RWTCNT); > + > + while (readb_relaxed(priv->base + RWTCSRA) & RWTCSRA_WRFLG) > + cpu_relax(); > + > + rwdt_write(priv, 0x80, RWTCSRA); > + return 0; > +} > + > static const struct watchdog_info rwdt_ident = { > .options = WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT, > .identity = "Renesas WDT Watchdog", > @@ -118,6 +136,7 @@ static const struct watchdog_ops rwdt_ops = { > .stop = rwdt_stop, > .ping = rwdt_init_timeout, > .get_timeleft = rwdt_get_timeleft, > + .restart = rwdt_restart, > }; > > static int rwdt_probe(struct platform_device *pdev) > @@ -203,13 +222,42 @@ static int rwdt_remove(struct platform_device *pdev) > return 0; > } > > -/* > - * This driver does also fit for R-Car Gen2 (r8a779[0-4]) WDT. However, for SMP > - * to work there, one also needs a RESET (RST) driver which does not exist yet > - * due to HW issues. This needs to be solved before adding compatibles here. > - */ Have those issues been resolved ? > +#ifdef CONFIG_PM > +static int rwdt_suspend(struct device *dev) > +{ > + struct platform_device *pdev; > + struct rwdt_priv *priv; > + > + pdev = to_platform_device(dev); > + priv = platform_get_drvdata(pdev); > + if (watchdog_active(&priv->wdev)) { > + rwdt_write(priv, priv->cks, RWTCSRA); > + } > + return 0; > +} > + > +static int rwdt_resume(struct device *dev) > +{ > + struct platform_device *pdev; > + struct rwdt_priv *priv; > + > + pdev = to_platform_device(dev); > + priv = platform_get_drvdata(pdev); > + if (watchdog_active(&priv->wdev)) { > + rwdt_write(priv, priv->cks | RWTCSRA_TME, RWTCSRA); > + } > + return 0; > +} > + > +static const struct dev_pm_ops rwdt_pm = { > + .suspend = rwdt_suspend, > + .resume = rwdt_resume, > +}; > +#endif > + > static const struct of_device_id rwdt_ids[] = { > { .compatible = "renesas,rcar-gen3-wdt", }, > + { .compatible = "renesas,rcar-gen2-wdt", }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, rwdt_ids); > @@ -218,6 +266,9 @@ static struct platform_driver rwdt_driver = { > .driver = { > .name = "renesas_wdt", > .of_match_table = rwdt_ids, > +#ifdef CONFIG_PM > + .pm = &rwdt_pm, > +#endif > }, > .probe = rwdt_probe, > .remove = rwdt_remove, > -- > 2.7.4 >
Hello Guenter, thank you for your feedback. > 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.castro@bp.renesas.com> > > Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@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. 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. 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? > > > v1->v3: > > * unified Gen2 and Gen3 drivers. > > > > drivers/watchdog/renesas_wdt.c | 61 ++++++++++++++++++++++++++++++++++++++---- > > 1 file changed, 56 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c > > index 831ef83..eedb016 100644 > > --- a/drivers/watchdog/renesas_wdt.c > > +++ b/drivers/watchdog/renesas_wdt.c > > @@ -107,6 +107,24 @@ static unsigned int rwdt_get_timeleft(struct watchdog_device *wdev) > > return DIV_BY_CLKS_PER_SEC(priv, 65536 - val); > > } > > > > +static int rwdt_restart(struct watchdog_device *wdev, unsigned long action, > > +void *data) > > +{ > > +struct rwdt_priv *priv = watchdog_get_drvdata(wdev); > > + > > +pm_runtime_get_sync(wdev->parent); > > + > > +rwdt_write(priv, 0x00, RWTCSRB); > > +rwdt_write(priv, 0x00, RWTCSRA); > > +rwdt_write(priv, 0xffff, RWTCNT); > > + > > +while (readb_relaxed(priv->base + RWTCSRA) & RWTCSRA_WRFLG) > > +cpu_relax(); > > + > > +rwdt_write(priv, 0x80, RWTCSRA); > > +return 0; > > +} > > + > > static const struct watchdog_info rwdt_ident = { > > .options = WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT, > > .identity = "Renesas WDT Watchdog", > > @@ -118,6 +136,7 @@ static const struct watchdog_ops rwdt_ops = { > > .stop = rwdt_stop, > > .ping = rwdt_init_timeout, > > .get_timeleft = rwdt_get_timeleft, > > +.restart = rwdt_restart, > > }; > > > > static int rwdt_probe(struct platform_device *pdev) > > @@ -203,13 +222,42 @@ static int rwdt_remove(struct platform_device *pdev) > > return 0; > > } > > > > -/* > > - * This driver does also fit for R-Car Gen2 (r8a779[0-4]) WDT. However, for SMP > > - * to work there, one also needs a RESET (RST) driver which does not exist yet > > - * due to HW issues. This needs to be solved before adding compatibles here. > > - */ > > Have those issues been resolved ? This patch series addresses precisely those issues. The first implementation used a flag that the watchdog driver needed to explicitly set to make everything work as expected. This new version uses a different concept that doesn't require such an hack, and that's why the proposal now is to merge Gen2 and Gen3 drivers. Thanks, Fab > > > +#ifdef CONFIG_PM > > +static int rwdt_suspend(struct device *dev) > > +{ > > +struct platform_device *pdev; > > +struct rwdt_priv *priv; > > + > > +pdev = to_platform_device(dev); > > +priv = platform_get_drvdata(pdev); > > +if (watchdog_active(&priv->wdev)) { > > +rwdt_write(priv, priv->cks, RWTCSRA); > > +} > > +return 0; > > +} > > + > > +static int rwdt_resume(struct device *dev) > > +{ > > +struct platform_device *pdev; > > +struct rwdt_priv *priv; > > + > > +pdev = to_platform_device(dev); > > +priv = platform_get_drvdata(pdev); > > +if (watchdog_active(&priv->wdev)) { > > +rwdt_write(priv, priv->cks | RWTCSRA_TME, RWTCSRA); > > +} > > +return 0; > > +} > > + > > +static const struct dev_pm_ops rwdt_pm = { > > +.suspend = rwdt_suspend, > > +.resume = rwdt_resume, > > +}; > > +#endif > > + > > static const struct of_device_id rwdt_ids[] = { > > { .compatible = "renesas,rcar-gen3-wdt", }, > > +{ .compatible = "renesas,rcar-gen2-wdt", }, > > { /* sentinel */ } > > }; > > MODULE_DEVICE_TABLE(of, rwdt_ids); > > @@ -218,6 +266,9 @@ static struct platform_driver rwdt_driver = { > > .driver = { > > .name = "renesas_wdt", > > .of_match_table = rwdt_ids, > > +#ifdef CONFIG_PM > > +.pm = &rwdt_pm, > > +#endif > > }, > > .probe = rwdt_probe, > > .remove = rwdt_remove, > > -- > > 2.7.4 > > Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
Hi Fabrizio, On Wed, Jan 31, 2018 at 11:47 AM, Fabrizio Castro <fabrizio.castro@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.castro@bp.renesas.com> >> > Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@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 -- 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
Hi Geert, thank you for your feedback! > Subject: Re: [RFC v3 11/25] watchdog: renesas_wdt: Add R-Car Gen2 support > > Hi Fabrizio, > > On Wed, Jan 31, 2018 at 11:47 AM, Fabrizio Castro > <fabrizio.castro@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.castro@bp.renesas.com> > >> > Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@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? yeah, there isn't much we can do about this. Even without rwdt_suspend in place, if the kernel crashes after disabling the rwdt clock the watchdog won't restart the system. > > > 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... ok, for the next version I'll register the restart callback only when we match "renesas,rcar-gen2-wdt". Since currently there is no watchdog support on Gen2 I believe such a change is perfectly safe. > > > 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. agreed then, I'll split the commit in two for the next version. Thanks, Fab > > 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 Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
On 01/31/2018 04:13 AM, Geert Uytterhoeven wrote: > Hi Fabrizio, > > On Wed, Jan 31, 2018 at 11:47 AM, Fabrizio Castro > <fabrizio.castro@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.castro@bp.renesas.com> >>>> Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@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? > If so, that would be buggy. >> 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. > You could just give it lower priority than PSCI. > Which leaves us with a few boards where we have to use the watchdog, and > wait until the timeout triggers... > Which means the veto is counter-productive and thus meaningless. Guenter >> 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 -- 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 >
Hello Guenter, Geert thank you for your feedback! > > > > 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. > > > > You could just give it lower priority than PSCI. I am looking at the implementation of machine_restart() for both arm and arm64, and it looks like that if arm_pm_restart gets set, then arm_pm_restart has the highest priority of all. If arm_pm_restart is not set, then the kernel will go through the restart_handler_list, and the registered priority for the restart method will matter at that point. My understanding is that Gen3 does set arm_pm_restart due to drivers/firmware/psci.c, and therefore other restart handlers shouldn't matter too much, but I am not familiar with Gen3 at all. Shall I just set the restart priority to 128 and leave the callback in place for both Gen2 and Gen3? Thanks, Fab > > > Which leaves us with a few boards where we have to use the watchdog, and > > wait until the timeout triggers... > > > > Which means the veto is counter-productive and thus meaningless. > > Guenter > Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
Hi Günter, On Wed, Jan 31, 2018 at 3:48 PM, Guenter Roeck <linux@roeck-us.net> wrote: > On 01/31/2018 04:13 AM, Geert Uytterhoeven wrote: >> On Wed, Jan 31, 2018 at 11:47 AM, Fabrizio Castro >> <fabrizio.castro@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.castro@bp.renesas.com> >>>>> Signed-off-by: Ramesh Shanmugasundaram >>>>> <ramesh.shanmugasundaram@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? >> > If so, that would be buggy. All watchdog drivers implementing a system suspend handler stop the watchdog before suspending the system. I guess if they wouldn't do that, the watchdog would restart the system while asleep? Exceptions are: - atlas7_wdt, sirfsoc_wdt: these seem to rely on another driver stopping the watchdog clock, so it behaves the same as all above, - diag288_wdt: this returns -EBUSY when trying to suspend the system while the watchdog is enabled, to put the burden of disabling the watchdog on the user, - ux500_wdt: this seems to use a hardware feature to automatically disable the watchdog during suspend. So only the last one seems to protect against kernel crashes after the watchdog's suspend method is called... Note that two drivers (iTCO_wdt and wdat_wdt) implements suspend_noirq instead of suspend, which decreases the window of running without a watchdog a bit. >>> 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. > > You could just give it lower priority than PSCI. > >> Which leaves us with a few boards where we have to use the watchdog, and >> wait until the timeout triggers... > > Which means the veto is counter-productive and thus meaningless. I do welcome that point of view ;-) 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
diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c index 831ef83..eedb016 100644 --- a/drivers/watchdog/renesas_wdt.c +++ b/drivers/watchdog/renesas_wdt.c @@ -107,6 +107,24 @@ static unsigned int rwdt_get_timeleft(struct watchdog_device *wdev) return DIV_BY_CLKS_PER_SEC(priv, 65536 - val); } +static int rwdt_restart(struct watchdog_device *wdev, unsigned long action, + void *data) +{ + struct rwdt_priv *priv = watchdog_get_drvdata(wdev); + + pm_runtime_get_sync(wdev->parent); + + rwdt_write(priv, 0x00, RWTCSRB); + rwdt_write(priv, 0x00, RWTCSRA); + rwdt_write(priv, 0xffff, RWTCNT); + + while (readb_relaxed(priv->base + RWTCSRA) & RWTCSRA_WRFLG) + cpu_relax(); + + rwdt_write(priv, 0x80, RWTCSRA); + return 0; +} + static const struct watchdog_info rwdt_ident = { .options = WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT, .identity = "Renesas WDT Watchdog", @@ -118,6 +136,7 @@ static const struct watchdog_ops rwdt_ops = { .stop = rwdt_stop, .ping = rwdt_init_timeout, .get_timeleft = rwdt_get_timeleft, + .restart = rwdt_restart, }; static int rwdt_probe(struct platform_device *pdev) @@ -203,13 +222,42 @@ static int rwdt_remove(struct platform_device *pdev) return 0; } -/* - * This driver does also fit for R-Car Gen2 (r8a779[0-4]) WDT. However, for SMP - * to work there, one also needs a RESET (RST) driver which does not exist yet - * due to HW issues. This needs to be solved before adding compatibles here. - */ +#ifdef CONFIG_PM +static int rwdt_suspend(struct device *dev) +{ + struct platform_device *pdev; + struct rwdt_priv *priv; + + pdev = to_platform_device(dev); + priv = platform_get_drvdata(pdev); + if (watchdog_active(&priv->wdev)) { + rwdt_write(priv, priv->cks, RWTCSRA); + } + return 0; +} + +static int rwdt_resume(struct device *dev) +{ + struct platform_device *pdev; + struct rwdt_priv *priv; + + pdev = to_platform_device(dev); + priv = platform_get_drvdata(pdev); + if (watchdog_active(&priv->wdev)) { + rwdt_write(priv, priv->cks | RWTCSRA_TME, RWTCSRA); + } + return 0; +} + +static const struct dev_pm_ops rwdt_pm = { + .suspend = rwdt_suspend, + .resume = rwdt_resume, +}; +#endif + static const struct of_device_id rwdt_ids[] = { { .compatible = "renesas,rcar-gen3-wdt", }, + { .compatible = "renesas,rcar-gen2-wdt", }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, rwdt_ids); @@ -218,6 +266,9 @@ static struct platform_driver rwdt_driver = { .driver = { .name = "renesas_wdt", .of_match_table = rwdt_ids, +#ifdef CONFIG_PM + .pm = &rwdt_pm, +#endif }, .probe = rwdt_probe, .remove = rwdt_remove,