Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1512727 > unrolled thread

[PATCH 0/6] staging: iio: regulator clean-up

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/6] staging: iio: regulator clean-up Eva Rachel Retuya <eraretuya@gmail.com> - 2016-10-31 18:10 +0100
    [PATCH 4/6] staging: iio: ad7192: rename regulator 'reg' to 'avdd' Eva Rachel Retuya <eraretuya@gmail.com> - 2016-10-31 18:10 +0100
      Re: [PATCH 4/6] staging: iio: ad7192: rename regulator 'reg' to  'avdd' Jonathan Cameron <jic23@kernel.org> - 2016-11-05 17:30 +0100
    [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get() Eva Rachel Retuya <eraretuya@gmail.com> - 2016-10-31 18:10 +0100
      Re: [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get() Matt Ranostay <matt@ranostay.consulting> - 2016-11-01 05:10 +0100
        Re: [PATCH 1/6] staging: iio: set proper supply name to  devm_regulator_get() Lars-Peter Clausen <lars@metafoo.de> - 2016-11-01 21:00 +0100
          Re: [PATCH 1/6] staging: iio: set proper supply name to  devm_regulator_get() Jonathan Cameron <jic23@kernel.org> - 2016-11-05 14:00 +0100
            Re: [PATCH 1/6] staging: iio: set proper supply name to  devm_regulator_get() Jonathan Cameron <jic23@kernel.org> - 2016-11-05 17:20 +0100
        Re: [PATCH 1/6] staging: iio: set proper supply name to  devm_regulator_get() Eva Rachel Retuya <eraretuya@gmail.com> - 2016-11-04 11:20 +0100
      Re: [PATCH 1/6] staging: iio: set proper supply name to  devm_regulator_get() Jonathan Cameron <jic23@kernel.org> - 2016-11-05 17:20 +0100
    [PATCH 6/6] staging: iio: ad9832: clean-up regulator 'reg' Eva Rachel Retuya <eraretuya@gmail.com> - 2016-10-31 18:10 +0100
      Re: [PATCH 6/6] staging: iio: ad9832: clean-up regulator 'reg' Jonathan Cameron <jic23@kernel.org> - 2016-11-05 17:30 +0100
    [PATCH 2/6] staging: iio: rework regulator handling Eva Rachel Retuya <eraretuya@gmail.com> - 2016-10-31 18:10 +0100
      Re: [PATCH 2/6] staging: iio: rework regulator handling Jonathan Cameron <jic23@kernel.org> - 2016-11-05 17:20 +0100
    [PATCH 5/6] staging: iio: ad9832: add DVDD regulator Eva Rachel Retuya <eraretuya@gmail.com> - 2016-10-31 18:10 +0100
      Re: [PATCH 5/6] staging: iio: ad9832: add DVDD regulator Jonathan Cameron <jic23@kernel.org> - 2016-11-05 17:30 +0100
    [PATCH 3/6] staging: iio: ad7192: add DVdd regulator Eva Rachel Retuya <eraretuya@gmail.com> - 2016-10-31 18:10 +0100
      Re: [PATCH 3/6] staging: iio: ad7192: add DVdd regulator Jonathan Cameron <jic23@kernel.org> - 2016-11-05 17:20 +0100

#1512727 — [PATCH 0/6] staging: iio: regulator clean-up

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

[toc] | [next] | [standalone]

#1512728 — [PATCH 4/6] staging: iio: ad7192: rename regulator 'reg' to 'avdd'

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

[toc] | [prev] | [next] | [standalone]

#1515584 — Re: [PATCH 4/6] staging: iio: ad7192: rename regulator 'reg' to 'avdd'

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;
>  }
> 

[toc] | [prev] | [next] | [standalone]

#1512729 — [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get()

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

[toc] | [prev] | [next] | [standalone]

#1513074 — Re: [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get()

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
>

[toc] | [prev] | [next] | [standalone]

#1513455 — Re: [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get()

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.

[toc] | [prev] | [next] | [standalone]

#1515539 — Re: [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get()

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

[toc] | [prev] | [next] | [standalone]

#1515576 — Re: [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get()

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
> 

[toc] | [prev] | [next] | [standalone]

#1515106 — Re: [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get()

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
> >

[toc] | [prev] | [next] | [standalone]

#1515579 — Re: [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get()

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)
> 

[toc] | [prev] | [next] | [standalone]

#1512732 — [PATCH 6/6] staging: iio: ad9832: clean-up regulator 'reg'

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

[toc] | [prev] | [next] | [standalone]

#1515581 — Re: [PATCH 6/6] staging: iio: ad9832: clean-up regulator 'reg'

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;
> 

[toc] | [prev] | [next] | [standalone]

#1512733 — [PATCH 2/6] staging: iio: rework regulator handling

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

[toc] | [prev] | [next] | [standalone]

#1515578 — Re: [PATCH 2/6] staging: iio: rework regulator handling

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;
>  }
> 

[toc] | [prev] | [next] | [standalone]

#1512734 — [PATCH 5/6] staging: iio: ad9832: add DVDD regulator

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

[toc] | [prev] | [next] | [standalone]

#1515580 — Re: [PATCH 5/6] staging: iio: ad9832: add DVDD regulator

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;
> 

[toc] | [prev] | [next] | [standalone]

#1512735 — [PATCH 3/6] staging: iio: ad7192: add DVdd regulator

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

[toc] | [prev] | [next] | [standalone]

#1515574 — Re: [PATCH 3/6] staging: iio: ad7192: add DVdd regulator

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;
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web