Hi Balaji, On 2026-08-11T05:18:35, Balaji Selvanathan <[email protected]> wrote: > cmd: reset: dispatch named reset modes via the reboot-mode framework > > Teach the "reset" command to trigger the reset modes registered with the > reboot-mode framework in addition to the existing cold and warm resets: > > reset cold reset (unchanged) > reset -w warm reset (unchanged) > reset -<mode> reset into a mode declared in the device tree > reset -l list the available reset modes > > For example "reset -edl" enters Qualcomm EDL/download mode when the psci > device tree node carries a "reboot-mode" subnode with > "mode-edl = <0x80000000 0x00000001>". The command never parses the magic > values itself; it strips the leading '-' and hands the mode name to > reboot_mode_request(), so the values stay in the device tree and are not > hardcoded per SoC. An unknown mode prints the registered modes rather than > silently falling through to a cold reset.
I'm going to argue that '-edl' is a bit strange. It looks like three separate flags (-e, -d, -l) and may end up being confusing. Perhaps we should allocate a proper flag for download mode, e.g. -d ? Otherwise, I suggest using a flag to specify the mode, e.g. '-m <mode>' > > Signed-off-by: Balaji Selvanathan <[email protected]> > > cmd/boot.c | 6 +++++- > drivers/sysreset/sysreset-uclass.c | 27 ++++++++++++++++++++++++++- > 2 files changed, 31 insertions(+), 2 deletions(-) > diff --git a/cmd/boot.c b/cmd/boot.c > @@ -60,7 +60,11 @@ U_BOOT_CMD( > reset, 2, 0, do_reset, > "Perform RESET of the CPU", > "- cold boot without level specifier\n" > - "reset -w - warm reset if implemented" > + "reset -w - warm reset if implemented\n" > +#if IS_ENABLED(CONFIG_DM_REBOOT_MODE) > + "reset -<mode> - reset into a mode declared in the device tree\n" > + "reset -l - list the available reset modes\n" > +#endif > ); The help text is gated on CONFIG_DM_REBOOT_MODE, but the dispatch code below only exists under CONFIG_SYSRESET_CMD_RESET. Boards that use an arch do_reset() (e.g. arch/arm/lib/reset.c) will advertise 'reset -<mode>' in help but silently cold-reset when it is used. Either gate the help on both, or move the dispatch into a common location (perhaps a new do_reset() wrapper in cmd/boot.c that calls into the reboot-mode helpers before falling through to the arch/sysreset implementation). > diff --git a/drivers/sysreset/sysreset-uclass.c > b/drivers/sysreset/sysreset-uclass.c > @@ -13,6 +13,7 @@ > #include <hang.h> > #include <log.h> > #include <regmap.h> > +#include <reboot-mode/reboot-mode.h> > #include <spl.h> We normally put subdirs at the end of the sorting order. > diff --git a/drivers/sysreset/sysreset-uclass.c > b/drivers/sysreset/sysreset-uclass.c > @@ -125,8 +126,32 @@ int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, > char *const argv[]) > - if (argc == 2 && argv[1][0] == '-' && argv[1][1] == 'w') { > + if (argc == 2 && argv[1][0] == '-' && argv[1][1] == 'w' && > + !argv[1][2]) { > reset_type = SYSRESET_WARM; > + } else if (CONFIG_IS_ENABLED(DM_REBOOT_MODE) && argc == 2 && > + argv[1][0] == '-') { Please use IS_ENABLED() to match the help text in cmd/boot.c - CONFIG_IS_ENABLED() picks up the SPL/TPL variant and there's no reason for that in a command handler. > diff --git a/drivers/sysreset/sysreset-uclass.c > b/drivers/sysreset/sysreset-uclass.c > @@ -125,8 +126,32 @@ int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, > char *const argv[]) > + printf("resetting into \"%s\" mode ...\n", name); > + mdelay(100); > + > + /* Does not return on success. */ > + reboot_mode_request(name); > + > + printf("Unknown reset mode \"%s\"\n", name); > + reboot_mode_list(); > + return CMD_RET_USAGE; Two things. First, please look the mode up before printing 'resetting into ...' and delaying - as written, an unknown mode causes a bogus "resetting into" line, a 100ms pause and then the error, which reads oddly on the console. Second, an unknown mode is a runtime failure, not a usage error, so CMD_RET_FAILURE is a better fit; CMD_RET_USAGE will splat the full command help on top of the list you just printed. Please add a blank line before the final return in functions > diff --git a/drivers/sysreset/sysreset-uclass.c > b/drivers/sysreset/sysreset-uclass.c > @@ -125,8 +126,32 @@ int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, > char *const argv[]) > + if (!strcmp(name, "l")) > + return reboot_mode_list() ? CMD_RET_FAILURE : > + CMD_RET_SUCCESS; '-l' collides with any mode literally named 'l'. Very unlikely in practice, but worth a comment explaining that '-l' is reserved, and perhaps the mode-name parser in patch 1 should reject a name of 'l' or a name starting with '-' so the collision is caught at bind time rather than at 'reset -l'. Regards, Simon
