[PATCH v4 next 0/6] IIO: sca3000: devm resource management

[PATCH v4 next 0/6] IIO: sca3000: devm resource management

[Harshit Mogalapalli]

This is an attempt to update sca3000 accelerometer driver to make use
of devm_ based helpers where needed. I have split it into 6 patches.

Patch 1 - some refactoring and simplification of dev.
Patch 2 - switches request_threaded_irq() over to the devm helper
Patch 3 - make stop_all_interrupts() return void
Patch 4 - Make use of guard() in sca3000_stop_all_interrupts() function.
Patch 5 - Used devm_add_action_or_reset() for disabling interrupts.
(Ensured the ordering of teardown bits remain same)
Patch 6 - manage device registration with devm helper

Thanks to David and Jonathan for reviewing v1 and v2.

Yet to be addressed tasks: (Would like to take this up as a separate
activity)
1. We shouldn't be using the spi_device_id at all. [Thanks to onathan
and David]
2. Modernize other functions to make use of autocleanup style locking
which simpifies the code and makes error paths cleaner.

I will be working on these two above tasks and will be sending a
different patches for those.

The series builds cleanly and I have performed static analysis with
smatch checker but haven't tested on actual hardware.

v1 -> v2: changes are documented in patches where necessary.
v2 -> v3: Address comments from David Lechner.
v3 -> v4: Address comments from Andy.

Thanks for your time.

Regards,
Harshit


Harshit Mogalapalli (6):
  iio: sca3000: reuse device pointer for devm helpers
  iio: sca3000: switch IRQ handling to devm helpers
  iio: sca3000: make stop_all_interrupts() return void
  iio: sca3000: use guard(mutex) to simplify return paths
  iio: sca3000: stop interrupts via devm_add_action_or_reset()
  iio: sca3000: manage device registration with devm helper

 drivers/iio/accel/sca3000.c | 88 +++++++++++++++----------------------
 1 file changed, 35 insertions(+), 53 deletions(-)

-- 
2.47.3

[PATCH v4 next 1/6] iio: sca3000: reuse device pointer for devm helpers

[Harshit Mogalapalli]

Cache struct device *dev and feed it to the devm helpers to simplify
the probe function. While at it, also use reverse christmas tree order
for local variables.

No functional changes.

Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
v3->v4: Add RB from Andy and use a single line for
devm_iio_kfifo_buffer_setup()
---
 drivers/iio/accel/sca3000.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
index 9ef4d6e27466..fa0ec2e1a1a2 100644
--- a/drivers/iio/accel/sca3000.c
+++ b/drivers/iio/accel/sca3000.c
@@ -1439,11 +1439,12 @@ static const struct iio_info sca3000_info = {
 
 static int sca3000_probe(struct spi_device *spi)
 {
-	int ret;
+	struct device *dev = &spi->dev;
 	struct sca3000_state *st;
 	struct iio_dev *indio_dev;
+	int ret;
 
-	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
 	if (!indio_dev)
 		return -ENOMEM;
 
@@ -1466,8 +1467,7 @@ static int sca3000_probe(struct spi_device *spi)
 	}
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-	ret = devm_iio_kfifo_buffer_setup(&spi->dev, indio_dev,
-					  &sca3000_ring_setup_ops);
+	ret = devm_iio_kfifo_buffer_setup(dev, indio_dev, &sca3000_ring_setup_ops);
 	if (ret)
 		return ret;
 
-- 
2.47.3

[PATCH v4 next 2/6] iio: sca3000: switch IRQ handling to devm helpers

[Harshit Mogalapalli]

Convert the threaded IRQ registration to devm_request_threaded_irq() so
that the probe and remove paths can drop manual freeing of irqs.

No functionality change.

Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
v3->v4: Corrected RB address from David and Added RB from Andy.
---
 drivers/iio/accel/sca3000.c | 25 ++++++++-----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
index fa0ec2e1a1a2..4bad152009e8 100644
--- a/drivers/iio/accel/sca3000.c
+++ b/drivers/iio/accel/sca3000.c
@@ -1472,34 +1472,27 @@ static int sca3000_probe(struct spi_device *spi)
 		return ret;
 
 	if (spi->irq) {
-		ret = request_threaded_irq(spi->irq,
-					   NULL,
-					   &sca3000_event_handler,
-					   IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-					   "sca3000",
-					   indio_dev);
+		ret = devm_request_threaded_irq(dev, spi->irq, NULL,
+						&sca3000_event_handler,
+						IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+						"sca3000",
+						indio_dev);
 		if (ret)
 			return ret;
 	}
 	ret = sca3000_clean_setup(st);
 	if (ret)
-		goto error_free_irq;
+		return ret;
 
 	ret = sca3000_print_rev(indio_dev);
 	if (ret)
-		goto error_free_irq;
+		return ret;
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
-		goto error_free_irq;
+		return ret;
 
 	return 0;
-
-error_free_irq:
-	if (spi->irq)
-		free_irq(spi->irq, indio_dev);
-
-	return ret;
 }
 
 static int sca3000_stop_all_interrupts(struct sca3000_state *st)
@@ -1529,8 +1522,6 @@ static void sca3000_remove(struct spi_device *spi)
 
 	/* Must ensure no interrupts can be generated after this! */
 	sca3000_stop_all_interrupts(st);
-	if (spi->irq)
-		free_irq(spi->irq, indio_dev);
 }
 
 static const struct spi_device_id sca3000_id[] = {
-- 
2.47.3

[PATCH v4 next 3/6] iio: sca3000: make stop_all_interrupts() return void

[Harshit Mogalapalli]

sca3000_stop_all_interrupts() is called only from the driver remove
path and its return value is discarded, so convert the helper to return
void.

No functional change.

Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
 drivers/iio/accel/sca3000.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
index 4bad152009e8..f9680071bbbe 100644
--- a/drivers/iio/accel/sca3000.c
+++ b/drivers/iio/accel/sca3000.c
@@ -1495,22 +1495,21 @@ static int sca3000_probe(struct spi_device *spi)
 	return 0;
 }
 
-static int sca3000_stop_all_interrupts(struct sca3000_state *st)
+static void sca3000_stop_all_interrupts(struct sca3000_state *st)
 {
 	int ret;
 
 	mutex_lock(&st->lock);
 	ret = sca3000_read_data_short(st, SCA3000_REG_INT_MASK_ADDR, 1);
 	if (ret)
-		goto error_ret;
-	ret = sca3000_write_reg(st, SCA3000_REG_INT_MASK_ADDR,
-				(st->rx[0] &
-				 ~(SCA3000_REG_INT_MASK_RING_THREE_QUARTER |
-				   SCA3000_REG_INT_MASK_RING_HALF |
-				   SCA3000_REG_INT_MASK_ALL_INTS)));
-error_ret:
+		goto out_unlock;
+	sca3000_write_reg(st, SCA3000_REG_INT_MASK_ADDR,
+			  (st->rx[0] &
+			   ~(SCA3000_REG_INT_MASK_RING_THREE_QUARTER |
+			     SCA3000_REG_INT_MASK_RING_HALF |
+			     SCA3000_REG_INT_MASK_ALL_INTS)));
+out_unlock:
 	mutex_unlock(&st->lock);
-	return ret;
 }
 
 static void sca3000_remove(struct spi_device *spi)
-- 
2.47.3

[PATCH v4 next 4/6] iio: sca3000: use guard(mutex) to simplify return paths

[Harshit Mogalapalli]

Switch sca3000_stop_all_interrupts() to use guard(mutex) to simplify the
error paths without needing a goto.

Suggested-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
V3->V4: Move this patch earlier this simplifies a fucntion involved in
the movement.[Thanks to Andy for the suggestion]
---
 drivers/iio/accel/sca3000.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
index f9680071bbbe..a46990453ed0 100644
--- a/drivers/iio/accel/sca3000.c
+++ b/drivers/iio/accel/sca3000.c
@@ -7,6 +7,7 @@
  * See industrialio/accels/sca3000.h for comments.
  */
 
+#include <linux/cleanup.h>
 #include <linux/interrupt.h>
 #include <linux/fs.h>
 #include <linux/device.h>
