[PATCH v2 0/4] Better smartphone support (Qualcomm)

[PATCH v2 0/4] Better smartphone support (Qualcomm)

[Caleb Connolly]

Improve support for running U-Boot on [Qualcomm] smartphones, with:

* A boot menu which can be entered by holding power during boot
* Hang on panic (so errors can be read on the display)
* Serial gadget which can be enabled by holding volume down or via
  boot menu

In its current form, U-Boot's boot menu is perfectly adequate, however
once an option is picked U-Boot will just drop to the CLI again.

On a phone (with no keyboard), dropping to the CLI effectively makes the
device unusable until manually rebooted, since there is no way to run
any commands...

To rectify this, let's revive CONFIG_BOOT_RETRY for the
hush parser and make it run the contents of "bootcmd", as well as
re-reading the value of the bootretry environment variable. In tandem,
this provides for a flexible menu system which we can enter and exit.
See qcom-phone.env in patch 1 for more details.

Finally, provide some rough-ish documentation to describe how Qualcomm
phone support works and outline some debugging steps for when UART and
the framebuffer are unavailable.

This series depends on CONFIG_REMAP_PHONE_KEYS [1] to enable navigating the
boot menu when using upstream DT.

[1]: https://lore.kernel.org/u-boot/20240714194948.1271135-1-caleb.connolly@linaro.org/

---
Changes in v2:
- Fix CONFIG_BUTTON_REMAP_PHONE_KEYS name in qcom-phone.config
- Add Tom and Simon to CC
- Link to v1: https://lore.kernel.org/r/20240831-qcom-phones-v1-0-08614edd8e35@linaro.org

---
Caleb Connolly (4):
      board/qualcomm: introduce phone config
      cli_hush: run bootmenu on retry
      bootretry: check for bootretry variable changes
      doc: board/qualcomm: describe phone support and bringup

 board/qualcomm/qcom-phone.config |  12 ++++
 board/qualcomm/qcom-phone.env    |  49 ++++++++++++++++
 boot/bootretry.c                 |   2 +
 common/cli_hush.c                |   2 +-
 common/cli_hush_upstream.c       |   2 +-
 doc/board/qualcomm/board.rst     |   5 ++
 doc/board/qualcomm/index.rst     |   1 +
 doc/board/qualcomm/phones.rst    | 122 +++++++++++++++++++++++++++++++++++++++
 8 files changed, 193 insertions(+), 2 deletions(-)
---
change-id: 20240831-qcom-phones-c789d14de657
base-commit: 00292c6b39b3f7dcb34e8c3ae6ef276ac862a146

// Caleb (they/them)

[PATCH v2 1/4] board/qualcomm: introduce phone config

[Caleb Connolly]

Phones don't have keyboards! Introduce a phone-specific config fragment
and associated environment file to make U-Boot more useful on these
devices. This allows for navigating via the buttons and enabling
various USB gadget modes or displaying info about U-Boot.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 board/qualcomm/qcom-phone.config | 12 ++++++++++
 board/qualcomm/qcom-phone.env    | 49 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/board/qualcomm/qcom-phone.config b/board/qualcomm/qcom-phone.config
new file mode 100644
index 000000000000..388d4c3e7398
--- /dev/null
+++ b/board/qualcomm/qcom-phone.config
@@ -0,0 +1,12 @@
+# Settings for phones
+CONFIG_USE_DEFAULT_ENV_FILE=y
+CONFIG_DEFAULT_ENV_FILE="board/qualcomm/qcom-phone.env"
+CONFIG_PANIC_HANG=y
+CONFIG_CMD_PAUSE=y
+CONFIG_BOOTSTD=y
+CONFIG_BOOT_RETRY=y
+CONFIG_BOOT_RETRY_TIME=1
+CONFIG_BUTTON_REMAP_PHONE_KEYS=y
+
+# Many phones don't actually define a serial port in their DTS
+# CONFIG_REQUIRE_SERIAL_CONSOLE is not set
diff --git a/board/qualcomm/qcom-phone.env b/board/qualcomm/qcom-phone.env
new file mode 100644
index 000000000000..2a0791c888c6
--- /dev/null
+++ b/board/qualcomm/qcom-phone.env
@@ -0,0 +1,49 @@
+bootdelay=0
+bootretry=1
+stdin=serial,button-kbd
+stdout=serial,vidconsole
+stderr=serial,vidconsole
+
+# Fastboot is keen to use the address from kconfig, but we
+# allocate its buffer at runtime.
+fastboot=fastboot -l $fastboot_addr_r usb 0
+
+# Shortcut to enable USB serial gadget and disable bootretry
+serial_gadget=setenv stdin serial,button-kbd,usbacm; \
+	setenv stdout serial,vidconsole,usbacm; \
+	setenv stderr serial,vidconsole,usbacm; \
+	setenv bootretry -1; \
+	echo Enabled U-Boot console serial gadget
+
+# bootretry will run this command over and over, if we fail once
+# then bail out to the boot menu instead (with a pause to read
+# the error message)
+bootcmd=bootefi bootmgr; pause; run menucmd
+
+# When entering the menu (either from button press or failed boot)
+# remap bootcmd so it will re-open the menu and we won't get stuck
+# at the console with no way to type
+menucmd=setenv bootcmd run menucmd; bootmenu -1
+
+# Pause is used so the output can be read on the display
+bootmenu_0=Boot=bootefi bootmgr; pause
+bootmenu_1=Enable serial console gadget=run serial_gadget
+bootmenu_2=Enable USB mass storage=ums 0 scsi 0
+bootmenu_3=Reset device=reset
+bootmenu_4=Dump clocks=clk dump; pause
+bootmenu_5=Dump environment=printenv; pause
+bootmenu_6=Board info=bdinfo; pause
+bootmenu_7=Dump bootargs=fdt print /chosen bootargs; pause
+bootmenu_8=Enable fastboot mode=run fastboot
+# Disabling bootretry means we'll just drop the shell
+bootmenu_9=Drop to shell=setenv bootretry -1
+
+# Allow holding the power button while U-Boot loads to enter
+# the boot menu
+button_cmd_0_name=pwrkey
+button_cmd_0=run menucmd
+
+# Hold volume down to drop to a shell with the USB serial gadget
+# enabled for debugging
+button_cmd_1_name=Volume down
+button_cmd_1=run serial_gadget

