Re: [PATCH v1 2/7] iio: adc: Add basic support for AD4170

[Marcelo Schmitt]

> > > > +	ret = devm_regulator_get_enable_read_voltage(dev, "refin1n");
> > > > +	if (ret < 0 && ret != -ENODEV)
> > > > +		return dev_err_probe(dev, ret, "Failed to get REFIN- voltage.\n");
> > > > +
> > > > +	/* Negative supplies are assumed to provide negative voltage */
> > > > +	st->vrefs_uv[AD4170_REFIN1N_SUP] = ret == -ENODEV ? -ENODEV : -ret;  
> > > 
> > > Maybe to early for me but the comment does not make it clear to me why the negation?
> > > Won't the regulator return a negative voltage?  
> > 
> > devm_regulator_get_enable_read_voltage(), regulator_get_voltage(), and anything
> > about reading the regulator voltage returns either a positive voltage value or
> > a negative error code. I couldn't find out how to read a negative voltage with
> > regulator API. So, for now, this is making the simplifying assumption that
> > the negative end of external reference supplies is always below GND level (even
> > though they could be positive).
> 
> Hmm. We went around this a long time back but I can't remember what the outcome was...
> https://lore.kernel.org/linux-iio/544AC56F16B56944AEC3BD4E3D59177137546EF3FC@xxxxxxxxxxxxxxxxxxxxxxx/ 
> looks like the thread.
> 
> Take a look at dac/ad5791.c for example of a negative reference

AD5791 references are always either only negative (V_REFNF, V_REFNS) or only
positive (V_REFPS, V_REFPF) so the driver is fine with only adding a negative
sign to the voltage obtained from the negative supply.

AD4170 external references can be set to a negative voltage in some setups, or
to positive voltages on other setups. They're also constrained on the resulting
nominal reference voltage (REFIN+ − REFIN−) which is typically 2.5V but can
range from 1V minmum AVDD maximum. From that perspective, maybe this could be
abstracted as a differential voltage supply? Though, the multiplexer in the
chip can bet set to connect individual REFIN+, REFIN−, ..., REFIN2- supplies
ends to ADC analog inputs so an API like get_voltage(reg, &voltage)) would make
it easier to handle those configurations.

>From the mentioned thread, I see no hard objection to extending the regulator
framework in that sense so I'll start working on something to provide better
support for these cases where we have negative voltage.

Thanks,
Marcelo