Re: [PATCH 3/5] iio: ABI: add sinc4+sinc1+pf1 filter_type

[Andy Shevchenko]

On Fri, Aug 07, 2026 at 07:19:48PM -0500, David Lechner (TI) wrote:
> Document "sinc4+sinc1+pf1" as a valid value for the filter_type
> attributes (used by the ti-ads112c14 driver).

This patch does much more than that. And I can't easily see what, so many lines
shuffled. Seems like two patches were squashed indeliberately?

...

> +static int ads112c14_set_filter_type(struct iio_dev *indio_dev,
> +				     struct iio_chan_spec const *chan,
> +				     unsigned int val)
> +{
> +	struct ads112c14_data *data = iio_priv(indio_dev);
> +	struct ads112c14_channel_state *channel_state;
> +	int ret;
> +
> +	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> +	if (IIO_DEV_ACQUIRE_FAILED(claim))
> +		return -EBUSY;
> +
> +	guard(mutex)(&data->lock);
> +
> +	channel_state = &data->channel_states[chan->scan_index];
> +
> +	ret = ads112c14_get_filter_type_from_state(channel_state);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * channel_state->filter_osr affects multiple attributes, so don't modify
> +	 * it if the filter type is already set to the requested value.
> +	 */
> +	if (ret == val)
> +		return 0;
> +
> +	/* Otherwise, pick an arbitrary default for each type. */
> +	switch (val) {

I'm wondering if the above can be written as

	case ret:
		/* ...comment... */
		break;

(it is more of the curiosity about C rather than practical suggestion, as I
 understand the cons of this).

> +	case ADS112C14_FILTER_TYPE_SINC4:
> +		channel_state->filter_osr = ADS112C14_DATA_RATE_CFG_FLTR_OSR_16;
> +		break;
> +	case ADS112C14_FILTER_TYPE_SINC4_SINC1:
> +		channel_state->filter_osr = ADS112C14_DATA_RATE_CFG_FLTR_OSR_128;
> +		break;
> +	case ADS112C14_FILTER_TYPE_SINC4_SINC1_PF1:
> +		channel_state->filter_osr = ADS112C14_DATA_RATE_CFG_FLTR_OSR_25SPS;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko