public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH PATCH v1] watchdog: Fix Watchdog Reset while in U-Boot Prompt
@ 2016-07-13 10:56 Andreas J. Reichel
  2016-08-01  1:01 ` Simon Glass
  2016-10-18 22:47 ` [U-Boot] [U-Boot, v1] watchdog: Fix Watchdog Reset while in U-Boot Prompt Tom Rini
  0 siblings, 2 replies; 8+ messages in thread
From: Andreas J. Reichel @ 2016-07-13 10:56 UTC (permalink / raw)
  To: u-boot

Hardware: CM-FX6 Module from Compulab

This patch fixes unwanted watchdog resets while the user enters
a command at the U-Boot prompt.

As found on the CM-FX6 board from Compulab, when having enabled the
watchdog, a missing WATCHDOG_RESET call in common/console.c causes
this and alike boards to reset when the watchdog's timeout has
elapsed while waiting at the U-Boot prompt.

Despite the user could press several keys within the watchdog
timeout limit, the while loop in cli_readline.c, line 261, does only
call WATCHDOG_RESET if first == 1, which gets set to 0 in the 1st
loop iteration. This leads to a watchdog timeout no matter if the
user presses keys or not.

Although, this affects other boards as well as it touches
common/console.c, the macro WATCHDOG_RESET expands to {} if watchdog
support isn't configured. Hence, there's no harm caused and no need to
surround it by #ifdef in this case.

 * Symptom:
   U-Boot resets after watchdog times out when in commandline prompt
   and watchdog is enabled.

 * Reasoning:
   When U-Boot shows the commandline prompt, the following function
   call stack is executed while waiting for a keypress:

   common/main.c:
                    main_loop          => common/cli.c: cli_loop() =>
   common/cli_hush.c:
                    parse_file_outer   => parse_stream_outer       =>
                    parse_stream       => b_getch(i)               =>
                    i->get(i)          => file_get                 =>
                    get_user_input     => cmdedit_read_input       =>
                    uboot_cli_readline =>
   common/cli_readline.c:
                    cli_readline       => cli_readline_into_buffer =>
                    cread_line         => getcmd_getch (== getc)   =>
   common/console.c:
                    fgetc              => console_tstc

   common/console.c:
   (with CONFIG_CONSOLE_MUX is set)

   - in console_tstc line 181:
   If dev->tstc(dev) returns 0, the global tstcdev variable doesn't get
   set. This is the case if no character is in the serial buffer.

   - in fgetc(int file), line 297:
   Program flow keeps looping because tstcdev does not get set.
   Therefore WATCHDOG_RESET is not called, as mx_serial_tstc from
   drivers/serial/serial_mxc.c does not call it.

 * Solution:
   Add WATCHDOG_RESET into the loop of console_tstc.

   Note: Macro expands to {} if not configured, so no #ifdef is needed.

 * Comment:

Signed-off-by: Christian Storm <christian.storm@tngtech.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Andreas J. Reichel <Andreas.Reichel@tngtech.com>
---

 common/console.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/common/console.c b/common/console.c
index 12293f3..054b6cd 100644
--- a/common/console.c
+++ b/common/console.c
@@ -16,6 +16,7 @@
 #include <stdio_dev.h>
 #include <exports.h>
 #include <environment.h>
