[U-Boot] [PATCH] watchdog: omap_wdt: improve watchdog reset path

[Ruslan Bilovol]

On Thu, Mar 1, 2018 at 4:33 PM, Tom Rini <trini at konsulko.com> wrote:
> On Thu, Mar 01, 2018 at 04:10:44PM +0200, Ruslan Bilovol wrote:
>> Hi Lukasz,
>>
>> On Thu, Mar 1, 2018 at 12:53 PM, Lukasz Majewski <lukma at denx.de> wrote:
>> > Hi Ruslan,
>> >
>> >> Remove busy looping during watchdog reset.
>> >> Each polling of W_PEND_WTGR bit ("finish posted
>> >> write") after watchdog reset takes 120-140us
>> >> on BeagleBone Black board. Current U-Boot code
>> >> has watchdog resets in random places and often
>> >> there is situation when watchdog is reset
>> >> few times in a row in nested functions.
>> >> This adds extra delays and slows the whole system.
>> >>
>> >> Instead of polling W_PEND_WTGR bit, we skip
>> >> watchdog reset if the bit is set. Anyway, watchdog
>> >> is in the middle of reset *right now*, so we can
>> >> just return.
>> >
>> > It seems like similar problem may be in the Linux kernel driver:
>> > v4.16-rc1/source/drivers/watchdog/omap_wdt.c#L74
>> >
>> > Linux driver also waits for the write.
>>
>> Right, Linux driver has similar code but it doesn't affect performance.
>> This is because of watchdog usage in Linux, it is enabled and
>> reset by userspace. This is quite rare event.
>> Moreover, since Linux has interrupts enabled and has scheduling,
>> such busy loops in the omap driver are not very different to
>> just mdelay() usage. The system can handle interrupts, and can
>> even do preemption if PREEMPT is enabled.
>> So use of such busy loops in that particular case shouldn't cause
>> any noticeable performance issues.
>
> True.  But, can you also submit a patch to the kernel side (and CC me) ?
> That's far more likely to get the attention of the engineers that might
> know of corner cases with the various SoC families where we might need
> to keep doing what we're doing or other possible problems.  Thanks!

Do you mean just resend this patch, but include kernel watchdog
mailing list?
Or reimplement it for kernel?

In last case it doesn't have much sense - it brings almost no
value to the driver, since watchdog usage approach by kernel
is too different comparing to U-Boot.
Yet also kernel driver is more complicated and such a change may
trigger it's rework. Probably will need to introduce additional
synchronization at least.

By the way, if needed, I can resend this patch including watchdog
kernel mailing list and related maintainer, so kernel folks can review
and provide some feedback.

Best regards,
Ruslan Bilovol