| Message ID | a6999fc24a4c61498e55f23861997382df2ab4f4.1736201898.git.Jonathan.Santos@analog.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: ad7768-1: Add features, improvements, and fixes | expand |
On 1/7/25 9:26 AM, Jonathan Santos wrote: > From: Sergiu Cuciurean <sergiu.cuciurean@analog.com> > > The VCM output voltage can be used as a common-mode voltage within the > amplifier preconditioning circuits external to the AD7768-1. > > This change exports the VCM as an iio attribute and makes it available > for configuration. We model common mode voltage inputs as a regulator consumer (e.g. vcm-supply in DT), so should we make this a regulator provider instead? It could be used with "regulator-output" to be able to control it from sysfs or if the amplifier ends up in the devicetree for other reasons, the amplifier driver could control the regulator instead of requiring it to by done in sysfs.
On 01/07, David Lechner wrote: > On 1/7/25 9:26 AM, Jonathan Santos wrote: > > From: Sergiu Cuciurean <sergiu.cuciurean@analog.com> > > > > The VCM output voltage can be used as a common-mode voltage within the > > amplifier preconditioning circuits external to the AD7768-1. > > > > This change exports the VCM as an iio attribute and makes it available > > for configuration. > > We model common mode voltage inputs as a regulator consumer (e.g. vcm-supply in > DT), so should we make this a regulator provider instead? > > It could be used with "regulator-output" to be able to control it from sysfs > or if the amplifier ends up in the devicetree for other reasons, the amplifier > driver could control the regulator instead of requiring it to by done in sysfs. > Ok, that is an interesting approach, I will try this. It makes sense to have it in the devicetree.
On Sat, 11 Jan 2025 23:37:29 -0300 Jonathan Santos <jonath4nns@gmail.com> wrote: > On 01/07, David Lechner wrote: > > On 1/7/25 9:26 AM, Jonathan Santos wrote: > > > From: Sergiu Cuciurean <sergiu.cuciurean@analog.com> > > > > > > The VCM output voltage can be used as a common-mode voltage within the > > > amplifier preconditioning circuits external to the AD7768-1. > > > > > > This change exports the VCM as an iio attribute and makes it available > > > for configuration. > > > > We model common mode voltage inputs as a regulator consumer (e.g. vcm-supply in > > DT), so should we make this a regulator provider instead? > > > > It could be used with "regulator-output" to be able to control it from sysfs > > or if the amplifier ends up in the devicetree for other reasons, the amplifier > > driver could control the regulator instead of requiring it to by done in sysfs. > > > > Ok, that is an interesting approach, I will try this. It makes sense to > have it in the devicetree. I'd be a bit surprised if this ended up being varied in real systems. So maybe start with a solution that has DT provide a value suitable to the amplifier (ideally because the amplifier requests it) and we can figure more dynamic stuff out later if needed. Jonathan
diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c index eaa9a12737ac..574d735f2c3a 100644 --- a/drivers/iio/adc/ad7768-1.c +++ b/drivers/iio/adc/ad7768-1.c @@ -119,6 +119,17 @@ struct ad7768_clk_configuration { enum ad7768_pwrmode pwrmode; }; +static const char * const ad7768_vcm_modes[] = { + "(AVDD1-AVSS)/2", + "2V5", + "2V05", + "1V9", + "1V65", + "1V1", + "0V9", + "OFF", +}; + static const struct ad7768_clk_configuration ad7768_clk_config[] = { { AD7768_MCLK_DIV_2, AD7768_DEC_RATE_8, 16, AD7768_FAST_MODE }, { AD7768_MCLK_DIV_2, AD7768_DEC_RATE_16, 32, AD7768_FAST_MODE }, @@ -133,12 +144,34 @@ static const struct ad7768_clk_configuration ad7768_clk_config[] = { { AD7768_MCLK_DIV_16, AD7768_DEC_RATE_1024, 16384, AD7768_ECO_MODE }, }; +static int ad7768_get_vcm(struct iio_dev *dev, const struct iio_chan_spec *chan); +static int ad7768_set_vcm(struct iio_dev *dev, const struct iio_chan_spec *chan, + unsigned int mode); + +static const struct iio_enum ad7768_vcm_mode_enum = { + .items = ad7768_vcm_modes, + .num_items = ARRAY_SIZE(ad7768_vcm_modes), + .set = ad7768_set_vcm, + .get = ad7768_get_vcm, +}; + +static struct iio_chan_spec_ext_info ad7768_ext_info[] = { + IIO_ENUM("common_mode_voltage", + IIO_SHARED_BY_ALL, + &ad7768_vcm_mode_enum), + IIO_ENUM_AVAILABLE("common_mode_voltage", + IIO_SHARED_BY_ALL, + &ad7768_vcm_mode_enum), + { }, +}; + static const struct iio_chan_spec ad7768_channels[] = { { .type = IIO_VOLTAGE, .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), + .ext_info = ad7768_ext_info, .indexed = 1, .channel = 0, .scan_index = 0, @@ -159,6 +192,7 @@ struct ad7768_state { struct clk *mclk; unsigned int mclk_freq; unsigned int samp_freq; + unsigned int common_mode_voltage; struct completion completion; struct iio_trigger *trig; struct gpio_desc *gpio_sync_in; @@ -329,6 +363,28 @@ static int ad7768_set_freq(struct ad7768_state *st, return 0; } +static int ad7768_get_vcm(struct iio_dev *dev, const struct iio_chan_spec *chan) +{ + struct ad7768_state *st = iio_priv(dev); + + return st->common_mode_voltage; +} + +static int ad7768_set_vcm(struct iio_dev *dev, + const struct iio_chan_spec *chan, + unsigned int mode) +{ + int ret; + struct ad7768_state *st = iio_priv(dev); + + ret = ad7768_spi_reg_write(st, AD7768_REG_ANALOG2, mode); + + if (ret == 0) + st->common_mode_voltage = mode; + + return ret; +} + static ssize_t ad7768_sampling_freq_avail(struct device *dev, struct device_attribute *attr, char *buf)