Re: [PATCH 02/14] iio: adc: rzg2l_adc: Use devres helpers to request pre-deasserted reset controls

[Jonathan Cameron <jic23@kernel.org>]

On Tue,  3 Dec 2024 13:13:02 +0200
Claudiu <claudiu.beznea@tuxon.dev> wrote:

> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Starting with commit d872bed85036 ("reset: Add devres helpers to request
> pre-deasserted reset controls"), devres helpers are available to simplify
> the process of requesting pre-deasserted reset controls. Update the
> rzg2l_adc driver to utilize these helpers, reducing complexity in this
> way.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Hi Claudia,

Minor comments below.

Thanks

Jonathan

> ---
>  drivers/iio/adc/rzg2l_adc.c | 37 ++-----------------------------------
>  1 file changed, 2 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
> index cd3a7e46ea53..7039949a7554 100644
> --- a/drivers/iio/adc/rzg2l_adc.c
> +++ b/drivers/iio/adc/rzg2l_adc.c
> @@ -415,11 +415,6 @@ static void rzg2l_adc_pm_runtime_set_suspended(void *data)
>  	pm_runtime_set_suspended(dev->parent);
>  }
>  
> -static void rzg2l_adc_reset_assert(void *data)
> -{
> -	reset_control_assert(data);
> -}
> -
>  static int rzg2l_adc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -456,46 +451,18 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
>  		return PTR_ERR(adc->adclk);
>  	}
>  
> -	adc->adrstn = devm_reset_control_get_exclusive(dev, "adrst-n");
> +	adc->adrstn = devm_reset_control_get_exclusive_deasserted(dev, "adrst-n");
>  	if (IS_ERR(adc->adrstn)) {
>  		dev_err(dev, "failed to get adrstn\n");
I'd be tempted to keep the error message from below rather than this one.
As we can only conclude the deassert failed to happen, not whether it was the
get step or the deassert itself.

Also, use dev_err_probe() throughout and definitely for anything you are touching
in this series. Ideally add a patch converting all the other places where it
is useful in things only called from probe.
	return dev_err_probe(dev, PTR_ERR(adc->adrstn),
			     "failed to deassert adrstn pin\n");

>  		return PTR_ERR(adc->adrstn);
>  	}
>  
> -	adc->presetn = devm_reset_control_get_exclusive(dev, "presetn");
> +	adc->presetn = devm_reset_control_get_exclusive_deasserted(dev, "presetn");
>  	if (IS_ERR(adc->presetn)) {
>  		dev_err(dev, "failed to get presetn\n");
>  		return PTR_ERR(adc->presetn);
>  	}
>  
> -	ret = reset_control_deassert(adc->adrstn);
> -	if (ret) {
> -		dev_err(&pdev->dev, "failed to deassert adrstn pin, %d\n", ret);
> -		return ret;
> -	}
> -
> -	ret = devm_add_action_or_reset(&pdev->dev,
> -				       rzg2l_adc_reset_assert, adc->adrstn);
> -	if (ret) {
> -		dev_err(&pdev->dev, "failed to register adrstn assert devm action, %d\n",
> -			ret);
> -		return ret;
> -	}
> -
> -	ret = reset_control_deassert(adc->presetn);
> -	if (ret) {
> -		dev_err(&pdev->dev, "failed to deassert presetn pin, %d\n", ret);
> -		return ret;
> -	}
> -
> -	ret = devm_add_action_or_reset(&pdev->dev,
> -				       rzg2l_adc_reset_assert, adc->presetn);
> -	if (ret) {
> -		dev_err(&pdev->dev, "failed to register presetn assert devm action, %d\n",
> -			ret);
> -		return ret;
> -	}
> -
>  	ret = rzg2l_adc_hw_init(adc);
>  	if (ret) {
>  		dev_err(&pdev->dev, "failed to initialize ADC HW, %d\n", ret);