linux-watchdog.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] watchdog: iTCO_wdt: Full reinitialize on resume
@ 2021-09-28 16:53 Thomas Weißschuh
  2021-10-02 13:23 ` Guenter Roeck
  0 siblings, 1 reply; 3+ messages in thread
From: Thomas Weißschuh @ 2021-09-28 16:53 UTC (permalink / raw)
  To: Rafael J . Wysocki, Wim Van Sebroeck, Guenter Roeck, linux-watchdog
  Cc: Thomas Weißschuh, linux-kernel

The Thinkpad T460s always needs driver-side suspend-resume handling.
If the watchdog is not stopped before suspend then the system will hang
on resume.
If the interval is not set before starting the watchdog then the machine
will instantly be reset after resume.

Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=198019

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/watchdog/iTCO_wdt.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index 643c6c2d0b72..2297a0a1e5fc 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -47,6 +47,7 @@
 /* Includes */
 #include <linux/acpi.h>			/* For ACPI support */
 #include <linux/bits.h>			/* For BIT() */
+#include <linux/dmi.h>			/* For DMI matching */
 #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) */
@@ -605,9 +606,20 @@ static int iTCO_wdt_probe(struct platform_device *pdev)
  */
 
 #ifdef CONFIG_ACPI
+static const struct dmi_system_id iTCO_wdt_force_suspend[] = {
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_PRODUCT_FAMILY, "ThinkPad T460s"),
+		},
+	},
+	{ },
+};
+
 static inline bool need_suspend(void)
 {
-	return acpi_target_system_state() == ACPI_STATE_S0;
+	return acpi_target_system_state() == ACPI_STATE_S0 ||
+		dmi_check_system(iTCO_wdt_force_suspend);
 }
 #else
 static inline bool need_suspend(void) { return true; }
@@ -631,8 +643,10 @@ static int iTCO_wdt_resume_noirq(struct device *dev)
 {
 	struct iTCO_wdt_private *p = dev_get_drvdata(dev);
 
-	if (p->suspended)
+	if (p->suspended) {
+		iTCO_wdt_set_timeout(&p->wddev, p->wddev.timeout);
 		iTCO_wdt_start(&p->wddev);
+	}
 
 	return 0;
 }

base-commit: 41e73feb1024929e75eaf2f7cd93f35a3feb331b
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] watchdog: iTCO_wdt: Full reinitialize on resume
  2021-09-28 16:53 [PATCH] watchdog: iTCO_wdt: Full reinitialize on resume Thomas Weißschuh
@ 2021-10-02 13:23 ` Guenter Roeck
  2021-10-02 13:43   ` Thomas Weißschuh
  0 siblings, 1 reply; 3+ messages in thread
From: Guenter Roeck @ 2021-10-02 13:23 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Rafael J . Wysocki, Wim Van Sebroeck, linux-watchdog, linux-kernel

On Tue, Sep 28, 2021 at 06:53:43PM +0200, Thomas Weißschuh wrote:
> The Thinkpad T460s always needs driver-side suspend-resume handling.
> If the watchdog is not stopped before suspend then the system will hang
> on resume.
> If the interval is not set before starting the watchdog then the machine
> will instantly be reset after resume.
> 
> Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=198019

The Fixes: tag references a SHA, not a bugzilla bug.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>  drivers/watchdog/iTCO_wdt.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> index 643c6c2d0b72..2297a0a1e5fc 100644
> --- a/drivers/watchdog/iTCO_wdt.c
> +++ b/drivers/watchdog/iTCO_wdt.c
> @@ -47,6 +47,7 @@
>  /* Includes */
>  #include <linux/acpi.h>			/* For ACPI support */
>  #include <linux/bits.h>			/* For BIT() */
> +#include <linux/dmi.h>			/* For DMI matching */
>  #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) */
> @@ -605,9 +606,20 @@ static int iTCO_wdt_probe(struct platform_device *pdev)
>   */
>  
>  #ifdef CONFIG_ACPI
> +static const struct dmi_system_id iTCO_wdt_force_suspend[] = {
> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> +			DMI_MATCH(DMI_PRODUCT_FAMILY, "ThinkPad T460s"),
> +		},
> +	},
> +	{ },
> +};
> +
>  static inline bool need_suspend(void)
>  {
> -	return acpi_target_system_state() == ACPI_STATE_S0;
> +	return acpi_target_system_state() == ACPI_STATE_S0 ||
> +		dmi_check_system(iTCO_wdt_force_suspend);
>  }
>  #else
>  static inline bool need_suspend(void) { return true; }
> @@ -631,8 +643,10 @@ static int iTCO_wdt_resume_noirq(struct device *dev)
>  {
>  	struct iTCO_wdt_private *p = dev_get_drvdata(dev);
>  
> -	if (p->suspended)
> +	if (p->suspended) {
> +		iTCO_wdt_set_timeout(&p->wddev, p->wddev.timeout);
>  		iTCO_wdt_start(&p->wddev);
> +	}
>  
>  	return 0;
>  }
> 
> base-commit: 41e73feb1024929e75eaf2f7cd93f35a3feb331b
> -- 
> 2.33.0
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] watchdog: iTCO_wdt: Full reinitialize on resume
  2021-10-02 13:23 ` Guenter Roeck
