* [U-Boot] [PATCH] xyz-modem: Change getc timeout loop waiting
@ 2016-11-21 8:18 Tomas Melin
2016-11-29 1:07 ` [U-Boot] " Tom Rini
2018-08-01 0:15 ` Alexander Sverdlin
0 siblings, 2 replies; 7+ messages in thread
From: Tomas Melin @ 2016-11-21 8:18 UTC (permalink / raw)
To: u-boot
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(-)
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 ())
{
--
2.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] xyz-modem: Change getc timeout loop waiting
2016-11-21 8:18 [U-Boot] [PATCH] xyz-modem: Change getc timeout loop waiting Tomas Melin
@ 2016-11-29 1:07 ` Tom Rini
2018-08-01 0:15 ` Alexander Sverdlin
1 sibling, 0 replies; 7+ messages in thread
From: Tom Rini @ 2016-11-29 1:07 UTC (permalink / raw)
To: u-boot
On Mon, Nov 21, 2016 at 10:18:51AM +0200, tomas.melin at 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!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161128/1bd15f84/attachment.sig>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] xyz-modem: Change getc timeout loop waiting
2016-11-21 8:18 [U-Boot] [PATCH] xyz-modem: Change getc timeout loop waiting Tomas Melin
2016-11-29 1:07 ` [U-Boot] " Tom Rini
@ 2018-08-01 0:15 ` Alexander Sverdlin
2018-08-01 5:44 ` Tomas Melin
1 sibling, 1 reply; 7+ messages in thread
From: Alexander Sverdlin @ 2018-08-01 0:15 UTC (permalink / raw)
To: u-boot
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 ())
> {
--
Alexander Sverdlin.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] xyz-modem: Change getc timeout loop waiting
2018-08-01 0:15 ` Alexander Sverdlin
@ 2018-08-01 5:44 ` Tomas Melin
2018-08-01 5:54 ` Alexander Sverdlin
0 siblings, 1 reply; 7+ messages in thread
From: Tomas Melin @ 2018-08-01 5:44 UTC (permalink / raw)
To: u-boot
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 ())
>> {
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] xyz-modem: Change getc timeout loop waiting
2018-08-01 5:44 ` Tomas Melin
@ 2018-08-01 5:54 ` Alexander Sverdlin
2018-08-01 6:16 ` Tomas Melin
0 siblings, 1 reply; 7+ messages in thread
From: Alexander Sverdlin @ 2018-08-01 5:54 UTC (permalink / raw)
To: u-boot
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.
--
Alexander Sverdlin.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] xyz-modem: Change getc timeout loop waiting
2018-08-01 5:54 ` Alexander Sverdlin
@ 2018-08-01 6:16 ` Tomas Melin
2018-08-02 14:47 ` Alexander Sverdlin
0 siblings, 1 reply; 7+ messages in thread
From: Tomas Melin @ 2018-08-01 6:16 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] xyz-modem: Change getc timeout loop waiting
2018-08-01 6:16 ` Tomas Melin
@ 2018-08-02 14:47 ` Alexander Sverdlin
0 siblings, 0 replies; 7+ messages in thread
From: Alexander Sverdlin @ 2018-08-02 14:47 UTC (permalink / raw)
To: u-boot
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.
--
Alexander Sverdlin.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-08-02 14:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-21 8:18 [U-Boot] [PATCH] xyz-modem: Change getc timeout loop waiting Tomas Melin
2016-11-29 1:07 ` [U-Boot] " Tom Rini
2018-08-01 0:15 ` Alexander Sverdlin
2018-08-01 5:44 ` Tomas Melin
2018-08-01 5:54 ` Alexander Sverdlin
2018-08-01 6:16 ` Tomas Melin
2018-08-02 14:47 ` Alexander Sverdlin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.