[PATCH v3] watchdog: imx2_wdt: Add power management support.

[PATCH v3] watchdog: imx2_wdt: Add power management support.

[Xiubo Li]

Add power management operations(suspend and resume) as part of
dev_pm_ops for IMX2 watchdog driver.

If PM will be supported, please make sure that the wdev->clk
could disable the watchdog's counter input clock source or can
mask watchdog's reset request to the core.

If watchdog is still used by consumers and resumes from deep
sleep state, we need to restart the watchdog again without
enabling the timer.

If watchdog been has started --> stopped by the consumers and
resumes from non-deep sleep state, then start the timer again.

If watchdog has been started --> stopped by the consumers and
resumes from deep sleep state, will do nothing. The watchdog
will be restarted by consumers next time to be used.

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---

Change in V3:
- Uses standard multi-line comments.


 drivers/watchdog/imx2_wdt.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 7e12f88..22ab599 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -326,6 +326,65 @@ static void imx2_wdt_shutdown(struct platform_device *pdev)
 	}
 }
 
+#ifdef CONFIG_PM_SLEEP
+/* Disable watchdog if it is active or non-active but still running */
+static int imx2_wdt_suspend(struct device *dev)
+{
+	struct watchdog_device *wdog = dev_get_drvdata(dev);
+	struct imx2_wdt_device *wdev = watchdog_get_drvdata(wdog);
+
+	/* The watchdog IP block is running */
+	if (imx2_wdt_is_running(wdev)) {
+		imx2_wdt_set_timeout(wdog, IMX2_WDT_MAX_TIME);
+		imx2_wdt_ping(wdog);
+
+		/* The watchdog is not active */
+		if (!watchdog_active(wdog))
+			del_timer_sync(&wdev->timer);
+	}
+
+	clk_disable_unprepare(wdev->clk);
+
+	return 0;
+}
+
+/* Enable watchdog and configure it if necessary */
+static int imx2_wdt_resume(struct device *dev)
+{
+	struct watchdog_device *wdog = dev_get_drvdata(dev);
+	struct imx2_wdt_device *wdev = watchdog_get_drvdata(wdog);
+
+	clk_prepare_enable(wdev->clk);
+
+	if (watchdog_active(wdog) && !imx2_wdt_is_running(wdev)) {
+		/*
+		 * If the watchdog is still active and resumes
+		 * from deep sleep state, need to restart the
+		 * watchdog again.
+		 */
+		imx2_wdt_setup(wdog);
+		imx2_wdt_set_timeout(wdog, wdog->timeout);
+		imx2_wdt_ping(wdog);
+	} else if (imx2_wdt_is_running(wdev)) {
+		/* Resuming from non-deep sleep state. */
+		imx2_wdt_set_timeout(wdog, wdog->timeout);
+		imx2_wdt_ping(wdog);
+		/*
+		 * But the watchdog is not active, then start
+		 * the timer again.
+		 */
+		if (!watchdog_active(wdog))
+			mod_timer(&wdev->timer,
+				  jiffies + wdog->timeout * HZ / 2);
+	}
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(imx2_wdt_pm_ops, imx2_wdt_suspend,
+			 imx2_wdt_resume);
+
 static const struct of_device_id imx2_wdt_dt_ids[] = {
 	{ .compatible = "fsl,imx21-wdt", },
 	{ /* sentinel */ }
@@ -338,6 +397,7 @@ static struct platform_driver imx2_wdt_driver = {
 	.driver		= {
 		.name	= DRIVER_NAME,
 		.owner	= THIS_MODULE,
+		.pm     = &imx2_wdt_pm_ops,
 		.of_match_table = imx2_wdt_dt_ids,
 	},
 };
-- 
2.1.0.27.g96db324

Re: [PATCH v3] watchdog: imx2_wdt: Add power management support.

[Guenter Roeck]

On Thu, Oct 16, 2014 at 11:44:15AM +0800, Xiubo Li wrote:
> Add power management operations(suspend and resume) as part of
> dev_pm_ops for IMX2 watchdog driver.
> 
> If PM will be supported, please make sure that the wdev->clk
> could disable the watchdog's counter input clock source or can
> mask watchdog's reset request to the core.
> 
> If watchdog is still used by consumers and resumes from deep
> sleep state, we need to restart the watchdog again without
> enabling the timer.
> 
> If watchdog been has started --> stopped by the consumers and
> resumes from non-deep sleep state, then start the timer again.
> 
> If watchdog has been started --> stopped by the consumers and
> resumes from deep sleep state, will do nothing. The watchdog
> will be restarted by consumers next time to be used.
> 
> Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>

Looks ok to me.

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Change in V3:
> - Uses standard multi-line comments.
> 
> 
>  drivers/watchdog/imx2_wdt.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 
> diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
> index 7e12f88..22ab599 100644
> --- a/drivers/watchdog/imx2_wdt.c
> +++ b/drivers/watchdog/imx2_wdt.c
> @@ -326,6 +326,65 @@ static void imx2_wdt_shutdown(struct platform_device *pdev)
>  	}
>  }
>  
> +#ifdef CONFIG_PM_SLEEP
> +/* Disable watchdog if it is active or non-active but still running */
> +static int imx2_wdt_suspend(struct device *dev)
> +{
> +	struct watchdog_device *wdog = dev_get_drvdata(dev);
> +	struct imx2_wdt_device *wdev = watchdog_get_drvdata(wdog);
> +
> +	/* The watchdog IP block is running */
> +	if (imx2_wdt_is_running(wdev)) {
> +		imx2_wdt_set_timeout(wdog, IMX2_WDT_MAX_TIME);
> +		imx2_wdt_ping(wdog);
> +
> +		/* The watchdog is not active */
> +		if (!watchdog_active(wdog))
> +			del_timer_sync(&wdev->timer);
> +	}
> +
> +	clk_disable_unprepare(wdev->clk);
> +
> +	return 0;
> +}
> +
> +/* Enable watchdog and configure it if necessary */
> +static int imx2_wdt_resume(struct device *dev)
> +{
> +	struct watchdog_device *wdog = dev_get_drvdata(dev);
> +	struct imx2_wdt_device *wdev = watchdog_get_drvdata(wdog);
> +
> +	clk_prepare_enable(wdev->clk);
> +
> +	if (watchdog_active(wdog) && !imx2_wdt_is_running(wdev)) {
> +		/*
> +		 * If the watchdog is still active and resumes
> +		 * from deep sleep state, need to restart the
> +		 * watchdog again.
> +		 */
> +		imx2_wdt_setup(wdog);
> +		imx2_wdt_set_timeout(wdog, wdog->timeout);
> +		imx2_wdt_ping(wdog);
> +	} else if (imx2_wdt_is_running(wdev)) {
> +		/* Resuming from non-deep sleep state. */
> +		imx2_wdt_set_timeout(wdog, wdog->timeout);
> +		imx2_wdt_ping(wdog);
> +		/*
> +		 * But the watchdog is not active, then start
> +		 * the timer again.
> +		 */
> +		if (!watchdog_active(wdog))
> +			mod_timer(&wdev->timer,
> +				  jiffies + wdog->timeout * HZ / 2);
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(imx2_wdt_pm_ops, imx2_wdt_suspend,
> +			 imx2_wdt_resume);
> +
>  static const struct of_device_id imx2_wdt_dt_ids[] = {
>  	{ .compatible = "fsl,imx21-wdt", },
>  	{ /* sentinel */ }
> @@ -338,6 +397,7 @@ static struct platform_driver imx2_wdt_driver = {
>  	.driver		= {
>  		.name	= DRIVER_NAME,
>  		.owner	= THIS_MODULE,
> +		.pm     = &imx2_wdt_pm_ops,
>  		.of_match_table = imx2_wdt_dt_ids,
>  	},
>  };
> -- 
> 2.1.0.27.g96db324
> 

Re: [PATCH v3] watchdog: imx2_wdt: Add power management support.

[Wim Van Sebroeck]

Hi,

> Add power management operations(suspend and resume) as part of
> dev_pm_ops for IMX2 watchdog driver.
> 
> If PM will be supported, please make sure that the wdev->clk
> could disable the watchdog's counter input clock source or can
> mask watchdog's reset request to the core.
> 
> If watchdog is still used by consumers and resumes from deep
> sleep state, we need to restart the watchdog again without
> enabling the timer.
> 
> If watchdog been has started --> stopped by the consumers and
> resumes from non-deep sleep state, then start the timer again.
> 
> If watchdog has been started --> stopped by the consumers and
> resumes from deep sleep state, will do nothing. The watchdog
> will be restarted by consumers next time to be used.
> 
> Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> ---
> 
> Change in V3:
> - Uses standard multi-line comments.

V1 went upstream (my mistake), diff between V1 and V3 is now in linux-watchdog-next.

Kind regards,
Wim.

RE: [PATCH v3] watchdog: imx2_wdt: Add power management support.

[Li.Xiubo]

Hi,

Get it.

Thanks very much.

BRs
Xiubo


> -----Original Message-----
> From: wimvs@spo001.leaseweb.nl [mailto:wimvs@spo001.leaseweb.nl] On Behalf Of
> Wim Van Sebroeck
> Sent: Monday, December 22, 2014 10:05 PM
> To: Xiubo Li-B47053
> Cc: linux@roeck-us.net; linux-watchdog@vger.kernel.org
> Subject: Re: [PATCH v3] watchdog: imx2_wdt: Add power management support.
> 
> Hi,
> 
> > Add power management operations(suspend and resume) as part of
> > dev_pm_ops for IMX2 watchdog driver.
> >
> > If PM will be supported, please make sure that the wdev->clk
> > could disable the watchdog's counter input clock source or can
> > mask watchdog's reset request to the core.
> >
> > If watchdog is still used by consumers and resumes from deep
> > sleep state, we need to restart the watchdog again without
> > enabling the timer.
> >
> > If watchdog been has started --> stopped by the consumers and
> > resumes from non-deep sleep state, then start the timer again.
> >
> > If watchdog has been started --> stopped by the consumers and
> > resumes from deep sleep state, will do nothing. The watchdog
> > will be restarted by consumers next time to be used.
> >
> > Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> > ---
> >
> > Change in V3:
> > - Uses standard multi-line comments.
> 
> V1 went upstream (my mistake), diff between V1 and V3 is now in linux-
> watchdog-next.
> 
> Kind regards,
> Wim.