[PATCH 00/10] Use ADIS devm functions for trigger/buffer setup

[PATCH 00/10] Use ADIS devm functions for trigger/buffer setup

[Nuno Sá]

This series updates all users of `adis_setup_buffer_and_trigger()` to
use the devm version of it. The ultimate goal of the series is to drop
it from the lib as stated by Jonathan in [1].

[1]: https://marc.info/?l=linux-iio&m=158326796924980&w=2

Nuno Sá (10):
  iio: adis16201: Use Managed device functions
  iio: adis16209: Use Managed device functions
  iio: adis16136: Use Managed device functions
  iio: adis16260: Use Managed device functions
  iio: adis16400: Use Managed device functions
  iio: adis16460: Use Managed device functions
  iio: adis16480: Use Managed device functions
  staging: iio: adis16203: Use Managed device functions
  staging: iio: adis16240: Use Managed device functions
  iio: adis: Drop non Managed device functions

 drivers/iio/accel/adis16201.c         | 16 ++-----
 drivers/iio/accel/adis16209.c         | 15 ++-----
 drivers/iio/gyro/adis16136.c          |  9 +---
 drivers/iio/gyro/adis16260.c          | 15 ++-----
 drivers/iio/imu/adis16400.c           | 15 ++-----
 drivers/iio/imu/adis16460.c           | 13 ++----
 drivers/iio/imu/adis16480.c           |  7 +--
 drivers/iio/imu/adis_buffer.c         | 64 ++-------------------------
 drivers/iio/imu/adis_trigger.c        | 60 -------------------------
 drivers/staging/iio/accel/adis16203.c | 16 ++-----
 drivers/staging/iio/accel/adis16240.c | 14 ++----
 include/linux/iio/imu/adis.h          | 27 -----------
 12 files changed, 30 insertions(+), 241 deletions(-)

-- 
2.28.0

[PATCH 01/10] iio: adis16201: Use Managed device functions

[Nuno Sá]

Use the adis managed device functions to setup the buffer and the trigger.
The ultimate goal will be to completely drop the non devm version from
the lib.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/accel/adis16201.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/accel/adis16201.c b/drivers/iio/accel/adis16201.c
index 59a24c355a1a..a375ec25448a 100644
--- a/drivers/iio/accel/adis16201.c
+++ b/drivers/iio/accel/adis16201.c
@@ -281,32 +281,22 @@ static int adis16201_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = adis_setup_buffer_and_trigger(st, indio_dev, NULL);
+	ret = devm_adis_setup_buffer_and_trigger(st, indio_dev, NULL);
 	if (ret)
 		return ret;
 
 	ret = adis_initial_startup(st);
 	if (ret)
-		goto error_cleanup_buffer_trigger;
-
-	ret = iio_device_register(indio_dev);
-	if (ret < 0)
-		goto error_cleanup_buffer_trigger;
-
-	return 0;
+		return ret;
 
-error_cleanup_buffer_trigger:
-	adis_cleanup_buffer_and_trigger(st, indio_dev);
-	return ret;
+	return iio_device_register(indio_dev);
 }
 
 static int adis16201_remove(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct adis *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
-	adis_cleanup_buffer_and_trigger(st, indio_dev);
 
 	return 0;
 }
-- 
2.28.0

[PATCH 02/10] iio: adis16209: Use Managed device functions

[Nuno Sá]

Use the adis managed device functions to setup the buffer and the trigger.
The ultimate goal will be to completely drop the non devm version from
the lib.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/accel/adis16209.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/accel/adis16209.c b/drivers/iio/accel/adis16209.c
index 3d5538e2f76e..2410d8470451 100644
--- a/drivers/iio/accel/adis16209.c
+++ b/drivers/iio/accel/adis16209.c
@@ -291,31 +291,22 @@ static int adis16209_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = adis_setup_buffer_and_trigger(st, indio_dev, NULL);
+	ret = devm_adis_setup_buffer_and_trigger(st, indio_dev, NULL);
 	if (ret)
 		return ret;
 
 	ret = adis_initial_startup(st);
 	if (ret)
-		goto error_cleanup_buffer_trigger;
-	ret = iio_device_register(indio_dev);
-	if (ret)
-		goto error_cleanup_buffer_trigger;
-
-	return 0;
+		return ret;
 
-error_cleanup_buffer_trigger:
-	adis_cleanup_buffer_and_trigger(st, indio_dev);
-	return ret;
+	return iio_device_register(indio_dev);
 }
 
 static int adis16209_remove(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct adis *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
-	adis_cleanup_buffer_and_trigger(st, indio_dev);
 
 	return 0;
 }
-- 
2.28.0

[PATCH 03/10] iio: adis16136: Use Managed device functions

[Nuno Sá]

Use the adis managed device functions to setup the buffer and the trigger.
The ultimate goal will be to completely drop the non devm version from
the lib.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/gyro/adis16136.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/gyro/adis16136.c b/drivers/iio/gyro/adis16136.c
index d8a96f6bbae2..a4b060d9d23f 100644
--- a/drivers/iio/gyro/adis16136.c
+++ b/drivers/iio/gyro/adis16136.c
@@ -552,13 +552,13 @@ static int adis16136_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = adis_setup_buffer_and_trigger(&adis16136->adis, indio_dev, NULL);
+	ret = devm_adis_setup_buffer_and_trigger(&adis16136->adis, indio_dev, NULL);
 	if (ret)
 		return ret;
 
 	ret = adis16136_initial_setup(indio_dev);
 	if (ret)
-		goto error_cleanup_buffer;
+		return ret;
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
@@ -570,21 +570,16 @@ static int adis16136_probe(struct spi_device *spi)
 
 error_stop_device:
 	adis16136_stop_device(indio_dev);
-error_cleanup_buffer:
-	adis_cleanup_buffer_and_trigger(&adis16136->adis, indio_dev);
 	return ret;
 }
 
 static int adis16136_remove(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct adis16136 *adis16136 = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
 	adis16136_stop_device(indio_dev);
 
-	adis_cleanup_buffer_and_trigger(&adis16136->adis, indio_dev);
-
 	return 0;
 }
 
-- 
2.28.0

[PATCH 04/10] iio: adis16260: Use Managed device functions

[Nuno Sá]

Use the adis managed device functions to setup the buffer and the trigger.
The ultimate goal will be to completely drop the non devm version from
the lib.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/gyro/adis16260.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/gyro/adis16260.c b/drivers/iio/gyro/adis16260.c
index e638d56e1574..a515925e3356 100644
--- a/drivers/iio/gyro/adis16260.c
+++ b/drivers/iio/gyro/adis16260.c
@@ -390,33 +390,24 @@ static int adis16260_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = adis_setup_buffer_and_trigger(&adis16260->adis, indio_dev, NULL);
+	ret = devm_adis_setup_buffer_and_trigger(&adis16260->adis, indio_dev, NULL);
 	if (ret)
 		return ret;
 
 	/* Get the device into a sane initial state */
 	ret = adis_initial_startup(&adis16260->adis);
 	if (ret)
-		goto error_cleanup_buffer_trigger;
-	ret = iio_device_register(indio_dev);
-	if (ret)
-		goto error_cleanup_buffer_trigger;
-
-	return 0;
+		return ret;
 
-error_cleanup_buffer_trigger:
-	adis_cleanup_buffer_and_trigger(&adis16260->adis, indio_dev);
-	return ret;
+	return iio_device_register(indio_dev);
 }
 
 static int adis16260_remove(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct adis16260 *adis16260 = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
 	adis16260_stop_device(indio_dev);
-	adis_cleanup_buffer_and_trigger(&adis16260->adis, indio_dev);
 
 	return 0;
 }
