[PATCH 0/4] iio: Few cleanups to vf610 and ti-adc12138

[PATCH 0/4] iio: Few cleanups to vf610 and ti-adc12138

[Krzysztof Kozlowski]

Just few cleanups.  Not tested on hardware.  Only the first patch could
have an observable effect.

Best regards,
Krzysztof

---
Krzysztof Kozlowski (4):
      iio: adc: ti-adc12138: Simplify with devm_clk_get_enabled()
      iio: adc: vf610: Drop -ENOMEM error message
      iio: adc: vf610: Simplify with dev_err_probe
      iio: dac: vf610: Simplify with devm_clk_get_enabled()

 drivers/iio/adc/ti-adc12138.c | 30 +++++++++++-------------------
 drivers/iio/dac/vf610_dac.c   | 23 +++++------------------
 2 files changed, 16 insertions(+), 37 deletions(-)
---
base-commit: a62b7a37e6fcf4a675b1548e7c168b96ec836442
change-id: 20250713-iio-clk-get-enabled-4764482c8ff3

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

[PATCH 1/4] iio: adc: ti-adc12138: Simplify with devm_clk_get_enabled()

[Krzysztof Kozlowski]

Driver is getting clock and almost immediately enabling it, with the
devm_request_irq() as the only relevant code executed between, thus the
probe path and cleanups can be simplified with devm_clk_get_enabled().

Move devm_request_irq() earlier, so the interrupt handler will be
registered before clock is enabled.  This might be important in case
regulator supplies are enabled by other device driver and this device
raises interrupt immediately after clock sarts ticking.

The change does not reverse cleanup paths - first regulator will be
disabled, then clock and finally interrupt handler freed.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

Not tested on hardware.
---
 drivers/iio/adc/ti-adc12138.c | 30 +++++++++++-------------------
 1 file changed, 11 insertions(+), 19 deletions(-)

