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

[Daniel Golle <daniel@makrotopia.org>]

On Fri, Nov 01, 2024 at 03:20:11PM +0800, Weijie Gao wrote:
> 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@gmail.com>
> > > Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
> > 
> > Reviewed-by: Daniel Golle <daniel@makrotopia.org>
> > Tested-by: Daniel Golle <daniel@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?

Combined with CONFIG_SERIAL_RX_BUFFER it does resolve the issue and I no
longer manage to get dropped into console.

Thank you a lot for taking care of this!