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: <9e20ce95-a401-46ca-94aa-bd4488b45025@baylibre.com>
Date: Wed, 24 Sep 2025 08:17:21 -0500
From: David Lechner <dlechner@...libre.com>
To: Uwe Kleine-König <u.kleine-koenig@...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

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@...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?

> 
> 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


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