On Tuesday 20 June 2023 11:20:35 Simon Glass wrote: > Hi Tom, Pali, > > On Wed, 14 Jun 2023 at 20:51, Tom Rini <tr...@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-...@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
