[PATCH] menu: fix the logic checking whether ESC key is pressed

[Simon Glass]

Hi Weijie,

On Fri, 18 Oct 2024 at 02:38, Weijie Gao <weijie.gao at mediatek.com> wrote:
>
> 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.
>
> Signed-off-by: Weijie Gao <weijie.gao at mediatek.com>
> ---
>  common/menu.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/common/menu.c b/common/menu.c
> index 5fd705039fc..bd2ac3e0ed9 100644
> --- a/common/menu.c
> +++ b/common/menu.c
> @@ -566,7 +566,7 @@ enum bootmenu_key bootmenu_loop(struct bootmenu_data *menu,
>                                 struct cli_ch_state *cch)
>  {
>         enum bootmenu_key key;
> -       int c;
> +       int c, errchar = 0;
>         int choice;
>
>         c = cli_ch_process(cch, 0);
> @@ -574,7 +574,8 @@ enum bootmenu_key bootmenu_loop(struct bootmenu_data *menu,
>                 while (!c && !tstc()) {
>                         schedule();
>                         mdelay(10);
> -                       c = cli_ch_process(cch, -ETIMEDOUT);
> +                       c = cli_ch_process(cch, errchar);
> +                       errchar = -ETIMEDOUT;
>                 }
>                 if (!c) {
>                         c = getchar();
> --
> 2.34.1
>

This looks reasonable from a first glance, but please can you rebase
on -master ?

Regards,
Simon