On 6/1/24 8:01 AM, Jonathan Cameron wrote: > On Fri, 31 May 2024 16:19:36 -0500 > David Lechner <dlechner@xxxxxxxxxxxx> wrote: > >> This makes use of the new devm_regulator_get_enable_read_voltage() >> function to reduce boilerplate code. >> >> Signed-off-by: David Lechner <dlechner@xxxxxxxxxxxx> > A possible corner case inline. > > Patches 2-4 lgtm. >> --- >> drivers/iio/adc/ad7944.c | 62 +++++++++++++++--------------------------------- >> 1 file changed, 19 insertions(+), 43 deletions(-) >> >> diff --git a/drivers/iio/adc/ad7944.c b/drivers/iio/adc/ad7944.c >> index e2cb64cef476..42bbcb904778 100644 >> --- a/drivers/iio/adc/ad7944.c >> +++ b/drivers/iio/adc/ad7944.c >> @@ -464,23 +464,16 @@ static const char * const ad7944_power_supplies[] = { >> "avdd", "dvdd", "bvdd", "vio" >> }; >> >> -static void ad7944_ref_disable(void *ref) >> -{ >> - regulator_disable(ref); >> -} >> - >> static int ad7944_probe(struct spi_device *spi) >> { >> const struct ad7944_chip_info *chip_info; >> struct device *dev = &spi->dev; >> struct iio_dev *indio_dev; >> struct ad7944_adc *adc; >> - bool have_refin = false; >> - struct regulator *ref; >> struct iio_chan_spec *chain_chan; >> unsigned long *chain_scan_masks; >> u32 n_chain_dev; >> - int ret; >> + int ret, ref_mv, refin_mv; >> >> indio_dev = devm_iio_device_alloc(dev, sizeof(*adc)); >> if (!indio_dev) >> @@ -531,47 +524,30 @@ static int ad7944_probe(struct spi_device *spi) >> * - external reference: REF is connected, REFIN is not connected >> */ >> >> - ref = devm_regulator_get_optional(dev, "ref"); >> - if (IS_ERR(ref)) { >> - if (PTR_ERR(ref) != -ENODEV) >> - return dev_err_probe(dev, PTR_ERR(ref), >> - "failed to get REF supply\n"); >> - >> - ref = NULL; >> - } >> + ret = devm_regulator_get_enable_read_voltage(dev, "ref"); >> + if (ret == -ENODEV) >> + ref_mv = 0; >> + else if (ret < 0) >> + return dev_err_probe(dev, ret, "failed to get REF voltage\n"); >> + else >> + ref_mv = ret / 1000; >> >> - ret = devm_regulator_get_enable_optional(dev, "refin"); >> - if (ret == 0) >> - have_refin = true; >> - else if (ret != -ENODEV) >> - return dev_err_probe(dev, ret, >> - "failed to get and enable REFIN supply\n"); >> + ret = devm_regulator_get_enable_read_voltage(dev, "refin"); >> + if (ret == -ENODEV) >> + refin_mv = 0; >> + else if (ret < 0) >> + return dev_err_probe(dev, ret, "failed to get REFIN voltage\n"); >> + else >> + refin_mv = ret / 1000; > How does refin_mv get used? Previously we never queried it's voltage (I assume > because it supplies an internal reference? > > Are there any regulators that are real enough to enable but for which a voltage > can't be queried? I think fixed regulators with gpio control are in this > category... > Hmm... don't remember why I did it that way (was a while ago). I will bring back the have_refin flag instead.