-- 
2.46.2

[PATCH v2 2/4] cli_hush: run bootmenu on retry

[Caleb Connolly]

When CONFIG_BOOT_RETRY is enabled, make hush shell always run the
bootcmd. This is an improvement over failing to build.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 common/cli_hush.c          | 2 +-
 common/cli_hush_upstream.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/cli_hush.c b/common/cli_hush.c
index a6a8edce1f43..3c6fd6f00a1d 100644
--- a/common/cli_hush.c
+++ b/common/cli_hush.c
@@ -1028,9 +1028,9 @@ static void get_user_input(struct in_str *i)
 	  puts("\nTimeout waiting for command\n");
 #  ifdef CONFIG_RESET_TO_RETRY
 	  do_reset(NULL, 0, 0, NULL);
 #  else
-#	error "This currently only works with CONFIG_RESET_TO_RETRY enabled"
+	strcpy(console_buffer, "run bootcmd\n");
 #  endif
 	}
 #endif
 	if (n == -1 ) {
diff --git a/common/cli_hush_upstream.c b/common/cli_hush_upstream.c
index ab5aa5f9b36a..93eb0d85a95a 100644
--- a/common/cli_hush_upstream.c
+++ b/common/cli_hush_upstream.c
@@ -2907,9 +2907,9 @@ static void get_user_input(struct in_str *i)
 		puts("\nTimeout waiting for command\n");
 #  ifdef CONFIG_RESET_TO_RETRY
 		do_reset(NULL, 0, 0, NULL);
 #  else
-#	error "This currently only works with CONFIG_RESET_TO_RETRY enabled"
+	strcpy(console_buffer, "run bootcmd\n");
 #  endif
 	}
 # endif
 	if (n == -1 ) {

-- 
2.46.2

[PATCH v2 3/4] bootretry: check for bootretry variable changes

[Caleb Connolly]

To enable more complex sequencing of the bootmenu, autoboot, and
bootretry, handle changes to the bootretry variable between tries. This
makes it possible to turn bootretry off (e.g. to drop to a shell) and
then back on again.

This makes it possible to have a persistent bootmenu (the only way to
navigate U-Boot on devices like smartphones which lack a physical
keyboard) by having bootcmd be defined to launch the bootmenu. This
allows for menu options like enabling USB mass storage gadget to return
back to the boot menu once the gadget is shut down.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 boot/bootretry.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/boot/bootretry.c b/boot/bootretry.c
index 587b2de7d6b0..a60767eaa2ee 100644
--- a/boot/bootretry.c
+++ b/boot/bootretry.c
@@ -36,8 +36,10 @@ void bootretry_init_cmd_timeout(void)
  * reset command line timeout to retry_time seconds
  */
 void bootretry_reset_cmd_timeout(void)
 {
+	/* Parse changes to bootretry */
+	bootretry_init_cmd_timeout();
 	endtime = endtick(retry_time);
 }
 
 int bootretry_tstc_timeout(void)

-- 
2.46.2

[PATCH v2 4/4] doc: board/qualcomm: describe phone support and bringup

[Caleb Connolly]

Add some documentation which attempts to describe Qualcomm smartphone
support with the qcom-phone.config fragment, as well as a high level
debugging guide for diagnosing U-Boot issues when UART and framebuffer
are unavailable.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 doc/board/qualcomm/board.rst  |   5 ++
 doc/board/qualcomm/index.rst  |   1 +
 doc/board/qualcomm/phones.rst | 122 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 128 insertions(+)

diff --git a/doc/board/qualcomm/board.rst b/doc/board/qualcomm/board.rst
index 4d793209f9e3..003d59a18ebd 100644
--- a/doc/board/qualcomm/board.rst
+++ b/doc/board/qualcomm/board.rst
@@ -89,8 +89,13 @@ Or for db410c (and other boards not supported by the generic target)::
 
 	make CROSS_COMPILE=aarch64-linux-gnu- O=.output dragonboard410c_defconfig
 	make O=.output -j$(nproc)
 
+Or for smartphones::
+
+	make CROSS_COMPILE=aarch64-linux-gnu- O=.output qcom_defconfig qcom-phone.config
+	make O=.output -j$(nproc)
+
 - gzip u-boot::
 
 	gzip u-boot-nodtb.bin
 
diff --git a/doc/board/qualcomm/index.rst b/doc/board/qualcomm/index.rst
index 8c7969987a97..db59b81134b8 100644
--- a/doc/board/qualcomm/index.rst
+++ b/doc/board/qualcomm/index.rst
@@ -9,4 +9,5 @@ Qualcomm
    dragonboard410c
    rb3gen2
    board
    debugging
+   phones
diff --git a/doc/board/qualcomm/phones.rst b/doc/board/qualcomm/phones.rst
new file mode 100644
index 000000000000..0726f782b675
--- /dev/null
+++ b/doc/board/qualcomm/phones.rst
@@ -0,0 +1,122 @@
+.. SPDX-License-Identifier: GPL-2.0+
+.. sectionauthor:: Caleb Connolly <caleb.connolly@linaro.org>
+
+======================================
+Booting U-Boot on Qualcomm smartphones
+======================================
+
+About this
+----------
+
+This page attempts to the describe U-Boot support for Qualcomm phones, as a user guide but also a
+technical introduction to How Stuff Works to help new porters.
+
+In broad strokes, U-Boot should boot if the SoC is supported, and the device is already capable of
+booting an upstream Linux kernel.
+
+The list of supported Qualcomm SoCs changes often, for now it is best to look in
+``drivers/clk/qcom/`` to get a rough idea.
+
+For building instructions, see :doc:`board`.
+
+Phone bringup
+-------------
+
+It is usually easier to get Linux booting first, there are many good resources for this such as the
+`postmarketOS wiki`_. Once the device can boot Linux with logs on the display and ideally USB gadget
+support, it is highly likely that U-Boot will boot as well.
+
+For logs on display, you should have a simple framebuffer node defined in your DT, newer devices
+require that this follow the downstream naming scheme (that the DTB is compiled with labels enabled
+and the framebuffer reserved-memory region is labelled ``cont_splash``). Once this is working in
+Linux it should also work in U-Boot.
+
+In practise, U-Boot still has many more papercuts than Linux, which can be sticking points when
+porting a new device. In particular, drivers failing to bind/probe (especially pre-relocation) can
+be near-impossible to debug without UART since U-Boot will simply panic with no way to inform you of
+the error. As a result, bringing up a new device can be quite frustrating, but there are quite a few
+things you can try.
+
+The phone config
+^^^^^^^^^^^^^^^^
+
+Since most phones lack a physical keyboard or serial port, a special config fragment and environment
+file can be used to provide a more seamless experience. This can be enabled by generating the config
+with::
+
+	make CROSS_COMPILE=aarch64-linux-gnu- O=.output qcom_defconfig qcom-phone.config
+
+The config and associated environment file can be found in board/qualcomm/. The main changes are:
+
+- Panic on hang (so the panic message can be read on the display)
+- Boot retry (to automatically open and re-open the bootmenu)
+- A boot menu with helpful shortcuts (including USB console gadget)
+- Launch the boot menu if power is held during boot or on boot failure
+
+Hang/crash bisection
+--------------------
+
+Without a way to get logs, we can still get quite far with only a few bits of information: what
+happens when you ``fastboot boot u-boot.img``?
+
+Does the device disconnect?
+^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+This can be verified by watching ``dmesg -w``. It likely means the boot image doesn't match what the
+bootloader expected, use ``unpack_bootimg`` to compare it with a known-good boot image (ideally one
+with an upstream kernel).
+
+Does the device hang?
+^^^^^^^^^^^^^^^^^^^^^
+
+Since ``qcom-phone.config`` enables CONFIG_PANIC_HANG, this likely means that you're successfully
+executing U-Boot code (yay!), but something is causing a panic.
+
+It could also be due to a bad memory or register access triggering a secure interrupt, it's worth
+waiting for around a minute to see if the device eventually reboots or goes to crashdump mode. You
+can also disable CONFIG_PANIC_HANG and see if that causes the device to reboot instead, if so then
+it is definitely a U-Boot panic.
+
+With enough time and patience, it should be possible to narrow down the cause of the panic by
+inserting calls to ``reset_cpu()``. You can then determine where execution stops by seeing if the
+device hangs or resets.
+
+A good place to start is ``board_fdt_blob_setup()`` in ``arch/arm/mach-snapdragon/board.c``, this
+function is called extremely early so adding a reset call is a good way to validate that U-Boot is
+definitely running.
+
+You can then do a binary search starting from the end of ``board_init_f()`` / start of
+``board_init_r()`` and work from there using the init sequences for reference.
+
+The Qualcomm RAM parsing code is a likely culprit, as ABL is known to sometimes give bogus entries
+in the memory node which can trip U-Boot up. We should eventually handle all of these correctly
+upstream.
+
+To rule out crashes that might be caused by specific drivers, it's a good idea to disable them and
+re-enable them one by one. Here is a non-exhaustive list of drivers to disable:
+
+- pinctrl
+- mmc
+- scsi/ufs
+- usb (dwc3)
+- phy
+- clk (remove clock references from your framebuffer node in DT)
+
+Ideally, it would be possible to use the framebuffer as an early console / debug output, at the time
+of writing there are out of tree patches for this but they haven't been submitted upstream yet.
+
+Does the device reboot or go to crashdump mode?
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+On many devices crashdump mode is disabled, so they will reboot instead (maybe after some delay).
+The same approach as suggested above can be used to figure out where the crash occurs.
+
+If the device is rebooting, you can insert calls to ``hang()`` instead of ``reset_cpu()`` when
+following the instructions above.
+
+The most likely cause of a crashdump is the pinctrl/gpio driver or the SMMU driver, ensure that the
+``apps_smmu`` node in your SoCs devicetree file has one of its compatible strings referenced in
+``drivers/iommu/qcom-hyp-smmu.c``, you can also try disabling the pinctrl driver for your SoC (or
+``CONFIG_PINCTRL`` altogether).
+
+.. _`postmarketOS wiki`: https://wiki.postmarketos.org/wiki/Mainlining

-- 
2.46.2

Re: [PATCH v2 2/4] cli_hush: run bootmenu on retry

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1614 bytes --]

