Re: [PATCH 3/3] iio: adc: ltc2497: add 2x conversion speed mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] I2C transfer buffer `cmd` is allocated on the stack, which strictly violates DMA mapping rules. If the underlying I2C host controller relies on DMA and attempts to map the transfer buffer, calling `dma_map_single()` on a stack address will fail or cause a kernel panic on architectures with `CONFIG_VMAP_STACK` enabled (as the stack may not be physically contiguous). The driver author correctly used an `__aligned(IIO_DMA_MINALIGN)` heap buffer for `st->data.d8` / `st->data.d32` to safely handle DMA, making this stack allocation an oversight.
- [Medium] Missing update of `time_prev` before returning `-ERESTARTSYS` on an interrupted wait leads to state corruption and spurious I2C NACKs (`-EIO`). When switching modes or channels, the driver issues a dummy write to start a new conversion and then calls `msleep_interruptible()`. If this sleep is interrupted by a signal, the driver returns early without updating `ddata->time_prev` to reflect the newly started conversion. On the next read retry, `ltc2497core_wait_conv()` calculates the elapsed time against this stale `time_prev`. If switching from a faster mode (e.g., 76ms) to a slower mode (150ms), the wait logic may falsely conclude the 150ms delay is satisfied because the time elapsed since the *old* `time_prev` exceeds the remaining wait time. It then directly executes a premature I2C read before the ADC has finished converting, resulting in a hardware NACK.
--

--- Patch [3]: [PATCH 3/3] iio: adc: ltc2497: add 2x conversion speed mode ---
commit 3de76fde39bdb91c8ae6ce607b01c574bc7d76f1
Author: Andrei Stancovici <andrei.stancovici@analog.com>

iio: adc: ltc2497: add 2x conversion speed mode

This commit introduces a 2x output rate mode for the LTC2499 ADC by exposing
a new sampling_frequency attribute on voltage channels. It updates the
conversion wait times and the two-byte command path to accommodate the speed
mode configuration.

> diff --git a/drivers/iio/adc/ltc2497-core.c b/drivers/iio/adc/ltc2497-core.c
> index 4ca7271ca0071..68b53a983e053 100644
> --- a/drivers/iio/adc/ltc2497-core.c
> +++ b/drivers/iio/adc/ltc2497-core.c
[ ... ]
> @@ -61,7 +105,9 @@ static int ltc2497core_read(struct ltc2497core_driverdata *ddata, u8 address, in
>  			return ret;
>  		ddata->addr_prev = address;
>  
> -		if (msleep_interruptible(LTC2497_CONVERSION_TIME_MS))
> +		/* The reprogram above starts a conversion in the new mode. */
> +		ddata->conv_time_prev = conv_time_ms;
> +		if (msleep_interruptible(conv_time_ms))
>  			return -ERESTARTSYS;

[Severity: Medium]
Will this lead to state corruption and spurious I2C NACKs if the sleep is
interrupted?

When switching modes or channels in ltc2497core_read(), a dummy write starts
a new conversion and then calls msleep_interruptible(). If this sleep is
interrupted by a signal, the driver returns early without updating
ddata->time_prev to reflect the newly started conversion.

On the next read retry, ltc2497core_wait_conv() calculates the elapsed time
against the stale time_prev. If switching from a faster mode (e.g., 76ms) to
a slower mode (150ms), the wait logic may falsely conclude the 150ms delay is
satisfied because the time elapsed since the old time_prev exceeds the
remaining wait time. It then directly executes a premature I2C read before
the ADC has finished converting, resulting in a hardware NACK (-EIO).

>  	}
[ ... ]
> diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c
> index 4e0f5c7671f41..6bb016f495e6e 100644
> --- a/drivers/iio/adc/ltc2497.c
> +++ b/drivers/iio/adc/ltc2497.c
[ ... ]
> @@ -104,6 +107,8 @@ static int ltc2497_result_and_measure(struct ltc2497core_driverdata *ddata,
>  		} else {
>  			cmd[0] = LTC2497_ENABLE | address;
>  			cmd[1] = LTC2499_EN2;
> +			if (ddata->sped_2x)
> +				cmd[1] |= LTC2499_SPD;
>  		}
>  
>  		ret = i2c_master_send(st->client, cmd, sizeof(cmd));

[Severity: High]
Is it safe to pass a stack-allocated buffer to i2c_master_send()?

If the underlying I2C host controller relies on DMA and attempts to map the
transfer buffer, calling dma_map_single() on a stack address will fail or
cause a kernel panic on architectures with CONFIG_VMAP_STACK enabled.

Should we use an __aligned(IIO_DMA_MINALIGN) heap buffer for this transfer
instead, similar to how st->data is handled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805094135.11762-1-andrei.stancovici@analog.com?part=3