Received-SPF: pass (google.com: domain of linux-kernel+bounces-41164-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Sat, 27 Jan 2024 11:03:14 +0000
From: Conor Dooley <conor@kernel.org>
To: Peter Rosin <peda@axentia.se>
Cc: Naresh Solanki <naresh.solanki@9elements.com>,
	Jonathan Cameron <jic23@kernel.org>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>, mazziesaccount@gmail.com,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] dt-bindings: iio: afe: voltage-divider: Add
 io-channel-cells
Message-ID: <20240127-hunting-wick-fc1eed1af6b1@spud>
References: <20240126115509.1459425-1-naresh.solanki@9elements.com>
 <20240126-cinnamon-flatware-e042b5773f17@spud>
 <CABqG17hzZf2mme0v7hALhpd6-N3ZHqxdH-AhFg5eF9sbLSC2gw@mail.gmail.com>
 <20240126-scale-serrated-33686467d91b@spud>
 <CABqG17jp6YRGyTmNitz-xDdyhWOPgfT_XpXxw-OJLnXQ777vAA@mail.gmail.com>
 <20240126-blaspheme-calculate-a4134dc1ed68@spud>
 <536971eb-51f0-40e5-d025-7c4c1d683d49@axentia.se>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="PEKFPbYYOtLi9l3Q"
Content-Disposition: inline
In-Reply-To: <536971eb-51f0-40e5-d025-7c4c1d683d49@axentia.se>


--PEKFPbYYOtLi9l3Q
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sat, Jan 27, 2024 at 10:40:31AM +0100, Peter Rosin wrote:
>=20
>=20
> 2024-01-26 at 23:14, Conor Dooley wrote:
> > On Fri, Jan 26, 2024 at 11:10:36PM +0530, Naresh Solanki wrote:
>=20
> > I did look at what you have there and I think your dts is wrong.
> >=20
> > The iio-hwmon binding says:
> > | description: >
> > |   Bindings for hardware monitoring devices connected to ADC controlle=
rs
> > |   supporting the Industrial I/O bindings.
> > |=20
> > |   io-channels:
> > |     minItems: 1
> > |     maxItems: 51 # Should be enough
> > |     description: >
> > |       List of phandles to ADC channels to read the monitoring values
> >=20
> > And then you have:
> > |	iio-hwmon {
> > |		compatible =3D "iio-hwmon";
> > |		// Voltage sensors top to down
> > |		io-channels =3D <&p12v_vd 0>, <&p5v_aux_vd 0>, <&p5v_bmc_aux_vd 0>, =
<&p3v3_aux_vd 0>,
> > |			<&p3v3_bmc_aux_vd 0>, <&p1v8_bmc_aux_vd 0>, <&adc1 4>, <&adc0 2>, <=
&adc1 0>,
> > |			<&p2V5_aux_vd 0>, <&p3v3_rtc_vd 0>;
> > |	};
> > |
> > |	p12v_vd: voltage_divider1 {
> > |		compatible =3D "voltage-divider";
> > |		io-channels =3D <&adc1 3>;
> > |		#io-channel-cells =3D <1>;
> > |
> > |		/* Scale the system voltage by 1127/127 to fit the ADC range.
> > |		 * Use small nominator to prevent integer overflow.
> > |		 */
> > |		output-ohms =3D <15>;
> > |		full-ohms =3D <133>;
> > |	};
> >=20
> > A voltage divider is _not_ an ADC channel, so I don't know why you are
> > treating it as one in the iio-hwmon entry. Can you explain this please?
>=20
> This is the exact intent of the voltage divider (and the other bindings
> handled by the iio-rescaler). The raw ADC reports the voltage at its inpu=
t,
> which is fine, but if there is an analog frontend in front of the ADC
> such as a voltage divider the voltage at the ADC is not the interesting
> property. You are likely to want the "real" voltage before the voltage
> divider to better understand the value.
>=20
> In this case it's much more interesting to see values such as 12.050V
> which is presumably close to the nominal voltage (12V? guessing from
> the node name) rather than some unscaled raw ADC voltage (in this
> example it would be ~1.359V, which tells you rather little w/o rescaling
> it first).

Thanks for explaining it. Naresh, can you respin please with an
explanation of why the property belongs in the binding please?

> It's all in the description of the binding...

Obviously it was not sufficiently clear, it's not as if I didn't look at
it...

Cheers,
Conor.

--PEKFPbYYOtLi9l3Q
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZbTi8gAKCRB4tDGHoIJi
0irBAQC/E6pH/v9G1cPxD4totgtOdP6R6un1YtOfN56hp58+AwEAt2eplZHFIOo4
pnncaWC1QWLMPW8QQ0qV9zSenk8Tcws=
=qI/n
-----END PGP SIGNATURE-----

--PEKFPbYYOtLi9l3Q--