Received-SPF: pass (google.com: domain of linux-kernel+bounces-195023-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <e0be3356bf809035963c4801f05b9db2675c111e.camel@gmail.com>
Subject: Re: [PATCH v3 1/6] dt-bindings: adc: ad7173: add support for ad411x
From: Nuno =?ISO-8859-1?Q?S=E1?= <noname.nuno@gmail.com>
To: Conor Dooley <conor@kernel.org>, "Ceclan, Dumitru"
 <mitrutzceclan@gmail.com>
Cc: dumitru.ceclan@analog.com, Lars-Peter Clausen <lars@metafoo.de>, Michael
 Hennerich <Michael.Hennerich@analog.com>, Jonathan Cameron
 <jic23@kernel.org>, Rob Herring <robh@kernel.org>, Krzysztof Kozlowski
 <krzysztof.kozlowski+dt@linaro.org>,  Conor Dooley <conor+dt@kernel.org>,
 David Lechner <dlechner@baylibre.com>, linux-iio@vger.kernel.org, 
 devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Date: Thu, 30 May 2024 09:50:51 +0200
In-Reply-To: <20240529-slit-verse-0fb06f3556fb@spud>
References: <20240527-ad4111-v3-0-7e9eddbbd3eb@analog.com>
	 <20240527-ad4111-v3-1-7e9eddbbd3eb@analog.com>
	 <20240527-arguably-said-361184ad848e@spud>
	 <d87ae6ef-090d-4e47-bde4-4d08fd445ac1@gmail.com>
	 <20240528-filtrate-cloning-b9152322a3da@spud>
	 <a1c75105-6447-4b67-b7d2-326ad9b19b82@gmail.com>
	 <20240529-slit-verse-0fb06f3556fb@spud>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.52.1 (3.52.1-1.fc40) 
Precedence: bulk
MIME-Version: 1.0

On Wed, 2024-05-29 at 17:04 +0100, Conor Dooley wrote:
> On Wed, May 29, 2024 at 04:38:53PM +0300, Ceclan, Dumitru wrote:
> > On 28/05/2024 20:52, Conor Dooley wrote:
> > > On Tue, May 28, 2024 at 03:16:07PM +0300, Ceclan, Dumitru wrote:
> > > > On 27/05/2024 20:48, Conor Dooley wrote:
> > > > > On Mon, May 27, 2024 at 08:02:34PM +0300, Dumitru Ceclan via B4 R=
elay
> > > > > wrote:
> > > > > > From: Dumitru Ceclan <dumitru.ceclan@analog.com>
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 adi,channel-type:
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 description:
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Used to=
 differentiate between different channel types as the
> > > > > > device
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 r=
egister configurations are the same for all usage types.
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Both ps=
eudo-differential and single-ended channels will use
> > > > > > the
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 s=
ingle-ended specifier.
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 $ref: /schemas/type=
s.yaml#/definitions/string
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 enum:
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 - singl=
e-ended
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 - diffe=
rential
> > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 default: differenti=
al
> > > > >=20
> > > > > I dunno if my brain just ain't workin' right today, or if this is=
 not
> > > > > sufficiently explained, but why is this property needed? You've g=
ot
> > > > > diff-channels and single-channels already, why can you not infer =
the
> > > > > information you need from them? What should software do with this
> > > > > information?
> > > > > Additionally, "pseudo-differential" is not explained in this bind=
ing.
> > > >=20
> > > > In previous thread we arrived to the conclusion single-ended and
> > > > pseudo-differential channels should be marked with the flag
> > > > "differential=3Dfalse" in the IIO channel struct. This cannot
> > > > really be inferred as any input pair could be used in that
> > > > manner and the only difference would be in external wiring.
> > > >=20
> > > > Single-channels cannot be used to define such a channel as
> > > > two voltage inputs need to be selected. Also, we are already
> > > > using single-channel to define the current channels.
> > >=20
> > > If I understand correctly, the property could be simplified to a flag
> > > then, since it's only the pseudo differential mode that you cannot be
> > > sure of?
> > > You know when you're single-ended based on single-channel, so the
> > > additional info you need is only in the pseudo-differential case.
> > >=20
> > Yes, it could just be a boolean flag. The only thing I have against
> > that is the awkwardness of having both diff-channels and
> > differential=3Dfalse within a channel definition.
>=20
> What I was suggesting was more like "adi,pseudo-differential" (you don't
> need to set the =3Dfalse or w/e, flag properties work based on present/no=
t
> present). I think that would avoid the awkwardness?
>=20

Yeah, that was also my understanding of your reply...=C2=A0But I think you'=
re also
mentioning to have this flag together with the single-channel property?=C2=
=A0

I'm a bit confused because it seems to me that single-channel only gets one=
 input
while we need to select two for pseudo-differential/single-ended. Is this c=
orrect
Dumitru?

FWIW, I think we already have that awkwardness in the current form. If I'm =
not
missing anything, what we have in the driver is pretty much:

if (not diff && single-channel)
	// then current channel
else
	// relies on the channel-type stuff

So, effectively with the above we have

diff-channels =3D <1 0>;

but then wait, not so fast

adi,channel-type =3D "single-ended"

To me the above is equally awkward (not sure if there's any precedence in u=
sing diff-
channels like this though)...

I would like for this to be explicit... If we have diff-channels, then it's=
 surely
differential.

If not we could use the single-channel property and instead of using an ext=
ra flag we
could make the property having either 1 or 2 items. If we have 1, then it's=
 a current
channel. If we have 2, then it's voltage single-ended/pseudo-differential.=
=20

David's suggestion is also pretty good (and I like it's more explicit about=
 what's
going on) so I would likely go with it...

- Nuno S=C3=A1


> > > >=20