[2/4] iio: ABI: add ABI file for the LTC2664 DAC

Commit Message

Kim Seer Paller April 12, 2024, 3:21 a.m. UTC

Define the sysfs interface for toggle capable channels.

Toggle enabled channels will have:

 * out_voltageY_toggle_en
 * out_voltageY_raw0
 * out_voltageY_raw1
 * out_voltageY_symbol

The common interface present in all channels is:

 * out_voltageY_raw (not present in toggle enabled channels)
 * out_voltageY_raw_available
 * out_voltageY_powerdown
 * out_voltageY_scale
 * out_voltageY_offset

Co-developed-by: Michael Hennerich <[e-mail]>
Signed-off-by: Michael Hennerich <[e-mail]>
Signed-off-by: Kim Seer Paller <[e-mail]>
---
 .../ABI/testing/sysfs-bus-iio-dac-ltc2664     | 30 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 31 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664

Comments

David Lechner April 12, 2024, 9:26 p.m. UTC | #1

On Thu, Apr 11, 2024 at 10:21 PM Kim Seer Paller
<[e-mail]> wrote:
>
> Define the sysfs interface for toggle capable channels.
>
> Toggle enabled channels will have:
>
>  * out_voltageY_toggle_en

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

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?

In any case, it would be helpful if the documentation below explained
a bit better the intention and conditions required to use each
attribute (or add a .rst documentation file for these chips to explain
how to use it in more detail since this is rather complex feature).

>  * out_voltageY_raw0
>  * out_voltageY_raw1

I guess there is no enum iio_modifier that fits this. It seems like we
could still have out_voltageY_raw for register A so that users that
don't need to do any toggling can use standard ABI. And maybe
out_voltageY_raw_toggled for register B (question for Jonathan)?

Or just have 8 channels instead of 4 where even channels are register
A and odd channels are register B?

>  * out_voltageY_symbol

"symbol" is a confusing name. It sounds like this just supports
toggling one channel individually so _toggle_select would make more
sense to me. Are there plans for supporting toggling multiple channels
at the same time using a software trigger as well?

>
> The common interface present in all channels is:
>
>  * out_voltageY_raw (not present in toggle enabled channels)

As mentioned above, I don't think we need to have to make a
distinction between toggle enabled channels and not enabled channels.

>  * out_voltageY_raw_available
>  * out_voltageY_powerdown

Is _powerdown a standard attribute? I don't see it documented
anywhere. Perhaps you meant _en (via IIO_CHAN_INFO_ENABLE)?


>  * out_voltageY_scale
>  * out_voltageY_offset
>
> Co-developed-by: Michael Hennerich <[e-mail]>
> Signed-off-by: Michael Hennerich <[e-mail]>
> Signed-off-by: Kim Seer Paller <[e-mail]>
> ---
>  .../ABI/testing/sysfs-bus-iio-dac-ltc2664     | 30 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 31 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664 b/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664
> new file mode 100644
> index 000000000..4b656b7af
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664
> @@ -0,0 +1,30 @@
> +What:          /sys/bus/iio/devices/iio:deviceX/out_voltageY_toggle_en
> +KernelVersion: 5.18
> +Contact:       [e-mail]
> +Description:
> +               Toggle enable. Write 1 to enable toggle or 0 to disable it. This is
> +               useful when one wants to change the DAC output codes. The way it should
> +               be done is:
> +
> +               - disable toggle operation;
> +               - change out_voltageY_raw0 and out_voltageY_raw1;
> +               - enable toggle operation.
> +
> +What:          /sys/bus/iio/devices/iio:deviceX/out_voltageY_raw0
> +What:          /sys/bus/iio/devices/iio:deviceX/out_voltageY_raw1
> +KernelVersion: 5.18
> +Contact:       [e-mail]
> +Description:
> +               It has the same meaning as out_voltageY_raw. This attribute is
> +               specific to toggle enabled channels and refers to the DAC output
> +               code in INPUT_A (_raw0) and INPUT_B (_raw1). The same scale and offset
> +               as in out_voltageY_raw applies.
> +
> +What:          /sys/bus/iio/devices/iio:deviceX/out_voltageY_symbol
> +KernelVersion: 5.18
> +Contact:       [e-mail]
> +Description:
> +               Performs a SW toggle. This attribute is specific to toggle
> +               enabled channels and allows to toggle between out_voltageY_raw0
> +               and out_voltageY_raw1 through software. Writing 0 will select
> +               out_voltageY_raw0 while 1 selects out_voltageY_raw1.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bd8645f6e..9ed00b364 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12842,6 +12842,7 @@ M:      Kim Seer Paller <[e-mail]>
>  L:     [e-mail]
>  S:     Supported
>  W:     https://ez.analog.com/linux-software-drivers
> +F:     Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664
>  F:     Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
>
>  LTC2688 IIO DAC DRIVER
> --
> 2.34.1
>