@@ -1499,17 +1500,17 @@ static void sca3000_stop_all_interrupts(struct sca3000_state *st)
 {
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
+
 	ret = sca3000_read_data_short(st, SCA3000_REG_INT_MASK_ADDR, 1);
 	if (ret)
-		goto out_unlock;
+		return;
+
 	sca3000_write_reg(st, SCA3000_REG_INT_MASK_ADDR,
 			  (st->rx[0] &
 			   ~(SCA3000_REG_INT_MASK_RING_THREE_QUARTER |
 			     SCA3000_REG_INT_MASK_RING_HALF |
 			     SCA3000_REG_INT_MASK_ALL_INTS)));
-out_unlock:
-	mutex_unlock(&st->lock);
 }
 
 static void sca3000_remove(struct spi_device *spi)
-- 
2.47.3

[PATCH v4 next 5/6] iio: sca3000: stop interrupts via devm_add_action_or_reset()

[Harshit Mogalapalli]

sca3000_stop_all_interrupts() is moved above the probe routine so the
new function sca3000_disable_interrupts() used in probe can directly
call it without additional declaration.

Used devm_add_action_or_reset() for shutting down the interrupts.
Make sca3000_stop_all_interrupts() return void now that it always hooks
into devm cleanup.

No functional change intended.

Suggested-by: David Lechner <dlechner@baylibre.com>
Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
v3->v4: had to rebase becuase guard change is now moved earlier [Thanks
to Andy for the suggestion]
---
 drivers/iio/accel/sca3000.c | 45 ++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
index a46990453ed0..c3a8f34d4852 100644
--- a/drivers/iio/accel/sca3000.c
+++ b/drivers/iio/accel/sca3000.c
@@ -1438,6 +1438,25 @@ static const struct iio_info sca3000_info = {
 	.write_event_config = &sca3000_write_event_config,
 };
 
+static void sca3000_stop_all_interrupts(void *data)
+{
+	struct iio_dev *indio_dev = data;
+	struct sca3000_state *st = iio_priv(indio_dev);
+	int ret;
+
+	guard(mutex)(&st->lock);
+
+	ret = sca3000_read_data_short(st, SCA3000_REG_INT_MASK_ADDR, 1);
+	if (ret)
+		return;
+
+	sca3000_write_reg(st, SCA3000_REG_INT_MASK_ADDR,
+			  (st->rx[0] &
+			   ~(SCA3000_REG_INT_MASK_RING_THREE_QUARTER |
+			     SCA3000_REG_INT_MASK_RING_HALF |
+			     SCA3000_REG_INT_MASK_ALL_INTS)));
+}
+
 static int sca3000_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
@@ -1489,39 +1508,23 @@ static int sca3000_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_add_action_or_reset(dev, sca3000_stop_all_interrupts,
+				       indio_dev);
 	if (ret)
 		return ret;
 
-	return 0;
-}
-
-static void sca3000_stop_all_interrupts(struct sca3000_state *st)
-{
-	int ret;
-
-	guard(mutex)(&st->lock);
-
-	ret = sca3000_read_data_short(st, SCA3000_REG_INT_MASK_ADDR, 1);
+	ret = iio_device_register(indio_dev);
 	if (ret)
-		return;
+		return ret;
 
-	sca3000_write_reg(st, SCA3000_REG_INT_MASK_ADDR,
-			  (st->rx[0] &
-			   ~(SCA3000_REG_INT_MASK_RING_THREE_QUARTER |
-			     SCA3000_REG_INT_MASK_RING_HALF |
-			     SCA3000_REG_INT_MASK_ALL_INTS)));
+	return 0;
 }
 
 static void sca3000_remove(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct sca3000_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
-
-	/* Must ensure no interrupts can be generated after this! */
-	sca3000_stop_all_interrupts(st);
 }
 
 static const struct spi_device_id sca3000_id[] = {
-- 
2.47.3

[PATCH v4 next 6/6] iio: sca3000: manage device registration with devm helper

[Harshit Mogalapalli]

Convert the iio registration to use devm_* helpers so the probe no
longer needs a separate cleanup path and remove callback can also drop
the unregister. After this there is no need for having a remove
callback, so remote it.

No functional change intended.

Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
 drivers/iio/accel/sca3000.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
index c3a8f34d4852..88900b9d5ee1 100644
--- a/drivers/iio/accel/sca3000.c
+++ b/drivers/iio/accel/sca3000.c
@@ -1513,18 +1513,7 @@ static int sca3000_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = iio_device_register(indio_dev);
-	if (ret)
-		return ret;
-
-	return 0;
-}
-
-static void sca3000_remove(struct spi_device *spi)
-{
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-
-	iio_device_unregister(indio_dev);
+	return devm_iio_device_register(dev, indio_dev);
 }
 
 static const struct spi_device_id sca3000_id[] = {
@@ -1541,7 +1530,6 @@ static struct spi_driver sca3000_driver = {
 		.name = "sca3000",
 	},
 	.probe = sca3000_probe,
-	.remove = sca3000_remove,
 	.id_table = sca3000_id,
 };
 module_spi_driver(sca3000_driver);
-- 
2.47.3

Re: [PATCH v4 next 4/6] iio: sca3000: use guard(mutex) to simplify return paths

[Andy Shevchenko]

On Tue, Feb 03, 2026 at 10:11:59PM -0800, Harshit Mogalapalli wrote:
> Switch sca3000_stop_all_interrupts() to use guard(mutex) to simplify the
> error paths without needing a goto.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 next 5/6] iio: sca3000: stop interrupts via devm_add_action_or_reset()

[Andy Shevchenko]

On Tue, Feb 03, 2026 at 10:12:00PM -0800, Harshit Mogalapalli wrote:
> sca3000_stop_all_interrupts() is moved above the probe routine so the
> new function sca3000_disable_interrupts() used in probe can directly
> call it without additional declaration.
> 
> Used devm_add_action_or_reset() for shutting down the interrupts.
> Make sca3000_stop_all_interrupts() return void now that it always hooks
> into devm cleanup.
> 
> No functional change intended.

This patch overloaded by extra thing. What you should do is just move code
upper without _any_ modifications being done _before_ even patching it for
guard()(). With that additional patch the rest will look much easier to
review.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 next 3/6] iio: sca3000: make stop_all_interrupts() return void

[Andy Shevchenko]

On Tue, Feb 03, 2026 at 10:11:58PM -0800, Harshit Mogalapalli wrote:
> sca3000_stop_all_interrupts() is called only from the driver remove
> path and its return value is discarded, so convert the helper to return
> void.
> 
> No functional change.

Yeah, as I just replied, the prerequisite for this should be move upper in the
code where you are going to use it in the future.

...

>  	ret = sca3000_read_data_short(st, SCA3000_REG_INT_MASK_ADDR, 1);
>  	if (ret)
> -		goto error_ret;

> -error_ret:
> +		goto out_unlock;

> +out_unlock:

While this is correct change semantically, it's not needed as very soon
the other patch drops this for good, hence leave the label name unmodified.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 next 6/6] iio: sca3000: manage device registration with devm helper

[Andy Shevchenko]

On Tue, Feb 03, 2026 at 10:12:01PM -0800, Harshit Mogalapalli wrote:
> Convert the iio registration to use devm_* helpers so the probe no
> longer needs a separate cleanup path and remove callback can also drop
> the unregister. After this there is no need for having a remove
> callback, so remote it.
> 
> No functional change intended.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 next 3/6] iio: sca3000: make stop_all_interrupts() return void

[Harshit Mogalapalli]

Hi Andy,

On 04/02/26 14:33, Andy Shevchenko wrote:
> On Tue, Feb 03, 2026 at 10:11:58PM -0800, Harshit Mogalapalli wrote:
>> sca3000_stop_all_interrupts() is called only from the driver remove
>> path and its return value is discarded, so convert the helper to return
>> void.
>>
>> No functional change.
> 
> Yeah, as I just replied, the prerequisite for this should be move upper in the
> code where you are going to use it in the future.
> 

Sure, will do that! That would make reviewing simpler.
> ...
> 
>>   	ret = sca3000_read_data_short(st, SCA3000_REG_INT_MASK_ADDR, 1);
>>   	if (ret)
>> -		goto error_ret;
> 
>> -error_ret:
>> +		goto out_unlock;
> 
>> +out_unlock:
> 
> While this is correct change semantically, it's not needed as very soon
> the other patch drops this for good, hence leave the label name unmodified.
> 

Agree, I was doubtful on which is the preferred approach as its not 
really a return anymore. But thanks for explaining.

Regards,
Harshit

Re: [PATCH v4 next 5/6] iio: sca3000: stop interrupts via devm_add_action_or_reset()

[Harshit Mogalapalli]

Hi Andy,

On 04/02/26 14:31, Andy Shevchenko wrote:
> On Tue, Feb 03, 2026 at 10:12:00PM -0800, Harshit Mogalapalli wrote:
>> sca3000_stop_all_interrupts() is moved above the probe routine so the
>> new function sca3000_disable_interrupts() used in probe can directly
>> call it without additional declaration.
>>
>> Used devm_add_action_or_reset() for shutting down the interrupts.
>> Make sca3000_stop_all_interrupts() return void now that it always hooks
>> into devm cleanup.
>>
>> No functional change intended.
> 
> This patch overloaded by extra thing. What you should do is just move code
> upper without _any_ modifications being done _before_ even patching it for
> guard()(). With that additional patch the rest will look much easier to
> review.
> 

I agree, thanks a lot for the suggestions!

Regards,
Harshit

Re: [PATCH v4 next 3/6] iio: sca3000: make stop_all_interrupts() return void

[Andy Shevchenko]

On Wed, Feb 04, 2026 at 02:50:54PM +0530, Harshit Mogalapalli wrote:
> On 04/02/26 14:33, Andy Shevchenko wrote:
> > On Tue, Feb 03, 2026 at 10:11:58PM -0800, Harshit Mogalapalli wrote:

...

> > >   	ret = sca3000_read_data_short(st, SCA3000_REG_INT_MASK_ADDR, 1);
> > >   	if (ret)
> > > -		goto error_ret;
> > 
> > > -error_ret:
> > > +		goto out_unlock;
> > 
> > > +out_unlock:
> > 
> > While this is correct change semantically, it's not needed as very soon
> > the other patch drops this for good, hence leave the label name unmodified.
> 
> Agree, I was doubtful on which is the preferred approach as its not really a
> return anymore. But thanks for explaining.

The common sense says that we need to avoid ping-pong coding (*) in the series.

*It's when one patch in the series adds the code that's going to be deleted or
heavily modified just later in the very same series.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 next 3/6] iio: sca3000: make stop_all_interrupts() return void

[Harshit Mogalapalli]

Hi Andy,

On 04/02/26 15:31, Andy Shevchenko wrote:
> On Wed, Feb 04, 2026 at 02:50:54PM +0530, Harshit Mogalapalli wrote:
>> On 04/02/26 14:33, Andy Shevchenko wrote:
>>> On Tue, Feb 03, 2026 at 10:11:58PM -0800, Harshit Mogalapalli wrote:
> 
> ...
> 
>>>>    	ret = sca3000_read_data_short(st, SCA3000_REG_INT_MASK_ADDR, 1);
>>>>    	if (ret)
>>>> -		goto error_ret;
>>>
>>>> -error_ret:
>>>> +		goto out_unlock;
>>>
>>>> +out_unlock:
>>>
>>> While this is correct change semantically, it's not needed as very soon
>>> the other patch drops this for good, hence leave the label name unmodified.
>>
>> Agree, I was doubtful on which is the preferred approach as its not really a
>> return anymore. But thanks for explaining.
> 
> The common sense says that we need to avoid ping-pong coding (*) in the series.
> 
> *It's when one patch in the series adds the code that's going to be deleted or
> heavily modified just later in the very same series.
> 

Sure thanks for explaining, I was thinking about it from a backport 
point of view(say this patch is auto selected as a prerequisite for 
applying another patch, but guard patch is not selected because there is 
no cleanup.h is some older kernel), in that case if this patch alone 
gets backported but not the gaurd() patch, err_return label might not 
look great. But I agree its not likely to happen in this case.

Thanks for sharing your thoughts.

Regards,
Harshit