Hi Quentin, On Tue, 28 Jul 2026 at 05:43, Quentin Schulz <[email protected]> wrote: > > > > On 7/28/26 1:38 PM, Simon Glass wrote: > > 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. > > > > True, that's also misleading. We could flush() also and then hopefully > the message really will be printed.
Yes, perhaps as a Kconfig option though, since some platforms reset due to a fatal error, etc. Also it couldbe a follow-up, not blocking this series. > > > 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. > > > > We're iterating over all sysreset drivers in increasing power reset > levels until we find one that works and do that reset, so filtering for > supported levels by drivers... I'm not sure what that would help with > here? What am I missing? Hmm yes if the driver returns -ENOSYS then we know that type is not supported, so it doesn't add much. Regards, Simon
