[PATCH] watchdog: PM: use DEFINE_SIMPLE_DEV_PM_OPS and pm_sleep_ptr for dev_pm_ops

[PATCH] watchdog: PM: use DEFINE_SIMPLE_DEV_PM_OPS and pm_sleep_ptr for dev_pm_ops

[Li Jun]

Replace the deprecated SIMPLE_DEV_PM_OPS with DEFINE_SIMPLE_DEV_PM_OPS.
This also drops the __maybe_unused annotations on the suspend/resume
callbacks, since pm_sleep_ptr() inside the new macro already handles
the case where CONFIG_PM_SLEEP is disabled. Wrap the dev_pm_ops
assignment with pm_sleep_ptr() so the entire dev_pm_ops structure
is dropped when CONFIG_PM_SLEEP is disabled, instead of leaving
it as a dead struct with NULL callbacks.

Signed-off-by: Li Jun <lijun01@kylinos.cn>
---
 drivers/watchdog/cadence_wdt.c      | 8 ++++----
 drivers/watchdog/da9062_wdt.c       | 8 ++++----
 drivers/watchdog/keembay_wdt.c      | 8 ++++----
 drivers/watchdog/msc313e_wdt.c      | 8 ++++----
 drivers/watchdog/of_xilinx_wdt.c    | 8 ++++----
 drivers/watchdog/pm8916_wdt.c       | 8 ++++----
 drivers/watchdog/stmp3xxx_rtc_wdt.c | 8 ++++----
 7 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/watchdog/cadence_wdt.c b/drivers/watchdog/cadence_wdt.c
index ad46b33ce5f2..ba2efccef467 100644
--- a/drivers/watchdog/cadence_wdt.c
+++ b/drivers/watchdog/cadence_wdt.c
@@ -359,7 +359,7 @@ static int cdns_wdt_probe(struct platform_device *pdev)
  * @dev: handle to the device structure.
  * Return: 0 always.
  */
