[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e6a02296-fa72-50e8-d5e8-25bb5d496085@kernel.org>
Date: Sun, 2 Apr 2017 13:19:23 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Fabrice Gasnier <fabrice.gasnier@...com>, linux@...linux.org.uk,
robh+dt@...nel.org, linux-arm-kernel@...ts.infradead.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: linux-iio@...r.kernel.org, mark.rutland@....com,
mcoquelin.stm32@...il.com, alexandre.torgue@...com,
lars@...afoo.de, knaack.h@....de, pmeerw@...erw.net,
benjamin.gaignard@...aro.org, benjamin.gaignard@...com,
linus.walleij@...aro.org, amelie.delaunay@...com,
"Hennerich, Michael" <Michael.Hennerich@...log.com>
Subject: Re: [PATCH 4/4] iio: dac: stm32: add support for waveform generator
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@...com>
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).
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...
> ---
> 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
> +KernelVersion: 4.12
> +Contact: fabrice.gasnier@...com
> +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.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/mamp
> +What: /sys/bus/iio/devices/iio:deviceX/mamp_available
> +KernelVersion: 4.12
> +Contact: fabrice.gasnier@...com
> +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.
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[] = {
>
Powered by blists - more mailing lists