console: USB: KBD: Fix incorrect autoboot timeout

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 <[email protected]>

Quoted 7 lines

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 <[email protected]>

Applied to u-boot-usb

Dear Marek Vasut,

Quoted 5 lines · In message <[email protected]> you wrote:

Quoted 5 lines

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.

Quoted 3 lines

Quoted 1 line

Signed-off-by Jim Lin <[email protected]>

Applied to u-boot-usb

This is common code, and essentially unrelated to USB. It should go through Tom, not through the USB repo.

Wolfgang,

Quoted 20 lines

-----Original Message-----
From: Wolfgang Denk [mailto:wd at denx.de]
Sent: Saturday, January 26, 2013 6:14 AM
To: Marek Vasut
Cc: Jim Lin; u-boot at lists.denx.de; Tom Warren; trini at ti.com; albert.u.boot at aribaud.net
Subject: Re: [PATCH 1/1 v3] console: USB: KBD: Fix incorrect autoboot timeout

Dear Marek Vasut,

Quoted 2 lines · In message <[email protected]> you wrote:

Quoted 2 lines

Autoboot timeout defined by CONFIG_BOOTDELAY will not be accurate if CONFIG_USB_KEYBOARD and CONFIG_SYS_USB_EVENT_POLL are

defined in

Quoted 3 lines

Quoted 3 lines

configuration file and when tstc() function for checking key pressed takes longer time than 10 ms (e.g., 50 ms) to finish.

Quoted 6 lines

Quoted 3 lines

Quoted 1 line

Signed-off-by Jim Lin <[email protected]>

Applied to u-boot-usb

This is common code, and essentially unrelated to USB. It should go through Tom, not through the USB repo.

Why do you expect this to go thru the Tegra tree?

Wolfgang,

Quoted 20 lines

-----Original Message----- From: Wolfgang Denk
[mailto:wd at denx.de] Sent: Saturday, January 26, 2013 6:14 AM To:
Marek Vasut Cc: Jim Lin; u-boot at lists.denx.de; Tom Warren; trini at ti.com; albert.u.boot at aribaud.net Subject: Re: [PATCH 1/1 v3] console: USB: KBD: Fix incorrect autoboot timeout

Dear Marek Vasut,

Quoted 4 lines · In message <[email protected]> you wrote:

Quoted 4 lines

Autoboot timeout defined by CONFIG_BOOTDELAY will not be accurate if CONFIG_USB_KEYBOARD and CONFIG_SYS_USB_EVENT_POLL are

defined in

Quoted 6 lines

Quoted 5 lines

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 <[email protected]>

Quoted 4 lines

Quoted 1 line

Applied to u-boot-usb

This is common code, and essentially unrelated to USB. It should go through Tom, not through the USB repo.

Why do you expect this to go thru the Tegra tree? It's common code
- shouldn't it at least go thru my parent repo (ARM), if no directly into u-boot.git/master?

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 <[email protected]>

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 <[email protected]>

---
Changes in v2:
- use do-while and get_timer to count timeout.
Changes in v3:
- revert original udelay(10000); for safety.

 common/main.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

common/main.c +4 -4

index b145f85..dcd2a42 100644
@@ -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	*/

Quoted 66 lines

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 <[email protected]>

---

Changes in v2:
- use do-while and get_timer to count timeout.

Changes in v3:
- revert original udelay(10000); for safety.

 common/main.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

common/main.c +1 -0

index b145f85..dcd2a42 100644
@@ -225,6 +225,7 @@ static inline
 
 int abortboot(int bootdelay)
 {
 
 	int abort = 0;
 
+	unsigned long ts;

#ifdef CONFIG_MENUPROMPT

common/main.c +4 -5

 	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	*/

@@ -264,7 +264,7 @@ int abortboot(int bootdelay)
 
 				break;
 
 			}
 			udelay(10000);
 
-		}
+		} while (!abort && get_timer(ts) < 1000);

common/main.c int abortboot(int bootdelay)

Quoted 3 lines

common/main.c +0 -0

 		printf("\b\b\b%2d ", bootdelay);
 
 	}

This change is causing problems with auto-delay on one of my boards by making it inaccurate :-(

The question is what should get_timer() be returning? If it is meant to be milliseconds then I guess I need to fix get_timer() for my board. However, if it is just meant to be timer ticks at the SYS_HZ rate then I don't see how the above change guarantees the do-while loop waits 1000 ms per iteration without normalising to SYS_HZ.

Dear Jon Hunter,

Quoted 74 lines · Jim Lin

Quoted 66 lines

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 <[email protected]>

---

Changes in v2:
- use do-while and get_timer to count timeout.

Changes in v3:
- revert original udelay(10000); for safety.

 common/main.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

common/main.c +1 -0

index b145f85..dcd2a42 100644
@@ -225,6 +225,7 @@ static inline
 
 int abortboot(int bootdelay)
 {
 
 	int abort = 0;
 
+	unsigned long ts;

#ifdef CONFIG_MENUPROMPT

common/main.c +4 -5

 	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	*/

@@ -264,7 +264,7 @@ int abortboot(int bootdelay)
 
 				break;
 
 			}
 			udelay(10000);
 
-		}
+		} while (!abort && get_timer(ts) < 1000);

common/main.c int abortboot(int bootdelay)

Quoted 3 lines

common/main.c +0 -0

 		printf("\b\b\b%2d ", bootdelay);
 
 	}

This change is causing problems with auto-delay on one of my boards by making it inaccurate :-(

Quoted 5 lines

The question is what should get_timer() be returning? If it is meant to be milliseconds then I guess I need to fix get_timer() for my board. However, if it is just meant to be timer ticks at the SYS_HZ rate then I don't see how the above change guarantees the do-while loop waits 1000 ms per iteration without normalising to SYS_HZ.

What board is it ?

Quoted 4 lines

Quoted 4 lines

The question is what should get_timer() be returning? If it is meant to be milliseconds then I guess I need to fix get_timer() for my board. However, if it is just meant to be timer ticks at the SYS_HZ rate then I

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 <[email protected]>

---
Changes in v2:
- use do-while and get_timer to count timeout.
Changes in v3:
- revert original udelay(10000); for safety.

 common/main.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

common/main.c index b145f85..dcd2a42 renamed from common/main.c +0 -0

100644

common/main.c +4 -4

@@ -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	*/

Quoted 4 lines · Jim Lin

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.

This change is causing problems with auto-delay on one of my boards by making it inaccurate :-(

Interesting. I just noticed the same problem, and posted the following patch to fix it:

http://lists.denx.de/pipermail/u-boot/2013-March/149625.html

This was on the Raspberry Pi.

Dear Stephen Warren,

Quoted 4 lines · In message <[email protected]> you wrote:

Interesting. I just noticed the same problem, and posted the following patch to fix it:

Quoted 3 lines

http://lists.denx.de/pipermail/u-boot/2013-March/149625.html

This was on the Raspberry Pi.

You mean the Raspberry Pi uses SYS_HZ != 1000 ???

This needs fixing!!

I understand that you may wish to have SYS_HZ == 1000, but shouldn't the code still normalise the get_timer() value to SYS_HZ?

If I grep through the u-boot source I see a lot of instances where get_timer() is normalised to SYS_HZ.

grep -r "get_timer" * | grep "SYS_HZ"