Skip to contentSkip to conversation

common/xyzModem.c: Fix delay timeout calculation

PATCH New Andrew F. Davis 2 participants 5 messages Aug 25, 2016 – Sep 20, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 common/xyzModem.c: Fix delay timeout calculation

When waiting for input in CYGACC_COMM_IF_GETC_TIMEOUT we delay 2 seconds by incrementing and checking a counter variable every 20 uSeconds. The overhead in the loop calling tstc() millions of times causes the timeout to be closer to 20 seconds. Delay longer per iteration to reduce overhead and bring the timeout back closer to the correct time.

---
 common/xyzModem.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
common/xyzModem.c +2 -3
index 5656aac..c3f2afc 100644
@@ -71,11 +71,10 @@ 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))
+  while (!tstc () && (counter < xyzModem_CHAR_TIMEOUT))
     {
-      udelay (DELAY);
+      mdelay (1);
       counter++;
     }
   if (tstc ())
Signature · 1 line
2.9.3
#1 lore raw

PATCH 1/1 common/xyzModem.c: Fix delay timeout calculation

Quoted 19 lines · Andrew F. Davis

When waiting for input in CYGACC_COMM_IF_GETC_TIMEOUT we delay 2 seconds by incrementing and checking a counter variable every 20 uSeconds. The overhead in the loop calling tstc() millions of times causes the timeout to be closer to 20 seconds. Delay longer per iteration to reduce overhead and bring the timeout back closer to the correct time.

---
 common/xyzModem.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