On Fri, Oct 04, 2024 at 02:46:51PM +0200, Caleb Connolly wrote:
> When CONFIG_BOOT_RETRY is enabled, make hush shell always run the
> bootcmd. This is an improvement over failing to build.
> 
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>  common/cli_hush.c          | 2 +-
>  common/cli_hush_upstream.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/common/cli_hush.c b/common/cli_hush.c
> index a6a8edce1f43..3c6fd6f00a1d 100644
> --- a/common/cli_hush.c
> +++ b/common/cli_hush.c
> @@ -1028,9 +1028,9 @@ static void get_user_input(struct in_str *i)
>  	  puts("\nTimeout waiting for command\n");
>  #  ifdef CONFIG_RESET_TO_RETRY
>  	  do_reset(NULL, 0, 0, NULL);
>  #  else
> -#	error "This currently only works with CONFIG_RESET_TO_RETRY enabled"
> +	strcpy(console_buffer, "run bootcmd\n");
>  #  endif
>  	}
>  #endif
>  	if (n == -1 ) {
> diff --git a/common/cli_hush_upstream.c b/common/cli_hush_upstream.c
> index ab5aa5f9b36a..93eb0d85a95a 100644
> --- a/common/cli_hush_upstream.c
> +++ b/common/cli_hush_upstream.c
> @@ -2907,9 +2907,9 @@ static void get_user_input(struct in_str *i)
>  		puts("\nTimeout waiting for command\n");
>  #  ifdef CONFIG_RESET_TO_RETRY
>  		do_reset(NULL, 0, 0, NULL);
>  #  else
> -#	error "This currently only works with CONFIG_RESET_TO_RETRY enabled"
> +	strcpy(console_buffer, "run bootcmd\n");
>  #  endif
>  	}
>  # endif
>  	if (n == -1 ) {

Lets add RETRY_BOOTCMD or something instead, I feel like there's some
locked-down systems that rely on this behavior.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 3/4] bootretry: check for bootretry variable changes

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 776 bytes --]

