[PATCH 00/24] bootstd: Add a boot menu

[PATCH 00/24] bootstd: Add a boot menu

[Simon Glass]

So far standard boot lacks a boot menu, although it is possible to create
a rudimentary one using the existing 'bootmenu' command.

Even then, this text-based menu offer only basic functionality and does
not take full advantage of the displays which are common on many devices.

This series provides a 'bootflow menu' command which allows the user to
select from the available bootflows. An attempt is made to show the name
of the available operating systems, by reading more information into the
bootflow. A logo can be read also, where supported, so that this can be
presented to the user when an option is highlighted.

Full use is made of TrueType fonts, if enabled. For cases where only a
serial console is available, it falls back to a simple text-based menu.

All of this is implementing using a new 'expo' construct, a collection of
scenes (like menu screens) which can be navigated by the user to view
information and select option. This is fairly general and should be able
to cope with a wider array of use cases, with less hacking of the menu
code, such as is currently needed for CMD_BOOTEFI_BOOTMGR.

Of course it would be possible to enhance the existing menu rather than
creating a new setup. Instead it seems better to make the existing menu
use expo, if code space permits. It avoids the event-loop problem and
should be more extensible, given its loosely coupled components and use of
IDs instead of pointers. Further motivation is provided in the
documentation.

For now the CLI keypress-decoding code is split out to be used by the new
menu. The key codes defined by menu.h are reused also.

This is of course just a starting point. Some ideas for future work are
included in the documentation.


Simon Glass (24):
  sandbox: Enable mmc command and legacy images
  cli: Move readline character-processing to a state machine
  bootmenu: Add a few comments
  menu: Rename KEY_... to BKEY_...
  menu: Update bootmenu_autoboot_loop() to return the code
  menu: Update bootmenu_loop() to return the code
  menu: Use a switch statement
  menu: Make use of CLI character processing
  image: Add a function to find a script in an image
  video: Enable VIDEO_ANSI by default only with EFI
  video: truetype: Rename the metrics function
  video: Fix unchnaged typo
  video: Add font functions to the vidconsole API
  bootstd: Read the Operating System name for distro/scripts
  bootstd: Allow reading a logo for the OS
  menu: Factor out menu-keypress decoding
  expo: Add basic implementation
  expo: Add support for scenes
  expo: Add support for scene menus
  expo: Add basic tests
  bootstd: Support creating a boot menu
  bootstd: Add a test for the bootstd menu
  bootstd: Support setting a theme for the menu
  expo: Add documentation

 arch/sandbox/dts/test.dts              |  11 +
 boot/Kconfig                           |  12 +
 boot/Makefile                          |   3 +
 boot/bootflow.c                        |   1 +
 boot/bootflow_internal.h               |  37 ++
 boot/bootflow_menu.c                   | 281 ++++++++++++++
 boot/bootmeth-uclass.c                 |  69 +++-
 boot/bootmeth_distro.c                 |  36 ++
 boot/bootmeth_script.c                 |  38 ++
 boot/bootstd-uclass.c                  |   2 +
 boot/expo.c                            | 133 +++++++
 boot/scene.c                           | 385 +++++++++++++++++++
 boot/scene_internal.h                  | 123 ++++++
 boot/scene_menu.c                      | 372 ++++++++++++++++++
 cmd/bootflow.c                         |  44 ++-
 cmd/bootmenu.c                         |  19 +-
 cmd/eficonfig.c                        |  38 +-
 cmd/font.c                             |   6 +-
 cmd/source.c                           | 173 +++++----
 common/Makefile                        |   6 +-
 common/cli_getch.c                     | 208 ++++++++++
 common/cli_readline.c                  | 150 ++------
 common/menu.c                          | 157 ++++----
 configs/sandbox_defconfig              |   2 +
 configs/sandbox_flattree_defconfig     |   2 +
 configs/snow_defconfig                 |   4 +
 configs/tools-only_defconfig           |   2 +
 doc/develop/expo.rst                   | 184 +++++++++
 doc/develop/index.rst                  |   1 +
 drivers/video/Kconfig                  |   7 +-
 drivers/video/console_truetype.c       |  35 +-
 drivers/video/vidconsole-uclass.c      |  33 ++
 include/bootflow.h                     |  40 ++
 include/bootmeth.h                     |  16 +
 include/bootstd.h                      |   4 +
 include/cli.h                          |  74 ++++
 include/expo.h                         | 474 +++++++++++++++++++++++
 include/image.h                        |  13 +
 include/menu.h                         |  77 +++-
 include/video.h                        |   2 +-
 include/video_console.h                |  63 ++-
 test/boot/Makefile                     |   2 +
 test/boot/bootflow.c                   | 130 +++++++
 test/boot/expo.c                       | 510 +++++++++++++++++++++++++
 test/py/tests/bootstd/armbian.bmp.xz   | Bin 0 -> 1384 bytes
 test/py/tests/bootstd/mmc4.img.xz      | Bin 0 -> 7072 bytes
 test/py/tests/test_android/test_avb.py |   2 +
 test/py/tests/test_ut.py               | 216 ++++++++++-
 48 files changed, 3813 insertions(+), 384 deletions(-)
 create mode 100644 boot/bootflow_internal.h
 create mode 100644 boot/bootflow_menu.c
 create mode 100644 boot/expo.c
 create mode 100644 boot/scene.c
 create mode 100644 boot/scene_internal.h
 create mode 100644 boot/scene_menu.c
 create mode 100644 common/cli_getch.c
 create mode 100644 doc/develop/expo.rst
 create mode 100644 include/expo.h
 create mode 100644 test/boot/expo.c
 create mode 100644 test/py/tests/bootstd/armbian.bmp.xz
 create mode 100644 test/py/tests/bootstd/mmc4.img.xz

-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 01/24] sandbox: Enable mmc command and legacy images

[Simon Glass]

The mmc command is useful for testing mmc disk images in sandbox, so
enable it. We also need to enable legacy images so that we can run tests
which use them.

Disable it for a few avb tests since MMC is not implemented there yet.

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

 configs/sandbox_defconfig              | 2 ++
 configs/sandbox_flattree_defconfig     | 2 ++
 test/py/tests/test_android/test_avb.py | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index a192a0762b3..7e5126d79b8 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -13,6 +13,7 @@ CONFIG_FIT=y
 CONFIG_FIT_RSASSA_PSS=y
 CONFIG_FIT_CIPHER=y
 CONFIG_FIT_VERBOSE=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
 CONFIG_BOOTSTAGE=y
 CONFIG_BOOTSTAGE_REPORT=y
 CONFIG_BOOTSTAGE_FDT=y
@@ -71,6 +72,7 @@ CONFIG_CMD_IDE=y
 CONFIG_CMD_I2C=y
 CONFIG_CMD_LOADM=y
 CONFIG_CMD_LSBLK=y
+CONFIG_CMD_MMC=y
 CONFIG_CMD_MUX=y
 CONFIG_CMD_OSD=y
 CONFIG_CMD_PCI=y
diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig
index 38535e83d1c..e4ad072a230 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -11,6 +11,7 @@ CONFIG_DISTRO_DEFAULTS=y
 CONFIG_FIT=y
 CONFIG_FIT_SIGNATURE=y
 CONFIG_FIT_VERBOSE=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
 CONFIG_BOOTSTAGE=y
 CONFIG_BOOTSTAGE_REPORT=y
 CONFIG_BOOTSTAGE_FDT=y
@@ -40,6 +41,7 @@ CONFIG_CMD_DEMO=y
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_GPT=y
 CONFIG_CMD_I2C=y
+CONFIG_CMD_MMC=y
 CONFIG_CMD_OSD=y
 CONFIG_CMD_PCI=y
 CONFIG_CMD_REMOTEPROC=y
diff --git a/test/py/tests/test_android/test_avb.py b/test/py/tests/test_android/test_avb.py
index a3f883136b0..bc5c5b55821 100644
--- a/test/py/tests/test_android/test_avb.py
+++ b/test/py/tests/test_android/test_avb.py
@@ -39,6 +39,7 @@ def test_avb_verify(u_boot_console):
 
 @pytest.mark.buildconfigspec('cmd_avb')
 @pytest.mark.buildconfigspec('cmd_mmc')