diff --git a/drivers/iio/adc/ti-adc12138.c b/drivers/iio/adc/ti-adc12138.c
index 9dc465a10ffc8d9f596e34215af685999235d690..e5ec4b073daae33d0e51cf21a3520f0ab2184828 100644
--- a/drivers/iio/adc/ti-adc12138.c
+++ b/drivers/iio/adc/ti-adc12138.c
@@ -38,15 +38,13 @@ enum {
 struct adc12138 {
 	struct spi_device *spi;
 	unsigned int id;
-	/* conversion clock */
-	struct clk *cclk;
 	/* positive analog voltage reference */
 	struct regulator *vref_p;
 	/* negative analog voltage reference */
 	struct regulator *vref_n;
 	struct mutex lock;
 	struct completion complete;
-	/* The number of cclk periods for the S/H's acquisition time */
+	/* The number of conversion clock periods for the S/H's acquisition time */
 	unsigned int acquisition_time;
 	/*
 	 * Maximum size needed: 16x 2 bytes ADC data + 8 bytes timestamp.
@@ -400,6 +398,7 @@ static int adc12138_probe(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev;
 	struct adc12138 *adc;
+	struct clk *cclk;
 	int ret;
 
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
@@ -435,9 +434,14 @@ static int adc12138_probe(struct spi_device *spi)
 	if (ret)
 		adc->acquisition_time = 10;
 
-	adc->cclk = devm_clk_get(&spi->dev, NULL);
-	if (IS_ERR(adc->cclk))
-		return PTR_ERR(adc->cclk);
+	ret = devm_request_irq(&spi->dev, spi->irq, adc12138_eoc_handler,
+			       IRQF_TRIGGER_RISING, indio_dev->name, indio_dev);
+	if (ret)
+		return ret;
+
+	cclk = devm_clk_get_enabled(&spi->dev, NULL);
+	if (IS_ERR(cclk))
+		return PTR_ERR(cclk);
 
 	adc->vref_p = devm_regulator_get(&spi->dev, "vref-p");
 	if (IS_ERR(adc->vref_p))
@@ -454,18 +458,9 @@ static int adc12138_probe(struct spi_device *spi)
 			return ret;
 	}
 
-	ret = devm_request_irq(&spi->dev, spi->irq, adc12138_eoc_handler,
-			       IRQF_TRIGGER_RISING, indio_dev->name, indio_dev);
-	if (ret)
-		return ret;
-
-	ret = clk_prepare_enable(adc->cclk);
-	if (ret)
-		return ret;
-
 	ret = regulator_enable(adc->vref_p);
 	if (ret)
-		goto err_clk_disable;
+		return ret;
 
 	if (!IS_ERR(adc->vref_n)) {
 		ret = regulator_enable(adc->vref_n);
@@ -496,8 +491,6 @@ static int adc12138_probe(struct spi_device *spi)
 		regulator_disable(adc->vref_n);
 err_vref_p_disable:
 	regulator_disable(adc->vref_p);
-err_clk_disable:
-	clk_disable_unprepare(adc->cclk);
 
 	return ret;
 }
@@ -512,7 +505,6 @@ static void adc12138_remove(struct spi_device *spi)
 	if (!IS_ERR(adc->vref_n))
 		regulator_disable(adc->vref_n);
 	regulator_disable(adc->vref_p);
-	clk_disable_unprepare(adc->cclk);
 }
 
 static const struct of_device_id adc12138_dt_ids[] = {

-- 
2.43.0

Re: [PATCH 1/4] iio: adc: ti-adc12138: Simplify with devm_clk_get_enabled()

[Jonathan Cameron]

On Sun, 13 Jul 2025 17:59:55 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> Driver is getting clock and almost immediately enabling it, with the
> devm_request_irq() as the only relevant code executed between, thus the
> probe path and cleanups can be simplified with devm_clk_get_enabled().
> 
> Move devm_request_irq() earlier, so the interrupt handler will be
> registered before clock is enabled.  This might be important in case
> regulator supplies are enabled by other device driver and this device
> raises interrupt immediately after clock sarts ticking.
> 
> The change does not reverse cleanup paths - first regulator will be
> disabled, then clock and finally interrupt handler freed.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Seems no one cares (or is paying attention) about the ordering change
and we both think it is fine so applied to the testing branch of iio.git
for 6.18. I'll rebase on rc1 once available

Thanks,

Jonathan

> 
> ---
> 
> Not tested on hardware.
> ---
>  drivers/iio/adc/ti-adc12138.c | 30 +++++++++++-------------------
>  1 file changed, 11 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-adc12138.c b/drivers/iio/adc/ti-adc12138.c
> index 9dc465a10ffc8d9f596e34215af685999235d690..e5ec4b073daae33d0e51cf21a3520f0ab2184828 100644
> --- a/drivers/iio/adc/ti-adc12138.c
> +++ b/drivers/iio/adc/ti-adc12138.c
> @@ -38,15 +38,13 @@ enum {
>  struct adc12138 {
>  	struct spi_device *spi;
>  	unsigned int id;
> -	/* conversion clock */
> -	struct clk *cclk;
>  	/* positive analog voltage reference */
>  	struct regulator *vref_p;
>  	/* negative analog voltage reference */
>  	struct regulator *vref_n;
>  	struct mutex lock;
>  	struct completion complete;
> -	/* The number of cclk periods for the S/H's acquisition time */
> +	/* The number of conversion clock periods for the S/H's acquisition time */
>  	unsigned int acquisition_time;
>  	/*
>  	 * Maximum size needed: 16x 2 bytes ADC data + 8 bytes timestamp.
> @@ -400,6 +398,7 @@ static int adc12138_probe(struct spi_device *spi)
>  {
>  	struct iio_dev *indio_dev;
>  	struct adc12138 *adc;
> +	struct clk *cclk;
>  	int ret;
>  
>  	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
> @@ -435,9 +434,14 @@ static int adc12138_probe(struct spi_device *spi)
>  	if (ret)
>  		adc->acquisition_time = 10;
>  
> -	adc->cclk = devm_clk_get(&spi->dev, NULL);
> -	if (IS_ERR(adc->cclk))
> -		return PTR_ERR(adc->cclk);
> +	ret = devm_request_irq(&spi->dev, spi->irq, adc12138_eoc_handler,
> +			       IRQF_TRIGGER_RISING, indio_dev->name, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	cclk = devm_clk_get_enabled(&spi->dev, NULL);
> +	if (IS_ERR(cclk))
> +		return PTR_ERR(cclk);
>  
>  	adc->vref_p = devm_regulator_get(&spi->dev, "vref-p");
>  	if (IS_ERR(adc->vref_p))
> @@ -454,18 +458,9 @@ static int adc12138_probe(struct spi_device *spi)
>  			return ret;
>  	}
>  
> -	ret = devm_request_irq(&spi->dev, spi->irq, adc12138_eoc_handler,
> -			       IRQF_TRIGGER_RISING, indio_dev->name, indio_dev);
> -	if (ret)
> -		return ret;
> -
> -	ret = clk_prepare_enable(adc->cclk);
> -	if (ret)
> -		return ret;
> -
>  	ret = regulator_enable(adc->vref_p);
>  	if (ret)
> -		goto err_clk_disable;
> +		return ret;
>  
>  	if (!IS_ERR(adc->vref_n)) {
>  		ret = regulator_enable(adc->vref_n);
> @@ -496,8 +491,6 @@ static int adc12138_probe(struct spi_device *spi)
>  		regulator_disable(adc->vref_n);
>  err_vref_p_disable:
>  	regulator_disable(adc->vref_p);
> -err_clk_disable:
> -	clk_disable_unprepare(adc->cclk);
>  
>  	return ret;
>  }
> @@ -512,7 +505,6 @@ static void adc12138_remove(struct spi_device *spi)
>  	if (!IS_ERR(adc->vref_n))
>  		regulator_disable(adc->vref_n);
>  	regulator_disable(adc->vref_p);
> -	clk_disable_unprepare(adc->cclk);
>  }
>  
>  static const struct of_device_id adc12138_dt_ids[] = {
> 

[PATCH 2/4] iio: adc: vf610: Drop -ENOMEM error message

[Krzysztof Kozlowski]

Core already prints detailed error messages on ENOMEM errors and drivers
should avoid repeating it.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/iio/dac/vf610_dac.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iio/dac/vf610_dac.c b/drivers/iio/dac/vf610_dac.c
index b30ff7bb44002d4f02c5eb4ac7633774d16b7813..b7ee16ab4edd596564580e7fc9cfb556b2a0ba1d 100644
--- a/drivers/iio/dac/vf610_dac.c
+++ b/drivers/iio/dac/vf610_dac.c
@@ -178,10 +178,8 @@ static int vf610_dac_probe(struct platform_device *pdev)
 
 	indio_dev = devm_iio_device_alloc(&pdev->dev,
 					sizeof(struct vf610_dac));
-	if (!indio_dev) {
-		dev_err(&pdev->dev, "Failed allocating iio device\n");
+	if (!indio_dev)
 		return -ENOMEM;
-	}
 
 	info = iio_priv(indio_dev);
 	info->dev = &pdev->dev;

-- 
2.43.0

[PATCH 3/4] iio: adc: vf610: Simplify with dev_err_probe

[Krzysztof Kozlowski]

Use dev_err_probe() to make error code handling simpler and handle
deferred probe nicely (avoid spamming logs).

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/iio/dac/vf610_dac.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/dac/vf610_dac.c b/drivers/iio/dac/vf610_dac.c
index b7ee16ab4edd596564580e7fc9cfb556b2a0ba1d..ddf90ae65a2c2458ccd4fa855f3dc56b923aaaa6 100644
--- a/drivers/iio/dac/vf610_dac.c
+++ b/drivers/iio/dac/vf610_dac.c
@@ -189,11 +189,9 @@ static int vf610_dac_probe(struct platform_device *pdev)
 		return PTR_ERR(info->regs);
 
 	info->clk = devm_clk_get(&pdev->dev, "dac");
-	if (IS_ERR(info->clk)) {
-		dev_err(&pdev->dev, "Failed getting clock, err = %ld\n",
-			PTR_ERR(info->clk));
-		return PTR_ERR(info->clk);
-	}
+	if (IS_ERR(info->clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(info->clk),
+				     "Failed getting clock\n");
 
 	platform_set_drvdata(pdev, indio_dev);
 

-- 
2.43.0

[PATCH 4/4] iio: dac: vf610: Simplify with devm_clk_get_enabled()

[Krzysztof Kozlowski]

Driver is getting clock and almost immediately enabling it, with no
relevant code executed between, thus the probe path and cleanups can be
simplified with devm_clk_get_enabled().

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

Not tested on hardware.
---
 drivers/iio/dac/vf610_dac.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/iio/dac/vf610_dac.c b/drivers/iio/dac/vf610_dac.c
index ddf90ae65a2c2458ccd4fa855f3dc56b923aaaa6..93639599b2b9895ed3307907216697d05cbc2d79 100644
--- a/drivers/iio/dac/vf610_dac.c
+++ b/drivers/iio/dac/vf610_dac.c
@@ -188,7 +188,7 @@ static int vf610_dac_probe(struct platform_device *pdev)
 	if (IS_ERR(info->regs))
 		return PTR_ERR(info->regs);
 
-	info->clk = devm_clk_get(&pdev->dev, "dac");
+	info->clk = devm_clk_get_enabled(&pdev->dev, "dac");
 	if (IS_ERR(info->clk))
 		return dev_err_probe(&pdev->dev, PTR_ERR(info->clk),
 				     "Failed getting clock\n");
@@ -203,13 +203,6 @@ static int vf610_dac_probe(struct platform_device *pdev)
 
 	mutex_init(&info->lock);
 
-	ret = clk_prepare_enable(info->clk);
-	if (ret) {
-		dev_err(&pdev->dev,
-			"Could not prepare or enable the clock\n");
-		return ret;
-	}
-
 	vf610_dac_init(info);
 
 	ret = iio_device_register(indio_dev);
@@ -222,7 +215,6 @@ static int vf610_dac_probe(struct platform_device *pdev)
 
 error_iio_device_register:
 	vf610_dac_exit(info);
-	clk_disable_unprepare(info->clk);
 
 	return ret;
 }
@@ -234,7 +226,6 @@ static void vf610_dac_remove(struct platform_device *pdev)
 
 	iio_device_unregister(indio_dev);
 	vf610_dac_exit(info);
-	clk_disable_unprepare(info->clk);
 }
 
 static int vf610_dac_suspend(struct device *dev)

-- 
2.43.0

Re: [PATCH 0/4] iio: Few cleanups to vf610 and ti-adc12138

[Jonathan Cameron]

On Sun, 13 Jul 2025 17:59:54 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> Just few cleanups.  Not tested on hardware.  Only the first patch could
> have an observable effect.
> 
Applied 2-4.  As the 1st patch is more complex - I'll let that sit a while
first to see if anyone had views on the ordering change.

Jonathan

> Best regards,
> Krzysztof
> 
> ---
> Krzysztof Kozlowski (4):
>       iio: adc: ti-adc12138: Simplify with devm_clk_get_enabled()
>       iio: adc: vf610: Drop -ENOMEM error message
>       iio: adc: vf610: Simplify with dev_err_probe
>       iio: dac: vf610: Simplify with devm_clk_get_enabled()
> 
>  drivers/iio/adc/ti-adc12138.c | 30 +++++++++++-------------------
>  drivers/iio/dac/vf610_dac.c   | 23 +++++------------------
>  2 files changed, 16 insertions(+), 37 deletions(-)
> ---
> base-commit: a62b7a37e6fcf4a675b1548e7c168b96ec836442
> change-id: 20250713-iio-clk-get-enabled-4764482c8ff3
> 
> Best regards,