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

[Simon Glass]

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?

Regards,
Simon