[PATCH v2] boot: add support for button commands

[Dragan Simic]

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.

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.

> 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 at gmail.com> # Tegra30
> Signed-off-by: Caleb Connolly <caleb.connolly at 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 at 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