Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1276025

Re: [PATCH v4 3/5] watchdog: ts4800: add driver for TS-4800 watchdog

From Guenter Roeck <linux@roeck-us.net>
Newsgroups linux.kernel
Subject Re: [PATCH v4 3/5] watchdog: ts4800: add driver for TS-4800 watchdog
Date 2015-11-24 03:40 +0100
Message-ID <qybGq-1Y4-19@gated-at.bofh.it> (permalink)
References <qy1e2-3DU-3@gated-at.bofh.it> <qy1e2-3DU-15@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v4 3/5] watchdog: ts4800: add driver for TS-4800 watchdog Damien Riegel <damien.riegel@savoirfairelinux.com> - 2015-11-23 16:30 +0100
  Re: [PATCH v4 3/5] watchdog: ts4800: add driver for TS-4800 watchdog Guenter Roeck <linux@roeck-us.net> - 2015-11-24 03:40 +0100
    Re: [PATCH v4 3/5] watchdog: ts4800: add driver for TS-4800 watchdog Damien Riegel <damien.riegel@savoirfairelinux.com> - 2015-11-24 04:00 +0100

csiph-web