[PATCH] watchdog: allow overriding the rate-limiting logic

[Rasmus Villemoes]

On 12/03/2020 12.58, Stefan Roese wrote:
> Hi Rasmus,
> 
> (added Christophe to Cc)
> 
> On 12.03.20 12:40, Rasmus Villemoes wrote:
>> On the MPC8309-derived board I'm currently working on, the current
>> rate-limiting logic in the generic watchdog_reset function poses two
>> problems:
>>
>> First, the hard-coded limit of 1000ms is too large for the
>> external GPIO-triggered watchdog we have.
> 
> I remember a discussion a few weeks ago with Christophe, where you
> pointed out, that there is already the official "hw_margin_ms" DT
> property for this delay here. Why don't you introduce it here so that
> all boards can use it as well?

Well, I considered that. Reading the value is probably just adding a
dev_read_u32_default(..., 1000) next to the one reading the timeout-sec
property in initr_watchdog(). But what is a good place to stash the
result? It really belongs with the device, but as long as only one is
supported I suppose one could move initr_watchdog to wdt-uclass.c and
use a static variable in that file.

I can certainly do that. That leaves the other problem.

>> Second, in the SPL, the decrementer interrupt is not enabled, so the
>> get_timer() call calls always returns 0, so wdt_reset() is never
>> actually called. Enabling that interrupt (i.e. calling
>> interrupt_init() somewhere in my early board code) is a bit
>> problematic, since U-Boot proper gets loaded to address 0, hence
>> overwriting exception vectors - and the reason I even need to care
>> about the watchdog in the SPL is that the signature verification takes
>> a rather long time, so just disabling interrupts before loading U-Boot
>> proper doesn't work.
>>
>> Both problems can be solved by allowing the board to override the
>> rate-limiting logic. For example, in my case I will implement the
>> function in terms of get_ticks() (i.e. the time base registers on
>> PPC) which do not depend on interrupts, and use a threshold of
>> gd->bus_clk / (4*16) ticks (~62ms).
> 
> I would assume that you might run into multiple issues, when your timer
> infrastructure is not working correctly in SPL. 

None so far, except for the ratelimiting in terms of get_timer() not
working.

> Can't you instead "fix"
> this by using this get_ticks() option for the get_timer() functions in
> SPL so that you can use the common functions here and in all other
> places in SPL as well?

I don't understand what you mean. On PPC, get_timer(0) just returns a
static variable which is incremented from the timer_interrupt(). When
that interrupt is not enabled, that variable is always 0.

I can enable the timer interrupt and get the time (in terms of
get_timer) going, but as I wrote, I would have to disable that interrupt
again before actually loading U-Boot [you can probably imagine the fun
of debugging what happens when one overwrites the exception vectors], so
time would stop passing as far as get_timer() is concerned, which means
that again the watchdog would no longer be serviced. So if there is to
be any ratelimiting at all, it really has to be based on a time that
does tick without relying on interrupts.

Rasmus