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

Tom Rini
2018-03-05T14:40:27Z

On Sun, Mar 04, 2018 at 04:00:15PM +0200, Ruslan Bilovol wrote:
> On Thu, Mar 1, 2018 at 4:33 PM, Tom Rini <trini@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@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?

Well, I'm going to set aside what I said here since we've gotten some TI
folks to review the patch here.

--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180305/b152e509/attachment.sig>