All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] iio: adc: Maxim and TI ADC driver cleanups
@ 2021-05-16 17:25 Jonathan Cameron
  2021-05-16 17:25 ` [PATCH 1/8] iio: adc: max11100: Use get_unaligned_be16() rather than opencoding Jonathan Cameron
                   ` (8 more replies)
  0 siblings, 9 replies; 22+ messages in thread
From: Jonathan Cameron @ 2021-05-16 17:25 UTC (permalink / raw)
  To: linux-iio; +Cc: Alexandru Ardelean, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

A few random bits of tidying up for a Sunday evening.
Mostly devm stuff, but other things in here as well that came up whilst
I was looking.

Note this was a fairly random selection, so isn't implying there
aren't similar cleanups to be done in other TI and Maxim drivers.
(there definitely are plenty more of these if anyone is bored ;)

Cc: Alexandru Ardelean <aardelean@deviqon.com>

Jonathan Cameron (8):
  iio: adc: max11100: Use get_unaligned_be16() rather than opencoding.
  iio: adc: max11100: Use devm_ functions for rest of probe()
  iio: adc: max1118: Use devm_ managed functions for all of probe
  iio: adc: max1118: Avoid jumping back and forth between spi and iio
    structures
  iio: adc: ti-adc081c: Use devm managed functions for all of probe()
  iio: adc: ti-adc0832: Use devm managed functions for all of probe()
  iio: adc: ti-adc108s102: Use devm managed functions for all of probe()
  iio: adc: ti-adc161s626: Use devm managed functions for all of probe.

 drivers/iio/adc/max11100.c      | 32 ++++++----------
 drivers/iio/adc/max1118.c       | 68 ++++++++++++---------------------
 drivers/iio/adc/ti-adc081c.c    | 43 +++++++--------------
 drivers/iio/adc/ti-adc0832.c    | 39 ++++++-------------
 drivers/iio/adc/ti-adc108s102.c | 45 ++++++++--------------
 drivers/iio/adc/ti-adc161s626.c | 50 +++++++++---------------
 6 files changed, 94 insertions(+), 183 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/8] iio: adc: max11100: Use get_unaligned_be16() rather than opencoding.
  2021-05-16 17:25 [PATCH 0/8] iio: adc: Maxim and TI ADC driver cleanups Jonathan Cameron
@ 2021-05-16 17:25 ` Jonathan Cameron
  2021-05-17  6:59   ` Alexandru Ardelean
  2021-05-16 17:25 ` [PATCH 2/8] iio: adc: max11100: Use devm_ functions for rest of probe() Jonathan Cameron
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2021-05-16 17:25 UTC (permalink / raw)
  To: linux-iio; +Cc: Alexandru Ardelean, Jonathan Cameron, Jacopo Mondi

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

The function is more explicit in showing the intent + quicker on some
platforms.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/iio/adc/max11100.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
index 6cf21758ca66..69d607fa17aa 100644
--- a/drivers/iio/adc/max11100.c
+++ b/drivers/iio/adc/max11100.c
@@ -12,6 +12,7 @@
 #include <linux/module.h>
 #include <linux/regulator/consumer.h>
 #include <linux/spi/spi.h>
+#include <asm/unaligned.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/driver.h>
@@ -63,7 +64,7 @@ static int max11100_read_single(struct iio_dev *indio_dev, int *val)
 		return -EINVAL;
 	}
 
-	*val = (state->buffer[1] << 8) | state->buffer[2];
+	*val = get_unaligned_be16(&state->buffer[1]);
 
 	return 0;
 }
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/8] iio: adc: max11100: Use devm_ functions for rest of probe()
  2021-05-16 17:25 [PATCH 0/8] iio: adc: Maxim and TI ADC driver cleanups Jonathan Cameron
  2021-05-16 17:25 ` [PATCH 1/8] iio: adc: max11100: Use get_unaligned_be16() rather than opencoding Jonathan Cameron
@ 2021-05-16 17:25 ` Jonathan Cameron
  2021-05-17  7:08   ` Alexandru Ardelean
  2021-05-16 17:25 ` [PATCH 3/8] iio: adc: max1118: Use devm_ managed functions for all of probe Jonathan Cameron
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2021-05-16 17:25 UTC (permalink / raw)
  To: linux-iio; +Cc: Alexandru Ardelean, Jonathan Cameron, Jacopo Mondi

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

By using devm_add_action_or_reset() to manage the regulator disable,
it becomes simple to use managed functions for all of remove.
This simplifies error handling and allows us to drop the remove()
function entirely.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/iio/adc/max11100.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
index 69d607fa17aa..9951f6a6a4b9 100644
--- a/drivers/iio/adc/max11100.c
+++ b/drivers/iio/adc/max11100.c
@@ -102,6 +102,11 @@ static const struct iio_info max11100_info = {
 	.read_raw = max11100_read_raw,
 };
 
+static void max11100_regulator_disable(void *reg)
+{
+	regulator_disable(reg);
+}
+
 static int max11100_probe(struct spi_device *spi)
 {
 	int ret;
@@ -131,27 +136,12 @@ static int max11100_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_add_action_or_reset(&spi->dev, max11100_regulator_disable,
+				state->vref_reg);
 	if (ret)
-		goto disable_regulator;
-
-	return 0;
-
-disable_regulator:
-	regulator_disable(state->vref_reg);
-
-	return ret;
-}
-
-static int max11100_remove(struct spi_device *spi)
-{
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct max11100_state *state = iio_priv(indio_dev);
-
-	iio_device_unregister(indio_dev);
-	regulator_disable(state->vref_reg);
+		return ret;
 
-	return 0;
+	return devm_iio_device_register(&spi->dev, indio_dev);
 }
 
 static const struct of_device_id max11100_ids[] = {
@@ -166,7 +156,6 @@ static struct spi_driver max11100_driver = {
 		.of_match_table = max11100_ids,
 	},
 	.probe		= max11100_probe,
-	.remove		= max11100_remove,
 };
 
 module_spi_driver(max11100_driver);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/8] iio: adc: max1118: Use devm_ managed functions for all of probe
  2021-05-16 17:25 [PATCH 0/8] iio: adc: Maxim and TI ADC driver cleanups Jonathan Cameron
  2021-05-16 17:25 ` [PATCH 1/8] iio: adc: max11100: Use get_unaligned_be16() rather than opencoding Jonathan Cameron
  2021-05-16 17:25 ` [PATCH 2/8] iio: adc: max11100: Use devm_ functions for rest of probe() Jonathan Cameron
@ 2021-05-16 17:25 ` Jonathan Cameron
  2021-05-17  7:18   ` Alexandru Ardelean
  2021-05-16 17:25 ` [PATCH 4/8] iio: adc: max1118: Avoid jumping back and forth between spi and iio structures Jonathan Cameron
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2021-05-16 17:25 UTC (permalink / raw)
  To: linux-iio; +Cc: Alexandru Ardelean, Jonathan Cameron, Akinobu Mita

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This simplifies error handling and allows us to drop the remove
function entirely.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/iio/adc/max1118.c | 46 +++++++++++++--------------------------
 1 file changed, 15 insertions(+), 31 deletions(-)

diff --git a/drivers/iio/adc/max1118.c b/drivers/iio/adc/max1118.c
index 6efb0b43d938..4dfbed63ad7f 100644
--- a/drivers/iio/adc/max1118.c
+++ b/drivers/iio/adc/max1118.c
@@ -201,6 +201,11 @@ static irqreturn_t max1118_trigger_handler(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
+static void max1118_reg_disable(void *reg)
+{
+	regulator_disable(reg);
+}
+
 static int max1118_probe(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev;
@@ -225,6 +230,12 @@ static int max1118_probe(struct spi_device *spi)
 		ret = regulator_enable(adc->reg);
 		if (ret)
 			return ret;
+
+		ret = devm_add_action_or_reset(&spi->dev, max1118_reg_disable,
+					       adc->reg);
+		if (ret)
+			return ret;
+
 	}
 
 	spi_set_drvdata(spi, indio_dev);
@@ -243,38 +254,12 @@ static int max1118_probe(struct spi_device *spi)
 	 */
 	max1118_read(spi, 0);
 
-	ret = iio_triggered_buffer_setup(indio_dev, NULL,
-					max1118_trigger_handler, NULL);
+	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
+					      max1118_trigger_handler, NULL);
 	if (ret)
-		goto err_reg_disable;
-
-	ret = iio_device_register(indio_dev);
-	if (ret)
-		goto err_buffer_cleanup;
-
-	return 0;
-
-err_buffer_cleanup:
-	iio_triggered_buffer_cleanup(indio_dev);
-err_reg_disable:
-	if (id->driver_data == max1118)
-		regulator_disable(adc->reg);
-
-	return ret;
-}
-
-static int max1118_remove(struct spi_device *spi)
-{
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct max1118 *adc = iio_priv(indio_dev);
-	const struct spi_device_id *id = spi_get_device_id(spi);
-
-	iio_device_unregister(indio_dev);
-	iio_triggered_buffer_cleanup(indio_dev);
-	if (id->driver_data == max1118)
-		return regulator_disable(adc->reg);
+		return ret;
 
-	return 0;
+	return devm_iio_device_register(&spi->dev, indio_dev);
 }
 
 static const struct spi_device_id max1118_id[] = {
@@ -299,7 +284,6 @@ static struct spi_driver max1118_spi_driver = {
 		.of_match_table = max1118_dt_ids,
 	},
 	.probe = max1118_probe,
-	.remove = max1118_remove,
 	.id_table = max1118_id,
 };
 module_spi_driver(max1118_spi_driver);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 4/8] iio: adc: max1118: Avoid jumping back and forth between spi and iio structures
  2021-05-16 17:25 [PATCH 0/8] iio: adc: Maxim and TI ADC driver cleanups Jonathan Cameron
                   ` (2 preceding siblings ...)
  2021-05-16 17:25 ` [PATCH 3/8] iio: adc: max1118: Use devm_ managed functions for all of probe Jonathan Cameron
