Re: [RFC] [PATCH 5/10] Generic Watchdog Timer Driver

[Wim Van Sebroeck <wim@iguana.be>]

Hi Mike,

>> diff --git a/drivers/watchdog/core/watchdog_dev.c b/drivers/watchdog/core/watchdog_dev.c
>> index 2bf4f67..b80c6e6 100644
>> --- a/drivers/watchdog/core/watchdog_dev.c
>> +++ b/drivers/watchdog/core/watchdog_dev.c
>> @@ -221,6 +221,26 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd,
>>   			return -EOPNOTSUPP;
>>   		watchdog_ping(wdd);
>>   		return 0;
>> +	case WDIOC_SETTIMEOUT:
>> +		if ((wdd->ops->set_timeout == NULL) ||
>> +		    !(wdd->info->options&  WDIOF_SETTIMEOUT))
>> +			return -EOPNOTSUPP;
>> +		if (get_user(val, p))
>> +			return -EFAULT;
>
> Should we sanity check that val > 0 here?  I realize that the driver  
> would need to do it's own sanity checking, but a value of 0 or < 0 makes  
> no sense to the core itself.
>
> Maybe timeout is best expressed as u32 if this is the case.

the sanity check is what is needed in patch 11. Here we should check that the value is between the low and high timeout value.
The timeout value should be an unsigned int at least. Up till now we used values from 1 till 65535.

I'll look into the code to see what val exactly was...

Kind regards,
Wim.