-- 
2.28.0

[PATCH 05/10] iio: adis16400: Use Managed device functions

[Nuno Sá]

Use the adis managed device functions to setup the buffer and the trigger.
The ultimate goal will be to completely drop the non devm version from
the lib.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/imu/adis16400.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/imu/adis16400.c b/drivers/iio/imu/adis16400.c
index 1ebe3e50d3e6..7bf4b0065022 100644
--- a/drivers/iio/imu/adis16400.c
+++ b/drivers/iio/imu/adis16400.c
@@ -1201,37 +1201,30 @@ static int adis16400_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = adis_setup_buffer_and_trigger(&st->adis, indio_dev,
-			adis16400_trigger_handler);
+	ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev, adis16400_trigger_handler);
 	if (ret)
 		return ret;
 
 	/* Get the device into a sane initial state */
 	ret = adis16400_initial_setup(indio_dev);
 	if (ret)
-		goto error_cleanup_buffer;
+		return ret;
+
 	ret = iio_device_register(indio_dev);
 	if (ret)
-		goto error_cleanup_buffer;
+		return ret;
 
 	adis16400_debugfs_init(indio_dev);
 	return 0;
-
-error_cleanup_buffer:
-	adis_cleanup_buffer_and_trigger(&st->adis, indio_dev);
-	return ret;
 }
 
 static int adis16400_remove(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct adis16400_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
 	adis16400_stop_device(indio_dev);
 
-	adis_cleanup_buffer_and_trigger(&st->adis, indio_dev);
-
 	return 0;
 }
 
-- 
2.28.0

[PATCH 06/10] iio: adis16460: Use Managed device functions

[Nuno Sá]

Use the adis managed device functions to setup the buffer and the trigger.
The ultimate goal will be to completely drop the non devm version from
the lib.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/imu/adis16460.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c
index b26a5f1bc51a..5c04cef173a8 100644
--- a/drivers/iio/imu/adis16460.c
+++ b/drivers/iio/imu/adis16460.c
@@ -403,7 +403,7 @@ static int adis16460_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL);
+	ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL);
 	if (ret)
 		return ret;
 
@@ -411,30 +411,23 @@ static int adis16460_probe(struct spi_device *spi)
 
 	ret = __adis_initial_startup(&st->adis);
 	if (ret)
-		goto error_cleanup_buffer;
+		return ret;
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
-		goto error_cleanup_buffer;
+		return ret;
 
 	adis16460_debugfs_init(indio_dev);
 
 	return 0;
-
-error_cleanup_buffer:
-	adis_cleanup_buffer_and_trigger(&st->adis, indio_dev);
-	return ret;
 }
 
 static int adis16460_remove(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct adis16460 *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
 
-	adis_cleanup_buffer_and_trigger(&st->adis, indio_dev);
-
 	return 0;
 }
 
-- 
2.28.0

[PATCH 07/10] iio: adis16480: Use Managed device functions

[Nuno Sá]

Use the adis managed device functions to setup the buffer and the trigger.
The ultimate goal will be to completely drop the non devm version from
the lib.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/imu/adis16480.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
index 1eb4f98076f1..b6a129a70d4b 100644
--- a/drivers/iio/imu/adis16480.c
+++ b/drivers/iio/imu/adis16480.c
@@ -1264,20 +1264,18 @@ static int adis16480_probe(struct spi_device *spi)
 		st->clk_freq = st->chip_info->int_clk;
 	}
 
-	ret = adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL);
+	ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL);
 	if (ret)
 		goto error_clk_disable_unprepare;
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
-		goto error_cleanup_buffer;
+		goto error_clk_disable_unprepare;
 
 	adis16480_debugfs_init(indio_dev);
 
 	return 0;
 
-error_cleanup_buffer:
-	adis_cleanup_buffer_and_trigger(&st->adis, indio_dev);
 error_clk_disable_unprepare:
 	clk_disable_unprepare(st->ext_clk);
 error_stop_device:
@@ -1293,7 +1291,6 @@ static int adis16480_remove(struct spi_device *spi)
 	iio_device_unregister(indio_dev);
 	adis16480_stop_device(indio_dev);
 
-	adis_cleanup_buffer_and_trigger(&st->adis, indio_dev);
 	clk_disable_unprepare(st->ext_clk);
 
 	return 0;
-- 
2.28.0

[PATCH 08/10] staging: iio: adis16203: Use Managed device functions

[Nuno Sá]

Use the adis managed device functions to setup the buffer and the trigger.
The ultimate goal will be to completely drop the non devm version from
the lib.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/staging/iio/accel/adis16203.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/iio/accel/adis16203.c b/drivers/staging/iio/accel/adis16203.c
index c7798908ef0e..49362017d2e6 100644
--- a/drivers/staging/iio/accel/adis16203.c
+++ b/drivers/staging/iio/accel/adis16203.c
@@ -286,33 +286,23 @@ static int adis16203_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = adis_setup_buffer_and_trigger(st, indio_dev, NULL);
+	ret = devm_adis_setup_buffer_and_trigger(st, indio_dev, NULL);
 	if (ret)
 		return ret;
 
 	/* Get the device into a sane initial state */
 	ret = adis_initial_startup(st);
 	if (ret)
-		goto error_cleanup_buffer_trigger;
-
-	ret = iio_device_register(indio_dev);
-	if (ret)
-		goto error_cleanup_buffer_trigger;
-
-	return 0;
+		return ret;
 
-error_cleanup_buffer_trigger:
-	adis_cleanup_buffer_and_trigger(st, indio_dev);
-	return ret;
+	return iio_device_register(indio_dev);
 }
 
 static int adis16203_remove(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct adis *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
-	adis_cleanup_buffer_and_trigger(st, indio_dev);
 
 	return 0;
 }
-- 
2.28.0

[PATCH 09/10] staging: iio: adis16240: Use Managed device functions

[Nuno Sá]

Use the adis managed device functions to setup the buffer and the trigger.
The ultimate goal will be to completely drop the non devm version from
the lib.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/staging/iio/accel/adis16240.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/iio/accel/adis16240.c b/drivers/staging/iio/accel/adis16240.c
index 38ec40b458c9..26b57f0a3917 100644
--- a/drivers/staging/iio/accel/adis16240.c
+++ b/drivers/staging/iio/accel/adis16240.c
@@ -415,31 +415,23 @@ static int adis16240_probe(struct spi_device *spi)
 	ret = adis_init(st, indio_dev, spi, &adis16240_data);
 	if (ret)
 		return ret;
-	ret = adis_setup_buffer_and_trigger(st, indio_dev, NULL);
+	ret = devm_adis_setup_buffer_and_trigger(st, indio_dev, NULL);
 	if (ret)
 		return ret;
 
 	/* Get the device into a sane initial state */
 	ret = adis_initial_startup(st);
 	if (ret)
-		goto error_cleanup_buffer_trigger;
-	ret = iio_device_register(indio_dev);
-	if (ret)
-		goto error_cleanup_buffer_trigger;
-	return 0;
+		return ret;
 
-error_cleanup_buffer_trigger:
-	adis_cleanup_buffer_and_trigger(st, indio_dev);
-	return ret;
+	return iio_device_register(indio_dev);
 }
 
 static int adis16240_remove(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct adis *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
-	adis_cleanup_buffer_and_trigger(st, indio_dev);
 
 	return 0;
 }
-- 
2.28.0

[PATCH 10/10] iio: adis: Drop non Managed device functions

[Nuno Sá]