@ 2021-05-16 17:25 ` Jonathan Cameron
  2021-05-17  7:24   ` Alexandru Ardelean
  2021-05-16 17:25 ` [PATCH 5/8] iio: adc: ti-adc081c: Use devm managed functions for all of probe() Jonathan Cameron
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2021-05-16 17:25 UTC (permalink / raw)
  To: linux-iio; +Cc: Alexandru Ardelean, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Changing from passing the spi structure into various functions to
passing struct iio_dev avoids use of spi_get_drvdata and lets us
stop setting that at all.  Previous code was unnecessarily complex.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/max1118.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/adc/max1118.c b/drivers/iio/adc/max1118.c
index 4dfbed63ad7f..8cec9d949083 100644
--- a/drivers/iio/adc/max1118.c
+++ b/drivers/iio/adc/max1118.c
@@ -66,9 +66,8 @@ static const struct iio_chan_spec max1118_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(2),
 };
 
-static int max1118_read(struct spi_device *spi, int channel)
+static int max1118_read(struct iio_dev *indio_dev, int channel)
 {
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 	struct max1118 *adc = iio_priv(indio_dev);
 	struct spi_transfer xfers[] = {
 		/*
@@ -103,9 +102,9 @@ static int max1118_read(struct spi_device *spi, int channel)
 	int ret;
 
 	if (channel == 0)
-		ret = spi_sync_transfer(spi, xfers + 1, 2);
+		ret = spi_sync_transfer(adc->spi, xfers + 1, 2);
 	else
-		ret = spi_sync_transfer(spi, xfers, 3);
+		ret = spi_sync_transfer(adc->spi, xfers, 3);
 
 	if (ret)
 		return ret;
@@ -113,11 +112,10 @@ static int max1118_read(struct spi_device *spi, int channel)
 	return adc->data;
 }
 
-static int max1118_get_vref_mV(struct spi_device *spi)
+static int max1118_get_vref_mV(struct iio_dev *indio_dev)
 {
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 	struct max1118 *adc = iio_priv(indio_dev);
-	const struct spi_device_id *id = spi_get_device_id(spi);
+	const struct spi_device_id *id = spi_get_device_id(adc->spi);
 	int vref_uV;
 
 	switch (id->driver_data) {
@@ -144,14 +142,14 @@ static int max1118_read_raw(struct iio_dev *indio_dev,
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		mutex_lock(&adc->lock);
-		*val = max1118_read(adc->spi, chan->channel);
+		*val = max1118_read(indio_dev, chan->channel);
 		mutex_unlock(&adc->lock);
 		if (*val < 0)
 			return *val;
 
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-		*val = max1118_get_vref_mV(adc->spi);
+		*val = max1118_get_vref_mV(indio_dev);
 		if (*val < 0)
 			return *val;
 		*val2 = 8;
@@ -180,7 +178,7 @@ static irqreturn_t max1118_trigger_handler(int irq, void *p)
 			indio_dev->masklength) {
 		const struct iio_chan_spec *scan_chan =
 				&indio_dev->channels[scan_index];
-		int ret = max1118_read(adc->spi, scan_chan->channel);
+		int ret = max1118_read(indio_dev, scan_chan->channel);
 
 		if (ret < 0) {
 			dev_warn(&adc->spi->dev,
@@ -238,8 +236,6 @@ static int max1118_probe(struct spi_device *spi)
 
 	}
 
-	spi_set_drvdata(spi, indio_dev);
-
 	indio_dev->name = spi_get_device_id(spi)->name;
 	indio_dev->info = &max1118_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
@@ -252,7 +248,7 @@ static int max1118_probe(struct spi_device *spi)
 	 * a conversion has been completed, the MAX1117/MAX1118/MAX1119 will go
 	 * into AutoShutdown mode until the next conversion is initiated.
 	 */
-	max1118_read(spi, 0);
+	max1118_read(indio_dev, 0);
 
 	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
 					      max1118_trigger_handler, NULL);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 5/8] iio: adc: ti-adc081c: Use devm managed functions for all of probe()
  2021-05-16 17:25 [PATCH 0/8] iio: adc: Maxim and TI ADC driver cleanups Jonathan Cameron
                   ` (3 preceding siblings ...)
  2021-05-16 17:25 ` [PATCH 4/8] iio: adc: max1118: Avoid jumping back and forth between spi and iio structures Jonathan Cameron
@ 2021-05-16 17:25 ` Jonathan Cameron
  2021-05-17  7:26   ` Alexandru Ardelean
  2021-05-16 17:25 ` [PATCH 6/8] iio: adc: ti-adc0832: " Jonathan Cameron
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2021-05-16 17:25 UTC (permalink / raw)
  To: linux-iio; +Cc: Alexandru Ardelean, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Simplifies error handling and allows us to drop remove() entirely.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/ti-adc081c.c | 43 ++++++++++++------------------------
 1 file changed, 14 insertions(+), 29 deletions(-)

