[PATCH v2 next 0/5] IIO: sca3000: devm resource management

[PATCH v2 next 0/5] IIO: sca3000: devm resource management

[Harshit Mogalapalli]

Hi,

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

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

Thanks a lot to Jonathan for detailed review of V1.

During v1 review comments, Jonathan Cameron kindly provided some great
suggestions to improve this driver. One of them is making use of
gaurd(mutex) - autocleanup style, patch 5 does that for one function.

Yet to be addressed tasks:
1. We shouldn't be using the spi_device_id at all.
2. Modernize other fucntions 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.

Thanks for your time.

Regards,
Harshit

Harshit Mogalapalli (5):
  iio: sca3000: cache SPI device ID in probe
  iio: sca3000: switch IRQ handling to devm helpers
  iio: sca3000: stop interrupts via devm_add_action_or_reset()
  iio: sca3000: manage device registration with devm helper
  iio: sca3000: use guard(mutex) to simplify return paths

 drivers/iio/accel/sca3000.c | 99 +++++++++++++++++--------------------
 1 file changed, 44 insertions(+), 55 deletions(-)

-- 
2.47.3

[PATCH v2 next 1/5] iio: sca3000: cache SPI device ID in probe

[Harshit Mogalapalli]

Store the spi_device_id at probe entry and reuse it for the name and
chip info instead of calling spi_get_device_id() repeatedly.

Reuse the local dev pointer for resource managed helpers. While at it,
reshuffle variable list in a reverse Christmas tree order.

No functional change intended.

Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
v1->v2: No change
---
 drivers/iio/accel/sca3000.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
index 9ef4d6e27466..afe6ef61a53b 100644
--- a/drivers/iio/accel/sca3000.c
+++ b/drivers/iio/accel/sca3000.c
@@ -1439,11 +1439,13 @@ static const struct iio_info sca3000_info = {
 
 static int sca3000_probe(struct spi_device *spi)
 {
-	int ret;
+	const struct spi_device_id *id = spi_get_device_id(spi);
+	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;
 
@@ -1451,10 +1453,9 @@ static int sca3000_probe(struct spi_device *spi)
 	spi_set_drvdata(spi, indio_dev);
 	st->us = spi;
 	mutex_init(&st->lock);
-	st->info = &sca3000_spi_chip_info_tbl[spi_get_device_id(spi)
-					      ->driver_data];
+	st->info = &sca3000_spi_chip_info_tbl[id->driver_data];
 
-	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->name = id->name;
 	indio_dev->info = &sca3000_info;
 	if (st->info->temp_output) {
 		indio_dev->channels = sca3000_channels_with_temp;
@@ -1466,7 +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,
+	ret = devm_iio_kfifo_buffer_setup(dev, indio_dev,
 					  &sca3000_ring_setup_ops);
 	if (ret)
 		return ret;
-- 
2.47.3

[PATCH v2 next 2/5] 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>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
v1->v2: Fix indentation in devm_request_threaded_irq() call[ Thanks to
Jonathan]
---
 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 afe6ef61a53b..37ef724d5dc5 100644
--- a/drivers/iio/accel/sca3000.c
+++ b/drivers/iio/accel/sca3000.c
@@ -1473,34 +1473,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)
@@ -1530,8 +1523,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 v2 next 3/5] 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.

No functional change intended.

Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
v1->v2: Jonathan found a broken tear down sequence that was introduced
by my ptach 3 in v1: https://lore.kernel.org/all/20260131163218.2a4b93e5@jic23-huawei/

So first converted the interrupt disabling task to devm based call,
order of tear down is as follows after this patch: iio_unregister_device
is called in the remove() callback, post which any interrupts will be
disabled with devm_add_action_or_reset() call.
---
 drivers/iio/accel/sca3000.c | 53 ++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
index 37ef724d5dc5..4faffeea328c 100644
--- a/drivers/iio/accel/sca3000.c
+++ b/drivers/iio/accel/sca3000.c
@@ -1437,6 +1437,33 @@ static const struct iio_info sca3000_info = {
 	.write_event_config = &sca3000_write_event_config,
 };
 
+static int 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:
+	mutex_unlock(&st->lock);
+	return ret;
+}
+
+static void sca3000_disable_interrupts(void *data)
+{
+	struct iio_dev *indio_dev = data;
+	struct sca3000_state *st = iio_priv(indio_dev);
+
+	/* Must ensure no interrupts can be generated after this! */
+	sca3000_stop_all_interrupts(st);
+}
+
 static int sca3000_probe(struct spi_device *spi)
 {
 	const struct spi_device_id *id = spi_get_device_id(spi);
@@ -1489,6 +1516,10 @@ static int sca3000_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
+	ret = devm_add_action_or_reset(dev, sca3000_disable_interrupts, indio_dev);
+	if (ret)
+		return ret;
+
 	ret = iio_device_register(indio_dev);
 	if (ret)
 		return ret;
@@ -1496,33 +1527,11 @@ static int sca3000_probe(struct spi_device *spi)
 	return 0;
 }
 
