| Message ID | 20240327-regulator-get-enable-get-votlage-v1-4-5f4517faa059@baylibre.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | regulator: new APIs for voltage reference supplies | expand |
On Wed, 27 Mar 2024 18:18:53 -0500 David Lechner <dlechner@baylibre.com> wrote: > We can reduce boilerplate code by using > devm_regulator_get_enable_get_voltage(). There is a change in behaviour in this one. I'd like some explanation in the patch title for why it is always safe to get the voltage of avdd_mv when previously it wasn't. There seems to me to be a corner case where a DTS is not providing the entry because it's always powered on so we get a stub regulator but that doesn't matter because we aren't in DIN_THRESHOLD_MOD_AVDD. After this change that dts is broken as now we read the voltage whatever. You could use the optional form and then fail the probe if in a mode where the value will be used? > > Signed-off-by: David Lechner <dlechner@baylibre.com> > --- > drivers/iio/addac/ad74115.c | 28 +++------------------------- > 1 file changed, 3 insertions(+), 25 deletions(-) > > diff --git a/drivers/iio/addac/ad74115.c b/drivers/iio/addac/ad74115.c > index e6bc5eb3788d..01073d7de6aa 100644 > --- a/drivers/iio/addac/ad74115.c > +++ b/drivers/iio/addac/ad74115.c > @@ -199,7 +199,6 @@ struct ad74115_state { > struct spi_device *spi; > struct regmap *regmap; > struct iio_trigger *trig; > - struct regulator *avdd; > > /* > * Synchronize consecutive operations when doing a one-shot > @@ -1672,14 +1671,6 @@ static int ad74115_setup(struct iio_dev *indio_dev) > if (ret) > return ret; > > - if (val == AD74115_DIN_THRESHOLD_MODE_AVDD) { > - ret = regulator_get_voltage(st->avdd); > - if (ret < 0) > - return ret; > - > - st->avdd_mv = ret / 1000; > - } > - > st->din_threshold_mode = val; > > ret = ad74115_apply_fw_prop(st, &ad74115_dac_bipolar_fw_prop, &val); > @@ -1788,11 +1779,6 @@ static int ad74115_reset(struct ad74115_state *st) > return 0; > } > > -static void ad74115_regulator_disable(void *data) > -{ > - regulator_disable(data); > -} > - > static int ad74115_setup_trigger(struct iio_dev *indio_dev) > { > struct ad74115_state *st = iio_priv(indio_dev); > @@ -1855,19 +1841,11 @@ static int ad74115_probe(struct spi_device *spi) > indio_dev->modes = INDIO_DIRECT_MODE; > indio_dev->info = &ad74115_info; > > - st->avdd = devm_regulator_get(dev, "avdd"); > - if (IS_ERR(st->avdd)) > - return PTR_ERR(st->avdd); > - > - ret = regulator_enable(st->avdd); > - if (ret) { > - dev_err(dev, "Failed to enable avdd regulator\n"); > + ret = devm_regulator_get_enable_get_voltage(dev, "avdd"); > + if (ret < 0) > return ret; > - } > > - ret = devm_add_action_or_reset(dev, ad74115_regulator_disable, st->avdd); > - if (ret) > - return ret; > + st->avdd_mv = ret / 1000; > > ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulator_names), > regulator_names); >
diff --git a/drivers/iio/addac/ad74115.c b/drivers/iio/addac/ad74115.c index e6bc5eb3788d..01073d7de6aa 100644 --- a/drivers/iio/addac/ad74115.c +++ b/drivers/iio/addac/ad74115.c @@ -199,7 +199,6 @@ struct ad74115_state { struct spi_device *spi; struct regmap *regmap; struct iio_trigger *trig; - struct regulator *avdd; /* * Synchronize consecutive operations when doing a one-shot @@ -1672,14 +1671,6 @@ static int ad74115_setup(struct iio_dev *indio_dev) if (ret) return ret; - if (val == AD74115_DIN_THRESHOLD_MODE_AVDD) { - ret = regulator_get_voltage(st->avdd); - if (ret < 0) - return ret; - - st->avdd_mv = ret / 1000; - } - st->din_threshold_mode = val; ret = ad74115_apply_fw_prop(st, &ad74115_dac_bipolar_fw_prop, &val); @@ -1788,11 +1779,6 @@ static int ad74115_reset(struct ad74115_state *st) return 0; } -static void ad74115_regulator_disable(void *data) -{ - regulator_disable(data); -} - static int ad74115_setup_trigger(struct iio_dev *indio_dev) { struct ad74115_state *st = iio_priv(indio_dev); @@ -1855,19 +1841,11 @@ static int ad74115_probe(struct spi_device *spi) indio_dev->modes = INDIO_DIRECT_MODE; indio_dev->info = &ad74115_info; - st->avdd = devm_regulator_get(dev, "avdd"); - if (IS_ERR(st->avdd)) - return PTR_ERR(st->avdd); - - ret = regulator_enable(st->avdd); - if (ret) { - dev_err(dev, "Failed to enable avdd regulator\n"); + ret = devm_regulator_get_enable_get_voltage(dev, "avdd"); + if (ret < 0) return ret; - } - ret = devm_add_action_or_reset(dev, ad74115_regulator_disable, st->avdd); - if (ret) - return ret; + st->avdd_mv = ret / 1000; ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulator_names), regulator_names);
We can reduce boilerplate code by using devm_regulator_get_enable_get_voltage(). Signed-off-by: David Lechner <dlechner@baylibre.com> --- drivers/iio/addac/ad74115.c | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-)