[PATCH u-boot 0/3] Revert broken Bootmenu commits

[Simon Glass]

Hi Tom, Pali,

On Wed, 14 Jun 2023 at 20:51, Tom Rini <trini at konsulko.com> wrote:
>
> On Sun, Jun 11, 2023 at 02:53:21PM +0200, Pali Rohár wrote:
>
> > These 3 commits broke support for U-Boot Bootmenu on Nokia N900.
> > Issues were reported more than month ago, but nobody reacted to them.
> > So revert these broken commits for now, to fix U-Boot Bootmenu support.
> >
> > With these revered commits, U-Boot Bootmenu from master branch is
> > working fine again on Nokia N900.
> >
> > Pali Rohár (3):
> >   Revert "video: Enable VIDEO_ANSI by default only with EFI"
> >   Revert "menu: Factor out menu-keypress decoding"
> >   Revert "menu: Make use of CLI character processing"
> >
> >  cmd/bootmenu.c        |   9 ++--
> >  cmd/eficonfig.c       |  12 ++---
> >  common/cli_getch.c    |  12 ++---
> >  common/menu.c         | 122 ++++++++++++++++++++++++++----------------
> >  drivers/video/Kconfig |   7 +--
> >  include/cli.h         |   4 +-
> >  include/menu.h        |  17 +-----
> >  7 files changed, 91 insertions(+), 92 deletions(-)
>
> Following up over here, while
> https://patchwork.ozlabs.org/project/uboot/patch/20230612201434.861700-1-sjg@chromium.org/
> addresses some of the issues, but not all (as it clearly isn't working
> in all of the cases Pali has explained), looking in to the VIDEO_ANSI
> part of this too, all three of these reverts are related seemingly to
> something escape-character related.  I'm not taking any of the revert
> commits in just yet, but will by the next -rc if we don't have something
> that fixes all of the issues.

I did send a series [1] with a fix for the nokia_rx51 keyboard driver,
including the previous ansi-code patch. Given that:

- this problem doesn't seem to manifest on other boards
- it does not cause any CI test to fail
- there seem to be bugs in the nokia_rx51 implementation which are a
possible/likely cause
- the nokia_rx51 CI uses a 10-year-old out-of-tree QEMU
- the problem goes away if debugging is added, suggesting it is
related to timing

...I don't think a revert is appropriate.

Pali, can you please take a look and see if you can debug what is
actually going on? Here is my guess:

1. When an arrow key is pressed, cli_ch_process() handles it by being
passed the codes in sequence: \e [ B
2. Calling cli_ch_process() with ichar = 0 causes it to think the
sequence is finished: \e [ \0 B   (this doesn't work since the \0 ends
the sequence)
3. nokia_rx51 keyboard driver is returning '\0' even when key codes
are pending, causing cli_ch_process() to be told that the sequence is
done

It would help to move the keyboard driver into drivers/input/ so it is
easier to find.

Regards,
Simon

[1] https://patchwork.ozlabs.org/project/uboot/list/?series=360134