Tue, May 30, 2023 at 09:53:09AM +0200, fl.scratchpad@xxxxxxxxx kirjoitti:
> From: Fabrizio Lamarque <fl.scratchpad@xxxxxxxxx>
>
> 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);
> + }
--
With Best Regards,
Andy Shevchenko
