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

[David Lechner]

On 5/22/26 8:23 AM, Liviu Stan wrote:
> 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?

I would still move it before the write so that it looks correct
and others can copy/paste the logic on other devices.