[PATCH v2 0/5] iio: adc: bcm_iproc_adc: use device-managed resources

[PATCH v2 0/5] iio: adc: bcm_iproc_adc: use device-managed resources

[mdshahid03]

From: Mohammad Shahid <mdshahid03@gmail.com>

Changes since v1:

- Sort headers and make the driver IWYU compliant.
- Use devm_clk_get_enabled() and devm_add_action_or_reset() for
- clock and ADC cleanup.
- Use devm_mutex_init() and devm_iio_device_register(), removing
  the manual cleanup paths and remove() callback.

I checked the history of IPROC_ADC_AUXIN_SCAN_ENA and the related
iProc touchscreen driver. There is no documented dependency requiring
the bit to be cleared before tsc_clk is enabled.

I also checked the touchscreen driver's shared-register handling. It
enables tsc_clk before configuring the shared REGCTL2, and its
history does not indicate an ordering requirement involving
IPROC_ADC_AUXIN_SCAN_ENA.

Therefore, devm_clk_get_enabled() is used as suggested.

linux/delay.h is retained because the driver directly uses udelay().

---
Mohammad Shahid (5):
  iio: adc: bcm_iproc_adc: sort headers alphabetically
  iio: adc: bcm_iproc_adc: fix include dependencies
  iio: adc: bcm_iproc_adc: use devm-managed cleanup
  iio: adc: bcm_iproc_adc: use devm-managed mutex initialization
  iio: adc: bcm_iproc_adc: use devm_iio_device_register()

 drivers/iio/adc/bcm_iproc_adc.c | 62 ++++++++++++++-------------------
 1 file changed, 26 insertions(+), 36 deletions(-)

-- 
2.43.0

[PATCH v2 1/5] 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

[PATCH v2 2/5] iio: adc: bcm_iproc_adc: fix include dependencies

[mdshahid03]

From: Mohammad Shahid <mdshahid03@gmail.com>

Add the headers required directly by the driver for completion and
mutex APIs.

Remove the unused `linux/io.h` header now that register access is
handled through regmap APIs.

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

diff --git a/drivers/iio/adc/bcm_iproc_adc.c b/drivers/iio/adc/bcm_iproc_adc.c
index e46e3761993a..2b59819c4ca4 100644
--- a/drivers/iio/adc/bcm_iproc_adc.c
+++ b/drivers/iio/adc/bcm_iproc_adc.c
@@ -4,11 +4,12 @@
  */
 
 #include <linux/clk.h>
+#include <linux/completion.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
-#include <linux/io.h>
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 
-- 
2.43.0

Re: [PATCH v2 2/5] iio: adc: bcm_iproc_adc: fix include dependencies

[Marcelo Schmitt]

On 08/30, mdshahid03@gmail.com wrote:
> From: Mohammad Shahid <mdshahid03@gmail.com>
> 
> Add the headers required directly by the driver for completion and
> mutex APIs.
> 
> Remove the unused `linux/io.h` header now that register access is
> handled through regmap APIs.
> 
> Signed-off-by: Mohammad Shahid <mdshahid03@gmail.com>
> ---
>  drivers/iio/adc/bcm_iproc_adc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/bcm_iproc_adc.c b/drivers/iio/adc/bcm_iproc_adc.c
> index e46e3761993a..2b59819c4ca4 100644
> --- a/drivers/iio/adc/bcm_iproc_adc.c
> +++ b/drivers/iio/adc/bcm_iproc_adc.c
> @@ -4,11 +4,12 @@
>   */
>  
>  #include <linux/clk.h>
> +#include <linux/completion.h>
>  #include <linux/delay.h>
>  #include <linux/interrupt.h>
> -#include <linux/io.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/module.h>
> +#include <linux/mutex.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>

I think that's okay. Though, if you go for a v3, you may also add 
#include <linux/bits.h>
#include <linux/types.h>
#include <linux/array_size.h>
#include <linux/device-id/of.h>
#include <linux/iio/types.h>
which mostly completes the list of includes iwyu tool reports to be missing.

With best regards,
Marcelo

Re: [PATCH v2 2/5] iio: adc: bcm_iproc_adc: fix include dependencies

[Jonathan Cameron]

