[PATCH -next 0/4] iio: Use devm_clk_get_enabled() helper function

[PATCH -next 0/4] iio: Use devm_clk_get_enabled() helper function

[Jinjie Ruan]

Commit 7ef9651e9792 ("clk: Provide new devm_clk helpers for prepared
and enabled clocks") provides a new helper function to prepare and
enable a clock which keeps prepared (or enabled) during the whole
lifetime of the driver.

Jinjie Ruan (4):
  iio: adc: at91_adc: Use devm_clk_get_enabled() helper function
  iio: adc: mt6577_auxadc: Use devm_clk_get_enabled() helper function
  iio: adc: spear_adc: Use devm_clk_get_enabled() helper function
  iio: frequency: adf4350: Use devm_clk_get_enabled() helper function

 drivers/iio/adc/at91_adc.c      | 38 +++++++++------------------------
 drivers/iio/adc/mt6577_auxadc.c | 17 +++------------
 drivers/iio/adc/spear_adc.c     | 30 ++++++--------------------
 drivers/iio/frequency/adf4350.c | 20 +++++------------
 4 files changed, 25 insertions(+), 80 deletions(-)

-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH -next 1/4] iio: adc: at91_adc: Use devm_clk_get_enabled() helper function

[Jinjie Ruan]

The devm_clk_get_enabled() helper:
    - calls devm_clk_get()
    - calls clk_prepare_enable() and registers what is needed in order to
      call clk_disable_unprepare() when needed, as a managed resource.

This simplifies the code.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 drivers/iio/adc/at91_adc.c | 38 ++++++++++----------------------------
 1 file changed, 10 insertions(+), 28 deletions(-)

diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index de6650f9c4b1..318e33ce22fb 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -1087,32 +1087,20 @@ static int at91_adc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	st->clk = devm_clk_get(&pdev->dev, "adc_clk");
+	st->clk = devm_clk_get_enabled(&pdev->dev, "adc_clk");
 	if (IS_ERR(st->clk)) {
-		dev_err(&pdev->dev, "Failed to get the clock.\n");
-		ret = PTR_ERR(st->clk);
-		goto error_free_irq;
-	}
-
-	ret = clk_prepare_enable(st->clk);
-	if (ret) {
 		dev_err(&pdev->dev,
 			"Could not prepare or enable the clock.\n");
+		ret = PTR_ERR(st->clk);
 		goto error_free_irq;
 	}
 
