[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <81255bd6069702ee896562dc1a03451a5b455b6b.camel@gmail.com>
Date: Mon, 21 Oct 2024 13:09:15 +0200
From: Nuno Sá <noname.nuno@...il.com>
To: Julien Stephan <jstephan@...libre.com>, Lars-Peter Clausen
<lars@...afoo.de>, Michael Hennerich <Michael.Hennerich@...log.com>, Nuno
Sá <nuno.sa@...log.com>, David Lechner
<dlechner@...libre.com>, Jonathan Cameron <jic23@...nel.org>, Rob Herring
<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Liam Girdwood <lgirdwood@...il.com>, Mark Brown
<broonie@...nel.org>, Jonathan Corbet <corbet@....net>
Cc: linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, Conor Dooley <conor.dooley@...rochip.com>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>, linux-doc@...r.kernel.org
Subject: Re: [PATCH v2 2/5] iio: adc: ad7380: use
devm_regulator_get_enable_read_voltage()
On Mon, 2024-10-21 at 12:00 +0200, Julien Stephan wrote:
> Use devm_regulator_get_enable_read_voltage() to simplify the code.
>
> Signed-off-by: Julien Stephan <jstephan@...libre.com>
> ---
LGTM,
Reviewed-by: Nuno Sa <nuno.sa@...log.com>
> drivers/iio/adc/ad7380.c | 81 +++++++++++++----------------------------------
> -
> 1 file changed, 21 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c
> index
> e8bddfb0d07dbcf3e2a43344a4e0516f4d617548..e033c734191143a25a490b09c730dbf95f79
> 6737 100644
> --- a/drivers/iio/adc/ad7380.c
> +++ b/drivers/iio/adc/ad7380.c
> @@ -956,7 +956,7 @@ static const struct iio_info ad7380_info = {
> .debugfs_reg_access = &ad7380_debugfs_reg_access,
> };
>
> -static int ad7380_init(struct ad7380_state *st, struct regulator *vref)
> +static int ad7380_init(struct ad7380_state *st, bool external_ref_en)
> {
> int ret;
>
> @@ -968,13 +968,13 @@ static int ad7380_init(struct ad7380_state *st, struct
> regulator *vref)
> if (ret < 0)
> return ret;
>
> - /* select internal or external reference voltage */
> - ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG1,
> - AD7380_CONFIG1_REFSEL,
> - FIELD_PREP(AD7380_CONFIG1_REFSEL,
> - vref ? 1 : 0));
> - if (ret < 0)
> - return ret;
> + if (external_ref_en) {
> + /* select external reference voltage */
> + ret = regmap_set_bits(st->regmap, AD7380_REG_ADDR_CONFIG1,
> + AD7380_CONFIG1_REFSEL);
> + if (ret < 0)
> + return ret;
> + }
>
> /* This is the default value after reset. */
> st->oversampling_ratio = 1;
> @@ -987,16 +987,11 @@ static int ad7380_init(struct ad7380_state *st, struct
> regulator *vref)
> FIELD_PREP(AD7380_CONFIG2_SDO, 1));
> }
>
> -static void ad7380_regulator_disable(void *p)
> -{
> - regulator_disable(p);
> -}
> -
> static int ad7380_probe(struct spi_device *spi)
> {
> struct iio_dev *indio_dev;
> struct ad7380_state *st;
> - struct regulator *vref;
> + bool external_ref_en;
> int ret, i;
>
> indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> @@ -1009,37 +1004,17 @@ static int ad7380_probe(struct spi_device *spi)
> if (!st->chip_info)
> return dev_err_probe(&spi->dev, -EINVAL, "missing match
> data\n");
>
> - vref = devm_regulator_get_optional(&spi->dev, "refio");
> - if (IS_ERR(vref)) {
> - if (PTR_ERR(vref) != -ENODEV)
> - return dev_err_probe(&spi->dev, PTR_ERR(vref),
> - "Failed to get refio
> regulator\n");
> -
> - vref = NULL;
> - }
> -
> /*
> * If there is no REFIO supply, then it means that we are using
> * the internal 2.5V reference, otherwise REFIO is reference voltage.
> */
> - if (vref) {
> - ret = regulator_enable(vref);
> - if (ret)
> - return ret;
> + 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");
>
> - ret = devm_add_action_or_reset(&spi->dev,
> - ad7380_regulator_disable,
> vref);
> - if (ret)
> - return ret;
> -
> - ret = regulator_get_voltage(vref);
> - if (ret < 0)
> - return ret;
> -
> - st->vref_mv = ret / 1000;
> - } else {
> - st->vref_mv = AD7380_INTERNAL_REF_MV;
> - }
> + 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,
> @@ -1050,27 +1025,13 @@ static int ad7380_probe(struct spi_device *spi)
> * input pin.
> */
> for (i = 0; i < st->chip_info->num_vcm_supplies; i++) {
> - struct regulator *vcm;
> -
> - vcm = devm_regulator_get(&spi->dev,
> - st->chip_info->vcm_supplies[i]);
> - if (IS_ERR(vcm))
> - return dev_err_probe(&spi->dev, PTR_ERR(vcm),
> - "Failed to get %s regulator\n",
> - st->chip_info->vcm_supplies[i]);
> + const char *vcm = st->chip_info->vcm_supplies[i];
>
> - ret = regulator_enable(vcm);
> - if (ret)
> - return ret;
> -
> - ret = devm_add_action_or_reset(&spi->dev,
> - ad7380_regulator_disable,
> vcm);
> - if (ret)
> - return ret;
> -
> - ret = regulator_get_voltage(vcm);
> + ret = devm_regulator_get_enable_read_voltage(&spi->dev, vcm);
> if (ret < 0)
> - return ret;
> + return dev_err_probe(&spi->dev, ret,
> + "Failed to get %s regulator\n",
> + vcm);
>
> st->vcm_mv[i] = ret / 1000;
> }
> @@ -1135,7 +1096,7 @@ static int ad7380_probe(struct spi_device *spi)
> if (ret)
> return ret;
>
> - ret = ad7380_init(st, vref);
> + ret = ad7380_init(st, external_ref_en);
> if (ret)
> return ret;
>
>
Powered by blists - more mailing lists