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

[Sam Protsenko]

On 1 March 2018 at 03:15, Ruslan Bilovol <ruslan.bilovol at gmail.com> wrote:
> 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.
>
> This noticeably increases performance of the
> system. Below are some measurements on BBB:
>  - DFU upload over USB                 15% faster
>  - fastboot image upload               3x times faster
>  - USB ep0 transfers with 4k packets   20% faster
>
> Signed-off-by: Ruslan Bilovol <ruslan.bilovol at gmail.com>
> ---
>  drivers/watchdog/omap_wdt.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
> index 7b1f429..d724c96 100644
> --- a/drivers/watchdog/omap_wdt.c
> +++ b/drivers/watchdog/omap_wdt.c
> @@ -53,16 +53,25 @@ void hw_watchdog_reset(void)
>  {
>         struct wd_timer *wdt = (struct wd_timer *)WDT_BASE;
>
> -       /* wait for posted write to complete */
> -       while ((readl(&wdt->wdtwwps)) & WDT_WWPS_PEND_WTGR)
> -               ;
> +       /*
> +        * Somebody just triggered watchdog reset and write to WTGR register
> +        * is in progress. It is resetting right now, no need to trigger it
> +        * again
> +        */
> +       if ((readl(&wdt->wdtwwps)) & WDT_WWPS_PEND_WTGR)
> +               return;
>
>         wdt_trgr_pattern = ~wdt_trgr_pattern;
>         writel(wdt_trgr_pattern, &wdt->wdtwtgr);
>
> -       /* wait for posted write to complete */
> -       while ((readl(&wdt->wdtwwps) & WDT_WWPS_PEND_WTGR))
> -               ;
> +       /*
> +        * Don't wait for posted write to complete, i.e. don't check
> +        * WDT_WWPS_PEND_WTGR bit in WWPS register. There is no writes to
> +        * WTGR register outside of this func, and if entering it
> +        * we see WDT_WWPS_PEND_WTGR bit set, it means watchdog reset
> +        * was just triggered. This prevents us from wasting time in busy
> +        * polling of WDT_WWPS_PEND_WTGR bit.
> +        */
>  }
>
>  static int omap_wdt_set_timeout(unsigned int timeout)
> --
> 1.9.1
>

Tested-by: Sam Protsenko <semen.protsenko at linaro.org>

Tested this patch on my BeagleBone Black Rev A5A. Works fine. I can
see USB throughput improvement when doing "fastboot flash". Here are
my results:

 - before patch: 2.72 MiB/sec
 - when disabling CONFIG_OMAP_WATCHDOG: 5.13 MiB/sec
 - with this patch: 5.08 MiB/sec

So I can see 1.87 times improvement in speed, which is really good
news for us, especially for flashing big images. I wonder, what is the
max USB speed limit for transfer via EP1. E.g., what will we see if
test it via Gadget Zero in Linux? Maybe there is a room for further
improvements. But those are only my thoughts.

Thanks, Ruslan!