All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.