linux-kernel - Re: [PATCH] iio: adc: ad7124: fix temperature channel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f8af47c8-b2e4-45b6-8c2d-36f952327d00@baylibre.com>
Date: Wed, 24 Sep 2025 08:22:21 -0500
From: David Lechner <dlechner@...libre.com>
To: Marcelo Schmitt <marcelo.schmitt1@...il.com>
Cc: Michael Hennerich <Michael.Hennerich@...log.com>,
 Jonathan Cameron <jic23@...nel.org>, Nuno Sá
 <nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>,
 Uwe Kleine-König <u.kleine-koenig@...libre.com>,
 Jonathan Cameron <Jonathan.Cameron@...wei.com>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iio: adc: ad7124: fix temperature channel

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@...libre.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@...log.com>
> 
>>
>> ---
>> base-commit: 411e8b72c181e4f49352c12ced0fd8426eb683aa
>> change-id: 20250923-iio-adc-ad7124-fix-temperature-channel-5900f7302886
>>
>> Best regards,
>> -- 
>> David Lechner <dlechner@...libre.com>
>>
>>