* [PATCH] PM / watchdog: iTCO: stop watchdog during system suspend
@ 2015-04-02 0:31 Rafael J. Wysocki
2015-04-02 0:52 ` Fu, Borun
2015-04-02 1:21 ` Guenter Roeck
0 siblings, 2 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2015-04-02 0:31 UTC (permalink / raw)
To: linux-watchdog
Cc: Linux Kernel Mailing List, Linux PM list, Wim Van Sebroeck,
Aubrey Li, Borun Fu
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
If the target sleep state of the system is not an ACPI sleep state
(S1, S2 or S3), the TCO watchdog needs to be stopped during system
suspend, because it may not be possible to ping it any more after
timekeeping has been suspended (suspend-to-idle does that for
one example).
For this reason, provide ->suspend_noirq and ->resume_noirq
callbacks for the iTCO watchdog driver and use them to stop
and restart the watchdog during system suspend and resume,
respectively, if the system is not going to enter an ACPI
sleep state (in which case the watchdog will be stopped
by the platform firmware before the state is entered).
Reported-by: Borun Fu <borun.fu@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/watchdog/iTCO_wdt.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
Index: linux-pm/drivers/watchdog/iTCO_wdt.c
===================================================================
--- linux-pm.orig/drivers/watchdog/iTCO_wdt.c
+++ linux-pm/drivers/watchdog/iTCO_wdt.c
@@ -51,6 +51,7 @@
#define DRV_VERSION "1.11"
/* Includes */
+#include <linux/acpi.h> /* For ACPI support */
#include <linux/module.h> /* For module specific items */
#include <linux/moduleparam.h> /* For new moduleparam's */
#include <linux/types.h> /* For standard types (like size_t) */
@@ -103,6 +104,8 @@ static struct { /* this is private data
struct platform_device *dev;
/* the PCI-device */
struct pci_dev *pdev;
+ /* whether or not the watchdog has been suspended */
+ bool suspended;
} iTCO_wdt_private;
/* module parameters */
@@ -571,12 +574,60 @@ static void iTCO_wdt_shutdown(struct pla
iTCO_wdt_stop(NULL);
}
+#ifdef CONFIG_PM_SLEEP
+/*
+ * Suspend-to-idle requires this, because it stops the ticks and timekeeping, so
+ * the watchdog cannot be pinged while in that state. In ACPI sleep states the
+ * watchdog is stopped by the platform firmware.
+ */
+
+#ifdef CONFIG_ACPI
+static inline bool need_suspend(void)
+{
+ return acpi_target_system_state() == ACPI_STATE_S0;
+}
+#else
+static inline bool need_suspend(void) { return true; }
+#endif
+
+static int iTCO_wdt_suspend_noirq(struct device *dev)
+{
+ int ret = 0;
+
+ iTCO_wdt_private.suspended = false;
+ if (watchdog_active(&iTCO_wdt_watchdog_dev) && need_suspend()) {
+ ret = iTCO_wdt_stop(&iTCO_wdt_watchdog_dev);
+ if (!ret)
+ iTCO_wdt_private.suspended = true;
+ }
+ return ret;
+}
+
+static int iTCO_wdt_resume_noirq(struct device *dev)
+{
+ if (iTCO_wdt_private.suspended)
+ iTCO_wdt_start(&iTCO_wdt_watchdog_dev);
+
+ return 0;
+}
+
+struct dev_pm_ops iTCO_wdt_pm = {
+ .suspend_noirq = iTCO_wdt_suspend_noirq,
+ .resume_noirq = iTCO_wdt_resume_noirq,
+};
+
+#define ITCO_WDT_PM_OPS &iTCO_wdt_pm
+#else
+#define ITCO_WDT_PM_OPS NULL
+#endif /* CONFIG_PM_SLEEP */
+
static struct platform_driver iTCO_wdt_driver = {
.probe = iTCO_wdt_probe,
.remove = iTCO_wdt_remove,
.shutdown = iTCO_wdt_shutdown,
.driver = {
.name = DRV_NAME,
+ .pm = ITCO_WDT_PM_OPS,
},
};
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] PM / watchdog: iTCO: stop watchdog during system suspend
2015-04-02 0:31 [PATCH] PM / watchdog: iTCO: stop watchdog during system suspend Rafael J. Wysocki
@ 2015-04-02 0:52 ` Fu, Borun
2015-04-02 1:11 ` Guenter Roeck
2015-04-02 1:21 ` Guenter Roeck
1 sibling, 1 reply; 9+ messages in thread
From: Fu, Borun @ 2015-04-02 0:52 UTC (permalink / raw)
To: Rafael J. Wysocki, linux-watchdog
Cc: Linux Kernel Mailing List, Linux PM list, Wim Van Sebroeck, Li,
Aubrey, Wang, Frank
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4028 bytes --]
Hi Rafael,
watchdog_active() function is not implemented in your patch. Pls. add it.
static int watchdog_active(struct device *dev)
{
unsigned int val;
spin_lock(&iTCO_wdt_private.io_lock);
val = inw(TCO1_CNT);
spin_unlock(&iTCO_wdt_private.io_lock);
/* Bit 11: TCO Timer Halt */
if (val & 0x0800)
return 0;
else
return 1;
}
Regards,
Borun
-----Original Message-----
From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
Sent: Thursday, April 02, 2015 8:31
To: linux-watchdog@vger.kernel.org
Cc: Linux Kernel Mailing List; Linux PM list; Wim Van Sebroeck; Li, Aubrey; Fu, Borun
Subject: [PATCH] PM / watchdog: iTCO: stop watchdog during system suspend
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
If the target sleep state of the system is not an ACPI sleep state (S1, S2 or S3), the TCO watchdog needs to be stopped during system suspend, because it may not be possible to ping it any more after timekeeping has been suspended (suspend-to-idle does that for one example).
For this reason, provide ->suspend_noirq and ->resume_noirq callbacks for the iTCO watchdog driver and use them to stop and restart the watchdog during system suspend and resume, respectively, if the system is not going to enter an ACPI sleep state (in which case the watchdog will be stopped by the platform firmware before the state is entered).
Reported-by: Borun Fu <borun.fu@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/watchdog/iTCO_wdt.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
Index: linux-pm/drivers/watchdog/iTCO_wdt.c
===================================================================
--- linux-pm.orig/drivers/watchdog/iTCO_wdt.c
+++ linux-pm/drivers/watchdog/iTCO_wdt.c
@@ -51,6 +51,7 @@
#define DRV_VERSION "1.11"
/* Includes */
+#include <linux/acpi.h> /* For ACPI support */
#include <linux/module.h> /* For module specific items */
#include <linux/moduleparam.h> /* For new moduleparam's */
#include <linux/types.h> /* For standard types (like size_t) */
@@ -103,6 +104,8 @@ static struct { /* this is private data
struct platform_device *dev;
/* the PCI-device */
struct pci_dev *pdev;
+ /* whether or not the watchdog has been suspended */
+ bool suspended;
} iTCO_wdt_private;
/* module parameters */
@@ -571,12 +574,60 @@ static void iTCO_wdt_shutdown(struct pla
iTCO_wdt_stop(NULL);
}
+#ifdef CONFIG_PM_SLEEP
+/*
+ * Suspend-to-idle requires this, because it stops the ticks and
+timekeeping, so
+ * the watchdog cannot be pinged while in that state. In ACPI sleep
+states the
+ * watchdog is stopped by the platform firmware.
+ */
+
+#ifdef CONFIG_ACPI
+static inline bool need_suspend(void)
+{
+ return acpi_target_system_state() == ACPI_STATE_S0; } #else static
+inline bool need_suspend(void) { return true; } #endif
+
+static int iTCO_wdt_suspend_noirq(struct device *dev) {
+ int ret = 0;
+
+ iTCO_wdt_private.suspended = false;
+ if (watchdog_active(&iTCO_wdt_watchdog_dev) && need_suspend()) {
+ ret = iTCO_wdt_stop(&iTCO_wdt_watchdog_dev);
+ if (!ret)
+ iTCO_wdt_private.suspended = true;
+ }
+ return ret;
+}
+
+static int iTCO_wdt_resume_noirq(struct device *dev) {
+ if (iTCO_wdt_private.suspended)
+ iTCO_wdt_start(&iTCO_wdt_watchdog_dev);
+
+ return 0;
+}
+
+struct dev_pm_ops iTCO_wdt_pm = {
+ .suspend_noirq = iTCO_wdt_suspend_noirq,
+ .resume_noirq = iTCO_wdt_resume_noirq, };
+
+#define ITCO_WDT_PM_OPS &iTCO_wdt_pm
+#else
+#define ITCO_WDT_PM_OPS NULL
+#endif /* CONFIG_PM_SLEEP */
+
static struct platform_driver iTCO_wdt_driver = {
.probe = iTCO_wdt_probe,
.remove = iTCO_wdt_remove,
.shutdown = iTCO_wdt_shutdown,
.driver = {
.name = DRV_NAME,
+ .pm = ITCO_WDT_PM_OPS,
},
};
ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] PM / watchdog: iTCO: stop watchdog during system suspend
2015-04-02 0:52 ` Fu, Borun
@ 2015-04-02 1:11 ` Guenter Roeck
2015-04-02 2:13 ` Fu, Borun
0 siblings, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2015-04-02 1:11 UTC (permalink / raw)
To: Fu, Borun, Rafael J. Wysocki, linux-watchdog
Cc: Linux Kernel Mailing List, Linux PM list, Wim Van Sebroeck, Li,
Aubrey, Wang, Frank
On 04/01/2015 05:52 PM, Fu, Borun wrote:
> Hi Rafael,
> watchdog_active() function is not implemented in your patch. Pls. add it.
>
Please explain why using the standard watchdog_active() would not work here.
Guenter
> static int watchdog_active(struct device *dev)
> {
> unsigned int val;
>
> spin_lock(&iTCO_wdt_private.io_lock);
> val = inw(TCO1_CNT);
> spin_unlock(&iTCO_wdt_private.io_lock);
>
> /* Bit 11: TCO Timer Halt */
> if (val & 0x0800)
> return 0;
> else
> return 1;
> }
>
> Regards,
> Borun
> -----Original Message-----
> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> Sent: Thursday, April 02, 2015 8:31
> To: linux-watchdog@vger.kernel.org
> Cc: Linux Kernel Mailing List; Linux PM list; Wim Van Sebroeck; Li, Aubrey; Fu, Borun
> Subject: [PATCH] PM / watchdog: iTCO: stop watchdog during system suspend
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> If the target sleep state of the system is not an ACPI sleep state (S1, S2 or S3), the TCO watchdog needs to be stopped during system suspend, because it may not be possible to ping it any more after timekeeping has been suspended (suspend-to-idle does that for one example).
>
> For this reason, provide ->suspend_noirq and ->resume_noirq callbacks for the iTCO watchdog driver and use them to stop and restart the watchdog during system suspend and resume, respectively, if the system is not going to enter an ACPI sleep state (in which case the watchdog will be stopped by the platform firmware before the state is entered).
>
> Reported-by: Borun Fu <borun.fu@intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/watchdog/iTCO_wdt.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> Index: linux-pm/drivers/watchdog/iTCO_wdt.c
> ===================================================================
> --- linux-pm.orig/drivers/watchdog/iTCO_wdt.c
> +++ linux-pm/drivers/watchdog/iTCO_wdt.c
> @@ -51,6 +51,7 @@
> #define DRV_VERSION "1.11"
>
> /* Includes */
> +#include <linux/acpi.h> /* For ACPI support */
> #include <linux/module.h> /* For module specific items */
> #include <linux/moduleparam.h> /* For new moduleparam's */
> #include <linux/types.h> /* For standard types (like size_t) */
> @@ -103,6 +104,8 @@ static struct { /* this is private data
> struct platform_device *dev;
> /* the PCI-device */
> struct pci_dev *pdev;
> + /* whether or not the watchdog has been suspended */
> + bool suspended;
> } iTCO_wdt_private;
>
> /* module parameters */
> @@ -571,12 +574,60 @@ static void iTCO_wdt_shutdown(struct pla
> iTCO_wdt_stop(NULL);
> }
>
> +#ifdef CONFIG_PM_SLEEP
> +/*
> + * Suspend-to-idle requires this, because it stops the ticks and
> +timekeeping, so
> + * the watchdog cannot be pinged while in that state. In ACPI sleep
> +states the
> + * watchdog is stopped by the platform firmware.
> + */
> +
> +#ifdef CONFIG_ACPI
> +static inline bool need_suspend(void)
> +{
> + return acpi_target_system_state() == ACPI_STATE_S0; } #else static
> +inline bool need_suspend(void) { return true; } #endif
> +
> +static int iTCO_wdt_suspend_noirq(struct device *dev) {
> + int ret = 0;
> +
> + iTCO_wdt_private.suspended = false;
> + if (watchdog_active(&iTCO_wdt_watchdog_dev) && need_suspend()) {
> + ret = iTCO_wdt_stop(&iTCO_wdt_watchdog_dev);
> + if (!ret)
> + iTCO_wdt_private.suspended = true;
> + }
> + return ret;
> +}
> +
> +static int iTCO_wdt_resume_noirq(struct device *dev) {
> + if (iTCO_wdt_private.suspended)
> + iTCO_wdt_start(&iTCO_wdt_watchdog_dev);
> +
> + return 0;
> +}
> +
> +struct dev_pm_ops iTCO_wdt_pm = {
> + .suspend_noirq = iTCO_wdt_suspend_noirq,
> + .resume_noirq = iTCO_wdt_resume_noirq, };
> +
> +#define ITCO_WDT_PM_OPS &iTCO_wdt_pm
> +#else
> +#define ITCO_WDT_PM_OPS NULL
> +#endif /* CONFIG_PM_SLEEP */
> +
> static struct platform_driver iTCO_wdt_driver = {
> .probe = iTCO_wdt_probe,
> .remove = iTCO_wdt_remove,
> .shutdown = iTCO_wdt_shutdown,
> .driver = {
> .name = DRV_NAME,
> + .pm = ITCO_WDT_PM_OPS,
> },
> };
>
>
> N�����r��y���b�X��ǧv�^�){.n�+����{���\�� �{ay�\x1dʇڙ�,j\a��f���h���z�\x1e�w���\f���j:+v���w�j�m����\a����zZ+�����ݢj"��!tml=
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] PM / watchdog: iTCO: stop watchdog during system suspend
2015-04-02 0:31 [PATCH] PM / watchdog: iTCO: stop watchdog during system suspend Rafael J. Wysocki
2015-04-02 0:52 ` Fu, Borun
@ 2015-04-02 1:21 ` Guenter Roeck
2015-04-02 22:04 ` Rafael J. Wysocki
1 sibling, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2015-04-02 1:21 UTC (permalink / raw)
To: Rafael J. Wysocki, linux-watchdog
Cc: Linux Kernel Mailing List, Linux PM list, Wim Van Sebroeck,
Aubrey Li, Borun Fu
On 04/01/2015 05:31 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> If the target sleep state of the system is not an ACPI sleep state
> (S1, S2 or S3), the TCO watchdog needs to be stopped during system
> suspend, because it may not be possible to ping it any more after
> timekeeping has been suspended (suspend-to-idle does that for
> one example).
>
> For this reason, provide ->suspend_noirq and ->resume_noirq
> callbacks for the iTCO watchdog driver and use them to stop
> and restart the watchdog during system suspend and resume,
> respectively, if the system is not going to enter an ACPI
> sleep state (in which case the watchdog will be stopped
> by the platform firmware before the state is entered).
>
> Reported-by: Borun Fu <borun.fu@intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/watchdog/iTCO_wdt.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> Index: linux-pm/drivers/watchdog/iTCO_wdt.c
> ===================================================================
> --- linux-pm.orig/drivers/watchdog/iTCO_wdt.c
> +++ linux-pm/drivers/watchdog/iTCO_wdt.c
> @@ -51,6 +51,7 @@
> #define DRV_VERSION "1.11"
>
> /* Includes */
> +#include <linux/acpi.h> /* For ACPI support */
> #include <linux/module.h> /* For module specific items */
> #include <linux/moduleparam.h> /* For new moduleparam's */
> #include <linux/types.h> /* For standard types (like size_t) */
> @@ -103,6 +104,8 @@ static struct { /* this is private data
> struct platform_device *dev;
> /* the PCI-device */
> struct pci_dev *pdev;
> + /* whether or not the watchdog has been suspended */
> + bool suspended;
> } iTCO_wdt_private;
>
> /* module parameters */
> @@ -571,12 +574,60 @@ static void iTCO_wdt_shutdown(struct pla
> iTCO_wdt_stop(NULL);
> }
>
> +#ifdef CONFIG_PM_SLEEP
> +/*
> + * Suspend-to-idle requires this, because it stops the ticks and timekeeping, so
> + * the watchdog cannot be pinged while in that state. In ACPI sleep states the
> + * watchdog is stopped by the platform firmware.
> + */
> +
> +#ifdef CONFIG_ACPI
> +static inline bool need_suspend(void)
> +{
> + return acpi_target_system_state() == ACPI_STATE_S0;
> +}
> +#else
> +static inline bool need_suspend(void) { return true; }
> +#endif
> +
> +static int iTCO_wdt_suspend_noirq(struct device *dev)
> +{
> + int ret = 0;
> +
> + iTCO_wdt_private.suspended = false;
> + if (watchdog_active(&iTCO_wdt_watchdog_dev) && need_suspend()) {
> + ret = iTCO_wdt_stop(&iTCO_wdt_watchdog_dev);
> + if (!ret)
> + iTCO_wdt_private.suspended = true;
> + }
> + return ret;
> +}
> +
> +static int iTCO_wdt_resume_noirq(struct device *dev)
> +{
> + if (iTCO_wdt_private.suspended)
> + iTCO_wdt_start(&iTCO_wdt_watchdog_dev);
> +
> + return 0;
> +}
> +
> +struct dev_pm_ops iTCO_wdt_pm = {
> + .suspend_noirq = iTCO_wdt_suspend_noirq,
> + .resume_noirq = iTCO_wdt_resume_noirq,
Hi Rafael,
This only covers suspend and resume, but not any of the other
sleep operations (like SIMPLE_DEV_PM_OPS would do).
Is that intentional ?
> +};
> +
> +#define ITCO_WDT_PM_OPS &iTCO_wdt_pm
Checkpatch wants to see (&iTCO_wdt_pm).
ERROR: Macros with complex values should be enclosed in parentheses
Thanks,
Guenter
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] PM / watchdog: iTCO: stop watchdog during system suspend
2015-04-02 1:11 ` Guenter Roeck
@ 2015-04-02 2:13 ` Fu, Borun
0 siblings, 0 replies; 9+ messages in thread
From: Fu, Borun @ 2015-04-02 2:13 UTC (permalink / raw)
To: Guenter Roeck, Rafael J. Wysocki, linux-watchdog
Cc: Linux Kernel Mailing List, Linux PM list, Wim Van Sebroeck, Li, Aubrey
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4921 bytes --]
Sorry, my fault. It should work as well.
-----Original Message-----
From: Guenter Roeck [mailto:linux@roeck-us.net]
Sent: Thursday, April 02, 2015 9:11
To: Fu, Borun; Rafael J. Wysocki; linux-watchdog@vger.kernel.org
Cc: Linux Kernel Mailing List; Linux PM list; Wim Van Sebroeck; Li, Aubrey; Wang, Frank
Subject: Re: [PATCH] PM / watchdog: iTCO: stop watchdog during system suspend
On 04/01/2015 05:52 PM, Fu, Borun wrote:
> Hi Rafael,
> watchdog_active() function is not implemented in your patch. Pls. add it.
>
Please explain why using the standard watchdog_active() would not work here.
Guenter
> static int watchdog_active(struct device *dev) {
> unsigned int val;
>
> spin_lock(&iTCO_wdt_private.io_lock);
> val = inw(TCO1_CNT);
> spin_unlock(&iTCO_wdt_private.io_lock);
>
> /* Bit 11: TCO Timer Halt */
> if (val & 0x0800)
> return 0;
> else
> return 1;
> }
>
> Regards,
> Borun
> -----Original Message-----
> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> Sent: Thursday, April 02, 2015 8:31
> To: linux-watchdog@vger.kernel.org
> Cc: Linux Kernel Mailing List; Linux PM list; Wim Van Sebroeck; Li,
> Aubrey; Fu, Borun
> Subject: [PATCH] PM / watchdog: iTCO: stop watchdog during system
> suspend
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> If the target sleep state of the system is not an ACPI sleep state (S1, S2 or S3), the TCO watchdog needs to be stopped during system suspend, because it may not be possible to ping it any more after timekeeping has been suspended (suspend-to-idle does that for one example).
>
> For this reason, provide ->suspend_noirq and ->resume_noirq callbacks for the iTCO watchdog driver and use them to stop and restart the watchdog during system suspend and resume, respectively, if the system is not going to enter an ACPI sleep state (in which case the watchdog will be stopped by the platform firmware before the state is entered).
>
> Reported-by: Borun Fu <borun.fu@intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/watchdog/iTCO_wdt.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> Index: linux-pm/drivers/watchdog/iTCO_wdt.c
> ===================================================================
> --- linux-pm.orig/drivers/watchdog/iTCO_wdt.c
> +++ linux-pm/drivers/watchdog/iTCO_wdt.c
> @@ -51,6 +51,7 @@
> #define DRV_VERSION "1.11"
>
> /* Includes */
> +#include <linux/acpi.h> /* For ACPI support */
> #include <linux/module.h> /* For module specific items */
> #include <linux/moduleparam.h> /* For new moduleparam's */
> #include <linux/types.h> /* For standard types (like size_t) */
> @@ -103,6 +104,8 @@ static struct { /* this is private data
> struct platform_device *dev;
> /* the PCI-device */
> struct pci_dev *pdev;
> + /* whether or not the watchdog has been suspended */
> + bool suspended;
> } iTCO_wdt_private;
>
> /* module parameters */
> @@ -571,12 +574,60 @@ static void iTCO_wdt_shutdown(struct pla
> iTCO_wdt_stop(NULL);
> }
>
> +#ifdef CONFIG_PM_SLEEP
> +/*
> + * Suspend-to-idle requires this, because it stops the ticks and
> +timekeeping, so
> + * the watchdog cannot be pinged while in that state. In ACPI sleep
> +states the
> + * watchdog is stopped by the platform firmware.
> + */
> +
> +#ifdef CONFIG_ACPI
> +static inline bool need_suspend(void) {
> + return acpi_target_system_state() == ACPI_STATE_S0; } #else static
> +inline bool need_suspend(void) { return true; } #endif
> +
> +static int iTCO_wdt_suspend_noirq(struct device *dev) {
> + int ret = 0;
> +
> + iTCO_wdt_private.suspended = false;
> + if (watchdog_active(&iTCO_wdt_watchdog_dev) && need_suspend()) {
> + ret = iTCO_wdt_stop(&iTCO_wdt_watchdog_dev);
> + if (!ret)
> + iTCO_wdt_private.suspended = true;
> + }
> + return ret;
> +}
> +
> +static int iTCO_wdt_resume_noirq(struct device *dev) {
> + if (iTCO_wdt_private.suspended)
> + iTCO_wdt_start(&iTCO_wdt_watchdog_dev);
> +
> + return 0;
> +}
> +
> +struct dev_pm_ops iTCO_wdt_pm = {
> + .suspend_noirq = iTCO_wdt_suspend_noirq,
> + .resume_noirq = iTCO_wdt_resume_noirq, };
> +
> +#define ITCO_WDT_PM_OPS &iTCO_wdt_pm
> +#else
> +#define ITCO_WDT_PM_OPS NULL
> +#endif /* CONFIG_PM_SLEEP */
> +
> static struct platform_driver iTCO_wdt_driver = {
> .probe = iTCO_wdt_probe,
> .remove = iTCO_wdt_remove,
> .shutdown = iTCO_wdt_shutdown,
> .driver = {
> .name = DRV_NAME,
> + .pm = ITCO_WDT_PM_OPS,
> },
> };
>
>
> N r y b X ǧv ^ )Þº{.n + { \ {ay \x1dÊÚ ,j
> f h z \x1e w
j:+v w j m zZ+ ݢj" !tml=
>
ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] PM / watchdog: iTCO: stop watchdog during system suspend
2015-04-02 1:21 ` Guenter Roeck
@ 2015-04-02 22:04 ` Rafael J. Wysocki
2015-04-03 2:53 ` Fu, Borun
2015-04-03 5:48 ` Guenter Roeck
0 siblings, 2 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2015-04-02 22:04 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-watchdog, Linux Kernel Mailing List, Linux PM list,
Wim Van Sebroeck, Aubrey Li, Borun Fu
On Wednesday, April 01, 2015 06:21:40 PM Guenter Roeck wrote:
> On 04/01/2015 05:31 PM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
[cut]
> Hi Rafael,
>
> This only covers suspend and resume, but not any of the other
> sleep operations (like SIMPLE_DEV_PM_OPS would do).
> Is that intentional ?
Yes, it is:
(1) The problemm at hand is strictly specific to system suspend.
(2) The SIMPLE_DEV_PM_OPS etc macros don't cover the _noirq callback variants
(and we want to use the _noirq variants, because we want the watchdog to
be stopped as late and restarted as early as reasonably possible).
(3) I'm not even sure if adding runtime PM support to this driver would make
any sense. :-)
>
> > +};
> > +
> > +#define ITCO_WDT_PM_OPS &iTCO_wdt_pm
>
> Checkpatch wants to see (&iTCO_wdt_pm).
>
> ERROR: Macros with complex values should be enclosed in parentheses
Please find an updated patch below.
Rafael
---
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: PM / watchdog: iTCO: stop watchdog during system suspend
If the target sleep state of the system is not an ACPI sleep state
(S1, S2 or S3), the TCO watchdog needs to be stopped during system
suspend, because it may not be possible to ping it any more after
timekeeping has been suspended (suspend-to-idle does that for
one example).
For this reason, provide ->suspend_noirq and ->resume_noirq
callbacks for the iTCO watchdog driver and use them to stop
and restart the watchdog during system suspend and resume,
respectively, if the system is not going to enter an ACPI
sleep state (in which case the watchdog will be stopped
by the platform firmware before the state is entered).
Reported-by: Borun Fu <borun.fu@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/watchdog/iTCO_wdt.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
Index: linux-pm/drivers/watchdog/iTCO_wdt.c
===================================================================
--- linux-pm.orig/drivers/watchdog/iTCO_wdt.c
+++ linux-pm/drivers/watchdog/iTCO_wdt.c
@@ -51,6 +51,7 @@
#define DRV_VERSION "1.11"
/* Includes */
+#include <linux/acpi.h> /* For ACPI support */
#include <linux/module.h> /* For module specific items */
#include <linux/moduleparam.h> /* For new moduleparam's */
#include <linux/types.h> /* For standard types (like size_t) */
@@ -103,6 +104,8 @@ static struct { /* this is private data
struct platform_device *dev;
/* the PCI-device */
struct pci_dev *pdev;
+ /* whether or not the watchdog has been suspended */
+ bool suspended;
} iTCO_wdt_private;
/* module parameters */
@@ -571,12 +574,60 @@ static void iTCO_wdt_shutdown(struct pla
iTCO_wdt_stop(NULL);
}
+#ifdef CONFIG_PM_SLEEP
+/*
+ * Suspend-to-idle requires this, because it stops the ticks and timekeeping, so
+ * the watchdog cannot be pinged while in that state. In ACPI sleep states the
+ * watchdog is stopped by the platform firmware.
+ */
+
+#ifdef CONFIG_ACPI
+static inline bool need_suspend(void)
+{
+ return acpi_target_system_state() == ACPI_STATE_S0;
+}
+#else
+static inline bool need_suspend(void) { return true; }
+#endif
+
+static int iTCO_wdt_suspend_noirq(struct device *dev)
+{
+ int ret = 0;
+
+ iTCO_wdt_private.suspended = false;
+ if (watchdog_active(&iTCO_wdt_watchdog_dev) && need_suspend()) {
+ ret = iTCO_wdt_stop(&iTCO_wdt_watchdog_dev);
+ if (!ret)
+ iTCO_wdt_private.suspended = true;
+ }
+ return ret;
+}
+
+static int iTCO_wdt_resume_noirq(struct device *dev)
+{
+ if (iTCO_wdt_private.suspended)
+ iTCO_wdt_start(&iTCO_wdt_watchdog_dev);
+
+ return 0;
+}
+
+struct dev_pm_ops iTCO_wdt_pm = {
+ .suspend_noirq = iTCO_wdt_suspend_noirq,
+ .resume_noirq = iTCO_wdt_resume_noirq,
+};
+
+#define ITCO_WDT_PM_OPS (&iTCO_wdt_pm)
+#else
+#define ITCO_WDT_PM_OPS NULL
+#endif /* CONFIG_PM_SLEEP */
+
static struct platform_driver iTCO_wdt_driver = {
.probe = iTCO_wdt_probe,
.remove = iTCO_wdt_remove,
.shutdown = iTCO_wdt_shutdown,
.driver = {
.name = DRV_NAME,
+ .pm = ITCO_WDT_PM_OPS,
},
};
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] PM / watchdog: iTCO: stop watchdog during system suspend
2015-04-02 22:04 ` Rafael J. Wysocki
@ 2015-04-03 2:53 ` Fu, Borun
2015-04-03 5:48 ` Guenter Roeck
1 sibling, 0 replies; 9+ messages in thread
From: Fu, Borun @ 2015-04-03 2:53 UTC (permalink / raw)
To: Rafael J. Wysocki, Guenter Roeck
Cc: linux-watchdog, Linux Kernel Mailing List, Linux PM list,
Wim Van Sebroeck, Li, Aubrey
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4595 bytes --]
On Apr 3, 2015 06:04, Rafael J. Wysocki wrote:
> On Wednesday, April 01, 2015 06:21:40 PM Guenter Roeck wrote:
>> On 04/01/2015 05:31 PM, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> [cut]
>
>> Hi Rafael,
>>
>> This only covers suspend and resume, but not any of the other
>> sleep operations (like SIMPLE_DEV_PM_OPS would do).
>> Is that intentional ?
>
> Yes, it is:
> (1) The problemm at hand is strictly specific to system suspend.
> (2) The SIMPLE_DEV_PM_OPS etc macros don't cover the _noirq callback
> variants
> (and we want to use the _noirq variants, because we want the
> watchdog to be stopped as late and restarted as early as reasonably
> possible). (3) I'm not even sure if adding runtime PM support to
> this driver would make any sense. :-)
>>
>>> +};
>>> +
>>> +#define ITCO_WDT_PM_OPS &iTCO_wdt_pm
>>
>> Checkpatch wants to see (&iTCO_wdt_pm).
>>
>> ERROR: Macros with complex values should be enclosed in parentheses
>
> Please find an updated patch below.
>
> Rafael
>
>
Tested-by: Borun Fu <borun.fu@intel.com>
Borun
> ---
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Subject: PM / watchdog: iTCO: stop watchdog during system suspend
>
> If the target sleep state of the system is not an ACPI sleep state
> (S1, S2 or S3), the TCO watchdog needs to be stopped during system
> suspend, because it may not be possible to ping it any more after
> timekeeping has been suspended (suspend-to-idle does that for
> one example).
>
> For this reason, provide ->suspend_noirq and ->resume_noirq
> callbacks for the iTCO watchdog driver and use them to stop
> and restart the watchdog during system suspend and resume,
> respectively, if the system is not going to enter an ACPI
> sleep state (in which case the watchdog will be stopped
> by the platform firmware before the state is entered).
>
> Reported-by: Borun Fu <borun.fu@intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/watchdog/iTCO_wdt.c | 51
> ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51
> insertions(+)
> Index: linux-pm/drivers/watchdog/iTCO_wdt.c
> ================================================================ === ---
> linux-pm.orig/drivers/watchdog/iTCO_wdt.c +++
> linux-pm/drivers/watchdog/iTCO_wdt.c @@ -51,6 +51,7 @@
> #define DRV_VERSION "1.11"
>
> /* Includes */ +#include <linux/acpi.h> /* For ACPI support */
> #include <linux/module.h> /* For module specific items */ #include
> <linux/moduleparam.h> /* For new moduleparam's */ #include
> <linux/types.h> /* For standard types (like size_t) */ @@ -103,6
> +104,8 @@ static struct { /* this is private data struct
> platform_device *dev; /* the PCI-device */ struct pci_dev *pdev;
> + /* whether or not the watchdog has been suspended */
> + bool suspended;
> } iTCO_wdt_private;
>
> /* module parameters */ @@ -571,12 +574,60 @@ static void
> iTCO_wdt_shutdown(struct pla iTCO_wdt_stop(NULL); }
> +#ifdef CONFIG_PM_SLEEP +/* + * Suspend-to-idle requires this, because
> it stops the ticks and timekeeping, so + * the watchdog cannot be pinged
> while in that state. In ACPI sleep states the + * watchdog is stopped
> by the platform firmware. + */ + +#ifdef CONFIG_ACPI +static inline bool
> need_suspend(void) +{ + return acpi_target_system_state() ==
> ACPI_STATE_S0; +} +#else +static inline bool need_suspend(void) { return
> true; } +#endif + +static int iTCO_wdt_suspend_noirq(struct device *dev)
> +{ + int ret = 0; + + iTCO_wdt_private.suspended = false; + if
> (watchdog_active(&iTCO_wdt_watchdog_dev) && need_suspend()) { + ret =
> iTCO_wdt_stop(&iTCO_wdt_watchdog_dev); + if (!ret)
> + iTCO_wdt_private.suspended = true; + } + return ret; +} + +static
> int iTCO_wdt_resume_noirq(struct device *dev) +{ + if
> (iTCO_wdt_private.suspended) + iTCO_wdt_start(&iTCO_wdt_watchdog_dev);
> + + return 0; +} + +struct dev_pm_ops iTCO_wdt_pm = { + .suspend_noirq =
> iTCO_wdt_suspend_noirq, + .resume_noirq = iTCO_wdt_resume_noirq, +}; +
> +#define ITCO_WDT_PM_OPS (&iTCO_wdt_pm) +#else +#define
> ITCO_WDT_PM_OPS NULL +#endif /* CONFIG_PM_SLEEP */ +
> static struct platform_driver iTCO_wdt_driver = { .probe =
> iTCO_wdt_probe, .remove = iTCO_wdt_remove, .shutdown =
> iTCO_wdt_shutdown, .driver = { .name = DRV_NAME, + .pm
> = ITCO_WDT_PM_OPS, }, };
ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] PM / watchdog: iTCO: stop watchdog during system suspend
2015-04-02 22:04 ` Rafael J. Wysocki
2015-04-03 2:53 ` Fu, Borun
@ 2015-04-03 5:48 ` Guenter Roeck
2015-04-03 11:04 ` Rafael J. Wysocki
1 sibling, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2015-04-03 5:48 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-watchdog, Linux Kernel Mailing List, Linux PM list,
Wim Van Sebroeck, Aubrey Li, Borun Fu
On 04/02/2015 03:04 PM, Rafael J. Wysocki wrote:
[ ... ]
>
> ---
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Subject: PM / watchdog: iTCO: stop watchdog during system suspend
>
> If the target sleep state of the system is not an ACPI sleep state
> (S1, S2 or S3), the TCO watchdog needs to be stopped during system
> suspend, because it may not be possible to ping it any more after
> timekeeping has been suspended (suspend-to-idle does that for
> one example).
>
> For this reason, provide ->suspend_noirq and ->resume_noirq
> callbacks for the iTCO watchdog driver and use them to stop
> and restart the watchdog during system suspend and resume,
> respectively, if the system is not going to enter an ACPI
> sleep state (in which case the watchdog will be stopped
> by the platform firmware before the state is entered).
>
> Reported-by: Borun Fu <borun.fu@intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/watchdog/iTCO_wdt.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> Index: linux-pm/drivers/watchdog/iTCO_wdt.c
> ===================================================================
> --- linux-pm.orig/drivers/watchdog/iTCO_wdt.c
> +++ linux-pm/drivers/watchdog/iTCO_wdt.c
> @@ -51,6 +51,7 @@
> #define DRV_VERSION "1.11"
>
> /* Includes */
> +#include <linux/acpi.h> /* For ACPI support */
> #include <linux/module.h> /* For module specific items */
> #include <linux/moduleparam.h> /* For new moduleparam's */
> #include <linux/types.h> /* For standard types (like size_t) */
> @@ -103,6 +104,8 @@ static struct { /* this is private data
> struct platform_device *dev;
> /* the PCI-device */
> struct pci_dev *pdev;
> + /* whether or not the watchdog has been suspended */
> + bool suspended;
> } iTCO_wdt_private;
>
> /* module parameters */
> @@ -571,12 +574,60 @@ static void iTCO_wdt_shutdown(struct pla
> iTCO_wdt_stop(NULL);
> }
>
> +#ifdef CONFIG_PM_SLEEP
> +/*
> + * Suspend-to-idle requires this, because it stops the ticks and timekeeping, so
> + * the watchdog cannot be pinged while in that state. In ACPI sleep states the
> + * watchdog is stopped by the platform firmware.
> + */
> +
> +#ifdef CONFIG_ACPI
> +static inline bool need_suspend(void)
> +{
> + return acpi_target_system_state() == ACPI_STATE_S0;
> +}
> +#else
> +static inline bool need_suspend(void) { return true; }
> +#endif
> +
> +static int iTCO_wdt_suspend_noirq(struct device *dev)
> +{
> + int ret = 0;
> +
> + iTCO_wdt_private.suspended = false;
> + if (watchdog_active(&iTCO_wdt_watchdog_dev) && need_suspend()) {
> + ret = iTCO_wdt_stop(&iTCO_wdt_watchdog_dev);
> + if (!ret)
> + iTCO_wdt_private.suspended = true;
> + }
> + return ret;
> +}
> +
> +static int iTCO_wdt_resume_noirq(struct device *dev)
> +{
> + if (iTCO_wdt_private.suspended)
> + iTCO_wdt_start(&iTCO_wdt_watchdog_dev);
> +
> + return 0;
> +}
> +
> +struct dev_pm_ops iTCO_wdt_pm = {
> + .suspend_noirq = iTCO_wdt_suspend_noirq,
> + .resume_noirq = iTCO_wdt_resume_noirq,
> +};
> +
> +#define ITCO_WDT_PM_OPS (&iTCO_wdt_pm)
> +#else
> +#define ITCO_WDT_PM_OPS NULL
> +#endif /* CONFIG_PM_SLEEP */
> +
> static struct platform_driver iTCO_wdt_driver = {
> .probe = iTCO_wdt_probe,
> .remove = iTCO_wdt_remove,
> .shutdown = iTCO_wdt_shutdown,
> .driver = {
> .name = DRV_NAME,
> + .pm = ITCO_WDT_PM_OPS,
> },
> };
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] PM / watchdog: iTCO: stop watchdog during system suspend
2015-04-03 5:48 ` Guenter Roeck
@ 2015-04-03 11:04 ` Rafael J. Wysocki
0 siblings, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2015-04-03 11:04 UTC (permalink / raw)
To: Guenter Roeck, Wim Van Sebroeck
Cc: linux-watchdog, Linux Kernel Mailing List, Linux PM list,
Aubrey Li, Borun Fu
On Thursday, April 02, 2015 10:48:39 PM Guenter Roeck wrote:
> On 04/02/2015 03:04 PM, Rafael J. Wysocki wrote:
> [ ... ]
> >
> > ---
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Subject: PM / watchdog: iTCO: stop watchdog during system suspend
> >
> > If the target sleep state of the system is not an ACPI sleep state
> > (S1, S2 or S3), the TCO watchdog needs to be stopped during system
> > suspend, because it may not be possible to ping it any more after
> > timekeeping has been suspended (suspend-to-idle does that for
> > one example).
> >
> > For this reason, provide ->suspend_noirq and ->resume_noirq
> > callbacks for the iTCO watchdog driver and use them to stop
> > and restart the watchdog during system suspend and resume,
> > respectively, if the system is not going to enter an ACPI
> > sleep state (in which case the watchdog will be stopped
> > by the platform firmware before the state is entered).
> >
> > Reported-by: Borun Fu <borun.fu@intel.com>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Thanks!
I'll take it into the PM tree, unless anyone objects.
Rafael
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-04-03 10:40 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-02 0:31 [PATCH] PM / watchdog: iTCO: stop watchdog during system suspend Rafael J. Wysocki
2015-04-02 0:52 ` Fu, Borun
2015-04-02 1:11 ` Guenter Roeck
2015-04-02 2:13 ` Fu, Borun
2015-04-02 1:21 ` Guenter Roeck
2015-04-02 22:04 ` Rafael J. Wysocki
2015-04-03 2:53 ` Fu, Borun
2015-04-03 5:48 ` Guenter Roeck
2015-04-03 11:04 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).