linux-kernel - Re: [PATCH 07/15] iio: adc: ad4030: Add SPI offload support

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: <aLcEvmwbysTaEprV@debian-BULLSEYE-live-builder-AMD64>
Date: Tue, 2 Sep 2025 11:52:46 -0300
From: Marcelo Schmitt <marcelo.schmitt1@...il.com>
To: David Lechner <dlechner@...libre.com>
Cc: Marcelo Schmitt <marcelo.schmitt@...log.com>, linux-iio@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
	devicetree@...r.kernel.org, linux-spi@...r.kernel.org,
	jic23@...nel.org, Michael.Hennerich@...log.com, nuno.sa@...log.com,
	eblanc@...libre.com, andy@...nel.org, corbet@....net,
	robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
	broonie@...nel.org, Jonathan.Cameron@...wei.com,
	andriy.shevchenko@...ux.intel.com, ahaslam@...libre.com,
	sergiu.cuciurean@...log.com, tgamblin@...libre.com
Subject: Re: [PATCH 07/15] iio: adc: ad4030: Add SPI offload support

Hi David,

On 08/30, David Lechner wrote:
> On 8/29/25 7:42 PM, Marcelo Schmitt wrote:
> > AD4030 and similar ADCs can capture data at sample rates up to 2 mega
> > samples per second (MSPS). Not all SPI controllers are able to achieve
> > such high throughputs and even when the controller is fast enough to run
> > transfers at the required speed, it may be costly to the CPU to handle
> > transfer data at such high sample rates.  Add SPI offload support for
> > AD4030 and similar ADCs so to enable ADC data capture at maximum sample
> > rates.
> > 
...
> > +
> > +static int __ad4030_set_sampling_freq(struct ad4030_state *st, unsigned int freq)
> > +{
...
> > +	do {
> > +		conv_wf.duty_length_ns = target;
> > +		ret = pwm_round_waveform_might_sleep(st->conv_trigger, &conv_wf);
> > +		if (ret)
> > +			return ret;
> > +		target += 10;
> > +	} while (conv_wf.duty_length_ns < 10);
> > +
> > +	offload_period_ns = conv_wf.period_length_ns;
> > +
> > +	ret = regmap_read(st->regmap, AD4030_REG_MODES, &mode);
> > +	if (ret)
> > +		return ret;
> > +	if (FIELD_GET(AD4030_REG_MODES_MASK_OUT_DATA_MODE, mode) == AD4030_OUT_DATA_MD_30_AVERAGED_DIFF) {
> 
> Since this depends on the oversampling ration, we need to defer this
> until we start a buffered read. Otherwise if someone sets sampling
> frequency first and the changes the oversampling ratio later, then
> the PWM period will not be correct.
> 

Yes, that's one of the problems I noticed when testing yesterday and probably
the reason why I initially thought the scale was buggy for averaging/oversampling.

> Alternatly, we could update this both when sampling freqency and
> when oversampling ratio are updated. This would allow returning an
> error if the oversampling ratio is too big for the requested
> sampling frequency.
> 

Sure, this sounds to be the best way of keeping track of the sampling frequency
and oversampling ratio combination. Otherwise, unadvised users (like me) could
set the oversampling ratio then run transfers without the driver being able to
update the CNV trigger waveform according to the number of samples to average.
The result of that is the device doesn't do oversampling (despite the
oversampling attribute value).

I also see your comments to this and other patches and I think I agree with them
all. Will re-spin to apply the requested changes and provide support for SPI
related stuff in a separate series.

Thanks,
Marcelo