Received-SPF: pass (google.com: domain of linux-kernel+bounces-133951-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Sat, 6 Apr 2024 15:26:37 +0100
From: Jonathan Cameron <jic23@kernel.org>
To: "Ceclan, Dumitru" <mitrutzceclan@gmail.com>
Cc: David Lechner <dlechner@baylibre.com>, dumitru.ceclan@analog.com,
 Lars-Peter Clausen <lars@metafoo.de>, Michael Hennerich
 <Michael.Hennerich@analog.com>, Rob Herring <robh@kernel.org>, Krzysztof
 Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley
 <conor+dt@kernel.org>, linux-iio@vger.kernel.org,
 devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/6] dt-bindings: adc: ad7173: add support for ad411x
Message-ID: <20240406152637.5d53c34f@jic23-huawei>
In-Reply-To: <aa84a3c5-a3e6-4c76-9b67-624ed8d8c704@gmail.com>
References: <20240401-ad4111-v1-0-34618a9cc502@analog.com>
	<20240401-ad4111-v1-1-34618a9cc502@analog.com>
	<CAMknhBHeKAQ45=5-dL1T1tv-mZcPN+bNo3vxWJYgWpEPE+8p3Q@mail.gmail.com>
	<CAMknhBGVuMSg+OpS5QTLWi9vA=Xa33AJ+cVS8ZCDyKsAVEe-ww@mail.gmail.com>
	<0a72de29-6d25-4d2d-9824-ca407af69153@gmail.com>
	<CAMknhBHhxi7mN88+peU7BGkzSP2vtipCuvM-XfQzgusqKvARsg@mail.gmail.com>
	<aa84a3c5-a3e6-4c76-9b67-624ed8d8c704@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On Thu, 4 Apr 2024 16:08:56 +0300
"Ceclan, Dumitru" <mitrutzceclan@gmail.com> wrote:

> On 03/04/2024 18:22, David Lechner wrote:
> > On Wed, Apr 3, 2024 at 2:50=E2=80=AFAM Ceclan, Dumitru <mitrutzceclan@g=
mail.com> wrote: =20
> >> On 02/04/2024 00:16, David Lechner wrote: =20
> >>> On Mon, Apr 1, 2024 at 2:37=E2=80=AFPM David Lechner <dlechner@baylib=
re.com> wrote: =20
> >>>> On Mon, Apr 1, 2024 at 10:10=E2=80=AFAM Dumitru Ceclan via B4 Relay
> >>>> <devnull+dumitru.ceclan.analog.com@kernel.org> wrote: =20
> >>>>> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
> >>>>> =20
> >> ...
> >> =20
> >>>>>      properties:
> >>>>>        reg:
> >>>>> +        description:
> >>>>> +          Reg values 16-19 are only permitted for ad4111/ad4112 cu=
rrent channels.
> >>>>>          minimum: 0
> >>>>> -        maximum: 15
> >>>>> +        maximum: 19 =20
> >>>> This looks wrong. Isn't reg describing the number of logical channels
> >>>> (# of channel config registers)?
> >>>>
> >>>> After reviewing the driver, I see that > 16 is used as a way of
> >>>> flagging current inputs, but still seems like the wrong way to do it.
> >>>> See suggestion below.
> >>>> =20
> >>>>>        diff-channels:
> >>>>> +        description:
> >>>>> +          For using current channels specify only the positive cha=
nnel.
> >>>>> +            (IIN2+, IIN2=E2=88=92) -> diff-channels =3D <2 0> =20
> >>>> I find this a bit confusing since 2 is already VIN2 and 0 is already
> >>>> VIN0. I think it would make more sense to assign unique channel
> >>>> numbers individually to the negative and positive current inputs.
> >>>> Also, I think it makes sense to use the same numbers that the
> >>>> registers in the datasheet use (8 - 11 for negative and 12 to 15 for
> >>>> positive).
> >>>>
> >>>> So: (IIN2+, IIN2=E2=88=92) -> diff-channels =3D <13 10> =20
> >>> Thinking about this a bit more...
> >>>
> >>> Since the current inputs have dedicated pins and aren't mix-and-match
> >>> with multiple valid wiring configurations like the voltage inputs, do
> >>> we even need to describe them in the devicetree?
> >>>
> >>> In the driver, the current channels would just be hard-coded like the
> >>> temperature channel since there isn't any application-specific
> >>> variation. =20
> >>  Sure, but we still need to offer the user a way to configure which
> >> current inputs he wants and if they should use bipolar or unipolar cod=
ing. =20
> > From the datasheet, it looks like only positive current input is
> > allowed so I'm not sure bipolar applies here. But, yes, if there is
> > some other variation in wiring or electrical signal that needs to be
> > describe here, then it makes sense to allow a channel configuration
> > node for it. =20
>=20
> AD4111 datasheet pg.29:
> When the ADC is configured for bipolar operation, the output
> code is offset binary with a negative full-scale voltage resulting
> in a code of 000 =E2=80=A6 000, a zero differential input voltage resulti=
ng in
> a code of 100 =E2=80=A6 000, and a positive full-scale input voltage
> resulting in a code of 111 =E2=80=A6 111. The output code for any
> analog input voltage can be represented as
> Code =3D 2^(N =E2=80=93 1) =C3=97 ((V_IN =C3=97 0.1/V REF) + 1)
> The output code for any input current is represented as
> Code =3D 2^(N =E2=88=92 1) =C3=97 ((I_IN =C3=97 50 =CE=A9/V REF) + 1)
>=20
> I would say bipolar applies here, not a great idea because of the limitat=
ion on
>  the negative side (Input Current Range min:=E2=88=920.5 max:+24 mA) so s=
till, the option
>  is available.
Just to check I am correct in thinking you 'might' use bipolar if you want
to be able to measure small negative currents, but the range is much larger
in the positive direction?

J
>=20