[PATCH] watchdog: rti: drop hack manipulating WDT clock rate

[Sverdlin, Alexander]

Thanks for quick feedback, Jan!

On Thu, 2024-11-21 at 07:16 +0100, Jan Kiszka wrote:
> On 20.11.24 23:24, A. Sverdlin wrote:
> > From: Alexander Sverdlin <alexander.sverdlin at siemens.com>
> > 
> > The hack itself seems to be copied from Linux rti_wdt.c, but the WDT reset
> > principle is different in U-Boot. While Linux relies on correct frequencies
> > and timers and doesn't check the actual WDT counter value U-Boot driver
> > seems to be more rubust: it does compare RTIDWDCNTR vs RTIDWDPRLD.
> 
> robust
> 
> > 
> > Now the root cause of the original motivation to manipulate the clock rate
> > is said to be understood and fixed in Linux commit cae58516534e
> > ("watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate a safety margin")
> > which simultaneously removed the hack itself.
> > 
> > While is fix part of the mentioned patch is neither applicable nor requied
> 
> required

I'll re-spin with those two typos corrected.

> > for the U-Boot driver just drop the hack setting WDT clock rate to 90% of
> > the real rate. This has a nice effect that the WDT timeout is now as
> > requested and not 10% shorter.
> > 
> > Signed-off-by: Alexander Sverdlin <alexander.sverdlin at siemens.com>
> > ---
> >   drivers/watchdog/rti_wdt.c | 8 --------
> >   1 file changed, 8 deletions(-)
> > 
> > diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
> > index 99168d0cad03..f806e24e53da 100644
> > --- a/drivers/watchdog/rti_wdt.c
> > +++ b/drivers/watchdog/rti_wdt.c
> > @@ -186,14 +186,6 @@ static int rti_wdt_probe(struct udevice *dev)
> >   
> >   	priv->clk_hz = clk_get_rate(&clk);
> >   
> > -	/*
> > -	 * If watchdog is running at 32k clock, it is not accurate.
> > -	 * Adjust frequency down in this case so that it does not expire
> > -	 * earlier than expected.
> > -	 */
> > -	if (priv->clk_hz < 32768)
> > -		priv->clk_hz = priv->clk_hz * 9 / 10;
> > -
> >   	return 0;
> >   }
> >   
> 
> Makes sense when the kernel is updated to a stable version that contains
> "watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate a safety
> margin" because the intention of this change was "to have comparable
> preset values for both drivers".

Actually, I believe, this doesn't make any change on the kernel side,
because it takes the configured RTIDWDPRLD as the base for its calculations
of the period, but there is either "hack+no safety margin" or "no hack, but
2% safety margin" driver version.

> Heads up for my colleagues (you already use such a kernel) and a
> 
> Reviewed-by: Jan Kiszka <jan.kiszka at siemens.com>
> 
> for this here.

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com