All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] bootmenu: Allow to quit it via CTRL+C
@ 2020-12-26 18:02 Pali Rohár
  2020-12-26 18:44 ` Heinrich Schuchardt
  2020-12-27  0:04 ` [PATCH v2] bootmenu: Allow to quit it via ESC/CTRL+C Pali Rohár
  0 siblings, 2 replies; 17+ messages in thread
From: Pali Rohár @ 2020-12-26 18:02 UTC (permalink / raw)
  To: u-boot

When CTRL+C is pressed interrupt bootmenu and jump into U-Boot console.
As the last entry in bootmenu is always U-Boot console just choose the last
entry when CTRL+C is pressed.

It is useful when bootmenu is part of boot process and you want to
interrupt boot process by scripts which control U-Boot (serial) console.

Signed-off-by: Pali Roh?r <pali@kernel.org>
---
 cmd/bootmenu.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
index 1ba7b622e5..9b56bfaa9a 100644
--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -45,6 +45,7 @@ enum bootmenu_key {
 	KEY_UP,
 	KEY_DOWN,
 	KEY_SELECT,
+	KEY_QUIT,
 };
 
 static char *bootmenu_getoption(unsigned short int n)
@@ -109,6 +110,9 @@ static void bootmenu_autoboot_loop(struct bootmenu_data *menu,
 			case '\r':
 				*key = KEY_SELECT;
 				break;
+			case 0x3: /* ^C */
+				*key = KEY_QUIT;
+				break;
 			default:
 				*key = KEY_NONE;
 				break;
@@ -187,6 +191,10 @@ static void bootmenu_loop(struct bootmenu_data *menu,
 	/* enter key was pressed */
 	if (c == '\r')
 		*key = KEY_SELECT;
+
+	/* ^C was pressed */
+	if (c == 0x3)
+		*key = KEY_QUIT;
 }
 
 static char *bootmenu_choice_entry(void *data)
@@ -222,6 +230,12 @@ static char *bootmenu_choice_entry(void *data)
 			for (i = 0; i < menu->active; ++i)
 				iter = iter->next;
 			return iter->key;
+		case KEY_QUIT:
+			/* Quit by choosing the last entry - U-Boot console */
+			iter = menu->first;
+			while (iter->next)
+				iter = iter->next;
+			return iter->key;
 		default:
 			break;
 		}
@@ -389,7 +403,7 @@ static void menu_display_statusline(struct menu *m)
 	printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
 	puts(ANSI_CLEAR_LINE);
 	printf(ANSI_CURSOR_POSITION, menu->count + 6, 1);
-	puts("  Press UP/DOWN to move, ENTER to select");
+	puts("  Press UP/DOWN to move, ENTER to select, CTRL+C to quit");
 	puts(ANSI_CLEAR_LINE_TO_END);
 	printf(ANSI_CURSOR_POSITION, menu->count + 7, 1);
 	puts(ANSI_CLEAR_LINE);
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH] bootmenu: Allow to quit it via CTRL+C
  2020-12-26 18:02 [PATCH] bootmenu: Allow to quit it via CTRL+C Pali Rohár
@ 2020-12-26 18:44 ` Heinrich Schuchardt
  2020-12-26 19:03   ` Pali Rohár
  2020-12-27  0:04 ` [PATCH v2] bootmenu: Allow to quit it via ESC/CTRL+C Pali Rohár
  1 sibling, 1 reply; 17+ messages in thread
From: Heinrich Schuchardt @ 2020-12-26 18:44 UTC (permalink / raw)
  To: u-boot

Am 26. Dezember 2020 19:02:25 MEZ schrieb "Pali Roh?r" <pali@kernel.org>:
>When CTRL+C is pressed interrupt bootmenu and jump into U-Boot console.
>As the last entry in bootmenu is always U-Boot console just choose the
>last
>entry when CTRL+C is pressed.
>
>It is useful when bootmenu is part of boot process and you want to
>interrupt boot process by scripts which control U-Boot (serial)
>console.

Wouldn't the escape key be a better choice?

On the sandbox CTRL-C makes you quit U-Boot.

When hitting CTRL-C late on other systems it might interrupt a follow-up command.

--

The following is not directly related to your patch:

Is it really a good design that a user cannot be stopped from reaching the console?

I could think of use cases in the context of secure booting where you want to show a menu but you do not want to give access to the command line.

Best regards

Heinrich

>
>Signed-off-by: Pali Roh?r <pali@kernel.org>
>---
> cmd/bootmenu.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
>diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
>index 1ba7b622e5..9b56bfaa9a 100644
>--- a/cmd/bootmenu.c
>+++ b/cmd/bootmenu.c
>@@ -45,6 +45,7 @@ enum bootmenu_key {
> 	KEY_UP,
> 	KEY_DOWN,
> 	KEY_SELECT,
>+	KEY_QUIT,
> };
> 
> static char *bootmenu_getoption(unsigned short int n)
>@@ -109,6 +110,9 @@ static void bootmenu_autoboot_loop(struct
>bootmenu_data *menu,
> 			case '\r':
> 				*key = KEY_SELECT;
> 				break;
>+			case 0x3: /* ^C */
>+				*key = KEY_QUIT;
>+				break;
> 			default:
> 				*key = KEY_NONE;
> 				break;
>@@ -187,6 +191,10 @@ static void bootmenu_loop(struct bootmenu_data
>*menu,
> 	/* enter key was pressed */
> 	if (c == '\r')
> 		*key = KEY_SELECT;
>+
>+	/* ^C was pressed */
>+	if (c == 0x3)
>+		*key = KEY_QUIT;
> }
> 
> static char *bootmenu_choice_entry(void *data)
>@@ -222,6 +230,12 @@ static char *bootmenu_choice_entry(void *data)
> 			for (i = 0; i < menu->active; ++i)
> 				iter = iter->next;
> 			return iter->key;
>+		case KEY_QUIT:
>+			/* Quit by choosing the last entry - U-Boot console */
>+			iter = menu->first;
>+			while (iter->next)
>+				iter = iter->next;
>+			return iter->key;
> 		default:
> 			break;
> 		}
>@@ -389,7 +403,7 @@ static void menu_display_statusline(struct menu *m)
> 	printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
> 	puts(ANSI_CLEAR_LINE);
> 	printf(ANSI_CURSOR_POSITION, menu->count + 6, 1);
>-	puts("  Press UP/DOWN to move, ENTER to select");
>+	puts("  Press UP/DOWN to move, ENTER to select, CTRL+C to quit");
> 	puts(ANSI_CLEAR_LINE_TO_END);
> 	printf(ANSI_CURSOR_POSITION, menu->count + 7, 1);
> 	puts(ANSI_CLEAR_LINE);

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH] bootmenu: Allow to quit it via CTRL+C
  2020-12-26 18:44 ` Heinrich Schuchardt
@ 2020-12-26 19:03   ` Pali Rohár
  2020-12-26 19:10     ` Heinrich Schuchardt
  0 siblings, 1 reply; 17+ messages in thread
From: Pali Rohár @ 2020-12-26 19:03 UTC (permalink / raw)
  To: u-boot

On Saturday 26 December 2020 19:44:23 Heinrich Schuchardt wrote:
> Am 26. Dezember 2020 19:02:25 MEZ schrieb "Pali Roh?r" <pali@kernel.org>:
> >When CTRL+C is pressed interrupt bootmenu and jump into U-Boot console.
> >As the last entry in bootmenu is always U-Boot console just choose the
> >last
> >entry when CTRL+C is pressed.
> >
> >It is useful when bootmenu is part of boot process and you want to
> >interrupt boot process by scripts which control U-Boot (serial)
> >console.
> 
> Wouldn't the escape key be a better choice?

