| Message ID | 1479716331-94776-1-git-send-email-tomas.melin@vaisala.com |
|---|---|
| State | Accepted |
| Commit | 2c77c0d6524ebc2e34ea7a4485120225d2b936e6 |
| Delegated to: | Tom Rini |
| Headers | show |
On Mon, Nov 21, 2016 at 10:18:51AM +0200, tomas.melin@vaisala.com wrote: > This fixes the loop delay when using a hw watchdog. > > In case a watchdog is used that accesses CPU registers, > the defined delay of 20us in a tight loop will cause a > huge delay in the actual timeout seen. This is caused > by the fact that udelay will inheritantly call WATCHDOG_RESET. > Together with the omap wdt implementation, the seen timeout increases up to > around 30s. This makes the loop very slow and causes long > delays when using the modem. > > Instead, implement the 2 sec loop by using the timer interface to know > when to break out of the timeout loop. Watchdog kicking is taken care of > by getc(). > > Signed-off-by: Tomas Melin <tomas.melin@vaisala.com> Applied to u-boot/master, thanks!
Hello! On Mon, 21 Nov 2016 10:18:51 +0200 Tomas Melin <tomas.melin@vaisala.com> wrote: > This fixes the loop delay when using a hw watchdog. > > In case a watchdog is used that accesses CPU registers, > the defined delay of 20us in a tight loop will cause a > huge delay in the actual timeout seen. This is caused > by the fact that udelay will inheritantly call WATCHDOG_RESET. > Together with the omap wdt implementation, the seen timeout increases up to > around 30s. This makes the loop very slow and causes long > delays when using the modem. > > Instead, implement the 2 sec loop by using the timer interface to know > when to break out of the timeout loop. Watchdog kicking is taken care of > by getc(). > > Signed-off-by: Tomas Melin <tomas.melin@vaisala.com> This commit breaks YMODEM SPL->U-Boot boot on Beagle Bone, transfer is aborted (because of timeout) after 497kb (u-boot.img is around 570kb). Reverting the commit repairs YMODEM boot. > --- > common/xyzModem.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/common/xyzModem.c b/common/xyzModem.c > index 5656aac..e0d87db 100644 > --- a/common/xyzModem.c > +++ b/common/xyzModem.c > @@ -71,12 +71,12 @@ typedef int cyg_int32; > static int > CYGACC_COMM_IF_GETC_TIMEOUT (char chan, char *c) > { > -#define DELAY 20 > - unsigned long counter = 0; > - while (!tstc () && (counter < xyzModem_CHAR_TIMEOUT * 1000 / DELAY)) > + > + ulong now = get_timer(0); > + while (!tstc ()) > { > - udelay (DELAY); > - counter++; > + if (get_timer(now) > xyzModem_CHAR_TIMEOUT) > + break; > } > if (tstc ()) > {
Hi, On 08/01/2018 03:15 AM, Alexander Sverdlin wrote: > This commit breaks YMODEM SPL->U-Boot boot on Beagle Bone, > transfer is aborted (because of timeout) after 497kb > (u-boot.img is around 570kb). > Reverting the commit repairs YMODEM boot. Is the timeout a watchdog timeout or some communication freeze? In case watchdog is correctly configured, kicking should still happen. Tomas >> --- >> common/xyzModem.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/common/xyzModem.c b/common/xyzModem.c >> index 5656aac..e0d87db 100644 >> --- a/common/xyzModem.c >> +++ b/common/xyzModem.c >> @@ -71,12 +71,12 @@ typedef int cyg_int32; >> static int >> CYGACC_COMM_IF_GETC_TIMEOUT (char chan, char *c) >> { >> -#define DELAY 20 >> - unsigned long counter = 0; >> - while (!tstc () && (counter < xyzModem_CHAR_TIMEOUT * 1000 / DELAY)) >> + >> + ulong now = get_timer(0); >> + while (!tstc ()) >> { >> - udelay (DELAY); >> - counter++; >> + if (get_timer(now) > xyzModem_CHAR_TIMEOUT) >> + break; >> } >> if (tstc ()) >> { >
Hi! On Wed, 1 Aug 2018 08:44:13 +0300 Tomas Melin <tomas.melin@vaisala.com> wrote: > > This commit breaks YMODEM SPL->U-Boot boot on Beagle Bone, > > transfer is aborted (because of timeout) after 497kb > > (u-boot.img is around 570kb). > > Reverting the commit repairs YMODEM boot. > > Is the timeout a watchdog timeout or some communication freeze? I suppose, it reports false-positive timeout back to ymodem code. Because all it does is terminating communication gracefully (with 'C' and a bunch of CANs). I need to check if timer overflow is possible on Beaglebone, because the code is obviously not overflow-proof, but on the other hand it happens within minutes and at least the timer variable is 32 bit... > In case watchdog is correctly configured, kicking should still > happen.
Hi, On 08/01/2018 08:54 AM, Alexander Sverdlin wrote: >> Is the timeout a watchdog timeout or some communication freeze? > I suppose, it reports false-positive timeout back to ymodem code. > Because all it does is terminating communication gracefully (with 'C' > and a bunch of CANs). If you are using omap_wdt then check the configured value of the timeout (WDT_HW_TIMEOUT). Time the y-modem loading, and if the reset happens just after that configured value, it would indicate that the watchdog is not kicked, for one reason or another. Tomas
Hello Tomas, On Wed, 1 Aug 2018 09:16:38 +0300 Tomas Melin <tomas.melin@vaisala.com> wrote: > >> Is the timeout a watchdog timeout or some communication freeze? > > I suppose, it reports false-positive timeout back to ymodem code. > > Because all it does is terminating communication gracefully (with 'C' > > and a bunch of CANs). > If you are using omap_wdt then check the configured value of the timeout > (WDT_HW_TIMEOUT). > Time the y-modem loading, and if the reset happens just after that > configured value, it would indicate that the watchdog is not kicked, for > one reason or another. Yes, I'm using omap_wdt with default 60 seconds timeout, but as I mentioned already, it's not WDT kicking in, but your patch breaking YMODEM protocol reporting false timeout back to YMODEM code.
diff --git a/common/xyzModem.c b/common/xyzModem.c index 5656aac..e0d87db 100644 --- a/common/xyzModem.c +++ b/common/xyzModem.c @@ -71,12 +71,12 @@ typedef int cyg_int32; static int CYGACC_COMM_IF_GETC_TIMEOUT (char chan, char *c) { -#define DELAY 20 - unsigned long counter = 0; - while (!tstc () && (counter < xyzModem_CHAR_TIMEOUT * 1000 / DELAY)) + + ulong now = get_timer(0); + while (!tstc ()) { - udelay (DELAY); - counter++; + if (get_timer(now) > xyzModem_CHAR_TIMEOUT) + break; } if (tstc ()) {
This fixes the loop delay when using a hw watchdog. In case a watchdog is used that accesses CPU registers, the defined delay of 20us in a tight loop will cause a huge delay in the actual timeout seen. This is caused by the fact that udelay will inheritantly call WATCHDOG_RESET. Together with the omap wdt implementation, the seen timeout increases up to around 30s. This makes the loop very slow and causes long delays when using the modem. Instead, implement the 2 sec loop by using the timer interface to know when to break out of the timeout loop. Watchdog kicking is taken care of by getc(). Signed-off-by: Tomas Melin <tomas.melin@vaisala.com> --- common/xyzModem.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)