On Fri, Oct 04, 2024 at 02:46:52PM +0200, Caleb Connolly wrote:

> To enable more complex sequencing of the bootmenu, autoboot, and
> bootretry, handle changes to the bootretry variable between tries. This
> makes it possible to turn bootretry off (e.g. to drop to a shell) and
> then back on again.
> 
> This makes it possible to have a persistent bootmenu (the only way to
> navigate U-Boot on devices like smartphones which lack a physical
> keyboard) by having bootcmd be defined to launch the bootmenu. This
> allows for menu options like enabling USB mass storage gadget to return
> back to the boot menu once the gadget is shut down.
> 
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 1/4] board/qualcomm: introduce phone config

[Simon Glass]

On Fri, 4 Oct 2024 at 06:46, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
> Phones don't have keyboards! Introduce a phone-specific config fragment
> and associated environment file to make U-Boot more useful on these
> devices. This allows for navigating via the buttons and enabling
> various USB gadget modes or displaying info about U-Boot.
>
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>  board/qualcomm/qcom-phone.config | 12 ++++++++++
>  board/qualcomm/qcom-phone.env    | 49 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
>

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

Re: [PATCH v2 4/4] doc: board/qualcomm: describe phone support and bringup

[Simon Glass]

Hi Caleb,

On Fri, 4 Oct 2024 at 06:47, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
> Add some documentation which attempts to describe Qualcomm smartphone
> support with the qcom-phone.config fragment, as well as a high level
> debugging guide for diagnosing U-Boot issues when UART and framebuffer
> are unavailable.
>
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>  doc/board/qualcomm/board.rst  |   5 ++
>  doc/board/qualcomm/index.rst  |   1 +
>  doc/board/qualcomm/phones.rst | 122 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 128 insertions(+)

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

[..]

> diff --git a/doc/board/qualcomm/phones.rst b/doc/board/qualcomm/phones.rst
> new file mode 100644
> index 000000000000..0726f782b675
> --- /dev/null
> +++ b/doc/board/qualcomm/phones.rst
> @@ -0,0 +1,122 @@
[..]

> +Ideally, it would be possible to use the framebuffer as an early console / debug output, at the time
> +of writing there are out of tree patches for this but they haven't been submitted upstream yet.

That's quite intriguing...you should send those patches!

Regards,
Simon

Re: [PATCH v2 1/4] board/qualcomm: introduce phone config

[Julius Lehmann]

On 04.10.24 14:46, Caleb Connolly wrote:
> Phones don't have keyboards! Introduce a phone-specific config fragment
> and associated environment file to make U-Boot more useful on these
> devices. This allows for navigating via the buttons and enabling
> various USB gadget modes or displaying info about U-Boot.
> 
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>   board/qualcomm/qcom-phone.config | 12 ++++++++++
>   board/qualcomm/qcom-phone.env    | 49 ++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 61 insertions(+)
> 
> diff --git a/board/qualcomm/qcom-phone.config b/board/qualcomm/qcom-phone.config
> new file mode 100644
> index 000000000000..388d4c3e7398
> --- /dev/null
> +++ b/board/qualcomm/qcom-phone.config
> @@ -0,0 +1,12 @@
> +# Settings for phones
> +CONFIG_USE_DEFAULT_ENV_FILE=y
> +CONFIG_DEFAULT_ENV_FILE="board/qualcomm/qcom-phone.env"
> +CONFIG_PANIC_HANG=y
> +CONFIG_CMD_PAUSE=y
> +CONFIG_BOOTSTD=y
> +CONFIG_BOOT_RETRY=y
> +CONFIG_BOOT_RETRY_TIME=1
> +CONFIG_BUTTON_REMAP_PHONE_KEYS=y

should also include fastboot, serial gadget and 
CONFIG_CMD_USB_MASS_STORAGE. otherwise some menu options wouldn't work.

