From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Message-ID: <541B5156.803@elproma.com.pl> Date: Thu, 18 Sep 2014 23:40:38 +0200 From: =?windows-1250?Q?Janusz_U=BFycki?= MIME-Version: 1.0 To: Guenter Roeck , linux-watchdog@vger.kernel.org, Wim Van Sebroeck Subject: Re: watchdog's pm support preffered implementation References: <54088996.4040500@elproma.com.pl> <540C9383.9050307@roeck-us.net> <540D02E1.90403@elproma.com.pl> <540D1F8F.2080802@roeck-us.net> <54196BE2.2010800@elproma.com.pl> <541ABD07.5090407@elproma.com.pl> In-Reply-To: <541ABD07.5090407@elproma.com.pl> Content-Type: text/plain; charset=windows-1250; format=flowed Content-Transfer-Encoding: 8bit List-ID: Hi again, This is the second implementation. Which do you prefer? Subject: [PATCH] stmp3xxx_rtc_wdt: Add suspend/resume PM support Signed-off-by: Janusz Uzycki --- drivers/watchdog/stmp3xxx_rtc_wdt.c | 39 +++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/drivers/watchdog/stmp3xxx_rtc_wdt.c b/drivers/watchdog/stmp3xxx_rtc_wdt.c index 3546f03..1946277 100644 --- a/drivers/watchdog/stmp3xxx_rtc_wdt.c +++ b/drivers/watchdog/stmp3xxx_rtc_wdt.c @@ -95,9 +95,48 @@ static int stmp3xxx_wdt_remove(struct platform_device *pdev) return 0; } +#ifdef CONFIG_PM_SLEEP +/* There is no conflict with rtc/rtc-stmp3xxx.c parent + * because modified registers in PM functions are different */ +static int __maybe_unused stmp3xxx_wdt_suspend(struct device *dev) +{ + struct watchdog_device *wdd = dev_get_drvdata(dev); + + /* Is keep-on/ping timer suspended before? + * or additional driver-specific flag must be added + * to block watchdog ping in the timer? + * or disable WATCHDOG_KEEP_ON before wdt_stop + * and restore it in resume? */ + if (watchdog_active(wdd)) { + dev_info(wdd->dev, "%s: wdt was active\n", __func__); + return wdt_stop(wdd); + } + /* should we use pm_runtime like omap_wdt.c does? */ + + return 0; +} + +static int __maybe_unused stmp3xxx_wdt_resume(struct device *dev) +{ + struct watchdog_device *wdd = dev_get_drvdata(dev); + + if (watchdog_active(wdd)) { + dev_info(wdd->dev, "%s: wdt was active\n", __func__); + return wdt_start(wdd); + } + + return 0; +} +#endif /* CONFIG_PM_SLEEP */ + +static 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", + .owner = THIS_MODULE, + .pm = &stmp3xxx_wdt_pm_ops, }, .probe = stmp3xxx_wdt_probe, .remove = stmp3xxx_wdt_remove, -- 1.7.11.3 W dniu 2014-09-18 13:07, Janusz Użycki pisze: > > I work under suspend/resume PM support for stmp3xxx driver. > > Most of them use suspend() with struct platform_device and CONFIG_PM. > Only sp805, sirfsoc, s3c2410, dw drivers use suspend() with struct device > and SIMPLE_DEV_PM_OPS. > Which implementation is preffered today? > > best regards > Janusz > > P.S. Patch on the moment: > Signed-off-by: Janusz Uzycki > --- > drivers/watchdog/stmp3xxx_rtc_wdt.c | 41 +++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/drivers/watchdog/stmp3xxx_rtc_wdt.c > b/drivers/watchdog/stmp3xxx_rtc_wdt.c > index 3546f03..12060ab 100644 > --- a/drivers/watchdog/stmp3xxx_rtc_wdt.c > +++ b/drivers/watchdog/stmp3xxx_rtc_wdt.c > @@ -74,6 +74,7 @@ static int stmp3xxx_wdt_probe(struct platform_device > *pdev) > { > int ret; > > + platform_set_drvdata(pdev, &stmp3xxx_wdd); > watchdog_set_drvdata(&stmp3xxx_wdd, &pdev->dev); > > stmp3xxx_wdd.timeout = clamp_t(unsigned, heartbeat, 1, > STMP3XXX_MAX_TIMEOUT); > @@ -95,12 +96,52 @@ static int stmp3xxx_wdt_remove(struct > platform_device *pdev) > return 0; > } > > +#ifdef CONFIG_PM > +/* There is no conflict with rtc/rtc-stmp3xxx.c parent > + * because modified registers in PM functions are different */ > +static int stmp3xxx_wdt_suspend(struct platform_device *pdev, > + pm_message_t state) > +{ > + struct watchdog_device *wdd = platform_get_drvdata(pdev); > + > + /* Is keep-on/ping timer suspended before? > + * or additional driver-specific flag must be added > + * to block watchdog ping in the timer? > + * or disable WATCHDOG_KEEP_ON before wdt_stop > + * and restore it in resume? */ > + if (watchdog_active(wdd)) { > + dev_info(wdd->dev, "%s: wdt was active\n", __func__); > + return wdt_stop(wdd); > + } > + /* should we use pm_runtime like omap_wdt.c does? */ > + > + return 0; > +} > + > +static int stmp3xxx_wdt_resume(struct platform_device *pdev) > +{ > + struct watchdog_device *wdd = platform_get_drvdata(pdev); > + > + if (watchdog_active(wdd)) { > + dev_info(wdd->dev, "%s: wdt was active\n", __func__); > + return wdt_start(wdd); > + } > + > + return 0; > +} > +#else > +#define stmp3xxx_wdt_suspend NULL > +#define stmp3xxx_wdt_resume NULL > +#endif > + > static struct platform_driver stmp3xxx_wdt_driver = { > .driver = { > .name = "stmp3xxx_rtc_wdt", > }, > .probe = stmp3xxx_wdt_probe, > .remove = stmp3xxx_wdt_remove, > + .suspend = stmp3xxx_wdt_suspend, > + .resume = stmp3xxx_wdt_resume, > }; > module_platform_driver(stmp3xxx_wdt_driver); >