public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
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

  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