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

[E Shattow]

Hi,

On Fri, Oct 18, 2024 at 8:03 AM Simon Glass <sjg at chromium.org> wrote:
>
> 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

Does this affect eficonfig menu ? I was having some intermittent
trouble about the eficonfig menu quitting suddenly before this patch.
With this fix applied it's no worse but maybe better, I tried for some
minutes to reproduce the previous behaviour and was not able to make
the glitch happen.

Tested-By: E Shattow <lucent at gmail.com>