linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/7] Add support for triggered buffer mode to STM32 ADC
@ 2017-01-26 14:28 Fabrice Gasnier
  2017-01-26 14:28 ` [PATCH v2 1/7] iio: adc: stm32: add support for triggered buffer mode Fabrice Gasnier
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Fabrice Gasnier @ 2017-01-26 14:28 UTC (permalink / raw)
  To: jic23, linux, robh+dt, linux-arm-kernel, devicetree, linux-kernel
  Cc: linux-iio, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
	knaack.h, pmeerw, fabrice.gasnier, benjamin.gaignard,
	benjamin.gaignard

The following patches add support for triggered buffer mode.
These are based on top of "Add PWM and IIO timer drivers for STM32"
series. Reference:
https://lkml.org/lkml/2017/1/20/116

STM32 ADC, can use either interrupts or DMA to collect data.
Either timer trigger output (TRGO) or PWM can be used as trigger source.
This patchset has been tested on STM32F429 eval board.

- Example to enable timer1 PWM:
cd /sys/devices/platform/soc/40010000.timers/40010000.timers:pwm/pwm/pwmchip4/
echo 0 > export # timer 1 channel 1
echo 1000000 > pwm0/period # 1000Hz
echo 500000 > pwm0/duty_cycle
echo 1 > pwm0/enable

- Example to enable timer3 TRGO:
cd /sys/bus/iio/devices/
cat trigger6/name 
tim1_ch1
cat trigger0/name 
tim3_trgo
echo 1000 > trigger0/sampling_frequency

- Example to configure STM32ADC in triggered buffer mode, with timer1 PWM
  or timer3 TRGO:
cd /sys/bus/iio/devices/iio\:device0
echo tim1_ch1 > trigger/current_trigger
OR: echo tim3_trgo > trigger/current_trigger
echo 1 > scan_elements/in_voltage8_en
echo 1 > buffer/enable

---
Changes in v2:
- Mainly updates following Jonathan's remarks.
- make data buffer part of stm32_adc structure
- remove defines
- add comment on reading DR to clear OEC flag
- use bitmap_weight()
- fix error handling in stm32_adc_buffer_postenable()
- Rename and document custom 'trigger_polarity' attribute
- use iio_trigger_poll_chained() to avoid bounce to irq context
- rework DMA buffer allocation and use hwfifo_set_watermark()
- Fix typo: using stm32f429i-eval

Fabrice Gasnier (7):
  iio: adc: stm32: add support for triggered buffer mode
  iio: adc: stm32: Enable use of stm32 timer triggers
  iio: adc: stm32: add trigger polarity extended attribute
  Documentation: dt: iio: stm32-adc: optional dma support
  iio: adc: stm32: add optional dma support
  ARM: dts: stm32: Enable dma by default on stm32f4 adc
  ARM: dts: stm32: Enable pwm1 and pwm3 on stm32f429i-eval

 Documentation/ABI/testing/sysfs-bus-iio-adc-stm32  |  18 +
 .../devicetree/bindings/iio/adc/st,stm32-adc.txt   |   7 +
 arch/arm/boot/dts/stm32429i-eval.dts               |  28 +
 arch/arm/boot/dts/stm32f429.dtsi                   |   6 +
 drivers/iio/adc/Kconfig                            |   5 +
 drivers/iio/adc/stm32-adc-core.c                   |   1 +
 drivers/iio/adc/stm32-adc-core.h                   |   2 +
 drivers/iio/adc/stm32-adc.c                        | 633 ++++++++++++++++++++-
 8 files changed, 676 insertions(+), 24 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-stm32

-- 
1.9.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/7] iio: adc: stm32: add support for triggered buffer mode
  2017-01-26 14:28 [PATCH v2 0/7] Add support for triggered buffer mode to STM32 ADC Fabrice Gasnier
@ 2017-01-26 14:28 ` Fabrice Gasnier
  2017-01-28 18:36   ` Jonathan Cameron
  2017-01-26 14:28 ` [PATCH v2 2/7] iio: adc: stm32: Enable use of stm32 timer triggers Fabrice Gasnier
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Fabrice Gasnier @ 2017-01-26 14:28 UTC (permalink / raw)
  To: jic23, linux, robh+dt, linux-arm-kernel, devicetree, linux-kernel
  Cc: linux-iio, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
	knaack.h, pmeerw, fabrice.gasnier, benjamin.gaignard,
	benjamin.gaignard

STM32 ADC conversions can be launched using hardware triggers.
It can be used to start conversion sequences (group of channels).
Selected channels are select via sequence registers.
Trigger source is selected via 'extsel' (external trigger mux).
Trigger polarity is set to rising edge by default.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
Changes in v2:
based on Jonathan's comments:
- remove all STM32F4_SQx[SHIFT/MASK], put the numbers directly in sq
  description array.
- make data buffer part of stm32_adc structure (remove preenable and
  postdisable routines)
- add comment on reading DR to clear OEC flag
- use bitmap_weight()
- fix error handling in stm32_adc_buffer_postenable()
---
 drivers/iio/adc/Kconfig     |   2 +
 drivers/iio/adc/stm32-adc.c | 317 +++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 298 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 9c8b558..33341f4 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -446,6 +446,8 @@ config STM32_ADC_CORE
 	depends on ARCH_STM32 || COMPILE_TEST
 	depends on OF
 	depends on REGULATOR
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
 	help
 	  Select this option to enable the core driver for STMicroelectronics
 	  STM32 analog-to-digital converter (ADC).
diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index 5715e79..1e382b6 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -22,6 +22,10 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/module.h>
@@ -58,21 +62,37 @@
 
 /* STM32F4_ADC_CR2 - bit fields */
 #define STM32F4_SWSTART			BIT(30)
+#define STM32F4_EXTEN_SHIFT		28
 #define STM32F4_EXTEN_MASK		GENMASK(29, 28)
+#define STM32F4_EXTSEL_SHIFT		24
+#define STM32F4_EXTSEL_MASK		GENMASK(27, 24)
 #define STM32F4_EOCS			BIT(10)
 #define STM32F4_ADON			BIT(0)
 
-/* STM32F4_ADC_SQR1 - bit fields */
-#define STM32F4_L_SHIFT			20
-#define STM32F4_L_MASK			GENMASK(23, 20)
-
-/* STM32F4_ADC_SQR3 - bit fields */
-#define STM32F4_SQ1_SHIFT		0
-#define STM32F4_SQ1_MASK		GENMASK(4, 0)
-
+#define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
 #define STM32_ADC_TIMEOUT_US		100000
 #define STM32_ADC_TIMEOUT	(msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
 
+/* External trigger enable */
+enum stm32_adc_exten {
+	STM32_EXTEN_SWTRIG,
+	STM32_EXTEN_HWTRIG_RISING_EDGE,
+	STM32_EXTEN_HWTRIG_FALLING_EDGE,
+	STM32_EXTEN_HWTRIG_BOTH_EDGES,
+};
+
+/**
+ * stm32_adc_regs - stm32 ADC misc registers & bitfield desc
+ * @reg:		register offset
+ * @mask:		bitfield mask
+ * @shift:		left shift
+ */
+struct stm32_adc_regs {
+	int reg;
+	int mask;
+	int shift;
+};
+
 /**
  * struct stm32_adc - private data of each ADC IIO instance
  * @common:		reference to ADC block common data
@@ -82,15 +102,19 @@
  * @clk:		clock for this adc instance
  * @irq:		interrupt for this adc instance
  * @lock:		spinlock
+ * @bufi:		data buffer index
+ * @num_conv:		expected number of scan conversions
  */
 struct stm32_adc {
 	struct stm32_adc_common	*common;
 	u32			offset;
 	struct completion	completion;
-	u16			*buffer;
+	u16			buffer[STM32_ADC_MAX_SQ];
 	struct clk		*clk;
 	int			irq;
 	spinlock_t		lock;		/* interrupt lock */
+	unsigned int		bufi;
+	unsigned int		num_conv;
 };
 
 /**
@@ -126,6 +150,33 @@ struct stm32_adc_chan_spec {
 };
 
 /**
+ * stm32f4_sq - describe regular sequence registers
+ * - L: sequence len (register & bit field)
+ * - SQ1..SQ16: sequence entries (register & bit field)
+ */
+static const struct stm32_adc_regs stm32f4_sq[STM32_ADC_MAX_SQ + 1] = {
+	/* L: len bit field description to be kept as first element */
+	{ STM32F4_ADC_SQR1, GENMASK(23, 20), 20 },
+	/* SQ1..SQ16 registers & bit fields (reg, mask, shift) */
+	{ STM32F4_ADC_SQR3, GENMASK(4, 0), 0 },
+	{ STM32F4_ADC_SQR3, GENMASK(9, 5), 5 },
+	{ STM32F4_ADC_SQR3, GENMASK(14, 10), 10 },
+	{ STM32F4_ADC_SQR3, GENMASK(19, 15), 15 },
+	{ STM32F4_ADC_SQR3, GENMASK(24, 20), 20 },
+	{ STM32F4_ADC_SQR3, GENMASK(29, 25), 25 },
+	{ STM32F4_ADC_SQR2, GENMASK(4, 0), 0 },
+	{ STM32F4_ADC_SQR2, GENMASK(9, 5), 5 },
+	{ STM32F4_ADC_SQR2, GENMASK(14, 10), 10 },
+	{ STM32F4_ADC_SQR2, GENMASK(19, 15), 15 },
+	{ STM32F4_ADC_SQR2, GENMASK(24, 20), 20 },
+	{ STM32F4_ADC_SQR2, GENMASK(29, 25), 25 },
+	{ STM32F4_ADC_SQR1, GENMASK(4, 0), 0 },
+	{ STM32F4_ADC_SQR1, GENMASK(9, 5), 5 },
+	{ STM32F4_ADC_SQR1, GENMASK(14, 10), 10 },
+	{ STM32F4_ADC_SQR1, GENMASK(19, 15), 15 },
+};
+
+/**
  * STM32 ADC registers access routines
  * @adc: stm32 adc instance
  * @reg: reg offset in adc instance
@@ -211,6 +262,104 @@ static void stm32_adc_stop_conv(struct stm32_adc *adc)
 }
 
 /**
+ * stm32_adc_conf_scan_seq() - Build regular channels scan sequence
+ * @indio_dev: IIO device
+ * @scan_mask: channels to be converted
+ *
+ * Conversion sequence :
+ * Configure ADC scan sequence based on selected channels in scan_mask.
+ * Add channels to SQR registers, from scan_mask LSB to MSB, then
+ * program sequence len.
+ */
+static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
+				   const unsigned long *scan_mask)
+{
+	struct stm32_adc *adc = iio_priv(indio_dev);
+	const struct iio_chan_spec *chan;
+	u32 val, bit;
+	int i = 0;
+
+	for_each_set_bit(bit, scan_mask, indio_dev->masklength) {
+		chan = indio_dev->channels + bit;
+		/*
+		 * Assign one channel per SQ entry in regular
+		 * sequence, starting with SQ1.
+		 */
+		i++;
+		if (i > STM32_ADC_MAX_SQ)
+			return -EINVAL;
+
+		dev_dbg(&indio_dev->dev, "%s chan %d to SQ%d\n",
+			__func__, chan->channel, i);
+
+		val = stm32_adc_readl(adc, stm32f4_sq[i].reg);
+		val &= ~stm32f4_sq[i].mask;
+		val |= chan->channel << stm32f4_sq[i].shift;
+		stm32_adc_writel(adc, stm32f4_sq[i].reg, val);
+	}
+
+	if (!i)
+		return -EINVAL;
+
+	/* Sequence len */
+	val = stm32_adc_readl(adc, stm32f4_sq[0].reg);
+	val &= ~stm32f4_sq[0].mask;
+	val |= ((i - 1) << stm32f4_sq[0].shift);
+	stm32_adc_writel(adc, stm32f4_sq[0].reg, val);
+
+	return 0;
+}
+
+/**
+ * stm32_adc_get_trig_extsel() - Get external trigger selection
+ * @trig: trigger
+ *
+ * Returns trigger extsel value, if trig matches, -EINVAL otherwise.
+ */
+static int stm32_adc_get_trig_extsel(struct iio_trigger *trig)
+{
+	return -EINVAL;
+}
+
+/**
+ * stm32_adc_set_trig() - Set a regular trigger
+ * @indio_dev: IIO device
+ * @trig: IIO trigger
+ *
+ * Set trigger source/polarity (e.g. SW, or HW with polarity) :
+ * - if HW trigger disabled (e.g. trig == NULL, conversion launched by sw)
+ * - if HW trigger enabled, set source & polarity
+ */
+static int stm32_adc_set_trig(struct iio_dev *indio_dev,
+			      struct iio_trigger *trig)
+{
+	struct stm32_adc *adc = iio_priv(indio_dev);
+	u32 val, extsel = 0, exten = STM32_EXTEN_SWTRIG;
+	unsigned long flags;
+	int ret;
+
+	if (trig) {
+		ret = stm32_adc_get_trig_extsel(trig);
+		if (ret < 0)
+			return ret;
+
+		/* set trigger source and polarity (default to rising edge) */
+		extsel = ret;
+		exten = STM32_EXTEN_HWTRIG_RISING_EDGE;
+	}
+
+	spin_lock_irqsave(&adc->lock, flags);
+	val = stm32_adc_readl(adc, STM32F4_ADC_CR2);
+	val &= ~(STM32F4_EXTEN_MASK | STM32F4_EXTSEL_MASK);
+	val |= exten << STM32F4_EXTEN_SHIFT;
+	val |= extsel << STM32F4_EXTSEL_SHIFT;
+	stm32_adc_writel(adc, STM32F4_ADC_CR2, val);
+	spin_unlock_irqrestore(&adc->lock, flags);
+
+	return 0;
+}
+
+/**
  * stm32_adc_single_conv() - Performs a single conversion
  * @indio_dev: IIO device
  * @chan: IIO channel
@@ -228,21 +377,20 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
 	struct stm32_adc *adc = iio_priv(indio_dev);
 	long timeout;
 	u32 val;
-	u16 result;
 	int ret;
 
 	reinit_completion(&adc->completion);
 
-	adc->buffer = &result;
+	adc->bufi = 0;
 
-	/* Program chan number in regular sequence */
-	val = stm32_adc_readl(adc, STM32F4_ADC_SQR3);
-	val &= ~STM32F4_SQ1_MASK;
-	val |= chan->channel << STM32F4_SQ1_SHIFT;
-	stm32_adc_writel(adc, STM32F4_ADC_SQR3, val);
+	/* Program chan number in regular sequence (SQ1) */
+	val = stm32_adc_readl(adc, stm32f4_sq[1].reg);
+	val &= ~stm32f4_sq[1].mask;
+	val |= chan->channel << stm32f4_sq[1].shift;
+	stm32_adc_writel(adc, stm32f4_sq[1].reg, val);
 
 	/* Set regular sequence len (0 for 1 conversion) */
-	stm32_adc_clr_bits(adc, STM32F4_ADC_SQR1, STM32F4_L_MASK);
+	stm32_adc_clr_bits(adc, stm32f4_sq[0].reg, stm32f4_sq[0].mask);
 
 	/* Trigger detection disabled (conversion can be launched in SW) */
 	stm32_adc_clr_bits(adc, STM32F4_ADC_CR2, STM32F4_EXTEN_MASK);
@@ -258,7 +406,7 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
 	} else if (timeout < 0) {
 		ret = timeout;
 	} else {
-		*res = result;
+		*res = adc->buffer[0];
 		ret = IIO_VAL_INT;
 	}
 
