[PATCH] drivers: sysreset: Do not end sysreset_walk_arg on -EPROTONOSUPPORT

[Quentin Schulz]

Hi Varada,

Thanks for the patch!

On 7/3/26 9:24 AM, Varadarajan Narayanan wrote:
> If there are multiple sysreset devices implementing request_arg callback,
> the first sysreset device will consume the args and may return
> EPROTONOSUPPORT if it doesn't implement the given argument. This will stop
> the loop.
> 
> Since -EPROTONOSUPPORT is used to indicate absence of support for that
> argument, subsequent drivers should be given a chance to see if they handle
> it. Hence do not terminate the loop on -EPROTONOSUPPORT return code.
> 
> Fixes: fcb48b89813b ("drivers: sysreset: Add sysreset op that can take arguments")
> 
> Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan at oss.qualcomm.com>
> ---
>   drivers/sysreset/sysreset-uclass.c | 23 +++++++++++++----------
>   1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/sysreset/sysreset-uclass.c b/drivers/sysreset/sysreset-uclass.c
> index f25e09e9cd0..0fc096e7f0f 100644
> --- a/drivers/sysreset/sysreset-uclass.c
> +++ b/drivers/sysreset/sysreset-uclass.c
> @@ -89,14 +89,12 @@ int sysreset_walk_arg(int argc, char * const argv[])
>   	struct udevice *dev;
>   	int ret = -ENOSYS;
>   
> -	while (ret != -EINPROGRESS && ret != -EPROTONOSUPPORT) {
> -		for (uclass_first_device(UCLASS_SYSRESET, &dev);
> -		     dev;
> -		     uclass_next_device(&dev)) {
> -			ret = sysreset_request_arg(dev, argc, argv);
> -			if (ret == -EINPROGRESS || ret == -EPROTONOSUPPORT)
> -				break;
> -		}
> +	for (uclass_first_device(UCLASS_SYSRESET, &dev);
> +	     dev;
> +	     uclass_next_device(&dev)) {
> +		ret = sysreset_request_arg(dev, argc, argv);
> +		if (ret == -EINPROGRESS)
> +			break;
>   	}
>   
>   	return ret;
> @@ -153,6 +151,7 @@ void reset_cpu(void)
>   int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>   {
>   	enum sysreset_t reset_type = SYSRESET_COLD;
> +	int ret;
>   
>   	if (argc > 2)
>   		return CMD_RET_USAGE;
> @@ -165,8 +164,12 @@ int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>   	mdelay(100);
>   
>   #if IS_ENABLED(CONFIG_SYSRESET_CMD_RESET_ARGS)
> -	if (argc > 1 && sysreset_walk_arg(argc, argv) == -EINPROGRESS)
> -		return 0;
> +	if (argc > 1) {
> +		ret = sysreset_walk_arg(argc, argv);
> +		if (ret == -EINPROGRESS)
> +			return 0;
> +		log_err("No handler for reset command arguments (%d)\n", ret);

Just use printf to be consistent with the rest of the function.

But here's possibly another logic bug I think. If we pass -w to reset, 
this will try all available sysreset devices if any can handle the -w 
argument and then print that there's no handler for the reset command 
argument, which is to be expected.

So I'm wondering if we shouldn't bypass sysreset_walk_arg() entirely 
when -w is given as argument to the reset command.

Also, *any* argument starting with -w should do the warm reset, e.g. 
reset -warm should do it too as that's the current logic we have (we 
only check the first two characters of the argument).

Cheers,
Quentin