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