@@ -301,17 +449,56 @@ static int stm32_adc_read_raw(struct iio_dev *indio_dev,
 static irqreturn_t stm32_adc_isr(int irq, void *data)
 {
 	struct stm32_adc *adc = data;
+	struct iio_dev *indio_dev = iio_priv_to_dev(adc);
 	u32 status = stm32_adc_readl(adc, STM32F4_ADC_SR);
 
 	if (status & STM32F4_EOC) {
-		*adc->buffer = stm32_adc_readw(adc, STM32F4_ADC_DR);
-		complete(&adc->completion);
+		/* Reading DR also clears EOC status flag */
+		adc->buffer[adc->bufi] = stm32_adc_readw(adc, STM32F4_ADC_DR);
+		if (iio_buffer_enabled(indio_dev)) {
+			adc->bufi++;
+			if (adc->bufi >= adc->num_conv) {
+				stm32_adc_conv_irq_disable(adc);
+				iio_trigger_poll(indio_dev->trig);
+			}
+		} else {
+			complete(&adc->completion);
+		}
 		return IRQ_HANDLED;
 	}
 
 	return IRQ_NONE;
 }
 
+/**
+ * stm32_adc_validate_trigger() - validate trigger for stm32 adc
+ * @indio_dev: IIO device
+ * @trig: new trigger
+ *
+ * Returns: 0 if trig matches one of the triggers registered by stm32 adc
+ * driver, -EINVAL otherwise.
+ */
+static int stm32_adc_validate_trigger(struct iio_dev *indio_dev,
+				      struct iio_trigger *trig)
+{
+	return stm32_adc_get_trig_extsel(trig) < 0 ? -EINVAL : 0;
+}
+
+static int stm32_adc_update_scan_mode(struct iio_dev *indio_dev,
+				      const unsigned long *scan_mask)
+{
+	struct stm32_adc *adc = iio_priv(indio_dev);
+	int ret;
+
+	adc->num_conv = bitmap_weight(scan_mask, indio_dev->masklength);
+
+	ret = stm32_adc_conf_scan_seq(indio_dev, scan_mask);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static int stm32_adc_of_xlate(struct iio_dev *indio_dev,
 			      const struct of_phandle_args *iiospec)
 {
@@ -350,11 +537,86 @@ static int stm32_adc_debugfs_reg_access(struct iio_dev *indio_dev,
 
 static const struct iio_info stm32_adc_iio_info = {
 	.read_raw = stm32_adc_read_raw,
+	.validate_trigger = stm32_adc_validate_trigger,
+	.update_scan_mode = stm32_adc_update_scan_mode,
 	.debugfs_reg_access = stm32_adc_debugfs_reg_access,
 	.of_xlate = stm32_adc_of_xlate,
 	.driver_module = THIS_MODULE,
 };
 
+static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
+{
+	struct stm32_adc *adc = iio_priv(indio_dev);
+	int ret;
+
+	ret = stm32_adc_set_trig(indio_dev, indio_dev->trig);
+	if (ret) {
+		dev_err(&indio_dev->dev, "Can't set trigger\n");
+		return ret;
+	}
+
+	ret = iio_triggered_buffer_postenable(indio_dev);
+	if (ret < 0)
+		goto err_clr_trig;
+
+	/* Reset adc buffer index */
+	adc->bufi = 0;
+
+	stm32_adc_conv_irq_enable(adc);
+	stm32_adc_start_conv(adc);
+
+	return 0;
+
+err_clr_trig:
+	stm32_adc_set_trig(indio_dev, NULL);
+
+	return ret;
+}
+
+static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev)
+{
+	struct stm32_adc *adc = iio_priv(indio_dev);
+	int ret;
+
+	stm32_adc_stop_conv(adc);
+	stm32_adc_conv_irq_disable(adc);
+
+	ret = iio_triggered_buffer_predisable(indio_dev);
+	if (ret < 0)
+		dev_err(&indio_dev->dev, "predisable failed\n");
+
+	if (stm32_adc_set_trig(indio_dev, NULL))
+		dev_err(&indio_dev->dev, "Can't clear trigger\n");
+
+	return ret;
+}
+
+static const struct iio_buffer_setup_ops stm32_adc_buffer_setup_ops = {
+	.postenable = &stm32_adc_buffer_postenable,
+	.predisable = &stm32_adc_buffer_predisable,
+};
+
+static irqreturn_t stm32_adc_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct stm32_adc *adc = iio_priv(indio_dev);
+
+	dev_dbg(&indio_dev->dev, "%s bufi=%d\n", __func__, adc->bufi);
+
+	/* reset buffer index */
+	adc->bufi = 0;
+	iio_push_to_buffers_with_timestamp(indio_dev, adc->buffer,
+					   pf->timestamp);
+
+	iio_trigger_notify_done(indio_dev->trig);
+
+	/* re-enable eoc irq */
+	stm32_adc_conv_irq_enable(adc);
+
+	return IRQ_HANDLED;
+}
+
 static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
 				    struct iio_chan_spec *chan,
 				    const struct stm32_adc_chan_spec *channel,
@@ -471,14 +733,26 @@ static int stm32_adc_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_clk_disable;
 
+	ret = iio_triggered_buffer_setup(indio_dev,
+					 &iio_pollfunc_store_time,
+					 &stm32_adc_trigger_handler,
+					 &stm32_adc_buffer_setup_ops);
+	if (ret) {
+		dev_err(&pdev->dev, "buffer setup failed\n");
+		goto err_clk_disable;
+	}
+
 	ret = iio_device_register(indio_dev);
 	if (ret) {
 		dev_err(&pdev->dev, "iio dev register failed\n");
-		goto err_clk_disable;
+		goto err_buffer_cleanup;
 	}
 
 	return 0;
 
+err_buffer_cleanup:
+	iio_triggered_buffer_cleanup(indio_dev);
+
 err_clk_disable:
 	clk_disable_unprepare(adc->clk);
 
@@ -491,6 +765,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
 	struct iio_dev *indio_dev = iio_priv_to_dev(adc);
 
 	iio_device_unregister(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
 	clk_disable_unprepare(adc->clk);
 
 	return 0;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/7] iio: adc: stm32: Enable use of stm32 timer triggers
  2017-01-26 14:28 [PATCH v2 0/7] Add support for triggered buffer mode to STM32 ADC Fabrice Gasnier
  2017-01-26 14:28 ` [PATCH v2 1/7] iio: adc: stm32: add support for triggered buffer mode Fabrice Gasnier
@ 2017-01-26 14:28 ` Fabrice Gasnier
  2017-01-28 18:37   ` Jonathan Cameron
  2017-01-26 14:28 ` [PATCH v2 3/7] iio: adc: stm32: add trigger polarity extended attribute Fabrice Gasnier
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Fabrice Gasnier @ 2017-01-26 14:28 UTC (permalink / raw)
  To: jic23, linux, robh+dt, linux-arm-kernel, devicetree, linux-kernel
  Cc: linux-iio, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
	knaack.h, pmeerw, fabrice.gasnier, benjamin.gaignard,
	benjamin.gaignard

STM32 ADC has external timer trigger sources. Use stm32 timer triggers
API (e.g. is_stm32_timer_trigger()) with local ADC lookup table to
validate a trigger can be used.
This also provides correct trigger selection value (e.g. extsel).

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
Changes in v2:
- Add comment on dual check to validate trigger (and get extsel)
---
 drivers/iio/adc/Kconfig     |  2 ++
 drivers/iio/adc/stm32-adc.c | 65 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 33341f4..9a7b090 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -447,6 +447,8 @@ config STM32_ADC_CORE
 	depends on OF
 	depends on REGULATOR
 	select IIO_BUFFER
+	select MFD_STM32_TIMERS
+	select IIO_STM32_TIMER_TRIGGER
 	select IIO_TRIGGERED_BUFFER
 	help
 	  Select this option to enable the core driver for STMicroelectronics
diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index 1e382b6..87d984b 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -23,6 +23,7 @@
 #include <linux/delay.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
+#include <linux/iio/timer/stm32-timer-trigger.h>
 #include <linux/iio/trigger.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
@@ -81,6 +82,36 @@ enum stm32_adc_exten {
 	STM32_EXTEN_HWTRIG_BOTH_EDGES,
 };
 
+/* extsel - trigger mux selection value */
+enum stm32_adc_extsel {
+	STM32_EXT0,
+	STM32_EXT1,
+	STM32_EXT2,
+	STM32_EXT3,
+	STM32_EXT4,
+	STM32_EXT5,
+	STM32_EXT6,
+	STM32_EXT7,
+	STM32_EXT8,
+	STM32_EXT9,
+	STM32_EXT10,
+	STM32_EXT11,
+	STM32_EXT12,
+	STM32_EXT13,
+	STM32_EXT14,
+	STM32_EXT15,
+};
+
+/**
+ * struct stm32_adc_trig_info - ADC trigger info
+ * @name:		name of the trigger, corresponding to its source
+ * @extsel:		trigger selection
+ */
+struct stm32_adc_trig_info {
+	const char *name;
+	enum stm32_adc_extsel extsel;
+};
+
 /**
  * stm32_adc_regs - stm32 ADC misc registers & bitfield desc
  * @reg:		register offset
@@ -176,6 +207,26 @@ struct stm32_adc_chan_spec {
 	{ STM32F4_ADC_SQR1, GENMASK(19, 15), 15 },
 };
 
+/* STM32F4 external trigger sources for all instances */
+static struct stm32_adc_trig_info stm32f4_adc_trigs[] = {
+	{ TIM1_CH1, STM32_EXT0 },
+	{ TIM1_CH2, STM32_EXT1 },
+	{ TIM1_CH3, STM32_EXT2 },
+	{ TIM2_CH2, STM32_EXT3 },
+	{ TIM2_CH3, STM32_EXT4 },
+	{ TIM2_CH4, STM32_EXT5 },
+	{ TIM2_TRGO, STM32_EXT6 },
+	{ TIM3_CH1, STM32_EXT7 },
+	{ TIM3_TRGO, STM32_EXT8 },
+	{ TIM4_CH4, STM32_EXT9 },
+	{ TIM5_CH1, STM32_EXT10 },
+	{ TIM5_CH2, STM32_EXT11 },
+	{ TIM5_CH3, STM32_EXT12 },
+	{ TIM8_CH1, STM32_EXT13 },
+	{ TIM8_TRGO, STM32_EXT14 },
+	{}, /* sentinel */
+};
+
 /**
  * STM32 ADC registers access routines
  * @adc: stm32 adc instance
@@ -318,6 +369,20 @@ static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
  */
 static int stm32_adc_get_trig_extsel(struct iio_trigger *trig)
 {
+	int i;
+
+	/* lookup triggers registered by stm32 timer trigger driver */
+	for (i = 0; stm32f4_adc_trigs[i].name; i++) {
+		/**
+		 * Checking both stm32 timer trigger type and trig name
+		 * should be safe against arbitrary trigger names.
+		 */
+		if (is_stm32_timer_trigger(trig) &&
+		    !strcmp(stm32f4_adc_trigs[i].name, trig->name)) {
+			return stm32f4_adc_trigs[i].extsel;
+		}
+	}
+
 	return -EINVAL;
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/7] iio: adc: stm32: add trigger polarity extended attribute
  2017-01-26 14:28 [PATCH v2 0/7] Add support for triggered buffer mode to STM32 ADC Fabrice Gasnier
  2017-01-26 14:28 ` [PATCH v2 1/7] iio: adc: stm32: add support for triggered buffer mode Fabrice Gasnier
  2017-01-26 14:28 ` [PATCH v2 2/7] iio: adc: stm32: Enable use of stm32 timer triggers Fabrice Gasnier
@ 2017-01-26 14:28 ` Fabrice Gasnier
  2017-01-28 18:39   ` Jonathan Cameron
  2017-01-26 14:28 ` [PATCH v2 4/7] Documentation: dt: iio: stm32-adc: optional dma support Fabrice Gasnier
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Fabrice Gasnier @ 2017-01-26 14:28 UTC (permalink / raw)
  To: jic23, linux, robh+dt, linux-arm-kernel, devicetree, linux-kernel
  Cc: linux-iio, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
	knaack.h, pmeerw, fabrice.gasnier, benjamin.gaignard,
	benjamin.gaignard

Define extended attribute so that user may choose rising, falling or both
edges for external trigger sources.
Default to rising edge in case it isn't set.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
Changes in v2:
- Rename and document new trigger_polarity custom attribute
---
 Documentation/ABI/testing/sysfs-bus-iio-adc-stm32 | 18 +++++++++
 drivers/iio/adc/stm32-adc.c                       | 46 ++++++++++++++++++++++-
 2 files changed, 63 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-stm32

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-adc-stm32
new file mode 100644
index 0000000..efe4c85
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-stm32
@@ -0,0 +1,18 @@
+What:		/sys/bus/iio/devices/triggerX/trigger_polarity
+KernelVersion:	4.11
+Contact:	fabrice.gasnier@st.com
+Description:
+		The STM32 ADC can be configured to use external trigger sources
+		(e.g. timers, pwm or exti gpio). Then, it can be tuned to start
+		conversions on external trigger by either:
+		- "rising-edge"
+		- "falling-edge"
+		- "both-edges".
+		Reading returns current trigger polarity.
+		Writing value before enabling conversions sets trigger polarity.
+
+What:		/sys/bus/iio/devices/triggerX/trigger_polarity_available
+KernelVersion:	4.11
+Contact:	fabrice.gasnier@st.com
+Description:
+		List all available trigger_polarity settings.
diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index 87d984b..9a38f9a 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -135,6 +135,7 @@ struct stm32_adc_regs {
  * @lock:		spinlock
  * @bufi:		data buffer index
  * @num_conv:		expected number of scan conversions
+ * @trigger_polarity:	external trigger polarity (e.g. exten)
  */
 struct stm32_adc {
 	struct stm32_adc_common	*common;
@@ -146,6 +147,7 @@ struct stm32_adc {
 	spinlock_t		lock;		/* interrupt lock */
 	unsigned int		bufi;
 	unsigned int		num_conv;
+	u32			trigger_polarity;
 };
 
 /**
@@ -410,7 +412,7 @@ static int stm32_adc_set_trig(struct iio_dev *indio_dev,
 
 		/* set trigger source and polarity (default to rising edge) */
 		extsel = ret;
-		exten = STM32_EXTEN_HWTRIG_RISING_EDGE;
+		exten = adc->trigger_polarity + STM32_EXTEN_HWTRIG_RISING_EDGE;
 	}
 
 	spin_lock_irqsave(&adc->lock, flags);
@@ -424,6 +426,36 @@ static int stm32_adc_set_trig(struct iio_dev *indio_dev,
 	return 0;
 }
 
+static int stm32_adc_set_trig_pol(struct iio_dev *indio_dev,
+				  const struct iio_chan_spec *chan,
+				  unsigned int type)
+{
+	struct stm32_adc *adc = iio_priv(indio_dev);
+
+	adc->trigger_polarity = type;
+
+	return 0;
+}
+
+static int stm32_adc_get_trig_pol(struct iio_dev *indio_dev,
+				  const struct iio_chan_spec *chan)
+{
+	struct stm32_adc *adc = iio_priv(indio_dev);
+
+	return adc->trigger_polarity;
+}
+
+static const char * const stm32_trig_pol_items[] = {
+	"rising-edge", "falling-edge", "both-edges",
+};
+
+const struct iio_enum stm32_adc_trig_pol = {
+	.items = stm32_trig_pol_items,
+	.num_items = ARRAY_SIZE(stm32_trig_pol_items),
+	.get = stm32_adc_get_trig_pol,
+	.set = stm32_adc_set_trig_pol,
+};
+
 /**
  * stm32_adc_single_conv() - Performs a single conversion
  * @indio_dev: IIO device
@@ -682,6 +714,17 @@ static irqreturn_t stm32_adc_trigger_handler(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
+static const struct iio_chan_spec_ext_info stm32_adc_ext_info[] = {
+	IIO_ENUM("trigger_polarity", IIO_SHARED_BY_ALL, &stm32_adc_trig_pol),
+	{
+		.name = "trigger_polarity_available",
+		.shared = IIO_SHARED_BY_ALL,
+		.read = iio_enum_available_read,
+		.private = (uintptr_t)&stm32_adc_trig_pol,
+	},
+	{},
+};
+
 static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
 				    struct iio_chan_spec *chan,
 				    const struct stm32_adc_chan_spec *channel,
@@ -697,6 +740,7 @@ static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
 	chan->scan_type.sign = 'u';
 	chan->scan_type.realbits = 12;
 	chan->scan_type.storagebits = 16;
+	chan->ext_info = stm32_adc_ext_info;
 }
 
 static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 4/7] Documentation: dt: iio: stm32-adc: optional dma support
  2017-01-26 14:28 [PATCH v2 0/7] Add support for triggered buffer mode to STM32 ADC Fabrice Gasnier
                   ` (2 preceding siblings ...)
  2017-01-26 14:28 ` [PATCH v2 3/7] iio: adc: stm32: add trigger polarity extended attribute Fabrice Gasnier
@ 2017-01-26 14:28 ` Fabrice Gasnier
  2017-01-28 18:39   ` Jonathan Cameron
  2017-01-26 14:28 ` [PATCH v2 5/7] iio: adc: stm32: add " Fabrice Gasnier
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Fabrice Gasnier @ 2017-01-26 14:28 UTC (permalink / raw)
  To: jic23, linux, robh+dt, linux-arm-kernel, devicetree, linux-kernel
  Cc: linux-iio, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
	knaack.h, pmeerw, fabrice.gasnier, benjamin.gaignard,
	benjamin.gaignard

STM32 ADC can use dma. Add dt documentation for optional dma support.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
index 49ed82e..5dfc88e 100644
--- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
@@ -53,6 +53,11 @@ Required properties:
 - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
   Documentation/devicetree/bindings/iio/iio-bindings.txt
 
+Optional properties:
+- dmas: Phandle to dma channel for this ADC instance.
+  See ../../dma/dma.txt for details.
+- dma-names: Must be "rx" when dmas property is being used.
+
 Example:
 	adc: adc@40012000 {
 		compatible = "st,stm32f4-adc-core";
@@ -77,6 +82,8 @@ Example:
 			interrupt-parent = <&adc>;
 			interrupts = <0>;
 			st,adc-channels = <8>;
+			dmas = <&dma2 0 0 0x400 0x0>;
+			dma-names = "rx";
 		};
 		...
 		other adc child nodes follow...
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 5/7] iio: adc: stm32: add optional dma support
  2017-01-26 14:28 [PATCH v2 0/7] Add support for triggered buffer mode to STM32 ADC Fabrice Gasnier
                   ` (3 preceding siblings ...)
  2017-01-26 14:28 ` [PATCH v2 4/7] Documentation: dt: iio: stm32-adc: optional dma support Fabrice Gasnier
@ 2017-01-26 14:28 ` Fabrice Gasnier
  2017-01-28 18:40   ` Jonathan Cameron
  2017-01-28 18:41   ` Jonathan Cameron
  2017-01-26 14:28 ` [PATCH v2 6/7] ARM: dts: stm32: Enable dma by default on stm32f4 adc Fabrice Gasnier
                   ` (2 subsequent siblings)
  7 siblings, 2 replies; 16+ messages in thread
From: Fabrice Gasnier @ 2017-01-26 14:28 UTC (permalink / raw)
  To: jic23, linux, robh+dt, linux-arm-kernel, devicetree, linux-kernel
  Cc: linux-iio, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
	knaack.h, pmeerw, fabrice.gasnier, benjamin.gaignard,
	benjamin.gaignard

Add DMA optional support to STM32 ADC, as there is a limited number DMA
channels (request lines) that can be assigned to ADC. This way, driver
may fall back using interrupts when all DMA channels are in use for
other IPs.
Use dma cyclic mode with two periods. Allow to tune period length by
using watermark. Coherent memory is used for dma (max buffer size is
fixed to PAGE_SIZE).

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
Changes in v2:
- Use iio_trigger_poll_chained() avoids to bounce back into irq context.
  Remove irq_work.
- Rework dma buffer allocation and use. Allocation moved to probe time,
  fixed to PAGE_SIZE. Use hwfifo_set_watermark() routine to tune dma
  cyclic period length.
---
 drivers/iio/adc/Kconfig          |   1 +
 drivers/iio/adc/stm32-adc-core.c |   1 +
 drivers/iio/adc/stm32-adc-core.h |   2 +
 drivers/iio/adc/stm32-adc.c      | 227 ++++++++++++++++++++++++++++++++++++---
 4 files changed, 218 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 9a7b090..03a73f9 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -444,6 +444,7 @@ config ROCKCHIP_SARADC
 config STM32_ADC_CORE
 	tristate "STMicroelectronics STM32 adc core"
 	depends on ARCH_STM32 || COMPILE_TEST
+	depends on HAS_DMA
 	depends on OF
 	depends on REGULATOR
 	select IIO_BUFFER
diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
index 4214b0c..22b7c93 100644
--- a/drivers/iio/adc/stm32-adc-core.c
+++ b/drivers/iio/adc/stm32-adc-core.c
@@ -201,6 +201,7 @@ static int stm32_adc_probe(struct platform_device *pdev)
 	priv->common.base = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(priv->common.base))
 		return PTR_ERR(priv->common.base);
+	priv->common.phys_base = res->start;
 
 	priv->vref = devm_regulator_get(&pdev->dev, "vref");
 	if (IS_ERR(priv->vref)) {
diff --git a/drivers/iio/adc/stm32-adc-core.h b/drivers/iio/adc/stm32-adc-core.h
index 081fa5f..2ec7abb 100644
--- a/drivers/iio/adc/stm32-adc-core.h
+++ b/drivers/iio/adc/stm32-adc-core.h
@@ -42,10 +42,12 @@
 /**
  * struct stm32_adc_common - stm32 ADC driver common data (for all instances)
  * @base:		control registers base cpu addr
+ * @phys_base:		control registers base physical addr
  * @vref_mv:		vref voltage (mv)
  */
 struct stm32_adc_common {
 	void __iomem			*base;
+	phys_addr_t			phys_base;
 	int				vref_mv;
 };
 
diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index 9a38f9a..8a30039 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -21,6 +21,8 @@
 
 #include <linux/clk.h>
 #include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/timer/stm32-timer-trigger.h>
@@ -68,12 +70,16 @@
 #define STM32F4_EXTSEL_SHIFT		24
 #define STM32F4_EXTSEL_MASK		GENMASK(27, 24)
 #define STM32F4_EOCS			BIT(10)
+#define STM32F4_DDS			BIT(9)
+#define STM32F4_DMA			BIT(8)
 #define STM32F4_ADON			BIT(0)
 
 #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
 #define STM32_ADC_TIMEOUT_US		100000
 #define STM32_ADC_TIMEOUT	(msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
 
+#define STM32_DMA_BUFFER_SIZE		PAGE_SIZE
+
 /* External trigger enable */
 enum stm32_adc_exten {
 	STM32_EXTEN_SWTRIG,
@@ -136,6 +142,10 @@ struct stm32_adc_regs {
  * @bufi:		data buffer index
  * @num_conv:		expected number of scan conversions
  * @trigger_polarity:	external trigger polarity (e.g. exten)
+ * @dma_chan:		dma channel
+ * @rx_buf:		dma rx buffer cpu address
+ * @rx_dma_buf:		dma rx buffer bus address
+ * @rx_buf_sz:		dma rx buffer size
  */
 struct stm32_adc {
 	struct stm32_adc_common	*common;
@@ -148,6 +158,10 @@ struct stm32_adc {
 	unsigned int		bufi;
 	unsigned int		num_conv;
 	u32			trigger_polarity;
+	struct dma_chan		*dma_chan;
+	u8			*rx_buf;
+	dma_addr_t		rx_dma_buf;
+	unsigned int		rx_buf_sz;
 };
 
 /**
@@ -291,10 +305,21 @@ static void stm32_adc_conv_irq_disable(struct stm32_adc *adc)
 /**
  * stm32_adc_start_conv() - Start conversions for regular channels.
  * @adc: stm32 adc instance
+ * @dma: use dma to transfer conversion result
+ *
+ * Start conversions for regular channels.
+ * Also take care of normal or DMA mode. Circular DMA may be used for regular
+ * conversions, in IIO buffer modes. Otherwise, use ADC interrupt with direct
+ * DR read instead (e.g. read_raw, or triggered buffer mode without DMA).
  */
-static void stm32_adc_start_conv(struct stm32_adc *adc)
+static void stm32_adc_start_conv(struct stm32_adc *adc, bool dma)
 {
 	stm32_adc_set_bits(adc, STM32F4_ADC_CR1, STM32F4_SCAN);
+
+	if (dma)
+		stm32_adc_set_bits(adc, STM32F4_ADC_CR2,
+				   STM32F4_DMA | STM32F4_DDS);
+
 	stm32_adc_set_bits(adc, STM32F4_ADC_CR2, STM32F4_EOCS | STM32F4_ADON);
 
 	/* Wait for Power-up time (tSTAB from datasheet) */
@@ -311,7 +336,8 @@ static void stm32_adc_stop_conv(struct stm32_adc *adc)
 	stm32_adc_clr_bits(adc, STM32F4_ADC_SR, STM32F4_STRT);
 
 	stm32_adc_clr_bits(adc, STM32F4_ADC_CR1, STM32F4_SCAN);
-	stm32_adc_clr_bits(adc, STM32F4_ADC_CR2, STM32F4_ADON);
+	stm32_adc_clr_bits(adc, STM32F4_ADC_CR2,
+			   STM32F4_ADON | STM32F4_DMA | STM32F4_DDS);
 }
 
 /**
@@ -494,7 +520,7 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
 
 	stm32_adc_conv_irq_enable(adc);
 
-	stm32_adc_start_conv(adc);
+	stm32_adc_start_conv(adc, false);
 
 	timeout = wait_for_completion_interruptible_timeout(
 					&adc->completion, STM32_ADC_TIMEOUT);
@@ -581,6 +607,23 @@ static int stm32_adc_validate_trigger(struct iio_dev *indio_dev,
 	return stm32_adc_get_trig_extsel(trig) < 0 ? -EINVAL : 0;
 }
 
+static int stm32_adc_set_watermark(struct iio_dev *indio_dev, unsigned val)
+{
+	struct stm32_adc *adc = iio_priv(indio_dev);
+	unsigned int watermark = STM32_DMA_BUFFER_SIZE / 2;
+
+	/*
+	 * dma cyclic transfers are used, buffer is split into two periods.
+	 * There should be :
+	 * - always one buffer (period) dma is working on
+	 * - one buffer (period) driver can push with iio_trigger_poll().
+	 */
+	watermark = min(watermark, val * sizeof(u16));
+	adc->rx_buf_sz = watermark * 2;
+
+	return 0;
+}
+
 static int stm32_adc_update_scan_mode(struct iio_dev *indio_dev,
 				      const unsigned long *scan_mask)
 {
@@ -635,12 +678,83 @@ static int stm32_adc_debugfs_reg_access(struct iio_dev *indio_dev,
 static const struct iio_info stm32_adc_iio_info = {
 	.read_raw = stm32_adc_read_raw,
 	.validate_trigger = stm32_adc_validate_trigger,
+	.hwfifo_set_watermark = stm32_adc_set_watermark,
 	.update_scan_mode = stm32_adc_update_scan_mode,
 	.debugfs_reg_access = stm32_adc_debugfs_reg_access,
 	.of_xlate = stm32_adc_of_xlate,
 	.driver_module = THIS_MODULE,
 };
 
+static unsigned int stm32_adc_dma_residue(struct stm32_adc *adc)
+{
+	struct dma_tx_state state;
+	enum dma_status status;
+
+	status = dmaengine_tx_status(adc->dma_chan,
+				     adc->dma_chan->cookie,
+				     &state);
+	if (status == DMA_IN_PROGRESS) {
+		/* Residue is size in bytes from end of buffer */
+		unsigned int i = adc->rx_buf_sz - state.residue;
+		unsigned int size;
+
+		/* Return available bytes */
+		if (i >= adc->bufi)
+			size = i - adc->bufi;
+		else
+			size = adc->rx_buf_sz + i - adc->bufi;
+
+		return size;
+	}
+
+	return 0;
+}
+
+static void stm32_adc_dma_buffer_done(void *data)
+{
+	struct iio_dev *indio_dev = data;
+
+	iio_trigger_poll_chained(indio_dev->trig);
+}
+
+static int stm32_adc_dma_start(struct iio_dev *indio_dev)
+{
+	struct stm32_adc *adc = iio_priv(indio_dev);
+	struct dma_async_tx_descriptor *desc;
+	dma_cookie_t cookie;
+	int ret;
+
+	if (!adc->dma_chan)
+		return 0;
+
+	dev_dbg(&indio_dev->dev, "%s size=%d watermark=%d\n", __func__,
+		adc->rx_buf_sz, adc->rx_buf_sz / 2);
+
+	/* Prepare a DMA cyclic transaction */
+	desc = dmaengine_prep_dma_cyclic(adc->dma_chan,
+					 adc->rx_dma_buf,
+					 adc->rx_buf_sz, adc->rx_buf_sz / 2,
+					 DMA_DEV_TO_MEM,
+					 DMA_PREP_INTERRUPT);
+	if (!desc)
+		return -EBUSY;
+
+	desc->callback = stm32_adc_dma_buffer_done;
+	desc->callback_param = indio_dev;
+
+	cookie = dmaengine_submit(desc);
+	ret = dma_submit_error(cookie);
+	if (ret) {
+		dmaengine_terminate_all(adc->dma_chan);
+		return ret;
+	}
+
+	/* Issue pending DMA requests */
+	dma_async_issue_pending(adc->dma_chan);
+
+	return 0;
+}
+
 static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
 {
 	struct stm32_adc *adc = iio_priv(indio_dev);
@@ -652,18 +766,29 @@ static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
 		return ret;
 	}
 
+	ret = stm32_adc_dma_start(indio_dev);
+	if (ret) {
+		dev_err(&indio_dev->dev, "Can't start dma\n");
+		goto err_clr_trig;
+	}
+
 	ret = iio_triggered_buffer_postenable(indio_dev);
 	if (ret < 0)
-		goto err_clr_trig;
+		goto err_stop_dma;
 
 	/* Reset adc buffer index */
 	adc->bufi = 0;
 
-	stm32_adc_conv_irq_enable(adc);
-	stm32_adc_start_conv(adc);
+	if (!adc->dma_chan)
+		stm32_adc_conv_irq_enable(adc);
+
+	stm32_adc_start_conv(adc, !!adc->dma_chan);
 
 	return 0;
 
+err_stop_dma:
+	if (adc->dma_chan)
+		dmaengine_terminate_all(adc->dma_chan);
 err_clr_trig:
 	stm32_adc_set_trig(indio_dev, NULL);
 
@@ -676,12 +801,16 @@ static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev)
 	int ret;
 
 	stm32_adc_stop_conv(adc);
-	stm32_adc_conv_irq_disable(adc);
+	if (!adc->dma_chan)
+		stm32_adc_conv_irq_disable(adc);
 
 	ret = iio_triggered_buffer_predisable(indio_dev);
 	if (ret < 0)
 		dev_err(&indio_dev->dev, "predisable failed\n");
 
+	if (adc->dma_chan)
+		dmaengine_terminate_all(adc->dma_chan);
+
 	if (stm32_adc_set_trig(indio_dev, NULL))
 		dev_err(&indio_dev->dev, "Can't clear trigger\n");
 
@@ -701,15 +830,31 @@ static irqreturn_t stm32_adc_trigger_handler(int irq, void *p)
 
 	dev_dbg(&indio_dev->dev, "%s bufi=%d\n", __func__, adc->bufi);
 
-	/* reset buffer index */
-	adc->bufi = 0;
-	iio_push_to_buffers_with_timestamp(indio_dev, adc->buffer,
-					   pf->timestamp);
+	if (!adc->dma_chan) {
+		/* reset buffer index */
+		adc->bufi = 0;
+		iio_push_to_buffers_with_timestamp(indio_dev, adc->buffer,
+						   pf->timestamp);
+	} else {
+		int residue = stm32_adc_dma_residue(adc);
+
+		while (residue >= indio_dev->scan_bytes) {
+			u16 *buffer = (u16 *)&adc->rx_buf[adc->bufi];
+
+			iio_push_to_buffers_with_timestamp(indio_dev, buffer,
+							   pf->timestamp);
+			residue -= indio_dev->scan_bytes;
+			adc->bufi += indio_dev->scan_bytes;
+			if (adc->bufi >= adc->rx_buf_sz)
+				adc->bufi = 0;
+		}
+	}
 
 	iio_trigger_notify_done(indio_dev->trig);
 
 	/* re-enable eoc irq */
-	stm32_adc_conv_irq_enable(adc);
+	if (!adc->dma_chan)
+		stm32_adc_conv_irq_enable(adc);
 
 	return IRQ_HANDLED;
 }
@@ -781,6 +926,45 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
 	return 0;
 }
 
+static int stm32_adc_dma_request(struct iio_dev *indio_dev)
+{
+	struct stm32_adc *adc = iio_priv(indio_dev);
+	struct dma_slave_config config;
+	int ret;
+
+	adc->dma_chan = dma_request_slave_channel(&indio_dev->dev, "rx");
+	if (!adc->dma_chan)
+		return 0;
+
+	adc->rx_buf = dma_alloc_coherent(adc->dma_chan->device->dev,
+					 STM32_DMA_BUFFER_SIZE,
+					 &adc->rx_dma_buf, GFP_KERNEL);
+	if (!adc->rx_buf) {
+		goto err_release;
+		ret = -ENOMEM;
+	}
+
+	/* Configure DMA channel to read data register */
+	memset(&config, 0, sizeof(config));
+	config.src_addr = (dma_addr_t)adc->common->phys_base;
+	config.src_addr += adc->offset + STM32F4_ADC_DR;
+	config.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+
+	ret = dmaengine_slave_config(adc->dma_chan, &config);
+	if (ret)
+		goto err_free;
+
+	return 0;
+
+err_free:
+	dma_free_coherent(adc->dma_chan->device->dev, STM32_DMA_BUFFER_SIZE,
+			  adc->rx_buf, adc->rx_dma_buf);
+err_release:
+	dma_release_channel(adc->dma_chan);
+
+	return ret;
+}
+
 static int stm32_adc_probe(struct platform_device *pdev)
 {
 	struct iio_dev *indio_dev;
@@ -842,13 +1026,17 @@ static int stm32_adc_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_clk_disable;
 
+	ret = stm32_adc_dma_request(indio_dev);
+	if (ret < 0)
+		goto err_clk_disable;
+
 	ret = iio_triggered_buffer_setup(indio_dev,
 					 &iio_pollfunc_store_time,
 					 &stm32_adc_trigger_handler,
 					 &stm32_adc_buffer_setup_ops);
 	if (ret) {
 		dev_err(&pdev->dev, "buffer setup failed\n");
-		goto err_clk_disable;
+		goto err_dma_disable;
 	}
 
 	ret = iio_device_register(indio_dev);
@@ -862,6 +1050,13 @@ static int stm32_adc_probe(struct platform_device *pdev)
 err_buffer_cleanup:
 	iio_triggered_buffer_cleanup(indio_dev);
 
+err_dma_disable:
+	if (adc->dma_chan) {
+		dma_free_coherent(adc->dma_chan->device->dev,
+				  STM32_DMA_BUFFER_SIZE,
+				  adc->rx_buf, adc->rx_dma_buf);
+		dma_release_channel(adc->dma_chan);
+	}
 err_clk_disable:
 	clk_disable_unprepare(adc->clk);
 
@@ -875,6 +1070,12 @@ static int stm32_adc_remove(struct platform_device *pdev)
 
 	iio_device_unregister(indio_dev);
 	iio_triggered_buffer_cleanup(indio_dev);
+	if (adc->dma_chan) {
+		dma_free_coherent(adc->dma_chan->device->dev,
+				  STM32_DMA_BUFFER_SIZE,
+				  adc->rx_buf, adc->rx_dma_buf);
+		dma_release_channel(adc->dma_chan);
+	}
 	clk_disable_unprepare(adc->clk);
 
 	return 0;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 6/7] ARM: dts: stm32: Enable dma by default on stm32f4 adc
  2017-01-26 14:28 [PATCH v2 0/7] Add support for triggered buffer mode to STM32 ADC Fabrice Gasnier
                   ` (4 preceding siblings ...)
  2017-01-26 14:28 ` [PATCH v2 5/7] iio: adc: stm32: add " Fabrice Gasnier
@ 2017-01-26 14:28 ` Fabrice Gasnier
  2017-01-26 14:28 ` [PATCH v2 7/7] ARM: dts: stm32: Enable pwm1 and pwm3 on stm32f429i-eval Fabrice Gasnier
  2017-04-03 14:53 ` [PATCH v2 0/7] Add support for triggered buffer mode to STM32 ADC Alexandre Torgue
  7 siblings, 0 replies; 16+ messages in thread
From: Fabrice Gasnier @ 2017-01-26 14:28 UTC (permalink / raw)
  To: jic23, linux, robh+dt, linux-arm-kernel, devicetree, linux-kernel
  Cc: linux-iio, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
	knaack.h, pmeerw, fabrice.gasnier, benjamin.gaignard,
	benjamin.gaignard

Configure STM32F4 ADC to use dma by default.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 arch/arm/boot/dts/stm32f429.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index 8bf650d..ab42b58 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -416,6 +416,8 @@
 				clocks = <&rcc 0 168>;
 				interrupt-parent = <&adc>;
 				interrupts = <0>;
+				dmas = <&dma2 0 0 0x400 0x0>;
+				dma-names = "rx";
 				status = "disabled";
 			};
 
@@ -426,6 +428,8 @@
 				clocks = <&rcc 0 169>;
 				interrupt-parent = <&adc>;
 				interrupts = <1>;
+				dmas = <&dma2 3 1 0x400 0x0>;
+				dma-names = "rx";
 				status = "disabled";
 			};
 
@@ -436,6 +440,8 @@
 				clocks = <&rcc 0 170>;
 				interrupt-parent = <&adc>;
 				interrupts = <2>;
+				dmas = <&dma2 1 2 0x400 0x0>;
+				dma-names = "rx";
 				status = "disabled";
 			};
 		};
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 7/7] ARM: dts: stm32: Enable pwm1 and pwm3 on stm32f429i-eval
  2017-01-26 14:28 [PATCH v2 0/7] Add support for triggered buffer mode to STM32 ADC Fabrice Gasnier
                   ` (5 preceding siblings ...)
  2017-01-26 14:28 ` [PATCH v2 6/7] ARM: dts: stm32: Enable dma by default on stm32f4 adc Fabrice Gasnier
@ 2017-01-26 14:28 ` Fabrice Gasnier
  2017-04-03 14:53 ` [PATCH v2 0/7] Add support for triggered buffer mode to STM32 ADC Alexandre Torgue
  7 siblings, 0 replies; 16+ messages in thread
From: Fabrice Gasnier @ 2017-01-26 14:28 UTC (permalink / raw)
  To: jic23, linux, robh+dt, linux-arm-kernel, devicetree, linux-kernel
  Cc: linux-iio, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
	knaack.h, pmeerw, fabrice.gasnier, benjamin.gaignard,
	benjamin.gaignard

Define and enable pwm1 and pwm3, timers1 & 3 trigger outputs on
on stm32f429i-eval board.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
Changes in v2:
- Fix typo
- Sort phandles
---
 arch/arm/boot/dts/stm32429i-eval.dts | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm/boot/dts/stm32429i-eval.dts b/arch/arm/boot/dts/stm32429i-eval.dts
index 2181220..507ee9d 100644
--- a/arch/arm/boot/dts/stm32429i-eval.dts
+++ b/arch/arm/boot/dts/stm32429i-eval.dts
@@ -157,6 +157,34 @@
 	};
 };
 
+&timers1 {
+	status = "okay";
+
+	pwm {
+		pinctrl-0 = <&pwm1_pins>;
+		pinctrl-names = "default";
+		status = "okay";
+	};
+
+	timer@0 {
+		status = "okay";
+	};
+};
+
+&timers3 {
+	status = "okay";
+
+	pwm {
+		pinctrl-0 = <&pwm3_pins>;
+		pinctrl-names = "default";
+		status = "okay";
+	};
+
+	timer@2 {
+		status = "okay";
+	};
+};
+
 &usart1 {
 	pinctrl-0 = <&usart1_pins_a>;
 	pinctrl-names = "default";
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/7] iio: adc: stm32: add support for triggered buffer mode
  2017-01-26 14:28 ` [PATCH v2 1/7] iio: adc: stm32: add support for triggered buffer mode Fabrice Gasnier
@ 2017-01-28 18:36   ` Jonathan Cameron
  0 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2017-01-28 18:36 UTC (permalink / raw)
  To: Fabrice Gasnier, linux, robh+dt, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: linux-iio, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
	knaack.h, pmeerw, benjamin.gaignard, benjamin.gaignard

On 26/01/17 14:28, Fabrice Gasnier wrote:
> STM32 ADC conversions can be launched using hardware triggers.
> It can be used to start conversion sequences (group of channels).
> Selected channels are select via sequence registers.
> Trigger source is selected via 'extsel' (external trigger mux).
> Trigger polarity is set to rising edge by default.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
I thought about leaving this longer to see if anyone else wanted
to comment, but seeing as the changes are minor and the original
patches were about for over a week I'm happy to take these now.

If anyone was planning on taking a look, give me a shout and I'll
yank them back out for a bit.

Anyhow, I've pulled in the immutable branch from mfd to get the
prerequisites and applied this to my togreg branch - initially pushed
out as testing to see what the autobuilders make of them.

This series came together very nicely (and quickly *touch wood*)

Thanks,

Jonathan 
> ---
> Changes in v2:
> based on Jonathan's comments:
> - remove all STM32F4_SQx[SHIFT/MASK], put the numbers directly in sq
>   description array.
> - make data buffer part of stm32_adc structure (remove preenable and
>   postdisable routines)
> - add comment on reading DR to clear OEC flag
> - use bitmap_weight()
> - fix error handling in stm32_adc_buffer_postenable()
> ---
>  drivers/iio/adc/Kconfig     |   2 +
>  drivers/iio/adc/stm32-adc.c | 317 +++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 298 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 9c8b558..33341f4 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -446,6 +446,8 @@ config STM32_ADC_CORE
>  	depends on ARCH_STM32 || COMPILE_TEST
>  	depends on OF
>  	depends on REGULATOR
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
>  	help
>  	  Select this option to enable the core driver for STMicroelectronics
>  	  STM32 analog-to-digital converter (ADC).
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 5715e79..1e382b6 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -22,6 +22,10 @@
>  #include <linux/clk.h>
>  #include <linux/delay.h>
>  #include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
> @@ -58,21 +62,37 @@
>  
>  /* STM32F4_ADC_CR2 - bit fields */
>  #define STM32F4_SWSTART			BIT(30)
> +#define STM32F4_EXTEN_SHIFT		28
>  #define STM32F4_EXTEN_MASK		GENMASK(29, 28)
> +#define STM32F4_EXTSEL_SHIFT		24
> +#define STM32F4_EXTSEL_MASK		GENMASK(27, 24)
>  #define STM32F4_EOCS			BIT(10)
>  #define STM32F4_ADON			BIT(0)
>  
> -/* STM32F4_ADC_SQR1 - bit fields */
> -#define STM32F4_L_SHIFT			20
> -#define STM32F4_L_MASK			GENMASK(23, 20)
> -
> -/* STM32F4_ADC_SQR3 - bit fields */
> -#define STM32F4_SQ1_SHIFT		0
> -#define STM32F4_SQ1_MASK		GENMASK(4, 0)
> -
> +#define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
>  #define STM32_ADC_TIMEOUT_US		100000
>  #define STM32_ADC_TIMEOUT	(msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
>  
> +/* External trigger enable */
> +enum stm32_adc_exten {
> +	STM32_EXTEN_SWTRIG,
> +	STM32_EXTEN_HWTRIG_RISING_EDGE,
> +	STM32_EXTEN_HWTRIG_FALLING_EDGE,
> +	STM32_EXTEN_HWTRIG_BOTH_EDGES,
> +};
> +
> +/**
> + * stm32_adc_regs - stm32 ADC misc registers & bitfield desc
> + * @reg:		register offset
> + * @mask:		bitfield mask
> + * @shift:		left shift
> + */
> +struct stm32_adc_regs {
> +	int reg;
> +	int mask;
> +	int shift;
> +};
> +
>  /**
>   * struct stm32_adc - private data of each ADC IIO instance
>   * @common:		reference to ADC block common data
> @@ -82,15 +102,19 @@
>   * @clk:		clock for this adc instance
>   * @irq:		interrupt for this adc instance
>   * @lock:		spinlock
> + * @bufi:		data buffer index
> + * @num_conv:		expected number of scan conversions
>   */
>  struct stm32_adc {
>  	struct stm32_adc_common	*common;
>  	u32			offset;
>  	struct completion	completion;
> -	u16			*buffer;
> +	u16			buffer[STM32_ADC_MAX_SQ];
>  	struct clk		*clk;
>  	int			irq;
>  	spinlock_t		lock;		/* interrupt lock */
> +	unsigned int		bufi;
> +	unsigned int		num_conv;
>  };
>  
>  /**
> @@ -126,6 +150,33 @@ struct stm32_adc_chan_spec {
>  };
>  
>  /**
> + * stm32f4_sq - describe regular sequence registers
> + * - L: sequence len (register & bit field)
> + * - SQ1..SQ16: sequence entries (register & bit field)
> + */
> +static const struct stm32_adc_regs stm32f4_sq[STM32_ADC_MAX_SQ + 1] = {
> +	/* L: len bit field description to be kept as first element */
> +	{ STM32F4_ADC_SQR1, GENMASK(23, 20), 20 },
> +	/* SQ1..SQ16 registers & bit fields (reg, mask, shift) */
> +	{ STM32F4_ADC_SQR3, GENMASK(4, 0), 0 },
> +	{ STM32F4_ADC_SQR3, GENMASK(9, 5), 5 },
> +	{ STM32F4_ADC_SQR3, GENMASK(14, 10), 10 },
> +	{ STM32F4_ADC_SQR3, GENMASK(19, 15), 15 },
> +	{ STM32F4_ADC_SQR3, GENMASK(24, 20), 20 },
> +	{ STM32F4_ADC_SQR3, GENMASK(29, 25), 25 },
> +	{ STM32F4_ADC_SQR2, GENMASK(4, 0), 0 },
> +	{ STM32F4_ADC_SQR2, GENMASK(9, 5), 5 },
> +	{ STM32F4_ADC_SQR2, GENMASK(14, 10), 10 },
> +	{ STM32F4_ADC_SQR2, GENMASK(19, 15), 15 },
> +	{ STM32F4_ADC_SQR2, GENMASK(24, 20), 20 },
> +	{ STM32F4_ADC_SQR2, GENMASK(29, 25), 25 },
> +	{ STM32F4_ADC_SQR1, GENMASK(4, 0), 0 },
> +	{ STM32F4_ADC_SQR1, GENMASK(9, 5), 5 },
> +	{ STM32F4_ADC_SQR1, GENMASK(14, 10), 10 },
> +	{ STM32F4_ADC_SQR1, GENMASK(19, 15), 15 },
> +};
> +
> +/**
>   * STM32 ADC registers access routines
>   * @adc: stm32 adc instance
>   * @reg: reg offset in adc instance
> @@ -211,6 +262,104 @@ static void stm32_adc_stop_conv(struct stm32_adc *adc)
>  }
>  
>  /**
> + * stm32_adc_conf_scan_seq() - Build regular channels scan sequence
> + * @indio_dev: IIO device
> + * @scan_mask: channels to be converted
> + *
> + * Conversion sequence :
> + * Configure ADC scan sequence based on selected channels in scan_mask.
> + * Add channels to SQR registers, from scan_mask LSB to MSB, then
> + * program sequence len.
> + */
> +static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
> +				   const unsigned long *scan_mask)
> +{
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +	const struct iio_chan_spec *chan;
> +	u32 val, bit;
> +	int i = 0;
> +
> +	for_each_set_bit(bit, scan_mask, indio_dev->masklength) {
> +		chan = indio_dev->channels + bit;
> +		/*
> +		 * Assign one channel per SQ entry in regular
> +		 * sequence, starting with SQ1.
> +		 */
> +		i++;
> +		if (i > STM32_ADC_MAX_SQ)
> +			return -EINVAL;
> +
> +		dev_dbg(&indio_dev->dev, "%s chan %d to SQ%d\n",
> +			__func__, chan->channel, i);
> +
> +		val = stm32_adc_readl(adc, stm32f4_sq[i].reg);
> +		val &= ~stm32f4_sq[i].mask;
> +		val |= chan->channel << stm32f4_sq[i].shift;
> +		stm32_adc_writel(adc, stm32f4_sq[i].reg, val);
> +	}
> +
> +	if (!i)
> +		return -EINVAL;
> +
> +	/* Sequence len */
> +	val = stm32_adc_readl(adc, stm32f4_sq[0].reg);
> +	val &= ~stm32f4_sq[0].mask;
> +	val |= ((i - 1) << stm32f4_sq[0].shift);
> +	stm32_adc_writel(adc, stm32f4_sq[0].reg, val);
> +
> +	return 0;
> +}
> +
> +/**
> + * stm32_adc_get_trig_extsel() - Get external trigger selection
> + * @trig: trigger
> + *
> + * Returns trigger extsel value, if trig matches, -EINVAL otherwise.
> + */
> +static int stm32_adc_get_trig_extsel(struct iio_trigger *trig)
> +{
> +	return -EINVAL;
> +}
> +
> +/**
> + * stm32_adc_set_trig() - Set a regular trigger
> + * @indio_dev: IIO device
> + * @trig: IIO trigger
> + *
> + * Set trigger source/polarity (e.g. SW, or HW with polarity) :
> + * - if HW trigger disabled (e.g. trig == NULL, conversion launched by sw)
> + * - if HW trigger enabled, set source & polarity
> + */
> +static int stm32_adc_set_trig(struct iio_dev *indio_dev,
> +			      struct iio_trigger *trig)
> +{
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +	u32 val, extsel = 0, exten = STM32_EXTEN_SWTRIG;
> +	unsigned long flags;
> +	int ret;
> +
> +	if (trig) {
> +		ret = stm32_adc_get_trig_extsel(trig);
> +		if (ret < 0)
> +			return ret;
> +
> +		/* set trigger source and polarity (default to rising edge) */
> +		extsel = ret;
> +		exten = STM32_EXTEN_HWTRIG_RISING_EDGE;
> +	}
> +
> +	spin_lock_irqsave(&adc->lock, flags);
> +	val = stm32_adc_readl(adc, STM32F4_ADC_CR2);
> +	val &= ~(STM32F4_EXTEN_MASK | STM32F4_EXTSEL_MASK);
> +	val |= exten << STM32F4_EXTEN_SHIFT;
> +	val |= extsel << STM32F4_EXTSEL_SHIFT;
> +	stm32_adc_writel(adc, STM32F4_ADC_CR2, val);
> +	spin_unlock_irqrestore(&adc->lock, flags);
> +
> +	return 0;
> +}
> +
> +/**
>   * stm32_adc_single_conv() - Performs a single conversion
>   * @indio_dev: IIO device
>   * @chan: IIO channel
> @@ -228,21 +377,20 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
>  	struct stm32_adc *adc = iio_priv(indio_dev);
>  	long timeout;
>  	u32 val;
> -	u16 result;
>  	int ret;
>  
>  	reinit_completion(&adc->completion);
>  
> -	adc->buffer = &result;
> +	adc->bufi = 0;
>  
> -	/* Program chan number in regular sequence */
> -	val = stm32_adc_readl(adc, STM32F4_ADC_SQR3);
> -	val &= ~STM32F4_SQ1_MASK;
> -	val |= chan->channel << STM32F4_SQ1_SHIFT;
> -	stm32_adc_writel(adc, STM32F4_ADC_SQR3, val);
> +	/* Program chan number in regular sequence (SQ1) */
> +	val = stm32_adc_readl(adc, stm32f4_sq[1].reg);
> +	val &= ~stm32f4_sq[1].mask;
> +	val |= chan->channel << stm32f4_sq[1].shift;
> +	stm32_adc_writel(adc, stm32f4_sq[1].reg, val);
>  
>  	/* Set regular sequence len (0 for 1 conversion) */
> -	stm32_adc_clr_bits(adc, STM32F4_ADC_SQR1, STM32F4_L_MASK);
> +	stm32_adc_clr_bits(adc, stm32f4_sq[0].reg, stm32f4_sq[0].mask);
>  
>  	/* Trigger detection disabled (conversion can be launched in SW) */
>  	stm32_adc_clr_bits(adc, STM32F4_ADC_CR2, STM32F4_EXTEN_MASK);
> @@ -258,7 +406,7 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
>  	} else if (timeout < 0) {
>  		ret = timeout;
>  	} else {
> -		*res = result;
> +		*res = adc->buffer[0];
>  		ret = IIO_VAL_INT;
>  	}
>  
> @@ -301,17 +449,56 @@ static int stm32_adc_read_raw(struct iio_dev *indio_dev,
>  static irqreturn_t stm32_adc_isr(int irq, void *data)
>  {
>  	struct stm32_adc *adc = data;
> +	struct iio_dev *indio_dev = iio_priv_to_dev(adc);
>  	u32 status = stm32_adc_readl(adc, STM32F4_ADC_SR);
>  
>  	if (status & STM32F4_EOC) {
> -		*adc->buffer = stm32_adc_readw(adc, STM32F4_ADC_DR);
> -		complete(&adc->completion);
> +		/* Reading DR also clears EOC status flag */
> +		adc->buffer[adc->bufi] = stm32_adc_readw(adc, STM32F4_ADC_DR);
> +		if (iio_buffer_enabled(indio_dev)) {
> +			adc->bufi++;
> +			if (adc->bufi >= adc->num_conv) {
> +				stm32_adc_conv_irq_disable(adc);
> +				iio_trigger_poll(indio_dev->trig);
> +			}
> +		} else {
> +			complete(&adc->completion);
> +		}
>  		return IRQ_HANDLED;
>  	}
>  
>  	return IRQ_NONE;
>  }
>  
> +/**
> + * stm32_adc_validate_trigger() - validate trigger for stm32 adc
> + * @indio_dev: IIO device
> + * @trig: new trigger
> + *
> + * Returns: 0 if trig matches one of the triggers registered by stm32 adc
> + * driver, -EINVAL otherwise.
> + */
> +static int stm32_adc_validate_trigger(struct iio_dev *indio_dev,
> +				      struct iio_trigger *trig)
> +{
> +	return stm32_adc_get_trig_extsel(trig) < 0 ? -EINVAL : 0;
> +}
> +
> +static int stm32_adc_update_scan_mode(struct iio_dev *indio_dev,
> +				      const unsigned long *scan_mask)
> +{
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +	int ret;
> +
> +	adc->num_conv = bitmap_weight(scan_mask, indio_dev->masklength);
> +
> +	ret = stm32_adc_conf_scan_seq(indio_dev, scan_mask);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
>  static int stm32_adc_of_xlate(struct iio_dev *indio_dev,
>  			      const struct of_phandle_args *iiospec)
>  {
> @@ -350,11 +537,86 @@ static int stm32_adc_debugfs_reg_access(struct iio_dev *indio_dev,
>  
>  static const struct iio_info stm32_adc_iio_info = {
>  	.read_raw = stm32_adc_read_raw,
> +	.validate_trigger = stm32_adc_validate_trigger,
> +	.update_scan_mode = stm32_adc_update_scan_mode,
>  	.debugfs_reg_access = stm32_adc_debugfs_reg_access,
>  	.of_xlate = stm32_adc_of_xlate,
>  	.driver_module = THIS_MODULE,
>  };
>  
> +static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
> +{
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = stm32_adc_set_trig(indio_dev, indio_dev->trig);
> +	if (ret) {
> +		dev_err(&indio_dev->dev, "Can't set trigger\n");
> +		return ret;
> +	}
> +
> +	ret = iio_triggered_buffer_postenable(indio_dev);
> +	if (ret < 0)
> +		goto err_clr_trig;
> +
> +	/* Reset adc buffer index */
> +	adc->bufi = 0;
> +
> +	stm32_adc_conv_irq_enable(adc);
> +	stm32_adc_start_conv(adc);
> +
> +	return 0;
> +
> +err_clr_trig:
> +	stm32_adc_set_trig(indio_dev, NULL);
> +
> +	return ret;
> +}
> +
> +static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev)
> +{
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +	int ret;
> +
> +	stm32_adc_stop_conv(adc);
> +	stm32_adc_conv_irq_disable(adc);
> +
> +	ret = iio_triggered_buffer_predisable(indio_dev);
> +	if (ret < 0)
> +		dev_err(&indio_dev->dev, "predisable failed\n");
> +
> +	if (stm32_adc_set_trig(indio_dev, NULL))
> +		dev_err(&indio_dev->dev, "Can't clear trigger\n");
> +
> +	return ret;
> +}
> +
> +static const struct iio_buffer_setup_ops stm32_adc_buffer_setup_ops = {
> +	.postenable = &stm32_adc_buffer_postenable,
> +	.predisable = &stm32_adc_buffer_predisable,
> +};
> +
> +static irqreturn_t stm32_adc_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +
> +	dev_dbg(&indio_dev->dev, "%s bufi=%d\n", __func__, adc->bufi);
> +
> +	/* reset buffer index */
> +	adc->bufi = 0;
> +	iio_push_to_buffers_with_timestamp(indio_dev, adc->buffer,
> +					   pf->timestamp);
> +
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	/* re-enable eoc irq */
> +	stm32_adc_conv_irq_enable(adc);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>  				    struct iio_chan_spec *chan,
>  				    const struct stm32_adc_chan_spec *channel,
> @@ -471,14 +733,26 @@ static int stm32_adc_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto err_clk_disable;
>  
> +	ret = iio_triggered_buffer_setup(indio_dev,
> +					 &iio_pollfunc_store_time,
> +					 &stm32_adc_trigger_handler,
> +					 &stm32_adc_buffer_setup_ops);
> +	if (ret) {
> +		dev_err(&pdev->dev, "buffer setup failed\n");
> +		goto err_clk_disable;
> +	}
> +
>  	ret = iio_device_register(indio_dev);
>  	if (ret) {
>  		dev_err(&pdev->dev, "iio dev register failed\n");
> -		goto err_clk_disable;
> +		goto err_buffer_cleanup;
>  	}
>  
>  	return 0;
>  
> +err_buffer_cleanup:
> +	iio_triggered_buffer_cleanup(indio_dev);
> +
>  err_clk_disable:
>  	clk_disable_unprepare(adc->clk);
>  
> @@ -491,6 +765,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
>  	struct iio_dev *indio_dev = iio_priv_to_dev(adc);
>  
>  	iio_device_unregister(indio_dev);
> +	iio_triggered_buffer_cleanup(indio_dev);
>  	clk_disable_unprepare(adc->clk);
>  
>  	return 0;
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/7] iio: adc: stm32: Enable use of stm32 timer triggers
  2017-01-26 14:28 ` [PATCH v2 2/7] iio: adc: stm32: Enable use of stm32 timer triggers Fabrice Gasnier
@ 2017-01-28 18:37   ` Jonathan Cameron
  0 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2017-01-28 18:37 UTC (permalink / raw)
  To: Fabrice Gasnier, linux, robh+dt, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: linux-iio, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
	knaack.h, pmeerw, benjamin.gaignard, benjamin.gaignard

On 26/01/17 14:28, Fabrice Gasnier wrote:
> STM32 ADC has external timer trigger sources. Use stm32 timer triggers
> API (e.g. is_stm32_timer_trigger()) with local ADC lookup table to
> validate a trigger can be used.
> This also provides correct trigger selection value (e.g. extsel).
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Applied to the togreg branch of iio.git.

Thanks,

Jonathan
> ---
> Changes in v2:
> - Add comment on dual check to validate trigger (and get extsel)
> ---
>  drivers/iio/adc/Kconfig     |  2 ++
>  drivers/iio/adc/stm32-adc.c | 65 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 67 insertions(+)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 33341f4..9a7b090 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -447,6 +447,8 @@ config STM32_ADC_CORE
>  	depends on OF
>  	depends on REGULATOR
>  	select IIO_BUFFER
> +	select MFD_STM32_TIMERS
> +	select IIO_STM32_TIMER_TRIGGER
>  	select IIO_TRIGGERED_BUFFER
>  	help
>  	  Select this option to enable the core driver for STMicroelectronics
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 1e382b6..87d984b 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -23,6 +23,7 @@
>  #include <linux/delay.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/buffer.h>
> +#include <linux/iio/timer/stm32-timer-trigger.h>
>  #include <linux/iio/trigger.h>
>  #include <linux/iio/trigger_consumer.h>
>  #include <linux/iio/triggered_buffer.h>
> @@ -81,6 +82,36 @@ enum stm32_adc_exten {
>  	STM32_EXTEN_HWTRIG_BOTH_EDGES,
>  };
>  
> +/* extsel - trigger mux selection value */
> +enum stm32_adc_extsel {
> +	STM32_EXT0,
> +	STM32_EXT1,
> +	STM32_EXT2,
> +	STM32_EXT3,
> +	STM32_EXT4,
> +	STM32_EXT5,
> +	STM32_EXT6,
> +	STM32_EXT7,
> +	STM32_EXT8,
> +	STM32_EXT9,
> +	STM32_EXT10,
> +	STM32_EXT11,
> +	STM32_EXT12,
> +	STM32_EXT13,
> +	STM32_EXT14,
> +	STM32_EXT15,
> +};
> +
> +/**
> + * struct stm32_adc_trig_info - ADC trigger info
> + * @name:		name of the trigger, corresponding to its source
> + * @extsel:		trigger selection
> + */
> +struct stm32_adc_trig_info {
> +	const char *name;
> +	enum stm32_adc_extsel extsel;
> +};
> +
>  /**
>   * stm32_adc_regs - stm32 ADC misc registers & bitfield desc
>   * @reg:		register offset
> @@ -176,6 +207,26 @@ struct stm32_adc_chan_spec {
>  	{ STM32F4_ADC_SQR1, GENMASK(19, 15), 15 },
>  };
>  
> +/* STM32F4 external trigger sources for all instances */
> +static struct stm32_adc_trig_info stm32f4_adc_trigs[] = {
> +	{ TIM1_CH1, STM32_EXT0 },
> +	{ TIM1_CH2, STM32_EXT1 },
> +	{ TIM1_CH3, STM32_EXT2 },
> +	{ TIM2_CH2, STM32_EXT3 },
> +	{ TIM2_CH3, STM32_EXT4 },
> +	{ TIM2_CH4, STM32_EXT5 },
> +	{ TIM2_TRGO, STM32_EXT6 },
> +	{ TIM3_CH1, STM32_EXT7 },
> +	{ TIM3_TRGO, STM32_EXT8 },
> +	{ TIM4_CH4, STM32_EXT9 },
> +	{ TIM5_CH1, STM32_EXT10 },
> +	{ TIM5_CH2, STM32_EXT11 },
> +	{ TIM5_CH3, STM32_EXT12 },
> +	{ TIM8_CH1, STM32_EXT13 },
> +	{ TIM8_TRGO, STM32_EXT14 },
> +	{}, /* sentinel */
> +};
> +
>  /**
>   * STM32 ADC registers access routines
>   * @adc: stm32 adc instance
> @@ -318,6 +369,20 @@ static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
>   */
>  static int stm32_adc_get_trig_extsel(struct iio_trigger *trig)
>  {
> +	int i;
> +
> +	/* lookup triggers registered by stm32 timer trigger driver */
> +	for (i = 0; stm32f4_adc_trigs[i].name; i++) {
> +		/**
> +		 * Checking both stm32 timer trigger type and trig name
> +		 * should be safe against arbitrary trigger names.
> +		 */
> +		if (is_stm32_timer_trigger(trig) &&
> +		    !strcmp(stm32f4_adc_trigs[i].name, trig->name)) {
> +			return stm32f4_adc_trigs[i].extsel;
> +		}
> +	}
> +
>  	return -EINVAL;
>  }
>  
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/7] iio: adc: stm32: add trigger polarity extended attribute
  2017-01-26 14:28 ` [PATCH v2 3/7] iio: adc: stm32: add trigger polarity extended attribute Fabrice Gasnier
@ 2017-01-28 18:39   ` Jonathan Cameron
  0 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2017-01-28 18:39 UTC (permalink / raw)
  To: Fabrice Gasnier, linux, robh+dt, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: linux-iio, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
	knaack.h, pmeerw, benjamin.gaignard, benjamin.gaignard

On 26/01/17 14:28, Fabrice Gasnier wrote:
> Define extended attribute so that user may choose rising, falling or both
> edges for external trigger sources.
> Default to rising edge in case it isn't set.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Applied though as discussed we might want some ability to add
a default for external triggers via dt.

Can be added later.

Pushed out as testing for the autobuilders to play with it.

Thanks,

Jonathan
> ---
> Changes in v2:
> - Rename and document new trigger_polarity custom attribute
> ---
>  Documentation/ABI/testing/sysfs-bus-iio-adc-stm32 | 18 +++++++++
>  drivers/iio/adc/stm32-adc.c                       | 46 ++++++++++++++++++++++-
>  2 files changed, 63 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-stm32
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-adc-stm32
> new file mode 100644
> index 0000000..efe4c85
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-stm32
> @@ -0,0 +1,18 @@
> +What:		/sys/bus/iio/devices/triggerX/trigger_polarity
> +KernelVersion:	4.11
> +Contact:	fabrice.gasnier@st.com
> +Description:
> +		The STM32 ADC can be configured to use external trigger sources
> +		(e.g. timers, pwm or exti gpio). Then, it can be tuned to start
> +		conversions on external trigger by either:
> +		- "rising-edge"
> +		- "falling-edge"
> +		- "both-edges".
> +		Reading returns current trigger polarity.
> +		Writing value before enabling conversions sets trigger polarity.
> +
> +What:		/sys/bus/iio/devices/triggerX/trigger_polarity_available
> +KernelVersion:	4.11
> +Contact:	fabrice.gasnier@st.com
> +Description:
> +		List all available trigger_polarity settings.
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 87d984b..9a38f9a 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -135,6 +135,7 @@ struct stm32_adc_regs {
>   * @lock:		spinlock
>   * @bufi:		data buffer index
>   * @num_conv:		expected number of scan conversions
> + * @trigger_polarity:	external trigger polarity (e.g. exten)
>   */
>  struct stm32_adc {
>  	struct stm32_adc_common	*common;
> @@ -146,6 +147,7 @@ struct stm32_adc {
>  	spinlock_t		lock;		/* interrupt lock */
>  	unsigned int		bufi;
>  	unsigned int		num_conv;
> +	u32			trigger_polarity;
>  };
>  
>  /**
> @@ -410,7 +412,7 @@ static int stm32_adc_set_trig(struct iio_dev *indio_dev,
>  
>  		/* set trigger source and polarity (default to rising edge) */
>  		extsel = ret;
> -		exten = STM32_EXTEN_HWTRIG_RISING_EDGE;
> +		exten = adc->trigger_polarity + STM32_EXTEN_HWTRIG_RISING_EDGE;
>  	}
>  
>  	spin_lock_irqsave(&adc->lock, flags);
> @@ -424,6 +426,36 @@ static int stm32_adc_set_trig(struct iio_dev *indio_dev,
>  	return 0;
>  }
>  
> +static int stm32_adc_set_trig_pol(struct iio_dev *indio_dev,
> +				  const struct iio_chan_spec *chan,
> +				  unsigned int type)
> +{
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +
> +	adc->trigger_polarity = type;
> +
> +	return 0;
> +}
> +
> +static int stm32_adc_get_trig_pol(struct iio_dev *indio_dev,
> +				  const struct iio_chan_spec *chan)
> +{
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +
> +	return adc->trigger_polarity;
> +}
> +
> +static const char * const stm32_trig_pol_items[] = {
> +	"rising-edge", "falling-edge", "both-edges",
> +};
> +
> +const struct iio_enum stm32_adc_trig_pol = {
> +	.items = stm32_trig_pol_items,
> +	.num_items = ARRAY_SIZE(stm32_trig_pol_items),
> +	.get = stm32_adc_get_trig_pol,
> +	.set = stm32_adc_set_trig_pol,
> +};
> +
>  /**
>   * stm32_adc_single_conv() - Performs a single conversion
>   * @indio_dev: IIO device
> @@ -682,6 +714,17 @@ static irqreturn_t stm32_adc_trigger_handler(int irq, void *p)
>  	return IRQ_HANDLED;
>  }
>  
> +static const struct iio_chan_spec_ext_info stm32_adc_ext_info[] = {
> +	IIO_ENUM("trigger_polarity", IIO_SHARED_BY_ALL, &stm32_adc_trig_pol),
> +	{
> +		.name = "trigger_polarity_available",
> +		.shared = IIO_SHARED_BY_ALL,
> +		.read = iio_enum_available_read,
> +		.private = (uintptr_t)&stm32_adc_trig_pol,
> +	},
> +	{},
> +};
> +
>  static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>  				    struct iio_chan_spec *chan,
>  				    const struct stm32_adc_chan_spec *channel,
> @@ -697,6 +740,7 @@ static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>  	chan->scan_type.sign = 'u';
>  	chan->scan_type.realbits = 12;
>  	chan->scan_type.storagebits = 16;
> +	chan->ext_info = stm32_adc_ext_info;
>  }
>  
>  static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/7] Documentation: dt: iio: stm32-adc: optional dma support
  2017-01-26 14:28 ` [PATCH v2 4/7] Documentation: dt: iio: stm32-adc: optional dma support Fabrice Gasnier
@ 2017-01-28 18:39   ` Jonathan Cameron
  0 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2017-01-28 18:39 UTC (permalink / raw)
  To: Fabrice Gasnier, linux, robh+dt, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: linux-iio, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
	knaack.h, pmeerw, benjamin.gaignard, benjamin.gaignard

On 26/01/17 14:28, Fabrice Gasnier wrote:
> STM32 ADC can use dma. Add dt documentation for optional dma support.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Acked-by: Rob Herring <robh@kernel.org>
Applied.
> ---
>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> index 49ed82e..5dfc88e 100644
> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> @@ -53,6 +53,11 @@ Required properties:
>  - #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
>    Documentation/devicetree/bindings/iio/iio-bindings.txt
>  
> +Optional properties:
> +- dmas: Phandle to dma channel for this ADC instance.
> +  See ../../dma/dma.txt for details.
> +- dma-names: Must be "rx" when dmas property is being used.
> +
>  Example:
>  	adc: adc@40012000 {
>  		compatible = "st,stm32f4-adc-core";
> @@ -77,6 +82,8 @@ Example:
>  			interrupt-parent = <&adc>;
>  			interrupts = <0>;
>  			st,adc-channels = <8>;
> +			dmas = <&dma2 0 0 0x400 0x0>;
> +			dma-names = "rx";
>  		};
>  		...
>  		other adc child nodes follow...
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 5/7] iio: adc: stm32: add optional dma support
  2017-01-26 14:28 ` [PATCH v2 5/7] iio: adc: stm32: add " Fabrice Gasnier
@ 2017-01-28 18:40   ` Jonathan Cameron
  2017-01-28 18:41   ` Jonathan Cameron
  1 sibling, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2017-01-28 18:40 UTC (permalink / raw)
  To: Fabrice Gasnier, linux, robh+dt, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: linux-iio, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
	knaack.h, pmeerw, benjamin.gaignard, benjamin.gaignard

On 26/01/17 14:28, Fabrice Gasnier wrote:
> Add DMA optional support to STM32 ADC, as there is a limited number DMA
> channels (request lines) that can be assigned to ADC. This way, driver
> may fall back using interrupts when all DMA channels are in use for
> other IPs.
> Use dma cyclic mode with two periods. Allow to tune period length by
> using watermark. Coherent memory is used for dma (max buffer size is
> fixed to PAGE_SIZE).
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Applied to the togreg branch of iio.git - pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan
> ---
> Changes in v2:
> - Use iio_trigger_poll_chained() avoids to bounce back into irq context.
>   Remove irq_work.
> - Rework dma buffer allocation and use. Allocation moved to probe time,
>   fixed to PAGE_SIZE. Use hwfifo_set_watermark() routine to tune dma
>   cyclic period length.
> ---
>  drivers/iio/adc/Kconfig          |   1 +
>  drivers/iio/adc/stm32-adc-core.c |   1 +
>  drivers/iio/adc/stm32-adc-core.h |   2 +
>  drivers/iio/adc/stm32-adc.c      | 227 ++++++++++++++++++++++++++++++++++++---
>  4 files changed, 218 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 9a7b090..03a73f9 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -444,6 +444,7 @@ config ROCKCHIP_SARADC
>  config STM32_ADC_CORE
>  	tristate "STMicroelectronics STM32 adc core"
>  	depends on ARCH_STM32 || COMPILE_TEST
> +	depends on HAS_DMA
>  	depends on OF
>  	depends on REGULATOR
>  	select IIO_BUFFER
> diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
> index 4214b0c..22b7c93 100644
> --- a/drivers/iio/adc/stm32-adc-core.c
> +++ b/drivers/iio/adc/stm32-adc-core.c
> @@ -201,6 +201,7 @@ static int stm32_adc_probe(struct platform_device *pdev)
>  	priv->common.base = devm_ioremap_resource(&pdev->dev, res);
>  	if (IS_ERR(priv->common.base))
>  		return PTR_ERR(priv->common.base);
> +	priv->common.phys_base = res->start;
>  
>  	priv->vref = devm_regulator_get(&pdev->dev, "vref");
>  	if (IS_ERR(priv->vref)) {
> diff --git a/drivers/iio/adc/stm32-adc-core.h b/drivers/iio/adc/stm32-adc-core.h
> index 081fa5f..2ec7abb 100644
> --- a/drivers/iio/adc/stm32-adc-core.h
> +++ b/drivers/iio/adc/stm32-adc-core.h
> @@ -42,10 +42,12 @@
>  /**
>   * struct stm32_adc_common - stm32 ADC driver common data (for all instances)
>   * @base:		control registers base cpu addr
> + * @phys_base:		control registers base physical addr
>   * @vref_mv:		vref voltage (mv)
>   */
>  struct stm32_adc_common {
>  	void __iomem			*base;
> +	phys_addr_t			phys_base;
>  	int				vref_mv;
>  };
>  
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 9a38f9a..8a30039 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -21,6 +21,8 @@
>  
>  #include <linux/clk.h>
>  #include <linux/delay.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/dmaengine.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/buffer.h>
>  #include <linux/iio/timer/stm32-timer-trigger.h>
> @@ -68,12 +70,16 @@
>  #define STM32F4_EXTSEL_SHIFT		24
>  #define STM32F4_EXTSEL_MASK		GENMASK(27, 24)
>  #define STM32F4_EOCS			BIT(10)
> +#define STM32F4_DDS			BIT(9)
> +#define STM32F4_DMA			BIT(8)
>  #define STM32F4_ADON			BIT(0)
>  
>  #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
>  #define STM32_ADC_TIMEOUT_US		100000
>  #define STM32_ADC_TIMEOUT	(msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
>  
> +#define STM32_DMA_BUFFER_SIZE		PAGE_SIZE
> +
>  /* External trigger enable */
>  enum stm32_adc_exten {
>  	STM32_EXTEN_SWTRIG,
> @@ -136,6 +142,10 @@ struct stm32_adc_regs {
>   * @bufi:		data buffer index
>   * @num_conv:		expected number of scan conversions
>   * @trigger_polarity:	external trigger polarity (e.g. exten)
> + * @dma_chan:		dma channel
> + * @rx_buf:		dma rx buffer cpu address
> + * @rx_dma_buf:		dma rx buffer bus address
> + * @rx_buf_sz:		dma rx buffer size
>   */
>  struct stm32_adc {
>  	struct stm32_adc_common	*common;
> @@ -148,6 +158,10 @@ struct stm32_adc {
>  	unsigned int		bufi;
>  	unsigned int		num_conv;
>  	u32			trigger_polarity;
> +	struct dma_chan		*dma_chan;
> +	u8			*rx_buf;
> +	dma_addr_t		rx_dma_buf;
> +	unsigned int		rx_buf_sz;
>  };
>  
>  /**
> @@ -291,10 +305,21 @@ static void stm32_adc_conv_irq_disable(struct stm32_adc *adc)
>  /**
>   * stm32_adc_start_conv() - Start conversions for regular channels.
>   * @adc: stm32 adc instance
> + * @dma: use dma to transfer conversion result
> + *
> + * Start conversions for regular channels.
> + * Also take care of normal or DMA mode. Circular DMA may be used for regular
> + * conversions, in IIO buffer modes. Otherwise, use ADC interrupt with direct
> + * DR read instead (e.g. read_raw, or triggered buffer mode without DMA).
>   */
> -static void stm32_adc_start_conv(struct stm32_adc *adc)
> +static void stm32_adc_start_conv(struct stm32_adc *adc, bool dma)
>  {
>  	stm32_adc_set_bits(adc, STM32F4_ADC_CR1, STM32F4_SCAN);
> +
> +	if (dma)
> +		stm32_adc_set_bits(adc, STM32F4_ADC_CR2,
> +				   STM32F4_DMA | STM32F4_DDS);
> +
>  	stm32_adc_set_bits(adc, STM32F4_ADC_CR2, STM32F4_EOCS | STM32F4_ADON);
>  
>  	/* Wait for Power-up time (tSTAB from datasheet) */
> @@ -311,7 +336,8 @@ static void stm32_adc_stop_conv(struct stm32_adc *adc)
>  	stm32_adc_clr_bits(adc, STM32F4_ADC_SR, STM32F4_STRT);
>  
>  	stm32_adc_clr_bits(adc, STM32F4_ADC_CR1, STM32F4_SCAN);
> -	stm32_adc_clr_bits(adc, STM32F4_ADC_CR2, STM32F4_ADON);
> +	stm32_adc_clr_bits(adc, STM32F4_ADC_CR2,
> +			   STM32F4_ADON | STM32F4_DMA | STM32F4_DDS);
>  }
>  
>  /**
> @@ -494,7 +520,7 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
>  
>  	stm32_adc_conv_irq_enable(adc);
>  
> -	stm32_adc_start_conv(adc);
> +	stm32_adc_start_conv(adc, false);
>  
>  	timeout = wait_for_completion_interruptible_timeout(
>  					&adc->completion, STM32_ADC_TIMEOUT);
> @@ -581,6 +607,23 @@ static int stm32_adc_validate_trigger(struct iio_dev *indio_dev,
>  	return stm32_adc_get_trig_extsel(trig) < 0 ? -EINVAL : 0;
>  }
>  
> +static int stm32_adc_set_watermark(struct iio_dev *indio_dev, unsigned val)
> +{
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +	unsigned int watermark = STM32_DMA_BUFFER_SIZE / 2;
> +
> +	/*
> +	 * dma cyclic transfers are used, buffer is split into two periods.
> +	 * There should be :
> +	 * - always one buffer (period) dma is working on
> +	 * - one buffer (period) driver can push with iio_trigger_poll().
> +	 */
> +	watermark = min(watermark, val * sizeof(u16));
> +	adc->rx_buf_sz = watermark * 2;
> +
> +	return 0;
> +}
> +
>  static int stm32_adc_update_scan_mode(struct iio_dev *indio_dev,
>  				      const unsigned long *scan_mask)
>  {
> @@ -635,12 +678,83 @@ static int stm32_adc_debugfs_reg_access(struct iio_dev *indio_dev,
>  static const struct iio_info stm32_adc_iio_info = {
>  	.read_raw = stm32_adc_read_raw,
>  	.validate_trigger = stm32_adc_validate_trigger,
> +	.hwfifo_set_watermark = stm32_adc_set_watermark,
>  	.update_scan_mode = stm32_adc_update_scan_mode,
>  	.debugfs_reg_access = stm32_adc_debugfs_reg_access,
>  	.of_xlate = stm32_adc_of_xlate,
>  	.driver_module = THIS_MODULE,
>  };
>  
> +static unsigned int stm32_adc_dma_residue(struct stm32_adc *adc)
> +{
> +	struct dma_tx_state state;
> +	enum dma_status status;
> +
> +	status = dmaengine_tx_status(adc->dma_chan,
> +				     adc->dma_chan->cookie,
> +				     &state);
> +	if (status == DMA_IN_PROGRESS) {
> +		/* Residue is size in bytes from end of buffer */
> +		unsigned int i = adc->rx_buf_sz - state.residue;
> +		unsigned int size;
> +
> +		/* Return available bytes */
> +		if (i >= adc->bufi)
> +			size = i - adc->bufi;
> +		else
> +			size = adc->rx_buf_sz + i - adc->bufi;
> +
> +		return size;
> +	}
> +
> +	return 0;
> +}
> +
> +static void stm32_adc_dma_buffer_done(void *data)
> +{
> +	struct iio_dev *indio_dev = data;
> +
> +	iio_trigger_poll_chained(indio_dev->trig);
> +}
> +
> +static int stm32_adc_dma_start(struct iio_dev *indio_dev)
> +{
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +	struct dma_async_tx_descriptor *desc;
> +	dma_cookie_t cookie;
> +	int ret;
> +
> +	if (!adc->dma_chan)
> +		return 0;
> +
> +	dev_dbg(&indio_dev->dev, "%s size=%d watermark=%d\n", __func__,
> +		adc->rx_buf_sz, adc->rx_buf_sz / 2);
> +
> +	/* Prepare a DMA cyclic transaction */
> +	desc = dmaengine_prep_dma_cyclic(adc->dma_chan,
> +					 adc->rx_dma_buf,
> +					 adc->rx_buf_sz, adc->rx_buf_sz / 2,
> +					 DMA_DEV_TO_MEM,
> +					 DMA_PREP_INTERRUPT);
> +	if (!desc)
> +		return -EBUSY;
> +
> +	desc->callback = stm32_adc_dma_buffer_done;
> +	desc->callback_param = indio_dev;
> +
> +	cookie = dmaengine_submit(desc);
> +	ret = dma_submit_error(cookie);
> +	if (ret) {
> +		dmaengine_terminate_all(adc->dma_chan);
> +		return ret;
> +	}
> +
> +	/* Issue pending DMA requests */
> +	dma_async_issue_pending(adc->dma_chan);
> +
> +	return 0;
> +}
> +
>  static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
>  {
>  	struct stm32_adc *adc = iio_priv(indio_dev);
> @@ -652,18 +766,29 @@ static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
>  		return ret;
>  	}
>  
> +	ret = stm32_adc_dma_start(indio_dev);
> +	if (ret) {
> +		dev_err(&indio_dev->dev, "Can't start dma\n");
> +		goto err_clr_trig;
> +	}
> +
>  	ret = iio_triggered_buffer_postenable(indio_dev);
>  	if (ret < 0)
> -		goto err_clr_trig;
> +		goto err_stop_dma;
>  
>  	/* Reset adc buffer index */
>  	adc->bufi = 0;
>  
> -	stm32_adc_conv_irq_enable(adc);
> -	stm32_adc_start_conv(adc);
> +	if (!adc->dma_chan)
> +		stm32_adc_conv_irq_enable(adc);
> +
> +	stm32_adc_start_conv(adc, !!adc->dma_chan);
>  
>  	return 0;
>  
> +err_stop_dma:
> +	if (adc->dma_chan)
> +		dmaengine_terminate_all(adc->dma_chan);
>  err_clr_trig:
>  	stm32_adc_set_trig(indio_dev, NULL);
>  
> @@ -676,12 +801,16 @@ static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev)
>  	int ret;
>  
>  	stm32_adc_stop_conv(adc);
> -	stm32_adc_conv_irq_disable(adc);
> +	if (!adc->dma_chan)
> +		stm32_adc_conv_irq_disable(adc);
>  
>  	ret = iio_triggered_buffer_predisable(indio_dev);
>  	if (ret < 0)
>  		dev_err(&indio_dev->dev, "predisable failed\n");
>  
> +	if (adc->dma_chan)
> +		dmaengine_terminate_all(adc->dma_chan);
> +
>  	if (stm32_adc_set_trig(indio_dev, NULL))
>  		dev_err(&indio_dev->dev, "Can't clear trigger\n");
>  
> @@ -701,15 +830,31 @@ static irqreturn_t stm32_adc_trigger_handler(int irq, void *p)
>  
>  	dev_dbg(&indio_dev->dev, "%s bufi=%d\n", __func__, adc->bufi);
>  
> -	/* reset buffer index */
> -	adc->bufi = 0;
> -	iio_push_to_buffers_with_timestamp(indio_dev, adc->buffer,
> -					   pf->timestamp);
> +	if (!adc->dma_chan) {
> +		/* reset buffer index */
> +		adc->bufi = 0;
> +		iio_push_to_buffers_with_timestamp(indio_dev, adc->buffer,
> +						   pf->timestamp);
> +	} else {
> +		int residue = stm32_adc_dma_residue(adc);
> +
> +		while (residue >= indio_dev->scan_bytes) {
> +			u16 *buffer = (u16 *)&adc->rx_buf[adc->bufi];
> +
> +			iio_push_to_buffers_with_timestamp(indio_dev, buffer,
> +							   pf->timestamp);
> +			residue -= indio_dev->scan_bytes;
> +			adc->bufi += indio_dev->scan_bytes;
> +			if (adc->bufi >= adc->rx_buf_sz)
> +				adc->bufi = 0;
> +		}
> +	}
>  
>  	iio_trigger_notify_done(indio_dev->trig);
>  
>  	/* re-enable eoc irq */
> -	stm32_adc_conv_irq_enable(adc);
> +	if (!adc->dma_chan)
> +		stm32_adc_conv_irq_enable(adc);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -781,6 +926,45 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>  	return 0;
>  }
>  
> +static int stm32_adc_dma_request(struct iio_dev *indio_dev)
> +{
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +	struct dma_slave_config config;
> +	int ret;
> +
> +	adc->dma_chan = dma_request_slave_channel(&indio_dev->dev, "rx");
> +	if (!adc->dma_chan)
> +		return 0;
> +
> +	adc->rx_buf = dma_alloc_coherent(adc->dma_chan->device->dev,
> +					 STM32_DMA_BUFFER_SIZE,
> +					 &adc->rx_dma_buf, GFP_KERNEL);
> +	if (!adc->rx_buf) {
> +		goto err_release;
> +		ret = -ENOMEM;
> +	}
> +
> +	/* Configure DMA channel to read data register */
> +	memset(&config, 0, sizeof(config));
> +	config.src_addr = (dma_addr_t)adc->common->phys_base;
> +	config.src_addr += adc->offset + STM32F4_ADC_DR;
> +	config.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> +
> +	ret = dmaengine_slave_config(adc->dma_chan, &config);
> +	if (ret)
> +		goto err_free;
> +
> +	return 0;
> +
> +err_free:
> +	dma_free_coherent(adc->dma_chan->device->dev, STM32_DMA_BUFFER_SIZE,
> +			  adc->rx_buf, adc->rx_dma_buf);
> +err_release:
> +	dma_release_channel(adc->dma_chan);
> +
> +	return ret;
> +}
> +
>  static int stm32_adc_probe(struct platform_device *pdev)
>  {
>  	struct iio_dev *indio_dev;
> @@ -842,13 +1026,17 @@ static int stm32_adc_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto err_clk_disable;
>  
> +	ret = stm32_adc_dma_request(indio_dev);
> +	if (ret < 0)
> +		goto err_clk_disable;
> +
>  	ret = iio_triggered_buffer_setup(indio_dev,
>  					 &iio_pollfunc_store_time,
>  					 &stm32_adc_trigger_handler,
>  					 &stm32_adc_buffer_setup_ops);
>  	if (ret) {
>  		dev_err(&pdev->dev, "buffer setup failed\n");
> -		goto err_clk_disable;
> +		goto err_dma_disable;
>  	}
>  
>  	ret = iio_device_register(indio_dev);
> @@ -862,6 +1050,13 @@ static int stm32_adc_probe(struct platform_device *pdev)
>  err_buffer_cleanup:
>  	iio_triggered_buffer_cleanup(indio_dev);
>  
> +err_dma_disable:
> +	if (adc->dma_chan) {
> +		dma_free_coherent(adc->dma_chan->device->dev,
> +				  STM32_DMA_BUFFER_SIZE,
> +				  adc->rx_buf, adc->rx_dma_buf);
> +		dma_release_channel(adc->dma_chan);
> +	}
>  err_clk_disable:
>  	clk_disable_unprepare(adc->clk);
>  
> @@ -875,6 +1070,12 @@ static int stm32_adc_remove(struct platform_device *pdev)
>  
>  	iio_device_unregister(indio_dev);
>  	iio_triggered_buffer_cleanup(indio_dev);
> +	if (adc->dma_chan) {
> +		dma_free_coherent(adc->dma_chan->device->dev,
> +				  STM32_DMA_BUFFER_SIZE,
> +				  adc->rx_buf, adc->rx_dma_buf);
> +		dma_release_channel(adc->dma_chan);
> +	}
>  	clk_disable_unprepare(adc->clk);
>  
>  	return 0;
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 5/7] iio: adc: stm32: add optional dma support
  2017-01-26 14:28 ` [PATCH v2 5/7] iio: adc: stm32: add " Fabrice Gasnier
  2017-01-28 18:40   ` Jonathan Cameron
@ 2017-01-28 18:41   ` Jonathan Cameron
  2017-01-30  8:57     ` Fabrice Gasnier
  1 sibling, 1 reply; 16+ messages in thread
From: Jonathan Cameron @ 2017-01-28 18:41 UTC (permalink / raw)
  To: Fabrice Gasnier, linux, robh+dt, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: linux-iio, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
	knaack.h, pmeerw, benjamin.gaignard, benjamin.gaignard

On 26/01/17 14:28, Fabrice Gasnier wrote:
> Add DMA optional support to STM32 ADC, as there is a limited number DMA
> channels (request lines) that can be assigned to ADC. This way, driver
> may fall back using interrupts when all DMA channels are in use for
> other IPs.
> Use dma cyclic mode with two periods. Allow to tune period length by
> using watermark. Coherent memory is used for dma (max buffer size is
> fixed to PAGE_SIZE).
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
I am happy with this whole series, except this patch gives me:

drivers/iio/adc/stm32-adc.c:478:23: warning: symbol 'stm32_adc_trig_pol' was not declared. Should it be static?
drivers/iio/adc/stm32-adc.c:621:21: error: incompatible types in comparison expression (different type sizes)
  CC [M]  drivers/iio/adc/stm32-adc.o
In file included from ./include/linux/clk.h:16:0,
                 from drivers/iio/adc/stm32-adc.c:22:
drivers/iio/adc/stm32-adc.c: In function ‘stm32_adc_set_watermark’:
./include/linux/kernel.h:753:16: warning: comparison of distinct pointer types lacks a cast
  (void) (&min1 == &min2);   \
                ^
./include/linux/kernel.h:756:2: note: in expansion of macro ‘__min’
  __min(typeof(x), typeof(y),   \
  ^~~~~
drivers/iio/adc/stm32-adc.c:621:14: note: in expansion of macro ‘min’
  watermark = min(watermark, val * sizeof(u16));
              ^~~

The static is obviously fine so I've added that.
The second looks to be because sizeof(u16) is a size_t which is signed IIRC.
Anyhow, a cast of that to unsigned should I think be harmless and fixes the
warning.

Please check I did these right.

They are in the testing branch of iio.git.

Thanks,

Jonathan

> ---
> Changes in v2:
> - Use iio_trigger_poll_chained() avoids to bounce back into irq context.
>   Remove irq_work.
> - Rework dma buffer allocation and use. Allocation moved to probe time,
>   fixed to PAGE_SIZE. Use hwfifo_set_watermark() routine to tune dma
>   cyclic period length.
> ---
>  drivers/iio/adc/Kconfig          |   1 +
>  drivers/iio/adc/stm32-adc-core.c |   1 +
>  drivers/iio/adc/stm32-adc-core.h |   2 +
>  drivers/iio/adc/stm32-adc.c      | 227 ++++++++++++++++++++++++++++++++++++---
>  4 files changed, 218 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 9a7b090..03a73f9 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -444,6 +444,7 @@ config ROCKCHIP_SARADC
>  config STM32_ADC_CORE
>  	tristate "STMicroelectronics STM32 adc core"
>  	depends on ARCH_STM32 || COMPILE_TEST
> +	depends on HAS_DMA
>  	depends on OF
>  	depends on REGULATOR
>  	select IIO_BUFFER
> diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
> index 4214b0c..22b7c93 100644
> --- a/drivers/iio/adc/stm32-adc-core.c
> +++ b/drivers/iio/adc/stm32-adc-core.c
> @@ -201,6 +201,7 @@ static int stm32_adc_probe(struct platform_device *pdev)
>  	priv->common.base = devm_ioremap_resource(&pdev->dev, res);
>  	if (IS_ERR(priv->common.base))
>  		return PTR_ERR(priv->common.base);
> +	priv->common.phys_base = res->start;
>  
>  	priv->vref = devm_regulator_get(&pdev->dev, "vref");
>  	if (IS_ERR(priv->vref)) {
> diff --git a/drivers/iio/adc/stm32-adc-core.h b/drivers/iio/adc/stm32-adc-core.h
> index 081fa5f..2ec7abb 100644
> --- a/drivers/iio/adc/stm32-adc-core.h
> +++ b/drivers/iio/adc/stm32-adc-core.h
> @@ -42,10 +42,12 @@
>  /**
>   * struct stm32_adc_common - stm32 ADC driver common data (for all instances)
>   * @base:		control registers base cpu addr
> + * @phys_base:		control registers base physical addr
>   * @vref_mv:		vref voltage (mv)
>   */
>  struct stm32_adc_common {
>  	void __iomem			*base;
> +	phys_addr_t			phys_base;
>  	int				vref_mv;
>  };
>  
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 9a38f9a..8a30039 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -21,6 +21,8 @@
>  
>  #include <linux/clk.h>
>  #include <linux/delay.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/dmaengine.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/buffer.h>
>  #include <linux/iio/timer/stm32-timer-trigger.h>
> @@ -68,12 +70,16 @@
>  #define STM32F4_EXTSEL_SHIFT		24
>  #define STM32F4_EXTSEL_MASK		GENMASK(27, 24)
>  #define STM32F4_EOCS			BIT(10)
> +#define STM32F4_DDS			BIT(9)
> +#define STM32F4_DMA			BIT(8)
>  #define STM32F4_ADON			BIT(0)
>  
>  #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
>  #define STM32_ADC_TIMEOUT_US		100000
>  #define STM32_ADC_TIMEOUT	(msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
>  
> +#define STM32_DMA_BUFFER_SIZE		PAGE_SIZE
> +
>  /* External trigger enable */
>  enum stm32_adc_exten {
>  	STM32_EXTEN_SWTRIG,
> @@ -136,6 +142,10 @@ struct stm32_adc_regs {
>   * @bufi:		data buffer index
>   * @num_conv:		expected number of scan conversions
>   * @trigger_polarity:	external trigger polarity (e.g. exten)
> + * @dma_chan:		dma channel
> + * @rx_buf:		dma rx buffer cpu address
> + * @rx_dma_buf:		dma rx buffer bus address
> + * @rx_buf_sz:		dma rx buffer size
>   */
>  struct stm32_adc {
>  	struct stm32_adc_common	*common;
> @@ -148,6 +158,10 @@ struct stm32_adc {
>  	unsigned int		bufi;
>  	unsigned int		num_conv;
>  	u32			trigger_polarity;
> +	struct dma_chan		*dma_chan;
> +	u8			*rx_buf;
> +	dma_addr_t		rx_dma_buf;
> +	unsigned int		rx_buf_sz;
>  };
>  
>  /**
> @@ -291,10 +305,21 @@ static void stm32_adc_conv_irq_disable(struct stm32_adc *adc)
>  /**
>   * stm32_adc_start_conv() - Start conversions for regular channels.
>   * @adc: stm32 adc instance
> + * @dma: use dma to transfer conversion result
> + *
> + * Start conversions for regular channels.
> + * Also take care of normal or DMA mode. Circular DMA may be used for regular
> + * conversions, in IIO buffer modes. Otherwise, use ADC interrupt with direct
> + * DR read instead (e.g. read_raw, or triggered buffer mode without DMA).
>   */
> -static void stm32_adc_start_conv(struct stm32_adc *adc)
> +static void stm32_adc_start_conv(struct stm32_adc *adc, bool dma)
>  {
>  	stm32_adc_set_bits(adc, STM32F4_ADC_CR1, STM32F4_SCAN);
> +
> +	if (dma)
> +		stm32_adc_set_bits(adc, STM32F4_ADC_CR2,
> +				   STM32F4_DMA | STM32F4_DDS);
> +
>  	stm32_adc_set_bits(adc, STM32F4_ADC_CR2, STM32F4_EOCS | STM32F4_ADON);
>  
>  	/* Wait for Power-up time (tSTAB from datasheet) */
> @@ -311,7 +336,8 @@ static void stm32_adc_stop_conv(struct stm32_adc *adc)
>  	stm32_adc_clr_bits(adc, STM32F4_ADC_SR, STM32F4_STRT);
>  
>  	stm32_adc_clr_bits(adc, STM32F4_ADC_CR1, STM32F4_SCAN);
> -	stm32_adc_clr_bits(adc, STM32F4_ADC_CR2, STM32F4_ADON);
> +	stm32_adc_clr_bits(adc, STM32F4_ADC_CR2,
> +			   STM32F4_ADON | STM32F4_DMA | STM32F4_DDS);
>  }
>  
>  /**
> @@ -494,7 +520,7 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
>  
>  	stm32_adc_conv_irq_enable(adc);
>  
> -	stm32_adc_start_conv(adc);
> +	stm32_adc_start_conv(adc, false);
>  
>  	timeout = wait_for_completion_interruptible_timeout(
>  					&adc->completion, STM32_ADC_TIMEOUT);
> @@ -581,6 +607,23 @@ static int stm32_adc_validate_trigger(struct iio_dev *indio_dev,
>  	return stm32_adc_get_trig_extsel(trig) < 0 ? -EINVAL : 0;
>  }
>  
> +static int stm32_adc_set_watermark(struct iio_dev *indio_dev, unsigned val)
> +{
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +	unsigned int watermark = STM32_DMA_BUFFER_SIZE / 2;
> +
> +	/*
> +	 * dma cyclic transfers are used, buffer is split into two periods.
> +	 * There should be :
> +	 * - always one buffer (period) dma is working on
> +	 * - one buffer (period) driver can push with iio_trigger_poll().
> +	 */
> +	watermark = min(watermark, val * sizeof(u16));
> +	adc->rx_buf_sz = watermark * 2;
> +
> +	return 0;
> +}
> +
>  static int stm32_adc_update_scan_mode(struct iio_dev *indio_dev,
>  				      const unsigned long *scan_mask)
>  {
> @@ -635,12 +678,83 @@ static int stm32_adc_debugfs_reg_access(struct iio_dev *indio_dev,
>  static const struct iio_info stm32_adc_iio_info = {
>  	.read_raw = stm32_adc_read_raw,
>  	.validate_trigger = stm32_adc_validate_trigger,
> +	.hwfifo_set_watermark = stm32_adc_set_watermark,
>  	.update_scan_mode = stm32_adc_update_scan_mode,
>  	.debugfs_reg_access = stm32_adc_debugfs_reg_access,
>  	.of_xlate = stm32_adc_of_xlate,
>  	.driver_module = THIS_MODULE,
>  };
>  
> +static unsigned int stm32_adc_dma_residue(struct stm32_adc *adc)
> +{
> +	struct dma_tx_state state;
> +	enum dma_status status;
> +
> +	status = dmaengine_tx_status(adc->dma_chan,
> +				     adc->dma_chan->cookie,
> +				     &state);
> +	if (status == DMA_IN_PROGRESS) {
> +		/* Residue is size in bytes from end of buffer */
> +		unsigned int i = adc->rx_buf_sz - state.residue;
> +		unsigned int size;
> +
> +		/* Return available bytes */
> +		if (i >= adc->bufi)
> +			size = i - adc->bufi;
> +		else
> +			size = adc->rx_buf_sz + i - adc->bufi;
> +
> +		return size;
> +	}
> +
> +	return 0;
> +}
> +
> +static void stm32_adc_dma_buffer_done(void *data)
> +{
> +	struct iio_dev *indio_dev = data;
> +
> +	iio_trigger_poll_chained(indio_dev->trig);
> +}
> +
> +static int stm32_adc_dma_start(struct iio_dev *indio_dev)
> +{
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +	struct dma_async_tx_descriptor *desc;
> +	dma_cookie_t cookie;
> +	int ret;
> +
> +	if (!adc->dma_chan)
> +		return 0;
> +
> +	dev_dbg(&indio_dev->dev, "%s size=%d watermark=%d\n", __func__,
> +		adc->rx_buf_sz, adc->rx_buf_sz / 2);
> +
> +	/* Prepare a DMA cyclic transaction */
> +	desc = dmaengine_prep_dma_cyclic(adc->dma_chan,
> +					 adc->rx_dma_buf,
> +					 adc->rx_buf_sz, adc->rx_buf_sz / 2,
> +					 DMA_DEV_TO_MEM,
> +					 DMA_PREP_INTERRUPT);
> +	if (!desc)
> +		return -EBUSY;
> +
> +	desc->callback = stm32_adc_dma_buffer_done;
> +	desc->callback_param = indio_dev;
> +
> +	cookie = dmaengine_submit(desc);
> +	ret = dma_submit_error(cookie);
> +	if (ret) {
> +		dmaengine_terminate_all(adc->dma_chan);
> +		return ret;
> +	}
> +
> +	/* Issue pending DMA requests */
> +	dma_async_issue_pending(adc->dma_chan);
> +
> +	return 0;
> +}
> +
>  static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
>  {
>  	struct stm32_adc *adc = iio_priv(indio_dev);
> @@ -652,18 +766,29 @@ static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
>  		return ret;
>  	}
>  
> +	ret = stm32_adc_dma_start(indio_dev);
> +	if (ret) {
> +		dev_err(&indio_dev->dev, "Can't start dma\n");
> +		goto err_clr_trig;
> +	}
> +
>  	ret = iio_triggered_buffer_postenable(indio_dev);
>  	if (ret < 0)
> -		goto err_clr_trig;
> +		goto err_stop_dma;
>  
>  	/* Reset adc buffer index */
>  	adc->bufi = 0;
>  
> -	stm32_adc_conv_irq_enable(adc);
> -	stm32_adc_start_conv(adc);
> +	if (!adc->dma_chan)
> +		stm32_adc_conv_irq_enable(adc);
> +
> +	stm32_adc_start_conv(adc, !!adc->dma_chan);
>  
>  	return 0;
>  
> +err_stop_dma:
> +	if (adc->dma_chan)
> +		dmaengine_terminate_all(adc->dma_chan);
>  err_clr_trig:
>  	stm32_adc_set_trig(indio_dev, NULL);
>  
> @@ -676,12 +801,16 @@ static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev)
>  	int ret;
>  
>  	stm32_adc_stop_conv(adc);
> -	stm32_adc_conv_irq_disable(adc);
> +	if (!adc->dma_chan)
> +		stm32_adc_conv_irq_disable(adc);
>  
>  	ret = iio_triggered_buffer_predisable(indio_dev);
>  	if (ret < 0)
>  		dev_err(&indio_dev->dev, "predisable failed\n");
>  
> +	if (adc->dma_chan)
> +		dmaengine_terminate_all(adc->dma_chan);
> +
>  	if (stm32_adc_set_trig(indio_dev, NULL))
>  		dev_err(&indio_dev->dev, "Can't clear trigger\n");
>  
> @@ -701,15 +830,31 @@ static irqreturn_t stm32_adc_trigger_handler(int irq, void *p)
>  
>  	dev_dbg(&indio_dev->dev, "%s bufi=%d\n", __func__, adc->bufi);
>  
> -	/* reset buffer index */
> -	adc->bufi = 0;
> -	iio_push_to_buffers_with_timestamp(indio_dev, adc->buffer,
> -					   pf->timestamp);
> +	if (!adc->dma_chan) {
> +		/* reset buffer index */
> +		adc->bufi = 0;
> +		iio_push_to_buffers_with_timestamp(indio_dev, adc->buffer,
> +						   pf->timestamp);
> +	} else {
> +		int residue = stm32_adc_dma_residue(adc);
> +
> +		while (residue >= indio_dev->scan_bytes) {
> +			u16 *buffer = (u16 *)&adc->rx_buf[adc->bufi];
> +
> +			iio_push_to_buffers_with_timestamp(indio_dev, buffer,
> +							   pf->timestamp);
> +			residue -= indio_dev->scan_bytes;
> +			adc->bufi += indio_dev->scan_bytes;
> +			if (adc->bufi >= adc->rx_buf_sz)
> +				adc->bufi = 0;
> +		}
> +	}
>  
>  	iio_trigger_notify_done(indio_dev->trig);
>  
>  	/* re-enable eoc irq */
> -	stm32_adc_conv_irq_enable(adc);
> +	if (!adc->dma_chan)
> +		stm32_adc_conv_irq_enable(adc);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -781,6 +926,45 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>  	return 0;
>  }
>  
> +static int stm32_adc_dma_request(struct iio_dev *indio_dev)
> +{
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +	struct dma_slave_config config;
> +	int ret;
> +
> +	adc->dma_chan = dma_request_slave_channel(&indio_dev->dev, "rx");
> +	if (!adc->dma_chan)
> +		return 0;
> +
> +	adc->rx_buf = dma_alloc_coherent(adc->dma_chan->device->dev,
> +					 STM32_DMA_BUFFER_SIZE,
> +					 &adc->rx_dma_buf, GFP_KERNEL);
> +	if (!adc->rx_buf) {
> +		goto err_release;
> +		ret = -ENOMEM;
> +	}
> +
> +	/* Configure DMA channel to read data register */
> +	memset(&config, 0, sizeof(config));
> +	config.src_addr = (dma_addr_t)adc->common->phys_base;
> +	config.src_addr += adc->offset + STM32F4_ADC_DR;
> +	config.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> +
> +	ret = dmaengine_slave_config(adc->dma_chan, &config);
> +	if (ret)
> +		goto err_free;
> +
> +	return 0;
> +
> +err_free:
> +	dma_free_coherent(adc->dma_chan->device->dev, STM32_DMA_BUFFER_SIZE,
> +			  adc->rx_buf, adc->rx_dma_buf);
> +err_release:
> +	dma_release_channel(adc->dma_chan);
> +
> +	return ret;
> +}
> +
>  static int stm32_adc_probe(struct platform_device *pdev)
>  {
>  	struct iio_dev *indio_dev;
> @@ -842,13 +1026,17 @@ static int stm32_adc_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto err_clk_disable;
>  
> +	ret = stm32_adc_dma_request(indio_dev);
> +	if (ret < 0)
> +		goto err_clk_disable;
> +
>  	ret = iio_triggered_buffer_setup(indio_dev,
>  					 &iio_pollfunc_store_time,
>  					 &stm32_adc_trigger_handler,
>  					 &stm32_adc_buffer_setup_ops);
>  	if (ret) {
>  		dev_err(&pdev->dev, "buffer setup failed\n");
> -		goto err_clk_disable;
> +		goto err_dma_disable;
>  	}
>  
>  	ret = iio_device_register(indio_dev);
> @@ -862,6 +1050,13 @@ static int stm32_adc_probe(struct platform_device *pdev)
>  err_buffer_cleanup:
>  	iio_triggered_buffer_cleanup(indio_dev);
>  
> +err_dma_disable:
> +	if (adc->dma_chan) {
> +		dma_free_coherent(adc->dma_chan->device->dev,
> +				  STM32_DMA_BUFFER_SIZE,
> +				  adc->rx_buf, adc->rx_dma_buf);
> +		dma_release_channel(adc->dma_chan);
> +	}
>  err_clk_disable:
>  	clk_disable_unprepare(adc->clk);
>  
> @@ -875,6 +1070,12 @@ static int stm32_adc_remove(struct platform_device *pdev)
>  
>  	iio_device_unregister(indio_dev);
>  	iio_triggered_buffer_cleanup(indio_dev);
> +	if (adc->dma_chan) {
> +		dma_free_coherent(adc->dma_chan->device->dev,
> +				  STM32_DMA_BUFFER_SIZE,
> +				  adc->rx_buf, adc->rx_dma_buf);
> +		dma_release_channel(adc->dma_chan);
> +	}
>  	clk_disable_unprepare(adc->clk);
>  
>  	return 0;
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 5/7] iio: adc: stm32: add optional dma support
  2017-01-28 18:41   ` Jonathan Cameron
@ 2017-01-30  8:57     ` Fabrice Gasnier
  0 siblings, 0 replies; 16+ messages in thread
From: Fabrice Gasnier @ 2017-01-30  8:57 UTC (permalink / raw)
  To: Jonathan Cameron, linux, robh+dt, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: linux-iio, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
	knaack.h, pmeerw, benjamin.gaignard, benjamin.gaignard

On 01/28/2017 07:41 PM, Jonathan Cameron wrote:
> On 26/01/17 14:28, Fabrice Gasnier wrote:
>> Add DMA optional support to STM32 ADC, as there is a limited number DMA
>> channels (request lines) that can be assigned to ADC. This way, driver
>> may fall back using interrupts when all DMA channels are in use for
>> other IPs.
>> Use dma cyclic mode with two periods. Allow to tune period length by
>> using watermark. Coherent memory is used for dma (max buffer size is
>> fixed to PAGE_SIZE).
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> I am happy with this whole series, except this patch gives me:
>
> drivers/iio/adc/stm32-adc.c:478:23: warning: symbol 'stm32_adc_trig_pol' was not declared. Should it be static?
> drivers/iio/adc/stm32-adc.c:621:21: error: incompatible types in comparison expression (different type sizes)
>    CC [M]  drivers/iio/adc/stm32-adc.o
> In file included from ./include/linux/clk.h:16:0,
>                   from drivers/iio/adc/stm32-adc.c:22:
> drivers/iio/adc/stm32-adc.c: In function ‘stm32_adc_set_watermark’:
> ./include/linux/kernel.h:753:16: warning: comparison of distinct pointer types lacks a cast
>    (void) (&min1 == &min2);   \
>                  ^
> ./include/linux/kernel.h:756:2: note: in expansion of macro ‘__min’
>    __min(typeof(x), typeof(y),   \
>    ^~~~~
> drivers/iio/adc/stm32-adc.c:621:14: note: in expansion of macro ‘min’
>    watermark = min(watermark, val * sizeof(u16));
>                ^~~
>
> The static is obviously fine so I've added that.
> The second looks to be because sizeof(u16) is a size_t which is signed IIRC.
> Anyhow, a cast of that to unsigned should I think be harmless and fixes the
> warning.
>
> Please check I did these right.

Hi Jonathan,

I just checked, this is ok for me.

Many thanks!
Best Regards,
Fabrice
>
> They are in the testing branch of iio.git.
>
> Thanks,
>
> Jonathan
>
>> ---
>> Changes in v2:
>> - Use iio_trigger_poll_chained() avoids to bounce back into irq context.
>>    Remove irq_work.
>> - Rework dma buffer allocation and use. Allocation moved to probe time,
>>    fixed to PAGE_SIZE. Use hwfifo_set_watermark() routine to tune dma
>>    cyclic period length.
>> ---
>>   drivers/iio/adc/Kconfig          |   1 +
>>   drivers/iio/adc/stm32-adc-core.c |   1 +
>>   drivers/iio/adc/stm32-adc-core.h |   2 +
>>   drivers/iio/adc/stm32-adc.c      | 227 ++++++++++++++++++++++++++++++++++++---
>>   4 files changed, 218 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> index 9a7b090..03a73f9 100644
>> --- a/drivers/iio/adc/Kconfig
>> +++ b/drivers/iio/adc/Kconfig
>> @@ -444,6 +444,7 @@ config ROCKCHIP_SARADC
>>   config STM32_ADC_CORE
>>   	tristate "STMicroelectronics STM32 adc core"
>>   	depends on ARCH_STM32 || COMPILE_TEST
>> +	depends on HAS_DMA
>>   	depends on OF
>>   	depends on REGULATOR
>>   	select IIO_BUFFER
>> diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
>> index 4214b0c..22b7c93 100644
>> --- a/drivers/iio/adc/stm32-adc-core.c
>> +++ b/drivers/iio/adc/stm32-adc-core.c
>> @@ -201,6 +201,7 @@ static int stm32_adc_probe(struct platform_device *pdev)
>>   	priv->common.base = devm_ioremap_resource(&pdev->dev, res);
>>   	if (IS_ERR(priv->common.base))
>>   		return PTR_ERR(priv->common.base);
>> +	priv->common.phys_base = res->start;
>>   
>>   	priv->vref = devm_regulator_get(&pdev->dev, "vref");
>>   	if (IS_ERR(priv->vref)) {
>> diff --git a/drivers/iio/adc/stm32-adc-core.h b/drivers/iio/adc/stm32-adc-core.h
>> index 081fa5f..2ec7abb 100644
>> --- a/drivers/iio/adc/stm32-adc-core.h
>> +++ b/drivers/iio/adc/stm32-adc-core.h
>> @@ -42,10 +42,12 @@
>>   /**
>>    * struct stm32_adc_common - stm32 ADC driver common data (for all instances)
>>    * @base:		control registers base cpu addr
>> + * @phys_base:		control registers base physical addr
>>    * @vref_mv:		vref voltage (mv)
>>    */
>>   struct stm32_adc_common {
>>   	void __iomem			*base;
>> +	phys_addr_t			phys_base;
>>   	int				vref_mv;
>>   };
>>   
>> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
>> index 9a38f9a..8a30039 100644
>> --- a/drivers/iio/adc/stm32-adc.c
>> +++ b/drivers/iio/adc/stm32-adc.c
>> @@ -21,6 +21,8 @@
>>   
>>   #include <linux/clk.h>
>>   #include <linux/delay.h>
>> +#include <linux/dma-mapping.h>
>> +#include <linux/dmaengine.h>
>>   #include <linux/iio/iio.h>
>>   #include <linux/iio/buffer.h>
>>   #include <linux/iio/timer/stm32-timer-trigger.h>
>> @@ -68,12 +70,16 @@
>>   #define STM32F4_EXTSEL_SHIFT		24
>>   #define STM32F4_EXTSEL_MASK		GENMASK(27, 24)
>>   #define STM32F4_EOCS			BIT(10)
>> +#define STM32F4_DDS			BIT(9)
>> +#define STM32F4_DMA			BIT(8)
>>   #define STM32F4_ADON			BIT(0)
>>   
>>   #define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
>>   #define STM32_ADC_TIMEOUT_US		100000
>>   #define STM32_ADC_TIMEOUT	(msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
>>   
>> +#define STM32_DMA_BUFFER_SIZE		PAGE_SIZE
>> +
>>   /* External trigger enable */
>>   enum stm32_adc_exten {
>>   	STM32_EXTEN_SWTRIG,
>> @@ -136,6 +142,10 @@ struct stm32_adc_regs {
>>    * @bufi:		data buffer index
>>    * @num_conv:		expected number of scan conversions
>>    * @trigger_polarity:	external trigger polarity (e.g. exten)
>> + * @dma_chan:		dma channel
>> + * @rx_buf:		dma rx buffer cpu address
>> + * @rx_dma_buf:		dma rx buffer bus address
>> + * @rx_buf_sz:		dma rx buffer size
>>    */
>>   struct stm32_adc {
>>   	struct stm32_adc_common	*common;
>> @@ -148,6 +158,10 @@ struct stm32_adc {
>>   	unsigned int		bufi;
>>   	unsigned int		num_conv;
>>   	u32			trigger_polarity;
>> +	struct dma_chan		*dma_chan;
>> +	u8			*rx_buf;
>> +	dma_addr_t		rx_dma_buf;
>> +	unsigned int		rx_buf_sz;
>>   };
>>   
>>   /**
>> @@ -291,10 +305,21 @@ static void stm32_adc_conv_irq_disable(struct stm32_adc *adc)
>>   /**
>>    * stm32_adc_start_conv() - Start conversions for regular channels.
>>    * @adc: stm32 adc instance
>> + * @dma: use dma to transfer conversion result
>> + *
>> + * Start conversions for regular channels.
>> + * Also take care of normal or DMA mode. Circular DMA may be used for regular
>> + * conversions, in IIO buffer modes. Otherwise, use ADC interrupt with direct
>> + * DR read instead (e.g. read_raw, or triggered buffer mode without DMA).
>>    */
>> -static void stm32_adc_start_conv(struct stm32_adc *adc)
>> +static void stm32_adc_start_conv(struct stm32_adc *adc, bool dma)
>>   {
>>   	stm32_adc_set_bits(adc, STM32F4_ADC_CR1, STM32F4_SCAN);
>> +
>> +	if (dma)
>> +		stm32_adc_set_bits(adc, STM32F4_ADC_CR2,
>> +				   STM32F4_DMA | STM32F4_DDS);
>> +
>>   	stm32_adc_set_bits(adc, STM32F4_ADC_CR2, STM32F4_EOCS | STM32F4_ADON);
>>   
>>   	/* Wait for Power-up time (tSTAB from datasheet) */
>> @@ -311,7 +336,8 @@ static void stm32_adc_stop_conv(struct stm32_adc *adc)
>>   	stm32_adc_clr_bits(adc, STM32F4_ADC_SR, STM32F4_STRT);
>>   
>>   	stm32_adc_clr_bits(adc, STM32F4_ADC_CR1, STM32F4_SCAN);
>> -	stm32_adc_clr_bits(adc, STM32F4_ADC_CR2, STM32F4_ADON);
>> +	stm32_adc_clr_bits(adc, STM32F4_ADC_CR2,
>> +			   STM32F4_ADON | STM32F4_DMA | STM32F4_DDS);
>>   }
>>   
>>   /**
>> @@ -494,7 +520,7 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
>>   
>>   	stm32_adc_conv_irq_enable(adc);
>>   
>> -	stm32_adc_start_conv(adc);
>> +	stm32_adc_start_conv(adc, false);
>>   
>>   	timeout = wait_for_completion_interruptible_timeout(
>>   					&adc->completion, STM32_ADC_TIMEOUT);
>> @@ -581,6 +607,23 @@ static int stm32_adc_validate_trigger(struct iio_dev *indio_dev,
>>   	return stm32_adc_get_trig_extsel(trig) < 0 ? -EINVAL : 0;
>>   }
>>   
>> +static int stm32_adc_set_watermark(struct iio_dev *indio_dev, unsigned val)
>> +{
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +	unsigned int watermark = STM32_DMA_BUFFER_SIZE / 2;
>> +
>> +	/*
>> +	 * dma cyclic transfers are used, buffer is split into two periods.
>> +	 * There should be :
>> +	 * - always one buffer (period) dma is working on
>> +	 * - one buffer (period) driver can push with iio_trigger_poll().
>> +	 */
>> +	watermark = min(watermark, val * sizeof(u16));
>> +	adc->rx_buf_sz = watermark * 2;
>> +
>> +	return 0;
>> +}
>> +
>>   static int stm32_adc_update_scan_mode(struct iio_dev *indio_dev,
>>   				      const unsigned long *scan_mask)
>>   {
>> @@ -635,12 +678,83 @@ static int stm32_adc_debugfs_reg_access(struct iio_dev *indio_dev,
>>   static const struct iio_info stm32_adc_iio_info = {
>>   	.read_raw = stm32_adc_read_raw,
>>   	.validate_trigger = stm32_adc_validate_trigger,
>> +	.hwfifo_set_watermark = stm32_adc_set_watermark,
>>   	.update_scan_mode = stm32_adc_update_scan_mode,
>>   	.debugfs_reg_access = stm32_adc_debugfs_reg_access,
>>   	.of_xlate = stm32_adc_of_xlate,
>>   	.driver_module = THIS_MODULE,
>>   };
>>   
>> +static unsigned int stm32_adc_dma_residue(struct stm32_adc *adc)
>> +{
>> +	struct dma_tx_state state;
>> +	enum dma_status status;
>> +
>> +	status = dmaengine_tx_status(adc->dma_chan,
>> +				     adc->dma_chan->cookie,
>> +				     &state);
>> +	if (status == DMA_IN_PROGRESS) {
>> +		/* Residue is size in bytes from end of buffer */
>> +		unsigned int i = adc->rx_buf_sz - state.residue;
>> +		unsigned int size;
>> +
>> +		/* Return available bytes */
>> +		if (i >= adc->bufi)
>> +			size = i - adc->bufi;
>> +		else
>> +			size = adc->rx_buf_sz + i - adc->bufi;
>> +
>> +		return size;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void stm32_adc_dma_buffer_done(void *data)
>> +{
>> +	struct iio_dev *indio_dev = data;
>> +
>> +	iio_trigger_poll_chained(indio_dev->trig);
>> +}
>> +
>> +static int stm32_adc_dma_start(struct iio_dev *indio_dev)
>> +{
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +	struct dma_async_tx_descriptor *desc;
>> +	dma_cookie_t cookie;
>> +	int ret;
>> +
>> +	if (!adc->dma_chan)
>> +		return 0;
>> +
>> +	dev_dbg(&indio_dev->dev, "%s size=%d watermark=%d\n", __func__,
>> +		adc->rx_buf_sz, adc->rx_buf_sz / 2);
>> +
>> +	/* Prepare a DMA cyclic transaction */
>> +	desc = dmaengine_prep_dma_cyclic(adc->dma_chan,
>> +					 adc->rx_dma_buf,
>> +					 adc->rx_buf_sz, adc->rx_buf_sz / 2,
>> +					 DMA_DEV_TO_MEM,
>> +					 DMA_PREP_INTERRUPT);
>> +	if (!desc)
>> +		return -EBUSY;
>> +
>> +	desc->callback = stm32_adc_dma_buffer_done;
>> +	desc->callback_param = indio_dev;
>> +
>> +	cookie = dmaengine_submit(desc);
>> +	ret = dma_submit_error(cookie);
>> +	if (ret) {
>> +		dmaengine_terminate_all(adc->dma_chan);
>> +		return ret;
>> +	}
>> +
>> +	/* Issue pending DMA requests */
>> +	dma_async_issue_pending(adc->dma_chan);
>> +
>> +	return 0;
>> +}
>> +
>>   static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
>>   {
>>   	struct stm32_adc *adc = iio_priv(indio_dev);
>> @@ -652,18 +766,29 @@ static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
>>   		return ret;
>>   	}
>>   
>> +	ret = stm32_adc_dma_start(indio_dev);
>> +	if (ret) {
>> +		dev_err(&indio_dev->dev, "Can't start dma\n");
>> +		goto err_clr_trig;
>> +	}
>> +
>>   	ret = iio_triggered_buffer_postenable(indio_dev);
>>   	if (ret < 0)
>> -		goto err_clr_trig;
>> +		goto err_stop_dma;
>>   
>>   	/* Reset adc buffer index */
>>   	adc->bufi = 0;
>>   
>> -	stm32_adc_conv_irq_enable(adc);
>> -	stm32_adc_start_conv(adc);
>> +	if (!adc->dma_chan)
>> +		stm32_adc_conv_irq_enable(adc);
>> +
>> +	stm32_adc_start_conv(adc, !!adc->dma_chan);
>>   
>>   	return 0;
>>   
>> +err_stop_dma:
>> +	if (adc->dma_chan)
>> +		dmaengine_terminate_all(adc->dma_chan);
>>   err_clr_trig:
>>   	stm32_adc_set_trig(indio_dev, NULL);
>>   
>> @@ -676,12 +801,16 @@ static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev)
>>   	int ret;
>>   
>>   	stm32_adc_stop_conv(adc);
>> -	stm32_adc_conv_irq_disable(adc);
>> +	if (!adc->dma_chan)
>> +		stm32_adc_conv_irq_disable(adc);
>>   
>>   	ret = iio_triggered_buffer_predisable(indio_dev);
>>   	if (ret < 0)
>>   		dev_err(&indio_dev->dev, "predisable failed\n");
>>   
>> +	if (adc->dma_chan)
>> +		dmaengine_terminate_all(adc->dma_chan);
>> +
>>   	if (stm32_adc_set_trig(indio_dev, NULL))
>>   		dev_err(&indio_dev->dev, "Can't clear trigger\n");
>>   
>> @@ -701,15 +830,31 @@ static irqreturn_t stm32_adc_trigger_handler(int irq, void *p)
>>   
>>   	dev_dbg(&indio_dev->dev, "%s bufi=%d\n", __func__, adc->bufi);
>>   
>> -	/* reset buffer index */
>> -	adc->bufi = 0;
>> -	iio_push_to_buffers_with_timestamp(indio_dev, adc->buffer,
>> -					   pf->timestamp);
>> +	if (!adc->dma_chan) {
>> +		/* reset buffer index */
>> +		adc->bufi = 0;
>> +		iio_push_to_buffers_with_timestamp(indio_dev, adc->buffer,
>> +						   pf->timestamp);
>> +	} else {
>> +		int residue = stm32_adc_dma_residue(adc);
>> +
>> +		while (residue >= indio_dev->scan_bytes) {
>> +			u16 *buffer = (u16 *)&adc->rx_buf[adc->bufi];
>> +
>> +			iio_push_to_buffers_with_timestamp(indio_dev, buffer,
>> +							   pf->timestamp);
>> +			residue -= indio_dev->scan_bytes;
>> +			adc->bufi += indio_dev->scan_bytes;
>> +			if (adc->bufi >= adc->rx_buf_sz)
>> +				adc->bufi = 0;
>> +		}
>> +	}
>>   
>>   	iio_trigger_notify_done(indio_dev->trig);
>>   
>>   	/* re-enable eoc irq */
>> -	stm32_adc_conv_irq_enable(adc);
>> +	if (!adc->dma_chan)
>> +		stm32_adc_conv_irq_enable(adc);
>>   
>>   	return IRQ_HANDLED;
>>   }
>> @@ -781,6 +926,45 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>>   	return 0;
>>   }
>>   
>> +static int stm32_adc_dma_request(struct iio_dev *indio_dev)
>> +{
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +	struct dma_slave_config config;
>> +	int ret;
>> +
>> +	adc->dma_chan = dma_request_slave_channel(&indio_dev->dev, "rx");
>> +	if (!adc->dma_chan)
>> +		return 0;
>> +
>> +	adc->rx_buf = dma_alloc_coherent(adc->dma_chan->device->dev,
>> +					 STM32_DMA_BUFFER_SIZE,
>> +					 &adc->rx_dma_buf, GFP_KERNEL);
>> +	if (!adc->rx_buf) {
>> +		goto err_release;
>> +		ret = -ENOMEM;
>> +	}
>> +
>> +	/* Configure DMA channel to read data register */
>> +	memset(&config, 0, sizeof(config));
>> +	config.src_addr = (dma_addr_t)adc->common->phys_base;
>> +	config.src_addr += adc->offset + STM32F4_ADC_DR;
>> +	config.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>> +
>> +	ret = dmaengine_slave_config(adc->dma_chan, &config);
>> +	if (ret)
>> +		goto err_free;
>> +
>> +	return 0;
>> +
>> +err_free:
>> +	dma_free_coherent(adc->dma_chan->device->dev, STM32_DMA_BUFFER_SIZE,
>> +			  adc->rx_buf, adc->rx_dma_buf);
>> +err_release:
>> +	dma_release_channel(adc->dma_chan);
>> +
>> +	return ret;
>> +}
>> +
>>   static int stm32_adc_probe(struct platform_device *pdev)
>>   {
>>   	struct iio_dev *indio_dev;
>> @@ -842,13 +1026,17 @@ static int stm32_adc_probe(struct platform_device *pdev)
>>   	if (ret < 0)
>>   		goto err_clk_disable;
>>   
>> +	ret = stm32_adc_dma_request(indio_dev);
>> +	if (ret < 0)
>> +		goto err_clk_disable;
>> +
>>   	ret = iio_triggered_buffer_setup(indio_dev,
>>   					 &iio_pollfunc_store_time,
>>   					 &stm32_adc_trigger_handler,
>>   					 &stm32_adc_buffer_setup_ops);
>>   	if (ret) {
>>   		dev_err(&pdev->dev, "buffer setup failed\n");
>> -		goto err_clk_disable;
>> +		goto err_dma_disable;
>>   	}
>>   
>>   	ret = iio_device_register(indio_dev);
>> @@ -862,6 +1050,13 @@ static int stm32_adc_probe(struct platform_device *pdev)
>>   err_buffer_cleanup:
>>   	iio_triggered_buffer_cleanup(indio_dev);
>>   
>> +err_dma_disable:
>> +	if (adc->dma_chan) {
>> +		dma_free_coherent(adc->dma_chan->device->dev,
>> +				  STM32_DMA_BUFFER_SIZE,
>> +				  adc->rx_buf, adc->rx_dma_buf);
>> +		dma_release_channel(adc->dma_chan);
>> +	}
>>   err_clk_disable:
>>   	clk_disable_unprepare(adc->clk);
>>   
>> @@ -875,6 +1070,12 @@ static int stm32_adc_remove(struct platform_device *pdev)
>>   
>>   	iio_device_unregister(indio_dev);
>>   	iio_triggered_buffer_cleanup(indio_dev);
>> +	if (adc->dma_chan) {
>> +		dma_free_coherent(adc->dma_chan->device->dev,
>> +				  STM32_DMA_BUFFER_SIZE,
>> +				  adc->rx_buf, adc->rx_dma_buf);
>> +		dma_release_channel(adc->dma_chan);
>> +	}
>>   	clk_disable_unprepare(adc->clk);
>>   
>>   	return 0;
>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/7] Add support for triggered buffer mode to STM32 ADC
  2017-01-26 14:28 [PATCH v2 0/7] Add support for triggered buffer mode to STM32 ADC Fabrice Gasnier
                   ` (6 preceding siblings ...)
  2017-01-26 14:28 ` [PATCH v2 7/7] ARM: dts: stm32: Enable pwm1 and pwm3 on stm32f429i-eval Fabrice Gasnier
@ 2017-04-03 14:53 ` Alexandre Torgue
  7 siblings, 0 replies; 16+ messages in thread
From: Alexandre Torgue @ 2017-04-03 14:53 UTC (permalink / raw)
  To: Fabrice Gasnier, jic23, linux, robh+dt, linux-arm-kernel,
	devicetree, linux-kernel
  Cc: linux-iio, mark.rutland, mcoquelin.stm32, lars, knaack.h, pmeerw,
	benjamin.gaignard, benjamin.gaignard

Hi

On 01/26/2017 03:28 PM, Fabrice Gasnier wrote:
> The following patches add support for triggered buffer mode.
> These are based on top of "Add PWM and IIO timer drivers for STM32"
> series. Reference:
> https://lkml.org/lkml/2017/1/20/116
>
> STM32 ADC, can use either interrupts or DMA to collect data.
> Either timer trigger output (TRGO) or PWM can be used as trigger source.
> This patchset has been tested on STM32F429 eval board.
>
...
> Fabrice Gasnier (7):
>   iio: adc: stm32: add support for triggered buffer mode
>   iio: adc: stm32: Enable use of stm32 timer triggers
>   iio: adc: stm32: add trigger polarity extended attribute
>   Documentation: dt: iio: stm32-adc: optional dma support
>   iio: adc: stm32: add optional dma support
>   ARM: dts: stm32: Enable dma by default on stm32f4 adc
>   ARM: dts: stm32: Enable pwm1 and pwm3 on stm32f429i-eval

Patches 6 & 7 (DT) applied on stm32-dt-for-v4.12

Regards
Alex

>
>  Documentation/ABI/testing/sysfs-bus-iio-adc-stm32  |  18 +
>  .../devicetree/bindings/iio/adc/st,stm32-adc.txt   |   7 +
>  arch/arm/boot/dts/stm32429i-eval.dts               |  28 +
>  arch/arm/boot/dts/stm32f429.dtsi                   |   6 +
>  drivers/iio/adc/Kconfig                            |   5 +
>  drivers/iio/adc/stm32-adc-core.c                   |   1 +
>  drivers/iio/adc/stm32-adc-core.h                   |   2 +
>  drivers/iio/adc/stm32-adc.c                        | 633 ++++++++++++++++++++-
>  8 files changed, 676 insertions(+), 24 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-stm32
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2017-04-03 14:55 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-26 14:28 [PATCH v2 0/7] Add support for triggered buffer mode to STM32 ADC Fabrice Gasnier
2017-01-26 14:28 ` [PATCH v2 1/7] iio: adc: stm32: add support for triggered buffer mode Fabrice Gasnier
2017-01-28 18:36   ` Jonathan Cameron
2017-01-26 14:28 ` [PATCH v2 2/7] iio: adc: stm32: Enable use of stm32 timer triggers Fabrice Gasnier
2017-01-28 18:37   ` Jonathan Cameron
2017-01-26 14:28 ` [PATCH v2 3/7] iio: adc: stm32: add trigger polarity extended attribute Fabrice Gasnier
2017-01-28 18:39   ` Jonathan Cameron
2017-01-26 14:28 ` [PATCH v2 4/7] Documentation: dt: iio: stm32-adc: optional dma support Fabrice Gasnier
2017-01-28 18:39   ` Jonathan Cameron
2017-01-26 14:28 ` [PATCH v2 5/7] iio: adc: stm32: add " Fabrice Gasnier
2017-01-28 18:40   ` Jonathan Cameron
2017-01-28 18:41   ` Jonathan Cameron
2017-01-30  8:57     ` Fabrice Gasnier
2017-01-26 14:28 ` [PATCH v2 6/7] ARM: dts: stm32: Enable dma by default on stm32f4 adc Fabrice Gasnier
2017-01-26 14:28 ` [PATCH v2 7/7] ARM: dts: stm32: Enable pwm1 and pwm3 on stm32f429i-eval Fabrice Gasnier
2017-04-03 14:53 ` [PATCH v2 0/7] Add support for triggered buffer mode to STM32 ADC Alexandre Torgue

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).