diff --git a/drivers/iio/adc/ti-adc081c.c b/drivers/iio/adc/ti-adc081c.c
index b64718daa201..16fc608db36a 100644
--- a/drivers/iio/adc/ti-adc081c.c
+++ b/drivers/iio/adc/ti-adc081c.c
@@ -146,6 +146,11 @@ static irqreturn_t adc081c_trigger_handler(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
+static void adc081c_reg_disable(void *reg)
+{
+	regulator_disable(reg);
+}
+
 static int adc081c_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
@@ -175,6 +180,11 @@ static int adc081c_probe(struct i2c_client *client,
 	if (err < 0)
 		return err;
 
+	err = devm_add_action_or_reset(&client->dev, adc081c_reg_disable,
+				       adc->ref);
+	if (err)
+		return err;
+
 	iio->name = dev_name(&client->dev);
 	iio->modes = INDIO_DIRECT_MODE;
 	iio->info = &adc081c_info;
@@ -182,38 +192,14 @@ static int adc081c_probe(struct i2c_client *client,
 	iio->channels = model->channels;
 	iio->num_channels = ADC081C_NUM_CHANNELS;
 
-	err = iio_triggered_buffer_setup(iio, NULL, adc081c_trigger_handler, NULL);
+	err = devm_iio_triggered_buffer_setup(&client->dev, iio, NULL,
+					      adc081c_trigger_handler, NULL);
 	if (err < 0) {
 		dev_err(&client->dev, "iio triggered buffer setup failed\n");
-		goto err_regulator_disable;
+		return err;
 	}
 
-	err = iio_device_register(iio);
-	if (err < 0)
-		goto err_buffer_cleanup;
-
-	i2c_set_clientdata(client, iio);
-
-	return 0;
-
-err_buffer_cleanup:
-	iio_triggered_buffer_cleanup(iio);
-err_regulator_disable:
-	regulator_disable(adc->ref);
-
-	return err;
-}
-
-static int adc081c_remove(struct i2c_client *client)
-{
-	struct iio_dev *iio = i2c_get_clientdata(client);
-	struct adc081c *adc = iio_priv(iio);
-
-	iio_device_unregister(iio);
-	iio_triggered_buffer_cleanup(iio);
-	regulator_disable(adc->ref);
-
-	return 0;
+	return devm_iio_device_register(&client->dev, iio);
 }
 
 static const struct i2c_device_id adc081c_id[] = {
@@ -238,7 +224,6 @@ static struct i2c_driver adc081c_driver = {
 		.of_match_table = adc081c_of_match,
 	},
 	.probe = adc081c_probe,
-	.remove = adc081c_remove,
 	.id_table = adc081c_id,
 };
 module_i2c_driver(adc081c_driver);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 6/8] iio: adc: ti-adc0832: Use devm managed functions for all of probe()
  2021-05-16 17:25 [PATCH 0/8] iio: adc: Maxim and TI ADC driver cleanups Jonathan Cameron
                   ` (4 preceding siblings ...)
  2021-05-16 17:25 ` [PATCH 5/8] iio: adc: ti-adc081c: Use devm managed functions for all of probe() Jonathan Cameron
@ 2021-05-16 17:25 ` Jonathan Cameron
  2021-05-17  7:28   ` Alexandru Ardelean
  2021-05-16 17:25 ` [PATCH 7/8] iio: adc: ti-adc108s102: " Jonathan Cameron
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2021-05-16 17:25 UTC (permalink / raw)
  To: linux-iio; +Cc: Alexandru Ardelean, Jonathan Cameron, Akinobu Mita

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Simplifies error handling, plus allows us to drop the remove()
function entirely.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/iio/adc/ti-adc0832.c | 39 +++++++++++-------------------------
 1 file changed, 12 insertions(+), 27 deletions(-)

diff --git a/drivers/iio/adc/ti-adc0832.c b/drivers/iio/adc/ti-adc0832.c
index 0261b3cfc92b..fb5e72600b96 100644
--- a/drivers/iio/adc/ti-adc0832.c
+++ b/drivers/iio/adc/ti-adc0832.c
@@ -236,6 +236,11 @@ static irqreturn_t adc0832_trigger_handler(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
+static void adc0832_reg_disable(void *reg)
+{
+	regulator_disable(reg);
+}
+
 static int adc0832_probe(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev;
@@ -287,36 +292,17 @@ static int adc0832_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	spi_set_drvdata(spi, indio_dev);
-
-	ret = iio_triggered_buffer_setup(indio_dev, NULL,
-					 adc0832_trigger_handler, NULL);
+	ret = devm_add_action_or_reset(&spi->dev, adc0832_reg_disable,
+				       adc->reg);
 	if (ret)
-		goto err_reg_disable;
+		return ret;
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
+					      adc0832_trigger_handler, NULL);
 	if (ret)
-		goto err_buffer_cleanup;
-
-	return 0;
-err_buffer_cleanup:
-	iio_triggered_buffer_cleanup(indio_dev);
-err_reg_disable:
-	regulator_disable(adc->reg);
-
-	return ret;
-}
-
-static int adc0832_remove(struct spi_device *spi)
-{
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct adc0832 *adc = iio_priv(indio_dev);
-
-	iio_device_unregister(indio_dev);
-	iio_triggered_buffer_cleanup(indio_dev);
-	regulator_disable(adc->reg);
+		return ret;
 
-	return 0;
+	return devm_iio_device_register(&spi->dev, indio_dev);
 }
 
 static const struct of_device_id adc0832_dt_ids[] = {
@@ -343,7 +329,6 @@ static struct spi_driver adc0832_driver = {
 		.of_match_table = adc0832_dt_ids,
 	},
 	.probe = adc0832_probe,
-	.remove = adc0832_remove,
 	.id_table = adc0832_id,
 };
 module_spi_driver(adc0832_driver);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 7/8] iio: adc: ti-adc108s102: Use devm managed functions for all of probe()
  2021-05-16 17:25 [PATCH 0/8] iio: adc: Maxim and TI ADC driver cleanups Jonathan Cameron
                   ` (5 preceding siblings ...)
  2021-05-16 17:25 ` [PATCH 6/8] iio: adc: ti-adc0832: " Jonathan Cameron
@ 2021-05-16 17:25 ` Jonathan Cameron
  2021-05-17  7:31   ` Alexandru Ardelean
  2021-05-16 17:25 ` [PATCH 8/8] iio: adc: ti-adc161s626: Use devm managed functions for all of probe Jonathan Cameron
  2021-05-22 18:31 ` [PATCH 0/8] iio: adc: Maxim and TI ADC driver cleanups Jonathan Cameron
  8 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2021-05-16 17:25 UTC (permalink / raw)
  To: linux-iio; +Cc: Alexandru Ardelean, Jonathan Cameron, Bogdan Pricop

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Simplifies error handling and lets us drop remove() entirely.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Bogdan Pricop <bogdan.pricop@emutex.com>
---
 drivers/iio/adc/ti-adc108s102.c | 45 +++++++++++----------------------
 1 file changed, 15 insertions(+), 30 deletions(-)

diff --git a/drivers/iio/adc/ti-adc108s102.c b/drivers/iio/adc/ti-adc108s102.c
index 183b2245e89b..db902aef2abe 100644
--- a/drivers/iio/adc/ti-adc108s102.c
+++ b/drivers/iio/adc/ti-adc108s102.c
@@ -215,6 +215,11 @@ static const struct iio_info adc108s102_info = {
 	.update_scan_mode	= &adc108s102_update_scan_mode,
 };
 
+static void adc108s102_reg_disable(void *reg)
+{
+	regulator_disable(reg);
+}
+
 static int adc108s102_probe(struct spi_device *spi)
 {
 	struct adc108s102_state *st;
@@ -239,6 +244,10 @@ static int adc108s102_probe(struct spi_device *spi)
 			dev_err(&spi->dev, "Cannot enable vref regulator\n");
 			return ret;
 		}
+		ret = devm_add_action_or_reset(&spi->dev, adc108s102_reg_disable,
+					       st->reg);
+		if (ret)
+			return ret;
 
 		ret = regulator_get_voltage(st->reg);
 		if (ret < 0) {
@@ -249,7 +258,6 @@ static int adc108s102_probe(struct spi_device *spi)
 		st->va_millivolt = ret / 1000;
 	}
 
-	spi_set_drvdata(spi, indio_dev);
 	st->spi = spi;
 
 	indio_dev->name = spi->modalias;
@@ -266,40 +274,18 @@ static int adc108s102_probe(struct spi_device *spi)
 	spi_message_init_with_transfers(&st->scan_single_msg,
 					&st->scan_single_xfer, 1);
 
-	ret = iio_triggered_buffer_setup(indio_dev, NULL,
-					 &adc108s102_trigger_handler, NULL);
+	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
+					      &adc108s102_trigger_handler,
+					      NULL);
 	if (ret)
-		goto error_disable_reg;
+		return ret;
 
-	ret = iio_device_register(indio_dev);
-	if (ret) {
+	ret = devm_iio_device_register(&spi->dev, indio_dev);
+	if (ret)
 		dev_err(&spi->dev, "Failed to register IIO device\n");
-		goto error_cleanup_triggered_buffer;
-	}
-	return 0;
-
-error_cleanup_triggered_buffer:
-	iio_triggered_buffer_cleanup(indio_dev);
-
-error_disable_reg:
-	regulator_disable(st->reg);
-
 	return ret;
 }
 
-static int adc108s102_remove(struct spi_device *spi)
-{
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct adc108s102_state *st = iio_priv(indio_dev);
-
-	iio_device_unregister(indio_dev);
-	iio_triggered_buffer_cleanup(indio_dev);
-
-	regulator_disable(st->reg);
-
-	return 0;
-}
-
 static const struct of_device_id adc108s102_of_match[] = {
 	{ .compatible = "ti,adc108s102" },
 	{ }
@@ -327,7 +313,6 @@ static struct spi_driver adc108s102_driver = {
 		.acpi_match_table = ACPI_PTR(adc108s102_acpi_ids),
 	},
 	.probe		= adc108s102_probe,
-	.remove		= adc108s102_remove,
 	.id_table	= adc108s102_id,
 };
 module_spi_driver(adc108s102_driver);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 8/8] iio: adc: ti-adc161s626: Use devm managed functions for all of probe.
  2021-05-16 17:25 [PATCH 0/8] iio: adc: Maxim and TI ADC driver cleanups Jonathan Cameron
                   ` (6 preceding siblings ...)
  2021-05-16 17:25 ` [PATCH 7/8] iio: adc: ti-adc108s102: " Jonathan Cameron
@ 2021-05-16 17:25 ` Jonathan Cameron
  2021-05-16 20:04   ` Matt Ranostay
  2021-05-17  7:35   ` Alexandru Ardelean
  2021-05-22 18:31 ` [PATCH 0/8] iio: adc: Maxim and TI ADC driver cleanups Jonathan Cameron
  8 siblings, 2 replies; 22+ messages in thread
From: Jonathan Cameron @ 2021-05-16 17:25 UTC (permalink / raw)
  To: linux-iio; +Cc: Alexandru Ardelean, Jonathan Cameron, Matt Ranostay

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Simplifies error handling and allows us to drop remove entirely.

The regulator handling in this driver was unusual as it would try to
acquire the regulator, but if that failed with an error would continue.

We should get a stub regulator if one isn't provided in DT and an error
could indicate an actual problem preventing the device being powered
(perhaps a need to defer). So this handling is cleaned up (arguably
that might be a fix but given no one has run into it, I haven't broken
it out separately.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Matt Ranostay <matt.ranostay@konsulko.com>
---
 drivers/iio/adc/ti-adc161s626.c | 50 ++++++++++++---------------------
 1 file changed, 18 insertions(+), 32 deletions(-)

diff --git a/drivers/iio/adc/ti-adc161s626.c b/drivers/iio/adc/ti-adc161s626.c
index 607791ffe7f0..9f2f25cf9a49 100644
--- a/drivers/iio/adc/ti-adc161s626.c
+++ b/drivers/iio/adc/ti-adc161s626.c
@@ -169,6 +169,11 @@ static const struct iio_info ti_adc_info = {
 	.read_raw = ti_adc_read_raw,
 };
 
+static void ti_adc_reg_disable(void *reg)
+{
+	regulator_disable(reg);
+}
+
 static int ti_adc_probe(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev;
@@ -203,42 +208,24 @@ static int ti_adc_probe(struct spi_device *spi)
 	}
 
 	data->ref = devm_regulator_get(&spi->dev, "vdda");
-	if (!IS_ERR(data->ref)) {
-		ret = regulator_enable(data->ref);
-		if (ret < 0)
-			return ret;
-	}
+	if (IS_ERR(data->ref))
+		return PTR_ERR(data->ref);
 
-	ret = iio_triggered_buffer_setup(indio_dev, NULL,
-					 ti_adc_trigger_handler, NULL);
-	if (ret)
-		goto error_regulator_disable;
+	ret = regulator_enable(data->ref);
+	if (ret < 0)
+		return ret;
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_add_action_or_reset(&spi->dev, ti_adc_reg_disable,
+				       data->ref);
 	if (ret)
-		goto error_unreg_buffer;
-
-	return 0;
+		return ret;
 
-error_unreg_buffer:
-	iio_triggered_buffer_cleanup(indio_dev);
-
-error_regulator_disable:
-	regulator_disable(data->ref);
-
-	return ret;
-}
-
-static int ti_adc_remove(struct spi_device *spi)
-{
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct ti_adc_data *data = iio_priv(indio_dev);
-
-	iio_device_unregister(indio_dev);
-	iio_triggered_buffer_cleanup(indio_dev);
-	regulator_disable(data->ref);
+	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
+					      ti_adc_trigger_handler, NULL);
+	if (ret)
+		return ret;
 
-	return 0;
+	return devm_iio_device_register(&spi->dev, indio_dev);
 }
 
 static const struct of_device_id ti_adc_dt_ids[] = {
@@ -261,7 +248,6 @@ static struct spi_driver ti_adc_driver = {
 		.of_match_table = ti_adc_dt_ids,
 	},
 	.probe		= ti_adc_probe,
-	.remove		= ti_adc_remove,
 	.id_table	= ti_adc_id,
 };
 module_spi_driver(ti_adc_driver);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 8/8] iio: adc: ti-adc161s626: Use devm managed functions for all of probe.
  2021-05-16 17:25 ` [PATCH 8/8] iio: adc: ti-adc161s626: Use devm managed functions for all of probe Jonathan Cameron
@ 2021-05-16 20:04   ` Matt Ranostay
  2021-05-17  7:35   ` Alexandru Ardelean
  1 sibling, 0 replies; 22+ messages in thread
From: Matt Ranostay @ 2021-05-16 20:04 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: open list:IIO SUBSYSTEM AND DRIVERS, Alexandru Ardelean,
	Jonathan Cameron

On Sun, May 16, 2021 at 10:26 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Simplifies error handling and allows us to drop remove entirely.
>
> The regulator handling in this driver was unusual as it would try to
> acquire the regulator, but if that failed with an error would continue.
>
> We should get a stub regulator if one isn't provided in DT and an error
> could indicate an actual problem preventing the device being powered
> (perhaps a need to defer). So this handling is cleaned up (arguably
> that might be a fix but given no one has run into it, I haven't broken
> it out separately.
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Matt Ranostay <matt.ranostay@konsulko.com>

Acked-by: Matt Ranostay <matt.ranostay@konsulko.com>

> ---
>  drivers/iio/adc/ti-adc161s626.c | 50 ++++++++++++---------------------
>  1 file changed, 18 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/iio/adc/ti-adc161s626.c b/drivers/iio/adc/ti-adc161s626.c
> index 607791ffe7f0..9f2f25cf9a49 100644
> --- a/drivers/iio/adc/ti-adc161s626.c
> +++ b/drivers/iio/adc/ti-adc161s626.c
> @@ -169,6 +169,11 @@ static const struct iio_info ti_adc_info = {
>         .read_raw = ti_adc_read_raw,
>  };
>
> +static void ti_adc_reg_disable(void *reg)
> +{
> +       regulator_disable(reg);
> +}
> +
>  static int ti_adc_probe(struct spi_device *spi)
>  {
>         struct iio_dev *indio_dev;
> @@ -203,42 +208,24 @@ static int ti_adc_probe(struct spi_device *spi)
>         }
>
>         data->ref = devm_regulator_get(&spi->dev, "vdda");
> -       if (!IS_ERR(data->ref)) {
> -               ret = regulator_enable(data->ref);
> -               if (ret < 0)
> -                       return ret;
> -       }
> +       if (IS_ERR(data->ref))
> +               return PTR_ERR(data->ref);
>
> -       ret = iio_triggered_buffer_setup(indio_dev, NULL,
> -                                        ti_adc_trigger_handler, NULL);
> -       if (ret)
> -               goto error_regulator_disable;
> +       ret = regulator_enable(data->ref);
> +       if (ret < 0)
> +               return ret;
>
> -       ret = iio_device_register(indio_dev);
> +       ret = devm_add_action_or_reset(&spi->dev, ti_adc_reg_disable,
> +                                      data->ref);
>         if (ret)
> -               goto error_unreg_buffer;
> -
> -       return 0;
> +               return ret;
>
> -error_unreg_buffer:
> -       iio_triggered_buffer_cleanup(indio_dev);
> -
> -error_regulator_disable:
> -       regulator_disable(data->ref);
> -
> -       return ret;
> -}
> -
> -static int ti_adc_remove(struct spi_device *spi)
> -{
> -       struct iio_dev *indio_dev = spi_get_drvdata(spi);
> -       struct ti_adc_data *data = iio_priv(indio_dev);
> -
> -       iio_device_unregister(indio_dev);
> -       iio_triggered_buffer_cleanup(indio_dev);
> -       regulator_disable(data->ref);
> +       ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
> +                                             ti_adc_trigger_handler, NULL);
> +       if (ret)
> +               return ret;
>
> -       return 0;
> +       return devm_iio_device_register(&spi->dev, indio_dev);
>  }
>
>  static const struct of_device_id ti_adc_dt_ids[] = {
> @@ -261,7 +248,6 @@ static struct spi_driver ti_adc_driver = {
>                 .of_match_table = ti_adc_dt_ids,
>         },
>         .probe          = ti_adc_probe,
> -       .remove         = ti_adc_remove,
>         .id_table       = ti_adc_id,
>  };
>  module_spi_driver(ti_adc_driver);
> --
> 2.31.1
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/8] iio: adc: max11100: Use get_unaligned_be16() rather than opencoding.
  2021-05-16 17:25 ` [PATCH 1/8] iio: adc: max11100: Use get_unaligned_be16() rather than opencoding Jonathan Cameron
@ 2021-05-17  6:59   ` Alexandru Ardelean
  0 siblings, 0 replies; 22+ messages in thread
