[RFC PATCH] watchdog: base rate-limiting on get_ticks() rather than get_timer()

[Stefan Roese]

On 05.06.20 14:13, Stefan Roese wrote:
> On 05.06.20 14:11, Rasmus Villemoes wrote:
>> On 05/06/2020 13.48, Stefan Roese wrote:
>>> On 05.06.20 13:16, Rasmus Villemoes wrote:
>>>> This is what I had in mind. I also considered making it a config knob
>>>> (possibly just auto-selected based on $ARCH) whether to use
>>>> get_timer() or get_ticks(), but that becomes quite ugly.
>>>
>>> I hesitate a bit, moving with this generic code from get_timer()
>>> to get_ticks() for all boards. Did you test this patch on other
>>> platforms, like some ARM boards?
>>>
>>> Please note that I don't reject it - just asking.
>>
>> Yeah, I'm not really too happy about it myself, exactly because it
>> affects all arches/platforms. And no, I don't have other hardware handy
>> unfortunately. So it's very much an RFC where I hope someone with
>> knowledge of the various arches can say whether one can expect
>> get_ticks() to be at least as widely available as get_timer().
> 
> I'll try to test it on a non-powerpc platform soon'ish.

I've tested it on an MIPS based platform (gardena-smart-gateway-mt7688)
and it works there without any issues AFAICT.

Two more remarks:

Could you please run a build test with this patch applied for all
boards (Travis, Azure...)?

And do you see an issue, that the timer-wrap fixed by Chris with this
patch [1] will not re-occur with your "ticks" approach?

Thanks,
Stefan

[1]
Author: Chris Packham <judge.packham at gmail.com>  2020-02-24 01:20:33
Committer: Stefan Roese <sr at denx.de>  2020-03-16 11:25:12
Parent: db41d65a97f335167e1fbc0400a83333b5157703 (common: Move hang() to 
the same header as panic())
Child:  b4d9452c442769e6dc25649ac02db2d5ed5ea0c8 (watchdog: move 
initr_watchdog() to wdt-uclass.c)
Branches: master, watchdog-ratelimit-2020-06-05-rfc and many more (84)
Follows: v2020.04-rc3
Precedes: v2020.04-rc4

     watchdog: Handle timer wrap around

     On some platforms/architectures the value from get_timer() can wrap.
     This is particularly problematic when long-running code needs to 
measure
     a time difference as is the case with watchdog_reset() which tries to
     avoid tickling the watchdog too frequently.

     Use time_after() from time.h instead of a plain > comparison to avoid
     any issues with the time wrapping on a system that has been sitting in
     u-boot for a long time.

     Signed-off-by: Chris Packham <judge.packham at gmail.com>
     Reviewed-by: Stefan Roese <sr at denx.de>