Messages in this thread | | | Subject | Re: [PATCH v4 3/5] watchdog: ts4800: add driver for TS-4800 watchdog | From | Guenter Roeck <> | Date | Mon, 23 Nov 2015 18:32:15 -0800 |
| |
Hi Damien,
On 11/23/2015 07:17 AM, Damien Riegel wrote: > This watchdog is instantiated in a FPGA that is memory mapped. It is > made of only one register, called the feed register. Writing to this > register will re-arm the watchdog for a given time (and enable it if it > was disable). It can be disabled by writing a special value into it. > > It is part of a syscon block, and the watchdog register offset in this > block varies from board to board. This offset is passed in the syscon > property after the phandle to the syscon node. > > Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com> > ---
[ ... ]
> + > +static int ts4800_wdt_set_timeout(struct watchdog_device *wdd, > + unsigned int timeout) > +{ > + struct ts4800_wdt *wdt = watchdog_get_drvdata(wdd); > + int i; > + > + for (i = 0; i <= MAX_TIMEOUT_INDEX; i++) { > + if (ts4800_wdt_map[i].timeout >= timeout) > + break; > + }
If the loop does not break, i will have a value of MAX_TIMEOUT_INDEX + 1, or 2, pointing after the end of the table. That should never happen, but still ...
I preferred the earlier version, where you had an extra function. Only my suggestion was to have that function return MAX_TIMEOUT_INDEX instead of an error. Alternatively, the check above needs to be "i < MAX_TIMEOUT_INDEX".
Guenter
| |