Drop `adis_setup_buffer_and_trigger()`. All users were updated to use
the devm version of this function. This avoids having almost the same
code repeated.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/imu/adis_buffer.c  | 64 +++-------------------------------
 drivers/iio/imu/adis_trigger.c | 60 -------------------------------
 include/linux/iio/imu/adis.h   | 27 --------------
 3 files changed, 4 insertions(+), 147 deletions(-)

diff --git a/drivers/iio/imu/adis_buffer.c b/drivers/iio/imu/adis_buffer.c
index 5b4225ee09b9..df6144285470 100644
--- a/drivers/iio/imu/adis_buffer.c
+++ b/drivers/iio/imu/adis_buffer.c
@@ -169,48 +169,6 @@ static void adis_buffer_cleanup(void *arg)
 	kfree(adis->xfer);
 }
 
-/**
- * adis_setup_buffer_and_trigger() - Sets up buffer and trigger for the adis device
- * @adis: The adis device.
- * @indio_dev: The IIO device.
- * @trigger_handler: Optional trigger handler, may be NULL.
- *
- * Returns 0 on success, a negative error code otherwise.
- *
- * This function sets up the buffer and trigger for a adis devices.  If
- * 'trigger_handler' is NULL the default trigger handler will be used. The
- * default trigger handler will simply read the registers assigned to the
- * currently active channels.
- *
- * adis_cleanup_buffer_and_trigger() should be called to free the resources
- * allocated by this function.
- */
-int adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev,
-	irqreturn_t (*trigger_handler)(int, void *))
-{
-	int ret;
-
-	if (!trigger_handler)
-		trigger_handler = adis_trigger_handler;
-
-	ret = iio_triggered_buffer_setup(indio_dev, &iio_pollfunc_store_time,
-		trigger_handler, NULL);
-	if (ret)
-		return ret;
-
-	if (adis->spi->irq) {
-		ret = adis_probe_trigger(adis, indio_dev);
-		if (ret)
-			goto error_buffer_cleanup;
-	}
-	return 0;
-
-error_buffer_cleanup:
-	iio_triggered_buffer_cleanup(indio_dev);
-	return ret;
-}
-EXPORT_SYMBOL_GPL(adis_setup_buffer_and_trigger);
-
 /**
  * devm_adis_setup_buffer_and_trigger() - Sets up buffer and trigger for
  *					  the managed adis device
@@ -220,7 +178,10 @@ EXPORT_SYMBOL_GPL(adis_setup_buffer_and_trigger);
  *
  * Returns 0 on success, a negative error code otherwise.
  *
- * This function perfoms exactly the same as adis_setup_buffer_and_trigger()
+ * This function sets up the buffer and trigger for a adis devices.  If
+ * 'trigger_handler' is NULL the default trigger handler will be used. The
+ * default trigger handler will simply read the registers assigned to the
+ * currently active channels.
  */
 int
 devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev,
@@ -248,20 +209,3 @@ devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev,
 }
 EXPORT_SYMBOL_GPL(devm_adis_setup_buffer_and_trigger);
 
-/**
- * adis_cleanup_buffer_and_trigger() - Free buffer and trigger resources
- * @adis: The adis device.
- * @indio_dev: The IIO device.
- *
- * Frees resources allocated by adis_setup_buffer_and_trigger()
- */
-void adis_cleanup_buffer_and_trigger(struct adis *adis,
-	struct iio_dev *indio_dev)
-{
-	if (adis->spi->irq)
-		adis_remove_trigger(adis);
-	kfree(adis->buffer);
-	kfree(adis->xfer);
-	iio_triggered_buffer_cleanup(indio_dev);
-}
-EXPORT_SYMBOL_GPL(adis_cleanup_buffer_and_trigger);
diff --git a/drivers/iio/imu/adis_trigger.c b/drivers/iio/imu/adis_trigger.c
index 8afe71947c00..64e0ba51cb18 100644
--- a/drivers/iio/imu/adis_trigger.c
+++ b/drivers/iio/imu/adis_trigger.c
@@ -55,53 +55,6 @@ static int adis_validate_irq_flag(struct adis *adis)
 
 	return 0;
 }
-/**
- * adis_probe_trigger() - Sets up trigger for a adis device
- * @adis: The adis device
- * @indio_dev: The IIO device
- *
- * Returns 0 on success or a negative error code
- *
- * adis_remove_trigger() should be used to free the trigger.
- */
-int adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev)
-{
-	int ret;
-
-	adis->trig = iio_trigger_alloc("%s-dev%d", indio_dev->name,
-					indio_dev->id);
-	if (adis->trig == NULL)
-		return -ENOMEM;
-
-	adis_trigger_setup(adis);
-
-	ret = adis_validate_irq_flag(adis);
-	if (ret)
-		return ret;
-
-	ret = request_irq(adis->spi->irq,
-			  &iio_trigger_generic_data_rdy_poll,
-			  adis->irq_flag,
-			  indio_dev->name,
-			  adis->trig);
-	if (ret)
-		goto error_free_trig;
-
-	ret = iio_trigger_register(adis->trig);
-
-	indio_dev->trig = iio_trigger_get(adis->trig);
-	if (ret)
-		goto error_free_irq;
-
-	return 0;
-
-error_free_irq:
-	free_irq(adis->spi->irq, adis->trig);
-error_free_trig:
-	iio_trigger_free(adis->trig);
-	return ret;
-}
-EXPORT_SYMBOL_GPL(adis_probe_trigger);
 
 /**
  * devm_adis_probe_trigger() - Sets up trigger for a managed adis device
@@ -137,16 +90,3 @@ int devm_adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev)
 }
 EXPORT_SYMBOL_GPL(devm_adis_probe_trigger);
 
-/**
- * adis_remove_trigger() - Remove trigger for a adis devices
- * @adis: The adis device
- *
- * Removes the trigger previously registered with adis_probe_trigger().
- */
-void adis_remove_trigger(struct adis *adis)
-{
-	iio_trigger_unregister(adis->trig);
-	free_irq(adis->spi->irq, adis->trig);
-	iio_trigger_free(adis->trig);
-}
-EXPORT_SYMBOL_GPL(adis_remove_trigger);
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 2df67448f0d1..01ba691da2f3 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -517,14 +517,8 @@ struct adis_burst {
 int
 devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev,
 				   irq_handler_t trigger_handler);
-int adis_setup_buffer_and_trigger(struct adis *adis,
-	struct iio_dev *indio_dev, irqreturn_t (*trigger_handler)(int, void *));
-void adis_cleanup_buffer_and_trigger(struct adis *adis,
-	struct iio_dev *indio_dev);
 
 int devm_adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev);
-int adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev);
-void adis_remove_trigger(struct adis *adis);
 
 int adis_update_scan_mode(struct iio_dev *indio_dev,
 	const unsigned long *scan_mask);
@@ -538,33 +532,12 @@ devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev,
 	return 0;
 }
 
-static inline int adis_setup_buffer_and_trigger(struct adis *adis,
-	struct iio_dev *indio_dev, irqreturn_t (*trigger_handler)(int, void *))
-{
-	return 0;
-}
-
-static inline void adis_cleanup_buffer_and_trigger(struct adis *adis,
-	struct iio_dev *indio_dev)
-{
-}
-
 static inline int devm_adis_probe_trigger(struct adis *adis,
 					  struct iio_dev *indio_dev)
 {
 	return 0;
 }
 
-static inline int adis_probe_trigger(struct adis *adis,
-	struct iio_dev *indio_dev)
-{
-	return 0;
-}
-
-static inline void adis_remove_trigger(struct adis *adis)
-{
-}
-
 #define adis_update_scan_mode NULL
 
 #endif /* CONFIG_IIO_BUFFER */
-- 
2.28.0

Re: [PATCH 01/10] iio: adis16201: Use Managed device functions

[Jonathan Cameron]

On Tue, 15 Sep 2020 11:33:36 +0200
Nuno Sá <nuno.sa@analog.com> wrote:

> Use the adis managed device functions to setup the buffer and the trigger.
> The ultimate goal will be to completely drop the non devm version from
> the lib.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---
>  drivers/iio/accel/adis16201.c | 16 +++-------------
>  1 file changed, 3 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/accel/adis16201.c b/drivers/iio/accel/adis16201.c
> index 59a24c355a1a..a375ec25448a 100644
> --- a/drivers/iio/accel/adis16201.c
> +++ b/drivers/iio/accel/adis16201.c
> @@ -281,32 +281,22 @@ static int adis16201_probe(struct spi_device *spi)
>  	if (ret)
>  		return ret;
>  
> -	ret = adis_setup_buffer_and_trigger(st, indio_dev, NULL);
> +	ret = devm_adis_setup_buffer_and_trigger(st, indio_dev, NULL);
>  	if (ret)
>  		return ret;
>  
>  	ret = adis_initial_startup(st);
>  	if (ret)
> -		goto error_cleanup_buffer_trigger;
> -
> -	ret = iio_device_register(indio_dev);
> -	if (ret < 0)
> -		goto error_cleanup_buffer_trigger;
> -
> -	return 0;
> +		return ret;
>  
> -error_cleanup_buffer_trigger:
> -	adis_cleanup_buffer_and_trigger(st, indio_dev);
> -	return ret;
> +	return iio_device_register(indio_dev);
>  }
>  
>  static int adis16201_remove(struct spi_device *spi)
>  {
>  	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> -	struct adis *st = iio_priv(indio_dev);
>  
>  	iio_device_unregister(indio_dev);

If all you have left in remove is a call to iio_device_unregister()
why not just use devm_iio_device_register in probe and drop the
remove function entirely?

> -	adis_cleanup_buffer_and_trigger(st, indio_dev);
>  
>  	return 0;
>  }

Re: [PATCH 03/10] iio: adis16136: Use Managed device functions

[Jonathan Cameron]

On Tue, 15 Sep 2020 11:33:38 +0200
Nuno Sá <nuno.sa@analog.com> wrote:

> Use the adis managed device functions to setup the buffer and the trigger.
> The ultimate goal will be to completely drop the non devm version from
> the lib.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---
>  drivers/iio/gyro/adis16136.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/gyro/adis16136.c b/drivers/iio/gyro/adis16136.c
> index d8a96f6bbae2..a4b060d9d23f 100644
> --- a/drivers/iio/gyro/adis16136.c
> +++ b/drivers/iio/gyro/adis16136.c
> @@ -552,13 +552,13 @@ static int adis16136_probe(struct spi_device *spi)
>  	if (ret)
>  		return ret;
>  
> -	ret = adis_setup_buffer_and_trigger(&adis16136->adis, indio_dev, NULL);
> +	ret = devm_adis_setup_buffer_and_trigger(&adis16136->adis, indio_dev, NULL);
>  	if (ret)
>  		return ret;
>  
>  	ret = adis16136_initial_setup(indio_dev);
>  	if (ret)
> -		goto error_cleanup_buffer;
> +		return ret;
>  
>  	ret = iio_device_register(indio_dev);
>  	if (ret)
> @@ -570,21 +570,16 @@ static int adis16136_probe(struct spi_device *spi)
>  
>  error_stop_device:
>  	adis16136_stop_device(indio_dev);
> -error_cleanup_buffer:
> -	adis_cleanup_buffer_and_trigger(&adis16136->adis, indio_dev);
>  	return ret;
>  }
>  
>  static int adis16136_remove(struct spi_device *spi)
>  {
>  	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> -	struct adis16136 *adis16136 = iio_priv(indio_dev);
>  
>  	iio_device_unregister(indio_dev);
>  	adis16136_stop_device(indio_dev);

As adis16136_stop_device() is always run in remove, I'd suggest using
devm_add_action_or_reset at appropriate place in probe to use
the automated unrolling for that as well, letting you get rid of this
remove function.

>  
> -	adis_cleanup_buffer_and_trigger(&adis16136->adis, indio_dev);
> -
>  	return 0;
>  }
>  

Re: [PATCH 07/10] iio: adis16480: Use Managed device functions

[Jonathan Cameron]

On Tue, 15 Sep 2020 11:33:42 +0200
Nuno Sá <nuno.sa@analog.com> wrote:

> Use the adis managed device functions to setup the buffer and the trigger.
> The ultimate goal will be to completely drop the non devm version from
> the lib.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---
>  drivers/iio/imu/adis16480.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
> index 1eb4f98076f1..b6a129a70d4b 100644
> --- a/drivers/iio/imu/adis16480.c
> +++ b/drivers/iio/imu/adis16480.c
> @@ -1264,20 +1264,18 @@ static int adis16480_probe(struct spi_device *spi)
>  		st->clk_freq = st->chip_info->int_clk;
>  	}
>  
> -	ret = adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL);
> +	ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL);
>  	if (ret)
>  		goto error_clk_disable_unprepare;
>  
>  	ret = iio_device_register(indio_dev);
>  	if (ret)
> -		goto error_cleanup_buffer;
> +		goto error_clk_disable_unprepare;
>  
>  	adis16480_debugfs_init(indio_dev);
>  
>  	return 0;
>  
> -error_cleanup_buffer:
> -	adis_cleanup_buffer_and_trigger(&st->adis, indio_dev);
>  error_clk_disable_unprepare:
>  	clk_disable_unprepare(st->ext_clk);
>  error_stop_device:
> @@ -1293,7 +1291,6 @@ static int adis16480_remove(struct spi_device *spi)
>  	iio_device_unregister(indio_dev);
>  	adis16480_stop_device(indio_dev);
>  
> -	adis_cleanup_buffer_and_trigger(&st->adis, indio_dev);
This change the remove order so that it will not be a reverse of the order
seen in probe.   Perhaps you can solve that by using
devm_add_action_or_reset() calls to handle the few other remaining things
in remove?

Thanks,

Jonathan

>  	clk_disable_unprepare(st->ext_clk);
>  
>  	return 0;

Re: [PATCH 10/10] iio: adis: Drop non Managed device functions

[Jonathan Cameron]

On Tue, 15 Sep 2020 11:33:45 +0200
Nuno Sá <nuno.sa@analog.com> wrote:

> Drop `adis_setup_buffer_and_trigger()`. All users were updated to use
> the devm version of this function. This avoids having almost the same
> code repeated.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>

Good to see this cleanup, as long as we tidy up the few issues in the
earlier patches.

Note I think I only commented on first instance of each thing to fix.
Please carry them through all the patches.

Thanks,

Jonathan

