[PATCH v2 2/5] iio: adc: ad7380: use devm_regulator_get_enable_read_voltage()

Julien Stephan posted 5 patches 1 month ago
There is a newer version of this series
[PATCH v2 2/5] iio: adc: ad7380: use devm_regulator_get_enable_read_voltage()
Posted by Julien Stephan 1 month ago
Use devm_regulator_get_enable_read_voltage() to simplify the code.

Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
 drivers/iio/adc/ad7380.c | 81 +++++++++++++-----------------------------------
 1 file changed, 21 insertions(+), 60 deletions(-)

diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c
index e8bddfb0d07dbcf3e2a43344a4e0516f4d617548..e033c734191143a25a490b09c730dbf95f796737 100644
--- a/drivers/iio/adc/ad7380.c
+++ b/drivers/iio/adc/ad7380.c
@@ -956,7 +956,7 @@ static const struct iio_info ad7380_info = {
 	.debugfs_reg_access = &ad7380_debugfs_reg_access,
 };
 
-static int ad7380_init(struct ad7380_state *st, struct regulator *vref)
+static int ad7380_init(struct ad7380_state *st, bool external_ref_en)
 {
 	int ret;
 
@@ -968,13 +968,13 @@ static int ad7380_init(struct ad7380_state *st, struct regulator *vref)
 	if (ret < 0)
 		return ret;
 
-	/* select internal or external reference voltage */
-	ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG1,
-				 AD7380_CONFIG1_REFSEL,
-				 FIELD_PREP(AD7380_CONFIG1_REFSEL,
-					    vref ? 1 : 0));
-	if (ret < 0)
-		return ret;
+	if (external_ref_en) {
+		/* select external reference voltage */
+		ret = regmap_set_bits(st->regmap, AD7380_REG_ADDR_CONFIG1,
+				      AD7380_CONFIG1_REFSEL);
+		if (ret < 0)
+			return ret;
+	}
 
 	/* This is the default value after reset. */
 	st->oversampling_ratio = 1;
@@ -987,16 +987,11 @@ static int ad7380_init(struct ad7380_state *st, struct regulator *vref)
 				  FIELD_PREP(AD7380_CONFIG2_SDO, 1));
 }
 
