| Message ID | 20240612-iio-adc-ref-supply-refactor-v2-5-fa622e7354e9@baylibre.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | iio: adc: use devm_regulator_get_enable_read_voltage round 1 | expand |
On Wed, 2024-06-12 at 16:03 -0500, David Lechner wrote: > This makes use of the new devm_regulator_get_enable_read_voltage() > function to reduce boilerplate code. > > Signed-off-by: David Lechner <dlechner@baylibre.com> > --- > > v2 changes: > - don't read voltage from refin regulator > - avoid else in return value checks > --- > drivers/iio/adc/ad7944.c | 54 +++++++++++------------------------------------- > 1 file changed, 12 insertions(+), 42 deletions(-) > > diff --git a/drivers/iio/adc/ad7944.c b/drivers/iio/adc/ad7944.c > index e2cb64cef476..f8bf03feba07 100644 > --- a/drivers/iio/adc/ad7944.c > +++ b/drivers/iio/adc/ad7944.c > @@ -464,23 +464,17 @@ 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; > + bool have_refin; > struct iio_chan_spec *chain_chan; > unsigned long *chain_scan_masks; > u32 n_chain_dev; > - int ret; > + int ret, ref_mv; > > indio_dev = devm_iio_device_alloc(dev, sizeof(*adc)); > if (!indio_dev) > @@ -531,47 +525,23 @@ 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"); > + ret = devm_regulator_get_enable_read_voltage(dev, "ref"); > + if (ret < 0 && ret != -ENODEV) > + return dev_err_probe(dev, ret, "failed to get REF voltage\n"); > > - ref = NULL; > - } > + ref_mv = ret == -ENODEV ? 0 : 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"); > + if (ret < 0 && ret == -ENODEV) > + return dev_err_probe(dev, ret, "failed to get REFIN voltage\n"); > + ret != -ENODEV right? - Nuno Sá
On 6/14/24 10:16 AM, Nuno Sá wrote: > On Wed, 2024-06-12 at 16:03 -0500, David Lechner wrote: >> This makes use of the new devm_regulator_get_enable_read_voltage() >> function to reduce boilerplate code. >> >> Signed-off-by: David Lechner <dlechner@baylibre.com> >> --- >> >> v2 changes: >> - don't read voltage from refin regulator >> - avoid else in return value checks >> --- >> drivers/iio/adc/ad7944.c | 54 +++++++++++------------------------------------- >> 1 file changed, 12 insertions(+), 42 deletions(-) >> >> diff --git a/drivers/iio/adc/ad7944.c b/drivers/iio/adc/ad7944.c >> index e2cb64cef476..f8bf03feba07 100644 >> --- a/drivers/iio/adc/ad7944.c >> +++ b/drivers/iio/adc/ad7944.c >> @@ -464,23 +464,17 @@ 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; >> + bool have_refin; >> struct iio_chan_spec *chain_chan; >> unsigned long *chain_scan_masks; >> u32 n_chain_dev; >> - int ret; >> + int ret, ref_mv; >> >> indio_dev = devm_iio_device_alloc(dev, sizeof(*adc)); >> if (!indio_dev) >> @@ -531,47 +525,23 @@ 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"); >> + ret = devm_regulator_get_enable_read_voltage(dev, "ref"); >> + if (ret < 0 && ret != -ENODEV) >> + return dev_err_probe(dev, ret, "failed to get REF voltage\n"); >> >> - ref = NULL; >> - } >> + ref_mv = ret == -ENODEV ? 0 : 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"); >> + if (ret < 0 && ret == -ENODEV) >> + return dev_err_probe(dev, ret, "failed to get REFIN voltage\n"); >> + > > ret != -ENODEV right? oof, yeah messed that one too > > - Nuno Sá > >
On Fri, 14 Jun 2024 10:19:43 -0500 David Lechner <dlechner@baylibre.com> wrote: > On 6/14/24 10:16 AM, Nuno Sá wrote: > > On Wed, 2024-06-12 at 16:03 -0500, David Lechner wrote: > >> This makes use of the new devm_regulator_get_enable_read_voltage() > >> function to reduce boilerplate code. > >> > >> Signed-off-by: David Lechner <dlechner@baylibre.com> > >> --- > >> > >> v2 changes: > >> - don't read voltage from refin regulator > >> - avoid else in return value checks > >> --- > >> drivers/iio/adc/ad7944.c | 54 +++++++++++------------------------------------- > >> 1 file changed, 12 insertions(+), 42 deletions(-) > >> > >> diff --git a/drivers/iio/adc/ad7944.c b/drivers/iio/adc/ad7944.c > >> index e2cb64cef476..f8bf03feba07 100644 > >> --- a/drivers/iio/adc/ad7944.c > >> +++ b/drivers/iio/adc/ad7944.c > >> @@ -464,23 +464,17 @@ 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; > >> + bool have_refin; > >> struct iio_chan_spec *chain_chan; > >> unsigned long *chain_scan_masks; > >> u32 n_chain_dev; > >> - int ret; > >> + int ret, ref_mv; > >> > >> indio_dev = devm_iio_device_alloc(dev, sizeof(*adc)); > >> if (!indio_dev) > >> @@ -531,47 +525,23 @@ 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"); > >> + ret = devm_regulator_get_enable_read_voltage(dev, "ref"); > >> + if (ret < 0 && ret != -ENODEV) > >> + return dev_err_probe(dev, ret, "failed to get REF voltage\n"); > >> > >> - ref = NULL; > >> - } > >> + ref_mv = ret == -ENODEV ? 0 : 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"); > >> + if (ret < 0 && ret == -ENODEV) > >> + return dev_err_probe(dev, ret, "failed to get REFIN voltage\n"); > >> + > > > > ret != -ENODEV right? > > oof, yeah messed that one too > Fixed up as well and applied. Enough patches bouncing around that I'd rather clear these little things by hand than see the patch again :) Jonathan > > > > - Nuno Sá > > > > >
diff --git a/drivers/iio/adc/ad7944.c b/drivers/iio/adc/ad7944.c index e2cb64cef476..f8bf03feba07 100644 --- a/drivers/iio/adc/ad7944.c +++ b/drivers/iio/adc/ad7944.c @@ -464,23 +464,17 @@ 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; + bool have_refin; struct iio_chan_spec *chain_chan; unsigned long *chain_scan_masks; u32 n_chain_dev; - int ret; + int ret, ref_mv; indio_dev = devm_iio_device_alloc(dev, sizeof(*adc)); if (!indio_dev) @@ -531,47 +525,23 @@ 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"); + ret = devm_regulator_get_enable_read_voltage(dev, "ref"); + if (ret < 0 && ret != -ENODEV) + return dev_err_probe(dev, ret, "failed to get REF voltage\n"); - ref = NULL; - } + ref_mv = ret == -ENODEV ? 0 : 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"); + if (ret < 0 && ret == -ENODEV) + return dev_err_probe(dev, ret, "failed to get REFIN voltage\n"); + + have_refin = ret != -ENODEV; - if (have_refin && ref) + if (have_refin && ref_mv) return dev_err_probe(dev, -EINVAL, "cannot have both refin and ref supplies\n"); - if (ref) { - ret = regulator_enable(ref); - if (ret) - return dev_err_probe(dev, ret, - "failed to enable REF supply\n"); - - ret = devm_add_action_or_reset(dev, ad7944_ref_disable, ref); - if (ret) - return ret; - - ret = regulator_get_voltage(ref); - if (ret < 0) - return dev_err_probe(dev, ret, - "failed to get REF voltage\n"); - - /* external reference */ - adc->ref_mv = ret / 1000; - } else { - /* internal reference */ - adc->ref_mv = AD7944_INTERNAL_REF_MV; - } + adc->ref_mv = ref_mv ?: AD7944_INTERNAL_REF_MV; adc->cnv = devm_gpiod_get_optional(dev, "cnv", GPIOD_OUT_LOW); if (IS_ERR(adc->cnv))
This makes use of the new devm_regulator_get_enable_read_voltage() function to reduce boilerplate code. Signed-off-by: David Lechner <dlechner@baylibre.com> --- v2 changes: - don't read voltage from refin regulator - avoid else in return value checks --- drivers/iio/adc/ad7944.c | 54 +++++++++++------------------------------------- 1 file changed, 12 insertions(+), 42 deletions(-)