| Message ID | 20211213152658.26225-2-biju.das.jz@bp.renesas.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/4] watchdog: rzg2l_wdt: Fix 32bit overflow issue | expand |
Hi All, Gentle ping. Are we happy with this patch? Please let me know. Regards, Biju > Subject: [PATCH v2 2/4] watchdog: rzg2l_wdt: Add error check for > reset_control_{deassert/reset} > > If reset_control_deassert() fails, then we won't be able to access the > device registers. Therefore check the return code of > reset_control_deassert() and bail out in case of error. > > While at it remove the unnecessary pm_runtime_resume_and_get() from > probe(), as it turns on the clocks. > > Replace reset_control_assert()->reset_control_reset() in rzg2l_wdt > _stop() and remove the unnecessary reset_control_deassert() from > rzg2l_wdt_start(). Also add error check for reset_control_reset() > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > v1->v2: > * Updated commit description and removed Rb tag from Guenter, > since there is code change > * Replaced reset_control_assert with reset_control_reset in stop > and removed reset_control_deassert() from start. > * > --- > drivers/watchdog/rzg2l_wdt.c | 34 +++++++++++++++------------------- > 1 file changed, 15 insertions(+), 19 deletions(-) > > diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c > index 96f2a018ab62..0e62d7be153c 100644 > --- a/drivers/watchdog/rzg2l_wdt.c > +++ b/drivers/watchdog/rzg2l_wdt.c > @@ -86,7 +86,6 @@ static int rzg2l_wdt_start(struct watchdog_device *wdev) > { > struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev); > > - reset_control_deassert(priv->rstc); > pm_runtime_get_sync(wdev->parent); > > /* Initialize time out */ > @@ -106,18 +105,24 @@ static int rzg2l_wdt_stop(struct watchdog_device > *wdev) > struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev); > > pm_runtime_put(wdev->parent); > - reset_control_assert(priv->rstc); > > - return 0; > + /* Reset the module for stopping watchdog */ > + return reset_control_reset(priv->rstc); > } > > static int rzg2l_wdt_restart(struct watchdog_device *wdev, > unsigned long action, void *data) { > struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev); > + int ret; > > /* Reset the module before we modify any register */ > - reset_control_reset(priv->rstc); > + ret = reset_control_reset(priv->rstc); > + if (ret) { > + dev_err(wdev->parent, "failed to reset"); > + return ret; > + } > + > pm_runtime_get_sync(wdev->parent); > > /* smallest counter value to reboot soon */ @@ -151,12 +156,11 @@ > static const struct watchdog_ops rzg2l_wdt_ops = { > .restart = rzg2l_wdt_restart, > }; > > -static void rzg2l_wdt_reset_assert_pm_disable_put(void *data) > +static void rzg2l_wdt_reset_assert_pm_disable(void *data) > { > struct watchdog_device *wdev = data; > struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev); > > - pm_runtime_put(wdev->parent); > pm_runtime_disable(wdev->parent); > reset_control_assert(priv->rstc); > } > @@ -204,13 +208,11 @@ static int rzg2l_wdt_probe(struct platform_device > *pdev) > return dev_err_probe(&pdev->dev, PTR_ERR(priv->rstc), > "failed to get cpg reset"); > > - reset_control_deassert(priv->rstc); > + ret = reset_control_deassert(priv->rstc); > + if (ret) > + return dev_err_probe(dev, ret, "failed to deassert"); > + > pm_runtime_enable(&pdev->dev); > - ret = pm_runtime_resume_and_get(&pdev->dev); > - if (ret < 0) { > - dev_err(dev, "pm_runtime_resume_and_get failed ret=%pe", > ERR_PTR(ret)); > - goto out_pm_get; > - } > > priv->wdev.info = &rzg2l_wdt_ident; > priv->wdev.ops = &rzg2l_wdt_ops; > @@ -222,7 +224,7 @@ static int rzg2l_wdt_probe(struct platform_device > *pdev) > > watchdog_set_drvdata(&priv->wdev, priv); > ret = devm_add_action_or_reset(&pdev->dev, > - rzg2l_wdt_reset_assert_pm_disable_put, > + rzg2l_wdt_reset_assert_pm_disable, > &priv->wdev); > if (ret < 0) > return ret; > @@ -235,12 +237,6 @@ static int rzg2l_wdt_probe(struct platform_device > *pdev) > dev_warn(dev, "Specified timeout invalid, using default"); > > return devm_watchdog_register_device(&pdev->dev, &priv->wdev); > - > -out_pm_get: > - pm_runtime_disable(dev); > - reset_control_assert(priv->rstc); > - > - return ret; > } > > static const struct of_device_id rzg2l_wdt_ids[] = { > -- > 2.17.1
diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c index 96f2a018ab62..0e62d7be153c 100644 --- a/drivers/watchdog/rzg2l_wdt.c +++ b/drivers/watchdog/rzg2l_wdt.c @@ -86,7 +86,6 @@ static int rzg2l_wdt_start(struct watchdog_device *wdev) { struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev); - reset_control_deassert(priv->rstc); pm_runtime_get_sync(wdev->parent); /* Initialize time out */ @@ -106,18 +105,24 @@ static int rzg2l_wdt_stop(struct watchdog_device *wdev) struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev); pm_runtime_put(wdev->parent); - reset_control_assert(priv->rstc); - return 0; + /* Reset the module for stopping watchdog */ + return reset_control_reset(priv->rstc); } static int rzg2l_wdt_restart(struct watchdog_device *wdev, unsigned long action, void *data) { struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev); + int ret; /* Reset the module before we modify any register */ - reset_control_reset(priv->rstc); + ret = reset_control_reset(priv->rstc); + if (ret) { + dev_err(wdev->parent, "failed to reset"); + return ret; + } + pm_runtime_get_sync(wdev->parent); /* smallest counter value to reboot soon */ @@ -151,12 +156,11 @@ static const struct watchdog_ops rzg2l_wdt_ops = { .restart = rzg2l_wdt_restart, }; -static void rzg2l_wdt_reset_assert_pm_disable_put(void *data) +static void rzg2l_wdt_reset_assert_pm_disable(void *data) { struct watchdog_device *wdev = data; struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev); - pm_runtime_put(wdev->parent); pm_runtime_disable(wdev->parent); reset_control_assert(priv->rstc); } @@ -204,13 +208,11 @@ static int rzg2l_wdt_probe(struct platform_device *pdev) return dev_err_probe(&pdev->dev, PTR_ERR(priv->rstc), "failed to get cpg reset"); - reset_control_deassert(priv->rstc); + ret = reset_control_deassert(priv->rstc); + if (ret) + return dev_err_probe(dev, ret, "failed to deassert"); + pm_runtime_enable(&pdev->dev); - ret = pm_runtime_resume_and_get(&pdev->dev); - if (ret < 0) { - dev_err(dev, "pm_runtime_resume_and_get failed ret=%pe", ERR_PTR(ret)); - goto out_pm_get; - } priv->wdev.info = &rzg2l_wdt_ident; priv->wdev.ops = &rzg2l_wdt_ops; @@ -222,7 +224,7 @@ static int rzg2l_wdt_probe(struct platform_device *pdev) watchdog_set_drvdata(&priv->wdev, priv); ret = devm_add_action_or_reset(&pdev->dev, - rzg2l_wdt_reset_assert_pm_disable_put, + rzg2l_wdt_reset_assert_pm_disable, &priv->wdev); if (ret < 0) return ret; @@ -235,12 +237,6 @@ static int rzg2l_wdt_probe(struct platform_device *pdev) dev_warn(dev, "Specified timeout invalid, using default"); return devm_watchdog_register_device(&pdev->dev, &priv->wdev); - -out_pm_get: - pm_runtime_disable(dev); - reset_control_assert(priv->rstc); - - return ret; } static const struct of_device_id rzg2l_wdt_ids[] = {
If reset_control_deassert() fails, then we won't be able to access the device registers. Therefore check the return code of reset_control_deassert() and bail out in case of error. While at it remove the unnecessary pm_runtime_resume_and_get() from probe(), as it turns on the clocks. Replace reset_control_assert()->reset_control_reset() in rzg2l_wdt _stop() and remove the unnecessary reset_control_deassert() from rzg2l_wdt_start(). Also add error check for reset_control_reset() Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v1->v2: * Updated commit description and removed Rb tag from Guenter, since there is code change * Replaced reset_control_assert with reset_control_reset in stop and removed reset_control_deassert() from start. * --- drivers/watchdog/rzg2l_wdt.c | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-)