On 05/21/2015 03:50 AM, Fu Wei wrote:
Hi Guenter, Great thanks, I have got your review feedback, I will fix the problems :-) For update_limits, I also don't want to have that in the watchdog driver, and you can't find it in my last version. And this version, I integrate the watchdog_init_timeout and watchdog_init_pretimeout. so I can not add a driver specific "update_limits" between them. I think maybe we can not make this "update_limits" after calling init_timeouts, because we need to test and verify the timeout setting right after init_pretimeout by max_timeout and min_timeout. that is
Don't you have to do that in set_pretimeout as well, and even adjust timeout if necessary ?
why I call update_limits right after init_pretimeout and before init_timeout. any suggestion ? Great thanks ! :-)
Any sequence of set_timeout() set_pretimeout() can result in an invalid timeout, since the practical timeout limits changed. Therefore, you'll have to validate (and possibly update) the timeout after or during each call to set_pretimeout(). Changing min_timeout and max_timeout won't help there, that validation still has to happen. Given this, some other driver developer may choose to not change the timeout limits at all, validate (and if necessary silently adjust) the timeout value in the set_timeout function, and call the set_timeout function from set_pretimeout to ensure that the timeout is still valid after the call to set_pretimeout. At least I would most likely implement it that way. This means your choice of continuously updating the timeout limits is really a driver developer choice, not something that is needed in the driver core. It might even be problematic, since driver developers might forget to re-validate the current timeout after changing the pretimeout in a similar situation. Having said that, and looking through the SBSA driver code, it seems to me that it does not re-validate the current timeout after setting a new pretimeout. Unless I am missing something, set_timeout(10); set_pretimeout(20); might therefore result in an invalid / unexpected timeout value. Setting set_timeout(10); set_pretimeout(10); might have even more interesting results. Can you try what happens with your driver if you set those values ? Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
