Re: [PATCH u-boot 0/3] Revert broken Bootmenu commits
Hi, (added Jaehoon Chung to Cc) On 7/10/23 18:03, Stefan Roese wrote: On 7/10/23 17:17, Tom Rini wrote: On Mon, Jul 10, 2023 at 04:10:39PM +0200, Pali Rohár wrote: On Sunday 25 June 2023 21:08:19 Tom Rini wrote: On Sun, Jun 25, 2023 at 05:15:34PM +0200, Pali Rohár wrote: On Sunday 25 June 2023 10:52:13 Tom Rini wrote: On Sun, Jun 25, 2023 at 09:50:39AM +0200, Pali Rohár wrote: On Saturday 24 June 2023 12:58:07 Tom Rini wrote: On Sat, Jun 24, 2023 at 10:50:41AM +0200, Pali Rohár wrote: On Tuesday 20 June 2023 11:20:35 Simon Glass wrote: Hi Tom, Pali, On Wed, 14 Jun 2023 at 20:51, Tom Rini 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. Well, unless your changes here break something else, I don't think a fix for your problem will be ignored. This is something which I read here more times in the past... and reality was different. Should I remind you that there are waiting eMMC fixes for mvebu and again discussion about them stalled? Or rather should I say that it is again ignored? No, you should just say they're still waiting for review. Since they're waiting for review. The MMC custodian has been asked to review them, and hasn't yet. And they don't appear to be super critical changes, and they conflict with other series, so yes, they'll get picked up when the custodian has time to review everything. And what is "critical change" if it is not fixing booting (from eMMC)? The when and where, and since when. Since re-reading everything in https://patchwork.ozlabs.org/project/uboot/list/?submitter=78810 that's not this revert series doesn't say "fix booting on $platform that was broken by ...". It says clean up. Clean up can wait. "Fix eMMC boot" - This is not clear that it is a "fix"??? Or are you again going to play a game "I do not see your patches"? Ok, then here is direct link: https://lore.kernel.org/u-boot/20230413205750.10641-1-p...@kernel.org/ Oh, oh, I think I get it now. You're wondering why I guess: https://patchwork.ozlabs.org/project/uboot/patch/20230505193710.n35h2ofq6fogk4bq@pali/
Re: [PATCH u-boot 0/3] Revert broken Bootmenu commits
On 7/10/23 17:17, Tom Rini wrote: On Mon, Jul 10, 2023 at 04:10:39PM +0200, Pali Rohár wrote: On Sunday 25 June 2023 21:08:19 Tom Rini wrote: On Sun, Jun 25, 2023 at 05:15:34PM +0200, Pali Rohár wrote: On Sunday 25 June 2023 10:52:13 Tom Rini wrote: On Sun, Jun 25, 2023 at 09:50:39AM +0200, Pali Rohár wrote: On Saturday 24 June 2023 12:58:07 Tom Rini wrote: On Sat, Jun 24, 2023 at 10:50:41AM +0200, Pali Rohár wrote: On Tuesday 20 June 2023 11:20:35 Simon Glass wrote: Hi Tom, Pali, On Wed, 14 Jun 2023 at 20:51, Tom Rini 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. Well, unless your changes here break something else, I don't think a fix for your problem will be ignored. This is something which I read here more times in the past... and reality was different. Should I remind you that there are waiting eMMC fixes for mvebu and again discussion about them stalled? Or rather should I say that it is again ignored? No, you should just say they're still waiting for review. Since they're waiting for review. The MMC custodian has been asked to review them, and hasn't yet. And they don't appear to be super critical changes, and they conflict with other series, so yes, they'll get picked up when the custodian has time to review everything. And what is "critical change" if it is not fixing booting (from eMMC)? The when and where, and since when. Since re-reading everything in https://patchwork.ozlabs.org/project/uboot/list/?submitter=78810 that's not this revert series doesn't say "fix booting on $platform that was broken by ...". It says clean up. Clean up can wait. "Fix eMMC boot" - This is not clear that it is a "fix"??? Or are you again going to play a game "I do not see your patches"? Ok, then here is direct link: https://lore.kernel.org/u-boot/20230413205750.10641-1-p...@kernel.org/ Oh, oh, I think I get it now. You're wondering why I guess: https://patchwork.ozlabs.org/project/uboot/patch/20230505193710.n35h2ofq6fogk4bq@pali/ https://patchwork.ozlabs.org/project/uboot/patch/20230516184943.7206-1-p...@kernel.org/ haven't been picked up? They're assigned to Stefan. Hmmm, I might have missed
Re: [PATCH u-boot 0/3] Revert broken Bootmenu commits
On Mon, Jul 10, 2023 at 04:10:39PM +0200, Pali Rohár wrote: > On Sunday 25 June 2023 21:08:19 Tom Rini wrote: > > On Sun, Jun 25, 2023 at 05:15:34PM +0200, Pali Rohár wrote: > > > On Sunday 25 June 2023 10:52:13 Tom Rini wrote: > > > > On Sun, Jun 25, 2023 at 09:50:39AM +0200, Pali Rohár wrote: > > > > > On Saturday 24 June 2023 12:58:07 Tom Rini wrote: > > > > > > On Sat, Jun 24, 2023 at 10:50:41AM +0200, Pali Rohár wrote: > > > > > > > On Tuesday 20 June 2023 11:20:35 Simon Glass wrote: > > > > > > > > Hi Tom, Pali, > > > > > > > > > > > > > > > > On Wed, 14 Jun 2023 at 20:51, Tom Rini > > > > > > > > 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 > > > > > > >
Re: [PATCH u-boot 0/3] Revert broken Bootmenu commits
Hi Pali, On Mon, 10 Jul 2023 at 08:10, Pali Rohár wrote: > > On Sunday 25 June 2023 21:08:19 Tom Rini wrote: > > On Sun, Jun 25, 2023 at 05:15:34PM +0200, Pali Rohár wrote: > > > On Sunday 25 June 2023 10:52:13 Tom Rini wrote: > > > > On Sun, Jun 25, 2023 at 09:50:39AM +0200, Pali Rohár wrote: > > > > > On Saturday 24 June 2023 12:58:07 Tom Rini wrote: > > > > > > On Sat, Jun 24, 2023 at 10:50:41AM +0200, Pali Rohár wrote: > > > > > > > On Tuesday 20 June 2023 11:20:35 Simon Glass wrote: > > > > > > > > Hi Tom, Pali, > > > > > > > > > > > > > > > > On Wed, 14 Jun 2023 at 20:51, Tom Rini > > > > > > > > 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
Re: [PATCH u-boot 0/3] Revert broken Bootmenu commits
On Sunday 25 June 2023 21:08:19 Tom Rini wrote: > On Sun, Jun 25, 2023 at 05:15:34PM +0200, Pali Rohár wrote: > > On Sunday 25 June 2023 10:52:13 Tom Rini wrote: > > > On Sun, Jun 25, 2023 at 09:50:39AM +0200, Pali Rohár wrote: > > > > On Saturday 24 June 2023 12:58:07 Tom Rini wrote: > > > > > On Sat, Jun 24, 2023 at 10:50:41AM +0200, Pali Rohár wrote: > > > > > > On Tuesday 20 June 2023 11:20:35 Simon Glass wrote: > > > > > > > Hi Tom, Pali, > > > > > > > > > > > > > > On Wed, 14 Jun 2023 at 20:51, Tom Rini 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. > > > > > > > > > > Well, unless your changes here break something else, I don't think a > > > > > fix > > > > > for your problem will be ignored. > > > > > > > > This is something which I read here more times in the past... and > > > > reality was different. > > > > > > > > Should I
Re: [PATCH u-boot 0/3] Revert broken Bootmenu commits
On Sun, Jun 25, 2023 at 05:15:34PM +0200, Pali Rohár wrote: > On Sunday 25 June 2023 10:52:13 Tom Rini wrote: > > On Sun, Jun 25, 2023 at 09:50:39AM +0200, Pali Rohár wrote: > > > On Saturday 24 June 2023 12:58:07 Tom Rini wrote: > > > > On Sat, Jun 24, 2023 at 10:50:41AM +0200, Pali Rohár wrote: > > > > > On Tuesday 20 June 2023 11:20:35 Simon Glass wrote: > > > > > > Hi Tom, Pali, > > > > > > > > > > > > On Wed, 14 Jun 2023 at 20:51, Tom Rini 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. > > > > > > > > Well, unless your changes here break something else, I don't think a fix > > > > for your problem will be ignored. > > > > > > This is something which I read here more times in the past... and > > > reality was different. > > > > > > Should I remind you that there are waiting eMMC fixes for mvebu and > > > again discussion about them stalled? Or rather should I say that it is > > > again ignored? > > > > No, you should just say they're still waiting for review. Since they're > > waiting for review. The MMC custodian has been asked to review them, and > > hasn't yet.
Re: [PATCH u-boot 0/3] Revert broken Bootmenu commits
On Sunday 25 June 2023 10:52:13 Tom Rini wrote: > On Sun, Jun 25, 2023 at 09:50:39AM +0200, Pali Rohár wrote: > > On Saturday 24 June 2023 12:58:07 Tom Rini wrote: > > > On Sat, Jun 24, 2023 at 10:50:41AM +0200, Pali Rohár wrote: > > > > On Tuesday 20 June 2023 11:20:35 Simon Glass wrote: > > > > > Hi Tom, Pali, > > > > > > > > > > On Wed, 14 Jun 2023 at 20:51, Tom Rini 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. > > > > > > Well, unless your changes here break something else, I don't think a fix > > > for your problem will be ignored. > > > > This is something which I read here more times in the past... and > > reality was different. > > > > Should I remind you that there are waiting eMMC fixes for mvebu and > > again discussion about them stalled? Or rather should I say that it is > > again ignored? > > No, you should just say they're still waiting for review. Since they're > waiting for review. The MMC custodian has been asked to review them, and > hasn't yet. And they don't appear to be super critical changes, and they > conflict with other series, so yes, they'll get picked up when the > custodian has time to review everything. And what is "critical change" if it is not fixing booting (from eMMC)? > > I have already spend time on this and have done everything needed to > > make
Re: [PATCH u-boot 0/3] Revert broken Bootmenu commits
On Sun, Jun 25, 2023 at 09:50:39AM +0200, Pali Rohár wrote: > On Saturday 24 June 2023 12:58:07 Tom Rini wrote: > > On Sat, Jun 24, 2023 at 10:50:41AM +0200, Pali Rohár wrote: > > > On Tuesday 20 June 2023 11:20:35 Simon Glass wrote: > > > > Hi Tom, Pali, > > > > > > > > On Wed, 14 Jun 2023 at 20:51, Tom Rini 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. > > > > Well, unless your changes here break something else, I don't think a fix > > for your problem will be ignored. > > This is something which I read here more times in the past... and > reality was different. > > Should I remind you that there are waiting eMMC fixes for mvebu and > again discussion about them stalled? Or rather should I say that it is > again ignored? No, you should just say they're still waiting for review. Since they're waiting for review. The MMC custodian has been asked to review them, and hasn't yet. And they don't appear to be super critical changes, and they conflict with other series, so yes, they'll get picked up when the custodian has time to review everything. > I have already spend time on this and have done everything needed to > make bootmenu working again. I have also prepared patches which are > fixing this problem and which were also tested. Note that "I reverted the commit" is not a fix. > And if you want something more from me then you show me why this time it > would be different, and again empty promises. What I'd like is for you to not assume worst-faith. If you can fix the problem
Re: [PATCH u-boot 0/3] Revert broken Bootmenu commits
On Saturday 24 June 2023 12:58:07 Tom Rini wrote: > On Sat, Jun 24, 2023 at 10:50:41AM +0200, Pali Rohár wrote: > > On Tuesday 20 June 2023 11:20:35 Simon Glass wrote: > > > Hi Tom, Pali, > > > > > > On Wed, 14 Jun 2023 at 20:51, Tom Rini 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. > > Well, unless your changes here break something else, I don't think a fix > for your problem will be ignored. This is something which I read here more times in the past... and reality was different. Should I remind you that there are waiting eMMC fixes for mvebu and again discussion about them stalled? Or rather should I say that it is again ignored? I have already spend time on this and have done everything needed to make bootmenu working again. I have also prepared patches which are fixing this problem and which were also tested. And if you want something more from me then you show me why this time it would be different, and again empty promises. > > 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. > > Unfortunately you're also the only person able to replicate the problem, > and Simon has tried (and posted fixes to your platform for other issues, > and debugging patches to hopefully making finding the problem easier). Have somebody else even tried to reproduce this issue on any other HW board or any other qemu board?
Re: [PATCH u-boot 0/3] Revert broken Bootmenu commits
On Sat, Jun 24, 2023 at 10:50:41AM +0200, Pali Rohár wrote: > On Tuesday 20 June 2023 11:20:35 Simon Glass wrote: > > Hi Tom, Pali, > > > > On Wed, 14 Jun 2023 at 20:51, Tom Rini 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. Well, unless your changes here break something else, I don't think a fix for your problem will 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. Unfortunately you're also the only person able to replicate the problem, and Simon has tried (and posted fixes to your platform for other issues, and debugging patches to hopefully making finding the problem easier). -- Tom signature.asc Description: PGP signature
Re: [PATCH u-boot 0/3] Revert broken Bootmenu commits
On Tuesday 20 June 2023 11:20:35 Simon Glass wrote: > Hi Tom, Pali, > > On Wed, 14 Jun 2023 at 20:51, Tom Rini 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
Re: [PATCH u-boot 0/3] Revert broken Bootmenu commits
Hi Tom, Pali, On Wed, 14 Jun 2023 at 20:51, Tom Rini 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. 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
Re: [PATCH u-boot 0/3] Revert broken Bootmenu commits
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. -- Tom signature.asc Description: PGP signature
