[U-Boot] [PATCH] xyz-modem: Fix timeout loop waiting with WATCHDOG
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@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)
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@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!
Hi,
On 16 September 2017 at 07:43, Tom Rini trini@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@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().
Regards, Simon
Simon,
On 9/16/2017 9:43 PM, Simon Glass wrote:
Hi,
On 16 September 2017 at 07:43, Tom Rini trini@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@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.
Thanks and regards, Lokesh
Regards, Simon
Hi,
On 16 September 2017 at 23:12, Lokesh Vutla lokeshvutla@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@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@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
On Monday 25 September 2017 07:45 AM, Simon Glass wrote:
Hi,
On 16 September 2017 at 23:12, Lokesh Vutla lokeshvutla@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@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@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
Hi Lokesh,
On 27 September 2017 at 00:27, Lokesh Vutla lokeshvutla@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@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@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@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?
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
participants (3)
-
Lokesh Vutla -
Simon Glass -
Tom Rini