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
>
