| Message ID | 20230530075311.400686-4-fl.scratchpad@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Fix ad7192 driver issues | expand |
Tue, May 30, 2023 at 09:53:09AM +0200, fl.scratchpad@gmail.com kirjoitti: > From: Fabrizio Lamarque <fl.scratchpad@gmail.com> > > Add missing vref-supply and fix avdd-supply used as if it were vref. > > AD7192 requires three independent voltage sources, digital supply (on > pin DVdd), analog supply (on AVdd) and reference voltage (VRef on > alternate pin pair REFIN1 or REFIN2). > > Emit a warning message when AVdd is used in place of VRef for backwards > compatibility. ... > + st->vref = devm_regulator_get_optional(&spi->dev, "vref"); > + if (!IS_ERR(st->vref)) { > + ret = regulator_enable(st->vref); > + if (ret) > + return dev_err_probe(&spi->dev, ret, > + "Failed to enable specified VRef supply\n"); > + > + ret = devm_add_action_or_reset(&spi->dev, ad7192_reg_disable, st->vref); > + if (ret) > + return ret; > + } else if (PTR_ERR(st->vref) != -ENODEV) { > + return PTR_ERR(st->vref); > + } Wouldn't this be better? if (IS_ERR(st->vref)) { if (PTR_ERR(st->vref) != -ENODEV) return PTR_ERR(st->vref); } else { ... > if (ret) > return dev_err_probe(&spi->dev, ret, "Failed to enable specified DVdd supply\n"); > > - ret = regulator_get_voltage(st->avdd); > + One blank line is enough. > + if (!IS_ERR(st->vref)) { > + ret = regulator_get_voltage(st->vref); Why negative conditional? Usual pattern is to check for errors first, so if (IS_ERR(st->vref)) { dev_warn(...); ... } else { ret = regulator_get_voltage(st->vref); } > + } else { > + dev_warn(&spi->dev, "Using AVdd in place of VRef. Likely an old DTS\n"); > + ret = regulator_get_voltage(st->avdd); > + }
diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c index 5a9c8898f8af..bc41323ea810 100644 --- a/drivers/iio/adc/ad7192.c +++ b/drivers/iio/adc/ad7192.c @@ -177,6 +177,7 @@ struct ad7192_chip_info { struct ad7192_state { const struct ad7192_chip_info *chip_info; struct regulator *avdd; + struct regulator *vref; struct clk *mclk; u16 int_vref_mv; u32 fclk; @@ -1014,11 +1015,31 @@ static int ad7192_probe(struct spi_device *spi) if (ret) return ret; + st->vref = devm_regulator_get_optional(&spi->dev, "vref"); + if (!IS_ERR(st->vref)) { + ret = regulator_enable(st->vref); + if (ret) + return dev_err_probe(&spi->dev, ret, + "Failed to enable specified VRef supply\n"); + + ret = devm_add_action_or_reset(&spi->dev, ad7192_reg_disable, st->vref); + if (ret) + return ret; + } else if (PTR_ERR(st->vref) != -ENODEV) { + return PTR_ERR(st->vref); + } + ret = devm_regulator_get_enable(&spi->dev, "dvdd"); if (ret) return dev_err_probe(&spi->dev, ret, "Failed to enable specified DVdd supply\n"); - ret = regulator_get_voltage(st->avdd); + + if (!IS_ERR(st->vref)) { + ret = regulator_get_voltage(st->vref); + } else { + dev_warn(&spi->dev, "Using AVdd in place of VRef. Likely an old DTS\n"); + ret = regulator_get_voltage(st->avdd); + } if (ret < 0) { dev_err(&spi->dev, "Device tree error, reference voltage undefined\n"); return ret;