Re: [PATCH 2/8] reboot-mode: add trigger op and request/list helpers

Hi Balaji,

On 2026-08-11T05:18:35, Balaji Selvanathan
<[email protected]> wrote:
> reboot-mode: add trigger op and request/list helpers
>
> The reboot-mode framework so far only reads a value from a backing store
> on boot (get/set) and maps it to an environment variable. Triggering a
> reset into a specific mode on demand - e.g. rebooting a Qualcomm SoC into
> EDL via a PSCI SYSTEM_RESET2 vendor reset - needs a different approach.
>
> Add an trigger() op to struct reboot_mode_ops. It takes the
> decoded magic cells for a mode and resets the system immediately; on
> success it does not return. Backing-store drivers (nvmem, gpio, rtc)
> leave it NULL and are unaffected.

Nit: 'Add a trigger() op'.

>
> Add two uclass helpers built on top of it:
>
>   - reboot_mode_request(name) looks up a mode by name across all
>     UCLASS_REBOOT_MODE devices and, if the owning device can trigger,
>     resets into it. Returns -ENOENT if no triggerable mode matches.
>
>   - reboot_mode_list() enumerates every triggerable mode registered with
>     the framework, so a command can present the available modes to the
> [...]
>
> drivers/reboot-mode/reboot-mode-uclass.c | 48 ++++++++++++++++++++++++++++++++
>  include/reboot-mode/reboot-mode.h        | 40 ++++++++++++++++++++++++++
>  2 files changed, 88 insertions(+)

> diff --git a/drivers/reboot-mode/reboot-mode-uclass.c b/drivers/reboot-mode/reboot-mode-uclass.c
> @@ -54,6 +54,54 @@ int dm_reboot_mode_update(struct udevice *dev)
> +int reboot_mode_list(void)
> +{
> +     const struct reboot_mode_uclass_platdata *plat_data;
> +     struct reboot_mode_ops *ops;
> +     struct udevice *dev;
> +     int i;
> +
> +     printf("Available reset modes:\n");
> +
> +     uclass_foreach_dev_probe(UCLASS_REBOOT_MODE, dev) {
> +             ops = reboot_mode_get_ops(dev);
> +             if (!ops || !ops->trigger)
> +                     continue;
> +
> +             plat_data = dev_get_uclass_plat(dev);
> +             for (i = 0; i < plat_data->count; i++)
> +                     printf("  %s\n", plat_data->modes[i].mode_name);
> +     }
> +
> +     return 0;
> +}

We normally try to keep printing out of the uclass, unless it needs to
be in a shared place and called from other places (e.g. multiple
commands need it). Also 'reset -l' on a board with no triggerable
modes prints only the header, which looks odd. So can you move to cmd/
if possible?

> diff --git a/drivers/reboot-mode/reboot-mode-uclass.c b/drivers/reboot-mode/reboot-mode-uclass.c
> @@ -54,6 +54,54 @@ int dm_reboot_mode_update(struct udevice *dev)
> +             plat_data = dev_get_uclass_plat(dev);
> +             for (i = 0; i < plat_data->count; i++) {
> +                     if (strcmp(plat_data->modes[i].mode_name, name))
> +                             continue;
> +
> +                     /* Does not return on success. */
> +                     return ops->trigger(dev, plat_data->modes[i].magic,
> +                                         plat_data->modes[i].count);
> +             }

If trigger() ever does return 0 (buggy driver, or a future async
path), reboot_mode_request() hands 0 back to the caller and 'reset
-edl' silently falls through with no reset and no diagnostic. Please
either treat a 0 return as an error here, or document in the ops
kernel-doc that trigger() must return a -ve value if it fails to
reset. Also worth stating in the header what happens when two devices
declare the same mode name - first hit wins, I assume.

> diff --git a/include/reboot-mode/reboot-mode.h b/include/reboot-mode/reboot-mode.h
> @@ -46,6 +46,22 @@ struct reboot_mode_ops {
> +      * @dev:        Device to trigger
> +      * @magic:      Array of @count 32-bit magic cells describing the mode
> +      * @count:      Number of valid cells in @magic (1 to
> +      *              REBOOT_MODE_MAX_MAGIC)
> +      * Return: does not return on success; -ve on error
> +      */
> +     int (*trigger)(struct udevice *dev, const u32 *magic, int count);

count is stored as u8 in struct reboot_mode_mode but passed here as
int. Please make the op prototype uint to match, so a caller cannot
pass a negative value that the driver then has to defend against.

Regards,
Simon