Messages in this thread | | | Date | Mon, 24 Jan 2022 08:21:05 -0800 | From | Guenter Roeck <> | Subject | Re: [PATCH v2] watchdog: imx2_wdg: Alow ping on suspend |
| |
On 1/24/22 04:00, Alistair Francis wrote: > The i.MX watchdog cannot be disabled by softwrae once it has been
s/softwrae/software/
> 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 hardwrae in low power mode. Not all i.MX
s/hardwrae/hardware/
> 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 freeze or mem sleep states.
I don't think "mem" adds any value here. Just make it 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> > ---
Change log goes here.
> 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;
I don't see the purpose of this variable. Unless I am missing something is is set but never used.
> + 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");
This is ok as long as there is only a single chip requiring this change. If there are more, the 'data' field in struct of_device_id should be used instead.
> 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);
| |