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

[Weijie Gao (高惟杰)]

On Mon, 2024-10-28 at 18:01 +0100, Simon Glass wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 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.
> 

I can confirm that the input data grows faster that the consumption
from bootmenu (mainly due to the menu being repainted too frequently).
So no matter how large the buffer is, FIFO overrun will eventually
happen.

A long time key (up/down) press will change the active menu item to
either the first or the last one, and then the menu does not need a
reprint.

So I decide to add an extra function for the common menu part to checkif the menu needs reprint.
> Regards,
> Simon