lkml.org 
[lkml]   [2015]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v4 3/5] watchdog: ts4800: add driver for TS-4800 watchdog
From
Date
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



\
 
 \ /
  Last update: 2015-11-24 04:01    [W:0.256 / U:0.184 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site