-static int __maybe_unused cdns_wdt_suspend(struct device *dev)
+static int cdns_wdt_suspend(struct device *dev)
 {
 	struct cdns_wdt *wdt = dev_get_drvdata(dev);
 
@@ -377,7 +377,7 @@ static int __maybe_unused cdns_wdt_suspend(struct device *dev)
  * @dev: handle to the device structure.
  * Return: 0 on success, errno otherwise.
  */
-static int __maybe_unused cdns_wdt_resume(struct device *dev)
+static int cdns_wdt_resume(struct device *dev)
 {
 	int ret;
 	struct cdns_wdt *wdt = dev_get_drvdata(dev);
@@ -394,7 +394,7 @@ static int __maybe_unused cdns_wdt_resume(struct device *dev)
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(cdns_wdt_pm_ops, cdns_wdt_suspend, cdns_wdt_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(cdns_wdt_pm_ops, cdns_wdt_suspend, cdns_wdt_resume);
 
 static const struct of_device_id cdns_wdt_of_match[] = {
 	{ .compatible = "cdns,wdt-r1p2", },
@@ -408,7 +408,7 @@ static struct platform_driver cdns_wdt_driver = {
 	.driver		= {
 		.name	= "cdns-wdt",
 		.of_match_table = cdns_wdt_of_match,
-		.pm	= &cdns_wdt_pm_ops,
+		.pm	= pm_sleep_ptr(&cdns_wdt_pm_ops),
 	},
 };
 
diff --git a/drivers/watchdog/da9062_wdt.c b/drivers/watchdog/da9062_wdt.c
index 426962547df1..2b6946478527 100644
--- a/drivers/watchdog/da9062_wdt.c
+++ b/drivers/watchdog/da9062_wdt.c
@@ -248,7 +248,7 @@ static int da9062_wdt_probe(struct platform_device *pdev)
 	return devm_watchdog_register_device(dev, &wdt->wdtdev);
 }
 
-static int __maybe_unused da9062_wdt_suspend(struct device *dev)
+static int da9062_wdt_suspend(struct device *dev)
 {
 	struct watchdog_device *wdd = dev_get_drvdata(dev);
 	struct da9062_watchdog *wdt = watchdog_get_drvdata(wdd);
@@ -262,7 +262,7 @@ static int __maybe_unused da9062_wdt_suspend(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused da9062_wdt_resume(struct device *dev)
+static int da9062_wdt_resume(struct device *dev)
 {
 	struct watchdog_device *wdd = dev_get_drvdata(dev);
 	struct da9062_watchdog *wdt = watchdog_get_drvdata(wdd);
@@ -276,14 +276,14 @@ static int __maybe_unused da9062_wdt_resume(struct device *dev)
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(da9062_wdt_pm_ops,
+static DEFINE_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,
+		.pm = pm_sleep_ptr(&da9062_wdt_pm_ops),
 		.of_match_table = da9062_compatible_id_table,
 	},
 };
diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
index 7c8e53921e40..20b9eb6a2727 100644
--- a/drivers/watchdog/keembay_wdt.c
+++ b/drivers/watchdog/keembay_wdt.c
@@ -250,7 +250,7 @@ static int keembay_wdt_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int __maybe_unused keembay_wdt_suspend(struct device *dev)
+static int keembay_wdt_suspend(struct device *dev)
 {
 	struct keembay_wdt *wdt = dev_get_drvdata(dev);
 
@@ -260,7 +260,7 @@ static int __maybe_unused keembay_wdt_suspend(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused keembay_wdt_resume(struct device *dev)
+static int keembay_wdt_resume(struct device *dev)
 {
 	struct keembay_wdt *wdt = dev_get_drvdata(dev);
 
@@ -270,7 +270,7 @@ static int __maybe_unused keembay_wdt_resume(struct device *dev)
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(keembay_wdt_pm_ops, keembay_wdt_suspend,
+static DEFINE_SIMPLE_DEV_PM_OPS(keembay_wdt_pm_ops, keembay_wdt_suspend,
 			 keembay_wdt_resume);
 
 static const struct of_device_id keembay_wdt_match[] = {
@@ -284,7 +284,7 @@ static struct platform_driver keembay_wdt_driver = {
 	.driver	= {
 		.name		= "keembay_wdt",
 		.of_match_table	= keembay_wdt_match,
-		.pm		= &keembay_wdt_pm_ops,
+		.pm		= pm_sleep_ptr(&keembay_wdt_pm_ops),
 	},
 };
 
diff --git a/drivers/watchdog/msc313e_wdt.c b/drivers/watchdog/msc313e_wdt.c
index f69d66971c41..e283fc7c60c3 100644
--- a/drivers/watchdog/msc313e_wdt.c
+++ b/drivers/watchdog/msc313e_wdt.c
@@ -133,7 +133,7 @@ static int msc313e_wdt_probe(struct platform_device *pdev)
 	return devm_watchdog_register_device(dev, &priv->wdev);
 }
 
-static int __maybe_unused msc313e_wdt_suspend(struct device *dev)
+static int msc313e_wdt_suspend(struct device *dev)
 {
 	struct msc313e_wdt_priv *priv = dev_get_drvdata(dev);
 
@@ -143,7 +143,7 @@ static int __maybe_unused msc313e_wdt_suspend(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused msc313e_wdt_resume(struct device *dev)
+static int msc313e_wdt_resume(struct device *dev)
 {
 	struct msc313e_wdt_priv *priv = dev_get_drvdata(dev);
 
@@ -153,13 +153,13 @@ static int __maybe_unused msc313e_wdt_resume(struct device *dev)
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(msc313e_wdt_pm_ops, msc313e_wdt_suspend, msc313e_wdt_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(msc313e_wdt_pm_ops, msc313e_wdt_suspend, msc313e_wdt_resume);
 
 static struct platform_driver msc313e_wdt_driver = {
 	.driver = {
 		.name = "msc313e-wdt",
 		.of_match_table = msc313e_wdt_of_match,
-		.pm = &msc313e_wdt_pm_ops,
+		.pm = pm_sleep_ptr(&msc313e_wdt_pm_ops),
 	},
 	.probe = msc313e_wdt_probe,
 };
diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index 352853e6fe71..d6b1adfe31d1 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -251,7 +251,7 @@ static int xwdt_probe(struct platform_device *pdev)
  * @dev: handle to the device structure.
  * Return: 0 always.
  */
-static int __maybe_unused xwdt_suspend(struct device *dev)
+static int xwdt_suspend(struct device *dev)
 {
 	struct xwdt_device *xdev = dev_get_drvdata(dev);
 
@@ -267,7 +267,7 @@ static int __maybe_unused xwdt_suspend(struct device *dev)
  * @dev: handle to the device structure.
  * Return: 0 on success, errno otherwise.
  */
-static int __maybe_unused xwdt_resume(struct device *dev)
+static int xwdt_resume(struct device *dev)
 {
 	struct xwdt_device *xdev = dev_get_drvdata(dev);
 	int ret = 0;
@@ -278,7 +278,7 @@ static int __maybe_unused xwdt_resume(struct device *dev)
 	return ret;
 }
 
-static SIMPLE_DEV_PM_OPS(xwdt_pm_ops, xwdt_suspend, xwdt_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(xwdt_pm_ops, xwdt_suspend, xwdt_resume);
 
 /* Match table for of_platform binding */
 static const struct of_device_id xwdt_of_match[] = {
@@ -293,7 +293,7 @@ static struct platform_driver xwdt_driver = {
 	.driver = {
 		.name  = WATCHDOG_NAME,
 		.of_match_table = xwdt_of_match,
-		.pm = &xwdt_pm_ops,
+		.pm = pm_sleep_ptr(&xwdt_pm_ops),
 	},
 };
 
diff --git a/drivers/watchdog/pm8916_wdt.c b/drivers/watchdog/pm8916_wdt.c
index 007ed139ab96..a7ae57fb2453 100644
--- a/drivers/watchdog/pm8916_wdt.c
+++ b/drivers/watchdog/pm8916_wdt.c
@@ -233,7 +233,7 @@ static int pm8916_wdt_probe(struct platform_device *pdev)
 	return devm_watchdog_register_device(dev, &wdt->wdev);
 }
 
-static int __maybe_unused pm8916_wdt_suspend(struct device *dev)
+static int pm8916_wdt_suspend(struct device *dev)
 {
 	struct pm8916_wdt *wdt = dev_get_drvdata(dev);
 
@@ -243,7 +243,7 @@ static int __maybe_unused pm8916_wdt_suspend(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused pm8916_wdt_resume(struct device *dev)
+static int pm8916_wdt_resume(struct device *dev)
 {
 	struct pm8916_wdt *wdt = dev_get_drvdata(dev);
 
@@ -253,7 +253,7 @@ static int __maybe_unused pm8916_wdt_resume(struct device *dev)
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(pm8916_wdt_pm_ops, pm8916_wdt_suspend,
+static DEFINE_SIMPLE_DEV_PM_OPS(pm8916_wdt_pm_ops, pm8916_wdt_suspend,
 			 pm8916_wdt_resume);
 
 static const struct of_device_id pm8916_wdt_id_table[] = {
@@ -267,7 +267,7 @@ static struct platform_driver pm8916_wdt_driver = {
 	.driver = {
 		.name = "pm8916-wdt",
 		.of_match_table = pm8916_wdt_id_table,
-		.pm = &pm8916_wdt_pm_ops,
+		.pm = pm_sleep_ptr(&pm8916_wdt_pm_ops),
 	},
 };
 module_platform_driver(pm8916_wdt_driver);
diff --git a/drivers/watchdog/stmp3xxx_rtc_wdt.c b/drivers/watchdog/stmp3xxx_rtc_wdt.c
index 060447101f48..958e68746f8e 100644
--- a/drivers/watchdog/stmp3xxx_rtc_wdt.c
+++ b/drivers/watchdog/stmp3xxx_rtc_wdt.c
@@ -114,7 +114,7 @@ static void stmp3xxx_wdt_remove(struct platform_device *pdev)
 	unregister_reboot_notifier(&wdt_notifier);
 }
 
-static int __maybe_unused stmp3xxx_wdt_suspend(struct device *dev)
+static int stmp3xxx_wdt_suspend(struct device *dev)
 {
 	struct watchdog_device *wdd = &stmp3xxx_wdd;
 
@@ -124,7 +124,7 @@ static int __maybe_unused stmp3xxx_wdt_suspend(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused stmp3xxx_wdt_resume(struct device *dev)
+static int stmp3xxx_wdt_resume(struct device *dev)
 {
 	struct watchdog_device *wdd = &stmp3xxx_wdd;
 
@@ -134,13 +134,13 @@ static int __maybe_unused stmp3xxx_wdt_resume(struct device *dev)
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(stmp3xxx_wdt_pm_ops,
+static DEFINE_SIMPLE_DEV_PM_OPS(stmp3xxx_wdt_pm_ops,
 			 stmp3xxx_wdt_suspend, stmp3xxx_wdt_resume);
 
 static struct platform_driver stmp3xxx_wdt_driver = {
 	.driver = {
 		.name = "stmp3xxx_rtc_wdt",
-		.pm = &stmp3xxx_wdt_pm_ops,
+		.pm = pm_sleep_ptr(&stmp3xxx_wdt_pm_ops),
 	},
 	.probe = stmp3xxx_wdt_probe,
 	.remove = stmp3xxx_wdt_remove,
-- 
2.25.1

Re: [PATCH] watchdog: PM: use DEFINE_SIMPLE_DEV_PM_OPS and pm_sleep_ptr for dev_pm_ops

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Unmanaged clock leading to a potential system crash or unbalanced clock disable when the watchdog is already running at boot.
- [High] The watchdog is not stopped during system suspend if it was started by the bootloader but not opened by userspace, leading to an unexpected system reset.
--

commit 100d7c43f251465f9d9e677807d28017a2e6ff77
Author: Li Jun <lijun01@kylinos.cn>
Subject: watchdog: PM: use DEFINE_SIMPLE_DEV_PM_OPS and pm_sleep_ptr for dev_pm_ops

This commit replaces the deprecated SIMPLE_DEV_PM_OPS with
DEFINE_SIMPLE_DEV_PM_OPS and uses pm_sleep_ptr() to conditionally drop the
dev_pm_ops structure when CONFIG_PM_SLEEP is disabled.

> diff --git a/drivers/watchdog/da9062_wdt.c b/drivers/watchdog/da9062_wdt.c
> --- a/drivers/watchdog/da9062_wdt.c
> +++ b/drivers/watchdog/da9062_wdt.c
[ ... ]
> @@ -248,7 +248,7 @@ static int da9062_wdt_probe(struct platform_device *pdev)
>  	return devm_watchdog_register_device(dev, &wdt->wdtdev);
>  }
>  
> -static int __maybe_unused da9062_wdt_suspend(struct device *dev)
> +static int da9062_wdt_suspend(struct device *dev)
>  {
>  	struct watchdog_device *wdd = dev_get_drvdata(dev);

[Severity: High]
This isn't a bug introduced by this patch, but does da9062_wdt_suspend()
properly handle a hardware watchdog that was left running by the bootloader?

If the hardware watchdog is started by the bootloader (WDOG_HW_RUNNING is set)
but userspace has not opened /dev/watchdog, watchdog_active() evaluates to
false. During system suspend, the callback checks only watchdog_active()
and skips stopping the hardware timer:

    if (watchdog_active(wdd))
        return da9062_wdt_stop(wdd);

Meanwhile, the watchdog core suspends its background ping worker. The hardware
timer continues ticking without being pinged and will eventually reset the
system unexpectedly while it is suspended.

[ ... ]
> diff --git a/drivers/watchdog/msc313e_wdt.c b/drivers/watchdog/msc313e_wdt.c
> --- a/drivers/watchdog/msc313e_wdt.c
> +++ b/drivers/watchdog/msc313e_wdt.c
> @@ -133,7 +133,7 @@ static int msc313e_wdt_probe(struct platform_device *pdev)
>  	return devm_watchdog_register_device(dev, &priv->wdev);
>  }

[Severity: High]
This is a pre-existing issue, but could the clock handling in
msc313e_wdt_probe() lead to a system crash or an unbalanced disable warning?

In msc313e_wdt_probe(), the clock is acquired but not logically enabled.
If the bootloader left the watchdog running, WDOG_HW_RUNNING is set. The
watchdog core immediately starts a background ping worker, bypassing the
driver's start() callback, so the clock remains logically disabled in the
Common Clock Framework (CCF).

During late boot, CCF disables all unused clocks, turning off the hardware
clock. Subsequent pings from the worker will access unclocked registers,
causing a bus fault.

Additionally, if the watchdog is later stopped, msc313e_wdt_stop() executes:

    clk_disable_unprepare(priv->clk);

Since the clock was never enabled by the driver during start, this will
trigger an unbalanced clock disable warning.

> -static int __maybe_unused msc313e_wdt_suspend(struct device *dev)
> +static int msc313e_wdt_suspend(struct device *dev)
>  {
>  	struct msc313e_wdt_priv *priv = dev_get_drvdata(dev);

[Severity: High]
This is also a pre-existing issue, but this suspend function appears to share
the exact same logic flaw as da9062_wdt_suspend() mentioned above.

If WDOG_HW_RUNNING is set but userspace hasn't opened the device, the driver
skips stopping the hardware timer because watchdog_active() is false:

    if (watchdog_active(&priv->wdev))
        msc313e_wdt_stop(&priv->wdev);

With the core suspending the ping worker, the un-pinged hardware timer will
reset the system during suspend.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910124730.1127847-1-lijun01@kylinos.cn?part=1

Re: [PATCH] watchdog: PM: use DEFINE_SIMPLE_DEV_PM_OPS and pm_sleep_ptr for dev_pm_ops

[Guenter Roeck]

On Thu, Sep 10, 2026 at 08:47:30PM +0800, Li Jun wrote:
> Replace the deprecated SIMPLE_DEV_PM_OPS with DEFINE_SIMPLE_DEV_PM_OPS.
> This also drops the __maybe_unused annotations on the suspend/resume
> callbacks, since pm_sleep_ptr() inside the new macro already handles
> the case where CONFIG_PM_SLEEP is disabled. Wrap the dev_pm_ops
> assignment with pm_sleep_ptr() so the entire dev_pm_ops structure
> is dropped when CONFIG_PM_SLEEP is disabled, instead of leaving
> it as a dead struct with NULL callbacks.
> 
> Signed-off-by: Li Jun <lijun01@kylinos.cn>

Applied.

Thanks,
Guenter