-static int 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:
-	mutex_unlock(&st->lock);
-	return ret;
-}
-
 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 v2 next 4/5] 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>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
v1-> v2: This was a patch 3 in v1 but moved it to patch4 to preserve the
teardown sequence (unregister the iio device first, we can disable
interrupts later) -- Thanks to Jonathan for the guidance on V1.
---
 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 4faffeea328c..989074de18b4 100644
--- a/drivers/iio/accel/sca3000.c
+++ b/drivers/iio/accel/sca3000.c
@@ -1520,18 +1520,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[] = {
@@ -1548,7 +1537,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

[PATCH v2 next 5/5] iio: sca3000: use guard(mutex) to simplify return paths

[Harshit Mogalapalli]

Switch sca3000_stop_all_interrupts() to use guard(mutex).

Suggested-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
 drivers/iio/accel/sca3000.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
index 989074de18b4..1fccc493a0d7 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>
@@ -1441,18 +1442,17 @@ static int 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 error_ret;
-	ret = sca3000_write_reg(st, SCA3000_REG_INT_MASK_ADDR,
-				(st->rx[0] &
+		return 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)));
-error_ret:
-	mutex_unlock(&st->lock);
-	return ret;
 }
 
 static void sca3000_disable_interrupts(void *data)
-- 
2.47.3

Re: [PATCH v2 next 1/5] iio: sca3000: cache SPI device ID in probe

[David Lechner]

On 2/2/26 1:40 PM, Harshit Mogalapalli wrote:
> Store the spi_device_id at probe entry and reuse it for the name and
> chip info instead of calling spi_get_device_id() repeatedly.

Even better would be to replace spi_get_device_id() with
pi_get_device_match_data() and turn sca3000_spi_chip_info_tbl[]
into individual structs that are used directly as the match
data pointers.

> 
> Reuse the local dev pointer for resource managed helpers. While at it,
> reshuffle variable list in a reverse Christmas tree order.

The dev cleanup sounds like a second unrelated change that can be split
out from the spi_get_device_id() changes.

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

[David Lechner]

On 2/2/26 1:40 PM, 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.
> 
> No functional change intended.
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
> ---
> v1->v2: Jonathan found a broken tear down sequence that was introduced
> by my ptach 3 in v1: https://lore.kernel.org/all/20260131163218.2a4b93e5@jic23-huawei/
> 
> So first converted the interrupt disabling task to devm based call,
> order of tear down is as follows after this patch: iio_unregister_device
> is called in the remove() callback, post which any interrupts will be
> disabled with devm_add_action_or_reset() call.
> ---
>  drivers/iio/accel/sca3000.c | 53 ++++++++++++++++++++++---------------
>  1 file changed, 31 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
> index 37ef724d5dc5..4faffeea328c 100644
> --- a/drivers/iio/accel/sca3000.c
> +++ b/drivers/iio/accel/sca3000.c
> @@ -1437,6 +1437,33 @@ static const struct iio_info sca3000_info = {
>  	.write_event_config = &sca3000_write_event_config,
>  };
>  
> +static int sca3000_stop_all_interrupts(struct sca3000_state *st)

Return value is ignored now, so we can make this void and not return.

> +{
> +	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:
> +	mutex_unlock(&st->lock);
> +	return ret;
> +}
> +
> +static void sca3000_disable_interrupts(void *data)
> +{
> +	struct iio_dev *indio_dev = data;
> +	struct sca3000_state *st = iio_priv(indio_dev);
> +
> +	/* Must ensure no interrupts can be generated after this! */
> +	sca3000_stop_all_interrupts(st);
> +}

This wrapper doesn't seem necessary. We can combine to two functions
above into one.

Re: [PATCH v2 next 2/5] iio: sca3000: switch IRQ handling to devm helpers

[David Lechner]

On 2/2/26 1:40 PM, Harshit Mogalapalli wrote:
> 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>
> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
> ---
> v1->v2: Fix indentation in devm_request_threaded_irq() call[ Thanks to
> Jonathan]
> ---
>  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 afe6ef61a53b..37ef724d5dc5 100644
> --- a/drivers/iio/accel/sca3000.c
> +++ b/drivers/iio/accel/sca3000.c
> @@ -1473,34 +1473,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;
Could return directly here already, but it gets cleaned up in a
later patch, so its OK.

Reviewed-by: David Lechner <dlechner@baylibe.com>

Re: [PATCH v2 next 4/5] iio: sca3000: manage device registration with devm helper

[David Lechner]

On 2/2/26 1:40 PM, 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.
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
> ---
Reviewed-by: David Lechner <dlechner@baylibe.com>

Re: [PATCH v2 next 5/5] iio: sca3000: use guard(mutex) to simplify return paths

[David Lechner]

On 2/2/26 1:40 PM, Harshit Mogalapalli wrote:
> Switch sca3000_stop_all_interrupts() to use guard(mutex).

Should always say why this patch should be considered in the commit
message. E.g. in this one we simplify the code by avoiding a goto.

> 
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
> ---
Reviewed-by: David Lechner <dlechner@baylibe.com>

Re: [PATCH v2 next 0/5] IIO: sca3000: devm resource management

[Andy Shevchenko]

On Mon, Feb 02, 2026 at 11:40:07AM -0800, Harshit Mogalapalli wrote:
> Hi,
> 
> This is an attempt to update sca3000 accelerometer driver to make use
> of devm_ based helpers where needed. I have split it into 5 patches.
> 
> Patch 1 - some refactoring and simplification.
> Patch 2 - switches request_threaded_irq() over to the devm helper
> Patch 3 - Used devm_add_action_or_reset() for disabling interrupts.
> (Ensured the ordering of teardown bits remain same)
> Patch 4 - manage device registration with devm helper
> Patch 5 - Make use of guard() in sca3000_stop_all_interrupts() function.
> 
> Thanks a lot to Jonathan for detailed review of V1.
> 
> During v1 review comments, Jonathan Cameron kindly provided some great
> suggestions to improve this driver. One of them is making use of
> gaurd(mutex) - autocleanup style, patch 5 does that for one function.
> 
> Yet to be addressed tasks:
> 1. We shouldn't be using the spi_device_id at all.
> 2. Modernize other fucntions 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.

David already reviewed and v3 is anticipated, so I try to look into that one.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 next 1/5] iio: sca3000: cache SPI device ID in probe

[Harshit Mogalapalli]

Hi David,

On 03/02/26 04:31, David Lechner wrote:
> On 2/2/26 1:40 PM, Harshit Mogalapalli wrote:
>> Store the spi_device_id at probe entry and reuse it for the name and
>> chip info instead of calling spi_get_device_id() repeatedly.
> 
> Even better would be to replace spi_get_device_id() with
> pi_get_device_match_data() and turn sca3000_spi_chip_info_tbl[]
> into individual structs that are used directly as the match
> data pointers.
>

Thanks for the review.

Jonathan also suggested this on v1, but thought of doing this item 
separate to this patch series(as its a functional change) , would you 
mind if I do it in a separate patch after this series ?


>>
>> Reuse the local dev pointer for resource managed helpers. While at it,
>> reshuffle variable list in a reverse Christmas tree order.
> 
> The dev cleanup sounds like a second unrelated change that can be split
> out from the spi_get_device_id() changes.
> 

Sure I will separate dev cleanup and device_id extraction into separate 
patches.

Thanks for the suggestion.

Regards,
Harshit

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

[Harshit Mogalapalli]

Hi David,