> +
> +# Many phones don't actually define a serial port in their DTS
> +# CONFIG_REQUIRE_SERIAL_CONSOLE is not set
> diff --git a/board/qualcomm/qcom-phone.env b/board/qualcomm/qcom-phone.env
> new file mode 100644
> index 000000000000..2a0791c888c6
> --- /dev/null
> +++ b/board/qualcomm/qcom-phone.env
> @@ -0,0 +1,49 @@
> +bootdelay=0
> +bootretry=1
> +stdin=serial,button-kbd
> +stdout=serial,vidconsole
> +stderr=serial,vidconsole
> +
> +# Fastboot is keen to use the address from kconfig, but we
> +# allocate its buffer at runtime.
> +fastboot=fastboot -l $fastboot_addr_r usb 0
> +
> +# Shortcut to enable USB serial gadget and disable bootretry
> +serial_gadget=setenv stdin serial,button-kbd,usbacm; \
> +	setenv stdout serial,vidconsole,usbacm; \
> +	setenv stderr serial,vidconsole,usbacm; \
> +	setenv bootretry -1; \
> +	echo Enabled U-Boot console serial gadget
this doesn't work. according to doc 
(https://docs.u-boot.org/en/latest/usage/environment.html), multiline 
variables don't need "\".

but even without "\" it only executes the first line.

> +
> +# bootretry will run this command over and over, if we fail once
> +# then bail out to the boot menu instead (with a pause to read
> +# the error message)
> +bootcmd=bootefi bootmgr; pause; run menucmd
> +
> +# When entering the menu (either from button press or failed boot)
> +# remap bootcmd so it will re-open the menu and we won't get stuck
> +# at the console with no way to type
> +menucmd=setenv bootcmd run menucmd; bootmenu -1
> +
> +# Pause is used so the output can be read on the display
> +bootmenu_0=Boot=bootefi bootmgr; pause
> +bootmenu_1=Enable serial console gadget=run serial_gadget
> +bootmenu_2=Enable USB mass storage=ums 0 scsi 0
> +bootmenu_3=Reset device=reset
> +bootmenu_4=Dump clocks=clk dump; pause
> +bootmenu_5=Dump environment=printenv; pause
> +bootmenu_6=Board info=bdinfo; pause
> +bootmenu_7=Dump bootargs=fdt print /chosen bootargs; pause
> +bootmenu_8=Enable fastboot mode=run fastboot
> +# Disabling bootretry means we'll just drop the shell
> +bootmenu_9=Drop to shell=setenv bootretry -1

maybe at an entry for ufetch? ;)

> +
> +# Allow holding the power button while U-Boot loads to enter
> +# the boot menu
> +button_cmd_0_name=pwrkey
> +button_cmd_0=run menucmd
> +
> +# Hold volume down to drop to a shell with the USB serial gadget
> +# enabled for debugging
> +button_cmd_1_name=Volume down
> +button_cmd_1=run serial_gadget
> 

Re: [PATCH v2 1/4] board/qualcomm: introduce phone config

[Alexey Minnekhanov]

On 12/6/24 19:50, Julius Lehmann wrote:
> On 04.10.24 14:46, Caleb Connolly wrote:
>> Phones don't have keyboards! Introduce a phone-specific config fragment
>> and associated environment file to make U-Boot more useful on these
>> devices. This allows for navigating via the buttons and enabling
>> various USB gadget modes or displaying info about U-Boot.
>>
>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
>> ---
>>   board/qualcomm/qcom-phone.config | 12 ++++++++++
>>   board/qualcomm/qcom-phone.env    | 49 ++++++++++++++++++++++++++++++ 
>> ++++++++++
>>   2 files changed, 61 insertions(+)
>>
>> diff --git a/board/qualcomm/qcom-phone.config b/board/qualcomm/qcom- 
>> phone.config
>> new file mode 100644
>> index 000000000000..388d4c3e7398
>> --- /dev/null
>> +++ b/board/qualcomm/qcom-phone.config
>> @@ -0,0 +1,12 @@
>> +# Settings for phones
>> +CONFIG_USE_DEFAULT_ENV_FILE=y
>> +CONFIG_DEFAULT_ENV_FILE="board/qualcomm/qcom-phone.env"
>> +CONFIG_PANIC_HANG=y
>> +CONFIG_CMD_PAUSE=y
>> +CONFIG_BOOTSTD=y
>> +CONFIG_BOOT_RETRY=y
>> +CONFIG_BOOT_RETRY_TIME=1
>> +CONFIG_BUTTON_REMAP_PHONE_KEYS=y
> 
> should also include fastboot, serial gadget and 
> CONFIG_CMD_USB_MASS_STORAGE. otherwise some menu options wouldn't work.

+1, should probably include CONFIG_USB_FUNCTION_ACM to enable output to 
usbacm?

[PATCH v2 1/4] board/qualcomm: introduce phone config

[Caleb Connolly]

Phones don't have keyboards! Introduce a phone-specific config fragment
and associated environment file to make U-Boot more useful on these
devices. This allows for navigating via the buttons and enabling
various USB gadget modes or displaying info about U-Boot.

Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 board/qualcomm/qcom-phone.config | 17 ++++++++++++++
 board/qualcomm/qcom-phone.env    | 49 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/board/qualcomm/qcom-phone.config b/board/qualcomm/qcom-phone.config
new file mode 100644
index 0000000000000000000000000000000000000000..a2aa882285b61746a243a7a1c7c384f33839f1b2
--- /dev/null
+++ b/board/qualcomm/qcom-phone.config
@@ -0,0 +1,17 @@
+# Settings for phones
+CONFIG_DEFAULT_ENV_FILE="board/qualcomm/qcom-phone.env"
+# Hang on panic so the error message can be read
+CONFIG_PANIC_HANG=y
+# We use pause in various places to allow text to be read
+# before it scrolls off the screen
+CONFIG_CMD_PAUSE=y
+CONFIG_BOOT_RETRY=y
+CONFIG_BOOT_RETRY_TIME=1
+CONFIG_BUTTON_REMAP_PHONE_KEYS=y
+CONFIG_RETRY_BOOTCMD=y
+CONFIG_FASTBOOT_BUF_ADDR=0x1A000000
+CONFIG_USB_FUNCTION_FASTBOOT=y
+CONFIG_USB_FUNCTION_ACM=y
+
+# Many phones don't actually define a serial port in their DTS
+# CONFIG_REQUIRE_SERIAL_CONSOLE is not set
diff --git a/board/qualcomm/qcom-phone.env b/board/qualcomm/qcom-phone.env
new file mode 100644
index 0000000000000000000000000000000000000000..2a0791c888c62b9f2257448358dd196a02e4b6fb
--- /dev/null
+++ b/board/qualcomm/qcom-phone.env
@@ -0,0 +1,49 @@
+bootdelay=0
+bootretry=1
+stdin=serial,button-kbd
+stdout=serial,vidconsole
+stderr=serial,vidconsole
+
+# Fastboot is keen to use the address from kconfig, but we
+# allocate its buffer at runtime.
+fastboot=fastboot -l $fastboot_addr_r usb 0
+
+# Shortcut to enable USB serial gadget and disable bootretry
+serial_gadget=setenv stdin serial,button-kbd,usbacm; \
+	setenv stdout serial,vidconsole,usbacm; \
+	setenv stderr serial,vidconsole,usbacm; \
+	setenv bootretry -1; \
+	echo Enabled U-Boot console serial gadget
+
+# bootretry will run this command over and over, if we fail once
+# then bail out to the boot menu instead (with a pause to read
+# the error message)
+bootcmd=bootefi bootmgr; pause; run menucmd
+
+# When entering the menu (either from button press or failed boot)
+# remap bootcmd so it will re-open the menu and we won't get stuck
+# at the console with no way to type
+menucmd=setenv bootcmd run menucmd; bootmenu -1
+
+# Pause is used so the output can be read on the display
+bootmenu_0=Boot=bootefi bootmgr; pause
+bootmenu_1=Enable serial console gadget=run serial_gadget
+bootmenu_2=Enable USB mass storage=ums 0 scsi 0
+bootmenu_3=Reset device=reset
+bootmenu_4=Dump clocks=clk dump; pause
+bootmenu_5=Dump environment=printenv; pause
+bootmenu_6=Board info=bdinfo; pause
+bootmenu_7=Dump bootargs=fdt print /chosen bootargs; pause
+bootmenu_8=Enable fastboot mode=run fastboot
+# Disabling bootretry means we'll just drop the shell
+bootmenu_9=Drop to shell=setenv bootretry -1
+
+# Allow holding the power button while U-Boot loads to enter
+# the boot menu
+button_cmd_0_name=pwrkey
+button_cmd_0=run menucmd
+
+# Hold volume down to drop to a shell with the USB serial gadget
+# enabled for debugging
+button_cmd_1_name=Volume down
+button_cmd_1=run serial_gadget

-- 
2.48.1

Re: [PATCH v2 1/4] board/qualcomm: introduce phone config

[Sam Day]

'Ello Caleb,

On Tue Mar 11, 2025 at 1:31 PM CET, Caleb Connolly wrote:
> Phones don't have keyboards! Introduce a phone-specific config fragment
> and associated environment file to make U-Boot more useful on these
> devices. This allows for navigating via the buttons and enabling
> various USB gadget modes or displaying info about U-Boot.
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>  board/qualcomm/qcom-phone.config | 17 ++++++++++++++
>  board/qualcomm/qcom-phone.env    | 49 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+)
>
> diff --git a/board/qualcomm/qcom-phone.config b/board/qualcomm/qcom-phone.config
> new file mode 100644
> index 0000000000000000000000000000000000000000..a2aa882285b61746a243a7a1c7c384f33839f1b2
> --- /dev/null
> +++ b/board/qualcomm/qcom-phone.config
> @@ -0,0 +1,17 @@
> +# Settings for phones
> +CONFIG_DEFAULT_ENV_FILE="board/qualcomm/qcom-phone.env"
> +# Hang on panic so the error message can be read
> +CONFIG_PANIC_HANG=y
> +# We use pause in various places to allow text to be read
> +# before it scrolls off the screen
> +CONFIG_CMD_PAUSE=y
> +CONFIG_BOOT_RETRY=y
> +CONFIG_BOOT_RETRY_TIME=1
> +CONFIG_BUTTON_REMAP_PHONE_KEYS=y

Maybe also include CONFIG_CMD_UMS_ABORT_KEYED=y here? Then folks can
bail out of UMS mode without needing to hard reboot their device.

> +CONFIG_RETRY_BOOTCMD=y
> +CONFIG_FASTBOOT_BUF_ADDR=0x1A000000
> +CONFIG_USB_FUNCTION_FASTBOOT=y

I would propose to also add:

CONFIG_CONSOLE_RECORD=y
CONFIG_CONSOLE_RECORD_INIT_F=y
CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000
CONFIG_FASTBOOT_CMD_OEM_CONSOLE=y

This allows folks to pull down the U-Boot logs with `fastboot oem log`

CONFIG_FASTBOOT_FLASH=y
CONFIG_FASTBOOT_FLASH_MMC_DEV=0

