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

[Simon Glass]

Hi Weijie,

On Mon, 28 Oct 2024 at 03:58, Weijie Gao <weijie.gao at mediatek.com> wrote:
>
> On Mon, 2024-10-28 at 03:33 +0100, Marek Vasut wrote:
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> >
> >
> > On 10/28/24 1:25 AM, Weijie Gao wrote:
> > > Hi Daniel,
> > >
> > > On Sat, 2024-10-26 at 20:23 +0100, Daniel Golle wrote:
> > > > External email : Please do not click links or open attachments
> > > > until
> > > > you have verified the sender or the content.
> > > >
> > > >
> > > > Hi!
> > > >
> > > > On Mon, Oct 21, 2024 at 08:56:42AM +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.
> > > >
> > > > The patch helps to fix the problem when dealing with single key
> > > > presses. However, apparently this is not enough to fix the issue
> > > > in
> > > > all situations. I still manage to drop into U-Boot console with
> > > > an
> > > > extra '[A' when simply holding down the arrow-up key for a while.
> > > >
> > >
> > > Holding down the arrow-up key for a while can cause the PC to send
> > > large amounts of data to the UART Rx FIFO. Since filogic UARTs
> > > usually
> > > have only 32 bytes FIFO, it's very easy to cause Rx FIFO overflow,
> > > and then you'll still be dropped into U-Boot console due to data
> > > loss.
> > > I've already confirmed this during solving this issue and this can
> > > only
> > > be solved by increasing the baudrate.
> >
> > Does CONFIG_SERIAL_RX_BUFFER help with those overflows ?
>
> CONFIG_SERIAL_RX_BUFFER can't fully elimate the overflow. I can still
> reproduce this issue by holding down the arrow-up key for more than
> 15 seconds.

Is this due to serial, or the delays caused by drawing to the display?

If the serial buffer is always full, one option might be to detect this
(tstc() returning non-zero 10 times in a row) and discard all output until
a key is not pressed for 200ms or so.

Regards,
Simon