From: Alexandru Ardelean @ 2021-05-17  6:59 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Jonathan Cameron, Jacopo Mondi

On Sun, 16 May 2021 at 20:26, Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> The function is more explicit in showing the intent + quicker on some
> platforms.

Reviewed-by: Alexandru Ardelean <aardelean@deviqon.com>

>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  drivers/iio/adc/max11100.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
> index 6cf21758ca66..69d607fa17aa 100644
> --- a/drivers/iio/adc/max11100.c
> +++ b/drivers/iio/adc/max11100.c
> @@ -12,6 +12,7 @@
>  #include <linux/module.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/spi/spi.h>
> +#include <asm/unaligned.h>
>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/driver.h>
> @@ -63,7 +64,7 @@ static int max11100_read_single(struct iio_dev *indio_dev, int *val)
>                 return -EINVAL;
>         }
>
> -       *val = (state->buffer[1] << 8) | state->buffer[2];
> +       *val = get_unaligned_be16(&state->buffer[1]);
>
>         return 0;
>  }
> --
> 2.31.1
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/8] iio: adc: max11100: Use devm_ functions for rest of probe()
  2021-05-16 17:25 ` [PATCH 2/8] iio: adc: max11100: Use devm_ functions for rest of probe() Jonathan Cameron
@ 2021-05-17  7:08   ` Alexandru Ardelean
  2021-05-17  7:22     ` Alexandru Ardelean
  0 siblings, 1 reply; 22+ messages in thread
From: Alexandru Ardelean @ 2021-05-17  7:08 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Jonathan Cameron, Jacopo Mondi

On Sun, 16 May 2021 at 20:26, Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> By using devm_add_action_or_reset() to manage the regulator disable,
> it becomes simple to use managed functions for all of remove.
> This simplifies error handling and allows us to drop the remove()
> function entirely.
>

Reviewed-by: Alexandru Ardelean <aardelean@deviqon.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  drivers/iio/adc/max11100.c | 29 +++++++++--------------------
>  1 file changed, 9 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
> index 69d607fa17aa..9951f6a6a4b9 100644
> --- a/drivers/iio/adc/max11100.c
> +++ b/drivers/iio/adc/max11100.c
> @@ -102,6 +102,11 @@ static const struct iio_info max11100_info = {
>         .read_raw = max11100_read_raw,
>  };
>
> +static void max11100_regulator_disable(void *reg)
> +{
> +       regulator_disable(reg);
> +}
> +
>  static int max11100_probe(struct spi_device *spi)
>  {
>         int ret;
> @@ -131,27 +136,12 @@ static int max11100_probe(struct spi_device *spi)
>         if (ret)
>                 return ret;
>
> -       ret = iio_device_register(indio_dev);
> +       ret = devm_add_action_or_reset(&spi->dev, max11100_regulator_disable,
> +                               state->vref_reg);
>         if (ret)
> -               goto disable_regulator;
> -
> -       return 0;
> -
> -disable_regulator:
> -       regulator_disable(state->vref_reg);
> -
> -       return ret;
> -}
> -
> -static int max11100_remove(struct spi_device *spi)
> -{
> -       struct iio_dev *indio_dev = spi_get_drvdata(spi);
> -       struct max11100_state *state = iio_priv(indio_dev);
> -
> -       iio_device_unregister(indio_dev);
> -       regulator_disable(state->vref_reg);
> +               return ret;
>
> -       return 0;
> +       return devm_iio_device_register(&spi->dev, indio_dev);
>  }
>
>  static const struct of_device_id max11100_ids[] = {
> @@ -166,7 +156,6 @@ static struct spi_driver max11100_driver = {
>                 .of_match_table = max11100_ids,
>         },
>         .probe          = max11100_probe,
> -       .remove         = max11100_remove,
>  };
>
>  module_spi_driver(max11100_driver);
> --
> 2.31.1
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/8] iio: adc: max1118: Use devm_ managed functions for all of probe
  2021-05-16 17:25 ` [PATCH 3/8] iio: adc: max1118: Use devm_ managed functions for all of probe Jonathan Cameron
@ 2021-05-17  7:18   ` Alexandru Ardelean
  0 siblings, 0 replies; 22+ messages in thread
From: Alexandru Ardelean @ 2021-05-17  7:18 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Jonathan Cameron, Akinobu Mita

On Sun, 16 May 2021 at 20:26, Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> This simplifies error handling and allows us to drop the remove
> function entirely.
>

Reviewed-by: Alexandru Ardelean <aardelean@deviqon.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Akinobu Mita <akinobu.mita@gmail.com>
> ---
>  drivers/iio/adc/max1118.c | 46 +++++++++++++--------------------------
>  1 file changed, 15 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/iio/adc/max1118.c b/drivers/iio/adc/max1118.c
> index 6efb0b43d938..4dfbed63ad7f 100644
> --- a/drivers/iio/adc/max1118.c
> +++ b/drivers/iio/adc/max1118.c
> @@ -201,6 +201,11 @@ static irqreturn_t max1118_trigger_handler(int irq, void *p)
>         return IRQ_HANDLED;
>  }
>
> +static void max1118_reg_disable(void *reg)
> +{
> +       regulator_disable(reg);
> +}
> +
>  static int max1118_probe(struct spi_device *spi)
>  {
>         struct iio_dev *indio_dev;
> @@ -225,6 +230,12 @@ static int max1118_probe(struct spi_device *spi)
>                 ret = regulator_enable(adc->reg);
>                 if (ret)
>                         return ret;
> +
> +               ret = devm_add_action_or_reset(&spi->dev, max1118_reg_disable,
> +                                              adc->reg);
> +               if (ret)
> +                       return ret;
> +
>         }
>
>         spi_set_drvdata(spi, indio_dev);
> @@ -243,38 +254,12 @@ static int max1118_probe(struct spi_device *spi)
>          */
>         max1118_read(spi, 0);
>
> -       ret = iio_triggered_buffer_setup(indio_dev, NULL,
> -                                       max1118_trigger_handler, NULL);
> +       ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
> +                                             max1118_trigger_handler, NULL);
>         if (ret)
> -               goto err_reg_disable;
> -
> -       ret = iio_device_register(indio_dev);
> -       if (ret)
> -               goto err_buffer_cleanup;
> -
> -       return 0;
> -
> -err_buffer_cleanup:
> -       iio_triggered_buffer_cleanup(indio_dev);
> -err_reg_disable:
> -       if (id->driver_data == max1118)
> -               regulator_disable(adc->reg);
> -
> -       return ret;
> -}
> -
> -static int max1118_remove(struct spi_device *spi)
> -{
> -       struct iio_dev *indio_dev = spi_get_drvdata(spi);
> -       struct max1118 *adc = iio_priv(indio_dev);
> -       const struct spi_device_id *id = spi_get_device_id(spi);
> -
> -       iio_device_unregister(indio_dev);
> -       iio_triggered_buffer_cleanup(indio_dev);
> -       if (id->driver_data == max1118)
> -               return regulator_disable(adc->reg);
> +               return ret;
>
> -       return 0;
> +       return devm_iio_device_register(&spi->dev, indio_dev);
>  }
>
>  static const struct spi_device_id max1118_id[] = {
> @@ -299,7 +284,6 @@ static struct spi_driver max1118_spi_driver = {
>                 .of_match_table = max1118_dt_ids,
>         },
>         .probe = max1118_probe,
> -       .remove = max1118_remove,
>         .id_table = max1118_id,
>  };
>  module_spi_driver(max1118_spi_driver);
> --
> 2.31.1
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/8] iio: adc: max11100: Use devm_ functions for rest of probe()
  2021-05-17  7:08   ` Alexandru Ardelean