@ 2021-10-02 13:43   ` Thomas Weißschuh
  0 siblings, 0 replies; 3+ messages in thread
From: Thomas Weißschuh @ 2021-10-02 13:43 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Rafael J . Wysocki, Wim Van Sebroeck, Mark Pearson,
	linux-watchdog, linux-kernel

On 2021-10-02T06:23-0700, Guenter Roeck wrote:
> On Tue, Sep 28, 2021 at 06:53:43PM +0200, Thomas Weißschuh wrote:
> > The Thinkpad T460s always needs driver-side suspend-resume handling.
> > If the watchdog is not stopped before suspend then the system will hang
> > on resume.
> > If the interval is not set before starting the watchdog then the machine
> > will instantly be reset after resume.
> > 
> > Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=198019
> 
> The Fixes: tag references a SHA, not a bugzilla bug.

Thanks, I'll change that for v2.

> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > ---
> >  drivers/watchdog/iTCO_wdt.c | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> > index 643c6c2d0b72..2297a0a1e5fc 100644
> > --- a/drivers/watchdog/iTCO_wdt.c
> > +++ b/drivers/watchdog/iTCO_wdt.c
> > @@ -47,6 +47,7 @@
> >  /* Includes */
> >  #include <linux/acpi.h>			/* For ACPI support */
> >  #include <linux/bits.h>			/* For BIT() */
> > +#include <linux/dmi.h>			/* For DMI matching */
> >  #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) */
> > @@ -605,9 +606,20 @@ static int iTCO_wdt_probe(struct platform_device *pdev)
> >   */
> >  
> >  #ifdef CONFIG_ACPI
> > +static const struct dmi_system_id iTCO_wdt_force_suspend[] = {
> > +	{
> > +		.matches = {
> > +			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> > +			DMI_MATCH(DMI_PRODUCT_FAMILY, "ThinkPad T460s"),
> > +		},
> > +	},
> > +	{ },
> > +};
> > +
> >  static inline bool need_suspend(void)
> >  {
> > -	return acpi_target_system_state() == ACPI_STATE_S0;
> > +	return acpi_target_system_state() == ACPI_STATE_S0 ||
> > +		dmi_check_system(iTCO_wdt_force_suspend);
> >  }
> >  #else
> >  static inline bool need_suspend(void) { return true; }
> > @@ -631,8 +643,10 @@ static int iTCO_wdt_resume_noirq(struct device *dev)
> >  {
> >  	struct iTCO_wdt_private *p = dev_get_drvdata(dev);
> >  
> > -	if (p->suspended)
> > +	if (p->suspended) {
> > +		iTCO_wdt_set_timeout(&p->wddev, p->wddev.timeout);
> >  		iTCO_wdt_start(&p->wddev);
> > +	}
> >  
> >  	return 0;
> >  }
> > 
> > base-commit: 41e73feb1024929e75eaf2f7cd93f35a3feb331b
> > -- 
> > 2.33.0

Is the current way with the dmi table the correct way to do it?

I'm also CC-ing Mark Person from Lenovo who may be able to take a look and
ask their firmware team to maybe fix this on their side.

Thomas

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-10-02 13:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-28 16:53 [PATCH] watchdog: iTCO_wdt: Full reinitialize on resume Thomas Weißschuh
2021-10-02 13:23 ` Guenter Roeck
2021-10-02 13:43   ` Thomas Weißschuh

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).