Re: [PATCH v3 8/9] iio: adc: ti-ads1262: support REFOUT and VBIAS regulators

On 8/7/26 10:58 PM, Kurt Borja wrote:
> Register the "refout" and "vbias" regulators to be able to use them as
> common mode supplies.
> 
> Signed-off-by: Kurt Borja <[email protected]>
> ---
>  drivers/iio/adc/Kconfig      |  1 +
>  drivers/iio/adc/ti-ads1262.c | 90 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 91 insertions(+)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index b9b561be8347..e5206438ac90 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -1845,6 +1845,7 @@ config TI_ADS1262
>  	tristate "Texas Instruments ADS1262"
>  	depends on SPI
>  	select REGMAP
> +	select REGULATOR
>  	select IIO_BUFFER
>  	select IIO_TRIGGERED_BUFFER
>  	help
> diff --git a/drivers/iio/adc/ti-ads1262.c b/drivers/iio/adc/ti-ads1262.c
> index 24a7ecb9fbd4..533574169b04 100644
> --- a/drivers/iio/adc/ti-ads1262.c
> +++ b/drivers/iio/adc/ti-ads1262.c
> @@ -26,6 +26,7 @@
>  #include <linux/property.h>
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/regulator/driver.h>
>  #include <linux/spi/spi.h>
>  #include <linux/string.h>
>  #include <linux/types.h>
> @@ -961,6 +962,91 @@ static irqreturn_t ads1262_irq_handler(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>  
> +static int ads1262_regulator_enable(struct regulator_dev *rdev)
> +{
> +	struct ads1262 *st = rdev_get_drvdata(rdev);
> +
> +	guard(mutex)(&st->xfer_lock);
> +
> +	return regmap_set_bits(st->regmap, ADS1262_POWER_REG,
> +			       ADS1262_POWER_VBIAS_MASK);
> +}
> +
> +static int ads1262_regulator_disable(struct regulator_dev *rdev)
> +{
> +	struct ads1262 *st = rdev_get_drvdata(rdev);
> +
> +	guard(mutex)(&st->xfer_lock);
> +
> +	return regmap_clear_bits(st->regmap, ADS1262_POWER_REG,
> +				 ADS1262_POWER_VBIAS_MASK);
> +}
> +
> +static int ads1262_regulator_is_enabled(struct regulator_dev *rdev)
> +{
> +	struct ads1262 *st = rdev_get_drvdata(rdev);
> +	unsigned int val;
> +	int ret;
> +
> +	guard(mutex)(&st->xfer_lock);
> +
> +	ret = regmap_read(st->regmap, ADS1262_POWER_REG, &val);

Can be a bit simpler with regmap_test_bits().

> +	if (ret)
> +		return ret;
> +
> +	return field_get(ADS1262_POWER_VBIAS_MASK, val);
> +}
> +
> +static const struct regulator_ops ads1262_vbias_regulator_ops = {
> +	.enable = ads1262_regulator_enable,
> +	.disable = ads1262_regulator_disable,
> +	.is_enabled = ads1262_regulator_is_enabled,

This should also have a get_voltage() op that returns
(VAVDD + VAVSS) / 2. Otherwise it won't be usable as a
common mode voltage.

> +};
> +
> +static const struct regulator_ops ads1262_refout_regulator_ops = { };
> +
> +static const struct regulator_desc ads1262_vbias_regulator_desc = {
> +	.name = "vbias",
> +	.of_match = "vbias",
> +	.regulators_node = "regulators",
> +	.supply_name = "avdd",

What does supply_name do? Make "avdd-supply" the parent supply?

> +	.ops = &ads1262_vbias_regulator_ops,
> +	.type = REGULATOR_VOLTAGE,
> +	.owner = THIS_MODULE,
> +};
> +
> +static const struct regulator_desc ads1262_refout_regulator_desc = {
> +	.name = "refout",
> +	.of_match = "refout",
> +	.regulators_node = "regulators",
> +	.supply_name = "avdd",
> +	.n_voltages = 1,
> +	.fixed_uV = 2500000,
> +	.ops = &ads1262_refout_regulator_ops,
> +	.type = REGULATOR_VOLTAGE,
> +	.owner = THIS_MODULE,
> +};
> +
> +static int ads1262_register_regulators(struct ads1262 *st)
> +{
> +	struct device *dev = &st->spi->dev;
> +	struct regulator_config config = {
> +		.dev = dev,
> +		.driver_data = st,
> +	};
> +	struct regulator_dev *rdev;

Should we do...

	if (!device_property_present(dev, "regulators"))
		return 0;

here since regulators is not a required property?

> +
> +	rdev = devm_regulator_register(dev, &ads1262_refout_regulator_desc,
> +				       &config);
> +	if (IS_ERR(rdev))
> +		return PTR_ERR(rdev);
> +
> +	rdev = devm_regulator_register(dev, &ads1262_vbias_regulator_desc,
> +				       &config);
> +
> +	return PTR_ERR_OR_ZERO(rdev);
> +}
> +
>  static int ads1262_dev_configure(struct ads1262 *st)
>  {
>  	struct device *dev = &st->spi->dev;
> @@ -1707,6 +1793,10 @@ static int ads1262_spi_probe(struct spi_device *spi)
>  	if (ret)
>  		return dev_err_probe(dev, ret, "failed to configure device\n");
>  
> +	ret = ads1262_register_regulators(st);
> +	if (ret)
> +		return ret;
> +
>  	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
>  					      iio_pollfunc_store_time,
>  					      ads1262_trigger_handler,
>