Re: [PATCH v1 1/4] reset: Allow per-board reset type

[dmukhin@ford.com]

Hi Simon,

Thanks for reviews!

On Mon, May 25, 2026 at 09:28:30AM -0600, Simon Glass wrote:
> Hi Denis,
> 
> On 2026-05-22T01:23:09, 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 board_sysreset_default() to obtain board-specific default reset
> > type to enable reset command on such boards.
> >
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> >
> > drivers/sysreset/sysreset-uclass.c | 7 ++++++-
> >  include/sysreset.h                 | 7 +++++++
> >  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> > diff --git a/drivers/sysreset/sysreset-uclass.c b/drivers/sysreset/sysreset-uclass.c
> > @@ -150,9 +150,14 @@ void reset_cpu(void)
> >  }
> >
> >  #if IS_ENABLED(CONFIG_SYSRESET_CMD_RESET)
> > +__weak enum sysreset_t board_sysreset_default(void)
> > +{
> > +     return SYSRESET_COLD;
> > +}
> > +
> 
> I'm not convinced a weak board hook is right here. Since the value is
> a compile-time constant per board, a Kconfig choice
> (SYSRESET_CMD_DEFAULT_COLD / _WARM) would be cleaner: no per-board C,
> visible from defconfig, and no new ABI for the sake of prototypes.
> What do you think?

TBH, I did weak symbol only to minimize the diff: this way the change
is isolated to sysreset-uclass.c.

I agree, Kconfig is a cleaner way, will fix that in v2.

> 
> The cover letter says cold reset must still be enabled for the
> production variant. If the board gets proper cold-reset support before
> shipping, the override really only wants to live in the prototype's
> board code - a Kconfig option makes that lifecycle obvious.
> 
> We could perhaps go in a different direction and use a sysinfo driver
> to specify the default, but that seems like overkill.
> 
> Regards,
> Simon

Thanks,
Denis