[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aYnxCc1wv8MVfv2b@debian-BULLSEYE-live-builder-AMD64>
Date: Mon, 9 Feb 2026 11:36:57 -0300
From: Marcelo Schmitt <marcelo.schmitt1@...il.com>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: Marcelo Schmitt <marcelo.schmitt@...log.com>, linux-iio@...r.kernel.org,
devicetree@...r.kernel.org, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, jic23@...nel.org,
michael.hennerich@...log.com, nuno.sa@...log.com,
eblanc@...libre.com, dlechner@...libre.com, andy@...nel.org,
robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
corbet@....net, Trevor Gamblin <tgamblin@...libre.com>,
Axel Haslam <ahaslam@...libre.com>
Subject: Re: [PATCH v8 5/8] iio: adc: ad4030: Add SPI offload support
On 02/08, Andy Shevchenko wrote:
> On Fri, Feb 06, 2026 at 04:01:33PM -0300, 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 to enable data capture at maximum sample rates.
>
> ...
>
> > +static int ad4030_update_conversion_rate(struct ad4030_state *st,
> > + unsigned int freq_hz, unsigned int avg_log2)
> > +{
> > + struct spi_offload_trigger_config *config = &st->offload_trigger_config;
> > + unsigned int offload_period_ns, cnv_rate_hz;
> > + struct pwm_waveform cnv_wf = { };
> > + u64 target = AD4030_TCNVH_NS;
> > + u64 offload_offset_ns;
> > + int ret;
> > +
> > + /*
> > + * When averaging/oversampling over N samples, we fire the offload
> > + * trigger once at every N pulses of the CNV signal. Conversely, the CNV
> > + * signal needs to be N times faster than the offload trigger. Take that
> > + * into account to correctly re-evaluate both the PWM waveform connected
> > + * to CNV and the SPI offload trigger.
> > + */
> > + cnv_rate_hz = freq_hz << avg_log2;
> > +
> > + cnv_wf.period_length_ns = DIV_ROUND_CLOSEST(NSEC_PER_SEC, cnv_rate_hz);
>
> See below.
>
> > + /*
> > + * The datasheet lists a minimum time of 9.8 ns, but no maximum. If the
> > + * rounded PWM's value is less than 10, increase the target value by 10
> > + * and attempt to round the waveform again, until the value is at least
> > + * 10 ns. Use a separate variable to represent the target in case the
> > + * rounding is severe enough to keep putting the first few results under
> > + * the minimum 10ns condition checked by the while loop.
> > + */
> > + do {
> > + cnv_wf.duty_length_ns = target;
> > + ret = pwm_round_waveform_might_sleep(st->cnv_trigger, &cnv_wf);
> > + if (ret)
> > + return ret;
> > + target += AD4030_TCNVH_NS;
> > + } while (cnv_wf.duty_length_ns < AD4030_TCNVH_NS);
>
> Does the above have a side-effect on period_length_ns? If not, the below check
> should be moved up, otherwise here should be a short comment explaining the
> side-effect(s).
Yes, pwm_round_waveform_might_sleep() might round the period down. I'd add the
following comment to clarify that
/*
* The CNV waveform period (period_length_ns) might get rounded down by
* pwm_round_waveform_might_sleep(). Check the resultant PWM period
* is not smaller than the minimum data conversion cycle time.
*/
> > + if (!in_range(cnv_wf.period_length_ns, AD4030_TCYC_NS, INT_MAX))
> > + return -EINVAL;
>
Best regards,
Marcelo
Powered by blists - more mailing lists