I can add also escape key. But has escape key stable ANSI sequence which
is needed to catch? If you tell me which bytes to catch (for escape key)
I will add it.

> On the sandbox CTRL-C makes you quit U-Boot.
> 
> When hitting CTRL-C late on other systems it might interrupt a follow-up command.

I understand your concerns. But all other commands catch also CTRL+C so
I think that bootmenu command should it too.

> --
> 
> The following is not directly related to your patch:
> 
> Is it really a good design that a user cannot be stopped from reaching the console?

Currently "U-Boot console" option is always added to menu list. But
option is doing only one thing: exiting the current U-Boot command and
returning back to the monitor mode.

> I could think of use cases in the context of secure booting where you want to show a menu but you do not want to give access to the command line.

If bootmenu is not started from monitor mode then this "U-Boot console"
command just enter into monitor mode. And I think that you can already
block monitor mode and therefore has ability of this secure boot scheme.

> Best regards
> 
> Heinrich
> 
> >
> >Signed-off-by: Pali Roh?r <pali@kernel.org>
> >---
> > cmd/bootmenu.c | 16 +++++++++++++++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
> >diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> >index 1ba7b622e5..9b56bfaa9a 100644
> >--- a/cmd/bootmenu.c
> >+++ b/cmd/bootmenu.c
> >@@ -45,6 +45,7 @@ enum bootmenu_key {
> > 	KEY_UP,
> > 	KEY_DOWN,
> > 	KEY_SELECT,
> >+	KEY_QUIT,
> > };
> > 
> > static char *bootmenu_getoption(unsigned short int n)
> >@@ -109,6 +110,9 @@ static void bootmenu_autoboot_loop(struct
> >bootmenu_data *menu,
> > 			case '\r':
> > 				*key = KEY_SELECT;
> > 				break;
> >+			case 0x3: /* ^C */
> >+				*key = KEY_QUIT;
> >+				break;
> > 			default:
> > 				*key = KEY_NONE;
> > 				break;
> >@@ -187,6 +191,10 @@ static void bootmenu_loop(struct bootmenu_data
> >*menu,
> > 	/* enter key was pressed */
> > 	if (c == '\r')
> > 		*key = KEY_SELECT;
> >+
> >+	/* ^C was pressed */
> >+	if (c == 0x3)
> >+		*key = KEY_QUIT;
> > }
> > 
> > static char *bootmenu_choice_entry(void *data)
> >@@ -222,6 +230,12 @@ static char *bootmenu_choice_entry(void *data)
> > 			for (i = 0; i < menu->active; ++i)
> > 				iter = iter->next;
> > 			return iter->key;
> >+		case KEY_QUIT:
> >+			/* Quit by choosing the last entry - U-Boot console */
> >+			iter = menu->first;
> >+			while (iter->next)
> >+				iter = iter->next;
> >+			return iter->key;
> > 		default:
> > 			break;
> > 		}
> >@@ -389,7 +403,7 @@ static void menu_display_statusline(struct menu *m)
> > 	printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
> > 	puts(ANSI_CLEAR_LINE);
> > 	printf(ANSI_CURSOR_POSITION, menu->count + 6, 1);
> >-	puts("  Press UP/DOWN to move, ENTER to select");
> >+	puts("  Press UP/DOWN to move, ENTER to select, CTRL+C to quit");
> > 	puts(ANSI_CLEAR_LINE_TO_END);
> > 	printf(ANSI_CURSOR_POSITION, menu->count + 7, 1);
> > 	puts(ANSI_CLEAR_LINE);
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH] bootmenu: Allow to quit it via CTRL+C
  2020-12-26 19:03   ` Pali Rohár
@ 2020-12-26 19:10     ` Heinrich Schuchardt
  2020-12-26 19:15       ` Pali Rohár
  0 siblings, 1 reply; 17+ messages in thread
From: Heinrich Schuchardt @ 2020-12-26 19:10 UTC (permalink / raw)
  To: u-boot

Am 26. Dezember 2020 20:03:56 MEZ schrieb "Pali Roh?r" <pali@kernel.org>:
>On Saturday 26 December 2020 19:44:23 Heinrich Schuchardt wrote:
>> Am 26. Dezember 2020 19:02:25 MEZ schrieb "Pali Roh?r"
><pali@kernel.org>:
>> >When CTRL+C is pressed interrupt bootmenu and jump into U-Boot
>console.
>> >As the last entry in bootmenu is always U-Boot console just choose
>the
>> >last
>> >entry when CTRL+C is pressed.
>> >
>> >It is useful when bootmenu is part of boot process and you want to
>> >interrupt boot process by scripts which control U-Boot (serial)
>> >console.
>> 
>> Wouldn't the escape key be a better choice?
>
>I can add also escape key. But has escape key stable ANSI sequence
>which
>is needed to catch? If you tell me which bytes to catch (for escape
>key)
>I will add it.

0x1b is Escape

>
>> On the sandbox CTRL-C makes you quit U-Boot.
>> 
>> When hitting CTRL-C late on other systems it might interrupt a
>follow-up command.
>
>I understand your concerns. But all other commands catch also CTRL+C so
>I think that bootmenu command should it too.
>
>> --
>> 
>> The following is not directly related to your patch:
>> 
>> Is it really a good design that a user cannot be stopped from
>reaching the console?
>
>Currently "U-Boot console" option is always added to menu list. But
>option is doing only one thing: exiting the current U-Boot command and
>returning back to the monitor mode.
>
>> I could think of use cases in the context of secure booting where you
>want to show a menu but you do not want to give access to the command
>line.
>
>If bootmenu is not started from monitor mode then this "U-Boot console"
>command just enter into monitor mode. And I think that you can already
>block monitor mode and therefore has ability of this secure boot
>scheme.
>
>> Best regards
>> 
>> Heinrich
>> 
>> >
>> >Signed-off-by: Pali Roh?r <pali@kernel.org>
>> >---
>> > cmd/bootmenu.c | 16 +++++++++++++++-
>> > 1 file changed, 15 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
>> >index 1ba7b622e5..9b56bfaa9a 100644
>> >--- a/cmd/bootmenu.c
>> >+++ b/cmd/bootmenu.c
>> >@@ -45,6 +45,7 @@ enum bootmenu_key {
>> > 	KEY_UP,
>> > 	KEY_DOWN,
>> > 	KEY_SELECT,
>> >+	KEY_QUIT,
>> > };
>> > 
>> > static char *bootmenu_getoption(unsigned short int n)
>> >@@ -109,6 +110,9 @@ static void bootmenu_autoboot_loop(struct
>> >bootmenu_data *menu,
>> > 			case '\r':
>> > 				*key = KEY_SELECT;
>> > 				break;
>> >+			case 0x3: /* ^C */
>> >+				*key = KEY_QUIT;
>> >+				break;
>> > 			default:
>> > 				*key = KEY_NONE;
>> > 				break;
>> >@@ -187,6 +191,10 @@ static void bootmenu_loop(struct bootmenu_data
>> >*menu,
>> > 	/* enter key was pressed */
>> > 	if (c == '\r')
>> > 		*key = KEY_SELECT;
>> >+
>> >+	/* ^C was pressed */
>> >+	if (c == 0x3)
>> >+		*key = KEY_QUIT;
>> > }
>> > 
>> > static char *bootmenu_choice_entry(void *data)
>> >@@ -222,6 +230,12 @@ static char *bootmenu_choice_entry(void *data)
>> > 			for (i = 0; i < menu->active; ++i)
>> > 				iter = iter->next;
>> > 			return iter->key;
>> >+		case KEY_QUIT:
>> >+			/* Quit by choosing the last entry - U-Boot console */
>> >+			iter = menu->first;
>> >+			while (iter->next)
>> >+				iter = iter->next;
>> >+			return iter->key;
>> > 		default:
>> > 			break;
>> > 		}
>> >@@ -389,7 +403,7 @@ static void menu_display_statusline(struct menu
>*m)
>> > 	printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
>> > 	puts(ANSI_CLEAR_LINE);
>> > 	printf(ANSI_CURSOR_POSITION, menu->count + 6, 1);
>> >-	puts("  Press UP/DOWN to move, ENTER to select");
>> >+	puts("  Press UP/DOWN to move, ENTER to select, CTRL+C to quit");
>> > 	puts(ANSI_CLEAR_LINE_TO_END);
>> > 	printf(ANSI_CURSOR_POSITION, menu->count + 7, 1);
>> > 	puts(ANSI_CLEAR_LINE);
>> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH] bootmenu: Allow to quit it via CTRL+C
  2020-12-26 19:10     ` Heinrich Schuchardt
@ 2020-12-26 19:15       ` Pali Rohár
  2020-12-26 19:44         ` Heinrich Schuchardt
  2020-12-26 22:32         ` [maemo-leste] " Pavel Machek
  0 siblings, 2 replies; 17+ messages in thread
From: Pali Rohár @ 2020-12-26 19:15 UTC (permalink / raw)
  To: u-boot

On Saturday 26 December 2020 20:10:10 Heinrich Schuchardt wrote:
> Am 26. Dezember 2020 20:03:56 MEZ schrieb "Pali Roh?r" <pali@kernel.org>:
> >On Saturday 26 December 2020 19:44:23 Heinrich Schuchardt wrote:
> >> Am 26. Dezember 2020 19:02:25 MEZ schrieb "Pali Roh?r"
> ><pali@kernel.org>:
> >> >When CTRL+C is pressed interrupt bootmenu and jump into U-Boot
> >console.
> >> >As the last entry in bootmenu is always U-Boot console just choose
> >the
> >> >last
> >> >entry when CTRL+C is pressed.
> >> >
> >> >It is useful when bootmenu is part of boot process and you want to
> >> >interrupt boot process by scripts which control U-Boot (serial)
> >> >console.
> >> 
> >> Wouldn't the escape key be a better choice?
> >
> >I can add also escape key. But has escape key stable ANSI sequence
> >which
> >is needed to catch? If you tell me which bytes to catch (for escape
> >key)
> >I will add it.
> 
> 0x1b is Escape

Does not work. 0x1b is not escape key. It is start of the ANSI escape
sequence which matches also existing keys up and down.

> >
> >> On the sandbox CTRL-C makes you quit U-Boot.
> >> 
> >> When hitting CTRL-C late on other systems it might interrupt a
> >follow-up command.
> >
> >I understand your concerns. But all other commands catch also CTRL+C so
> >I think that bootmenu command should it too.
> >
> >> --
> >> 
> >> The following is not directly related to your patch:
> >> 
> >> Is it really a good design that a user cannot be stopped from
> >reaching the console?
> >
> >Currently "U-Boot console" option is always added to menu list. But
> >option is doing only one thing: exiting the current U-Boot command and
> >returning back to the monitor mode.
> >
> >> I could think of use cases in the context of secure booting where you
> >want to show a menu but you do not want to give access to the command
> >line.
> >
> >If bootmenu is not started from monitor mode then this "U-Boot console"
> >command just enter into monitor mode. And I think that you can already
> >block monitor mode and therefore has ability of this secure boot
> >scheme.
> >
> >> Best regards
> >> 
> >> Heinrich
> >> 
> >> >
> >> >Signed-off-by: Pali Roh?r <pali@kernel.org>
> >> >---
> >> > cmd/bootmenu.c | 16 +++++++++++++++-
> >> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >> >
> >> >diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> >> >index 1ba7b622e5..9b56bfaa9a 100644
> >> >--- a/cmd/bootmenu.c
> >> >+++ b/cmd/bootmenu.c
> >> >@@ -45,6 +45,7 @@ enum bootmenu_key {
> >> > 	KEY_UP,
> >> > 	KEY_DOWN,
> >> > 	KEY_SELECT,
> >> >+	KEY_QUIT,
> >> > };
> >> > 
> >> > static char *bootmenu_getoption(unsigned short int n)
> >> >@@ -109,6 +110,9 @@ static void bootmenu_autoboot_loop(struct
> >> >bootmenu_data *menu,
> >> > 			case '\r':
> >> > 				*key = KEY_SELECT;
> >> > 				break;
> >> >+			case 0x3: /* ^C */
> >> >+				*key = KEY_QUIT;
> >> >+				break;
> >> > 			default:
> >> > 				*key = KEY_NONE;
> >> > 				break;
> >> >@@ -187,6 +191,10 @@ static void bootmenu_loop(struct bootmenu_data
> >> >*menu,
> >> > 	/* enter key was pressed */
> >> > 	if (c == '\r')
> >> > 		*key = KEY_SELECT;
> >> >+
> >> >+	/* ^C was pressed */
> >> >+	if (c == 0x3)
> >> >+		*key = KEY_QUIT;
> >> > }
> >> > 
> >> > static char *bootmenu_choice_entry(void *data)
> >> >@@ -222,6 +230,12 @@ static char *bootmenu_choice_entry(void *data)
> >> > 			for (i = 0; i < menu->active; ++i)
> >> > 				iter = iter->next;
> >> > 			return iter->key;
> >> >+		case KEY_QUIT:
> >> >+			/* Quit by choosing the last entry - U-Boot console */
> >> >+			iter = menu->first;
> >> >+			while (iter->next)
> >> >+				iter = iter->next;
> >> >+			return iter->key;
> >> > 		default:
> >> > 			break;
> >> > 		}
> >> >@@ -389,7 +403,7 @@ static void menu_display_statusline(struct menu
> >*m)
> >> > 	printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
> >> > 	puts(ANSI_CLEAR_LINE);
> >> > 	printf(ANSI_CURSOR_POSITION, menu->count + 6, 1);
> >> >-	puts("  Press UP/DOWN to move, ENTER to select");
> >> >+	puts("  Press UP/DOWN to move, ENTER to select, CTRL+C to quit");
> >> > 	puts(ANSI_CLEAR_LINE_TO_END);
> >> > 	printf(ANSI_CURSOR_POSITION, menu->count + 7, 1);
> >> > 	puts(ANSI_CLEAR_LINE);
> >> 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH] bootmenu: Allow to quit it via CTRL+C
  2020-12-26 19:15       ` Pali Rohár