On 03/02/26 04:34, David Lechner wrote:
> On 2/2/26 1:40 PM, 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.
>>
>> No functional change intended.
>>
>> Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
>> ---
>> v1->v2: Jonathan found a broken tear down sequence that was introduced
>> by my ptach 3 in v1: https://lore.kernel.org/all/20260131163218.2a4b93e5@jic23-huawei/
>>
>> So first converted the interrupt disabling task to devm based call,
>> order of tear down is as follows after this patch: iio_unregister_device
>> is called in the remove() callback, post which any interrupts will be
>> disabled with devm_add_action_or_reset() call.
>> ---
>>   drivers/iio/accel/sca3000.c | 53 ++++++++++++++++++++++---------------
>>   1 file changed, 31 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
>> index 37ef724d5dc5..4faffeea328c 100644
>> --- a/drivers/iio/accel/sca3000.c
>> +++ b/drivers/iio/accel/sca3000.c
>> @@ -1437,6 +1437,33 @@ static const struct iio_info sca3000_info = {
>>   	.write_event_config = &sca3000_write_event_config,
>>   };
>>   
>> +static int sca3000_stop_all_interrupts(struct sca3000_state *st)
> 
> Return value is ignored now, so we can make this void and not return.
> 
>> +{
>> +	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:
>> +	mutex_unlock(&st->lock);
>> +	return ret;
>> +}
>> +
>> +static void sca3000_disable_interrupts(void *data)
>> +{
>> +	struct iio_dev *indio_dev = data;
>> +	struct sca3000_state *st = iio_priv(indio_dev);
>> +
>> +	/* Must ensure no interrupts can be generated after this! */
>> +	sca3000_stop_all_interrupts(st);
>> +}
> 
> This wrapper doesn't seem necessary. We can combine to two functions
> above into one.

Thanks for the review, Sure, will remove this extra wrapper and add the 
needed things in the same sca3000_stop_all_interrupts() function.

Regards,
Harshit
> 

Re: [PATCH v2 next 0/5] IIO: sca3000: devm resource management

[Harshit Mogalapalli]

Hi Andy,

On 03/02/26 15:18, Andy Shevchenko wrote:
> On Mon, Feb 02, 2026 at 11:40:07AM -0800, Harshit Mogalapalli wrote:
>> Hi,
>>
>> This is an attempt to update sca3000 accelerometer driver to make use
>> of devm_ based helpers where needed. I have split it into 5 patches.
>>
>> Patch 1 - some refactoring and simplification.
>> Patch 2 - switches request_threaded_irq() over to the devm helper
>> Patch 3 - Used devm_add_action_or_reset() for disabling interrupts.
>> (Ensured the ordering of teardown bits remain same)
>> Patch 4 - manage device registration with devm helper
>> Patch 5 - Make use of guard() in sca3000_stop_all_interrupts() function.
>>
>> Thanks a lot to Jonathan for detailed review of V1.
>>
>> During v1 review comments, Jonathan Cameron kindly provided some great
>> suggestions to improve this driver. One of them is making use of
>> gaurd(mutex) - autocleanup style, patch 5 does that for one function.
>>
>> Yet to be addressed tasks:
>> 1. We shouldn't be using the spi_device_id at all.
>> 2. Modernize other fucntions 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.
> 
> David already reviewed and v3 is anticipated, so I try to look into that one.
> 

Sure, I am working on v3.

Thank you very much.


Regards,
Harshit

Re: [PATCH v2 next 4/5] iio: sca3000: manage device registration with devm helper

[Harshit Mogalapalli]

Hi David,

On 03/02/26 04:37, David Lechner wrote:
> On 2/2/26 1:40 PM, 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.
>>
>> Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
>> ---
> Reviewed-by: David Lechner <dlechner@baylibe.com>

While sending v3 with this RB tag added, I found that there is a missing 
r in baylibre in this and another patch.

I will wait for the comments on v3 first before fixing this.

Thanks,
Harshit

Re: [PATCH v2 next 1/5] iio: sca3000: cache SPI device ID in probe

[David Lechner]

On 2/3/26 4:24 AM, Harshit Mogalapalli wrote:
> Hi David,
> 
> On 03/02/26 04:31, David Lechner wrote:
>> On 2/2/26 1:40 PM, Harshit Mogalapalli wrote:
>>> Store the spi_device_id at probe entry and reuse it for the name and
>>> chip info instead of calling spi_get_device_id() repeatedly.
>>
>> Even better would be to replace spi_get_device_id() with
>> pi_get_device_match_data() and turn sca3000_spi_chip_info_tbl[]
>> into individual structs that are used directly as the match
>> data pointers.
>>
> 
> Thanks for the review.
> 
> Jonathan also suggested this on v1, but thought of doing this item separate to this patch series(as its a functional change) , would you mind if I do it in a separate patch after this series ?

I'm not sure why it would be considered a functional change. If you want to
save it for a separate series, that is fine. If we do that though, there
is probably not much point in cleaning up spi_get_device_id() first since
it will all be replaced anyway.