-	st->adc_clk = devm_clk_get(&pdev->dev, "adc_op_clk");
+	st->adc_clk = devm_clk_get_enabled(&pdev->dev, "adc_op_clk");
 	if (IS_ERR(st->adc_clk)) {
-		dev_err(&pdev->dev, "Failed to get the ADC clock.\n");
-		ret = PTR_ERR(st->adc_clk);
-		goto error_disable_clk;
-	}
-
-	ret = clk_prepare_enable(st->adc_clk);
-	if (ret) {
 		dev_err(&pdev->dev,
 			"Could not prepare or enable the ADC clock.\n");
-		goto error_disable_clk;
+		ret = PTR_ERR(st->adc_clk);
+		goto error_free_irq;
 	}
 
 	/*
@@ -1132,7 +1120,7 @@ static int at91_adc_probe(struct platform_device *pdev)
 	if (!st->startup_time) {
 		dev_err(&pdev->dev, "No startup time available.\n");
 		ret = -EINVAL;
-		goto error_disable_adc_clk;
+		goto error_free_irq;
 	}
 	ticks = (*st->caps->calc_startup_ticks)(st->startup_time, adc_clk_khz);
 
@@ -1160,7 +1148,7 @@ static int at91_adc_probe(struct platform_device *pdev)
 	ret = at91_adc_channel_init(idev);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Couldn't initialize the channels.\n");
-		goto error_disable_adc_clk;
+		goto error_free_irq;
 	}
 
 	init_waitqueue_head(&st->wq_data_avail);
@@ -1175,19 +1163,19 @@ static int at91_adc_probe(struct platform_device *pdev)
 		ret = at91_adc_buffer_init(idev);
 		if (ret < 0) {
 			dev_err(&pdev->dev, "Couldn't initialize the buffer.\n");
-			goto error_disable_adc_clk;
+			goto error_free_irq;
 		}
 
 		ret = at91_adc_trigger_init(idev);
 		if (ret < 0) {
 			dev_err(&pdev->dev, "Couldn't setup the triggers.\n");
 			at91_adc_buffer_remove(idev);
-			goto error_disable_adc_clk;
+			goto error_free_irq;
 		}
 	} else {
 		ret = at91_ts_register(idev, pdev);
 		if (ret)
-			goto error_disable_adc_clk;
+			goto error_free_irq;
 
 		at91_ts_hw_init(idev, adc_clk_khz);
 	}
@@ -1207,10 +1195,6 @@ static int at91_adc_probe(struct platform_device *pdev)
 	} else {
 		at91_ts_unregister(st);
 	}
-error_disable_adc_clk:
-	clk_disable_unprepare(st->adc_clk);
-error_disable_clk:
-	clk_disable_unprepare(st->clk);
 error_free_irq:
 	free_irq(st->irq, idev);
 	return ret;
@@ -1228,8 +1212,6 @@ static int at91_adc_remove(struct platform_device *pdev)
 	} else {
 		at91_ts_unregister(st);
 	}
-	clk_disable_unprepare(st->adc_clk);
-	clk_disable_unprepare(st->clk);
 	free_irq(st->irq, idev);
 
 	return 0;
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH -next 1/4] iio: adc: at91_adc: Use devm_clk_get_enabled() helper function

[Jonathan Cameron]

On Fri, 25 Aug 2023 18:57:43 +0800
Jinjie Ruan <ruanjinjie@huawei.com> wrote:

> The devm_clk_get_enabled() helper:
>     - calls devm_clk_get()
>     - calls clk_prepare_enable() and registers what is needed in order to
>       call clk_disable_unprepare() when needed, as a managed resource.
> 
> This simplifies the code.
> 
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>

This looks good, but should ideally be proceeded by a separate patch moving to
devm_request_irq() as then the exit paths will become much simpler and you
can also move to return dev_err_probe() save a few pointless lines of code
and give the benefit of debug info for deferred return codes.




> ---
>  drivers/iio/adc/at91_adc.c | 38 ++++++++++----------------------------
>  1 file changed, 10 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index de6650f9c4b1..318e33ce22fb 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -1087,32 +1087,20 @@ static int at91_adc_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	st->clk = devm_clk_get(&pdev->dev, "adc_clk");
> +	st->clk = devm_clk_get_enabled(&pdev->dev, "adc_clk");
>  	if (IS_ERR(st->clk)) {
> -		dev_err(&pdev->dev, "Failed to get the clock.\n");
> -		ret = PTR_ERR(st->clk);
> -		goto error_free_irq;
> -	}
> -
> -	ret = clk_prepare_enable(st->clk);
> -	if (ret) {
>  		dev_err(&pdev->dev,
>  			"Could not prepare or enable the clock.\n");
> +		ret = PTR_ERR(st->clk);
>  		goto error_free_irq;
>  	}
>  
> -	st->adc_clk = devm_clk_get(&pdev->dev, "adc_op_clk");
> +	st->adc_clk = devm_clk_get_enabled(&pdev->dev, "adc_op_clk");
>  	if (IS_ERR(st->adc_clk)) {
> -		dev_err(&pdev->dev, "Failed to get the ADC clock.\n");
> -		ret = PTR_ERR(st->adc_clk);
> -		goto error_disable_clk;
> -	}
> -
> -	ret = clk_prepare_enable(st->adc_clk);
> -	if (ret) {
>  		dev_err(&pdev->dev,
>  			"Could not prepare or enable the ADC clock.\n");
> -		goto error_disable_clk;
> +		ret = PTR_ERR(st->adc_clk);
> +		goto error_free_irq;
>  	}
>  
>  	/*
> @@ -1132,7 +1120,7 @@ static int at91_adc_probe(struct platform_device *pdev)
>  	if (!st->startup_time) {
>  		dev_err(&pdev->dev, "No startup time available.\n");
>  		ret = -EINVAL;
> -		goto error_disable_adc_clk;
> +		goto error_free_irq;
>  	}
>  	ticks = (*st->caps->calc_startup_ticks)(st->startup_time, adc_clk_khz);
>  
> @@ -1160,7 +1148,7 @@ static int at91_adc_probe(struct platform_device *pdev)
>  	ret = at91_adc_channel_init(idev);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "Couldn't initialize the channels.\n");
> -		goto error_disable_adc_clk;
> +		goto error_free_irq;
>  	}
>  
>  	init_waitqueue_head(&st->wq_data_avail);
> @@ -1175,19 +1163,19 @@ static int at91_adc_probe(struct platform_device *pdev)
>  		ret = at91_adc_buffer_init(idev);
>  		if (ret < 0) {
>  			dev_err(&pdev->dev, "Couldn't initialize the buffer.\n");
> -			goto error_disable_adc_clk;
> +			goto error_free_irq;
>  		}
>  
>  		ret = at91_adc_trigger_init(idev);
>  		if (ret < 0) {
>  			dev_err(&pdev->dev, "Couldn't setup the triggers.\n");
>  			at91_adc_buffer_remove(idev);
> -			goto error_disable_adc_clk;
> +			goto error_free_irq;
>  		}
>  	} else {
>  		ret = at91_ts_register(idev, pdev);
>  		if (ret)
> -			goto error_disable_adc_clk;
> +			goto error_free_irq;
>  
>  		at91_ts_hw_init(idev, adc_clk_khz);
>  	}
> @@ -1207,10 +1195,6 @@ static int at91_adc_probe(struct platform_device *pdev)
>  	} else {
>  		at91_ts_unregister(st);
>  	}
> -error_disable_adc_clk:
> -	clk_disable_unprepare(st->adc_clk);
> -error_disable_clk:
> -	clk_disable_unprepare(st->clk);
>  error_free_irq:
>  	free_irq(st->irq, idev);
>  	return ret;
> @@ -1228,8 +1212,6 @@ static int at91_adc_remove(struct platform_device *pdev)
>  	} else {
>  		at91_ts_unregister(st);
>  	}
> -	clk_disable_unprepare(st->adc_clk);
> -	clk_disable_unprepare(st->clk);
>  	free_irq(st->irq, idev);
>  
>  	return 0;


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH -next 1/4] iio: adc: at91_adc: Use devm_clk_get_enabled() helper function

[Ruan Jinjie]

On 2023/8/25 20:50, Jonathan Cameron wrote:
> te patch moving to
> devm_request_irq() as then the exit paths will become much simpler and you
> can also move to return dev_err_probe() save a few pointless lines of code
> and give the benefit of debug info for deferred return codes.

Right！I'll cleanup these together in v2.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH -next 2/4] iio: adc: mt6577_auxadc: Use devm_clk_get_enabled() helper function

[Jinjie Ruan]

The devm_clk_get_enabled() helper:
    - calls devm_clk_get()
    - calls clk_prepare_enable() and registers what is needed in order to
      call clk_disable_unprepare() when needed, as a managed resource.

This simplifies the code.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 drivers/iio/adc/mt6577_auxadc.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
index 0e134777bdd2..ea42fd7a8c99 100644
--- a/drivers/iio/adc/mt6577_auxadc.c
+++ b/drivers/iio/adc/mt6577_auxadc.c
@@ -270,23 +270,16 @@ static int mt6577_auxadc_probe(struct platform_device *pdev)
 		return PTR_ERR(adc_dev->reg_base);
 	}
 
-	adc_dev->adc_clk = devm_clk_get(&pdev->dev, "main");
+	adc_dev->adc_clk = devm_clk_get_enabled(&pdev->dev, "main");
 	if (IS_ERR(adc_dev->adc_clk)) {
-		dev_err(&pdev->dev, "failed to get auxadc clock\n");
-		return PTR_ERR(adc_dev->adc_clk);
-	}
-
-	ret = clk_prepare_enable(adc_dev->adc_clk);
-	if (ret) {
 		dev_err(&pdev->dev, "failed to enable auxadc clock\n");
-		return ret;
+		return PTR_ERR(adc_dev->adc_clk);
 	}
 
 	adc_clk_rate = clk_get_rate(adc_dev->adc_clk);
 	if (!adc_clk_rate) {
-		ret = -EINVAL;
 		dev_err(&pdev->dev, "null clock rate\n");
-		goto err_disable_clk;
+		return -EINVAL;
 	}
 
 	adc_dev->dev_comp = device_get_match_data(&pdev->dev);
@@ -310,8 +303,6 @@ static int mt6577_auxadc_probe(struct platform_device *pdev)
 err_power_off:
 	mt6577_auxadc_mod_reg(adc_dev->reg_base + MT6577_AUXADC_MISC,
 			      0, MT6577_AUXADC_PDN_EN);
-err_disable_clk:
-	clk_disable_unprepare(adc_dev->adc_clk);
 	return ret;
 }
 
@@ -325,8 +316,6 @@ static int mt6577_auxadc_remove(struct platform_device *pdev)
 	mt6577_auxadc_mod_reg(adc_dev->reg_base + MT6577_AUXADC_MISC,
 			      0, MT6577_AUXADC_PDN_EN);
 
-	clk_disable_unprepare(adc_dev->adc_clk);
-
 	return 0;
 }
 
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH -next 2/4] iio: adc: mt6577_auxadc: Use devm_clk_get_enabled() helper function

[Jonathan Cameron]

On Fri, 25 Aug 2023 18:57:44 +0800
Jinjie Ruan <ruanjinjie@huawei.com> wrote:

> The devm_clk_get_enabled() helper:
>     - calls devm_clk_get()
>     - calls clk_prepare_enable() and registers what is needed in order to
>       call clk_disable_unprepare() when needed, as a managed resource.
> 
> This simplifies the code.
> 
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>

Applied.  This is another case that would benefit from use of dev_err_probe()
for all error messages in the probe routine.  Should also be easy to convert
to fully devm managed with one devm_add_action_or_reset() and appropriate
callback.

Jonathan

> ---
>  drivers/iio/adc/mt6577_auxadc.c | 17 +++--------------
>  1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
> index 0e134777bdd2..ea42fd7a8c99 100644
> --- a/drivers/iio/adc/mt6577_auxadc.c
> +++ b/drivers/iio/adc/mt6577_auxadc.c
> @@ -270,23 +270,16 @@ static int mt6577_auxadc_probe(struct platform_device *pdev)
>  		return PTR_ERR(adc_dev->reg_base);
>  	}
>  
> -	adc_dev->adc_clk = devm_clk_get(&pdev->dev, "main");
> +	adc_dev->adc_clk = devm_clk_get_enabled(&pdev->dev, "main");
>  	if (IS_ERR(adc_dev->adc_clk)) {
> -		dev_err(&pdev->dev, "failed to get auxadc clock\n");
> -		return PTR_ERR(adc_dev->adc_clk);
> -	}
> -
> -	ret = clk_prepare_enable(adc_dev->adc_clk);
> -	if (ret) {
>  		dev_err(&pdev->dev, "failed to enable auxadc clock\n");
> -		return ret;
> +		return PTR_ERR(adc_dev->adc_clk);
>  	}
>  
>  	adc_clk_rate = clk_get_rate(adc_dev->adc_clk);
>  	if (!adc_clk_rate) {
> -		ret = -EINVAL;
>  		dev_err(&pdev->dev, "null clock rate\n");
> -		goto err_disable_clk;
> +		return -EINVAL;
>  	}
>  
>  	adc_dev->dev_comp = device_get_match_data(&pdev->dev);
> @@ -310,8 +303,6 @@ static int mt6577_auxadc_probe(struct platform_device *pdev)
>  err_power_off:
>  	mt6577_auxadc_mod_reg(adc_dev->reg_base + MT6577_AUXADC_MISC,
>  			      0, MT6577_AUXADC_PDN_EN);
> -err_disable_clk:
> -	clk_disable_unprepare(adc_dev->adc_clk);
>  	return ret;
>  }
>  
> @@ -325,8 +316,6 @@ static int mt6577_auxadc_remove(struct platform_device *pdev)
>  	mt6577_auxadc_mod_reg(adc_dev->reg_base + MT6577_AUXADC_MISC,
>  			      0, MT6577_AUXADC_PDN_EN);
>  
> -	clk_disable_unprepare(adc_dev->adc_clk);
> -
>  	return 0;
>  }
>  


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH -next 2/4] iio: adc: mt6577_auxadc: Use devm_clk_get_enabled() helper function

[Ruan Jinjie]

On 2023/8/25 20:54, Jonathan Cameron wrote:
> On Fri, 25 Aug 2023 18:57:44 +0800
> Jinjie Ruan <ruanjinjie@huawei.com> wrote:
> 
>> The devm_clk_get_enabled() helper:
>>     - calls devm_clk_get()
>>     - calls clk_prepare_enable() and registers what is needed in order to
>>       call clk_disable_unprepare() when needed, as a managed resource.
>>
>> This simplifies the code.
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> 
> Applied.  This is another case that would benefit from use of dev_err_probe()
> for all error messages in the probe routine.  Should also be easy to convert
> to fully devm managed with one devm_add_action_or_reset() and appropriate
> callback.

Right! Use dev_err_probe() to simplify it is a good idea.

> 
> Jonathan
> 
>> ---
>>  drivers/iio/adc/mt6577_auxadc.c | 17 +++--------------
>>  1 file changed, 3 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
>> index 0e134777bdd2..ea42fd7a8c99 100644
>> --- a/drivers/iio/adc/mt6577_auxadc.c
>> +++ b/drivers/iio/adc/mt6577_auxadc.c
>> @@ -270,23 +270,16 @@ static int mt6577_auxadc_probe(struct platform_device *pdev)
>>  		return PTR_ERR(adc_dev->reg_base);
>>  	}
>>  
>> -	adc_dev->adc_clk = devm_clk_get(&pdev->dev, "main");
>> +	adc_dev->adc_clk = devm_clk_get_enabled(&pdev->dev, "main");
>>  	if (IS_ERR(adc_dev->adc_clk)) {
>> -		dev_err(&pdev->dev, "failed to get auxadc clock\n");
>> -		return PTR_ERR(adc_dev->adc_clk);
>> -	}
>> -
>> -	ret = clk_prepare_enable(adc_dev->adc_clk);
>> -	if (ret) {
>>  		dev_err(&pdev->dev, "failed to enable auxadc clock\n");
>> -		return ret;
>> +		return PTR_ERR(adc_dev->adc_clk);
>>  	}
>>  
>>  	adc_clk_rate = clk_get_rate(adc_dev->adc_clk);
>>  	if (!adc_clk_rate) {
>> -		ret = -EINVAL;
>>  		dev_err(&pdev->dev, "null clock rate\n");
>> -		goto err_disable_clk;
>> +		return -EINVAL;
>>  	}
>>  
>>  	adc_dev->dev_comp = device_get_match_data(&pdev->dev);
>> @@ -310,8 +303,6 @@ static int mt6577_auxadc_probe(struct platform_device *pdev)
>>  err_power_off:
>>  	mt6577_auxadc_mod_reg(adc_dev->reg_base + MT6577_AUXADC_MISC,
>>  			      0, MT6577_AUXADC_PDN_EN);
>> -err_disable_clk:
>> -	clk_disable_unprepare(adc_dev->adc_clk);
>>  	return ret;
>>  }
>>  
>> @@ -325,8 +316,6 @@ static int mt6577_auxadc_remove(struct platform_device *pdev)
>>  	mt6577_auxadc_mod_reg(adc_dev->reg_base + MT6577_AUXADC_MISC,
>>  			      0, MT6577_AUXADC_PDN_EN);
>>  
>> -	clk_disable_unprepare(adc_dev->adc_clk);
>> -
>>  	return 0;
>>  }
>>  
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH -next 3/4] iio: adc: spear_adc: Use devm_clk_get_enabled() helper function

[Jinjie Ruan]

The devm_clk_get_enabled() helper:
    - calls devm_clk_get()
    - calls clk_prepare_enable() and registers what is needed in order to
      call clk_disable_unprepare() when needed, as a managed resource.

This simplifies the code.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 drivers/iio/adc/spear_adc.c | 30 +++++++-----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/drivers/iio/adc/spear_adc.c b/drivers/iio/adc/spear_adc.c
index ad54ef798109..cd7708aa95af 100644
--- a/drivers/iio/adc/spear_adc.c
+++ b/drivers/iio/adc/spear_adc.c
@@ -297,36 +297,27 @@ static int spear_adc_probe(struct platform_device *pdev)
 	st->adc_base_spear3xx =
 		(struct adc_regs_spear3xx __iomem *)st->adc_base_spear6xx;
 
-	st->clk = devm_clk_get(dev, NULL);
+	st->clk = devm_clk_get_enabled(dev, NULL);
 	if (IS_ERR(st->clk)) {
-		dev_err(dev, "failed getting clock\n");
-		return PTR_ERR(st->clk);
-	}
-
-	ret = clk_prepare_enable(st->clk);
-	if (ret) {
 		dev_err(dev, "failed enabling clock\n");
-		return ret;
+		return PTR_ERR(st->clk);
 	}
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
-		ret = irq;
-		goto errout2;
-	}
+	if (irq < 0)
+		return irq;
 
 	ret = devm_request_irq(dev, irq, spear_adc_isr, 0, SPEAR_ADC_MOD_NAME,
 			       st);
 	if (ret < 0) {
 		dev_err(dev, "failed requesting interrupt\n");
-		goto errout2;
+		return ret;
 	}
 
 	if (of_property_read_u32(np, "sampling-frequency",
 				 &st->sampling_freq)) {
 		dev_err(dev, "sampling-frequency missing in DT\n");
-		ret = -EINVAL;
-		goto errout2;
+		return -EINVAL;
 	}
 
 	/*
@@ -355,24 +346,17 @@ static int spear_adc_probe(struct platform_device *pdev)
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
-		goto errout2;
+		return ret;
 
 	dev_info(dev, "SPEAR ADC driver loaded, IRQ %d\n", irq);
 
 	return 0;
-
-errout2:
-	clk_disable_unprepare(st->clk);
-	return ret;
 }
 
 static int spear_adc_remove(struct platform_device *pdev)
 {
 	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
-	struct spear_adc_state *st = iio_priv(indio_dev);
-
 	iio_device_unregister(indio_dev);
-	clk_disable_unprepare(st->clk);
 
 	return 0;
 }
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH -next 3/4] iio: adc: spear_adc: Use devm_clk_get_enabled() helper function

[Jonathan Cameron]

On Fri, 25 Aug 2023 18:57:45 +0800
Jinjie Ruan <ruanjinjie@huawei.com> wrote:

> The devm_clk_get_enabled() helper:
>     - calls devm_clk_get()
>     - calls clk_prepare_enable() and registers what is needed in order to
>       call clk_disable_unprepare() when needed, as a managed resource.
> 
> This simplifies the code.
> 
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Also switch to devm_iio_device_register() and drop the remove function.

Jonathan

> ---
>  drivers/iio/adc/spear_adc.c | 30 +++++++-----------------------
>  1 file changed, 7 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/iio/adc/spear_adc.c b/drivers/iio/adc/spear_adc.c
> index ad54ef798109..cd7708aa95af 100644
> --- a/drivers/iio/adc/spear_adc.c
> +++ b/drivers/iio/adc/spear_adc.c
> @@ -297,36 +297,27 @@ static int spear_adc_probe(struct platform_device *pdev)
>  	st->adc_base_spear3xx =
>  		(struct adc_regs_spear3xx __iomem *)st->adc_base_spear6xx;
>  
> -	st->clk = devm_clk_get(dev, NULL);
> +	st->clk = devm_clk_get_enabled(dev, NULL);
>  	if (IS_ERR(st->clk)) {
> -		dev_err(dev, "failed getting clock\n");
> -		return PTR_ERR(st->clk);
> -	}
> -
> -	ret = clk_prepare_enable(st->clk);
> -	if (ret) {
>  		dev_err(dev, "failed enabling clock\n");
> -		return ret;
> +		return PTR_ERR(st->clk);
>  	}
>  
>  	irq = platform_get_irq(pdev, 0);
> -	if (irq < 0) {
> -		ret = irq;
> -		goto errout2;
> -	}
> +	if (irq < 0)
> +		return irq;
>  
>  	ret = devm_request_irq(dev, irq, spear_adc_isr, 0, SPEAR_ADC_MOD_NAME,
>  			       st);
>  	if (ret < 0) {
>  		dev_err(dev, "failed requesting interrupt\n");
> -		goto errout2;
> +		return ret;
>  	}
>  
>  	if (of_property_read_u32(np, "sampling-frequency",
>  				 &st->sampling_freq)) {
>  		dev_err(dev, "sampling-frequency missing in DT\n");
> -		ret = -EINVAL;
> -		goto errout2;
> +		return -EINVAL;
>  	}
>  
>  	/*
> @@ -355,24 +346,17 @@ static int spear_adc_probe(struct platform_device *pdev)
>  
>  	ret = iio_device_register(indio_dev);
>  	if (ret)
> -		goto errout2;
> +		return ret;
>  
>  	dev_info(dev, "SPEAR ADC driver loaded, IRQ %d\n", irq);
>  
>  	return 0;
> -
> -errout2:
> -	clk_disable_unprepare(st->clk);
> -	return ret;
>  }
>  
>  static int spear_adc_remove(struct platform_device *pdev)
>  {
>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> -	struct spear_adc_state *st = iio_priv(indio_dev);
> -
>  	iio_device_unregister(indio_dev);
> -	clk_disable_unprepare(st->clk);
>  
>  	return 0;
>  }


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH -next 4/4] iio: frequency: adf4350: Use devm_clk_get_enabled() helper function

[Jinjie Ruan]

The devm_clk_get_enabled() helper:
    - calls devm_clk_get()
    - calls clk_prepare_enable() and registers what is needed in order to
      call clk_disable_unprepare() when needed, as a managed resource.

This simplifies the code.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 drivers/iio/frequency/adf4350.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c
index 85e289700c3c..22330d1768ff 100644
--- a/drivers/iio/frequency/adf4350.c
+++ b/drivers/iio/frequency/adf4350.c
@@ -491,20 +491,14 @@ static int adf4350_probe(struct spi_device *spi)
 	}
 
 	if (!pdata->clkin) {
-		clk = devm_clk_get(&spi->dev, "clkin");
+		clk = devm_clk_get_enabled(&spi->dev, "clkin");
 		if (IS_ERR(clk))
-			return -EPROBE_DEFER;
-
-		ret = clk_prepare_enable(clk);
-		if (ret < 0)
-			return ret;
+			return PTR_ERR(clk);
 	}
 
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
-	if (indio_dev == NULL) {
-		ret =  -ENOMEM;
-		goto error_disable_clk;
-	}
+	if (indio_dev == NULL)
+		return -ENOMEM;
 
 	st = iio_priv(indio_dev);
 
@@ -512,7 +506,7 @@ static int adf4350_probe(struct spi_device *spi)
 	if (!IS_ERR(st->reg)) {
 		ret = regulator_enable(st->reg);
 		if (ret)
-			goto error_disable_clk;
+			return ret;
 	}
 
 	spi_set_drvdata(spi, indio_dev);
@@ -564,8 +558,6 @@ static int adf4350_probe(struct spi_device *spi)
 error_disable_reg:
 	if (!IS_ERR(st->reg))
 		regulator_disable(st->reg);
-error_disable_clk:
-	clk_disable_unprepare(clk);
 
 	return ret;
 }
@@ -581,8 +573,6 @@ static void adf4350_remove(struct spi_device *spi)
 
 	iio_device_unregister(indio_dev);
 
-	clk_disable_unprepare(st->clk);
-
 	if (!IS_ERR(reg))
 		regulator_disable(reg);
 }
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH -next 4/4] iio: frequency: adf4350: Use devm_clk_get_enabled() helper function

[Jonathan Cameron]

On Fri, 25 Aug 2023 18:57:46 +0800
Jinjie Ruan <ruanjinjie@huawei.com> wrote:

> The devm_clk_get_enabled() helper:
>     - calls devm_clk_get()
>     - calls clk_prepare_enable() and registers what is needed in order to
>       call clk_disable_unprepare() when needed, as a managed resource.
> 
> This simplifies the code.
> 
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>

I'm fairly sure st->reg is never used except for disable. As such you can finish
the job here by moving to equivalent devm_regulator enabled call and devm_iio_device_register()
That should avoid any potential ordering issues which are a bit un obvious with just
this patch

Jonathan

> ---
>  drivers/iio/frequency/adf4350.c | 20 +++++---------------
>  1 file changed, 5 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c
> index 85e289700c3c..22330d1768ff 100644
> --- a/drivers/iio/frequency/adf4350.c
> +++ b/drivers/iio/frequency/adf4350.c
> @@ -491,20 +491,14 @@ static int adf4350_probe(struct spi_device *spi)
>  	}
>  
>  	if (!pdata->clkin) {
> -		clk = devm_clk_get(&spi->dev, "clkin");
> +		clk = devm_clk_get_enabled(&spi->dev, "clkin");
>  		if (IS_ERR(clk))
> -			return -EPROBE_DEFER;
> -
> -		ret = clk_prepare_enable(clk);
> -		if (ret < 0)
> -			return ret;
> +			return PTR_ERR(clk);
>  	}
>  
>  	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> -	if (indio_dev == NULL) {
> -		ret =  -ENOMEM;
> -		goto error_disable_clk;
> -	}
> +	if (indio_dev == NULL)
> +		return -ENOMEM;
>  
>  	st = iio_priv(indio_dev);
>  
> @@ -512,7 +506,7 @@ static int adf4350_probe(struct spi_device *spi)
>  	if (!IS_ERR(st->reg)) {
>  		ret = regulator_enable(st->reg);
>  		if (ret)
> -			goto error_disable_clk;
> +			return ret;
>  	}
>  
>  	spi_set_drvdata(spi, indio_dev);
> @@ -564,8 +558,6 @@ static int adf4350_probe(struct spi_device *spi)
>  error_disable_reg:
>  	if (!IS_ERR(st->reg))
>  		regulator_disable(st->reg);
> -error_disable_clk:
> -	clk_disable_unprepare(clk);
>  
>  	return ret;
>  }
> @@ -581,8 +573,6 @@ static void adf4350_remove(struct spi_device *spi)
>  
>  	iio_device_unregister(indio_dev);
>  
> -	clk_disable_unprepare(st->clk);
> -
>  	if (!IS_ERR(reg))
>  		regulator_disable(reg);
>  }


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel