Skip to content

Commit

Permalink
uboot-mediatek: import patches improving menu navigation
Browse files Browse the repository at this point in the history
Using the arrow keys to navigate the U-Boot menu often leads to being
dropped into the U-Boot shell unexpectedly.
This can be prevented in most cases by improving the logic to detect
the arrow key ESC sequence and only reprinting the menu if actually
needed.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
  • Loading branch information
dangowrt committed Nov 1, 2024
1 parent 2b173ab commit 985b10f
Show file tree
Hide file tree
Showing 5 changed files with 277 additions and 26 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
From 72b4ba8417d33516b8489bac3c90dbbbf781a3d2 Mon Sep 17 00:00:00 2001
From: Weijie Gao <weijie.gao@mediatek.com>
Date: Tue, 29 Oct 2024 17:47:10 +0800
Subject: [PATCH 1/3] menu: fix the logic checking whether ESC key is pressed

It's observed that the bootmenu on a serial console sometimes
incorrectly quitted with superfluous characters filled to command
line input:

> *** U-Boot Boot Menu ***
>
> 1. Startup system (Default)
> 2. Upgrade firmware
> 3. Upgrade ATF BL2
> 4. Upgrade ATF FIP
> 5. Load image
> 0. U-Boot console
>
>
> Press UP/DOWN to move, ENTER to select, ESC to quit
>MT7988> [B

Analysis shows it was caused by the wrong logic of bootmenu_loop:

At first the bootmenu_loop received the first ESC char correctly.

However, during the second call to bootmenu_loop, there's no data
in the UART Rx FIFO. Due to the low baudrate, the second char of
the down array key sequence hasn't be fully received.

But bootmenu_loop just did a mdelay(10), and then treated it as a
single ESC key press event. It didn't even try tstc() again after
the 10ms timeout.

This patch fixes this issue by letting bootmenu_loop check tstc()
twice.

Tested-By: E Shattow <lucent@gmail.com>
Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
---
common/menu.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

--- a/common/menu.c
+++ b/common/menu.c
@@ -525,14 +525,15 @@ enum bootmenu_key bootmenu_loop(struct b
struct cli_ch_state *cch)
{
enum bootmenu_key key;
- int c;
+ int c, errchar = 0;

c = cli_ch_process(cch, 0);
if (!c) {
while (!c && !tstc()) {
schedule();
mdelay(10);
- c = cli_ch_process(cch, -ETIMEDOUT);
+ c = cli_ch_process(cch, errchar);
+ errchar = -ETIMEDOUT;
}
if (!c) {
c = getchar();
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
From f1cbdd3330f0055dfbff0ef7d86276c4cc3cff2a Mon Sep 17 00:00:00 2001
From: Weijie Gao <weijie.gao@mediatek.com>
Date: Tue, 29 Oct 2024 17:47:16 +0800
Subject: [PATCH 2/3] menu: add support to check if menu needs to be reprinted

This patch adds a new callback named need_reprint for menu.
The need_reprint will be called before printing the menu. If the
callback exists and returns FALSE, menu printing will be canceled.

This is very useful if the menu was not changed. It can save time
for serial-based menu to handle more input data.

Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
---
boot/pxe_utils.c | 2 +-
cmd/bootmenu.c | 2 +-
cmd/eficonfig.c | 2 +-
common/menu.c | 11 +++++++++++
include/menu.h | 1 +
5 files changed, 15 insertions(+), 3 deletions(-)

--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -1449,7 +1449,7 @@ static struct menu *pxe_menu_to_menu(str
* Create a menu and add items for all the labels.
*/
m = menu_create(cfg->title, DIV_ROUND_UP(cfg->timeout, 10),
- cfg->prompt, NULL, label_print, NULL, NULL);
+ cfg->prompt, NULL, label_print, NULL, NULL, NULL);
if (!m)
return NULL;

--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -506,7 +506,7 @@ static enum bootmenu_ret bootmenu_show(i

menu = menu_create(NULL, bootmenu->delay, 1, menu_display_statusline,
bootmenu_print_entry, bootmenu_choice_entry,
- bootmenu);
+ NULL, bootmenu);
if (!menu) {
bootmenu_destroy(bootmenu);
return BOOTMENU_RET_FAIL;
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -443,7 +443,7 @@ efi_status_t eficonfig_process_common(st
efi_menu->menu_desc = menu_desc;

menu = menu_create(NULL, 0, 1, display_statusline, item_data_print,
- item_choice, efi_menu);
+ item_choice, NULL, efi_menu);
if (!menu)
return EFI_INVALID_PARAMETER;

--- a/common/menu.c
+++ b/common/menu.c
@@ -43,6 +43,7 @@ struct menu {
void (*display_statusline)(struct menu *);
void (*item_data_print)(void *);
char *(*item_choice)(void *);
+ bool (*need_reprint)(void *);
void *item_choice_data;
struct list_head items;
int item_cnt;
@@ -117,6 +118,11 @@ static inline void *menu_item_destroy(st
*/
static inline void menu_display(struct menu *m)
{
+ if (m->need_reprint) {
+ if (!m->need_reprint(m->item_choice_data))
+ return;
+ }
+
if (m->title) {
puts(m->title);
putc('\n');
@@ -362,6 +368,9 @@ int menu_item_add(struct menu *m, char *
* item. Returns a key string corresponding to the chosen item or NULL if
* no item has been selected.
*
+ * need_reprint - If not NULL, will be called before printing the menu.
+ * Returning FALSE means the menu does not need reprint.
+ *
* item_choice_data - Will be passed as the argument to the item_choice function
*
* Returns a pointer to the menu if successful, or NULL if there is
@@ -371,6 +380,7 @@ struct menu *menu_create(char *title, in
void (*display_statusline)(struct menu *),
void (*item_data_print)(void *),
char *(*item_choice)(void *),
+ bool (*need_reprint)(void *),
void *item_choice_data)
{
struct menu *m;
@@ -386,6 +396,7 @@ struct menu *menu_create(char *title, in
m->display_statusline = display_statusline;
m->item_data_print = item_data_print;
m->item_choice = item_choice;
+ m->need_reprint = need_reprint;
m->item_choice_data = item_choice_data;
m->item_cnt = 0;

--- a/include/menu.h
+++ b/include/menu.h
@@ -13,6 +13,7 @@ struct menu *menu_create(char *title, in
void (*display_statusline)(struct menu *),
void (*item_data_print)(void *),
char *(*item_choice)(void *),
+ bool (*need_reprint)(void *),
void *item_choice_data);
int menu_default_set(struct menu *m, char *item_key);
int menu_get_choice(struct menu *m, void **choice);
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
From 702752cfae954648d6133bdff19283343b3339ef Mon Sep 17 00:00:00 2001
From: Weijie Gao <weijie.gao@mediatek.com>
Date: Tue, 29 Oct 2024 17:47:22 +0800
Subject: [PATCH 3/3] bootmenu: add reprint check

Record the last active menu item and check if it equals to the
current selected item before reprint.

Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
---
cmd/bootmenu.c | 16 +++++++++++++++-
include/menu.h | 1 +
2 files changed, 16 insertions(+), 1 deletion(-)

--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -103,11 +103,13 @@ static char *bootmenu_choice_entry(void

switch (key) {
case BKEY_UP:
+ menu->last_active = menu->active;
if (menu->active > 0)
--menu->active;
/* no menu key selected, regenerate menu */
return NULL;
case BKEY_DOWN:
+ menu->last_active = menu->active;
if (menu->active < menu->count - 1)
++menu->active;
/* no menu key selected, regenerate menu */
@@ -133,6 +135,17 @@ static char *bootmenu_choice_entry(void
return NULL;
}

+static bool bootmenu_need_reprint(void *data)
+{
+ struct bootmenu_data *menu = data;
+ bool need_reprint;
+
+ need_reprint = menu->last_active != menu->active;
+ menu->last_active = menu->active;
+
+ return need_reprint;
+}
+
static void bootmenu_destroy(struct bootmenu_data *menu)
{
struct bootmenu_entry *iter = menu->first;
@@ -332,6 +345,7 @@ static struct bootmenu_data *bootmenu_cr

menu->delay = delay;
menu->active = 0;
+ menu->last_active = -1;
menu->first = NULL;

default_str = env_get("bootmenu_default");
@@ -506,7 +520,7 @@ static enum bootmenu_ret bootmenu_show(i

menu = menu_create(NULL, bootmenu->delay, 1, menu_display_statusline,
bootmenu_print_entry, bootmenu_choice_entry,
- NULL, bootmenu);
+ bootmenu_need_reprint, bootmenu);
if (!menu) {
bootmenu_destroy(bootmenu);
return BOOTMENU_RET_FAIL;
--- a/include/menu.h
+++ b/include/menu.h
@@ -40,6 +40,7 @@ int menu_show(int bootdelay);
struct bootmenu_data {
int delay; /* delay for autoboot */
int active; /* active menu entry */
+ int last_active; /* last active menu entry */
int count; /* total count of menu entries */
struct bootmenu_entry *first; /* first menu entry */
};
Loading

0 comments on commit 985b10f

Please sign in to comment.