On 6/26/19 12:23 AM, Guenter Roeck wrote: > On Tue, Jun 25, 2019 at 02:11:11PM +0000, Melin Tomas wrote: >> Cadence watchdog HW supports prescaler values of >> 8, 64, 512 and 4096. For low frequency input clocks, the smaller >> prescaler values are preferrable to improve the granularity and >> extend the timeout range towards the lower end. >> >> Prescaler logic is extended to support timeout values [1s - 4095s], >> with prescaler selected based on input clock frequency. >> For clock frequencies higher than ~2 MHz, the largest prescaler >> value is used. >> > I have two problems with this patch: > > "improve the granularity and extend the timeout range towards the lower > end" suggests that the timeout is not always selected in multiples of > one second. Since the set-timeout function is supposed to return the > actually selected timeout, this points to a possible bug. > > Also, "extended to support timeout values [1s - 4095s]" is at the very > least misleading since timeouts larger than 516 seconds are not selectable > even after this patch has been applied (see CDNS_WDT_MAX_TIMEOUT and its > use). > > I am not opposed to making better use of the prescaler, but it needs > to be documented properly, and if the timeout granularity is larger > than 1 second, the actual timeout needs to be calculated and reflected > in wdd->timeout. Thanks for valuable feedback. I'll check and rework this and get back with an updated version. thanks, Tomas
