Re: [PATCH v3 1/6] reset: Allow per-board reset type

[dmukhin@ford.com]

On Mon, Jun 01, 2026 at 08:54:18AM -0600, Simon Glass wrote:
> Hi Denis,
> 
> On 2026-05-29T03:48:33, None <dmukhin@ford.com> wrote:
> > reset: Allow per-board reset type
> >
> > Some prototype boards may not have (temporarily) all required reset
> > types supported (e.g. only warm reset supported).
> >
> > Add a Kconfig choice to select the board-specific default reset type
> > used by the reset command, allowing it to work on such boards.
> >
> > Keep cold reset as the default.
> >
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> >
> > drivers/sysreset/Kconfig           | 31 +++++++++++++++++++++++++++++++
> >  drivers/sysreset/sysreset-uclass.c | 19 ++++++++++++++++++-
> >  2 files changed, 49 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>

Thanks!

> 
> > diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
> > @@ -57,6 +57,37 @@ config SYSRESET_CMD_RESET_ARGS
> > +config SYSRESET_CMD_RESET_DEFAULT_WARM
> > +     bool "Warm reset"
> > +     help
> > +       Reset CPU while keeping GPIOs active.
> > +
> > +config SYSRESET_CMD_RESET_DEFAULT_COLD
> > +     bool "Cold reset"
> > +     help
> > +       Reset CPU and GPIOs.
> 
> This is very thin - as is the comment in include/sysreset.h - how about:
> 
> "Full cold reset of the machine, including CPU, GPIOs, PMIC and other
> logic in the machine"
> 
> > diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
> > @@ -57,6 +57,37 @@ config SYSRESET_CMD_RESET_ARGS
> > +config SYSRESET_CMD_RESET_DEFAULT_POWER
> > +     bool "Power reset"
> > +     help
> > +       Reset PMIC by removing and restoring power.
> > +
> > +config SYSRESET_CMD_RESET_DEFAULT_POWER_OFF
> > +     bool "Power off"
> > +     help
> > +       Turn off power.
> 
> Could we use the 'poweroff' command instead?

Will drop SYSRESET_CMD_RESET_DEFAULT_POWER_OFF in v4 and leave
a code commentary to rely on poweroff command.

> 
> > diff --git a/drivers/sysreset/sysreset-uclass.c b/drivers/sysreset/sysreset-uclass.c
> > @@ -150,9 +150,26 @@ void reset_cpu(void)
> > +static enum sysreset_t sysreset_get_default_type(void)
> > +{
> > +     if (IS_ENABLED(CONFIG_SYSRESET_CMD_RESET_DEFAULT_WARM))
> > +             return SYSRESET_WARM;
> > +
> > +     if (IS_ENABLED(CONFIG_SYSRESET_CMD_RESET_DEFAULT_COLD))
> > +             return SYSRESET_COLD;
> > +
> > +     if (IS_ENABLED(CONFIG_SYSRESET_CMD_RESET_DEFAULT_POWER))
> > +             return SYSRESET_POWER;
> > +
> > +     if (IS_ENABLED(CONFIG_SYSRESET_CMD_RESET_DEFAULT_POWER_OFF))
> > +             return SYSRESET_POWER_OFF;
> > +
> > +     return SYSRESET_COLD;
> > +}
> 
> The trailing return SYSRESET_COLD is unreachable, BTW.

Thanks for the catch!

> 
> Regards,
> Simon