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

[Pali Rohár]

On Tuesday 20 June 2023 11:20:35 Simon Glass wrote:
> 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.

Unfortunately it does not fix this issue :-( New patch series from [1]
applied on top of the master branch has still issue with DOWN key on
emulated UART terminal.

> 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

I can look at it later, but I'm loosing motivation to do whole debugging
for another issue again, because for the last year my fixes and other
patches were stalled and u-boot devs show me that they are not
interested even in commenting them. I do not want to work again on
something which would be ignored.

Hence, now I did only smaller - but still important - task: Locate exact
commits which broke particular feature and prepare reverts on top of the
master branch which _really_ fix the broken functionality - and make
code working again.

> 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