Patchwork [U-Boot,1/1,v2] console: USB: KBD: Fix incorrect autoboot timeout

login
register
mail settings
Submitter Jim Lin
Date Jan. 23, 2013, 10:38 a.m.
Message ID <1358937511-32664-1-git-send-email-jilin@nvidia.com>
Download mbox | patch
Permalink /patch/214882/
State Superseded
Delegated to: Tom Rini
Headers show

Comments

Jim Lin - Jan. 23, 2013, 10:38 a.m.
Autoboot timeout defined by CONFIG_BOOTDELAY will not be accurate if
CONFIG_USB_KEYBOARD and CONFIG_SYS_USB_EVENT_POLL are defined in
configuration file and when tstc() function for checking key pressed
takes longer time than 10 ms (e.g., 50 ms) to finish.

Signed-off-by: Jim Lin <jilin@nvidia.com>
---
Changes in v2:
   - use do-while and get_timer to count timeout

 common/main.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)
Wolfgang Denk - Jan. 23, 2013, 7:33 p.m.
Dear Jim Lin,

In message <1358937511-32664-1-git-send-email-jilin@nvidia.com> you wrote:
> Autoboot timeout defined by CONFIG_BOOTDELAY will not be accurate if
> CONFIG_USB_KEYBOARD and CONFIG_SYS_USB_EVENT_POLL are defined in
> configuration file and when tstc() function for checking key pressed
> takes longer time than 10 ms (e.g., 50 ms) to finish.

Thanks.  One minor nitpick...

> +		/* delay 1000 ms */
> +		ts = get_timer(0);
> +		do {
>  			if (tstc()) {	/* we got a key press	*/
>  				abort  = 1;	/* don't auto boot	*/
>  				bootdelay = 0;	/* no more delay	*/
> @@ -263,8 +263,7 @@ int abortboot(int bootdelay)
>  # endif
>  				break;
>  			}
> -			udelay(10000);
> -		}
> +		} while (!abort && get_timer(ts) < 1000);

I recommend to keep a short udelay() [say, an udelay(1000)] in the
loop, as this will make sure that watchdog still gets triggered on
systems that need this.

Thaks.

Best regards,

Wolfgang Denk
Jim Lin - Jan. 24, 2013, 10:38 a.m.
On Thu, 2013-01-24 at 03:33 +0800, Wolfgang Denk wrote:
> Dear Jim Lin,
> 
> In message <1358937511-32664-1-git-send-email-jilin@nvidia.com> you wrote:
> > Autoboot timeout defined by CONFIG_BOOTDELAY will not be accurate if
> > CONFIG_USB_KEYBOARD and CONFIG_SYS_USB_EVENT_POLL are defined in
> > configuration file and when tstc() function for checking key pressed
> > takes longer time than 10 ms (e.g., 50 ms) to finish.
> 
> Thanks.  One minor nitpick...
> 
> > +		/* delay 1000 ms */
> > +		ts = get_timer(0);
> > +		do {
> >  			if (tstc()) {	/* we got a key press	*/
> >  				abort  = 1;	/* don't auto boot	*/
> >  				bootdelay = 0;	/* no more delay	*/
> > @@ -263,8 +263,7 @@ int abortboot(int bootdelay)
> >  # endif
> >  				break;
> >  			}
> > -			udelay(10000);
> > -		}
> > +		} while (!abort && get_timer(ts) < 1000);
> 
> I recommend to keep a short udelay() [say, an udelay(1000)] in the
> loop, as this will make sure that watchdog still gets triggered on
> systems that need this.
> 
I will keep
   udelay(10000);
for safety in next patch.

Thanks.

-- nvpublic

Patch

diff --git a/common/main.c b/common/main.c
index b145f85..4dac01d 100644
--- a/common/main.c
+++ b/common/main.c
@@ -225,6 +225,7 @@  static inline
 int abortboot(int bootdelay)
 {
 	int abort = 0;
+	unsigned long ts;
 
 #ifdef CONFIG_MENUPROMPT
 	printf(CONFIG_MENUPROMPT);
@@ -248,11 +249,10 @@  int abortboot(int bootdelay)
 #endif
 
 	while ((bootdelay > 0) && (!abort)) {
-		int i;
-
 		--bootdelay;
-		/* delay 100 * 10ms */
-		for (i=0; !abort && i<100; ++i) {
+		/* delay 1000 ms */
+		ts = get_timer(0);
+		do {
 			if (tstc()) {	/* we got a key press	*/
 				abort  = 1;	/* don't auto boot	*/
 				bootdelay = 0;	/* no more delay	*/
@@ -263,8 +263,7 @@  int abortboot(int bootdelay)
 # endif
 				break;
 			}
-			udelay(10000);
-		}
+		} while (!abort && get_timer(ts) < 1000);
 
 		printf("\b\b\b%2d ", bootdelay);
 	}