All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/12] ad_sigma_delta: convert all drivers to device-managed
@ 2021-05-11  7:18 Alexandru Ardelean
  2021-05-11  7:18 ` [PATCH v2 01/12] iio: adc: ad7124: Fix missbalanced regulator enable / disable on error Alexandru Ardelean
                   ` (11 more replies)
  0 siblings, 12 replies; 15+ messages in thread
From: Alexandru Ardelean @ 2021-05-11  7:18 UTC (permalink / raw)
  To: linux-iio, linux-kernel
  Cc: jic23, Jonathan.Cameron, alexandru.tachici, linux, Alexandru Ardelean

Well, for lack of a better title that's what this series does.
It merges Jonathan's patches from:
  * https://lore.kernel.org/linux-iio/20210508182319.488551-1-jic23@kernel.org/
    Patch 3/3 was a polished a bit with my comments from that review and also
    to use the devm_ad_sd_setup_buffer_and_trigger() function.
  * https://lore.kernel.org/linux-iio/20210509114118.660422-1-jic23@kernel.org/
    Added only to base the conversion to devm_

The AD Sigma Delta family of ADC drivers share a lot of the logic in the
ad_sigma_delta lib-driver.

This set introduces a devm_ad_sd_setup_buffer_and_trigger() call, which
aims to replace the 'ad_sd_{setup,cleanup}_buffer_and_trigger()' pair.

This helps with converting the AD7780, AD7791, AD7793 and AD7192
drivers use be fully converted to device-managed functions.

Changelog v1 -> v2:
* https://lore.kernel.org/linux-iio/20210510125523.1271237-1-aardelean@deviqon.com/
* add my S-o-b tags on all patches; with @deviqon.com email
  - Note: I'm a little unsure about the correctness of these tags; there
    are a few mixed-in, with Reviewed-by & Signed-off-by; I'm fine if
    Jonathan tweaks these as needed;
* added patch 'iio: adc: ad7192: handle zero Avdd regulator value as error'
* all Fixes patches should be now at the beginning of the series

Alexandru Ardelean (8):
  iio: adc: ad7192: handle zero Avdd regulator value as error
  iio: adc: ad_sigma_delta: introduct
    devm_ad_sd_setup_buffer_and_trigger()
  iio: adc: ad7793: convert to device-managed functions
  iio: adc: ad7791: convert to device-managed functions
  iio: adc: ad7780: convert to device-managed functions
  iio: adc: ad7192: use devm_clk_get_optional() for mclk
  iio: adc: ad7192: convert to device-managed functions
  iio: adc: ad_sigma_delta: remove
    ad_sd_{setup,cleanup}_buffer_and_trigger()

Jonathan Cameron (4):
  iio: adc: ad7124: Fix missbalanced regulator enable / disable on
    error.
  iio: adc: ad7124: Fix potential overflow due to non sequential channel
    numbers
  iio: adc: ad7192: Avoid disabling a clock that was never enabled.
  iio: adc: ad7124: Use devm_ managed calls for all of probe() + drop
    remove()

 drivers/iio/adc/ad7124.c               | 84 ++++++++++-------------
 drivers/iio/adc/ad7192.c               | 94 ++++++++++++--------------
 drivers/iio/adc/ad7780.c               | 38 +++--------
 drivers/iio/adc/ad7791.c               | 44 ++++--------
 drivers/iio/adc/ad7793.c               | 53 ++++-----------
 drivers/iio/adc/ad_sigma_delta.c       | 82 ++++++++--------------
 include/linux/iio/adc/ad_sigma_delta.h |  4 +-
 7 files changed, 145 insertions(+), 254 deletions(-)

-- 
2.31.1


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

* [PATCH v2 01/12] iio: adc: ad7124: Fix missbalanced regulator enable / disable on error.
  2021-05-11  7:18 [PATCH v2 00/12] ad_sigma_delta: convert all drivers to device-managed Alexandru Ardelean
@ 2021-05-11  7:18 ` Alexandru Ardelean
  2021-05-11  7:18 ` [PATCH v2 02/12] iio: adc: ad7124: Fix potential overflow due to non sequential channel numbers Alexandru Ardelean
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Alexandru Ardelean @ 2021-05-11  7:18 UTC (permalink / raw)
  To: linux-iio, linux-kernel
  Cc: jic23, Jonathan.Cameron, alexandru.tachici, linux,
	Alexandru Ardelean, Alexandru Ardelean

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

If the devm_regulator_get() call succeeded but not the regulator_enable()
then regulator_disable() would be called on a regulator that was not
enabled.

Fix this by moving regulator enabling / disabling over to
devm_ management via devm_add_action_or_reset.

Fixes: b3af341bbd96 ("iio: adc: Add ad7124 support")
Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/iio/adc/ad7124.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
index 9d3952b4674f..437116a07cf1 100644
--- a/drivers/iio/adc/ad7124.c
+++ b/drivers/iio/adc/ad7124.c
@@ -850,6 +850,11 @@ static int ad7124_setup(struct ad7124_state *st)
 	return ret;
 }
 
+static void ad7124_reg_disable(void *r)
+{
+	regulator_disable(r);
+}
+
 static int ad7124_probe(struct spi_device *spi)
 {
 	const struct ad7124_chip_info *info;
@@ -895,17 +900,20 @@ static int ad7124_probe(struct spi_device *spi)
 		ret = regulator_enable(st->vref[i]);
 		if (ret)
 			return ret;
+
+		ret = devm_add_action_or_reset(&spi->dev, ad7124_reg_disable,
+					       st->vref[i]);
+		if (ret)
+			return ret;
 	}
 
 	st->mclk = devm_clk_get(&spi->dev, "mclk");
-	if (IS_ERR(st->mclk)) {
-		ret = PTR_ERR(st->mclk);
-		goto error_regulator_disable;
-	}
+	if (IS_ERR(st->mclk))
+		return PTR_ERR(st->mclk);
 
 	ret = clk_prepare_enable(st->mclk);
 	if (ret < 0)
-		goto error_regulator_disable;
+		return ret;
 
 	ret = ad7124_soft_reset(st);
 	if (ret < 0)
@@ -935,11 +943,6 @@ static int ad7124_probe(struct spi_device *spi)
 	ad_sd_cleanup_buffer_and_trigger(indio_dev);
 error_clk_disable_unprepare:
 	clk_disable_unprepare(st->mclk);
-error_regulator_disable:
-	for (i = ARRAY_SIZE(st->vref) - 1; i >= 0; i--) {
-		if (!IS_ERR_OR_NULL(st->vref[i]))
-			regulator_disable(st->vref[i]);
-	}
 
 	return ret;
 }
@@ -948,17 +951,11 @@ static int ad7124_remove(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 	struct ad7124_state *st = iio_priv(indio_dev);
-	int i;
 
 	iio_device_unregister(indio_dev);
 	ad_sd_cleanup_buffer_and_trigger(indio_dev);
 	clk_disable_unprepare(st->mclk);
 
-	for (i = ARRAY_SIZE(st->vref) - 1; i >= 0; i--) {
-		if (!IS_ERR_OR_NULL(st->vref[i]))
-			regulator_disable(st->vref[i]);
-	}
-
 	return 0;
 }
 
-- 
2.31.1


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

* [PATCH v2 02/12] iio: adc: ad7124: Fix potential overflow due to non sequential channel numbers
  2021-05-11  7:18 [PATCH v2 00/12] ad_sigma_delta: convert all drivers to device-managed Alexandru Ardelean
  2021-05-11  7:18 ` [PATCH v2 01/12] iio: adc: ad7124: Fix missbalanced regulator enable / disable on error Alexandru Ardelean
@ 2021-05-11  7:18 ` Alexandru Ardelean
  2021-05-11  7:18 ` [PATCH v2 03/12] iio: adc: ad7192: Avoid disabling a clock that was never enabled Alexandru Ardelean
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Alexandru Ardelean @ 2021-05-11  7:18 UTC (permalink / raw)
  To: linux-iio, linux-kernel
  Cc: jic23, Jonathan.Cameron, alexandru.tachici, linux,
	Alexandru Ardelean, Alexandru Ardelean

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

Channel numbering must start at 0 and then not have any holes, or
it is possible to overflow the available storage.  Note this bug was
introduced as part of a fix to ensure we didn't rely on the ordering
of child nodes.  So we need to support arbitrary ordering but they all
need to be there somewhere.

Note I hit this when using qemu to test the rest of this series.
Arguably this isn't the best fix, but it is probably the most minimal
option for backporting etc.

Fixes: d7857e4ee1ba6 ("iio: adc: ad7124: Fix DT channel configuration")
Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/iio/adc/ad7124.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
index 437116a07cf1..a27db78ea13e 100644
--- a/drivers/iio/adc/ad7124.c
+++ b/drivers/iio/adc/ad7124.c
@@ -771,6 +771,13 @@ static int ad7124_of_parse_channel_config(struct iio_dev *indio_dev,
 		if (ret)
 			goto err;
 
+		if (channel >= indio_dev->num_channels) {
+			dev_err(indio_dev->dev.parent,
+				"Channel index >= number of channels\n");
+			ret = -EINVAL;
+			goto err;
+		}
+
 		ret = of_property_read_u32_array(child, "diff-channels",
 						 ain, 2);
 		if (ret)
-- 
2.31.1


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

* [PATCH v2 03/12] iio: adc: ad7192: Avoid disabling a clock that was never enabled.
  2021-05-11  7:18 [PATCH v2 00/12] ad_sigma_delta: convert all drivers to device-managed Alexandru Ardelean
  2021-05-11  7:18 ` [PATCH v2 01/12] iio: adc: ad7124: Fix missbalanced regulator enable / disable on error Alexandru Ardelean
  2021-05-11  7:18 ` [PATCH v2 02/12] iio: adc: ad7124: Fix potential overflow due to non sequential channel numbers Alexandru Ardelean
@ 2021-05-11  7:18 ` Alexandru Ardelean
  2021-05-11  7:18 ` [PATCH v2 04/12] iio: adc: ad7192: handle zero Avdd regulator value as error Alexandru Ardelean
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Alexandru Ardelean @ 2021-05-11  7:18 UTC (permalink / raw)
  To: linux-iio, linux-kernel
  Cc: jic23, Jonathan.Cameron, alexandru.tachici, linux,
	Alexandru Ardelean, Alexandru Ardelean

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

Found by inspection.

If the internal clock source is being used, the driver doesn't
call clk_prepare_enable() and as such we should not call
clk_disable_unprepare()

Use the same condition to protect the disable path as is used
on the enable one.  Note this will all get simplified when
the driver moves over to a full devm_ flow, but that would make
backporting the fix harder.

Fix obviously predates move out of staging, but backporting will
become more complex (and is unlikely to happen), hence that patch
is given in the fixes tag.

Fixes: b581f748cce0 ("staging: iio: adc: ad7192: move out of staging")
Cc: Alexandru Tachici <alexandru.tachici@analog.com>
Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/iio/adc/ad7192.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index 2ed580521d81..d3be67aa0522 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -1014,7 +1014,9 @@ static int ad7192_probe(struct spi_device *spi)
 	return 0;
 
 error_disable_clk:
-	clk_disable_unprepare(st->mclk);
+	if (st->clock_sel == AD7192_CLK_EXT_MCLK1_2 ||
+	    st->clock_sel == AD7192_CLK_EXT_MCLK2)
+		clk_disable_unprepare(st->mclk);
 error_remove_trigger:
 	ad_sd_cleanup_buffer_and_trigger(indio_dev);
 error_disable_dvdd:
@@ -1031,7 +1033,9 @@ static int ad7192_remove(struct spi_device *spi)
 	struct ad7192_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
-	clk_disable_unprepare(st->mclk);
+	if (st->clock_sel == AD7192_CLK_EXT_MCLK1_2 ||
+	    st->clock_sel == AD7192_CLK_EXT_MCLK2)
+		clk_disable_unprepare(st->mclk);
 	ad_sd_cleanup_buffer_and_trigger(indio_dev);
 
 	regulator_disable(st->dvdd);
-- 
2.31.1


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

* [PATCH v2 04/12] iio: adc: ad7192: handle zero Avdd regulator value as error
  2021-05-11  7:18 [PATCH v2 00/12] ad_sigma_delta: convert all drivers to device-managed Alexandru Ardelean
                   ` (2 preceding siblings ...)
  2021-05-11  7:18 ` [PATCH v2 03/12] iio: adc: ad7192: Avoid disabling a clock that was never enabled Alexandru Ardelean
@ 2021-05-11  7:18 ` Alexandru Ardelean
  2021-05-11 14:13   ` Jonathan Cameron
  2021-05-11  7:18 ` [PATCH v2 05/12] iio: adc: ad_sigma_delta: introduct devm_ad_sd_setup_buffer_and_trigger() Alexandru Ardelean
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 15+ messages in thread
From: Alexandru Ardelean @ 2021-05-11  7:18 UTC (permalink / raw)
  To: linux-iio, linux-kernel
  Cc: jic23, Jonathan.Cameron, alexandru.tachici, linux, Alexandru Ardelean

This change fixes a corner-case, where the returned voltage is actually
zero. This is also what patch ab0afa65bbc7 ("staging: iio: adc: ad7192:
fail probe on get_voltage") was trying to do.

But as Jonathan pointed out, a zero-value would signal that the probe has
succeeded, putting the driver is a semi-initialized state.

Fixes: ab0afa65bbc7 ("staging: iio: adc: ad7192: fail probe on get_voltage")
Cc: Alexandru Tachici <alexandru.tachici@analog.com>
Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/iio/adc/ad7192.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index d3be67aa0522..79df54e0dc96 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -950,6 +950,11 @@ static int ad7192_probe(struct spi_device *spi)
 	}
 
 	voltage_uv = regulator_get_voltage(st->avdd);
+	if (voltage_uv == 0) {
+		ret = -EINVAL;
+		dev_err(&spi->dev, "Zero value provided for AVdd supply\n");
+		goto error_disable_avdd;
+	}
 
 	if (voltage_uv > 0) {
 		st->int_vref_mv = voltage_uv / 1000;
-- 
2.31.1


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

* [PATCH v2 05/12] iio: adc: ad_sigma_delta: introduct devm_ad_sd_setup_buffer_and_trigger()
  2021-05-11  7:18 [PATCH v2 00/12] ad_sigma_delta: convert all drivers to device-managed Alexandru Ardelean
                   ` (3 preceding siblings ...)
  2021-05-11  7:18 ` [PATCH v2 04/12] iio: adc: ad7192: handle zero Avdd regulator value as error Alexandru Ardelean
@ 2021-05-11  7:18 ` Alexandru Ardelean
  2021-05-11  7:18 ` [PATCH v2 06/12] iio: adc: ad7793: convert to device-managed functions Alexandru Ardelean
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Alexandru Ardelean @ 2021-05-11  7:18 UTC (permalink / raw)
  To: linux-iio, linux-kernel
  Cc: jic23, Jonathan.Cameron, alexandru.tachici, linux, Alexandru Ardelean

This is a version of ad_sd_setup_buffer_and_trigger() with all underlying
functions (that are used) being replaced with their device-managed
variants.

One thing to take care here is with {devm_}iio_trigger_alloc(), where both
functions take a parent-device object as the first parameter.

To make sure nothing quirky is happening, the devm_ad_sd_probe_trigger()
function is checking that the provided 'dev' reference is the same as the
one stored on the 'struct ad_sigma_delta' driver data.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/iio/adc/ad_sigma_delta.c       | 60 ++++++++++++++++++++++++++
 include/linux/iio/adc/ad_sigma_delta.h |  3 ++
 2 files changed, 63 insertions(+)

diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index 69b979331ccd..d5801a47be07 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -513,6 +513,46 @@ static int ad_sd_probe_trigger(struct iio_dev *indio_dev)
 	return ret;
 }
 
+static int devm_ad_sd_probe_trigger(struct device *dev, struct iio_dev *indio_dev)
+{
+	struct ad_sigma_delta *sigma_delta = iio_device_get_drvdata(indio_dev);
+	int ret;
+
+	if (dev != &sigma_delta->spi->dev) {
+		dev_err(dev, "Trigger parent should be '%s', got '%s'\n",
+			dev_name(dev), dev_name(&sigma_delta->spi->dev));
+		return -EFAULT;
+	}
+
+	sigma_delta->trig = devm_iio_trigger_alloc(dev, "%s-dev%d", indio_dev->name,
+						   iio_device_id(indio_dev));
+	if (sigma_delta->trig == NULL)
+		return -ENOMEM;
+
+	sigma_delta->trig->ops = &ad_sd_trigger_ops;
+	init_completion(&sigma_delta->completion);
+
+	sigma_delta->irq_dis = true;
+	ret = devm_request_irq(dev, sigma_delta->spi->irq,
+			       ad_sd_data_rdy_trig_poll,
+			       sigma_delta->info->irq_flags | IRQF_NO_AUTOEN,
+			       indio_dev->name,
+			       sigma_delta);
+	if (ret)
+		return ret;
+
+	iio_trigger_set_drvdata(sigma_delta->trig, sigma_delta);
+
+	ret = devm_iio_trigger_register(dev, sigma_delta->trig);
+	if (ret)
+		return ret;
+
+	/* select default trigger */
+	indio_dev->trig = iio_trigger_get(sigma_delta->trig);
+
+	return 0;
+}
+
 static void ad_sd_remove_trigger(struct iio_dev *indio_dev)
 {
 	struct ad_sigma_delta *sigma_delta = iio_device_get_drvdata(indio_dev);
@@ -556,6 +596,26 @@ void ad_sd_cleanup_buffer_and_trigger(struct iio_dev *indio_dev)
 }
 EXPORT_SYMBOL_GPL(ad_sd_cleanup_buffer_and_trigger);
 
