Received-SPF: pass (google.com: domain of linux-kernel+bounces-152225-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Sat, 20 Apr 2024 11:22:27 +0100
From: Jonathan Cameron <jic23@kernel.org>
To: Nuno =?UTF-8?B?U8Oh?= <noname.nuno@gmail.com>
Cc: David Lechner <dlechner@baylibre.com>, Kim Seer Paller
 <kimseer.paller@analog.com>, linux-iio@vger.kernel.org,
 devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Lars-Peter
 Clausen <lars@metafoo.de>, Rob Herring <robh@kernel.org>, Krzysztof
 Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley
 <conor+dt@kernel.org>, Liam Girdwood <lgirdwood@gmail.com>, Mark Brown
 <broonie@kernel.org>, Michael Hennerich <michael.hennerich@analog.com>
Subject: Re: [PATCH 2/4] iio: ABI: add ABI file for the LTC2664 DAC
Message-ID: <20240420112227.68577ac0@jic23-huawei>
In-Reply-To: <79734a9f54e320cd09aab83015f1c0e6d93e700c.camel@gmail.com>
References: <20240412032102.136071-1-kimseer.paller@analog.com>
	<20240412032102.136071-3-kimseer.paller@analog.com>
	<CAMknhBE9XihK27pRhyPwTNM3VQX=osYdDyCmjNspz1aqe_NVTw@mail.gmail.com>
	<20240413162517.4644b649@jic23-huawei>
	<79734a9f54e320cd09aab83015f1c0e6d93e700c.camel@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On Mon, 15 Apr 2024 14:45:52 +0200
Nuno S=C3=A1 <noname.nuno@gmail.com> wrote:

> On Sat, 2024-04-13 at 16:25 +0100, Jonathan Cameron wrote:
> > On Fri, 12 Apr 2024 16:26:17 -0500
> > David Lechner <dlechner@baylibre.com> wrote:
> >  =20
> > > On Thu, Apr 11, 2024 at 10:21=E2=80=AFPM Kim Seer Paller
> > > <kimseer.paller@analog.com> wrote: =20
> > > >=20
> > > > Define the sysfs interface for toggle capable channels.
> > > >=20
> > > > Toggle enabled channels will have:
> > > >=20
> > > > =C2=A0* out_voltageY_toggle_en=C2=A0  =20
> > The big missing thing in this ABI is a reference to existing precedence.
> > You aren't actually defining anything new, it just hasn't yet been gene=
ralized
> > beyond 1 device (unless you include PSK / FSK DDS drivers that are 'sti=
ll'
> > after
> > 13+ years in staging!)
> >=20
> > This patch needs to be generalizing that documentation from the ltc2688.
> >=20
> > Probably in sysfs-bus-iio-dac
> >  =20
> > >=20
> > > It looks like there are 3 toggle modes.
> > >=20
> > > Two involve the notion of "enabled" outputs that I assume this attrib=
ute is
> > > for:
> > >=20
> > > 1. Toggling all enabled pins at the same time using a software trigger
> > > (global toggle bit)
> > > 2. Toggling all enabled pins at the same time using a hardware trigger
> > > (TGP pin) and toggling pins
> > >  =20
> >=20
> > This is presumably the tricky one as that hardware toggle may not be in
> > control of the host CPU.
> >  =20
> > > The third mode though looks like it uses the same toggle select
> > > register for selecting A or B for each channel instead of enabling or
> > > disabling each channel.
> > >=20
> > > 3. Toggling all pins to A or B based on the toggle select register. No
> > > notion of enabled pins here.
> > >=20
> > > I haven't looked at the driver implementation, but it sounds like
> > > out_voltageY_toggle_en and out_voltageY_symbol would be writing to the
> > > same register in conflicting ways. So maybe we need yet another custom
> > > attribute to select the currently active toggle mode? =20
> >=20
> > This one feels like it could be handled as a software optimisation over
> > just changing the DAC value directly. =20
>=20
> Things may be slightly different in these devices. But for ltc2688 and AF=
AIR,
> the symbol attribute is about toggling between A and B through SW (not re=
ally
> enabling the mode). That interface will only pop up if there's no HW (PWM=
 for
> example) toggle present.

I can't remember if we discussed it at the time of that driver,
but from a userspace interface point of view, for a single channel there wo=
uld
be little point in this. I guess the key is it simultaneously switches
a bunch of channels.  Perhaps we can make that clearer in the ABI docs
(if it isn't already clear enough!)

So a software interface does seem appropriate.

There is a fun question of whether the toggle select is useful to software.
That is picking which of A or B each output uses for next toggle.
At first glance I don't think so, but I'm open to people suggesting why
that might need a userspace interface.
Superficially feels like anything that can be done with that interface can
also be done keeping all channels toggling to A or all to B at one time and
potentially a few more register writes.

Jonathan