| Message ID | 20200108095704.23233-4-m.felsch@pengutronix.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Explicit disable da9062 watchdog during suspend | expand |
On 08 January 2020 09:57, Marco Felsch wrote: > Disable the watchdog during suspend if it is enabled and re-enable it on > resume. So we can sleep without the interruptions. > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> Reviewed-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com> > --- > v2: > - add dlg,use-sw-pm check to differentiate between automatic and manual > disabling/enabling. > --- > drivers/watchdog/da9062_wdt.c | 37 > +++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > > diff --git a/drivers/watchdog/da9062_wdt.c b/drivers/watchdog/da9062_wdt.c > index e149e66a6ea9..c9b9d6394525 100644 > --- a/drivers/watchdog/da9062_wdt.c > +++ b/drivers/watchdog/da9062_wdt.c > @@ -15,6 +15,7 @@ > #include <linux/jiffies.h> > #include <linux/mfd/da9062/registers.h> > #include <linux/mfd/da9062/core.h> > +#include <linux/property.h> > #include <linux/regmap.h> > #include <linux/of.h> > > @@ -30,6 +31,7 @@ static const unsigned int wdt_timeout[] = { 0, 2, 4, 8, 16, 32, > 65, 131 }; > struct da9062_watchdog { > struct da9062 *hw; > struct watchdog_device wdtdev; > + bool use_sw_pm; > }; > > static unsigned int da9062_wdt_timeout_to_sel(unsigned int secs) > @@ -198,6 +200,8 @@ static int da9062_wdt_probe(struct platform_device > *pdev) > if (!wdt) > return -ENOMEM; > > + wdt->use_sw_pm = device_property_present(dev, "dlg,use-sw-pm"); > + > wdt->hw = chip; > > wdt->wdtdev.info = &da9062_watchdog_info; > @@ -212,6 +216,7 @@ static int da9062_wdt_probe(struct platform_device > *pdev) > watchdog_set_restart_priority(&wdt->wdtdev, 128); > > watchdog_set_drvdata(&wdt->wdtdev, wdt); > + dev_set_drvdata(dev, &wdt->wdtdev); > > ret = devm_watchdog_register_device(dev, &wdt->wdtdev); > if (ret < 0) > @@ -220,10 +225,42 @@ static int da9062_wdt_probe(struct platform_device > *pdev) > return da9062_wdt_ping(&wdt->wdtdev); > } > > +static int __maybe_unused da9062_wdt_suspend(struct device *dev) > +{ > + struct watchdog_device *wdd = dev_get_drvdata(dev); > + struct da9062_watchdog *wdt = watchdog_get_drvdata(wdd); > + > + if (!wdt->use_sw_pm) > + return 0; > + > + if (watchdog_active(wdd)) > + return da9062_wdt_stop(wdd); > + > + return 0; > +} > + > +static int __maybe_unused da9062_wdt_resume(struct device *dev) > +{ > + struct watchdog_device *wdd = dev_get_drvdata(dev); > + struct da9062_watchdog *wdt = watchdog_get_drvdata(wdd); > + > + if (!wdt->use_sw_pm) > + return 0; > + > + if (watchdog_active(wdd)) > + return da9062_wdt_start(wdd); > + > + return 0; > +} > + > +static SIMPLE_DEV_PM_OPS(da9062_wdt_pm_ops, > + da9062_wdt_suspend, da9062_wdt_resume); > + > static struct platform_driver da9062_wdt_driver = { > .probe = da9062_wdt_probe, > .driver = { > .name = "da9062-watchdog", > + .pm = &da9062_wdt_pm_ops, > .of_match_table = da9062_compatible_id_table, > }, > }; > -- > 2.20.1
On Wed, Jan 08, 2020 at 10:57:04AM +0100, Marco Felsch wrote: > Disable the watchdog during suspend if it is enabled and re-enable it on > resume. So we can sleep without the interruptions. > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > Reviewed-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com> Reviewed-by: Guenter Roeck <linux@roeck-us.net> > --- > v2: > - add dlg,use-sw-pm check to differentiate between automatic and manual > disabling/enabling. > --- > drivers/watchdog/da9062_wdt.c | 37 +++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > > diff --git a/drivers/watchdog/da9062_wdt.c b/drivers/watchdog/da9062_wdt.c > index e149e66a6ea9..c9b9d6394525 100644 > --- a/drivers/watchdog/da9062_wdt.c > +++ b/drivers/watchdog/da9062_wdt.c > @@ -15,6 +15,7 @@ > #include <linux/jiffies.h> > #include <linux/mfd/da9062/registers.h> > #include <linux/mfd/da9062/core.h> > +#include <linux/property.h> > #include <linux/regmap.h> > #include <linux/of.h> > > @@ -30,6 +31,7 @@ static const unsigned int wdt_timeout[] = { 0, 2, 4, 8, 16, 32, 65, 131 }; > struct da9062_watchdog { > struct da9062 *hw; > struct watchdog_device wdtdev; > + bool use_sw_pm; > }; > > static unsigned int da9062_wdt_timeout_to_sel(unsigned int secs) > @@ -198,6 +200,8 @@ static int da9062_wdt_probe(struct platform_device *pdev) > if (!wdt) > return -ENOMEM; > > + wdt->use_sw_pm = device_property_present(dev, "dlg,use-sw-pm"); > + > wdt->hw = chip; > > wdt->wdtdev.info = &da9062_watchdog_info; > @@ -212,6 +216,7 @@ static int da9062_wdt_probe(struct platform_device *pdev) > watchdog_set_restart_priority(&wdt->wdtdev, 128); > > watchdog_set_drvdata(&wdt->wdtdev, wdt); > + dev_set_drvdata(dev, &wdt->wdtdev); > > ret = devm_watchdog_register_device(dev, &wdt->wdtdev); > if (ret < 0) > @@ -220,10 +225,42 @@ static int da9062_wdt_probe(struct platform_device *pdev) > return da9062_wdt_ping(&wdt->wdtdev); > } > > +static int __maybe_unused da9062_wdt_suspend(struct device *dev) > +{ > + struct watchdog_device *wdd = dev_get_drvdata(dev); > + struct da9062_watchdog *wdt = watchdog_get_drvdata(wdd); > + > + if (!wdt->use_sw_pm) > + return 0; > + > + if (watchdog_active(wdd)) > + return da9062_wdt_stop(wdd); > + > + return 0; > +} > + > +static int __maybe_unused da9062_wdt_resume(struct device *dev) > +{ > + struct watchdog_device *wdd = dev_get_drvdata(dev); > + struct da9062_watchdog *wdt = watchdog_get_drvdata(wdd); > + > + if (!wdt->use_sw_pm) > + return 0; > + > + if (watchdog_active(wdd)) > + return da9062_wdt_start(wdd); > + > + return 0; > +} > + > +static SIMPLE_DEV_PM_OPS(da9062_wdt_pm_ops, > + da9062_wdt_suspend, da9062_wdt_resume); > + > static struct platform_driver da9062_wdt_driver = { > .probe = da9062_wdt_probe, > .driver = { > .name = "da9062-watchdog", > + .pm = &da9062_wdt_pm_ops, > .of_match_table = da9062_compatible_id_table, > }, > };
Hi Guenter, On 20-01-23 12:51, Guenter Roeck wrote: > On Wed, Jan 08, 2020 at 10:57:04AM +0100, Marco Felsch wrote: > > Disable the watchdog during suspend if it is enabled and re-enable it on > > resume. So we can sleep without the interruptions. > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > Reviewed-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com> > > Reviewed-by: Guenter Roeck <linux@roeck-us.net> I got an kbuild email so I checked the linux-next master tree. On linux-next this patch isn't used instead the old v1 was used... Regards, Marco > > --- > > v2: > > - add dlg,use-sw-pm check to differentiate between automatic and manual > > disabling/enabling. > > --- > > drivers/watchdog/da9062_wdt.c | 37 +++++++++++++++++++++++++++++++++++ > > 1 file changed, 37 insertions(+) > > > > diff --git a/drivers/watchdog/da9062_wdt.c b/drivers/watchdog/da9062_wdt.c > > index e149e66a6ea9..c9b9d6394525 100644 > > --- a/drivers/watchdog/da9062_wdt.c > > +++ b/drivers/watchdog/da9062_wdt.c > > @@ -15,6 +15,7 @@ > > #include <linux/jiffies.h> > > #include <linux/mfd/da9062/registers.h> > > #include <linux/mfd/da9062/core.h> > > +#include <linux/property.h> > > #include <linux/regmap.h> > > #include <linux/of.h> > > > > @@ -30,6 +31,7 @@ static const unsigned int wdt_timeout[] = { 0, 2, 4, 8, 16, 32, 65, 131 }; > > struct da9062_watchdog { > > struct da9062 *hw; > > struct watchdog_device wdtdev; > > + bool use_sw_pm; > > }; > > > > static unsigned int da9062_wdt_timeout_to_sel(unsigned int secs) > > @@ -198,6 +200,8 @@ static int da9062_wdt_probe(struct platform_device *pdev) > > if (!wdt) > > return -ENOMEM; > > > > + wdt->use_sw_pm = device_property_present(dev, "dlg,use-sw-pm"); > > + > > wdt->hw = chip; > > > > wdt->wdtdev.info = &da9062_watchdog_info; > > @@ -212,6 +216,7 @@ static int da9062_wdt_probe(struct platform_device *pdev) > > watchdog_set_restart_priority(&wdt->wdtdev, 128); > > > > watchdog_set_drvdata(&wdt->wdtdev, wdt); > > + dev_set_drvdata(dev, &wdt->wdtdev); > > > > ret = devm_watchdog_register_device(dev, &wdt->wdtdev); > > if (ret < 0) > > @@ -220,10 +225,42 @@ static int da9062_wdt_probe(struct platform_device *pdev) > > return da9062_wdt_ping(&wdt->wdtdev); > > } > > > > +static int __maybe_unused da9062_wdt_suspend(struct device *dev) > > +{ > > + struct watchdog_device *wdd = dev_get_drvdata(dev); > > + struct da9062_watchdog *wdt = watchdog_get_drvdata(wdd); > > + > > + if (!wdt->use_sw_pm) > > + return 0; > > + > > + if (watchdog_active(wdd)) > > + return da9062_wdt_stop(wdd); > > + > > + return 0; > > +} > > + > > +static int __maybe_unused da9062_wdt_resume(struct device *dev) > > +{ > > + struct watchdog_device *wdd = dev_get_drvdata(dev); > > + struct da9062_watchdog *wdt = watchdog_get_drvdata(wdd); > > + > > + if (!wdt->use_sw_pm) > > + return 0; > > + > > + if (watchdog_active(wdd)) > > + return da9062_wdt_start(wdd); > > + > > + return 0; > > +} > > + > > +static SIMPLE_DEV_PM_OPS(da9062_wdt_pm_ops, > > + da9062_wdt_suspend, da9062_wdt_resume); > > + > > static struct platform_driver da9062_wdt_driver = { > > .probe = da9062_wdt_probe, > > .driver = { > > .name = "da9062-watchdog", > > + .pm = &da9062_wdt_pm_ops, > > .of_match_table = da9062_compatible_id_table, > > }, > > }; >
On 2/6/20 1:00 AM, Marco Felsch wrote: > Hi Guenter, > > On 20-01-23 12:51, Guenter Roeck wrote: >> On Wed, Jan 08, 2020 at 10:57:04AM +0100, Marco Felsch wrote: >>> Disable the watchdog during suspend if it is enabled and re-enable it on >>> resume. So we can sleep without the interruptions. >>> >>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> >>> Reviewed-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com> >> >> Reviewed-by: Guenter Roeck <linux@roeck-us.net> > > I got an kbuild email so I checked the linux-next master tree. On > linux-next this patch isn't used instead the old v1 was used... > FWIW, The subject line of this patch doesn't include "v2". Guenter > Regards, > Marco > >>> --- >>> v2: >>> - add dlg,use-sw-pm check to differentiate between automatic and manual >>> disabling/enabling. >>> --- >>> drivers/watchdog/da9062_wdt.c | 37 +++++++++++++++++++++++++++++++++++ >>> 1 file changed, 37 insertions(+) >>> >>> diff --git a/drivers/watchdog/da9062_wdt.c b/drivers/watchdog/da9062_wdt.c >>> index e149e66a6ea9..c9b9d6394525 100644 >>> --- a/drivers/watchdog/da9062_wdt.c >>> +++ b/drivers/watchdog/da9062_wdt.c >>> @@ -15,6 +15,7 @@ >>> #include <linux/jiffies.h> >>> #include <linux/mfd/da9062/registers.h> >>> #include <linux/mfd/da9062/core.h> >>> +#include <linux/property.h> >>> #include <linux/regmap.h> >>> #include <linux/of.h> >>> >>> @@ -30,6 +31,7 @@ static const unsigned int wdt_timeout[] = { 0, 2, 4, 8, 16, 32, 65, 131 }; >>> struct da9062_watchdog { >>> struct da9062 *hw; >>> struct watchdog_device wdtdev; >>> + bool use_sw_pm; >>> }; >>> >>> static unsigned int da9062_wdt_timeout_to_sel(unsigned int secs) >>> @@ -198,6 +200,8 @@ static int da9062_wdt_probe(struct platform_device *pdev) >>> if (!wdt) >>> return -ENOMEM; >>> >>> + wdt->use_sw_pm = device_property_present(dev, "dlg,use-sw-pm"); >>> + >>> wdt->hw = chip; >>> >>> wdt->wdtdev.info = &da9062_watchdog_info; >>> @@ -212,6 +216,7 @@ static int da9062_wdt_probe(struct platform_device *pdev) >>> watchdog_set_restart_priority(&wdt->wdtdev, 128); >>> >>> watchdog_set_drvdata(&wdt->wdtdev, wdt); >>> + dev_set_drvdata(dev, &wdt->wdtdev); >>> >>> ret = devm_watchdog_register_device(dev, &wdt->wdtdev); >>> if (ret < 0) >>> @@ -220,10 +225,42 @@ static int da9062_wdt_probe(struct platform_device *pdev) >>> return da9062_wdt_ping(&wdt->wdtdev); >>> } >>> >>> +static int __maybe_unused da9062_wdt_suspend(struct device *dev) >>> +{ >>> + struct watchdog_device *wdd = dev_get_drvdata(dev); >>> + struct da9062_watchdog *wdt = watchdog_get_drvdata(wdd); >>> + >>> + if (!wdt->use_sw_pm) >>> + return 0; >>> + >>> + if (watchdog_active(wdd)) >>> + return da9062_wdt_stop(wdd); >>> + >>> + return 0; >>> +} >>> + >>> +static int __maybe_unused da9062_wdt_resume(struct device *dev) >>> +{ >>> + struct watchdog_device *wdd = dev_get_drvdata(dev); >>> + struct da9062_watchdog *wdt = watchdog_get_drvdata(wdd); >>> + >>> + if (!wdt->use_sw_pm) >>> + return 0; >>> + >>> + if (watchdog_active(wdd)) >>> + return da9062_wdt_start(wdd); >>> + >>> + return 0; >>> +} >>> + >>> +static SIMPLE_DEV_PM_OPS(da9062_wdt_pm_ops, >>> + da9062_wdt_suspend, da9062_wdt_resume); >>> + >>> static struct platform_driver da9062_wdt_driver = { >>> .probe = da9062_wdt_probe, >>> .driver = { >>> .name = "da9062-watchdog", >>> + .pm = &da9062_wdt_pm_ops, >>> .of_match_table = da9062_compatible_id_table, >>> }, >>> }; >> >
On 2/6/20 1:00 AM, Marco Felsch wrote: > Hi Guenter, > > On 20-01-23 12:51, Guenter Roeck wrote: >> On Wed, Jan 08, 2020 at 10:57:04AM +0100, Marco Felsch wrote: >>> Disable the watchdog during suspend if it is enabled and re-enable it on >>> resume. So we can sleep without the interruptions. >>> >>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> >>> Reviewed-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com> >> >> Reviewed-by: Guenter Roeck <linux@roeck-us.net> > > I got an kbuild email so I checked the linux-next master tree. On > linux-next this patch isn't used instead the old v1 was used... > Yes, it appears that I picked the wrong version as well, and Wim picked it up from there. But, really, what do you expect if you don't tag your submissions with version numbers ? I would suggest to send a follow-up patch to fix what is in -next. Guenter
On 20-02-06 06:45, Guenter Roeck wrote: > On 2/6/20 1:00 AM, Marco Felsch wrote: > > Hi Guenter, > > > > On 20-01-23 12:51, Guenter Roeck wrote: > > > On Wed, Jan 08, 2020 at 10:57:04AM +0100, Marco Felsch wrote: > > > > Disable the watchdog during suspend if it is enabled and re-enable it on > > > > resume. So we can sleep without the interruptions. > > > > > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > > > Reviewed-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com> > > > > > > Reviewed-by: Guenter Roeck <linux@roeck-us.net> > > > > I got an kbuild email so I checked the linux-next master tree. On > > linux-next this patch isn't used instead the old v1 was used... > > > > Yes, it appears that I picked the wrong version as well, and Wim picked it > up from there. But, really, what do you expect if you don't tag your > submissions with version numbers ? Pls check the cover-letter I said sorry their. As I said it was a mistake. > I would suggest to send a follow-up patch to fix what is in -next. A 'real' patch or should I send a 'fixup!' patch? Regards, Marco > Guenter >
diff --git a/drivers/watchdog/da9062_wdt.c b/drivers/watchdog/da9062_wdt.c index e149e66a6ea9..c9b9d6394525 100644 --- a/drivers/watchdog/da9062_wdt.c +++ b/drivers/watchdog/da9062_wdt.c @@ -15,6 +15,7 @@ #include <linux/jiffies.h> #include <linux/mfd/da9062/registers.h> #include <linux/mfd/da9062/core.h> +#include <linux/property.h> #include <linux/regmap.h> #include <linux/of.h> @@ -30,6 +31,7 @@ static const unsigned int wdt_timeout[] = { 0, 2, 4, 8, 16, 32, 65, 131 }; struct da9062_watchdog { struct da9062 *hw; struct watchdog_device wdtdev; + bool use_sw_pm; }; static unsigned int da9062_wdt_timeout_to_sel(unsigned int secs) @@ -198,6 +200,8 @@ static int da9062_wdt_probe(struct platform_device *pdev) if (!wdt) return -ENOMEM; + wdt->use_sw_pm = device_property_present(dev, "dlg,use-sw-pm"); + wdt->hw = chip; wdt->wdtdev.info = &da9062_watchdog_info; @@ -212,6 +216,7 @@ static int da9062_wdt_probe(struct platform_device *pdev) watchdog_set_restart_priority(&wdt->wdtdev, 128); watchdog_set_drvdata(&wdt->wdtdev, wdt); + dev_set_drvdata(dev, &wdt->wdtdev); ret = devm_watchdog_register_device(dev, &wdt->wdtdev); if (ret < 0) @@ -220,10 +225,42 @@ static int da9062_wdt_probe(struct platform_device *pdev) return da9062_wdt_ping(&wdt->wdtdev); } +static int __maybe_unused da9062_wdt_suspend(struct device *dev) +{ + struct watchdog_device *wdd = dev_get_drvdata(dev); + struct da9062_watchdog *wdt = watchdog_get_drvdata(wdd); + + if (!wdt->use_sw_pm) + return 0; + + if (watchdog_active(wdd)) + return da9062_wdt_stop(wdd); + + return 0; +} + +static int __maybe_unused da9062_wdt_resume(struct device *dev) +{ + struct watchdog_device *wdd = dev_get_drvdata(dev); + struct da9062_watchdog *wdt = watchdog_get_drvdata(wdd); + + if (!wdt->use_sw_pm) + return 0; + + if (watchdog_active(wdd)) + return da9062_wdt_start(wdd); + + return 0; +} + +static SIMPLE_DEV_PM_OPS(da9062_wdt_pm_ops, + da9062_wdt_suspend, da9062_wdt_resume); + static struct platform_driver da9062_wdt_driver = { .probe = da9062_wdt_probe, .driver = { .name = "da9062-watchdog", + .pm = &da9062_wdt_pm_ops, .of_match_table = da9062_compatible_id_table, }, };
Disable the watchdog during suspend if it is enabled and re-enable it on resume. So we can sleep without the interruptions. Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> --- v2: - add dlg,use-sw-pm check to differentiate between automatic and manual disabling/enabling. --- drivers/watchdog/da9062_wdt.c | 37 +++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)