Jonathan Cameron April 13, 2024, 3:25 p.m. UTC | #2

On Fri, 12 Apr 2024 16:26:17 -0500
David Lechner <[e-mail]> wrote:

> On Thu, Apr 11, 2024 at 10:21 PM Kim Seer Paller
> <[e-mail]> 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.

> 
> In any case, it would be helpful if the documentation below explained
> a bit better the intention and conditions required to use each
> attribute (or add a .rst documentation file for these chips to explain
> how to use it in more detail since this is rather complex feature).
> 
> >  * out_voltageY_raw0
> >  * out_voltageY_raw1  
> 
> I guess there is no enum iio_modifier that fits this. It seems like we
> could still have out_voltageY_raw for register A so that users that
> don't need to do any toggling can use standard ABI. And maybe
> out_voltageY_raw_toggled for register B (question for Jonathan)?

There is precedence for doing it like this (ltc2688)
Note that we should only see these attribute for changes specifically
configured for 'hardware' triggered toggling.

Note that we cannot have duplicate documentation so we need to create
a common docs file covering this and existing ltc2688 ABI that overlaps.
That may need some generalising to cover both parts.

> 
> Or just have 8 channels instead of 4 where even channels are register
> A and odd channels are register B?
> 
> >  * out_voltageY_symbol  
> 
> "symbol" is a confusing name. It sounds like this just supports
> toggling one channel individually so _toggle_select would make more
> sense to me. Are there plans for supporting toggling multiple channels
> at the same time using a software trigger as well?

Again, precedence should have been called out.  It's not great ABI
but it corresponds to earlier work on Frequency Shift Keying DDS devices
(and I think Phase Shift Keying as well) in which this is call symbol.
Hence the name.

> 
> >
> > The common interface present in all channels is:
> >
> >  * out_voltageY_raw (not present in toggle enabled channels)  
> 
> As mentioned above, I don't think we need to have to make a
> distinction between toggle enabled channels and not enabled channels.

Was a while back but I think that last time this turned up we concluded
we did need a different interface because the current 'toggle value'
is not in our control.  Hence you are programming one channel that
does different things - think of it as setting the Max and Min values
for a generated waveform - perhaps the toggle pin is connected to a PWM
for example.

> 
> >  * out_voltageY_raw_available
> >  * out_voltageY_powerdown  
> 
> Is _powerdown a standard attribute? I don't see it documented
> anywhere. Perhaps you meant _en (via IIO_CHAN_INFO_ENABLE)?

It's there in Documentation/ABI/testing/sysfs-bus-iio
Different form simple enable (which came much later as ABI) because
it means entering a powerdown state in which a particular thing happens
on the output pin.  It is defined alongside powerdown_mode which 
defines what happens. (often a choice between different impedance / High Z etc)