@ 2020-12-26 19:44         ` Heinrich Schuchardt
  2020-12-26 19:50           ` Pali Rohár
  2020-12-26 22:32         ` [maemo-leste] " Pavel Machek
  1 sibling, 1 reply; 17+ messages in thread
From: Heinrich Schuchardt @ 2020-12-26 19:44 UTC (permalink / raw)
  To: u-boot

Am 26. Dezember 2020 20:15:40 MEZ schrieb "Pali Roh?r" <pali@kernel.org>:
>On Saturday 26 December 2020 20:10:10 Heinrich Schuchardt wrote:
>> Am 26. Dezember 2020 20:03:56 MEZ schrieb "Pali Roh?r"
><pali@kernel.org>:
>> >On Saturday 26 December 2020 19:44:23 Heinrich Schuchardt wrote:
>> >> Am 26. Dezember 2020 19:02:25 MEZ schrieb "Pali Roh?r"
>> ><pali@kernel.org>:
>> >> >When CTRL+C is pressed interrupt bootmenu and jump into U-Boot
>> >console.
>> >> >As the last entry in bootmenu is always U-Boot console just
>choose
>> >the
>> >> >last
>> >> >entry when CTRL+C is pressed.
>> >> >
>> >> >It is useful when bootmenu is part of boot process and you want
>to
>> >> >interrupt boot process by scripts which control U-Boot (serial)
>> >> >console.
>> >> 
>> >> Wouldn't the escape key be a better choice?
>> >
>> >I can add also escape key. But has escape key stable ANSI sequence
>> >which
>> >is needed to catch? If you tell me which bytes to catch (for escape
>> >key)
>> >I will add it.
>> 
>> 0x1b is Escape
>
>Does not work. 0x1b is not escape key. It is start of the ANSI escape
>sequence which matches also existing keys up and down.

On the serial console you have to hit the key twice.

The device driver will tranlate it to a single 0x1b which is the char you want to react to.

>
>> >
>> >> On the sandbox CTRL-C makes you quit U-Boot.
>> >> 
>> >> When hitting CTRL-C late on other systems it might interrupt a
>> >follow-up command.
>> >
>> >I understand your concerns. But all other commands catch also CTRL+C
>so
>> >I think that bootmenu command should it too.
>> >
>> >> --
>> >> 
>> >> The following is not directly related to your patch:
>> >> 
>> >> Is it really a good design that a user cannot be stopped from
>> >reaching the console?
>> >
>> >Currently "U-Boot console" option is always added to menu list. But
>> >option is doing only one thing: exiting the current U-Boot command
>and
>> >returning back to the monitor mode.
>> >
>> >> I could think of use cases in the context of secure booting where
>you
>> >want to show a menu but you do not want to give access to the
>command
>> >line.
>> >
>> >If bootmenu is not started from monitor mode then this "U-Boot
>console"
>> >command just enter into monitor mode. And I think that you can
>already
>> >block monitor mode and therefore has ability of this secure boot
>> >scheme.
>> >
>> >> Best regards
>> >> 
>> >> Heinrich
>> >> 
>> >> >
>> >> >Signed-off-by: Pali Roh?r <pali@kernel.org>
>> >> >---
>> >> > cmd/bootmenu.c | 16 +++++++++++++++-
>> >> > 1 file changed, 15 insertions(+), 1 deletion(-)
>> >> >
>> >> >diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
>> >> >index 1ba7b622e5..9b56bfaa9a 100644
>> >> >--- a/cmd/bootmenu.c
>> >> >+++ b/cmd/bootmenu.c
>> >> >@@ -45,6 +45,7 @@ enum bootmenu_key {
>> >> > 	KEY_UP,
>> >> > 	KEY_DOWN,
>> >> > 	KEY_SELECT,
>> >> >+	KEY_QUIT,
>> >> > };
>> >> > 
>> >> > static char *bootmenu_getoption(unsigned short int n)
>> >> >@@ -109,6 +110,9 @@ static void bootmenu_autoboot_loop(struct
>> >> >bootmenu_data *menu,
>> >> > 			case '\r':
>> >> > 				*key = KEY_SELECT;
>> >> > 				break;
>> >> >+			case 0x3: /* ^C */
>> >> >+				*key = KEY_QUIT;
>> >> >+				break;
>> >> > 			default:
>> >> > 				*key = KEY_NONE;
>> >> > 				break;
>> >> >@@ -187,6 +191,10 @@ static void bootmenu_loop(struct
>bootmenu_data
>> >> >*menu,
>> >> > 	/* enter key was pressed */
>> >> > 	if (c == '\r')
>> >> > 		*key = KEY_SELECT;
>> >> >+
>> >> >+	/* ^C was pressed */
>> >> >+	if (c == 0x3)
>> >> >+		*key = KEY_QUIT;
>> >> > }
>> >> > 
>> >> > static char *bootmenu_choice_entry(void *data)
>> >> >@@ -222,6 +230,12 @@ static char *bootmenu_choice_entry(void
>*data)
>> >> > 			for (i = 0; i < menu->active; ++i)
>> >> > 				iter = iter->next;
>> >> > 			return iter->key;
>> >> >+		case KEY_QUIT:
>> >> >+			/* Quit by choosing the last entry - U-Boot console */
>> >> >+			iter = menu->first;
>> >> >+			while (iter->next)
>> >> >+				iter = iter->next;
>> >> >+			return iter->key;
>> >> > 		default:
>> >> > 			break;
>> >> > 		}
>> >> >@@ -389,7 +403,7 @@ static void menu_display_statusline(struct
>menu
>> >*m)
>> >> > 	printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
>> >> > 	puts(ANSI_CLEAR_LINE);
>> >> > 	printf(ANSI_CURSOR_POSITION, menu->count + 6, 1);
>> >> >-	puts("  Press UP/DOWN to move, ENTER to select");
>> >> >+	puts("  Press UP/DOWN to move, ENTER to select, CTRL+C to
>quit");
>> >> > 	puts(ANSI_CLEAR_LINE_TO_END);
>> >> > 	printf(ANSI_CURSOR_POSITION, menu->count + 7, 1);
>> >> > 	puts(ANSI_CLEAR_LINE);
>> >> 
>> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH] bootmenu: Allow to quit it via CTRL+C
  2020-12-26 19:44         ` Heinrich Schuchardt
@ 2020-12-26 19:50           ` Pali Rohár
  2020-12-26 22:15             ` Heinrich Schuchardt
  0 siblings, 1 reply; 17+ messages in thread
From: Pali Rohár @ 2020-12-26 19:50 UTC (permalink / raw)
  To: u-boot

On Saturday 26 December 2020 20:44:45 Heinrich Schuchardt wrote:
> Am 26. Dezember 2020 20:15:40 MEZ schrieb "Pali Roh?r" <pali@kernel.org>:
> >On Saturday 26 December 2020 20:10:10 Heinrich Schuchardt wrote:
> >> Am 26. Dezember 2020 20:03:56 MEZ schrieb "Pali Roh?r"
> ><pali@kernel.org>:
> >> >On Saturday 26 December 2020 19:44:23 Heinrich Schuchardt wrote:
> >> >> Am 26. Dezember 2020 19:02:25 MEZ schrieb "Pali Roh?r"
> >> ><pali@kernel.org>:
> >> >> >When CTRL+C is pressed interrupt bootmenu and jump into U-Boot
> >> >console.
> >> >> >As the last entry in bootmenu is always U-Boot console just
> >choose
> >> >the
> >> >> >last
> >> >> >entry when CTRL+C is pressed.
> >> >> >
> >> >> >It is useful when bootmenu is part of boot process and you want
> >to
> >> >> >interrupt boot process by scripts which control U-Boot (serial)
> >> >> >console.
> >> >> 
> >> >> Wouldn't the escape key be a better choice?
> >> >
> >> >I can add also escape key. But has escape key stable ANSI sequence
> >> >which
> >> >is needed to catch? If you tell me which bytes to catch (for escape
> >> >key)
> >> >I will add it.
> >> 
> >> 0x1b is Escape
> >
> >Does not work. 0x1b is not escape key. It is start of the ANSI escape
> >sequence which matches also existing keys up and down.
> 
> On the serial console you have to hit the key twice.
> 
> The device driver will tranlate it to a single 0x1b which is the char you want to react to.

