Hi Quentin, On Thu, 23 Jul 2026 at 10:43, Quentin Schulz <[email protected]> wrote: > > Hi Simon, > > On 6/27/26 10:30 AM, Simon Glass wrote: > > Hi Quentin, > > > > On Fri, 26 Jun 2026 at 16:01, Quentin Schulz <[email protected]> > > wrote: > >> > >> Hi Denis, > >> > >> On 6/3/26 9:07 AM, [email protected] wrote: > >>> Add a diagnostic console trace indicating the reset type. > >>> > >>> Signed-off-by: Denis Mukhin <[email protected]> > >>> --- > >>> Changes since v3: > >>> - moved get_reset_type_str() next to do_reset() > >>> --- > >>> drivers/sysreset/sysreset-uclass.c | 18 +++++++++++++++++- > >>> 1 file changed, 17 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/sysreset/sysreset-uclass.c > >>> b/drivers/sysreset/sysreset-uclass.c > >>> index 1ba698b37285..2b7717857ce6 100644 > >>> --- a/drivers/sysreset/sysreset-uclass.c > >>> +++ b/drivers/sysreset/sysreset-uclass.c > >>> @@ -161,6 +161,22 @@ static enum sysreset_t > >>> sysreset_get_default_type(void) > >>> return SYSRESET_COLD; > >>> } > >>> > >>> +static const char *get_reset_type_str(enum sysreset_t reset_type) > >>> +{ > >>> + switch (reset_type) { > >>> + case SYSRESET_WARM: > >>> + return "warm"; > >>> + case SYSRESET_COLD: > >>> + return "cold"; > >>> + case SYSRESET_POWER: > >>> + return "power"; > >>> + case SYSRESET_POWER_OFF: > >>> + return "power off"; > >>> + default: > >>> + return "unknown"; > >>> + } > >>> +} > >>> + > >>> int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, char *const > >>> argv[]) > >>> { > >>> enum sysreset_t reset_type = sysreset_get_default_type(); > >>> @@ -181,7 +197,7 @@ int do_reset(struct cmd_tbl *cmdtp, int flag, int > >>> argc, char *const argv[]) > >>> } > >>> } > >>> > >>> - printf("resetting ...\n"); > >>> + printf("resetting (%s)...\n", get_reset_type_str(reset_type)); > >> > >> NACK, this is potentially misleading as sysreset drivers can end up > >> performing something different (see sysreset_walk_arg() just below). > > > > That's always been the case, but it was silent - do you suggest that > > We only said we are resetting, not which kind. > > > it prints a new reset message for each type? Bear in mind that when > > one succeeds it is unlikely that the serial console will be updated > > before the reset happens. In the majority of cases the requested reset > > succeeds. > > > > Indeed. > > > I quite like showing which reset is being attempted. > > I don't think it's worth the potential confusion. After all, we may very > well NOT do the requested kind of reset. Wording will be important if we > really want to do this. We need to be clear something else may be done > in the end. > > We could print before attempting each sysreset type which kind we're > trying... I think it's actually a good idea, because it isn't > necessarily clear that even if you request a warm reset, you may still > get a cold reset (or even a power-off) if no driver supports doing a > warm reset. At the same time, we don't have a delay between printing and > attempting a reset, so indeed like you said it may never get printed. > > So, first line in the while-loop in sysreset_walk would say "attempting > sysreset <type>" and after the for-loop we check on ret != -EINPROGRESS > and then print "no driver could do sysreset <type>, trying more > aggressive reset" or something like that. Before return, we check on > -EINPROGRESS and tell the user we failed to do any kind of reset. We > probably should have some of those messages rather debug messages > though, I fear it may be a bit verbose.
The odd thing is that the last thing you would likely see (with this approach) is the reset type before the one that actually succeeded. It could be a bit confusing. But I suppose that would be OK, since we are resetting and people should be aware that this can result in lost serial output. Another approach (perhaps a follow-up to this series) could be to have the driver indicate (e.g. with plat data) which reset types it supports, so we don't bother trying unsupported ones. Regards, Simon
