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 <fabrice.gasnier@xxxxxx> > > 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: fabrice.gasnier@xxxxxx >> +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: fabrice.gasnier@xxxxxx >> +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[] = { >> > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html