Re: [PATCH v3 3/3] iio: adc: ti-ads112c14: add continuous mode support

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 07 Aug 2026 16:19:48 -0500
"David Lechner (TI)" <dlechner@baylibre.com> wrote:

> Add support for continuous mode in the TI ADS112C14 ADC driver. In this
> mode the ADC itself is starting each conversion, so we add a trigger
> based on the DRDY interrupt to read each sample. This mode is also
> limited in that only one channel can be enabled at a time since the
> chip does not have a sequencer or simultaneous sampling capability.
> Continuous mode will only be used when this new trigger is the current
> trigger.
> 
> Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
Hi David,

regmap_assign_bits() usage here is a bit odd.
It is just a bool taking wrapper around set_bits and clear_bits.

It 'works' here because the values are 0 and 1.
There isn't a natural bool for these two modes, so to make this look right
you'd end up with something like:

ADS112C14_DEVICE_CFG_CONV_MODE_SINGLE_NOT_CONT 
and that is horrible. So I'd just use FIELD_PREP() and definitions
for the two values.


Otherwise looks fine to me.

J
> +
> +static int ads112c14_buffer_predisable(struct iio_dev *indio_dev)
> +{
> +	struct ads112c14_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	if (!ads112c14_using_drdy_trigger(indio_dev))
> +		return 0;
> +
> +	guard(mutex)(&data->lock);
> +
> +	ret = regmap_write(data->regmap, ADS112C14_REG_CONVERSION_CTRL,
> +			   ADS112C14_CONVERSION_CTRL_STOP);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_assign_bits(data->regmap, ADS112C14_REG_DEVICE_CFG,
> +				  ADS112C14_DEVICE_CFG_CONV_MODE,
> +				  ADS112C14_DEVICE_CFG_CONV_MODE_SINGLE_SHOT);

This looks odd as last parameter that takes is a boolean.
I think you just want an update_bits + appropriate FIELD_PREP()


> +}
> +
> +static const struct iio_buffer_setup_ops ads112c14_buffer_setup_ops = {
> +	.postenable = ads112c14_buffer_postenable,
> +	.predisable = ads112c14_buffer_predisable,
> +	.validate_scan_mask = ads112c14_validate_scan_mask,
> +};
> +
>  static int ads112c14_populate_idac_mag(u32 current_nA, u8 *idac_mag)
>  {
>  	u32 current_uA = current_nA / (NANO / MICRO);
> @@ -1480,6 +1594,19 @@ static int ads112c14_probe(struct i2c_client *client)
>  				       0, dev_name(dev), indio_dev);
>  		if (ret)
>  			return ret;
> +
> +		data->drdy_trig = devm_iio_trigger_alloc(dev, "%s-dev%d-drdy",
> +							 info->name,
> +							 iio_device_id(indio_dev));
> +		if (!data->drdy_trig)
> +			return -ENOMEM;
> +
> +		data->drdy_trig->ops = &ads112c14_trigger_ops;
> +		iio_trigger_set_drvdata(data->drdy_trig, indio_dev);
> +
> +		ret = devm_iio_trigger_register(dev, data->drdy_trig);
> +		if (ret)
> +			return ret;
>  	}
>  
>  	ads112c14_populate_tables(data);
> @@ -1490,7 +1617,8 @@ static int ads112c14_probe(struct i2c_client *client)
>  
>  	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
>  					      iio_pollfunc_store_time,
> -					      ads112c14_trigger_handler, NULL);
> +					      ads112c14_trigger_handler,
> +					      &ads112c14_buffer_setup_ops);
>  	if (ret)
>  		return ret;
>  
>