+#include <watchdog.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -294,6 +295,7 @@ int fgetc(int file)
 		 * Effectively poll for input wherever it may be available.
 		 */
 		for (;;) {
+			WATCHDOG_RESET();
 			/*
 			 * Upper layer may have already called tstc() so
 			 * check for that first.
-- 
2.8.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH PATCH v1] watchdog: Fix Watchdog Reset while in U-Boot Prompt
  2016-07-13 10:56 [U-Boot] [PATCH PATCH v1] watchdog: Fix Watchdog Reset while in U-Boot Prompt Andreas J. Reichel
@ 2016-08-01  1:01 ` Simon Glass
  2016-08-01 11:49   ` [U-Boot] [PATCH v2] " Andreas J. Reichel
  2016-08-01 13:32   ` [U-Boot] [PATCH v2 0/1] Fix U-Boot Prompt on CM-FX6 with enabled watchdog Andreas J. Reichel
  2016-10-18 22:47 ` [U-Boot] [U-Boot, v1] watchdog: Fix Watchdog Reset while in U-Boot Prompt Tom Rini
  1 sibling, 2 replies; 8+ messages in thread
From: Simon Glass @ 2016-08-01  1:01 UTC (permalink / raw)
  To: u-boot

Hi,

On 13 July 2016 at 04:56, Andreas J. Reichel
<Andreas.Reichel@tngtech.com> wrote:
> Hardware: CM-FX6 Module from Compulab
>
> This patch fixes unwanted watchdog resets while the user enters
> a command at the U-Boot prompt.
>
> As found on the CM-FX6 board from Compulab, when having enabled the
> watchdog, a missing WATCHDOG_RESET call in common/console.c causes
> this and alike boards to reset when the watchdog's timeout has
> elapsed while waiting at the U-Boot prompt.
>
> Despite the user could press several keys within the watchdog
> timeout limit, the while loop in cli_readline.c, line 261, does only
> call WATCHDOG_RESET if first == 1, which gets set to 0 in the 1st
> loop iteration. This leads to a watchdog timeout no matter if the
> user presses keys or not.
>
> Although, this affects other boards as well as it touches
> common/console.c, the macro WATCHDOG_RESET expands to {} if watchdog
> support isn't configured. Hence, there's no harm caused and no need to
> surround it by #ifdef in this case.
>
>  * Symptom:
>    U-Boot resets after watchdog times out when in commandline prompt
>    and watchdog is enabled.
>
>  * Reasoning:
>    When U-Boot shows the commandline prompt, the following function
>    call stack is executed while waiting for a keypress:
>
>    common/main.c:
>                     main_loop          => common/cli.c: cli_loop() =>
>    common/cli_hush.c:
>                     parse_file_outer   => parse_stream_outer       =>
>                     parse_stream       => b_getch(i)               =>
>                     i->get(i)          => file_get                 =>
>                     get_user_input     => cmdedit_read_input       =>
>                     uboot_cli_readline =>
>    common/cli_readline.c:
>                     cli_readline       => cli_readline_into_buffer =>
>                     cread_line         => getcmd_getch (== getc)   =>
>    common/console.c:
>                     fgetc              => console_tstc
>
>    common/console.c:
>    (with CONFIG_CONSOLE_MUX is set)
>
>    - in console_tstc line 181:
>    If dev->tstc(dev) returns 0, the global tstcdev variable doesn't get
>    set. This is the case if no character is in the serial buffer.
>
>    - in fgetc(int file), line 297:
>    Program flow keeps looping because tstcdev does not get set.
>    Therefore WATCHDOG_RESET is not called, as mx_serial_tstc from
>    drivers/serial/serial_mxc.c does not call it.
>
>  * Solution:
>    Add WATCHDOG_RESET into the loop of console_tstc.
>
>    Note: Macro expands to {} if not configured, so no #ifdef is needed.
>
>  * Comment:
>
> Signed-off-by: Christian Storm <christian.storm@tngtech.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> Signed-off-by: Andreas J. Reichel <Andreas.Reichel@tngtech.com>

Thanks for the detailed info.

Would it be better to put this in _serial_tstc()?

> ---
>
>  common/console.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/common/console.c b/common/console.c
> index 12293f3..054b6cd 100644
> --- a/common/console.c
> +++ b/common/console.c
> @@ -16,6 +16,7 @@
>  #include <stdio_dev.h>
>  #include <exports.h>
>  #include <environment.h>
> +#include <watchdog.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> @@ -294,6 +295,7 @@ int fgetc(int file)
>                  * Effectively poll for input wherever it may be available.
>                  */
>                 for (;;) {
> +                       WATCHDOG_RESET();
>                         /*
>                          * Upper layer may have already called tstc() so
>                          * check for that first.
> --
> 2.8.2
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v2] watchdog: Fix Watchdog Reset while in U-Boot Prompt
  2016-08-01  1:01 ` Simon Glass
@ 2016-08-01 11:49   ` Andreas J. Reichel
  2016-09-06  1:04     ` Simon Glass
  2016-08-01 13:32   ` [U-Boot] [PATCH v2 0/1] Fix U-Boot Prompt on CM-FX6 with enabled watchdog Andreas J. Reichel
  1 sibling, 1 reply; 8+ messages in thread
From: Andreas J. Reichel @ 2016-08-01 11:49 UTC (permalink / raw)
  To: u-boot

This patch fixes unwanted watchdog resets while the user enters
a command at the U-Boot prompt.

As found on the CM-FX6 board from Compulab, when having enabled the
watchdog, a missing WATCHDOG_RESET call in _serial_tstc() in
(/drivers/serial/serial-uclass.c) causes this and alike boards to
reset when the watchdog's timeout has elapsed while waiting at the
U-Boot prompt.

Despite the user could press several keys within the watchdog
timeout limit, the while loop in cli_readline.c, line 261, does only
call WATCHDOG_RESET if first == 1, which gets set to 0 in the 1st
loop iteration. This leads to a watchdog timeout no matter if the
user presses keys or not.

The problem is solved with a call to WATCHDOG_RESET in _serial_tstc,
defined in drivers/serial/serial-uclass.c.

Since the macro WATCHDOG_RESET expands to {} if watchdog support
isn't configured, there's no need to surround it by #ifdef in this
case.

 * Symptom:
   U-Boot resets after watchdog times out when in commandline prompt
   and watchdog is enabled.

 * Reasoning:
   When U-Boot shows the commandline prompt, the following function
   call stack is executed while waiting for a keypress:

   common/main.c:
                    main_loop          => common/cli.c: cli_loop() =>
   common/cli_hush.c:
                    parse_file_outer   => parse_stream_outer       =>
                    parse_stream       => b_getch(i)               =>
                    i->get(i)          => file_get                 =>
                    get_user_input     => cmdedit_read_input       =>
                    uboot_cli_readline =>
   common/cli_readline.c:
                    cli_readline       => cli_readline_into_buffer =>
                    cread_line         => getcmd_getch (== getc)   =>
   commonn/console.c:
                    fgetc              => console_tstc

   - in console_tstc line 181:
   If dev->tstc(dev) returns 0, the global tstcdev variable doesn't
   get set. This is the case if no character is in the serial buffer.

   - in fgetc(int file), line 297:
   Program flow keeps looping because tstcdev does not get set.
   Therefore WATCHDOG_RESET is not called, as mx_serial_tstc from
   drivers/serial/serial_mxc.c does not call it.

   - Initialization calls drv_system_init in stdio.c, which sets
   dev.tstc = stdio_serial_tstc. Thus, dev->tstc(dev) calls serial_tstc()
   which in turn calls _serial_tstc().

   Hence, _serial_tstc() needs to call WATCHDOG_RESET() to periodically
   reset the watchdog while cli_readline waits for user input.

Signed-off-by: Christian Storm <christian.storm@tngtech.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Andreas J. Reichel <Andreas.Reichel@tngtech.com>
---

Changes in v2:
 - Move WATCHDOG_RESET() call from common/console.c to
   drivers/serial/serial-uclass.c.

 drivers/serial/serial-uclass.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index 0ce5c44..72cf808 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -155,6 +155,7 @@ static int _serial_tstc(struct udevice *dev)
 {
 	struct dm_serial_ops *ops = serial_get_ops(dev);
 
+	WATCHDOG_RESET();
 	if (ops->pending)
 		return ops->pending(dev, true);
 
-- 
2.8.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v2 0/1] Fix U-Boot Prompt on CM-FX6 with enabled watchdog
  2016-08-01  1:01 ` Simon Glass
  2016-08-01 11:49   ` [U-Boot] [PATCH v2] " Andreas J. Reichel
@ 2016-08-01 13:32   ` Andreas J. Reichel
  1 sibling, 0 replies; 8+ messages in thread
From: Andreas J. Reichel @ 2016-08-01 13:32 UTC (permalink / raw)
  To: u-boot


This patch fixes unwanted watchdog resets while the user enters
a command at the U-Boot prompt.

As found on the CM-FX6 board from Compulab, when having enabled the
watchdog, a missing WATCHDOG_RESET call in _serial_tstc() in
(/drivers/serial/serial-uclass.c) causes this and alike boards to
reset when the watchdog's timeout has elapsed while waiting at the
U-Boot prompt.

Despite the user could press several keys within the watchdog
timeout limit, the while loop in cli_readline.c, line 261, does only
call WATCHDOG_RESET if first == 1, which gets set to 0 in the 1st
loop iteration. This leads to a watchdog timeout no matter if the
user presses keys or not.

The problem is solved with a call to WATCHDOG_RESET in _serial_tstc,
defined in drivers/serial/serial-uclass.c.

Since the macro WATCHDOG_RESET expands to {} if watchdog support
isn't configured, there's no need to surround it by #ifdef in this
case.

Changes in v2:
 - Move WATCHDOG_RESET() call from common/console.c to
   drivers/serial/serial-uclass.c.

Andreas J. Reichel (1):
  watchdog: Fix Watchdog Reset while in U-Boot Prompt

 drivers/serial/serial-uclass.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.8.2

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v2] watchdog: Fix Watchdog Reset while in U-Boot Prompt
  2016-08-01 11:49   ` [U-Boot] [PATCH v2] " Andreas J. Reichel
@ 2016-09-06  1:04     ` Simon Glass
  2016-09-19 11:59       ` Andreas Reichel
  0 siblings, 1 reply; 8+ messages in thread
From: Simon Glass @ 2016-09-06  1:04 UTC (permalink / raw)
  To: u-boot

+Tom, in case this should go into the release.

On 1 August 2016 at 05:49, Andreas J. Reichel
<Andreas.Reichel@tngtech.com> wrote:
> This patch fixes unwanted watchdog resets while the user enters
> a command at the U-Boot prompt.
>
> As found on the CM-FX6 board from Compulab, when having enabled the
> watchdog, a missing WATCHDOG_RESET call in _serial_tstc() in
> (/drivers/serial/serial-uclass.c) causes this and alike boards to
> reset when the watchdog's timeout has elapsed while waiting at the
> U-Boot prompt.
>
> Despite the user could press several keys within the watchdog
> timeout limit, the while loop in cli_readline.c, line 261, does only
> call WATCHDOG_RESET if first == 1, which gets set to 0 in the 1st
> loop iteration. This leads to a watchdog timeout no matter if the
> user presses keys or not.
>
> The problem is solved with a call to WATCHDOG_RESET in _serial_tstc,
> defined in drivers/serial/serial-uclass.c.
>
> Since the macro WATCHDOG_RESET expands to {} if watchdog support
> isn't configured, there's no need to surround it by #ifdef in this
> case.
>
>  * Symptom:
>    U-Boot resets after watchdog times out when in commandline prompt
>    and watchdog is enabled.
>
>  * Reasoning:
>    When U-Boot shows the commandline prompt, the following function
>    call stack is executed while waiting for a keypress:
>
>    common/main.c:
>                     main_loop          => common/cli.c: cli_loop() =>
>    common/cli_hush.c:
>                     parse_file_outer   => parse_stream_outer       =>
>                     parse_stream       => b_getch(i)               =>
>                     i->get(i)          => file_get                 =>
>                     get_user_input     => cmdedit_read_input       =>
>                     uboot_cli_readline =>
>    common/cli_readline.c:
>                     cli_readline       => cli_readline_into_buffer =>
>                     cread_line         => getcmd_getch (== getc)   =>
>    commonn/console.c:
>                     fgetc              => console_tstc
>
>    - in console_tstc line 181:
>    If dev->tstc(dev) returns 0, the global tstcdev variable doesn't
>    get set. This is the case if no character is in the serial buffer.
>
>    - in fgetc(int file), line 297:
>    Program flow keeps looping because tstcdev does not get set.
>    Therefore WATCHDOG_RESET is not called, as mx_serial_tstc from
>    drivers/serial/serial_mxc.c does not call it.
>
>    - Initialization calls drv_system_init in stdio.c, which sets
>    dev.tstc = stdio_serial_tstc. Thus, dev->tstc(dev) calls serial_tstc()
>    which in turn calls _serial_tstc().
>
>    Hence, _serial_tstc() needs to call WATCHDOG_RESET() to periodically
>    reset the watchdog while cli_readline waits for user input.
>
> Signed-off-by: Christian Storm <christian.storm@tngtech.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> Signed-off-by: Andreas J. Reichel <Andreas.Reichel@tngtech.com>
> ---
>
> Changes in v2:
>  - Move WATCHDOG_RESET() call from common/console.c to
>    drivers/serial/serial-uclass.c.
>
>  drivers/serial/serial-uclass.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
> index 0ce5c44..72cf808 100644
> --- a/drivers/serial/serial-uclass.c
> +++ b/drivers/serial/serial-uclass.c
> @@ -155,6 +155,7 @@ static int _serial_tstc(struct udevice *dev)
>  {
>         struct dm_serial_ops *ops = serial_get_ops(dev);
>
> +       WATCHDOG_RESET();
>         if (ops->pending)
>                 return ops->pending(dev, true);

Great explanation, thank you.

Acked-by: Simon Glass <sjg@chromium.org>

>
> --
> 2.8.2
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v2] watchdog: Fix Watchdog Reset while in U-Boot Prompt
  2016-09-06  1:04     ` Simon Glass
@ 2016-09-19 11:59       ` Andreas Reichel
  2016-10-15  1:45         ` Tom Rini
  0 siblings, 1 reply; 8+ messages in thread
From: Andreas Reichel @ 2016-09-19 11:59 UTC (permalink / raw)
  To: u-boot

Hi Tom,

is my patch going to be applied or is the problem resolved
otherwhise?

Kind regards
    Andreas

On Mo, Sep 05, 2016 at 07:04:53 -0600, Simon Glass wrote:
> +Tom, in case this should go into the release.
> 
> On 1 August 2016 at 05:49, Andreas J. Reichel
> <Andreas.Reichel@tngtech.com> wrote:
> > This patch fixes unwanted watchdog resets while the user enters
> > a command at the U-Boot prompt.
> >
> > As found on the CM-FX6 board from Compulab, when having enabled the
> > watchdog, a missing WATCHDOG_RESET call in _serial_tstc() in
> > (/drivers/serial/serial-uclass.c) causes this and alike boards to
> > reset when the watchdog's timeout has elapsed while waiting at the
> > U-Boot prompt.
> >
> > Despite the user could press several keys within the watchdog
> > timeout limit, the while loop in cli_readline.c, line 261, does only
> > call WATCHDOG_RESET if first == 1, which gets set to 0 in the 1st
> > loop iteration. This leads to a watchdog timeout no matter if the
> > user presses keys or not.
> >
> > The problem is solved with a call to WATCHDOG_RESET in _serial_tstc,
> > defined in drivers/serial/serial-uclass.c.
> >
> > Since the macro WATCHDOG_RESET expands to {} if watchdog support
> > isn't configured, there's no need to surround it by #ifdef in this
> > case.
> >
> >  * Symptom:
> >    U-Boot resets after watchdog times out when in commandline prompt
> >    and watchdog is enabled.
> >
> >  * Reasoning:
> >    When U-Boot shows the commandline prompt, the following function
> >    call stack is executed while waiting for a keypress:
> >
> >    common/main.c:
> >                     main_loop          => common/cli.c: cli_loop() =>
> >    common/cli_hush.c:
> >                     parse_file_outer   => parse_stream_outer       =>
> >                     parse_stream       => b_getch(i)               =>
> >                     i->get(i)          => file_get                 =>
> >                     get_user_input     => cmdedit_read_input       =>
> >                     uboot_cli_readline =>
> >    common/cli_readline.c:
> >                     cli_readline       => cli_readline_into_buffer =>
> >                     cread_line         => getcmd_getch (== getc)   =>
> >    commonn/console.c:
> >                     fgetc              => console_tstc
> >
> >    - in console_tstc line 181:
> >    If dev->tstc(dev) returns 0, the global tstcdev variable doesn't
> >    get set. This is the case if no character is in the serial buffer.
> >
> >    - in fgetc(int file), line 297:
> >    Program flow keeps looping because tstcdev does not get set.
> >    Therefore WATCHDOG_RESET is not called, as mx_serial_tstc from
> >    drivers/serial/serial_mxc.c does not call it.
> >
> >    - Initialization calls drv_system_init in stdio.c, which sets
> >    dev.tstc = stdio_serial_tstc. Thus, dev->tstc(dev) calls serial_tstc()
> >    which in turn calls _serial_tstc().
> >
> >    Hence, _serial_tstc() needs to call WATCHDOG_RESET() to periodically
> >    reset the watchdog while cli_readline waits for user input.
> >
> > Signed-off-by: Christian Storm <christian.storm@tngtech.com>
> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > Signed-off-by: Andreas J. Reichel <Andreas.Reichel@tngtech.com>
> > ---
> >
> > Changes in v2:
> >  - Move WATCHDOG_RESET() call from common/console.c to
> >    drivers/serial/serial-uclass.c.
> >
> >  drivers/serial/serial-uclass.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
> > index 0ce5c44..72cf808 100644
> > --- a/drivers/serial/serial-uclass.c
> > +++ b/drivers/serial/serial-uclass.c
> > @@ -155,6 +155,7 @@ static int _serial_tstc(struct udevice *dev)
> >  {
> >         struct dm_serial_ops *ops = serial_get_ops(dev);
> >
> > +       WATCHDOG_RESET();
> >         if (ops->pending)
> >                 return ops->pending(dev, true);
> 
> Great explanation, thank you.
> 
> Acked-by: Simon Glass <sjg@chromium.org>
> 
> >
> > --
> > 2.8.2
> >

-- 
Andreas Reichel 
Dipl.-Phys. (Univ.) 
Software Consultant

Andreas.Reichel at tngtech.com 
+49-174-3180074

TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterf?hring
Gesch?ftsf?hrer: Henrik Klagges, Christoph Stock, Dr. Robert Dahlke
Sitz: Unterf?hring * Amtsgericht M?nchen * HRB 135082

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v2] watchdog: Fix Watchdog Reset while in U-Boot Prompt
  2016-09-19 11:59       ` Andreas Reichel
@ 2016-10-15  1:45         ` Tom Rini
  0 siblings, 0 replies; 8+ messages in thread
From: Tom Rini @ 2016-10-15  1:45 UTC (permalink / raw)
  To: u-boot

On Mon, Sep 19, 2016 at 01:59:44PM +0200, Andreas Reichel wrote:
> Hi Tom,
> 
> is my patch going to be applied or is the problem resolved
> otherwhise?

Sorry, lost in my patchwork queue.  I'll pick this up soon now.

> 
> Kind regards
>     Andreas
> 
> On Mo, Sep 05, 2016 at 07:04:53 -0600, Simon Glass wrote:
> > +Tom, in case this should go into the release.
> > 
> > On 1 August 2016 at 05:49, Andreas J. Reichel
> > <Andreas.Reichel@tngtech.com> wrote:
> > > This patch fixes unwanted watchdog resets while the user enters
> > > a command at the U-Boot prompt.
> > >
> > > As found on the CM-FX6 board from Compulab, when having enabled the
> > > watchdog, a missing WATCHDOG_RESET call in _serial_tstc() in
> > > (/drivers/serial/serial-uclass.c) causes this and alike boards to
> > > reset when the watchdog's timeout has elapsed while waiting at the
> > > U-Boot prompt.
> > >
> > > Despite the user could press several keys within the watchdog
> > > timeout limit, the while loop in cli_readline.c, line 261, does only
> > > call WATCHDOG_RESET if first == 1, which gets set to 0 in the 1st
> > > loop iteration. This leads to a watchdog timeout no matter if the
> > > user presses keys or not.
> > >
> > > The problem is solved with a call to WATCHDOG_RESET in _serial_tstc,
> > > defined in drivers/serial/serial-uclass.c.
> > >
> > > Since the macro WATCHDOG_RESET expands to {} if watchdog support
> > > isn't configured, there's no need to surround it by #ifdef in this
> > > case.
> > >
> > >  * Symptom:
> > >    U-Boot resets after watchdog times out when in commandline prompt
> > >    and watchdog is enabled.
> > >
> > >  * Reasoning:
> > >    When U-Boot shows the commandline prompt, the following function
> > >    call stack is executed while waiting for a keypress:
> > >
> > >    common/main.c:
> > >                     main_loop          => common/cli.c: cli_loop() =>
> > >    common/cli_hush.c:
> > >                     parse_file_outer   => parse_stream_outer       =>
> > >                     parse_stream       => b_getch(i)               =>
> > >                     i->get(i)          => file_get                 =>
> > >                     get_user_input     => cmdedit_read_input       =>
> > >                     uboot_cli_readline =>
> > >    common/cli_readline.c:
> > >                     cli_readline       => cli_readline_into_buffer =>
> > >                     cread_line         => getcmd_getch (== getc)   =>
> > >    commonn/console.c:
> > >                     fgetc              => console_tstc
> > >
> > >    - in console_tstc line 181:
> > >    If dev->tstc(dev) returns 0, the global tstcdev variable doesn't
> > >    get set. This is the case if no character is in the serial buffer.
> > >
> > >    - in fgetc(int file), line 297:
> > >    Program flow keeps looping because tstcdev does not get set.
> > >    Therefore WATCHDOG_RESET is not called, as mx_serial_tstc from
> > >    drivers/serial/serial_mxc.c does not call it.
> > >
> > >    - Initialization calls drv_system_init in stdio.c, which sets
> > >    dev.tstc = stdio_serial_tstc. Thus, dev->tstc(dev) calls serial_tstc()
> > >    which in turn calls _serial_tstc().
> > >
> > >    Hence, _serial_tstc() needs to call WATCHDOG_RESET() to periodically
> > >    reset the watchdog while cli_readline waits for user input.
> > >
> > > Signed-off-by: Christian Storm <christian.storm@tngtech.com>
> > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > > Signed-off-by: Andreas J. Reichel <Andreas.Reichel@tngtech.com>
> > > ---
> > >
> > > Changes in v2:
> > >  - Move WATCHDOG_RESET() call from common/console.c to
> > >    drivers/serial/serial-uclass.c.
> > >
> > >  drivers/serial/serial-uclass.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
> > > index 0ce5c44..72cf808 100644
> > > --- a/drivers/serial/serial-uclass.c
> > > +++ b/drivers/serial/serial-uclass.c
> > > @@ -155,6 +155,7 @@ static int _serial_tstc(struct udevice *dev)
> > >  {
> > >         struct dm_serial_ops *ops = serial_get_ops(dev);
> > >
> > > +       WATCHDOG_RESET();
> > >         if (ops->pending)
> > >                 return ops->pending(dev, true);
> > 
> > Great explanation, thank you.
> > 
> > Acked-by: Simon Glass <sjg@chromium.org>
> > 
> > >
> > > --
> > > 2.8.2
> > >
> 
> -- 
> Andreas Reichel 
> Dipl.-Phys. (Univ.) 
> Software Consultant
> 
> Andreas.Reichel at tngtech.com 
> +49-174-3180074
> 
> TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterf?hring
> Gesch?ftsf?hrer: Henrik Klagges, Christoph Stock, Dr. Robert Dahlke
> Sitz: Unterf?hring * Amtsgericht M?nchen * HRB 135082

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161014/76937bc6/attachment.sig>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [U-Boot, v1] watchdog: Fix Watchdog Reset while in U-Boot Prompt
  2016-07-13 10:56 [U-Boot] [PATCH PATCH v1] watchdog: Fix Watchdog Reset while in U-Boot Prompt Andreas J. Reichel
  2016-08-01  1:01 ` Simon Glass
@ 2016-10-18 22:47 ` Tom Rini
  1 sibling, 0 replies; 8+ messages in thread
From: Tom Rini @ 2016-10-18 22:47 UTC (permalink / raw)
  To: u-boot

On Wed, Jul 13, 2016 at 12:56:51PM +0200, Andreas J. Reichel wrote:

> Hardware: CM-FX6 Module from Compulab
> 
> This patch fixes unwanted watchdog resets while the user enters
> a command at the U-Boot prompt.
> 
> As found on the CM-FX6 board from Compulab, when having enabled the
> watchdog, a missing WATCHDOG_RESET call in common/console.c causes
> this and alike boards to reset when the watchdog's timeout has
> elapsed while waiting at the U-Boot prompt.
> 
> Despite the user could press several keys within the watchdog
> timeout limit, the while loop in cli_readline.c, line 261, does only
> call WATCHDOG_RESET if first == 1, which gets set to 0 in the 1st
> loop iteration. This leads to a watchdog timeout no matter if the
> user presses keys or not.
> 
> Although, this affects other boards as well as it touches
> common/console.c, the macro WATCHDOG_RESET expands to {} if watchdog
> support isn't configured. Hence, there's no harm caused and no need to
> surround it by #ifdef in this case.
> 
>  * Symptom:
>    U-Boot resets after watchdog times out when in commandline prompt
>    and watchdog is enabled.
> 
>  * Reasoning:
>    When U-Boot shows the commandline prompt, the following function
>    call stack is executed while waiting for a keypress:
> 
>    common/main.c:
>                     main_loop          => common/cli.c: cli_loop() =>
>    common/cli_hush.c:
>                     parse_file_outer   => parse_stream_outer       =>
>                     parse_stream       => b_getch(i)               =>
>                     i->get(i)          => file_get                 =>
>                     get_user_input     => cmdedit_read_input       =>
>                     uboot_cli_readline =>
>    common/cli_readline.c:
>                     cli_readline       => cli_readline_into_buffer =>
>                     cread_line         => getcmd_getch (== getc)   =>
>    common/console.c:
>                     fgetc              => console_tstc
> 
>    common/console.c:
>    (with CONFIG_CONSOLE_MUX is set)
> 
>    - in console_tstc line 181:
>    If dev->tstc(dev) returns 0, the global tstcdev variable doesn't get
>    set. This is the case if no character is in the serial buffer.
> 
>    - in fgetc(int file), line 297:
>    Program flow keeps looping because tstcdev does not get set.
>    Therefore WATCHDOG_RESET is not called, as mx_serial_tstc from
>    drivers/serial/serial_mxc.c does not call it.
> 
>  * Solution:
>    Add WATCHDOG_RESET into the loop of console_tstc.
> 
>    Note: Macro expands to {} if not configured, so no #ifdef is needed.
> 
>  * Comment:
> 
> Signed-off-by: Christian Storm <christian.storm@tngtech.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> Signed-off-by: Andreas J. Reichel <Andreas.Reichel@tngtech.com>
> Acked-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161018/abca43d0/attachment.sig>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-10-18 22:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-13 10:56 [U-Boot] [PATCH PATCH v1] watchdog: Fix Watchdog Reset while in U-Boot Prompt Andreas J. Reichel
2016-08-01  1:01 ` Simon Glass
2016-08-01 11:49   ` [U-Boot] [PATCH v2] " Andreas J. Reichel
2016-09-06  1:04     ` Simon Glass
2016-09-19 11:59       ` Andreas Reichel
2016-10-15  1:45         ` Tom Rini
2016-08-01 13:32   ` [U-Boot] [PATCH v2 0/1] Fix U-Boot Prompt on CM-FX6 with enabled watchdog Andreas J. Reichel
2016-10-18 22:47 ` [U-Boot] [U-Boot, v1] watchdog: Fix Watchdog Reset while in U-Boot Prompt Tom Rini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox