All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] staging: iio: regulator clean-up
@ 2016-10-31 17:04 Eva Rachel Retuya
  2016-10-31 17:04 ` [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get() Eva Rachel Retuya
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Eva Rachel Retuya @ 2016-10-31 17:04 UTC (permalink / raw)
  To: linux-iio, devel, linux-kernel
  Cc: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh,
	Eva Rachel Retuya

Rework regulator handling: 
* use supply names found on the datasheet
* fix regulator usage
* declare digital supplies previously missing

Semantic patch used to detect affected drivers:
@r1@
expression reg;
position p;
@@

reg = \(devm_regulator_get@p\|regulator_get@p\)(...);
if (IS_ERR(reg)) {
	...
	PTR_ERR(reg)
	...
}

@@
position p != r1.p;
@@

* \(devm_regulator_get@p\|regulator_get@p\)(...)

Eva Rachel Retuya (6):
  staging: iio: set proper supply name to devm_regulator_get()
  staging: iio: rework regulator handling
  staging: iio: ad7192: add DVdd regulator
  staging: iio: ad7192: rename regulator 'reg' to 'avdd'
  staging: iio: ad9832: add DVDD regulator
  staging: iio: ad9832: clean-up regulator 'reg'

 drivers/staging/iio/adc/ad7192.c                | 43 +++++++++++++------
 drivers/staging/iio/adc/ad7780.c                | 22 +++++-----
 drivers/staging/iio/frequency/ad9832.c          | 56 +++++++++++++++----------
 drivers/staging/iio/frequency/ad9832.h          |  6 ++-
 drivers/staging/iio/frequency/ad9834.c          | 19 +++++----
 drivers/staging/iio/impedance-analyzer/ad5933.c | 21 +++++-----
 6 files changed, 101 insertions(+), 66 deletions(-)

-- 
2.7.4

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

* [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get()
  2016-10-31 17:04 [PATCH 0/6] staging: iio: regulator clean-up Eva Rachel Retuya
@ 2016-10-31 17:04 ` Eva Rachel Retuya
  2016-11-01  4:03   ` Matt Ranostay
  2016-11-05 16:14   ` Jonathan Cameron
  2016-10-31 17:04 ` [PATCH 2/6] staging: iio: rework regulator handling Eva Rachel Retuya
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 18+ messages in thread
From: Eva Rachel Retuya @ 2016-10-31 17:04 UTC (permalink / raw)
  To: linux-iio, devel, linux-kernel
  Cc: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh,
	Eva Rachel Retuya

The name passed to devm_regulator_get() should match the name of the
supply as specified in the device datasheet. This makes it clear what
power supply is being referred to in case of presence of other
regulators.

Currently, the supply name specified on the affected devices is 'vcc'.
Use lowercase version of the datasheet name to specify the supply
voltage.

Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
---
 drivers/staging/iio/adc/ad7192.c                | 2 +-
 drivers/staging/iio/adc/ad7780.c                | 2 +-
 drivers/staging/iio/frequency/ad9832.c          | 2 +-
 drivers/staging/iio/frequency/ad9834.c          | 2 +-
 drivers/staging/iio/impedance-analyzer/ad5933.c | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index bfa12ce..41fb32d 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -633,7 +633,7 @@ static int ad7192_probe(struct spi_device *spi)
 
 	st = iio_priv(indio_dev);
 
-	st->reg = devm_regulator_get(&spi->dev, "vcc");
+	st->reg = devm_regulator_get(&spi->dev, "avdd");
 	if (!IS_ERR(st->reg)) {
 		ret = regulator_enable(st->reg);
 		if (ret)
diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c
index c9a0c2a..a88236e 100644
--- a/drivers/staging/iio/adc/ad7780.c
+++ b/drivers/staging/iio/adc/ad7780.c
@@ -173,7 +173,7 @@ static int ad7780_probe(struct spi_device *spi)
 
 	ad_sd_init(&st->sd, indio_dev, spi, &ad7780_sigma_delta_info);
 
-	st->reg = devm_regulator_get(&spi->dev, "vcc");
+	st->reg = devm_regulator_get(&spi->dev, "avdd");
 	if (!IS_ERR(st->reg)) {
 		ret = regulator_enable(st->reg);
 		if (ret)
diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index 358400b..744c8ee 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -212,7 +212,7 @@ static int ad9832_probe(struct spi_device *spi)
 		return -ENODEV;
 	}
 
-	reg = devm_regulator_get(&spi->dev, "vcc");
+	reg = devm_regulator_get(&spi->dev, "avdd");
 	if (!IS_ERR(reg)) {
 		ret = regulator_enable(reg);
 		if (ret)
diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
index 6366216..ca3cea6 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -329,7 +329,7 @@ static int ad9834_probe(struct spi_device *spi)
 		return -ENODEV;
 	}
 
-	reg = devm_regulator_get(&spi->dev, "vcc");
+	reg = devm_regulator_get(&spi->dev, "avdd");
 	if (!IS_ERR(reg)) {
 		ret = regulator_enable(reg);
 		if (ret)
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
index 5eecf1c..62f61bc 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -723,7 +723,7 @@ static int ad5933_probe(struct i2c_client *client,
 	if (!pdata)
 		pdata = &ad5933_default_pdata;
 
-	st->reg = devm_regulator_get(&client->dev, "vcc");
+	st->reg = devm_regulator_get(&client->dev, "vdd");
 	if (!IS_ERR(st->reg)) {
 		ret = regulator_enable(st->reg);
 		if (ret)
-- 
2.7.4

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

* [PATCH 2/6] staging: iio: rework regulator handling
  2016-10-31 17:04 [PATCH 0/6] staging: iio: regulator clean-up Eva Rachel Retuya
  2016-10-31 17:04 ` [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get() Eva Rachel Retuya
@ 2016-10-31 17:04 ` Eva Rachel Retuya
  2016-11-05 16:18   ` Jonathan Cameron
  2016-10-31 17:04 ` [PATCH 3/6] staging: iio: ad7192: add DVdd regulator Eva Rachel Retuya
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Eva Rachel Retuya @ 2016-10-31 17:04 UTC (permalink / raw)
  To: linux-iio, devel, linux-kernel
  Cc: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh,
	Eva Rachel Retuya

Currently, the affected drivers ignore all errors from regulator_get().
The way it is now, it also breaks probe deferral (EPROBE_DEFER). The
correct behavior is to propagate the error to the upper layers so they
can handle it accordingly.

Rework the regulator handling so that it matches the standard behavior.
If the specific design uses a static always-on regulator and does not
explicitly specify it, regulator_get() will return the dummy regulator.

The following semantic patch was used to apply the change:
@r1@
expression reg, dev, en, volt;
@@

reg = \(devm_regulator_get\|regulator_get\)(dev, ...);
if (
- !
   IS_ERR(reg))
+	return PTR_ERR(reg);
(
- {	en = regulator_enable(reg);
-	if (en) return en; }
+
+	en = regulator_enable(reg);
+	if (en) {
+		dev_err(dev, "Failed to enable specified supply\n");
+		return en; }
|
+
- {	en = regulator_enable(reg);
-	if (en)	return en;
-	volt = regulator_get_voltage(reg); }
+	en = regulator_enable(reg);
+	if (en) {
+		dev_err(dev, "Failed to enable specified supply\n");
+		return en;
+	}
+	volt = regulator_get_voltage(reg);
)

@r2@
expression arg;
@@

-	if (!IS_ERR(arg)) regulator_disable(arg);
+	regulator_disable(arg);

Hand-edit the debugging prints with the supply name to become more
specific.

Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
---
 drivers/staging/iio/adc/ad7192.c                | 18 +++++++++---------
 drivers/staging/iio/adc/ad7780.c                | 18 +++++++++---------
 drivers/staging/iio/frequency/ad9832.c          | 17 +++++++++--------
 drivers/staging/iio/frequency/ad9834.c          | 17 +++++++++--------
 drivers/staging/iio/impedance-analyzer/ad5933.c | 19 ++++++++++---------
 5 files changed, 46 insertions(+), 43 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index 41fb32d..29e32b7 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -634,13 +634,15 @@ static int ad7192_probe(struct spi_device *spi)
 	st = iio_priv(indio_dev);
 
 	st->reg = devm_regulator_get(&spi->dev, "avdd");
-	if (!IS_ERR(st->reg)) {
-		ret = regulator_enable(st->reg);
-		if (ret)
-			return ret;
+	if (IS_ERR(st->reg))
+		return PTR_ERR(st->reg);
 
-		voltage_uv = regulator_get_voltage(st->reg);
+	ret = regulator_enable(st->reg);
+	if (ret) {
+		dev_err(&spi->dev, "Failed to enable specified AVdd supply\n");
+		return ret;
 	}
+	voltage_uv = regulator_get_voltage(st->reg);
 
 	if (pdata->vref_mv)
 		st->int_vref_mv = pdata->vref_mv;
@@ -689,8 +691,7 @@ static int ad7192_probe(struct spi_device *spi)
 error_remove_trigger:
 	ad_sd_cleanup_buffer_and_trigger(indio_dev);
 error_disable_reg:
-	if (!IS_ERR(st->reg))
-		regulator_disable(st->reg);
+	regulator_disable(st->reg);
 
 	return ret;
 }
@@ -703,8 +704,7 @@ static int ad7192_remove(struct spi_device *spi)
 	iio_device_unregister(indio_dev);
 	ad_sd_cleanup_buffer_and_trigger(indio_dev);
 
-	if (!IS_ERR(st->reg))
-		regulator_disable(st->reg);
+	regulator_disable(st->reg);
 
 	return 0;
 }
diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c
index a88236e..e149600 100644
--- a/drivers/staging/iio/adc/ad7780.c
+++ b/drivers/staging/iio/adc/ad7780.c
@@ -174,13 +174,15 @@ static int ad7780_probe(struct spi_device *spi)
 	ad_sd_init(&st->sd, indio_dev, spi, &ad7780_sigma_delta_info);
 
 	st->reg = devm_regulator_get(&spi->dev, "avdd");
-	if (!IS_ERR(st->reg)) {
-		ret = regulator_enable(st->reg);
-		if (ret)
-			return ret;
+	if (IS_ERR(st->reg))
+		return PTR_ERR(st->reg);
 
-		voltage_uv = regulator_get_voltage(st->reg);
+	ret = regulator_enable(st->reg);
+	if (ret) {
+		dev_err(&spi->dev, "Failed to enable specified AVdd supply\n");
+		return ret;
 	}
+	voltage_uv = regulator_get_voltage(st->reg);
 
 	st->chip_info =
 		&ad7780_chip_info_tbl[spi_get_device_id(spi)->driver_data];
@@ -222,8 +224,7 @@ static int ad7780_probe(struct spi_device *spi)
 error_cleanup_buffer_and_trigger:
 	ad_sd_cleanup_buffer_and_trigger(indio_dev);
 error_disable_reg:
-	if (!IS_ERR(st->reg))
-		regulator_disable(st->reg);
+	regulator_disable(st->reg);
 
 	return ret;
 }
@@ -236,8 +237,7 @@ static int ad7780_remove(struct spi_device *spi)
 	iio_device_unregister(indio_dev);
 	ad_sd_cleanup_buffer_and_trigger(indio_dev);
 
-	if (!IS_ERR(st->reg))
-		regulator_disable(st->reg);
+	regulator_disable(st->reg);
 
 	return 0;
 }
diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index 744c8ee..7d8dc6c 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -213,10 +213,13 @@ static int ad9832_probe(struct spi_device *spi)
 	}
 
 	reg = devm_regulator_get(&spi->dev, "avdd");
-	if (!IS_ERR(reg)) {
-		ret = regulator_enable(reg);
-		if (ret)
-			return ret;
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
+
+	ret = regulator_enable(reg);
+	if (ret) {
+		dev_err(&spi->dev, "Failed to enable specified AVDD supply\n");
+		return ret;
 	}
 
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
@@ -311,8 +314,7 @@ static int ad9832_probe(struct spi_device *spi)
 	return 0;
 
 error_disable_reg:
-	if (!IS_ERR(reg))
-		regulator_disable(reg);
+	regulator_disable(reg);
 
 	return ret;
 }
@@ -323,8 +325,7 @@ static int ad9832_remove(struct spi_device *spi)
 	struct ad9832_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
-	if (!IS_ERR(st->reg))
-		regulator_disable(st->reg);
+	regulator_disable(st->reg);
 
 	return 0;
 }
diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
index ca3cea6..19216af 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -330,10 +330,13 @@ static int ad9834_probe(struct spi_device *spi)
 	}
 
 	reg = devm_regulator_get(&spi->dev, "avdd");
-	if (!IS_ERR(reg)) {
-		ret = regulator_enable(reg);
-		if (ret)
-			return ret;
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
+
+	ret = regulator_enable(reg);
+	if (ret) {
+		dev_err(&spi->dev, "Failed to enable specified AVDD supply\n");
+		return ret;
 	}
 
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
@@ -416,8 +419,7 @@ static int ad9834_probe(struct spi_device *spi)
 	return 0;
 
 error_disable_reg:
-	if (!IS_ERR(reg))
-		regulator_disable(reg);
+	regulator_disable(reg);
 
 	return ret;
 }
@@ -428,8 +430,7 @@ static int ad9834_remove(struct spi_device *spi)
 	struct ad9834_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
-	if (!IS_ERR(st->reg))
-		regulator_disable(st->reg);
+	regulator_disable(st->reg);
 
 	return 0;
 }
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
index 62f61bc..8f2736a 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -724,12 +724,15 @@ static int ad5933_probe(struct i2c_client *client,
 		pdata = &ad5933_default_pdata;
 
 	st->reg = devm_regulator_get(&client->dev, "vdd");
-	if (!IS_ERR(st->reg)) {
-		ret = regulator_enable(st->reg);
-		if (ret)
-			return ret;
-		voltage_uv = regulator_get_voltage(st->reg);
+	if (IS_ERR(st->reg))
+		return PTR_ERR(st->reg);
+
+	ret = regulator_enable(st->reg);
+	if (ret) {
+		dev_err(&client->dev, "Failed to enable specified VDD supply\n");
+		return ret;
 	}
+	voltage_uv = regulator_get_voltage(st->reg);
 
 	if (voltage_uv)
 		st->vref_mv = voltage_uv / 1000;
@@ -772,8 +775,7 @@ static int ad5933_probe(struct i2c_client *client,
 error_unreg_ring:
 	iio_kfifo_free(indio_dev->buffer);
 error_disable_reg:
-	if (!IS_ERR(st->reg))
-		regulator_disable(st->reg);
+	regulator_disable(st->reg);
 
 	return ret;
 }
@@ -785,8 +787,7 @@ static int ad5933_remove(struct i2c_client *client)
 
 	iio_device_unregister(indio_dev);
 	iio_kfifo_free(indio_dev->buffer);
-	if (!IS_ERR(st->reg))
-		regulator_disable(st->reg);
+	regulator_disable(st->reg);
 
 	return 0;
 }
-- 
2.7.4

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

* [PATCH 3/6] staging: iio: ad7192: add DVdd regulator
  2016-10-31 17:04 [PATCH 0/6] staging: iio: regulator clean-up Eva Rachel Retuya
  2016-10-31 17:04 ` [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get() Eva Rachel Retuya
  2016-10-31 17:04 ` [PATCH 2/6] staging: iio: rework regulator handling Eva Rachel Retuya
@ 2016-10-31 17:04 ` Eva Rachel Retuya
  2016-11-05 16:19   ` Jonathan Cameron
  2016-10-31 17:04 ` [PATCH 4/6] staging: iio: ad7192: rename regulator 'reg' to 'avdd' Eva Rachel Retuya
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Eva Rachel Retuya @ 2016-10-31 17:04 UTC (permalink / raw)
  To: linux-iio, devel, linux-kernel
  Cc: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh,
	Eva Rachel Retuya

The AD7190/AD7192/AD7193/AD7195 is supplied with two power sources:
AVdd as analog supply voltage and DVdd as digital supply voltage.

Attempt to fetch and enable the regulator 'dvdd'. Bail out if any error
occurs.

Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
---
 drivers/staging/iio/adc/ad7192.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index 29e32b7..3f8dc66 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -153,6 +153,7 @@
 
 struct ad7192_state {
 	struct regulator		*reg;
+	struct regulator		*dvdd;
 	u16				int_vref_mv;
 	u32				mclk;
 	u32				f_order;
@@ -642,6 +643,19 @@ static int ad7192_probe(struct spi_device *spi)
 		dev_err(&spi->dev, "Failed to enable specified AVdd supply\n");
 		return ret;
 	}
+
+	st->dvdd = devm_regulator_get(&spi->dev, "dvdd");
+	if (IS_ERR(st->dvdd)) {
+		ret = PTR_ERR(st->dvdd);
+		goto error_disable_reg;
+	}
+
+	ret = regulator_enable(st->dvdd);
+	if (ret) {
+		dev_err(&spi->dev, "Failed to enable specified DVdd supply\n");
+		goto error_disable_reg;
+	}
+
 	voltage_uv = regulator_get_voltage(st->reg);
 
 	if (pdata->vref_mv)
@@ -677,7 +691,7 @@ static int ad7192_probe(struct spi_device *spi)
 
 	ret = ad_sd_setup_buffer_and_trigger(indio_dev);
 	if (ret)
-		goto error_disable_reg;
+		goto error_disable_dvdd;
 
 	ret = ad7192_setup(st, pdata);
 	if (ret)
@@ -690,6 +704,8 @@ static int ad7192_probe(struct spi_device *spi)
 
 error_remove_trigger:
 	ad_sd_cleanup_buffer_and_trigger(indio_dev);
+error_disable_dvdd:
+	regulator_disable(st->dvdd);
 error_disable_reg:
 	regulator_disable(st->reg);
 
@@ -704,6 +720,7 @@ static int ad7192_remove(struct spi_device *spi)
 	iio_device_unregister(indio_dev);
 	ad_sd_cleanup_buffer_and_trigger(indio_dev);
 
+	regulator_disable(st->dvdd);
 	regulator_disable(st->reg);
 
 	return 0;
-- 
2.7.4

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

* [PATCH 4/6] staging: iio: ad7192: rename regulator 'reg' to 'avdd'
  2016-10-31 17:04 [PATCH 0/6] staging: iio: regulator clean-up Eva Rachel Retuya
                   ` (2 preceding siblings ...)
  2016-10-31 17:04 ` [PATCH 3/6] staging: iio: ad7192: add DVdd regulator Eva Rachel Retuya
@ 2016-10-31 17:04 ` Eva Rachel Retuya
  2016-11-05 16:21   ` Jonathan Cameron
  2016-10-31 17:04 ` [PATCH 5/6] staging: iio: ad9832: add DVDD regulator Eva Rachel Retuya
  2016-10-31 17:04 ` [PATCH 6/6] staging: iio: ad9832: clean-up regulator 'reg' Eva Rachel Retuya
  5 siblings, 1 reply; 18+ messages in thread
From: Eva Rachel Retuya @ 2016-10-31 17:04 UTC (permalink / raw)
  To: linux-iio, devel, linux-kernel
  Cc: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh,
	Eva Rachel Retuya

Rename regulator 'reg' to 'avdd' so as to be clear what regulator it
stands for specifically. Also, update the goto label accordingly.

Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
---
 drivers/staging/iio/adc/ad7192.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index 3f8dc66..1fb68c0 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -152,7 +152,7 @@
  */
 
 struct ad7192_state {
-	struct regulator		*reg;
+	struct regulator		*avdd;
 	struct regulator		*dvdd;
 	u16				int_vref_mv;
 	u32				mclk;
@@ -634,11 +634,11 @@ static int ad7192_probe(struct spi_device *spi)
 
 	st = iio_priv(indio_dev);
 
-	st->reg = devm_regulator_get(&spi->dev, "avdd");
-	if (IS_ERR(st->reg))
-		return PTR_ERR(st->reg);
+	st->avdd = devm_regulator_get(&spi->dev, "avdd");
+	if (IS_ERR(st->avdd))
+		return PTR_ERR(st->avdd);
 
-	ret = regulator_enable(st->reg);
+	ret = regulator_enable(st->avdd);
 	if (ret) {
 		dev_err(&spi->dev, "Failed to enable specified AVdd supply\n");
 		return ret;
@@ -647,16 +647,16 @@ static int ad7192_probe(struct spi_device *spi)
 	st->dvdd = devm_regulator_get(&spi->dev, "dvdd");
 	if (IS_ERR(st->dvdd)) {
 		ret = PTR_ERR(st->dvdd);
-		goto error_disable_reg;
+		goto error_disable_avdd;
 	}
 
 	ret = regulator_enable(st->dvdd);
 	if (ret) {
 		dev_err(&spi->dev, "Failed to enable specified DVdd supply\n");
-		goto error_disable_reg;
+		goto error_disable_avdd;
 	}
 
-	voltage_uv = regulator_get_voltage(st->reg);
+	voltage_uv = regulator_get_voltage(st->avdd);
 
 	if (pdata->vref_mv)
 		st->int_vref_mv = pdata->vref_mv;
@@ -706,8 +706,8 @@ static int ad7192_probe(struct spi_device *spi)
 	ad_sd_cleanup_buffer_and_trigger(indio_dev);
 error_disable_dvdd:
 	regulator_disable(st->dvdd);
-error_disable_reg:
-	regulator_disable(st->reg);
+error_disable_avdd:
+	regulator_disable(st->avdd);
 
 	return ret;
 }
@@ -721,7 +721,7 @@ static int ad7192_remove(struct spi_device *spi)
 	ad_sd_cleanup_buffer_and_trigger(indio_dev);
 
 	regulator_disable(st->dvdd);
-	regulator_disable(st->reg);
+	regulator_disable(st->avdd);
 
 	return 0;
 }
-- 
2.7.4

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

* [PATCH 5/6] staging: iio: ad9832: add DVDD regulator
  2016-10-31 17:04 [PATCH 0/6] staging: iio: regulator clean-up Eva Rachel Retuya
                   ` (3 preceding siblings ...)
  2016-10-31 17:04 ` [PATCH 4/6] staging: iio: ad7192: rename regulator 'reg' to 'avdd' Eva Rachel Retuya
@ 2016-10-31 17:04 ` Eva Rachel Retuya
  2016-11-05 16:22   ` Jonathan Cameron
  2016-10-31 17:04 ` [PATCH 6/6] staging: iio: ad9832: clean-up regulator 'reg' Eva Rachel Retuya
  5 siblings, 1 reply; 18+ messages in thread
From: Eva Rachel Retuya @ 2016-10-31 17:04 UTC (permalink / raw)
  To: linux-iio, devel, linux-kernel
  Cc: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh,
	Eva Rachel Retuya

The AD9832/AD9835 is supplied with two power sources: AVDD as analog
supply voltage and DVDD as digital supply voltage.

Attempt to fetch and enable the regulator 'dvdd'. Bail out if any error
occurs.

Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
---
 drivers/staging/iio/frequency/ad9832.c | 33 ++++++++++++++++++++++++---------
 drivers/staging/iio/frequency/ad9832.h |  2 ++
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index 7d8dc6c..6a5ab02 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -222,10 +222,22 @@ static int ad9832_probe(struct spi_device *spi)
 		return ret;
 	}
 
+	st->dvdd = devm_regulator_get(&spi->dev, "dvdd");
+	if (IS_ERR(st->dvdd)) {
+		ret = PTR_ERR(st->dvdd);
+		goto error_disable_reg;
+	}
+
+	ret = regulator_enable(st->dvdd);
+	if (ret) {
+		dev_err(&spi->dev, "Failed to enable specified DVDD supply\n");
+		goto error_disable_reg;
+	}
+
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
 	if (!indio_dev) {
 		ret = -ENOMEM;
-		goto error_disable_reg;
+		goto error_disable_dvdd;
 	}
 	spi_set_drvdata(spi, indio_dev);
 	st = iio_priv(indio_dev);
@@ -280,39 +292,41 @@ static int ad9832_probe(struct spi_device *spi)
 	ret = spi_sync(st->spi, &st->msg);
 	if (ret) {
 		dev_err(&spi->dev, "device init failed\n");
-		goto error_disable_reg;
+		goto error_disable_dvdd;
 	}
 
 	ret = ad9832_write_frequency(st, AD9832_FREQ0HM, pdata->freq0);
 	if (ret)
-		goto error_disable_reg;
+		goto error_disable_dvdd;
 
 	ret = ad9832_write_frequency(st, AD9832_FREQ1HM, pdata->freq1);
 	if (ret)
-		goto error_disable_reg;
+		goto error_disable_dvdd;
 
 	ret = ad9832_write_phase(st, AD9832_PHASE0H, pdata->phase0);
 	if (ret)
-		goto error_disable_reg;
+		goto error_disable_dvdd;
 
 	ret = ad9832_write_phase(st, AD9832_PHASE1H, pdata->phase1);
 	if (ret)
-		goto error_disable_reg;
+		goto error_disable_dvdd;
 
 	ret = ad9832_write_phase(st, AD9832_PHASE2H, pdata->phase2);
 	if (ret)
-		goto error_disable_reg;
+		goto error_disable_dvdd;
 
 	ret = ad9832_write_phase(st, AD9832_PHASE3H, pdata->phase3);
 	if (ret)
-		goto error_disable_reg;
+		goto error_disable_dvdd;
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
-		goto error_disable_reg;
+		goto error_disable_dvdd;
 
 	return 0;
 
+error_disable_dvdd:
+	regulator_disable(st->dvdd);
 error_disable_reg:
 	regulator_disable(reg);
 
@@ -325,6 +339,7 @@ static int ad9832_remove(struct spi_device *spi)
 	struct ad9832_state *st = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
+	regulator_disable(st->dvdd);
 	regulator_disable(st->reg);
 
 	return 0;
diff --git a/drivers/staging/iio/frequency/ad9832.h b/drivers/staging/iio/frequency/ad9832.h
index d32323b..eb0e7f2 100644
--- a/drivers/staging/iio/frequency/ad9832.h
+++ b/drivers/staging/iio/frequency/ad9832.h
@@ -59,6 +59,7 @@
  * struct ad9832_state - driver instance specific data
  * @spi:		spi_device
  * @reg:		supply regulator
+ * @dvdd:		supply regulator for the digital section
  * @mclk:		external master clock
  * @ctrl_fp:		cached frequency/phase control word
  * @ctrl_ss:		cached sync/selsrc control word
@@ -77,6 +78,7 @@
 struct ad9832_state {
 	struct spi_device		*spi;
 	struct regulator		*reg;
+	struct regulator		*dvdd;
 	unsigned long			mclk;
 	unsigned short			ctrl_fp;
 	unsigned short			ctrl_ss;
-- 
2.7.4

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

* [PATCH 6/6] staging: iio: ad9832: clean-up regulator 'reg'
  2016-10-31 17:04 [PATCH 0/6] staging: iio: regulator clean-up Eva Rachel Retuya
                   ` (4 preceding siblings ...)
  2016-10-31 17:04 ` [PATCH 5/6] staging: iio: ad9832: add DVDD regulator Eva Rachel Retuya
@ 2016-10-31 17:04 ` Eva Rachel Retuya
  2016-11-05 16:23   ` Jonathan Cameron
  5 siblings, 1 reply; 18+ messages in thread
From: Eva Rachel Retuya @ 2016-10-31 17:04 UTC (permalink / raw)
  To: linux-iio, devel, linux-kernel
  Cc: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh,
	Eva Rachel Retuya

Rename regulator 'reg' to 'avdd' so as to be clear what regulator it
stands for specifically. Additionally, get rid of local variable 'reg'
and use direct assignment instead. Update also the goto label pertaining
to the avdd regulator during disable.

Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
---
 drivers/staging/iio/frequency/ad9832.c | 20 +++++++++-----------
 drivers/staging/iio/frequency/ad9832.h |  4 ++--
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index 6a5ab02..639047f 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -204,7 +204,6 @@ static int ad9832_probe(struct spi_device *spi)
 	struct ad9832_platform_data *pdata = dev_get_platdata(&spi->dev);
 	struct iio_dev *indio_dev;
 	struct ad9832_state *st;
-	struct regulator *reg;
 	int ret;
 
 	if (!pdata) {
@@ -212,11 +211,11 @@ static int ad9832_probe(struct spi_device *spi)
 		return -ENODEV;
 	}
 
-	reg = devm_regulator_get(&spi->dev, "avdd");
-	if (IS_ERR(reg))
-		return PTR_ERR(reg);
+	st->avdd = devm_regulator_get(&spi->dev, "avdd");
+	if (IS_ERR(st->avdd))
+		return PTR_ERR(st->avdd);
 
-	ret = regulator_enable(reg);
+	ret = regulator_enable(st->avdd);
 	if (ret) {
 		dev_err(&spi->dev, "Failed to enable specified AVDD supply\n");
 		return ret;
@@ -225,13 +224,13 @@ static int ad9832_probe(struct spi_device *spi)
 	st->dvdd = devm_regulator_get(&spi->dev, "dvdd");
 	if (IS_ERR(st->dvdd)) {
 		ret = PTR_ERR(st->dvdd);
-		goto error_disable_reg;
+		goto error_disable_avdd;
 	}
 
 	ret = regulator_enable(st->dvdd);
 	if (ret) {
 		dev_err(&spi->dev, "Failed to enable specified DVDD supply\n");
-		goto error_disable_reg;
+		goto error_disable_avdd;
 	}
 
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
@@ -241,7 +240,6 @@ static int ad9832_probe(struct spi_device *spi)
 	}
 	spi_set_drvdata(spi, indio_dev);
 	st = iio_priv(indio_dev);
-	st->reg = reg;
 	st->mclk = pdata->mclk;
 	st->spi = spi;
 
@@ -327,8 +325,8 @@ static int ad9832_probe(struct spi_device *spi)
 
 error_disable_dvdd:
 	regulator_disable(st->dvdd);
-error_disable_reg:
-	regulator_disable(reg);
+error_disable_avdd:
+	regulator_disable(st->avdd);
 
 	return ret;
 }
@@ -340,7 +338,7 @@ static int ad9832_remove(struct spi_device *spi)
 
 	iio_device_unregister(indio_dev);
 	regulator_disable(st->dvdd);
-	regulator_disable(st->reg);
+	regulator_disable(st->avdd);
 
 	return 0;
 }
diff --git a/drivers/staging/iio/frequency/ad9832.h b/drivers/staging/iio/frequency/ad9832.h
index eb0e7f2..1b08b04 100644
--- a/drivers/staging/iio/frequency/ad9832.h
+++ b/drivers/staging/iio/frequency/ad9832.h
@@ -58,7 +58,7 @@
 /**
  * struct ad9832_state - driver instance specific data
  * @spi:		spi_device
- * @reg:		supply regulator
+ * @avdd:		supply regulator for the analog section
  * @dvdd:		supply regulator for the digital section
  * @mclk:		external master clock
  * @ctrl_fp:		cached frequency/phase control word
@@ -77,7 +77,7 @@
 
 struct ad9832_state {
 	struct spi_device		*spi;
-	struct regulator		*reg;
+	struct regulator		*avdd;
 	struct regulator		*dvdd;
 	unsigned long			mclk;
 	unsigned short			ctrl_fp;
-- 
2.7.4

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

* Re: [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get()
  2016-10-31 17:04 ` [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get() Eva Rachel Retuya
@ 2016-11-01  4:03   ` Matt Ranostay
  2016-11-01 19:58     ` Lars-Peter Clausen
  2016-11-04 10:17     ` Eva Rachel Retuya
  2016-11-05 16:14   ` Jonathan Cameron
  1 sibling, 2 replies; 18+ messages in thread
From: Matt Ranostay @ 2016-11-01  4:03 UTC (permalink / raw)
  To: Eva Rachel Retuya
  Cc: linux-iio, devel, Linux Kernel, Lars-Peter Clausen,
	Michael.Hennerich, Jonathan Cameron, knaack.h, pmeerw, gregkh

On Mon, Oct 31, 2016 at 10:04 AM, Eva Rachel Retuya <eraretuya@gmail.com> wrote:
> The name passed to devm_regulator_get() should match the name of the
> supply as specified in the device datasheet. This makes it clear what
> power supply is being referred to in case of presence of other
> regulators.
>
> Currently, the supply name specified on the affected devices is 'vcc'.
> Use lowercase version of the datasheet name to specify the supply
> voltage.
>

Aren't we possibly breaking current device tree definitions that
people may have? We should still check the old name after the new
datasheet name in my opinion.

> Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
> ---
>  drivers/staging/iio/adc/ad7192.c                | 2 +-
>  drivers/staging/iio/adc/ad7780.c                | 2 +-
>  drivers/staging/iio/frequency/ad9832.c          | 2 +-
>  drivers/staging/iio/frequency/ad9834.c          | 2 +-
>  drivers/staging/iio/impedance-analyzer/ad5933.c | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> index bfa12ce..41fb32d 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -633,7 +633,7 @@ static int ad7192_probe(struct spi_device *spi)
>
>         st = iio_priv(indio_dev);
>
> -       st->reg = devm_regulator_get(&spi->dev, "vcc");
> +       st->reg = devm_regulator_get(&spi->dev, "avdd");
>         if (!IS_ERR(st->reg)) {
>                 ret = regulator_enable(st->reg);
>                 if (ret)
> diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c
> index c9a0c2a..a88236e 100644
> --- a/drivers/staging/iio/adc/ad7780.c
> +++ b/drivers/staging/iio/adc/ad7780.c
> @@ -173,7 +173,7 @@ static int ad7780_probe(struct spi_device *spi)
>
>         ad_sd_init(&st->sd, indio_dev, spi, &ad7780_sigma_delta_info);
>
> -       st->reg = devm_regulator_get(&spi->dev, "vcc");
> +       st->reg = devm_regulator_get(&spi->dev, "avdd");
>         if (!IS_ERR(st->reg)) {
>                 ret = regulator_enable(st->reg);
>                 if (ret)
> diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> index 358400b..744c8ee 100644
> --- a/drivers/staging/iio/frequency/ad9832.c
> +++ b/drivers/staging/iio/frequency/ad9832.c
> @@ -212,7 +212,7 @@ static int ad9832_probe(struct spi_device *spi)
>                 return -ENODEV;
>         }
>
> -       reg = devm_regulator_get(&spi->dev, "vcc");
> +       reg = devm_regulator_get(&spi->dev, "avdd");
>         if (!IS_ERR(reg)) {
>                 ret = regulator_enable(reg);
>                 if (ret)
> diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
> index 6366216..ca3cea6 100644
> --- a/drivers/staging/iio/frequency/ad9834.c
> +++ b/drivers/staging/iio/frequency/ad9834.c
> @@ -329,7 +329,7 @@ static int ad9834_probe(struct spi_device *spi)
>                 return -ENODEV;
>         }
>
> -       reg = devm_regulator_get(&spi->dev, "vcc");
> +       reg = devm_regulator_get(&spi->dev, "avdd");
>         if (!IS_ERR(reg)) {
>                 ret = regulator_enable(reg);
>                 if (ret)
> diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
> index 5eecf1c..62f61bc 100644
> --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> @@ -723,7 +723,7 @@ static int ad5933_probe(struct i2c_client *client,
>         if (!pdata)
>                 pdata = &ad5933_default_pdata;
>
> -       st->reg = devm_regulator_get(&client->dev, "vcc");
> +       st->reg = devm_regulator_get(&client->dev, "vdd");
>         if (!IS_ERR(st->reg)) {
>                 ret = regulator_enable(st->reg);
>                 if (ret)
> --
> 2.7.4
>

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

* Re: [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get()
  2016-11-01  4:03   ` Matt Ranostay
@ 2016-11-01 19:58     ` Lars-Peter Clausen
  2016-11-05 12:58       ` Jonathan Cameron
  2016-11-04 10:17     ` Eva Rachel Retuya
  1 sibling, 1 reply; 18+ messages in thread
From: Lars-Peter Clausen @ 2016-11-01 19:58 UTC (permalink / raw)
  To: Matt Ranostay, Eva Rachel Retuya
  Cc: linux-iio, devel, Linux Kernel, Michael.Hennerich,
	Jonathan Cameron, knaack.h, pmeerw, gregkh

On 11/01/2016 05:03 AM, Matt Ranostay wrote:
> On Mon, Oct 31, 2016 at 10:04 AM, Eva Rachel Retuya <eraretuya@gmail.com> wrote:
>> The name passed to devm_regulator_get() should match the name of the
>> supply as specified in the device datasheet. This makes it clear what
>> power supply is being referred to in case of presence of other
>> regulators.
>>
>> Currently, the supply name specified on the affected devices is 'vcc'.
>> Use lowercase version of the datasheet name to specify the supply
>> voltage.
>>
> 
> Aren't we possibly breaking current device tree definitions that
> people may have? We should still check the old name after the new
> datasheet name in my opinion.

None of those drivers have DT bindings, so I think we are OK. And they are
in staging anyway.

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

* Re: [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get()
  2016-11-01  4:03   ` Matt Ranostay
  2016-11-01 19:58     ` Lars-Peter Clausen
@ 2016-11-04 10:17     ` Eva Rachel Retuya
  1 sibling, 0 replies; 18+ messages in thread
From: Eva Rachel Retuya @ 2016-11-04 10:17 UTC (permalink / raw)
  To: Matt Ranostay
  Cc: linux-iio, devel, Linux Kernel, Lars-Peter Clausen,
	Michael.Hennerich, Jonathan Cameron, knaack.h, pmeerw, gregkh

Hello Matt,

On Mon, Oct 31, 2016 at 09:03:57PM -0700, Matt Ranostay wrote:
> On Mon, Oct 31, 2016 at 10:04 AM, Eva Rachel Retuya <eraretuya@gmail.com> wrote:
> > The name passed to devm_regulator_get() should match the name of the
> > supply as specified in the device datasheet. This makes it clear what
> > power supply is being referred to in case of presence of other
> > regulators.
> >
> > Currently, the supply name specified on the affected devices is 'vcc'.
> > Use lowercase version of the datasheet name to specify the supply
> > voltage.
> >
> 
> Aren't we possibly breaking current device tree definitions that
> people may have? We should still check the old name after the new
> datasheet name in my opinion.
> 

I was told it was okay to rename those IIO drivers under staging. I also
double-checked if there are available DT bindings for these drivers and
found none.

Thanks,
Eva

> > Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> > Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
> > ---
> >  drivers/staging/iio/adc/ad7192.c                | 2 +-
> >  drivers/staging/iio/adc/ad7780.c                | 2 +-
> >  drivers/staging/iio/frequency/ad9832.c          | 2 +-
> >  drivers/staging/iio/frequency/ad9834.c          | 2 +-
> >  drivers/staging/iio/impedance-analyzer/ad5933.c | 2 +-
> >  5 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> > index bfa12ce..41fb32d 100644
> > --- a/drivers/staging/iio/adc/ad7192.c
> > +++ b/drivers/staging/iio/adc/ad7192.c
> > @@ -633,7 +633,7 @@ static int ad7192_probe(struct spi_device *spi)
> >
> >         st = iio_priv(indio_dev);
> >
> > -       st->reg = devm_regulator_get(&spi->dev, "vcc");
> > +       st->reg = devm_regulator_get(&spi->dev, "avdd");
> >         if (!IS_ERR(st->reg)) {
> >                 ret = regulator_enable(st->reg);
> >                 if (ret)
> > diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c
> > index c9a0c2a..a88236e 100644
> > --- a/drivers/staging/iio/adc/ad7780.c
> > +++ b/drivers/staging/iio/adc/ad7780.c
> > @@ -173,7 +173,7 @@ static int ad7780_probe(struct spi_device *spi)
> >
> >         ad_sd_init(&st->sd, indio_dev, spi, &ad7780_sigma_delta_info);
> >
> > -       st->reg = devm_regulator_get(&spi->dev, "vcc");
> > +       st->reg = devm_regulator_get(&spi->dev, "avdd");
> >         if (!IS_ERR(st->reg)) {
> >                 ret = regulator_enable(st->reg);
> >                 if (ret)
> > diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> > index 358400b..744c8ee 100644
> > --- a/drivers/staging/iio/frequency/ad9832.c
> > +++ b/drivers/staging/iio/frequency/ad9832.c
> > @@ -212,7 +212,7 @@ static int ad9832_probe(struct spi_device *spi)
> >                 return -ENODEV;
> >         }
> >
> > -       reg = devm_regulator_get(&spi->dev, "vcc");
> > +       reg = devm_regulator_get(&spi->dev, "avdd");
> >         if (!IS_ERR(reg)) {
> >                 ret = regulator_enable(reg);
> >                 if (ret)
> > diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
> > index 6366216..ca3cea6 100644
> > --- a/drivers/staging/iio/frequency/ad9834.c
> > +++ b/drivers/staging/iio/frequency/ad9834.c
> > @@ -329,7 +329,7 @@ static int ad9834_probe(struct spi_device *spi)
> >                 return -ENODEV;
> >         }
> >
> > -       reg = devm_regulator_get(&spi->dev, "vcc");
> > +       reg = devm_regulator_get(&spi->dev, "avdd");
> >         if (!IS_ERR(reg)) {
> >                 ret = regulator_enable(reg);
> >                 if (ret)
> > diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
> > index 5eecf1c..62f61bc 100644
> > --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> > +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> > @@ -723,7 +723,7 @@ static int ad5933_probe(struct i2c_client *client,
> >         if (!pdata)
> >                 pdata = &ad5933_default_pdata;
> >
> > -       st->reg = devm_regulator_get(&client->dev, "vcc");
> > +       st->reg = devm_regulator_get(&client->dev, "vdd");
> >         if (!IS_ERR(st->reg)) {
> >                 ret = regulator_enable(st->reg);
> >                 if (ret)
> > --
> > 2.7.4
> >

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

* Re: [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get()
  2016-11-01 19:58     ` Lars-Peter Clausen
@ 2016-11-05 12:58       ` Jonathan Cameron
  2016-11-05 16:14         ` Jonathan Cameron
  0 siblings, 1 reply; 18+ messages in thread
From: Jonathan Cameron @ 2016-11-05 12:58 UTC (permalink / raw)
  To: Lars-Peter Clausen, Matt Ranostay, Eva Rachel Retuya
  Cc: linux-iio, devel, Linux Kernel, Michael.Hennerich, knaack.h,
	pmeerw, gregkh

On 01/11/16 19:58, Lars-Peter Clausen wrote:
> On 11/01/2016 05:03 AM, Matt Ranostay wrote:
>> On Mon, Oct 31, 2016 at 10:04 AM, Eva Rachel Retuya <eraretuya@gmail.com> wrote:
>>> The name passed to devm_regulator_get() should match the name of the
>>> supply as specified in the device datasheet. This makes it clear what
>>> power supply is being referred to in case of presence of other
>>> regulators.
>>>
>>> Currently, the supply name specified on the affected devices is 'vcc'.
>>> Use lowercase version of the datasheet name to specify the supply
>>> voltage.
>>>
>>
>> Aren't we possibly breaking current device tree definitions that
>> people may have? We should still check the old name after the new
>> datasheet name in my opinion.
> 
> None of those drivers have DT bindings, so I think we are OK. And they are
> in staging anyway.
> 
I agree on this.  These are technically in kernel interfaces so we
are fine to change them.  Would have been more interesting if
there were DT bindings however and we would have had to support
the old and new naming for a while at least (i.e. probably forever
as we'd never get around to cleaning it up!)

Jonathan

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

* Re: [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get()
  2016-11-05 12:58       ` Jonathan Cameron
@ 2016-11-05 16:14         ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2016-11-05 16:14 UTC (permalink / raw)
  To: Lars-Peter Clausen, Matt Ranostay, Eva Rachel Retuya
  Cc: linux-iio, devel, Linux Kernel, Michael.Hennerich, knaack.h,
	pmeerw, gregkh

On 05/11/16 12:58, Jonathan Cameron wrote:
> On 01/11/16 19:58, Lars-Peter Clausen wrote:
>> On 11/01/2016 05:03 AM, Matt Ranostay wrote:
>>> On Mon, Oct 31, 2016 at 10:04 AM, Eva Rachel Retuya <eraretuya@gmail.com> wrote:
>>>> The name passed to devm_regulator_get() should match the name of the
>>>> supply as specified in the device datasheet. This makes it clear what
>>>> power supply is being referred to in case of presence of other
>>>> regulators.
>>>>
>>>> Currently, the supply name specified on the affected devices is 'vcc'.
>>>> Use lowercase version of the datasheet name to specify the supply
>>>> voltage.
>>>>
>>>
>>> Aren't we possibly breaking current device tree definitions that
>>> people may have? We should still check the old name after the new
>>> datasheet name in my opinion.
>>
>> None of those drivers have DT bindings, so I think we are OK. And they are
>> in staging anyway.
>>
> I agree on this.  These are technically in kernel interfaces so we
> are fine to change them.  Would have been more interesting if
> there were DT bindings however and we would have had to support
> the old and new naming for a while at least (i.e. probably forever
> as we'd never get around to cleaning it up!)
Of course, the old 'magic matching' that i2c and spi do which could allow
these devices to be instantiated with regulators from the device tree
makes it just possible someone is doing this.

We'll take the good kernel approach of perhaps considering a adding
in compatibility support for the old option if someone screams.

We are find for some of them though as they NEED platform data to probe
anyway which device tree obviously can't magically supply.

The ad7780 is the only risky one I think.

Jonathan
> 
> Jonathan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

* Re: [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get()
  2016-10-31 17:04 ` [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get() Eva Rachel Retuya
  2016-11-01  4:03   ` Matt Ranostay
@ 2016-11-05 16:14   ` Jonathan Cameron
  1 sibling, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2016-11-05 16:14 UTC (permalink / raw)
  To: Eva Rachel Retuya, linux-iio, devel, linux-kernel
  Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh

On 31/10/16 17:04, Eva Rachel Retuya wrote:
> The name passed to devm_regulator_get() should match the name of the
> supply as specified in the device datasheet. This makes it clear what
> power supply is being referred to in case of presence of other
> regulators.
> 
> Currently, the supply name specified on the affected devices is 'vcc'.
> Use lowercase version of the datasheet name to specify the supply
> voltage.
> 
> Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
Applied to the togreg branch of iio.git which will be initially pushed
out as testing for the autobuilders to play with them.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/adc/ad7192.c                | 2 +-
>  drivers/staging/iio/adc/ad7780.c                | 2 +-
>  drivers/staging/iio/frequency/ad9832.c          | 2 +-
>  drivers/staging/iio/frequency/ad9834.c          | 2 +-
>  drivers/staging/iio/impedance-analyzer/ad5933.c | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> index bfa12ce..41fb32d 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -633,7 +633,7 @@ static int ad7192_probe(struct spi_device *spi)
>  
>  	st = iio_priv(indio_dev);
>  
> -	st->reg = devm_regulator_get(&spi->dev, "vcc");
> +	st->reg = devm_regulator_get(&spi->dev, "avdd");
>  	if (!IS_ERR(st->reg)) {
>  		ret = regulator_enable(st->reg);
>  		if (ret)
> diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c
> index c9a0c2a..a88236e 100644
> --- a/drivers/staging/iio/adc/ad7780.c
> +++ b/drivers/staging/iio/adc/ad7780.c
> @@ -173,7 +173,7 @@ static int ad7780_probe(struct spi_device *spi)
>  
>  	ad_sd_init(&st->sd, indio_dev, spi, &ad7780_sigma_delta_info);
>  
> -	st->reg = devm_regulator_get(&spi->dev, "vcc");
> +	st->reg = devm_regulator_get(&spi->dev, "avdd");
>  	if (!IS_ERR(st->reg)) {
>  		ret = regulator_enable(st->reg);
>  		if (ret)
> diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> index 358400b..744c8ee 100644
> --- a/drivers/staging/iio/frequency/ad9832.c
> +++ b/drivers/staging/iio/frequency/ad9832.c
> @@ -212,7 +212,7 @@ static int ad9832_probe(struct spi_device *spi)
>  		return -ENODEV;
>  	}
>  
> -	reg = devm_regulator_get(&spi->dev, "vcc");
> +	reg = devm_regulator_get(&spi->dev, "avdd");
>  	if (!IS_ERR(reg)) {
>  		ret = regulator_enable(reg);
>  		if (ret)
> diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
> index 6366216..ca3cea6 100644
> --- a/drivers/staging/iio/frequency/ad9834.c
> +++ b/drivers/staging/iio/frequency/ad9834.c
> @@ -329,7 +329,7 @@ static int ad9834_probe(struct spi_device *spi)
>  		return -ENODEV;
>  	}
>  
> -	reg = devm_regulator_get(&spi->dev, "vcc");
> +	reg = devm_regulator_get(&spi->dev, "avdd");
>  	if (!IS_ERR(reg)) {
>  		ret = regulator_enable(reg);
>  		if (ret)
> diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
> index 5eecf1c..62f61bc 100644
> --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> @@ -723,7 +723,7 @@ static int ad5933_probe(struct i2c_client *client,
>  	if (!pdata)
>  		pdata = &ad5933_default_pdata;
>  
> -	st->reg = devm_regulator_get(&client->dev, "vcc");
> +	st->reg = devm_regulator_get(&client->dev, "vdd");
>  	if (!IS_ERR(st->reg)) {
>  		ret = regulator_enable(st->reg);
>  		if (ret)
> 

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

* Re: [PATCH 2/6] staging: iio: rework regulator handling
  2016-10-31 17:04 ` [PATCH 2/6] staging: iio: rework regulator handling Eva Rachel Retuya
@ 2016-11-05 16:18   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2016-11-05 16:18 UTC (permalink / raw)
  To: Eva Rachel Retuya, linux-iio, devel, linux-kernel
  Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh

On 31/10/16 17:04, Eva Rachel Retuya wrote:
> Currently, the affected drivers ignore all errors from regulator_get().
> The way it is now, it also breaks probe deferral (EPROBE_DEFER). The
> correct behavior is to propagate the error to the upper layers so they
> can handle it accordingly.
> 
> Rework the regulator handling so that it matches the standard behavior.
> If the specific design uses a static always-on regulator and does not
> explicitly specify it, regulator_get() will return the dummy regulator.
> 
> The following semantic patch was used to apply the change:
> @r1@
> expression reg, dev, en, volt;
> @@
> 
> reg = \(devm_regulator_get\|regulator_get\)(dev, ...);
> if (
> - !
>    IS_ERR(reg))
> +	return PTR_ERR(reg);
> (
> - {	en = regulator_enable(reg);
> -	if (en) return en; }
> +
> +	en = regulator_enable(reg);
> +	if (en) {
> +		dev_err(dev, "Failed to enable specified supply\n");
> +		return en; }
> |
> +
> - {	en = regulator_enable(reg);
> -	if (en)	return en;
> -	volt = regulator_get_voltage(reg); }
> +	en = regulator_enable(reg);
> +	if (en) {
> +		dev_err(dev, "Failed to enable specified supply\n");
> +		return en;
> +	}
> +	volt = regulator_get_voltage(reg);
> )
> 
> @r2@
> expression arg;
> @@
> 
> -	if (!IS_ERR(arg)) regulator_disable(arg);
> +	regulator_disable(arg);
> 
> Hand-edit the debugging prints with the supply name to become more
> specific.
> 
> Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
Nice patch.

Applied to the togreg branch of iio.git and will be pushed out as
testing for the autobuilders to play with them.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/adc/ad7192.c                | 18 +++++++++---------
>  drivers/staging/iio/adc/ad7780.c                | 18 +++++++++---------
>  drivers/staging/iio/frequency/ad9832.c          | 17 +++++++++--------
>  drivers/staging/iio/frequency/ad9834.c          | 17 +++++++++--------
>  drivers/staging/iio/impedance-analyzer/ad5933.c | 19 ++++++++++---------
>  5 files changed, 46 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> index 41fb32d..29e32b7 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -634,13 +634,15 @@ static int ad7192_probe(struct spi_device *spi)
>  	st = iio_priv(indio_dev);
>  
>  	st->reg = devm_regulator_get(&spi->dev, "avdd");
> -	if (!IS_ERR(st->reg)) {
> -		ret = regulator_enable(st->reg);
> -		if (ret)
> -			return ret;
> +	if (IS_ERR(st->reg))
> +		return PTR_ERR(st->reg);
>  
> -		voltage_uv = regulator_get_voltage(st->reg);
> +	ret = regulator_enable(st->reg);
> +	if (ret) {
> +		dev_err(&spi->dev, "Failed to enable specified AVdd supply\n");
> +		return ret;
>  	}
> +	voltage_uv = regulator_get_voltage(st->reg);
>  
>  	if (pdata->vref_mv)
>  		st->int_vref_mv = pdata->vref_mv;
> @@ -689,8 +691,7 @@ static int ad7192_probe(struct spi_device *spi)
>  error_remove_trigger:
>  	ad_sd_cleanup_buffer_and_trigger(indio_dev);
>  error_disable_reg:
> -	if (!IS_ERR(st->reg))
> -		regulator_disable(st->reg);
> +	regulator_disable(st->reg);
>  
>  	return ret;
>  }
> @@ -703,8 +704,7 @@ static int ad7192_remove(struct spi_device *spi)
>  	iio_device_unregister(indio_dev);
>  	ad_sd_cleanup_buffer_and_trigger(indio_dev);
>  
> -	if (!IS_ERR(st->reg))
> -		regulator_disable(st->reg);
> +	regulator_disable(st->reg);
>  
>  	return 0;
>  }
> diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c
> index a88236e..e149600 100644
> --- a/drivers/staging/iio/adc/ad7780.c
> +++ b/drivers/staging/iio/adc/ad7780.c
> @@ -174,13 +174,15 @@ static int ad7780_probe(struct spi_device *spi)
>  	ad_sd_init(&st->sd, indio_dev, spi, &ad7780_sigma_delta_info);
>  
>  	st->reg = devm_regulator_get(&spi->dev, "avdd");
> -	if (!IS_ERR(st->reg)) {
> -		ret = regulator_enable(st->reg);
> -		if (ret)
> -			return ret;
> +	if (IS_ERR(st->reg))
> +		return PTR_ERR(st->reg);
>  
> -		voltage_uv = regulator_get_voltage(st->reg);
> +	ret = regulator_enable(st->reg);
> +	if (ret) {
> +		dev_err(&spi->dev, "Failed to enable specified AVdd supply\n");
> +		return ret;
>  	}
> +	voltage_uv = regulator_get_voltage(st->reg);
>  
>  	st->chip_info =
>  		&ad7780_chip_info_tbl[spi_get_device_id(spi)->driver_data];
> @@ -222,8 +224,7 @@ static int ad7780_probe(struct spi_device *spi)
>  error_cleanup_buffer_and_trigger:
>  	ad_sd_cleanup_buffer_and_trigger(indio_dev);
>  error_disable_reg:
> -	if (!IS_ERR(st->reg))
> -		regulator_disable(st->reg);
> +	regulator_disable(st->reg);
>  
>  	return ret;
>  }
> @@ -236,8 +237,7 @@ static int ad7780_remove(struct spi_device *spi)
>  	iio_device_unregister(indio_dev);
>  	ad_sd_cleanup_buffer_and_trigger(indio_dev);
>  
> -	if (!IS_ERR(st->reg))
> -		regulator_disable(st->reg);
> +	regulator_disable(st->reg);
>  
>  	return 0;
>  }
> diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> index 744c8ee..7d8dc6c 100644
> --- a/drivers/staging/iio/frequency/ad9832.c
> +++ b/drivers/staging/iio/frequency/ad9832.c
> @@ -213,10 +213,13 @@ static int ad9832_probe(struct spi_device *spi)
>  	}
>  
>  	reg = devm_regulator_get(&spi->dev, "avdd");
> -	if (!IS_ERR(reg)) {
> -		ret = regulator_enable(reg);
> -		if (ret)
> -			return ret;
> +	if (IS_ERR(reg))
> +		return PTR_ERR(reg);
> +
> +	ret = regulator_enable(reg);
> +	if (ret) {
> +		dev_err(&spi->dev, "Failed to enable specified AVDD supply\n");
> +		return ret;
>  	}
>  
>  	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> @@ -311,8 +314,7 @@ static int ad9832_probe(struct spi_device *spi)
>  	return 0;
>  
>  error_disable_reg:
> -	if (!IS_ERR(reg))
> -		regulator_disable(reg);
> +	regulator_disable(reg);
>  
>  	return ret;
>  }
> @@ -323,8 +325,7 @@ static int ad9832_remove(struct spi_device *spi)
>  	struct ad9832_state *st = iio_priv(indio_dev);
>  
>  	iio_device_unregister(indio_dev);
> -	if (!IS_ERR(st->reg))
> -		regulator_disable(st->reg);
> +	regulator_disable(st->reg);
>  
>  	return 0;
>  }
> diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
> index ca3cea6..19216af 100644
> --- a/drivers/staging/iio/frequency/ad9834.c
> +++ b/drivers/staging/iio/frequency/ad9834.c
> @@ -330,10 +330,13 @@ static int ad9834_probe(struct spi_device *spi)
>  	}
>  
>  	reg = devm_regulator_get(&spi->dev, "avdd");
> -	if (!IS_ERR(reg)) {
> -		ret = regulator_enable(reg);
> -		if (ret)
> -			return ret;
> +	if (IS_ERR(reg))
> +		return PTR_ERR(reg);
> +
> +	ret = regulator_enable(reg);
> +	if (ret) {
> +		dev_err(&spi->dev, "Failed to enable specified AVDD supply\n");
> +		return ret;
>  	}
>  
>  	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> @@ -416,8 +419,7 @@ static int ad9834_probe(struct spi_device *spi)
>  	return 0;
>  
>  error_disable_reg:
> -	if (!IS_ERR(reg))
> -		regulator_disable(reg);
> +	regulator_disable(reg);
>  
>  	return ret;
>  }
> @@ -428,8 +430,7 @@ static int ad9834_remove(struct spi_device *spi)
>  	struct ad9834_state *st = iio_priv(indio_dev);
>  
>  	iio_device_unregister(indio_dev);
> -	if (!IS_ERR(st->reg))
> -		regulator_disable(st->reg);
> +	regulator_disable(st->reg);
>  
>  	return 0;
>  }
> diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
> index 62f61bc..8f2736a 100644
> --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> @@ -724,12 +724,15 @@ static int ad5933_probe(struct i2c_client *client,
>  		pdata = &ad5933_default_pdata;
>  
>  	st->reg = devm_regulator_get(&client->dev, "vdd");
> -	if (!IS_ERR(st->reg)) {
> -		ret = regulator_enable(st->reg);
> -		if (ret)
> -			return ret;
> -		voltage_uv = regulator_get_voltage(st->reg);
> +	if (IS_ERR(st->reg))
> +		return PTR_ERR(st->reg);
> +
> +	ret = regulator_enable(st->reg);
> +	if (ret) {
> +		dev_err(&client->dev, "Failed to enable specified VDD supply\n");
> +		return ret;
>  	}
> +	voltage_uv = regulator_get_voltage(st->reg);
>  
>  	if (voltage_uv)
>  		st->vref_mv = voltage_uv / 1000;
> @@ -772,8 +775,7 @@ static int ad5933_probe(struct i2c_client *client,
>  error_unreg_ring:
>  	iio_kfifo_free(indio_dev->buffer);
>  error_disable_reg:
> -	if (!IS_ERR(st->reg))
> -		regulator_disable(st->reg);
> +	regulator_disable(st->reg);
>  
>  	return ret;
>  }
> @@ -785,8 +787,7 @@ static int ad5933_remove(struct i2c_client *client)
>  
>  	iio_device_unregister(indio_dev);
>  	iio_kfifo_free(indio_dev->buffer);
> -	if (!IS_ERR(st->reg))
> -		regulator_disable(st->reg);
> +	regulator_disable(st->reg);
>  
>  	return 0;
>  }
> 

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

* Re: [PATCH 3/6] staging: iio: ad7192: add DVdd regulator
  2016-10-31 17:04 ` [PATCH 3/6] staging: iio: ad7192: add DVdd regulator Eva Rachel Retuya
@ 2016-11-05 16:19   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2016-11-05 16:19 UTC (permalink / raw)
  To: Eva Rachel Retuya, linux-iio, devel, linux-kernel
  Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh

On 31/10/16 17:04, Eva Rachel Retuya wrote:
> The AD7190/AD7192/AD7193/AD7195 is supplied with two power sources:
> AVdd as analog supply voltage and DVdd as digital supply voltage.
> 
> Attempt to fetch and enable the regulator 'dvdd'. Bail out if any error
> occurs.
> 
> Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
Looks good.  Applied to the togreg branch of iio.git etc etc.

Jonathan
> ---
>  drivers/staging/iio/adc/ad7192.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> index 29e32b7..3f8dc66 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -153,6 +153,7 @@
>  
>  struct ad7192_state {
>  	struct regulator		*reg;
> +	struct regulator		*dvdd;
>  	u16				int_vref_mv;
>  	u32				mclk;
>  	u32				f_order;
> @@ -642,6 +643,19 @@ static int ad7192_probe(struct spi_device *spi)
>  		dev_err(&spi->dev, "Failed to enable specified AVdd supply\n");
>  		return ret;
>  	}
> +
> +	st->dvdd = devm_regulator_get(&spi->dev, "dvdd");
> +	if (IS_ERR(st->dvdd)) {
> +		ret = PTR_ERR(st->dvdd);
> +		goto error_disable_reg;
> +	}
> +
> +	ret = regulator_enable(st->dvdd);
> +	if (ret) {
> +		dev_err(&spi->dev, "Failed to enable specified DVdd supply\n");
> +		goto error_disable_reg;
> +	}
> +
>  	voltage_uv = regulator_get_voltage(st->reg);
>  
>  	if (pdata->vref_mv)
> @@ -677,7 +691,7 @@ static int ad7192_probe(struct spi_device *spi)
>  
>  	ret = ad_sd_setup_buffer_and_trigger(indio_dev);
>  	if (ret)
> -		goto error_disable_reg;
> +		goto error_disable_dvdd;
>  
>  	ret = ad7192_setup(st, pdata);
>  	if (ret)
> @@ -690,6 +704,8 @@ static int ad7192_probe(struct spi_device *spi)
>  
>  error_remove_trigger:
>  	ad_sd_cleanup_buffer_and_trigger(indio_dev);
> +error_disable_dvdd:
> +	regulator_disable(st->dvdd);
>  error_disable_reg:
>  	regulator_disable(st->reg);
>  
> @@ -704,6 +720,7 @@ static int ad7192_remove(struct spi_device *spi)
>  	iio_device_unregister(indio_dev);
>  	ad_sd_cleanup_buffer_and_trigger(indio_dev);
>  
> +	regulator_disable(st->dvdd);
>  	regulator_disable(st->reg);
>  
>  	return 0;
> 

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

* Re: [PATCH 4/6] staging: iio: ad7192: rename regulator 'reg' to 'avdd'
  2016-10-31 17:04 ` [PATCH 4/6] staging: iio: ad7192: rename regulator 'reg' to 'avdd' Eva Rachel Retuya
@ 2016-11-05 16:21   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2016-11-05 16:21 UTC (permalink / raw)
  To: Eva Rachel Retuya, linux-iio, devel, linux-kernel
  Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh

On 31/10/16 17:04, Eva Rachel Retuya wrote:
> Rename regulator 'reg' to 'avdd' so as to be clear what regulator it
> stands for specifically. Also, update the goto label accordingly.
> 
> Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
Applied.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/adc/ad7192.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> index 3f8dc66..1fb68c0 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -152,7 +152,7 @@
>   */
>  
>  struct ad7192_state {
> -	struct regulator		*reg;
> +	struct regulator		*avdd;
>  	struct regulator		*dvdd;
>  	u16				int_vref_mv;
>  	u32				mclk;
> @@ -634,11 +634,11 @@ static int ad7192_probe(struct spi_device *spi)
>  
>  	st = iio_priv(indio_dev);
>  
> -	st->reg = devm_regulator_get(&spi->dev, "avdd");
> -	if (IS_ERR(st->reg))
> -		return PTR_ERR(st->reg);
> +	st->avdd = devm_regulator_get(&spi->dev, "avdd");
> +	if (IS_ERR(st->avdd))
> +		return PTR_ERR(st->avdd);
>  
> -	ret = regulator_enable(st->reg);
> +	ret = regulator_enable(st->avdd);
>  	if (ret) {
>  		dev_err(&spi->dev, "Failed to enable specified AVdd supply\n");
>  		return ret;
> @@ -647,16 +647,16 @@ static int ad7192_probe(struct spi_device *spi)
>  	st->dvdd = devm_regulator_get(&spi->dev, "dvdd");
>  	if (IS_ERR(st->dvdd)) {
>  		ret = PTR_ERR(st->dvdd);
> -		goto error_disable_reg;
> +		goto error_disable_avdd;
>  	}
>  
>  	ret = regulator_enable(st->dvdd);
>  	if (ret) {
>  		dev_err(&spi->dev, "Failed to enable specified DVdd supply\n");
> -		goto error_disable_reg;
> +		goto error_disable_avdd;
>  	}
>  
> -	voltage_uv = regulator_get_voltage(st->reg);
> +	voltage_uv = regulator_get_voltage(st->avdd);
>  
>  	if (pdata->vref_mv)
>  		st->int_vref_mv = pdata->vref_mv;
> @@ -706,8 +706,8 @@ static int ad7192_probe(struct spi_device *spi)
>  	ad_sd_cleanup_buffer_and_trigger(indio_dev);
>  error_disable_dvdd:
>  	regulator_disable(st->dvdd);
> -error_disable_reg:
> -	regulator_disable(st->reg);
> +error_disable_avdd:
> +	regulator_disable(st->avdd);
>  
>  	return ret;
>  }
> @@ -721,7 +721,7 @@ static int ad7192_remove(struct spi_device *spi)
>  	ad_sd_cleanup_buffer_and_trigger(indio_dev);
>  
>  	regulator_disable(st->dvdd);
> -	regulator_disable(st->reg);
> +	regulator_disable(st->avdd);
>  
>  	return 0;
>  }
> 

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

* Re: [PATCH 5/6] staging: iio: ad9832: add DVDD regulator
  2016-10-31 17:04 ` [PATCH 5/6] staging: iio: ad9832: add DVDD regulator Eva Rachel Retuya
@ 2016-11-05 16:22   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2016-11-05 16:22 UTC (permalink / raw)
  To: Eva Rachel Retuya, linux-iio, devel, linux-kernel
  Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh

On 31/10/16 17:04, Eva Rachel Retuya wrote:
> The AD9832/AD9835 is supplied with two power sources: AVDD as analog
> supply voltage and DVDD as digital supply voltage.
> 
> Attempt to fetch and enable the regulator 'dvdd'. Bail out if any error
> occurs.
> 
> Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
Applied. 

Thanks,

Jonathan
> ---
>  drivers/staging/iio/frequency/ad9832.c | 33 ++++++++++++++++++++++++---------
>  drivers/staging/iio/frequency/ad9832.h |  2 ++
>  2 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> index 7d8dc6c..6a5ab02 100644
> --- a/drivers/staging/iio/frequency/ad9832.c
> +++ b/drivers/staging/iio/frequency/ad9832.c
> @@ -222,10 +222,22 @@ static int ad9832_probe(struct spi_device *spi)
>  		return ret;
>  	}
>  
> +	st->dvdd = devm_regulator_get(&spi->dev, "dvdd");
> +	if (IS_ERR(st->dvdd)) {
> +		ret = PTR_ERR(st->dvdd);
> +		goto error_disable_reg;
> +	}
> +
> +	ret = regulator_enable(st->dvdd);
> +	if (ret) {
> +		dev_err(&spi->dev, "Failed to enable specified DVDD supply\n");
> +		goto error_disable_reg;
> +	}
> +
>  	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
>  	if (!indio_dev) {
>  		ret = -ENOMEM;
> -		goto error_disable_reg;
> +		goto error_disable_dvdd;
>  	}
>  	spi_set_drvdata(spi, indio_dev);
>  	st = iio_priv(indio_dev);
> @@ -280,39 +292,41 @@ static int ad9832_probe(struct spi_device *spi)
>  	ret = spi_sync(st->spi, &st->msg);
>  	if (ret) {
>  		dev_err(&spi->dev, "device init failed\n");
> -		goto error_disable_reg;
> +		goto error_disable_dvdd;
>  	}
>  
>  	ret = ad9832_write_frequency(st, AD9832_FREQ0HM, pdata->freq0);
>  	if (ret)
> -		goto error_disable_reg;
> +		goto error_disable_dvdd;
>  
>  	ret = ad9832_write_frequency(st, AD9832_FREQ1HM, pdata->freq1);
>  	if (ret)
> -		goto error_disable_reg;
> +		goto error_disable_dvdd;
>  
>  	ret = ad9832_write_phase(st, AD9832_PHASE0H, pdata->phase0);
>  	if (ret)
> -		goto error_disable_reg;
> +		goto error_disable_dvdd;
>  
>  	ret = ad9832_write_phase(st, AD9832_PHASE1H, pdata->phase1);
>  	if (ret)
> -		goto error_disable_reg;
> +		goto error_disable_dvdd;
>  
>  	ret = ad9832_write_phase(st, AD9832_PHASE2H, pdata->phase2);
>  	if (ret)
> -		goto error_disable_reg;
> +		goto error_disable_dvdd;
>  
>  	ret = ad9832_write_phase(st, AD9832_PHASE3H, pdata->phase3);
>  	if (ret)
> -		goto error_disable_reg;
> +		goto error_disable_dvdd;
>  
>  	ret = iio_device_register(indio_dev);
>  	if (ret)
> -		goto error_disable_reg;
> +		goto error_disable_dvdd;
>  
>  	return 0;
>  
> +error_disable_dvdd:
> +	regulator_disable(st->dvdd);
>  error_disable_reg:
>  	regulator_disable(reg);
>  
> @@ -325,6 +339,7 @@ static int ad9832_remove(struct spi_device *spi)
>  	struct ad9832_state *st = iio_priv(indio_dev);
>  
>  	iio_device_unregister(indio_dev);
> +	regulator_disable(st->dvdd);
>  	regulator_disable(st->reg);
>  
>  	return 0;
> diff --git a/drivers/staging/iio/frequency/ad9832.h b/drivers/staging/iio/frequency/ad9832.h
> index d32323b..eb0e7f2 100644
> --- a/drivers/staging/iio/frequency/ad9832.h
> +++ b/drivers/staging/iio/frequency/ad9832.h
> @@ -59,6 +59,7 @@
>   * struct ad9832_state - driver instance specific data
>   * @spi:		spi_device
>   * @reg:		supply regulator
> + * @dvdd:		supply regulator for the digital section
>   * @mclk:		external master clock
>   * @ctrl_fp:		cached frequency/phase control word
>   * @ctrl_ss:		cached sync/selsrc control word
> @@ -77,6 +78,7 @@
>  struct ad9832_state {
>  	struct spi_device		*spi;
>  	struct regulator		*reg;
> +	struct regulator		*dvdd;
>  	unsigned long			mclk;
>  	unsigned short			ctrl_fp;
>  	unsigned short			ctrl_ss;
> 

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

* Re: [PATCH 6/6] staging: iio: ad9832: clean-up regulator 'reg'
  2016-10-31 17:04 ` [PATCH 6/6] staging: iio: ad9832: clean-up regulator 'reg' Eva Rachel Retuya
@ 2016-11-05 16:23   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2016-11-05 16:23 UTC (permalink / raw)
  To: Eva Rachel Retuya, linux-iio, devel, linux-kernel
  Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh

On 31/10/16 17:04, Eva Rachel Retuya wrote:
> Rename regulator 'reg' to 'avdd' so as to be clear what regulator it
> stands for specifically. Additionally, get rid of local variable 'reg'
> and use direct assignment instead. Update also the goto label pertaining
> to the avdd regulator during disable.
> 
> Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
Applied to the togreg branch of iio.git and shortly pushed out as
testing for the autobuilders to play with it.

A nice little series, thanks!

Jonathan
> ---
>  drivers/staging/iio/frequency/ad9832.c | 20 +++++++++-----------
>  drivers/staging/iio/frequency/ad9832.h |  4 ++--
>  2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> index 6a5ab02..639047f 100644
> --- a/drivers/staging/iio/frequency/ad9832.c
> +++ b/drivers/staging/iio/frequency/ad9832.c
> @@ -204,7 +204,6 @@ static int ad9832_probe(struct spi_device *spi)
>  	struct ad9832_platform_data *pdata = dev_get_platdata(&spi->dev);
>  	struct iio_dev *indio_dev;
>  	struct ad9832_state *st;
> -	struct regulator *reg;
>  	int ret;
>  
>  	if (!pdata) {
> @@ -212,11 +211,11 @@ static int ad9832_probe(struct spi_device *spi)
>  		return -ENODEV;
>  	}
>  
> -	reg = devm_regulator_get(&spi->dev, "avdd");
> -	if (IS_ERR(reg))
> -		return PTR_ERR(reg);
> +	st->avdd = devm_regulator_get(&spi->dev, "avdd");
> +	if (IS_ERR(st->avdd))
> +		return PTR_ERR(st->avdd);
>  
> -	ret = regulator_enable(reg);
> +	ret = regulator_enable(st->avdd);
>  	if (ret) {
>  		dev_err(&spi->dev, "Failed to enable specified AVDD supply\n");
>  		return ret;
> @@ -225,13 +224,13 @@ static int ad9832_probe(struct spi_device *spi)
>  	st->dvdd = devm_regulator_get(&spi->dev, "dvdd");
>  	if (IS_ERR(st->dvdd)) {
>  		ret = PTR_ERR(st->dvdd);
> -		goto error_disable_reg;
> +		goto error_disable_avdd;
>  	}
>  
>  	ret = regulator_enable(st->dvdd);
>  	if (ret) {
>  		dev_err(&spi->dev, "Failed to enable specified DVDD supply\n");
> -		goto error_disable_reg;
> +		goto error_disable_avdd;
>  	}
>  
>  	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> @@ -241,7 +240,6 @@ static int ad9832_probe(struct spi_device *spi)
>  	}
>  	spi_set_drvdata(spi, indio_dev);
>  	st = iio_priv(indio_dev);
> -	st->reg = reg;
>  	st->mclk = pdata->mclk;
>  	st->spi = spi;
>  
> @@ -327,8 +325,8 @@ static int ad9832_probe(struct spi_device *spi)
>  
>  error_disable_dvdd:
>  	regulator_disable(st->dvdd);
> -error_disable_reg:
> -	regulator_disable(reg);
> +error_disable_avdd:
> +	regulator_disable(st->avdd);
>  
>  	return ret;
>  }
> @@ -340,7 +338,7 @@ static int ad9832_remove(struct spi_device *spi)
>  
>  	iio_device_unregister(indio_dev);
>  	regulator_disable(st->dvdd);
> -	regulator_disable(st->reg);
> +	regulator_disable(st->avdd);
>  
>  	return 0;
>  }
> diff --git a/drivers/staging/iio/frequency/ad9832.h b/drivers/staging/iio/frequency/ad9832.h
> index eb0e7f2..1b08b04 100644
> --- a/drivers/staging/iio/frequency/ad9832.h
> +++ b/drivers/staging/iio/frequency/ad9832.h
> @@ -58,7 +58,7 @@
>  /**
>   * struct ad9832_state - driver instance specific data
>   * @spi:		spi_device
> - * @reg:		supply regulator
> + * @avdd:		supply regulator for the analog section
>   * @dvdd:		supply regulator for the digital section
>   * @mclk:		external master clock
>   * @ctrl_fp:		cached frequency/phase control word
> @@ -77,7 +77,7 @@
>  
>  struct ad9832_state {
>  	struct spi_device		*spi;
> -	struct regulator		*reg;
> +	struct regulator		*avdd;
>  	struct regulator		*dvdd;
>  	unsigned long			mclk;
>  	unsigned short			ctrl_fp;
> 

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

end of thread, other threads:[~2016-11-05 16:23 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-31 17:04 [PATCH 0/6] staging: iio: regulator clean-up Eva Rachel Retuya
2016-10-31 17:04 ` [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get() Eva Rachel Retuya
2016-11-01  4:03   ` Matt Ranostay
2016-11-01 19:58     ` Lars-Peter Clausen
2016-11-05 12:58       ` Jonathan Cameron
2016-11-05 16:14         ` Jonathan Cameron
2016-11-04 10:17     ` Eva Rachel Retuya
2016-11-05 16:14   ` Jonathan Cameron
2016-10-31 17:04 ` [PATCH 2/6] staging: iio: rework regulator handling Eva Rachel Retuya
2016-11-05 16:18   ` Jonathan Cameron
2016-10-31 17:04 ` [PATCH 3/6] staging: iio: ad7192: add DVdd regulator Eva Rachel Retuya
2016-11-05 16:19   ` Jonathan Cameron
2016-10-31 17:04 ` [PATCH 4/6] staging: iio: ad7192: rename regulator 'reg' to 'avdd' Eva Rachel Retuya
2016-11-05 16:21   ` Jonathan Cameron
2016-10-31 17:04 ` [PATCH 5/6] staging: iio: ad9832: add DVDD regulator Eva Rachel Retuya
2016-11-05 16:22   ` Jonathan Cameron
2016-10-31 17:04 ` [PATCH 6/6] staging: iio: ad9832: clean-up regulator 'reg' Eva Rachel Retuya
2016-11-05 16:23   ` 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.