Re: [PATCH v4 03/10] iio: adc: add the ti-ads1262 driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Regmap debugfs accesses bypass the driver's custom xfer_lock, triggering lockdep splats and risking SPI message corruption.
- [Medium] Missing device stop on interrupted conversion causes a race condition and data desynchronization.
- [Low] Incompatible pointer type passed to `regmap_read`.
--

commit 6093db1ff45e9e897720bc6b278550ffbef67e85
Author: Kurt Borja <kuurtb@gmail.com>

iio: adc: add the ti-ads1262 driver

Add the ti-ads1262 driver with initial support for the primary ADC (ADC1).
Various features such as accurate timeout delays, per-channel reference
sources, scale, offset, and diagnostics are intentionally left out for
future support.

> diff --git a/drivers/iio/adc/ti-ads1262.c b/drivers/iio/adc/ti-ads1262.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/iio/adc/ti-ads1262.c

[ ... ]

> +static int ads1262_channel_read(struct iio_dev *indio_dev,
> +				const struct iio_chan_spec *spec, __be32 *val)
> +{
> +	struct ads1262 *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> +	if (IIO_DEV_ACQUIRE_FAILED(claim))
> +		return -EBUSY;
> +
> +	ret = ads1262_set_runmode(st, ADS1262_RUNMODE_PULSE);
> +	if (ret)
> +		return ret;
> +
> +	ret = ads1262_channel_enable(st, spec);
> +	if (ret)
> +		return ret;
> +
> +	reinit_completion(&st->drdy);
> +
> +	ret = ads1262_dev_start_one(st);
> +	if (ret)
> +		return ret;
> +
> +	ret = ads1262_wait_for_conversion(st);
> +	if (ret)
> +		return ret;

[Severity: Medium]
Does this error path leave the hardware running if the wait is interrupted?

If ads1262_wait_for_conversion() returns an error (like -ERESTARTSYS from
a signal), the driver exits immediately without stopping the hardware. If a
subsequent read calls reinit_completion(&st->drdy), but the abandoned
hardware conversion finishes and asserts the DRDY interrupt right after,
could the new read prematurely fetch stale data from the old conversion?

Should this path ensure the hardware is stopped and synchronized with the
interrupt handler on timeout or interrupt?

[ ... ]

> +static int ads1262_dev_configure(struct ads1262 *st)
> +{
> +	struct device *dev = &st->spi->dev;
> +	int id, ret;
> +
> +	ret = ads1262_dev_reset(st);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to reset device\n");
> +
> +	guard(mutex)(&st->xfer_lock);
> +
> +	ret = regmap_read(st->regmap, ADS1262_ID_REG, &id);

[Severity: Low]
Is the type of id correct here?

The regmap_read() API expects an unsigned int * for its third argument, but
id is declared as an int. Could this trigger a -Wpointer-sign compiler
warning?

[ ... ]

> +static const struct regmap_config ads1262_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.writeable_reg = ads1262_writeable_reg,
> +	.readable_reg = ads1262_readable_reg,
> +	.volatile_reg = ads1262_volatile_reg,
> +	.reg_defaults = ads1262_reg_defaults,
> +	.num_reg_defaults = ARRAY_SIZE(ads1262_reg_defaults),
> +	.max_register = ADS1262_ADC2FSC1_REG,
> +	.can_sleep = true,
> +	.cache_type = REGCACHE_MAPLE,
> +};

[ ... ]

> +static int ads1262_regmap_read(void *context, const void *reg_buf,
> +			       size_t reg_size, void *val_buf, size_t val_size)
> +{
> +	struct ads1262 *st = context;
> +	u8 tx[2];
> +
> +	lockdep_assert_held(&st->xfer_lock);

[Severity: Medium]
Will this lockdep assertion fail if the register is read via debugfs?

Because ads1262_regmap_config does not set disable_locking = true and does
not provide custom lock/unlock callbacks, regmap uses its own internal
mutex. When regmap's debugfs interface is read, the regmap core acquires
its internal mutex and calls the driver's ads1262_regmap_read() callback
without holding st->xfer_lock.

Could this bypass the xfer_lock, triggering the lockdep splat and allowing
debugfs SPI operations to interleave with driver-initiated SPI transactions?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828-ads126x-v4-0-1dc27e9c0260@gmail.com?part=3