Which device driver? bootmenu code is already catching and reacting to
the byte 0x1b as part of the key up and key down matching. So single
0x1b for sure cannot be caught in bootmenu code as then key up and key
down keys stops working. If it is really 0x1b byte then it needs to be
somehow escaped and therefore be part of some longer, not single byte
sequence.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH] bootmenu: Allow to quit it via CTRL+C
  2020-12-26 19:50           ` Pali Rohár
@ 2020-12-26 22:15             ` Heinrich Schuchardt
  2020-12-26 22:35               ` Pali Rohár
  0 siblings, 1 reply; 17+ messages in thread
From: Heinrich Schuchardt @ 2020-12-26 22:15 UTC (permalink / raw)
  To: u-boot

On 12/26/20 8:50 PM, Pali Roh?r wrote:
> On Saturday 26 December 2020 20:44:45 Heinrich Schuchardt wrote:
>> Am 26. Dezember 2020 20:15:40 MEZ schrieb "Pali Roh?r" <pali@kernel.org>:
>>> On Saturday 26 December 2020 20:10:10 Heinrich Schuchardt wrote:
>>>> Am 26. Dezember 2020 20:03:56 MEZ schrieb "Pali Roh?r"
>>> <pali@kernel.org>:
>>>>> On Saturday 26 December 2020 19:44:23 Heinrich Schuchardt wrote:
>>>>>> Am 26. Dezember 2020 19:02:25 MEZ schrieb "Pali Roh?r"
>>>>> <pali@kernel.org>:
>>>>>>> When CTRL+C is pressed interrupt bootmenu and jump into U-Boot
>>>>> console.
>>>>>>> As the last entry in bootmenu is always U-Boot console just
>>> choose
>>>>> the
>>>>>>> last
>>>>>>> entry when CTRL+C is pressed.
>>>>>>>
>>>>>>> It is useful when bootmenu is part of boot process and you want
>>> to
>>>>>>> interrupt boot process by scripts which control U-Boot (serial)
>>>>>>> console.
>>>>>>
>>>>>> Wouldn't the escape key be a better choice?
>>>>>
>>>>> I can add also escape key. But has escape key stable ANSI sequence
>>>>> which
>>>>> is needed to catch? If you tell me which bytes to catch (for escape
>>>>> key)
>>>>> I will add it.
>>>>
>>>> 0x1b is Escape
>>>
>>> Does not work. 0x1b is not escape key. It is start of the ANSI escape
>>> sequence which matches also existing keys up and down.
>>
>> On the serial console you have to hit the key twice.
>>
>> The device driver will tranlate it to a single 0x1b which is the char you want to react to.
>
> Which device driver? bootmenu code is already catching and reacting to
> the byte 0x1b as part of the key up and key down matching. So single
> 0x1b for sure cannot be caught in bootmenu code as then key up and key
> down keys stops working. If it is really 0x1b byte then it needs to be
> somehow escaped and therefore be part of some longer, not single byte
> sequence.
>

diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
index 1ba7b622e5..26222ff8b7 100644
--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -44,6 +44,7 @@ enum bootmenu_key {
  	KEY_NONE = 0,
  	KEY_UP,
  	KEY_DOWN,
+	KEY_QUIT,
  	KEY_SELECT,
  };

@@ -153,11 +154,18 @@ static void bootmenu_loop(struct bootmenu_data *menu,
  		break;
  	case 1:
  		/* Second char of ANSI '[' */
-		if (c == '[') {
+		switch (c) {
+		case '\e':
+			*esc = 0;
+			*key = KEY_QUIT;
+			break;
+		case '[':
  			*esc = 2;
  			*key = KEY_NONE;
-		} else {
+			break;
+		default:
  			*esc = 0;
+			break;
  		}
  		break;
  	case 2:
@@ -217,6 +225,12 @@ static char *bootmenu_choice_entry(void *data)
  				++menu->active;
  			/* no menu key selected, regenerate menu */
  			return NULL;
+		case KEY_QUIT:
+			/* Quit by choosing the last entry - U-Boot console */
+			iter = menu->first;
+			while (iter->next)
+				iter = iter->next;
+			return iter->key;
  		case KEY_SELECT:
  			iter = menu->first;
  			for (i = 0; i < menu->active; ++i)

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [maemo-leste] [PATCH] bootmenu: Allow to quit it via CTRL+C
  2020-12-26 19:15       ` Pali Rohár
  2020-12-26 19:44         ` Heinrich Schuchardt
@ 2020-12-26 22:32         ` Pavel Machek
  2020-12-26 22:42           ` Pali Rohár
  1 sibling, 1 reply; 17+ messages in thread
From: Pavel Machek @ 2020-12-26 22:32 UTC (permalink / raw)
  To: u-boot

On Sat 2020-12-26 20:15:40, Pali Roh?r wrote:
> On Saturday 26 December 2020 20:10:10 Heinrich Schuchardt wrote:
> > Am 26. Dezember 2020 20:03:56 MEZ schrieb "Pali Roh?r" <pali@kernel.org>:
> > >On Saturday 26 December 2020 19:44:23 Heinrich Schuchardt wrote:
> > >> Am 26. Dezember 2020 19:02:25 MEZ schrieb "Pali Roh?r"
> > ><pali@kernel.org>:
> > >> >When CTRL+C is pressed interrupt bootmenu and jump into U-Boot
> > >console.
> > >> >As the last entry in bootmenu is always U-Boot console just choose
> > >the
> > >> >last
> > >> >entry when CTRL+C is pressed.
> > >> >
> > >> >It is useful when bootmenu is part of boot process and you want to
> > >> >interrupt boot process by scripts which control U-Boot (serial)
> > >> >console.
> > >> 
> > >> Wouldn't the escape key be a better choice?
> > >
> > >I can add also escape key. But has escape key stable ANSI sequence
> > >which
> > >is needed to catch? If you tell me which bytes to catch (for escape
> > >key)
> > >I will add it.
> > 
> > 0x1b is Escape
> 
> Does not work. 0x1b is not escape key. It is start of the ANSI escape
> sequence which matches also existing keys up and down.

Unfortunately, 0x1b _is_ escape key. That is long standing bug of
vt100 terminal.

Usually timeout is used for detection. 0x1b followed by delay is
escape key; 0x1b followed by [ is some other key.

Best regards,							Pavel
-- 
http://www.livejournal.com/~pavelmachek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201226/4bc41d97/attachment.sig>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH] bootmenu: Allow to quit it via CTRL+C
  2020-12-26 22:15             ` Heinrich Schuchardt
@ 2020-12-26 22:35               ` Pali Rohár
  0 siblings, 0 replies; 17+ messages in thread
From: Pali Rohár @ 2020-12-26 22:35 UTC (permalink / raw)
  To: u-boot

On Saturday 26 December 2020 23:15:29 Heinrich Schuchardt wrote:
> On 12/26/20 8:50 PM, Pali Roh?r wrote:
> > On Saturday 26 December 2020 20:44:45 Heinrich Schuchardt wrote:
> > > Am 26. Dezember 2020 20:15:40 MEZ schrieb "Pali Roh?r" <pali@kernel.org>:
> > > > On Saturday 26 December 2020 20:10:10 Heinrich Schuchardt wrote:
> > > > > Am 26. Dezember 2020 20:03:56 MEZ schrieb "Pali Roh?r"
> > > > <pali@kernel.org>:
> > > > > > On Saturday 26 December 2020 19:44:23 Heinrich Schuchardt wrote:
> > > > > > > Am 26. Dezember 2020 19:02:25 MEZ schrieb "Pali Roh?r"
> > > > > > <pali@kernel.org>:
> > > > > > > > When CTRL+C is pressed interrupt bootmenu and jump into U-Boot
> > > > > > console.
> > > > > > > > As the last entry in bootmenu is always U-Boot console just
> > > > choose
> > > > > > the
> > > > > > > > last
> > > > > > > > entry when CTRL+C is pressed.
> > > > > > > > 
> > > > > > > > It is useful when bootmenu is part of boot process and you want
> > > > to
> > > > > > > > interrupt boot process by scripts which control U-Boot (serial)
> > > > > > > > console.
> > > > > > > 
> > > > > > > Wouldn't the escape key be a better choice?
> > > > > > 
> > > > > > I can add also escape key. But has escape key stable ANSI sequence
> > > > > > which
> > > > > > is needed to catch? If you tell me which bytes to catch (for escape
> > > > > > key)
> > > > > > I will add it.
> > > > > 
> > > > > 0x1b is Escape
> > > > 
> > > > Does not work. 0x1b is not escape key. It is start of the ANSI escape
> > > > sequence which matches also existing keys up and down.
> > > 
> > > On the serial console you have to hit the key twice.
> > > 
> > > The device driver will tranlate it to a single 0x1b which is the char you want to react to.
> > 
> > Which device driver? bootmenu code is already catching and reacting to
> > the byte 0x1b as part of the key up and key down matching. So single
> > 0x1b for sure cannot be caught in bootmenu code as then key up and key
> > down keys stops working. If it is really 0x1b byte then it needs to be
> > somehow escaped and therefore be part of some longer, not single byte
> > sequence.
> > 
> 
> diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> index 1ba7b622e5..26222ff8b7 100644
> --- a/cmd/bootmenu.c
> +++ b/cmd/bootmenu.c
> @@ -44,6 +44,7 @@ enum bootmenu_key {
>  	KEY_NONE = 0,
>  	KEY_UP,
>  	KEY_DOWN,
> +	KEY_QUIT,
>  	KEY_SELECT,
>  };
> 
> @@ -153,11 +154,18 @@ static void bootmenu_loop(struct bootmenu_data *menu,
>  		break;
>  	case 1:
>  		/* Second char of ANSI '[' */
> -		if (c == '[') {
> +		switch (c) {
> +		case '\e':
> +			*esc = 0;
> +			*key = KEY_QUIT;
> +			break;

Thanks! Now I see what you mean. Basically 0x1b is escaped by escape
(0x1b) character.

> +		case '[':
>  			*esc = 2;
>  			*key = KEY_NONE;
> -		} else {
> +			break;
> +		default:
>  			*esc = 0;
> +			break;
>  		}
>  		break;
>  	case 2:
> @@ -217,6 +225,12 @@ static char *bootmenu_choice_entry(void *data)
>  				++menu->active;
>  			/* no menu key selected, regenerate menu */
>  			return NULL;
> +		case KEY_QUIT:
> +			/* Quit by choosing the last entry - U-Boot console */
> +			iter = menu->first;
> +			while (iter->next)
> +				iter = iter->next;
> +			return iter->key;
>  		case KEY_SELECT:
>  			iter = menu->first;
>  			for (i = 0; i < menu->active; ++i)
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [maemo-leste] [PATCH] bootmenu: Allow to quit it via CTRL+C
  2020-12-26 22:32         ` [maemo-leste] " Pavel Machek
@ 2020-12-26 22:42           ` Pali Rohár
  2020-12-26 22:48             ` Pavel Machek
  2020-12-26 23:04             ` Heinrich Schuchardt
  0 siblings, 2 replies; 17+ messages in thread
From: Pali Rohár @ 2020-12-26 22:42 UTC (permalink / raw)
  To: u-boot

On Saturday 26 December 2020 23:32:27 Pavel Machek wrote:
> On Sat 2020-12-26 20:15:40, Pali Roh?r wrote:
> > On Saturday 26 December 2020 20:10:10 Heinrich Schuchardt wrote:
> > > Am 26. Dezember 2020 20:03:56 MEZ schrieb "Pali Roh?r" <pali@kernel.org>:
> > > >On Saturday 26 December 2020 19:44:23 Heinrich Schuchardt wrote:
> > > >> Am 26. Dezember 2020 19:02:25 MEZ schrieb "Pali Roh?r"
> > > ><pali@kernel.org>:
> > > >> >When CTRL+C is pressed interrupt bootmenu and jump into U-Boot
> > > >console.
> > > >> >As the last entry in bootmenu is always U-Boot console just choose
> > > >the
> > > >> >last
> > > >> >entry when CTRL+C is pressed.
> > > >> >
> > > >> >It is useful when bootmenu is part of boot process and you want to
> > > >> >interrupt boot process by scripts which control U-Boot (serial)
> > > >> >console.
> > > >> 
> > > >> Wouldn't the escape key be a better choice?
> > > >
> > > >I can add also escape key. But has escape key stable ANSI sequence
> > > >which
> > > >is needed to catch? If you tell me which bytes to catch (for escape
> > > >key)
> > > >I will add it.
> > > 
> > > 0x1b is Escape
> > 
> > Does not work. 0x1b is not escape key. It is start of the ANSI escape
> > sequence which matches also existing keys up and down.
> 
> Unfortunately, 0x1b _is_ escape key. That is long standing bug of
> vt100 terminal.

Ok. And has xterm (or other terminals) somehow fixed it? Should we
expect that some other terminals send something different for ESC key?

> Usually timeout is used for detection. 0x1b followed by delay is
> escape key; 0x1b followed by [ is some other key.

Any idea how long timeout should be used for this detection?

Heinrich wrote in his patch that sequence of 0x1b 0x1b should be handled
by escape key. Does it mean that we need to handle both 0x1b+timeout and
also 0x1b+0x1b as a escape key? Or we should handle 0x1b+timeout or 0x1
followed by any non '[' character as escape key?


Anyway, this bootmenu was initially written for Nokia N900 (used on LCD
display with integrated keyboard) and this device does not have ESC key.
So I would like to have CTRL+C in bootmenu working independently of ESC
key support.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [maemo-leste] [PATCH] bootmenu: Allow to quit it via CTRL+C
  2020-12-26 22:42           ` Pali Rohár
@ 2020-12-26 22:48             ` Pavel Machek
  2020-12-26 23:04             ` Heinrich Schuchardt
  1 sibling, 0 replies; 17+ messages in thread
From: Pavel Machek @ 2020-12-26 22:48 UTC (permalink / raw)
  To: u-boot

Hi!

> > > > >> >entry when CTRL+C is pressed.
> > > > >> >
> > > > >> >It is useful when bootmenu is part of boot process and you want to
> > > > >> >interrupt boot process by scripts which control U-Boot (serial)
> > > > >> >console.
> > > > >> 
> > > > >> Wouldn't the escape key be a better choice?
> > > > >
> > > > >I can add also escape key. But has escape key stable ANSI sequence
> > > > >which
> > > > >is needed to catch? If you tell me which bytes to catch (for escape
> > > > >key)
> > > > >I will add it.
> > > > 
> > > > 0x1b is Escape
> > > 
> > > Does not work. 0x1b is not escape key. It is start of the ANSI escape
> > > sequence which matches also existing keys up and down.
> > 
> > Unfortunately, 0x1b _is_ escape key. That is long standing bug of
> > vt100 terminal.
> 
> Ok. And has xterm (or other terminals) somehow fixed it? Should we
> expect that some other terminals send something different for ESC
>key?

Not really. Try it:

pavel at duo:~$ cat | hexdump -C
^[ahoj
00000000  1b 61 68 6f 6a 0a                                 |.ahoj.|
00000006

> > Usually timeout is used for detection. 0x1b followed by delay is
> > escape key; 0x1b followed by [ is some other key.
> 
> Any idea how long timeout should be used for this detection?
> 
> Heinrich wrote in his patch that sequence of 0x1b 0x1b should be handled
> by escape key. Does it mean that we need to handle both 0x1b+timeout and
> also 0x1b+0x1b as a escape key? Or we should handle 0x1b+timeout or 0x1
> followed by any non '[' character as escape key?

Yes, that makes some sense. 10msec would be right value for the
timeout, I'd say.

> Anyway, this bootmenu was initially written for Nokia N900 (used on LCD
> display with integrated keyboard) and this device does not have ESC key.
> So I would like to have CTRL+C in bootmenu working independently of ESC
> key support.

Or maybe "q" is good choice for the bootmenu?

Best regards,
									Pavel
-- 
http://www.livejournal.com/~pavelmachek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201226/02cb5dd2/attachment.sig>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [maemo-leste] [PATCH] bootmenu: Allow to quit it via CTRL+C
  2020-12-26 22:42           ` Pali Rohár
  2020-12-26 22:48             ` Pavel Machek
@ 2020-12-26 23:04             ` Heinrich Schuchardt
  2020-12-26 23:16               ` Pali Rohár
  1 sibling, 1 reply; 17+ messages in thread
From: Heinrich Schuchardt @ 2020-12-26 23:04 UTC (permalink / raw)
  To: u-boot

On 12/26/20 11:42 PM, Pali Roh?r wrote:
> On Saturday 26 December 2020 23:32:27 Pavel Machek wrote:
>> On Sat 2020-12-26 20:15:40, Pali Roh?r wrote:
>>> On Saturday 26 December 2020 20:10:10 Heinrich Schuchardt wrote:
>>>> Am 26. Dezember 2020 20:03:56 MEZ schrieb "Pali Roh?r" <pali@kernel.org>:
>>>>> On Saturday 26 December 2020 19:44:23 Heinrich Schuchardt wrote:
>>>>>> Am 26. Dezember 2020 19:02:25 MEZ schrieb "Pali Roh?r"
>>>>> <pali@kernel.org>:
>>>>>>> When CTRL+C is pressed interrupt bootmenu and jump into U-Boot
>>>>> console.
>>>>>>> As the last entry in bootmenu is always U-Boot console just choose
>>>>> the
>>>>>>> last
>>>>>>> entry when CTRL+C is pressed.
>>>>>>>
>>>>>>> It is useful when bootmenu is part of boot process and you want to
>>>>>>> interrupt boot process by scripts which control U-Boot (serial)
>>>>>>> console.
>>>>>>
>>>>>> Wouldn't the escape key be a better choice?
>>>>>
>>>>> I can add also escape key. But has escape key stable ANSI sequence
>>>>> which
>>>>> is needed to catch? If you tell me which bytes to catch (for escape
>>>>> key)
>>>>> I will add it.
>>>>
>>>> 0x1b is Escape
>>>
>>> Does not work. 0x1b is not escape key. It is start of the ANSI escape
>>> sequence which matches also existing keys up and down.
>>
>> Unfortunately, 0x1b _is_ escape key. That is long standing bug of
>> vt100 terminal.
>
> Ok. And has xterm (or other terminals) somehow fixed it? Should we
> expect that some other terminals send something different for ESC key?
>
>> Usually timeout is used for detection. 0x1b followed by delay is
>> escape key; 0x1b followed by [ is some other key.
>
> Any idea how long timeout should be used for this detection?
>
> Heinrich wrote in his patch that sequence of 0x1b 0x1b should be handled
> by escape key. Does it mean that we need to handle both 0x1b+timeout and
> also 0x1b+0x1b as a escape key? Or we should handle 0x1b+timeout or 0x1
> followed by any non '[' character as escape key?

'man console_codes' teaches that that there are escape sequences not
starting with CSI (ESC, '[').

At 1200 baud waiting for 10 ms would be enough to differentiate between
a single key stroke an escape sequence. The 1 ms wait used in
do_conitrace() will only work at >= 19200 baud.

>
>
> Anyway, this bootmenu was initially written for Nokia N900 (used on LCD
> display with integrated keyboard) and this device does not have ESC key.
> So I would like to have CTRL+C in bootmenu working independently of ESC
> key support.
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [maemo-leste] [PATCH] bootmenu: Allow to quit it via CTRL+C
  2020-12-26 23:04             ` Heinrich Schuchardt
@ 2020-12-26 23:16               ` Pali Rohár
  0 siblings, 0 replies; 17+ messages in thread
From: Pali Rohár @ 2020-12-26 23:16 UTC (permalink / raw)
  To: u-boot

On Sunday 27 December 2020 00:04:53 Heinrich Schuchardt wrote:
> On 12/26/20 11:42 PM, Pali Roh?r wrote:
> > On Saturday 26 December 2020 23:32:27 Pavel Machek wrote:
> > > On Sat 2020-12-26 20:15:40, Pali Roh?r wrote:
> > > > On Saturday 26 December 2020 20:10:10 Heinrich Schuchardt wrote:
> > > > > Am 26. Dezember 2020 20:03:56 MEZ schrieb "Pali Roh?r" <pali@kernel.org>:
> > > > > > On Saturday 26 December 2020 19:44:23 Heinrich Schuchardt wrote:
> > > > > > > Am 26. Dezember 2020 19:02:25 MEZ schrieb "Pali Roh?r"
> > > > > > <pali@kernel.org>:
> > > > > > > > When CTRL+C is pressed interrupt bootmenu and jump into U-Boot
> > > > > > console.
> > > > > > > > As the last entry in bootmenu is always U-Boot console just choose
> > > > > > the
> > > > > > > > last
> > > > > > > > entry when CTRL+C is pressed.
> > > > > > > > 
> > > > > > > > It is useful when bootmenu is part of boot process and you want to
> > > > > > > > interrupt boot process by scripts which control U-Boot (serial)
> > > > > > > > console.
> > > > > > > 
> > > > > > > Wouldn't the escape key be a better choice?
> > > > > > 
> > > > > > I can add also escape key. But has escape key stable ANSI sequence
> > > > > > which
> > > > > > is needed to catch? If you tell me which bytes to catch (for escape
> > > > > > key)
> > > > > > I will add it.
> > > > > 
> > > > > 0x1b is Escape
> > > > 
> > > > Does not work. 0x1b is not escape key. It is start of the ANSI escape
> > > > sequence which matches also existing keys up and down.
> > > 
> > > Unfortunately, 0x1b _is_ escape key. That is long standing bug of
> > > vt100 terminal.
> > 
> > Ok. And has xterm (or other terminals) somehow fixed it? Should we
> > expect that some other terminals send something different for ESC key?
> > 
> > > Usually timeout is used for detection. 0x1b followed by delay is
> > > escape key; 0x1b followed by [ is some other key.
> > 
> > Any idea how long timeout should be used for this detection?
> > 
> > Heinrich wrote in his patch that sequence of 0x1b 0x1b should be handled
> > by escape key. Does it mean that we need to handle both 0x1b+timeout and
> > also 0x1b+0x1b as a escape key? Or we should handle 0x1b+timeout or 0x1
> > followed by any non '[' character as escape key?
> 
> 'man console_codes' teaches that that there are escape sequences not
> starting with CSI (ESC, '[').

Thanks! I did not know about this manpage.

> At 1200 baud waiting for 10 ms would be enough to differentiate between
> a single key stroke an escape sequence. The 1 ms wait used in
> do_conitrace() will only work at >= 19200 baud.
> 
> > 
> > 
> > Anyway, this bootmenu was initially written for Nokia N900 (used on LCD
> > display with integrated keyboard) and this device does not have ESC key.
> > So I would like to have CTRL+C in bootmenu working independently of ESC
> > key support.
> > 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2] bootmenu: Allow to quit it via ESC/CTRL+C
  2020-12-26 18:02 [PATCH] bootmenu: Allow to quit it via CTRL+C Pali Rohár
  2020-12-26 18:44 ` Heinrich Schuchardt
@ 2020-12-27  0:04 ` Pali Rohár
  2020-12-29  3:32   ` Simon Glass
  2021-01-18 13:01   ` Tom Rini
  1 sibling, 2 replies; 17+ messages in thread
From: Pali Rohár @ 2020-12-27  0:04 UTC (permalink / raw)
  To: u-boot

When ESC/CTRL+C is pressed interrupt bootmenu and jump into U-Boot console.
As the last entry in bootmenu is always U-Boot console just choose the last
entry when ESC or CTRL+C is pressed.

ESC key is detected when either no other character appears after '\e'
within 10ms or when non-'[' appears after '\e'.

It is useful when bootmenu is part of boot process and you want to
interrupt boot process by scripts which control U-Boot (serial) console.

Signed-off-by: Pali Roh?r <pali@kernel.org>

---
Changes in v2:
* Added support also for ESC key
---
 cmd/bootmenu.c | 42 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 7 deletions(-)

diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
index 1ba7b622e5..409ef9a848 100644
--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -45,6 +45,7 @@ enum bootmenu_key {
 	KEY_UP,
 	KEY_DOWN,
 	KEY_SELECT,
+	KEY_QUIT,
 };
 
 static char *bootmenu_getoption(unsigned short int n)
@@ -109,6 +110,9 @@ static void bootmenu_autoboot_loop(struct bootmenu_data *menu,
 			case '\r':
 				*key = KEY_SELECT;
 				break;
+			case 0x3: /* ^C */
+				*key = KEY_QUIT;
+				break;
 			default:
 				*key = KEY_NONE;
 				break;
@@ -136,13 +140,25 @@ static void bootmenu_loop(struct bootmenu_data *menu,
 {
 	int c;
 
-	while (!tstc()) {
-		WATCHDOG_RESET();
-		mdelay(10);
+	if (*esc == 1) {
+		if (tstc()) {
+			c = getchar();
+		} else {
+			WATCHDOG_RESET();
+			mdelay(10);
+			if (tstc())
+				c = getchar();
+			else
+				c = '\e';
+		}
+	} else {
+		while (!tstc()) {
+			WATCHDOG_RESET();
+			mdelay(10);
+		}
+		c = getchar();
 	}
 
-	c = getchar();
-
 	switch (*esc) {
 	case 0:
 		/* First char of ANSI escape sequence '\e' */
@@ -157,7 +173,9 @@ static void bootmenu_loop(struct bootmenu_data *menu,
 			*esc = 2;
 			*key = KEY_NONE;
 		} else {
-			*esc = 0;
+		/* Alone ESC key was pressed */
+			*key = KEY_QUIT;
+			*esc = (c == '\e') ? 1 : 0;
 		}
 		break;
 	case 2:
@@ -187,6 +205,10 @@ static void bootmenu_loop(struct bootmenu_data *menu,
 	/* enter key was pressed */
 	if (c == '\r')
 		*key = KEY_SELECT;
+
+	/* ^C was pressed */
+	if (c == 0x3)
+		*key = KEY_QUIT;
 }
 
 static char *bootmenu_choice_entry(void *data)
@@ -222,6 +244,12 @@ static char *bootmenu_choice_entry(void *data)
 			for (i = 0; i < menu->active; ++i)
 				iter = iter->next;
 			return iter->key;
+		case KEY_QUIT:
+			/* Quit by choosing the last entry - U-Boot console */
+			iter = menu->first;
+			while (iter->next)
+				iter = iter->next;
+			return iter->key;
 		default:
 			break;
 		}
@@ -389,7 +417,7 @@ static void menu_display_statusline(struct menu *m)
 	printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
 	puts(ANSI_CLEAR_LINE);
 	printf(ANSI_CURSOR_POSITION, menu->count + 6, 1);
-	puts("  Press UP/DOWN to move, ENTER to select");
+	puts("  Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit");
 	puts(ANSI_CLEAR_LINE_TO_END);
 	printf(ANSI_CURSOR_POSITION, menu->count + 7, 1);
 	puts(ANSI_CLEAR_LINE);
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2] bootmenu: Allow to quit it via ESC/CTRL+C
  2020-12-27  0:04 ` [PATCH v2] bootmenu: Allow to quit it via ESC/CTRL+C Pali Rohár
@ 2020-12-29  3:32   ` Simon Glass
  2021-01-18 13:01   ` Tom Rini
  1 sibling, 0 replies; 17+ messages in thread
From: Simon Glass @ 2020-12-29  3:32 UTC (permalink / raw)
  To: u-boot

On Sat, 26 Dec 2020 at 17:05, Pali Roh?r <pali@kernel.org> wrote:
>
> When ESC/CTRL+C is pressed interrupt bootmenu and jump into U-Boot console.
> As the last entry in bootmenu is always U-Boot console just choose the last
> entry when ESC or CTRL+C is pressed.
>
> ESC key is detected when either no other character appears after '\e'
> within 10ms or when non-'[' appears after '\e'.
>
> It is useful when bootmenu is part of boot process and you want to
> interrupt boot process by scripts which control U-Boot (serial) console.
>
> Signed-off-by: Pali Roh?r <pali@kernel.org>
>
> ---
> Changes in v2:
> * Added support also for ESC key
> ---
>  cmd/bootmenu.c | 42 +++++++++++++++++++++++++++++++++++-------
>  1 file changed, 35 insertions(+), 7 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2] bootmenu: Allow to quit it via ESC/CTRL+C
  2020-12-27  0:04 ` [PATCH v2] bootmenu: Allow to quit it via ESC/CTRL+C Pali Rohár
  2020-12-29  3:32   ` Simon Glass
@ 2021-01-18 13:01   ` Tom Rini
  1 sibling, 0 replies; 17+ messages in thread
From: Tom Rini @ 2021-01-18 13:01 UTC (permalink / raw)
  To: u-boot

On Sun, Dec 27, 2020 at 01:04:38AM +0100, Pali Roh?r wrote:

> When ESC/CTRL+C is pressed interrupt bootmenu and jump into U-Boot console.
> As the last entry in bootmenu is always U-Boot console just choose the last
> entry when ESC or CTRL+C is pressed.
> 
> ESC key is detected when either no other character appears after '\e'
> within 10ms or when non-'[' appears after '\e'.
> 
> It is useful when bootmenu is part of boot process and you want to
> interrupt boot process by scripts which control U-Boot (serial) console.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210118/86ccf084/attachment.sig>

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-01-18 13:01 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-26 18:02 [PATCH] bootmenu: Allow to quit it via CTRL+C Pali Rohár
2020-12-26 18:44 ` Heinrich Schuchardt
2020-12-26 19:03   ` Pali Rohár
2020-12-26 19:10     ` Heinrich Schuchardt
2020-12-26 19:15       ` Pali Rohár
2020-12-26 19:44         ` Heinrich Schuchardt
2020-12-26 19:50           ` Pali Rohár
2020-12-26 22:15             ` Heinrich Schuchardt
2020-12-26 22:35               ` Pali Rohár
2020-12-26 22:32         ` [maemo-leste] " Pavel Machek
2020-12-26 22:42           ` Pali Rohár
2020-12-26 22:48             ` Pavel Machek
2020-12-26 23:04             ` Heinrich Schuchardt
2020-12-26 23:16               ` Pali Rohár
2020-12-27  0:04 ` [PATCH v2] bootmenu: Allow to quit it via ESC/CTRL+C Pali Rohár
2020-12-29  3:32   ` Simon Glass
2021-01-18 13:01   ` Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.