On 10.04.2024 16:51, Biju Das wrote: > Hi Claudiu, > >> -----Original Message----- >> From: Claudiu <claudiu.beznea@xxxxxxxxx> >> Sent: Wednesday, April 10, 2024 2:41 PM >> Subject: [PATCH RESEND v8 03/10] watchdog: rzg2l_wdt: Use pm_runtime_resume_and_get() >> >> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> >> >> pm_runtime_get_sync() may return with error. In case it returns with error >> dev->power.usage_count needs to be decremented. >> dev->pm_runtime_resume_and_get() >> takes care of this. Thus use it. >> >> Along with it the rzg2l_wdt_set_timeout() function was updated to propagate the result of >> rzg2l_wdt_start() to its caller. >> >> Fixes: 2cbc5cd0b55f ("watchdog: Add Watchdog Timer driver for RZ/G2L") >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> >> --- >> >> Changes in v8: >> - none >> >> Changes in v7: >> - none >> >> Changes in v6: >> - none >> >> Changes in v5: >> - none >> >> Changes in v4: >> - none >> >> Changes in v3: >> - none >> >> Changes in v2: >> - propagate the return code of rzg2l_wdt_start() to it's callers >> >> >> drivers/watchdog/rzg2l_wdt.c | 11 ++++++++--- >> 1 file changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c index >> 1741f98ca67c..d87d4f50180c 100644 >> --- a/drivers/watchdog/rzg2l_wdt.c >> +++ b/drivers/watchdog/rzg2l_wdt.c >> @@ -123,8 +123,11 @@ static void rzg2l_wdt_init_timeout(struct watchdog_device *wdev) static int >> rzg2l_wdt_start(struct watchdog_device *wdev) { >> struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev); >> + int ret; >> >> - pm_runtime_get_sync(wdev->parent); >> + ret = pm_runtime_resume_and_get(wdev->parent); >> + if (ret) >> + return ret; >> >> /* Initialize time out */ >> rzg2l_wdt_init_timeout(wdev); >> @@ -150,6 +153,8 @@ static int rzg2l_wdt_stop(struct watchdog_device *wdev) >> >> static int rzg2l_wdt_set_timeout(struct watchdog_device *wdev, unsigned int timeout) { >> + int ret = 0; >> + >> wdev->timeout = timeout; >> >> /* >> @@ -159,10 +164,10 @@ static int rzg2l_wdt_set_timeout(struct watchdog_device *wdev, unsigned int >> time >> */ >> if (watchdog_active(wdev)) { >> rzg2l_wdt_stop(wdev); >> - rzg2l_wdt_start(wdev); >> + ret = rzg2l_wdt_start(wdev); > > This IP won't be able to update WDT settings once you have set it. > > But we can update it, if we do a reset assert followed by deassert. > So the previous code looks correct to me. > > Current case is if the WDT is active, then start it. Maybe I ma missing something here. > I'm not sure I got you correctly. This patch keeps the previous functionality, thus, if the watchdog is active rzg2l_wdt_stop() is called which does a reset assert. Then rzg2l_wdt_start() is called which does the reset deassert. Thank you, Claudiu Beznea > Cheers, > Biju > >> } >> >> - return 0; >> + return ret; >> } >> >> static int rzg2l_wdt_restart(struct watchdog_device *wdev, >> -- >> 2.39.2 >