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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux