diff mbox series

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

Message ID 20250311-qcom-phones-v2-1-83dcd88a6a87@linaro.org
State New
Headers show
Series Better smartphone support (Qualcomm) | expand

Commit Message

Caleb Connolly March 11, 2025, 12:31 p.m. UTC
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(+)

Comments

Sam Day March 30, 2025, 8:23 a.m. UTC | #1
'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
Caleb Connolly March 30, 2025, 1:30 p.m. UTC | #2
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
> 
>
diff mbox series

Patch

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