[PATCH v1 1/1] driver: watchdog: get timeout and clock from dt file

[Stefan Roese]

On 31.03.20 08:08, Rayagonda Kokatanur wrote:
> From: Bharat Kumar Reddy Gooty <bharat.gooty at broadcom.com>
> 
> Get the watchdog default timeout value and clock from the DTS file

The default timeout is already read from the DT. Please see below...

> Signed-off-by: Bharat Kumar Reddy Gooty <bharat.gooty at broadcom.com>
> Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur at broadcom.com>
> ---
>   drivers/watchdog/sp805_wdt.c | 36 ++++++++++++++++++++++++++++++++++--
>   1 file changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
> index ca3ccbe76c..e8f54d6804 100644
> --- a/drivers/watchdog/sp805_wdt.c
> +++ b/drivers/watchdog/sp805_wdt.c
> @@ -34,8 +34,31 @@ DECLARE_GLOBAL_DATA_PTR;
>   
>   struct sp805_wdt_priv {
>   	void __iomem *reg;
> +	u32 timeout_msec;
> +	u32 clk_mhz;
>   };
>   
> +static u32 msec_to_ticks(struct udevice *dev)
> +{
> +	u32 timeout_msec;
> +	u32 msec;
> +	struct sp805_wdt_priv *priv = dev_get_priv(dev);
> +
> +	timeout_msec = env_get_ulong("wdt_timeout_msec", 10, 0);
> +	if (timeout_msec) {
> +		dev_dbg(dev, "Overriding timeout :%u\n", timeout_msec);
> +		msec = timeout_msec;
> +	} else {
> +		msec = priv->timeout_msec;
> +	}

Why is this overriding via environment needed? BTW: You should mention
such a change in the commit text as well.

> +
> +	timeout_msec = (msec / 2) * (priv->clk_mhz / 1000);
> +
> +	dev_dbg(dev, "ticks :%u\n", timeout_msec);
> +
> +	return timeout_msec;
> +}
> +
>   static int sp805_wdt_reset(struct udevice *dev)
>   {
>   	struct sp805_wdt_priv *priv = dev_get_priv(dev);
> @@ -63,8 +86,11 @@ static int sp805_wdt_start(struct udevice *dev, u64 timeout, ulong flags)
>   	 * set 120s, the gd->bus_clk is less than 1145MHz, the load_value will
>   	 * not overflow.
>   	 */
> -	load_value = (gd->bus_clk) /
> -		(2 * 1000 * SYS_FSL_WDT_CLK_DIV) * load_time;
> +	if (gd->bus_clk)
> +		load_value = (gd->bus_clk) /
> +			(2 * 1000 * SYS_FSL_WDT_CLK_DIV) * load_time;
> +	else /* platform provide clk */
> +		load_value = msec_to_ticks(dev);

Nitpicking: Please use paranthesis on multi-line statements.

>   	writel(UNLOCK, priv->reg + WDTLOCK);
>   	writel(load_value, priv->reg + WDTLOAD);
> @@ -110,6 +136,12 @@ static int sp805_wdt_ofdata_to_platdata(struct udevice *dev)
>   	if (IS_ERR(priv->reg))
>   		return PTR_ERR(priv->reg);
>   
> +	if (dev_read_u32(dev, "timeout-msec", &priv->timeout_msec))
> +		return -ENODATA;

This does not seem to be an official DT binding. At least I was unable
to find it in the latest Linux tree.

You are aware that the WDT core code already reads the WDT timeout from
the DT using the official "timeout-sec" property? Do you need a higher
graned timeout value (ms vs s)?

Thanks,
Stefan