linux-kernel - Re: [PATCH v2] iio: adc: ad7124: fix sample rate for multi-channel use

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] [day] [month] [year] [list]

Message-ID: <88936759-77f5-479c-8cdb-121fc08f068f@baylibre.com>
Date: Thu, 4 Sep 2025 16:36:34 -0500
From: David Lechner <dlechner@...libre.com>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: Michael Hennerich <Michael.Hennerich@...log.com>,
 Jonathan Cameron <jic23@...nel.org>, Nuno Sá
 <nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] iio: adc: ad7124: fix sample rate for multi-channel
 use

On 9/4/25 1:21 PM, Andy Shevchenko wrote:
> On Thu, Sep 04, 2025 at 11:19:56AM -0500, David Lechner wrote:
>> Change how the FS[10:0] field of the FILTER register is calculated to
>> get consistent sample rates when only one channel is enabled vs when
>> multiple channels are enabled in a buffered read.
>>
>> By default, the AD7124 allows larger sampling frequencies when only one
>> channel is enabled. It assumes that you will discard the first sample or
>> so to allow for settling time and then no additional settling time is
>> needed between samples because there is no multiplexing due to only one
>> channel being enabled. The conversion formula to convert between the
>> sampling frequency and the FS[10:0] field is:
>>
>>     fADC = fCLK / (FS[10:0] x 32)
>>
>> which is what the driver has been using.
>>
>> On the other hand, when multiple channels are enabled, there is
>> additional settling time needed when switching between channels so the
>> calculation to convert between becomes:
>>
>>     fADC = fCLK / (FS[10:0] x 32 x (4 + AVG - 1))
>>
>> where AVG depends on the filter type selected and the power mode.
>>
>> The FILTER register has a SINGLE_CYCLE bit that can be set to force the
>> single channel case to use the same timing as the multi-channel case.
>>
>> Before this change, the first formula was always used, so if all of the
>> in_voltageY_sampling_frequency attributes were set to 10 Hz, then doing
>> a buffered read with 1 channel enabled would result in the requested
>> sampling frequency of 10 Hz. But when more than one channel was
>> enabled, the actual sampling frequency would be 2.5 Hz per channel,
>> which is 1/4 of the requested frequency.
>>
>> After this change, the SINGLE_CYCLE flag is now always enabled and the
>> multi-channel formula is now always used. This causes the sampling
>> frequency to be consistent regardless of the number of channels enabled.
>>
>> Technically, the sincx+sinc1 filter modes can't currently be selected
>> so there is some temporarily dead code in ad7124_get_avg() until filter
>> support is added.
>>
>> The AD7124_FILTER_FS define is moved while we are touching this to
>> keep the bit fields in descending order to be consistent with the rest
>> of the file.
> 
> ...
> 
>>  	tmp = FIELD_PREP(AD7124_FILTER_FILTER, cfg->filter_type) |
>> +		AD7124_FILTER_SINGLE_CYCLE |
>>  		FIELD_PREP(AD7124_FILTER_FS, cfg->odr_sel_bits);
> 
> Seems to me that this is not indented correctly, with that in mind I would
> 
> 	tmp = FIELD_PREP(AD7124_FILTER_FILTER, cfg->filter_type) |
> 	      FIELD_PREP(AD7124_FILTER_FS, cfg->odr_sel_bits) |
> 	      AD7124_FILTER_SINGLE_CYCLE;
> 
> 

All 3 of these lines are being removed in a later patch (haven't sent it to the
mailing list yet). So I will leave this as-is to keep the diff to a minimum.