Re: [PATCH 4/5] iio: adc: ti-ads1262: Add calibration support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Using stack-allocated variables as buffers for SPI DMA transfers via `regmap_bulk_read()` and `regmap_bulk_write()` violates SPI subsystem constraints.
- [Medium] A partially uninitialized stack variable is read, which triggers Kernel Memory Sanitizer (KMSAN) warnings and constitutes undefined behavior.
- [Low] Implicit pointer conversion from `int *` to `u32 *` causes a `-Wincompatible-pointer-types` compiler warning.
- [Low] The `calib_range` array is missing a `const` qualifier.
--

commit 610c80294ac29a44a2dda7d05bb059e7d60a62d2
Author: Kurt Borja <kuurtb@gmail.com>

iio: adc: ti-ads1262: Add calibration support

This commit adds channel calibration support to the ti-ads1262 ADC driver
by implementing scale and bias calibration via regmap bulk operations.

> diff --git a/drivers/iio/adc/ti-ads1262.c b/drivers/iio/adc/ti-ads1262.c
> index 6d5f22836ad8b..b33505e7fdc7a 100644
> --- a/drivers/iio/adc/ti-ads1262.c
> +++ b/drivers/iio/adc/ti-ads1262.c
[ ... ]
> @@ -453,6 +457,32 @@ static int ads1262_dev_start_one(struct ads1262 *st, u8 runmode)
>  	return 0;
>  }
>  
> +static int ads1262_read_calib(struct ads1262 *st, unsigned int reg, u32 *val)
> +{
> +	__le32 lval;
> +	int ret;
> +
> +	/*
> +	 * The calibration word is a signed 24 bit LSB-first value.
> +	 */
> +	ret = regmap_bulk_read(st->regmap, reg, &lval, 3);

[Severity: High]
Does this safely handle DMA transfers? Passing a stack-allocated variable
like lval to regmap_bulk_read() eventually initiates an SPI DMA transfer.
Using stack memory for DMA transfers can cause spi_map_buf() to fail with
-EINVAL on systems with vmap stacks, or lead to memory corruption due to
cache line tearing on architectures without hardware cache coherency.

> +	if (ret)
> +		return ret;
> +	*val = sign_extend32(le32_to_cpu(lval), 23);

[Severity: Medium]
Is it possible this reads uninitialized memory? The stack variable lval is
declared without initialization. When passed to regmap_bulk_read() above,
only the first 3 bytes are populated. Calling le32_to_cpu(lval) will then
read all 4 bytes, including the uninitialized 4th byte, which can trigger a
KMSAN use-of-uninitialized-value warning.

> +
> +	return 0;
> +}
> +
> +static int ads1262_write_calib(struct ads1262 *st, unsigned int reg, u32 val)
> +{
> +	__le32 lval = cpu_to_le32(val);
> +
> +	/*
> +	 * The calibration word is a signed 24 bit LSB-first value.
> +	 */
> +	return regmap_bulk_write(st->regmap, reg, &lval, 3);

[Severity: High]
Can this also cause issues with SPI DMA? Similar to the read function,
passing the stack variable lval to regmap_bulk_write() routes a stack
address into the SPI subsystem, violating DMA constraints.

> +}
[ ... ]
> @@ -673,6 +703,18 @@ static int ads1262_read_raw(struct iio_dev *indio_dev,
>  		*val2 = ads1262_conv_delay_avail[mode][1];
>  		return IIO_VAL_INT_PLUS_NANO;
>  
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		ret = ads1262_read_calib(st, ADS1262_FSCAL0_REG, val);

[Severity: Low]
Will this cause a compiler warning for incompatible pointer types?
The val parameter provided by the IIO subsystem to ads1262_read_raw() is
an int pointer, but ads1262_read_calib() explicitly expects a u32 pointer.
Since Linux 6.8, incompatible pointer type warnings are treated as fatal
errors and can break the build.

> +		if (ret)
> +			return ret;
> +		return IIO_VAL_INT;
> +
[ ... ]
> @@ -682,6 +724,9 @@ static int ads1262_read_avail(struct iio_dev *indio_dev,
>  			      struct iio_chan_spec const *chan, const int **vals,
>  			      int *type, int *length, long mask)
>  {
> +	static int calib_range[3] = { ADS1262_CALIB_WORD_MIN, 1,
> +				      ADS1262_CALIB_WORD_MAX };

[Severity: Low]
Should this array be declared as const? Omitting the const qualifier places
calib_range in the mutable data section instead of the read-only rodata
section, leaving it unprotected against accidental runtime modifications.

> +
>  	switch (mask) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-ads126x-v1-0-894c788d03ed@gmail.com?part=4