Re: [PATCH v3 06/12] reset: Extract __reset_control_get_from_provider()

[Philipp Zabel <p.zabel@pengutronix.de>]

On Mi, 2026-09-02 at 13:29 +0200, Geert Uytterhoeven wrote:
> Extract the code to create a reset_control structure from a given
> provider into its own function, so it can be reused later.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v3:
>   - New.
> ---
>  drivers/reset/core.c | 92 ++++++++++++++++++++++++--------------------
>  1 file changed, 50 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index 38e189d04d09b270..8af7cd2e8a5029df 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -1135,6 +1135,54 @@ __reset_find_rcdev(const struct fwnode_reference_args *args, bool gpio_fallback)
>  	return NULL;
>  }
>  
> +static struct reset_control *
> +__reset_control_get_from_provider(const struct fwnode_reference_args *args,
> +				  struct fwnode_handle *consumer, int index,
> +				  bool gpio_fallback,
> +				  enum reset_control_flags flags)
> +{
> +	struct reset_control *rstc = ERR_PTR(-EINVAL);
> +	struct reset_controller_dev *rcdev;
> +	int rstc_id = -EINVAL;
> +
> +	guard(mutex)(&reset_list_mutex);
> +
> +	rcdev = __reset_find_rcdev(args, gpio_fallback);
> +	if (!rcdev)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	if (WARN_ON(args->nargs != rcdev->fwnode_reset_n_cells))
> +		return ERR_PTR(-EINVAL);
> +
> +	if (rcdev->of_xlate && is_of_node(consumer)) {
> +		struct device_node *np = to_of_node(consumer);
> +		struct of_phandle_args of_args;
> +		int ret;
> +
> +		ret = of_parse_phandle_with_args(np,
> +					 gpio_fallback ? "reset-gpios" : "resets",
> +					 gpio_fallback ? "#gpio-cells" : "#reset-cells",
> +					 gpio_fallback ? 0 : index,
> +					 &of_args);
> +		if (ret)
> +			return ERR_PTR(ret);
> +
> +		rstc_id = rcdev->of_xlate(rcdev, &of_args);
> +		of_node_put(of_args.np);
> +	} else if (rcdev->fwnode_xlate) {
> +		rstc_id = rcdev->fwnode_xlate(rcdev, args);
> +	}
> +	if (rstc_id < 0)
> +		return ERR_PTR(rstc_id);
> +
> +	flags &= ~RESET_CONTROL_FLAGS_BIT_OPTIONAL;

This should stay in __fwnode_reset_control_get().

> +
> +	scoped_guard(mutex, &rcdev->lock)
> +		rstc = __reset_control_get_internal(rcdev, rstc_id, flags);
> +
> +	return rstc;

Just return __reset_control_get_internal(...) directly, the rstc local
variable is not needed.

> +}
> +
>  struct reset_control *
>  __fwnode_reset_control_get(struct fwnode_handle *fwnode, const char *id, int index,
>  			   enum reset_control_flags flags)
> @@ -1142,10 +1190,7 @@ __fwnode_reset_control_get(struct fwnode_handle *fwnode, const char *id, int ind
>  	bool optional = flags & RESET_CONTROL_FLAGS_BIT_OPTIONAL;
>  	bool gpio_fallback = false;
>  	struct reset_control *rstc = ERR_PTR(-EINVAL);
> -	struct reset_controller_dev *rcdev;
>  	struct fwnode_reference_args args;
> -	struct of_phandle_args of_args;
> -	int rstc_id = -EINVAL;
>  	int ret;
>  
>  	if (!fwnode)
> @@ -1185,46 +1230,9 @@ __fwnode_reset_control_get(struct fwnode_handle *fwnode, const char *id, int ind
>  		}
>  	}
>  
> -	guard(mutex)(&reset_list_mutex);
> -
> -	rcdev = __reset_find_rcdev(&args, gpio_fallback);
> -	if (!rcdev) {
> -		rstc = ERR_PTR(-EPROBE_DEFER);
> -		goto out_put;
> -	}
> -
> -	if (WARN_ON(args.nargs != rcdev->fwnode_reset_n_cells)) {
> -		rstc = ERR_PTR(-EINVAL);
> -		goto out_put;
> -	}
> -
> -	if (rcdev->of_xlate && is_of_node(fwnode)) {
> -		ret = of_parse_phandle_with_args(to_of_node(fwnode),
> -					 gpio_fallback ? "reset-gpios" : "resets",
> -					 gpio_fallback ? "#gpio-cells" : "#reset-cells",
> -					 gpio_fallback ? 0 : index,

Hmm, I think this gpio_fallback disambiguation is not needed. In the
gpio_fallback case (lookup via "reset-gpios") we should always get a
reset_gpio rcdev with fwnode_xlate, not of_xlate.
Not an issue for this refactor, though.

> -					 &of_args);
> -		if (ret) {
> -			rstc = ERR_PTR(ret);
> -			goto out_put;
> -		}
> -
> -		rstc_id = rcdev->of_xlate(rcdev, &of_args);
> -		of_node_put(of_args.np);
> -	} else if (rcdev->fwnode_xlate) {
> -		rstc_id = rcdev->fwnode_xlate(rcdev, &args);
> -	}
> -	if (rstc_id < 0) {
> -		rstc = ERR_PTR(rstc_id);
> -		goto out_put;
> -	}
> -
> -	flags &= ~RESET_CONTROL_FLAGS_BIT_OPTIONAL;

Keep this here.

> -
> -	scoped_guard(mutex, &rcdev->lock)
> -		rstc = __reset_control_get_internal(rcdev, rstc_id, flags);
> +	rstc = __reset_control_get_from_provider(&args, fwnode, index,
> +						 gpio_fallback, flags);
>  
> -out_put:
>  	fwnode_handle_put(args.fwnode);
>  
>  	return rstc;

With that,

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp