Re: [PATCH v4 3/7] reset: Print reset type on diagnostic console

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi Simon,

On 6/27/26 10:30 AM, Simon Glass wrote:
> Hi Quentin,
> 
> On Fri, 26 Jun 2026 at 16:01, Quentin Schulz <quentin.schulz@cherry.de> wrote:
>>
>> Hi Denis,
>>
>> On 6/3/26 9:07 AM, dmukhin@ford.com wrote:
>>> Add a diagnostic console trace indicating the reset type.
>>>
>>> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
>>> ---
>>> 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.

Cheers,
Quentin