Re: [PATCH v5 06/11] iio: adc: ad4170: Add support for buffered data capture

[Andy Shevchenko <andy@kernel.org>]

On Tue, Jun 10, 2025 at 05:32:43PM -0300, Marcelo Schmitt wrote:
> Extend the AD4170 driver to allow buffered data capture in continuous read
> mode. In continuous read mode, the chip skips the instruction phase and
> outputs just ADC sample data, enabling faster sample rates to be reached.
> The internal channel sequencer always starts sampling from channel 0 and
> channel 0 must be enabled if more than one channel is selected for data
> capture. The scan mask validation callback checks if the aforementioned
> condition is met.

...

> +static bool ad4170_validate_scan_mask(struct iio_dev *indio_dev,
> +				      const unsigned long *scan_mask)
> +{
> +	unsigned int masklength = iio_get_masklength(indio_dev);
> +
> +	/*
> +	 * The channel sequencer cycles through the enabled channels in
> +	 * sequential order, from channel 0 to channel 15, bypassing disabled
> +	 * channels. When more than one channel is enabled, channel 0 must
> +	 * always be enabled. See datasheet channel_en register description at
> +	 * page 95.
> +	 */
> +	if (bitmap_weight(scan_mask, masklength) > 1)
> +		return test_bit(0, scan_mask);
> +
> +	return bitmap_weight(scan_mask, masklength) == 1;

Hopefully compiler is smart enough to see the two calls for the same which
can't be modified on the fly, but it definitely can't assume that the
mask is one word long, meaning the bitmap API will choose the slow path
for them. I would rather take a temporary variable approach.

It also minimizes the risk of race conditions in case something changes the
data beneath (which is quite a nasty situation to begin with).

> +}

-- 
With Best Regards,
Andy Shevchenko