-static void ad7380_regulator_disable(void *p)
-{
-	regulator_disable(p);
-}
-
 static int ad7380_probe(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev;
 	struct ad7380_state *st;
-	struct regulator *vref;
+	bool external_ref_en;
 	int ret, i;
 
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
@@ -1009,37 +1004,17 @@ static int ad7380_probe(struct spi_device *spi)
 	if (!st->chip_info)
 		return dev_err_probe(&spi->dev, -EINVAL, "missing match data\n");
 
-	vref = devm_regulator_get_optional(&spi->dev, "refio");
-	if (IS_ERR(vref)) {
-		if (PTR_ERR(vref) != -ENODEV)
-			return dev_err_probe(&spi->dev, PTR_ERR(vref),
-					     "Failed to get refio regulator\n");
-
-		vref = NULL;
-	}
-
 	/*
 	 * If there is no REFIO supply, then it means that we are using
 	 * the internal 2.5V reference, otherwise REFIO is reference voltage.
 	 */
-	if (vref) {
-		ret = regulator_enable(vref);
-		if (ret)
-			return ret;
+	ret = devm_regulator_get_enable_read_voltage(&spi->dev, "refio");
+	if (ret < 0 && ret != -ENODEV)
+		return dev_err_probe(&spi->dev, ret,
+				     "Failed to get refio regulator\n");
 
-		ret = devm_add_action_or_reset(&spi->dev,
-					       ad7380_regulator_disable, vref);
-		if (ret)
-			return ret;
-
-		ret = regulator_get_voltage(vref);
-		if (ret < 0)
-			return ret;
-
-		st->vref_mv = ret / 1000;
-	} else {
-		st->vref_mv = AD7380_INTERNAL_REF_MV;
-	}
+	external_ref_en = ret != -ENODEV;
+	st->vref_mv = external_ref_en ? ret / 1000 : AD7380_INTERNAL_REF_MV;
 
 	if (st->chip_info->num_vcm_supplies > ARRAY_SIZE(st->vcm_mv))
 		return dev_err_probe(&spi->dev, -EINVAL,
@@ -1050,27 +1025,13 @@ static int ad7380_probe(struct spi_device *spi)
 	 * input pin.
 	 */
 	for (i = 0; i < st->chip_info->num_vcm_supplies; i++) {
-		struct regulator *vcm;
-
-		vcm = devm_regulator_get(&spi->dev,
-					 st->chip_info->vcm_supplies[i]);
-		if (IS_ERR(vcm))
-			return dev_err_probe(&spi->dev, PTR_ERR(vcm),
-					     "Failed to get %s regulator\n",
-					     st->chip_info->vcm_supplies[i]);
+		const char *vcm = st->chip_info->vcm_supplies[i];
 
-		ret = regulator_enable(vcm);
-		if (ret)
-			return ret;
-
-		ret = devm_add_action_or_reset(&spi->dev,
-					       ad7380_regulator_disable, vcm);
-		if (ret)
-			return ret;
-
-		ret = regulator_get_voltage(vcm);
+		ret = devm_regulator_get_enable_read_voltage(&spi->dev, vcm);
 		if (ret < 0)
-			return ret;
+			return dev_err_probe(&spi->dev, ret,
+					     "Failed to get %s regulator\n",
+					     vcm);
 
 		st->vcm_mv[i] = ret / 1000;
 	}
@@ -1135,7 +1096,7 @@ static int ad7380_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = ad7380_init(st, vref);
+	ret = ad7380_init(st, external_ref_en);
 	if (ret)
 		return ret;
 

-- 
2.47.0
Re: [PATCH v2 2/5] iio: adc: ad7380: use devm_regulator_get_enable_read_voltage()
Posted by Nuno Sá 1 month ago
On Mon, 2024-10-21 at 12:00 +0200, Julien Stephan wrote:
> Use devm_regulator_get_enable_read_voltage() to simplify the code.
> 
> Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> ---

LGTM,

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

>  drivers/iio/adc/ad7380.c | 81 +++++++++++++----------------------------------
> -
>  1 file changed, 21 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c
> index
> e8bddfb0d07dbcf3e2a43344a4e0516f4d617548..e033c734191143a25a490b09c730dbf95f79
> 6737 100644
> --- a/drivers/iio/adc/ad7380.c
> +++ b/drivers/iio/adc/ad7380.c
> @@ -956,7 +956,7 @@ static const struct iio_info ad7380_info = {
>  	.debugfs_reg_access = &ad7380_debugfs_reg_access,
>  };
>  
> -static int ad7380_init(struct ad7380_state *st, struct regulator *vref)
> +static int ad7380_init(struct ad7380_state *st, bool external_ref_en)
>  {
>  	int ret;
>  
> @@ -968,13 +968,13 @@ static int ad7380_init(struct ad7380_state *st, struct
> regulator *vref)
>  	if (ret < 0)
>  		return ret;
>  
> -	/* select internal or external reference voltage */
> -	ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG1,
> -				 AD7380_CONFIG1_REFSEL,
> -				 FIELD_PREP(AD7380_CONFIG1_REFSEL,
> -					    vref ? 1 : 0));
> -	if (ret < 0)
> -		return ret;
> +	if (external_ref_en) {
> +		/* select external reference voltage */
> +		ret = regmap_set_bits(st->regmap, AD7380_REG_ADDR_CONFIG1,
> +				      AD7380_CONFIG1_REFSEL);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
>  	/* This is the default value after reset. */
>  	st->oversampling_ratio = 1;
> @@ -987,16 +987,11 @@ static int ad7380_init(struct ad7380_state *st, struct
> regulator *vref)
>  				  FIELD_PREP(AD7380_CONFIG2_SDO, 1));
>  }
>  
> -static void ad7380_regulator_disable(void *p)
> -{
> -	regulator_disable(p);
> -}
> -
>  static int ad7380_probe(struct spi_device *spi)
>  {
>  	struct iio_dev *indio_dev;
>  	struct ad7380_state *st;
> -	struct regulator *vref;
> +	bool external_ref_en;
>  	int ret, i;
>  
>  	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> @@ -1009,37 +1004,17 @@ static int ad7380_probe(struct spi_device *spi)
>  	if (!st->chip_info)
>  		return dev_err_probe(&spi->dev, -EINVAL, "missing match
> data\n");
>  
> -	vref = devm_regulator_get_optional(&spi->dev, "refio");
> -	if (IS_ERR(vref)) {
> -		if (PTR_ERR(vref) != -ENODEV)
> -			return dev_err_probe(&spi->dev, PTR_ERR(vref),
> -					     "Failed to get refio
> regulator\n");
> -
> -		vref = NULL;
> -	}
> -
>  	/*
>  	 * If there is no REFIO supply, then it means that we are using
>  	 * the internal 2.5V reference, otherwise REFIO is reference voltage.
>  	 */
> -	if (vref) {
> -		ret = regulator_enable(vref);
> -		if (ret)
> -			return ret;
> +	ret = devm_regulator_get_enable_read_voltage(&spi->dev, "refio");
> +	if (ret < 0 && ret != -ENODEV)
> +		return dev_err_probe(&spi->dev, ret,
> +				     "Failed to get refio regulator\n");
>  
> -		ret = devm_add_action_or_reset(&spi->dev,
> -					       ad7380_regulator_disable,
> vref);
> -		if (ret)
> -			return ret;
> -
> -		ret = regulator_get_voltage(vref);
> -		if (ret < 0)
> -			return ret;
> -
> -		st->vref_mv = ret / 1000;
> -	} else {
> -		st->vref_mv = AD7380_INTERNAL_REF_MV;
> -	}
> +	external_ref_en = ret != -ENODEV;
> +	st->vref_mv = external_ref_en ? ret / 1000 : AD7380_INTERNAL_REF_MV;
>  
>  	if (st->chip_info->num_vcm_supplies > ARRAY_SIZE(st->vcm_mv))
>  		return dev_err_probe(&spi->dev, -EINVAL,
> @@ -1050,27 +1025,13 @@ static int ad7380_probe(struct spi_device *spi)
>  	 * input pin.
>  	 */
>  	for (i = 0; i < st->chip_info->num_vcm_supplies; i++) {
> -		struct regulator *vcm;
> -
> -		vcm = devm_regulator_get(&spi->dev,
> -					 st->chip_info->vcm_supplies[i]);
> -		if (IS_ERR(vcm))
> -			return dev_err_probe(&spi->dev, PTR_ERR(vcm),
> -					     "Failed to get %s regulator\n",
> -					     st->chip_info->vcm_supplies[i]);
> +		const char *vcm = st->chip_info->vcm_supplies[i];
>  
> -		ret = regulator_enable(vcm);
> -		if (ret)
> -			return ret;
> -
> -		ret = devm_add_action_or_reset(&spi->dev,
> -					       ad7380_regulator_disable,
> vcm);
> -		if (ret)
> -			return ret;
> -
> -		ret = regulator_get_voltage(vcm);
> +		ret = devm_regulator_get_enable_read_voltage(&spi->dev, vcm);
>  		if (ret < 0)
> -			return ret;
> +			return dev_err_probe(&spi->dev, ret,
> +					     "Failed to get %s regulator\n",
> +					     vcm);
>  
>  		st->vcm_mv[i] = ret / 1000;
>  	}
> @@ -1135,7 +1096,7 @@ static int ad7380_probe(struct spi_device *spi)
>  	if (ret)
>  		return ret;
>  
> -	ret = ad7380_init(st, vref);
> +	ret = ad7380_init(st, external_ref_en);
>  	if (ret)
>  		return ret;
>  
>