+@pytest.mark.notbuildconfigspec('sandbox')
 def test_avb_mmc_uuid(u_boot_console):
     """Check if 'avb get_uuid' works, compare results with
     'part list mmc 1' output
@@ -97,6 +98,7 @@ def test_avb_is_unlocked(u_boot_console):
 
 @pytest.mark.buildconfigspec('cmd_avb')
 @pytest.mark.buildconfigspec('cmd_mmc')
+@pytest.mark.notbuildconfigspec('sandbox')
 def test_avb_mmc_read(u_boot_console):
     """Test mmc read operation
     """
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 02/24] cli: Move readline character-processing to a state machine

[Simon Glass]

The current cread_line() function is very long. It handles the escape
processing inline. The menu command does similar processing but at the
character level, so there is some duplication.

Split the character processing into a new function cli_ch_process() which
processes individual characters and returns the resulting input character,
taking account of escape sequences. It requires the caller to set up and
maintain its state.

Update cread_line() to use this new function.

The only intended functional change is that an invalid escape sequence
does not add invalid/control characters into the input buffer, but instead
discards these.

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

 common/Makefile       |   6 +-
 common/cli_getch.c    | 204 ++++++++++++++++++++++++++++++++++++++++++
 common/cli_readline.c | 150 +++++--------------------------
 include/cli.h         |  72 +++++++++++++++
 4 files changed, 301 insertions(+), 131 deletions(-)
 create mode 100644 common/cli_getch.c

diff --git a/common/Makefile b/common/Makefile
index 1d56c9f2895..7cd76360321 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -42,7 +42,7 @@ obj-$(CONFIG_LCD_ROTATION) += lcd_console_rotation.o
 obj-$(CONFIG_MENU) += menu.o
 obj-$(CONFIG_UPDATE_COMMON) += update.o
 obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o
-obj-$(CONFIG_CMDLINE) += cli_readline.o cli_simple.o
+obj-$(CONFIG_CMDLINE) += cli_getch.o cli_readline.o cli_simple.o
 
 endif # !CONFIG_SPL_BUILD
 
@@ -97,8 +97,8 @@ obj-y += eeprom/eeprom_field.o eeprom/eeprom_layout.o
 endif
 
 obj-y += cli.o
-obj-$(CONFIG_FSL_DDR_INTERACTIVE) += cli_simple.o cli_readline.o
-obj-$(CONFIG_STM32MP1_DDR_INTERACTIVE) += cli_simple.o cli_readline.o
+obj-$(CONFIG_FSL_DDR_INTERACTIVE) += cli_getch.o cli_simple.o cli_readline.o
+obj-$(CONFIG_STM32MP1_DDR_INTERACTIVE) += cli_getch.o cli_simple.o cli_readline.o
 obj-$(CONFIG_DFU_OVER_USB) += dfu.o
 obj-y += command.o
 obj-$(CONFIG_$(SPL_TPL_)LOG) += log.o
diff --git a/common/cli_getch.c b/common/cli_getch.c
new file mode 100644
index 00000000000..9eeea7fef29
--- /dev/null
+++ b/common/cli_getch.c
@@ -0,0 +1,204 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2000
+ * Wolfgang Denk, DENX Software Engineering, wd@denx.de.
+ *
+ * Copyright 2022 Google LLC
+ */
+
+#include <common.h>
+#include <cli.h>
+
+/**
+ * enum cli_esc_state_t - indicates what to do with an escape character
+ *
+ * @ESC_REJECT: Invalid escape sequence, so the esc_save[] characters are
+ *	returned from each subsequent call to cli_ch_esc()
+ * @ESC_SAVE: Character should be saved in esc_save until we have another one
+ * @ESC_CONVERTED: Escape sequence has been completed and the resulting
+ *	character is available
+ */
+enum cli_esc_state_t {
+	ESC_REJECT,
+	ESC_SAVE,
+	ESC_CONVERTED
+};
+
+void cli_ch_init(struct cli_ch_state *cch)
+{
+	memset(cch, '\0', sizeof(*cch));
+}
+
+/**
+ * cli_ch_esc() - Process a character in an ongoing escape sequence
+ *
+ * @cch: State information
+ * @ichar: Character to process
+ * @actp: Returns the action to take
+ * Returns: Output character if *actp is ESC_CONVERTED, else 0
+ */
+static int cli_ch_esc(struct cli_ch_state *cch, int ichar,
+		      enum cli_esc_state_t *actp)
+{
+	enum cli_esc_state_t act = ESC_REJECT;
+
+	switch (cch->esc_len) {
+	case 1:
+		if (ichar == '[' || ichar == 'O')
+			act = ESC_SAVE;
+		break;
+	case 2:
+		switch (ichar) {
+		case 'D':	/* <- key */
+			ichar = CTL_CH('b');
+			act = ESC_CONVERTED;
+			break;	/* pass off to ^B handler */
+		case 'C':	/* -> key */
+			ichar = CTL_CH('f');
+			act = ESC_CONVERTED;
+			break;	/* pass off to ^F handler */
+		case 'H':	/* Home key */
+			ichar = CTL_CH('a');
+			act = ESC_CONVERTED;
+			break;	/* pass off to ^A handler */
+		case 'F':	/* End key */
+			ichar = CTL_CH('e');
+			act = ESC_CONVERTED;
+			break;	/* pass off to ^E handler */
+		case 'A':	/* up arrow */
+			ichar = CTL_CH('p');
+			act = ESC_CONVERTED;
+			break;	/* pass off to ^P handler */
+		case 'B':	/* down arrow */
+			ichar = CTL_CH('n');
+			act = ESC_CONVERTED;
+			break;	/* pass off to ^N handler */
+		case '1':
+		case '2':
+		case '3':
+		case '4':
+		case '7':
+		case '8':
+			if (cch->esc_save[1] == '[') {
+				/* see if next character is ~ */
+				act = ESC_SAVE;
+			}
+			break;
+		}
+		break;
+	case 3:
+		switch (ichar) {
+		case '~':
+			switch (cch->esc_save[2]) {
+			case '3':	/* Delete key */
+				ichar = CTL_CH('d');
+				act = ESC_CONVERTED;
+				break;	/* pass to ^D handler */
+			case '1':	/* Home key */
+			case '7':
+				ichar = CTL_CH('a');
+				act = ESC_CONVERTED;
+				break;	/* pass to ^A handler */
+			case '4':	/* End key */
+			case '8':
+				ichar = CTL_CH('e');
+				act = ESC_CONVERTED;
+				break;	/* pass to ^E handler */
+			}
+			break;
+		case '0':
+			if (cch->esc_save[2] == '2')
+				act = ESC_SAVE;
+			break;
+		}
+		break;
+	case 4:
+		switch (ichar) {
+		case '0':
+		case '1':
+			act = ESC_SAVE;
+			break;		/* bracketed paste */
+		}
+		break;
+	case 5:
+		if (ichar == '~') {	/* bracketed paste */
+			ichar = 0;
+			act = ESC_CONVERTED;
+		}
+	}
+
+	*actp = act;
+
+	return act == ESC_CONVERTED ? ichar : 0;
+}
+
+int cli_ch_process(struct cli_ch_state *cch, int ichar)
+{
+	/*
+	 * ichar=0x0 when error occurs in U-Boot getchar() or when the caller
+	 * wants to check if there are more characters saved in the escape
+	 * sequence
+	 */
+	if (!ichar) {
+		if (cch->emit_upto) {
+			if (cch->emit_upto < cch->esc_len)
+				return cch->esc_save[cch->emit_upto++];
+			cch->emit_upto = 0;
+		}
+		return 0;
+	} else if (ichar == -ETIMEDOUT) {
+		/*
+		 * If we are in an escape sequence but nothing has followed the
+		 * Escape character, then the user probably just pressed the
+		 * Escape key. Return it and clear the sequence.
+		 */
+		if (cch->esc_len) {
+			cch->esc_len = 0;
+			return '\e';
+		}
+
+		/* Otherwise there is nothing to return */
+		return 0;
+	}
+
+	if (ichar == '\n' || ichar == '\r')
+		return '\n';
+
+	/* handle standard linux xterm esc sequences for arrow key, etc. */
+	if (cch->esc_len != 0) {
+		enum cli_esc_state_t act;
+
+		ichar = cli_ch_esc(cch, ichar, &act);
+
+		switch (act) {
+		case ESC_SAVE:
+			/* save this character and return nothing */
+			cch->esc_save[cch->esc_len++] = ichar;
+			return 0;
+		case ESC_REJECT:
+			/*
+			 * invalid escape sequence, start returning the
+			 * characters in it
+			 */
+			cch->esc_save[cch->esc_len++] = ichar;
+			return cch->esc_save[cch->emit_upto++];
+		case ESC_CONVERTED:
+			/* valid escape sequence, return the resulting char */
+			cch->esc_len = 0;
+			return ichar;
+		}
+	}
+
+	if (ichar == '\e') {
+		if (!cch->esc_len) {
+			cch->esc_save[cch->esc_len] = ichar;
+			cch->esc_len = 1;
+		} else {
+			puts("impossible condition #876\n");
+			cch->esc_len = 0;
+		}
+		return 0;
+	}
+
+	return ichar;
+}
diff --git a/common/cli_readline.c b/common/cli_readline.c
index e86ee73faf7..973ddb06275 100644
--- a/common/cli_readline.c
+++ b/common/cli_readline.c
@@ -62,7 +62,6 @@ static char *delete_char (char *buffer, char *p, int *colp, int *np, int plen)
 
 #define putnstr(str, n)	printf("%.*s", (int)n, str)
 
-#define CTL_CH(c)		((c) - 'a' + 1)
 #define CTL_BACKSPACE		('\b')
 #define DEL			((char)255)
 #define DEL7			((char)127)
@@ -247,156 +246,53 @@ static void cread_add_str(char *str, int strsize, int insert,
 static int cread_line(const char *const prompt, char *buf, unsigned int *len,
 		int timeout)
 {
+	struct cli_ch_state s_cch, *cch = &s_cch;
 	unsigned long num = 0;
 	unsigned long eol_num = 0;
 	unsigned long wlen;
 	char ichar;
 	int insert = 1;
-	int esc_len = 0;
-	char esc_save[8];
 	int init_len = strlen(buf);
 	int first = 1;
 
+	cli_ch_init(cch);
+
 	if (init_len)
 		cread_add_str(buf, init_len, 1, &num, &eol_num, buf, *len);
 
 	while (1) {
-		if (bootretry_tstc_timeout())
-			return -2;	/* timed out */
-		if (first && timeout) {
-			uint64_t etime = endtick(timeout);
-
-			while (!tstc()) {	/* while no incoming data */
-				if (get_ticks() >= etime)
-					return -2;	/* timed out */
-				WATCHDOG_RESET();
+		/* Check for saved characters */
+		ichar = cli_ch_process(cch, 0);
+
+		if (!ichar) {
+			if (bootretry_tstc_timeout())
+				return -2;	/* timed out */
+			if (first && timeout) {
+				u64 etime = endtick(timeout);
+
+				while (!tstc()) {	/* while no incoming data */
+					if (get_ticks() >= etime)
+						return -2;	/* timed out */
+					WATCHDOG_RESET();
+				}
+				first = 0;
 			}
-			first = 0;
+
+			ichar = getcmd_getch();
 		}
 
-		ichar = getcmd_getch();
+		ichar = cli_ch_process(cch, ichar);
 
 		/* ichar=0x0 when error occurs in U-Boot getc */
 		if (!ichar)
 			continue;
 
-		if ((ichar == '\n') || (ichar == '\r')) {
+		if (ichar == '\n') {
 			putc('\n');
 			break;
 		}
 
-		/*
-		 * handle standard linux xterm esc sequences for arrow key, etc.
-		 */
-		if (esc_len != 0) {
-			enum { ESC_REJECT, ESC_SAVE, ESC_CONVERTED } act = ESC_REJECT;
-
-			if (esc_len == 1) {
-				if (ichar == '[' || ichar == 'O')
-					act = ESC_SAVE;
-			} else if (esc_len == 2) {
-				switch (ichar) {
-				case 'D':	/* <- key */
-					ichar = CTL_CH('b');
-					act = ESC_CONVERTED;
-					break;	/* pass off to ^B handler */
-				case 'C':	/* -> key */
-					ichar = CTL_CH('f');
-					act = ESC_CONVERTED;
-					break;	/* pass off to ^F handler */
-				case 'H':	/* Home key */
-					ichar = CTL_CH('a');
-					act = ESC_CONVERTED;
-					break;	/* pass off to ^A handler */
-				case 'F':	/* End key */
-					ichar = CTL_CH('e');
-					act = ESC_CONVERTED;
-					break;	/* pass off to ^E handler */
-				case 'A':	/* up arrow */
-					ichar = CTL_CH('p');
-					act = ESC_CONVERTED;
-					break;	/* pass off to ^P handler */
-				case 'B':	/* down arrow */
-					ichar = CTL_CH('n');
-					act = ESC_CONVERTED;
-					break;	/* pass off to ^N handler */
-				case '1':
-				case '2':
-				case '3':
-				case '4':
-				case '7':
-				case '8':
-					if (esc_save[1] == '[') {
-						/* see if next character is ~ */
-						act = ESC_SAVE;
-					}
-					break;
-				}
-			} else if (esc_len == 3) {
-				switch (ichar) {
-				case '~':
-					switch (esc_save[2]) {
-					case '3':	/* Delete key */
-						ichar = CTL_CH('d');
-						act = ESC_CONVERTED;
-						break;	/* pass to ^D handler */
-					case '1':	/* Home key */
-					case '7':
-						ichar = CTL_CH('a');
-						act = ESC_CONVERTED;
-						break;	/* pass to ^A handler */
-					case '4':	/* End key */
-					case '8':
-						ichar = CTL_CH('e');
-						act = ESC_CONVERTED;
-						break;	/* pass to ^E handler */
-					}
-					break;
-				case '0':
-					if (esc_save[2] == '2')
-						act = ESC_SAVE;
-					break;
-				}
-			} else if (esc_len == 4) {
-				switch (ichar) {
-				case '0':
-				case '1':
-					act = ESC_SAVE;
-					break;		/* bracketed paste */
-				}
-			} else if (esc_len == 5) {
-				if (ichar == '~') {	/* bracketed paste */
-					ichar = 0;
-					act = ESC_CONVERTED;
-				}
-			}
-			switch (act) {
-			case ESC_SAVE:
-				esc_save[esc_len++] = ichar;
-				continue;
-			case ESC_REJECT:
-				esc_save[esc_len++] = ichar;
-				cread_add_str(esc_save, esc_len, insert,
-					      &num, &eol_num, buf, *len);
-				esc_len = 0;
-				continue;
-			case ESC_CONVERTED:
-				esc_len = 0;
-				break;
-			}
-		}
-
 		switch (ichar) {
-		case 0x1b:
-			if (esc_len == 0) {
-				esc_save[esc_len] = ichar;
-				esc_len = 1;
-			} else {
-				puts("impossible condition #876\n");
-				esc_len = 0;
-			}
-			break;
-
 		case CTL_CH('a'):
 			BEGINNING_OF_LINE();
 			break;
@@ -465,8 +361,6 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len,
 		{
 			char *hline;
 
-			esc_len = 0;
-
 			if (ichar == CTL_CH('p'))
 				hline = hist_prev();
 			else
diff --git a/include/cli.h b/include/cli.h
index ba5b8ebd36e..863519e4b13 100644
--- a/include/cli.h
+++ b/include/cli.h
@@ -7,6 +7,21 @@
 #ifndef __CLI_H
 #define __CLI_H
 
+#include <stdbool.h>
+
+/**
+ * struct cli_ch_state - state information for reading cmdline characters
+ *
+ * @esc_len: Number of escape characters read so far
+ * @esc_save: Escape characters collected so far
+ * @emit_upto: Next character to emit from esc_save (0 if not emitting)
+ */
+struct cli_ch_state {
+	int esc_len;
+	char esc_save[8];
+	int emit_upto;
+};
+
 /**
  * Go into the command loop
  *
@@ -154,5 +169,62 @@ void cli_loop(void);
 void cli_init(void);
 
 #define endtick(seconds) (get_ticks() + (uint64_t)(seconds) * get_tbclk())
+#define CTL_CH(c)		((c) - 'a' + 1)
+
+/**
+ * cli_ch_init() - Set up the initial state to process input characters
+ *
+ * @cch: State to set up
+ */
+void cli_ch_init(struct cli_ch_state *cch);
+
+/**
+ * cli_ch_process() - Process an input character
+ *
+ * When @ichar is 0, this function returns any characters from an invalid escape
+ * sequence which are still pending in the buffer
+ *
+ * Otherwise it processes the input character. If it is an escape character,
+ * then an escape sequence is started and the function returns 0. If we are in
+ * the middle of an escape sequence, the character is processed and may result
+ * in returning 0 (if more characters are needed) or a valid character (if
+ * @ichar finishes the sequence).
+ *
+ * If @ichar is a valid character and there is no escape sequence in progress,
+ * then it is returned as is.
+ *
+ * If the Enter key is pressed, '\n' is returned.
+ *
+ * Usage should be like this::
+ *
+ *    struct cli_ch_state cch;
+ *
+ *    cli_ch_init(cch);
+ *    do
+ *       {
+ *       int ichar, ch;
+ *
+ *       ichar = cli_ch_process(cch, 0);
+ *       if (!ichar) {
+ *          ch = getchar();
+ *          ichar = cli_ch_process(cch, ch);
+ *       }
+ *       (handle the ichar character)
+ *    } while (!done)
+ *
+ * If tstc() is used to look for keypresses, this function can be called with
+ * @ichar set to -ETIMEDOUT if there is no character after 5-10ms. This allows
+ * the ambgiuity between the Escape key and the arrow keys (which generate an
+ * escape character followed by other characters) to be resolved.
+ *
+ * @cch: Current state
+ * @ichar: Input character to process, or 0 if none, or -ETIMEDOUT if no
+ * character has been received within a small number of milliseconds (this
+ * cancels any existing escape sequence and allows pressing the Escape key to
+ * work)
+ * Returns: Resulting input character after processing, 0 if none, '\e' if
+ * an existing escape sequence was cancelled
+ */
+int cli_ch_process(struct cli_ch_state *cch, int ichar);
 
 #endif
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 03/24] bootmenu: Add a few comments

[Simon Glass]

The behaviour of these two functions is completely undocumented. Add some
notes so the poor, suffering dev can figure out what is going on.

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

 include/menu.h | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/include/menu.h b/include/menu.h
index 702aacb170c..0b4d9734149 100644
--- a/include/menu.h
+++ b/include/menu.h
@@ -42,6 +42,7 @@ struct bootmenu_data {
 	struct bootmenu_entry *first;	/* first menu entry */
 };
 
+/** enum bootmenu_key - keys that can be returned by the bootmenu */
 enum bootmenu_key {
 	KEY_NONE = 0,
 	KEY_UP,
@@ -53,8 +54,49 @@ enum bootmenu_key {
 	KEY_SPACE,
 };
 
+/**
+ * bootmenu_autoboot_loop() - handle autobooting if no key is pressed
+ *
+ * This shows a prompt to allow the user to press a key to interrupt auto boot
+ * of the first menu option.
+ *
+ * It then waits for the required time (menu->delay in seconds) for a key to be
+ * pressed. If nothing is pressed in that time, @key returns KEY_SELECT
+ * indicating that the current option should be chosen.
+ *
+ * @menu: Menu being processed
+ * @key: Returns the code for the key the user pressed:
+ *	enter: KEY_SELECT
+ *	Ctrl-C: KEY_QUIT
+ *	anything else: KEY_NONE
+ * @esc: Set to 1 if the escape key is pressed, otherwise not updated
+ */
 void bootmenu_autoboot_loop(struct bootmenu_data *menu,
 			    enum bootmenu_key *key, int *esc);
+
+/**
+ * bootmenu_loop() - handle waiting for a keypress when autoboot is disabled
+ *
+ * This is used when the menu delay is negative, indicating that the delay has
+ * elapsed, or there was no delay to begin with.
+ *
+ * It reads a character and processes it, returning a menu-key code if a
+ * character is recognised
+ *
+ * @menu: Menu being processed
+ * @key: Returns the code for the key the user pressed:
+ *	enter: KEY_SELECT
+ *	Ctrl-C: KEY_QUIT
+ *	Up arrow: KEY_UP
+ *	Down arrow: KEY_DOWN
+ *	Escape (by itself): KEY_QUIT
+ *	Plus: KEY_PLUS
+ *	Minus: KEY_MINUS
+ *	Space: KEY_SPACE
+ * @esc: On input, a non-zero value indicates that an escape sequence has
+ *	resulted in that many characters so far. On exit this is updated to the
+ *	new number of characters
+ */
 void bootmenu_loop(struct bootmenu_data *menu,
 		   enum bootmenu_key *key, int *esc);
 
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 04/24] menu: Rename KEY_... to BKEY_...

[Simon Glass]

This enum values conflict with linux/input.h so rename them.

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

 cmd/bootmenu.c  | 10 +++++-----
 cmd/eficonfig.c | 26 +++++++++++++-------------
 common/menu.c   | 34 +++++++++++++++++-----------------
 include/menu.h  | 32 ++++++++++++++++----------------
 4 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
index 3340be16325..c80004c54dc 100644
--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -86,7 +86,7 @@ static char *bootmenu_choice_entry(void *data)
 {
 	struct bootmenu_data *menu = data;
 	struct bootmenu_entry *iter;
-	enum bootmenu_key key = KEY_NONE;
+	enum bootmenu_key key = BKEY_NONE;
 	int esc = 0;
 	int i;
 
@@ -100,22 +100,22 @@ static char *bootmenu_choice_entry(void *data)
 		}
 
 		switch (key) {
-		case KEY_UP:
+		case BKEY_UP:
 			if (menu->active > 0)
 				--menu->active;
 			/* no menu key selected, regenerate menu */
 			return NULL;
-		case KEY_DOWN:
+		case BKEY_DOWN:
 			if (menu->active < menu->count - 1)
 				++menu->active;
 			/* no menu key selected, regenerate menu */
 			return NULL;
-		case KEY_SELECT:
+		case BKEY_SELECT:
 			iter = menu->first;
 			for (i = 0; i < menu->active; ++i)
 				iter = iter->next;
 			return iter->key;
-		case KEY_QUIT:
+		case BKEY_QUIT:
 			/* Quit by choosing the last entry - U-Boot console */
 			iter = menu->first;
 			while (iter->next)
diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 2595dd95631..de7ce24f5ac 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -193,31 +193,31 @@ static char *eficonfig_choice_entry(void *data)
 	int esc = 0;
 	struct list_head *pos, *n;
 	struct eficonfig_entry *entry;
-	enum bootmenu_key key = KEY_NONE;
+	enum bootmenu_key key = BKEY_NONE;
 	struct efimenu *efi_menu = data;
 
 	while (1) {
 		bootmenu_loop((struct bootmenu_data *)efi_menu, &key, &esc);
 
 		switch (key) {
-		case KEY_UP:
+		case BKEY_UP:
 			if (efi_menu->active > 0)
 				--efi_menu->active;
 			/* no menu key selected, regenerate menu */
 			return NULL;
-		case KEY_DOWN:
+		case BKEY_DOWN:
 			if (efi_menu->active < efi_menu->count - 1)
 				++efi_menu->active;
 			/* no menu key selected, regenerate menu */
 			return NULL;
-		case KEY_SELECT:
+		case BKEY_SELECT:
 			list_for_each_safe(pos, n, &efi_menu->list) {
 				entry = list_entry(pos, struct eficonfig_entry, list);
 				if (entry->num == efi_menu->active)
 					return entry->key;
 			}
 			break;
-		case KEY_QUIT:
+		case BKEY_QUIT:
 			/* Quit by choosing the last entry */
 			entry = list_last_entry(&efi_menu->list, struct eficonfig_entry, list);
 			return entry->key;
@@ -1864,14 +1864,14 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
 	int esc = 0;
 	struct list_head *pos, *n;
 	struct eficonfig_boot_order *tmp;
-	enum bootmenu_key key = KEY_NONE;
+	enum bootmenu_key key = BKEY_NONE;
 	struct eficonfig_boot_order *entry;
 
 	while (1) {
 		bootmenu_loop(NULL, &key, &esc);
 
 		switch (key) {
-		case KEY_PLUS:
+		case BKEY_PLUS:
 			if (efi_menu->active > 0) {
 				list_for_each_safe(pos, n, &efi_menu->list) {
 					entry = list_entry(pos, struct eficonfig_boot_order, list);
@@ -1885,11 +1885,11 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
 				list_add(&tmp->list, &entry->list);
 			}
 			fallthrough;
-		case KEY_UP:
+		case BKEY_UP:
 			if (efi_menu->active > 0)
 				--efi_menu->active;
 			return EFI_NOT_READY;
-		case KEY_MINUS:
+		case BKEY_MINUS:
 			if (efi_menu->active < efi_menu->count - 3) {
 				list_for_each_safe(pos, n, &efi_menu->list) {
 					entry = list_entry(pos, struct eficonfig_boot_order, list);
@@ -1905,11 +1905,11 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
 				++efi_menu->active;
 			}
 			return EFI_NOT_READY;
-		case KEY_DOWN:
+		case BKEY_DOWN:
 			if (efi_menu->active < efi_menu->count - 1)
 				++efi_menu->active;
 			return EFI_NOT_READY;
-		case KEY_SELECT:
+		case BKEY_SELECT:
 			/* "Save" */
 			if (efi_menu->active == efi_menu->count - 2)
 				return EFI_SUCCESS;
@@ -1919,7 +1919,7 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
 				return EFI_ABORTED;
 
 			break;
-		case KEY_SPACE:
+		case BKEY_SPACE:
 			if (efi_menu->active < efi_menu->count - 2) {
 				list_for_each_safe(pos, n, &efi_menu->list) {
 					entry = list_entry(pos, struct eficonfig_boot_order, list);
@@ -1930,7 +1930,7 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
 				}
 			}
 			break;
-		case KEY_QUIT:
+		case BKEY_QUIT:
 			return EFI_ABORTED;
 		default:
 			/* Pressed key is not valid, no need to regenerate the menu */
diff --git a/common/menu.c b/common/menu.c
index 0d19601cf53..087e4c246e2 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -446,16 +446,16 @@ void bootmenu_autoboot_loop(struct bootmenu_data *menu,
 			switch (c) {
 			case '\e':
 				*esc = 1;
-				*key = KEY_NONE;
+				*key = BKEY_NONE;
 				break;
 			case '\r':
-				*key = KEY_SELECT;
+				*key = BKEY_SELECT;
 				break;
 			case 0x3: /* ^C */
-				*key = KEY_QUIT;
+				*key = BKEY_QUIT;
 				break;
 			default:
-				*key = KEY_NONE;
+				*key = BKEY_NONE;
 				break;
 			}
 
@@ -471,7 +471,7 @@ void bootmenu_autoboot_loop(struct bootmenu_data *menu,
 	printf(ANSI_CURSOR_POSITION ANSI_CLEAR_LINE, menu->count + 5, 1);
 
 	if (menu->delay == 0)
-		*key = KEY_SELECT;
+		*key = BKEY_SELECT;
 }
 
 void bootmenu_loop(struct bootmenu_data *menu,
@@ -503,17 +503,17 @@ void bootmenu_loop(struct bootmenu_data *menu,
 		/* First char of ANSI escape sequence '\e' */
 		if (c == '\e') {
 			*esc = 1;
-			*key = KEY_NONE;
+			*key = BKEY_NONE;
 		}
 		break;
 	case 1:
 		/* Second char of ANSI '[' */
 		if (c == '[') {
 			*esc = 2;
-			*key = KEY_NONE;
+			*key = BKEY_NONE;
 		} else {
 		/* Alone ESC key was pressed */
-			*key = KEY_QUIT;
+			*key = BKEY_QUIT;
 			*esc = (c == '\e') ? 1 : 0;
 		}
 		break;
@@ -522,7 +522,7 @@ void bootmenu_loop(struct bootmenu_data *menu,
 		/* Third char of ANSI (number '1') - optional */
 		if (*esc == 2 && c == '1') {
 			*esc = 3;
-			*key = KEY_NONE;
+			*key = BKEY_NONE;
 			break;
 		}
 
@@ -530,31 +530,31 @@ void bootmenu_loop(struct bootmenu_data *menu,
 
 		/* ANSI 'A' - key up was pressed */
 		if (c == 'A')
-			*key = KEY_UP;
+			*key = BKEY_UP;
 		/* ANSI 'B' - key down was pressed */
 		else if (c == 'B')
-			*key = KEY_DOWN;
+			*key = BKEY_DOWN;
 		/* other key was pressed */
 		else
-			*key = KEY_NONE;
+			*key = BKEY_NONE;
 
 		break;
 	}
 
 	/* enter key was pressed */
 	if (c == '\r')
-		*key = KEY_SELECT;
+		*key = BKEY_SELECT;
 
 	/* ^C was pressed */
 	if (c == 0x3)
-		*key = KEY_QUIT;
+		*key = BKEY_QUIT;
 
 	if (c == '+')
-		*key = KEY_PLUS;
+		*key = BKEY_PLUS;
 
 	if (c == '-')
-		*key = KEY_MINUS;
+		*key = BKEY_MINUS;
 
 	if (c == ' ')
-		*key = KEY_SPACE;
+		*key = BKEY_SPACE;
 }
diff --git a/include/menu.h b/include/menu.h
index 0b4d9734149..29b457921e9 100644
--- a/include/menu.h
+++ b/include/menu.h
@@ -44,14 +44,14 @@ struct bootmenu_data {
 
 /** enum bootmenu_key - keys that can be returned by the bootmenu */
 enum bootmenu_key {
-	KEY_NONE = 0,
-	KEY_UP,
-	KEY_DOWN,
-	KEY_SELECT,
-	KEY_QUIT,
-	KEY_PLUS,
-	KEY_MINUS,
-	KEY_SPACE,
+	BKEY_NONE = 0,
+	BKEY_UP,
+	BKEY_DOWN,
+	BKEY_SELECT,
+	BKEY_QUIT,
+	BKEY_PLUS,
+	BKEY_MINUS,
+	BKEY_SPACE,
 };
 
 /**
@@ -85,14 +85,14 @@ void bootmenu_autoboot_loop(struct bootmenu_data *menu,
  *
  * @menu: Menu being processed
  * @key: Returns the code for the key the user pressed:
- *	enter: KEY_SELECT
- *	Ctrl-C: KEY_QUIT
- *	Up arrow: KEY_UP
- *	Down arrow: KEY_DOWN
- *	Escape (by itself): KEY_QUIT
- *	Plus: KEY_PLUS
- *	Minus: KEY_MINUS
- *	Space: KEY_SPACE
+ *	enter: BKEY_SELECT
+ *	Ctrl-C: BKEY_QUIT
+ *	Up arrow: BKEY_UP
+ *	Down arrow: BKEY_DOWN
+ *	Escape (by itself): BKEY_QUIT
+ *	Plus: BKEY_PLUS
+ *	Minus: BKEY_MINUS
+ *	Space: BKEY_SPACE
  * @esc: On input, a non-zero value indicates that an escape sequence has
  *	resulted in that many characters so far. On exit this is updated to the
  *	new number of characters
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 05/24] menu: Update bootmenu_autoboot_loop() to return the code

[Simon Glass]

Use the return value to save having to pass around a pointer. This also
resolves any ambiguity about what *key contains when the function is
called.

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

 cmd/bootmenu.c |  2 +-
 common/menu.c  | 16 +++++++++-------
 include/menu.h |  7 +++----
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
index c80004c54dc..0e22f504fe4 100644
--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -93,7 +93,7 @@ static char *bootmenu_choice_entry(void *data)
 	while (1) {
 		if (menu->delay >= 0) {
 			/* Autoboot was not stopped */
-			bootmenu_autoboot_loop(menu, &key, &esc);
+			key = bootmenu_autoboot_loop(menu, &esc);
 		} else {
 			/* Some key was pressed, so autoboot was stopped */
 			bootmenu_loop(menu, &key, &esc);
diff --git a/common/menu.c b/common/menu.c
index 087e4c246e2..bccc104a39b 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -425,9 +425,9 @@ int menu_destroy(struct menu *m)
 	return 1;
 }
 
-void bootmenu_autoboot_loop(struct bootmenu_data *menu,
-			    enum bootmenu_key *key, int *esc)
+enum bootmenu_key bootmenu_autoboot_loop(struct bootmenu_data *menu, int *esc)
 {
+	enum bootmenu_key key = BKEY_NONE;
 	int i, c;
 
 	while (menu->delay > 0) {
@@ -446,16 +446,16 @@ void bootmenu_autoboot_loop(struct bootmenu_data *menu,
 			switch (c) {
 			case '\e':
 				*esc = 1;
-				*key = BKEY_NONE;
+				key = BKEY_NONE;
 				break;
 			case '\r':
-				*key = BKEY_SELECT;
+				key = BKEY_SELECT;
 				break;
 			case 0x3: /* ^C */
-				*key = BKEY_QUIT;
+				key = BKEY_QUIT;
 				break;
 			default:
-				*key = BKEY_NONE;
+				key = BKEY_NONE;
 				break;
 			}
 
@@ -471,7 +471,9 @@ void bootmenu_autoboot_loop(struct bootmenu_data *menu,
 	printf(ANSI_CURSOR_POSITION ANSI_CLEAR_LINE, menu->count + 5, 1);
 
 	if (menu->delay == 0)
-		*key = BKEY_SELECT;
+		key = BKEY_SELECT;
+
+	return key;
 }
 
 void bootmenu_loop(struct bootmenu_data *menu,
diff --git a/include/menu.h b/include/menu.h
index 29b457921e9..9f30a3c1acd 100644
--- a/include/menu.h
+++ b/include/menu.h
@@ -65,14 +65,13 @@ enum bootmenu_key {
  * indicating that the current option should be chosen.
  *
  * @menu: Menu being processed
- * @key: Returns the code for the key the user pressed:
+ * @esc: Set to 1 if the escape key is pressed, otherwise not updated
+ * Returns: code for the key the user pressed:
  *	enter: KEY_SELECT
  *	Ctrl-C: KEY_QUIT
  *	anything else: KEY_NONE
- * @esc: Set to 1 if the escape key is pressed, otherwise not updated
  */
-void bootmenu_autoboot_loop(struct bootmenu_data *menu,
-			    enum bootmenu_key *key, int *esc);
+enum bootmenu_key bootmenu_autoboot_loop(struct bootmenu_data *menu, int *esc);
 
 /**
  * bootmenu_loop() - handle waiting for a keypress when autoboot is disabled
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 06/24] menu: Update bootmenu_loop() to return the code

[Simon Glass]

Use the return value to save having to pass around a pointer. This also
resolves any ambiguity about what *key contains when the function is
called.

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

 cmd/bootmenu.c  |  2 +-
 cmd/eficonfig.c |  4 ++--
 common/menu.c   | 30 ++++++++++++++++--------------
 include/menu.h  | 11 +++++------
 4 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
index 0e22f504fe4..573afe16609 100644
--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -96,7 +96,7 @@ static char *bootmenu_choice_entry(void *data)
 			key = bootmenu_autoboot_loop(menu, &esc);
 		} else {
 			/* Some key was pressed, so autoboot was stopped */
-			bootmenu_loop(menu, &key, &esc);
+			key = bootmenu_loop(menu, &esc);
 		}
 
 		switch (key) {
diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index de7ce24f5ac..18f173e33ab 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -197,7 +197,7 @@ static char *eficonfig_choice_entry(void *data)
 	struct efimenu *efi_menu = data;
 
 	while (1) {
-		bootmenu_loop((struct bootmenu_data *)efi_menu, &key, &esc);
+		key = bootmenu_loop((struct bootmenu_data *)efi_menu, &esc);
 
 		switch (key) {
 		case BKEY_UP:
@@ -1868,7 +1868,7 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
 	struct eficonfig_boot_order *entry;
 
 	while (1) {
-		bootmenu_loop(NULL, &key, &esc);
+		key = bootmenu_loop(NULL, &esc);
 
 		switch (key) {
 		case BKEY_PLUS:
diff --git a/common/menu.c b/common/menu.c
index bccc104a39b..22947f5d693 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -476,9 +476,9 @@ enum bootmenu_key bootmenu_autoboot_loop(struct bootmenu_data *menu, int *esc)
 	return key;
 }
 
-void bootmenu_loop(struct bootmenu_data *menu,
-		   enum bootmenu_key *key, int *esc)
+enum bootmenu_key bootmenu_loop(struct bootmenu_data *menu, int *esc)
 {
+	enum bootmenu_key key = BKEY_NONE;
 	int c;
 
 	if (*esc == 1) {
@@ -505,17 +505,17 @@ void bootmenu_loop(struct bootmenu_data *menu,
 		/* First char of ANSI escape sequence '\e' */
 		if (c == '\e') {
 			*esc = 1;
-			*key = BKEY_NONE;
+			key = BKEY_NONE;
 		}
 		break;
 	case 1:
 		/* Second char of ANSI '[' */
 		if (c == '[') {
 			*esc = 2;
-			*key = BKEY_NONE;
+			key = BKEY_NONE;
 		} else {
 		/* Alone ESC key was pressed */
-			*key = BKEY_QUIT;
+			key = BKEY_QUIT;
 			*esc = (c == '\e') ? 1 : 0;
 		}
 		break;
@@ -524,7 +524,7 @@ void bootmenu_loop(struct bootmenu_data *menu,
 		/* Third char of ANSI (number '1') - optional */
 		if (*esc == 2 && c == '1') {
 			*esc = 3;
-			*key = BKEY_NONE;
+			key = BKEY_NONE;
 			break;
 		}
 
@@ -532,31 +532,33 @@ void bootmenu_loop(struct bootmenu_data *menu,
 
 		/* ANSI 'A' - key up was pressed */
 		if (c == 'A')
-			*key = BKEY_UP;
+			key = BKEY_UP;
 		/* ANSI 'B' - key down was pressed */
 		else if (c == 'B')
-			*key = BKEY_DOWN;
+			key = BKEY_DOWN;
 		/* other key was pressed */
 		else
-			*key = BKEY_NONE;
+			key = BKEY_NONE;
 
 		break;
 	}
 
 	/* enter key was pressed */
 	if (c == '\r')
-		*key = BKEY_SELECT;
+		key = BKEY_SELECT;
 
 	/* ^C was pressed */
 	if (c == 0x3)
-		*key = BKEY_QUIT;
+		key = BKEY_QUIT;
 
 	if (c == '+')
-		*key = BKEY_PLUS;
+		key = BKEY_PLUS;
 
 	if (c == '-')
-		*key = BKEY_MINUS;
+		key = BKEY_MINUS;
 
 	if (c == ' ')
-		*key = BKEY_SPACE;
+		key = BKEY_SPACE;
+
+	return key;
 }
diff --git a/include/menu.h b/include/menu.h
index 9f30a3c1acd..8b9b36214f7 100644
--- a/include/menu.h
+++ b/include/menu.h
@@ -83,7 +83,10 @@ enum bootmenu_key bootmenu_autoboot_loop(struct bootmenu_data *menu, int *esc);
  * character is recognised
  *
  * @menu: Menu being processed
- * @key: Returns the code for the key the user pressed:
+ * @esc: On input, a non-zero value indicates that an escape sequence has
+ *	resulted in that many characters so far. On exit this is updated to the
+ *	new number of characters
+ * Returns: code for the key the user pressed:
  *	enter: BKEY_SELECT
  *	Ctrl-C: BKEY_QUIT
  *	Up arrow: BKEY_UP
@@ -92,11 +95,7 @@ enum bootmenu_key bootmenu_autoboot_loop(struct bootmenu_data *menu, int *esc);
  *	Plus: BKEY_PLUS
  *	Minus: BKEY_MINUS
  *	Space: BKEY_SPACE
- * @esc: On input, a non-zero value indicates that an escape sequence has
- *	resulted in that many characters so far. On exit this is updated to the
- *	new number of characters
  */
-void bootmenu_loop(struct bootmenu_data *menu,
-		   enum bootmenu_key *key, int *esc);
+enum bootmenu_key bootmenu_loop(struct bootmenu_data *menu, int *esc);
 
 #endif /* __MENU_H__ */
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 07/24] menu: Use a switch statement

[Simon Glass]

Convert the long line of if() statements to a switch() since this makes
better use of the C language.

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

 common/menu.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/common/menu.c b/common/menu.c
index 22947f5d693..1aa78b762a4 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -543,22 +543,31 @@ enum bootmenu_key bootmenu_loop(struct bootmenu_data *menu, int *esc)
 		break;
 	}
 
-	/* enter key was pressed */
-	if (c == '\r')
+	switch (c) {
+	case '\r':
+		/* enter key was pressed */
 		key = BKEY_SELECT;
-
-	/* ^C was pressed */
-	if (c == 0x3)
+		break;
+	case CTL_CH('c'):
+		/* ^C was pressed */
 		key = BKEY_QUIT;
-
-	if (c == '+')
+		break;
+	case CTL_CH('p'):
+		key = BKEY_UP;
+		break;
+	case CTL_CH('n'):
+		key = BKEY_DOWN;
+		break;
+	case '+':
 		key = BKEY_PLUS;
-
-	if (c == '-')
+		break;
+	case '-':
 		key = BKEY_MINUS;
-
-	if (c == ' ')
+		break;
+	case ' ':
 		key = BKEY_SPACE;
+		break;
+	}
 
 	return key;
 }
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 09/24] image: Add a function to find a script in an image

[Simon Glass]

Split this functionality out of the 'source' command so it can be used
from another place.

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

 cmd/source.c    | 173 ++++++++++++++++++++++++++----------------------
 include/image.h |  13 ++++
 2 files changed, 107 insertions(+), 79 deletions(-)

diff --git a/cmd/source.c b/cmd/source.c
index 698d9f86d96..dfa995f1df6 100644
--- a/cmd/source.c
+++ b/cmd/source.c
@@ -24,7 +24,6 @@
 #include <asm/byteorder.h>
 #include <asm/io.h>
 
-#if defined(CONFIG_FIT)
 /**
  * get_default_image() - Return default property from /images
  *
@@ -40,123 +39,139 @@ static const char *get_default_image(const void *fit)
 
 	return fdt_getprop(fit, images_noffset, FIT_DEFAULT_PROP, NULL);
 }
-#endif
 
-int image_source_script(ulong addr, const char *fit_uname)
+int image_locate_script(void *buf, int size, const char *fit_uname,
+			char **datap, uint *lenp)
 {
 	ulong		len;
-#if defined(CONFIG_LEGACY_IMAGE_FORMAT)
 	const struct legacy_img_hdr *hdr;
-#endif
 	u32		*data;
 	int		verify;
-	void *buf;
-#if defined(CONFIG_FIT)
 	const void*	fit_hdr;
 	int		noffset;
 	const void	*fit_data;
 	size_t		fit_len;
-#endif
 
 	verify = env_get_yesno("verify");
 
-	buf = map_sysmem(addr, 0);
 	switch (genimg_get_format(buf)) {
-#if defined(CONFIG_LEGACY_IMAGE_FORMAT)
 	case IMAGE_FORMAT_LEGACY:
-		hdr = buf;
+		if (IS_ENABLED(CONFIG_LEGACY_IMAGE_FORMAT)) {
+			hdr = buf;
 
-		if (!image_check_magic (hdr)) {
-			puts ("Bad magic number\n");
-			return 1;
-		}
+			if (!image_check_magic(hdr)) {
+				puts("Bad magic number\n");
+				return 1;
+			}
 
-		if (!image_check_hcrc (hdr)) {
-			puts ("Bad header crc\n");
-			return 1;
-		}
+			if (!image_check_hcrc(hdr)) {
+				puts("Bad header crc\n");
+				return 1;
+			}
+
+			if (verify) {
+				if (!image_check_dcrc(hdr)) {
+					puts("Bad data crc\n");
+					return 1;
+				}
+			}
 
-		if (verify) {
-			if (!image_check_dcrc (hdr)) {
-				puts ("Bad data crc\n");
+			if (!image_check_type(hdr, IH_TYPE_SCRIPT)) {
+				puts("Bad image type\n");
 				return 1;
 			}
-		}
 
-		if (!image_check_type (hdr, IH_TYPE_SCRIPT)) {
-			puts ("Bad image type\n");
-			return 1;
-		}
+			/* get length of script */
+			data = (u32 *)image_get_data(hdr);
 
-		/* get length of script */
-		data = (u32 *)image_get_data (hdr);
+			len = uimage_to_cpu(*data);
+			if (!len) {
+				puts("Empty Script\n");
+				return 1;
+			}
 
-		if ((len = uimage_to_cpu (*data)) == 0) {
-			puts ("Empty Script\n");
-			return 1;
+			/*
+			 * scripts are just multi-image files with one
+			 * component, so seek past the zero-terminated sequence
+			 * of image lengths to get to the actual image data
+			 */
+			while (*data++);
 		}
-
-		/*
-		 * scripts are just multi-image files with one component, seek
-		 * past the zero-terminated sequence of image lengths to get
-		 * to the actual image data
-		 */
-		while (*data++);
 		break;
-#endif
-#if defined(CONFIG_FIT)
 	case IMAGE_FORMAT_FIT:
-		fit_hdr = buf;
-		if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) {
-			puts ("Bad FIT image format\n");
-			return 1;
-		}
+		if (IS_ENABLED(CONFIG_FIT)) {
+			fit_hdr = buf;
+			if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) {
+				puts("Bad FIT image format\n");
+				return 1;
+			}
 
-		if (!fit_uname)
-			fit_uname = get_default_image(fit_hdr);
+			if (!fit_uname)
+				fit_uname = get_default_image(fit_hdr);
 
-		if (!fit_uname) {
-			puts("No FIT subimage unit name\n");
-			return 1;
-		}
+			if (!fit_uname) {
+				puts("No FIT subimage unit name\n");
+				return 1;
+			}
 
-		/* get script component image node offset */
-		noffset = fit_image_get_node (fit_hdr, fit_uname);
-		if (noffset < 0) {
-			printf ("Can't find '%s' FIT subimage\n", fit_uname);
-			return 1;
-		}
+			/* get script component image node offset */
+			noffset = fit_image_get_node(fit_hdr, fit_uname);
+			if (noffset < 0) {
+				printf("Can't find '%s' FIT subimage\n",
+				       fit_uname);
+				return 1;
+			}
 
-		if (!fit_image_check_type (fit_hdr, noffset, IH_TYPE_SCRIPT)) {
-			puts ("Not a image image\n");
-			return 1;
-		}
+			if (!fit_image_check_type(fit_hdr, noffset,
+						  IH_TYPE_SCRIPT)) {
+				puts("Not a image image\n");
+				return 1;
+			}
+
+			/* verify integrity */
+			if (verify) {
+				if (!fit_image_verify(fit_hdr, noffset)) {
+					puts("Bad Data Hash\n");
+					return 1;
+				}
+			}
 
-		/* verify integrity */
-		if (verify) {
-			if (!fit_image_verify(fit_hdr, noffset)) {
-				puts ("Bad Data Hash\n");
+			/* get script subimage data address and length */
+			if (fit_image_get_data(fit_hdr, noffset, &fit_data, &fit_len)) {
+				puts("Could not find script subimage data\n");
 				return 1;
 			}
-		}
 
-		/* get script subimage data address and length */
-		if (fit_image_get_data (fit_hdr, noffset, &fit_data, &fit_len)) {
-			puts ("Could not find script subimage data\n");
-			return 1;
+			data = (u32 *)fit_data;
+			len = (ulong)fit_len;
 		}
-
-		data = (u32 *)fit_data;
-		len = (ulong)fit_len;
 		break;
-#endif
 	default:
-		puts ("Wrong image format for \"source\" command\n");
-		return 1;
+		puts("Wrong image format for \"source\" command\n");
+		return -EPERM;
 	}
 
-	debug("** Script length: %ld\n", len);
-	return run_command_list((char *)data, len, 0);
+	*datap = (char *)data;
+	*lenp = len;
+
+	return 0;
+}
+
+int image_source_script(ulong addr, const char *fit_uname)
+{
+	char *data;
+	void *buf;
+	uint len;
+	int ret;
+
+	buf = map_sysmem(addr, 0);
+	ret = image_locate_script(buf, 0, fit_uname, &data, &len);
+	unmap_sysmem(buf);
+	if (ret)
+		return CMD_RET_FAILURE;
+
+	debug("** Script length: %d\n", len);
+	return run_command_list(data, len, 0);
 }
 
 /**************************************************/
diff --git a/include/image.h b/include/image.h
index feb8e90c2e8..1cbc51ca228 100644
--- a/include/image.h
+++ b/include/image.h
@@ -720,6 +720,19 @@ int fit_image_load(struct bootm_headers *images, ulong addr,
  */
 int image_source_script(ulong addr, const char *fit_uname);
 
+/**
+ * image_locate_script() - Locate the raw script in an image
+ *
+ * @buf: Address of image
+ * @size: Size of image in bytes
+ * @fit_uname: Node name of FIT image to read
+ * @datap: Returns pointer to raw script on success
+ * @lenp: Returns size of raw script on success
+ * @return 0 if OK, non-zero on error
+ */
+int image_locate_script(void *buf, int size, const char *fit_uname,
+			char **datap, uint *lenp);
+
 /**
  * fit_get_node_from_config() - Look up an image a FIT by type
  *
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 10/24] video: Enable VIDEO_ANSI by default only with EFI

[Simon Glass]

This is not generally needed unless EFI_LOADER is used. Adjust the default
setting to reduce the size of the U-Boot build.

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

 drivers/video/Kconfig | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 32938376655..ed77815504b 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -118,10 +118,13 @@ config VIDEO_BPP32
 config VIDEO_ANSI
 	bool "Support ANSI escape sequences in video console"
 	depends on DM_VIDEO
-	default y
+	default y if EFI_LOADER
 	help
 	  Enable ANSI escape sequence decoding for a more fully functional
-	  console.
+	  console. Functionality includes changing the text colour and moving
+	  the cursor. These date from the 1970s and are still widely used today
+	  to control a text terminal. U-Boot implements these by decoding the
+	  sequences and performing the appropriate operation.
 
 config VIDEO_MIPI_DSI
 	bool "Support MIPI DSI interface"
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 12/24] video: Fix unchnaged typo

[Simon Glass]

Fix this typo in the header file.

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

 include/video.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/video.h b/include/video.h
index 529f9685183..7dee94cc265 100644
--- a/include/video.h
+++ b/include/video.h
@@ -248,7 +248,7 @@ void video_bmp_get_info(void *bmp_image, ulong *widthp, ulong *heightp,
  *		  that direction
  *		- if a coordinate is -ve then it will be offset to the
  *		  left/top of the centre by that many pixels
- *		- if a coordinate is positive it will be used unchnaged.
+ *		- if a coordinate is positive it will be used unchanged.
  * Return: 0 if OK, -ve on error
  */
 int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y,
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 14/24] bootstd: Read the Operating System name for distro/scripts

[Simon Glass]

Add the concept of an OS name to the bootflow. This typically includes the
OS name, version and kernel version.

Implement this for the distro and script bootmeths so that it works with
Armbian and older version of Fedora.

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

 boot/bootflow.c        |  1 +
 boot/bootmeth_distro.c | 36 ++++++++++++++++++++++++++++++++++++
 boot/bootmeth_script.c | 34 ++++++++++++++++++++++++++++++++++
 cmd/bootflow.c         |  1 +
 include/bootflow.h     |  3 +++
 test/boot/bootflow.c   |  1 +
 6 files changed, 76 insertions(+)

diff --git a/boot/bootflow.c b/boot/bootflow.c
index f9ad4099244..163cd4953dd 100644
--- a/boot/bootflow.c
+++ b/boot/bootflow.c
@@ -354,6 +354,7 @@ void bootflow_free(struct bootflow *bflow)
 	free(bflow->subdir);
 	free(bflow->fname);
 	free(bflow->buf);
+	free(bflow->os_name);
 }
 
 void bootflow_remove(struct bootflow *bflow)
diff --git a/boot/bootmeth_distro.c b/boot/bootmeth_distro.c
index 5c6c687f0a6..6ef0fa1f2c9 100644
--- a/boot/bootmeth_distro.c
+++ b/boot/bootmeth_distro.c
@@ -66,6 +66,38 @@ static int distro_check(struct udevice *dev, struct bootflow_iter *iter)
 	return 0;
 }
 
+/**
+ * distro_fill_info() - Decode the extlinux file to find out distro info
+ *
+ * @bflow: Bootflow to process
+ * @return 0 if OK, -ve on error
+ */
+static int distro_fill_info(struct bootflow *bflow)
+{
+	struct membuff mb;
+	char line[200];
+	char *data;
+	int len;
+
+	log_debug("parsing bflow file size %x\n", bflow->size);
+	membuff_init(&mb, bflow->buf, bflow->size);
+	membuff_putraw(&mb, bflow->size, true, &data);
+	while (len = membuff_readline(&mb, line, sizeof(line) - 1, ' '), len) {
+		char *tok, *p = line;
+
+		tok = strsep(&p, " ");
+		if (p) {
+			if (!strcmp("label", tok)) {
+				bflow->os_name = strdup(p);
+				if (!bflow->os_name)
+					return log_msg_ret("os", -ENOMEM);
+			}
+		}
+	}
+
+	return 0;
+}
+
 static int distro_read_bootflow(struct udevice *dev, struct bootflow *bflow)
 {
 	struct blk_desc *desc;
@@ -99,6 +131,10 @@ static int distro_read_bootflow(struct udevice *dev, struct bootflow *bflow)
 	if (ret)
 		return log_msg_ret("read", ret);
 
+	ret = distro_fill_info(bflow);
+	if (ret)
+		return log_msg_ret("inf", ret);
+
 	return 0;
 }
 
diff --git a/boot/bootmeth_script.c b/boot/bootmeth_script.c
index d1c3f940037..8ba9d93a080 100644
--- a/boot/bootmeth_script.c
+++ b/boot/bootmeth_script.c
@@ -35,6 +35,36 @@ static int script_check(struct udevice *dev, struct bootflow_iter *iter)
 	return 0;
 }
 
+/**
+ * script_fill_info() - Decode the U-Boot script to find out distro info
+ *
+ * @bflow: Bootflow to process
+ * @return 0 if OK, -ve on error
+ */
+static int script_fill_info(struct bootflow *bflow)
+{
+	char *name = NULL;
+	char *data;
+	uint len;
+	int ret;
+
+	log_debug("parsing bflow file size %x\n", bflow->size);
+
+	ret = image_locate_script(bflow->buf, bflow->size, NULL, &data, &len);
+	if (!ret) {
+		if (strstr(data, "armbianEnv"))
+			name = "Armbian";
+	}
+
+	if (name) {
+		bflow->os_name = strdup(name);
+		if (!bflow->os_name)
+			return log_msg_ret("os", -ENOMEM);
+	}
+
+	return 0;
+}
+
 static int script_read_bootflow(struct udevice *dev, struct bootflow *bflow)
 {
 	struct blk_desc *desc = NULL;
@@ -75,6 +105,10 @@ static int script_read_bootflow(struct udevice *dev, struct bootflow *bflow)
 	if (ret)
 		return log_msg_ret("read", ret);
 
+	ret = script_fill_info(bflow);
+	if (ret)
+		return log_msg_ret("inf", ret);
+
 	return 0;
 }
 
diff --git a/cmd/bootflow.c b/cmd/bootflow.c
index 313103d2775..6b8ac8c8504 100644
--- a/cmd/bootflow.c
+++ b/cmd/bootflow.c
@@ -337,6 +337,7 @@ static int do_bootflow_info(struct cmd_tbl *cmdtp, int flag, int argc,
 	printf("Filename:  %s\n", bflow->fname);
 	printf("Buffer:    %lx\n", (ulong)map_to_sysmem(bflow->buf));
 	printf("Size:      %x (%d bytes)\n", bflow->size, bflow->size);
+	printf("OS:        %s\n", bflow->os_name ? bflow->os_name : "(none)");
 	printf("Error:     %d\n", bflow->err);
 	if (dump && bflow->buf) {
 		/* Set some sort of maximum on the size */
diff --git a/include/bootflow.h b/include/bootflow.h
index 32dbbbbe261..776158c65df 100644
--- a/include/bootflow.h
+++ b/include/bootflow.h
@@ -52,6 +52,8 @@ enum bootflow_state_t {
  * @buf: Bootflow file contents (allocated)
  * @size: Size of bootflow file in bytes
  * @err: Error number received (0 if OK)
+ * @os_name: Name of the OS / distro being booted, or NULL if not known
+ *	(allocated)
  */
 struct bootflow {
 	struct list_head bm_node;
@@ -68,6 +70,7 @@ struct bootflow {
 	char *buf;
 	int size;
 	int err;
+	char *os_name;
 };
 
 /**
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index e1e07082105..3296316cf0d 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -188,6 +188,7 @@ static int bootflow_cmd_info(struct unit_test_state *uts)
 	ut_assert_nextline("Filename:  /extlinux/extlinux.conf");
 	ut_assert_nextlinen("Buffer:    ");
 	ut_assert_nextline("Size:      253 (595 bytes)");
+	ut_assert_nextline("OS:        Fedora-Workstation-armhfp-31-1.9 (5.3.7-301.fc31.armv7hl)");
 	ut_assert_nextline("Error:     0");
 	ut_assert_console_end();
 
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 15/24] bootstd: Allow reading a logo for the OS

[Simon Glass]

Some operating systems provide a logo in bmp format. Read this in if
present so it can be displayed in the menu.

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

 boot/bootmeth-uclass.c | 69 ++++++++++++++++++++++++++++++++++++------
 boot/bootmeth_script.c |  4 +++
 cmd/bootflow.c         |  6 ++++
 include/bootflow.h     |  4 +++
 include/bootmeth.h     | 16 ++++++++++
 test/boot/bootflow.c   |  1 +
 6 files changed, 91 insertions(+), 9 deletions(-)

diff --git a/boot/bootmeth-uclass.c b/boot/bootmeth-uclass.c
index 25552dd96f6..4c3529d1555 100644
--- a/boot/bootmeth-uclass.c
+++ b/boot/bootmeth-uclass.c
@@ -290,25 +290,19 @@ int bootmeth_try_file(struct bootflow *bflow, struct blk_desc *desc,
 	return 0;
 }
 
-int bootmeth_alloc_file(struct bootflow *bflow, uint size_limit, uint align)
+static int alloc_file(const char *fname, uint size, void **bufp)
 {
 	loff_t bytes_read;
 	ulong addr;
 	char *buf;
-	uint size;
 	int ret;
 
-	size = bflow->size;
-	log_debug("   - script file size %x\n", size);
-	if (size > size_limit)
-		return log_msg_ret("chk", -E2BIG);
-
-	buf = memalign(align, size + 1);
+	buf = malloc(size + 1);
 	if (!buf)
 		return log_msg_ret("buf", -ENOMEM);
 	addr = map_to_sysmem(buf);
 
-	ret = fs_read(bflow->fname, addr, 0, 0, &bytes_read);
+	ret = fs_read(fname, addr, 0, size, &bytes_read);
 	if (ret) {
 		free(buf);
 		return log_msg_ret("read", ret);
@@ -316,12 +310,69 @@ int bootmeth_alloc_file(struct bootflow *bflow, uint size_limit, uint align)
 	if (size != bytes_read)
 		return log_msg_ret("bread", -EINVAL);
 	buf[size] = '\0';
+
+	*bufp = buf;
+
+	return 0;
+}
+
+int bootmeth_alloc_file(struct bootflow *bflow, uint size_limit, uint align)
+{
+	void *buf;
+	uint size;
+	int ret;
+
+	size = bflow->size;
+	log_debug("   - script file size %x\n", size);
+	if (size > size_limit)
+		return log_msg_ret("chk", -E2BIG);
+
+	ret = alloc_file(bflow->fname, bflow->size, &buf);
+	if (ret)
+		return log_msg_ret("all", ret);
+
 	bflow->state = BOOTFLOWST_READY;
 	bflow->buf = buf;
 
 	return 0;
 }
 
+int bootmeth_alloc_other(struct bootflow *bflow, const char *fname,
+			 void **bufp, uint *sizep)
+{
+	struct blk_desc *desc = NULL;
+	char path[200];
+	loff_t size;
+	void *buf;
+	int ret;
+
+	snprintf(path, sizeof(path), "%s%s", bflow->subdir, fname);
+	log_debug("trying: %s\n", path);
+
+	if (bflow->blk)
+		desc = dev_get_uclass_plat(bflow->blk);
+
+	ret = setup_fs(bflow, desc);
+	if (ret)
+		return log_msg_ret("fs", ret);
+
+	ret = fs_size(path, &size);
+	log_debug("   %s - err=%d\n", path, ret);
+
+	ret = setup_fs(bflow, desc);
+	if (ret)
+		return log_msg_ret("fs", ret);
+
+	ret = alloc_file(path, size, &buf);
+	if (ret)
+		return log_msg_ret("all", ret);
+
+	*bufp = buf;
+	*sizep = size;
+
+	return 0;
+}
+
 int bootmeth_common_read_file(struct udevice *dev, struct bootflow *bflow,
 			      const char *file_path, ulong addr, ulong *sizep)
 {
diff --git a/boot/bootmeth_script.c b/boot/bootmeth_script.c
index 8ba9d93a080..abfe0580d92 100644
--- a/boot/bootmeth_script.c
+++ b/boot/bootmeth_script.c
@@ -109,6 +109,10 @@ static int script_read_bootflow(struct udevice *dev, struct bootflow *bflow)
 	if (ret)
 		return log_msg_ret("inf", ret);
 
+	ret = bootmeth_alloc_other(bflow, "boot.bmp", &bflow->logo,
+				   &bflow->logo_size);
+	/* ignore error */
+
 	return 0;
 }
 
diff --git a/cmd/bootflow.c b/cmd/bootflow.c
index 6b8ac8c8504..495ef85f25b 100644
--- a/cmd/bootflow.c
+++ b/cmd/bootflow.c
@@ -338,6 +338,12 @@ static int do_bootflow_info(struct cmd_tbl *cmdtp, int flag, int argc,
 	printf("Buffer:    %lx\n", (ulong)map_to_sysmem(bflow->buf));
 	printf("Size:      %x (%d bytes)\n", bflow->size, bflow->size);
 	printf("OS:        %s\n", bflow->os_name ? bflow->os_name : "(none)");
+	printf("Logo:      %s\n", bflow->logo ?
+	       simple_xtoa((ulong)map_to_sysmem(bflow->logo)) : "(none)");
+	if (bflow->logo) {
+		printf("Logo size: %x (%d bytes)\n", bflow->logo_size,
+		       bflow->logo_size);
+	}
 	printf("Error:     %d\n", bflow->err);
 	if (dump && bflow->buf) {
 		/* Set some sort of maximum on the size */
diff --git a/include/bootflow.h b/include/bootflow.h
index 776158c65df..8a07ab30191 100644
--- a/include/bootflow.h
+++ b/include/bootflow.h
@@ -49,6 +49,8 @@ enum bootflow_state_t {
  * @state: Current state (enum bootflow_state_t)
  * @subdir: Subdirectory to fetch files from (with trailing /), or NULL if none
  * @fname: Filename of bootflow file (allocated)
+ * @logo: Logo to display for this bootflow (BMP format)
+ * @logo_size: Size of the logo in bytes
  * @buf: Bootflow file contents (allocated)
  * @size: Size of bootflow file in bytes
  * @err: Error number received (0 if OK)
@@ -67,6 +69,8 @@ struct bootflow {
 	enum bootflow_state_t state;
 	char *subdir;
 	char *fname;
+	void *logo;
+	uint logo_size;
 	char *buf;
 	int size;
 	int err;
diff --git a/include/bootmeth.h b/include/bootmeth.h
index 50ded055f3f..669b14ce81e 100644
--- a/include/bootmeth.h
+++ b/include/bootmeth.h
@@ -265,6 +265,22 @@ int bootmeth_try_file(struct bootflow *bflow, struct blk_desc *desc,
  */
 int bootmeth_alloc_file(struct bootflow *bflow, uint size_limit, uint align);
 
+/**
+ * bootmeth_alloc_other() - Allocate and read a file for a bootflow
+ *
+ * This reads an arbitrary file in the same directory as the bootflow,
+ * allocating memory for it. The buffer is one byte larger than the file length,
+ * so that it can be nul-terminated.
+ *
+ * @bflow: Information about file to read
+ * @fname: Filename to read from (within bootflow->subdir)
+ * @bufp: Returns a pointer to the allocated buffer
+ * @sizep: Returns the size of the buffer
+ * Return: 0 if OK,  -ENOMEM if out of memory, other -ve on other error
+ */
+int bootmeth_alloc_other(struct bootflow *bflow, const char *fname,
+			 void **bufp, uint *sizep);
+
 /**
  * bootmeth_common_read_file() - Common handler for reading a file
  *
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index 3296316cf0d..00dfd990687 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -189,6 +189,7 @@ static int bootflow_cmd_info(struct unit_test_state *uts)
 	ut_assert_nextlinen("Buffer:    ");
 	ut_assert_nextline("Size:      253 (595 bytes)");
 	ut_assert_nextline("OS:        Fedora-Workstation-armhfp-31-1.9 (5.3.7-301.fc31.armv7hl)");
+	ut_assert_nextline("Logo:      (none)");
 	ut_assert_nextline("Error:     0");
 	ut_assert_console_end();
 
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 16/24] menu: Factor out menu-keypress decoding

[Simon Glass]

Move this code into a separate function so that it can be used in the new
VBE menu.

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

 common/menu.c  | 48 ++++++++++++++++++++++++++++++------------------
 include/menu.h | 10 ++++++++++
 2 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/common/menu.c b/common/menu.c
index c2e3ec592e3..4606cb7d1b1 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -483,26 +483,11 @@ enum bootmenu_key bootmenu_autoboot_loop(struct bootmenu_data *menu,
 	return key;
 }
 
-enum bootmenu_key bootmenu_loop(struct bootmenu_data *menu,
-				struct cli_ch_state *cch)
+enum bootmenu_key bootmenu_conv_key(int ichar)
 {
-	enum bootmenu_key key = BKEY_NONE;
-	int c;
-
-	c = cli_ch_process(cch, 0);
-	if (!c) {
-		while (!c && !tstc()) {
-			WATCHDOG_RESET();
-			mdelay(10);
-			c = cli_ch_process(cch, -ETIMEDOUT);
-		}
-		if (!c) {
-			c = getchar();
-			c = cli_ch_process(cch, c);
-		}
-	}
+	enum bootmenu_key key;
 
-	switch (c) {
+	switch (ichar) {
 	case '\n':
 		/* enter key was pressed */
 		key = BKEY_SELECT;
@@ -527,7 +512,34 @@ enum bootmenu_key bootmenu_loop(struct bootmenu_data *menu,
 	case ' ':
 		key = BKEY_SPACE;
 		break;
+	default:
+		key = BKEY_NONE;
+		break;
+	}
+
+	return key;
+}
+
+enum bootmenu_key bootmenu_loop(struct bootmenu_data *menu,
+				struct cli_ch_state *cch)
+{
+	enum bootmenu_key key;
+	int c;
+
+	c = cli_ch_process(cch, 0);
+	if (!c) {
+		while (!c && !tstc()) {
+			WATCHDOG_RESET();
+			mdelay(10);
+			c = cli_ch_process(cch, -ETIMEDOUT);
+		}
+		if (!c) {
+			c = getchar();
+			c = cli_ch_process(cch, c);
+		}
 	}
 
+	key = bootmenu_conv_key(c);
+
 	return key;
 }
diff --git a/include/menu.h b/include/menu.h
index 3996075a337..1e88141d6bf 100644
--- a/include/menu.h
+++ b/include/menu.h
@@ -53,6 +53,8 @@ enum bootmenu_key {
 	BKEY_PLUS,
 	BKEY_MINUS,
 	BKEY_SPACE,
+
+	BKEY_COUNT,
 };
 
 /**
@@ -101,4 +103,12 @@ enum bootmenu_key bootmenu_autoboot_loop(struct bootmenu_data *menu,
 enum bootmenu_key bootmenu_loop(struct bootmenu_data *menu,
 				struct cli_ch_state *cch);
 
+/**
+ * bootmenu_conv_key() - Convert a U-Boot keypress into a menu key
+ *
+ * @ichar: Keypress to convert (ASCII, including control characters)
+ * Returns: Menu key that corresponds to @ichar, or BKEY_NONE if none
+ */
+enum bootmenu_key bootmenu_conv_key(int ichar);
+
 #endif /* __MENU_H__ */
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 17/24] expo: Add basic implementation

[Simon Glass]

An expo is a way of presenting and collecting information from the
user. It consists of a collection of 'scenes' of which only one is
presented at a time. An expo is typically used to show a boot menu
and allow settings to be changed.

One created, the same expo can be automatically presented in graphical
form using a vidconsole, or in text form on a serial console.

Add an initial implementation of the expo itself. Supports for scenes
and objects is provided later.

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

 boot/Kconfig                 |  12 +
 boot/expo.c                  | 133 ++++++++++
 configs/tools-only_defconfig |   2 +
 include/expo.h               | 474 +++++++++++++++++++++++++++++++++++
 4 files changed, 621 insertions(+)
 create mode 100644 boot/expo.c
 create mode 100644 include/expo.h

diff --git a/boot/Kconfig b/boot/Kconfig
index 4e9e6a3f8a5..40c508eaaf1 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -566,6 +566,18 @@ config VPL_BOOTMETH_VBE_SIMPLE_FW
 
 endif # BOOTMETH_VBE
 
+config EXPO
+	bool "Support for expos - groups of scenes displaying a UI"
+	default y if BOOTMETH_VBE
+	help
+	  An expo is a way of presenting and collecting information from the
+	  user. It consists of a collection of 'scenes' of which only one is
+	  presented at a time. An expo is typically used to show a boot menu
+	  and allow settings to be changed.
+
+	  The expo can be presented in graphics form using a vidconsole, or in
+	  text form on a serial console.
+
 config BOOTMETH_SANDBOX
 	def_bool y
 	depends on SANDBOX
diff --git a/boot/expo.c b/boot/expo.c
new file mode 100644
index 00000000000..0274d416457
--- /dev/null
+++ b/boot/expo.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Implementation of a expo, a collection of scenes providing menu options
+ *
+ * Copyright 2022 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <expo.h>
+#include <malloc.h>
+#include <video.h>
+#include "scene_internal.h"
+
+int expo_new(const char *name, void *priv, struct expo **expp)
+{
+	struct expo *exp;
+
+	exp = calloc(1, sizeof(struct expo));
+	if (!exp)
+		return log_msg_ret("expo", -ENOMEM);
+	exp->name = strdup(name);
+	if (!exp->name) {
+		free(exp);
+		return log_msg_ret("name", -ENOMEM);
+	}
+	exp->priv = priv;
+	INIT_LIST_HEAD(&exp->scene_head);
+
+	*expp = exp;
+
+	return 0;
+}
+
+void expo_destroy(struct expo *exp)
+{
+	struct scene *scn, *next;
+
+	list_for_each_entry_safe(scn, next, &exp->scene_head, sibling)
+		scene_destroy(scn);
+
+	free(exp->name);
+	free(exp);
+}
+
+int expo_set_display(struct expo *exp, struct udevice *dev)
+{
+	exp->display = dev;
+
+	return 0;
+}
+
+void exp_set_text_mode(struct expo *exp, bool text_mode)
+{
+	exp->text_mode = text_mode;
+}
+
+struct scene *expo_lookup_scene_id(struct expo *exp, uint scene_id)
+{
+	struct scene *scn;
+
+	list_for_each_entry(scn, &exp->scene_head, sibling) {
+		if (scn->id == scene_id)
+			return scn;
+	}
+
+	return NULL;
+}
+
+int expo_set_scene_id(struct expo *exp, uint scene_id)
+{
+	if (!expo_lookup_scene_id(exp, scene_id))
+		return log_msg_ret("id", -ENOENT);
+	exp->scene_id = scene_id;
+
+	return 0;
+}
+
+int expo_render(struct expo *exp)
+{
+	struct udevice *dev = exp->display;
+	struct video_priv *vid_priv = dev_get_uclass_priv(dev);
+	struct scene *scn = NULL;
+	u32 colour;
+	int ret;
+
+	colour = video_index_to_colour(vid_priv, VID_WHITE);
+	ret = video_fill(dev, colour);
+	if (ret)
+		return log_msg_ret("fill", ret);
+
+	if (exp->scene_id) {
+		scn = expo_lookup_scene_id(exp, exp->scene_id);
+		if (!scn)
+			return log_msg_ret("scn", -ENOENT);
+
+		ret = scene_render(scn);
+		if (ret)
+			return log_msg_ret("ren", ret);
+	}
+
+	video_sync(dev, true);
+
+	return scn ? 0 : -ECHILD;
+}
+
+int expo_send_key(struct expo *exp, int key)
+{
+	struct scene *scn = NULL;
+
+	if (exp->scene_id) {
+		int ret;
+
+		scn = expo_lookup_scene_id(exp, exp->scene_id);
+		if (!scn)
+			return log_msg_ret("scn", -ENOENT);
+
+		ret = scene_send_key(scn, key, &exp->action);
+		if (ret)
+			return log_msg_ret("key", ret);
+	}
+
+	return scn ? 0 : -ECHILD;
+}
+
+int expo_action_get(struct expo *exp, struct expo_action *act)
+{
+	*act = exp->action;
+	exp->action.type = EXPOACT_NONE;
+
+	return act->type == EXPOACT_NONE ? -EAGAIN : 0;
+}
diff --git a/configs/tools-only_defconfig b/configs/tools-only_defconfig
index 88a1271f7e3..6da9a0a2cd7 100644
--- a/configs/tools-only_defconfig
+++ b/configs/tools-only_defconfig
@@ -7,6 +7,8 @@ CONFIG_ANDROID_BOOT_IMAGE=y
 CONFIG_FIT=y
 CONFIG_TIMESTAMP=y
 CONFIG_FIT_SIGNATURE=y
+# CONFIG_BOOTSTD_FULL is not set
+# CONFIG_BOOTMETH_VBE is not set
 CONFIG_USE_BOOTCOMMAND=y
 CONFIG_BOOTCOMMAND="run distro_bootcmd"
 # CONFIG_CMD_BOOTD is not set
diff --git a/include/expo.h b/include/expo.h
new file mode 100644
index 00000000000..2660f8cf15f
--- /dev/null
+++ b/include/expo.h
@@ -0,0 +1,474 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2022 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#ifndef __SCENE_H
+#define __SCENE_H
+
+#include <linux/list.h>
+
+struct udevice;
+
+/**
+ * enum expoact_type - types of actions reported by the expo
+ *
+ * @EXPOACT_NONE: no action
+ * @EXPOACT_POINT: menu item was highlighted (@id indicates which)
+ * @EXPOACT_SELECT: menu item was selected (@id indicates which)
+ * @EXPOACT_QUIT: request to exit the menu
+ */
+enum expoact_type {
+	EXPOACT_NONE,
+	EXPOACT_POINT,
+	EXPOACT_SELECT,
+	EXPOACT_QUIT,
+};
+
+/**
+ * struct expo_action - an action report by the expo
+ *
+ * @type: Action type (EXPOACT_NONE if there is no action)
+ * @select: Used for EXPOACT_POINT and EXPOACT_SELECT
+ * @id: ID number of the object affected.
+ */
+struct expo_action {
+	enum expoact_type type;
+	union {
+		struct {
+			int id;
+		} select;
+	};
+};
+
+/**
+ * struct expo - information about an expo
+ *
+ * A group of scenes which can be presented to the user, typically to obtain
+ * input or to make a selection.
+ *
+ * @name: Name of the expo (allocated)
+ * @display: Display to use (`UCLASS_VIDEO`), or NULL to use text mode
+ * @scene_id: Current scene ID (0 if none)
+ * @next_id: Next ID number to use, for automatic allocation
+ * @action: Action selected by user. At present only one is supported, with the
+ * type set to EXPOACT_NONE if there is no action
+ * @text_mode: true to use text mode for the menu (no vidconsole)
+ * @priv: Private data for the controller
+ * @scene_head: List of scenes
+ */
+struct expo {
+	char *name;
+	struct udevice *display;
+	uint scene_id;
+	uint next_id;
+	struct expo_action action;
+	bool text_mode;
+	void *priv;
+	struct list_head scene_head;
+};
+
+/**
+ * struct scene - information about a scene in an expo
+ *
+ * A collection of text/image/menu items in an expo
+ *
+ * @expo: Expo this scene is part of
+ * @name: Name of the scene (allocated)
+ * @id: ID number of the scene
+ * @title: Title of the scene (allocated)
+ * @sibling: Node to link this scene to its siblings
+ * @obj_head: List of objects in the scene
+ */
+struct scene {
+	struct expo *expo;
+	char *name;
+	uint id;
+	char *title;
+	struct list_head sibling;
+	struct list_head obj_head;
+};
+
+/**
+ * enum scene_obj_t - type of a scene object
+ *
+ * @SCENEOBJT_NONE: Used to indicate that the type does not matter
+ * @SCENEOBJT_IMAGE: Image data to render
+ * @SCENEOBJT_TEXT: Text line to render
+ * @SCENEOBJT_MENU: Menu containing items the user can select
+ */
+enum scene_obj_t {
+	SCENEOBJT_NONE		= 0,
+	SCENEOBJT_IMAGE,
+	SCENEOBJT_TEXT,
+	SCENEOBJT_MENU,
+};
+
+/**
+ * struct scene_obj - information about an object in a scene
+ *
+ * @scene: Scene that this object relates to
+ * @name: Name of the object (allocated)
+ * @id: ID number of the object
+ * @type: Type of this object
+ * @x: x position, in pixels from left side
+ * @y: y position, in pixels from top
+ * @hide: true if the object should be hidden
+ * @sibling: Node to link this object to its siblings
+ */
+struct scene_obj {
+	struct scene *scene;
+	char *name;
+	uint id;
+	enum scene_obj_t type;
+	int x;
+	int y;
+	bool hide;
+	struct list_head sibling;
+};
+
+/**
+ * struct scene_obj_img - information about an image object in a scene
+ *
+ * This is a rectangular image which is blitted onto the display
+ *
+ * @obj: Basic object information
+ * @data: Image data in BMP format
+ */
+struct scene_obj_img {
+	struct scene_obj obj;
+	char *data;
+};
+
+/**
+ * struct scene_obj_txt - information about a text object in a scene
+ *
+ * This is a single-line text object
+ *
+ * @obj: Basic object information
+ * @str: Text string to display (allocated by caller)
+ * @font_name: Name of font (allocated by caller)
+ * @font_size: Nominal size of font in pixels
+ */
+struct scene_obj_txt {
+	struct scene_obj obj;
+	const char *str;
+	const char *font_name;
+	uint font_size;
+};
+
+/**
+ * struct scene_obj_menu - information about a menu object in a scene
+ *
+ * A menu has a number of items which can be selected by the user
+ *
+ * It also has:
+ *
+ * - a text/image object (@pointer_id) which points to the current item
+ *   (@cur_item_id)
+ *
+ * - a preview object which shows an image related to the current item
+ *
+ * @obj: Basic object information
+ * @title_id: ID of the title text, or 0 if none
+ * @cur_item_id: ID of the current menu item, or 0 if none
+ * @pointer_id: ID of the object pointing to the current selection
+ * @item_head: List of items in the menu
+ */
+struct scene_obj_menu {
+	struct scene_obj obj;
+	uint title_id;
+	uint cur_item_id;
+	uint pointer_id;
+	struct list_head item_head;
+};
+
+/**
+ * enum scene_menuitem_flags_t - flags for menu items
+ *
+ * @SCENEMIF_GAP_BEFORE: Add a gap before this item
+ */
+enum scene_menuitem_flags_t {
+	SCENEMIF_GAP_BEFORE	= 1 << 0,
+};
+
+/**
+ * struct scene_menuitem - a menu item in a menu
+ *
+ * A menu item has:
+ *
+ * - text object holding the name (short) and description (can be longer)
+ * - a text object holding the keypress
+ *
+ * @name: Name of the item (this is allocated by this call)
+ * @id: ID number of the object
+ * @key_id: ID of text object to use as the keypress to show
+ * @label_id: ID of text object to use as the label text
+ * @desc_id: ID of text object to use as the description text
+ * @preview_id: ID of the preview object, or 0 if none
+ * @flags: Flags for this item
+ * @sibling: Node to link this item to its siblings
+ */
+struct scene_menuitem {
+	char *name;
+	uint id;
+	uint key_id;
+	uint label_id;
+	uint desc_id;
+	uint preview_id;
+	uint flags;
+	struct list_head sibling;
+};
+
+/**
+ * expo_new() - create a new expo
+ *
+ * Allocates a new expo
+ *
+ * @name: Name of expo (this is allocated by this call)
+ * @priv: Private data for the controller
+ * @expp: Returns a pointer to the new expo on success
+ * Returns: 0 if OK, -ENOMEM if out of memory
+ */
+int expo_new(const char *name, void *priv, struct expo **expp);
+
+/**
+ * expo_destroy() - Destroy an expo and free all its memory
+ *
+ * @exp: Expo to destroy
+ */
+void expo_destroy(struct expo *exp);
+
+/**
+ * expo_set_display() - set the display to use for a expo
+ *
+ * @exp: Expo to update
+ * @dev: Display to use (`UCLASS_VIDEO`), NULL to use text mode
+ * Returns: 0 (always)
+ */
+int expo_set_display(struct expo *exp, struct udevice *dev);
+
+/**
+ * expo_set_scene_id() - Set the current scene ID
+ *
+ * @exp: Expo to update
+ * @scene_id: New scene ID to use (0 to select no scene)
+ * Returns: 0 if OK, -ENOENT if there is no scene with that ID
+ */
+int expo_set_scene_id(struct expo *exp, uint scene_id);
+
+/**
+ * expo_render() - render the expo on the display / console
+ *
+ * @exp: Expo to render
+ *
+ * Returns: 0 if OK, -ECHILD if there is no current scene, -ENOENT if the
+ * current scene is not found, other error if something else goes wrong
+ */
+int expo_render(struct expo *exp);
+
+/**
+ * exp_set_text_mode() - Controls whether the expo renders in text mode
+ *
+ * @exp: Expo to update
+ * @text_mode: true to use text mode, false to use the console
+ */
+void exp_set_text_mode(struct expo *exp, bool text_mode);
+
+/**
+ * scene_new() - create a new scene in a expo
+ *
+ * The scene is given the ID @id which must be unique across all scenes, objects
+ * and items. The expo's @next_id is updated to at least @id + 1
+ *
+ * @exp: Expo to update
+ * @name: Name to use (this is allocated by this call)
+ * @id: ID to use for the new scene (0 to allocate one)
+ * @scnp: Returns a pointer to the new scene on success
+ * Returns: ID number for the scene (typically @id), or -ve on error
+ */
+int scene_new(struct expo *exp, const char *name, uint id, struct scene **scnp);
+
+/**
+ * expo_lookup_scene_id() - Look up a scene by ID
+ *
+ * @exp: Expo to check
+ * @scene_id: Scene ID to look up
+ * @returns pointer to scene if found, else NULL
+ */
+struct scene *expo_lookup_scene_id(struct expo *exp, uint scene_id);
+
+/**
+ * scene_title_set() - set the scene title
+ *
+ * @scn: Scene to update
+ * @title: Title to set, NULL if none (this is allocated by this call)
+ * Returns: 0 if OK, -ENOMEM if out of memory
+ */
+int scene_title_set(struct scene *scn, const char *title);
+
+/**
+ * scene_obj_count() - Count the number of objects in a scene
+ *
+ * @scn: Scene to check
+ * Returns: number of objects in the scene, 0 if none
+ */
+int scene_obj_count(struct scene *scn);
+
+/**
+ * scene_img_add() - add a new image to a scene
+ *
+ * @scn: Scene to update
+ * @name: Name to use (this is allocated by this call)
+ * @id: ID to use for the new object (0 to allocate one)
+ * @data: Pointer to image data
+ * @imgp: If non-NULL, returns the new object
+ * Returns: ID number for the object (typically @id), or -ve on error
+ */
+int scene_img_add(struct scene *scn, const char *name, uint id, char *data,
+		  struct scene_obj_img **imgp);
+
+/**
+ * scene_txt_add() - add a new text object to a scene
+ *
+ * @scn: Scene to update
+ * @name: Name to use (this is allocated by this call)
+ * @id: ID to use for the new object (0 to allocate one)
+ * @str: Pointer to text to display (allocated by caller)
+ * @txtp: If non-NULL, returns the new object
+ * Returns: ID number for the object (typically @id), or -ve on error
+ */
+int scene_txt_add(struct scene *scn, const char *name, uint id, char *str,
+		  struct scene_obj_txt **txtp);
+
+/**
+ *  scene_menu_add() - create a menu
+ *
+ * @scn: Scene to update
+ * @name: Name to use (this is allocated by this call)
+ * @id: ID to use for the new object (0 to allocate one)
+ * @menup: If non-NULL, returns the new object
+ * Returns: ID number for the object (typically @id), or -ve on error
+ */
+int scene_menu_add(struct scene *scn, const char *name, uint id,
+		   struct scene_obj_menu **menup);
+
+/**
+ * scene_txt_set_font() - Set the font for an object
+ *
+ * @scn: Scene to update
+ * @id: ID of object to update
+ * @font_name: Font name to use (allocated by caller)
+ * @font_size: Font size to use (nominal height in pixels)
+ */
+int scene_txt_set_font(struct scene *scn, uint id, const char *font_name,
+		       uint font_size);
+
+/**
+ * scene_obj_set_pos() - Set the postion of an object
+ *
+ * @scn: Scene to update
+ * @id: ID of object to update
+ * @x: x position, in pixels from left side
+ * @y: y position, in pixels from top
+ * Returns: 0 if OK, -ENOENT if @id is invalid
+ */
+int scene_obj_set_pos(struct scene *scn, uint id, int x, int y);
+
+/**
+ * scene_obj_set_hide() - Set whether an object is hidden
+ *
+ * The update happens when the expo is next rendered.
+ *
+ * @scn: Scene to update
+ * @id: ID of object to update
+ * @hide: true to hide the object, false to show it
+ * Returns: 0 if OK, -ENOENT if @id is invalid
+ */
+int scene_obj_set_hide(struct scene *scn, uint id, bool hide);
+
+/**
+ * scene_menu_set_title() - Set the title of a menu
+ *
+ * @scn: Scene to update
+ * @id: ID of menu object to update
+ * @title_id: ID of text object to use as the title
+ * Returns: 0 if OK, -ENOENT if @id is invalid, -EINVAL if @title_id is invalid
+ */
+int scene_menu_set_title(struct scene *scn, uint id, uint title_id);
+
+/**
+ * scene_menu_set_pointer() - Set the item pointer for a menu
+ *
+ * This is a visual indicator of the current item, typically a ">" character
+ * which sits next to the current item and moves when the user presses the
+ * up/down arrow keys
+ *
+ * @scn: Scene to update
+ * @id: ID of menu object to update
+ * @cur_item_id: ID of text or image object to use as a pointer to the current
+ * item
+ * Returns: 0 if OK, -ENOENT if @id is invalid, -EINVAL if @cur_item_id is invalid
+ */
+int scene_menu_set_pointer(struct scene *scn, uint id, uint cur_item_id);
+
+/**
+ * scene_obj_get_hw() - Get width and height of an object in a scene
+ *
+ * @scn: Scene to check
+ * @id: ID of menu object to check
+ * @widthp: If non-NULL, returns width of object in pixels
+ * Returns: Height of object in pixels
+ */
+int scene_obj_get_hw(struct scene *scn, uint id, int *widthp);
+
+/**
+ * scene_item_add() - Add an item to a menu
+ *
+ * @scn: Scene to update
+ * @menu_id: ID of menu object to update
+ * @name: Name to use (this is allocated by this call)
+ * @id: ID to use for the new object (0 to allocate one)
+ * @key_id: ID of text object to use as the keypress to show
+ * @label_id: ID of text object to use as the label text
+ * @desc_id: ID of text object to use as the description text
+ * @preview_id: ID of object to use as the preview (text or image)
+ * @flags: Flags for this item (enum scene_menuitem_flags_t)
+ * @itemp: If non-NULL, returns the new object
+ * Returns: ID number for the item (typically @id), or -ve on error
+ */
+int scene_menuitem_add(struct scene *scn, uint menu_id, const char *name,
+		       uint id, uint key_id, uint label_id, uint desc_id,
+		       uint preview_id, uint flags,
+		       struct scene_menuitem **itemp);
+
+/**
+ * scene_arrange() - Arrange the scene to deal with object sizes
+ *
+ * Updates any menus in the scene so that their objects are in the right place.
+ *
+ * @scn: Scene to arrange
+ * Returns: 0 if OK, -ve on error
+ */
+int scene_arrange(struct scene *scn);
+
+/**
+ * expo_send_key() - set a keypress to the expo
+ *
+ * @exp: Expo to receive the key
+ * @key: Key to send (ASCII or enum bootmenu_key)
+ * Returns: 0 if OK, -ECHILD if there is no current scene
+ */
+int expo_send_key(struct expo *exp, int key);
+
+/**
+ * expo_action_get() - read user input from the expo
+ *
+ * @exp: Expo to check
+ * @act: Returns action
+ * Returns: 0 if OK, -EAGAIN if there was no action to return
+ */
+int expo_action_get(struct expo *exp, struct expo_action *act);
+
+#endif /*__SCENE_H */
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 18/24] expo: Add support for scenes

[Simon Glass]

A scene is a single screen within an expo. It is possible to move between
scenes but only one can be displayed at once.

Add a basic implementation.

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

 boot/scene.c          | 385 ++++++++++++++++++++++++++++++++++++++++++
 boot/scene_internal.h | 123 ++++++++++++++
 2 files changed, 508 insertions(+)
 create mode 100644 boot/scene.c
 create mode 100644 boot/scene_internal.h

diff --git a/boot/scene.c b/boot/scene.c
new file mode 100644
index 00000000000..cc0e7f58da0
--- /dev/null
+++ b/boot/scene.c
@@ -0,0 +1,385 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Implementation of a scene, a collection of text/image/menu items in an expo
+ *
+ * Copyright 2022 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <expo.h>
+#include <malloc.h>
+#include <mapmem.h>
+#include <video.h>
+#include <video_console.h>
+#include <linux/input.h>
+#include "scene_internal.h"
+
+uint resolve_id(struct expo *exp, uint id)
+{
+	if (!id)
+		id = exp->next_id++;
+	else if (id >= exp->next_id)
+		exp->next_id = id + 1;
+
+	return id;
+}
+
+int scene_new(struct expo *exp, const char *name, uint id, struct scene **scnp)
+{
+	struct scene *scn;
+
+	scn = calloc(1, sizeof(struct scene));
+	if (!scn)
+		return log_msg_ret("expo", -ENOMEM);
+	scn->name = strdup(name);
+	if (!scn->name) {
+		free(scn);
+		return log_msg_ret("name", -ENOMEM);
+	}
+
+	INIT_LIST_HEAD(&scn->obj_head);
+	scn->id = resolve_id(exp, id);
+	scn->expo = exp;
+	list_add_tail(&scn->sibling, &exp->scene_head);
+
+	*scnp = scn;
+
+	return scn->id;
+}
+
+void scene_obj_destroy(struct scene_obj *obj)
+{
+	if (obj->type == SCENEOBJT_MENU)
+		scene_menu_destroy((struct scene_obj_menu *)obj);
+	free(obj->name);
+	free(obj);
+}
+
+void scene_destroy(struct scene *scn)
+{
+	struct scene_obj *obj, *next;
+
+	list_for_each_entry_safe(obj, next, &scn->obj_head, sibling)
+		scene_obj_destroy(obj);
+
+	free(scn->name);
+	free(scn->title);
+	free(scn);
+}
+
+int scene_title_set(struct scene *scn, const char *title)
+{
+	free(scn->title);
+	scn->title = strdup(title);
+	if (!scn->title)
+		return log_msg_ret("tit", -ENOMEM);
+
+	return 0;
+}
+
+int scene_obj_count(struct scene *scn)
+{
+	struct scene_obj *obj;
+	int count = 0;
+
+	list_for_each_entry(obj, &scn->obj_head, sibling)
+		count++;
+
+	return count;
+}
+
+void *scene_obj_find(struct scene *scn, uint id, enum scene_obj_t type)
+{
+	struct scene_obj *obj;
+
+	list_for_each_entry(obj, &scn->obj_head, sibling) {
+		if (obj->id == id &&
+		    (type == SCENEOBJT_NONE || obj->type == type))
+			return obj;
+	}
+
+	return NULL;
+}
+
+int scene_obj_add(struct scene *scn, const char *name, uint id,
+		  enum scene_obj_t type, uint size, struct scene_obj **objp)
+{
+	struct scene_obj *obj;
+
+	obj = calloc(1, size);
+	if (!obj)
+		return log_msg_ret("obj", -ENOMEM);
+	obj->name = strdup(name);
+	if (!obj->name) {
+		free(obj);
+		return log_msg_ret("name", -ENOMEM);
+	}
+
+	obj->id = resolve_id(scn->expo, id);
+	obj->scene = scn;
+	obj->type = type;
+	list_add_tail(&obj->sibling, &scn->obj_head);
+	*objp = obj;
+
+	return obj->id;
+}
+
+int scene_img_add(struct scene *scn, const char *name, uint id, char *data,
+		  struct scene_obj_img **imgp)
+{
+	struct scene_obj_img *img;
+	int ret;
+
+	ret = scene_obj_add(scn, name, id, SCENEOBJT_IMAGE,
+			    sizeof(struct scene_obj_img),
+			    (struct scene_obj **)&img);
+	if (ret < 0)
+		return log_msg_ret("obj", -ENOMEM);
+
+	img->data = data;
+
+	if (imgp)
+		*imgp = img;
+
+	return img->obj.id;
+}
+
+int scene_txt_add(struct scene *scn, const char *name, uint id, char *str,
+		  struct scene_obj_txt **txtp)
+{
+	struct scene_obj_txt *txt;
+	int ret;
+
+	ret = scene_obj_add(scn, name, id, SCENEOBJT_TEXT,
+			    sizeof(struct scene_obj_txt),
+			    (struct scene_obj **)&txt);
+	if (ret < 0)
+		return log_msg_ret("obj", -ENOMEM);
+
+	txt->str = str;
+
+	if (txtp)
+		*txtp = txt;
+
+	return txt->obj.id;
+}
+
+int scene_txt_set_font(struct scene *scn, uint id, const char *font_name,
+		       uint font_size)
+{
+	struct scene_obj_txt *txt;
+
+	txt = scene_obj_find(scn, id, SCENEOBJT_TEXT);
+	if (!txt)
+		return log_msg_ret("find", -ENOENT);
+	txt->font_name = font_name;
+	txt->font_size = font_size;
+
+	return 0;
+}
+
+int scene_obj_set_pos(struct scene *scn, uint id, int x, int y)
+{
+	struct scene_obj *obj;
+
+	obj = scene_obj_find(scn, id, SCENEOBJT_NONE);
+	if (!obj)
+		return log_msg_ret("find", -ENOENT);
+	obj->x = x;
+	obj->y = y;
+	if (obj->type == SCENEOBJT_MENU)
+		scene_menu_arrange(scn, (struct scene_obj_menu *)obj);
+
+	return 0;
+}
+
+int scene_obj_set_hide(struct scene *scn, uint id, bool hide)
+{
+	struct scene_obj *obj;
+
+	obj = scene_obj_find(scn, id, SCENEOBJT_NONE);
+	if (!obj)
+		return log_msg_ret("find", -ENOENT);
+	obj->hide = hide;
+
+	return 0;
+}
+
+int scene_obj_get_hw(struct scene *scn, uint id, int *widthp)
+{
+	struct scene_obj *obj;
+
+	obj = scene_obj_find(scn, id, SCENEOBJT_NONE);
+	if (!obj)
+		return log_msg_ret("find", -ENOENT);
+
+	switch (obj->type) {
+	case SCENEOBJT_NONE:
+	case SCENEOBJT_MENU:
+		break;
+	case SCENEOBJT_IMAGE: {
+		struct scene_obj_img *img = (struct scene_obj_img *)obj;
+		ulong width, height;
+		uint bpix;
+
+		video_bmp_get_info(img->data, &width, &height, &bpix);
+		if (widthp)
+			*widthp = width;
+		return height;
+	}
+	case SCENEOBJT_TEXT: {
+		struct scene_obj_txt *txt = (struct scene_obj_txt *)obj;
+		struct expo *exp = scn->expo;
+
+		if (widthp)
+			*widthp = 16; /* fake value for now */
+		if (txt->font_size)
+			return txt->font_size;
+		if (exp->display)
+			return video_default_font_height(exp->display);
+
+		/* use a sensible default */
+		return 16;
+	}
+	}
+
+	return 0;
+}
+
+/**
+ * scene_obj_render() - Render an object
+ *
+ */
+static int scene_obj_render(struct scene_obj *obj, bool text_mode)
+{
+	struct scene *scn = obj->scene;
+	struct expo *exp = scn->expo;
+	struct udevice *cons, *dev = exp->display;
+	int x, y, ret;
+
+	cons = NULL;
+	if (!text_mode) {
+		ret = device_find_first_child_by_uclass(dev,
+							UCLASS_VIDEO_CONSOLE,
+							&cons);
+	}
+
+	x = obj->x;
+	y = obj->y;
+
+	switch (obj->type) {
+	case SCENEOBJT_NONE:
+		break;
+	case SCENEOBJT_IMAGE: {
+		struct scene_obj_img *img = (struct scene_obj_img *)obj;
+
+		if (!cons)
+			return -ENOTSUPP;
+		ret = video_bmp_display(dev, map_to_sysmem(img->data), x, y,
+					true);
+		if (ret < 0)
+			return log_msg_ret("img", ret);
+		break;
+	}
+	case SCENEOBJT_TEXT: {
+		struct scene_obj_txt *txt = (struct scene_obj_txt *)obj;
+
+		if (!cons)
+			return -ENOTSUPP;
+
+		if (txt->font_name || txt->font_size) {
+			ret = vidconsole_select_font(cons,
+						     txt->font_name,
+						     txt->font_size);
+		} else {
+			ret = vidconsole_select_font(cons, NULL, 0);
+		}
+		if (ret && ret != -ENOSYS)
+			return log_msg_ret("font", ret);
+		vidconsole_set_cursor_pos(cons, x, y);
+		vidconsole_put_string(cons, txt->str);
+		break;
+	}
+	case SCENEOBJT_MENU: {
+		struct scene_obj_menu *menu = (struct scene_obj_menu *)obj;
+		/*
+		 * With a vidconsole, the text and item pointer are rendered as
+		 * normal objects so we don't need to do anything here. The menu
+		 * simply controls where they are positioned.
+		 */
+		if (cons)
+			return -ENOTSUPP;
+
+		ret = scene_menu_display(menu);
+		if (ret < 0)
+			return log_msg_ret("img", ret);
+
+		break;
+	}
+	}
+
+	return 0;
+}
+
+int scene_arrange(struct scene *scn)
+{
+	struct scene_obj *obj;
+	int ret;
+
+	list_for_each_entry(obj, &scn->obj_head, sibling) {
+		if (obj->type == SCENEOBJT_MENU) {
+			struct scene_obj_menu *menu;
+
+			menu = (struct scene_obj_menu *)obj,
+			ret = scene_menu_arrange(scn, menu);
+			if (ret)
+				return log_msg_ret("arr", ret);
+		}
+	}
+
+	return 0;
+}
+
+int scene_render(struct scene *scn)
+{
+	struct expo *exp = scn->expo;
+	struct scene_obj *obj;
+	int ret;
+
+	list_for_each_entry(obj, &scn->obj_head, sibling) {
+		if (!obj->hide) {
+			ret = scene_obj_render(obj, exp->text_mode);
+			if (ret && ret != -ENOTSUPP)
+				return log_msg_ret("ren", ret);
+		}
+	}
+
+	return 0;
+}
+
+int scene_send_key(struct scene *scn, int key, struct expo_action *event)
+{
+	struct scene_obj *obj;
+	int ret;
+
+	list_for_each_entry(obj, &scn->obj_head, sibling) {
+		if (obj->type == SCENEOBJT_MENU) {
+			struct scene_obj_menu *menu;
+
+			menu = (struct scene_obj_menu *)obj,
+			ret = scene_menu_send_key(scn, menu, key, event);
+			if (ret)
+				return log_msg_ret("key", ret);
+
+			/* only allow one menu */
+			ret = scene_menu_arrange(scn, menu);
+			if (ret)
+				return log_msg_ret("arr", ret);
+			break;
+		}
+	}
+
+	return 0;
+}
diff --git a/boot/scene_internal.h b/boot/scene_internal.h
new file mode 100644
index 00000000000..e8fd765811e
--- /dev/null
+++ b/boot/scene_internal.h
@@ -0,0 +1,123 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Internal header file for scenes
+ *
+ * Copyright 2022 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#ifndef __SCENE_INTERNAL_H
+#define __SCENE_INTERNAL_H
+
+/**
+ * expo_lookup_scene_id() - Look up a scene ID
+ *
+ * @exp: Expo to use
+ * @id: scene ID to look up
+ * Returns: Scene for that ID, or NULL if none
+ */
+struct scene *expo_lookup_scene_id(struct expo *exp, uint scene_id);
+
+/**
+ * resolve_id() - Automatically allocate an ID if needed
+ *
+ * @exp: Expo to use
+ * @id: ID to use, or 0 to auto-allocate one
+ * @return: Either @id, or the auto-allocated ID
+ */
+uint resolve_id(struct expo *exp, uint id);
+
+/**
+ * scene_obj_find() - Find an object in a scene
+ *
+ * Note that @type is used to restrict the search when the object type is known.
+ * If any type is acceptable, set @type to SCENEOBJT_NONE
+ *
+ * @scn: Scene to search
+ * @id: ID of object to find
+ * @type: Type of the object, or SCENEOBJT_NONE to match any type
+ */
+void *scene_obj_find(struct scene *scn, uint id, enum scene_obj_t type);
+
+/**
+ * scene_obj_add() - Add a new object to a scene
+ *
+ * @scn: Scene to update
+ * @name: Name to use (this is allocated by this call)
+ * @id: ID to use for the new object (0 to allocate one)
+ * @type: Type of object to add
+ * @size: Size to allocate for the object, in bytes
+ * @objp: Returns a pointer to the new object (must not be NULL)
+ * Returns: ID number for the object (generally @id), or -ve on error
+ */
+int scene_obj_add(struct scene *scn, const char *name, uint id,
+		  enum scene_obj_t type, uint size, struct scene_obj **objp);
+
+/**
+ * scene_menu_arrange() - Set the position of things in the menu
+ *
+ * This updates any items associated with a menu to make sure they are
+ * positioned correctly relative to the menu. It also selects the first item
+ * if not already done
+ *
+ * @scn: Scene to update
+ * @menu: Menu to process
+ */
+int scene_menu_arrange(struct scene *scn, struct scene_obj_menu *menu);
+
+/**
+ * scene_menu_send_key() - Send a key to a menu for processing
+ *
+ * @scn: Scene to use
+ * @menu: Menu to use
+ * @key: Key code to send (KEY_...)
+ * @event: Place to put any event which is generated by the key
+ * @return 0 if OK, -ENOTTY if there is no current menu item, other -ve on other
+ *	error
+ */
+int scene_menu_send_key(struct scene *scn, struct scene_obj_menu *menu, int key,
+			struct expo_action *event);
+
+/**
+ * scene_menu_destroy() - Destroy a menu in a scene
+ *
+ * @scn: Scene to destroy
+ */
+void scene_menu_destroy(struct scene_obj_menu *menu);
+
+/**
+ * scene_menu_display() - Display a menu as text
+ *
+ * @menu: Menu to display
+ * @return 0 if OK, -ENOENT if @id is invalid
+ */
+int scene_menu_display(struct scene_obj_menu *menu);
+
+/**
+ * scene_destroy() - Destroy a scene and all its memory
+ *
+ * @scn: Scene to destroy
+ */
+void scene_destroy(struct scene *scn);
+
+/**
+ * scene_render() - Render a scene
+ *
+ * This is called from expo_render()
+ *
+ * @scn: Scene to render
+ * Returns: 0 if OK, -ve on error
+ */
+int scene_render(struct scene *scn);
+
+/**
+ * scene_send_key() - set a keypress to a scene
+ *
+ * @scn: Scene to receive the key
+ * @key: Key to send (KEYCODE_UP)
+ * @event: Returns resulting event from this keypress
+ * Returns: 0 if OK, -ve on error
+ */
+int scene_send_key(struct scene *scn, int key, struct expo_action *event);
+
+#endif /* __SCENE_INTERNAL_H */
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 19/24] expo: Add support for scene menus

[Simon Glass]

A menu is a key part of the expo design. It consists of a number of items
which the user can select from.

Add the initial implementation of this.

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

 boot/Makefile     |   2 +
 boot/scene_menu.c | 372 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 374 insertions(+)
 create mode 100644 boot/scene_menu.c

diff --git a/boot/Makefile b/boot/Makefile
index f0c31549213..0b30fcd64a9 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -47,6 +47,8 @@ ifdef CONFIG_SPL_BUILD
 obj-$(CONFIG_SPL_LOAD_FIT) += common_fit.o
 endif
 
+obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE) += expo.o scene.o scene_menu.o
+
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE) += vbe.o vbe_request.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE) += vbe_simple.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE_FW) += vbe_simple_fw.o
diff --git a/boot/scene_menu.c b/boot/scene_menu.c
new file mode 100644
index 00000000000..609295567c5
--- /dev/null
+++ b/boot/scene_menu.c
@@ -0,0 +1,372 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Implementation of a menu in a scene
+ *
+ * Copyright 2022 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#define LOG_CATEGORY	LOGC_BOOT
+
+#include <common.h>
+#include <dm.h>
+#include <expo.h>
+#include <malloc.h>
+#include <mapmem.h>
+#include <menu.h>
+#include <video.h>
+#include <video_console.h>
+#include <linux/input.h>
+#include "scene_internal.h"
+
+static void scene_menuitem_destroy(struct scene_menuitem *item)
+{
+	free(item->name);
+	free(item);
+}
+
+void scene_menu_destroy(struct scene_obj_menu *menu)
+{
+	struct scene_menuitem *item, *next;
+
+	list_for_each_entry_safe(item, next, &menu->item_head, sibling)
+		scene_menuitem_destroy(item);
+}
+
+/**
+ * menu_point_to_item() - Point to a particular menu item
+ *
+ * Sets the currently pointed-to / highlighted menu item
+ */
+static void menu_point_to_item(struct scene_obj_menu *menu, uint item_id)
+{
+	menu->cur_item_id = item_id;
+}
+
+int scene_menu_arrange(struct scene *scn, struct scene_obj_menu *menu)
+{
+	struct scene_menuitem *item;
+	int y, cur_y;
+	int ret;
+
+	y = menu->obj.y;
+	if (menu->title_id) {
+		ret = scene_obj_set_pos(scn, menu->title_id, menu->obj.x, y);
+		if (ret < 0)
+			return log_msg_ret("tit", ret);
+
+		ret = scene_obj_get_hw(scn, menu->title_id, NULL);
+		if (ret < 0)
+			return log_msg_ret("hei", ret);
+
+		y += ret * 2;
+	}
+
+	/*
+	 * Currently everything is hard-coded to particular columns so this
+	 * won't work on small displays and looks strange if the font size is
+	 * small. This can be updated once text measuring is supported in
+	 * vidconsole
+	 */
+	cur_y = -1;
+	list_for_each_entry(item, &menu->item_head, sibling) {
+		int height;
+
+		ret = scene_obj_get_hw(scn, item->desc_id, NULL);
+		if (ret < 0)
+			return log_msg_ret("get", ret);
+		height = ret;
+
+		if (item->flags & SCENEMIF_GAP_BEFORE)
+			y += height;
+
+		/* select an item if not done already */
+		if (!menu->cur_item_id)
+			menu_point_to_item(menu, item->id);
+
+		/*
+		 * Put the label on the left, then leave a space for the
+		 * pointer, then the key and the description
+		 */
+		if (item->label_id) {
+			ret = scene_obj_set_pos(scn, item->label_id, menu->obj.x,
+						y);
+			if (ret < 0)
+				return log_msg_ret("nam", ret);
+		}
+
+		ret = scene_obj_set_pos(scn, item->key_id, menu->obj.x + 230,
+					y);
+		if (ret < 0)
+			return log_msg_ret("key", ret);
+
+		ret = scene_obj_set_pos(scn, item->desc_id, menu->obj.x + 280,
+					y);
+		if (ret < 0)
+			return log_msg_ret("des", ret);
+
+		if (menu->cur_item_id == item->id)
+			cur_y = y;
+
+		if (item->preview_id) {
+			bool hide;
+
+			/*
+			 * put all previews on top of each other, on the right
+			 * size of the display
+			 */
+			ret = scene_obj_set_pos(scn, item->preview_id, -4, y);
+			if (ret < 0)
+				return log_msg_ret("prev", ret);
+
+			hide = menu->cur_item_id != item->id;
+			ret = scene_obj_set_hide(scn, item->preview_id, hide);
+			if (ret < 0)
+				return log_msg_ret("hid", ret);
+		}
+
+		y += height;
+	}
+
+	if (menu->pointer_id && cur_y != -1) {
+		/*
+		 * put the pointer to the right of and level with the item it
+		 * points to
+		 */
+		ret = scene_obj_set_pos(scn, menu->pointer_id,
+					menu->obj.x + 200, cur_y);
+		if (ret < 0)
+			return log_msg_ret("ptr", ret);
+	}
+
+	return 0;
+}
+
+int scene_menu_add(struct scene *scn, const char *name, uint id,
+		   struct scene_obj_menu **menup)
+{
+	struct scene_obj_menu *menu;
+	int ret;
+
+	ret = scene_obj_add(scn, name, id, SCENEOBJT_MENU,
+			    sizeof(struct scene_obj_menu),
+			    (struct scene_obj **)&menu);
+	if (ret < 0)
+		return log_msg_ret("obj", -ENOMEM);
+
+	if (menup)
+		*menup = menu;
+	INIT_LIST_HEAD(&menu->item_head);
+
+	ret = scene_menu_arrange(scn, menu);
+	if (ret)
+		return log_msg_ret("pos", ret);
+
+	return menu->obj.id;
+}
+
+static struct scene_menuitem *scene_menu_find_key(struct scene *scn,
+						  struct scene_obj_menu *menu,
+						  int key)
+{
+	struct scene_menuitem *item;
+
+	list_for_each_entry(item, &menu->item_head, sibling) {
+		if (item->key_id) {
+			struct scene_obj_txt *txt;
+
+			txt = scene_obj_find(scn, item->key_id, SCENEOBJT_TEXT);
+			if (txt && txt->str && *txt->str == key)
+				return item;
+		}
+	}
+
+	return NULL;
+}
+
+int scene_menu_send_key(struct scene *scn, struct scene_obj_menu *menu, int key,
+			struct expo_action *event)
+{
+	struct scene_menuitem *item, *cur, *key_item;
+
+	cur = NULL;
+	key_item = NULL;
+
+	if (!list_empty(&menu->item_head)) {
+		list_for_each_entry(item, &menu->item_head, sibling) {
+			/* select an item if not done already */
+			if (menu->cur_item_id == item->id) {
+				cur = item;
+				break;
+			}
+		}
+	}
+
+	if (!cur)
+		return -ENOTTY;
+
+	switch (key) {
+	case BKEY_UP:
+		if (item != list_first_entry(&menu->item_head,
+					     struct scene_menuitem, sibling)) {
+			item = list_entry(item->sibling.prev,
+					  struct scene_menuitem, sibling);
+			event->type = EXPOACT_POINT;
+			event->select.id = item->id;
+			log_debug("up to item %d\n", event->select.id);
+		}
+		break;
+	case BKEY_DOWN:
+		if (!list_is_last(&item->sibling, &menu->item_head)) {
+			item = list_entry(item->sibling.next,
+					  struct scene_menuitem, sibling);
+			event->type = EXPOACT_POINT;
+			event->select.id = item->id;
+			log_debug("down to item %d\n", event->select.id);
+		}
+		break;
+	case BKEY_SELECT:
+		event->type = EXPOACT_SELECT;
+		event->select.id = item->id;
+		log_debug("select item %d\n", event->select.id);
+		break;
+	case BKEY_QUIT:
+		event->type = EXPOACT_QUIT;
+		log_debug("quit\n");
+		break;
+	case '0'...'9':
+		key_item = scene_menu_find_key(scn, menu, key);
+		if (key_item) {
+			event->type = EXPOACT_SELECT;
+			event->select.id = key_item->id;
+		}
+		break;
+	}
+
+	menu_point_to_item(menu, item->id);
+
+	return 0;
+}
+
+int scene_menuitem_add(struct scene *scn, uint menu_id, const char *name,
+		       uint id, uint key_id, uint label_id, uint desc_id,
+		       uint preview_id, uint flags,
+		       struct scene_menuitem **itemp)
+{
+	struct scene_obj_menu *menu;
+	struct scene_menuitem *item;
+	int ret;
+
+	menu = scene_obj_find(scn, menu_id, SCENEOBJT_MENU);
+	if (!menu)
+		return log_msg_ret("find", -ENOENT);
+
+	/* Check that the text ID is valid */
+	if (!scene_obj_find(scn, desc_id, SCENEOBJT_TEXT))
+		return log_msg_ret("txt", -EINVAL);
+
+	item = calloc(1, sizeof(struct scene_obj_menu));
+	if (!item)
+		return log_msg_ret("item", -ENOMEM);
+	item->name = strdup(name);
+	if (!item->name) {
+		free(item);
+		return log_msg_ret("name", -ENOMEM);
+	}
+
+	item->id = resolve_id(scn->expo, id);
+	item->key_id = key_id;
+	item->label_id = label_id;
+	item->desc_id = desc_id;
+	item->preview_id = preview_id;
+	item->flags = flags;
+	list_add_tail(&item->sibling, &menu->item_head);
+
+	ret = scene_menu_arrange(scn, menu);
+	if (ret)
+		return log_msg_ret("pos", ret);
+
+	if (itemp)
+		*itemp = item;
+
+	return item->id;
+}
+
+int scene_menu_set_title(struct scene *scn, uint id, uint title_id)
+{
+	struct scene_obj_menu *menu;
+	struct scene_obj_txt *txt;
+
+	menu = scene_obj_find(scn, id, SCENEOBJT_MENU);
+	if (!menu)
+		return log_msg_ret("menu", -ENOENT);
+
+	/* Check that the ID is valid */
+	if (title_id) {
+		txt = scene_obj_find(scn, title_id, SCENEOBJT_TEXT);
+		if (!txt)
+			return log_msg_ret("txt", -EINVAL);
+	}
+
+	menu->title_id = title_id;
+
+	return 0;
+}
+
+int scene_menu_set_pointer(struct scene *scn, uint id, uint pointer_id)
+{
+	struct scene_obj_menu *menu;
+	struct scene_obj *obj;
+
+	menu = scene_obj_find(scn, id, SCENEOBJT_MENU);
+	if (!menu)
+		return log_msg_ret("menu", -ENOENT);
+
+	/* Check that the ID is valid */
+	if (pointer_id) {
+		obj = scene_obj_find(scn, pointer_id, SCENEOBJT_NONE);
+		if (!obj)
+			return log_msg_ret("obj", -EINVAL);
+	}
+
+	menu->pointer_id = pointer_id;
+
+	return 0;
+}
+
+int scene_menu_display(struct scene_obj_menu *menu)
+{
+	struct scene *scn = menu->obj.scene;
+	struct scene_obj_txt *pointer;
+	struct scene_menuitem *item;
+
+	printf("U-Boot    :    Boot Menu\n\n");
+	if (menu->title_id) {
+		struct scene_obj_txt *txt;
+
+		txt = scene_obj_find(scn, menu->title_id, SCENEOBJT_TEXT);
+		if (!txt)
+			return log_msg_ret("txt", -EINVAL);
+
+		printf("%s\n\n", txt->str);
+	}
+
+	if (list_empty(&menu->item_head))
+		return 0;
+
+	pointer = scene_obj_find(scn, menu->pointer_id, SCENEOBJT_TEXT);
+
+	list_for_each_entry(item, &menu->item_head, sibling) {
+		struct scene_obj_txt *key = NULL, *label = NULL;
+		struct scene_obj_txt *desc = NULL;
+
+		key = scene_obj_find(scn, item->key_id, SCENEOBJT_TEXT);
+		label = scene_obj_find(scn, item->label_id, SCENEOBJT_TEXT);
+		desc = scene_obj_find(scn, item->desc_id, SCENEOBJT_TEXT);
+		printf("%3s  %3s  %-10s  %s\n",
+		       pointer && menu->cur_item_id == item->id ? pointer->str :
+		       "", key->str, label->str, desc->str);
+	}
+
+	return -ENOTSUPP;
+}
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 20/24] expo: Add basic tests

[Simon Glass]

Add some tests for the expo, including setting up and rendering an expo.

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

 test/boot/Makefile |   2 +
 test/boot/expo.c   | 510 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 512 insertions(+)
 create mode 100644 test/boot/expo.c

diff --git a/test/boot/Makefile b/test/boot/Makefile
index 167629a03ee..820a374d49b 100644
--- a/test/boot/Makefile
+++ b/test/boot/Makefile
@@ -5,6 +5,8 @@
 obj-$(CONFIG_BOOTSTD) += bootdev.o bootstd_common.o bootflow.o bootmeth.o
 obj-$(CONFIG_FIT) += image.o
 
+obj-$(CONFIG_EXPO) += expo.o
+
 ifdef CONFIG_OF_LIVE
 obj-$(CONFIG_BOOTMETH_VBE_SIMPLE) += vbe_simple.o
 endif
diff --git a/test/boot/expo.c b/test/boot/expo.c
new file mode 100644
index 00000000000..50718d39558
--- /dev/null
+++ b/test/boot/expo.c
@@ -0,0 +1,510 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2022 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <expo.h>
+#include <menu.h>
+#include <video.h>
+#include <linux/input.h>
+#include <test/suites.h>
+#include <test/ut.h>
+#include "bootstd_common.h"
+#include "../../boot/scene_internal.h"
+
+enum {
+	/* scenes */
+	SCENE1		= 7,
+	SCENE2,
+
+	/* objects */
+	OBJ_LOGO,
+	OBJ_TEXT,
+	OBJ_TEXT2,
+	OBJ_MENU,
+	OBJ_MENU_TITLE,
+
+	/* menu items */
+	ITEM1,
+	ITEM1_LABEL,
+	ITEM1_DESC,
+	ITEM1_KEY,
+	ITEM1_PREVIEW,
+
+	ITEM2,
+	ITEM2_LABEL,
+	ITEM2_DESC,
+	ITEM2_KEY,
+	ITEM2_PREVIEW,
+
+	/* pointer to current item */
+	POINTER_TEXT,
+};
+
+#define BAD_POINTER	((void *)1)
+
+/* names for various things */
+#define EXPO_NAME	"my menus"
+#define SCENE_NAME1	"main"
+#define SCENE_NAME2	"second"
+#define SCENE_TITLE	"Main Menu"
+#define LOGO_NAME	"logo"
+
+/* Check base expo support */
+static int expo_base(struct unit_test_state *uts)
+{
+	struct udevice *dev;
+	struct expo *exp;
+	ulong start_mem;
+	char name[100];
+	int i;
+
+	ut_assertok(uclass_first_device_err(UCLASS_VIDEO, &dev));
+
+	start_mem = ut_check_free();
+
+	exp = NULL;
+	strcpy(name, EXPO_NAME);
+	ut_assertok(expo_new(name, NULL, &exp));
+	*name = '\0';
+	ut_assertnonnull(exp);
+	ut_asserteq(0, exp->scene_id);
+	ut_asserteq(0, exp->next_id);
+
+	/* Make sure the name was allocated */
+	ut_assertnonnull(exp->name);
+	ut_asserteq_str(EXPO_NAME, exp->name);
+
+	ut_assertok(expo_set_display(exp, dev));
+	expo_destroy(exp);
+	ut_assertok(ut_check_delta(start_mem));
+
+	/* test handling out-of-memory conditions */
+	for (i = 0; i < 2; i++) {
+		struct expo *exp2;
+
+		malloc_enable_testing(i);
+		exp2 = BAD_POINTER;
+		ut_asserteq(-ENOMEM, expo_new(EXPO_NAME, NULL, &exp2));
+		ut_asserteq_ptr(BAD_POINTER, exp2);
+		malloc_disable_testing();
+	}
+
+	return 0;
+}
+BOOTSTD_TEST(expo_base, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
+
+/* Check creating a scene */
+static int expo_scene(struct unit_test_state *uts)
+{
+	struct scene *scn;
+	struct expo *exp;
+	ulong start_mem;
+	char name[100];
+	int id;
+
+	start_mem = ut_check_free();
+
+	ut_assertok(expo_new(EXPO_NAME, NULL, &exp));
+
+	scn = NULL;
+	ut_asserteq(0, exp->next_id);
+	strcpy(name, SCENE_NAME1);
+	id = scene_new(exp, name, SCENE1, &scn);
+	*name = '\0';
+	ut_assertnonnull(scn);
+	ut_asserteq(SCENE1, id);
+	ut_asserteq(SCENE1 + 1, exp->next_id);
+	ut_asserteq_ptr(exp, scn->expo);
+
+	/* Make sure the name was allocated */
+	ut_assertnonnull(scn->name);
+	ut_asserteq_str(SCENE_NAME1, scn->name);
+
+	/* Set the title */
+	strcpy(name, SCENE_TITLE);
+	ut_assertok(scene_title_set(scn, name));
+	*name = '\0';
+	ut_assertnonnull(scn->title);
+	ut_asserteq_str(SCENE_TITLE, scn->title);
+
+	/* Use an allocated ID */
+	scn = NULL;
+	id = scene_new(exp, SCENE_NAME2, 0, &scn);
+	ut_assertnonnull(scn);
+	ut_asserteq(SCENE2, id);
+	ut_asserteq(SCENE2 + 1, exp->next_id);
+	ut_asserteq_ptr(exp, scn->expo);
+
+	ut_asserteq_str(SCENE_NAME2, scn->name);
+
+	expo_destroy(exp);
+
+	ut_assertok(ut_check_delta(start_mem));
+
+	return 0;
+}
+BOOTSTD_TEST(expo_scene, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
+
+/* Check creating a scene with objects */
+static int expo_object(struct unit_test_state *uts)
+{
+	struct scene_obj_img *img;
+	struct scene_obj_txt *txt;
+	struct scene *scn;
+	struct expo *exp;
+	ulong start_mem;
+	char name[100];
+	char *data;
+	int id;
+
+	start_mem = ut_check_free();
+
+	ut_assertok(expo_new(EXPO_NAME, NULL, &exp));
+	id = scene_new(exp, SCENE_NAME1, SCENE1, &scn);
+	ut_assert(id > 0);
+
+	ut_asserteq(0, scene_obj_count(scn));
+
+	data = NULL;
+	strcpy(name, LOGO_NAME);
+	id = scene_img_add(scn, name, OBJ_LOGO, data, &img);
+	ut_assert(id > 0);
+	*name = '\0';
+	ut_assertnonnull(img);
+	ut_asserteq(OBJ_LOGO, id);
+	ut_asserteq(OBJ_LOGO + 1, exp->next_id);
+	ut_asserteq_ptr(scn, img->obj.scene);
+	ut_asserteq(SCENEOBJT_IMAGE, img->obj.type);
+
+	ut_asserteq_ptr(data, img->data);
+
+	/* Make sure the name was allocated */
+	ut_assertnonnull(scn->name);
+	ut_asserteq_str(SCENE_NAME1, scn->name);
+
+	ut_asserteq(1, scene_obj_count(scn));
+
+	id = scene_txt_add(scn, "text", OBJ_TEXT, "my string", &txt);
+	ut_assert(id > 0);
+	ut_assertnonnull(txt);
+	ut_asserteq(OBJ_TEXT, id);
+	ut_asserteq(SCENEOBJT_TEXT, txt->obj.type);
+	ut_asserteq(2, scene_obj_count(scn));
+
+	/* Check passing NULL as the final parameter */
+	id = scene_txt_add(scn, "text2", OBJ_TEXT, "another string", NULL);
+	ut_assert(id > 0);
+	ut_asserteq(3, scene_obj_count(scn));
+
+	expo_destroy(exp);
+
+	ut_assertok(ut_check_delta(start_mem));
+
+	return 0;
+}
+BOOTSTD_TEST(expo_object, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
+
+/* Check setting object attributes */
+static int expo_object_attr(struct unit_test_state *uts)
+{
+	struct scene_obj_menu *menu;
+	struct scene_obj_img *img;
+	struct scene_obj_txt *txt;
+	struct scene *scn;
+	struct expo *exp;
+	ulong start_mem;
+	char name[100];
+	char *data;
+	int id;
+
+	start_mem = ut_check_free();
+
+	ut_assertok(expo_new(EXPO_NAME, NULL, &exp));
+	id = scene_new(exp, SCENE_NAME1, SCENE1, &scn);
+	ut_assert(id > 0);
+
+	data = NULL;
+	id = scene_img_add(scn, LOGO_NAME, OBJ_LOGO, data, &img);
+	ut_assert(id > 0);
+
+	ut_assertok(scene_obj_set_pos(scn, OBJ_LOGO, 123, 456));
+	ut_asserteq(123, img->obj.x);
+	ut_asserteq(456, img->obj.y);
+
+	ut_asserteq(-ENOENT, scene_obj_set_pos(scn, OBJ_TEXT2, 0, 0));
+
+	id = scene_txt_add(scn, "text", OBJ_TEXT, "my string", &txt);
+	ut_assert(id > 0);
+
+	strcpy(name, "font2");
+	ut_assertok(scene_txt_set_font(scn, OBJ_TEXT, name, 42));
+	ut_asserteq_ptr(name, txt->font_name);
+	ut_asserteq(42, txt->font_size);
+
+	ut_asserteq(-ENOENT, scene_txt_set_font(scn, OBJ_TEXT2, name, 42));
+
+	id = scene_menu_add(scn, "main", OBJ_MENU, &menu);
+	ut_assert(id > 0);
+
+	ut_assertok(scene_menu_set_title(scn, OBJ_MENU, OBJ_TEXT));
+
+	ut_asserteq(-ENOENT, scene_menu_set_title(scn, OBJ_TEXT2, OBJ_TEXT));
+	ut_asserteq(-EINVAL, scene_menu_set_title(scn, OBJ_MENU, OBJ_TEXT2));
+
+	expo_destroy(exp);
+
+	ut_assertok(ut_check_delta(start_mem));
+
+	return 0;
+}
+BOOTSTD_TEST(expo_object_attr, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
+
+/* Check creating a scene with a menu */
+static int expo_object_menu(struct unit_test_state *uts)
+{
+	struct scene_obj_menu *menu;
+	struct scene_menuitem *item;
+	int id, label_id, desc_id, key_id, pointer_id, preview_id;
+	struct scene_obj_txt *ptr, *name1, *desc1, *key1, *tit, *prev1;
+	struct scene *scn;
+	struct expo *exp;
+	ulong start_mem;
+
+	start_mem = ut_check_free();
+
+	ut_assertok(expo_new(EXPO_NAME, NULL, &exp));
+	id = scene_new(exp, SCENE_NAME1, SCENE1, &scn);
+	ut_assert(id > 0);
+
+	id = scene_menu_add(scn, "main", OBJ_MENU, &menu);
+	ut_assert(id > 0);
+	ut_assertnonnull(menu);
+	ut_asserteq(OBJ_MENU, id);
+	ut_asserteq(SCENEOBJT_MENU, menu->obj.type);
+	ut_asserteq(0, menu->title_id);
+	ut_asserteq(0, menu->pointer_id);
+
+	ut_assertok(scene_obj_set_pos(scn, OBJ_MENU, 50, 400));
+	ut_asserteq(50, menu->obj.x);
+	ut_asserteq(400, menu->obj.y);
+
+	id = scene_txt_add(scn, "title", OBJ_MENU_TITLE, "Main Menu", &tit);
+	ut_assert(id > 0);
+	ut_assertok(scene_menu_set_title(scn, OBJ_MENU, OBJ_MENU_TITLE));
+	ut_asserteq(OBJ_MENU_TITLE, menu->title_id);
+
+	pointer_id = scene_txt_add(scn, "cur_item", POINTER_TEXT, ">", &ptr);
+	ut_assert(pointer_id > 0);
+
+	ut_assertok(scene_menu_set_pointer(scn, OBJ_MENU, POINTER_TEXT));
+	ut_asserteq(POINTER_TEXT, menu->pointer_id);
+
+	label_id = scene_txt_add(scn, "label1", ITEM1_LABEL, "Play", &name1);
+	ut_assert(label_id > 0);
+
+	desc_id = scene_txt_add(scn, "desc1", ITEM1_DESC, "Lord Melchett",
+				&desc1);
+	ut_assert(desc_id > 0);
+
+	key_id = scene_txt_add(scn, "item1-key", ITEM1_KEY, "1", &key1);
+	ut_assert(key_id > 0);
+
+	preview_id = scene_txt_add(scn, "item1-preview", ITEM1_PREVIEW,
+				   "(preview1)", &prev1);
+	ut_assert(preview_id > 0);
+
+	id = scene_menuitem_add(scn, OBJ_MENU, "linux", ITEM1, ITEM1_KEY,
+				ITEM1_LABEL, ITEM1_DESC, ITEM1_PREVIEW, 0,
+				&item);
+	ut_asserteq(ITEM1, id);
+	ut_asserteq(id, item->id);
+	ut_asserteq(key_id, item->key_id);
+	ut_asserteq(label_id, item->label_id);
+	ut_asserteq(desc_id, item->desc_id);
+	ut_asserteq(preview_id, item->preview_id);
+
+	/* adding an item should cause the first item to become current */
+	ut_asserteq(id, menu->cur_item_id);
+
+	/* the title should be at the top */
+	ut_asserteq(menu->obj.x, tit->obj.x);
+	ut_asserteq(menu->obj.y, tit->obj.y);
+
+	/* the first item should be next */
+	ut_asserteq(menu->obj.x, name1->obj.x);
+	ut_asserteq(menu->obj.y + 32, name1->obj.y);
+
+	ut_asserteq(menu->obj.x + 230, key1->obj.x);
+	ut_asserteq(menu->obj.y + 32, key1->obj.y);
+
+	ut_asserteq(menu->obj.x + 200, ptr->obj.x);
+	ut_asserteq(menu->obj.y + 32, ptr->obj.y);
+
+	ut_asserteq(menu->obj.x + 280, desc1->obj.x);
+	ut_asserteq(menu->obj.y + 32, desc1->obj.y);
+
+	ut_asserteq(-4, prev1->obj.x);
+	ut_asserteq(menu->obj.y + 32, prev1->obj.y);
+	ut_asserteq(false, prev1->obj.hide);
+
+	expo_destroy(exp);
+
+	ut_assertok(ut_check_delta(start_mem));
+
+	return 0;
+}
+BOOTSTD_TEST(expo_object_menu, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
+
+/* Check rendering a scene */
+static int expo_render_image(struct unit_test_state *uts)
+{
+	struct scene_obj_menu *menu;
+	struct scene *scn, *scn2;
+	struct expo_action act;
+	struct scene_obj *obj;
+	struct udevice *dev;
+	struct expo *exp;
+	int id;
+
+	console_record_reset_enable();
+	ut_assertok(uclass_first_device_err(UCLASS_VIDEO, &dev));
+
+	ut_assertok(expo_new(EXPO_NAME, NULL, &exp));
+	id = scene_new(exp, SCENE_NAME1, SCENE1, &scn);
+	ut_assert(id > 0);
+	ut_assertok(expo_set_display(exp, dev));
+
+	id = scene_img_add(scn, "logo", OBJ_LOGO, video_get_u_boot_logo(), NULL);
+	ut_assert(id > 0);
+	ut_assertok(scene_obj_set_pos(scn, OBJ_LOGO, 50, 20));
+
+	id = scene_txt_add(scn, "text", OBJ_TEXT, "my string", NULL);
+	ut_assert(id > 0);
+	ut_assertok(scene_txt_set_font(scn, OBJ_TEXT, "cantoraone_regular",
+				       40));
+	ut_assertok(scene_obj_set_pos(scn, OBJ_TEXT, 400, 100));
+
+	id = scene_txt_add(scn, "text", OBJ_TEXT2, "another string", NULL);
+	ut_assert(id > 0);
+	ut_assertok(scene_txt_set_font(scn, OBJ_TEXT2, "nimbus_sans_l_regular",
+				       60));
+	ut_assertok(scene_obj_set_pos(scn, OBJ_TEXT2, 200, 600));
+
+	id = scene_menu_add(scn, "main", OBJ_MENU, &menu);
+	ut_assert(id > 0);
+
+	id = scene_txt_add(scn, "title", OBJ_MENU_TITLE, "Main Menu", NULL);
+	ut_assert(id > 0);
+	ut_assertok(scene_menu_set_title(scn, OBJ_MENU, OBJ_MENU_TITLE));
+
+	id = scene_txt_add(scn, "cur_item", POINTER_TEXT, ">", NULL);
+	ut_assert(id > 0);
+	ut_assertok(scene_menu_set_pointer(scn, OBJ_MENU, POINTER_TEXT));
+
+	id = scene_txt_add(scn, "label1", ITEM1_LABEL, "Play", NULL);
+	ut_assert(id > 0);
+	id = scene_txt_add(scn, "item1 txt", ITEM1_DESC, "Lord Melchett", NULL);
+	ut_assert(id > 0);
+	id = scene_txt_add(scn, "item1-key", ITEM1_KEY, "1", NULL);
+	ut_assert(id > 0);
+	id = scene_img_add(scn, "item1-preview", ITEM1_PREVIEW,
+			   video_get_u_boot_logo(), NULL);
+	id = scene_menuitem_add(scn, OBJ_MENU, "item1", ITEM1, ITEM1_KEY,
+				ITEM1_LABEL, ITEM1_DESC, ITEM1_PREVIEW, 0, NULL);
+	ut_assert(id > 0);
+
+	id = scene_txt_add(scn, "label2", ITEM2_LABEL, "Now", NULL);
+	ut_assert(id > 0);
+	id = scene_txt_add(scn, "item2 txt", ITEM2_DESC, "Lord Percy", NULL);
+	ut_assert(id > 0);
+	id = scene_txt_add(scn, "item2-key", ITEM2_KEY, "2", NULL);
+	ut_assert(id > 0);
+	id = scene_img_add(scn, "item2-preview", ITEM2_PREVIEW,
+			   video_get_u_boot_logo(), NULL);
+	ut_assert(id > 0);
+
+	id = scene_menuitem_add(scn, OBJ_MENU, "item2", ITEM2, ITEM2_KEY,
+				ITEM2_LABEL, ITEM2_DESC, ITEM2_PREVIEW, 0, NULL);
+	ut_assert(id > 0);
+
+	ut_assertok(scene_obj_set_pos(scn, OBJ_MENU, 50, 400));
+
+	scn2 = expo_lookup_scene_id(exp, SCENE1);
+	ut_asserteq_ptr(scn, scn2);
+	scn2 = expo_lookup_scene_id(exp, SCENE2);
+	ut_assertnull(scn2);
+
+	/* render without a scene */
+	ut_asserteq(-ECHILD, expo_render(exp));
+
+	/* render it */
+	expo_set_scene_id(exp, SCENE1);
+	ut_assertok(expo_render(exp));
+
+	/* move down */
+	ut_assertok(expo_send_key(exp, BKEY_DOWN));
+
+	ut_assertok(expo_action_get(exp, &act));
+
+	ut_asserteq(EXPOACT_POINT, act.type);
+	ut_asserteq(ITEM2, act.select.id);
+	ut_assertok(expo_render(exp));
+
+	/* make sure only the preview for the second item is shown */
+	obj = scene_obj_find(scn, ITEM1_PREVIEW, SCENEOBJT_NONE);
+	ut_asserteq(true, obj->hide);
+
+	obj = scene_obj_find(scn, ITEM2_PREVIEW, SCENEOBJT_NONE);
+	ut_asserteq(false, obj->hide);
+
+	/* select it */
+	ut_assertok(expo_send_key(exp, BKEY_SELECT));
+
+	ut_assertok(expo_action_get(exp, &act));
+	ut_asserteq(EXPOACT_SELECT, act.type);
+	ut_asserteq(ITEM2, act.select.id);
+
+	/* make sure the action doesn't come again */
+	ut_asserteq(-EAGAIN, expo_action_get(exp, &act));
+
+	/* make sure there was no console output */
+	ut_assert_console_end();
+
+	/* now try in text mode */
+	exp_set_text_mode(exp, true);
+	ut_assertok(expo_render(exp));
+
+	ut_assert_nextline("U-Boot    :    Boot Menu");
+	ut_assert_nextline("%s", "");
+	ut_assert_nextline("Main Menu");
+	ut_assert_nextline("%s", "");
+	ut_assert_nextline("       1  Play        Lord Melchett");
+	ut_assert_nextline("  >    2  Now         Lord Percy");
+
+	/* Move back up to the first item */
+	ut_assertok(expo_send_key(exp, BKEY_UP));
+
+	ut_assertok(expo_action_get(exp, &act));
+
+	ut_asserteq(EXPOACT_POINT, act.type);
+	ut_asserteq(ITEM1, act.select.id);
+
+	ut_assertok(expo_render(exp));
+	ut_assert_nextline("U-Boot    :    Boot Menu");
+	ut_assert_nextline("%s", "");
+	ut_assert_nextline("Main Menu");
+	ut_assert_nextline("%s", "");
+	ut_assert_nextline("  >    1  Play        Lord Melchett");
+	ut_assert_nextline("       2  Now         Lord Percy");
+
+	ut_assert_console_end();
+
+	expo_destroy(exp);
+
+	return 0;
+}
+BOOTSTD_TEST(expo_render_image, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 21/24] bootstd: Support creating a boot menu

[Simon Glass]

Create an expo to handle the boot menu. For now this is quite simple, with
just a header, some menu items and a pointer to show the current one.

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

 boot/Makefile            |   1 +
 boot/bootflow_internal.h |  37 ++++++
 boot/bootflow_menu.c     | 238 +++++++++++++++++++++++++++++++++++++++
 cmd/bootflow.c           |  37 +++++-
 include/bootflow.h       |  23 ++++
 5 files changed, 334 insertions(+), 2 deletions(-)
 create mode 100644 boot/bootflow_internal.h
 create mode 100644 boot/bootflow_menu.c

diff --git a/boot/Makefile b/boot/Makefile
index 0b30fcd64a9..f990e66f522 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SANDBOX) += bootmeth_sandbox.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SCRIPT) += bootmeth_script.o
 ifdef CONFIG_$(SPL_TPL_)BOOTSTD_FULL
 obj-$(CONFIG_$(SPL_TPL_)CMD_BOOTEFI_BOOTMGR) += bootmeth_efi_mgr.o
+obj-$(CONFIG_$(SPL_TPL_)BOOTSTD) += bootflow_menu.o
 endif
 
 obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += image-fdt.o
diff --git a/boot/bootflow_internal.h b/boot/bootflow_internal.h
new file mode 100644
index 00000000000..f2d279fd04f
--- /dev/null
+++ b/boot/bootflow_internal.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Internal header file for bootflow
+ *
+ * Copyright 2022 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#ifndef __BOOTFLOW_INTERNAL_H
+#define __BOOTFLOW_INTERNAL_H
+
+/* expo IDs for elements of the bootflow menu */
+enum {
+	START,
+
+	/* scene */
+	MAIN,
+
+	/* objects */
+	OBJ_U_BOOT_LOGO,
+	OBJ_MENU,
+	OBJ_PROMPT,
+	OBJ_MENU_TITLE,
+	OBJ_POINTER,
+
+	/* menu items / components (bootflow number is added to these) */
+	ITEM = 100,
+	ITEM_LABEL = 200,
+	ITEM_DESC = 300,
+	ITEM_KEY = 400,
+	ITEM_PREVIEW = 500,
+
+	/* left margin for the main menu */
+	MARGIN_LEFT	 = 100,
+};
+
+#endif /* __BOOTFLOW_INTERNAL_H */
diff --git a/boot/bootflow_menu.c b/boot/bootflow_menu.c
new file mode 100644
index 00000000000..0d7fb506f3f
--- /dev/null
+++ b/boot/bootflow_menu.c
@@ -0,0 +1,238 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Provide a menu of available bootflows and related options
+ *
+ * Copyright 2022 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#define LOG_CATEGORY UCLASS_BOOTSTD
+
+#include <common.h>
+#include <bootflow.h>
+#include <bootstd.h>
+#include <cli.h>
+#include <dm.h>
+#include <expo.h>
+#include <malloc.h>
+#include <menu.h>
+#include <video_console.h>
+#include <watchdog.h>
+#include <linux/delay.h>
+#include "bootflow_internal.h"
+
+/**
+ * struct menu_priv - information about the menu
+ *
+ * @num_bootflows: Number of bootflows in the menu
+ */
+struct menu_priv {
+	int num_bootflows;
+};
+
+int bootflow_menu_new(struct expo **expp)
+{
+	struct udevice *last_bootdev;
+	struct scene_obj_menu *menu;
+	struct menu_priv *priv;
+	struct bootflow *bflow;
+	struct scene *scn;
+	struct expo *exp;
+	void *logo;
+	int ret, i;
+
+	priv = calloc(1, sizeof(*priv));
+	if (!priv)
+		return log_msg_ret("prv", -ENOMEM);
+
+	ret = expo_new("bootflows", priv, &exp);
+	if (ret)
+		return log_msg_ret("exp", ret);
+
+	ret = scene_new(exp, "main", MAIN, &scn);
+	if (ret < 0)
+		return log_msg_ret("scn", ret);
+
+	ret |= scene_txt_add(scn, "prompt", OBJ_PROMPT,
+			     "Use UP and DOWN to choose, ENTER to select",
+			     NULL);
+
+	ret = scene_menu_add(scn, "main", OBJ_MENU, &menu);
+	ret |= scene_obj_set_pos(scn, OBJ_MENU, MARGIN_LEFT, 100);
+	ret |= scene_txt_add(scn, "title", OBJ_MENU_TITLE, "U-Boot - Boot Menu",
+			     NULL);
+	ret |= scene_menu_set_title(scn, OBJ_MENU, OBJ_PROMPT);
+
+	logo = video_get_u_boot_logo();
+	if (logo) {
+		ret |= scene_img_add(scn, "ulogo", OBJ_U_BOOT_LOGO, logo, NULL);
+		ret |= scene_obj_set_pos(scn, OBJ_U_BOOT_LOGO, -4, 4);
+	}
+
+	ret |= scene_txt_add(scn, "cur_item", OBJ_POINTER, ">", NULL);
+	ret |= scene_menu_set_pointer(scn, OBJ_MENU, OBJ_POINTER);
+	if (ret < 0)
+		return log_msg_ret("new", -EINVAL);
+
+	last_bootdev = NULL;
+	for (ret = bootflow_first_glob(&bflow), i = 0; !ret && i < 36;
+	     ret = bootflow_next_glob(&bflow), i++) {
+		char str[2], *label, *key;
+		uint preview_id;
+		bool add_gap;
+
+		if (bflow->state != BOOTFLOWST_READY)
+			continue;
+
+		*str = i < 10 ? '0' + i : 'A' + i - 10;
+		str[1] = '\0';
+		key = strdup(str);
+		if (!key)
+			return log_msg_ret("key", -ENOMEM);
+		label = strdup(dev_get_parent(bflow->dev)->name);
+		if (!label) {
+			free(key);
+			return log_msg_ret("nam", -ENOMEM);
+		}
+
+		add_gap = last_bootdev != bflow->dev;
+		last_bootdev = bflow->dev;
+
+		ret = scene_txt_add(scn, "label", ITEM_LABEL + i, label, NULL);
+		ret = scene_txt_add(scn, "desc", ITEM_DESC + i,
+				    bflow->os_name ? bflow->os_name :
+				    bflow->name, NULL);
+		ret |= scene_txt_add(scn, "key", ITEM_KEY + i, key, NULL);
+		preview_id = 0;
+		if (bflow->logo) {
+			preview_id = ITEM_PREVIEW + i;
+			ret |= scene_img_add(scn, "preview", preview_id,
+					     bflow->logo, NULL);
+		}
+		ret |= scene_menuitem_add(scn, OBJ_MENU, "item", ITEM + i,
+					  ITEM_KEY + i, ITEM_LABEL + i,
+					  ITEM_DESC + i, preview_id,
+					  add_gap ? SCENEMIF_GAP_BEFORE : 0,
+					  NULL);
+
+		if (ret < 0)
+			return log_msg_ret("itm", -EINVAL);
+		ret = 0;
+		priv->num_bootflows++;
+	}
+
+	*expp = exp;
+
+	return 0;
+}
+
+int bootflow_menu_run(struct bootstd_priv *std, bool text_mode,
+		      struct bootflow **bflowp)
+{
+	struct cli_ch_state s_cch, *cch = &s_cch;
+	struct bootflow *sel_bflow;
+	struct udevice *dev;
+	struct expo *exp;
+	uint sel_id;
+	bool done;
+	int ret;
+
+	cli_ch_init(cch);
+
+	sel_bflow = NULL;
+	*bflowp = NULL;
+
+	ret = bootflow_menu_new(&exp);
+	if (ret)
+		return log_msg_ret("exp", ret);
+
+	/* For now we only support a video console */
+	ret = uclass_first_device_err(UCLASS_VIDEO, &dev);
+	if (ret)
+		return log_msg_ret("vid", ret);
+	ret = expo_set_display(exp, dev);
+	if (ret)
+		return log_msg_ret("dis", ret);
+
+	ret = expo_set_scene_id(exp, MAIN);
+	if (ret)
+		return log_msg_ret("scn", ret);
+
+	if (text_mode)
+		exp_set_text_mode(exp, text_mode);
+
+	done = false;
+	do {
+		struct expo_action act;
+		int ichar, key;
+
+		ret = expo_render(exp);
+		if (ret)
+			break;
+
+		ichar = cli_ch_process(cch, 0);
+		if (!ichar) {
+			while (!ichar && !tstc()) {
+				WATCHDOG_RESET();
+				mdelay(2);
+				ichar = cli_ch_process(cch, -ETIMEDOUT);
+			}
+			if (!ichar) {
+				ichar = getchar();
+				ichar = cli_ch_process(cch, ichar);
+			}
+		}
+
+		key = 0;
+		if (ichar) {
+			key = bootmenu_conv_key(ichar);
+			if (key == BKEY_NONE)
+				key = ichar;
+		}
+		if (!key)
+			continue;
+
+		ret = expo_send_key(exp, key);
+		if (ret)
+			break;
+
+		ret = expo_action_get(exp, &act);
+		if (!ret) {
+			switch (act.type) {
+			case EXPOACT_SELECT:
+				sel_id = act.select.id;
+				done = true;
+				break;
+			case EXPOACT_QUIT:
+				done = true;
+				break;
+			default:
+				break;
+			}
+		}
+	} while (!done);
+
+	if (ret)
+		return log_msg_ret("end", ret);
+
+	if (sel_id) {
+		struct bootflow *bflow;
+		int i;
+
+		for (ret = bootflow_first_glob(&bflow), i = 0; !ret && i < 36;
+		     ret = bootflow_next_glob(&bflow), i++) {
+			if (i == sel_id - ITEM) {
+				sel_bflow = bflow;
+				break;
+			}
+		}
+	}
+
+	expo_destroy(exp);
+
+	if (!sel_bflow)
+		return -EAGAIN;
+	*bflowp = sel_bflow;
+
+	return 0;
+}
diff --git a/cmd/bootflow.c b/cmd/bootflow.c
index 495ef85f25b..2b6ed26fdcb 100644
--- a/cmd/bootflow.c
+++ b/cmd/bootflow.c
@@ -389,6 +389,37 @@ static int do_bootflow_boot(struct cmd_tbl *cmdtp, int flag, int argc,
 
 	return 0;
 }
+
+static int do_bootflow_menu(struct cmd_tbl *cmdtp, int flag, int argc,
+			    char *const argv[])
+{
+	struct bootstd_priv *std;
+	struct bootflow *bflow;
+	bool text_mode = false;
+	int ret;
+
+	if (argc > 1 && *argv[1] == '-')
+		text_mode = strchr(argv[1], 't');
+
+	ret = bootstd_get_priv(&std);
+	if (ret)
+		return CMD_RET_FAILURE;
+
+	ret = bootflow_menu_run(std, text_mode, &bflow);
+	if (ret) {
+		if (ret == -EAGAIN)
+			printf("Nothing chosen\n");
+		else
+			printf("Menu failed (err=%d)\n", ret);
+
+		return CMD_RET_FAILURE;
+	}
+
+	printf("Selected: %s\n", bflow->os_name ? bflow->os_name : bflow->name);
+	std->cur_bootflow = bflow;
+
+	return 0;
+}
 #endif /* CONFIG_CMD_BOOTFLOW_FULL */
 
 #ifdef CONFIG_SYS_LONGHELP
@@ -398,7 +429,8 @@ static char bootflow_help_text[] =
 	"bootflow list [-e]             - list scanned bootflows (-e errors)\n"
 	"bootflow select [<num>|<name>] - select a bootflow\n"
 	"bootflow info [-d]             - show info on current bootflow (-d dump bootflow)\n"
-	"bootflow boot                  - boot current bootflow (or first available if none selected)";
+	"bootflow boot                  - boot current bootflow (or first available if none selected)\n"
+	"bootflow menu [-t]             - show a menu of available bootflows";
 #else
 	"scan - boot first available bootflow\n";
 #endif
@@ -410,6 +442,7 @@ U_BOOT_CMD_WITH_SUBCMDS(bootflow, "Boot flows", bootflow_help_text,
 	U_BOOT_SUBCMD_MKENT(list, 2, 1, do_bootflow_list),
 	U_BOOT_SUBCMD_MKENT(select, 2, 1, do_bootflow_select),
 	U_BOOT_SUBCMD_MKENT(info, 2, 1, do_bootflow_info),
-	U_BOOT_SUBCMD_MKENT(boot, 1, 1, do_bootflow_boot)
+	U_BOOT_SUBCMD_MKENT(boot, 1, 1, do_bootflow_boot),
+	U_BOOT_SUBCMD_MKENT(menu, 2, 1, do_bootflow_menu),
 #endif
 );
diff --git a/include/bootflow.h b/include/bootflow.h
index 8a07ab30191..e7a09568f1b 100644
--- a/include/bootflow.h
+++ b/include/bootflow.h
@@ -9,6 +9,9 @@
 
 #include <linux/list.h>
 
+struct bootstd_priv;
+struct expo;
+
 /**
  * enum bootflow_state_t - states that a particular bootflow can be in
  *
@@ -336,4 +339,24 @@ int bootflow_iter_uses_network(const struct bootflow_iter *iter);
  */
 int bootflow_iter_uses_system(const struct bootflow_iter *iter);
 
+/**
+ * bootflow_menu_new() - Create a new bootflow menu
+ *
+ * @expp: Returns the expo created
+ * Returns 0 on success, -ve on error
+ */
+int bootflow_menu_new(struct expo **expp);
+
+/**
+ * bootflow_menu_run() - Create and run a menu of available bootflows
+ *
+ * @std: Bootstd information
+ * @text_mode: Uses a text-based menu suitable for a serial port
+ * @bflowp: Returns chosen bootflow (set to NULL if nothing is chosen)
+ * @return 0 if an option was chosen, -EAGAIN if nothing was chosen, -ve on
+ * error
+ */
+int bootflow_menu_run(struct bootstd_priv *std, bool text_mode,
+		      struct bootflow **bflowp);
+
 #endif
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 22/24] bootstd: Add a test for the bootstd menu

[Simon Glass]

Add a test which checks that two operating systems can be displayed in a
menu, allowing one to be selected.

Enable a few things on snow so that the unit tests build.

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

 arch/sandbox/dts/test.dts            |  11 ++
 configs/snow_defconfig               |   4 +
 test/boot/bootflow.c                 |  51 +++++++
 test/py/tests/bootstd/armbian.bmp.xz | Bin 0 -> 1384 bytes
 test/py/tests/bootstd/mmc4.img.xz    | Bin 0 -> 7072 bytes
 test/py/tests/test_ut.py             | 216 ++++++++++++++++++++++++---
 6 files changed, 264 insertions(+), 18 deletions(-)
 create mode 100644 test/py/tests/bootstd/armbian.bmp.xz
 create mode 100644 test/py/tests/bootstd/mmc4.img.xz

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 7303e7cfa2f..9e258fdc26b 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -94,6 +94,10 @@
 			compatible = "u-boot,distro-efi";
 		};
 
+		theme {
+			font-size = <30>;
+		};
+
 		/*
 		 * This is used for the VBE OS-request tests. A FAT filesystem
 		 * created in a partition with the VBE information appearing
@@ -1003,6 +1007,13 @@
 		non-removable;
 	};
 
+	/* This is used for bootstd bootmenu tests */
+	mmc4 {
+		status = "disabled";
+		compatible = "sandbox,mmc";
+		filename = "mmc4.img";
+	};
+
 	pch {
 		compatible = "sandbox,pch";
 	};
diff --git a/configs/snow_defconfig b/configs/snow_defconfig
index c83c89b3094..2116905d18e 100644
--- a/configs/snow_defconfig
+++ b/configs/snow_defconfig
@@ -28,7 +28,11 @@ CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
 CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x2050000
 CONFIG_FIT=y
 CONFIG_FIT_BEST_MATCH=y
+CONFIG_BOOTSTD_FULL=y
 CONFIG_SILENT_CONSOLE=y
+CONFIG_BLOBLIST=y
+# CONFIG_SPL_BLOBLIST is not set
+CONFIG_BLOBLIST_ADDR=0x43d00000
 # CONFIG_SPL_FRAMEWORK is not set
 CONFIG_SPL_FOOTPRINT_LIMIT=y
 CONFIG_SPL_MAX_FOOTPRINT=0x3800
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index 00dfd990687..abafa44b2ed 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -11,15 +11,21 @@
 #include <bootflow.h>
 #include <bootmeth.h>
 #include <bootstd.h>
+#include <cli.h>
 #include <dm.h>
 #ifdef CONFIG_SANDBOX
 #include <asm/test.h>
 #endif
+#include <dm/device-internal.h>
 #include <dm/lists.h>
 #include <test/suites.h>
 #include <test/ut.h>
 #include "bootstd_common.h"
 
+DECLARE_GLOBAL_DATA_PTR;
+
+extern U_BOOT_DRIVER(bootmeth_script);
+
 static int inject_response(struct unit_test_state *uts)
 {
 	/*
@@ -462,3 +468,48 @@ static int bootflow_cmd_boot(struct unit_test_state *uts)
 	return 0;
 }
 BOOTSTD_TEST(bootflow_cmd_boot, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
+
+/* Check 'bootflow menu' to select a bootflow */
+static int bootflow_cmd_menu(struct unit_test_state *uts)
+{
+	static const char *order[] = {"mmc2", "mmc1", "mmc4", NULL};
+	struct udevice *dev, *bootstd;
+	struct bootstd_priv *std;
+	const char **old_order;
+	char prev[3];
+	ofnode node;
+
+	/* Enable the mmc4 node since we need a second bootflow */
+	node = ofnode_path("/mmc4");
+	ut_assertok(lists_bind_fdt(gd->dm_root, node, &dev, NULL, false));
+
+	/* Enable the script bootmeth too */
+	ut_assertok(uclass_first_device_err(UCLASS_BOOTSTD, &bootstd));
+	ut_assertok(device_bind(bootstd, DM_DRIVER_REF(bootmeth_script),
+				"bootmeth_script", 0, ofnode_null(), &dev));
+
+	/* Change the order to include mmc4 */
+	std = dev_get_priv(bootstd);
+	old_order = std->bootdev_order;
+	std->bootdev_order = order;
+
+	console_record_reset_enable();
+	ut_assertok(run_command("bootflow scan", 0));
+	ut_assert_console_end();
+
+	/* Restore the order used by the device tree */
+	std->bootdev_order = old_order;
+
+	/* Add keypresses to move to and select the second one in the list */
+	prev[0] = CTL_CH('n');
+	prev[1] = '\r';
+	prev[2] = '\0';
+	ut_asserteq(2, console_in_puts(prev));
+
+	ut_assertok(run_command("bootflow menu", 0));
+	ut_assert_nextline("Selected: Armbian");
+	ut_assert_console_end();
+
+	return 0;
+}
+BOOTSTD_TEST(bootflow_cmd_menu, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
diff --git a/test/py/tests/bootstd/armbian.bmp.xz b/test/py/tests/bootstd/armbian.bmp.xz
new file mode 100644
index 0000000000000000000000000000000000000000..ad137ea6e6df5c51d0a98a4be190c4cb4c48fc76
GIT binary patch
literal 1384
zcmV-u1(*8$H+ooF000E$*0e?f03iV!0000G&sfahT|osUT>v2yL`vV4GWns~_Xm1L
zxE>Ubk@J6u_+aW`8~-8B2<e|Au4!68N=d*)`gT$sdsQ6~IO}ImLOcgScjC#+Pwg7i
z%N{PxBxSeP-e!(>G301rYP}LjG-Uj!5v5M@cMEp)w7Jy+>!JE+yYjbr*Cj;GV^2Hw
z@W7BuO=bDneVey~z?{IPxUWVi=>AB+fXkNrMXePTCtS4$20)TF&Ej-}em=4#*uIHD
z1l@)}!B8#S*Y2}K?!cbS0x0Q6+%GVl9r84w9-r^ww#S)^#5L%?tS_gQGFqN4x0;M*
z069Xd{Tg$MBJXPB3ZuxY@@+^>NZzLHs%(t8LBUGbn8_tB+-cv$3n422Xoz1d8hrHM
z(HW_#2QB54kA#LC*EWh*<^|;Iq{oeo2`LwHJ&s-Um&v;`i<Hh_{uQ<aDpBW%)4f3j
z%htcy{*A=zeeIUWny<X_ityCve&sL(46xV?51|w+KZF*%Bj?&`#$%GmZ)*@=?t?-!
zaURO*dr)zuJYIVQADTWr5l1b}?i2otPY;ASIu%ZQYrcbsCH%S6W%<9ak5(_(a)gj)
zGAT^3y*cYq`8#=(_a50+F6668Cs*<5_OqF9DD8<%OzUmJ-ZN(p#q}ZMIh1I;*Tb{{
z47ZuQ;sIfdkNL{ba>lv|L2QbW*>IAi?Ac$V1;eoHwRcj6<3Yj~Om#>)(xD*U9^*iF
z(_&Bn;ATGLy(x8wegW)L3qq6kQv=_<ZC`EUJM;PXU}^ZJHP%v#gkbAUx&fvHtH>3Z
zx*?Kp$;0{#iDe<z!u*+<(t;1|g6X;C2Sovpdcao)09^lsGTfCfirjb_!>|0~1!@^T
z)sbCy$vmaHvbwE}%8<l;aDj#1o{xBJ%mDXP5KTGzl2ZYzUGShKYy76tHEBf(KK$iS
z5L&cFsoG%pCEr7*AzehFd+qNf|Ch(&0|Pm7tXaRb><EIiC6c%TQPzH2^;r6?oQrk_
zVLI8GCW88UnCOeO1NGpLO%L#XOvdtRV`nczGDXrab<~YeRQi~0If#H9%BAZ!g#bYV
znD;X}b-7fS?$Ml_;CI3mWm4uU_xv5pR}NQsYRxo<%AXP$%4N$dI|1t6{yv3k_;6TI
zh_mFpJ?Z0M8yd=wU5gUQOEc|7QkjFELHn(kyXPW?0<7m?T$3(J+EINmZgnI<NKa$r
z(9(+{4`Z3Ji=T0ZEKLGu_kxz#l(<5&+ZA(g{QK;T8K0hR3%c7Vxuu3WQggES!W_37
z3m8|BG<mx`zT9fzs$|2>WHO)D1ZmGr2!sS+OWbZF=;oH_jv+}huqHA-6Xk^mvDd5M
z(n<NtWpM%9SiUE&k8UD|qWNcT9v0`$AEkUuS{6LkVwA^oS9ev(*{1v$WkOWIrl@lC
z^KwdBFO$7Rn<_&S!IR;dY-3GOc0C#d0gb2GhTKb;r`YWEx|cU7MelgjEYTPr4}cMs
z9#15ELINv@YeO)Up{)+)qPgl0C`|92yvN?fG?V3I?UYgndEIk!FduFJYuDwsX=jin
z1-5O%AD7g66AlQAMtA;HB17+a7&VX34owZ=2o-^vZfRP-Zzkj^0ko|ce>V_bZUl6s
z0M21Xax4^PqqUQB_#oh0zKUmjc`-*=$Yu=#ARfS%t8K^XZk83$pxE{bst#FcVg*`V
z50vnrqts+Y9yvx&WN-3m0S*!nB~!7YuX!%)BKGIjHybq_@N8&zRFA>YNjS{Vc4FH4
qta<tX0000nxT(en3ebcA0l^Bwx&Z)ZZO2}*#Ao{g000001X)^sXQz_@

literal 0
HcmV?d00001

diff --git a/test/py/tests/bootstd/mmc4.img.xz b/test/py/tests/bootstd/mmc4.img.xz
new file mode 100644
index 0000000000000000000000000000000000000000..f4db011969fa8bea352ee2b8318dfdd72ccf72d8
GIT binary patch
literal 7072
zcmeHMXHXPemhDEeB*{$<O3qP00SQfRGB!aPP%@H(AV?6T$yrcB10Ol3mYf?TXAlq(
z5SpApY@lIgYWK%h?T>YK=i8dCo!_rs)w`$8ednHYvC;uhGXMY=J=Xxn0k8v^0002R
zICSLp_5uFxGXTI1d#!1VGNTr`mE;p*%0&LKnku_|*GGQJP@UYW>mQ)!S^JbvQYkY(
z46W26RjbZTot!9{G(@Wx*b0~aDkttKa_KAZeVurk05wnjsJ_IcR{H{__QF(ucVN)d
z-lkL|MungQ%rzK*PZ?3resA^mSYFFcDUpiomLlbs$1BrccSmp2c);YP*MZAdA4P;r
zx)LHz--dSCq8=FjI!}@k#Vf2iaO-y0Q6rg5x>r_d#oRJh%S~@#`fl6K0NSzbTVcAs
zVU9<L`Mg+pePzJ!J<4LvriA%s`1lv^3nICYVLq4dNVy|ayAZ65TweLbm%fY>|8&%T
zehHBty>Ke+c_zbISABGfV`}TDd7fk`WnO^o*aJOcQsyS>S(b6*O&6LGScxq-nbF&l
z*qMRP8MmRcj(#7Av-nQP<^Upc`t<1=?K~Rt2J!%<2ZDf`%-k@-T&=+>XQBar-B(1t
zIWn5YMc0ek@OGsbLxJFV9r0XZj~xB>g{ATEQ_?k_0UO039vtE!mUDLlehppJo8dgN
z_#G_I+Il8h88;u0@txv$alb}7m5QSvYx_x@TO6VHAEUJN{PX7=TC9YhuOzoIKNvq*
z(=3aAaItuM_%+49PLt%wKG{E%^SD=CZRrWN>fHFNs57I?s72}X2*y>K&x&IUGm4|e
zOy*1KwUVDx6VJ`VZB6)j?7{0&n1MYu@P1k)Sb&WChpcfXJj9eR2ReE+Irq~w_3j$b
zl11cmE}08${Wex|jtL-PP{I{Nwye8FaMmkX9lxr0%8?%}5o~jUo;cZ3W#&62hWv2C
zq6bxHm3rO0GA}2!5GgY+TCzoXl6i<HuPwhEZD*y57kg^b5SG7MraJp1?ikPZNrIcL
zWm0~yA+Om3WK1ful9W8`>-7zkC)_Ufwe?e04UaqZC+nq>&zo>;{2hB8+4c>IFBu=|
z9F?u{9~Co9i)(W<K{f(#J?GgE9J6;AIE8eXWq)nEX&3XeYa_gs$y6!^Sg9BtSVnd}
z4Dk0&oblw#pU8OhbvHHQ_<+>E)`0>k;nxwH%!CXBw$?Gx)U+_uVIfdkQIq}M(;=yJ
z-ba1<51*$@-(mh)=>A<06cw4}+7?*rw+t@|Y{W}_TFrVOT0;%x++b-W-ApK%oQk`V
zm)XekQd)nd%lpW6DvvK<N$#W3QDme$c(#sd+qU?wSycn0k)zQmNT-vi_aq?5k^8V`
zN>GJ={HlXtkISHGSkWn70bl`l4BhopV0ur(D1>@MPSyN5c{zG=3<m<24Fe~Mca(Pm
z@0ssaB3lI~K3m<cIvn95)1MKRJECbdg*SW1Q7<1_;&|;0J~5+qjq;KT(v)>+QZ~Vd
zOFv*Wfw?*#Fp#K;Az5Ue&L=$@8(O@co4%_{I1A$T^=P!tN0q%>NI2kjzJwI6;+-9X
znN3>wHy$Ib#}h_KH&eH%Ag9)<Je!^cH@ok*hsh#Hc&D*_dkCz3#O7tv$|rBbj*?IQ
zq&%eB?~Ozibto2kcbVvDPWVD!VQagZiCMeCoRMT=Qg$0&v58COO9M>4A_uOcB8Ic<
znHrI<IN6a6n6$KKV)Y0Wm7Zho9zOmk()XXlMqcXW89s07s>OUGXE3=QRL?n6%I?7d
zVo6}4rC?ybi&@xmt==9{L{z_?xwST&`JU2qoTv%4WQjLx&6hQ}fr+`kDMIj66pztv
zPi3Y7T`K;z*bxtm52Bx2Np|eeIl83UjT9{XHNHGF5y<dlH`KZW<ivcwHOBvgdSmBF
z7GmwGL!{c41<ZuXwUBEhxkTsoh0#vujspM+?$^B_xAqSb3v0hcyqi9A1j&qoncQ3E
zoScYx$e?6JeuY*>(zMoSMH$14?dZ4S!J0!+=)yENaE`praq+X~_#1M}6QO2xcioGQ
zi@Jo9Z)gsfL*C}+9^|VaPeQ7!+q%6dkAX5#)|IIhK~>{+K5q64Jvwx7Uk9QlOo%=A
zQm`?@3o1~tYlPP>N6=0%PVgD(%az3&BCz?F(XI=v*9kW}{cqbYe>5FHf8s#Oh1LwV
zs3sZJ1oSC-QoI!>RQTCyfHCoj+(WMmmd2@iGWcg3_l;9OwkD%wB4>=ZzC~)I1VfOL
zH0d9;KaJCp{h`(Ua^l^5M)Z({nrJh0{KZ2O{_r;`g-4kH`Vv=hLYu(nlhM5)ZbR$|
zOWa9HY%4$Z(lBb5kWGWNWPB4)j!Mqj)H?X5(GlE7K;&zWo>3A<I{qElpt~G(mzZ=$
zAq}u_>klHny|P;~t_n{Q+T`L6nS}eR?m5MrusxpPh{?2!=r?>xC)_?4pf3Cjoeb=J
z^=u;fzpID<aubHQeF=02?0Fd$V>_w>MHvu{aiXsO<$cC{Paq-Q$<m8P8n2ho)UYe4
zX1fDi!cWSI=YEjadv_YQDWpp3nNvwv+)9W3_|@S~W0r+$z28^r2o`}DR)b|OZOxOk
zoQMHCl`{K62yXpzh<-+0C|jV^VoLhY8JgjqVTo2JvD!BtU=Ko0;7h8f61psizvF>_
zhD`p06nf?8iVJZ|0p$O?QU50{>c2B~{4SpQ<O+3&TS|P}-vI2?7cK?H!wilvLv~p3
zO+?UJDk3wXe~ceH%_K>NhUFRN=DxPINO4o{wIR5rlg)QAqLVMtY~};qXYG~o-7lzq
zr^#Z|Af39Og=6}WGuvG5U@_=rzY^cEVUqs-f!46Il;&3ftN7UM(Zk#F_Y=(DoSB!f
z@Y((nK+FO|dBVA7eJ{l6(TzI(D)Oz=h?w8|?>IE3WommRaXrD+59F{W6!c;RYz-{O
z81@tW!QRy&&*X*J_5D7<XFuoLGlO*;Sat3>HgnS$ZFQDBZg2)F6H8obxdS@cS1ydm
zfZ_3`miv=L)6)mUGsp>F<jH)cZ+ftuy==8!Za&sfA(CMM7oL2>WL(o|;?k!VxP2NW
zcsz46t~T$<1j$J~&<0Z6PgwnBAlsLt{MH!M*Wr2m1(TcKU+a}N4yTB7w5)bs-dWOg
zZr^(qeh*zlFr%Jgf0yixY|F3i&TH@9x<W3AZM?-B&FIlt&epmHl5}iWmxU3L&d24c
zpjLZ)ksbFd#n8%M!O#!WjncUJA}XvIr-Jbeyy;C$;Mi|-pD2@b?sc$zgsu(o^4X2i
zvdyPx*}aGXY@&wb&>n`1KAd?z>^H{fa`k7-)OB<|-$ue&p*KX-*QF1$6NyiqVqY<Z
zea1@mMdat=39B|)eJB)8#jSc63sJok&TV-tS}D7k*hz&x5Y})8&*z%f5Yr|EoC<ZM
z9Co~C1LB0t(Ar_%o|Hku<@mUeV$3FjkMr=GCZ%7DFx;@J51&Qh$_Z47po2q3-SmWD
zOg;_s$XB`T-WuUCk&X^$%Y|%t991!7VGp|o!p-|R!)rC|Evdoe@+=QWFRRYWY(2_G
z0~n!EK%ycXJ}L7C_n7C^v7-m4t#OvPcZn-bUTd}6F8aWgAI0roVXB)!mE^JqgLJB^
z>yLVg4i^xzNqOD3jQ(NmPesZH0cT8HpwQxqgS0maYiC|<Bih=>6&3ewX6xnLT{0Y;
zbs-`Ltj!&d<D>Z=TgK%0{=!ssRIEDWP-#6`x`s{235*_&ot;{`eb*-B!x_^UAFT^2
zJr7bD$u%w_NP!R()+!wIP!F`6!5oYeURk;buv`7q>}#=6b--)lW_A}xe-xRtRvg}h
z>?-jSU^574^ka+BO`Q$5c-V5o4@XWhq;qthe0fpIzO4RjyJ|n`a7DZp6()&-1eM<O
z70Du^jPvx8UGF4nGa1@?zGJL-y}|u5B4QDgEv7`r8kXFq8cWJCsZGX>k-56xqUsH*
zYDynV=W$Ie-eM<=ov)z6eWa(Boe0$mUU@Z73E_w)$y0W0%VFjCE-km%Dre!}3KE$<
zMt^Av33Qsh7$7`}$nojO)Lk|73otQfFd_5pD6y(?ut&wZ)Rfe>@=~o>XBTi4E2zqb
zlZBkVk3@I6Dsptbg*5Pv)~Aj=stGM~yOg41_vX75uHt03as8eLR0}x_kiH=9jwpM8
z@ffZ_Q%cjiYD6`zVbYLZ%$!SV6jXRNh5HtR$hLR04X#CrA+EjYt!uB<zv+0b&Z9Ff
z_Qt}h(9n5+W6yw(9pdV+z+3f8+eU5+-9<VKHrydDOfcnmr-n0Y=0j><Lt)g>3C-L-
zX<^HBjgby6dOxj!VJz5gz0hF%BonvtoUYw8jhdtZ5bxS=ndC%7H>euQsTRKLlwB&4
zblF2t#mbF$^bvydX#_50NZII&v*S<)^Jw{?Pb)a2t!4DXiMgDXMHBj2;|uPoS(Dun
zDp!n+Yhd2-wmBvuA*Sb7Xy!L$Q{ADCBtDUAt;%H`3$2bqtqDVb@^1GEnDhUG9{&gF
zGQ%qNArZI4c<R3)H5c?DA(uafo||oyuVipgsv6DOPPZizH9zEuiKcYvGB1WRGmw1b
zXd=*ULGe{}%xZ)bA(nQgw$)k{`)S47X1=Hzlfrul#|?BBy@oo-kAP%k7_BD}0yW7}
zHg4a<Uq==fH%Nf&yFLs^V88nhnl0weF_{GFVi%u%lk|i3xQ86Zb=0tC>B%dUoTy#e
zZ)+$$AXL3VhCOBEp_}7g-i1xysGU-plDn;pCDJi9-M|RRVay*))XdTywC-C$D?)!F
zgwn8mocFU!qRMaLncreDZK0>^YPTd}S|Ee2AlqyN_&gLzVU)jJA{kbWytrP@@^3Uf
ztbiW792Pvz-${w&L|yOpE09P>2H@GLiBNWRDe{)F{-msXR9YG92($m17dD>FLQhvC
z+zXGJFB+Tjy{g>u=euIynw~9Gm*-taa2A9WBv|q3q*(CIH0JVizMP3}TOD+AX_<A_
zO3^KhC*YZQ(F=8PLsXob+6XY=mF#@c1KbHEEPwaV5W+8<YFx44)*Dt|F+=vpoN>1~
zLWxcjHFI;Q?GiZ*dayo#?vocSN}AE3Ji*ayu6NCGG<9#P%9ktL<42F8BujipOgl<D
zo*R2%j7~BNwAzi}L+3{c%S%g|^bxZyRm=f?oVzQ$ooD3jfxFs|dIw;z3EJfL)$vzE
z@4bx59f%cP6CW#IQxtUF`_OfVwX%@zZ8XM2*9e7G7Tm8w+2S#iafq+4tT3kg^tjJd
z)u3l<q?1Xco6WYZSrbmo{dUe9`N0<wIzQqHo;c1tr+Bw5tz+dNf)9{#hsNLk$lso&
zVFN*erMDO+l86O@Mj~`|!}N#T1FP=uf2R`;3J}p=PP(8hs)xbEZ7BS5avmV%ukL5D
zJT*3@pVJV$T&W3;%6iEXrAi`qSfSB8jp02aV!r4aSz)B+x>^2MF>fns-SkpMtol{6
z@7zK09fX|L;Y+bRwWIFoql2<8au^S$_pF-yq+_o_m9xJ*44f~E;AnOG1xl=5lBVrq
zPu2P3wUiufo*2FsacSRDUHunn_xM<rQ^kXaQ(H<C2K}{}ML^_AQikLoXo@=-17g*U
z+uge5_xXW$A5K+_P<IFh5kB8lBih?;A{}fl!o3%}A37$uI3{CjwrN^^7}uGnpKTeJ
z&dy7ZGU>mUb3A;Dq~CSw)HHACW-VA+S5~?7iXdME(dwl2)}jfsS!i-<lx=3hH@oMo
z$Vc-8O%+&dD(K~1aWFB`ks$x+k)3%B%hSv}VM6WxA<bajw-O##XHyY1Rv8~cVTI==
zN5UR5n+jvApA|zL(<AN>d*UT^eRkO-rt~;3VJr77NYNm`2^L3bhb-_&PVHH%wx(|R
zO{|7#bK=8S_xf4WHF>*tf+N7Z><8wyh~8AGF3*F@H2^aaFJuiMPm5SCR2qgWSO5|x
zAk|zvsUED4f8VG5W7EFh9lQVMK_d{iUI1}R3l#sY-}IAZi{LUe!hV)c)?p8Pt@md*
z-v1*=Vn#Q{|7H;deg`<vBtSW+4!U3#`M;4F{F5RG8J-#Vm#O}1mjAom{col^8z}Kx
z6#LKF(VxqM+@Np;I%tQ0{c>=rb(`3Fy276vh5ddx3j@Crga1XfzmsYqghl{7Kzq!n
pcF<o}Z2;3}KI-mavEBe6Mk*vEgcwDdLRgEkzry+3A+D*}UjQf@%fJ8t

literal 0
HcmV?d00001

diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
index 9d423903736..1fde063bb07 100644
--- a/test/py/tests/test_ut.py
+++ b/test/py/tests/test_ut.py
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 # Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
 
+import getpass
 import gzip
 import os
 import os.path
@@ -17,29 +18,212 @@ def mkdir_cond(dirname):
     if not os.path.exists(dirname):
         os.mkdir(dirname)
 
-def setup_bootflow_image(u_boot_console):
-    """Create a 20MB disk image with a single FAT partition"""
-    cons = u_boot_console
-    fname = os.path.join(cons.config.source_dir, 'mmc1.img')
+def setup_image(cons, mmc_dev, part_type):
+    """Create a 20MB disk image with a single partition
+
+    Args:
+        cons (ConsoleBase): Console touse
+        mmc_dev (int): MMC device number to use, e.g. 1
+        part_type (int): Partition type, e.g. 0xc for FAT32
+
+    Returns:
+        tuple:
+            str: Filename of MMC image
+            str: Directory name of 'mnt' directory
+    """
+    fname = os.path.join(cons.config.source_dir, f'mmc{mmc_dev}.img')
     mnt = os.path.join(cons.config.persistent_data_dir, 'mnt')
     mkdir_cond(mnt)
 
     u_boot_utils.run_and_log(cons, 'qemu-img create %s 20M' % fname)
     u_boot_utils.run_and_log(cons, 'sudo sfdisk %s' % fname,
-                             stdin=b'type=c')
+                             stdin=f'type={part_type:x}'.encode('utf-8'))
+    return fname, mnt
+
+def mount_image(cons, fname, mnt, fstype):
+    """Create a filesystem and mount it on partition 1
+
+    Args:
+        cons (ConsoleBase): Console to use
+        fname (str): Filename of MMC image
+        mnt (str): Directory name of 'mnt' directory
+        fstype (str): Filesystem type ('vfat' or 'ext4')
+
+    Returns:
+        str: Name of loop device used
+    """
+    out = u_boot_utils.run_and_log(cons, 'sudo losetup --show -f -P %s' % fname)
+    loop = out.strip()
+    part = f'{loop}p1'
+    u_boot_utils.run_and_log(cons, f'sudo mkfs.{fstype} {part}')
+    opts = ''
+    if fstype == 'vfat':
+         opts += ' -o uid={os.getuid()},gid={os.getgid()}'
+    u_boot_utils.run_and_log(cons, f'sudo mount -o loop {part} {mnt}{opts}')
+    u_boot_utils.run_and_log(cons, f'sudo chown {getpass.getuser()} {mnt}')
+    return loop
+
+def copy_prepared_image(cons, mmc_dev, fname):
+    """Use a prepared image since we cannot create one
+
+    Args:
+        cons (ConsoleBase): Console touse
+        mmc_dev (int): MMC device number
+        fname (str): Filename of MMC image
+    """
+    infname = os.path.join(cons.config.source_dir,
+                           f'test/py/tests/bootstd/mmc{mmc_dev}.img.xz')
+    u_boot_utils.run_and_log(
+        cons,
+        ['sh', '-c', 'xz -dc %s >%s' % (infname, fname)])
+
+def setup_bootmenu_image(cons):
+    """Create a 20MB disk image with a single ext4 partition
+
+    This is modelled on Armbian 22.08 Jammy
+    """
+    mmc_dev = 4
+    fname, mnt = setup_image(cons, mmc_dev, 0x83)
 
     loop = None
     mounted = False
     complete = False
     try:
-        out = u_boot_utils.run_and_log(cons,
-                                       'sudo losetup --show -f -P %s' % fname)
-        loop = out.strip()
-        fatpart = '%sp1' % loop
-        u_boot_utils.run_and_log(cons, 'sudo mkfs.vfat %s' % fatpart)
+        loop = mount_image(cons, fname, mnt, 'ext4')
+        mounted = True
+
+        vmlinux = 'Image'
+        initrd = 'uInitrd'
+        dtbdir = 'dtb'
+        script = '''# DO NOT EDIT THIS FILE
+#
+# Please edit /boot/armbianEnv.txt to set supported parameters
+#
+
+setenv load_addr "0x9000000"
+setenv overlay_error "false"
+# default values
+setenv rootdev "/dev/mmcblk%dp1"
+setenv verbosity "1"
+setenv console "both"
+setenv bootlogo "false"
+setenv rootfstype "ext4"
+setenv docker_optimizations "on"
+setenv earlycon "off"
+
+echo "Boot script loaded from ${devtype} ${devnum}"
+
+if test -e ${devtype} ${devnum} ${prefix}armbianEnv.txt; then
+	load ${devtype} ${devnum} ${load_addr} ${prefix}armbianEnv.txt
+	env import -t ${load_addr} ${filesize}
+fi
+
+if test "${logo}" = "disabled"; then setenv logo "logo.nologo"; fi
+
+if test "${console}" = "display" || test "${console}" = "both"; then setenv consoleargs "console=tty1"; fi
+if test "${console}" = "serial" || test "${console}" = "both"; then setenv consoleargs "console=ttyS2,1500000 ${consoleargs}"; fi
+if test "${earlycon}" = "on"; then setenv consoleargs "earlycon ${consoleargs}"; fi
+if test "${bootlogo}" = "true"; then setenv consoleargs "bootsplash.bootfile=bootsplash.armbian ${consoleargs}"; fi
+
+# get PARTUUID of first partition on SD/eMMC the boot script was loaded from
+if test "${devtype}" = "mmc"; then part uuid mmc ${devnum}:1 partuuid; fi
+
+setenv bootargs "root=${rootdev} rootwait rootfstype=${rootfstype} ${consoleargs} consoleblank=0 loglevel=${verbosity} ubootpart=${partuuid} usb-storage.quirks=${usbstoragequirks} ${extraargs} ${extraboardargs}"
+
+if test "${docker_optimizations}" = "on"; then setenv bootargs "${bootargs} cgroup_enable=cpuset cgroup_memory=1 cgroup_enable=memory swapaccount=1"; fi
+
+load ${devtype} ${devnum} ${ramdisk_addr_r} ${prefix}uInitrd
+load ${devtype} ${devnum} ${kernel_addr_r} ${prefix}Image
+
+load ${devtype} ${devnum} ${fdt_addr_r} ${prefix}dtb/${fdtfile}
+fdt addr ${fdt_addr_r}
+fdt resize 65536
+for overlay_file in ${overlays}; do
+	if load ${devtype} ${devnum} ${load_addr} ${prefix}dtb/rockchip/overlay/${overlay_prefix}-${overlay_file}.dtbo; then
+		echo "Applying kernel provided DT overlay ${overlay_prefix}-${overlay_file}.dtbo"
+		fdt apply ${load_addr} || setenv overlay_error "true"
+	fi
+done
+for overlay_file in ${user_overlays}; do
+	if load ${devtype} ${devnum} ${load_addr} ${prefix}overlay-user/${overlay_file}.dtbo; then
+		echo "Applying user provided DT overlay ${overlay_file}.dtbo"
+		fdt apply ${load_addr} || setenv overlay_error "true"
+	fi
+done
+if test "${overlay_error}" = "true"; then
+	echo "Error applying DT overlays, restoring original DT"
+	load ${devtype} ${devnum} ${fdt_addr_r} ${prefix}dtb/${fdtfile}
+else
+	if load ${devtype} ${devnum} ${load_addr} ${prefix}dtb/rockchip/overlay/${overlay_prefix}-fixup.scr; then
+		echo "Applying kernel provided DT fixup script (${overlay_prefix}-fixup.scr)"
+		source ${load_addr}
+	fi
+	if test -e ${devtype} ${devnum} ${prefix}fixup.scr; then
+		load ${devtype} ${devnum} ${load_addr} ${prefix}fixup.scr
+		echo "Applying user provided fixup script (fixup.scr)"
+		source ${load_addr}
+	fi
+fi
+booti ${kernel_addr_r} ${ramdisk_addr_r} ${fdt_addr_r}
+
+# Recompile with:
+# mkimage -C none -A arm -T script -d /boot/boot.cmd /boot/boot.scr
+''' % (mmc_dev)
+        bootdir = os.path.join(mnt, 'boot')
+        mkdir_cond(bootdir)
+        cmd_fname = os.path.join(bootdir, 'boot.cmd')
+        scr_fname = os.path.join(bootdir, 'boot.scr')
+        with open(cmd_fname, 'w') as outf:
+            print(script, file=outf)
+
+        infname = os.path.join(cons.config.source_dir,
+                               'test/py/tests/bootstd/armbian.bmp.xz')
+        bmp_file = os.path.join(bootdir, 'boot.bmp')
+        u_boot_utils.run_and_log(
+            cons,
+            ['sh', '-c', f'xz -dc {infname} >{bmp_file}'])
+
+        u_boot_utils.run_and_log(
+            cons, f'mkimage -C none -A arm -T script -d {cmd_fname} {scr_fname}')
+
+        kernel = 'vmlinuz-5.15.63-rockchip64'
+        target = os.path.join(bootdir, kernel)
+        with open(target, 'wb') as outf:
+            print('kernel', outf)
+
+        symlink = os.path.join(bootdir, 'Image')
+        if os.path.exists(symlink):
+            os.remove(symlink)
+        u_boot_utils.run_and_log(
+            cons, f'echo here {kernel} {symlink}')
+        os.symlink(kernel, symlink)
+
         u_boot_utils.run_and_log(
-            cons, 'sudo mount -o loop %s %s -o uid=%d,gid=%d' %
-            (fatpart, mnt, os.getuid(), os.getgid()))
+            cons, f'mkimage -C none -A arm -T script -d {cmd_fname} {scr_fname}')
+        complete = True
+
+    except ValueError as exc:
+        print('Falled to create image, failing back to prepared copy: %s',
+              str(exc))
+    finally:
+        if mounted:
+            u_boot_utils.run_and_log(cons, 'sudo umount %s' % mnt)
+        if loop:
+            u_boot_utils.run_and_log(cons, 'sudo losetup -d %s' % loop)
+
+    if not complete:
+        copy_prepared_image(cons, mmc_dev, fname)
+
+def setup_bootflow_image(cons):
+    """Create a 20MB disk image with a single FAT partition"""
+    mmc_dev = 1
+    fname, mnt = setup_image(cons, mmc_dev, 0xc)
+
+    loop = None
+    mounted = False
+    complete = False
+    try:
+        loop = mount_image(cons, fname, mnt, 'vfat')
         mounted = True
 
         vmlinux = 'vmlinuz-5.3.7-301.fc31.armv7hl'
@@ -89,12 +273,7 @@ label Fedora-Workstation-armhfp-31-1.9 (5.3.7-301.fc31.armv7hl)
             u_boot_utils.run_and_log(cons, 'sudo losetup -d %s' % loop)
 
     if not complete:
-        # Use a prepared image since we cannot create one
-        infname = os.path.join(cons.config.source_dir,
-                               'test/py/tests/bootstd/mmc1.img.xz')
-        u_boot_utils.run_and_log(
-            cons,
-            ['sh', '-c', 'xz -dc %s >%s' % (infname, fname)])
+        copy_prepared_image(cons, mmc_dev, fname)
 
 
 @pytest.mark.buildconfigspec('ut_dm')
@@ -128,6 +307,7 @@ def test_ut_dm_init_bootstd(u_boot_console):
     """Initialise data for bootflow tests"""
 
     setup_bootflow_image(u_boot_console)
+    setup_bootmenu_image(u_boot_console)
 
     # Restart so that the new mmc1.img is picked up
     u_boot_console.restart_uboot()
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 23/24] bootstd: Support setting a theme for the menu

[Simon Glass]

Allow a theme to be set. For now this is very simple, just a default font
size to use for all elements.

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

 boot/bootflow_menu.c  | 43 ++++++++++++++++++++++
 boot/bootstd-uclass.c |  2 ++
 include/bootflow.h    | 10 ++++++
 include/bootstd.h     |  4 +++
 test/boot/bootflow.c  | 83 +++++++++++++++++++++++++++++++++++++++++--
 5 files changed, 139 insertions(+), 3 deletions(-)

diff --git a/boot/bootflow_menu.c b/boot/bootflow_menu.c
index 0d7fb506f3f..492526e9c3d 100644
--- a/boot/bootflow_menu.c
+++ b/boot/bootflow_menu.c
@@ -126,6 +126,43 @@ int bootflow_menu_new(struct expo **expp)
 	return 0;
 }
 
+int bootflow_menu_apply_theme(struct expo *exp, ofnode node)
+{
+	struct menu_priv *priv = exp->priv;
+	struct scene *scn;
+	u32 font_size;
+	int ret;
+
+	log_debug("Applying theme %s\n", ofnode_get_name(node));
+	scn = expo_lookup_scene_id(exp, MAIN);
+	if (!scn)
+		return log_msg_ret("scn", -ENOENT);
+
+	/* Avoid error-checking optional items */
+	if (!ofnode_read_u32(node, "font-size", &font_size)) {
+		int i;
+
+		log_debug("font size %d\n", font_size);
+		scene_txt_set_font(scn, OBJ_PROMPT, NULL, font_size);
+		scene_txt_set_font(scn, OBJ_POINTER, NULL, font_size);
+		for (i = 0; i < priv->num_bootflows; i++) {
+			ret = scene_txt_set_font(scn, ITEM_DESC + i, NULL,
+						 font_size);
+			if (ret)
+				return log_msg_ret("des", ret);
+			scene_txt_set_font(scn, ITEM_KEY + i, NULL, font_size);
+			scene_txt_set_font(scn, ITEM_LABEL + i, NULL,
+					   font_size);
+		}
+	}
+
+	ret = scene_arrange(scn);
+	if (ret)
+		return log_msg_ret("arr", ret);
+
+	return 0;
+}
+
 int bootflow_menu_run(struct bootstd_priv *std, bool text_mode,
 		      struct bootflow **bflowp)
 {
@@ -146,6 +183,12 @@ int bootflow_menu_run(struct bootstd_priv *std, bool text_mode,
 	if (ret)
 		return log_msg_ret("exp", ret);
 
+	if (ofnode_valid(std->theme)) {
+		ret = bootflow_menu_apply_theme(exp, std->theme);
+		if (ret)
+			return log_msg_ret("thm", ret);
+	}
+
 	/* For now we only support a video console */
 	ret = uclass_first_device_err(UCLASS_VIDEO, &dev);
 	if (ret)
diff --git a/boot/bootstd-uclass.c b/boot/bootstd-uclass.c
index 565c22a36e7..7887acdc11b 100644
--- a/boot/bootstd-uclass.c
+++ b/boot/bootstd-uclass.c
@@ -33,6 +33,8 @@ static int bootstd_of_to_plat(struct udevice *dev)
 					   &priv->prefixes);
 		dev_read_string_list(dev, "bootdev-order",
 				     &priv->bootdev_order);
+
+		priv->theme = ofnode_find_subnode(dev_ofnode(dev), "theme");
 	}
 
 	return 0;
diff --git a/include/bootflow.h b/include/bootflow.h
index e7a09568f1b..c201246c6de 100644
--- a/include/bootflow.h
+++ b/include/bootflow.h
@@ -7,6 +7,7 @@
 #ifndef __bootflow_h
 #define __bootflow_h
 
+#include <dm/ofnode_decl.h>
 #include <linux/list.h>
 
 struct bootstd_priv;
@@ -347,6 +348,15 @@ int bootflow_iter_uses_system(const struct bootflow_iter *iter);
  */
 int bootflow_menu_new(struct expo **expp);
 
+/**
+ * bootflow_menu_apply_theme() - Apply a theme to a bootmenu
+ *
+ * @exp: Expo to update
+ * @node: Node containing the theme information
+ * Returns 0 on success, -ve on error
+ */
+int bootflow_menu_apply_theme(struct expo *exp, ofnode node);
+
 /**
  * bootflow_menu_run() - Create and run a menu of available bootflows
  *
diff --git a/include/bootstd.h b/include/bootstd.h
index 01be249d16e..4fa0d531001 100644
--- a/include/bootstd.h
+++ b/include/bootstd.h
@@ -9,6 +9,8 @@
 #ifndef __bootstd_h
 #define __bootstd_h
 
+#include <dm/ofnode_decl.h>
+
 struct udevice;
 
 /**
@@ -27,6 +29,7 @@ struct udevice;
  * @bootmeth_count: Number of bootmeth devices in @bootmeth_order
  * @bootmeth_order: List of bootmeth devices to use, in order, NULL-terminated
  * @vbe_bootmeth: Currently selected VBE bootmeth, NULL if none
+ * @theme: Node containing the theme information
  */
 struct bootstd_priv {
 	const char **prefixes;
@@ -37,6 +40,7 @@ struct bootstd_priv {
 	int bootmeth_count;
 	struct udevice **bootmeth_order;
 	struct udevice *vbe_bootmeth;
+	ofnode theme;
 };
 
 /**
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index abafa44b2ed..5b76cd3ab14 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -13,6 +13,7 @@
 #include <bootstd.h>
 #include <cli.h>
 #include <dm.h>
+#include <expo.h>
 #ifdef CONFIG_SANDBOX
 #include <asm/test.h>
 #endif
@@ -21,6 +22,8 @@
 #include <test/suites.h>
 #include <test/ut.h>
 #include "bootstd_common.h"
+#include "../../boot/bootflow_internal.h"
+#include "../../boot/scene_internal.h"
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -469,14 +472,18 @@ static int bootflow_cmd_boot(struct unit_test_state *uts)
 }
 BOOTSTD_TEST(bootflow_cmd_boot, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
 
-/* Check 'bootflow menu' to select a bootflow */
-static int bootflow_cmd_menu(struct unit_test_state *uts)
+/**
+ * prep_mmc4_bootdev() - Set up the mmc4 bootdev so we can access a fake Armbian
+ *
+ * @uts: Unit test state
+ * Returns 0 on success, -ve on failure
+ */
+static int prep_mmc4_bootdev(struct unit_test_state *uts)
 {
 	static const char *order[] = {"mmc2", "mmc1", "mmc4", NULL};
 	struct udevice *dev, *bootstd;
 	struct bootstd_priv *std;
 	const char **old_order;
-	char prev[3];
 	ofnode node;
 
 	/* Enable the mmc4 node since we need a second bootflow */
@@ -500,6 +507,16 @@ static int bootflow_cmd_menu(struct unit_test_state *uts)
 	/* Restore the order used by the device tree */
 	std->bootdev_order = old_order;
 
+	return 0;
+}
+
+/* Check 'bootflow menu' to select a bootflow */
+static int bootflow_cmd_menu(struct unit_test_state *uts)
+{
+	char prev[3];
+
+	ut_assertok(prep_mmc4_bootdev(uts));
+
 	/* Add keypresses to move to and select the second one in the list */
 	prev[0] = CTL_CH('n');
 	prev[1] = '\r';
@@ -513,3 +530,63 @@ static int bootflow_cmd_menu(struct unit_test_state *uts)
 	return 0;
 }
 BOOTSTD_TEST(bootflow_cmd_menu, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
+
+/**
+ * check_font() - Check that the font size for an item matches expectations
+ *
+ * @uts: Unit test state
+ * @scn: Scene containing the text object
+ * @id: ID of the text object
+ * Returns 0 on success, -ve on failure
+ */
+static int check_font(struct unit_test_state *uts, struct scene *scn, uint id,
+		      int font_size)
+{
+	struct scene_obj_txt *txt;
+
+	txt = scene_obj_find(scn, id, SCENEOBJT_TEXT);
+	ut_assertnonnull(txt);
+
+	ut_asserteq(font_size, txt->font_size);
+
+	return 0;
+}
+
+/* Check themes work with a bootflow menu */
+static int bootflow_menu_theme(struct unit_test_state *uts)
+{
+	const int font_size = 30;
+	struct scene *scn;
+	struct expo *exp;
+	ofnode node;
+	int i;
+
+	ut_assertok(prep_mmc4_bootdev(uts));
+
+	ut_assertok(bootflow_menu_new(&exp));
+	node = ofnode_path("/bootstd/theme");
+	ut_assert(ofnode_valid(node));
+	ut_assertok(bootflow_menu_apply_theme(exp, node));
+
+	scn = expo_lookup_scene_id(exp, MAIN);
+	ut_assertnonnull(scn);
+
+	/*
+	 * Check that the txt objects have the correct font size from the
+	 * device tree node: bootstd/theme
+	 *
+	 * Check both menu items, since there are two bootflows
+	 */
+	ut_assertok(check_font(uts, scn, OBJ_PROMPT, font_size));
+	ut_assertok(check_font(uts, scn, OBJ_POINTER, font_size));
+	for (i = 0; i < 2; i++) {
+		ut_assertok(check_font(uts, scn, ITEM_DESC + i, font_size));
+		ut_assertok(check_font(uts, scn, ITEM_KEY + i, font_size));
+		ut_assertok(check_font(uts, scn, ITEM_LABEL + i, font_size));
+	}
+
+	expo_destroy(exp);
+
+	return 0;
+}
+BOOTSTD_TEST(bootflow_menu_theme, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
-- 
2.38.0.413.g74048e4d9e-goog

[PATCH 24/24] expo: Add documentation

[Simon Glass]

Add some documentation for the expo feature.

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

 doc/develop/expo.rst  | 184 ++++++++++++++++++++++++++++++++++++++++++
 doc/develop/index.rst |   1 +
 2 files changed, 185 insertions(+)
 create mode 100644 doc/develop/expo.rst

diff --git a/doc/develop/expo.rst b/doc/develop/expo.rst
new file mode 100644
index 00000000000..5e502450053
--- /dev/null
+++ b/doc/develop/expo.rst
@@ -0,0 +1,184 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+Expo menu
+=========
+
+U-Boot provides a menu implementation for use with selecting bootflows and
+changing U-Boot settings. This is in early stages of development.
+
+Motivation
+----------
+
+U-Boot already has a text-based menu system accessed via the
+:doc:`../usage/cmd/bootmenu`. This works using environment variables, or via
+some EFI-specific hacks.
+
+The command makes use of a lower-level `menu` implementation, which is quite
+flexible and can be used to make menu hierarchies.
+
+However this system is not flexible enough for use with standard boot. It does
+not support a graphical user interface and cannot currently support anything
+more than a very simple list of items. While it does support multiple menus in
+hierarchies, these are implemented by the caller. See for example `eficonfig.c`.
+
+Another challenge with the current menu implementation is that it controls
+the event loop, such that bootmenu_loop() does not return until a key is
+pressed. This makes it difficult to implement dynamic displays or to do other
+things while the menu is running, such as searching for more bootflows.
+
+For these reasons an attempt has been made to develop a more flexible system
+which can handle menus as well as other elements. This is called 'expo', short
+for exposition, in an attempt to avoid common words like display, screen, menu
+and the like. The primary goal is to support Verified Boot for Embedded (VBE),
+although it is available to any boot method, using the 'bootflow menu' command.
+
+Efforts have been made to use common code with the existing menu, including
+key processing in particular.
+
+Previous work looked at integrating Nuklear into U-Boot. This works fine and
+could provide a way to provide a more flexible UI, perhaps with expo dealing
+with the interface to Nuklear. But this is quite a big step and it may be years
+before this becomes desirable, if at all. For now, U-Boot only needs a fairly
+simple set of menus and options, so rendering them directly is fairly
+straightforward.
+
+Concepts
+--------
+
+The creator of the expo is here called a `controller` and it controls most
+aspects of the expo. This is the code that you must write to use expo.
+
+An `expo` is a set of scenes which can be presented to the user one at a time,
+to show information and obtain input from the user.
+
+A `scene` is a collection of objects which are displayed together on the screen.
+Only one scene is visible at a time and scenes do not share objects.
+
+A `scene object` is something that appears in the scene, such as some text, an
+image or a menu. Objects can be positioned and hidden.
+
+A `menu object` contains a title, a set of `menu items` and a pointer to the
+current item. Menu items consist of a keypress (indicating what to press to
+select the item), label and description. All three are shown in a single line
+within the menu. Items can also have a preview image, which is shown when the
+item is highlighted.
+
+All components have a name. This is purely for debugging, so it is easy to see
+what object is referred to. Of course the ID numbers can help as well, but they
+are less easy to distinguish.
+
+While the expo implementation provides support for handling keypresses and
+rendering on the display or serial port, it does not actually deal with reading
+input from the user, nor what should be done when a particular menu item is
+selected. This is deliberate since having the event loop outside the expo is
+more flexible, particularly in a single-threaded environment like U-Boot.
+
+Everything within an expo has a unique ID number. This is done so that it is
+easy to refer to things after the expo has been created. The expectation is that
+the controller declares an enum containing all of the elements in the expo,
+passing the ID of each object as it is created. When a menu item is selected,
+its ID is returned. When a object's font or position needs to change, the ID is
+passed to expo functions to indicate which object it is. It is possible for expo
+to auto-allocate IDs, but this is not recommended. The use of IDs is a
+convenience, removing the need for the controller to store pointers to objects,
+or even the IDs of objects. Programmatic creation of many items in a loop can be
+handled by allocating space in the enum for a maximum number of items, then
+adding the loop count to the enum values to obtain unique IDs.
+
+Menu objects do not have their own text and image objects. Instead they simply
+refer to objects which have been created. So a menu item is just a collection
+of IDs of text and image objects. When adding a menu item you must create these
+objects first, then create the menu item, passing in the relevant IDs.
+
+Creating an expo
+----------------
+
+To create an expo, use `expo_new()` followed by `scene_new()` to create a scene.
+Then add objects to the scene, using functions like `scene_txt_add()` and
+`scene_menu_add()`. For every menu item, add text and image objects, then create
+the menu item with `scene_menuitem_add()`, referring to those objects.
+
+Layout
+------
+
+Individual objects can be positioned using `scene_obj_set_pos()`. Menu items
+cannot be positioned manually: this is done by `scene_arrange()` which is called
+automatically when something changes. The menu itself determines the position of
+its items.
+
+Rendering
+---------
+
+Rendering is performed by calling `expo_render()`. This uses either the
+vidconsole, if present, or the serial console in `text mode`. Expo handles
+presentation automatically in either case, without any change in how the expo is
+created.
+
+For the vidconsole, Truetype fonts can be used if enabled, to enhance the
+quality of the display. For text mode, each menu item is shown in a single line,
+allowing easy selection using arrow keys.
+
+Input
+-----
+
+The controller is responsible for collecting keyboard input. A good way to do
+this is to use `cli_ch_process()`, since it handles conversion of escape
+sequences into keys. However, expo has some special menu-key codes for
+navigating the interface. These are defined in `enum bootmenu_key` and include
+`BKEY_UP` for moving up and `BKEY_SELECT` for selecting an item. You can use
+`bootmenu_conv_key()` to convert an ASCII key into one of these.
+
+Once a keypress is decoded, call `expo_send_key()` to send it to the expo. This
+may cause an update to the expo state and may produce an action.
+
+Actions
+-------
+
+Call `expo_action_get()` in the event loop to check for any actions that the
+expo wants to report. These can include selecting a particular menu item, or
+quitting the menu. Processing of these is the resposibility of your controller.
+
+Event loop
+----------
+
+Expo is intended to be used in an event loop. For an example loop, see
+`bootflow_menu_run()`. It is possible to perform other work in your event loop,
+such as scanning devices for more bootflows.
+
+Themes
+------
+
+Expo does not itself support themes. The bootflow_menu implement supposed a
+basic theme, applying font sizes to the various text objects in the expo.
+
+API documentation
+-----------------
+
+.. kernel-doc:: include/expo.h
+
+Future ideas
+------------
+
+Some ideas for future work:
+
+- Default menu item and a timeout
+- Higher-level / automatic / more flexible layout of objects
+- Image formats other than BMP
+- Use of ANSI sequences to control a serial terminal
+- Colour selection
+- Better support for handling lots of settings, e.g. with multiple menus and
+  radio/option widgets
+- Mouse support
+- Integrate Nuklear or some other library for a richer UI
+- Optimise rendering by only updating the display with changes since last render
+- Use expo to replace the existing menu implementation
+- Add a Kconfig option to drop the names to save code / data space
+- Add a Kconfig option to disable vidconsole support to save code / data space
+- Support both graphical and text menus at the same time on different devices
+- Implement proper measurement of object bounding boxes, to permit more exact
+  layout. This would tidy up the layout when Truetype is not used
+- Support unicode
+- Support curses for proper serial-terminal menus
+
+.. Simon Glass <sjg@chromium.org>
+.. 7-Oct-22
diff --git a/doc/develop/index.rst b/doc/develop/index.rst
index ce6b38e5762..f43ded4a57f 100644
--- a/doc/develop/index.rst
+++ b/doc/develop/index.rst
@@ -32,6 +32,7 @@ Implementation
    distro
    driver-model/index
    environment
+   expo
    event
    global_data
    logging
-- 
2.38.0.413.g74048e4d9e-goog

Re: [PATCH 00/24] bootstd: Add a boot menu

[Heinrich Schuchardt]

On 10/17/22 22:29, Simon Glass wrote:
> So far standard boot lacks a boot menu, although it is possible to create
> a rudimentary one using the existing 'bootmenu' command.
>
> Even then, this text-based menu offer only basic functionality and does
> not take full advantage of the displays which are common on many devices.
>
> This series provides a 'bootflow menu' command which allows the user to
> select from the available bootflows. An attempt is made to show the name
> of the available operating systems, by reading more information into the
> bootflow. A logo can be read also, where supported, so that this can be
> presented to the user when an option is highlighted.
>
> Full use is made of TrueType fonts, if enabled. For cases where only a
> serial console is available, it falls back to a simple text-based menu.
>
> All of this is implementing using a new 'expo' construct, a collection of
> scenes (like menu screens) which can be navigated by the user to view
> information and select option. This is fairly general and should be able
> to cope with a wider array of use cases, with less hacking of the menu
> code, such as is currently needed for CMD_BOOTEFI_BOOTMGR.
>
> Of course it would be possible to enhance the existing menu rather than
> creating a new setup. Instead it seems better to make the existing menu
> use expo, if code space permits. It avoids the event-loop problem and
> should be more extensible, given its loosely coupled components and use of
> IDs instead of pointers. Further motivation is provided in the
> documentation.
>
> For now the CLI keypress-decoding code is split out to be used by the new
> menu. The key codes defined by menu.h are reused also.
>
> This is of course just a starting point. Some ideas for future work are
> included in the documentation.

Hello Simon,

Linaro's implementation of a boot menu for UEFI booting showed that the
current basis of bootmenu is inadequate for properly designing a GUI
like user interface.

With your patch set you try to overcome this. Your design document in
patch 24 identifies a hierarchy of objects expo - scene - design element
but otherwise remains rather sketchy to me.

Before investing into code we should a more holistic design perspective.

First thing we need to define is our requirements, e.g.

** Which input devices do we want to support in future **

- keyboard
- mouse

** What type of GUI do we want to support **

- text terminal
- graphical output

** Which design elements do we need **

- lists
- trees
- grids
- menus
- labels
- buttons
- drop down lists
- combo boxes
- bitmaps
- screens (you call them scene)
- transactions (you call them expo)

** How do we want to structure our GUI **

- Good practice is to clearly separate model, view, and controller.

** Which constraints do we have to observe? **

- How much is the code base allowed to grow?
- Who will be the long term sponsor and maintainer?

After having this rough cut overview we may start with a draft
implementation and detail our design document.

How do you plan to organize the design work?

Best regards

Heinrich

Re: [PATCH 00/24] bootstd: Add a boot menu

[Simon Glass]

Hi Heinrich,

On Mon, 17 Oct 2022 at 15:11, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 10/17/22 22:29, Simon Glass wrote:
> > So far standard boot lacks a boot menu, although it is possible to create
> > a rudimentary one using the existing 'bootmenu' command.
> >
> > Even then, this text-based menu offer only basic functionality and does
> > not take full advantage of the displays which are common on many devices.
> >
> > This series provides a 'bootflow menu' command which allows the user to
> > select from the available bootflows. An attempt is made to show the name
> > of the available operating systems, by reading more information into the
> > bootflow. A logo can be read also, where supported, so that this can be
> > presented to the user when an option is highlighted.
> >
> > Full use is made of TrueType fonts, if enabled. For cases where only a
> > serial console is available, it falls back to a simple text-based menu.
> >
> > All of this is implementing using a new 'expo' construct, a collection of
> > scenes (like menu screens) which can be navigated by the user to view
> > information and select option. This is fairly general and should be able
> > to cope with a wider array of use cases, with less hacking of the menu
> > code, such as is currently needed for CMD_BOOTEFI_BOOTMGR.
> >
> > Of course it would be possible to enhance the existing menu rather than
> > creating a new setup. Instead it seems better to make the existing menu
> > use expo, if code space permits. It avoids the event-loop problem and
> > should be more extensible, given its loosely coupled components and use of
> > IDs instead of pointers. Further motivation is provided in the
> > documentation.
> >
> > For now the CLI keypress-decoding code is split out to be used by the new
> > menu. The key codes defined by menu.h are reused also.
> >
> > This is of course just a starting point. Some ideas for future work are
> > included in the documentation.
>
> Hello Simon,
>
> Linaro's implementation of a boot menu for UEFI booting showed that the
> current basis of bootmenu is inadequate for properly designing a GUI
> like user interface.
>
> With your patch set you try to overcome this. Your design document in
> patch 24 identifies a hierarchy of objects expo - scene - design element
> but otherwise remains rather sketchy to me.

For now the doc is here:

https://docs.google.com/document/d/1VQeApnLlH6xKm_OI36AhWkJLUEd9OXEvIJXB8aM2de8/edit?usp=sharing&resourcekey=0-DwgHpR2S8vJEJzvvwPb-AQ

>
> Before investing into code we should a more holistic design perspective.
>
> First thing we need to define is our requirements, e.g.
>
> ** Which input devices do we want to support in future **
>
> - keyboard
> - mouse

I have patches for mouse, actually.

>
> ** What type of GUI do we want to support **
>
> - text terminal
> - graphical output
>
> ** Which design elements do we need **
>
> - lists
> - trees
> - grids
> - menus
> - labels
> - buttons
> - drop down lists
> - combo boxes
> - bitmaps
> - screens (you call them scene)
> - transactions (you call them expo)

(this series has rudimentary menus, labels and bitmaps, but does not
support the others)

>
> ** How do we want to structure our GUI **
>
> - Good practice is to clearly separate model, view, and controller.

Expo is the view. Controller is the client code. Model is currently
the expo, updated by the controller. That part of it needs though if
we are to expand the capabilities of this effort.

>
> ** Which constraints do we have to observe? **
>
> - How much is the code base allowed to grow?
> - Who will be the long term sponsor and maintainer?
>
> After having this rough cut overview we may start with a draft
> implementation and detail our design document.
>
> How do you plan to organize the design work?

See also the bottom of the docs for some future work, also whether we
should consider using a GUI library like Nuklear in the future:

https://patchwork.ozlabs.org/project/uboot/patch/20221017203000.2207887-25-sjg@chromium.org/

It has evolved since that doc. The doc needs to evolve further.
Perhaps we can start by agreeing the requirements.

Regards,
Simon

Re: [PATCH 03/24] bootmenu: Add a few comments

[Heinrich Schuchardt]

On 10/17/22 22:29, Simon Glass wrote:
> The behaviour of these two functions is completely undocumented. Add some
> notes so the poor, suffering dev can figure out what is going on.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   include/menu.h | 42 ++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 42 insertions(+)
>
> diff --git a/include/menu.h b/include/menu.h
> index 702aacb170c..0b4d9734149 100644
> --- a/include/menu.h
> +++ b/include/menu.h
> @@ -42,6 +42,7 @@ struct bootmenu_data {
>   	struct bootmenu_entry *first;	/* first menu entry */
>   };
>
> +/** enum bootmenu_key - keys that can be returned by the bootmenu */
>   enum bootmenu_key {
>   	KEY_NONE = 0,
>   	KEY_UP,
> @@ -53,8 +54,49 @@ enum bootmenu_key {
>   	KEY_SPACE,
>   };
>
> +/**
> + * bootmenu_autoboot_loop() - handle autobooting if no key is pressed
> + *
> + * This shows a prompt to allow the user to press a key to interrupt auto boot
> + * of the first menu option.
> + *
> + * It then waits for the required time (menu->delay in seconds) for a key to be
> + * pressed. If nothing is pressed in that time, @key returns KEY_SELECT
> + * indicating that the current option should be chosen.
> + *
> + * @menu: Menu being processed
> + * @key: Returns the code for the key the user pressed:
> + *	enter: KEY_SELECT
> + *	Ctrl-C: KEY_QUIT
> + *	anything else: KEY_NONE
> + * @esc: Set to 1 if the escape key is pressed, otherwise not updated
> + */
>   void bootmenu_autoboot_loop(struct bootmenu_data *menu,
>   			    enum bootmenu_key *key, int *esc);
> +
> +/**
> + * bootmenu_loop() - handle waiting for a keypress when autoboot is disabled
> + *
> + * This is used when the menu delay is negative, indicating that the delay has
> + * elapsed, or there was no delay to begin with.

Unfortunately the description does not match the code.

This function is entered if some key was pressed, so autoboot was
stopped. When the delay elapses the default action is taken by the caller.

> + *
> + * It reads a character and processes it, returning a menu-key code if a
> + * character is recognised
> + *
> + * @menu: Menu being processed
> + * @key: Returns the code for the key the user pressed:
> + *	enter: KEY_SELECT
> + *	Ctrl-C: KEY_QUIT
> + *	Up arrow: KEY_UP
> + *	Down arrow: KEY_DOWN
> + *	Escape (by itself): KEY_QUIT
> + *	Plus: KEY_PLUS
> + *	Minus: KEY_MINUS
> + *	Space: KEY_SPACE

We already discussed that this list is to change. We should support
accelerator keys in menus.

> + * @esc: On input, a non-zero value indicates that an escape sequence has

1 indicates that the ESC key has been pressed.
All other values indicate that the ESC key has not been pressed.

--

The whole design is broken in that the concept of a menu is not properly
encapsulated.

A function like bootmenu_autoboot_loop() should not be exported.

The view side of a menu should be in a function that takes the following
arguments:

- the location (x,y) on the screen
- the size (dx, dy) of the displayed menu
   (further items should be viewed by vertical scrolling,
   more characters by horizontal scrolling)
- a list of menu items to display
- a event function called whenever the selected menu item changes (or NULL)

Such an event function will allow to display extra information for a
menu item.

The model would be an array of utf-8 strings.

The controller will invoke the view function and receive the index of
the selected item as the return value (or -1 if ESC is pressed).

Best regards

Heinrich

> + *	resulted in that many characters so far. On exit this is updated to the
> + *	new number of characters
> + */
>   void bootmenu_loop(struct bootmenu_data *menu,
>   		   enum bootmenu_key *key, int *esc);
>

Re: [PATCH 04/24] menu: Rename KEY_... to BKEY_...

[Heinrich Schuchardt]

On 10/17/22 22:29, Simon Glass wrote:
> This enum values conflict with linux/input.h so rename them.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   cmd/bootmenu.c  | 10 +++++-----
>   cmd/eficonfig.c | 26 +++++++++++++-------------
>   common/menu.c   | 34 +++++++++++++++++-----------------
>   include/menu.h  | 32 ++++++++++++++++----------------
>   4 files changed, 51 insertions(+), 51 deletions(-)
>
> diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> index 3340be16325..c80004c54dc 100644
> --- a/cmd/bootmenu.c
> +++ b/cmd/bootmenu.c
> @@ -86,7 +86,7 @@ static char *bootmenu_choice_entry(void *data)
>   {

The argument should be of type struct bootmenu_data* and not void.

This function and the enum should be move to common/menu.c and not exported.

common/menu.c does not import linux/input.h so there will be no conflict
but that sould not stop you from renaming the constants.

Best regards

Heinrich

>   	struct bootmenu_data *menu = data;
>   	struct bootmenu_entry *iter;
> -	enum bootmenu_key key = KEY_NONE;
> +	enum bootmenu_key key = BKEY_NONE;
>   	int esc = 0;
>   	int i;
>
> @@ -100,22 +100,22 @@ static char *bootmenu_choice_entry(void *data)
>   		}
>
>   		switch (key) {
> -		case KEY_UP:
> +		case BKEY_UP:
>   			if (menu->active > 0)
>   				--menu->active;
>   			/* no menu key selected, regenerate menu */
>   			return NULL;
> -		case KEY_DOWN:
> +		case BKEY_DOWN:
>   			if (menu->active < menu->count - 1)
>   				++menu->active;
>   			/* no menu key selected, regenerate menu */
>   			return NULL;
> -		case KEY_SELECT:
> +		case BKEY_SELECT:
>   			iter = menu->first;
>   			for (i = 0; i < menu->active; ++i)
>   				iter = iter->next;
>   			return iter->key;
> -		case KEY_QUIT:
> +		case BKEY_QUIT:
>   			/* Quit by choosing the last entry - U-Boot console */
>   			iter = menu->first;
>   			while (iter->next)
> diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> index 2595dd95631..de7ce24f5ac 100644
> --- a/cmd/eficonfig.c
> +++ b/cmd/eficonfig.c
> @@ -193,31 +193,31 @@ static char *eficonfig_choice_entry(void *data)
>   	int esc = 0;
>   	struct list_head *pos, *n;
>   	struct eficonfig_entry *entry;
> -	enum bootmenu_key key = KEY_NONE;
> +	enum bootmenu_key key = BKEY_NONE;
>   	struct efimenu *efi_menu = data;
>
>   	while (1) {
>   		bootmenu_loop((struct bootmenu_data *)efi_menu, &key, &esc);
>
>   		switch (key) {
> -		case KEY_UP:
> +		case BKEY_UP:
>   			if (efi_menu->active > 0)
>   				--efi_menu->active;
>   			/* no menu key selected, regenerate menu */
>   			return NULL;
> -		case KEY_DOWN:
> +		case BKEY_DOWN:
>   			if (efi_menu->active < efi_menu->count - 1)
>   				++efi_menu->active;
>   			/* no menu key selected, regenerate menu */
>   			return NULL;
> -		case KEY_SELECT:
> +		case BKEY_SELECT:
>   			list_for_each_safe(pos, n, &efi_menu->list) {
>   				entry = list_entry(pos, struct eficonfig_entry, list);
>   				if (entry->num == efi_menu->active)
>   					return entry->key;
>   			}
>   			break;
> -		case KEY_QUIT:
> +		case BKEY_QUIT:
>   			/* Quit by choosing the last entry */
>   			entry = list_last_entry(&efi_menu->list, struct eficonfig_entry, list);
>   			return entry->key;
> @@ -1864,14 +1864,14 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
>   	int esc = 0;
>   	struct list_head *pos, *n;
>   	struct eficonfig_boot_order *tmp;
> -	enum bootmenu_key key = KEY_NONE;
> +	enum bootmenu_key key = BKEY_NONE;
>   	struct eficonfig_boot_order *entry;
>
>   	while (1) {
>   		bootmenu_loop(NULL, &key, &esc);
>
>   		switch (key) {
> -		case KEY_PLUS:
> +		case BKEY_PLUS:
>   			if (efi_menu->active > 0) {
>   				list_for_each_safe(pos, n, &efi_menu->list) {
>   					entry = list_entry(pos, struct eficonfig_boot_order, list);
> @@ -1885,11 +1885,11 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
>   				list_add(&tmp->list, &entry->list);
>   			}
>   			fallthrough;
> -		case KEY_UP:
> +		case BKEY_UP:
>   			if (efi_menu->active > 0)
>   				--efi_menu->active;
>   			return EFI_NOT_READY;
> -		case KEY_MINUS:
> +		case BKEY_MINUS:
>   			if (efi_menu->active < efi_menu->count - 3) {
>   				list_for_each_safe(pos, n, &efi_menu->list) {
>   					entry = list_entry(pos, struct eficonfig_boot_order, list);
> @@ -1905,11 +1905,11 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
>   				++efi_menu->active;
>   			}
>   			return EFI_NOT_READY;
> -		case KEY_DOWN:
> +		case BKEY_DOWN:
>   			if (efi_menu->active < efi_menu->count - 1)
>   				++efi_menu->active;
>   			return EFI_NOT_READY;
> -		case KEY_SELECT:
> +		case BKEY_SELECT:
>   			/* "Save" */
>   			if (efi_menu->active == efi_menu->count - 2)
>   				return EFI_SUCCESS;
> @@ -1919,7 +1919,7 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
>   				return EFI_ABORTED;
>
>   			break;
> -		case KEY_SPACE:
> +		case BKEY_SPACE:
>   			if (efi_menu->active < efi_menu->count - 2) {
>   				list_for_each_safe(pos, n, &efi_menu->list) {
>   					entry = list_entry(pos, struct eficonfig_boot_order, list);
> @@ -1930,7 +1930,7 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
>   				}
>   			}
>   			break;
> -		case KEY_QUIT:
> +		case BKEY_QUIT:
>   			return EFI_ABORTED;
>   		default:
>   			/* Pressed key is not valid, no need to regenerate the menu */
> diff --git a/common/menu.c b/common/menu.c
> index 0d19601cf53..087e4c246e2 100644
> --- a/common/menu.c
> +++ b/common/menu.c
> @@ -446,16 +446,16 @@ void bootmenu_autoboot_loop(struct bootmenu_data *menu,
>   			switch (c) {
>   			case '\e':
>   				*esc = 1;
> -				*key = KEY_NONE;
> +				*key = BKEY_NONE;
>   				break;
>   			case '\r':
> -				*key = KEY_SELECT;
> +				*key = BKEY_SELECT;
>   				break;
>   			case 0x3: /* ^C */
> -				*key = KEY_QUIT;
> +				*key = BKEY_QUIT;
>   				break;
>   			default:
> -				*key = KEY_NONE;
> +				*key = BKEY_NONE;
>   				break;
>   			}
>
> @@ -471,7 +471,7 @@ void bootmenu_autoboot_loop(struct bootmenu_data *menu,
>   	printf(ANSI_CURSOR_POSITION ANSI_CLEAR_LINE, menu->count + 5, 1);
>
>   	if (menu->delay == 0)
> -		*key = KEY_SELECT;
> +		*key = BKEY_SELECT;
>   }
>
>   void bootmenu_loop(struct bootmenu_data *menu,
> @@ -503,17 +503,17 @@ void bootmenu_loop(struct bootmenu_data *menu,
>   		/* First char of ANSI escape sequence '\e' */
>   		if (c == '\e') {
>   			*esc = 1;
> -			*key = KEY_NONE;
> +			*key = BKEY_NONE;
>   		}
>   		break;
>   	case 1:
>   		/* Second char of ANSI '[' */
>   		if (c == '[') {
>   			*esc = 2;
> -			*key = KEY_NONE;
> +			*key = BKEY_NONE;
>   		} else {
>   		/* Alone ESC key was pressed */
> -			*key = KEY_QUIT;
> +			*key = BKEY_QUIT;
>   			*esc = (c == '\e') ? 1 : 0;
>   		}
>   		break;
> @@ -522,7 +522,7 @@ void bootmenu_loop(struct bootmenu_data *menu,
>   		/* Third char of ANSI (number '1') - optional */
>   		if (*esc == 2 && c == '1') {
>   			*esc = 3;
> -			*key = KEY_NONE;
> +			*key = BKEY_NONE;
>   			break;
>   		}
>
> @@ -530,31 +530,31 @@ void bootmenu_loop(struct bootmenu_data *menu,
>
>   		/* ANSI 'A' - key up was pressed */
>   		if (c == 'A')
> -			*key = KEY_UP;
> +			*key = BKEY_UP;
>   		/* ANSI 'B' - key down was pressed */
>   		else if (c == 'B')
> -			*key = KEY_DOWN;
> +			*key = BKEY_DOWN;
>   		/* other key was pressed */
>   		else
> -			*key = KEY_NONE;
> +			*key = BKEY_NONE;
>
>   		break;
>   	}
>
>   	/* enter key was pressed */
>   	if (c == '\r')
> -		*key = KEY_SELECT;
> +		*key = BKEY_SELECT;
>
>   	/* ^C was pressed */
>   	if (c == 0x3)
> -		*key = KEY_QUIT;
> +		*key = BKEY_QUIT;
>
>   	if (c == '+')
> -		*key = KEY_PLUS;
> +		*key = BKEY_PLUS;
>
>   	if (c == '-')
> -		*key = KEY_MINUS;
> +		*key = BKEY_MINUS;
>
>   	if (c == ' ')
> -		*key = KEY_SPACE;
> +		*key = BKEY_SPACE;
>   }
> diff --git a/include/menu.h b/include/menu.h
> index 0b4d9734149..29b457921e9 100644
> --- a/include/menu.h
> +++ b/include/menu.h
> @@ -44,14 +44,14 @@ struct bootmenu_data {
>
>   /** enum bootmenu_key - keys that can be returned by the bootmenu */
>   enum bootmenu_key {
> -	KEY_NONE = 0,
> -	KEY_UP,
> -	KEY_DOWN,
> -	KEY_SELECT,
> -	KEY_QUIT,
> -	KEY_PLUS,
> -	KEY_MINUS,
> -	KEY_SPACE,
> +	BKEY_NONE = 0,
> +	BKEY_UP,
> +	BKEY_DOWN,
> +	BKEY_SELECT,
> +	BKEY_QUIT,
> +	BKEY_PLUS,
> +	BKEY_MINUS,
> +	BKEY_SPACE,
>   };
>
>   /**
> @@ -85,14 +85,14 @@ void bootmenu_autoboot_loop(struct bootmenu_data *menu,
>    *
>    * @menu: Menu being processed
>    * @key: Returns the code for the key the user pressed:
> - *	enter: KEY_SELECT
> - *	Ctrl-C: KEY_QUIT
> - *	Up arrow: KEY_UP
> - *	Down arrow: KEY_DOWN
> - *	Escape (by itself): KEY_QUIT
> - *	Plus: KEY_PLUS
> - *	Minus: KEY_MINUS
> - *	Space: KEY_SPACE
> + *	enter: BKEY_SELECT
> + *	Ctrl-C: BKEY_QUIT
> + *	Up arrow: BKEY_UP
> + *	Down arrow: BKEY_DOWN
> + *	Escape (by itself): BKEY_QUIT
> + *	Plus: BKEY_PLUS
> + *	Minus: BKEY_MINUS
> + *	Space: BKEY_SPACE
>    * @esc: On input, a non-zero value indicates that an escape sequence has
>    *	resulted in that many characters so far. On exit this is updated to the
>    *	new number of characters

Re: [PATCH 05/24] menu: Update bootmenu_autoboot_loop() to return the code

[Heinrich Schuchardt]

On 10/17/22 22:29, Simon Glass wrote:
> Use the return value to save having to pass around a pointer. This also
> resolves any ambiguity about what *key contains when the function is
> called.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   cmd/bootmenu.c |  2 +-
>   common/menu.c  | 16 +++++++++-------
>   include/menu.h |  7 +++----
>   3 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> index c80004c54dc..0e22f504fe4 100644
> --- a/cmd/bootmenu.c
> +++ b/cmd/bootmenu.c
> @@ -93,7 +93,7 @@ static char *bootmenu_choice_entry(void *data)

First of all this function should be moved to common/menu.c.

>   	while (1) {
>   		if (menu->delay >= 0) {
>   			/* Autoboot was not stopped */
> -			bootmenu_autoboot_loop(menu, &key, &esc);
> +			key = bootmenu_autoboot_loop(menu, &esc);
>   		} else {
>   			/* Some key was pressed, so autoboot was stopped */
>   			bootmenu_loop(menu, &key, &esc);
> diff --git a/common/menu.c b/common/menu.c
> index 087e4c246e2..bccc104a39b 100644
> --- a/common/menu.c
> +++ b/common/menu.c
> @@ -425,9 +425,9 @@ int menu_destroy(struct menu *m)
>   	return 1;
>   }
>
> -void bootmenu_autoboot_loop(struct bootmenu_data *menu,
> -			    enum bootmenu_key *key, int *esc)
> +enum bootmenu_key bootmenu_autoboot_loop(struct bootmenu_data *menu, int *esc)
>   {
> +	enum bootmenu_key key = BKEY_NONE;
>   	int i, c;
>
>   	while (menu->delay > 0) {
> @@ -446,16 +446,16 @@ void bootmenu_autoboot_loop(struct bootmenu_data *menu,
>   			switch (c) {
>   			case '\e':
>   				*esc = 1;
> -				*key = BKEY_NONE;
> +				key = BKEY_NONE;
>   				break;
>   			case '\r':
> -				*key = BKEY_SELECT;
> +				key = BKEY_SELECT;
>   				break;
>   			case 0x3: /* ^C */
> -				*key = BKEY_QUIT;
> +				key = BKEY_QUIT;
>   				break;
>   			default:
> -				*key = BKEY_NONE;
> +				key = BKEY_NONE;
>   				break;
>   			}
>
> @@ -471,7 +471,9 @@ void bootmenu_autoboot_loop(struct bootmenu_data *menu,
>   	printf(ANSI_CURSOR_POSITION ANSI_CLEAR_LINE, menu->count + 5, 1);
>
>   	if (menu->delay == 0)
> -		*key = BKEY_SELECT;
> +		key = BKEY_SELECT;
> +
> +	return key;
>   }
>
>   void bootmenu_loop(struct bootmenu_data *menu,
> diff --git a/include/menu.h b/include/menu.h
> index 29b457921e9..9f30a3c1acd 100644
> --- a/include/menu.h
> +++ b/include/menu.h
> @@ -65,14 +65,13 @@ enum bootmenu_key {
>    * indicating that the current option should be chosen.
>    *
>    * @menu: Menu being processed
> - * @key: Returns the code for the key the user pressed:
> + * @esc: Set to 1 if the escape key is pressed, otherwise not updated
> + * Returns: code for the key the user pressed:
>    *	enter: KEY_SELECT
>    *	Ctrl-C: KEY_QUIT
>    *	anything else: KEY_NONE
> - * @esc: Set to 1 if the escape key is pressed, otherwise not updated
>    */
> -void bootmenu_autoboot_loop(struct bootmenu_data *menu,
> -			    enum bootmenu_key *key, int *esc);
> +enum bootmenu_key bootmenu_autoboot_loop(struct bootmenu_data *menu, int *esc);

This function should not be exported.

enum bootmenu cannot accomomdate accelerator keys. The return type
should be int.

Best regards

Heinrich

>
>   /**
>    * bootmenu_loop() - handle waiting for a keypress when autoboot is disabled

Re: [PATCH 07/24] menu: Use a switch statement

[Heinrich Schuchardt]

On 10/17/22 22:29, Simon Glass wrote:
> Convert the long line of if() statements to a switch() since this makes
> better use of the C language.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   common/menu.c | 31 ++++++++++++++++++++-----------
>   1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/common/menu.c b/common/menu.c
> index 22947f5d693..1aa78b762a4 100644
> --- a/common/menu.c
> +++ b/common/menu.c
> @@ -543,22 +543,31 @@ enum bootmenu_key bootmenu_loop(struct bootmenu_data *menu, int *esc)
>   		break;
>   	}
>
> -	/* enter key was pressed */
> -	if (c == '\r')
> +	switch (c) {
> +	case '\r':
> +		/* enter key was pressed */
>   		key = BKEY_SELECT;
> -
> -	/* ^C was pressed */
> -	if (c == 0x3)
> +		break;
> +	case CTL_CH('c'):
> +		/* ^C was pressed */
>   		key = BKEY_QUIT;
> -
> -	if (c == '+')
> +		break;
> +	case CTL_CH('p'):
> +		key = BKEY_UP;
> +		break;
> +	case CTL_CH('n'):
> +		key = BKEY_DOWN;
> +		break;
> +	case '+':
>   		key = BKEY_PLUS;
> -
> -	if (c == '-')
> +		break;
> +	case '-':
>   		key = BKEY_MINUS;
> -
> -	if (c == ' ')
> +		break;
> +	case ' ':
>   		key = BKEY_SPACE;
> +		break;
> +	}

The whole code is not well suited to parse all of the many different
escape sequences that can be sent to convey modifier keys or
non-character keys (think of <CTRL><ALT><F12> or <HOME>).

We should move the of logic of efi_cin_read_key_stroke_ex() to a library
function which is used both by bootmenu and UEFI and remove duplicate code.

Best regards

Heinrich

>
>   	return key;
>   }

Re: [PATCH 09/24] image: Add a function to find a script in an image

[Heinrich Schuchardt]

On 10/17/22 22:29, Simon Glass wrote:
> Split this functionality out of the 'source' command so it can be used
> from another place.

It is not obvious how this relates to changing the series.

Please, provide a description of the future use of the extracted library
function.

>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   cmd/source.c    | 173 ++++++++++++++++++++++++++----------------------
>   include/image.h |  13 ++++

If you extract a library function, it should not live in cmd/source.c.

Best regards

Heinrich

>   2 files changed, 107 insertions(+), 79 deletions(-)
>
> diff --git a/cmd/source.c b/cmd/source.c
> index 698d9f86d96..dfa995f1df6 100644
> --- a/cmd/source.c
> +++ b/cmd/source.c
> @@ -24,7 +24,6 @@
>   #include <asm/byteorder.h>
>   #include <asm/io.h>
>
> -#if defined(CONFIG_FIT)
>   /**
>    * get_default_image() - Return default property from /images
>    *
> @@ -40,123 +39,139 @@ static const char *get_default_image(const void *fit)
>
>   	return fdt_getprop(fit, images_noffset, FIT_DEFAULT_PROP, NULL);
>   }
> -#endif
>
> -int image_source_script(ulong addr, const char *fit_uname)
> +int image_locate_script(void *buf, int size, const char *fit_uname,
> +			char **datap, uint *lenp)
>   {
>   	ulong		len;
> -#if defined(CONFIG_LEGACY_IMAGE_FORMAT)
>   	const struct legacy_img_hdr *hdr;
> -#endif
>   	u32		*data;
>   	int		verify;
> -	void *buf;
> -#if defined(CONFIG_FIT)
>   	const void*	fit_hdr;
>   	int		noffset;
>   	const void	*fit_data;
>   	size_t		fit_len;
> -#endif
>
>   	verify = env_get_yesno("verify");
>
> -	buf = map_sysmem(addr, 0);
>   	switch (genimg_get_format(buf)) {
> -#if defined(CONFIG_LEGACY_IMAGE_FORMAT)
>   	case IMAGE_FORMAT_LEGACY:
> -		hdr = buf;
> +		if (IS_ENABLED(CONFIG_LEGACY_IMAGE_FORMAT)) {
> +			hdr = buf;
>
> -		if (!image_check_magic (hdr)) {
> -			puts ("Bad magic number\n");
> -			return 1;
> -		}
> +			if (!image_check_magic(hdr)) {
> +				puts("Bad magic number\n");
> +				return 1;
> +			}
>
> -		if (!image_check_hcrc (hdr)) {
> -			puts ("Bad header crc\n");
> -			return 1;
> -		}
> +			if (!image_check_hcrc(hdr)) {
> +				puts("Bad header crc\n");
> +				return 1;
> +			}
> +
> +			if (verify) {
> +				if (!image_check_dcrc(hdr)) {
> +					puts("Bad data crc\n");
> +					return 1;
> +				}
> +			}
>
> -		if (verify) {
> -			if (!image_check_dcrc (hdr)) {
> -				puts ("Bad data crc\n");
> +			if (!image_check_type(hdr, IH_TYPE_SCRIPT)) {
> +				puts("Bad image type\n");
>   				return 1;
>   			}
> -		}
>
> -		if (!image_check_type (hdr, IH_TYPE_SCRIPT)) {
> -			puts ("Bad image type\n");
> -			return 1;
> -		}
> +			/* get length of script */
> +			data = (u32 *)image_get_data(hdr);
>
> -		/* get length of script */
> -		data = (u32 *)image_get_data (hdr);
> +			len = uimage_to_cpu(*data);
> +			if (!len) {
> +				puts("Empty Script\n");
> +				return 1;
> +			}
>
> -		if ((len = uimage_to_cpu (*data)) == 0) {
> -			puts ("Empty Script\n");
> -			return 1;
> +			/*
> +			 * scripts are just multi-image files with one
> +			 * component, so seek past the zero-terminated sequence
> +			 * of image lengths to get to the actual image data
> +			 */
> +			while (*data++);
>   		}
> -
> -		/*
> -		 * scripts are just multi-image files with one component, seek
> -		 * past the zero-terminated sequence of image lengths to get
> -		 * to the actual image data
> -		 */
> -		while (*data++);
>   		break;
> -#endif
> -#if defined(CONFIG_FIT)
>   	case IMAGE_FORMAT_FIT:
> -		fit_hdr = buf;
> -		if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) {
> -			puts ("Bad FIT image format\n");
> -			return 1;
> -		}
> +		if (IS_ENABLED(CONFIG_FIT)) {
> +			fit_hdr = buf;
> +			if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) {
> +				puts("Bad FIT image format\n");
> +				return 1;
> +			}
>
> -		if (!fit_uname)
> -			fit_uname = get_default_image(fit_hdr);
> +			if (!fit_uname)
> +				fit_uname = get_default_image(fit_hdr);
>
> -		if (!fit_uname) {
> -			puts("No FIT subimage unit name\n");
> -			return 1;
> -		}
> +			if (!fit_uname) {
> +				puts("No FIT subimage unit name\n");
> +				return 1;
> +			}
>
> -		/* get script component image node offset */
> -		noffset = fit_image_get_node (fit_hdr, fit_uname);
> -		if (noffset < 0) {
> -			printf ("Can't find '%s' FIT subimage\n", fit_uname);
> -			return 1;
> -		}
> +			/* get script component image node offset */
> +			noffset = fit_image_get_node(fit_hdr, fit_uname);
> +			if (noffset < 0) {
> +				printf("Can't find '%s' FIT subimage\n",
> +				       fit_uname);
> +				return 1;
> +			}
>
> -		if (!fit_image_check_type (fit_hdr, noffset, IH_TYPE_SCRIPT)) {
> -			puts ("Not a image image\n");
> -			return 1;
> -		}
> +			if (!fit_image_check_type(fit_hdr, noffset,
> +						  IH_TYPE_SCRIPT)) {
> +				puts("Not a image image\n");
> +				return 1;
> +			}
> +
> +			/* verify integrity */
> +			if (verify) {
> +				if (!fit_image_verify(fit_hdr, noffset)) {
> +					puts("Bad Data Hash\n");
> +					return 1;
> +				}
> +			}
>
> -		/* verify integrity */
> -		if (verify) {
> -			if (!fit_image_verify(fit_hdr, noffset)) {
> -				puts ("Bad Data Hash\n");
> +			/* get script subimage data address and length */
> +			if (fit_image_get_data(fit_hdr, noffset, &fit_data, &fit_len)) {
> +				puts("Could not find script subimage data\n");
>   				return 1;
>   			}
> -		}
>
> -		/* get script subimage data address and length */
> -		if (fit_image_get_data (fit_hdr, noffset, &fit_data, &fit_len)) {
> -			puts ("Could not find script subimage data\n");
> -			return 1;
> +			data = (u32 *)fit_data;
> +			len = (ulong)fit_len;
>   		}
> -
> -		data = (u32 *)fit_data;
> -		len = (ulong)fit_len;
>   		break;
> -#endif
>   	default:
> -		puts ("Wrong image format for \"source\" command\n");
> -		return 1;
> +		puts("Wrong image format for \"source\" command\n");
> +		return -EPERM;
>   	}
>
> -	debug("** Script length: %ld\n", len);
> -	return run_command_list((char *)data, len, 0);
> +	*datap = (char *)data;
> +	*lenp = len;
> +
> +	return 0;
> +}
> +
> +int image_source_script(ulong addr, const char *fit_uname)
> +{
> +	char *data;
> +	void *buf;
> +	uint len;
> +	int ret;
> +
> +	buf = map_sysmem(addr, 0);
> +	ret = image_locate_script(buf, 0, fit_uname, &data, &len);
> +	unmap_sysmem(buf);
> +	if (ret)
> +		return CMD_RET_FAILURE;
> +
> +	debug("** Script length: %d\n", len);
> +	return run_command_list(data, len, 0);
>   }
>
>   /**************************************************/
> diff --git a/include/image.h b/include/image.h
> index feb8e90c2e8..1cbc51ca228 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -720,6 +720,19 @@ int fit_image_load(struct bootm_headers *images, ulong addr,
>    */
>   int image_source_script(ulong addr, const char *fit_uname);
>
> +/**
> + * image_locate_script() - Locate the raw script in an image
> + *
> + * @buf: Address of image
> + * @size: Size of image in bytes
> + * @fit_uname: Node name of FIT image to read
> + * @datap: Returns pointer to raw script on success
> + * @lenp: Returns size of raw script on success
> + * @return 0 if OK, non-zero on error
> + */
> +int image_locate_script(void *buf, int size, const char *fit_uname,
> +			char **datap, uint *lenp);
> +
>   /**
>    * fit_get_node_from_config() - Look up an image a FIT by type
>    *

Re: [PATCH 10/24] video: Enable VIDEO_ANSI by default only with EFI

[Heinrich Schuchardt]

On 10/17/22 22:29, Simon Glass wrote:
> This is not generally needed unless EFI_LOADER is used. Adjust the default
> setting to reduce the size of the U-Boot build.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   drivers/video/Kconfig | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 32938376655..ed77815504b 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -118,10 +118,13 @@ config VIDEO_BPP32
>   config VIDEO_ANSI
>   	bool "Support ANSI escape sequences in video console"
>   	depends on DM_VIDEO
> -	default y
> +	default y if EFI_LOADER

This will cripple the CLS command.

Best regards

Heinrich

>   	help
>   	  Enable ANSI escape sequence decoding for a more fully functional
> -	  console.
> +	  console. Functionality includes changing the text colour and moving
> +	  the cursor. These date from the 1970s and are still widely used today
> +	  to control a text terminal. U-Boot implements these by decoding the
> +	  sequences and performing the appropriate operation.
>
>   config VIDEO_MIPI_DSI
>   	bool "Support MIPI DSI interface"

Re: [PATCH 12/24] video: Fix unchnaged typo

[Heinrich Schuchardt]

On 10/17/22 22:29, Simon Glass wrote:
> Fix this typo in the header file.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

> ---
>
>   include/video.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/video.h b/include/video.h
> index 529f9685183..7dee94cc265 100644
> --- a/include/video.h
> +++ b/include/video.h
> @@ -248,7 +248,7 @@ void video_bmp_get_info(void *bmp_image, ulong *widthp, ulong *heightp,
>    *		  that direction
>    *		- if a coordinate is -ve then it will be offset to the
>    *		  left/top of the centre by that many pixels
> - *		- if a coordinate is positive it will be used unchnaged.
> + *		- if a coordinate is positive it will be used unchanged.
>    * Return: 0 if OK, -ve on error
>    */
>   int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y,

Re: [PATCH 14/24] bootstd: Read the Operating System name for distro/scripts

[Heinrich Schuchardt]

On 10/17/22 22:29, Simon Glass wrote:
> Add the concept of an OS name to the bootflow. This typically includes the
> OS name, version and kernel version.
>
> Implement this for the distro and script bootmeths so that it works with
> Armbian and older version of Fedora.

How do you plan to detect the distro?

Best regards

Heinrich

>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   boot/bootflow.c        |  1 +
>   boot/bootmeth_distro.c | 36 ++++++++++++++++++++++++++++++++++++
>   boot/bootmeth_script.c | 34 ++++++++++++++++++++++++++++++++++
>   cmd/bootflow.c         |  1 +
>   include/bootflow.h     |  3 +++
>   test/boot/bootflow.c   |  1 +
>   6 files changed, 76 insertions(+)
>
> diff --git a/boot/bootflow.c b/boot/bootflow.c
> index f9ad4099244..163cd4953dd 100644
> --- a/boot/bootflow.c
> +++ b/boot/bootflow.c
> @@ -354,6 +354,7 @@ void bootflow_free(struct bootflow *bflow)
>   	free(bflow->subdir);
>   	free(bflow->fname);
>   	free(bflow->buf);
> +	free(bflow->os_name);
>   }
>
>   void bootflow_remove(struct bootflow *bflow)
> diff --git a/boot/bootmeth_distro.c b/boot/bootmeth_distro.c
> index 5c6c687f0a6..6ef0fa1f2c9 100644
> --- a/boot/bootmeth_distro.c
> +++ b/boot/bootmeth_distro.c
> @@ -66,6 +66,38 @@ static int distro_check(struct udevice *dev, struct bootflow_iter *iter)
>   	return 0;
>   }
>
> +/**
> + * distro_fill_info() - Decode the extlinux file to find out distro info
> + *
> + * @bflow: Bootflow to process
> + * @return 0 if OK, -ve on error
> + */
> +static int distro_fill_info(struct bootflow *bflow)
> +{
> +	struct membuff mb;
> +	char line[200];
> +	char *data;
> +	int len;
> +
> +	log_debug("parsing bflow file size %x\n", bflow->size);
> +	membuff_init(&mb, bflow->buf, bflow->size);
> +	membuff_putraw(&mb, bflow->size, true, &data);
> +	while (len = membuff_readline(&mb, line, sizeof(line) - 1, ' '), len) {
> +		char *tok, *p = line;
> +
> +		tok = strsep(&p, " ");
> +		if (p) {
> +			if (!strcmp("label", tok)) {
> +				bflow->os_name = strdup(p);
> +				if (!bflow->os_name)
> +					return log_msg_ret("os", -ENOMEM);
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>   static int distro_read_bootflow(struct udevice *dev, struct bootflow *bflow)
>   {
>   	struct blk_desc *desc;
> @@ -99,6 +131,10 @@ static int distro_read_bootflow(struct udevice *dev, struct bootflow *bflow)
>   	if (ret)
>   		return log_msg_ret("read", ret);
>
> +	ret = distro_fill_info(bflow);
> +	if (ret)
> +		return log_msg_ret("inf", ret);
> +
>   	return 0;
>   }
>
> diff --git a/boot/bootmeth_script.c b/boot/bootmeth_script.c
> index d1c3f940037..8ba9d93a080 100644
> --- a/boot/bootmeth_script.c
> +++ b/boot/bootmeth_script.c
> @@ -35,6 +35,36 @@ static int script_check(struct udevice *dev, struct bootflow_iter *iter)
>   	return 0;
>   }
>
> +/**
> + * script_fill_info() - Decode the U-Boot script to find out distro info
> + *
> + * @bflow: Bootflow to process
> + * @return 0 if OK, -ve on error
> + */
> +static int script_fill_info(struct bootflow *bflow)
> +{
> +	char *name = NULL;
> +	char *data;
> +	uint len;
> +	int ret;
> +
> +	log_debug("parsing bflow file size %x\n", bflow->size);
> +
> +	ret = image_locate_script(bflow->buf, bflow->size, NULL, &data, &len);
> +	if (!ret) {
> +		if (strstr(data, "armbianEnv"))
> +			name = "Armbian";
> +	}
> +
> +	if (name) {
> +		bflow->os_name = strdup(name);
> +		if (!bflow->os_name)
> +			return log_msg_ret("os", -ENOMEM);
> +	}
> +
> +	return 0;
> +}
> +
>   static int script_read_bootflow(struct udevice *dev, struct bootflow *bflow)
>   {
>   	struct blk_desc *desc = NULL;
> @@ -75,6 +105,10 @@ static int script_read_bootflow(struct udevice *dev, struct bootflow *bflow)
>   	if (ret)
>   		return log_msg_ret("read", ret);
>
> +	ret = script_fill_info(bflow);
> +	if (ret)
> +		return log_msg_ret("inf", ret);
> +
>   	return 0;
>   }
>
> diff --git a/cmd/bootflow.c b/cmd/bootflow.c
> index 313103d2775..6b8ac8c8504 100644
> --- a/cmd/bootflow.c
> +++ b/cmd/bootflow.c
> @@ -337,6 +337,7 @@ static int do_bootflow_info(struct cmd_tbl *cmdtp, int flag, int argc,
>   	printf("Filename:  %s\n", bflow->fname);
>   	printf("Buffer:    %lx\n", (ulong)map_to_sysmem(bflow->buf));
>   	printf("Size:      %x (%d bytes)\n", bflow->size, bflow->size);
> +	printf("OS:        %s\n", bflow->os_name ? bflow->os_name : "(none)");
>   	printf("Error:     %d\n", bflow->err);
>   	if (dump && bflow->buf) {
>   		/* Set some sort of maximum on the size */
> diff --git a/include/bootflow.h b/include/bootflow.h
> index 32dbbbbe261..776158c65df 100644
> --- a/include/bootflow.h
> +++ b/include/bootflow.h
> @@ -52,6 +52,8 @@ enum bootflow_state_t {
>    * @buf: Bootflow file contents (allocated)
>    * @size: Size of bootflow file in bytes
>    * @err: Error number received (0 if OK)
> + * @os_name: Name of the OS / distro being booted, or NULL if not known
> + *	(allocated)
>    */
>   struct bootflow {
>   	struct list_head bm_node;
> @@ -68,6 +70,7 @@ struct bootflow {
>   	char *buf;
>   	int size;
>   	int err;
> +	char *os_name;
>   };
>
>   /**
> diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
> index e1e07082105..3296316cf0d 100644
> --- a/test/boot/bootflow.c
> +++ b/test/boot/bootflow.c
> @@ -188,6 +188,7 @@ static int bootflow_cmd_info(struct unit_test_state *uts)
>   	ut_assert_nextline("Filename:  /extlinux/extlinux.conf");
>   	ut_assert_nextlinen("Buffer:    ");
>   	ut_assert_nextline("Size:      253 (595 bytes)");
> +	ut_assert_nextline("OS:        Fedora-Workstation-armhfp-31-1.9 (5.3.7-301.fc31.armv7hl)");
>   	ut_assert_nextline("Error:     0");
>   	ut_assert_console_end();
>

Re: [PATCH 16/24] menu: Factor out menu-keypress decoding

[Heinrich Schuchardt]

On 10/17/22 22:29, Simon Glass wrote:
> Move this code into a separate function so that it can be used in the new
> VBE menu.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   common/menu.c  | 48 ++++++++++++++++++++++++++++++------------------
>   include/menu.h | 10 ++++++++++
>   2 files changed, 40 insertions(+), 18 deletions(-)
>
> diff --git a/common/menu.c b/common/menu.c
> index c2e3ec592e3..4606cb7d1b1 100644
> --- a/common/menu.c
> +++ b/common/menu.c
> @@ -483,26 +483,11 @@ enum bootmenu_key bootmenu_autoboot_loop(struct bootmenu_data *menu,
>   	return key;
>   }
>
> -enum bootmenu_key bootmenu_loop(struct bootmenu_data *menu,
> -				struct cli_ch_state *cch)
> +enum bootmenu_key bootmenu_conv_key(int ichar)
>   {

Please, extract a common library function from
efi_cin_read_key_stroke_ex(). We should avoid code duplication.

Best regards

Heinrich

> -	enum bootmenu_key key = BKEY_NONE;
> -	int c;
> -
> -	c = cli_ch_process(cch, 0);
> -	if (!c) {
> -		while (!c && !tstc()) {
> -			WATCHDOG_RESET();
> -			mdelay(10);
> -			c = cli_ch_process(cch, -ETIMEDOUT);
> -		}
> -		if (!c) {
> -			c = getchar();
> -			c = cli_ch_process(cch, c);
> -		}
> -	}
> +	enum bootmenu_key key;
>
> -	switch (c) {
> +	switch (ichar) {
>   	case '\n':
>   		/* enter key was pressed */
>   		key = BKEY_SELECT;
> @@ -527,7 +512,34 @@ enum bootmenu_key bootmenu_loop(struct bootmenu_data *menu,
>   	case ' ':
>   		key = BKEY_SPACE;
>   		break;
> +	default:
> +		key = BKEY_NONE;
> +		break;
> +	}
> +
> +	return key;
> +}
> +
> +enum bootmenu_key bootmenu_loop(struct bootmenu_data *menu,
> +				struct cli_ch_state *cch)
> +{
> +	enum bootmenu_key key;
> +	int c;
> +
> +	c = cli_ch_process(cch, 0);
> +	if (!c) {
> +		while (!c && !tstc()) {
> +			WATCHDOG_RESET();
> +			mdelay(10);
> +			c = cli_ch_process(cch, -ETIMEDOUT);
> +		}
> +		if (!c) {
> +			c = getchar();
> +			c = cli_ch_process(cch, c);
> +		}
>   	}
>
> +	key = bootmenu_conv_key(c);
> +
>   	return key;
>   }
> diff --git a/include/menu.h b/include/menu.h
> index 3996075a337..1e88141d6bf 100644
> --- a/include/menu.h
> +++ b/include/menu.h
> @@ -53,6 +53,8 @@ enum bootmenu_key {
>   	BKEY_PLUS,
>   	BKEY_MINUS,
>   	BKEY_SPACE,
> +
> +	BKEY_COUNT,
>   };
>
>   /**
> @@ -101,4 +103,12 @@ enum bootmenu_key bootmenu_autoboot_loop(struct bootmenu_data *menu,
>   enum bootmenu_key bootmenu_loop(struct bootmenu_data *menu,
>   				struct cli_ch_state *cch);
>
> +/**
> + * bootmenu_conv_key() - Convert a U-Boot keypress into a menu key
> + *
> + * @ichar: Keypress to convert (ASCII, including control characters)
> + * Returns: Menu key that corresponds to @ichar, or BKEY_NONE if none
> + */
> +enum bootmenu_key bootmenu_conv_key(int ichar);
> +
>   #endif /* __MENU_H__ */

Re: [PATCH 03/24] bootmenu: Add a few comments

[Simon Glass]

Hi Heinrich,

On Mon, 17 Oct 2022 at 15:46, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 10/17/22 22:29, Simon Glass wrote:
> > The behaviour of these two functions is completely undocumented. Add some
> > notes so the poor, suffering dev can figure out what is going on.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >   include/menu.h | 42 ++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 42 insertions(+)
> >
> > diff --git a/include/menu.h b/include/menu.h
> > index 702aacb170c..0b4d9734149 100644
> > --- a/include/menu.h
> > +++ b/include/menu.h
> > @@ -42,6 +42,7 @@ struct bootmenu_data {
> >       struct bootmenu_entry *first;   /* first menu entry */
> >   };
> >
> > +/** enum bootmenu_key - keys that can be returned by the bootmenu */
> >   enum bootmenu_key {
> >       KEY_NONE = 0,
> >       KEY_UP,
> > @@ -53,8 +54,49 @@ enum bootmenu_key {
> >       KEY_SPACE,
> >   };
> >
> > +/**
> > + * bootmenu_autoboot_loop() - handle autobooting if no key is pressed
> > + *
> > + * This shows a prompt to allow the user to press a key to interrupt auto boot
> > + * of the first menu option.
> > + *
> > + * It then waits for the required time (menu->delay in seconds) for a key to be
> > + * pressed. If nothing is pressed in that time, @key returns KEY_SELECT
> > + * indicating that the current option should be chosen.
> > + *
> > + * @menu: Menu being processed
> > + * @key: Returns the code for the key the user pressed:
> > + *   enter: KEY_SELECT
> > + *   Ctrl-C: KEY_QUIT
> > + *   anything else: KEY_NONE
> > + * @esc: Set to 1 if the escape key is pressed, otherwise not updated
> > + */
> >   void bootmenu_autoboot_loop(struct bootmenu_data *menu,
> >                           enum bootmenu_key *key, int *esc);
> > +
> > +/**
> > + * bootmenu_loop() - handle waiting for a keypress when autoboot is disabled
> > + *
> > + * This is used when the menu delay is negative, indicating that the delay has
> > + * elapsed, or there was no delay to begin with.
>
> Unfortunately the description does not match the code.
>
> This function is entered if some key was pressed, so autoboot was
> stopped. When the delay elapses the default action is taken by the caller.

This one doesn't do default action, right? That functionality is in
bootmenu_autoboot_loop() above.

>
> > + *
> > + * It reads a character and processes it, returning a menu-key code if a
> > + * character is recognised
> > + *
> > + * @menu: Menu being processed
> > + * @key: Returns the code for the key the user pressed:
> > + *   enter: KEY_SELECT
> > + *   Ctrl-C: KEY_QUIT
> > + *   Up arrow: KEY_UP
> > + *   Down arrow: KEY_DOWN
> > + *   Escape (by itself): KEY_QUIT
> > + *   Plus: KEY_PLUS
> > + *   Minus: KEY_MINUS
> > + *   Space: KEY_SPACE
>
> We already discussed that this list is to change. We should support
> accelerator keys in menus.

OK, but not related to my change here.

>
> > + * @esc: On input, a non-zero value indicates that an escape sequence has
>
> 1 indicates that the ESC key has been pressed.
> All other values indicate that the ESC key has not been pressed.
>
> --
>
> The whole design is broken in that the concept of a menu is not properly
> encapsulated.
>
> A function like bootmenu_autoboot_loop() should not be exported.
>
> The view side of a menu should be in a function that takes the following
> arguments:
>
> - the location (x,y) on the screen
> - the size (dx, dy) of the displayed menu
>    (further items should be viewed by vertical scrolling,
>    more characters by horizontal scrolling)
> - a list of menu items to display
> - a event function called whenever the selected menu item changes (or NULL)
>
> Such an event function will allow to display extra information for a
> menu item.
>
> The model would be an array of utf-8 strings.
>
> The controller will invoke the view function and receive the index of
> the selected item as the return value (or -1 if ESC is pressed).

I think the existing menu system is not useful as a basis for what we
want, which I why I designed expo. We should focus on getting that
right. Just to be clear, I will not be taking on any effort involved
in making things work with EFI. But you may wish to.

>
> Best regards
>
> Heinrich
>
> > + *   resulted in that many characters so far. On exit this is updated to the
> > + *   new number of characters
> > + */
> >   void bootmenu_loop(struct bootmenu_data *menu,
> >                  enum bootmenu_key *key, int *esc);
> >
>

Regards,
Simon

Re: [PATCH 04/24] menu: Rename KEY_... to BKEY_...

[Simon Glass]

Hi Heinrich,

On Mon, 17 Oct 2022 at 15:55, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 10/17/22 22:29, Simon Glass wrote:
> > This enum values conflict with linux/input.h so rename them.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >   cmd/bootmenu.c  | 10 +++++-----
> >   cmd/eficonfig.c | 26 +++++++++++++-------------
> >   common/menu.c   | 34 +++++++++++++++++-----------------
> >   include/menu.h  | 32 ++++++++++++++++----------------
> >   4 files changed, 51 insertions(+), 51 deletions(-)
> >
> > diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> > index 3340be16325..c80004c54dc 100644
> > --- a/cmd/bootmenu.c
> > +++ b/cmd/bootmenu.c
> > @@ -86,7 +86,7 @@ static char *bootmenu_choice_entry(void *data)
> >   {
>
> The argument should be of type struct bootmenu_data* and not void.
>
> This function and the enum should be move to common/menu.c and not exported.

That would be a separate patch.

>
> common/menu.c does not import linux/input.h so there will be no conflict
> but that sould not stop you from renaming the constants.

Yes, it's not the conflict, just the confusion.

Regards,
Simon

Re: [PATCH 05/24] menu: Update bootmenu_autoboot_loop() to return the code

[Simon Glass]

Hi Heinrich,

On Mon, 17 Oct 2022 at 15:53, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 10/17/22 22:29, Simon Glass wrote:
> > Use the return value to save having to pass around a pointer. This also
> > resolves any ambiguity about what *key contains when the function is
> > called.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >   cmd/bootmenu.c |  2 +-
> >   common/menu.c  | 16 +++++++++-------
> >   include/menu.h |  7 +++----
> >   3 files changed, 13 insertions(+), 12 deletions(-)
> >
> > diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
> > index c80004c54dc..0e22f504fe4 100644
> > --- a/cmd/bootmenu.c
> > +++ b/cmd/bootmenu.c
> > @@ -93,7 +93,7 @@ static char *bootmenu_choice_entry(void *data)
>
> First of all this function should be moved to common/menu.c.
>
> >       while (1) {
> >               if (menu->delay >= 0) {
> >                       /* Autoboot was not stopped */
> > -                     bootmenu_autoboot_loop(menu, &key, &esc);
> > +                     key = bootmenu_autoboot_loop(menu, &esc);
> >               } else {
> >                       /* Some key was pressed, so autoboot was stopped */
> >                       bootmenu_loop(menu, &key, &esc);
> > diff --git a/common/menu.c b/common/menu.c
> > index 087e4c246e2..bccc104a39b 100644
> > --- a/common/menu.c
> > +++ b/common/menu.c
> > @@ -425,9 +425,9 @@ int menu_destroy(struct menu *m)
> >       return 1;
> >   }
> >
> > -void bootmenu_autoboot_loop(struct bootmenu_data *menu,
> > -                         enum bootmenu_key *key, int *esc)
> > +enum bootmenu_key bootmenu_autoboot_loop(struct bootmenu_data *menu, int *esc)
> >   {
> > +     enum bootmenu_key key = BKEY_NONE;
> >       int i, c;
> >
> >       while (menu->delay > 0) {
> > @@ -446,16 +446,16 @@ void bootmenu_autoboot_loop(struct bootmenu_data *menu,
> >                       switch (c) {
> >                       case '\e':
> >                               *esc = 1;
> > -                             *key = BKEY_NONE;
> > +                             key = BKEY_NONE;
> >                               break;
> >                       case '\r':
> > -                             *key = BKEY_SELECT;
> > +                             key = BKEY_SELECT;
> >                               break;
> >                       case 0x3: /* ^C */
> > -                             *key = BKEY_QUIT;
> > +                             key = BKEY_QUIT;
> >                               break;
> >                       default:
> > -                             *key = BKEY_NONE;
> > +                             key = BKEY_NONE;
> >                               break;
> >                       }
> >
> > @@ -471,7 +471,9 @@ void bootmenu_autoboot_loop(struct bootmenu_data *menu,
> >       printf(ANSI_CURSOR_POSITION ANSI_CLEAR_LINE, menu->count + 5, 1);
> >
> >       if (menu->delay == 0)
> > -             *key = BKEY_SELECT;
> > +             key = BKEY_SELECT;
> > +
> > +     return key;
> >   }
> >
> >   void bootmenu_loop(struct bootmenu_data *menu,
> > diff --git a/include/menu.h b/include/menu.h
> > index 29b457921e9..9f30a3c1acd 100644
> > --- a/include/menu.h
> > +++ b/include/menu.h
> > @@ -65,14 +65,13 @@ enum bootmenu_key {
> >    * indicating that the current option should be chosen.
> >    *
> >    * @menu: Menu being processed
> > - * @key: Returns the code for the key the user pressed:
> > + * @esc: Set to 1 if the escape key is pressed, otherwise not updated
> > + * Returns: code for the key the user pressed:
> >    *  enter: KEY_SELECT
> >    *  Ctrl-C: KEY_QUIT
> >    *  anything else: KEY_NONE
> > - * @esc: Set to 1 if the escape key is pressed, otherwise not updated
> >    */
> > -void bootmenu_autoboot_loop(struct bootmenu_data *menu,
> > -                         enum bootmenu_key *key, int *esc);
> > +enum bootmenu_key bootmenu_autoboot_loop(struct bootmenu_data *menu, int *esc);
>
> This function should not be exported.

Out of scope for this patch.

>
> enum bootmenu cannot accomomdate accelerator keys. The return type
> should be int.

What accelerator keys? I don't see how they work at present.

Regards,
Simon

Re: [PATCH 07/24] menu: Use a switch statement

[Simon Glass]

Hi Heinrich,

On Mon, 17 Oct 2022 at 16:07, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 10/17/22 22:29, Simon Glass wrote:
> > Convert the long line of if() statements to a switch() since this makes
> > better use of the C language.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >   common/menu.c | 31 ++++++++++++++++++++-----------
> >   1 file changed, 20 insertions(+), 11 deletions(-)
> >
> > diff --git a/common/menu.c b/common/menu.c
> > index 22947f5d693..1aa78b762a4 100644
> > --- a/common/menu.c
> > +++ b/common/menu.c
> > @@ -543,22 +543,31 @@ enum bootmenu_key bootmenu_loop(struct bootmenu_data *menu, int *esc)
> >               break;
> >       }
> >
> > -     /* enter key was pressed */
> > -     if (c == '\r')
> > +     switch (c) {
> > +     case '\r':
> > +             /* enter key was pressed */
> >               key = BKEY_SELECT;
> > -
> > -     /* ^C was pressed */
> > -     if (c == 0x3)
> > +             break;
> > +     case CTL_CH('c'):
> > +             /* ^C was pressed */
> >               key = BKEY_QUIT;
> > -
> > -     if (c == '+')
> > +             break;
> > +     case CTL_CH('p'):
> > +             key = BKEY_UP;
> > +             break;
> > +     case CTL_CH('n'):
> > +             key = BKEY_DOWN;
> > +             break;
> > +     case '+':
> >               key = BKEY_PLUS;
> > -
> > -     if (c == '-')
> > +             break;
> > +     case '-':
> >               key = BKEY_MINUS;
> > -
> > -     if (c == ' ')
> > +             break;
> > +     case ' ':
> >               key = BKEY_SPACE;
> > +             break;
> > +     }
>
> The whole code is not well suited to parse all of the many different
> escape sequences that can be sent to convey modifier keys or
> non-character keys (think of <CTRL><ALT><F12> or <HOME>).
>
> We should move the of logic of efi_cin_read_key_stroke_ex() to a library
> function which is used both by bootmenu and UEFI and remove duplicate code.

Sorry I am not about to put effort into the EFI code :-)

Also I did create a library to handle keystrokes. See cli_getch.c

Regards,
Simon

Re: [PATCH 10/24] video: Enable VIDEO_ANSI by default only with EFI

[Simon Glass]

Hi Heinrich,

On Mon, 17 Oct 2022 at 16:08, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 10/17/22 22:29, Simon Glass wrote:
> > This is not generally needed unless EFI_LOADER is used. Adjust the default
> > setting to reduce the size of the U-Boot build.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >   drivers/video/Kconfig | 7 +++++--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> > index 32938376655..ed77815504b 100644
> > --- a/drivers/video/Kconfig
> > +++ b/drivers/video/Kconfig
> > @@ -118,10 +118,13 @@ config VIDEO_BPP32
> >   config VIDEO_ANSI
> >       bool "Support ANSI escape sequences in video console"
> >       depends on DM_VIDEO
> > -     default y
> > +     default y if EFI_LOADER
>
> This will cripple the CLS command.

I can add that as a dependency.

I actually don't like using cls as it makes debugging hard. You can't
see the previous output on the terminal!

Regards,
Simon

Re: [PATCH 09/24] image: Add a function to find a script in an image

[Simon Glass]

Hi Heinrich,

On Mon, 17 Oct 2022 at 16:10, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 10/17/22 22:29, Simon Glass wrote:
> > Split this functionality out of the 'source' command so it can be used
> > from another place.
>
> It is not obvious how this relates to changing the series.
>
> Please, provide a description of the future use of the extracted library
> function.
>
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >   cmd/source.c    | 173 ++++++++++++++++++++++++++----------------------
> >   include/image.h |  13 ++++
>
> If you extract a library function, it should not live in cmd/source.c.
>

I can put it in boot/image*.c perhaps.

Regards,
Simon

Re: [PATCH 14/24] bootstd: Read the Operating System name for distro/scripts

[Simon Glass]

Hi Heinrich,

On Mon, 17 Oct 2022 at 16:14, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 10/17/22 22:29, Simon Glass wrote:
> > Add the concept of an OS name to the bootflow. This typically includes the
> > OS name, version and kernel version.
> >
> > Implement this for the distro and script bootmeths so that it works with
> > Armbian and older version of Fedora.
>
> How do you plan to detect the distro?

Just by reading the information it provides. For Armbian you can tell
from the script. For Fedora there is a name in the boot file.

I hope we can standardise this at some point.

Regards,
Simon

Re: [PATCH 16/24] menu: Factor out menu-keypress decoding

[Simon Glass]

Hi Heinrich,

On Mon, 17 Oct 2022 at 16:23, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 10/17/22 22:29, Simon Glass wrote:
> > Move this code into a separate function so that it can be used in the new
> > VBE menu.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >   common/menu.c  | 48 ++++++++++++++++++++++++++++++------------------
> >   include/menu.h | 10 ++++++++++
> >   2 files changed, 40 insertions(+), 18 deletions(-)
> >
> > diff --git a/common/menu.c b/common/menu.c
> > index c2e3ec592e3..4606cb7d1b1 100644
> > --- a/common/menu.c
> > +++ b/common/menu.c
> > @@ -483,26 +483,11 @@ enum bootmenu_key bootmenu_autoboot_loop(struct bootmenu_data *menu,
> >       return key;
> >   }
> >
> > -enum bootmenu_key bootmenu_loop(struct bootmenu_data *menu,
> > -                             struct cli_ch_state *cch)
> > +enum bootmenu_key bootmenu_conv_key(int ichar)
> >   {
>
> Please, extract a common library function from
> efi_cin_read_key_stroke_ex(). We should avoid code duplication.

No, that is EFI code. It also seems to use unicode somehow. We should
have EFI code using normal U-Boot functions, not duplicating
functionality elsewhere.

As you can see, my approach here is to refactor the common code to do
what is needed. Please update the EFI code to do the same.

Re unicode, we need to be very careful...what is the use case for
unicode keyboard input?

Regards,
SImon

Re: [PATCH 00/24] bootstd: Add a boot menu

[Simon Glass]

Hi Heinrich,

On Mon, 17 Oct 2022 at 15:23, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Heinrich,
>
> On Mon, 17 Oct 2022 at 15:11, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > On 10/17/22 22:29, Simon Glass wrote:
> > > So far standard boot lacks a boot menu, although it is possible to create
> > > a rudimentary one using the existing 'bootmenu' command.
> > >
> > > Even then, this text-based menu offer only basic functionality and does
> > > not take full advantage of the displays which are common on many devices.
> > >
> > > This series provides a 'bootflow menu' command which allows the user to
> > > select from the available bootflows. An attempt is made to show the name
> > > of the available operating systems, by reading more information into the
> > > bootflow. A logo can be read also, where supported, so that this can be
> > > presented to the user when an option is highlighted.
> > >
> > > Full use is made of TrueType fonts, if enabled. For cases where only a
> > > serial console is available, it falls back to a simple text-based menu.
> > >
> > > All of this is implementing using a new 'expo' construct, a collection of
> > > scenes (like menu screens) which can be navigated by the user to view
> > > information and select option. This is fairly general and should be able
> > > to cope with a wider array of use cases, with less hacking of the menu
> > > code, such as is currently needed for CMD_BOOTEFI_BOOTMGR.
> > >
> > > Of course it would be possible to enhance the existing menu rather than
> > > creating a new setup. Instead it seems better to make the existing menu
> > > use expo, if code space permits. It avoids the event-loop problem and
> > > should be more extensible, given its loosely coupled components and use of
> > > IDs instead of pointers. Further motivation is provided in the
> > > documentation.
> > >
> > > For now the CLI keypress-decoding code is split out to be used by the new
> > > menu. The key codes defined by menu.h are reused also.
> > >
> > > This is of course just a starting point. Some ideas for future work are
> > > included in the documentation.
> >
> > Hello Simon,
> >
> > Linaro's implementation of a boot menu for UEFI booting showed that the
> > current basis of bootmenu is inadequate for properly designing a GUI
> > like user interface.
> >
> > With your patch set you try to overcome this. Your design document in
> > patch 24 identifies a hierarchy of objects expo - scene - design element
> > but otherwise remains rather sketchy to me.
>
> For now the doc is here:
>
> https://docs.google.com/document/d/1VQeApnLlH6xKm_OI36AhWkJLUEd9OXEvIJXB8aM2de8/edit?usp=sharing&resourcekey=0-DwgHpR2S8vJEJzvvwPb-AQ
>
> >
> > Before investing into code we should a more holistic design perspective.
> >
> > First thing we need to define is our requirements, e.g.
> >
> > ** Which input devices do we want to support in future **
> >
> > - keyboard
> > - mouse
>
> I have patches for mouse, actually.
>
> >
> > ** What type of GUI do we want to support **
> >
> > - text terminal
> > - graphical output
> >
> > ** Which design elements do we need **
> >
> > - lists
> > - trees
> > - grids
> > - menus
> > - labels
> > - buttons
> > - drop down lists
> > - combo boxes
> > - bitmaps
> > - screens (you call them scene)
> > - transactions (you call them expo)
>
> (this series has rudimentary menus, labels and bitmaps, but does not
> support the others)
>
> >
> > ** How do we want to structure our GUI **
> >
> > - Good practice is to clearly separate model, view, and controller.
>
> Expo is the view. Controller is the client code. Model is currently
> the expo, updated by the controller. That part of it needs though if
> we are to expand the capabilities of this effort.
>
> >
> > ** Which constraints do we have to observe? **
> >
> > - How much is the code base allowed to grow?
> > - Who will be the long term sponsor and maintainer?
> >
> > After having this rough cut overview we may start with a draft
> > implementation and detail our design document.
> >
> > How do you plan to organize the design work?
>
> See also the bottom of the docs for some future work, also whether we
> should consider using a GUI library like Nuklear in the future:
>
> https://patchwork.ozlabs.org/project/uboot/patch/20221017203000.2207887-25-sjg@chromium.org/
>
> It has evolved since that doc. The doc needs to evolve further.
> Perhaps we can start by agreeing the requirements.

I have updated the doc to accommodate some of your comments and to
indicate exactly how unicode and languages might be supported.

https://docs.google.com/document/d/1VQeApnLlH6xKm_OI36AhWkJLUEd9OXEvIJXB8aM2de8/edit?resourcekey=0-DwgHpR2S8vJEJzvvwPb-AQ#

Please add any comments to the doc, or put them in this thread.

I'll rework the patches a little and send out a new version.

Regards,
Simon