> ---
>  drivers/iio/imu/adis_buffer.c  | 64 +++-------------------------------
>  drivers/iio/imu/adis_trigger.c | 60 -------------------------------
>  include/linux/iio/imu/adis.h   | 27 --------------
>  3 files changed, 4 insertions(+), 147 deletions(-)
> 
> diff --git a/drivers/iio/imu/adis_buffer.c b/drivers/iio/imu/adis_buffer.c
> index 5b4225ee09b9..df6144285470 100644
> --- a/drivers/iio/imu/adis_buffer.c
> +++ b/drivers/iio/imu/adis_buffer.c
> @@ -169,48 +169,6 @@ static void adis_buffer_cleanup(void *arg)
>  	kfree(adis->xfer);
>  }
>  
> -/**
> - * adis_setup_buffer_and_trigger() - Sets up buffer and trigger for the adis device
> - * @adis: The adis device.
> - * @indio_dev: The IIO device.
> - * @trigger_handler: Optional trigger handler, may be NULL.
> - *
> - * Returns 0 on success, a negative error code otherwise.
> - *
> - * This function sets up the buffer and trigger for a adis devices.  If
> - * 'trigger_handler' is NULL the default trigger handler will be used. The
> - * default trigger handler will simply read the registers assigned to the
> - * currently active channels.
> - *
> - * adis_cleanup_buffer_and_trigger() should be called to free the resources
> - * allocated by this function.
> - */
> -int adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev,
> -	irqreturn_t (*trigger_handler)(int, void *))
> -{
> -	int ret;
> -
> -	if (!trigger_handler)
> -		trigger_handler = adis_trigger_handler;
> -
> -	ret = iio_triggered_buffer_setup(indio_dev, &iio_pollfunc_store_time,
> -		trigger_handler, NULL);
> -	if (ret)
> -		return ret;
> -
> -	if (adis->spi->irq) {
> -		ret = adis_probe_trigger(adis, indio_dev);
> -		if (ret)
> -			goto error_buffer_cleanup;
> -	}
> -	return 0;
> -
> -error_buffer_cleanup:
> -	iio_triggered_buffer_cleanup(indio_dev);
> -	return ret;
> -}
> -EXPORT_SYMBOL_GPL(adis_setup_buffer_and_trigger);
> -
>  /**
>   * devm_adis_setup_buffer_and_trigger() - Sets up buffer and trigger for
>   *					  the managed adis device
> @@ -220,7 +178,10 @@ EXPORT_SYMBOL_GPL(adis_setup_buffer_and_trigger);
>   *
>   * Returns 0 on success, a negative error code otherwise.
>   *
> - * This function perfoms exactly the same as adis_setup_buffer_and_trigger()
> + * This function sets up the buffer and trigger for a adis devices.  If
> + * 'trigger_handler' is NULL the default trigger handler will be used. The
> + * default trigger handler will simply read the registers assigned to the
> + * currently active channels.
>   */
>  int
>  devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev,
> @@ -248,20 +209,3 @@ devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev,
>  }
>  EXPORT_SYMBOL_GPL(devm_adis_setup_buffer_and_trigger);
>  
> -/**
> - * adis_cleanup_buffer_and_trigger() - Free buffer and trigger resources
> - * @adis: The adis device.
> - * @indio_dev: The IIO device.
> - *
> - * Frees resources allocated by adis_setup_buffer_and_trigger()
> - */
> -void adis_cleanup_buffer_and_trigger(struct adis *adis,
> -	struct iio_dev *indio_dev)
> -{
> -	if (adis->spi->irq)
> -		adis_remove_trigger(adis);
> -	kfree(adis->buffer);
> -	kfree(adis->xfer);
> -	iio_triggered_buffer_cleanup(indio_dev);
> -}
> -EXPORT_SYMBOL_GPL(adis_cleanup_buffer_and_trigger);
> diff --git a/drivers/iio/imu/adis_trigger.c b/drivers/iio/imu/adis_trigger.c
> index 8afe71947c00..64e0ba51cb18 100644
> --- a/drivers/iio/imu/adis_trigger.c
> +++ b/drivers/iio/imu/adis_trigger.c
> @@ -55,53 +55,6 @@ static int adis_validate_irq_flag(struct adis *adis)
>  
>  	return 0;
>  }
> -/**
> - * adis_probe_trigger() - Sets up trigger for a adis device
> - * @adis: The adis device
> - * @indio_dev: The IIO device
> - *
> - * Returns 0 on success or a negative error code
> - *
> - * adis_remove_trigger() should be used to free the trigger.
> - */
> -int adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev)
> -{
> -	int ret;
> -
> -	adis->trig = iio_trigger_alloc("%s-dev%d", indio_dev->name,
> -					indio_dev->id);
> -	if (adis->trig == NULL)
> -		return -ENOMEM;
> -
> -	adis_trigger_setup(adis);
> -
> -	ret = adis_validate_irq_flag(adis);
> -	if (ret)
> -		return ret;
> -
> -	ret = request_irq(adis->spi->irq,
> -			  &iio_trigger_generic_data_rdy_poll,
> -			  adis->irq_flag,
> -			  indio_dev->name,
> -			  adis->trig);
> -	if (ret)
> -		goto error_free_trig;
> -
> -	ret = iio_trigger_register(adis->trig);
> -
> -	indio_dev->trig = iio_trigger_get(adis->trig);
> -	if (ret)
> -		goto error_free_irq;
> -
> -	return 0;
> -
> -error_free_irq:
> -	free_irq(adis->spi->irq, adis->trig);
> -error_free_trig:
> -	iio_trigger_free(adis->trig);
> -	return ret;
> -}
> -EXPORT_SYMBOL_GPL(adis_probe_trigger);
>  
>  /**
>   * devm_adis_probe_trigger() - Sets up trigger for a managed adis device
> @@ -137,16 +90,3 @@ int devm_adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev)
>  }
>  EXPORT_SYMBOL_GPL(devm_adis_probe_trigger);
>  
> -/**
> - * adis_remove_trigger() - Remove trigger for a adis devices
> - * @adis: The adis device
> - *
> - * Removes the trigger previously registered with adis_probe_trigger().
> - */
> -void adis_remove_trigger(struct adis *adis)
> -{
> -	iio_trigger_unregister(adis->trig);
> -	free_irq(adis->spi->irq, adis->trig);
> -	iio_trigger_free(adis->trig);
> -}
> -EXPORT_SYMBOL_GPL(adis_remove_trigger);
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index 2df67448f0d1..01ba691da2f3 100644
> --- a/include/linux/iio/imu/adis.h
> +++ b/include/linux/iio/imu/adis.h
> @@ -517,14 +517,8 @@ struct adis_burst {
>  int
>  devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev,
>  				   irq_handler_t trigger_handler);
> -int adis_setup_buffer_and_trigger(struct adis *adis,
> -	struct iio_dev *indio_dev, irqreturn_t (*trigger_handler)(int, void *));
> -void adis_cleanup_buffer_and_trigger(struct adis *adis,
> -	struct iio_dev *indio_dev);
>  
>  int devm_adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev);
> -int adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev);
> -void adis_remove_trigger(struct adis *adis);
>  
>  int adis_update_scan_mode(struct iio_dev *indio_dev,
>  	const unsigned long *scan_mask);
> @@ -538,33 +532,12 @@ devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev,
>  	return 0;
>  }
>  
> -static inline int adis_setup_buffer_and_trigger(struct adis *adis,
> -	struct iio_dev *indio_dev, irqreturn_t (*trigger_handler)(int, void *))
> -{
> -	return 0;
> -}
> -
> -static inline void adis_cleanup_buffer_and_trigger(struct adis *adis,
> -	struct iio_dev *indio_dev)
> -{
> -}
> -
>  static inline int devm_adis_probe_trigger(struct adis *adis,
>  					  struct iio_dev *indio_dev)
>  {
>  	return 0;
>  }
>  
> -static inline int adis_probe_trigger(struct adis *adis,
> -	struct iio_dev *indio_dev)
> -{
> -	return 0;
> -}
> -
> -static inline void adis_remove_trigger(struct adis *adis)
> -{
> -}
> -
>  #define adis_update_scan_mode NULL
>  
>  #endif /* CONFIG_IIO_BUFFER */

RE: [PATCH 10/10] iio: adis: Drop non Managed device functions

[Sa, Nuno]