+/**
+ * devm_ad_sd_setup_buffer_and_trigger() - Device-managed buffer & trigger setup
+ * @dev: Device object to which to bind the life-time of the resources attached
+ * @indio_dev: The IIO device
+ */
+int devm_ad_sd_setup_buffer_and_trigger(struct device *dev, struct iio_dev *indio_dev)
+{
+	int ret;
+
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
+					      &iio_pollfunc_store_time,
+					      &ad_sd_trigger_handler,
+					      &ad_sd_buffer_setup_ops);
+	if (ret)
+		return ret;
+
+	return devm_ad_sd_probe_trigger(dev, indio_dev);
+}
+EXPORT_SYMBOL_GPL(devm_ad_sd_setup_buffer_and_trigger);
+
 /**
  * ad_sd_init() - Initializes a ad_sigma_delta struct
  * @sigma_delta: The ad_sigma_delta device
diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h
index 7199280d89ca..be81ad39fb7a 100644
--- a/include/linux/iio/adc/ad_sigma_delta.h
+++ b/include/linux/iio/adc/ad_sigma_delta.h
@@ -26,6 +26,7 @@ struct ad_sd_calib_data {
 };
 
 struct ad_sigma_delta;
+struct device;
 struct iio_dev;
 
 /**
@@ -135,6 +136,8 @@ int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev *indio_dev,
 int ad_sd_setup_buffer_and_trigger(struct iio_dev *indio_dev);
 void ad_sd_cleanup_buffer_and_trigger(struct iio_dev *indio_dev);
 
+int devm_ad_sd_setup_buffer_and_trigger(struct device *dev, struct iio_dev *indio_dev);
+
 int ad_sd_validate_trigger(struct iio_dev *indio_dev, struct iio_trigger *trig);
 
 #endif
-- 
2.31.1


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

* [PATCH v2 06/12] iio: adc: ad7793: convert to device-managed functions
  2021-05-11  7:18 [PATCH v2 00/12] ad_sigma_delta: convert all drivers to device-managed Alexandru Ardelean
                   ` (4 preceding siblings ...)
  2021-05-11  7:18 ` [PATCH v2 05/12] iio: adc: ad_sigma_delta: introduct devm_ad_sd_setup_buffer_and_trigger() Alexandru Ardelean
@ 2021-05-11  7:18 ` Alexandru Ardelean
  2021-05-11  7:18 ` [PATCH v2 07/12] iio: adc: ad7791: " Alexandru Ardelean
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Alexandru Ardelean @ 2021-05-11  7:18 UTC (permalink / raw)
  To: linux-iio, linux-kernel
  Cc: jic23, Jonathan.Cameron, alexandru.tachici, linux, Alexandru Ardelean

With the devm_ad_sd_setup_buffer_and_trigger() helper, it's a bit easier
now to convert the probe of the AD7793 driver to use device-managed
functions.

Only the regulator disable requires a devm_add_action_or_reset() callback.

This change does that, cleaning up the driver a bit.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/iio/adc/ad7793.c | 53 ++++++++++++----------------------------
 1 file changed, 15 insertions(+), 38 deletions(-)

diff --git a/drivers/iio/adc/ad7793.c b/drivers/iio/adc/ad7793.c
index 5e980a06258e..5dab2e5b5bac 100644
--- a/drivers/iio/adc/ad7793.c
+++ b/drivers/iio/adc/ad7793.c
@@ -768,6 +768,11 @@ static const struct ad7793_chip_info ad7793_chip_info_tbl[] = {
 	},
 };
 
+static void ad7793_reg_disable(void *reg)
+{
+	regulator_disable(reg);
+}
+
 static int ad7793_probe(struct spi_device *spi)
 {
 	const struct ad7793_platform_data *pdata = spi->dev.platform_data;
@@ -802,11 +807,13 @@ static int ad7793_probe(struct spi_device *spi)
 		if (ret)
 			return ret;
 
+		ret = devm_add_action_or_reset(&spi->dev, ad7793_reg_disable, st->reg);
+		if (ret)
+			return ret;
+
 		vref_mv = regulator_get_voltage(st->reg);
-		if (vref_mv < 0) {
-			ret = vref_mv;
-			goto error_disable_reg;
-		}
+		if (vref_mv < 0)
+			return vref_mv;
 
 		vref_mv /= 1000;
 	} else {
@@ -816,50 +823,21 @@ static int ad7793_probe(struct spi_device *spi)
 	st->chip_info =
 		&ad7793_chip_info_tbl[spi_get_device_id(spi)->driver_data];
 
-	spi_set_drvdata(spi, indio_dev);
-
 	indio_dev->name = spi_get_device_id(spi)->name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = st->chip_info->channels;
 	indio_dev->num_channels = st->chip_info->num_channels;
 	indio_dev->info = st->chip_info->iio_info;
 
-	ret = ad_sd_setup_buffer_and_trigger(indio_dev);
+	ret = devm_ad_sd_setup_buffer_and_trigger(&spi->dev, indio_dev);
 	if (ret)
-		goto error_disable_reg;
+		return ret;
 
 	ret = ad7793_setup(indio_dev, pdata, vref_mv);
 	if (ret)
-		goto error_remove_trigger;
-
-	ret = iio_device_register(indio_dev);
-	if (ret)
-		goto error_remove_trigger;
-
-	return 0;
-
-error_remove_trigger:
-	ad_sd_cleanup_buffer_and_trigger(indio_dev);
-error_disable_reg:
-	if (pdata->refsel != AD7793_REFSEL_INTERNAL)
-		regulator_disable(st->reg);
-
-	return ret;
-}
-
-static int ad7793_remove(struct spi_device *spi)
-{
-	const struct ad7793_platform_data *pdata = spi->dev.platform_data;
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct ad7793_state *st = iio_priv(indio_dev);
-
-	iio_device_unregister(indio_dev);
-	ad_sd_cleanup_buffer_and_trigger(indio_dev);
-
-	if (pdata->refsel != AD7793_REFSEL_INTERNAL)
-		regulator_disable(st->reg);
+		return ret;
 
-	return 0;
+	return devm_iio_device_register(&spi->dev, indio_dev);
 }
 
 static const struct spi_device_id ad7793_id[] = {
@@ -881,7 +859,6 @@ static struct spi_driver ad7793_driver = {
 		.name	= "ad7793",
 	},
 	.probe		= ad7793_probe,
-	.remove		= ad7793_remove,
 	.id_table	= ad7793_id,
 };
 module_spi_driver(ad7793_driver);
-- 
2.31.1


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

* [PATCH v2 07/12] iio: adc: ad7791: convert to device-managed functions
  2021-05-11  7:18 [PATCH v2 00/12] ad_sigma_delta: convert all drivers to device-managed Alexandru Ardelean
                   ` (5 preceding siblings ...)
  2021-05-11  7:18 ` [PATCH v2 06/12] iio: adc: ad7793: convert to device-managed functions Alexandru Ardelean
@ 2021-05-11  7:18 ` Alexandru Ardelean
  2021-05-11  7:18 ` [PATCH v2 08/12] iio: adc: ad7780: " Alexandru Ardelean
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Alexandru Ardelean @ 2021-05-11  7:18 UTC (permalink / raw)
  To: linux-iio, linux-kernel
  Cc: jic23, Jonathan.Cameron, alexandru.tachici, linux, Alexandru Ardelean

With the devm_ad_sd_setup_buffer_and_trigger() helper, it's a bit easier
now to convert the probe of the AD7791 driver to use device-managed
functions.

Only the regulator disable requires a devm_add_action_or_reset() callback.

This change does that, cleaning up the driver a bit.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/iio/adc/ad7791.c | 44 ++++++++++++----------------------------
 1 file changed, 13 insertions(+), 31 deletions(-)

diff --git a/drivers/iio/adc/ad7791.c b/drivers/iio/adc/ad7791.c
index d57ad966e17c..cb579aa89f39 100644
--- a/drivers/iio/adc/ad7791.c
+++ b/drivers/iio/adc/ad7791.c
@@ -394,6 +394,11 @@ static int ad7791_setup(struct ad7791_state *st,
 		st->mode);
 }
 
+static void ad7791_reg_disable(void *reg)
+{
+	regulator_disable(reg);
+}
+
 static int ad7791_probe(struct spi_device *spi)
 {
 	struct ad7791_platform_data *pdata = spi->dev.platform_data;
@@ -420,11 +425,13 @@ static int ad7791_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
+	ret = devm_add_action_or_reset(&spi->dev, ad7791_reg_disable, st->reg);
+	if (ret)
+		return ret;
+
 	st->info = &ad7791_chip_infos[spi_get_device_id(spi)->driver_data];
 	ad_sd_init(&st->sd, indio_dev, spi, &ad7791_sigma_delta_info);
 
-	spi_set_drvdata(spi, indio_dev);
-
 	indio_dev->name = spi_get_device_id(spi)->name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = st->info->channels;
@@ -434,39 +441,15 @@ static int ad7791_probe(struct spi_device *spi)
 	else
 		indio_dev->info = &ad7791_no_filter_info;
 
-	ret = ad_sd_setup_buffer_and_trigger(indio_dev);
+	ret = devm_ad_sd_setup_buffer_and_trigger(&spi->dev, indio_dev);
 	if (ret)
-		goto error_disable_reg;
+		return ret;
 
 	ret = ad7791_setup(st, pdata);
 	if (ret)
-		goto error_remove_trigger;
-
-	ret = iio_device_register(indio_dev);
-	if (ret)
-		goto error_remove_trigger;
-
-	return 0;
-
-error_remove_trigger:
-	ad_sd_cleanup_buffer_and_trigger(indio_dev);
-error_disable_reg:
-	regulator_disable(st->reg);
-
-	return ret;
-}
-
-static int ad7791_remove(struct spi_device *spi)
-{
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct ad7791_state *st = iio_priv(indio_dev);
-
-	iio_device_unregister(indio_dev);
-	ad_sd_cleanup_buffer_and_trigger(indio_dev);
-
-	regulator_disable(st->reg);
+		return ret;
 
-	return 0;
+	return devm_iio_device_register(&spi->dev, indio_dev);
 }
 
 static const struct spi_device_id ad7791_spi_ids[] = {
@@ -484,7 +467,6 @@ static struct spi_driver ad7791_driver = {
 		.name	= "ad7791",
 	},
 	.probe		= ad7791_probe,
-	.remove		= ad7791_remove,
 	.id_table	= ad7791_spi_ids,
 };
 module_spi_driver(ad7791_driver);
-- 
2.31.1


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

* [PATCH v2 08/12] iio: adc: ad7780: convert to device-managed functions
  2021-05-11  7:18 [PATCH v2 00/12] ad_sigma_delta: convert all drivers to device-managed Alexandru Ardelean
                   ` (6 preceding siblings ...)
  2021-05-11  7:18 ` [PATCH v2 07/12] iio: adc: ad7791: " Alexandru Ardelean
@ 2021-05-11  7:18 ` Alexandru Ardelean
  2021-05-11  7:18 ` [PATCH v2 09/12] iio: adc: ad7192: use devm_clk_get_optional() for mclk Alexandru Ardelean
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Alexandru Ardelean @ 2021-05-11  7:18 UTC (permalink / raw)
  To: linux-iio, linux-kernel
  Cc: jic23, Jonathan.Cameron, alexandru.tachici, linux, Alexandru Ardelean

With the devm_ad_sd_setup_buffer_and_trigger() helper, it's a bit easier
now to convert the probe of the AD7780 driver to use device-managed
functions.

Only the regulator disable requires a devm_add_action_or_reset() callback.

This change does that, cleaning up the driver a bit.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/iio/adc/ad7780.c | 38 ++++++++++----------------------------
 1 file changed, 10 insertions(+), 28 deletions(-)

diff --git a/drivers/iio/adc/ad7780.c b/drivers/iio/adc/ad7780.c
index 42e7e8e595d1..42bb952f4738 100644
--- a/drivers/iio/adc/ad7780.c
+++ b/drivers/iio/adc/ad7780.c
@@ -300,6 +300,11 @@ static int ad7780_init_gpios(struct device *dev, struct ad7780_state *st)
 	return 0;
 }
 
+static void ad7780_reg_disable(void *reg)
+{
+	regulator_disable(reg);
+}
+
 static int ad7780_probe(struct spi_device *spi)
 {
 	struct ad7780_state *st;
@@ -318,8 +323,6 @@ static int ad7780_probe(struct spi_device *spi)
 	st->chip_info =
 		&ad7780_chip_info_tbl[spi_get_device_id(spi)->driver_data];
 
-	spi_set_drvdata(spi, indio_dev);
-
 	indio_dev->name = spi_get_device_id(spi)->name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = &st->chip_info->channel;
@@ -340,35 +343,15 @@ static int ad7780_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	ret = ad_sd_setup_buffer_and_trigger(indio_dev);
+	ret = devm_add_action_or_reset(&spi->dev, ad7780_reg_disable, st->reg);
 	if (ret)
-		goto error_disable_reg;
+		return ret;
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_ad_sd_setup_buffer_and_trigger(&spi->dev, indio_dev);
 	if (ret)
-		goto error_cleanup_buffer_and_trigger;
-
-	return 0;
-
-error_cleanup_buffer_and_trigger:
-	ad_sd_cleanup_buffer_and_trigger(indio_dev);
-error_disable_reg:
-	regulator_disable(st->reg);
-
-	return ret;
-}
-
-static int ad7780_remove(struct spi_device *spi)
-{
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct ad7780_state *st = iio_priv(indio_dev);
-
-	iio_device_unregister(indio_dev);
-	ad_sd_cleanup_buffer_and_trigger(indio_dev);
-
-	regulator_disable(st->reg);
+		return ret;
 
-	return 0;
+	return devm_iio_device_register(&spi->dev, indio_dev);
 }
 
 static const struct spi_device_id ad7780_id[] = {
@@ -385,7 +368,6 @@ static struct spi_driver ad7780_driver = {
 		.name	= "ad7780",
 	},
 	.probe		= ad7780_probe,
-	.remove		= ad7780_remove,
 	.id_table	= ad7780_id,
 };
 module_spi_driver(ad7780_driver);
-- 
2.31.1


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

* [PATCH v2 09/12] iio: adc: ad7192: use devm_clk_get_optional() for mclk
  2021-05-11  7:18 [PATCH v2 00/12] ad_sigma_delta: convert all drivers to device-managed Alexandru Ardelean
                   ` (7 preceding siblings ...)
  2021-05-11  7:18 ` [PATCH v2 08/12] iio: adc: ad7780: " Alexandru Ardelean
@ 2021-05-11  7:18 ` Alexandru Ardelean
  2021-05-11  7:18 ` [PATCH v2 10/12] iio: adc: ad7192: convert to device-managed functions Alexandru Ardelean
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Alexandru Ardelean @ 2021-05-11  7:18 UTC (permalink / raw)
  To: linux-iio, linux-kernel
  Cc: jic23, Jonathan.Cameron, alexandru.tachici, linux, Alexandru Ardelean

The devm_clk_get_optional() helper returns NULL when devm_clk_get() returns
-ENOENT.
This makes things slightly cleaner. The added benefit is mostly cosmetic.

Also, a minor detail with this call, is that the reference for the parent
device is taken as `spi->dev` instead of `&st->sd.spi->dev` (which looks a
little quirky).

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/iio/adc/ad7192.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index 79df54e0dc96..18e731f1471a 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -326,7 +326,7 @@ static int ad7192_of_clock_select(struct ad7192_state *st)
 	clock_sel = AD7192_CLK_INT;
 
 	/* use internal clock */
-	if (PTR_ERR(st->mclk) == -ENOENT) {
+	if (st->mclk) {
 		if (of_property_read_bool(np, "adi,int-clock-output-enable"))
 			clock_sel = AD7192_CLK_INT_CO;
 	} else {
@@ -986,8 +986,8 @@ static int ad7192_probe(struct spi_device *spi)
 
 	st->fclk = AD7192_INT_FREQ_MHZ;
 
-	st->mclk = devm_clk_get(&st->sd.spi->dev, "mclk");
-	if (IS_ERR(st->mclk) && PTR_ERR(st->mclk) != -ENOENT) {
+	st->mclk = devm_clk_get_optional(&spi->dev, "mclk");
+	if (IS_ERR(st->mclk)) {
 		ret = PTR_ERR(st->mclk);
 		goto error_remove_trigger;
 	}
-- 
2.31.1


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

* [PATCH v2 10/12] iio: adc: ad7192: convert to device-managed functions
  2021-05-11  7:18 [PATCH v2 00/12] ad_sigma_delta: convert all drivers to device-managed Alexandru Ardelean
                   ` (8 preceding siblings ...)
  2021-05-11  7:18 ` [PATCH v2 09/12] iio: adc: ad7192: use devm_clk_get_optional() for mclk Alexandru Ardelean
@ 2021-05-11  7:18 ` Alexandru Ardelean
  2021-05-11  7:18 ` [PATCH v2 11/12] iio: adc: ad7124: Use devm_ managed calls for all of probe() + drop remove() Alexandru Ardelean
  2021-05-11  7:18 ` [PATCH v2 12/12] iio: adc: ad_sigma_delta: remove ad_sd_{setup,cleanup}_buffer_and_trigger() Alexandru Ardelean
  11 siblings, 0 replies; 15+ messages in thread
From: Alexandru Ardelean @ 2021-05-11  7:18 UTC (permalink / raw)
  To: linux-iio, linux-kernel
  Cc: jic23, Jonathan.Cameron, alexandru.tachici, linux, Alexandru Ardelean

With the devm_ad_sd_setup_buffer_and_trigger() helper, it's a bit easier
now to convert the probe of the AD7192 driver to use device-managed
functions.

The regulators and the mclk requires devm_add_action_or_reset() callbacks
though.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/iio/adc/ad7192.c | 93 ++++++++++++++++------------------------
 1 file changed, 37 insertions(+), 56 deletions(-)

diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index 18e731f1471a..50696959c018 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -908,6 +908,16 @@ static int ad7192_channels_config(struct iio_dev *indio_dev)
 	return 0;
 }
 
+static void ad7192_reg_disable(void *reg)
+{
+	regulator_disable(reg);
+}
+
+static void ad7192_clk_disable(void *clk)
+{
+	clk_disable_unprepare(clk);
+}
+
 static int ad7192_probe(struct spi_device *spi)
 {
 	struct ad7192_state *st;
@@ -937,41 +947,44 @@ static int ad7192_probe(struct spi_device *spi)
 		return ret;
 	}
 
+	ret = devm_add_action_or_reset(&spi->dev, ad7192_reg_disable, st->avdd);
+	if (ret)
+		return ret;
+
 	st->dvdd = devm_regulator_get(&spi->dev, "dvdd");
-	if (IS_ERR(st->dvdd)) {
-		ret = PTR_ERR(st->dvdd);
-		goto error_disable_avdd;
-	}
+	if (IS_ERR(st->dvdd))
+		return PTR_ERR(st->dvdd);
 
 	ret = regulator_enable(st->dvdd);
 	if (ret) {
 		dev_err(&spi->dev, "Failed to enable specified DVdd supply\n");
-		goto error_disable_avdd;
+		return ret;
 	}
 
+	ret = devm_add_action_or_reset(&spi->dev, ad7192_reg_disable, st->dvdd);
+	if (ret)
+		return ret;
+
 	voltage_uv = regulator_get_voltage(st->avdd);
 	if (voltage_uv == 0) {
-		ret = -EINVAL;
 		dev_err(&spi->dev, "Zero value provided for AVdd supply\n");
-		goto error_disable_avdd;
+		return -EINVAL;
 	}
 
 	if (voltage_uv > 0) {
 		st->int_vref_mv = voltage_uv / 1000;
 	} else {
-		ret = voltage_uv;
 		dev_err(&spi->dev, "Device tree error, reference voltage undefined\n");
-		goto error_disable_avdd;
+		return voltage_uv;
 	}
 
-	spi_set_drvdata(spi, indio_dev);
 	st->chip_info = of_device_get_match_data(&spi->dev);
 	indio_dev->name = st->chip_info->name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
 	ret = ad7192_channels_config(indio_dev);
 	if (ret < 0)
-		goto error_disable_dvdd;
+		return ret;
 
 	if (st->chip_info->chip_id == CHIPID_AD7195)
 		indio_dev->info = &ad7195_info;
@@ -980,17 +993,15 @@ static int ad7192_probe(struct spi_device *spi)
 
 	ad_sd_init(&st->sd, indio_dev, spi, &ad7192_sigma_delta_info);
 
-	ret = ad_sd_setup_buffer_and_trigger(indio_dev);
+	ret = devm_ad_sd_setup_buffer_and_trigger(&spi->dev, indio_dev);
 	if (ret)
-		goto error_disable_dvdd;
+		return ret;
 
 	st->fclk = AD7192_INT_FREQ_MHZ;
 
 	st->mclk = devm_clk_get_optional(&spi->dev, "mclk");
-	if (IS_ERR(st->mclk)) {
-		ret = PTR_ERR(st->mclk);
-		goto error_remove_trigger;
-	}
+	if (IS_ERR(st->mclk))
+		return PTR_ERR(st->mclk);
 
 	st->clock_sel = ad7192_of_clock_select(st);
 
@@ -998,55 +1009,26 @@ static int ad7192_probe(struct spi_device *spi)
 	    st->clock_sel == AD7192_CLK_EXT_MCLK2) {
 		ret = clk_prepare_enable(st->mclk);
 		if (ret < 0)
-			goto error_remove_trigger;
+			return ret;
+
+		ret = devm_add_action_or_reset(&spi->dev, ad7192_clk_disable,
+					       st->mclk);
+		if (ret)
+			return ret;
 
 		st->fclk = clk_get_rate(st->mclk);
 		if (!ad7192_valid_external_frequency(st->fclk)) {
-			ret = -EINVAL;
 			dev_err(&spi->dev,
 				"External clock frequency out of bounds\n");
-			goto error_disable_clk;
+			return -EINVAL;
 		}
 	}
 
 	ret = ad7192_setup(st, spi->dev.of_node);
 	if (ret)
-		goto error_disable_clk;
-
-	ret = iio_device_register(indio_dev);
-	if (ret < 0)
-		goto error_disable_clk;
-	return 0;
-
-error_disable_clk:
-	if (st->clock_sel == AD7192_CLK_EXT_MCLK1_2 ||
-	    st->clock_sel == AD7192_CLK_EXT_MCLK2)
-		clk_disable_unprepare(st->mclk);
-error_remove_trigger:
-	ad_sd_cleanup_buffer_and_trigger(indio_dev);
-error_disable_dvdd:
-	regulator_disable(st->dvdd);
-error_disable_avdd:
-	regulator_disable(st->avdd);
-
-	return ret;
-}
-
-static int ad7192_remove(struct spi_device *spi)
-{
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct ad7192_state *st = iio_priv(indio_dev);
-
-	iio_device_unregister(indio_dev);
-	if (st->clock_sel == AD7192_CLK_EXT_MCLK1_2 ||
-	    st->clock_sel == AD7192_CLK_EXT_MCLK2)
-		clk_disable_unprepare(st->mclk);
-	ad_sd_cleanup_buffer_and_trigger(indio_dev);
-
-	regulator_disable(st->dvdd);
-	regulator_disable(st->avdd);
+		return ret;
 
-	return 0;
+	return devm_iio_device_register(&spi->dev, indio_dev);
 }
 
 static const struct of_device_id ad7192_of_match[] = {
@@ -1064,7 +1046,6 @@ static struct spi_driver ad7192_driver = {
 		.of_match_table = ad7192_of_match,
 	},
 	.probe		= ad7192_probe,
-	.remove		= ad7192_remove,
 };
 module_spi_driver(ad7192_driver);
 
-- 
2.31.1


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

* [PATCH v2 11/12] iio: adc: ad7124: Use devm_ managed calls for all of probe() + drop remove()
  2021-05-11  7:18 [PATCH v2 00/12] ad_sigma_delta: convert all drivers to device-managed Alexandru Ardelean
                   ` (9 preceding siblings ...)
  2021-05-11  7:18 ` [PATCH v2 10/12] iio: adc: ad7192: convert to device-managed functions Alexandru Ardelean
@ 2021-05-11  7:18 ` Alexandru Ardelean
  2021-05-11  7:18 ` [PATCH v2 12/12] iio: adc: ad_sigma_delta: remove ad_sd_{setup,cleanup}_buffer_and_trigger() Alexandru Ardelean
  11 siblings, 0 replies; 15+ messages in thread
From: Alexandru Ardelean @ 2021-05-11  7:18 UTC (permalink / raw)
  To: linux-iio, linux-kernel
  Cc: jic23, Jonathan.Cameron, alexandru.tachici, linux,
	Alexandru Ardelean, Alexandru Ardelean

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

As not many steps were not already devm_ managed, use
devm_add_action_or_reset() to handle the rest.

This also uses the new devm_ad_sd_setup_buffer_and_trigger() function.

Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/iio/adc/ad7124.c | 48 +++++++++++++---------------------------
 1 file changed, 15 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
index a27db78ea13e..e45c600fccc0 100644
--- a/drivers/iio/adc/ad7124.c
+++ b/drivers/iio/adc/ad7124.c
@@ -862,6 +862,11 @@ static void ad7124_reg_disable(void *r)
 	regulator_disable(r);
 }
 
+static void ad7124_clk_disable(void *c)
+{
+	clk_disable_unprepare(c);
+}
+
 static int ad7124_probe(struct spi_device *spi)
 {
 	const struct ad7124_chip_info *info;
@@ -883,8 +888,6 @@ static int ad7124_probe(struct spi_device *spi)
 
 	ad_sd_init(&st->sd, indio_dev, spi, &ad7124_sigma_delta_info);
 
-	spi_set_drvdata(spi, indio_dev);
-
 	indio_dev->name = st->chip_info->name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &ad7124_info;
@@ -922,48 +925,28 @@ static int ad7124_probe(struct spi_device *spi)
 	if (ret < 0)
 		return ret;
 
+	ret = devm_add_action_or_reset(&spi->dev, ad7124_clk_disable, st->mclk);
+	if (ret)
+		return ret;
+
 	ret = ad7124_soft_reset(st);
 	if (ret < 0)
-		goto error_clk_disable_unprepare;
+		return ret;
 
 	ret = ad7124_check_chip_id(st);
 	if (ret)
-		goto error_clk_disable_unprepare;
+		return ret;
 
 	ret = ad7124_setup(st);
 	if (ret < 0)
-		goto error_clk_disable_unprepare;
+		return ret;
 
-	ret = ad_sd_setup_buffer_and_trigger(indio_dev);
+	ret = devm_ad_sd_setup_buffer_and_trigger(&spi->dev, indio_dev);
 	if (ret < 0)
-		goto error_clk_disable_unprepare;
-
-	ret = iio_device_register(indio_dev);
-	if (ret < 0) {
-		dev_err(&spi->dev, "Failed to register iio device\n");
-		goto error_remove_trigger;
-	}
-
-	return 0;
-
-error_remove_trigger:
-	ad_sd_cleanup_buffer_and_trigger(indio_dev);
-error_clk_disable_unprepare:
-	clk_disable_unprepare(st->mclk);
-
-	return ret;
-}
-
-static int ad7124_remove(struct spi_device *spi)
-{
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct ad7124_state *st = iio_priv(indio_dev);
+		return ret;
 
-	iio_device_unregister(indio_dev);
-	ad_sd_cleanup_buffer_and_trigger(indio_dev);
-	clk_disable_unprepare(st->mclk);
+	return devm_iio_device_register(&spi->dev, indio_dev);
 
-	return 0;
 }
 
 static const struct of_device_id ad7124_of_match[] = {
@@ -981,7 +964,6 @@ static struct spi_driver ad71124_driver = {
 		.of_match_table = ad7124_of_match,
 	},
 	.probe = ad7124_probe,
-	.remove	= ad7124_remove,
 };
 module_spi_driver(ad71124_driver);
 
-- 
2.31.1


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

* [PATCH v2 12/12] iio: adc: ad_sigma_delta: remove ad_sd_{setup,cleanup}_buffer_and_trigger()
  2021-05-11  7:18 [PATCH v2 00/12] ad_sigma_delta: convert all drivers to device-managed Alexandru Ardelean
                   ` (10 preceding siblings ...)
  2021-05-11  7:18 ` [PATCH v2 11/12] iio: adc: ad7124: Use devm_ managed calls for all of probe() + drop remove() Alexandru Ardelean
@ 2021-05-11  7:18 ` Alexandru Ardelean
  11 siblings, 0 replies; 15+ messages in thread
From: Alexandru Ardelean @ 2021-05-11  7:18 UTC (permalink / raw)
  To: linux-iio, linux-kernel
  Cc: jic23, Jonathan.Cameron, alexandru.tachici, linux, Alexandru Ardelean

Since all AD Sigma-Delta drivers now use the
devm_ad_sd_setup_buffer_and_trigger() function, we can remove the old
ad_sd_{setup,cleanup}_buffer_and_trigger() functions.

This way we can discourage new drivers that use the ad_sigma_delta
lib-driver to use these (older functions).

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/iio/adc/ad_sigma_delta.c       | 86 --------------------------
 include/linux/iio/adc/ad_sigma_delta.h |  3 -
 2 files changed, 89 deletions(-)

diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index d5801a47be07..1d652d9b2f5c 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -470,49 +470,6 @@ EXPORT_SYMBOL_GPL(ad_sd_validate_trigger);
 static const struct iio_trigger_ops ad_sd_trigger_ops = {
 };
 
-static int ad_sd_probe_trigger(struct iio_dev *indio_dev)
-{
-	struct ad_sigma_delta *sigma_delta = iio_device_get_drvdata(indio_dev);
-	int ret;
-
-	sigma_delta->trig = iio_trigger_alloc(&sigma_delta->spi->dev,
-					      "%s-dev%d", indio_dev->name,
-					      iio_device_id(indio_dev));
-	if (sigma_delta->trig == NULL) {
-		ret = -ENOMEM;
-		goto error_ret;
-	}
-	sigma_delta->trig->ops = &ad_sd_trigger_ops;
-	init_completion(&sigma_delta->completion);
-
-	sigma_delta->irq_dis = true;
-	ret = request_irq(sigma_delta->spi->irq,
-			  ad_sd_data_rdy_trig_poll,
-			  sigma_delta->info->irq_flags | IRQF_NO_AUTOEN,
-			  indio_dev->name,
-			  sigma_delta);
-	if (ret)
-		goto error_free_trig;
-
-	iio_trigger_set_drvdata(sigma_delta->trig, sigma_delta);
-
-	ret = iio_trigger_register(sigma_delta->trig);
-	if (ret)
-		goto error_free_irq;
-
-	/* select default trigger */
-	indio_dev->trig = iio_trigger_get(sigma_delta->trig);
-
-	return 0;
-
-error_free_irq:
-	free_irq(sigma_delta->spi->irq, sigma_delta);
-error_free_trig:
-	iio_trigger_free(sigma_delta->trig);
-error_ret:
-	return ret;
-}
-
 static int devm_ad_sd_probe_trigger(struct device *dev, struct iio_dev *indio_dev)
 {
 	struct ad_sigma_delta *sigma_delta = iio_device_get_drvdata(indio_dev);
@@ -553,49 +510,6 @@ static int devm_ad_sd_probe_trigger(struct device *dev, struct iio_dev *indio_de
 	return 0;
 }
 
-static void ad_sd_remove_trigger(struct iio_dev *indio_dev)
-{
-	struct ad_sigma_delta *sigma_delta = iio_device_get_drvdata(indio_dev);
-
-	iio_trigger_unregister(sigma_delta->trig);
-	free_irq(sigma_delta->spi->irq, sigma_delta);
-	iio_trigger_free(sigma_delta->trig);
-}
-
-/**
- * ad_sd_setup_buffer_and_trigger() -
- * @indio_dev: The IIO device
- */
-int ad_sd_setup_buffer_and_trigger(struct iio_dev *indio_dev)
-{
-	int ret;
-
-	ret = iio_triggered_buffer_setup(indio_dev, &iio_pollfunc_store_time,
-			&ad_sd_trigger_handler, &ad_sd_buffer_setup_ops);
-	if (ret)
-		return ret;
-
-	ret = ad_sd_probe_trigger(indio_dev);
-	if (ret) {
-		iio_triggered_buffer_cleanup(indio_dev);
-		return ret;
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(ad_sd_setup_buffer_and_trigger);
-
-/**
- * ad_sd_cleanup_buffer_and_trigger() -
- * @indio_dev: The IIO device
- */
-void ad_sd_cleanup_buffer_and_trigger(struct iio_dev *indio_dev)
-{
-	ad_sd_remove_trigger(indio_dev);
-	iio_triggered_buffer_cleanup(indio_dev);
-}
-EXPORT_SYMBOL_GPL(ad_sd_cleanup_buffer_and_trigger);
-
 /**
  * devm_ad_sd_setup_buffer_and_trigger() - Device-managed buffer & trigger setup
  * @dev: Device object to which to bind the life-time of the resources attached
diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h
index be81ad39fb7a..c525fd51652f 100644
--- a/include/linux/iio/adc/ad_sigma_delta.h
+++ b/include/linux/iio/adc/ad_sigma_delta.h
@@ -133,9 +133,6 @@ int ad_sd_calibrate_all(struct ad_sigma_delta *sigma_delta,
 int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev *indio_dev,
 	struct spi_device *spi, const struct ad_sigma_delta_info *info);
 
-int ad_sd_setup_buffer_and_trigger(struct iio_dev *indio_dev);
-void ad_sd_cleanup_buffer_and_trigger(struct iio_dev *indio_dev);
-
 int devm_ad_sd_setup_buffer_and_trigger(struct device *dev, struct iio_dev *indio_dev);
 
 int ad_sd_validate_trigger(struct iio_dev *indio_dev, struct iio_trigger *trig);
-- 
2.31.1


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

* Re: [PATCH v2 04/12] iio: adc: ad7192: handle zero Avdd regulator value as error
  2021-05-11  7:18 ` [PATCH v2 04/12] iio: adc: ad7192: handle zero Avdd regulator value as error Alexandru Ardelean
@ 2021-05-11 14:13   ` Jonathan Cameron
  2021-05-12  6:28     ` Alexandru Ardelean
  0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2021-05-11 14:13 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: linux-iio, linux-kernel, Jonathan.Cameron, alexandru.tachici, linux

On Tue, 11 May 2021 10:18:23 +0300
Alexandru Ardelean <aardelean@deviqon.com> wrote:

> This change fixes a corner-case, where the returned voltage is actually
> zero. This is also what patch ab0afa65bbc7 ("staging: iio: adc: ad7192:
> fail probe on get_voltage") was trying to do.
> 
> But as Jonathan pointed out, a zero-value would signal that the probe has
> succeeded, putting the driver is a semi-initialized state.
> 
> Fixes: ab0afa65bbc7 ("staging: iio: adc: ad7192: fail probe on get_voltage")
> Cc: Alexandru Tachici <alexandru.tachici@analog.com>
> Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>

Given that voltage_uv == 1 would result in a situation no worse than
for voltage_uv == 0 perhaps we should just change the following condition to

if (voltage_uv >= 0)  ?

Jonathan

> ---
>  drivers/iio/adc/ad7192.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
> index d3be67aa0522..79df54e0dc96 100644
> --- a/drivers/iio/adc/ad7192.c
> +++ b/drivers/iio/adc/ad7192.c
> @@ -950,6 +950,11 @@ static int ad7192_probe(struct spi_device *spi)
>  	}
>  
>  	voltage_uv = regulator_get_voltage(st->avdd);
> +	if (voltage_uv == 0) {
> +		ret = -EINVAL;
> +		dev_err(&spi->dev, "Zero value provided for AVdd supply\n");
> +		goto error_disable_avdd;
> +	}
>  
>  	if (voltage_uv > 0) {
>  		st->int_vref_mv = voltage_uv / 1000;


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

* Re: [PATCH v2 04/12] iio: adc: ad7192: handle zero Avdd regulator value as error
  2021-05-11 14:13   ` Jonathan Cameron
@ 2021-05-12  6:28     ` Alexandru Ardelean
  0 siblings, 0 replies; 15+ messages in thread
From: Alexandru Ardelean @ 2021-05-12  6:28 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, Linux Kernel Mailing List, Jonathan Cameron,
	alexandru.tachici, linux

On Tue, 11 May 2021 at 17:12, Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Tue, 11 May 2021 10:18:23 +0300
> Alexandru Ardelean <aardelean@deviqon.com> wrote:
>
> > This change fixes a corner-case, where the returned voltage is actually
> > zero. This is also what patch ab0afa65bbc7 ("staging: iio: adc: ad7192:
> > fail probe on get_voltage") was trying to do.
> >
> > But as Jonathan pointed out, a zero-value would signal that the probe has
> > succeeded, putting the driver is a semi-initialized state.
> >
> > Fixes: ab0afa65bbc7 ("staging: iio: adc: ad7192: fail probe on get_voltage")
> > Cc: Alexandru Tachici <alexandru.tachici@analog.com>
> > Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
>
> Given that voltage_uv == 1 would result in a situation no worse than
> for voltage_uv == 0 perhaps we should just change the following condition to
>
> if (voltage_uv >= 0)  ?

hmmm, you're right;
i think had some narrow vision about this;

will send a v3

>
> Jonathan
>
> > ---
> >  drivers/iio/adc/ad7192.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
> > index d3be67aa0522..79df54e0dc96 100644
> > --- a/drivers/iio/adc/ad7192.c
> > +++ b/drivers/iio/adc/ad7192.c
> > @@ -950,6 +950,11 @@ static int ad7192_probe(struct spi_device *spi)
> >       }
> >
> >       voltage_uv = regulator_get_voltage(st->avdd);
> > +     if (voltage_uv == 0) {
> > +             ret = -EINVAL;
> > +             dev_err(&spi->dev, "Zero value provided for AVdd supply\n");
> > +             goto error_disable_avdd;
> > +     }
> >
> >       if (voltage_uv > 0) {
> >               st->int_vref_mv = voltage_uv / 1000;
>

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

end of thread, other threads:[~2021-05-12  6:28 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-11  7:18 [PATCH v2 00/12] ad_sigma_delta: convert all drivers to device-managed Alexandru Ardelean
2021-05-11  7:18 ` [PATCH v2 01/12] iio: adc: ad7124: Fix missbalanced regulator enable / disable on error Alexandru Ardelean
2021-05-11  7:18 ` [PATCH v2 02/12] iio: adc: ad7124: Fix potential overflow due to non sequential channel numbers Alexandru Ardelean
2021-05-11  7:18 ` [PATCH v2 03/12] iio: adc: ad7192: Avoid disabling a clock that was never enabled Alexandru Ardelean
2021-05-11  7:18 ` [PATCH v2 04/12] iio: adc: ad7192: handle zero Avdd regulator value as error Alexandru Ardelean
2021-05-11 14:13   ` Jonathan Cameron
2021-05-12  6:28     ` Alexandru Ardelean
2021-05-11  7:18 ` [PATCH v2 05/12] iio: adc: ad_sigma_delta: introduct devm_ad_sd_setup_buffer_and_trigger() Alexandru Ardelean
2021-05-11  7:18 ` [PATCH v2 06/12] iio: adc: ad7793: convert to device-managed functions Alexandru Ardelean
2021-05-11  7:18 ` [PATCH v2 07/12] iio: adc: ad7791: " Alexandru Ardelean
2021-05-11  7:18 ` [PATCH v2 08/12] iio: adc: ad7780: " Alexandru Ardelean
2021-05-11  7:18 ` [PATCH v2 09/12] iio: adc: ad7192: use devm_clk_get_optional() for mclk Alexandru Ardelean
2021-05-11  7:18 ` [PATCH v2 10/12] iio: adc: ad7192: convert to device-managed functions Alexandru Ardelean
2021-05-11  7:18 ` [PATCH v2 11/12] iio: adc: ad7124: Use devm_ managed calls for all of probe() + drop remove() Alexandru Ardelean
2021-05-11  7:18 ` [PATCH v2 12/12] iio: adc: ad_sigma_delta: remove ad_sd_{setup,cleanup}_buffer_and_trigger() Alexandru Ardelean

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.