Allowing to flash to partitions via fastboot (not sure about if/how UFS
support works here though, I've only tested it on msm8916 with eMMC)

CONFIG_FASTBOOT_OEM_RUN=y

Allowing to run arbitrary commands (e.g `fastboot oem run:'bdinfo')

> +CONFIG_USB_FUNCTION_ACM=y
> +
> +# Many phones don't actually define a serial port in their DTS
> +# CONFIG_REQUIRE_SERIAL_CONSOLE is not set
> diff --git a/board/qualcomm/qcom-phone.env b/board/qualcomm/qcom-phone.env
> new file mode 100644
> index 0000000000000000000000000000000000000000..2a0791c888c62b9f2257448358dd196a02e4b6fb
> --- /dev/null
> +++ b/board/qualcomm/qcom-phone.env
> @@ -0,0 +1,49 @@
> +bootdelay=0
> +bootretry=1
> +stdin=serial,button-kbd
> +stdout=serial,vidconsole
> +stderr=serial,vidconsole
> +
> +# Fastboot is keen to use the address from kconfig, but we
> +# allocate its buffer at runtime.
> +fastboot=fastboot -l $fastboot_addr_r usb 0
> +
> +# Shortcut to enable USB serial gadget and disable bootretry
> +serial_gadget=setenv stdin serial,button-kbd,usbacm; \
> +	setenv stdout serial,vidconsole,usbacm; \
> +	setenv stderr serial,vidconsole,usbacm; \
> +	setenv bootretry -1; \
> +	echo Enabled U-Boot console serial gadget
> +
> +# bootretry will run this command over and over, if we fail once
> +# then bail out to the boot menu instead (with a pause to read
> +# the error message)
> +bootcmd=bootefi bootmgr; pause; run menucmd
> +
> +# When entering the menu (either from button press or failed boot)
> +# remap bootcmd so it will re-open the menu and we won't get stuck
> +# at the console with no way to type
> +menucmd=setenv bootcmd run menucmd; bootmenu -1
> +
> +# Pause is used so the output can be read on the display
> +bootmenu_0=Boot=bootefi bootmgr; pause
> +bootmenu_1=Enable serial console gadget=run serial_gadget
> +bootmenu_2=Enable USB mass storage=ums 0 scsi 0

This doesn't work on my fajita unless I run `scsi scan` first. I note
that this used to be in the preboot= section of default.env, maybe we
should bring that across?

Also, if you choose to include CONFIG_CMD_UMS_ABORT_KEYED=y per my
earlier suggestion, it would be worth adding an "echo press any key to
exit UMS mode" here or smth, I guess?

> +bootmenu_3=Reset device=reset
> +bootmenu_4=Dump clocks=clk dump; pause
> +bootmenu_5=Dump environment=printenv; pause
> +bootmenu_6=Board info=bdinfo; pause
> +bootmenu_7=Dump bootargs=fdt print /chosen bootargs; pause

I think options 4-7 should be at the bottom of the list since they're
less typical/useful than jumping into usbacm/fastboot/ums modes. Reset
device should be at the bottom of the list.

I'd maybe also go as far as suggesting that these could be removed
entirely? Once you're in serial mode (or fastboot mode with
CONFIG_FASTBOOT_OEM_RUN enabled) you can run these commands easily.

> +bootmenu_8=Enable fastboot mode=run fastboot
> +# Disabling bootretry means we'll just drop the shell
> +bootmenu_9=Drop to shell=setenv bootretry -1
> +
> +# Allow holding the power button while U-Boot loads to enter
> +# the boot menu
> +button_cmd_0_name=pwrkey

Two issues with using pwrkey as the boot menu trigger:

 * Many devices display a "custom OS" warning screen which can be paused
   with the power button, which this will conflict with.
 * When I trigger the boot menu this way, releasing pwrkey when the boot
   menu shows up causes the first boot option to be selected. I note
   this doesn't happen in your qcomlt branch so I'm guessing you did
   something extra there to prevent this undesired behaviour?

> +button_cmd_0=run menucmd
> +
> +# Hold volume down to drop to a shell with the USB serial gadget
> +# enabled for debugging
> +button_cmd_1_name=Volume down
> +button_cmd_1=run serial_gadget

I would propose to simplify this and just have one button_cmd that is
used to pop the menu, since the serial gadget option is available there
anyway.

-Sam

Re: [PATCH v2 1/4] board/qualcomm: introduce phone config

[Caleb Connolly]

Hi Sam,

On 3/30/25 10:23, Sam Day wrote:
> 'Ello Caleb,
> 
> On Tue Mar 11, 2025 at 1:31 PM CET, Caleb Connolly wrote:
>> Phones don't have keyboards! Introduce a phone-specific config fragment
>> and associated environment file to make U-Boot more useful on these
>> devices. This allows for navigating via the buttons and enabling
>> various USB gadget modes or displaying info about U-Boot.
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
>> ---
>>   board/qualcomm/qcom-phone.config | 17 ++++++++++++++
>>   board/qualcomm/qcom-phone.env    | 49 ++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 66 insertions(+)
>>
>> diff --git a/board/qualcomm/qcom-phone.config b/board/qualcomm/qcom-phone.config
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..a2aa882285b61746a243a7a1c7c384f33839f1b2
>> --- /dev/null
>> +++ b/board/qualcomm/qcom-phone.config
>> @@ -0,0 +1,17 @@
>> +# Settings for phones
>> +CONFIG_DEFAULT_ENV_FILE="board/qualcomm/qcom-phone.env"
>> +# Hang on panic so the error message can be read
>> +CONFIG_PANIC_HANG=y
>> +# We use pause in various places to allow text to be read
>> +# before it scrolls off the screen
>> +CONFIG_CMD_PAUSE=y
>> +CONFIG_BOOT_RETRY=y
>> +CONFIG_BOOT_RETRY_TIME=1
>> +CONFIG_BUTTON_REMAP_PHONE_KEYS=y
> 
> Maybe also include CONFIG_CMD_UMS_ABORT_KEYED=y here? Then folks can
> bail out of UMS mode without needing to hard reboot their device.

good idea heh
> 
>> +CONFIG_RETRY_BOOTCMD=y
>> +CONFIG_FASTBOOT_BUF_ADDR=0x1A000000
>> +CONFIG_USB_FUNCTION_FASTBOOT=y
> 
> I would propose to also add:
> 
> CONFIG_CONSOLE_RECORD=y
> CONFIG_CONSOLE_RECORD_INIT_F=y
> CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000
> CONFIG_FASTBOOT_CMD_OEM_CONSOLE=y
> 
> This allows folks to pull down the U-Boot logs with `fastboot oem log`

Awesome, will add this too
> 
> CONFIG_FASTBOOT_FLASH=y
> CONFIG_FASTBOOT_FLASH_MMC_DEV=0
> 
> Allowing to flash to partitions via fastboot (not sure about if/how UFS
> support works here though, I've only tested it on msm8916 with eMMC)

Unfortunately fastboot doesn't have a UFS backend (and the whole 
fastboot framework code is a big mess that needs a rewrite tbh).
> 
> CONFIG_FASTBOOT_OEM_RUN=y
> 
> Allowing to run arbitrary commands (e.g `fastboot oem run:'bdinfo')


for sure
> 
>> +CONFIG_USB_FUNCTION_ACM=y
>> +
>> +# Many phones don't actually define a serial port in their DTS
>> +# CONFIG_REQUIRE_SERIAL_CONSOLE is not set
>> diff --git a/board/qualcomm/qcom-phone.env b/board/qualcomm/qcom-phone.env
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..2a0791c888c62b9f2257448358dd196a02e4b6fb
>> --- /dev/null
>> +++ b/board/qualcomm/qcom-phone.env
>> @@ -0,0 +1,49 @@
>> +bootdelay=0
>> +bootretry=1
>> +stdin=serial,button-kbd
>> +stdout=serial,vidconsole
>> +stderr=serial,vidconsole
>> +
>> +# Fastboot is keen to use the address from kconfig, but we
>> +# allocate its buffer at runtime.
>> +fastboot=fastboot -l $fastboot_addr_r usb 0
>> +
>> +# Shortcut to enable USB serial gadget and disable bootretry
>> +serial_gadget=setenv stdin serial,button-kbd,usbacm; \
>> +	setenv stdout serial,vidconsole,usbacm; \
>> +	setenv stderr serial,vidconsole,usbacm; \
>> +	setenv bootretry -1; \
>> +	echo Enabled U-Boot console serial gadget
>> +
>> +# bootretry will run this command over and over, if we fail once
>> +# then bail out to the boot menu instead (with a pause to read
>> +# the error message)
>> +bootcmd=bootefi bootmgr; pause; run menucmd
>> +
>> +# When entering the menu (either from button press or failed boot)
>> +# remap bootcmd so it will re-open the menu and we won't get stuck
>> +# at the console with no way to type
>> +menucmd=setenv bootcmd run menucmd; bootmenu -1
>> +
>> +# Pause is used so the output can be read on the display
>> +bootmenu_0=Boot=bootefi bootmgr; pause
>> +bootmenu_1=Enable serial console gadget=run serial_gadget
>> +bootmenu_2=Enable USB mass storage=ums 0 scsi 0
> 
> This doesn't work on my fajita unless I run `scsi scan` first. I note
> that this used to be in the preboot= section of default.env, maybe we
> should bring that across?

ooh yeah, the capsule update series should fix this since it calls 
scsi_scan() as part of initialising the capsule update into, probably 
better safe though.
> 
> Also, if you choose to include CONFIG_CMD_UMS_ABORT_KEYED=y per my
> earlier suggestion, it would be worth adding an "echo press any key to
> exit UMS mode" here or smth, I guess?

Makes sense
> 
>> +bootmenu_3=Reset device=reset
>> +bootmenu_4=Dump clocks=clk dump; pause
>> +bootmenu_5=Dump environment=printenv; pause
>> +bootmenu_6=Board info=bdinfo; pause
>> +bootmenu_7=Dump bootargs=fdt print /chosen bootargs; pause
> 
> I think options 4-7 should be at the bottom of the list since they're
> less typical/useful than jumping into usbacm/fastboot/ums modes. Reset
> device should be at the bottom of the list.
> 
> I'd maybe also go as far as suggesting that these could be removed
> entirely? Once you're in serial mode (or fastboot mode with
> CONFIG_FASTBOOT_OEM_RUN enabled) you can run these commands easily.

yeah i'll revise this
> 
>> +bootmenu_8=Enable fastboot mode=run fastboot
>> +# Disabling bootretry means we'll just drop the shell
>> +bootmenu_9=Drop to shell=setenv bootretry -1
>> +
>> +# Allow holding the power button while U-Boot loads to enter
>> +# the boot menu
>> +button_cmd_0_name=pwrkey
> 
> Two issues with using pwrkey as the boot menu trigger:
> 
>   * Many devices display a "custom OS" warning screen which can be paused
>     with the power button, which this will conflict with.
>   * When I trigger the boot menu this way, releasing pwrkey when the boot
>     menu shows up causes the first boot option to be selected. I note
>     this doesn't happen in your qcomlt branch so I'm guessing you did
>     something extra there to prevent this undesired behaviour?

ahaha i have a patch that makes buttons trigger on press instead of 
release, i didn't work up the courage to propose it upstream yet though.

These are very good points. We'll never find a perfect option but a 
volume button is almost certainly better. Unfortunately not every device 
names the volume buttons the same so, urgh. I'll see what I can come up with
> 
>> +button_cmd_0=run menucmd
>> +
>> +# Hold volume down to drop to a shell with the USB serial gadget
>> +# enabled for debugging
>> +button_cmd_1_name=Volume down
>> +button_cmd_1=run serial_gadget
> 
> I would propose to simplify this and just have one button_cmd that is
> used to pop the menu, since the serial gadget option is available there
> anyway.

Makes sense to me.

Thanks a lot for taking the time to give this feedback.

Kind regards,
> 
> -Sam
> 
> 

-- 
Caleb (they/them)