Re: [PATCH RFC v2 7/9] watchdog: rzv2h: Add syscon support for RZ/T2H and RZ/N2H WDT control register

On 7/2/26 05:31, Prabhakar wrote:
> From: Lad Prabhakar <[email protected]>
> 
> On RZ/T2H and RZ/N2H SoCs the WDTDCR register resides in the second
> region of the System Controller (SYS) block at 0x81290000, which is
> shared with other peripherals. A dedicated SYS driver now manages this
> region and exposes it via a syscon regmap interface, making direct
> mapping of the WDTDCR register by the WDT driver no longer viable.
> 
> Update the WDT driver to support both access methods to maintain
> backward compatibility with old device trees that use direct mapping:
> 

WHy don't you convert the driver to use regmap first and then just
update the regmap pointer for syscon support ? If that is not possible
for some reason, I would expect to see a detailed explanation.

Thanks,
Guenter

>    New binding (syscon):
>      reg = <0 0x80082800 0 0x400>;
>      renesas,sys = <&sys1 2>;
> 
>    Old binding (direct mapping):
>      reg = <0 0x80082800 0 0x400>,
>            <0 0x81295108 0 0x04>;
> 
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> v1->v2:
> - No change.
> ---
>   drivers/watchdog/Kconfig     |  1 +
>   drivers/watchdog/rzv2h_wdt.c | 96 +++++++++++++++++++++++++++++-------
>   2 files changed, 79 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 08cb8612d41f..c48293f93ff5 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1007,6 +1007,7 @@ config RENESAS_RZV2HWDT
>   	depends on ARCH_RENESAS || COMPILE_TEST
>   	depends on PM || COMPILE_TEST
>   	select WATCHDOG_CORE
> +	select MFD_SYSCON
>   	help
>   	  This driver adds watchdog support for the integrated watchdogs in the
>   	  Renesas RZ/{G3E,V2H(P)} SoCs. These watchdogs can be used to reset a
> diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
> index e9545b8f5fd0..e14ff853a4e9 100644
> --- a/drivers/watchdog/rzv2h_wdt.c
> +++ b/drivers/watchdog/rzv2h_wdt.c
> @@ -8,10 +8,12 @@
>   #include <linux/delay.h>
>   #include <linux/io.h>
>   #include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
>   #include <linux/module.h>
>   #include <linux/of.h>
>   #include <linux/platform_device.h>
>   #include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
>   #include <linux/reset.h>
>   #include <linux/units.h>
>   #include <linux/watchdog.h>
> @@ -45,6 +47,10 @@
>   
>   #define WDT_DEFAULT_TIMEOUT	60U
>   
> +#define RZT2H_WDT_MAX_INSTANCES	6
> +
> +#define RZT2H_WDTDCR_OFFSET(n)	(0x5100 + (n) * 4)
> +
>   static bool nowayout = WATCHDOG_NOWAYOUT;
>   module_param(nowayout, bool, 0);
>   MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
> @@ -65,6 +71,11 @@ struct rzv2h_of_data {
>   	bool wdtdcr;
>   };
>   
> +struct rzv2h_sys_wdtdcr {
> +	struct regmap *regmap;
> +	unsigned int offset;
> +};
> +
>   struct rzv2h_wdt_priv {
>   	void __iomem *base;
>   	void __iomem *wdtdcr;
> @@ -73,6 +84,7 @@ struct rzv2h_wdt_priv {
>   	struct reset_control *rstc;
>   	struct watchdog_device wdev;
>   	const struct rzv2h_of_data *of_data;
> +	struct rzv2h_sys_wdtdcr sysc;
>   };
>   
>   static int rzv2h_wdt_ping(struct watchdog_device *wdev)
> @@ -89,9 +101,18 @@ static int rzv2h_wdt_ping(struct watchdog_device *wdev)
>   	return 0;
>   }
>   
> -static void rzt2h_wdt_wdtdcr_count_ctrl(struct rzv2h_wdt_priv *priv, bool start)
> +static int rzt2h_wdt_wdtdcr_count_ctrl(struct rzv2h_wdt_priv *priv, bool start)
>   {
> -	u32 reg = readl(priv->wdtdcr + WDTDCR);
> +	struct rzv2h_sys_wdtdcr *sysc = &priv->sysc;
> +	u32 reg;
> +
> +	if (sysc->regmap) {
> +		return regmap_update_bits(sysc->regmap, sysc->offset,
> +					  WDTDCR_WDTSTOPCTRL,
> +					  start ? 0 : WDTDCR_WDTSTOPCTRL);
> +	}
> +
> +	reg = readl(priv->wdtdcr + WDTDCR);
>   
>   	if (start)
>   		reg &= ~WDTDCR_WDTSTOPCTRL;
> @@ -99,16 +120,18 @@ static void rzt2h_wdt_wdtdcr_count_ctrl(struct rzv2h_wdt_priv *priv, bool start)
>   		reg |= WDTDCR_WDTSTOPCTRL;
>   
>   	writel(reg, priv->wdtdcr + WDTDCR);
> +
> +	return 0;
>   }
>   
> -static void rzt2h_wdt_wdtdcr_count_stop(struct rzv2h_wdt_priv *priv)
> +static int rzt2h_wdt_wdtdcr_count_stop(struct rzv2h_wdt_priv *priv)
>   {
> -	rzt2h_wdt_wdtdcr_count_ctrl(priv, false);
> +	return rzt2h_wdt_wdtdcr_count_ctrl(priv, false);
>   }
>   
> -static void rzt2h_wdt_wdtdcr_count_start(struct rzv2h_wdt_priv *priv)
> +static int rzt2h_wdt_wdtdcr_count_start(struct rzv2h_wdt_priv *priv)
>   {
> -	rzt2h_wdt_wdtdcr_count_ctrl(priv, true);
> +	return rzt2h_wdt_wdtdcr_count_ctrl(priv, true);
>   }
>   
>   static void rzv2h_wdt_setup(struct watchdog_device *wdev, u16 wdtcr)
> @@ -158,8 +181,14 @@ static int rzv2h_wdt_start(struct watchdog_device *wdev)
>   	rzv2h_wdt_setup(wdev, of_data->cks_max | WDTCR_RPSS_100 |
>   			WDTCR_RPES_0 | of_data->tops);
>   
> -	if (priv->of_data->wdtdcr)
> -		rzt2h_wdt_wdtdcr_count_start(priv);
> +	if (priv->of_data->wdtdcr) {
> +		ret = rzt2h_wdt_wdtdcr_count_start(priv);
> +		if (ret) {
> +			reset_control_assert(priv->rstc);
> +			pm_runtime_put(wdev->parent);
> +			return ret;
> +		}
> +	}
>   
>   	/*
>   	 * Down counting starts after writing the sequence 00h -> FFh to the
> @@ -179,8 +208,13 @@ static int rzv2h_wdt_stop(struct watchdog_device *wdev)
>   	if (ret)
>   		return ret;
>   
> -	if (priv->of_data->wdtdcr)
> -		rzt2h_wdt_wdtdcr_count_stop(priv);
> +	if (priv->of_data->wdtdcr) {
> +		ret = rzt2h_wdt_wdtdcr_count_stop(priv);
> +		if (ret) {
> +			reset_control_deassert(priv->rstc);
> +			return ret;
> +		}
> +	}
>   
>   	pm_runtime_put(wdev->parent);
>   
> @@ -196,9 +230,10 @@ static int rzv2h_wdt_restart(struct watchdog_device *wdev,
>   			     unsigned long action, void *data)
>   {
>   	struct rzv2h_wdt_priv *priv = watchdog_get_drvdata(wdev);
> +	bool active = watchdog_active(wdev);
>   	int ret;
>   
> -	if (!watchdog_active(wdev)) {
> +	if (!active) {
>   		ret = clk_enable(priv->pclk);
>   		if (ret)
>   			return ret;
> @@ -242,8 +277,17 @@ static int rzv2h_wdt_restart(struct watchdog_device *wdev,
>   	rzv2h_wdt_setup(wdev, priv->of_data->cks_min | WDTCR_RPSS_25 |
>   			WDTCR_RPES_75 | WDTCR_TOPS_1024);
>   
> -	if (priv->of_data->wdtdcr)
> -		rzt2h_wdt_wdtdcr_count_start(priv);
> +	if (priv->of_data->wdtdcr) {
> +		ret = rzt2h_wdt_wdtdcr_count_start(priv);
> +		if (ret) {
> +			if (!active) {
> +				reset_control_assert(priv->rstc);
> +				clk_disable(priv->oscclk);
> +				clk_disable(priv->pclk);
> +			}
> +			return ret;
> +		}
> +	}
>   
>   	rzv2h_wdt_ping(wdev);
>   
> @@ -264,21 +308,37 @@ static const struct watchdog_ops rzv2h_wdt_ops = {
>   static int rzt2h_wdt_wdtdcr_init(struct platform_device *pdev,
>   				 struct rzv2h_wdt_priv *priv)
>   {
> +	struct device_node *np = pdev->dev.of_node;
>   	int ret;
>   
> -	priv->wdtdcr = devm_platform_ioremap_resource(pdev, 1);
> -	if (IS_ERR(priv->wdtdcr))
> -		return PTR_ERR(priv->wdtdcr);
> +	if (of_property_present(np, "renesas,sys")) {
> +		struct rzv2h_sys_wdtdcr *sysc = &priv->sysc;
> +		unsigned int wdt_index;
> +
> +		sysc->regmap = syscon_regmap_lookup_by_phandle_args(np, "renesas,sys",
> +								    1, &wdt_index);
> +		if (IS_ERR(sysc->regmap))
> +			return PTR_ERR(sysc->regmap);
> +
> +		if (wdt_index >= RZT2H_WDT_MAX_INSTANCES)
> +			return -EINVAL;
> +
> +		sysc->offset = RZT2H_WDTDCR_OFFSET(wdt_index);
> +	} else {
> +		priv->wdtdcr = devm_platform_ioremap_resource(pdev, 1);
> +		if (IS_ERR(priv->wdtdcr))
> +			return PTR_ERR(priv->wdtdcr);
> +	}
>   
>   	ret = pm_runtime_resume_and_get(&pdev->dev);
>   	if (ret)
>   		return ret;
>   
> -	rzt2h_wdt_wdtdcr_count_stop(priv);
> +	ret = rzt2h_wdt_wdtdcr_count_stop(priv);
>   
>   	pm_runtime_put(&pdev->dev);
>   
> -	return 0;
> +	return ret;
>   }
>   
>   static int rzv2h_wdt_probe(struct platform_device *pdev)