Re: [PATCH] iio: adc: ad7124: fix temperature channel

[David Lechner <dlechner@baylibre.com>]

On 9/24/25 6:03 AM, Uwe Kleine-König wrote:
> Hello David,
> 
> On Tue, Sep 23, 2025 at 03:18:02PM -0500, David Lechner wrote:
>> Fix temperature channel not working due to gain and offset not being
>> initialized. This was causing the raw temperature readings to be always
>> 8388608 (0x800000).
>>
>> To fix it, we just make sure the gain and offset values are set to the
>> default values and still return early without doing an internal
>> calibration.
>>
>> While here, add a comment explaining why we don't bother calibrating
>> the temperature channel.
>>
>> Fixes: 47036a03a303 ("iio: adc: ad7124: Implement internal calibration at probe time")
>> Signed-off-by: David Lechner <dlechner@baylibre.com>
>> ---
>>  drivers/iio/adc/ad7124.c | 11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
>> index 374e39736584f55c1290db3e257dff2c60f884d2..94d90a63987c0f9886586db0c4bc1690855be2c1 100644
>> --- a/drivers/iio/adc/ad7124.c
>> +++ b/drivers/iio/adc/ad7124.c
>> @@ -1518,10 +1518,6 @@ static int __ad7124_calibrate_all(struct ad7124_state *st, struct iio_dev *indio
>>  	int ret, i;
>>  
>>  	for (i = 0; i < st->num_channels; i++) {
>> -
>> -		if (indio_dev->channels[i].type != IIO_VOLTAGE)
>> -			continue;
>> -
>>  		/*
>>  		 * For calibration the OFFSET register should hold its reset default
>>  		 * value. For the GAIN register there is no such requirement but
>> @@ -1531,6 +1527,13 @@ static int __ad7124_calibrate_all(struct ad7124_state *st, struct iio_dev *indio
>>  		st->channels[i].cfg.calibration_offset = 0x800000;
>>  		st->channels[i].cfg.calibration_gain = st->gain_default;
>>  
>> +		/*
>> +		 * Only the main voltage input channels are important enough
>> +		 * to be automatically calibrated here.
>> +		 */
>> +		if (indio_dev->channels[i].type != IIO_VOLTAGE)
>> +			continue;
>> +
> 
> I don't understand a detail of the problem. The commit log suggests that
> there is a general problem, but looking at the patch I suspect there is
> only a problem if at probe time the OFFSET and GAIN register for the
> temperature channel are different from their reset default setting.

What I failed to mention is that st->channels[i].cfg.calibration_offset
and st->channels[i].cfg.calibration_gain are zero-initialized. And that
these values are later programmed into the ADC. Programming these to
zero is what caused reading the raw value value to always return a fixed
value because the real value got multiplied by 0 in the ADC.

Is that enough to clear it up?

> 
> I think the patch is fine, but if my understanding is right the commit
> log is more dramatic than the issue really is, as it needs some fiddling
> with the ADC's registers between poweron and driver loading to trigger.
> 
> Best regards
> Uwe