Hi, Biju, On 31.01.2024 12:32, Biju Das wrote: > Hi Claudiu, > > Thanks for the feedback. > >> -----Original Message----- >> From: Claudiu <claudiu.beznea@xxxxxxxxx> >> Sent: Wednesday, January 31, 2024 10:20 AM >> Subject: [PATCH v2 04/11] watchdog: rzg2l_wdt: Check return status of >> pm_runtime_put() >> >> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> >> >> pm_runtime_put() may return an error code. Check its return status. >> >> Along with it the rzg2l_wdt_set_timeout() function was updated to >> propagate the result of rzg2l_wdt_stop() to its caller. >> >> Fixes: 2cbc5cd0b55f ("watchdog: Add Watchdog Timer driver for RZ/G2L") >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> >> --- >> >> Changes in v2: >> - propagate the return code of rzg2l_wdt_stop() to it's callers >> >> drivers/watchdog/rzg2l_wdt.c | 11 +++++++++-- >> 1 file changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c >> index d87d4f50180c..7bce093316c4 100644 >> --- a/drivers/watchdog/rzg2l_wdt.c >> +++ b/drivers/watchdog/rzg2l_wdt.c >> @@ -144,9 +144,13 @@ static int rzg2l_wdt_start(struct watchdog_device >> *wdev) static int rzg2l_wdt_stop(struct watchdog_device *wdev) { >> struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev); >> + int ret; >> >> rzg2l_wdt_reset(priv); >> - pm_runtime_put(wdev->parent); >> + >> + ret = pm_runtime_put(wdev->parent); >> + if (ret < 0) >> + return ret; > > Do we need to check the return code? So far we didn't hit this condition. > If you are planning to do it, then just > > return pm_runtime_put(wdev->parent); pm_runtime_put() may return 1 if the device is suspended (which is not considered error) as explained here: https://patchwork.kernel.org/project/linux-renesas-soc/patch/20240122111115.2861835-4-claudiu.beznea.uj@xxxxxxxxxxxxxx/ Thank you, Claudiu Beznea > > Cheers, > Biju > >> >> return 0; > > >> } >> @@ -163,7 +167,10 @@ static int rzg2l_wdt_set_timeout(struct >> watchdog_device *wdev, unsigned int time >> * to reset the module) so that it is updated with new timeout >> values. >> */ >> if (watchdog_active(wdev)) { >> - rzg2l_wdt_stop(wdev); >> + ret = rzg2l_wdt_stop(wdev); >> + if (ret) >> + return ret; >> + >> ret = rzg2l_wdt_start(wdev); >> } >> >> -- >> 2.39.2 >