Re: [PATCH v3 2/6] reset: Add explicit cold reset support

[dmukhin@ford.com]

On Mon, Jun 01, 2026 at 08:54:25AM -0600, Simon Glass wrote:
> Hi Denis,
> 
> On 2026-05-29T03:48:33, None <dmukhin@ford.com> wrote:
> > reset: Add explicit cold reset support
> >
> > Some prototype boards default to a non-cold reset type, e.g. warm reset.
> >
> > Add reset -c so users can excplicitly request a cold reset when needed.
> >
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> >
> > cmd/boot.c                         |  3 ++-
> >  drivers/sysreset/sysreset-uclass.c | 13 +++++++++++--
> >  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> > diff --git a/drivers/sysreset/sysreset-uclass.c b/drivers/sysreset/sysreset-uclass.c
> > @@ -174,8 +174,17 @@ int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> >       if (argc > 2)
> >               return CMD_RET_USAGE;
> >
> > -     if (argc == 2 && argv[1][0] == '-' && argv[1][1] == 'w') {
> > -             reset_type = SYSRESET_WARM;
> > +     if (argc == 2 && argv[1][0] == '-') {
> > +             switch (argv[1][1]) {
> > +             case 'c':
> > +                     reset_type = SYSRESET_COLD;
> > +                     break;
> > +             case 'w':
> > +                     reset_type = SYSRESET_WARM;
> > +                     break;
> > +             default:
> > +                     return CMD_RET_USAGE;
> > +             }
> >       }
> 
> This regresses reset -edl on Qualcomm. Previously, an unknown -x
> argument fell through to the SYSRESET_CMD_RESET_ARGS path, where
> qcom_psci_sysreset_request_arg() picks up -edl and returns
> -EINPROGRESS. With this change, argv[1][1] == 'e' hits the default
> branch and bails out with CMD_RET_USAGE before sysreset_walk_arg()
> runs, so reset -edl no longer works.
> 
> Please leave reset_type at its default for unrecognised flags (so the
> arg-dispatch path can still claim them), or call sysreset_walk_arg()
> first and only complain if nothing handled the argument. Also,
> matching only argv[1][1] means reset -cx silently behaves like reset
> -c - please tighten with strcmp() or a length check.

Yes, will do; thanks for catching this!

> 
> The could be a good usage for getopt() but sadly it increases code
> size too much [1]
> 
> BTW -edl doesn't follow the normal flags approach, so we should
> probably change that at some point.
> 
> > diff --git a/cmd/boot.c b/cmd/boot.c
> > @@ -59,10 +59,11 @@ U_BOOT_CMD(
> >  U_BOOT_CMD(
> >       reset, 2, 0,    do_reset,
> >       "Perform RESET of the CPU",
> > -     "- cold boot without level specifier\n"
> > +     "- reset without level specifier\n"
> >  #ifdef CONFIG_SYSRESET_QCOM_PSCI
> >       "reset -edl - Boot to Emergency DownLoad mode\n"
> >  #endif
> > +     "reset -c - cold reset if implemented\n"
> >       "reset -w - warm reset if implemented"
> >  );
> 
> The '- reset without level specifier' line reads oddly now that the
> level is no longer guaranteed to be cold - perhaps '- reset using the
> configured default type'.
> 
> > Add reset -c so users can excplicitly request a cold reset when needed.
> 
> Typo: excplicitly -> explicitly. Also please use single quotes around
> tokens like 'reset -c' per U-Boot convention.
> 
> Regards,
> Simon
> 
> [1] https://patchwork.ozlabs.org/project/uboot/cover/20260519233207.2765755-1-sjg@chromium.org/