[PATCH v2 4/5] iio: adc: ad7380: fix supplies for ad7380-4

Julien Stephan posted 5 patches 1 month ago
There is a newer version of this series
[PATCH v2 4/5] iio: adc: ad7380: fix supplies for ad7380-4
Posted by Julien Stephan 1 month ago
ad7380-4 is the only device in the family that does not have an internal
reference. It uses "refin" as a required external reference.
All other devices in the family use "refio"" as an optional external
reference.

Fixes: 737413da8704 ("iio: adc: ad7380: add support for ad738x-4 4 channels variants")
Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
 drivers/iio/adc/ad7380.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c
index e257f78d63edd7910fcb936ec5344922f8e70b99..65096717f0dd3ea6a4ff7020bc544d62b84cb8fd 100644
--- a/drivers/iio/adc/ad7380.c
+++ b/drivers/iio/adc/ad7380.c
@@ -89,6 +89,7 @@ struct ad7380_chip_info {
 	bool has_mux;
 	const char * const *supplies;
 	unsigned int num_supplies;
+	bool external_ref_only;
 	const char * const *vcm_supplies;
 	unsigned int num_vcm_supplies;
 	const unsigned long *available_scan_masks;
@@ -431,6 +432,7 @@ static const struct ad7380_chip_info ad7380_4_chip_info = {
 	.num_simult_channels = 4,
 	.supplies = ad7380_supplies,
 	.num_supplies = ARRAY_SIZE(ad7380_supplies),
+	.external_ref_only = true,
 	.available_scan_masks = ad7380_4_channel_scan_masks,
 	.timing_specs = &ad7380_4_timing,
 };
@@ -1047,17 +1049,31 @@ static int ad7380_probe(struct spi_device *spi)
 				     "Failed to enable power supplies\n");
 	msleep(T_POWERUP_MS);
 
