Re: [PATCH v8 3/5] iio: adc: ltc2378: Enable high-speed data capture

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Division by zero vulnerabilities in ltc2378_update_conversion_rate due to unhandled hardware rounding of period/frequency.
- [Low] Unlocked data race on st->cnv_Hz between read_raw and write_raw.
--

--- Patch [3]: [PATCH v8 3/5] iio: adc: ltc2378: Enable high-speed data capture ---
commit cad96004cef5739b24db063923b36678ea8e3617
Author: Marcelo Schmitt <marcelo.schmitt@analog.com>
Subject: iio: adc: ltc2378: Enable high-speed data capture

Make use of SPI transfer offloading to speed up data capture, enabling data
acquisition at faster sample rates (up to 2 MSPS).

> diff --git a/drivers/iio/adc/ltc2378.c b/drivers/iio/adc/ltc2378.c
> index 107388f870626..7baee02450d39 100644
> --- a/drivers/iio/adc/ltc2378.c
> +++ b/drivers/iio/adc/ltc2378.c
> @@ -264,7 +384,137 @@ static int ltc2378_read_raw(struct iio_dev *indio_dev,
>  			*val2 = chan->scan_type.realbits;
>  
>  		return IIO_VAL_FRACTIONAL_LOG2;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = st->cnv_Hz;

[Severity: Low]
Does this lockless read of st->cnv_Hz race with concurrent updates?

While ltc2378_write_raw() acquires the IIO direct mode lock before calling
ltc2378_update_conversion_rate() to update st->cnv_Hz, this read path appears
to access it without synchronization or READ_ONCE()/WRITE_ONCE() macros.

> +		return IIO_VAL_INT;

[ ... ]

> +static int ltc2378_update_conversion_rate(struct ltc2378_state *st, int freq_Hz)
> +{

[ ... ]

> +	/*
> +	 * Configure SPI offload PWM trigger.
> +	 * The trigger should fire after tBUSYLH + tCONV + tDSDOBUSYL.
> +	 * Minimum time needed: TBUSYLH (13ns) + TCONV (part-specific) + TDSDOBUSYL (5ns)
> +	 *
> +	 * Use the same period as CNV PWM to avoid timing issues.
> +	 * Convert back from period to frequency for the SPI offload API.
> +	 */
> +	offload_period_ns = cnv_wf.period_length_ns;
> +	config.periodic.frequency_hz = div_u64(HZ_PER_GHZ, offload_period_ns);

[Severity: High]
Can this result in a division by zero if offload_period_ns is 0?

If pwm_round_waveform_might_sleep() rounds the requested period down to 0,
offload_period_ns will be 0 and cause a kernel panic here.

> +	min_read_offset = LTC2378_TBUSYLH_NS + st->info->tconv_ns + LTC2378_TDSDOBUSYL_NS;

[ ... ]

> +	/* Check the minimum CNV to SCLK delay is met */
> +	if (config.periodic.offset_ns < min_read_offset)
> +		return -EDOM;
> +
> +	/* Check the PWM periods remain the same */
> +	offload_period_ns = div64_u64(HZ_PER_GHZ, config.periodic.frequency_hz);

[Severity: High]
Does this code panic if config.periodic.frequency_hz evaluates to 0?

If a user writes a very low sampling frequency, the PWM driver might round
the hardware period up to a value greater than HZ_PER_GHZ. If
offload_period_ns > HZ_PER_GHZ, the earlier calculation for
config.periodic.frequency_hz will evaluate to 0.

Additionally, if spi_offload_trigger_validate() mutates frequency_hz down
to 0, it would also result in a division by zero here.

> +	if (cnv_wf.period_length_ns != offload_period_ns)
> +		return -EDOM;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784835663.git.marcelo.schmitt@analog.com?part=3