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] [day] [month] [year] [list]

Message-ID: <apfh25e3k7dpho7dcjayl7g5dpq5lnsuhnc7xhbjlqtfw5lyql@lopr22johw5z>
Date: Wed, 24 Sep 2025 16:33:16 +0200
From: Uwe Kleine-König <u.kleine-koenig@...libre.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>, 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

Hello David,

On Wed, Sep 24, 2025 at 08:17:21AM -0500, David Lechner wrote:
> On 9/24/25 6:03 AM, Uwe Kleine-König wrote:
> > 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@...libre.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?

Yes, got the whole picture now. Thanks for explaining.

So maybe:

	For channels other than the voltage ones calibration is skipped (which
	is OK). However that results in the calibration register values
	tracked in st->channels[i].cfg all being zero. These zeros are
	then written to hardware before a measurement and thus resulting
	in temperature readings to be always 8388608 (0x800000). This is
	easily fixed by postponing the channel type check until
	st->channels[i].cfg is initialized with the uncalibrated default
	values.

Best regards
Uwe


Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)