> -----Original Message-----
> From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> Sent: Tuesday, September 15, 2020 12:10 PM
> To: Sa, Nuno <Nuno.Sa@analog.com>
> Cc: linux-iio@vger.kernel.org; devel@driverdev.osuosl.org; Lars-Peter
> Clausen <lars@metafoo.de>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Jonathan Cameron <jic23@kernel.org>;
> Hartmut Knaack <knaack.h@gmx.de>; Peter Meerwald-Stadler
> <pmeerw@pmeerw.net>; Bogdan, Dragos <Dragos.Bogdan@analog.com>;
> Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Ardelean, Alexandru
> <alexandru.Ardelean@analog.com>
> Subject: Re: [PATCH 10/10] iio: adis: Drop non Managed device functions
> 
> [External]
> 
> On Tue, 15 Sep 2020 11:33:45 +0200
> Nuno Sá <nuno.sa@analog.com> wrote:
> 
> > Drop `adis_setup_buffer_and_trigger()`. All users were updated to use
> > the devm version of this function. This avoids having almost the same
> > code repeated.
> >
> > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> 
> Good to see this cleanup, as long as we tidy up the few issues in the
> earlier patches.
> 
> Note I think I only commented on first instance of each thing to fix.
> Please carry them through all the patches.
> 
> Thanks,
> 
> Jonathan

So, I did thought about further cleaning the probe functions in order to get
rid of the remove function but it felt like a different series to me. Anyways, since
it's fine with you to include those changes here I will send a v2.

- Nuno Sá
 
> > ---
> >  drivers/iio/imu/adis_buffer.c  | 64 +++-------------------------------
> >  drivers/iio/imu/adis_trigger.c | 60 -------------------------------
> >  include/linux/iio/imu/adis.h   | 27 --------------
> >  3 files changed, 4 insertions(+), 147 deletions(-)
> >
> > diff --git a/drivers/iio/imu/adis_buffer.c b/drivers/iio/imu/adis_buffer.c
> > index 5b4225ee09b9..df6144285470 100644
> > --- a/drivers/iio/imu/adis_buffer.c
> > +++ b/drivers/iio/imu/adis_buffer.c
> > @@ -169,48 +169,6 @@ static void adis_buffer_cleanup(void *arg)
> >  	kfree(adis->xfer);
> >  }
> >
> > -/**
> > - * adis_setup_buffer_and_trigger() - Sets up buffer and trigger for the
> adis device
> > - * @adis: The adis device.
> > - * @indio_dev: The IIO device.
> > - * @trigger_handler: Optional trigger handler, may be NULL.
> > - *
> > - * Returns 0 on success, a negative error code otherwise.
> > - *
> > - * This function sets up the buffer and trigger for a adis devices.  If
> > - * 'trigger_handler' is NULL the default trigger handler will be used. The
> > - * default trigger handler will simply read the registers assigned to the
> > - * currently active channels.
> > - *
> > - * adis_cleanup_buffer_and_trigger() should be called to free the
> resources
> > - * allocated by this function.
> > - */
> > -int adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev
> *indio_dev,
> > -	irqreturn_t (*trigger_handler)(int, void *))
> > -{
> > -	int ret;
> > -
> > -	if (!trigger_handler)
> > -		trigger_handler = adis_trigger_handler;
> > -
> > -	ret = iio_triggered_buffer_setup(indio_dev,
> &iio_pollfunc_store_time,
> > -		trigger_handler, NULL);
> > -	if (ret)
> > -		return ret;
> > -
> > -	if (adis->spi->irq) {
> > -		ret = adis_probe_trigger(adis, indio_dev);
> > -		if (ret)
> > -			goto error_buffer_cleanup;
> > -	}
> > -	return 0;
> > -
> > -error_buffer_cleanup:
> > -	iio_triggered_buffer_cleanup(indio_dev);
> > -	return ret;
> > -}
> > -EXPORT_SYMBOL_GPL(adis_setup_buffer_and_trigger);
> > -
> >  /**
> >   * devm_adis_setup_buffer_and_trigger() - Sets up buffer and trigger for
> >   *					  the managed adis device
> > @@ -220,7 +178,10 @@
> EXPORT_SYMBOL_GPL(adis_setup_buffer_and_trigger);
> >   *
> >   * Returns 0 on success, a negative error code otherwise.
> >   *
> > - * This function perfoms exactly the same as
> adis_setup_buffer_and_trigger()
> > + * This function sets up the buffer and trigger for a adis devices.  If
> > + * 'trigger_handler' is NULL the default trigger handler will be used. The
> > + * default trigger handler will simply read the registers assigned to the
> > + * currently active channels.
> >   */
> >  int
> >  devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev
> *indio_dev,
> > @@ -248,20 +209,3 @@ devm_adis_setup_buffer_and_trigger(struct adis
> *adis, struct iio_dev *indio_dev,
> >  }
> >  EXPORT_SYMBOL_GPL(devm_adis_setup_buffer_and_trigger);
> >
> > -/**
> > - * adis_cleanup_buffer_and_trigger() - Free buffer and trigger resources
> > - * @adis: The adis device.
> > - * @indio_dev: The IIO device.
> > - *
> > - * Frees resources allocated by adis_setup_buffer_and_trigger()
> > - */
> > -void adis_cleanup_buffer_and_trigger(struct adis *adis,
> > -	struct iio_dev *indio_dev)
> > -{
> > -	if (adis->spi->irq)
> > -		adis_remove_trigger(adis);
> > -	kfree(adis->buffer);
> > -	kfree(adis->xfer);
> > -	iio_triggered_buffer_cleanup(indio_dev);
> > -}
> > -EXPORT_SYMBOL_GPL(adis_cleanup_buffer_and_trigger);
> > diff --git a/drivers/iio/imu/adis_trigger.c b/drivers/iio/imu/adis_trigger.c
> > index 8afe71947c00..64e0ba51cb18 100644
> > --- a/drivers/iio/imu/adis_trigger.c
> > +++ b/drivers/iio/imu/adis_trigger.c
> > @@ -55,53 +55,6 @@ static int adis_validate_irq_flag(struct adis *adis)
> >
> >  	return 0;
> >  }
> > -/**
> > - * adis_probe_trigger() - Sets up trigger for a adis device
> > - * @adis: The adis device
> > - * @indio_dev: The IIO device
> > - *
> > - * Returns 0 on success or a negative error code
> > - *
> > - * adis_remove_trigger() should be used to free the trigger.
> > - */
> > -int adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev)
> > -{
> > -	int ret;
> > -
> > -	adis->trig = iio_trigger_alloc("%s-dev%d", indio_dev->name,
> > -					indio_dev->id);
> > -	if (adis->trig == NULL)
> > -		return -ENOMEM;
> > -
> > -	adis_trigger_setup(adis);
> > -
> > -	ret = adis_validate_irq_flag(adis);
> > -	if (ret)
> > -		return ret;
> > -
> > -	ret = request_irq(adis->spi->irq,
> > -			  &iio_trigger_generic_data_rdy_poll,
> > -			  adis->irq_flag,
> > -			  indio_dev->name,
> > -			  adis->trig);
> > -	if (ret)
> > -		goto error_free_trig;
> > -
> > -	ret = iio_trigger_register(adis->trig);
> > -
> > -	indio_dev->trig = iio_trigger_get(adis->trig);
> > -	if (ret)
> > -		goto error_free_irq;
> > -
> > -	return 0;
> > -
> > -error_free_irq:
> > -	free_irq(adis->spi->irq, adis->trig);
> > -error_free_trig:
> > -	iio_trigger_free(adis->trig);
> > -	return ret;
> > -}
> > -EXPORT_SYMBOL_GPL(adis_probe_trigger);
> >
> >  /**
> >   * devm_adis_probe_trigger() - Sets up trigger for a managed adis device
> > @@ -137,16 +90,3 @@ int devm_adis_probe_trigger(struct adis *adis,
> struct iio_dev *indio_dev)
> >  }
> >  EXPORT_SYMBOL_GPL(devm_adis_probe_trigger);
> >
> > -/**
> > - * adis_remove_trigger() - Remove trigger for a adis devices
> > - * @adis: The adis device
> > - *
> > - * Removes the trigger previously registered with adis_probe_trigger().
> > - */
> > -void adis_remove_trigger(struct adis *adis)
> > -{
> > -	iio_trigger_unregister(adis->trig);
> > -	free_irq(adis->spi->irq, adis->trig);
> > -	iio_trigger_free(adis->trig);
> > -}
> > -EXPORT_SYMBOL_GPL(adis_remove_trigger);
> > diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> > index 2df67448f0d1..01ba691da2f3 100644
> > --- a/include/linux/iio/imu/adis.h
> > +++ b/include/linux/iio/imu/adis.h
> > @@ -517,14 +517,8 @@ struct adis_burst {
> >  int
> >  devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev
> *indio_dev,
> >  				   irq_handler_t trigger_handler);
> > -int adis_setup_buffer_and_trigger(struct adis *adis,
> > -	struct iio_dev *indio_dev, irqreturn_t (*trigger_handler)(int, void
> *));
> > -void adis_cleanup_buffer_and_trigger(struct adis *adis,
> > -	struct iio_dev *indio_dev);
> >
> >  int devm_adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev);
> > -int adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev);
> > -void adis_remove_trigger(struct adis *adis);
> >
> >  int adis_update_scan_mode(struct iio_dev *indio_dev,
> >  	const unsigned long *scan_mask);
> > @@ -538,33 +532,12 @@ devm_adis_setup_buffer_and_trigger(struct adis
> *adis, struct iio_dev *indio_dev,
> >  	return 0;
> >  }
> >
> > -static inline int adis_setup_buffer_and_trigger(struct adis *adis,
> > -	struct iio_dev *indio_dev, irqreturn_t (*trigger_handler)(int, void *))
> > -{
> > -	return 0;
> > -}
> > -
> > -static inline void adis_cleanup_buffer_and_trigger(struct adis *adis,
> > -	struct iio_dev *indio_dev)
> > -{
> > -}
> > -
> >  static inline int devm_adis_probe_trigger(struct adis *adis,
> >  					  struct iio_dev *indio_dev)
> >  {
> >  	return 0;
> >  }
> >
> > -static inline int adis_probe_trigger(struct adis *adis,
> > -	struct iio_dev *indio_dev)
> > -{
> > -	return 0;
> > -}
> > -
> > -static inline void adis_remove_trigger(struct adis *adis)
> > -{
> > -}
> > -
> >  #define adis_update_scan_mode NULL
> >
> >  #endif /* CONFIG_IIO_BUFFER */
> 

