* [U-Boot] [PATCH] common/xyzModem.c: Fix delay timeout calculation
@ 2016-08-25 18:36 Andrew F. Davis
0 siblings, 0 replies; 6+ messages in thread
From: Andrew F. Davis @ 2016-08-25 18:36 UTC (permalink / raw)
To: u-boot
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.
Signed-off-by: Andrew F. Davis <afd@ti.com>
---
common/xyzModem.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/common/xyzModem.c b/common/xyzModem.c
index 5656aac..c3f2afc 100644
--- a/common/xyzModem.c
+++ b/common/xyzModem.c
@@ -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 ())
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] common/xyzModem.c: Fix delay timeout calculation
@ 2016-08-25 18:43 Andrew F. Davis
2016-08-26 5:18 ` Stefan Roese
0 siblings, 1 reply; 6+ messages in thread
From: Andrew F. Davis @ 2016-08-25 18:43 UTC (permalink / raw)
To: u-boot
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.
Signed-off-by: Andrew F. Davis <afd@ti.com>
---
common/xyzModem.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/common/xyzModem.c b/common/xyzModem.c
index 5656aac..c3f2afc 100644
--- a/common/xyzModem.c
+++ b/common/xyzModem.c
@@ -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 ())
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] common/xyzModem.c: Fix delay timeout calculation
2016-08-25 18:43 [U-Boot] [PATCH] common/xyzModem.c: Fix delay timeout calculation Andrew F. Davis
@ 2016-08-26 5:18 ` Stefan Roese
2016-08-26 13:40 ` Andrew F. Davis
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Roese @ 2016-08-26 5:18 UTC (permalink / raw)
To: u-boot
On 25.08.2016 20:43, Andrew F. Davis wrote:
> 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.
>
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> ---
> common/xyzModem.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/common/xyzModem.c b/common/xyzModem.c
> index 5656aac..c3f2afc 100644
> --- a/common/xyzModem.c
> +++ b/common/xyzModem.c
> @@ -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 ())
Reviewed-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] common/xyzModem.c: Fix delay timeout calculation
2016-08-26 5:18 ` Stefan Roese
@ 2016-08-26 13:40 ` Andrew F. Davis
2016-08-26 13:43 ` Stefan Roese
0 siblings, 1 reply; 6+ messages in thread
From: Andrew F. Davis @ 2016-08-26 13:40 UTC (permalink / raw)
To: u-boot
On 08/26/2016 12:18 AM, Stefan Roese wrote:
> On 25.08.2016 20:43, Andrew F. Davis wrote:
>> 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.
>>
>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>> ---
>> common/xyzModem.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/common/xyzModem.c b/common/xyzModem.c
>> index 5656aac..c3f2afc 100644
>> --- a/common/xyzModem.c
>> +++ b/common/xyzModem.c
>> @@ -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++;
}
>> counter++;
>> }
>> if (tstc ())
>
> Reviewed-by: Stefan Roese <sr@denx.de>
>
> Thanks,
> Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] common/xyzModem.c: Fix delay timeout calculation
2016-08-26 13:40 ` Andrew F. Davis
@ 2016-08-26 13:43 ` Stefan Roese
2016-09-20 15:36 ` Andrew F. Davis
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Roese @ 2016-08-26 13:43 UTC (permalink / raw)
To: u-boot
Hi Andrew,
On 26.08.2016 15:40, Andrew F. Davis wrote:
> On 08/26/2016 12:18 AM, Stefan Roese wrote:
>> On 25.08.2016 20:43, Andrew F. Davis wrote:
>>> 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.
>>>
>>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>>> ---
>>> common/xyzModem.c | 5 ++---
>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/common/xyzModem.c b/common/xyzModem.c
>>> index 5656aac..c3f2afc 100644
>>> --- a/common/xyzModem.c
>>> +++ b/common/xyzModem.c
>>> @@ -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++;
> }
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] common/xyzModem.c: Fix delay timeout calculation
2016-08-26 13:43 ` Stefan Roese
@ 2016-09-20 15:36 ` Andrew F. Davis
0 siblings, 0 replies; 6+ messages in thread
From: Andrew F. Davis @ 2016-09-20 15:36 UTC (permalink / raw)
To: u-boot
On 08/26/2016 08:43 AM, Stefan Roese wrote:
> Hi Andrew,
>
> On 26.08.2016 15:40, Andrew F. Davis wrote:
>> On 08/26/2016 12:18 AM, Stefan Roese wrote:
>>> On 25.08.2016 20:43, Andrew F. Davis wrote:
>>>> 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.
>>>>
>>>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>>>> ---
>>>> common/xyzModem.c | 5 ++---
>>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/common/xyzModem.c b/common/xyzModem.c
>>>> index 5656aac..c3f2afc 100644
>>>> --- a/common/xyzModem.c
>>>> +++ b/common/xyzModem.c
>>>> @@ -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++;
>> }
>
> 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.
> Thanks,
> Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-20 15:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-25 18:43 [U-Boot] [PATCH] common/xyzModem.c: Fix delay timeout calculation Andrew F. Davis
2016-08-26 5:18 ` Stefan Roese
2016-08-26 13:40 ` Andrew F. Davis
2016-08-26 13:43 ` Stefan Roese
2016-09-20 15:36 ` Andrew F. Davis
-- strict thread matches above, loose matches on Subject: below --
2016-08-25 18:36 Andrew F. Davis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox