On Mon, 2024-10-21 at 02:07 -0700, E Shattow wrote:
>
> Hi,
>
> On Fri, Oct 18, 2024 at 8:03 AM Simon Glass <s...@chromium.org> wrote:
> >
> > Hi Weijie,
> >
> > On Fri, 18 Oct 2024 at 02:38, Weijie Gao <weijie....@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....@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.
I don't have efi environment to test. But I do see this function be
called in eficonfig.c (eficonfig_choice_change_boot_order). So I
believe this will affect eficonfig menu.
>
> Tested-By: E Shattow <luc...@gmail.com>