@ 2021-05-17  7:22     ` Alexandru Ardelean
  0 siblings, 0 replies; 22+ messages in thread
From: Alexandru Ardelean @ 2021-05-17  7:22 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Jonathan Cameron, Jacopo Mondi

On Mon, 17 May 2021 at 10:08, Alexandru Ardelean <aardelean@deviqon.com> wrote:
>
> On Sun, 16 May 2021 at 20:26, Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > By using devm_add_action_or_reset() to manage the regulator disable,
> > it becomes simple to use managed functions for all of remove.
> > This simplifies error handling and allows us to drop the remove()
> > function entirely.
> >
>

Apologies to come back on this.
But "spi_set_drvdata(spi, indio_dev);"  can be removed in this patch.
And while at it, the "state->vref_reg" parameter can be indented a bit.

> Reviewed-by: Alexandru Ardelean <aardelean@deviqon.com>
>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Cc: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  drivers/iio/adc/max11100.c | 29 +++++++++--------------------
> >  1 file changed, 9 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
> > index 69d607fa17aa..9951f6a6a4b9 100644
> > --- a/drivers/iio/adc/max11100.c
> > +++ b/drivers/iio/adc/max11100.c
> > @@ -102,6 +102,11 @@ static const struct iio_info max11100_info = {
> >         .read_raw = max11100_read_raw,
> >  };
> >
> > +static void max11100_regulator_disable(void *reg)
> > +{
> > +       regulator_disable(reg);
> > +}
> > +
> >  static int max11100_probe(struct spi_device *spi)
> >  {
> >         int ret;
> > @@ -131,27 +136,12 @@ static int max11100_probe(struct spi_device *spi)
> >         if (ret)
> >                 return ret;
> >
> > -       ret = iio_device_register(indio_dev);
> > +       ret = devm_add_action_or_reset(&spi->dev, max11100_regulator_disable,
> > +                               state->vref_reg);
> >         if (ret)
> > -               goto disable_regulator;
> > -
> > -       return 0;
> > -
> > -disable_regulator:
> > -       regulator_disable(state->vref_reg);
> > -
> > -       return ret;
> > -}
> > -
> > -static int max11100_remove(struct spi_device *spi)
> > -{
> > -       struct iio_dev *indio_dev = spi_get_drvdata(spi);
> > -       struct max11100_state *state = iio_priv(indio_dev);
> > -
> > -       iio_device_unregister(indio_dev);
> > -       regulator_disable(state->vref_reg);
> > +               return ret;
> >
> > -       return 0;
> > +       return devm_iio_device_register(&spi->dev, indio_dev);
> >  }
> >
> >  static const struct of_device_id max11100_ids[] = {
> > @@ -166,7 +156,6 @@ static struct spi_driver max11100_driver = {
> >                 .of_match_table = max11100_ids,
> >         },
> >         .probe          = max11100_probe,
> > -       .remove         = max11100_remove,
> >  };
> >
> >  module_spi_driver(max11100_driver);
> > --
> > 2.31.1
> >

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 4/8] iio: adc: max1118: Avoid jumping back and forth between spi and iio structures
  2021-05-16 17:25 ` [PATCH 4/8] iio: adc: max1118: Avoid jumping back and forth between spi and iio structures Jonathan Cameron
@ 2021-05-17  7:24   ` Alexandru Ardelean
  0 siblings, 0 replies; 22+ messages in thread
From: Alexandru Ardelean @ 2021-05-17  7:24 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Jonathan Cameron

On Sun, 16 May 2021 at 20:26, Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Changing from passing the spi structure into various functions to
> passing struct iio_dev avoids use of spi_get_drvdata and lets us
> stop setting that at all.  Previous code was unnecessarily complex.
>

Reviewed-by: Alexandru Ardelean <aardelean@deviqon.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/iio/adc/max1118.c | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/iio/adc/max1118.c b/drivers/iio/adc/max1118.c
> index 4dfbed63ad7f..8cec9d949083 100644
> --- a/drivers/iio/adc/max1118.c
> +++ b/drivers/iio/adc/max1118.c
> @@ -66,9 +66,8 @@ static const struct iio_chan_spec max1118_channels[] = {
>         IIO_CHAN_SOFT_TIMESTAMP(2),
>  };
>
> -static int max1118_read(struct spi_device *spi, int channel)
> +static int max1118_read(struct iio_dev *indio_dev, int channel)
>  {
> -       struct iio_dev *indio_dev = spi_get_drvdata(spi);
>         struct max1118 *adc = iio_priv(indio_dev);
>         struct spi_transfer xfers[] = {
>                 /*
> @@ -103,9 +102,9 @@ static int max1118_read(struct spi_device *spi, int channel)
>         int ret;
>
>         if (channel == 0)
> -               ret = spi_sync_transfer(spi, xfers + 1, 2);
> +               ret = spi_sync_transfer(adc->spi, xfers + 1, 2);
>         else
> -               ret = spi_sync_transfer(spi, xfers, 3);
> +               ret = spi_sync_transfer(adc->spi, xfers, 3);
>
>         if (ret)
>                 return ret;
> @@ -113,11 +112,10 @@ static int max1118_read(struct spi_device *spi, int channel)
>         return adc->data;
>  }
>
> -static int max1118_get_vref_mV(struct spi_device *spi)
> +static int max1118_get_vref_mV(struct iio_dev *indio_dev)
>  {
> -       struct iio_dev *indio_dev = spi_get_drvdata(spi);
>         struct max1118 *adc = iio_priv(indio_dev);
> -       const struct spi_device_id *id = spi_get_device_id(spi);
> +       const struct spi_device_id *id = spi_get_device_id(adc->spi);
>         int vref_uV;
>
>         switch (id->driver_data) {
> @@ -144,14 +142,14 @@ static int max1118_read_raw(struct iio_dev *indio_dev,
>         switch (mask) {
>         case IIO_CHAN_INFO_RAW:
>                 mutex_lock(&adc->lock);
> -               *val = max1118_read(adc->spi, chan->channel);
> +               *val = max1118_read(indio_dev, chan->channel);
>                 mutex_unlock(&adc->lock);
>                 if (*val < 0)
>                         return *val;
>
>                 return IIO_VAL_INT;
>         case IIO_CHAN_INFO_SCALE:
> -               *val = max1118_get_vref_mV(adc->spi);
> +               *val = max1118_get_vref_mV(indio_dev);
>                 if (*val < 0)
>                         return *val;
>                 *val2 = 8;
> @@ -180,7 +178,7 @@ static irqreturn_t max1118_trigger_handler(int irq, void *p)
>                         indio_dev->masklength) {
>                 const struct iio_chan_spec *scan_chan =
>                                 &indio_dev->channels[scan_index];
> -               int ret = max1118_read(adc->spi, scan_chan->channel);
> +               int ret = max1118_read(indio_dev, scan_chan->channel);
>
>                 if (ret < 0) {
>                         dev_warn(&adc->spi->dev,
> @@ -238,8 +236,6 @@ static int max1118_probe(struct spi_device *spi)
>
>         }
>
> -       spi_set_drvdata(spi, indio_dev);
> -
>         indio_dev->name = spi_get_device_id(spi)->name;
>         indio_dev->info = &max1118_info;
>         indio_dev->modes = INDIO_DIRECT_MODE;
> @@ -252,7 +248,7 @@ static int max1118_probe(struct spi_device *spi)
>          * a conversion has been completed, the MAX1117/MAX1118/MAX1119 will go
>          * into AutoShutdown mode until the next conversion is initiated.
>          */
> -       max1118_read(spi, 0);
> +       max1118_read(indio_dev, 0);
>
>         ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
>                                               max1118_trigger_handler, NULL);
> --
> 2.31.1
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] iio: adc: ti-adc081c: Use devm managed functions for all of probe()
  2021-05-16 17:25 ` [PATCH 5/8] iio: adc: ti-adc081c: Use devm managed functions for all of probe() Jonathan Cameron
