Re: [U-Boot] [PATCH v2] watchdog: Fix Watchdog Reset while in U-Boot Prompt

[Tom Rini]

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
> >  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 
> > > Signed-off-by: Jan Kiszka 
> > > Signed-off-by: Andreas J. Reichel 
> > > ---
> > >
> > > 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 
> > 
> > >
> > > --
> > > 2.8.2
> > >
> 
> -- 
> Andreas Reichel 
> Dipl.-Phys. (Univ.) 
> Software Consultant
> 
> andreas.reic...@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


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2] watchdog: Fix Watchdog Reset while in U-Boot Prompt

[Andreas Reichel]

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
>  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 
> > Signed-off-by: Jan Kiszka 
> > Signed-off-by: Andreas J. Reichel 
> > ---
> >
> > 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 
> 
> >
> > --
> > 2.8.2
> >

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

andreas.reic...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2] watchdog: Fix Watchdog Reset while in U-Boot Prompt

[Simon Glass]

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

On 1 August 2016 at 05:49, Andreas J. Reichel
 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 
> Signed-off-by: Jan Kiszka 
> Signed-off-by: Andreas J. Reichel 
> ---
>
> 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 

>
> --
> 2.8.2
>
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot