[PATCH v2 1/3] menu: fix the logic checking whether ESC key is pressed

[Weijie Gao]

Hi Daniel,

On Thu, 2024-10-31 at 19:16 +0000, Daniel Golle wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On Tue, Oct 29, 2024 at 05:47:10PM +0800, Weijie Gao 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.
> > 
> > Tested-By: E Shattow <lucent at gmail.com>
> > Signed-off-by: Weijie Gao <weijie.gao at mediatek.com>
> 
> Reviewed-by: Daniel Golle <daniel at makrotopia.org>
> Tested-by: Daniel Golle <daniel at makrotopia.org>
> 
> Together with 2/3 and 3/3 this now avoids dropping into the U-Boot
> shell
> as long as the menu is short enough. In case of menus with 8 entries
> of
> more I still manage to drop into shell when holding the arrow down
> key
> before the last item has been reached.
> That being said, it's already a very big improvement that this now
> only
> happens when holding down a button traversing over a menu with a high
> number of items.

I didn't encounter with this.
Did you try enabling CONFIG_SERIAL_RX_BUFFER?

> 
> 
> > ---
> >  common/menu.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/common/menu.c b/common/menu.c
> > index 8cc9bf06d9c..48ab7f0f398 100644
> > --- a/common/menu.c
> > +++ b/common/menu.c
> > @@ -525,14 +525,15 @@ enum bootmenu_key bootmenu_loop(struct
> > bootmenu_data *menu,
> >                               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();
> > --
> > 2.45.2
> >