On 10/01/2024 18:08, Dragan Simic wrote:
On 2024-01-09 12:51, Caleb Connolly wrote:
With the relatively new button API in U-Boot, it's now much easier to model the common usecase of mapping arbitrary actions to different buttons during boot - for example entering fastboot mode, setting some additional kernel cmdline arguments, or booting with a custom recovery ramdisk, to name a few.
Historically, this functionality has been implemented in board code, making it fixed for a given U-Boot binary and requiring the code be duplicated and modified for every board.
Implement a generic abstraction to run an arbitrary command during boot when a specific button is pressed. The button -> command mapping is configured via environment variables with the following format:
button_cmd_N_name=<button label> button_cmd_N=<command to run>
Where N is the mapping number starting from 0. For example:
button_cmd_0_name=vol_down button_cmd_0=fastboot usb 0
This will cause the device to enter fastboot mode if volume down is held during boot.
This is simply awesome, but I see one possible issue -- the need to have proper environment variables defined for a particular board or device, to make the buttons work as expected. Obviously, those environment variables can be absent or can become missing for numerous reasons.
Is CFG_EXTRA_ENV_SETTINGS not persistent enough?
I think that we should have an additional mechanism in place that defines the buttons and the associated commands even if no environment variables are found. Like a set of fallback defaults for a particular board or device, built into the U-Boot image. For example, Rockchip boards have those defaults pretty well defined.
A programmatic API for register button/cmd mapping from board_late_init() (for example) sounds sensible to me.
Is this really an issue that invalidates the implementation proposed here though? It feels much more like a nice-to-have addition that maybe we could leave out for now?
It also has a MUCH wider scope imo - should board override env or vice versa? What about triggering default AND custom actions for one button press? What if a board wants to register a callback function instead of running a command?) - these are questions I don't want to answer with this patch.
After we enter the cli loop the button commands are no longer valid, this allows the buttons to additionally be used for navigating a boot menu.
Tested-by: Svyatoslav Ryhel clamor95@gmail.com # Tegra30 Signed-off-by: Caleb Connolly caleb.connolly@linaro.org
Changes since v1: * make get_button_cmd() static. * use #if IS_ENABLED(CONFIG_BUTTON_CMD) instead of #ifdef CONFIG_BUTTON_CMD. * Kconfig fixes
boot/Kconfig | 15 +++++++ common/Makefile | 1 + common/button_cmd.c | 83 +++++++++++++++++++++++++++++++++++++++ common/main.c | 3 ++ doc/usage/environment.rst | 4 ++ include/button.h | 9 +++++ 6 files changed, 115 insertions(+) create mode 100644 common/button_cmd.c
diff --git a/boot/Kconfig b/boot/Kconfig index fbc49c5bca47..882835731ea9 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -20,6 +20,21 @@ config TIMESTAMP loaded that does not, the message 'Wrong FIT format: no timestamp' is shown.
+config BUTTON_CMD + bool "Support for running a command if a button is held during boot" + depends on CMDLINE + depends on BUTTON + help + For many embedded devices it's useful to enter a special flashing mode + such as fastboot mode when a button is held during boot. This option + allows arbitrary commands to be assigned to specific buttons. These will + be run after "preboot" if the button is held. Configuration is done via + the environment variables "button_cmd_N_name" and "button_cmd_N" where n is + the button number (starting from 0). e.g:
+ "button_cmd_0_name=vol_down" + "button_cmd_0=fastboot usb 0"
menuconfig FIT bool "Flattened Image Tree (FIT)" select HASH diff --git a/common/Makefile b/common/Makefile index cdeadf72026c..53105a6ce12a 100644 --- a/common/Makefile +++ b/common/Makefile @@ -10,6 +10,7 @@ obj-y += main.o obj-y += exports.o obj-$(CONFIG_HUSH_PARSER) += cli_hush.o obj-$(CONFIG_AUTOBOOT) += autoboot.o +obj-$(CONFIG_BUTTON_CMD) += button_cmd.o
# # boards obj-y += board_f.o diff --git a/common/button_cmd.c b/common/button_cmd.c new file mode 100644 index 000000000000..b6a8434d6f29 --- /dev/null +++ b/common/button_cmd.c @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (c) 2023 Linaro Ltd.
- * Author: Caleb Connolly caleb.connolly@linaro.org
- */
+#include <button.h> +#include <command.h> +#include <env.h> +#include <log.h> +#include <vsprintf.h>
+/* Some sane limit "just in case" */ +#define MAX_BTN_CMDS 32
+struct button_cmd { + bool pressed; + const char *btn_name; + const char *cmd; +};
+/*
- Button commands are set via environment variables, e.g.:
- button_cmd_N_name=Volume Up
- button_cmd_N=fastboot usb 0
- This function will retrieve the command for the given button N
- and populate the cmd struct with the command string and pressed
- state of the button.
- Returns 1 if a command was found, 0 otherwise.
- */
+static int get_button_cmd(int n, struct button_cmd *cmd) +{ + const char *cmd_str; + struct udevice *btn; + char buf[24];
+ snprintf(buf, sizeof(buf), "button_cmd_%d_name", n); + cmd->btn_name = env_get(buf); + if (!cmd->btn_name) + return 0;
+ button_get_by_label(cmd->btn_name, &btn); + if (!btn) { + log_err("No button labelled '%s'\n", cmd->btn_name); + return 0; + }
+ cmd->pressed = button_get_state(btn) == BUTTON_ON; + /* If the button isn't pressed then cmd->cmd will be unused so don't waste + * cycles reading it + */ + if (!cmd->pressed) + return 1;
+ snprintf(buf, sizeof(buf), "button_cmd_%d", n); + cmd_str = env_get(buf); + if (!cmd_str) { + log_err("No command set for button '%s'\n", cmd->btn_name); + return 0; + }
+ cmd->cmd = cmd_str;
+ return 1; +}
+void process_button_cmds(void) +{ + struct button_cmd cmd = {0}; + int i = 0;
+ while (get_button_cmd(i++, &cmd) && i < MAX_BTN_CMDS) { + if (!cmd.pressed) + continue;
+ log_info("BTN '%s'> %s\n", cmd.btn_name, cmd.cmd); + run_command(cmd.cmd, CMD_FLAG_ENV); + /* Don't run commands for multiple buttons */ + return; + } +} diff --git a/common/main.c b/common/main.c index 7c70de2e59a8..717e8b7e8bd2 100644 --- a/common/main.c +++ b/common/main.c @@ -8,6 +8,7 @@
#include <common.h> #include <autoboot.h> +#include <button.h> #include <bootstage.h> #include <cli.h> #include <command.h> @@ -61,6 +62,8 @@ void main_loop(void) efi_launch_capsules(); }
+ process_button_cmds();
s = bootdelay_process(); if (cli_process_fdt(&s)) cli_secure_boot_cmd(s); diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst index c57b717caaf3..ce5a9627f025 100644 --- a/doc/usage/environment.rst +++ b/doc/usage/environment.rst @@ -190,6 +190,10 @@ bootm_size bootstopkeysha256, bootdelaykey, bootstopkey See README.autoboot
+button_cmd_0, button_cmd_0_name ... button_cmd_N, button_cmd_N_name + Used to map commands to run when a button is held during boot. + See CONFIG_BUTTON_CMD.
updatefile Location of the software update file on a TFTP server, used by the automatic software update feature. Please refer to diff --git a/include/button.h b/include/button.h index 207f4a0f4dbd..8d38e521324d 100644 --- a/include/button.h +++ b/include/button.h @@ -74,4 +74,13 @@ enum button_state_t button_get_state(struct udevice *dev); */ int button_get_code(struct udevice *dev);
+#if IS_ENABLED(CONFIG_BUTTON_CMD) +/* Process button command mappings specified in the environment,
- running the commands for buttons which are pressed
- */
+void process_button_cmds(void); +#else +static inline void process_button_cmds(void) {} +#endif /* CONFIG_BUTTON_CMD */
#endif