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

[Simon Glass]

Hi Lokesh,

On 27 September 2017 at 00:27, Lokesh Vutla <lokeshvutla at ti.com> wrote:
>
>
> 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/

Well it is very strange. I cannot imagine that a serial line could
keep up with the CPU requesting bytes. I think this needs more
investigation.

Regards,
Simon