| Message ID | 20220323115752.360409-1-alistair@alistair23.me (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v4] watchdog: imx2_wdg: Allow ping on suspend | expand |
On 3/23/22 04:57, Alistair Francis wrote: > The i.MX watchdog cannot be disabled by software once it has been > enabled. This means that it can't be stopped before suspend. > > For systems that enter low power mode this is fine, as the watchdog will > be automatically stopped by hardware in low power mode. Not all i.MX > platforms support low power mode in the mainline kernel. For example the > i.MX7D does not enter low power mode and so will be rebooted 2 minutes > after entering sleep states. > > This patch introduces a device tree property "fsl,ping-during-suspend" > that can be used to enable ping on suspend support for these systems. > > Signed-off-by: Alistair Francis <alistair@alistair23.me> > Reviewed-by: Guenter Roeck <linux@roeck-us.net> What is the difference to earlier versions ? Guenter
On Wed, 23 Mar 2022, at 11:07 PM, Guenter Roeck wrote: > On 3/23/22 04:57, Alistair Francis wrote: > > The i.MX watchdog cannot be disabled by software once it has been > > enabled. This means that it can't be stopped before suspend. > > > > For systems that enter low power mode this is fine, as the watchdog will > > be automatically stopped by hardware in low power mode. Not all i.MX > > platforms support low power mode in the mainline kernel. For example the > > i.MX7D does not enter low power mode and so will be rebooted 2 minutes > > after entering sleep states. > > > > This patch introduces a device tree property "fsl,ping-during-suspend" > > that can be used to enable ping on suspend support for these systems. > > > > Signed-off-by: Alistair Francis <alistair@alistair23.me> > > Reviewed-by: Guenter Roeck <linux@roeck-us.net> > > What is the difference to earlier versions ? Sorry, I should have included a changelog. I just fixed the spelling of alow/allow in the title Alistair > > Guenter >
Hi, On Wed, Mar 23, 2022 at 09:57:52PM +1000, Alistair Francis wrote: > The i.MX watchdog cannot be disabled by software once it has been > enabled. This means that it can't be stopped before suspend. > > For systems that enter low power mode this is fine, as the watchdog will > be automatically stopped by hardware in low power mode. Not all i.MX > platforms support low power mode in the mainline kernel. For example the > i.MX7D does not enter low power mode and so will be rebooted 2 minutes > after entering sleep states. > > This patch introduces a device tree property "fsl,ping-during-suspend" > that can be used to enable ping on suspend support for these systems. This doesn't match the code anymore. Sascha > > Signed-off-by: Alistair Francis <alistair@alistair23.me> > Reviewed-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/watchdog/imx2_wdt.c | 27 ++++++++++++++++++++------- > 1 file changed, 20 insertions(+), 7 deletions(-) > > diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c > index 51bfb796898b..d0c5d47ddede 100644 > --- a/drivers/watchdog/imx2_wdt.c > +++ b/drivers/watchdog/imx2_wdt.c > @@ -66,6 +66,7 @@ struct imx2_wdt_device { > struct watchdog_device wdog; > bool ext_reset; > bool clk_is_on; > + bool no_ping; > }; > > static bool nowayout = WATCHDOG_NOWAYOUT; > @@ -312,12 +313,18 @@ static int __init imx2_wdt_probe(struct platform_device *pdev) > > wdev->ext_reset = of_property_read_bool(dev->of_node, > "fsl,ext-reset-output"); > + /* > + * The i.MX7D doesn't support low power mode, so we need to ping the watchdog > + * during suspend. > + */ > + wdev->no_ping = !of_device_is_compatible(dev->of_node, "fsl,imx7d-wdt"); > platform_set_drvdata(pdev, wdog); > watchdog_set_drvdata(wdog, wdev); > watchdog_set_nowayout(wdog, nowayout); > watchdog_set_restart_priority(wdog, 128); > watchdog_init_timeout(wdog, timeout, dev); > - watchdog_stop_ping_on_suspend(wdog); > + if (wdev->no_ping) > + watchdog_stop_ping_on_suspend(wdog); > > if (imx2_wdt_is_running(wdev)) { > imx2_wdt_set_timeout(wdog, wdog->timeout); > @@ -366,9 +373,11 @@ static int __maybe_unused imx2_wdt_suspend(struct device *dev) > imx2_wdt_ping(wdog); > } > > - clk_disable_unprepare(wdev->clk); > + if (wdev->no_ping) { > + clk_disable_unprepare(wdev->clk); > > - wdev->clk_is_on = false; > + wdev->clk_is_on = false; > + } > > return 0; > } > @@ -380,11 +389,14 @@ static int __maybe_unused imx2_wdt_resume(struct device *dev) > struct imx2_wdt_device *wdev = watchdog_get_drvdata(wdog); > int ret; > > - ret = clk_prepare_enable(wdev->clk); > - if (ret) > - return ret; > + if (wdev->no_ping) { > + ret = clk_prepare_enable(wdev->clk); > > - wdev->clk_is_on = true; > + if (ret) > + return ret; > + > + wdev->clk_is_on = true; > + } > > if (watchdog_active(wdog) && !imx2_wdt_is_running(wdev)) { > /* > @@ -407,6 +419,7 @@ static SIMPLE_DEV_PM_OPS(imx2_wdt_pm_ops, imx2_wdt_suspend, > > static const struct of_device_id imx2_wdt_dt_ids[] = { > { .compatible = "fsl,imx21-wdt", }, > + { .compatible = "fsl,imx7d-wdt", }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, imx2_wdt_dt_ids); > -- > 2.35.1 > >
diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c index 51bfb796898b..d0c5d47ddede 100644 --- a/drivers/watchdog/imx2_wdt.c +++ b/drivers/watchdog/imx2_wdt.c @@ -66,6 +66,7 @@ struct imx2_wdt_device { struct watchdog_device wdog; bool ext_reset; bool clk_is_on; + bool no_ping; }; static bool nowayout = WATCHDOG_NOWAYOUT; @@ -312,12 +313,18 @@ static int __init imx2_wdt_probe(struct platform_device *pdev) wdev->ext_reset = of_property_read_bool(dev->of_node, "fsl,ext-reset-output"); + /* + * The i.MX7D doesn't support low power mode, so we need to ping the watchdog + * during suspend. + */ + wdev->no_ping = !of_device_is_compatible(dev->of_node, "fsl,imx7d-wdt"); platform_set_drvdata(pdev, wdog); watchdog_set_drvdata(wdog, wdev); watchdog_set_nowayout(wdog, nowayout); watchdog_set_restart_priority(wdog, 128); watchdog_init_timeout(wdog, timeout, dev); - watchdog_stop_ping_on_suspend(wdog); + if (wdev->no_ping) + watchdog_stop_ping_on_suspend(wdog); if (imx2_wdt_is_running(wdev)) { imx2_wdt_set_timeout(wdog, wdog->timeout); @@ -366,9 +373,11 @@ static int __maybe_unused imx2_wdt_suspend(struct device *dev) imx2_wdt_ping(wdog); } - clk_disable_unprepare(wdev->clk); + if (wdev->no_ping) { + clk_disable_unprepare(wdev->clk); - wdev->clk_is_on = false; + wdev->clk_is_on = false; + } return 0; } @@ -380,11 +389,14 @@ static int __maybe_unused imx2_wdt_resume(struct device *dev) struct imx2_wdt_device *wdev = watchdog_get_drvdata(wdog); int ret; - ret = clk_prepare_enable(wdev->clk); - if (ret) - return ret; + if (wdev->no_ping) { + ret = clk_prepare_enable(wdev->clk); - wdev->clk_is_on = true; + if (ret) + return ret; + + wdev->clk_is_on = true; + } if (watchdog_active(wdog) && !imx2_wdt_is_running(wdev)) { /* @@ -407,6 +419,7 @@ static SIMPLE_DEV_PM_OPS(imx2_wdt_pm_ops, imx2_wdt_suspend, static const struct of_device_id imx2_wdt_dt_ids[] = { { .compatible = "fsl,imx21-wdt", }, + { .compatible = "fsl,imx7d-wdt", }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, imx2_wdt_dt_ids);