Hi Marek,

On 03/20/2013 07:19 PM, Marek Vasut wrote:

Dear Jon Hunter,

...

On 01/24/2013 05:05 AM, Jim Lin 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.

Signed-off-by: Jim Lin jilin@nvidia.com

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(-)

diff --git a/common/main.c b/common/main.c index b145f85..dcd2a42 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 */

@@ -264,7 +264,7 @@ int abortboot(int bootdelay)

		break;
	
	}
	udelay(10000);

• }
  

• } while (!abort && get_timer(ts) < 1000);
  
  

  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.

What board is it ?

It is an OMAP2420-H4 board (ARM11). The timer is using a very odd SYS_HZ value of ~46875 (I believe this is because this is the most they can divide down the 12MHz clock source by). The timer could be switched to use a 32kHz clock source instead of the 12MHz clock source and get something closer to 1ms. However, I would need to test to see if this causes any other problems.

Cheers Jon

Thanks for pointing out the potential issue.

-----Original Message----- From: Jon Hunter [mailto:jon-hunter@ti.com] Sent: Thursday, March 21, 2013 7:57 AM To: Jim Lin Cc: u-boot@lists.denx.de; marex@denx.de; wd@denx.de; trini@ti.com; Tom Warren Subject: Re: [U-Boot] [PATCH 1/1 v3] console: USB: KBD: Fix incorrect autoboot timeout

On 01/24/2013 05:05 AM, Jim Lin 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.

Signed-off-by: Jim Lin jilin@nvidia.com

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(-)

diff --git a/common/main.c b/common/main.c index b145f85..dcd2a42 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	*/
  

@@ -264,7 +264,7 @@ int abortboot(int bootdelay) break; } udelay(10000);

• }
  

• } while (!abort && get_timer(ts) < 1000);
  
  

  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.

For my board I made the following change to make it work ...

diff --git a/common/main.c b/common/main.c index a15f020..32c4f8a 100644 --- a/common/main.c +++ b/common/main.c @@ -264,7 +264,7 @@ int abortboot(int bootdelay) break; } udelay(10000); - } while (!abort && get_timer(ts) < 1000); + } while (!abort && !(get_timer(ts) / CONFIG_SYS_HZ));

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

-- nvpublic