> 
> 
> >  * out_voltageY_scale
> >  * out_voltageY_offset
> >
> > Co-developed-by: Michael Hennerich <[e-mail]>
> > Signed-off-by: Michael Hennerich <[e-mail]>
> > Signed-off-by: Kim Seer Paller <[e-mail]>
> > ---
> >  .../ABI/testing/sysfs-bus-iio-dac-ltc2664     | 30 +++++++++++++++++++
> >  MAINTAINERS                                   |  1 +
> >  2 files changed, 31 insertions(+)
> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664 b/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664
> > new file mode 100644
> > index 000000000..4b656b7af
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664
> > @@ -0,0 +1,30 @@
> > +What:          /sys/bus/iio/devices/iio:deviceX/out_voltageY_toggle_en
> > +KernelVersion: 5.18
> > +Contact:       [e-mail]
> > +Description:
> > +               Toggle enable. Write 1 to enable toggle or 0 to disable it. This is
> > +               useful when one wants to change the DAC output codes. The way it should
> > +               be done is:
> > +
> > +               - disable toggle operation;
> > +               - change out_voltageY_raw0 and out_voltageY_raw1;
> > +               - enable toggle operation.
> > +
> > +What:          /sys/bus/iio/devices/iio:deviceX/out_voltageY_raw0
> > +What:          /sys/bus/iio/devices/iio:deviceX/out_voltageY_raw1
> > +KernelVersion: 5.18
> > +Contact:       [e-mail]
> > +Description:
> > +               It has the same meaning as out_voltageY_raw. This attribute is
> > +               specific to toggle enabled channels and refers to the DAC output
> > +               code in INPUT_A (_raw0) and INPUT_B (_raw1). The same scale and offset
> > +               as in out_voltageY_raw applies.
> > +
> > +What:          /sys/bus/iio/devices/iio:deviceX/out_voltageY_symbol
> > +KernelVersion: 5.18
> > +Contact:       [e-mail]
> > +Description:
> > +               Performs a SW toggle. This attribute is specific to toggle
> > +               enabled channels and allows to toggle between out_voltageY_raw0
> > +               and out_voltageY_raw1 through software. Writing 0 will select
> > +               out_voltageY_raw0 while 1 selects out_voltageY_raw1.
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index bd8645f6e..9ed00b364 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -12842,6 +12842,7 @@ M:      Kim Seer Paller <[e-mail]>
> >  L:     [e-mail]
> >  S:     Supported
> >  W:     https://ez.analog.com/linux-software-drivers
> > +F:     Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664
> >  F:     Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
> >
> >  LTC2688 IIO DAC DRIVER
> > --
> > 2.34.1
> >

David Lechner April 13, 2024, 8:38 p.m. UTC | #3