Re: [PATCH 10/10] iio: adis: Drop non Managed device functions

[Jonathan Cameron]

On Tue, 15 Sep 2020 10:20:20 +0000
"Sa, Nuno" <Nuno.Sa@analog.com> wrote:

> > -----Original Message-----
> > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> > Sent: Tuesday, September 15, 2020 12:10 PM
> > To: Sa, Nuno <Nuno.Sa@analog.com>
> > Cc: linux-iio@vger.kernel.org; devel@driverdev.osuosl.org; Lars-Peter
> > Clausen <lars@metafoo.de>; Hennerich, Michael
> > <Michael.Hennerich@analog.com>; Jonathan Cameron <jic23@kernel.org>;
> > Hartmut Knaack <knaack.h@gmx.de>; Peter Meerwald-Stadler
> > <pmeerw@pmeerw.net>; Bogdan, Dragos <Dragos.Bogdan@analog.com>;
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Ardelean, Alexandru
> > <alexandru.Ardelean@analog.com>
> > Subject: Re: [PATCH 10/10] iio: adis: Drop non Managed device functions
> > 
> > [External]
> > 
> > On Tue, 15 Sep 2020 11:33:45 +0200
> > Nuno Sá <nuno.sa@analog.com> wrote:
> >   
> > > Drop `adis_setup_buffer_and_trigger()`. All users were updated to use
> > > the devm version of this function. This avoids having almost the same
> > > code repeated.
> > >
> > > Signed-off-by: Nuno Sá <nuno.sa@analog.com>  
> > 
> > Good to see this cleanup, as long as we tidy up the few issues in the
> > earlier patches.
> > 
> > Note I think I only commented on first instance of each thing to fix.
> > Please carry them through all the patches.
> > 
> > Thanks,
> > 
> > Jonathan  
> 
> So, I did thought about further cleaning the probe functions in order to get
> rid of the remove function but it felt like a different series to me. Anyways, since
> it's fine with you to include those changes here I will send a v2.

Understood.  Could have been separate, but it feels like trying to use devm
throughout is a good enough theme per patch to combine them.  This is particularly
true as you'd need to do 3 series if you wanted to separate this out without the
ordering issues.

1) Devm stuff before this particular function in probe.
2) This series
3) Devm stuff after the particular function in probe.

Definitely easier to do in one go and still easily reviewed individual patches!

Jonathan

