On Tue, Apr 30, 2024 at 11:30 AM Alisa-Dariana Roman <alisadariana@xxxxxxxxx> wrote: > > AINCOM should actually be a supply. AINx inputs are referenced to AINCOM > in pseudo-differential operation mode. AINCOM voltage represents the > offset of corresponding channels. > > Signed-off-by: Alisa-Dariana Roman <alisa.roman@xxxxxxxxxx> > --- > drivers/iio/adc/ad7192.c | 41 ++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 39 insertions(+), 2 deletions(-) > > diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c > index ace81e3817a1..3e797ff48086 100644 > --- a/drivers/iio/adc/ad7192.c > +++ b/drivers/iio/adc/ad7192.c > @@ -20,6 +20,7 @@ > #include <linux/module.h> > #include <linux/mod_devicetable.h> > #include <linux/property.h> > +#include <linux/units.h> > > #include <linux/iio/iio.h> > #include <linux/iio/sysfs.h> > @@ -186,6 +187,7 @@ struct ad7192_state { > struct regulator *vref; > struct clk *mclk; > u16 int_vref_mv; > + u32 aincom_mv; > u32 fclk; > u32 mode; > u32 conf; > @@ -742,10 +744,19 @@ static int ad7192_read_raw(struct iio_dev *indio_dev, > *val = -(1 << (chan->scan_type.realbits - 1)); > else > *val = 0; nit: add blank line before switch for readability > + switch (chan->type) { > + case IIO_VOLTAGE: Comment could be helpful here as a few things might not be obvious: * Only applies to pseudo-differential inputs (!chan->differential) * AINCOM voltage has to be converted to "raw" units. > + if (st->aincom_mv && !chan->differential) > + *val += DIV_ROUND_CLOSEST_ULL((u64)st->aincom_mv * NANO, > + st->scale_avail[gain][1]); > + return IIO_VAL_INT; > /* Kelvin to Celsius */ > - if (chan->type == IIO_TEMP) > + case IIO_TEMP: > *val -= 273 * ad7192_get_temp_scale(unipolar); > - return IIO_VAL_INT; > + return IIO_VAL_INT; > + default: > + return -EINVAL; > + } > case IIO_CHAN_INFO_SAMP_FREQ: > *val = DIV_ROUND_CLOSEST(ad7192_get_f_adc(st), 1024); > return IIO_VAL_INT; > @@ -1052,6 +1063,7 @@ static int ad7192_probe(struct spi_device *spi) > { > struct ad7192_state *st; > struct iio_dev *indio_dev; > + struct regulator *aincom; > int ret; > > if (!spi->irq) { > @@ -1067,6 +1079,31 @@ static int ad7192_probe(struct spi_device *spi) > > mutex_init(&st->lock); > > + /* > + * Regulator aincom is optional to maintain compatibility with older DT. > + * Newer firmware should provide a zero volt fixed supply if wired to > + * ground. > + */ > + aincom = devm_regulator_get_optional(&spi->dev, "aincom"); Do we need to consider other errors separate from -EINVAL? I've done something like this in other drivers: if (IS_ERR(aincom)) { if (PTR_ERR(aincom) != -EINVAL) return dev_err_probe(...); st->aincom_mv = 0; } else { ... st->aincom_mv = ...; } > + if (IS_ERR(aincom)) { > + st->aincom_mv = 0; > + } else { > + ret = regulator_enable(aincom); > + if (ret) > + return dev_err_probe(&spi->dev, ret, > + "Failed to enable specified AINCOM supply\n"); > + > + ret = devm_add_action_or_reset(&spi->dev, ad7192_reg_disable, aincom); > + if (ret) > + return ret; > + > + ret = regulator_get_voltage(aincom); > + if (ret < 0) > + return dev_err_probe(&spi->dev, ret, > + "Device tree error, AINCOM voltage undefined\n"); > + st->aincom_mv = ret / 1000; > + } > + > st->avdd = devm_regulator_get(&spi->dev, "avdd"); > if (IS_ERR(st->avdd)) > return PTR_ERR(st->avdd); > -- > 2.34.1 >