lkml.org 
[lkml]   [2013]   [Nov]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] watchdog: core: Fix watchdog_init_timeout() when invalid param / valid dt
On 11/26/2013 10:22 AM, Doug Anderson wrote:
> There was a minor bug in watchdog_init_timeout() where it would return
> an error code if someone specified an invalid parameter on the
> command line but then there was a valid parameter in the device tree
> as "timeout-sec".
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>

I thought that was on purpose.

Problem as I see it is that users would expect the timeout to be set to
the provided parameter, which would be silently ignored and replaced
by timeout-sec if the parameter is wrong and timeout-sec is specified.
Seems to me that the user should be informed about the problem,
and not be permitted to provide invalid parameters.

Thanks,
Guenter

> ---
> drivers/watchdog/watchdog_core.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
> index cec9b55..8d27753 100644
> --- a/drivers/watchdog/watchdog_core.c
> +++ b/drivers/watchdog/watchdog_core.c
> @@ -82,12 +82,12 @@ int watchdog_init_timeout(struct watchdog_device *wdd,
> wdd->timeout = timeout_parm;
> return ret;
> }
> - if (timeout_parm)
> - ret = -EINVAL;
>
> - /* try to get the timeout_sec property */
> + /* if no device tree then we're done */
> if (dev == NULL || dev->of_node == NULL)
> - return ret;
> + return (timeout_parm) ? -EINVAL : ret;

( ) is unnecessary.

> +
> + /* try to get the timeout_sec property */
> of_property_read_u32(dev->of_node, "timeout-sec", &t);
> if (!watchdog_timeout_invalid(wdd, t) && t)
> wdd->timeout = t;
>



\
 
 \ /
  Last update: 2013-11-26 20:21    [W:0.075 / U:0.224 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site