On Wed, 5 Apr 2023 16:01:12 +0200
Nicolas Frattaroli <frattaroli.nicolas@xxxxxxxxx> wrote:
> The MCP4922 family of chips has a vdd power input, which we
> model in our device tree binding for it. The driver should get
> and enable the vdd regulator as is appropriate.
>
> Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@xxxxxxxxx>
If, before doing this you add a patch using devm for all the
unwinding currently being done by hand in remove() you can
simplify this further use devm_regulator_get_enable()
(Note you can only do that for this regulator as we never touch it
after enable - more complex handling needed for the vref one as
described in review of patch 4.)
That conversion patch is pretty simple, so whilst I don't like asking
people to implement extra features, in this case the simplifications
to what you are doing here make that precusor work justified
Jonathan
> ---
> drivers/iio/dac/mcp4922.c | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/dac/mcp4922.c b/drivers/iio/dac/mcp4922.c
> index da4327624d45..0b9458cbbcff 100644
> --- a/drivers/iio/dac/mcp4922.c
> +++ b/drivers/iio/dac/mcp4922.c
> @@ -31,6 +31,7 @@ struct mcp4922_state {
> unsigned int value[MCP4922_NUM_CHANNELS];
> unsigned int vref_mv;
> struct regulator *vref_reg;
> + struct regulator *vdd_reg;
> u8 mosi[2] __aligned(IIO_DMA_MINALIGN);
> };
>
> @@ -148,10 +149,23 @@ static int mcp4922_probe(struct spi_device *spi)
> if (ret < 0) {
> dev_err(&spi->dev, "Failed to read vref regulator: %d\n",
> ret);
> - goto error_disable_reg;
> + goto error_disable_vref_reg;
> }
> state->vref_mv = ret / 1000;
>
> + state->vdd_reg = devm_regulator_get(&spi->dev, "vdd");
> + if (IS_ERR(state->vdd_reg)) {
> + ret = dev_err_probe(&spi->dev, PTR_ERR(state->vdd_reg),
> + "vdd regulator not specified\n");
> + goto error_disable_vref_reg;
> + }
> + ret = regulator_enable(state->vdd_reg);
> + if (ret) {
> + dev_err(&spi->dev, "Failed to enable vdd regulator: %d\n",
> + ret);
> + goto error_disable_vref_reg;
> + }
> +
> spi_set_drvdata(spi, indio_dev);
> id = spi_get_device_id(spi);
> indio_dev->info = &mcp4922_info;
> @@ -167,12 +181,13 @@ static int mcp4922_probe(struct spi_device *spi)
> if (ret) {
> dev_err(&spi->dev, "Failed to register iio device: %d\n",
> ret);
> - goto error_disable_reg;
> + goto error_disable_vdd_reg;
> }
>
> return 0;
> -
> -error_disable_reg:
> +error_disable_vdd_reg:
> + regulator_disable(state->vdd_reg);
> +error_disable_vref_reg:
> regulator_disable(state->vref_reg);
>
> return ret;
> @@ -185,6 +200,7 @@ static void mcp4922_remove(struct spi_device *spi)
>
> iio_device_unregister(indio_dev);
> state = iio_priv(indio_dev);
> + regulator_disable(state->vdd_reg);
> regulator_disable(state->vref_reg);
> }
>
