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: <aNPrlIYGrB8oSsfL@debian-BULLSEYE-live-builder-AMD64>
Date: Wed, 24 Sep 2025 10:01:08 -0300
From: Marcelo Schmitt <marcelo.schmitt1@...il.com>
To: David Lechner <dlechner@...libre.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

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.


> +		 */
> +		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,
 			},
 		};
 
 		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>
> 
>