On 4/13/24 10:25 AM, Jonathan Cameron wrote:
> On Fri, 12 Apr 2024 16:26:17 -0500
> David Lechner <[e-mail]> wrote:
> 
>> On Thu, Apr 11, 2024 at 10:21 PM Kim Seer Paller
>> <[e-mail]> 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.
> 
>>
>> In any case, it would be helpful if the documentation below explained
>> a bit better the intention and conditions required to use each
>> attribute (or add a .rst documentation file for these chips to explain
>> how to use it in more detail since this is rather complex feature).
>>
>>>  * out_voltageY_raw0
>>>  * out_voltageY_raw1  
>>
>> I guess there is no enum iio_modifier that fits this. It seems like we
>> could still have out_voltageY_raw for register A so that users that
>> don't need to do any toggling can use standard ABI. And maybe
>> out_voltageY_raw_toggled for register B (question for Jonathan)?
> 
> There is precedence for doing it like this (ltc2688)
> Note that we should only see these attribute for changes specifically
> configured for 'hardware' triggered toggling.
> 
> Note that we cannot have duplicate documentation so we need to create
> a common docs file covering this and existing ltc2688 ABI that overlaps.
> That may need some generalising to cover both parts.
> 
>>
>> Or just have 8 channels instead of 4 where even channels are register
>> A and odd channels are register B?
>>
>>>  * out_voltageY_symbol  
>>
>> "symbol" is a confusing name. It sounds like this just supports
>> toggling one channel individually so _toggle_select would make more
>> sense to me. Are there plans for supporting toggling multiple channels
>> at the same time using a software trigger as well?
> 
> Again, precedence should have been called out.  It's not great ABI
> but it corresponds to earlier work on Frequency Shift Keying DDS devices
> (and I think Phase Shift Keying as well) in which this is call symbol.
> Hence the name.
> 
>>
>>>
>>> The common interface present in all channels is:
>>>
>>>  * out_voltageY_raw (not present in toggle enabled channels)  
>>
>> As mentioned above, I don't think we need to have to make a
>> distinction between toggle enabled channels and not enabled channels.
> 
> Was a while back but I think that last time this turned up we concluded
> we did need a different interface because the current 'toggle value'
> is not in our control.  Hence you are programming one channel that
> does different things - think of it as setting the Max and Min values
> for a generated waveform - perhaps the toggle pin is connected to a PWM
> for example.
> 
>>
>>>  * out_voltageY_raw_available
>>>  * out_voltageY_powerdown  
>>
>> Is _powerdown a standard attribute? I don't see it documented
>> anywhere. Perhaps you meant _en (via IIO_CHAN_INFO_ENABLE)?
> 
> It's there in Documentation/ABI/testing/sysfs-bus-iio
> Different form simple enable (which came much later as ABI) because
> it means entering a powerdown state in which a particular thing happens
> on the output pin.  It is defined alongside powerdown_mode which 
> defines what happens. (often a choice between different impedance / High Z etc)
> 
> 
>>
>>
>>>  * out_voltageY_scale
>>>  * out_voltageY_offset
>>>
>>> Co-developed-by: Michael Hennerich <[e-mail]>
>>> Signed-off-by: Michael Hennerich <[e-mail]>
>>> Signed-off-by: Kim Seer Paller <[e-mail]>
>>> ---
>>>  .../ABI/testing/sysfs-bus-iio-dac-ltc2664     | 30 +++++++++++++++++++
>>>  MAINTAINERS                                   |  1 +
>>>  2 files changed, 31 insertions(+)
>>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664 b/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664
>>> new file mode 100644
>>> index 000000000..4b656b7af
>>> --- /dev/null
>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664
>>> @@ -0,0 +1,30 @@
>>> +What:          /sys/bus/iio/devices/iio:deviceX/out_voltageY_toggle_en
>>> +KernelVersion: 5.18
>>> +Contact:       [e-mail]
>>> +Description:
>>> +               Toggle enable. Write 1 to enable toggle or 0 to disable it. This is
>>> +               useful when one wants to change the DAC output codes. The way it should
>>> +               be done is:
>>> +
>>> +               - disable toggle operation;
>>> +               - change out_voltageY_raw0 and out_voltageY_raw1;
>>> +               - enable toggle operation.
>>> +
>>> +What:          /sys/bus/iio/devices/iio:deviceX/out_voltageY_raw0
>>> +What:          /sys/bus/iio/devices/iio:deviceX/out_voltageY_raw1
>>> +KernelVersion: 5.18
>>> +Contact:       [e-mail]
>>> +Description:
>>> +               It has the same meaning as out_voltageY_raw. This attribute is
>>> +               specific to toggle enabled channels and refers to the DAC output
>>> +               code in INPUT_A (_raw0) and INPUT_B (_raw1). The same scale and offset
>>> +               as in out_voltageY_raw applies.
>>> +
>>> +What:          /sys/bus/iio/devices/iio:deviceX/out_voltageY_symbol
>>> +KernelVersion: 5.18
>>> +Contact:       [e-mail]
>>> +Description:
>>> +               Performs a SW toggle. This attribute is specific to toggle
>>> +               enabled channels and allows to toggle between out_voltageY_raw0
>>> +               and out_voltageY_raw1 through software. Writing 0 will select
>>> +               out_voltageY_raw0 while 1 selects out_voltageY_raw1.
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index bd8645f6e..9ed00b364 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -12842,6 +12842,7 @@ M:      Kim Seer Paller <[e-mail]>
>>>  L:     [e-mail]
>>>  S:     Supported
>>>  W:     https://ez.analog.com/linux-software-drivers
>>> +F:     Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664
>>>  F:     Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
>>>
>>>  LTC2688 IIO DAC DRIVER
>>> --
>>> 2.34.1
>>>  
> 

Clearly I have a lot to learn on this one! Thanks for all of the info.

Nuno Sá April 15, 2024, 12:45 p.m. UTC | #4

On Sat, 2024-04-13 at 16:25 +0100, Jonathan Cameron wrote:
> On Fri, 12 Apr 2024 16:26:17 -0500
> David Lechner <[e-mail]> wrote:
> 
> > On Thu, Apr 11, 2024 at 10:21 PM Kim Seer Paller
> > <[e-mail]> 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.

