[U-Boot] [PATCH v2] menu: fix timeout duration

[U-Boot] [PATCH v2] menu: fix timeout duration

[Masahiro Yamada]

For distro-boot, the TIMEOUT directive in the boot script specifies
how long to pause in units of 1/10 sec. [1]

Commit 8594753ba0a7 ("menu: only timeout when menu is displayed")
corrected this by simply dividing the timeout value by 10 in
menu_interactive_choice().

I see two problems:

 - For example, "TIMEOUT 5" should wait for 0.5 sec, but the current
   implementation cannot handle the granularity of 1/10 sec.
   In fact, it never breaks because "m->timeout / 10" is zero,
   which means no timeout.

 - The menu API is used not only by cmd/pxe.c but also by
   common/autoboot.c .  For the latter case, the unit of the
   timeout value is _second_ because its default is associated
   with CONFIG_BOOTDELAY.

To fix the first issue, use DIV_ROUND_UP() so that the timeout value
is rounded up to the closest integer.

For the second issue, move the division to the boundary between
cmd/pxe.c and common/menu.c .  This is a more desirable place because
the comment of struct pxe_menu says:

 * timeout - time in tenths of a second to wait for a user key-press before
 *           booting the default label.

Then, the comment of menu_create() says:

 * timeout - A delay in seconds to wait for user input. If 0, timeout is
 * disabled, and the default choice will be returned unless prompt is 1.

[1] https://www.syslinux.org/wiki/index.php?title=SYSLINUX#TIMEOUT_timeout

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - In v1, I was misunderstanding.  I thought the unit of TIMEOUT was
    second, but it is actually 1/10 second.  Use round up to fix the
    "never timeout" problem.
    Also, move the division to the correct place to fix CONFIG_MENU_SHOW.

 cmd/pxe.c     | 4 ++--
 common/menu.c | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/cmd/pxe.c b/cmd/pxe.c
index 7649d92..5609545 100644
--- a/cmd/pxe.c
+++ b/cmd/pxe.c
@@ -1453,8 +1453,8 @@ static struct menu *pxe_menu_to_menu(struct pxe_menu *cfg)
 	/*
 	 * Create a menu and add items for all the labels.
 	 */
-	m = menu_create(cfg->title, cfg->timeout, cfg->prompt, label_print,
-			NULL, NULL);
+	m = menu_create(cfg->title, DIV_ROUND_UP(cfg->timeout, 10),
+			cfg->prompt, label_print, NULL, NULL);
 
 	if (!m)
 		return NULL;
diff --git a/common/menu.c b/common/menu.c
index bf2b471..0f0a29a 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -194,8 +194,7 @@ static inline int menu_interactive_choice(struct menu *m, void **choice)
 
 		if (!m->item_choice) {
 			readret = cli_readline_into_buffer("Enter choice: ",
-							   cbuf,
-							   m->timeout / 10);
+							   cbuf, m->timeout);
 
 			if (readret >= 0) {
 				choice_item = menu_item_by_key(m, cbuf);
-- 
2.7.4

[U-Boot] [U-Boot,v2] menu: fix timeout duration

[Tom Rini]

On Thu, May 24, 2018 at 05:04:57PM +0900, Masahiro Yamada wrote:

> For distro-boot, the TIMEOUT directive in the boot script specifies
> how long to pause in units of 1/10 sec. [1]
> 
> Commit 8594753ba0a7 ("menu: only timeout when menu is displayed")
> corrected this by simply dividing the timeout value by 10 in
> menu_interactive_choice().
> 
> I see two problems:
> 
>  - For example, "TIMEOUT 5" should wait for 0.5 sec, but the current
>    implementation cannot handle the granularity of 1/10 sec.
>    In fact, it never breaks because "m->timeout / 10" is zero,
>    which means no timeout.
> 
>  - The menu API is used not only by cmd/pxe.c but also by
>    common/autoboot.c .  For the latter case, the unit of the
>    timeout value is _second_ because its default is associated
>    with CONFIG_BOOTDELAY.
> 
> To fix the first issue, use DIV_ROUND_UP() so that the timeout value
> is rounded up to the closest integer.
> 
> For the second issue, move the division to the boundary between
> cmd/pxe.c and common/menu.c .  This is a more desirable place because
> the comment of struct pxe_menu says:
> 
>  * timeout - time in tenths of a second to wait for a user key-press before
>  *           booting the default label.
> 
> Then, the comment of menu_create() says:
> 
>  * timeout - A delay in seconds to wait for user input. If 0, timeout is
>  * disabled, and the default choice will be returned unless prompt is 1.
> 
> [1] https://www.syslinux.org/wiki/index.php?title=SYSLINUX#TIMEOUT_timeout
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180606/1f49291c/attachment.sig>