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 <[email protected]> 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 <[email protected]> > > > > 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
