RE: [PATCH v3 1/2] watchdog: rza_wdt: Support longer timeouts

[Chris Brandt <Chris.Brandt@renesas.com>]

On Monday, September 10, 2018 1, Guenter Roeck wrote:
> > #2. If the CKS is only 4-bits, the max HW timeout is 32 seconds. (so
> > 'timeout' can never be more that a u8).
> >
> That is not the point. The point is that there is no need to keep two
> 'timeout' variables.

But there was a reason.

The reason was that the upper layer would call the .ping() function 
without calling .start() again.

Meaning it would change 'timeout' in the structure, but not explicitly 
tell the driver.

That why I had to keep track of my own timeout (what the HW was actually
set to).

Every time the upper layer calls .ping(), I have to see if the timeout 
field still matches.


> > The number "4194304" is exactly how it is documented in the hardware
> > manual, that is why I'm using it that way. Yes, 0x400000 makes more
> > sense, but I like matching the device documenting as much as possible to
> > help the next person that comes along and has to debug this code.
> >
> Use at least a define.

OK.


> > Because when I was doing all my testing, I found cases where '.ping' was
> > called from the upper layer without '.start' being called first. So, I
> > changed the code as you see it now. This guaranteed I would also get the
> > timeout the user was requesting.
> >
> That would only happen if the watchdog is considered to be running.
> Also, we are talking about the set_timeout function which is the one which
> should set the timeout and update the HW if needed, ie if the watchdog is
> already running.

This driver doesn't have .set_timeout

static const struct watchdog_ops rza_wdt_ops = {
	.owner = THIS_MODULE,
	.start = rza_wdt_start,
	.stop = rza_wdt_stop,
	.ping = rza_wdt_ping,
	.restart = rza_wdt_restart,
};


If you really don't like checking in .ping, I can add a set_timeout 
function and remove the local priv->timeout.

Chris