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

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

Comments

Jim Lin - Jan. 23, 2013, 6:37 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>
---
 common/main.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)
Wolfgang Denk - Jan. 23, 2013, 8:38 a.m.
Dear Jim Lin,

In message <1358923034-2727-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.
...

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

This is a pretty awkward implementation.  If you feel the current
plain delay loop based approach is not exact enough, then please
implement a real timeout - you are calling the timer anyway, so just
use the timer values instead of a loop count.

Best regards,

Wolfgang Denk
Jim Lin - Jan. 23, 2013, 9:43 a.m.
On Wed, 2013-01-23 at 16:38 +0800, Wolfgang Denk wrote:
> Dear Jim Lin,
> 
> In message <1358923034-2727-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.
> ...
> 
> >  		/* delay 100 * 10ms */
> > -		for (i=0; !abort && i<100; ++i) {
> > +		j = 100;
> > +		for (i = 0; !abort && i < j; ++i) {
> > +			ts = get_timer(0);
> >  			if (tstc()) {	/* we got a key press	*/
> >  				abort  = 1;	/* don't auto boot	*/
> >  				bootdelay = 0;	/* no more delay	*/
> > @@ -263,7 +266,11 @@ int abortboot(int bootdelay)
> >  # endif
> >  				break;
> >  			}
> > -			udelay(10000);
> > +			ts = get_timer(ts);
> > +			if (ts < 10)
> > +				mdelay(10 - ts);
> > +			else
> > +				j = 1000 / (int) ts;
> >  		}
> 
> This is a pretty awkward implementation.  If you feel the current
> plain delay loop based approach is not exact enough, then please
> implement a real timeout - you are calling the timer anyway, so just
> use the timer values instead of a loop count.
> 
Thanks for pointing this out. I will improve it and resend.



-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Patch

diff --git a/common/main.c b/common/main.c
index b145f85..8cdfb8b 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,13 @@  int abortboot(int bootdelay)
 #endif
 
 	while ((bootdelay > 0) && (!abort)) {
-		int i;
+		int i, j;
 
 		--bootdelay;
 		/* delay 100 * 10ms */
-		for (i=0; !abort && i<100; ++i) {
+		j = 100;
+		for (i = 0; !abort && i < j; ++i) {
+			ts = get_timer(0);
 			if (tstc()) {	/* we got a key press	*/
 				abort  = 1;	/* don't auto boot	*/
 				bootdelay = 0;	/* no more delay	*/
@@ -263,7 +266,11 @@  int abortboot(int bootdelay)
 # endif
 				break;
 			}
-			udelay(10000);
+			ts = get_timer(ts);
+			if (ts < 10)
+				mdelay(10 - ts);
+			else
+				j = 1000 / (int) ts;
 		}
 
 		printf("\b\b\b%2d ", bootdelay);