| Message ID | 20240612-iio-adc-ref-supply-refactor-v2-3-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: > * avoid else in return value check > * use macro instead of comment to document internal reference voltage > --- > drivers/iio/adc/ad7292.c | 36 ++++++------------------------------ > 1 file changed, 6 insertions(+), 30 deletions(-) > > diff --git a/drivers/iio/adc/ad7292.c b/drivers/iio/adc/ad7292.c > index 6aadd14f459d..87ffe66058a1 100644 > --- a/drivers/iio/adc/ad7292.c > +++ b/drivers/iio/adc/ad7292.c > @@ -17,6 +17,8 @@ > > #define ADI_VENDOR_ID 0x0018 > > +#define AD7292_INTERNAL_REF_MV 1250 > + > /* AD7292 registers definition */ > #define AD7292_REG_VENDOR_ID 0x00 > #define AD7292_REG_CONF_BANK 0x05 > @@ -79,7 +81,6 @@ static const struct iio_chan_spec ad7292_channels_diff[] = { > > struct ad7292_state { > struct spi_device *spi; > - struct regulator *reg; > unsigned short vref_mv; > > __be16 d16 __aligned(IIO_DMA_MINALIGN); > @@ -250,13 +251,6 @@ static const struct iio_info ad7292_info = { > .read_raw = ad7292_read_raw, > }; > > -static void ad7292_regulator_disable(void *data) > -{ > - struct ad7292_state *st = data; > - > - regulator_disable(st->reg); > -} > - > static int ad7292_probe(struct spi_device *spi) > { > struct ad7292_state *st; > @@ -277,29 +271,11 @@ static int ad7292_probe(struct spi_device *spi) > return -EINVAL; > } > > - st->reg = devm_regulator_get_optional(&spi->dev, "vref"); > - if (!IS_ERR(st->reg)) { > - ret = regulator_enable(st->reg); > - if (ret) { > - dev_err(&spi->dev, > - "Failed to enable external vref supply\n"); > - return ret; > - } > - > - ret = devm_add_action_or_reset(&spi->dev, > - ad7292_regulator_disable, st); > - if (ret) > - return ret; > - > - ret = regulator_get_voltage(st->reg); > - if (ret < 0) > - return ret; > + ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vref"); > + if (ret < 0 && ret == -ENODEV) ret != -ENODEV? - Nuno Sá
On 6/14/24 10:11 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: >> * avoid else in return value check >> * use macro instead of comment to document internal reference voltage >> --- >> drivers/iio/adc/ad7292.c | 36 ++++++------------------------------ >> 1 file changed, 6 insertions(+), 30 deletions(-) >> >> diff --git a/drivers/iio/adc/ad7292.c b/drivers/iio/adc/ad7292.c >> index 6aadd14f459d..87ffe66058a1 100644 >> --- a/drivers/iio/adc/ad7292.c >> +++ b/drivers/iio/adc/ad7292.c >> @@ -17,6 +17,8 @@ >> >> #define ADI_VENDOR_ID 0x0018 >> >> +#define AD7292_INTERNAL_REF_MV 1250 >> + >> /* AD7292 registers definition */ >> #define AD7292_REG_VENDOR_ID 0x00 >> #define AD7292_REG_CONF_BANK 0x05 >> @@ -79,7 +81,6 @@ static const struct iio_chan_spec ad7292_channels_diff[] = { >> >> struct ad7292_state { >> struct spi_device *spi; >> - struct regulator *reg; >> unsigned short vref_mv; >> >> __be16 d16 __aligned(IIO_DMA_MINALIGN); >> @@ -250,13 +251,6 @@ static const struct iio_info ad7292_info = { >> .read_raw = ad7292_read_raw, >> }; >> >> -static void ad7292_regulator_disable(void *data) >> -{ >> - struct ad7292_state *st = data; >> - >> - regulator_disable(st->reg); >> -} >> - >> static int ad7292_probe(struct spi_device *spi) >> { >> struct ad7292_state *st; >> @@ -277,29 +271,11 @@ static int ad7292_probe(struct spi_device *spi) >> return -EINVAL; >> } >> >> - st->reg = devm_regulator_get_optional(&spi->dev, "vref"); >> - if (!IS_ERR(st->reg)) { >> - ret = regulator_enable(st->reg); >> - if (ret) { >> - dev_err(&spi->dev, >> - "Failed to enable external vref supply\n"); >> - return ret; >> - } >> - >> - ret = devm_add_action_or_reset(&spi->dev, >> - ad7292_regulator_disable, st); >> - if (ret) >> - return ret; >> - >> - ret = regulator_get_voltage(st->reg); >> - if (ret < 0) >> - return ret; >> + ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vref"); >> + if (ret < 0 && ret == -ENODEV) > > ret != -ENODEV? yup, I messed this one up > > - Nuno Sá >
On Fri, 14 Jun 2024 10:16:26 -0500 David Lechner <dlechner@baylibre.com> wrote: > On 6/14/24 10:11 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: > >> * avoid else in return value check > >> * use macro instead of comment to document internal reference voltage > >> --- > >> drivers/iio/adc/ad7292.c | 36 ++++++------------------------------ > >> 1 file changed, 6 insertions(+), 30 deletions(-) > >> > >> diff --git a/drivers/iio/adc/ad7292.c b/drivers/iio/adc/ad7292.c > >> index 6aadd14f459d..87ffe66058a1 100644 > >> --- a/drivers/iio/adc/ad7292.c > >> +++ b/drivers/iio/adc/ad7292.c > >> @@ -17,6 +17,8 @@ > >> > >> #define ADI_VENDOR_ID 0x0018 > >> > >> +#define AD7292_INTERNAL_REF_MV 1250 > >> + > >> /* AD7292 registers definition */ > >> #define AD7292_REG_VENDOR_ID 0x00 > >> #define AD7292_REG_CONF_BANK 0x05 > >> @@ -79,7 +81,6 @@ static const struct iio_chan_spec ad7292_channels_diff[] = { > >> > >> struct ad7292_state { > >> struct spi_device *spi; > >> - struct regulator *reg; > >> unsigned short vref_mv; > >> > >> __be16 d16 __aligned(IIO_DMA_MINALIGN); > >> @@ -250,13 +251,6 @@ static const struct iio_info ad7292_info = { > >> .read_raw = ad7292_read_raw, > >> }; > >> > >> -static void ad7292_regulator_disable(void *data) > >> -{ > >> - struct ad7292_state *st = data; > >> - > >> - regulator_disable(st->reg); > >> -} > >> - > >> static int ad7292_probe(struct spi_device *spi) > >> { > >> struct ad7292_state *st; > >> @@ -277,29 +271,11 @@ static int ad7292_probe(struct spi_device *spi) > >> return -EINVAL; > >> } > >> > >> - st->reg = devm_regulator_get_optional(&spi->dev, "vref"); > >> - if (!IS_ERR(st->reg)) { > >> - ret = regulator_enable(st->reg); > >> - if (ret) { > >> - dev_err(&spi->dev, > >> - "Failed to enable external vref supply\n"); > >> - return ret; > >> - } > >> - > >> - ret = devm_add_action_or_reset(&spi->dev, > >> - ad7292_regulator_disable, st); > >> - if (ret) > >> - return ret; > >> - > >> - ret = regulator_get_voltage(st->reg); > >> - if (ret < 0) > >> - return ret; > >> + ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vref"); > >> + if (ret < 0 && ret == -ENODEV) > > > > ret != -ENODEV? > > yup, I messed this one up Fixed up whilst applying. Applied > > > > > - Nuno Sá > > >
diff --git a/drivers/iio/adc/ad7292.c b/drivers/iio/adc/ad7292.c index 6aadd14f459d..87ffe66058a1 100644 --- a/drivers/iio/adc/ad7292.c +++ b/drivers/iio/adc/ad7292.c @@ -17,6 +17,8 @@ #define ADI_VENDOR_ID 0x0018 +#define AD7292_INTERNAL_REF_MV 1250 + /* AD7292 registers definition */ #define AD7292_REG_VENDOR_ID 0x00 #define AD7292_REG_CONF_BANK 0x05 @@ -79,7 +81,6 @@ static const struct iio_chan_spec ad7292_channels_diff[] = { struct ad7292_state { struct spi_device *spi; - struct regulator *reg; unsigned short vref_mv; __be16 d16 __aligned(IIO_DMA_MINALIGN); @@ -250,13 +251,6 @@ static const struct iio_info ad7292_info = { .read_raw = ad7292_read_raw, }; -static void ad7292_regulator_disable(void *data) -{ - struct ad7292_state *st = data; - - regulator_disable(st->reg); -} - static int ad7292_probe(struct spi_device *spi) { struct ad7292_state *st; @@ -277,29 +271,11 @@ static int ad7292_probe(struct spi_device *spi) return -EINVAL; } - st->reg = devm_regulator_get_optional(&spi->dev, "vref"); - if (!IS_ERR(st->reg)) { - ret = regulator_enable(st->reg); - if (ret) { - dev_err(&spi->dev, - "Failed to enable external vref supply\n"); - return ret; - } - - ret = devm_add_action_or_reset(&spi->dev, - ad7292_regulator_disable, st); - if (ret) - return ret; - - ret = regulator_get_voltage(st->reg); - if (ret < 0) - return ret; + ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vref"); + if (ret < 0 && ret == -ENODEV) + return ret; - st->vref_mv = ret / 1000; - } else { - /* Use the internal voltage reference. */ - st->vref_mv = 1250; - } + st->vref_mv = ret == -ENODEV ? AD7292_INTERNAL_REF_MV : ret / 1000; indio_dev->name = spi_get_device_id(spi)->name; indio_dev->modes = INDIO_DIRECT_MODE;
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: * avoid else in return value check * use macro instead of comment to document internal reference voltage --- drivers/iio/adc/ad7292.c | 36 ++++++------------------------------ 1 file changed, 6 insertions(+), 30 deletions(-)