| Message ID | 40c586006b5cee0570ae577db2b58e6e7e36a6e6.1741268122.git.Jonathan.Santos@analog.com |
|---|---|
| State | New |
| Headers | show |
| Series | iio: adc: ad7768-1: Add features, improvements, and fixes | expand |
Looks ok. One minor commnet inline. On 03/06, Jonathan Santos wrote: > The VCM output voltage can be used as a common-mode voltage within the > amplifier preconditioning circuits external to the AD7768-1. > > This change allows the user to configure VCM output using the regulator > framework. > > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> Acked-by: Marcelo Schmitt <marcelo.schmitt@analog.com> > --- > v4 Changes: > * Added iio_device_claim_direct_mode() to regulator callbacks to avoid register access > while in buffered mode. > * Changed regulator name to "ad7768-1-vcm". > * When regulator enable is called, it will set the last voltage selector configured. > * Disabled regulator before configuring it. > * Adressed other nits. > > v3 Changes: > * Register VCM output via the regulator framework for improved flexibility > and external integration. > > v2 Changes: > * VCM output support is now defined by a devicetree property, instead of > and IIO attribute. > --- > drivers/iio/adc/Kconfig | 1 + > drivers/iio/adc/ad7768-1.c | 181 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 182 insertions(+) > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index a2fdb7e03a66..d8f2ed477ba7 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -277,6 +277,7 @@ config AD7766 > config AD7768_1 > tristate "Analog Devices AD7768-1 ADC driver" > depends on SPI > + select REGULATOR > select REGMAP_SPI > select IIO_BUFFER > select IIO_TRIGGER > diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c > index e88e9431bb7a..2a6317f5b582 100644 > --- a/drivers/iio/adc/ad7768-1.c > +++ b/drivers/iio/adc/ad7768-1.c > @@ -12,8 +12,10 @@ > #include <linux/gpio/consumer.h> > #include <linux/kernel.h> > #include <linux/module.h> > +#include <linux/of.h> > #include <linux/regmap.h> > #include <linux/regulator/consumer.h> > +#include <linux/regulator/driver.h> > #include <linux/sysfs.h> > #include <linux/spi/spi.h> > > @@ -80,9 +82,15 @@ > #define AD7768_CONV_MODE_MSK GENMASK(2, 0) > #define AD7768_CONV_MODE(x) FIELD_PREP(AD7768_CONV_MODE_MSK, x) > > +/* AD7768_REG_ANALOG2 */ > +#define AD7768_REG_ANALOG2_VCM_MSK GENMASK(2, 0) > +#define AD7768_REG_ANALOG2_VCM(x) FIELD_PREP(AD7768_REG_ANALOG2_VCM_MSK, x) Should we enforce macro argument evaluation here? #define AD7768_REG_ANALOG2_VCM(x) FIELD_PREP(AD7768_REG_ANALOG2_VCM_MSK, (x)) > + > #define AD7768_RD_FLAG_MSK(x) (BIT(6) | ((x) & 0x3F)) > #define AD7768_WR_FLAG_MSK(x) ((x) & 0x3F) > > +#define AD7768_VCM_OFF 0x07 > + > enum ad7768_conv_mode { > AD7768_CONTINUOUS, > AD7768_ONE_SHOT, > @@ -160,6 +168,8 @@ struct ad7768_state { > struct regmap *regmap; > struct regmap *regmap24; > struct regulator *vref; > + struct regulator_dev *vcm_rdev; > + unsigned int vcm_output_sel; > struct clk *mclk; > unsigned int mclk_freq; > unsigned int samp_freq; > @@ -644,6 +654,172 @@ static int ad7768_triggered_buffer_alloc(struct iio_dev *indio_dev) > &ad7768_buffer_ops); > } > > +static int ad7768_vcm_enable(struct regulator_dev *rdev) > +{ > + struct iio_dev *indio_dev = rdev_get_drvdata(rdev); > + struct ad7768_state *st = iio_priv(indio_dev); > + int ret, regval; > + > + if (!indio_dev) > + return -EINVAL; > + > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret) > + return ret; > + > + /* To enable, set the last selected output */ > + regval = AD7768_REG_ANALOG2_VCM(st->vcm_output_sel + 1); > + ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2, > + AD7768_REG_ANALOG2_VCM_MSK, regval); > + iio_device_release_direct_mode(indio_dev); > + > + return ret; > +} > + > +static int ad7768_vcm_disable(struct regulator_dev *rdev) > +{ > + struct iio_dev *indio_dev = rdev_get_drvdata(rdev); > + struct ad7768_state *st = iio_priv(indio_dev); > + int ret; > + > + if (!indio_dev) > + return -EINVAL; > + > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret) > + return ret; > + > + ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2, > + AD7768_REG_ANALOG2_VCM_MSK, AD7768_VCM_OFF); > + iio_device_release_direct_mode(indio_dev); > + > + return ret; > +} > + > +static int ad7768_vcm_is_enabled(struct regulator_dev *rdev) > +{ > + struct iio_dev *indio_dev = rdev_get_drvdata(rdev); > + struct ad7768_state *st = iio_priv(indio_dev); > + int ret, val; > + > + if (!indio_dev) > + return -EINVAL; > + > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret) > + return ret; > + > + ret = regmap_read(st->regmap, AD7768_REG_ANALOG2, &val); > + if (ret) > + goto err_release; > + > + ret = FIELD_GET(AD7768_REG_ANALOG2_VCM_MSK, val) != AD7768_VCM_OFF; > +err_release: > + iio_device_release_direct_mode(indio_dev); > + > + return ret; > +} > + > +static int ad7768_set_voltage_sel(struct regulator_dev *rdev, > + unsigned int selector) > +{ > + unsigned int regval = AD7768_REG_ANALOG2_VCM(selector + 1); > + struct iio_dev *indio_dev = rdev_get_drvdata(rdev); > + struct ad7768_state *st = iio_priv(indio_dev); > + int ret; > + > + if (!indio_dev) > + return -EINVAL; > + > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret) > + return ret; > + > + ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2, > + AD7768_REG_ANALOG2_VCM_MSK, regval); > + iio_device_release_direct_mode(indio_dev); > + st->vcm_output_sel = selector; > + > + return ret; > +} > + > +static int ad7768_get_voltage_sel(struct regulator_dev *rdev) > +{ > + struct iio_dev *indio_dev = rdev_get_drvdata(rdev); > + struct ad7768_state *st = iio_priv(indio_dev); > + int ret, val; > + > + if (!indio_dev) > + return -EINVAL; > + > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret) > + return ret; > + > + ret = regmap_read(st->regmap, AD7768_REG_ANALOG2, &val); > + if (ret) > + goto err_release; > + > + val = FIELD_GET(AD7768_REG_ANALOG2_VCM_MSK, val); > + ret = clamp(val, 1, (int)rdev->desc->n_voltages) - 1; > +err_release: > + iio_device_release_direct_mode(indio_dev); > + > + return ret; > +} > + > +static const struct regulator_ops vcm_regulator_ops = { > + .enable = ad7768_vcm_enable, > + .disable = ad7768_vcm_disable, > + .is_enabled = ad7768_vcm_is_enabled, > + .list_voltage = regulator_list_voltage_table, > + .set_voltage_sel = ad7768_set_voltage_sel, > + .get_voltage_sel = ad7768_get_voltage_sel, > +}; > + > +static const unsigned int vcm_voltage_table[] = { > + 2500000, > + 2050000, > + 1650000, > + 1900000, > + 1100000, > + 900000, > +}; > + > +static const struct regulator_desc vcm_desc = { > + .name = "ad7768-1-vcm", > + .of_match = of_match_ptr("vcm-output"), > + .regulators_node = of_match_ptr("regulators"), > + .n_voltages = ARRAY_SIZE(vcm_voltage_table), > + .volt_table = vcm_voltage_table, > + .ops = &vcm_regulator_ops, > + .type = REGULATOR_VOLTAGE, > + .owner = THIS_MODULE, > +}; > + > +static int ad7768_register_regulators(struct device *dev, struct ad7768_state *st, > + struct iio_dev *indio_dev) > +{ > + struct regulator_config config = { > + .dev = dev, > + .driver_data = indio_dev, > + }; > + int ret; > + > + /* Disable the regulator before registering it */ > + ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2, > + AD7768_REG_ANALOG2_VCM_MSK, AD7768_VCM_OFF); > + if (ret) > + return -EINVAL; > + > + st->vcm_rdev = devm_regulator_register(dev, &vcm_desc, &config); > + if (IS_ERR(st->vcm_rdev)) > + return dev_err_probe(dev, PTR_ERR(st->vcm_rdev), > + "failed to register VCM regulator\n"); > + > + return 0; > +} > + > static int ad7768_probe(struct spi_device *spi) > { > struct ad7768_state *st; > @@ -708,6 +884,11 @@ static int ad7768_probe(struct spi_device *spi) > indio_dev->info = &ad7768_info; > indio_dev->modes = INDIO_DIRECT_MODE; > > + /* Register VCM output regulator */ > + ret = ad7768_register_regulators(&spi->dev, st, indio_dev); > + if (ret) > + return ret; > + > ret = ad7768_setup(st); > if (ret < 0) { > dev_err(&spi->dev, "AD7768 setup failed\n"); > -- > 2.34.1 >
On Thu, 6 Mar 2025 18:02:59 -0300 Jonathan Santos <Jonathan.Santos@analog.com> wrote: > The VCM output voltage can be used as a common-mode voltage within the > amplifier preconditioning circuits external to the AD7768-1. > > This change allows the user to configure VCM output using the regulator > framework. > > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> Looks fine but as likely you will be doing a v5, please switch to the new iio_device_claim/release_direct() functions. If I had applied up to here I'd probably just have tweaked this whilst applying but given a few other tweaks needed, please do this one as well for v5. Thanks, Jonathan > @@ -644,6 +654,172 @@ static int ad7768_triggered_buffer_alloc(struct iio_dev *indio_dev) > &ad7768_buffer_ops); > } > > +static int ad7768_vcm_enable(struct regulator_dev *rdev) > +{ > + struct iio_dev *indio_dev = rdev_get_drvdata(rdev); > + struct ad7768_state *st = iio_priv(indio_dev); > + int ret, regval; > + > + if (!indio_dev) > + return -EINVAL; > + > + ret = iio_device_claim_direct_mode(indio_dev); As below. > + if (ret) > + return ret; > + > + /* To enable, set the last selected output */ > + regval = AD7768_REG_ANALOG2_VCM(st->vcm_output_sel + 1); > + ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2, > + AD7768_REG_ANALOG2_VCM_MSK, regval); > + iio_device_release_direct_mode(indio_dev); > + > + return ret; > +} > + > +static int ad7768_vcm_disable(struct regulator_dev *rdev) > +{ > + struct iio_dev *indio_dev = rdev_get_drvdata(rdev); > + struct ad7768_state *st = iio_priv(indio_dev); > + int ret; > + > + if (!indio_dev) > + return -EINVAL; > + > + ret = iio_device_claim_direct_mode(indio_dev); As looks likely you'll be doing a v5 for other minor stuff please rebase on the testing branch of iio.git (as I've picked up some of this series) or togreg once that is pushed out and use if (!iio_device_claim_direct(indio_dev)) return -EBUSY; + iio_device_release_direct() to get the variants adjusted to play better with sparse. > + if (ret) > + return ret; > + > + ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2, > + AD7768_REG_ANALOG2_VCM_MSK, AD7768_VCM_OFF); > + iio_device_release_direct_mode(indio_dev); > + > + return ret; > +} > + > +static int ad7768_vcm_is_enabled(struct regulator_dev *rdev) > +{ > + struct iio_dev *indio_dev = rdev_get_drvdata(rdev); > + struct ad7768_state *st = iio_priv(indio_dev); > + int ret, val; > + > + if (!indio_dev) > + return -EINVAL; > + > + ret = iio_device_claim_direct_mode(indio_dev); As above. > + if (ret) > + return ret; > + > + ret = regmap_read(st->regmap, AD7768_REG_ANALOG2, &val); > + if (ret) > + goto err_release; > + > + ret = FIELD_GET(AD7768_REG_ANALOG2_VCM_MSK, val) != AD7768_VCM_OFF; > +err_release: > + iio_device_release_direct_mode(indio_dev); > + > + return ret; > +} > + > +static int ad7768_set_voltage_sel(struct regulator_dev *rdev, > + unsigned int selector) > +{ > + unsigned int regval = AD7768_REG_ANALOG2_VCM(selector + 1); > + struct iio_dev *indio_dev = rdev_get_drvdata(rdev); > + struct ad7768_state *st = iio_priv(indio_dev); > + int ret; > + > + if (!indio_dev) > + return -EINVAL; > + > + ret = iio_device_claim_direct_mode(indio_dev); As above. > + if (ret) > + return ret; > + > + ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2, > + AD7768_REG_ANALOG2_VCM_MSK, regval); > + iio_device_release_direct_mode(indio_dev); > + st->vcm_output_sel = selector; > + > + return ret; > +} > + > +static int ad7768_get_voltage_sel(struct regulator_dev *rdev) > +{ > + struct iio_dev *indio_dev = rdev_get_drvdata(rdev); > + struct ad7768_state *st = iio_priv(indio_dev); > + int ret, val; > + > + if (!indio_dev) > + return -EINVAL; > + > + ret = iio_device_claim_direct_mode(indio_dev); As above. > + if (ret) > + return ret; > + > + ret = regmap_read(st->regmap, AD7768_REG_ANALOG2, &val); > + if (ret) > + goto err_release; > + > + val = FIELD_GET(AD7768_REG_ANALOG2_VCM_MSK, val); > + ret = clamp(val, 1, (int)rdev->desc->n_voltages) - 1; > +err_release: > + iio_device_release_direct_mode(indio_dev); > + > + return ret; > +}
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index a2fdb7e03a66..d8f2ed477ba7 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -277,6 +277,7 @@ config AD7766 config AD7768_1 tristate "Analog Devices AD7768-1 ADC driver" depends on SPI + select REGULATOR select REGMAP_SPI select IIO_BUFFER select IIO_TRIGGER diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c index e88e9431bb7a..2a6317f5b582 100644 --- a/drivers/iio/adc/ad7768-1.c +++ b/drivers/iio/adc/ad7768-1.c @@ -12,8 +12,10 @@ #include <linux/gpio/consumer.h> #include <linux/kernel.h> #include <linux/module.h> +#include <linux/of.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h> +#include <linux/regulator/driver.h> #include <linux/sysfs.h> #include <linux/spi/spi.h> @@ -80,9 +82,15 @@ #define AD7768_CONV_MODE_MSK GENMASK(2, 0) #define AD7768_CONV_MODE(x) FIELD_PREP(AD7768_CONV_MODE_MSK, x) +/* AD7768_REG_ANALOG2 */ +#define AD7768_REG_ANALOG2_VCM_MSK GENMASK(2, 0) +#define AD7768_REG_ANALOG2_VCM(x) FIELD_PREP(AD7768_REG_ANALOG2_VCM_MSK, x) + #define AD7768_RD_FLAG_MSK(x) (BIT(6) | ((x) & 0x3F)) #define AD7768_WR_FLAG_MSK(x) ((x) & 0x3F) +#define AD7768_VCM_OFF 0x07 + enum ad7768_conv_mode { AD7768_CONTINUOUS, AD7768_ONE_SHOT, @@ -160,6 +168,8 @@ struct ad7768_state { struct regmap *regmap; struct regmap *regmap24; struct regulator *vref; + struct regulator_dev *vcm_rdev; + unsigned int vcm_output_sel; struct clk *mclk; unsigned int mclk_freq; unsigned int samp_freq; @@ -644,6 +654,172 @@ static int ad7768_triggered_buffer_alloc(struct iio_dev *indio_dev) &ad7768_buffer_ops); } +static int ad7768_vcm_enable(struct regulator_dev *rdev) +{ + struct iio_dev *indio_dev = rdev_get_drvdata(rdev); + struct ad7768_state *st = iio_priv(indio_dev); + int ret, regval; + + if (!indio_dev) + return -EINVAL; + + ret = iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + + /* To enable, set the last selected output */ + regval = AD7768_REG_ANALOG2_VCM(st->vcm_output_sel + 1); + ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2, + AD7768_REG_ANALOG2_VCM_MSK, regval); + iio_device_release_direct_mode(indio_dev); + + return ret; +} + +static int ad7768_vcm_disable(struct regulator_dev *rdev) +{ + struct iio_dev *indio_dev = rdev_get_drvdata(rdev); + struct ad7768_state *st = iio_priv(indio_dev); + int ret; + + if (!indio_dev) + return -EINVAL; + + ret = iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + + ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2, + AD7768_REG_ANALOG2_VCM_MSK, AD7768_VCM_OFF); + iio_device_release_direct_mode(indio_dev); + + return ret; +} + +static int ad7768_vcm_is_enabled(struct regulator_dev *rdev) +{ + struct iio_dev *indio_dev = rdev_get_drvdata(rdev); + struct ad7768_state *st = iio_priv(indio_dev); + int ret, val; + + if (!indio_dev) + return -EINVAL; + + ret = iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + + ret = regmap_read(st->regmap, AD7768_REG_ANALOG2, &val); + if (ret) + goto err_release; + + ret = FIELD_GET(AD7768_REG_ANALOG2_VCM_MSK, val) != AD7768_VCM_OFF; +err_release: + iio_device_release_direct_mode(indio_dev); + + return ret; +} + +static int ad7768_set_voltage_sel(struct regulator_dev *rdev, + unsigned int selector) +{ + unsigned int regval = AD7768_REG_ANALOG2_VCM(selector + 1); + struct iio_dev *indio_dev = rdev_get_drvdata(rdev); + struct ad7768_state *st = iio_priv(indio_dev); + int ret; + + if (!indio_dev) + return -EINVAL; + + ret = iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + + ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2, + AD7768_REG_ANALOG2_VCM_MSK, regval); + iio_device_release_direct_mode(indio_dev); + st->vcm_output_sel = selector; + + return ret; +} + +static int ad7768_get_voltage_sel(struct regulator_dev *rdev) +{ + struct iio_dev *indio_dev = rdev_get_drvdata(rdev); + struct ad7768_state *st = iio_priv(indio_dev); + int ret, val; + + if (!indio_dev) + return -EINVAL; + + ret = iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + + ret = regmap_read(st->regmap, AD7768_REG_ANALOG2, &val); + if (ret) + goto err_release; + + val = FIELD_GET(AD7768_REG_ANALOG2_VCM_MSK, val); + ret = clamp(val, 1, (int)rdev->desc->n_voltages) - 1; +err_release: + iio_device_release_direct_mode(indio_dev); + + return ret; +} + +static const struct regulator_ops vcm_regulator_ops = { + .enable = ad7768_vcm_enable, + .disable = ad7768_vcm_disable, + .is_enabled = ad7768_vcm_is_enabled, + .list_voltage = regulator_list_voltage_table, + .set_voltage_sel = ad7768_set_voltage_sel, + .get_voltage_sel = ad7768_get_voltage_sel, +}; + +static const unsigned int vcm_voltage_table[] = { + 2500000, + 2050000, + 1650000, + 1900000, + 1100000, + 900000, +}; + +static const struct regulator_desc vcm_desc = { + .name = "ad7768-1-vcm", + .of_match = of_match_ptr("vcm-output"), + .regulators_node = of_match_ptr("regulators"), + .n_voltages = ARRAY_SIZE(vcm_voltage_table), + .volt_table = vcm_voltage_table, + .ops = &vcm_regulator_ops, + .type = REGULATOR_VOLTAGE, + .owner = THIS_MODULE, +}; + +static int ad7768_register_regulators(struct device *dev, struct ad7768_state *st, + struct iio_dev *indio_dev) +{ + struct regulator_config config = { + .dev = dev, + .driver_data = indio_dev, + }; + int ret; + + /* Disable the regulator before registering it */ + ret = regmap_update_bits(st->regmap, AD7768_REG_ANALOG2, + AD7768_REG_ANALOG2_VCM_MSK, AD7768_VCM_OFF); + if (ret) + return -EINVAL; + + st->vcm_rdev = devm_regulator_register(dev, &vcm_desc, &config); + if (IS_ERR(st->vcm_rdev)) + return dev_err_probe(dev, PTR_ERR(st->vcm_rdev), + "failed to register VCM regulator\n"); + + return 0; +} + static int ad7768_probe(struct spi_device *spi) { struct ad7768_state *st; @@ -708,6 +884,11 @@ static int ad7768_probe(struct spi_device *spi) indio_dev->info = &ad7768_info; indio_dev->modes = INDIO_DIRECT_MODE; + /* Register VCM output regulator */ + ret = ad7768_register_regulators(&spi->dev, st, indio_dev); + if (ret) + return ret; + ret = ad7768_setup(st); if (ret < 0) { dev_err(&spi->dev, "AD7768 setup failed\n");
The VCM output voltage can be used as a common-mode voltage within the amplifier preconditioning circuits external to the AD7768-1. This change allows the user to configure VCM output using the regulator framework. Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> --- v4 Changes: * Added iio_device_claim_direct_mode() to regulator callbacks to avoid register access while in buffered mode. * Changed regulator name to "ad7768-1-vcm". * When regulator enable is called, it will set the last voltage selector configured. * Disabled regulator before configuring it. * Adressed other nits. v3 Changes: * Register VCM output via the regulator framework for improved flexibility and external integration. v2 Changes: * VCM output support is now defined by a devicetree property, instead of and IIO attribute. --- drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/ad7768-1.c | 181 +++++++++++++++++++++++++++++++++++++ 2 files changed, 182 insertions(+)