> 
> - Nuno Sá
>  
> > > ---
> > >  drivers/iio/imu/adis_buffer.c  | 64 +++-------------------------------
> > >  drivers/iio/imu/adis_trigger.c | 60 -------------------------------
> > >  include/linux/iio/imu/adis.h   | 27 --------------
> > >  3 files changed, 4 insertions(+), 147 deletions(-)
> > >
> > > diff --git a/drivers/iio/imu/adis_buffer.c b/drivers/iio/imu/adis_buffer.c
> > > index 5b4225ee09b9..df6144285470 100644
> > > --- a/drivers/iio/imu/adis_buffer.c
> > > +++ b/drivers/iio/imu/adis_buffer.c
> > > @@ -169,48 +169,6 @@ static void adis_buffer_cleanup(void *arg)
> > >  	kfree(adis->xfer);
> > >  }
> > >
> > > -/**
> > > - * adis_setup_buffer_and_trigger() - Sets up buffer and trigger for the  
> > adis device  
> > > - * @adis: The adis device.
> > > - * @indio_dev: The IIO device.
> > > - * @trigger_handler: Optional trigger handler, may be NULL.
> > > - *
> > > - * Returns 0 on success, a negative error code otherwise.
> > > - *
> > > - * This function sets up the buffer and trigger for a adis devices.  If
> > > - * 'trigger_handler' is NULL the default trigger handler will be used. The
> > > - * default trigger handler will simply read the registers assigned to the
> > > - * currently active channels.
> > > - *
> > > - * adis_cleanup_buffer_and_trigger() should be called to free the  
> > resources  
> > > - * allocated by this function.
> > > - */
> > > -int adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev  
> > *indio_dev,  
> > > -	irqreturn_t (*trigger_handler)(int, void *))
> > > -{
> > > -	int ret;
> > > -
> > > -	if (!trigger_handler)
> > > -		trigger_handler = adis_trigger_handler;
> > > -
> > > -	ret = iio_triggered_buffer_setup(indio_dev,  
> > &iio_pollfunc_store_time,  
> > > -		trigger_handler, NULL);
> > > -	if (ret)
> > > -		return ret;
> > > -
> > > -	if (adis->spi->irq) {
> > > -		ret = adis_probe_trigger(adis, indio_dev);
> > > -		if (ret)
> > > -			goto error_buffer_cleanup;
> > > -	}
> > > -	return 0;
> > > -
> > > -error_buffer_cleanup:
> > > -	iio_triggered_buffer_cleanup(indio_dev);
> > > -	return ret;
> > > -}
> > > -EXPORT_SYMBOL_GPL(adis_setup_buffer_and_trigger);
> > > -
> > >  /**
> > >   * devm_adis_setup_buffer_and_trigger() - Sets up buffer and trigger for
> > >   *					  the managed adis device
> > > @@ -220,7 +178,10 @@  
> > EXPORT_SYMBOL_GPL(adis_setup_buffer_and_trigger);  
> > >   *
> > >   * Returns 0 on success, a negative error code otherwise.
> > >   *
> > > - * This function perfoms exactly the same as  
> > adis_setup_buffer_and_trigger()  
> > > + * This function sets up the buffer and trigger for a adis devices.  If
> > > + * 'trigger_handler' is NULL the default trigger handler will be used. The
> > > + * default trigger handler will simply read the registers assigned to the
> > > + * currently active channels.
> > >   */
> > >  int
> > >  devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev  
> > *indio_dev,  
> > > @@ -248,20 +209,3 @@ devm_adis_setup_buffer_and_trigger(struct adis  
> > *adis, struct iio_dev *indio_dev,  
> > >  }
> > >  EXPORT_SYMBOL_GPL(devm_adis_setup_buffer_and_trigger);
> > >
> > > -/**
> > > - * adis_cleanup_buffer_and_trigger() - Free buffer and trigger resources
> > > - * @adis: The adis device.
> > > - * @indio_dev: The IIO device.
> > > - *
> > > - * Frees resources allocated by adis_setup_buffer_and_trigger()
> > > - */
> > > -void adis_cleanup_buffer_and_trigger(struct adis *adis,
> > > -	struct iio_dev *indio_dev)
> > > -{
> > > -	if (adis->spi->irq)
> > > -		adis_remove_trigger(adis);
> > > -	kfree(adis->buffer);
> > > -	kfree(adis->xfer);
> > > -	iio_triggered_buffer_cleanup(indio_dev);
> > > -}
> > > -EXPORT_SYMBOL_GPL(adis_cleanup_buffer_and_trigger);
> > > diff --git a/drivers/iio/imu/adis_trigger.c b/drivers/iio/imu/adis_trigger.c
> > > index 8afe71947c00..64e0ba51cb18 100644
> > > --- a/drivers/iio/imu/adis_trigger.c
> > > +++ b/drivers/iio/imu/adis_trigger.c
> > > @@ -55,53 +55,6 @@ static int adis_validate_irq_flag(struct adis *adis)
> > >
> > >  	return 0;
> > >  }
> > > -/**
> > > - * adis_probe_trigger() - Sets up trigger for a adis device
> > > - * @adis: The adis device
> > > - * @indio_dev: The IIO device
> > > - *
> > > - * Returns 0 on success or a negative error code
> > > - *
> > > - * adis_remove_trigger() should be used to free the trigger.
> > > - */
> > > -int adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev)
> > > -{
> > > -	int ret;
> > > -
> > > -	adis->trig = iio_trigger_alloc("%s-dev%d", indio_dev->name,
> > > -					indio_dev->id);
> > > -	if (adis->trig == NULL)
> > > -		return -ENOMEM;
> > > -
> > > -	adis_trigger_setup(adis);
> > > -
> > > -	ret = adis_validate_irq_flag(adis);
> > > -	if (ret)
> > > -		return ret;
> > > -
> > > -	ret = request_irq(adis->spi->irq,
> > > -			  &iio_trigger_generic_data_rdy_poll,
> > > -			  adis->irq_flag,
> > > -			  indio_dev->name,
> > > -			  adis->trig);
> > > -	if (ret)
> > > -		goto error_free_trig;
> > > -
> > > -	ret = iio_trigger_register(adis->trig);
> > > -
> > > -	indio_dev->trig = iio_trigger_get(adis->trig);
> > > -	if (ret)
> > > -		goto error_free_irq;
> > > -
> > > -	return 0;
> > > -
> > > -error_free_irq:
> > > -	free_irq(adis->spi->irq, adis->trig);
> > > -error_free_trig:
> > > -	iio_trigger_free(adis->trig);
> > > -	return ret;
> > > -}
> > > -EXPORT_SYMBOL_GPL(adis_probe_trigger);
> > >
> > >  /**
> > >   * devm_adis_probe_trigger() - Sets up trigger for a managed adis device
> > > @@ -137,16 +90,3 @@ int devm_adis_probe_trigger(struct adis *adis,  
> > struct iio_dev *indio_dev)  
> > >  }
> > >  EXPORT_SYMBOL_GPL(devm_adis_probe_trigger);
> > >
> > > -/**
> > > - * adis_remove_trigger() - Remove trigger for a adis devices
> > > - * @adis: The adis device
> > > - *
> > > - * Removes the trigger previously registered with adis_probe_trigger().
> > > - */
> > > -void adis_remove_trigger(struct adis *adis)
> > > -{
> > > -	iio_trigger_unregister(adis->trig);
> > > -	free_irq(adis->spi->irq, adis->trig);
> > > -	iio_trigger_free(adis->trig);
> > > -}
> > > -EXPORT_SYMBOL_GPL(adis_remove_trigger);
> > > diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> > > index 2df67448f0d1..01ba691da2f3 100644
> > > --- a/include/linux/iio/imu/adis.h
> > > +++ b/include/linux/iio/imu/adis.h
> > > @@ -517,14 +517,8 @@ struct adis_burst {
> > >  int
> > >  devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev  
> > *indio_dev,  
> > >  				   irq_handler_t trigger_handler);
> > > -int adis_setup_buffer_and_trigger(struct adis *adis,
> > > -	struct iio_dev *indio_dev, irqreturn_t (*trigger_handler)(int, void  
> > *));  
> > > -void adis_cleanup_buffer_and_trigger(struct adis *adis,
> > > -	struct iio_dev *indio_dev);
> > >
> > >  int devm_adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev);
> > > -int adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev);
> > > -void adis_remove_trigger(struct adis *adis);
> > >
> > >  int adis_update_scan_mode(struct iio_dev *indio_dev,
> > >  	const unsigned long *scan_mask);
> > > @@ -538,33 +532,12 @@ devm_adis_setup_buffer_and_trigger(struct adis  
> > *adis, struct iio_dev *indio_dev,  
> > >  	return 0;
> > >  }
> > >
> > > -static inline int adis_setup_buffer_and_trigger(struct adis *adis,
> > > -	struct iio_dev *indio_dev, irqreturn_t (*trigger_handler)(int, void *))
> > > -{
> > > -	return 0;
> > > -}
> > > -
> > > -static inline void adis_cleanup_buffer_and_trigger(struct adis *adis,
> > > -	struct iio_dev *indio_dev)
> > > -{
> > > -}
> > > -
> > >  static inline int devm_adis_probe_trigger(struct adis *adis,
> > >  					  struct iio_dev *indio_dev)
> > >  {
> > >  	return 0;
> > >  }
> > >
> > > -static inline int adis_probe_trigger(struct adis *adis,
> > > -	struct iio_dev *indio_dev)
> > > -{
> > > -	return 0;
> > > -}
> > > -
> > > -static inline void adis_remove_trigger(struct adis *adis)
> > > -{
> > > -}
> > > -
> > >  #define adis_update_scan_mode NULL
> > >
> > >  #endif /* CONFIG_IIO_BUFFER */  
> >   
>