JonathanOn 04/02/2017 02:19 PM, Jonathan Cameron wrote:
> On 31/03/17 12:45, Fabrice Gasnier wrote:
>> STM32 DAC has built-in noise or triangle waveform generator.
>> Waveform generator requires trigger to be configured.
>> - "wave" extended attribute selects noise or triangle.
>> - "mamp" extended attribute selects either LFSR (linear feedback
>>   shift register) mask for noise waveform, OR triangle amplitude.
>>
>> Signed-off-by: Fabrice Gasnier <[email protected]>
> 
> Looks like AN3126 is the relevant doc.
> (a quick note from this relevant to earlier patches- doc says
> 1-3 channels - perhaps build that from the start with that
> possibility in mind).
Hi Jonathan,

Just to clarify this, some products like STM32F334xx have 3 channels,
yes. Several STM32 DAC IPs (& so registers) are instantiated: DAC1 have
two outputs (dac1_out1 & dac1_out2), DAC2 have one output (e.g.
dac2_out1). Driver can be instantiated several times. Is it ok ?

> 
> As you probably know, this wanders into a large chunk of 'poorly'
> defined ABI within IIO as it stands.
> 
> Note there are a number of waveform generators still in staging.
> Not a lot of movement on getting them out of staging unfortunately
> (so far!)
> 
> However, let us keep those drivers in mind as we work on ABI and
> I definitely want some input from someone at Analog. 
> Lars, who is best for this? I see at least some of these were
> originally Michael's work.
> 
> They do have partial docs under
> drivers/staging/iio/Documentation/sysfs-bus-iio-dds
> I'll highlight thoughts from there as I look through this...

Thanks for pointing this out.

> 
> 
>> ---
>>  Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 |  32 ++++++
>>  drivers/iio/dac/stm32-dac.c                       | 124 
>> ++++++++++++++++++++++
>>  2 files changed, 156 insertions(+)
>>  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 0000000..c2432e1
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>> @@ -0,0 +1,32 @@
>> +What:               /sys/bus/iio/devices/iio:deviceX/wave
>> +What:               /sys/bus/iio/devices/iio:deviceX/wave_available
> Needs to be channel associated. Whilst in your case you have basically
> a pair of single channel devices, in more general case, it's not usual
> to have multiple parallel waveform generators clocked together.
> 
> Old ABI is:
> What:         /sys/bus/iio/devices/.../out_altvoltageX_outY_wavetype etc
> 
I'll rework this in V2.
> 
>> +KernelVersion:      4.12
>> +Contact:    [email protected]
>> +Description:
>> +            List and/or select waveform generation provided by STM32 DAC:
>> +            - "none": (default) means normal DAC operations
> none kind of hints at nothing coming out.  Perhaps 'flat' would be closer?
> i.e. only changes when someone tells it to.
> 
>> +            - "noise": select noise waveform
>> +            - "triangle": select triangle waveform
>> +            Note: when waveform generator is used, writing _raw sysfs entry
>> +            adds a DC offset to generated waveform. Reading it reports
>> +            current output value.
> Interesting.  This gets fiddly but one option would be to describe the whole
> device as a dds.
> 
> Then we have flat type above, combined with an _offset.

I'll update from 'none' to 'flat' in V2, and use _offset.
> 
>> +
>> +What:               /sys/bus/iio/devices/iio:deviceX/mamp
>> +What:               /sys/bus/iio/devices/iio:deviceX/mamp_available
>> +KernelVersion:      4.12
>> +Contact:    [email protected]
>> +Description:
>> +            List and select mask/amplitude used for noise/triangle waveform
>> +            generator, which are:
>> +            - "0": unmask bit 0 of LFSR / triangle amplitude equal to 1
>> +            - "1": unmask bit [1:0] of LFSR / triangle amplitude equal to 3
>> +            - "2": unmask bit [2:0] of LFSR / triangle amplitude equal to 7
>> +            - "3": unmask bit [3:0] of LFSR / triangle amplitude equal to 15
>> +            - "4": unmask bit [4:0] of LFSR / triangle amplitude equal to 31
>> +            - "5": unmask bit [5:0] of LFSR / triangle amplitude equal to 63
>> +            - "6": unmask bit [6:0] of LFSR / triangle amplitude equal to 
>> 127
>> +            - "7": unmask bit [7:0] of LFSR / triangle amplitude equal to 
>> 255
>> +            - "8": unmask bit [8:0] of LFSR / triangle amplitude equal to 
>> 511
>> +            - "9": unmask bit [9:0] of LFSR / triangle amplitude equal to 
>> 1023
>> +            - "10": unmask bit [10:0] of LFSR / triangle amplitude equal to 
>> 2047
>> +            - "11": unmask bit [11:0] of LFSR / triangle amplitude equal to 
>> 4095
> I don't fully understand what is going on here - so I'm guessing somewhat.
Sorry for this, this is basically amplitude.

