On Mon, 2024-12-16 at 15:44 -0600, David Lechner wrote: > Change the order of regulator enable and reset so that power supplies > are turned on before touching the reset line. Generally, chips should > have the VDRIVE supply enabled before applying voltage on any pins. > > While we are at it, remove the voltage level checks. If the power > supplies are not supplying the correct voltage, this is a hardware > design problem, not a software problem. > > Signed-off-by: David Lechner <dlechner@xxxxxxxxxxxx> > --- Reviewed-by: Nuno Sa <nuno.sa@xxxxxxxxxx> > Changes in v2: > - Dropped patches from "iio: use devm_regulator_get_enable_read_voltage > round 6" that have already been applied. > - New patch for ad7293 that just enables power supplies and no longer > reads the voltage. > - Link to v1: > https://lore.kernel.org/r/20241120-iio-regulator-cleanup-round-6-v1-0-d5a5360f7ec3@xxxxxxxxxxxx > --- > drivers/iio/dac/ad7293.c | 68 +++++++---------------------------------------- > - > 1 file changed, 9 insertions(+), 59 deletions(-) > > diff --git a/drivers/iio/dac/ad7293.c b/drivers/iio/dac/ad7293.c > index > 1d403267048240be1bf3d8b2a59685409b9087fd..d3f49b5337d2f512363d50b434d99d4e9b95 > 059f 100644 > --- a/drivers/iio/dac/ad7293.c > +++ b/drivers/iio/dac/ad7293.c > @@ -141,8 +141,6 @@ struct ad7293_state { > /* Protect against concurrent accesses to the device, page selection > and data content */ > struct mutex lock; > struct gpio_desc *gpio_reset; > - struct regulator *reg_avdd; > - struct regulator *reg_vdrive; > u8 page_select; > u8 data[3] __aligned(IIO_DMA_MINALIGN); > }; > @@ -777,6 +775,15 @@ static int ad7293_reset(struct ad7293_state *st) > static int ad7293_properties_parse(struct ad7293_state *st) > { > struct spi_device *spi = st->spi; > + int ret; > + > + ret = devm_regulator_get_enable(&spi->dev, "avdd"); > + if (ret) > + return dev_err_probe(&spi->dev, ret, "failed to enable > AVDD\n"); > + > + ret = devm_regulator_get_enable(&spi->dev, "vdrive"); > + if (ret) > + return dev_err_probe(&spi->dev, ret, "failed to enable > VDRIVE\n"); > > st->gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset", > GPIOD_OUT_HIGH); > @@ -784,24 +791,9 @@ static int ad7293_properties_parse(struct ad7293_state > *st) > return dev_err_probe(&spi->dev, PTR_ERR(st->gpio_reset), > "failed to get the reset GPIO\n"); > > - st->reg_avdd = devm_regulator_get(&spi->dev, "avdd"); > - if (IS_ERR(st->reg_avdd)) > - return dev_err_probe(&spi->dev, PTR_ERR(st->reg_avdd), > - "failed to get the AVDD voltage\n"); > - > - st->reg_vdrive = devm_regulator_get(&spi->dev, "vdrive"); > - if (IS_ERR(st->reg_vdrive)) > - return dev_err_probe(&spi->dev, PTR_ERR(st->reg_vdrive), > - "failed to get the VDRIVE voltage\n"); > - > return 0; > } > > -static void ad7293_reg_disable(void *data) > -{ > - regulator_disable(data); > -} > - > static int ad7293_init(struct ad7293_state *st) > { > int ret; > @@ -816,48 +808,6 @@ static int ad7293_init(struct ad7293_state *st) > if (ret) > return ret; > > - ret = regulator_enable(st->reg_avdd); > - if (ret) { > - dev_err(&spi->dev, > - "Failed to enable specified AVDD Voltage!\n"); > - return ret; > - } > - > - ret = devm_add_action_or_reset(&spi->dev, ad7293_reg_disable, > - st->reg_avdd); > - if (ret) > - return ret; > - > - ret = regulator_enable(st->reg_vdrive); > - if (ret) { > - dev_err(&spi->dev, > - "Failed to enable specified VDRIVE Voltage!\n"); > - return ret; > - } > - > - ret = devm_add_action_or_reset(&spi->dev, ad7293_reg_disable, > - st->reg_vdrive); > - if (ret) > - return ret; > - > - ret = regulator_get_voltage(st->reg_avdd); > - if (ret < 0) { > - dev_err(&spi->dev, "Failed to read avdd regulator: %d\n", > ret); > - return ret; > - } > - > - if (ret > 5500000 || ret < 4500000) > - return -EINVAL; > - > - ret = regulator_get_voltage(st->reg_vdrive); > - if (ret < 0) { > - dev_err(&spi->dev, > - "Failed to read vdrive regulator: %d\n", ret); > - return ret; > - } > - if (ret > 5500000 || ret < 1700000) > - return -EINVAL; > - > /* Check Chip ID */ > ret = __ad7293_spi_read(st, AD7293_REG_DEVICE_ID, &chip_id); > if (ret) > > --- > base-commit: 01958cb8a00d9721ae56ad1eef9cd7b22b5a34bb > change-id: 20241120-iio-regulator-cleanup-round-6-78b05be06718 > > Best regards,