> 
> > 
> > In any case, it would be helpful if the documentation below explained
> > a bit better the intention and conditions required to use each
> > attribute (or add a .rst documentation file for these chips to explain
> > how to use it in more detail since this is rather complex feature).
> > 
> > >  * out_voltageY_raw0
> > >  * out_voltageY_raw1  
> > 
> > I guess there is no enum iio_modifier that fits this. It seems like we
> > could still have out_voltageY_raw for register A so that users that
> > don't need to do any toggling can use standard ABI. And maybe
> > out_voltageY_raw_toggled for register B (question for Jonathan)?
> 
> There is precedence for doing it like this (ltc2688)
> Note that we should only see these attribute for changes specifically
> configured for 'hardware' triggered toggling.
> 
> Note that we cannot have duplicate documentation so we need to create
> a common docs file covering this and existing ltc2688 ABI that overlaps.
> That may need some generalising to cover both parts.
> 
> > 
> > Or just have 8 channels instead of 4 where even channels are register
> > A and odd channels are register B?
> > 
> > >  * out_voltageY_symbol  
> > 
> > "symbol" is a confusing name. It sounds like this just supports
> > toggling one channel individually so _toggle_select would make more
> > sense to me. Are there plans for supporting toggling multiple channels
> > at the same time using a software trigger as well?
> 
> Again, precedence should have been called out.  It's not great ABI
> but it corresponds to earlier work on Frequency Shift Keying DDS devices
> (and I think Phase Shift Keying as well) in which this is call symbol.
> Hence the name.
> 
> > 
> > > 
> > > The common interface present in all channels is:
> > > 
> > >  * out_voltageY_raw (not present in toggle enabled channels)  
> > 
> > As mentioned above, I don't think we need to have to make a
> > distinction between toggle enabled channels and not enabled channels.
> 
> Was a while back but I think that last time this turned up we concluded
> we did need a different interface because the current 'toggle value'
> is not in our control.  Hence you are programming one channel that
> does different things - think of it as setting the Max and Min values
> for a generated waveform - perhaps the toggle pin is connected to a PWM
> for example.
> 

Yeah and for the ltc2688 we also had the dither mode that has it's own unique
set of interfaces. Trying to deal with it all at runtime could be more confusing
than beneficial I guess.

- Nuno Sá
>

Jonathan Cameron April 20, 2024, 10:22 a.m. UTC | #5

On Mon, 15 Apr 2024 14:45:52 +0200
Nuno Sá <[e-mail]> wrote:

> On Sat, 2024-04-13 at 16:25 +0100, Jonathan Cameron wrote:
> > On Fri, 12 Apr 2024 16:26:17 -0500
> > David Lechner <[e-mail]> wrote:
> >   
> > > On Thu, Apr 11, 2024 at 10:21 PM Kim Seer Paller
> > > <[e-mail]> 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

Patch

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664 b/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664
new file mode 100644
index 000000000..4b656b7af
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664
@@ -0,0 +1,30 @@ 
+What:		/sys/bus/iio/devices/iio:deviceX/out_voltageY_toggle_en
+KernelVersion:	5.18
+Contact:	[e-mail]
+Description:
+		Toggle enable. Write 1 to enable toggle or 0 to disable it. This is
+		useful when one wants to change the DAC output codes. The way it should
+		be done is:
+
+		- disable toggle operation;
+		- change out_voltageY_raw0 and out_voltageY_raw1;
+		- enable toggle operation.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_voltageY_raw0
+What:		/sys/bus/iio/devices/iio:deviceX/out_voltageY_raw1
+KernelVersion:	5.18
+Contact:	[e-mail]
+Description:
+		It has the same meaning as out_voltageY_raw. This attribute is
+		specific to toggle enabled channels and refers to the DAC output
+		code in INPUT_A (_raw0) and INPUT_B (_raw1). The same scale and offset
+		as in out_voltageY_raw applies.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_voltageY_symbol
+KernelVersion:	5.18
+Contact:	[e-mail]
+Description:
+		Performs a SW toggle. This attribute is specific to toggle
+		enabled channels and allows to toggle between out_voltageY_raw0
+		and out_voltageY_raw1 through software. Writing 0 will select
+		out_voltageY_raw0 while 1 selects out_voltageY_raw1.
diff --git a/MAINTAINERS b/MAINTAINERS
index bd8645f6e..9ed00b364 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12842,6 +12842,7 @@  M:	Kim Seer Paller <[e-mail]>
 L:	[e-mail]
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2664
 F:	Documentation/devicetree/bindings/iio/dac/adi,ltc2664.yaml
 
 LTC2688 IIO DAC DRIVER