[U-Boot] [PATCH] common/xyzModem.c: Fix delay timeout calculation

[Andrew F. Davis]

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 at 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