[PATCH v2] boot: add support for button commands
Caleb Connolly
caleb.connolly at linaro.org
Wed Jan 10 21:09:47 CET 2024
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 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
--
// Caleb (they/them)
More information about the U-Boot
mailing list