[PATCH v1 0/3] iio: adc: bcm_iproc_adc: Convert to devm-managed resources

[PATCH v1 0/3] iio: adc: bcm_iproc_adc: Convert to devm-managed resources

[mdshahid03]

From: Mohammad Shahid <mdshahid03@gmail.com>

This series is a follow-up to the previously submitted probe cleanup
series for bcm_iproc_adc, which is already present in iio/testing.

As discussed with Jonathan, this series continues the cleanup
with a few additional driver cleanups and devm-managed resource
management.

Patch 1 sorts the driver headers alphabetically.

Patch 2 replaces the manual ADC and clock cleanup with
devm_add_action_or_reset(), allowing the probe cleanup labels and
manual cleanup in remove() to be removed.

Patch 3 converts mutex initialization to devm_mutex_init().

---
Mohammad Shahid (3):
  iio: adc: bcm_iproc_adc: sort headers alphabetically
  iio: adc: bcm_iproc_adc: use devm_add_action_or_reset()
  iio: adc: bcm_iproc_adc: use devm-managed mutex initialization

 drivers/iio/adc/bcm_iproc_adc.c | 51 +++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 19 deletions(-)


base-commit: 560c1aa128cafb238f460826abf43bc8b83aa0a6
-- 
2.43.0

[PATCH v1 1/3] iio: adc: bcm_iproc_adc: sort headers alphabetically

[mdshahid03]

From: Mohammad Shahid <mdshahid03@gmail.com>

Sort the Linux kernel headers alphabetically to improve consistency
and make the include list easier to maintain.

Signed-off-by: Mohammad Shahid <mdshahid03@gmail.com>
---
 drivers/iio/adc/bcm_iproc_adc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/bcm_iproc_adc.c b/drivers/iio/adc/bcm_iproc_adc.c
index ab66b97d2f04..e46e3761993a 100644
--- a/drivers/iio/adc/bcm_iproc_adc.c
+++ b/drivers/iio/adc/bcm_iproc_adc.c
@@ -3,14 +3,14 @@
  * Copyright 2016 Broadcom
  */
 
-#include <linux/module.h>
-#include <linux/io.h>
 #include <linux/clk.h>
-#include <linux/mfd/syscon.h>
-#include <linux/regmap.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 
 #include <linux/iio/iio.h>
 
-- 
2.43.0

Re: [PATCH v1 1/3] iio: adc: bcm_iproc_adc: sort headers alphabetically

[Marcelo Schmitt]

Hello Mohammad,

On 08/25, mdshahid03@gmail.com wrote:
> From: Mohammad Shahid <mdshahid03@gmail.com>
> 
> Sort the Linux kernel headers alphabetically to improve consistency
> and make the include list easier to maintain.
> 
> Signed-off-by: Mohammad Shahid <mdshahid03@gmail.com>
> ---
>  drivers/iio/adc/bcm_iproc_adc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/bcm_iproc_adc.c b/drivers/iio/adc/bcm_iproc_adc.c
> index ab66b97d2f04..e46e3761993a 100644
> --- a/drivers/iio/adc/bcm_iproc_adc.c
> +++ b/drivers/iio/adc/bcm_iproc_adc.c
> @@ -3,14 +3,14 @@
>   * Copyright 2016 Broadcom
>   */
>  
> -#include <linux/module.h>
> -#include <linux/io.h>
>  #include <linux/clk.h>
> -#include <linux/mfd/syscon.h>
> -#include <linux/regmap.h>
>  #include <linux/delay.h>
>  #include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  
>  #include <linux/iio/iio.h>

The sorting looks okay to me. Though, there are some includes missing and a
couple of them that don't seem to be needed (delay.h and io.h). Either in
replacement of this patch or as a follow on extra patch (not sure how maintainers
would like to have it in this case), it would be nice to also update the include
list according to include-what-you-use principles.

If you decide to work on the update to make it iwyu compliant, it may be
helpful to set an IIO tailored mapping file such as the one shared by Jonathan
a while ago.
https://lore.kernel.org/linux-iio/20250629194336.34a03946@jic23-huawei/

Re: [PATCH v1 1/3] iio: adc: bcm_iproc_adc: sort headers alphabetically

[Andy Shevchenko]

On Wed, Aug 26, 2026 at 12:28:22AM -0300, Marcelo Schmitt wrote:
> On 08/25, mdshahid03@gmail.com wrote:

...

> The sorting looks okay to me. Though, there are some includes missing and a
> couple of them that don't seem to be needed (delay.h and io.h). Either in
> replacement of this patch or as a follow on extra patch (not sure how maintainers
> would like to have it in this case),

Too many lines to sort, hence separate patch:
- sort
- apply IWYU

> it would be nice to also update the include list according to
> include-what-you-use principles.

