Re: [PATCH v2 5/7] Watchdog: introduce "pretimeout" into framework

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux