* [PATCH v3 next 0/6] IIO: sca3000: devm resource management
@ 2026-02-03 12:20 Harshit Mogalapalli
2026-02-03 12:20 ` [PATCH v3 next 1/6] iio: sca3000: cache SPI device ID in probe Harshit Mogalapalli
` (5 more replies)
0 siblings, 6 replies; 19+ messages in thread
From: Harshit Mogalapalli @ 2026-02-03 12:20 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Harshit Mogalapalli, Andrew Ijano, Antoniu Miclaus, linux-iio,
linux-kernel
Cc: kernel-janitors, error27, andriy.shevchenko
Hi,
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 spi_device.
Patch 2 - some refactoring and simplification of dev.
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 to David and Jonathan for reviewing v1 and v2.
Yet to be addressed tasks:
1. We shouldn't be using the spi_device_id at all. [Thanks to onathan
and David]
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.
v2 -> v3: Address comments from David Lechner.
Thanks for your time.
Regards,
Harshit
Harshit Mogalapalli (6):
iio: sca3000: cache SPI device ID in probe
iio: sca3000: reuse device pointer for devm helpers
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 | 91 +++++++++++++++----------------------
1 file changed, 36 insertions(+), 55 deletions(-)
--
2.47.3
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 next 1/6] iio: sca3000: cache SPI device ID in probe
2026-02-03 12:20 [PATCH v3 next 0/6] IIO: sca3000: devm resource management Harshit Mogalapalli
@ 2026-02-03 12:20 ` Harshit Mogalapalli
2026-02-03 15:19 ` Andy Shevchenko
2026-02-03 12:20 ` [PATCH v3 next 2/6] iio: sca3000: reuse device pointer for devm helpers Harshit Mogalapalli
` (4 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Harshit Mogalapalli @ 2026-02-03 12:20 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Harshit Mogalapalli, Antoniu Miclaus, Andrew Ijano, linux-iio,
linux-kernel
Cc: kernel-janitors, error27, andriy.shevchenko
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.
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>
---
v2->v3: Split cachine spi device id patch from the dev cleanup.
---
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..a0b431aff024 100644
--- a/drivers/iio/accel/sca3000.c
+++ b/drivers/iio/accel/sca3000.c
@@ -1439,9 +1439,10 @@ 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 sca3000_state *st;
struct iio_dev *indio_dev;
+ int ret;
indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
if (!indio_dev)
@@ -1451,10 +1452,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;
--
2.47.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 next 2/6] iio: sca3000: reuse device pointer for devm helpers
2026-02-03 12:20 [PATCH v3 next 0/6] IIO: sca3000: devm resource management Harshit Mogalapalli
2026-02-03 12:20 ` [PATCH v3 next 1/6] iio: sca3000: cache SPI device ID in probe Harshit Mogalapalli
@ 2026-02-03 12:20 ` Harshit Mogalapalli
2026-02-03 15:17 ` Andy Shevchenko
2026-02-03 12:20 ` [PATCH v3 next 3/6] iio: sca3000: switch IRQ handling to " Harshit Mogalapalli
` (3 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Harshit Mogalapalli @ 2026-02-03 12:20 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Harshit Mogalapalli, Antoniu Miclaus, Andrew Ijano, linux-iio,
linux-kernel
Cc: kernel-janitors, error27, andriy.shevchenko
Cache struct device *dev and feed it to the devm helpers to simplify
the probe function. No functional changes.
Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
v2 -> v3: Split this dev caching into a separate patch [Thanks David for
the suggestion ]
---
drivers/iio/accel/sca3000.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
index a0b431aff024..afe6ef61a53b 100644
--- a/drivers/iio/accel/sca3000.c
+++ b/drivers/iio/accel/sca3000.c
@@ -1440,11 +1440,12 @@ static const struct iio_info sca3000_info = {
static int sca3000_probe(struct spi_device *spi)
{
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;
@@ -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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 next 3/6] iio: sca3000: switch IRQ handling to devm helpers
2026-02-03 12:20 [PATCH v3 next 0/6] IIO: sca3000: devm resource management Harshit Mogalapalli
2026-02-03 12:20 ` [PATCH v3 next 1/6] iio: sca3000: cache SPI device ID in probe Harshit Mogalapalli
2026-02-03 12:20 ` [PATCH v3 next 2/6] iio: sca3000: reuse device pointer for devm helpers Harshit Mogalapalli
@ 2026-02-03 12:20 ` Harshit Mogalapalli
2026-02-03 15:20 ` Andy Shevchenko
2026-02-03 15:21 ` Andy Shevchenko
2026-02-03 12:20 ` [PATCH v3 next 4/6] iio: sca3000: stop interrupts via devm_add_action_or_reset() Harshit Mogalapalli
` (2 subsequent siblings)
5 siblings, 2 replies; 19+ messages in thread
From: Harshit Mogalapalli @ 2026-02-03 12:20 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Harshit Mogalapalli, Antoniu Miclaus, Andrew Ijano, linux-iio,
linux-kernel
Cc: kernel-janitors, error27, andriy.shevchenko, David Lechner
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@baylibe.com>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
v2->v3: No changes, add RB from David.
---
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 next 4/6] iio: sca3000: stop interrupts via devm_add_action_or_reset()
2026-02-03 12:20 [PATCH v3 next 0/6] IIO: sca3000: devm resource management Harshit Mogalapalli
` (2 preceding siblings ...)
2026-02-03 12:20 ` [PATCH v3 next 3/6] iio: sca3000: switch IRQ handling to " Harshit Mogalapalli
@ 2026-02-03 12:20 ` Harshit Mogalapalli
2026-02-03 15:24 ` Andy Shevchenko
2026-02-03 12:20 ` [PATCH v3 next 5/6] iio: sca3000: manage device registration with devm helper Harshit Mogalapalli
2026-02-03 12:20 ` [PATCH v3 next 6/6] iio: sca3000: use guard(mutex) to simplify return paths Harshit Mogalapalli
5 siblings, 1 reply; 19+ messages in thread
From: Harshit Mogalapalli @ 2026-02-03 12:20 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Harshit Mogalapalli, Antoniu Miclaus, Andrew Ijano, linux-iio,
linux-kernel
Cc: kernel-janitors, error27, andriy.shevchenko, David Lechner
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@baylibe.com>
Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
v2->v3: Don't add a new unneeded wrapper around
sca3000_stop_all_interrupts() and make it a void function.
---
drivers/iio/accel/sca3000.c | 46 +++++++++++++++++++------------------
1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
index 37ef724d5dc5..b89439756eda 100644
--- a/drivers/iio/accel/sca3000.c
+++ b/drivers/iio/accel/sca3000.c
@@ -1437,6 +1437,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;
+
+ mutex_lock(&st->lock);
+ ret = sca3000_read_data_short(st, SCA3000_REG_INT_MASK_ADDR, 1);
+ if (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);
+}
+
static int sca3000_probe(struct spi_device *spi)
{
const struct spi_device_id *id = spi_get_device_id(spi);
@@ -1489,6 +1508,11 @@ static int sca3000_probe(struct spi_device *spi)
if (ret)
return ret;
+ ret = devm_add_action_or_reset(dev, sca3000_stop_all_interrupts,
+ indio_dev);
+ if (ret)
+ return ret;
+
ret = iio_device_register(indio_dev);
if (ret)
return ret;
@@ -1496,33 +1520,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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 next 5/6] iio: sca3000: manage device registration with devm helper
2026-02-03 12:20 [PATCH v3 next 0/6] IIO: sca3000: devm resource management Harshit Mogalapalli
` (3 preceding siblings ...)
2026-02-03 12:20 ` [PATCH v3 next 4/6] iio: sca3000: stop interrupts via devm_add_action_or_reset() Harshit Mogalapalli
@ 2026-02-03 12:20 ` Harshit Mogalapalli
2026-02-03 15:23 ` Andy Shevchenko
2026-02-03 12:20 ` [PATCH v3 next 6/6] iio: sca3000: use guard(mutex) to simplify return paths Harshit Mogalapalli
5 siblings, 1 reply; 19+ messages in thread
From: Harshit Mogalapalli @ 2026-02-03 12:20 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Harshit Mogalapalli, Antoniu Miclaus, Andrew Ijano, linux-iio,
linux-kernel
Cc: kernel-janitors, error27, andriy.shevchenko, David Lechner
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@baylibe.com>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
v2->v3: No changes, add RB from David.
---
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 b89439756eda..419c30453805 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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 next 6/6] iio: sca3000: use guard(mutex) to simplify return paths
2026-02-03 12:20 [PATCH v3 next 0/6] IIO: sca3000: devm resource management Harshit Mogalapalli
` (4 preceding siblings ...)
2026-02-03 12:20 ` [PATCH v3 next 5/6] iio: sca3000: manage device registration with devm helper Harshit Mogalapalli
@ 2026-02-03 12:20 ` Harshit Mogalapalli
2026-02-03 15:23 ` Andy Shevchenko
5 siblings, 1 reply; 19+ messages in thread
From: Harshit Mogalapalli @ 2026-02-03 12:20 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Harshit Mogalapalli, Andrew Ijano, Antoniu Miclaus, linux-iio,
linux-kernel
Cc: kernel-janitors, error27, andriy.shevchenko
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>
---
v2->v3: Had to rewrite this patch as we have changes in patch3, Also
mention using guard() based auto cleanup simplifies error path( Thanks
to David for the suggestion ) -- didn't include RB of David from v2 as
the code is slightly different now.
---
drivers/iio/accel/sca3000.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
index 419c30453805..307e51279cb6 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>
@@ -1443,17 +1444,15 @@ static void sca3000_stop_all_interrupts(void *data)
struct sca3000_state *st = iio_priv(indio_dev);
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;
- 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);
+ if (!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)));
}
static int sca3000_probe(struct spi_device *spi)
--
2.47.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3 next 2/6] iio: sca3000: reuse device pointer for devm helpers
2026-02-03 12:20 ` [PATCH v3 next 2/6] iio: sca3000: reuse device pointer for devm helpers Harshit Mogalapalli
@ 2026-02-03 15:17 ` Andy Shevchenko
2026-02-03 15:59 ` Harshit Mogalapalli
0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2026-02-03 15:17 UTC (permalink / raw)
To: Harshit Mogalapalli
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Antoniu Miclaus, Andrew Ijano, linux-iio, linux-kernel,
kernel-janitors, error27
On Tue, Feb 03, 2026 at 04:20:46AM -0800, Harshit Mogalapalli wrote:
> Cache struct device *dev and feed it to the devm helpers to simplify
> the probe function. No functional changes.
With or without below being addressed,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
...
> - ret = devm_iio_kfifo_buffer_setup(&spi->dev, indio_dev,
> + ret = devm_iio_kfifo_buffer_setup(dev, indio_dev,
> &sca3000_ring_setup_ops);
Now make it a single line, it's just 83 characters.
> if (ret)
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 next 1/6] iio: sca3000: cache SPI device ID in probe
2026-02-03 12:20 ` [PATCH v3 next 1/6] iio: sca3000: cache SPI device ID in probe Harshit Mogalapalli
@ 2026-02-03 15:19 ` Andy Shevchenko
2026-02-03 15:55 ` Harshit Mogalapalli
0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2026-02-03 15:19 UTC (permalink / raw)
To: Harshit Mogalapalli
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Antoniu Miclaus, Andrew Ijano, linux-iio, linux-kernel,
kernel-janitors, error27
On Tue, Feb 03, 2026 at 04:20:45AM -0800, 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.
>
> While at it, reshuffle variable list in a reverse Christmas tree
> order. No functional change intended.
Did you miss David's comment? AFAIU the suggestion was to convert to
chip_info(). With that done, this should use spi_device_get_match_data()
(or something like that, I don't remember the correct spelling of that API).
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 next 3/6] iio: sca3000: switch IRQ handling to devm helpers
2026-02-03 12:20 ` [PATCH v3 next 3/6] iio: sca3000: switch IRQ handling to " Harshit Mogalapalli
@ 2026-02-03 15:20 ` Andy Shevchenko
2026-02-03 15:21 ` Andy Shevchenko
1 sibling, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2026-02-03 15:20 UTC (permalink / raw)
To: Harshit Mogalapalli
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Antoniu Miclaus, Andrew Ijano, linux-iio, linux-kernel,
kernel-janitors, error27, David Lechner
On Tue, Feb 03, 2026 at 04:20:47AM -0800, 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.
...
> if (spi->irq) {
Hmm, shouldn't this be ' > 0' check?
> - 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;
> }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 next 3/6] iio: sca3000: switch IRQ handling to devm helpers
2026-02-03 12:20 ` [PATCH v3 next 3/6] iio: sca3000: switch IRQ handling to " Harshit Mogalapalli
2026-02-03 15:20 ` Andy Shevchenko
@ 2026-02-03 15:21 ` Andy Shevchenko
1 sibling, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2026-02-03 15:21 UTC (permalink / raw)
To: Harshit Mogalapalli
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Antoniu Miclaus, Andrew Ijano, linux-iio, linux-kernel,
kernel-janitors, error27, David Lechner
On Tue, Feb 03, 2026 at 04:20:47AM -0800, 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.
Right, taking this into account my other reply can be addressed separately.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 next 6/6] iio: sca3000: use guard(mutex) to simplify return paths
2026-02-03 12:20 ` [PATCH v3 next 6/6] iio: sca3000: use guard(mutex) to simplify return paths Harshit Mogalapalli
@ 2026-02-03 15:23 ` Andy Shevchenko
2026-02-03 16:01 ` Harshit Mogalapalli
0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2026-02-03 15:23 UTC (permalink / raw)
To: Harshit Mogalapalli
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Andrew Ijano, Antoniu Miclaus, linux-iio, linux-kernel,
kernel-janitors, error27
On Tue, Feb 03, 2026 at 04:20:50AM -0800, Harshit Mogalapalli wrote:
> Switch sca3000_stop_all_interrupts() to use guard(mutex) to simplify the
> error paths without needing a goto.
...
> - if (ret)
> + if (!ret)
Leave the standard pattern to check for errors first.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 next 5/6] iio: sca3000: manage device registration with devm helper
2026-02-03 12:20 ` [PATCH v3 next 5/6] iio: sca3000: manage device registration with devm helper Harshit Mogalapalli
@ 2026-02-03 15:23 ` Andy Shevchenko
0 siblings, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2026-02-03 15:23 UTC (permalink / raw)
To: Harshit Mogalapalli
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Antoniu Miclaus, Andrew Ijano, linux-iio, linux-kernel,
kernel-janitors, error27, David Lechner
On Tue, Feb 03, 2026 at 04:20:49AM -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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 next 4/6] iio: sca3000: stop interrupts via devm_add_action_or_reset()
2026-02-03 12:20 ` [PATCH v3 next 4/6] iio: sca3000: stop interrupts via devm_add_action_or_reset() Harshit Mogalapalli
@ 2026-02-03 15:24 ` Andy Shevchenko
2026-02-03 15:59 ` Harshit Mogalapalli
0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2026-02-03 15:24 UTC (permalink / raw)
To: Harshit Mogalapalli
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Antoniu Miclaus, Andrew Ijano, linux-iio, linux-kernel,
kernel-janitors, error27, David Lechner
On Tue, Feb 03, 2026 at 04:20:48AM -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.
...
> + mutex_lock(&st->lock);
> +out_unlock:
> + mutex_unlock(&st->lock);
You copy the code that is going to be removed. It means the conversion to
guard()() should be done before this patch.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 next 1/6] iio: sca3000: cache SPI device ID in probe
2026-02-03 15:19 ` Andy Shevchenko
@ 2026-02-03 15:55 ` Harshit Mogalapalli
2026-02-03 17:54 ` Andy Shevchenko
0 siblings, 1 reply; 19+ messages in thread
From: Harshit Mogalapalli @ 2026-02-03 15:55 UTC (permalink / raw)
To: Andy Shevchenko, David Lechner
Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Antoniu Miclaus,
Andrew Ijano, linux-iio, linux-kernel, kernel-janitors, error27
Hi Andy,
On 03/02/26 20:49, Andy Shevchenko wrote:
> On Tue, Feb 03, 2026 at 04:20:45AM -0800, 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.
>>
>> While at it, reshuffle variable list in a reverse Christmas tree
>> order. No functional change intended.
>
> Did you miss David's comment? AFAIU the suggestion was to convert to
> chip_info(). With that done, this should use spi_device_get_match_data()
> (or something like that, I don't remember the correct spelling of that API).
>
I was thinking of dealing with it in a separate series(as documented in
the cover letter). Also David pointed out that if I am planning to do
that conversion separate, there is no neccesity of this patch, as it is
anyway going to be replaced soon when I have that ready. So I will drop
this simplification in my next version and leave the conversion to
spi_device_get_match_data() to another patch set. Will also include your
"irq > 0" check fixup in the following series. Hope you are okay with that ?
Thanks a lot for the review!
Regards,
Harshit
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 next 2/6] iio: sca3000: reuse device pointer for devm helpers
2026-02-03 15:17 ` Andy Shevchenko
@ 2026-02-03 15:59 ` Harshit Mogalapalli
0 siblings, 0 replies; 19+ messages in thread
From: Harshit Mogalapalli @ 2026-02-03 15:59 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Antoniu Miclaus, Andrew Ijano, linux-iio, linux-kernel,
kernel-janitors, error27
On 03/02/26 20:47, Andy Shevchenko wrote:
> On Tue, Feb 03, 2026 at 04:20:46AM -0800, Harshit Mogalapalli wrote:
>> Cache struct device *dev and feed it to the devm helpers to simplify
>> the probe function. No functional changes.
>
> With or without below being addressed,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>
> ...
>
>> - ret = devm_iio_kfifo_buffer_setup(&spi->dev, indio_dev,
>> + ret = devm_iio_kfifo_buffer_setup(dev, indio_dev,
>> &sca3000_ring_setup_ops);
>
> Now make it a single line, it's just 83 characters.
>
Sure, will do. I thought the wrap limit is 80, but there are other
instances in this file where the columns exceeded 80. Will address this
in my next version.
Thanks!
Regards,
Harshit
>> if (ret)
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 next 4/6] iio: sca3000: stop interrupts via devm_add_action_or_reset()
2026-02-03 15:24 ` Andy Shevchenko
@ 2026-02-03 15:59 ` Harshit Mogalapalli
0 siblings, 0 replies; 19+ messages in thread
From: Harshit Mogalapalli @ 2026-02-03 15:59 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Antoniu Miclaus, Andrew Ijano, linux-iio, linux-kernel,
kernel-janitors, error27, David Lechner
Hi Andy,
On 03/02/26 20:54, Andy Shevchenko wrote:
> On Tue, Feb 03, 2026 at 04:20:48AM -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.
>
> ...
>
>> + mutex_lock(&st->lock);
>
>> +out_unlock:
>> + mutex_unlock(&st->lock);
>
> You copy the code that is going to be removed. It means the conversion to
> guard()() should be done before this patch.
>
Sure will reorder the patches that way in my next version, thanks for
the suggestion.
Regards,
Harshit
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 next 6/6] iio: sca3000: use guard(mutex) to simplify return paths
2026-02-03 15:23 ` Andy Shevchenko
@ 2026-02-03 16:01 ` Harshit Mogalapalli
0 siblings, 0 replies; 19+ messages in thread
From: Harshit Mogalapalli @ 2026-02-03 16:01 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Andrew Ijano, Antoniu Miclaus, linux-iio, linux-kernel,
kernel-janitors, error27
Hi Andy,
On 03/02/26 20:53, Andy Shevchenko wrote:
> On Tue, Feb 03, 2026 at 04:20:50AM -0800, Harshit Mogalapalli wrote:
>> Switch sca3000_stop_all_interrupts() to use guard(mutex) to simplify the
>> error paths without needing a goto.
>
> ...
>
>> - if (ret)
>
>> + if (!ret)
>
> Leave the standard pattern to check for errors first.
>
Thanks, I will do that, I agree it would look simpler that way.
Regards,
Harshit
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 next 1/6] iio: sca3000: cache SPI device ID in probe
2026-02-03 15:55 ` Harshit Mogalapalli
@ 2026-02-03 17:54 ` Andy Shevchenko
0 siblings, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2026-02-03 17:54 UTC (permalink / raw)
To: Harshit Mogalapalli
Cc: David Lechner, Jonathan Cameron, Nuno Sá, Andy Shevchenko,
Antoniu Miclaus, Andrew Ijano, linux-iio, linux-kernel,
kernel-janitors, error27
On Tue, Feb 03, 2026 at 09:25:08PM +0530, Harshit Mogalapalli wrote:
> On 03/02/26 20:49, Andy Shevchenko wrote:
> > On Tue, Feb 03, 2026 at 04:20:45AM -0800, 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.
> > >
> > > While at it, reshuffle variable list in a reverse Christmas tree
> > > order. No functional change intended.
> >
> > Did you miss David's comment? AFAIU the suggestion was to convert to
> > chip_info(). With that done, this should use spi_device_get_match_data()
> > (or something like that, I don't remember the correct spelling of that API).
>
> I was thinking of dealing with it in a separate series(as documented in the
> cover letter). Also David pointed out that if I am planning to do that
> conversion separate, there is no neccesity of this patch, as it is anyway
> going to be replaced soon when I have that ready. So I will drop this
> simplification in my next version and leave the conversion to
> spi_device_get_match_data() to another patch set. Will also include your
> "irq > 0" check fixup in the following series. Hope you are okay with that ?
As long as there is no added churn I'm fine with the approach.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2026-02-03 17:54 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-03 12:20 [PATCH v3 next 0/6] IIO: sca3000: devm resource management Harshit Mogalapalli
2026-02-03 12:20 ` [PATCH v3 next 1/6] iio: sca3000: cache SPI device ID in probe Harshit Mogalapalli
2026-02-03 15:19 ` Andy Shevchenko
2026-02-03 15:55 ` Harshit Mogalapalli
2026-02-03 17:54 ` Andy Shevchenko
2026-02-03 12:20 ` [PATCH v3 next 2/6] iio: sca3000: reuse device pointer for devm helpers Harshit Mogalapalli
2026-02-03 15:17 ` Andy Shevchenko
2026-02-03 15:59 ` Harshit Mogalapalli
2026-02-03 12:20 ` [PATCH v3 next 3/6] iio: sca3000: switch IRQ handling to " Harshit Mogalapalli
2026-02-03 15:20 ` Andy Shevchenko
2026-02-03 15:21 ` Andy Shevchenko
2026-02-03 12:20 ` [PATCH v3 next 4/6] iio: sca3000: stop interrupts via devm_add_action_or_reset() Harshit Mogalapalli
2026-02-03 15:24 ` Andy Shevchenko
2026-02-03 15:59 ` Harshit Mogalapalli
2026-02-03 12:20 ` [PATCH v3 next 5/6] iio: sca3000: manage device registration with devm helper Harshit Mogalapalli
2026-02-03 15:23 ` Andy Shevchenko
2026-02-03 12:20 ` [PATCH v3 next 6/6] iio: sca3000: use guard(mutex) to simplify return paths Harshit Mogalapalli
2026-02-03 15:23 ` Andy Shevchenko
2026-02-03 16:01 ` Harshit Mogalapalli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox