Re: [PATCH v2 5/5] iio: dac: stm32: add support for waveform generator

2017-04-14 Thread Jonathan Cameron 
On 10/04/17 17:43, Fabrice Gasnier wrote:
> On 04/09/2017 11:34 AM, Jonathan Cameron wrote:
>> On 06/04/17 17:11, Fabrice Gasnier wrote:
>>> STM32 DAC has built-in noise or triangle waveform generator.
>>> - "wavetype" extended attribute selects noise or triangle.
>>> - "amplitude" extended attribute selects amplitude for waveform generator
>>>
>>> A DC offset can be added to waveform generator output. This can be done
>>> using out_voltage[1/2]_offset
>>>
>>> Waveform generator requires a trigger to be configured, to increment /
>>> decrement internal counter in case of triangle generator. Noise
>>> generator is a bit different,  but also requires a trigger to generate
>>> samples.
>>>
>>> Signed-off-by: Fabrice Gasnier 
>>
>> Various bits inline.  Mostly I think the blockers on this will be
>> making sure the ABI defined is generic enough to handle the more crazy
>> DDS chips out there... (basically the ones doing frequency modulation).
>>
>> Jonathan
>>> ---
>>> Changes in v2:
>>> - use _offset parameter to add DC offset to waveform generator
>> Conceptually this offset is just the normal DAC output value (particularly
> yes
>> in the flat case)  I guess we can paper over this by having the _raw
>> and this always have th same value, but it's a little inelegant.
>> Still people are going to expect _raw to control it when in DAC mode but
>> that makes limited sense in DDS mode.
>>
>> Mind you nothing stops us defining all DDS channels as the sum of whatever
>> the DDS is doing and whatever is _raw is set to. Perhaps we tidy this up
>> purely through documentation.  Think of the DDS as a modulation on top
>> of the DAC...
>>
>>> - Rework ABI to better fit existing DDS ABI: use out_voltageY_wavetype,
>>>   out_voltage_wavetype_available, out_voltageY_amplitude,
>>>   out_voltage_amplitude_available
>> Hmm. I'm thinking those amplitude values aren't nice and don't fit well
>> with the more general ABI.
>>
>> I suggested (but didn't really expand upon) having standard defined types
>> for each waveform then using scale to control the amplitude.
> 
> Do you mean _scale attribute ?
Yes
>>
>> Is that something that might work here?
> 
> I probably miss the point here...
>>
>> So say we have our triangle standard form having an amplitude of 1V Peak to
>> Peak. Then we can use scale to make it whatever we actually have in this
>> case?  The docs for wave type will need to describe those standard forms
>> though.
> ... scale is fixed here, in line with _raw attribute. In 'amplitude'
> description for STM32 DAC here (e.g. from 1...4095), same scale is used.
> Can you elaborate ?
Good point - it is already effectively been applied to the _raw
value - I'd forgotten that.

Seems like abuse of the ABI to use calibscale, so we might need something
new here - wavescale maybe or ddsscale?  Not sure.
> 
>>
>> Hmm. Whether this is worth doing is unclear however as we'll still have
>> to describe the 'frequency' in terms of the clock ticks (here the triggers)
> 
> Describing frequency may be an issue, not sure it makes senses in this
> case: 'clock ticks', e.g. triggers here may be timers, but also an EXTI
> (external...), or even software trig perhaps.
To describe the waveform in a remotely standard way we'll have to address
this to some degree.  What's the slope?  
> 
>> So maybe amplitude is worth having.  Again, looking for input from ADI lot
>> on this...  There are some really fiddly cases to describe were we are doing
>> symbol encoding so have multiple waveforms that we are switching between 
>> based
>> on some external signal. Any ABI needs to encompass that sort of usage.
>> Parts like the AD9833 for example...
>>
>>> - Better explain trigger usage in case of waveform generator.
>>> ---
>>>  Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 |  16 +++
>>>  drivers/iio/dac/stm32-dac-core.h  |   4 +
>>>  drivers/iio/dac/stm32-dac.c   | 158 
>>> +-
>>>  3 files changed, 177 insertions(+), 1 deletion(-)
>>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 
>>> b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>>> new file mode 100644
>>> index 000..8f1fa009
>>> --- /dev/null
>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>> Fair enough to initially introduced these for this part only, but I'd very
>> much like to see us agree on these sufficiently to get them into the main
>> docs asap so we have something to work with for getting the DDS chips out
>> of staging...
>>> @@ -0,0 +1,16 @@
>>> +What:  /sys/bus/iio/devices/iio:deviceX/out_voltageY_wavetype
>>> +What:  
>>> /sys/bus/iio/devices/iio:deviceX/out_voltage_wavetype_available
>>> +KernelVersion: 4.12
>>> +Contact:   fabrice.gasn...@st.com
>>> +Description:
>>> +   List and/or select waveform generation provided by STM32 DAC:
>>> +

Re: [PATCH v2 5/5] iio: dac: stm32: add support for waveform generator

2017-04-14 Thread Jonathan Cameron 
On 10/04/17 17:43, Fabrice Gasnier wrote:
> On 04/09/2017 11:34 AM, Jonathan Cameron wrote:
>> On 06/04/17 17:11, Fabrice Gasnier wrote:
>>> STM32 DAC has built-in noise or triangle waveform generator.
>>> - "wavetype" extended attribute selects noise or triangle.
>>> - "amplitude" extended attribute selects amplitude for waveform generator
>>>
>>> A DC offset can be added to waveform generator output. This can be done
>>> using out_voltage[1/2]_offset
>>>
>>> Waveform generator requires a trigger to be configured, to increment /
>>> decrement internal counter in case of triangle generator. Noise
>>> generator is a bit different,  but also requires a trigger to generate
>>> samples.
>>>
>>> Signed-off-by: Fabrice Gasnier 
>>
>> Various bits inline.  Mostly I think the blockers on this will be
>> making sure the ABI defined is generic enough to handle the more crazy
>> DDS chips out there... (basically the ones doing frequency modulation).
>>
>> Jonathan
>>> ---
>>> Changes in v2:
>>> - use _offset parameter to add DC offset to waveform generator
>> Conceptually this offset is just the normal DAC output value (particularly
> yes
>> in the flat case)  I guess we can paper over this by having the _raw
>> and this always have th same value, but it's a little inelegant.
>> Still people are going to expect _raw to control it when in DAC mode but
>> that makes limited sense in DDS mode.
>>
>> Mind you nothing stops us defining all DDS channels as the sum of whatever
>> the DDS is doing and whatever is _raw is set to. Perhaps we tidy this up
>> purely through documentation.  Think of the DDS as a modulation on top
>> of the DAC...
>>
>>> - Rework ABI to better fit existing DDS ABI: use out_voltageY_wavetype,
>>>   out_voltage_wavetype_available, out_voltageY_amplitude,
>>>   out_voltage_amplitude_available
>> Hmm. I'm thinking those amplitude values aren't nice and don't fit well
>> with the more general ABI.
>>
>> I suggested (but didn't really expand upon) having standard defined types
>> for each waveform then using scale to control the amplitude.
> 
> Do you mean _scale attribute ?
Yes
>>
>> Is that something that might work here?
> 
> I probably miss the point here...
>>
>> So say we have our triangle standard form having an amplitude of 1V Peak to
>> Peak. Then we can use scale to make it whatever we actually have in this
>> case?  The docs for wave type will need to describe those standard forms
>> though.
> ... scale is fixed here, in line with _raw attribute. In 'amplitude'
> description for STM32 DAC here (e.g. from 1...4095), same scale is used.
> Can you elaborate ?
Good point - it is already effectively been applied to the _raw
value - I'd forgotten that.

Seems like abuse of the ABI to use calibscale, so we might need something
new here - wavescale maybe or ddsscale?  Not sure.
> 
>>
>> Hmm. Whether this is worth doing is unclear however as we'll still have
>> to describe the 'frequency' in terms of the clock ticks (here the triggers)
> 
> Describing frequency may be an issue, not sure it makes senses in this
> case: 'clock ticks', e.g. triggers here may be timers, but also an EXTI
> (external...), or even software trig perhaps.
To describe the waveform in a remotely standard way we'll have to address
this to some degree.  What's the slope?  
> 
>> So maybe amplitude is worth having.  Again, looking for input from ADI lot
>> on this...  There are some really fiddly cases to describe were we are doing
>> symbol encoding so have multiple waveforms that we are switching between 
>> based
>> on some external signal. Any ABI needs to encompass that sort of usage.
>> Parts like the AD9833 for example...
>>
>>> - Better explain trigger usage in case of waveform generator.
>>> ---
>>>  Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 |  16 +++
>>>  drivers/iio/dac/stm32-dac-core.h  |   4 +
>>>  drivers/iio/dac/stm32-dac.c   | 158 
>>> +-
>>>  3 files changed, 177 insertions(+), 1 deletion(-)
>>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 
>>> b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>>> new file mode 100644
>>> index 000..8f1fa009
>>> --- /dev/null
>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>> Fair enough to initially introduced these for this part only, but I'd very
>> much like to see us agree on these sufficiently to get them into the main
>> docs asap so we have something to work with for getting the DDS chips out
>> of staging...
>>> @@ -0,0 +1,16 @@
>>> +What:  /sys/bus/iio/devices/iio:deviceX/out_voltageY_wavetype
>>> +What:  
>>> /sys/bus/iio/devices/iio:deviceX/out_voltage_wavetype_available
>>> +KernelVersion: 4.12
>>> +Contact:   fabrice.gasn...@st.com
>>> +Description:
>>> +   List and/or select waveform generation provided by STM32 DAC:
>>> +   - "flat": waveform

Re: [PATCH v2 5/5] iio: dac: stm32: add support for waveform generator

2017-04-10 Thread Fabrice Gasnier 
On 04/09/2017 11:34 AM, Jonathan Cameron wrote:
> On 06/04/17 17:11, Fabrice Gasnier wrote:
>> STM32 DAC has built-in noise or triangle waveform generator.
>> - "wavetype" extended attribute selects noise or triangle.
>> - "amplitude" extended attribute selects amplitude for waveform generator
>>
>> A DC offset can be added to waveform generator output. This can be done
>> using out_voltage[1/2]_offset
>>
>> Waveform generator requires a trigger to be configured, to increment /
>> decrement internal counter in case of triangle generator. Noise
>> generator is a bit different,  but also requires a trigger to generate
>> samples.
>>
>> Signed-off-by: Fabrice Gasnier 
> 
> Various bits inline.  Mostly I think the blockers on this will be
> making sure the ABI defined is generic enough to handle the more crazy
> DDS chips out there... (basically the ones doing frequency modulation).
> 
> Jonathan
>> ---
>> Changes in v2:
>> - use _offset parameter to add DC offset to waveform generator
> Conceptually this offset is just the normal DAC output value (particularly
yes
> in the flat case)  I guess we can paper over this by having the _raw
> and this always have th same value, but it's a little inelegant.
> Still people are going to expect _raw to control it when in DAC mode but
> that makes limited sense in DDS mode.
> 
> Mind you nothing stops us defining all DDS channels as the sum of whatever
> the DDS is doing and whatever is _raw is set to. Perhaps we tidy this up
> purely through documentation.  Think of the DDS as a modulation on top
> of the DAC...
> 
>> - Rework ABI to better fit existing DDS ABI: use out_voltageY_wavetype,
>>   out_voltage_wavetype_available, out_voltageY_amplitude,
>>   out_voltage_amplitude_available
> Hmm. I'm thinking those amplitude values aren't nice and don't fit well
> with the more general ABI.
> 
> I suggested (but didn't really expand upon) having standard defined types
> for each waveform then using scale to control the amplitude.

Do you mean _scale attribute ?
> 
> Is that something that might work here?

I probably miss the point here...
> 
> So say we have our triangle standard form having an amplitude of 1V Peak to
> Peak. Then we can use scale to make it whatever we actually have in this
> case?  The docs for wave type will need to describe those standard forms
> though.
... scale is fixed here, in line with _raw attribute. In 'amplitude'
description for STM32 DAC here (e.g. from 1...4095), same scale is used.
Can you elaborate ?

> 
> Hmm. Whether this is worth doing is unclear however as we'll still have
> to describe the 'frequency' in terms of the clock ticks (here the triggers)

Describing frequency may be an issue, not sure it makes senses in this
case: 'clock ticks', e.g. triggers here may be timers, but also an EXTI
(external...), or even software trig perhaps.

> So maybe amplitude is worth having.  Again, looking for input from ADI lot
> on this...  There are some really fiddly cases to describe were we are doing
> symbol encoding so have multiple waveforms that we are switching between based
> on some external signal. Any ABI needs to encompass that sort of usage.
> Parts like the AD9833 for example...
> 
>> - Better explain trigger usage in case of waveform generator.
>> ---
>>  Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 |  16 +++
>>  drivers/iio/dac/stm32-dac-core.h  |   4 +
>>  drivers/iio/dac/stm32-dac.c   | 158 
>> +-
>>  3 files changed, 177 insertions(+), 1 deletion(-)
>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 
>> b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>> new file mode 100644
>> index 000..8f1fa009
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
> Fair enough to initially introduced these for this part only, but I'd very
> much like to see us agree on these sufficiently to get them into the main
> docs asap so we have something to work with for getting the DDS chips out
> of staging...
>> @@ -0,0 +1,16 @@
>> +What:   /sys/bus/iio/devices/iio:deviceX/out_voltageY_wavetype
>> +What:   
>> /sys/bus/iio/devices/iio:deviceX/out_voltage_wavetype_available
>> +KernelVersion:  4.12
>> +Contact:fabrice.gasn...@st.com
>> +Description:
>> +List and/or select waveform generation provided by STM32 DAC:
>> +- "flat": waveform generator disabled (default)
>> +- "noise": select noise waveform
>> +- "triangle": select triangle waveform
>> +
>> +What:   /sys/bus/iio/devices/iio:deviceX/out_voltageY_amplitude
>> +What:   
>> /sys/bus/iio/devices/iio:deviceX/out_voltage_amplitude_available
>> +KernelVersion:  4.12
>> +Contact:fabrice.gasn...@st.com
>> +Description:
>> +List and/or select amplitude used for waveform

Re: [PATCH v2 5/5] iio: dac: stm32: add support for waveform generator

2017-04-10 Thread Fabrice Gasnier 
On 04/09/2017 11:34 AM, Jonathan Cameron wrote:
> On 06/04/17 17:11, Fabrice Gasnier wrote:
>> STM32 DAC has built-in noise or triangle waveform generator.
>> - "wavetype" extended attribute selects noise or triangle.
>> - "amplitude" extended attribute selects amplitude for waveform generator
>>
>> A DC offset can be added to waveform generator output. This can be done
>> using out_voltage[1/2]_offset
>>
>> Waveform generator requires a trigger to be configured, to increment /
>> decrement internal counter in case of triangle generator. Noise
>> generator is a bit different,  but also requires a trigger to generate
>> samples.
>>
>> Signed-off-by: Fabrice Gasnier 
> 
> Various bits inline.  Mostly I think the blockers on this will be
> making sure the ABI defined is generic enough to handle the more crazy
> DDS chips out there... (basically the ones doing frequency modulation).
> 
> Jonathan
>> ---
>> Changes in v2:
>> - use _offset parameter to add DC offset to waveform generator
> Conceptually this offset is just the normal DAC output value (particularly
yes
> in the flat case)  I guess we can paper over this by having the _raw
> and this always have th same value, but it's a little inelegant.
> Still people are going to expect _raw to control it when in DAC mode but
> that makes limited sense in DDS mode.
> 
> Mind you nothing stops us defining all DDS channels as the sum of whatever
> the DDS is doing and whatever is _raw is set to. Perhaps we tidy this up
> purely through documentation.  Think of the DDS as a modulation on top
> of the DAC...
> 
>> - Rework ABI to better fit existing DDS ABI: use out_voltageY_wavetype,
>>   out_voltage_wavetype_available, out_voltageY_amplitude,
>>   out_voltage_amplitude_available
> Hmm. I'm thinking those amplitude values aren't nice and don't fit well
> with the more general ABI.
> 
> I suggested (but didn't really expand upon) having standard defined types
> for each waveform then using scale to control the amplitude.

Do you mean _scale attribute ?
> 
> Is that something that might work here?

I probably miss the point here...
> 
> So say we have our triangle standard form having an amplitude of 1V Peak to
> Peak. Then we can use scale to make it whatever we actually have in this
> case?  The docs for wave type will need to describe those standard forms
> though.
... scale is fixed here, in line with _raw attribute. In 'amplitude'
description for STM32 DAC here (e.g. from 1...4095), same scale is used.
Can you elaborate ?

> 
> Hmm. Whether this is worth doing is unclear however as we'll still have
> to describe the 'frequency' in terms of the clock ticks (here the triggers)

Describing frequency may be an issue, not sure it makes senses in this
case: 'clock ticks', e.g. triggers here may be timers, but also an EXTI
(external...), or even software trig perhaps.

> So maybe amplitude is worth having.  Again, looking for input from ADI lot
> on this...  There are some really fiddly cases to describe were we are doing
> symbol encoding so have multiple waveforms that we are switching between based
> on some external signal. Any ABI needs to encompass that sort of usage.
> Parts like the AD9833 for example...
> 
>> - Better explain trigger usage in case of waveform generator.
>> ---
>>  Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 |  16 +++
>>  drivers/iio/dac/stm32-dac-core.h  |   4 +
>>  drivers/iio/dac/stm32-dac.c   | 158 
>> +-
>>  3 files changed, 177 insertions(+), 1 deletion(-)
>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 
>> b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>> new file mode 100644
>> index 000..8f1fa009
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
> Fair enough to initially introduced these for this part only, but I'd very
> much like to see us agree on these sufficiently to get them into the main
> docs asap so we have something to work with for getting the DDS chips out
> of staging...
>> @@ -0,0 +1,16 @@
>> +What:   /sys/bus/iio/devices/iio:deviceX/out_voltageY_wavetype
>> +What:   
>> /sys/bus/iio/devices/iio:deviceX/out_voltage_wavetype_available
>> +KernelVersion:  4.12
>> +Contact:fabrice.gasn...@st.com
>> +Description:
>> +List and/or select waveform generation provided by STM32 DAC:
>> +- "flat": waveform generator disabled (default)
>> +- "noise": select noise waveform
>> +- "triangle": select triangle waveform
>> +
>> +What:   /sys/bus/iio/devices/iio:deviceX/out_voltageY_amplitude
>> +What:   
>> /sys/bus/iio/devices/iio:deviceX/out_voltage_amplitude_available
>> +KernelVersion:  4.12
>> +Contact:fabrice.gasn...@st.com
>> +Description:
>> +List and/or select amplitude used for waveform generator
>> diff --git

Re: [PATCH v2 5/5] iio: dac: stm32: add support for waveform generator

2017-04-09 Thread Jonathan Cameron 
On 06/04/17 17:11, Fabrice Gasnier wrote:
> STM32 DAC has built-in noise or triangle waveform generator.
> - "wavetype" extended attribute selects noise or triangle.
> - "amplitude" extended attribute selects amplitude for waveform generator
> 
> A DC offset can be added to waveform generator output. This can be done
> using out_voltage[1/2]_offset
> 
> Waveform generator requires a trigger to be configured, to increment /
> decrement internal counter in case of triangle generator. Noise
> generator is a bit different,  but also requires a trigger to generate
> samples.
> 
> Signed-off-by: Fabrice Gasnier 

Various bits inline.  Mostly I think the blockers on this will be
making sure the ABI defined is generic enough to handle the more crazy
DDS chips out there... (basically the ones doing frequency modulation).

Jonathan
> ---
> Changes in v2:
> - use _offset parameter to add DC offset to waveform generator
Conceptually this offset is just the normal DAC output value (particularly
in the flat case)  I guess we can paper over this by having the _raw
and this always have th same value, but it's a little inelegant.
Still people are going to expect _raw to control it when in DAC mode but
that makes limited sense in DDS mode.

Mind you nothing stops us defining all DDS channels as the sum of whatever
the DDS is doing and whatever is _raw is set to. Perhaps we tidy this up
purely through documentation.  Think of the DDS as a modulation on top
of the DAC...

> - Rework ABI to better fit existing DDS ABI: use out_voltageY_wavetype,
>   out_voltage_wavetype_available, out_voltageY_amplitude,
>   out_voltage_amplitude_available
Hmm. I'm thinking those amplitude values aren't nice and don't fit well
with the more general ABI.

I suggested (but didn't really expand upon) having standard defined types
for each waveform then using scale to control the amplitude.

Is that something that might work here?

So say we have our triangle standard form having an amplitude of 1V Peak to
Peak. Then we can use scale to make it whatever we actually have in this
case?  The docs for wave type will need to describe those standard forms
though.

Hmm. Whether this is worth doing is unclear however as we'll still have
to describe the 'frequency' in terms of the clock ticks (here the triggers)
So maybe amplitude is worth having.  Again, looking for input from ADI lot
on this...  There are some really fiddly cases to describe were we are doing
symbol encoding so have multiple waveforms that we are switching between based
on some external signal. Any ABI needs to encompass that sort of usage.
Parts like the AD9833 for example...

> - Better explain trigger usage in case of waveform generator.
> ---
>  Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 |  16 +++
>  drivers/iio/dac/stm32-dac-core.h  |   4 +
>  drivers/iio/dac/stm32-dac.c   | 158 
> +-
>  3 files changed, 177 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 
> b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
> new file mode 100644
> index 000..8f1fa009
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
Fair enough to initially introduced these for this part only, but I'd very
much like to see us agree on these sufficiently to get them into the main
docs asap so we have something to work with for getting the DDS chips out
of staging...
> @@ -0,0 +1,16 @@
> +What:/sys/bus/iio/devices/iio:deviceX/out_voltageY_wavetype
> +What:
> /sys/bus/iio/devices/iio:deviceX/out_voltage_wavetype_available
> +KernelVersion:   4.12
> +Contact: fabrice.gasn...@st.com
> +Description:
> + List and/or select waveform generation provided by STM32 DAC:
> + - "flat": waveform generator disabled (default)
> + - "noise": select noise waveform
> + - "triangle": select triangle waveform
> +
> +What:/sys/bus/iio/devices/iio:deviceX/out_voltageY_amplitude
> +What:
> /sys/bus/iio/devices/iio:deviceX/out_voltage_amplitude_available
> +KernelVersion:   4.12
> +Contact: fabrice.gasn...@st.com
> +Description:
> + List and/or select amplitude used for waveform generator
> diff --git a/drivers/iio/dac/stm32-dac-core.h 
> b/drivers/iio/dac/stm32-dac-core.h
> index e51a468..0f02975 100644
> --- a/drivers/iio/dac/stm32-dac-core.h
> +++ b/drivers/iio/dac/stm32-dac-core.h
> @@ -37,8 +37,12 @@
>  #define STM32H7_DAC_CR_TEN1  BIT(1)
>  #define STM32H7_DAC_CR_TSEL1_SHIFT   2
>  #define STM32H7_DAC_CR_TSEL1 GENMASK(5, 2)
> +#define STM32_DAC_CR_WAVE1   GENMASK(7, 6)
> +#define STM32_DAC_CR_MAMP1   GENMASK(11, 8)
>  #define STM32H7_DAC_CR_HFSEL BIT(15)
>  #define STM32_DAC_CR_EN2 BIT(16)
> +#define

Re: [PATCH v2 5/5] iio: dac: stm32: add support for waveform generator

2017-04-09 Thread Jonathan Cameron 
On 06/04/17 17:11, Fabrice Gasnier wrote:
> STM32 DAC has built-in noise or triangle waveform generator.
> - "wavetype" extended attribute selects noise or triangle.
> - "amplitude" extended attribute selects amplitude for waveform generator
> 
> A DC offset can be added to waveform generator output. This can be done
> using out_voltage[1/2]_offset
> 
> Waveform generator requires a trigger to be configured, to increment /
> decrement internal counter in case of triangle generator. Noise
> generator is a bit different,  but also requires a trigger to generate
> samples.
> 
> Signed-off-by: Fabrice Gasnier 

Various bits inline.  Mostly I think the blockers on this will be
making sure the ABI defined is generic enough to handle the more crazy
DDS chips out there... (basically the ones doing frequency modulation).

Jonathan
> ---
> Changes in v2:
> - use _offset parameter to add DC offset to waveform generator
Conceptually this offset is just the normal DAC output value (particularly
in the flat case)  I guess we can paper over this by having the _raw
and this always have th same value, but it's a little inelegant.
Still people are going to expect _raw to control it when in DAC mode but
that makes limited sense in DDS mode.

Mind you nothing stops us defining all DDS channels as the sum of whatever
the DDS is doing and whatever is _raw is set to. Perhaps we tidy this up
purely through documentation.  Think of the DDS as a modulation on top
of the DAC...

> - Rework ABI to better fit existing DDS ABI: use out_voltageY_wavetype,
>   out_voltage_wavetype_available, out_voltageY_amplitude,
>   out_voltage_amplitude_available
Hmm. I'm thinking those amplitude values aren't nice and don't fit well
with the more general ABI.

I suggested (but didn't really expand upon) having standard defined types
for each waveform then using scale to control the amplitude.

Is that something that might work here?

So say we have our triangle standard form having an amplitude of 1V Peak to
Peak. Then we can use scale to make it whatever we actually have in this
case?  The docs for wave type will need to describe those standard forms
though.

Hmm. Whether this is worth doing is unclear however as we'll still have
to describe the 'frequency' in terms of the clock ticks (here the triggers)
So maybe amplitude is worth having.  Again, looking for input from ADI lot
on this...  There are some really fiddly cases to describe were we are doing
symbol encoding so have multiple waveforms that we are switching between based
on some external signal. Any ABI needs to encompass that sort of usage.
Parts like the AD9833 for example...

> - Better explain trigger usage in case of waveform generator.
> ---
>  Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 |  16 +++
>  drivers/iio/dac/stm32-dac-core.h  |   4 +
>  drivers/iio/dac/stm32-dac.c   | 158 
> +-
>  3 files changed, 177 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 
> b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
> new file mode 100644
> index 000..8f1fa009
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
Fair enough to initially introduced these for this part only, but I'd very
much like to see us agree on these sufficiently to get them into the main
docs asap so we have something to work with for getting the DDS chips out
of staging...
> @@ -0,0 +1,16 @@
> +What:/sys/bus/iio/devices/iio:deviceX/out_voltageY_wavetype
> +What:
> /sys/bus/iio/devices/iio:deviceX/out_voltage_wavetype_available
> +KernelVersion:   4.12
> +Contact: fabrice.gasn...@st.com
> +Description:
> + List and/or select waveform generation provided by STM32 DAC:
> + - "flat": waveform generator disabled (default)
> + - "noise": select noise waveform
> + - "triangle": select triangle waveform
> +
> +What:/sys/bus/iio/devices/iio:deviceX/out_voltageY_amplitude
> +What:
> /sys/bus/iio/devices/iio:deviceX/out_voltage_amplitude_available
> +KernelVersion:   4.12
> +Contact: fabrice.gasn...@st.com
> +Description:
> + List and/or select amplitude used for waveform generator
> diff --git a/drivers/iio/dac/stm32-dac-core.h 
> b/drivers/iio/dac/stm32-dac-core.h
> index e51a468..0f02975 100644
> --- a/drivers/iio/dac/stm32-dac-core.h
> +++ b/drivers/iio/dac/stm32-dac-core.h
> @@ -37,8 +37,12 @@
>  #define STM32H7_DAC_CR_TEN1  BIT(1)
>  #define STM32H7_DAC_CR_TSEL1_SHIFT   2
>  #define STM32H7_DAC_CR_TSEL1 GENMASK(5, 2)
> +#define STM32_DAC_CR_WAVE1   GENMASK(7, 6)
> +#define STM32_DAC_CR_MAMP1   GENMASK(11, 8)
>  #define STM32H7_DAC_CR_HFSEL BIT(15)
>  #define STM32_DAC_CR_EN2 BIT(16)
> +#define STM32_DAC_CR_WAVE2

[PATCH v2 5/5] iio: dac: stm32: add support for waveform generator

2017-04-06 Thread Fabrice Gasnier 
STM32 DAC has built-in noise or triangle waveform generator.
- "wavetype" extended attribute selects noise or triangle.
- "amplitude" extended attribute selects amplitude for waveform generator

A DC offset can be added to waveform generator output. This can be done
using out_voltage[1/2]_offset

Waveform generator requires a trigger to be configured, to increment /
decrement internal counter in case of triangle generator. Noise
generator is a bit different,  but also requires a trigger to generate
samples.

Signed-off-by: Fabrice Gasnier 
---
Changes in v2:
- use _offset parameter to add DC offset to waveform generator
- Rework ABI to better fit existing DDS ABI: use out_voltageY_wavetype,
  out_voltage_wavetype_available, out_voltageY_amplitude,
  out_voltage_amplitude_available
- Better explain trigger usage in case of waveform generator.
---
 Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 |  16 +++
 drivers/iio/dac/stm32-dac-core.h  |   4 +
 drivers/iio/dac/stm32-dac.c   | 158 +-
 3 files changed, 177 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-stm32

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 
b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
new file mode 100644
index 000..8f1fa009
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
@@ -0,0 +1,16 @@
+What:  /sys/bus/iio/devices/iio:deviceX/out_voltageY_wavetype
+What:  /sys/bus/iio/devices/iio:deviceX/out_voltage_wavetype_available
+KernelVersion: 4.12
+Contact:   fabrice.gasn...@st.com
+Description:
+   List and/or select waveform generation provided by STM32 DAC:
+   - "flat": waveform generator disabled (default)
+   - "noise": select noise waveform
+   - "triangle": select triangle waveform
+
+What:  /sys/bus/iio/devices/iio:deviceX/out_voltageY_amplitude
+What:  /sys/bus/iio/devices/iio:deviceX/out_voltage_amplitude_available
+KernelVersion: 4.12
+Contact:   fabrice.gasn...@st.com
+Description:
+   List and/or select amplitude used for waveform generator
diff --git a/drivers/iio/dac/stm32-dac-core.h b/drivers/iio/dac/stm32-dac-core.h
index e51a468..0f02975 100644
--- a/drivers/iio/dac/stm32-dac-core.h
+++ b/drivers/iio/dac/stm32-dac-core.h
@@ -37,8 +37,12 @@
 #define STM32H7_DAC_CR_TEN1BIT(1)
 #define STM32H7_DAC_CR_TSEL1_SHIFT 2
 #define STM32H7_DAC_CR_TSEL1   GENMASK(5, 2)
+#define STM32_DAC_CR_WAVE1 GENMASK(7, 6)
+#define STM32_DAC_CR_MAMP1 GENMASK(11, 8)
 #define STM32H7_DAC_CR_HFSEL   BIT(15)
 #define STM32_DAC_CR_EN2   BIT(16)
+#define STM32_DAC_CR_WAVE2 GENMASK(23, 22)
+#define STM32_DAC_CR_MAMP2 GENMASK(27, 24)
 
 /* STM32_DAC_SWTRIGR bit fields */
 #define STM32_DAC_SWTRIGR_SWTRIG1  BIT(0)
diff --git a/drivers/iio/dac/stm32-dac.c b/drivers/iio/dac/stm32-dac.c
index a7a078e..2ed75db 100644
--- a/drivers/iio/dac/stm32-dac.c
+++ b/drivers/iio/dac/stm32-dac.c
@@ -42,10 +42,12 @@
 /**
  * struct stm32_dac - private data of DAC driver
  * @common:reference to DAC common data
+ * @wavetype:  waveform generator
  * @swtrig:Using software trigger
  */
 struct stm32_dac {
struct stm32_dac_common *common;
+   u32 wavetype;
bool swtrig;
 };
 
@@ -222,6 +224,29 @@ static int stm32_dac_set_value(struct stm32_dac *dac, int 
channel, int val)
return ret;
 }
 
+static int stm32_dac_get_offset(struct stm32_dac *dac, int channel, int *val)
+{
+   int ret;
+
+   /* Offset is only relevant in waveform generation mode. */
+   if (!dac->wavetype) {
+   *val = 0;
+   return IIO_VAL_INT;
+   }
+
+   /*
+* In waveform generation mode, DC offset in DHR is added to waveform
+* generator output, then stored to DOR (data output register).
+* Read offset from DHR.
+*/
+   if (STM32_DAC_IS_CHAN_1(channel))
+   ret = regmap_read(dac->common->regmap, STM32_DAC_DHR12R1, val);
+   else
+   ret = regmap_read(dac->common->regmap, STM32_DAC_DHR12R2, val);
+
+   return ret ? ret : IIO_VAL_INT;
+}
+
 static int stm32_dac_read_raw(struct iio_dev *indio_dev,
  struct iio_chan_spec const *chan,
  int *val, int *val2, long mask)
@@ -231,6 +256,8 @@ static int stm32_dac_read_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_RAW:
return stm32_dac_get_value(dac, chan->channel, val);
+   case IIO_CHAN_INFO_OFFSET:
+   return stm32_dac_get_offset(dac, chan->channel, val);
case IIO_CHAN_INFO_SCALE:
*val = dac->common->vref_mv;
*val2 = chan->scan_type.realbits;
@@ -247,8 +274,16 @@ static int

[PATCH v2 5/5] iio: dac: stm32: add support for waveform generator

2017-04-06 Thread Fabrice Gasnier 
STM32 DAC has built-in noise or triangle waveform generator.
- "wavetype" extended attribute selects noise or triangle.
- "amplitude" extended attribute selects amplitude for waveform generator

A DC offset can be added to waveform generator output. This can be done
using out_voltage[1/2]_offset

Waveform generator requires a trigger to be configured, to increment /
decrement internal counter in case of triangle generator. Noise
generator is a bit different,  but also requires a trigger to generate
samples.

Signed-off-by: Fabrice Gasnier 
---
Changes in v2:
- use _offset parameter to add DC offset to waveform generator
- Rework ABI to better fit existing DDS ABI: use out_voltageY_wavetype,
  out_voltage_wavetype_available, out_voltageY_amplitude,
  out_voltage_amplitude_available
- Better explain trigger usage in case of waveform generator.
---
 Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 |  16 +++
 drivers/iio/dac/stm32-dac-core.h  |   4 +
 drivers/iio/dac/stm32-dac.c   | 158 +-
 3 files changed, 177 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-stm32

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 
b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
new file mode 100644
index 000..8f1fa009
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
@@ -0,0 +1,16 @@
+What:  /sys/bus/iio/devices/iio:deviceX/out_voltageY_wavetype
+What:  /sys/bus/iio/devices/iio:deviceX/out_voltage_wavetype_available
+KernelVersion: 4.12
+Contact:   fabrice.gasn...@st.com
+Description:
+   List and/or select waveform generation provided by STM32 DAC:
+   - "flat": waveform generator disabled (default)
+   - "noise": select noise waveform
+   - "triangle": select triangle waveform
+
+What:  /sys/bus/iio/devices/iio:deviceX/out_voltageY_amplitude
+What:  /sys/bus/iio/devices/iio:deviceX/out_voltage_amplitude_available
+KernelVersion: 4.12
+Contact:   fabrice.gasn...@st.com
+Description:
+   List and/or select amplitude used for waveform generator
diff --git a/drivers/iio/dac/stm32-dac-core.h b/drivers/iio/dac/stm32-dac-core.h
index e51a468..0f02975 100644
--- a/drivers/iio/dac/stm32-dac-core.h
+++ b/drivers/iio/dac/stm32-dac-core.h
@@ -37,8 +37,12 @@
 #define STM32H7_DAC_CR_TEN1BIT(1)
 #define STM32H7_DAC_CR_TSEL1_SHIFT 2
 #define STM32H7_DAC_CR_TSEL1   GENMASK(5, 2)
+#define STM32_DAC_CR_WAVE1 GENMASK(7, 6)
+#define STM32_DAC_CR_MAMP1 GENMASK(11, 8)
 #define STM32H7_DAC_CR_HFSEL   BIT(15)
 #define STM32_DAC_CR_EN2   BIT(16)
+#define STM32_DAC_CR_WAVE2 GENMASK(23, 22)
+#define STM32_DAC_CR_MAMP2 GENMASK(27, 24)
 
 /* STM32_DAC_SWTRIGR bit fields */
 #define STM32_DAC_SWTRIGR_SWTRIG1  BIT(0)
diff --git a/drivers/iio/dac/stm32-dac.c b/drivers/iio/dac/stm32-dac.c
index a7a078e..2ed75db 100644
--- a/drivers/iio/dac/stm32-dac.c
+++ b/drivers/iio/dac/stm32-dac.c
@@ -42,10 +42,12 @@
 /**
  * struct stm32_dac - private data of DAC driver
  * @common:reference to DAC common data
+ * @wavetype:  waveform generator
  * @swtrig:Using software trigger
  */
 struct stm32_dac {
struct stm32_dac_common *common;
+   u32 wavetype;
bool swtrig;
 };
 
@@ -222,6 +224,29 @@ static int stm32_dac_set_value(struct stm32_dac *dac, int 
channel, int val)
return ret;
 }
 
+static int stm32_dac_get_offset(struct stm32_dac *dac, int channel, int *val)
+{
+   int ret;
+
+   /* Offset is only relevant in waveform generation mode. */
+   if (!dac->wavetype) {
+   *val = 0;
+   return IIO_VAL_INT;
+   }
+
+   /*
+* In waveform generation mode, DC offset in DHR is added to waveform
+* generator output, then stored to DOR (data output register).
+* Read offset from DHR.
+*/
+   if (STM32_DAC_IS_CHAN_1(channel))
+   ret = regmap_read(dac->common->regmap, STM32_DAC_DHR12R1, val);
+   else
+   ret = regmap_read(dac->common->regmap, STM32_DAC_DHR12R2, val);
+
+   return ret ? ret : IIO_VAL_INT;
+}
+
 static int stm32_dac_read_raw(struct iio_dev *indio_dev,
  struct iio_chan_spec const *chan,
  int *val, int *val2, long mask)
@@ -231,6 +256,8 @@ static int stm32_dac_read_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_RAW:
return stm32_dac_get_value(dac, chan->channel, val);
+   case IIO_CHAN_INFO_OFFSET:
+   return stm32_dac_get_offset(dac, chan->channel, val);
case IIO_CHAN_INFO_SCALE:
*val = dac->common->vref_mv;
*val2 = chan->scan_type.realbits;
@@ -247,8 +274,16 @@ static int stm32_dac_write_raw(struct