common/xyzModem.c +0 -0
index 5656aac..c3f2afc 100644
@@ -71,11 +71,10 @@ typedef int cyg_int32;
 static int
 CYGACC_COMM_IF_GETC_TIMEOUT (char chan, char *c)
 {

common/xyzModem.c typedef int cyg_int32; · line 80

Quoted 10 lines
common/xyzModem.c +2 -3
-#define DELAY 20
   unsigned long counter = 0;
-  while (!tstc () && (counter < xyzModem_CHAR_TIMEOUT * 1000 / DELAY))
+  while (!tstc () && (counter < xyzModem_CHAR_TIMEOUT))
     {
-      udelay (DELAY);
+      mdelay (1);
       counter++;
     }
   if (tstc ())

Thanks,
Stefan

#2 lore raw

PATCH 1/1 common/xyzModem.c: Fix delay timeout calculation

Quoted 12 lines · Stefan Roese
Quoted 11 lines · Andrew F. Davis

When waiting for input in CYGACC_COMM_IF_GETC_TIMEOUT we delay 2 seconds by incrementing and checking a counter variable every 20 uSeconds. The overhead in the loop calling tstc() millions of times causes the timeout to be closer to 20 seconds. Delay longer per iteration to reduce overhead and bring the timeout back closer to the correct time.

---
 common/xyzModem.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
Quoted 15 lines
Quoted 5 lines
common/xyzModem.c +0 -0
index 5656aac..c3f2afc 100644

common/xyzModem.c typedef int cyg_int32; · line 77

Quoted 10 lines
common/xyzModem.c +2 -3
@@ -71,11 +71,10 @@ 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))
+  while (!tstc () && (counter < xyzModem_CHAR_TIMEOUT))
     {
-      udelay (DELAY);
+      mdelay (1);

Thinking about this more, all we have to change is to remove the "* 1000" and switch to mdelay. That's a smaller change and results in an even more correct timeout time.

CYGACC_COMM_IF_GETC_TIMEOUT (char chan, char *c)
{
#define DELAY 20
unsigned long counter = 0;
- while (!tstc () && (counter < xyzModem_CHAR_TIMEOUT * 1000 / DELAY))
+ while (!tstc () && (counter < xyzModem_CHAR_TIMEOUT / DELAY)) {
- udelay (DELAY);

+      mdelay (DELAY);
       counter++;

}

Quoted 8 lines

common/xyzModem.c typedef int cyg_int32; · line 80

Quoted 3 lines
common/xyzModem.c +0 -0
       counter++;
     }
   if (tstc ())

Thanks,
Stefan

#3 lore raw

PATCH 1/1 common/xyzModem.c: Fix delay timeout calculation

Hi Andrew,

Quoted 37 lines · Andrew F. Davis
Quoted 12 lines · Stefan Roese
Quoted 11 lines · Andrew F. Davis

When waiting for input in CYGACC_COMM_IF_GETC_TIMEOUT we delay 2 seconds by incrementing and checking a counter variable every 20 uSeconds. The overhead in the loop calling tstc() millions of times causes the timeout to be closer to 20 seconds. Delay longer per iteration to reduce overhead and bring the timeout back closer to the correct time.

---
 common/xyzModem.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
Quoted 15 lines
Quoted 5 lines
common/xyzModem.c +0 -0
index 5656aac..c3f2afc 100644

common/xyzModem.c typedef int cyg_int32; · line 77

Quoted 10 lines
common/xyzModem.c +2 -3
@@ -71,11 +71,10 @@ 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))
+  while (!tstc () && (counter < xyzModem_CHAR_TIMEOUT))
     {
-      udelay (DELAY);
+      mdelay (1);

Thinking about this more, all we have to change is to remove the "* 1000" and switch to mdelay. That's a smaller change and results in an even more correct timeout time.

CYGACC_COMM_IF_GETC_TIMEOUT (char chan, char *c)
{
#define DELAY 20
unsigned long counter = 0;

common/xyzModem.c typedef int cyg_int32; · line 79

Quoted 7 lines
common/xyzModem.c +2 -2
-  while (!tstc () && (counter < xyzModem_CHAR_TIMEOUT * 1000 / DELAY))
+  while (!tstc () && (counter < xyzModem_CHAR_TIMEOUT / DELAY))
   {
-      udelay (DELAY);
+      mdelay (DELAY);
       counter++;
   }

Yes, this should also work and is a less intrusive change. But the result is that the delay is at least 20ms each time. And the original version had a much smaller delay here.

Even better would be if you change the loop to a timer based timeout detection. Please grep for get_timer() to see some examples.

Thanks,
Stefan

#4 lore raw

PATCH 1/1 common/xyzModem.c: Fix delay timeout calculation

Quoted 50 lines · Stefan Roese

Hi Andrew,

Quoted 39 lines · Andrew F. Davis
Quoted 14 lines · Stefan Roese
Quoted 13 lines · Andrew F. Davis

When waiting for input in CYGACC_COMM_IF_GETC_TIMEOUT we delay 2 seconds by incrementing and checking a counter variable every 20 uSeconds. The overhead in the loop calling tstc() millions of times causes the timeout to be closer to 20 seconds. Delay longer per iteration
to reduce overhead and bring the timeout back closer to the correct time.

---
 common/xyzModem.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
Quoted 15 lines
Quoted 5 lines
common/xyzModem.c +0 -0
index 5656aac..c3f2afc 100644

common/xyzModem.c typedef int cyg_int32; · line 77

Quoted 10 lines
common/xyzModem.c +2 -3
@@ -71,11 +71,10 @@ 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))
+  while (!tstc () && (counter < xyzModem_CHAR_TIMEOUT))
     {
-      udelay (DELAY);
+      mdelay (1);

Thinking about this more, all we have to change is to remove the "* 1000" and switch to mdelay. That's a smaller change and results in an even more correct timeout time.

CYGACC_COMM_IF_GETC_TIMEOUT (char chan, char *c)
{
#define DELAY 20
unsigned long counter = 0;

common/xyzModem.c typedef int cyg_int32; · line 79

Quoted 7 lines
common/xyzModem.c +2 -2
-  while (!tstc () && (counter < xyzModem_CHAR_TIMEOUT * 1000 / DELAY))
+  while (!tstc () && (counter < xyzModem_CHAR_TIMEOUT / DELAY))
   {
-      udelay (DELAY);
+      mdelay (DELAY);
       counter++;
   }
Quoted 7 lines

Yes, this should also work and is a less intrusive change. But the result is that the delay is at least 20ms each time. And the original version had a much smaller delay here.

Even better would be if you change the loop to a timer based timeout detection. Please grep for get_timer() to see some examples.

I think that will require more work then needed right now, at some point a lot of the xyzModem stuff will need cleaned/re-written, for now this is a working fix for something that is broken, so I'll re-submit v2 with the above change.

Quoted 2 lines

Thanks,
Stefan