[U-Boot] [PATCH] xyz-modem: Fix timeout loop waiting with WATCHDOG

[Lokesh Vutla]

On Monday 25 September 2017 07:45 AM, Simon Glass wrote:
> Hi,
> 
> On 16 September 2017 at 23:12, Lokesh Vutla <lokeshvutla at ti.com> wrote:
>> Simon,
>>
>> On 9/16/2017 9:43 PM, Simon Glass wrote:
>>> Hi,
>>>
>>> On 16 September 2017 at 07:43, Tom Rini <trini at konsulko.com> wrote:
>>>> On Sat, Sep 16, 2017 at 05:14:31PM +0530, Lokesh Vutla wrote:
>>>>> Commit 2c77c0d6524eb ("xyz-modem: Change getc timeout loop waiting")
>>>>> fixes the loop delay when using a hw watchdog. But assuming that Watchdog
>>>>> kicking is taken care of by getc(). This is not true in case of DM_SERIAL.
>>>>> So, kick the watchdog before loop waiting, instead of relying on other
>>>>> functions.
>>>>>
>>>>> Signed-off-by: Lokesh Vutla <lokeshvutla at ti.com>
>>>>> ---
>>>>> - This fixes UART boot on AM335x-evm.
>>>>>
>>>>>  common/xyzModem.c | 2 ++
>>>>>  1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/common/xyzModem.c b/common/xyzModem.c
>>>>> index a0c5dfeece..8c4679473f 100644
>>>>> --- a/common/xyzModem.c
>>>>> +++ b/common/xyzModem.c
>>>>> @@ -26,6 +26,7 @@
>>>>>  #include <xyzModem.h>
>>>>>  #include <stdarg.h>
>>>>>  #include <crc.h>
>>>>> +#include <watchdog.h>
>>>>>
>>>>>  /* Assumption - run xyzModem protocol over the console port */
>>>>>
>>>>> @@ -64,6 +65,7 @@ CYGACC_COMM_IF_GETC_TIMEOUT (char chan, char *c)
>>>>>  {
>>>>>
>>>>>    ulong now = get_timer(0);
>>>>> +  WATCHDOG_RESET();
>>>>>    while (!tstc ())
>>>>>      {
>>>>>        if (get_timer(now) > xyzModem_CHAR_TIMEOUT)
>>>>
>>>> My worry is that other places also assume watchdog petted by getc().
>>>> Simon, is there a reason DM_SERIAL isn't doing what was done before?
>>>> Thanks!
>>>
>>> This should really be fixed at source. Is the problems in
>>> _serial_tstc()? The watchdog is reset with _serial_getc().
>>
>> Sorry, my bad. WATCHDOG_RESET() is being called in __serial_getc() when
>> err is -EAGAIN. But looks like it is never hitting this case. This way
>> ymodem fails as watchdog gets reset when downloading large files.
> 
> So every time we read a character there is one available? How can that
> happen? Surely the CPU can run faster than the serial port?

For a quick experiment, I added a infinite while loop[1] instead of
-EAGAIN. I never hit this case when downloading with ymodem. File
downloaded successfully with $patch even with the infinite while loop.
Am I missing something?

[1] http://pastebin.ubuntu.com/25625299/

Thanks and regards,
Lokesh