linux-kernel - Re: [PATCH v2 6/7] iio: adc: ad4170: Add support for internal temperature sensor

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: <20250504184448.43b8045e@jic23-huawei>
Date: Sun, 4 May 2025 18:44:48 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Marcelo Schmitt <marcelo.schmitt@...log.com>
Cc: <linux-iio@...r.kernel.org>, <devicetree@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, <lars@...afoo.de>,
 <Michael.Hennerich@...log.com>, <dlechner@...libre.com>,
 <nuno.sa@...log.com>, <andy@...nel.org>, <robh@...nel.org>,
 <krzk+dt@...nel.org>, <conor+dt@...nel.org>, <marcelo.schmitt1@...il.com>
Subject: Re: [PATCH v2 6/7] iio: adc: ad4170: Add support for internal
 temperature sensor

On Mon, 28 Apr 2025 09:29:12 -0300
Marcelo Schmitt <marcelo.schmitt@...log.com> wrote:

> The AD4170 has an internal temperature sensor that can be read using the
> ADC. Whenever possible, configure an IIO channel to provide the chip's
> temperature.
> 
> Reviewed-by: Nuno Sá <nuno.sa@...log.com>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@...log.com>

One trivial comment below.
I don't mind that much given you already did it this way though.

>  /*
>   * Receives the number of a multiplexed AD4170 input (ain_n), and stores the
>   * voltage (in µV) of the specified input into ain_voltage. If the input number
> @@ -1189,9 +1210,27 @@ static int ad4170_read_raw(struct iio_dev *indio_dev,
>  		return ret;
>  	case IIO_CHAN_INFO_SCALE:
>  		pga = FIELD_GET(AD4170_AFE_PGA_GAIN_MSK, setup->afe);
> -		*val = chan_info->scale_tbl[pga][0];
> -		*val2 = chan_info->scale_tbl[pga][1];
> -		return IIO_VAL_INT_PLUS_NANO;
> +		switch (chan->type) {
> +		case IIO_VOLTAGE:

When you know you are going to have changes like this in later patches
in the same posted series it is fine to make the diff smaller by
using a switch from the start.

Occasionally you'll get a comment from someone like me who can't
hold the whole set in my head and misses that is what is going on.
Just reply to say it was diff minimization and likely no one will ever
mind!

> +			*val = chan_info->scale_tbl[pga][0];
> +			*val2 = chan_info->scale_tbl[pga][1];
> +			return IIO_VAL_INT_PLUS_NANO;
> +
> +		case IIO_TEMP:
> +			/*
> +			 * The scale_tbl converts output codes to mV units so
> +			 * multiply by MILLI to make the factor convert to µV.
> +			 * Then, apply the temperature sensor change sensitivity
> +			 * of 477 μV/K. Finally, multiply the result by MILLI
> +			 * again to comply with milli degrees Celsius IIO ABI.
> +			 */
> +			*val = 0;
> +			*val2 = DIV_ROUND_CLOSEST(chan_info->scale_tbl[pga][1] * MILLI,
> +						  477) * MILLI;
> +			return IIO_VAL_INT_PLUS_NANO;
> +		default:
> +			return -EINVAL;
> +		}