I think best is to rename above to something like 'amplitude' and
'amplitude_available'.

I'll rework this in V2.
> 
> 
> Let us try describing these generically.  If we define standard 'forms' of 
> each
> waveform type.  Say a 0 to 1 V peak to peak, then we could use _scale to 
> control
> this nicely.
> 
>> diff --git a/drivers/iio/dac/stm32-dac.c b/drivers/iio/dac/stm32-dac.c
>> index 62e43e9..d7dda78 100644
>> --- a/drivers/iio/dac/stm32-dac.c
>> +++ b/drivers/iio/dac/stm32-dac.c
>> @@ -41,10 +41,14 @@
>>  /**
>>   * struct stm32_dac - private data of DAC driver
>>   * @common:         reference to DAC common data
>> + * @wave:           waveform generator
>> + * @mamp:           waveform mask/amplitude
>>   * @swtrig:         Using software trigger
>>   */
>>  struct stm32_dac {
>>      struct stm32_dac_common *common;
>> +    u32 wave;
>> +    u32 mamp;
>>      bool swtrig;
>>  };
>>  
>> @@ -157,6 +161,24 @@ static int stm32_dac_is_enabled(struct stm32_dac *dac, 
>> int channel)
>>      return !!en;
>>  }
>>  
>> +static int stm32_dac_wavegen(struct stm32_dac *dac, int channel)
>> +{
>> +    struct regmap *regmap = dac->common->regmap;
>> +    u32 mask, val;
>> +
>> +    if (channel == STM32_DAC_CHANNEL_1) {
>> +            val = FIELD_PREP(STM32_DAC_CR_WAVE1, dac->wave) |
>> +                    FIELD_PREP(STM32_DAC_CR_MAMP1, dac->mamp);
>> +            mask = STM32_DAC_CR_WAVE1 | STM32_DAC_CR_MAMP1;
>> +    } else {
>> +            val = FIELD_PREP(STM32_DAC_CR_WAVE2, dac->wave) |
>> +                    FIELD_PREP(STM32_DAC_CR_MAMP2, dac->mamp);
>> +            mask = STM32_DAC_CR_WAVE2 | STM32_DAC_CR_MAMP2;
>> +    }
>> +
>> +    return regmap_update_bits(regmap, STM32_DAC_CR, mask, val);
>> +}
>> +
>>  static int stm32_dac_enable(struct iio_dev *indio_dev, int channel)
>>  {
>>      struct stm32_dac *dac = iio_priv(indio_dev);
>> @@ -164,6 +186,17 @@ static int stm32_dac_enable(struct iio_dev *indio_dev, 
>> int channel)
>>              STM32_DAC_CR_EN1 : STM32_DAC_CR_EN2;
>>      int ret;
>>  
>> +    if (dac->wave && !indio_dev->trig) {
>> +            dev_err(&indio_dev->dev, "Wavegen requires a trigger\n");
>> +            return -EINVAL;
>> +    }
>> +
>> +    ret = stm32_dac_wavegen(dac, channel);
>> +    if (ret < 0) {
>> +            dev_err(&indio_dev->dev, "Wavegen setup failed\n");
>> +            return ret;
>> +    }
>> +
>>      ret = stm32_dac_set_trig(dac, indio_dev->trig, channel);
>>      if (ret < 0) {
>>              dev_err(&indio_dev->dev, "Trigger setup failed\n");
>> @@ -291,6 +324,96 @@ static int stm32_dac_debugfs_reg_access(struct iio_dev 
>> *indio_dev,
>>      .driver_module = THIS_MODULE,
>>  };
>>  
>> +/* waveform generator wave selection */
>> +static const char * const stm32_dac_wave_desc[] = {
>> +    "none",
>> +    "noise",
>> +    "triangle",
>> +};
>> +
>> +static int stm32_dac_set_wave(struct iio_dev *indio_dev,
>> +                          const struct iio_chan_spec *chan,
>> +                          unsigned int type)
>> +{
>> +    struct stm32_dac *dac = iio_priv(indio_dev);
>> +
>> +    if (stm32_dac_is_enabled(dac, chan->channel))
>> +            return -EBUSY;
>> +    dac->wave = type;
>> +
>> +    return 0;
>> +}
>> +
>> +static int stm32_dac_get_wave(struct iio_dev *indio_dev,
>> +                          const struct iio_chan_spec *chan)
>> +{
>> +    struct stm32_dac *dac = iio_priv(indio_dev);
>> +
>> +    return dac->wave;
>> +}
>> +
>> +static const struct iio_enum stm32_dac_wave_enum = {
>> +    .items = stm32_dac_wave_desc,
>> +    .num_items = ARRAY_SIZE(stm32_dac_wave_desc),
>> +    .get = stm32_dac_get_wave,
>> +    .set = stm32_dac_set_wave,
>> +};
>> +
>> +/*
>> + * waveform generator mask/amplitude selection:
>> + * - noise: LFSR mask (linear feedback shift register, umasks bit 0, 
>> [1:0]...)
>> + * - triangle: amplitude (equal to 1, 3, 5, 7... 4095)
>> + */
>> +static const char * const stm32_dac_mamp_desc[] = {
>> +    "0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11",
>> +};
>> +
>> +static int stm32_dac_set_mamp(struct iio_dev *indio_dev,
>> +                          const struct iio_chan_spec *chan,
>> +                          unsigned int type)
>> +{
>> +    struct stm32_dac *dac = iio_priv(indio_dev);
>> +
>> +    if (stm32_dac_is_enabled(dac, chan->channel))
>> +            return -EBUSY;
>> +    dac->mamp = type;
>> +
>> +    return 0;
>> +}
>> +
>> +static int  stm32_dac_get_mamp(struct iio_dev *indio_dev,
>> +                           const struct iio_chan_spec *chan)
>> +{
>> +    struct stm32_dac *dac = iio_priv(indio_dev);
>> +
>> +    return dac->mamp;
>> +}
>> +
>> +static const struct iio_enum stm32_dac_mamp_enum = {
>> +    .items = stm32_dac_mamp_desc,
>> +    .num_items = ARRAY_SIZE(stm32_dac_mamp_desc),
>> +    .get = stm32_dac_get_mamp,
>> +    .set = stm32_dac_set_mamp,
>> +};
>> +
>> +static const struct iio_chan_spec_ext_info stm32_dac_ext_info[] = {
>> +    IIO_ENUM("wave", IIO_SHARED_BY_ALL, &stm32_dac_wave_enum),
>> +    {
>> +            .name = "wave_available",
>> +            .shared = IIO_SHARED_BY_ALL,
>> +            .read = iio_enum_available_read,
>> +            .private = (uintptr_t)&stm32_dac_wave_enum,
>> +    },
>> +    IIO_ENUM("mamp", IIO_SHARED_BY_ALL, &stm32_dac_mamp_enum),
>> +    {
>> +            .name = "mamp_available",
>> +            .shared = IIO_SHARED_BY_ALL,
>> +            .read = iio_enum_available_read,
>> +            .private = (uintptr_t)&stm32_dac_mamp_enum,
>> +    },
>> +    {},
>> +};
>> +
>>  #define STM32_DAC_CHANNEL(chan, name) {             \
>>      .type = IIO_VOLTAGE,                    \
>>      .indexed = 1,                           \
>> @@ -306,6 +429,7 @@ static int stm32_dac_debugfs_reg_access(struct iio_dev 
>> *indio_dev,
>>              .storagebits = 16,              \
>>      },                                      \
>>      .datasheet_name = name,                 \
>> +    .ext_info = stm32_dac_ext_info          \
>>  }
>>  
>>  static const struct iio_chan_spec stm32_dac_channels[] = {
>>
> 

Reply via email to