@ 2021-05-17  7:26   ` Alexandru Ardelean
  0 siblings, 0 replies; 22+ messages in thread
From: Alexandru Ardelean @ 2021-05-17  7:26 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Jonathan Cameron

On Sun, 16 May 2021 at 20:26, Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Simplifies error handling and allows us to drop remove() entirely.
>

Reviewed-by: Alexandru Ardelean <aardelean@deviqon.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/iio/adc/ti-adc081c.c | 43 ++++++++++++------------------------
>  1 file changed, 14 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/iio/adc/ti-adc081c.c b/drivers/iio/adc/ti-adc081c.c
> index b64718daa201..16fc608db36a 100644
> --- a/drivers/iio/adc/ti-adc081c.c
> +++ b/drivers/iio/adc/ti-adc081c.c
> @@ -146,6 +146,11 @@ static irqreturn_t adc081c_trigger_handler(int irq, void *p)
>         return IRQ_HANDLED;
>  }
>
> +static void adc081c_reg_disable(void *reg)
> +{
> +       regulator_disable(reg);
> +}
> +
>  static int adc081c_probe(struct i2c_client *client,
>                          const struct i2c_device_id *id)
>  {
> @@ -175,6 +180,11 @@ static int adc081c_probe(struct i2c_client *client,
>         if (err < 0)
>                 return err;
>
> +       err = devm_add_action_or_reset(&client->dev, adc081c_reg_disable,
> +                                      adc->ref);
> +       if (err)
> +               return err;
> +
>         iio->name = dev_name(&client->dev);
>         iio->modes = INDIO_DIRECT_MODE;
>         iio->info = &adc081c_info;
> @@ -182,38 +192,14 @@ static int adc081c_probe(struct i2c_client *client,
>         iio->channels = model->channels;
>         iio->num_channels = ADC081C_NUM_CHANNELS;
>
> -       err = iio_triggered_buffer_setup(iio, NULL, adc081c_trigger_handler, NULL);
> +       err = devm_iio_triggered_buffer_setup(&client->dev, iio, NULL,
> +                                             adc081c_trigger_handler, NULL);
>         if (err < 0) {
>                 dev_err(&client->dev, "iio triggered buffer setup failed\n");
> -               goto err_regulator_disable;
> +               return err;
>         }
>
> -       err = iio_device_register(iio);
> -       if (err < 0)
> -               goto err_buffer_cleanup;
> -
> -       i2c_set_clientdata(client, iio);
> -
> -       return 0;
> -
> -err_buffer_cleanup:
> -       iio_triggered_buffer_cleanup(iio);
> -err_regulator_disable:
> -       regulator_disable(adc->ref);
> -
> -       return err;
> -}
> -
> -static int adc081c_remove(struct i2c_client *client)
> -{
> -       struct iio_dev *iio = i2c_get_clientdata(client);
> -       struct adc081c *adc = iio_priv(iio);
> -
> -       iio_device_unregister(iio);
> -       iio_triggered_buffer_cleanup(iio);
> -       regulator_disable(adc->ref);
> -
> -       return 0;
> +       return devm_iio_device_register(&client->dev, iio);
>  }
>
>  static const struct i2c_device_id adc081c_id[] = {
> @@ -238,7 +224,6 @@ static struct i2c_driver adc081c_driver = {
>                 .of_match_table = adc081c_of_match,
>         },
>         .probe = adc081c_probe,
> -       .remove = adc081c_remove,
>         .id_table = adc081c_id,
>  };
>  module_i2c_driver(adc081c_driver);
> --
> 2.31.1
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 6/8] iio: adc: ti-adc0832: Use devm managed functions for all of probe()
  2021-05-16 17:25 ` [PATCH 6/8] iio: adc: ti-adc0832: " Jonathan Cameron
@ 2021-05-17  7:28   ` Alexandru Ardelean
  0 siblings, 0 replies; 22+ messages in thread
From: Alexandru Ardelean @ 2021-05-17  7:28 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Jonathan Cameron, Akinobu Mita

On Sun, 16 May 2021 at 20:26, Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Simplifies error handling, plus allows us to drop the remove()
> function entirely.
>

Reviewed-by: Alexandru Ardelean <aardelean@deviqon.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Akinobu Mita <akinobu.mita@gmail.com>
> ---
>  drivers/iio/adc/ti-adc0832.c | 39 +++++++++++-------------------------
>  1 file changed, 12 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/iio/adc/ti-adc0832.c b/drivers/iio/adc/ti-adc0832.c
> index 0261b3cfc92b..fb5e72600b96 100644
> --- a/drivers/iio/adc/ti-adc0832.c
> +++ b/drivers/iio/adc/ti-adc0832.c
> @@ -236,6 +236,11 @@ static irqreturn_t adc0832_trigger_handler(int irq, void *p)
>         return IRQ_HANDLED;
>  }
>
> +static void adc0832_reg_disable(void *reg)
> +{
> +       regulator_disable(reg);
> +}
> +
>  static int adc0832_probe(struct spi_device *spi)
>  {
>         struct iio_dev *indio_dev;
> @@ -287,36 +292,17 @@ static int adc0832_probe(struct spi_device *spi)
>         if (ret)
>                 return ret;
>
> -       spi_set_drvdata(spi, indio_dev);
> -
> -       ret = iio_triggered_buffer_setup(indio_dev, NULL,
> -                                        adc0832_trigger_handler, NULL);
> +       ret = devm_add_action_or_reset(&spi->dev, adc0832_reg_disable,
> +                                      adc->reg);
>         if (ret)
> -               goto err_reg_disable;
> +               return ret;
>
> -       ret = iio_device_register(indio_dev);
> +       ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
> +                                             adc0832_trigger_handler, NULL);
>         if (ret)
> -               goto err_buffer_cleanup;
> -
> -       return 0;
> -err_buffer_cleanup:
> -       iio_triggered_buffer_cleanup(indio_dev);
> -err_reg_disable:
> -       regulator_disable(adc->reg);
> -
> -       return ret;
> -}
> -
> -static int adc0832_remove(struct spi_device *spi)
> -{
> -       struct iio_dev *indio_dev = spi_get_drvdata(spi);
> -       struct adc0832 *adc = iio_priv(indio_dev);
> -
> -       iio_device_unregister(indio_dev);
> -       iio_triggered_buffer_cleanup(indio_dev);
> -       regulator_disable(adc->reg);
> +               return ret;
>
> -       return 0;
> +       return devm_iio_device_register(&spi->dev, indio_dev);
>  }
>
>  static const struct of_device_id adc0832_dt_ids[] = {
> @@ -343,7 +329,6 @@ static struct spi_driver adc0832_driver = {
>                 .of_match_table = adc0832_dt_ids,
>         },
>         .probe = adc0832_probe,
> -       .remove = adc0832_remove,
>         .id_table = adc0832_id,
>  };
>  module_spi_driver(adc0832_driver);
> --
> 2.31.1
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 7/8] iio: adc: ti-adc108s102: Use devm managed functions for all of probe()
  2021-05-16 17:25 ` [PATCH 7/8] iio: adc: ti-adc108s102: " Jonathan Cameron
@ 2021-05-17  7:31   ` Alexandru Ardelean
  2021-05-17  7:37     ` Alexandru Ardelean
  0 siblings, 1 reply; 22+ messages in thread
From: Alexandru Ardelean @ 2021-05-17  7:31 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Jonathan Cameron, Bogdan Pricop

On Sun, 16 May 2021 at 20:26, Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Simplifies error handling and lets us drop remove() entirely.
>

Reviewed-by: Alexandru Ardelean <aardelean@deviqon.com>


> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Bogdan Pricop <bogdan.pricop@emutex.com>
> ---
>  drivers/iio/adc/ti-adc108s102.c | 45 +++++++++++----------------------
>  1 file changed, 15 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/iio/adc/ti-adc108s102.c b/drivers/iio/adc/ti-adc108s102.c
> index 183b2245e89b..db902aef2abe 100644
> --- a/drivers/iio/adc/ti-adc108s102.c
> +++ b/drivers/iio/adc/ti-adc108s102.c
> @@ -215,6 +215,11 @@ static const struct iio_info adc108s102_info = {
>         .update_scan_mode       = &adc108s102_update_scan_mode,
>  };
>
> +static void adc108s102_reg_disable(void *reg)
> +{
> +       regulator_disable(reg);
> +}
> +
>  static int adc108s102_probe(struct spi_device *spi)
>  {
>         struct adc108s102_state *st;
> @@ -239,6 +244,10 @@ static int adc108s102_probe(struct spi_device *spi)
>                         dev_err(&spi->dev, "Cannot enable vref regulator\n");
>                         return ret;
>                 }
> +               ret = devm_add_action_or_reset(&spi->dev, adc108s102_reg_disable,
> +                                              st->reg);
> +               if (ret)
> +                       return ret;
>
>                 ret = regulator_get_voltage(st->reg);
>                 if (ret < 0) {
> @@ -249,7 +258,6 @@ static int adc108s102_probe(struct spi_device *spi)
>                 st->va_millivolt = ret / 1000;
>         }
>
> -       spi_set_drvdata(spi, indio_dev);
>         st->spi = spi;
>
>         indio_dev->name = spi->modalias;
> @@ -266,40 +274,18 @@ static int adc108s102_probe(struct spi_device *spi)
>         spi_message_init_with_transfers(&st->scan_single_msg,
>                                         &st->scan_single_xfer, 1);
>
> -       ret = iio_triggered_buffer_setup(indio_dev, NULL,
> -                                        &adc108s102_trigger_handler, NULL);
> +       ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
> +                                             &adc108s102_trigger_handler,
> +                                             NULL);
>         if (ret)
> -               goto error_disable_reg;
> +               return ret;
>
> -       ret = iio_device_register(indio_dev);
> -       if (ret) {
> +       ret = devm_iio_device_register(&spi->dev, indio_dev);
> +       if (ret)
>                 dev_err(&spi->dev, "Failed to register IIO device\n");
> -               goto error_cleanup_triggered_buffer;
> -       }
> -       return 0;
> -
> -error_cleanup_triggered_buffer:
> -       iio_triggered_buffer_cleanup(indio_dev);
> -
> -error_disable_reg:
> -       regulator_disable(st->reg);
> -
>         return ret;
>  }
>
> -static int adc108s102_remove(struct spi_device *spi)
> -{
> -       struct iio_dev *indio_dev = spi_get_drvdata(spi);
> -       struct adc108s102_state *st = iio_priv(indio_dev);
> -
> -       iio_device_unregister(indio_dev);
> -       iio_triggered_buffer_cleanup(indio_dev);
> -
> -       regulator_disable(st->reg);
> -
> -       return 0;
> -}
> -
>  static const struct of_device_id adc108s102_of_match[] = {
>         { .compatible = "ti,adc108s102" },
>         { }
> @@ -327,7 +313,6 @@ static struct spi_driver adc108s102_driver = {
>                 .acpi_match_table = ACPI_PTR(adc108s102_acpi_ids),
>         },
>         .probe          = adc108s102_probe,
> -       .remove         = adc108s102_remove,
>         .id_table       = adc108s102_id,
>  };
>  module_spi_driver(adc108s102_driver);
> --
> 2.31.1
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 8/8] iio: adc: ti-adc161s626: Use devm managed functions for all of probe.
  2021-05-16 17:25 ` [PATCH 8/8] iio: adc: ti-adc161s626: Use devm managed functions for all of probe Jonathan Cameron
  2021-05-16 20:04   ` Matt Ranostay
@ 2021-05-17  7:35   ` Alexandru Ardelean
  1 sibling, 0 replies; 22+ messages in thread
From: Alexandru Ardelean @ 2021-05-17  7:35 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Jonathan Cameron, Matt Ranostay

On Sun, 16 May 2021 at 20:26, Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Simplifies error handling and allows us to drop remove entirely.
>
> The regulator handling in this driver was unusual as it would try to
> acquire the regulator, but if that failed with an error would continue.
>
> We should get a stub regulator if one isn't provided in DT and an error
> could indicate an actual problem preventing the device being powered
> (perhaps a need to defer). So this handling is cleaned up (arguably
> that might be a fix but given no one has run into it, I haven't broken
> it out separately.
>

This patch forgets to remove "spi_set_drvdata(spi, indio_dev);"
With that fixed

Reviewed-by: Alexandru Ardelean <aardelean@deviqon.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Matt Ranostay <matt.ranostay@konsulko.com>
> ---
>  drivers/iio/adc/ti-adc161s626.c | 50 ++++++++++++---------------------
>  1 file changed, 18 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/iio/adc/ti-adc161s626.c b/drivers/iio/adc/ti-adc161s626.c
> index 607791ffe7f0..9f2f25cf9a49 100644
> --- a/drivers/iio/adc/ti-adc161s626.c
> +++ b/drivers/iio/adc/ti-adc161s626.c
> @@ -169,6 +169,11 @@ static const struct iio_info ti_adc_info = {
>         .read_raw = ti_adc_read_raw,
>  };
>
> +static void ti_adc_reg_disable(void *reg)
> +{
> +       regulator_disable(reg);
> +}
> +
>  static int ti_adc_probe(struct spi_device *spi)
>  {
>         struct iio_dev *indio_dev;
> @@ -203,42 +208,24 @@ static int ti_adc_probe(struct spi_device *spi)
>         }
>
>         data->ref = devm_regulator_get(&spi->dev, "vdda");
> -       if (!IS_ERR(data->ref)) {
> -               ret = regulator_enable(data->ref);
> -               if (ret < 0)
> -                       return ret;
> -       }
> +       if (IS_ERR(data->ref))
> +               return PTR_ERR(data->ref);
>
> -       ret = iio_triggered_buffer_setup(indio_dev, NULL,
> -                                        ti_adc_trigger_handler, NULL);
> -       if (ret)
> -               goto error_regulator_disable;
> +       ret = regulator_enable(data->ref);
> +       if (ret < 0)
> +               return ret;
>
> -       ret = iio_device_register(indio_dev);
> +       ret = devm_add_action_or_reset(&spi->dev, ti_adc_reg_disable,
> +                                      data->ref);
>         if (ret)
> -               goto error_unreg_buffer;
> -
> -       return 0;
> +               return ret;
>
> -error_unreg_buffer:
> -       iio_triggered_buffer_cleanup(indio_dev);
> -
> -error_regulator_disable:
> -       regulator_disable(data->ref);
> -
> -       return ret;
> -}
> -
> -static int ti_adc_remove(struct spi_device *spi)
> -{
> -       struct iio_dev *indio_dev = spi_get_drvdata(spi);
> -       struct ti_adc_data *data = iio_priv(indio_dev);
> -
> -       iio_device_unregister(indio_dev);
> -       iio_triggered_buffer_cleanup(indio_dev);
> -       regulator_disable(data->ref);
> +       ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
> +                                             ti_adc_trigger_handler, NULL);
> +       if (ret)
> +               return ret;
>
> -       return 0;
> +       return devm_iio_device_register(&spi->dev, indio_dev);
>  }
>
>  static const struct of_device_id ti_adc_dt_ids[] = {
> @@ -261,7 +248,6 @@ static struct spi_driver ti_adc_driver = {
>                 .of_match_table = ti_adc_dt_ids,
>         },
>         .probe          = ti_adc_probe,
> -       .remove         = ti_adc_remove,
>         .id_table       = ti_adc_id,
>  };
>  module_spi_driver(ti_adc_driver);
> --
> 2.31.1
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 7/8] iio: adc: ti-adc108s102: Use devm managed functions for all of probe()
  2021-05-17  7:31   ` Alexandru Ardelean
@ 2021-05-17  7:37     ` Alexandru Ardelean
  2021-05-22 18:29       ` Jonathan Cameron
  0 siblings, 1 reply; 22+ messages in thread
From: Alexandru Ardelean @ 2021-05-17  7:37 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Jonathan Cameron

On Mon, 17 May 2021 at 10:31, Alexandru Ardelean <aardelean@deviqon.com> wrote:
>
> On Sun, 16 May 2021 at 20:26, Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > Simplifies error handling and lets us drop remove() entirely.
> >
>

// Removed Bogdan's email; it's not working;
Apologies to return on this.
This note can be disregarded, but this patch could do a direct return
for devm_iio_device_register().
It doesn't make much sense to print a failure there as that can be
also viewed via some broader kernel logging.
But it's also fine to leave it.

> Reviewed-by: Alexandru Ardelean <aardelean@deviqon.com>
>
>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Cc: Bogdan Pricop <bogdan.pricop@emutex.com>
> > ---
> >  drivers/iio/adc/ti-adc108s102.c | 45 +++++++++++----------------------
> >  1 file changed, 15 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ti-adc108s102.c b/drivers/iio/adc/ti-adc108s102.c
> > index 183b2245e89b..db902aef2abe 100644
> > --- a/drivers/iio/adc/ti-adc108s102.c
> > +++ b/drivers/iio/adc/ti-adc108s102.c
> > @@ -215,6 +215,11 @@ static const struct iio_info adc108s102_info = {
> >         .update_scan_mode       = &adc108s102_update_scan_mode,
> >  };
> >
> > +static void adc108s102_reg_disable(void *reg)
> > +{
> > +       regulator_disable(reg);
> > +}
> > +
> >  static int adc108s102_probe(struct spi_device *spi)
> >  {
> >         struct adc108s102_state *st;
> > @@ -239,6 +244,10 @@ static int adc108s102_probe(struct spi_device *spi)
> >                         dev_err(&spi->dev, "Cannot enable vref regulator\n");
> >                         return ret;
> >                 }
> > +               ret = devm_add_action_or_reset(&spi->dev, adc108s102_reg_disable,
> > +                                              st->reg);
> > +               if (ret)
> > +                       return ret;
> >
> >                 ret = regulator_get_voltage(st->reg);
> >                 if (ret < 0) {
> > @@ -249,7 +258,6 @@ static int adc108s102_probe(struct spi_device *spi)
> >                 st->va_millivolt = ret / 1000;
> >         }
> >
> > -       spi_set_drvdata(spi, indio_dev);
> >         st->spi = spi;
> >
> >         indio_dev->name = spi->modalias;
> > @@ -266,40 +274,18 @@ static int adc108s102_probe(struct spi_device *spi)
> >         spi_message_init_with_transfers(&st->scan_single_msg,
> >                                         &st->scan_single_xfer, 1);
> >
> > -       ret = iio_triggered_buffer_setup(indio_dev, NULL,
> > -                                        &adc108s102_trigger_handler, NULL);
> > +       ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
> > +                                             &adc108s102_trigger_handler,
> > +                                             NULL);
> >         if (ret)
> > -               goto error_disable_reg;
> > +               return ret;
> >
> > -       ret = iio_device_register(indio_dev);
> > -       if (ret) {
> > +       ret = devm_iio_device_register(&spi->dev, indio_dev);
> > +       if (ret)
> >                 dev_err(&spi->dev, "Failed to register IIO device\n");
> > -               goto error_cleanup_triggered_buffer;
> > -       }
> > -       return 0;
> > -
> > -error_cleanup_triggered_buffer:
> > -       iio_triggered_buffer_cleanup(indio_dev);
> > -
> > -error_disable_reg:
> > -       regulator_disable(st->reg);
> > -
> >         return ret;
> >  }
> >
> > -static int adc108s102_remove(struct spi_device *spi)
> > -{
> > -       struct iio_dev *indio_dev = spi_get_drvdata(spi);
> > -       struct adc108s102_state *st = iio_priv(indio_dev);
> > -
> > -       iio_device_unregister(indio_dev);
> > -       iio_triggered_buffer_cleanup(indio_dev);
> > -
> > -       regulator_disable(st->reg);
> > -
> > -       return 0;
> > -}
> > -
> >  static const struct of_device_id adc108s102_of_match[] = {
> >         { .compatible = "ti,adc108s102" },
> >         { }
> > @@ -327,7 +313,6 @@ static struct spi_driver adc108s102_driver = {
> >                 .acpi_match_table = ACPI_PTR(adc108s102_acpi_ids),
> >         },
> >         .probe          = adc108s102_probe,
> > -       .remove         = adc108s102_remove,
> >         .id_table       = adc108s102_id,
> >  };
> >  module_spi_driver(adc108s102_driver);
> > --
> > 2.31.1
> >

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 7/8] iio: adc: ti-adc108s102: Use devm managed functions for all of probe()
  2021-05-17  7:37     ` Alexandru Ardelean
@ 2021-05-22 18:29       ` Jonathan Cameron
  0 siblings, 0 replies; 22+ messages in thread
