From: Andrew F. Davis <afd@ti.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] common/xyzModem.c: Fix delay timeout calculation
Date: Tue, 20 Sep 2016 10:36:28 -0500 [thread overview]
Message-ID: <5a0e9717-fa32-f747-eec7-cef1652ccafb@ti.com> (raw)
In-Reply-To: <3c256ab8-a48a-43aa-a799-fe6a71c272c3@denx.de>
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
next prev parent reply other threads:[~2016-09-20 15:36 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
-- strict thread matches above, loose matches on Subject: below --
2016-08-25 18:36 Andrew F. Davis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5a0e9717-fa32-f747-eec7-cef1652ccafb@ti.com \
--to=afd@ti.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox