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

[David Lechner <dlechner@baylibre.com>]

On 9/24/25 8:01 AM, Marcelo Schmitt wrote:
> Hi,
> 
> On 09/23, 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).
> 
> Would
> 'Fix temperature channel not working due to gain and offset not being
> initialized to their default values.'
> be a more accurate description?
> 
> 
>>
>> 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>
>> ---
> ...
>>  	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.
> I think it would be more accurate to just say the offset and callibscale
> for temperature channel need to be at default values for the data sheet's
> equation for the temperature sensor to be accurate.

This is true for all channels, not just the temperature channel and
there is an existing comment (partially visible above) that says
something like this already.

> 
> 
>> +		 */
>> +		if (indio_dev->channels[i].type != IIO_VOLTAGE)
>> +			continue;
>> +
>>  		/*
>>  		 * Full-scale calibration isn't supported at gain 1, so skip in
>>  		 * that case. Note that untypically full-scale calibration has
> 
> Maybe, instead of moving the 'if(... IIO_VOLTAGE)' check, this could alternatively
> be set when initializing the temperature channel at ad7124_parse_channel_config().
> 
>  	if (num_channels < AD7124_MAX_CHANNELS) {
>  		st->channels[num_channels] = (struct ad7124_channel) {
>  			.nr = num_channels,
>  			.ain = FIELD_PREP(AD7124_CHANNEL_AINP, AD7124_CHANNEL_AINx_TEMPSENSOR) |
>  				FIELD_PREP(AD7124_CHANNEL_AINM, AD7124_CHANNEL_AINx_AVSS),
>  			.cfg = {
>  				.bipolar = true,
> +				.calibration_offset = 0x800000,
> +				.calibration_gain = st->gain_default,

st->gain_default has not been initialized at this point, so this would
not work without more rearranging.

>  			},
>  		};
>  
>  		chan[num_channels] = (struct iio_chan_spec) {
>  			.type = IIO_TEMP,
> 
> 
> Nevertheless, the current fix looks good to me as it is, so
> Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> 
>>
>> ---
>> base-commit: 411e8b72c181e4f49352c12ced0fd8426eb683aa
>> change-id: 20250923-iio-adc-ad7124-fix-temperature-channel-5900f7302886
>>
>> Best regards,
>> -- 
>> David Lechner <dlechner@baylibre.com>
>>
>>