On Mon, 31 Aug 2026 23:27:37 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 08/30, mdshahid03@gmail.com wrote:
> > From: Mohammad Shahid <mdshahid03@gmail.com>
> > 
> > Add the headers required directly by the driver for completion and
> > mutex APIs.
> > 
> > Remove the unused `linux/io.h` header now that register access is
> > handled through regmap APIs.
> > 
> > Signed-off-by: Mohammad Shahid <mdshahid03@gmail.com>
> > ---
> >  drivers/iio/adc/bcm_iproc_adc.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/adc/bcm_iproc_adc.c b/drivers/iio/adc/bcm_iproc_adc.c
> > index e46e3761993a..2b59819c4ca4 100644
> > --- a/drivers/iio/adc/bcm_iproc_adc.c
> > +++ b/drivers/iio/adc/bcm_iproc_adc.c
> > @@ -4,11 +4,12 @@
> >   */
> >  
> >  #include <linux/clk.h>
> > +#include <linux/completion.h>
> >  #include <linux/delay.h>
> >  #include <linux/interrupt.h>
> > -#include <linux/io.h>
> >  #include <linux/mfd/syscon.h>
> >  #include <linux/module.h>
> > +#include <linux/mutex.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/regmap.h>  
> 
> I think that's okay. Though, if you go for a v3, you may also add 
> #include <linux/bits.h>
> #include <linux/types.h>
> #include <linux/array_size.h>

These 3 are good.

> #include <linux/device-id/of.h>

Generally now assumed to be from appropriate bus header.

> #include <linux/iio/types.h>

Assumed to be via iio.h. I can't really remember why it is a separate header :(


> which mostly completes the list of includes iwyu tool reports to be missing.
> 
> With best regards,
> Marcelo

Re: [PATCH v2 2/5] iio: adc: bcm_iproc_adc: fix include dependencies

[Andy Shevchenko]

On Mon, Aug 31, 2026 at 11:27:37PM -0300, Marcelo Schmitt wrote:
> On 08/30, mdshahid03@gmail.com wrote:

> > Add the headers required directly by the driver for completion and
> > mutex APIs.
> > 
> > Remove the unused `linux/io.h` header now that register access is
> > handled through regmap APIs.

...

> >  #include <linux/clk.h>
> > +#include <linux/completion.h>
> >  #include <linux/delay.h>
> >  #include <linux/interrupt.h>
> > -#include <linux/io.h>
> >  #include <linux/mfd/syscon.h>
> >  #include <linux/module.h>
> > +#include <linux/mutex.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/regmap.h>
> 
> I think that's okay. Though, if you go for a v3, you may also add 
> #include <linux/bits.h>
> #include <linux/types.h>
> #include <linux/array_size.h>

> #include <linux/device-id/of.h>

This one is covered by platform_device.h.

> #include <linux/iio/types.h>

This one is covered by iio/iio.h.

> which mostly completes the list of includes iwyu tool reports to be missing.

The rest is indeed needs to be added. Thanks for spotting that!

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 3/5] iio: adc: bcm_iproc_adc: use devm-managed cleanup

[mdshahid03]

From: Mohammad Shahid <mdshahid03@gmail.com>

Use devm_clk_get_enabled() to manage the ADC clock and
devm_add_action_or_reset() to manage ADC cleanup.

Since the clock is only needed during probe, keep it as a local
variable instead of storing it in struct iproc_adc_priv.

Register the ADC cleanup action immediately after enabling the ADC
so that it is automatically performed on probe failure and device
removal.

This removes the manual clock cleanup callback and simplifies the
probe error paths.

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

diff --git a/drivers/iio/adc/bcm_iproc_adc.c b/drivers/iio/adc/bcm_iproc_adc.c
index 2b59819c4ca4..c7fbdc21f8cc 100644
--- a/drivers/iio/adc/bcm_iproc_adc.c
+++ b/drivers/iio/adc/bcm_iproc_adc.c
@@ -107,7 +107,6 @@ do { \
 
 struct iproc_adc_priv {
 	struct regmap *regmap;
-	struct clk *adc_clk;
 	struct mutex mutex;
 	int  irqno;
 	int chan_val;
@@ -449,6 +448,13 @@ 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 int iproc_adc_read_raw(struct iio_dev *indio_dev,
 			  struct iio_chan_spec const *chan,
 			  int *val,
@@ -508,6 +514,7 @@ static int iproc_adc_probe(struct platform_device *pdev)
 	struct iproc_adc_priv *adc_priv;
 	struct iio_dev *indio_dev = NULL;
 	struct device *dev = &pdev->dev;
+	struct clk *adc_clk;
 	int ret;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*adc_priv));
@@ -527,10 +534,9 @@ static int iproc_adc_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(adc_priv->regmap),
 				     "failed to get handle for tsc syscon\n");
 
-	adc_priv->adc_clk = devm_clk_get(dev, "tsc_clk");
-	if (IS_ERR(adc_priv->adc_clk))
-		return dev_err_probe(dev, PTR_ERR(adc_priv->adc_clk),
-				     "failed getting clock tsc_clk\n");
+	adc_clk = devm_clk_get_enabled(dev, "tsc_clk");
+	if (IS_ERR(adc_clk))
+		return dev_err_probe(dev, PTR_ERR(adc_clk), "failed to enable clock\n");
 
 	adc_priv->irqno = platform_get_irq(pdev, 0);
 	if (adc_priv->irqno < 0)
@@ -548,13 +554,13 @@ static int iproc_adc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = clk_prepare_enable(adc_priv->adc_clk);
+	ret = iproc_adc_enable(indio_dev);
 	if (ret)
-		return dev_err_probe(dev, ret, "failed to enable clock\n");
+		return ret;
 
-	ret = iproc_adc_enable(indio_dev);
+	ret = devm_add_action_or_reset(dev, iproc_adc_disable_action, indio_dev);
 	if (ret)
-		goto err_adc_enable;
+		return ret;
 
 	indio_dev->name = "iproc-static-adc";
 	indio_dev->info = &iproc_adc_iio_info;
@@ -563,29 +569,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 v2 3/5] iio: adc: bcm_iproc_adc: use devm-managed cleanup

[Jonathan Cameron]

> Use devm_clk_get_enabled() to manage the ADC clock and
> devm_add_action_or_reset() to manage ADC cleanup.
> 
> Since the clock is only needed during probe, keep it as a local
> variable instead of storing it in struct iproc_adc_priv.
>

Given this section needed discussion...

> Register the ADC cleanup action immediately after enabling the ADC
> so that it is automatically performed on probe failure and device
> removal.

I would have left this for a follow up patch rather than
doing it in this one. Here it is not really the number of changes but
rather focusing the reasoning on one thing.  Even without that
splitting the two changes would have been fine.

> 
> This removes the manual clock cleanup callback and simplifies the
> probe error paths.
> 
> Signed-off-by: Mohammad Shahid <mdshahid03@gmail.com>

Otherwise LGTM.  Note this is a minor thing so if nothing else
comes up after we've let this sit for a few days on the list
I'll probably apply it as it stands anyway.

Thanks,

Jonathan

-- 
Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

[PATCH v2 4/5] 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 c7fbdc21f8cc..11f2bc12c19a 100644
--- a/drivers/iio/adc/bcm_iproc_adc.c
+++ b/drivers/iio/adc/bcm_iproc_adc.c
@@ -524,7 +524,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

[PATCH v2 5/5] iio: adc: bcm_iproc_adc: use devm_iio_device_register()

[mdshahid03]

From: Mohammad Shahid <mdshahid03@gmail.com>

Use `devm_iio_device_register()` to automatically unregister the IIO
device when the device is detached.

This removes the manual `iio_device_unregister()` call and makes the
`iproc_adc_remove()` callback unnecessary.

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

diff --git a/drivers/iio/adc/bcm_iproc_adc.c b/drivers/iio/adc/bcm_iproc_adc.c
index 11f2bc12c19a..38a153d6fc70 100644
--- a/drivers/iio/adc/bcm_iproc_adc.c
+++ b/drivers/iio/adc/bcm_iproc_adc.c
@@ -570,7 +570,7 @@ static int iproc_adc_probe(struct platform_device *pdev)
 	indio_dev->channels = iproc_adc_iio_channels;
 	indio_dev->num_channels = ARRAY_SIZE(iproc_adc_iio_channels);
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(dev, indio_dev);
 	if (ret)
 		return dev_err_probe(dev, ret, "iio_device_register failed\n");
 
@@ -578,13 +578,6 @@ static int iproc_adc_probe(struct platform_device *pdev)
 
 }
 
-static void iproc_adc_remove(struct platform_device *pdev)
-{
-	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
-
-	iio_device_unregister(indio_dev);
-}
-
 static const struct of_device_id iproc_adc_of_match[] = {
 	{.compatible = "brcm,iproc-static-adc", },
 	{ }
@@ -593,7 +586,6 @@ MODULE_DEVICE_TABLE(of, iproc_adc_of_match);
 
 static struct platform_driver iproc_adc_driver = {
 	.probe = iproc_adc_probe,
-	.remove = iproc_adc_remove,
 	.driver = {
 		.name = "iproc-static-adc",
 		.of_match_table = iproc_adc_of_match,
-- 
2.43.0

Re: [PATCH v2 5/5] iio: adc: bcm_iproc_adc: use devm_iio_device_register()

[Jonathan Cameron]

> Use `devm_iio_device_register()` to automatically unregister the IIO
> device when the device is detached.
> 
> This removes the manual `iio_device_unregister()` call and makes the
> `iproc_adc_remove()` callback unnecessary.
Why is the quoting here useful but not in the previous patch?

I'd rather no ` unless there is some ambiguitiy wrt to free form
text vs references to specific types etc.  That doesn't happen for
function names as () is clear enough.

If nothing else comes up, I'll probably just tidy this up whilst applying.

Letting this series sit on list until it gets a few more eyes on it.

Thanks,

J

-- 
Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

Re: [PATCH v2 5/5] iio: adc: bcm_iproc_adc: use devm_iio_device_register()

[Marcelo Schmitt]

Hello Mohammad,

This patch looks overall good.
Two minor things inline.

On 08/30, mdshahid03@gmail.com wrote:
> From: Mohammad Shahid <mdshahid03@gmail.com>
> 
> Use `devm_iio_device_register()` to automatically unregister the IIO
> device when the device is detached.
> 
> This removes the manual `iio_device_unregister()` call and makes the
> `iproc_adc_remove()` callback unnecessary.
> 
> Signed-off-by: Mohammad Shahid <mdshahid03@gmail.com>
> ---
>  drivers/iio/adc/bcm_iproc_adc.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/adc/bcm_iproc_adc.c b/drivers/iio/adc/bcm_iproc_adc.c
> index 11f2bc12c19a..38a153d6fc70 100644
> --- a/drivers/iio/adc/bcm_iproc_adc.c
> +++ b/drivers/iio/adc/bcm_iproc_adc.c
> @@ -570,7 +570,7 @@ static int iproc_adc_probe(struct platform_device *pdev)
>  	indio_dev->channels = iproc_adc_iio_channels;
>  	indio_dev->num_channels = ARRAY_SIZE(iproc_adc_iio_channels);
>  
> -	ret = iio_device_register(indio_dev);
> +	ret = devm_iio_device_register(dev, indio_dev);
>  	if (ret)
>  		return dev_err_probe(dev, ret, "iio_device_register failed\n");
Optional change, the last return check can be skipped.
	return devm_iio_device_register(dev, indio_dev);
>  
> @@ -578,13 +578,6 @@ static int iproc_adc_probe(struct platform_device *pdev)
>  
>  }
>  
> -static void iproc_adc_remove(struct platform_device *pdev)
> -{
> -	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> -
> -	iio_device_unregister(indio_dev);
> -}
> -
Now that we won't call platform_get_drvdata() anymore, another thing that can
be removed is platform_set_drvdata(pdev, indio_dev);

With that dropped,
Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>

Re: [PATCH v2 0/5] iio: adc: bcm_iproc_adc: use device-managed resources

[mdshahid03]

Hi all,

Thanks for the reviews and suggestions.

For v3, I will incorporate the following changes:

Patch 2/5:
-Add the direct includes for <linux/bits.h>, <linux/types.h>, and <linux/array_size.h>.
 Keep <linux/device-id/of.h> and <linux/iio/types.h> out, as these are already provided through the appropriate headers.
Patch 5/5:
-Remove the now-unnecessary platform_set_drvdata().
-Return devm_iio_device_register() directly.
-Remove the unnecessary backticks around function names in the commit message.

For patch 3/5, I understand that splitting the ADC cleanup into a follow-up patch would be preferable from a patch-scope 
perspective.I will split this into a separate patch for v3.

Please let me know if I have misunderstood any of the review comments.

Thanks,

Shahid