From: Jonathan Cameron @ 2021-05-22 18:29 UTC (permalink / raw)
  To: Alexandru Ardelean; +Cc: linux-iio, Jonathan Cameron

On Mon, 17 May 2021 10:37:31 +0300
Alexandru Ardelean <aardelean@deviqon.com> wrote:

> On Mon, 17 May 2021 at 10:31, Alexandru Ardelean <aardelean@deviqon.com> wrote:
> >
> > On Sun, 16 May 2021 at 20:26, Jonathan Cameron <jic23@kernel.org> wrote:  
> > >
> > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > >
> > > Simplifies error handling and lets us drop remove() entirely.
> > >  
> >  
> 
> // Removed Bogdan's email; it's not working;
> Apologies to return on this.
> This note can be disregarded, but this patch could do a direct return
> for devm_iio_device_register().
> It doesn't make much sense to print a failure there as that can be
> also viewed via some broader kernel logging.
> But it's also fine to leave it.

Whilst I agree in the sense I would never have put it there in the first place
myself, I wanted this series to be one with no visible changes so
I'd rather leave it in place.

Thanks,

Jonathan
> 
> > Reviewed-by: Alexandru Ardelean <aardelean@deviqon.com>
> >
> >  
> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > Cc: Bogdan Pricop <bogdan.pricop@emutex.com>
> > > ---
> > >  drivers/iio/adc/ti-adc108s102.c | 45 +++++++++++----------------------
> > >  1 file changed, 15 insertions(+), 30 deletions(-)
> > >
> > > diff --git a/drivers/iio/adc/ti-adc108s102.c b/drivers/iio/adc/ti-adc108s102.c
> > > index 183b2245e89b..db902aef2abe 100644
> > > --- a/drivers/iio/adc/ti-adc108s102.c
> > > +++ b/drivers/iio/adc/ti-adc108s102.c
> > > @@ -215,6 +215,11 @@ static const struct iio_info adc108s102_info = {
> > >         .update_scan_mode       = &adc108s102_update_scan_mode,
> > >  };
> > >
> > > +static void adc108s102_reg_disable(void *reg)
> > > +{
> > > +       regulator_disable(reg);
> > > +}
> > > +
> > >  static int adc108s102_probe(struct spi_device *spi)
> > >  {
> > >         struct adc108s102_state *st;
> > > @@ -239,6 +244,10 @@ static int adc108s102_probe(struct spi_device *spi)
> > >                         dev_err(&spi->dev, "Cannot enable vref regulator\n");
> > >                         return ret;
> > >                 }
> > > +               ret = devm_add_action_or_reset(&spi->dev, adc108s102_reg_disable,
> > > +                                              st->reg);
> > > +               if (ret)
> > > +                       return ret;
> > >
> > >                 ret = regulator_get_voltage(st->reg);
> > >                 if (ret < 0) {
> > > @@ -249,7 +258,6 @@ static int adc108s102_probe(struct spi_device *spi)
> > >                 st->va_millivolt = ret / 1000;
> > >         }
> > >
> > > -       spi_set_drvdata(spi, indio_dev);
> > >         st->spi = spi;
> > >
> > >         indio_dev->name = spi->modalias;
> > > @@ -266,40 +274,18 @@ static int adc108s102_probe(struct spi_device *spi)
> > >         spi_message_init_with_transfers(&st->scan_single_msg,
> > >                                         &st->scan_single_xfer, 1);
> > >
> > > -       ret = iio_triggered_buffer_setup(indio_dev, NULL,
> > > -                                        &adc108s102_trigger_handler, NULL);
> > > +       ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
> > > +                                             &adc108s102_trigger_handler,
> > > +                                             NULL);
> > >         if (ret)
> > > -               goto error_disable_reg;
> > > +               return ret;
> > >
> > > -       ret = iio_device_register(indio_dev);
> > > -       if (ret) {
> > > +       ret = devm_iio_device_register(&spi->dev, indio_dev);
> > > +       if (ret)
> > >                 dev_err(&spi->dev, "Failed to register IIO device\n");
> > > -               goto error_cleanup_triggered_buffer;
> > > -       }
> > > -       return 0;
> > > -
> > > -error_cleanup_triggered_buffer:
> > > -       iio_triggered_buffer_cleanup(indio_dev);
> > > -
> > > -error_disable_reg:
> > > -       regulator_disable(st->reg);
> > > -
> > >         return ret;
> > >  }
> > >
> > > -static int adc108s102_remove(struct spi_device *spi)
> > > -{
> > > -       struct iio_dev *indio_dev = spi_get_drvdata(spi);
> > > -       struct adc108s102_state *st = iio_priv(indio_dev);
> > > -
> > > -       iio_device_unregister(indio_dev);
> > > -       iio_triggered_buffer_cleanup(indio_dev);
> > > -
> > > -       regulator_disable(st->reg);
> > > -
> > > -       return 0;
> > > -}
> > > -
> > >  static const struct of_device_id adc108s102_of_match[] = {
> > >         { .compatible = "ti,adc108s102" },
> > >         { }
> > > @@ -327,7 +313,6 @@ static struct spi_driver adc108s102_driver = {
> > >                 .acpi_match_table = ACPI_PTR(adc108s102_acpi_ids),
> > >         },
> > >         .probe          = adc108s102_probe,
> > > -       .remove         = adc108s102_remove,
> > >         .id_table       = adc108s102_id,
> > >  };
> > >  module_spi_driver(adc108s102_driver);
> > > --
> > > 2.31.1
> > >  


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/8] iio: adc: Maxim and TI ADC driver cleanups
  2021-05-16 17:25 [PATCH 0/8] iio: adc: Maxim and TI ADC driver cleanups Jonathan Cameron
                   ` (7 preceding siblings ...)
  2021-05-16 17:25 ` [PATCH 8/8] iio: adc: ti-adc161s626: Use devm managed functions for all of probe Jonathan Cameron
@ 2021-05-22 18:31 ` Jonathan Cameron
  8 siblings, 0 replies; 22+ messages in thread
From: Jonathan Cameron @ 2021-05-22 18:31 UTC (permalink / raw)
  To: linux-iio; +Cc: Alexandru Ardelean, Jonathan Cameron

On Sun, 16 May 2021 18:25:12 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> A few random bits of tidying up for a Sunday evening.
> Mostly devm stuff, but other things in here as well that came up whilst
> I was looking.
> 
> Note this was a fairly random selection, so isn't implying there
> aren't similar cleanups to be done in other TI and Maxim drivers.
> (there definitely are plenty more of these if anyone is bored ;)
> 
> Cc: Alexandru Ardelean <aardelean@deviqon.com>

Series applied with the minor tweak to drop spi_set_drvdata in two 
drivers as pointed out by Alex.

Thanks,

Jonathan

> 
> Jonathan Cameron (8):
>   iio: adc: max11100: Use get_unaligned_be16() rather than opencoding.
>   iio: adc: max11100: Use devm_ functions for rest of probe()
>   iio: adc: max1118: Use devm_ managed functions for all of probe
>   iio: adc: max1118: Avoid jumping back and forth between spi and iio
>     structures
>   iio: adc: ti-adc081c: Use devm managed functions for all of probe()
>   iio: adc: ti-adc0832: Use devm managed functions for all of probe()
>   iio: adc: ti-adc108s102: Use devm managed functions for all of probe()
>   iio: adc: ti-adc161s626: Use devm managed functions for all of probe.
> 
>  drivers/iio/adc/max11100.c      | 32 ++++++----------
>  drivers/iio/adc/max1118.c       | 68 ++++++++++++---------------------
>  drivers/iio/adc/ti-adc081c.c    | 43 +++++++--------------
>  drivers/iio/adc/ti-adc0832.c    | 39 ++++++-------------
>  drivers/iio/adc/ti-adc108s102.c | 45 ++++++++--------------
>  drivers/iio/adc/ti-adc161s626.c | 50 +++++++++---------------
>  6 files changed, 94 insertions(+), 183 deletions(-)
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2021-05-22 18:30 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-16 17:25 [PATCH 0/8] iio: adc: Maxim and TI ADC driver cleanups Jonathan Cameron
2021-05-16 17:25 ` [PATCH 1/8] iio: adc: max11100: Use get_unaligned_be16() rather than opencoding Jonathan Cameron
2021-05-17  6:59   ` Alexandru Ardelean
2021-05-16 17:25 ` [PATCH 2/8] iio: adc: max11100: Use devm_ functions for rest of probe() Jonathan Cameron
2021-05-17  7:08   ` Alexandru Ardelean
2021-05-17  7:22     ` Alexandru Ardelean
2021-05-16 17:25 ` [PATCH 3/8] iio: adc: max1118: Use devm_ managed functions for all of probe Jonathan Cameron
2021-05-17  7:18   ` Alexandru Ardelean
2021-05-16 17:25 ` [PATCH 4/8] iio: adc: max1118: Avoid jumping back and forth between spi and iio structures Jonathan Cameron
2021-05-17  7:24   ` Alexandru Ardelean
2021-05-16 17:25 ` [PATCH 5/8] iio: adc: ti-adc081c: Use devm managed functions for all of probe() Jonathan Cameron
2021-05-17  7:26   ` Alexandru Ardelean
2021-05-16 17:25 ` [PATCH 6/8] iio: adc: ti-adc0832: " Jonathan Cameron
2021-05-17  7:28   ` Alexandru Ardelean
2021-05-16 17:25 ` [PATCH 7/8] iio: adc: ti-adc108s102: " Jonathan Cameron
2021-05-17  7:31   ` Alexandru Ardelean
2021-05-17  7:37     ` Alexandru Ardelean
2021-05-22 18:29       ` Jonathan Cameron
2021-05-16 17:25 ` [PATCH 8/8] iio: adc: ti-adc161s626: Use devm managed functions for all of probe Jonathan Cameron
2021-05-16 20:04   ` Matt Ranostay
2021-05-17  7:35   ` Alexandru Ardelean
2021-05-22 18:31 ` [PATCH 0/8] iio: adc: Maxim and TI ADC driver cleanups Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.