public inbox for kernel-janitors@vger.kernel.org
 help / color / mirror / Atom feed
* [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