On Mon, Jan 22, 2024 at 09:39:27AM -0800, Guenter Roeck wrote: > On 1/22/24 03:11, Claudiu wrote: > > From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> > > > > The RZ/G3S supports deep sleep states where power to most of the IP blocks > > is cut off. To ensure proper working of the watchdog when resuming from > > such states, the suspend function is stopping the watchdog and the resume > > function is starting it. There is no need to configure the watchdog > > in case the watchdog was stopped prior to starting suspend. > > > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> > > --- > > drivers/watchdog/rzg2l_wdt.c | 26 ++++++++++++++++++++++++++ > > 1 file changed, 26 insertions(+) > > > > diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c > > index 9333dc1a75ab..186796b739f7 100644 > > --- a/drivers/watchdog/rzg2l_wdt.c > > +++ b/drivers/watchdog/rzg2l_wdt.c > > @@ -279,6 +279,7 @@ static int rzg2l_wdt_probe(struct platform_device *pdev) > > priv->wdev.timeout = WDT_DEFAULT_TIMEOUT; > > watchdog_set_drvdata(&priv->wdev, priv); > > + dev_set_drvdata(dev, priv); > > ret = devm_add_action_or_reset(&pdev->dev, rzg2l_wdt_pm_disable, &priv->wdev); > > if (ret) > > return ret; > > @@ -300,10 +301,35 @@ static const struct of_device_id rzg2l_wdt_ids[] = { > > }; > > MODULE_DEVICE_TABLE(of, rzg2l_wdt_ids); > > +static int rzg2l_wdt_suspend_late(struct device *dev) > > +{ > > + struct rzg2l_wdt_priv *priv = dev_get_drvdata(dev); > > + > > + if (!watchdog_active(&priv->wdev)) > > + return 0; > > + > > + return rzg2l_wdt_stop(&priv->wdev); > > +} > > + > > +static int rzg2l_wdt_resume_early(struct device *dev) > > +{ > > + struct rzg2l_wdt_priv *priv = dev_get_drvdata(dev); > > + > > + if (!watchdog_active(&priv->wdev)) > > + return 0; > > + > > + return rzg2l_wdt_start(&priv->wdev); > > +} > > + > > +static const struct dev_pm_ops rzg2l_wdt_pm_ops = { > > + LATE_SYSTEM_SLEEP_PM_OPS(rzg2l_wdt_suspend_late, rzg2l_wdt_resume_early) > > +}; > > + > > static struct platform_driver rzg2l_wdt_driver = { > > .driver = { > > .name = "rzg2l_wdt", > > .of_match_table = rzg2l_wdt_ids, > > + .pm = pm_ptr(&rzg2l_wdt_pm_ops), > > I think this will create a build error if CONFIG_PM=n because rzg2l_wdt_pm_ops > will be unused but is not marked with __maybe_unused. But then the driver won't be > operational with CONFIG_PM=n, so I really wonder if it makes sense to include any > such conditional code instead of making the driver depend on CONFIG_PM. > > I really don't think it is desirable to suggest that the driver would work with > CONFIG_PM=n if that isn't really true. > > Guenter Guenter, I'm working on a similar patch. Is your concern limited to the use of the "pm_ptr" macro? Or is it wider? Thanks Jerry -- ----------------------------------------------------------------------------- Jerry Hoemann Software Engineer Hewlett Packard Enterprise -----------------------------------------------------------------------------