[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250928110836.79ab434e@jic23-huawei>
Date: Sun, 28 Sep 2025 11:08:47 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Marcelo Schmitt <marcelo.schmitt@...log.com>
Cc: <linux-iio@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-doc@...r.kernel.org>, <linux-spi@...r.kernel.org>,
<linux-kernel@...r.kernel.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>, <marcelo.schmitt1@...il.com>,
Trevor Gamblin <tgamblin@...libre.com>, Axel Haslam <ahaslam@...libre.com>
Subject: Re: [PATCH v3 6/8] iio: adc: ad4030: Add SPI offload support
On Fri, 26 Sep 2025 17:40:29 -0300
Marcelo Schmitt <marcelo.schmitt@...log.com> 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.
>
> Co-developed-by: Trevor Gamblin <tgamblin@...libre.com>
> Signed-off-by: Trevor Gamblin <tgamblin@...libre.com>
> Co-developed-by: Axel Haslam <ahaslam@...libre.com>
> Signed-off-by: Axel Haslam <ahaslam@...libre.com>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@...log.com>
This isn't my area of speciality so I'll be looking for some review tags from others.
Comments inline are completely trivial things that'd I'd just fix up but
you'll be doing another spin for the bot error anyway so over to you!
Jonathan
> diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
> index cdf5933e9725..8fca98738e3e 100644
> --- a/drivers/iio/adc/ad4030.c
> +++ b/drivers/iio/adc/ad4030.c
> +
> +static int ad4030_update_conversion_rate(struct ad4030_state *st,
> + unsigned int freq, unsigned int avg_log2)
> +{
> + struct spi_offload_trigger_config *config = &st->offload_trigger_config;
> + struct pwm_waveform cnv_wf = { };
> + u64 target = AD4030_TCNVH_NS;
> + u64 offload_period_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.
> + */
> + freq <<= avg_log2;
> +
> + cnv_wf.period_length_ns = DIV_ROUND_CLOSEST(NSEC_PER_SEC, freq);
> + /*
> + * 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);
> +
> + if (!in_range(cnv_wf.period_length_ns, AD4030_TCYC_NS, INT_MAX))
> + return -EINVAL;
> +
> + offload_period_ns = cnv_wf.period_length_ns;
> + /*
> + * Make the offload trigger period be N times longer than the CNV PWM
> + * period when averaging over N samples.
> + */
> + offload_period_ns <<= avg_log2;
> +
> + config->periodic.frequency_hz = DIV_ROUND_UP_ULL(NSEC_PER_SEC,
Bonus space after =
> + offload_period_ns);
> @@ -869,7 +1035,9 @@ static int ad4030_get_current_scan_type(const struct iio_dev *indio_dev,
> static int ad4030_update_scan_mode(struct iio_dev *indio_dev,
> const unsigned long *scan_mask)
> {
> - return ad4030_set_mode(indio_dev, *scan_mask);
> + struct ad4030_state *st = iio_priv(indio_dev);
> +
> + return ad4030_set_mode(st, *scan_mask, st->avg_log2);
Trivial and entirely up to you but you can do the following without significant lost of
readability.
return ad4030_set_mode(iio_priv(indio_dev), &scan_mask, st->avg_log2);
> }
Powered by blists - more mailing lists