Re: [PATCH 01/16] watchdog: refactor watchdog_init_timeout

[Guenter Roeck <linux@roeck-us.net>]

On 4/14/19 6:42 AM, Wolfram Sang wrote:
> 
>>> The function is not easy to read and has two problems: a) -EINVAL is
>>> returned when the module parameter is invalid but the DT parameter is
>>> OK, and b) for the module parameter, zero is a valid value but for DT it
>>> is invalid.
>>>
>>
>> That was on purpose: A module parameter of 0 reflects that no module parameter
>> was provided, which is not an error. An explicit DT property with value 0 _is_
>> an error and does not make sense. "use the default", in the DT case, can and
>> should be expressed by providing no property, not by providing a property with
>> value 0.
> 
> OK, I can fix the code to do that as before. And add some documentation
> to describe that. Please let me know if it is documented already and I
> just missed it.
> 
The existing documentation is indeed a bit vague. Explicitly documenting the
expected behavior would be a good idea.  I keep having to explain how this is
supposed to work for almost every new driver. The most difficult part is to
get people to understand that the 'timeout' module parameter value should _not_
be pre-initialized with the default timeout but with 0.

Thanks,
Guenter