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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aEig-gz5_fTEheiW@smile.fi.intel.com>
Date: Wed, 11 Jun 2025 00:17:46 +0300
From: Andy Shevchenko <andy@...nel.org>
To: Marcelo Schmitt <marcelo.schmitt@...log.com>
Cc: linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
	linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
	jic23@...nel.org, lars@...afoo.de, Michael.Hennerich@...log.com,
	dlechner@...libre.com, nuno.sa@...log.com, robh@...nel.org,
	krzk+dt@...nel.org, conor+dt@...nel.org, linus.walleij@...aro.org,
	brgl@...ev.pl, marcelo.schmitt1@...il.com
Subject: Re: [PATCH v5 06/11] iio: adc: ad4170: Add support for buffered data
 capture

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