> If you decide to work on the update to make it iwyu compliant, it may be
> helpful to set an IIO tailored mapping file such as the one shared by Jonathan
> a while ago.
> https://lore.kernel.org/linux-iio/20250629194336.34a03946@jic23-huawei/

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1 2/3] iio: adc: bcm_iproc_adc: use devm_add_action_or_reset()

[mdshahid03]

From: Mohammad Shahid <mdshahid03@gmail.com>

Replace the manual ADC and clock cleanup in probe error paths
and remove() with devm_add_action_or_reset().

Register cleanup actions immediately after enabling the clock and
ADC so that the resources are automatically released on probe
failure and device removal.

This also allows the cleanup labels to be removed and the
iio_device_register() failure path to return directly.

Signed-off-by: Mohammad Shahid <mdshahid03@gmail.com>
---
 drivers/iio/adc/bcm_iproc_adc.c | 39 +++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/adc/bcm_iproc_adc.c b/drivers/iio/adc/bcm_iproc_adc.c
index e46e3761993a..1464c60bc83e 100644
--- a/drivers/iio/adc/bcm_iproc_adc.c
+++ b/drivers/iio/adc/bcm_iproc_adc.c
@@ -448,6 +448,20 @@ static void iproc_adc_disable(struct iio_dev *indio_dev)
 	}
 }
 
+static void iproc_adc_disable_action(void *data)
+{
+	struct iio_dev *indio_dev = data;
+
+	iproc_adc_disable(indio_dev);
+}
+
+static void iproc_adc_clk_disable(void *data)
+{
+	struct clk *clk = data;
+
+	clk_disable_unprepare(clk);
+}
+
 static int iproc_adc_read_raw(struct iio_dev *indio_dev,
 			  struct iio_chan_spec const *chan,
 			  int *val,
@@ -551,9 +565,17 @@ static int iproc_adc_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to enable clock\n");
 
+	ret = devm_add_action_or_reset(dev, iproc_adc_clk_disable, adc_priv->adc_clk);
+	if (ret)
+		return ret;
+
 	ret = iproc_adc_enable(indio_dev);
 	if (ret)
-		goto err_adc_enable;
+		return ret;
+
+	ret = devm_add_action_or_reset(dev, iproc_adc_disable_action, indio_dev);
+	if (ret)
+		return ret;
 
 	indio_dev->name = "iproc-static-adc";
 	indio_dev->info = &iproc_adc_iio_info;
@@ -562,29 +584,18 @@ static int iproc_adc_probe(struct platform_device *pdev)
 	indio_dev->num_channels = ARRAY_SIZE(iproc_adc_iio_channels);
 
 	ret = iio_device_register(indio_dev);
-	if (ret) {
-		dev_err(&pdev->dev, "iio_device_register failed:err %d\n", ret);
-		goto err_clk;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "iio_device_register failed\n");
 
 	return 0;
 
-err_clk:
-	iproc_adc_disable(indio_dev);
-err_adc_enable:
-	clk_disable_unprepare(adc_priv->adc_clk);
-
-	return ret;
 }
 
 static void iproc_adc_remove(struct platform_device *pdev)
 {
 	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
-	struct iproc_adc_priv *adc_priv = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
-	iproc_adc_disable(indio_dev);
-	clk_disable_unprepare(adc_priv->adc_clk);
 }
 
 static const struct of_device_id iproc_adc_of_match[] = {
-- 
2.43.0

Re: [PATCH v1 2/3] iio: adc: bcm_iproc_adc: use devm_add_action_or_reset()

[Marcelo Schmitt]

On 08/25, mdshahid03@gmail.com wrote:
> From: Mohammad Shahid <mdshahid03@gmail.com>
> 
> Replace the manual ADC and clock cleanup in probe error paths
> and remove() with devm_add_action_or_reset().
> 
> Register cleanup actions immediately after enabling the clock and
> ADC so that the resources are automatically released on probe
> failure and device removal.
> 
> This also allows the cleanup labels to be removed and the
> iio_device_register() failure path to return directly.
> 
> Signed-off-by: Mohammad Shahid <mdshahid03@gmail.com>
> ---
...
> +static void iproc_adc_clk_disable(void *data)
> +{
> +	struct clk *clk = data;
> +
> +	clk_disable_unprepare(clk);
> +}
> +
The above callback is not needed with devm_clk_get_enabled(). See comment below.

>  static int iproc_adc_read_raw(struct iio_dev *indio_dev,
>  			  struct iio_chan_spec const *chan,
>  			  int *val,
> @@ -551,9 +565,17 @@ static int iproc_adc_probe(struct platform_device *pdev)
>  	if (ret)
>  		return dev_err_probe(dev, ret, "failed to enable clock\n");
>  
> +	ret = devm_add_action_or_reset(dev, iproc_adc_clk_disable, adc_priv->adc_clk);
> +	if (ret)
> +		return ret;
> +
Hmm, the whole clock get/add_action/enable sequence can be replaced by
devm_clk_get_enabled(). Unless the ADC needs to clear IPROC_ADC_AUXIN_SCAN_ENA
before tsc_clk gets enabled. Otherwise, the update can be done with fewer
LOC by calling devm_clk_get_enabled(). Also, since adc_clk is only used during
device probe, it can be declared as a local variable rather than a field of
struct iproc_adc_priv. So, in addition to updating to devm interfaces, patch 2
can reduce struct iproc_adc_priv size by keeping adc_clk as a local variable of
iproc_adc_probe().

>  	ret = iproc_adc_enable(indio_dev);
>  	if (ret)
> -		goto err_adc_enable;
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(dev, iproc_adc_disable_action, indio_dev);
> +	if (ret)
> +		return ret;
>  
>  	indio_dev->name = "iproc-static-adc";
>  	indio_dev->info = &iproc_adc_iio_info;
> @@ -562,29 +584,18 @@ static int iproc_adc_probe(struct platform_device *pdev)
>  	indio_dev->num_channels = ARRAY_SIZE(iproc_adc_iio_channels);
>  
>  	ret = iio_device_register(indio_dev);
> -	if (ret) {
> -		dev_err(&pdev->dev, "iio_device_register failed:err %d\n", ret);
> -		goto err_clk;
> -	}
> +	if (ret)
> +		return dev_err_probe(dev, ret, "iio_device_register failed\n");
This looks good, and can become even more concise with iio_device_register().
In addition to patch 2 (clk and adc) and patch 3 (mutex) device managed patches,
add a patch 4 updating from iio_device_register() to devm_iio_device_register().
With that, all device resources shall be released on device detach and
iproc_adc_remove() won't be needed anymore.

>  
>  	return 0;
>  
> -err_clk:
> -	iproc_adc_disable(indio_dev);
> -err_adc_enable:
> -	clk_disable_unprepare(adc_priv->adc_clk);
> -
> -	return ret;
>  }
>  
>  static void iproc_adc_remove(struct platform_device *pdev)
>  {
>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> -	struct iproc_adc_priv *adc_priv = iio_priv(indio_dev);
>  
>  	iio_device_unregister(indio_dev);
> -	iproc_adc_disable(indio_dev);
> -	clk_disable_unprepare(adc_priv->adc_clk);
>  }

[PATCH v1 3/3] iio: adc: bcm_iproc_adc: use devm-managed mutex initialization

[mdshahid03]

From: Mohammad Shahid <mdshahid03@gmail.com>

Use devm_mutex_init() to tie the mutex lifetime to the device and
improve debugging when CONFIG_DEBUG_MUTEXES is enabled.

Signed-off-by: Mohammad Shahid <mdshahid03@gmail.com>
---
 drivers/iio/adc/bcm_iproc_adc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/bcm_iproc_adc.c b/drivers/iio/adc/bcm_iproc_adc.c
index 1464c60bc83e..9b299e2c3e0c 100644
--- a/drivers/iio/adc/bcm_iproc_adc.c
+++ b/drivers/iio/adc/bcm_iproc_adc.c
@@ -530,7 +530,9 @@ static int iproc_adc_probe(struct platform_device *pdev)
 	adc_priv = iio_priv(indio_dev);
 	platform_set_drvdata(pdev, indio_dev);
 
-	mutex_init(&adc_priv->mutex);
+	ret = devm_mutex_init(dev, &adc_priv->mutex);
+	if (ret)
+		return ret;
 
 	init_completion(&adc_priv->completion);
 
-- 
2.43.0

Re: [PATCH v1 3/3] iio: adc: bcm_iproc_adc: use devm-managed mutex initialization

[Marcelo Schmitt]

On 08/25, mdshahid03@gmail.com wrote:
> From: Mohammad Shahid <mdshahid03@gmail.com>
> 
> Use devm_mutex_init() to tie the mutex lifetime to the device and
> improve debugging when CONFIG_DEBUG_MUTEXES is enabled.
> 
> Signed-off-by: Mohammad Shahid <mdshahid03@gmail.com>
> ---
Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>

Though note the device driver currently lacks an include to mutex.h.

>  drivers/iio/adc/bcm_iproc_adc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/bcm_iproc_adc.c b/drivers/iio/adc/bcm_iproc_adc.c
> index 1464c60bc83e..9b299e2c3e0c 100644
> --- a/drivers/iio/adc/bcm_iproc_adc.c
> +++ b/drivers/iio/adc/bcm_iproc_adc.c
> @@ -530,7 +530,9 @@ static int iproc_adc_probe(struct platform_device *pdev)
>  	adc_priv = iio_priv(indio_dev);
>  	platform_set_drvdata(pdev, indio_dev);
>  
> -	mutex_init(&adc_priv->mutex);
> +	ret = devm_mutex_init(dev, &adc_priv->mutex);
> +	if (ret)
> +		return ret;
>  
>  	init_completion(&adc_priv->completion);