-	/*
-	 * If there is no REFIO supply, then it means that we are using
-	 * the internal 2.5V reference, otherwise REFIO is reference voltage.
-	 */
-	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");
+	if (st->chip_info->external_ref_only) {
+		ret = devm_regulator_get_enable_read_voltage(&spi->dev,
+							     "refin");
+		if (ret < 0)
+			return dev_err_probe(&spi->dev, ret,
+					     "Failed to get refin regulator\n");
+
+		st->vref_mv = ret / 1000;
 
-	external_ref_en = ret != -ENODEV;
-	st->vref_mv = external_ref_en ? ret / 1000 : AD7380_INTERNAL_REF_MV;
+		/* these chips don't have a register bit for this */
+		external_ref_en = false;
+	} else {
+		/*
+		 * If there is no REFIO supply, then it means that we are using
+		 * the internal reference, otherwise REFIO is reference voltage.
+		 */
+		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");
+
+		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,

-- 
2.47.0
Re: [PATCH v2 4/5] iio: adc: ad7380: fix supplies for ad7380-4
Posted by Nuno Sá 1 month ago
On Mon, 2024-10-21 at 12:00 +0200, Julien Stephan wrote:
> ad7380-4 is the only device in the family that does not have an internal
> reference. It uses "refin" as a required external reference.
> All other devices in the family use "refio"" as an optional external
> reference.
> 
> Fixes: 737413da8704 ("iio: adc: ad7380: add support for ad738x-4 4 channels
> variants")
> Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> ---

Hi Julien,

Patch looks good. Sorry if this already came out in the previous version or in
the other patchset you mention but shouldn't this fix come first in the series?

Anyways, for the patch itself:

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

>  drivers/iio/adc/ad7380.c | 36 ++++++++++++++++++++++++++----------
>  1 file changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c
> index
> e257f78d63edd7910fcb936ec5344922f8e70b99..65096717f0dd3ea6a4ff7020bc544d62b84c
> b8fd 100644
> --- a/drivers/iio/adc/ad7380.c
> +++ b/drivers/iio/adc/ad7380.c
> @@ -89,6 +89,7 @@ struct ad7380_chip_info {
>  	bool has_mux;
>  	const char * const *supplies;
>  	unsigned int num_supplies;
> +	bool external_ref_only;
>  	const char * const *vcm_supplies;
>  	unsigned int num_vcm_supplies;
>  	const unsigned long *available_scan_masks;
> @@ -431,6 +432,7 @@ static const struct ad7380_chip_info ad7380_4_chip_info =
> {
>  	.num_simult_channels = 4,
>  	.supplies = ad7380_supplies,
>  	.num_supplies = ARRAY_SIZE(ad7380_supplies),
> +	.external_ref_only = true,
>  	.available_scan_masks = ad7380_4_channel_scan_masks,
>  	.timing_specs = &ad7380_4_timing,
>  };
> @@ -1047,17 +1049,31 @@ static int ad7380_probe(struct spi_device *spi)
>  				     "Failed to enable power supplies\n");
>  	msleep(T_POWERUP_MS);
>  
> -	/*
> -	 * If there is no REFIO supply, then it means that we are using
> -	 * the internal 2.5V reference, otherwise REFIO is reference voltage.
> -	 */
> -	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");
> +	if (st->chip_info->external_ref_only) {
> +		ret = devm_regulator_get_enable_read_voltage(&spi->dev,
> +							     "refin");
> +		if (ret < 0)
> +			return dev_err_probe(&spi->dev, ret,
> +					     "Failed to get refin
> regulator\n");
> +
> +		st->vref_mv = ret / 1000;
>  
> -	external_ref_en = ret != -ENODEV;
> -	st->vref_mv = external_ref_en ? ret / 1000 : AD7380_INTERNAL_REF_MV;
> +		/* these chips don't have a register bit for this */
> +		external_ref_en = false;
> +	} else {
> +		/*
> +		 * If there is no REFIO supply, then it means that we are
> using
> +		 * the internal reference, otherwise REFIO is reference
> voltage.
> +		 */
> +		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");
> +
> +		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,
> 
Re: [PATCH v2 4/5] iio: adc: ad7380: fix supplies for ad7380-4
Posted by Julien Stephan 1 month ago
Le lun. 21 oct. 2024 à 13:18, Nuno Sá <noname.nuno@gmail.com> a écrit :
>
> On Mon, 2024-10-21 at 12:00 +0200, Julien Stephan wrote:
> > ad7380-4 is the only device in the family that does not have an internal
> > reference. It uses "refin" as a required external reference.
> > All other devices in the family use "refio"" as an optional external
> > reference.
> >
> > Fixes: 737413da8704 ("iio: adc: ad7380: add support for ad738x-4 4 channels
> > variants")
> > Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> > ---
>
> Hi Julien,
>
> Patch looks good. Sorry if this already came out in the previous version or in
> the other patchset you mention but shouldn't this fix come first in the series?
>

Hi Nuno,
That was my plan at first, but doing the
devm_regulator_get_enable_read_voltage() first, simplifies the next
changes ... and also eases the review :)

If needed I can do the rebase

Cheers
Julien

> Anyways, for the patch itself:
>
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
>
> >  drivers/iio/adc/ad7380.c | 36 ++++++++++++++++++++++++++----------
> >  1 file changed, 26 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c
> > index
> > e257f78d63edd7910fcb936ec5344922f8e70b99..65096717f0dd3ea6a4ff7020bc544d62b84c
> > b8fd 100644
> > --- a/drivers/iio/adc/ad7380.c
> > +++ b/drivers/iio/adc/ad7380.c
> > @@ -89,6 +89,7 @@ struct ad7380_chip_info {
> >       bool has_mux;
> >       const char * const *supplies;
> >       unsigned int num_supplies;
> > +     bool external_ref_only;
> >       const char * const *vcm_supplies;
> >       unsigned int num_vcm_supplies;
> >       const unsigned long *available_scan_masks;
> > @@ -431,6 +432,7 @@ static const struct ad7380_chip_info ad7380_4_chip_info =
> > {
> >       .num_simult_channels = 4,
> >       .supplies = ad7380_supplies,
> >       .num_supplies = ARRAY_SIZE(ad7380_supplies),
> > +     .external_ref_only = true,
> >       .available_scan_masks = ad7380_4_channel_scan_masks,
> >       .timing_specs = &ad7380_4_timing,
> >  };
> > @@ -1047,17 +1049,31 @@ static int ad7380_probe(struct spi_device *spi)
> >                                    "Failed to enable power supplies\n");
> >       msleep(T_POWERUP_MS);
> >
> > -     /*
> > -      * If there is no REFIO supply, then it means that we are using
> > -      * the internal 2.5V reference, otherwise REFIO is reference voltage.
> > -      */
> > -     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");
> > +     if (st->chip_info->external_ref_only) {
> > +             ret = devm_regulator_get_enable_read_voltage(&spi->dev,
> > +                                                          "refin");
> > +             if (ret < 0)
> > +                     return dev_err_probe(&spi->dev, ret,
> > +                                          "Failed to get refin
> > regulator\n");
> > +
> > +             st->vref_mv = ret / 1000;
> >
> > -     external_ref_en = ret != -ENODEV;
> > -     st->vref_mv = external_ref_en ? ret / 1000 : AD7380_INTERNAL_REF_MV;
> > +             /* these chips don't have a register bit for this */
> > +             external_ref_en = false;
> > +     } else {
> > +             /*
> > +              * If there is no REFIO supply, then it means that we are
> > using
> > +              * the internal reference, otherwise REFIO is reference
> > voltage.
> > +              */
> > +             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");
> > +
> > +             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,
> >
>