Hello Guenter,
On 03.05.21 16:31, Guenter Roeck wrote:
>> -static struct fintek_wdt watchdog = {
>> - .lock = __MUTEX_INITIALIZER(watchdog.lock),
>> -};
>> +static struct fintek_wdt watchdog;
>
> This should really be allocated, and "watchdog" is a terrible variable name
> even if static, especially given the previous patch.
I can add a follow up patch to change this. I maintained the old
state of things here to make review easier.
>
>>
>> /* Super I/O functions */
>> static inline int superio_inb(int base, int reg)
>> @@ -218,17 +206,9 @@ static inline void superio_exit(int base)
>> release_region(base, 2);
>> }
>>
>> -static int fintek_wdt_set_timeout(int timeout)
>> +static int fintek_wdt_set_timeout(struct watchdog_device *wdd, unsigned int timeout)
>> {
>> - if (timeout <= 0
>> - || timeout > max_timeout) {
>> - pr_err("watchdog timeout out of range\n");
>> - return -EINVAL;
>> - }
>> -
>> - mutex_lock(&watchdog.lock);
>> -
>> - watchdog.timeout = timeout;
>> + wdd->timeout = timeout;
>
> This needs to save the actual timeout, which differs from the
> configured one if larger than 255.
Oh, makes sense. Will changes with v4.
Thanks,
Ahmad
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
