2024-04-10 16:42:53

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH RESEND v8 07/10] watchdog: rzg2l_wdt: Rely on the reset driver for doing proper reset

On Wed, Apr 10, 2024 at 04:40:41PM +0300, Claudiu wrote:
> From: Claudiu Beznea <[email protected]>
>
> The reset driver has been adapted in commit da235d2fac21
> ("clk: renesas: rzg2l: Check reset monitor registers") to check the reset
> monitor bits before declaring reset asserts/de-asserts as
> successful/failure operations. With that, there is no need to keep the
> reset workaround for RZ/V2M in place in the watchdog driver.
>
> Signed-off-by: Claudiu Beznea <[email protected]>
> Reviewed-by: Philipp Zabel <[email protected]>

Reviewed-by: Guenter Roeck <[email protected]>

> ---
>
> Changes in v8:
> - none
>
> Changes in v7:
> - none
>
> Changes in v6:
> - none
>
> Changes in v5:
> - none
>
> Changes in v4:
> - collected tag
>
> Changes in v3:
> - none
>
> Changes in v2:
> - none
>
> drivers/watchdog/rzg2l_wdt.c | 39 ++++--------------------------------
> 1 file changed, 4 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c
> index 29eb47bcf984..42f1d5d6f07e 100644
> --- a/drivers/watchdog/rzg2l_wdt.c
> +++ b/drivers/watchdog/rzg2l_wdt.c
> @@ -8,7 +8,6 @@
> #include <linux/clk.h>
> #include <linux/delay.h>
> #include <linux/io.h>
> -#include <linux/iopoll.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/of.h>
> @@ -54,35 +53,11 @@ struct rzg2l_wdt_priv {
> struct reset_control *rstc;
> unsigned long osc_clk_rate;
> unsigned long delay;
> - unsigned long minimum_assertion_period;
> struct clk *pclk;
> struct clk *osc_clk;
> enum rz_wdt_type devtype;
> };
>
> -static int rzg2l_wdt_reset(struct rzg2l_wdt_priv *priv)
> -{
> - int err, status;
> -
> - if (priv->devtype == WDT_RZV2M) {
> - /* WDT needs TYPE-B reset control */
> - err = reset_control_assert(priv->rstc);
> - if (err)
> - return err;
> - ndelay(priv->minimum_assertion_period);
> - err = reset_control_deassert(priv->rstc);
> - if (err)
> - return err;
> - err = read_poll_timeout(reset_control_status, status,
> - status != 1, 0, 1000, false,
> - priv->rstc);
> - } else {
> - err = reset_control_reset(priv->rstc);
> - }
> -
> - return err;
> -}
> -
> static void rzg2l_wdt_wait_delay(struct rzg2l_wdt_priv *priv)
> {
> /* delay timer when change the setting register */
> @@ -187,13 +162,12 @@ 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;
>
> clk_prepare_enable(priv->pclk);
> clk_prepare_enable(priv->osc_clk);
>
> if (priv->devtype == WDT_RZG2L) {
> - int ret;
> -
> ret = reset_control_deassert(priv->rstc);
> if (ret)
> return ret;
> @@ -205,7 +179,9 @@ static int rzg2l_wdt_restart(struct watchdog_device *wdev,
> rzg2l_wdt_write(priv, PEEN_FORCE, PEEN);
> } else {
> /* RZ/V2M doesn't have parity error registers */
> - rzg2l_wdt_reset(priv);
> + ret = reset_control_reset(priv->rstc);
> + if (ret)
> + return ret;
>
> wdev->timeout = 0;
>
> @@ -297,13 +273,6 @@ static int rzg2l_wdt_probe(struct platform_device *pdev)
>
> priv->devtype = (uintptr_t)of_device_get_match_data(dev);
>
> - if (priv->devtype == WDT_RZV2M) {
> - priv->minimum_assertion_period = RZV2M_A_NSEC +
> - 3 * F2CYCLE_NSEC(pclk_rate) + 5 *
> - max(F2CYCLE_NSEC(priv->osc_clk_rate),
> - F2CYCLE_NSEC(pclk_rate));
> - }
> -
> pm_runtime_enable(&pdev->dev);
>
> priv->wdev.info = &rzg2l_wdt_ident;
> --
> 2.39.2
>