Date: Sat, 20 Apr 2024 11:22:27 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Nuno Sá <noname.nuno@...il.com>
Cc: David Lechner <dlechner@...libre.com>, Kim Seer Paller
 <kimseer.paller@...log.com>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, Lars-Peter
 Clausen <lars@...afoo.de>, Rob Herring <robh@...nel.org>, Krzysztof
 Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley
 <conor+dt@...nel.org>, Liam Girdwood <lgirdwood@...il.com>, Mark Brown
 <broonie@...nel.org>, Michael Hennerich <michael.hennerich@...log.com>
Subject: Re: [PATCH 2/4] iio: ABI: add ABI file for the LTC2664 DAC

On Mon, 15 Apr 2024 14:45:52 +0200
Nuno Sá <noname.nuno@...il.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@...libre.com> wrote:
> >   
> > > On Thu, Apr 11, 2024 at 10:21 PM Kim Seer Paller
> > > <kimseer.paller@...log.com> wrote:  
> > > > 
> > > > Define the sysfs interface for toggle capable channels.
> > > > 
> > > > Toggle enabled channels will have:
> > > > 
> > > >  * out_voltageY_toggle_en    
> > 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 generalized
> > beyond 1 device (unless you include PSK / FSK DDS drivers that are 'still'
> > after
> > 13+ years in staging!)
> > 
> > This patch needs to be generalizing that documentation from the ltc2688.
> > 
> > Probably in sysfs-bus-iio-dac
> >   
> > > 
> > > It looks like there are 3 toggle modes.
> > > 
> > > Two involve the notion of "enabled" outputs that I assume this attribute is
> > > for:
> > > 
> > > 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
> > >   
> > 
> > This is presumably the tricky one as that hardware toggle may not be in
> > control of the host CPU.
> >   
> > > 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.
> > > 
> > > 3. Toggling all pins to A or B based on the toggle select register. No
> > > notion of enabled pins here.
> > > 
> > > 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?  
> > 
> > This one feels like it could be handled as a software optimisation over
> > just changing the DAC value directly.  
> 
> Things may be slightly different in these devices. But for ltc2688 and AFAIR,
> the symbol attribute is about toggling between A and B through SW (not really
> 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 would
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

Powered by blists - more mailing lists