Re: [PATCH v2 2/4] bootchooser: extend cmd tool by option to set attempts_locked

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 6/16/25 17:06, Lars Schmidt wrote:
> This flag is normally set in userspace. But having
> the option of setting it in bootloader gives more debugging
> options.
> 
> Signed-off-by: Lars Schmidt <l.schmidt@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

There's a boot test pending review[1]. I intend to submit a test
exercising this feature and using the bootchooser -l/-L command later.

[1]:
https://lore.barebox.org/barebox/20250612085603.4190573-1-a.fatoum@barebox.org/T/#t

Cheers,
Ahmad

> ---
>  commands/bootchooser.c | 16 +++++++++++++++-
>  common/bootchooser.c   | 28 ++++++++++++++++++++++++++++
>  include/bootchooser.h  |  1 +
>  3 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/commands/bootchooser.c b/commands/bootchooser.c
> index 46b063e027db30073ada28487bc0434026cc081a..4199085f352b289e069f22870c0239aede38df61 100644
> --- a/commands/bootchooser.c
> +++ b/commands/bootchooser.c
> @@ -48,8 +48,9 @@ static int do_bootchooser(int argc, char *argv[])
>  	int info = 0;
>  	bool done_something = false;
>  	bool last_boot_successful = false;
> +	int lock_state = -1;
>  
> -	while ((opt = getopt(argc, argv, "a:p:is")) > 0) {
> +	while ((opt = getopt(argc, argv, "a:p:islL")) > 0) {
>  		switch (opt) {
>  		case 'a':
>  			if (!strcmp(optarg, "default"))
> @@ -69,6 +70,12 @@ static int do_bootchooser(int argc, char *argv[])
>  		case 's':
>  			last_boot_successful = true;
>  			break;
> +		case 'l':
> +			lock_state = true;
> +			break;
> +		case 'L':
> +			lock_state = false;
> +			break;
>  		default:
>  			return COMMAND_ERROR_USAGE;
>  		}
> @@ -109,6 +116,11 @@ static int do_bootchooser(int argc, char *argv[])
>  		done_something = true;
>  	}
>  
> +	if (lock_state >= 0) {
> +		ret = bootchooser_lock_attempts(bootchooser, lock_state);
> +		done_something = true;
> +	}
> +
>  	if (!done_something) {
>  		printf("Nothing to do\n");
>  		ret = COMMAND_ERROR_USAGE;
> @@ -126,6 +138,8 @@ BAREBOX_CMD_HELP_TEXT("Options:")
>  BAREBOX_CMD_HELP_OPT ("-a <n|default> [TARGETS]",  "set remaining attempts of given targets to 'n' or the default attempts")
>  BAREBOX_CMD_HELP_OPT ("-p <n|default> [TARGETS]",  "set priority of given targets to 'n' or the default priority")
>  BAREBOX_CMD_HELP_OPT ("-i",  "Show information about the bootchooser")
> +BAREBOX_CMD_HELP_OPT ("-l",  "Enable locking of remaining attempts")
> +BAREBOX_CMD_HELP_OPT ("-L",  "Disable locking of remaining attempts")
>  BAREBOX_CMD_HELP_OPT ("-s",  "Mark the last boot successful")
>  BAREBOX_CMD_HELP_END
>  
> diff --git a/common/bootchooser.c b/common/bootchooser.c
> index 50ce73682fb6bac8e4319119169dfb8334eb8762..7e4622aad5dfe02d8e798a249c46cd1c048dc3c9 100644
> --- a/common/bootchooser.c
> +++ b/common/bootchooser.c
> @@ -631,6 +631,9 @@ void bootchooser_info(struct bootchooser *bc)
>  
>  	printf("\nlast booted target: %s\n", bc->last_chosen ?
>  	       bc->last_chosen->name : "unknown");
> +
> +	printf("Locking of boot attempt counter: %s",
> +	       bc->attempts_locked ? "enabled" : "disabled");
>  }
>  
>  /**
> @@ -817,6 +820,31 @@ struct bootchooser_target *bootchooser_get_last_chosen(struct bootchooser *bc)
>  	return bc->last_chosen;
>  }
>  
> +/**
> + * bootchooser_lock_attempts - lock the bootchooser attempt counter
> + * @bc:		The bootchooser
> + * @locked:     Whether the attempt counter is locked or not.
> + *
> + * Instruct bootchooser to lock the boot attempts counter.
> + * This means remaining_attempts will not be counted down.
> + *
> + * Return: 0 for success, negative error code otherwise
> + */
> +int bootchooser_lock_attempts(struct bootchooser *bc, bool locked)
> +{
> +	uint32_t not_needed;
> +	/* We just need to check here, if the value exists in the device tree
> +	 * So if it doesn't exist, we can inform user about it for easier debugging
> +	 */
> +	if (getenv_u32(bc->state_prefix, "attempts_locked", &not_needed)) {
> +		pr_warn("Missing attempts_locked property in state DT node\n");
> +		return -ENOENT;
> +	}
> +
> +	bc->attempts_locked = locked;
> +	return 0;
> +}
> +
>  static int bootchooser_boot_one(struct bootchooser *bc, int *tryagain)
>  {
>  	char *system;
> diff --git a/include/bootchooser.h b/include/bootchooser.h
> index 31989163b236196d6355acb9e7d30a18b784baa2..69783b0b7494ac303a17a466d599559513fb6d17 100644
> --- a/include/bootchooser.h
> +++ b/include/bootchooser.h
> @@ -17,6 +17,7 @@ void bootchooser_info(struct bootchooser *bootchooser);
>  int bootchooser_boot(struct bootchooser *bc);
>  
>  struct bootchooser_target *bootchooser_get_last_chosen(struct bootchooser *bootchooser);
> +int bootchooser_lock_attempts(struct bootchooser *bc, bool locked);
>  const char *bootchooser_target_name(struct bootchooser_target *target);
>  struct bootchooser_target *bootchooser_target_by_name(struct bootchooser *bootchooser,
>  						      const char *name);
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |