Re: [PATCH v3 8/8] iio: temperature: ltc2983: Add support for ADT7604

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Reading the IIO_RESISTANCE channel bypasses hardware fault detection, causing hardware errors (like broken sensor traces) to be silently reported as valid data.

Pre-existing issues:
- [High] reinit_completion() is called after initiating the hardware conversion, creating a race condition that can cause spurious read timeouts.
--

commit d382a4cbefd41723ab5ddd91fd399d716c8faaec
Author: Liviu Stan <liviu.stan@analog.com>
Subject: iio: temperature: ltc2983: Add support for ADT7604

This commit adds support for the ADT7604 temperature sensor by repurposing
custom RTD and thermistor types as copper trace resistance and leak detector
sensors. It introduces two new software sensor types and exposes a new
IIO_RESISTANCE channel reading from a separate result register bank. The
ltc2983_chip_info struct is also extended to use a 64-bit supported_sensors
bitmask for the new types.

> diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
> index 326f843f42713..e2ffeee026eef 100644
> --- a/drivers/iio/temperature/ltc2983.c
> +++ b/drivers/iio/temperature/ltc2983.c

[ ... ]

> @@ -1205,7 +1502,8 @@ static struct ltc2983_sensor *ltc2983_temp_new(struct fwnode_handle *child,
>  }
>  
>  static int ltc2983_chan_read(struct ltc2983_data *st,
> -			const struct ltc2983_sensor *sensor, int *val)
> +			const struct ltc2983_sensor *sensor,
> +			u32 base_reg, int *val)
>  {
>  	struct device *dev = &st->spi->dev;
>  	u32 start_conversion = 0;

[Severity: High]
This is a pre-existing issue, but can this function experience spurious
read timeouts due to a race condition with the completion?

Looking at the rest of this function:

    ret = regmap_write(st->regmap, LTC2983_STATUS_REG, start_conversion);
    if (ret)
        return ret;

    reinit_completion(&st->completion);

If the hardware conversion finishes very quickly after regmap_write(),
could the interrupt fire and call complete() before the CPU executes
reinit_completion()? This would overwrite the completion event, causing the
subsequent wait_for_completion_timeout() to spuriously time out.

Would it be safer to call reinit_completion() before initiating the hardware
action?

[ ... ]

> @@ -1236,13 +1534,23 @@ static int ltc2983_chan_read(struct ltc2983_data *st,
>  	}
>  
>  	/* read the converted data */
> -	ret = regmap_bulk_read(st->regmap, LTC2983_RESULT_ADDR(sensor->chan),
> +	ret = regmap_bulk_read(st->regmap, LTC2983_RESULT_ADDR(sensor->chan, base_reg),
>  			       &st->temp, sizeof(st->temp));
>  	if (ret)
>  		return ret;
>  
>  	*val = __be32_to_cpu(st->temp);
>  
> +	if (base_reg == ADT7604_RES_RES_START_REG) {
> +		/*
> +		 * Resistance result register gives a plain unsigned value,
> +		 * D31 is always 0, no valid bit, no fault bits. Read bits[30:0]
> +		 * directly — the temperature result format does not apply here.
> +		 */
> +		*val &= GENMASK(30, 0);
> +		return 0;

[Severity: High]
Will returning early here bypass hardware fault detection for resistance
channels?

As the comment notes, the resistance result register does not contain fault
bits. However, hardware fault indicators (like Hard Fault, Soft Fault, or
Over Range) are stored in the primary Temperature Result register.

By returning before reading the Temperature Result register and calling
sensor->fault_handler(), does this code silently ignore sensor faults? This
might be problematic for sub-ohm copper traces, which only expose the
IIO_RESISTANCE channel, leaving users with no mechanism to detect a broken
trace.

> +	}
> +
>  	if (!(LTC2983_RES_VALID_MASK & *val)) {
>  		dev_err(dev, "Invalid conversion detected\n");
>  		return -EIO;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521164323.770626-1-liviu.stan@analog.com?part=8