| Message ID | 20241021-ad7380-fix-supplies-v2-4-2ca551b3352a@baylibre.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: ad7380: fix several supplies issues | expand |
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, >
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, > > >
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,
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(-)