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

[Liviu Stan]

On Thu, 21 May 2026 19:18:28 +0000 sashiko-bot@xxxxxxxxxx wrote:

> 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@xxxxxxxxxx>
> 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?

This is theoretically valid but in practice, the conversion requires at minimum
two 82ms cycles (167ms), so there is no realistic window for the interrupt to
race with reinit_completion(). What do you guys think?

> 
> [ ... ]
> 
> > @@ -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.
> 

First, I don't understand why Sashiko wrote "base_reg = 
ADT7604_RES_RES_START_REG", the patch has a "==" there. Second, the temperature
result register stores fault indicator bits for temperature reads, not resistance.

Thanks,
Liviu