[PATCH v3 0/6] Add support for PWM input capture on STM32

[PATCH v3 0/6] Add support for PWM input capture on STM32

[Fabrice Gasnier]

This series adds support for capture to stm32-pwm driver.
Capture is based on DMAs.
- First two patches add support for requesting DMAs to MFD core
- Next three patches add support for capture to stm32-pwm driver
- This has been tested on stm32429i-eval board.

---
Changes in v3:
- Dropped 2 precusor patches applied by Thierry in pwm tree:
  "pwm: stm32: fix, remove unused struct device"
  "pwm: stm32: protect common prescaler for all channels"
- Note: this series applies on top on pwm tree
- Implements Lee's comments on MFD part: rework stm32_timers_dma struct,
  exported routine prototype now use generic device struct, more
  various comments (see patch 2 changelog).

Resend v2:
- Add collected Acks

Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Rework pwm capture routines to adopt this change
- Comment on optional dma support, beautify DMAs probe

Fabrice Gasnier (6):
  dt-bindings: mfd: stm32-timers: add support for dmas
  mfd: stm32-timers: add support for dmas
  pwm: stm32: add capture support
  pwm: stm32: improve capture by tuning counter prescaler
  pwm: stm32: use input prescaler to improve period capture
  ARM: dts: stm32: Enable pwm3 input capture on stm32f429i-eval

 .../devicetree/bindings/mfd/stm32-timers.txt       |  20 ++
 arch/arm/boot/dts/stm32429i-eval.dts               |   3 +
 drivers/mfd/stm32-timers.c                         | 219 +++++++++++++++++-
 drivers/pwm/pwm-stm32.c                            | 257 +++++++++++++++++++++
 include/linux/mfd/stm32-timers.h                   |  44 ++++
 5 files changed, 541 insertions(+), 2 deletions(-)

-- 
1.9.1

[PATCH v3 0/6] Add support for PWM input capture on STM32

[Fabrice Gasnier]

This series adds support for capture to stm32-pwm driver.
Capture is based on DMAs.
- First two patches add support for requesting DMAs to MFD core
- Next three patches add support for capture to stm32-pwm driver
- This has been tested on stm32429i-eval board.

---
Changes in v3:
- Dropped 2 precusor patches applied by Thierry in pwm tree:
  "pwm: stm32: fix, remove unused struct device"
  "pwm: stm32: protect common prescaler for all channels"
- Note: this series applies on top on pwm tree
- Implements Lee's comments on MFD part: rework stm32_timers_dma struct,
  exported routine prototype now use generic device struct, more
  various comments (see patch 2 changelog).

Resend v2:
- Add collected Acks

Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Rework pwm capture routines to adopt this change
- Comment on optional dma support, beautify DMAs probe

Fabrice Gasnier (6):
  dt-bindings: mfd: stm32-timers: add support for dmas
  mfd: stm32-timers: add support for dmas
  pwm: stm32: add capture support
  pwm: stm32: improve capture by tuning counter prescaler
  pwm: stm32: use input prescaler to improve period capture
  ARM: dts: stm32: Enable pwm3 input capture on stm32f429i-eval

 .../devicetree/bindings/mfd/stm32-timers.txt       |  20 ++
 arch/arm/boot/dts/stm32429i-eval.dts               |   3 +
 drivers/mfd/stm32-timers.c                         | 219 +++++++++++++++++-
 drivers/pwm/pwm-stm32.c                            | 257 +++++++++++++++++++++
 include/linux/mfd/stm32-timers.h                   |  44 ++++
 5 files changed, 541 insertions(+), 2 deletions(-)

-- 
1.9.1

[PATCH v3 0/6] Add support for PWM input capture on STM32

[Fabrice Gasnier]

This series adds support for capture to stm32-pwm driver.
Capture is based on DMAs.
- First two patches add support for requesting DMAs to MFD core
- Next three patches add support for capture to stm32-pwm driver
- This has been tested on stm32429i-eval board.

---
Changes in v3:
- Dropped 2 precusor patches applied by Thierry in pwm tree:
  "pwm: stm32: fix, remove unused struct device"
  "pwm: stm32: protect common prescaler for all channels"
- Note: this series applies on top on pwm tree
- Implements Lee's comments on MFD part: rework stm32_timers_dma struct,
  exported routine prototype now use generic device struct, more
  various comments (see patch 2 changelog).

Resend v2:
- Add collected Acks

Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Rework pwm capture routines to adopt this change
- Comment on optional dma support, beautify DMAs probe

Fabrice Gasnier (6):
  dt-bindings: mfd: stm32-timers: add support for dmas
  mfd: stm32-timers: add support for dmas
  pwm: stm32: add capture support
  pwm: stm32: improve capture by tuning counter prescaler
  pwm: stm32: use input prescaler to improve period capture
  ARM: dts: stm32: Enable pwm3 input capture on stm32f429i-eval

 .../devicetree/bindings/mfd/stm32-timers.txt       |  20 ++
 arch/arm/boot/dts/stm32429i-eval.dts               |   3 +
 drivers/mfd/stm32-timers.c                         | 219 +++++++++++++++++-
 drivers/pwm/pwm-stm32.c                            | 257 +++++++++++++++++++++
 include/linux/mfd/stm32-timers.h                   |  44 ++++
 5 files changed, 541 insertions(+), 2 deletions(-)

-- 
1.9.1

[PATCH v3 1/6] dt-bindings: mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

Add support for DMAs to STM32 timers. STM32 Timers can support up to 7
dma requests: up to 4 channels, update, compare and trigger.
DMAs may be used to transfer data from pwm capture for instance.
DMA support is made optional, PWM capture support is also an option.
This is much more wise system-wide to avoid shortage on DMA request
lines as there's significant amount of timer instances that can
request up to 7 channels.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 .../devicetree/bindings/mfd/stm32-timers.txt         | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/stm32-timers.txt b/Documentation/devicetree/bindings/mfd/stm32-timers.txt
index 1db6e00..0e900b5 100644
--- a/Documentation/devicetree/bindings/mfd/stm32-timers.txt
+++ b/Documentation/devicetree/bindings/mfd/stm32-timers.txt
@@ -19,6 +19,11 @@ Required parameters:
 Optional parameters:
 - resets:		Phandle to the parent reset controller.
 			See ../reset/st,stm32-rcc.txt
+- dmas:			List of phandle to dma channels that can be used for
+			this timer instance. There may be up to 7 dma channels.
+- dma-names:		List of dma names. Must match 'dmas' property. Valid
+			names are: "ch1", "ch2", "ch3", "ch4", "up", "trig",
+			"com".
 
 Optional subnodes:
 - pwm:			See ../pwm/pwm-stm32.txt
@@ -44,3 +49,18 @@ Example:
 			reg = <0>;
 		};
 	};
+
+Example with all dmas:
+	timer@40010000 {
+		...
+		dmas = <&dmamux1 11 0x400 0x0>,
+		       <&dmamux1 12 0x400 0x0>,
+		       <&dmamux1 13 0x400 0x0>,
+		       <&dmamux1 14 0x400 0x0>,
+		       <&dmamux1 15 0x400 0x0>,
+		       <&dmamux1 16 0x400 0x0>,
+		       <&dmamux1 17 0x400 0x0>;
+		dma-names = "ch1", "ch2", "ch3", "ch4", "up", "trig", "com";
+		...
+		child nodes...
+	};
-- 
1.9.1

[PATCH v3 1/6] dt-bindings: mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

Add support for DMAs to STM32 timers. STM32 Timers can support up to 7
dma requests: up to 4 channels, update, compare and trigger.
DMAs may be used to transfer data from pwm capture for instance.
DMA support is made optional, PWM capture support is also an option.
This is much more wise system-wide to avoid shortage on DMA request
lines as there's significant amount of timer instances that can
request up to 7 channels.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 .../devicetree/bindings/mfd/stm32-timers.txt         | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/stm32-timers.txt b/Documentation/devicetree/bindings/mfd/stm32-timers.txt
index 1db6e00..0e900b5 100644
--- a/Documentation/devicetree/bindings/mfd/stm32-timers.txt
+++ b/Documentation/devicetree/bindings/mfd/stm32-timers.txt
@@ -19,6 +19,11 @@ Required parameters:
 Optional parameters:
 - resets:		Phandle to the parent reset controller.
 			See ../reset/st,stm32-rcc.txt
+- dmas:			List of phandle to dma channels that can be used for
+			this timer instance. There may be up to 7 dma channels.
+- dma-names:		List of dma names. Must match 'dmas' property. Valid
+			names are: "ch1", "ch2", "ch3", "ch4", "up", "trig",
+			"com".
 
 Optional subnodes:
 - pwm:			See ../pwm/pwm-stm32.txt
@@ -44,3 +49,18 @@ Example:
 			reg = <0>;
 		};
 	};
+
+Example with all dmas:
+	timer@40010000 {
+		...
+		dmas = <&dmamux1 11 0x400 0x0>,
+		       <&dmamux1 12 0x400 0x0>,
+		       <&dmamux1 13 0x400 0x0>,
+		       <&dmamux1 14 0x400 0x0>,
+		       <&dmamux1 15 0x400 0x0>,
+		       <&dmamux1 16 0x400 0x0>,
+		       <&dmamux1 17 0x400 0x0>;
+		dma-names = "ch1", "ch2", "ch3", "ch4", "up", "trig", "com";
+		...
+		child nodes...
+	};
-- 
1.9.1

[PATCH v3 1/6] dt-bindings: mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

Add support for DMAs to STM32 timers. STM32 Timers can support up to 7
dma requests: up to 4 channels, update, compare and trigger.
DMAs may be used to transfer data from pwm capture for instance.
DMA support is made optional, PWM capture support is also an option.
This is much more wise system-wide to avoid shortage on DMA request
lines as there's significant amount of timer instances that can
request up to 7 channels.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 .../devicetree/bindings/mfd/stm32-timers.txt         | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/stm32-timers.txt b/Documentation/devicetree/bindings/mfd/stm32-timers.txt
index 1db6e00..0e900b5 100644
--- a/Documentation/devicetree/bindings/mfd/stm32-timers.txt
+++ b/Documentation/devicetree/bindings/mfd/stm32-timers.txt
@@ -19,6 +19,11 @@ Required parameters:
 Optional parameters:
 - resets:		Phandle to the parent reset controller.
 			See ../reset/st,stm32-rcc.txt
+- dmas:			List of phandle to dma channels that can be used for
+			this timer instance. There may be up to 7 dma channels.
+- dma-names:		List of dma names. Must match 'dmas' property. Valid
+			names are: "ch1", "ch2", "ch3", "ch4", "up", "trig",
+			"com".
 
 Optional subnodes:
 - pwm:			See ../pwm/pwm-stm32.txt
@@ -44,3 +49,18 @@ Example:
 			reg = <0>;
 		};
 	};
+
+Example with all dmas:
+	timer at 40010000 {
+		...
+		dmas = <&dmamux1 11 0x400 0x0>,
+		       <&dmamux1 12 0x400 0x0>,
+		       <&dmamux1 13 0x400 0x0>,
+		       <&dmamux1 14 0x400 0x0>,
+		       <&dmamux1 15 0x400 0x0>,
+		       <&dmamux1 16 0x400 0x0>,
+		       <&dmamux1 17 0x400 0x0>;
+		dma-names = "ch1", "ch2", "ch3", "ch4", "up", "trig", "com";
+		...
+		child nodes...
+	};
-- 
1.9.1

[PATCH v3 2/6] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

STM32 Timers can support up to 7 DMA requests:
- 4 channels, update, compare and trigger.
Optionally request part, or all DMAs from stm32-timers MFD core.

Also add routine to implement burst reads using DMA from timer registers.
This is exported. So, it can be used by child drivers, PWM capture
for instance (but not limited to).

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
Changes in v3:
- Basically Lee's comments:
- rather create a struct stm32_timers_dma, and place a reference to it
  in existing ddata (instead of adding priv struct).
- rather use a struct device in exported routine prototype, and use
  standard helpers instead of ddata. Get rid of to_stm32_timers_priv().
- simplify error handling in probe (remove a goto)
- comment on devm_of_platform_*populate() usage.

Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Add comments on optional dma support
---
 drivers/mfd/stm32-timers.c       | 219 ++++++++++++++++++++++++++++++++++++++-
 include/linux/mfd/stm32-timers.h |  32 ++++++
 2 files changed, 249 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
index 1d347e5..98191ec 100644
--- a/drivers/mfd/stm32-timers.c
+++ b/drivers/mfd/stm32-timers.c
@@ -4,16 +4,165 @@
  * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
  */
 
+#include <linux/bitfield.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
 #include <linux/mfd/stm32-timers.h>
 #include <linux/module.h>
 #include <linux/of_platform.h>
 #include <linux/reset.h>
 
+#define STM32_TIMERS_MAX_REGISTERS	0x3fc
+
+struct stm32_timers_dma {
+	struct completion completion;
+	phys_addr_t phys_base;
+	struct mutex lock;		/* protect dma access */
+	struct dma_chan *chan;
+	struct dma_chan *chans[STM32_TIMERS_MAX_DMAS];
+};
+
+/* DIER register DMA enable bits */
+static const u32 stm32_timers_dier_dmaen[STM32_TIMERS_MAX_DMAS] = {
+	TIM_DIER_CC1DE,
+	TIM_DIER_CC2DE,
+	TIM_DIER_CC3DE,
+	TIM_DIER_CC4DE,
+	TIM_DIER_UIE,
+	TIM_DIER_TDE,
+	TIM_DIER_COMDE
+};
+
+static void stm32_timers_dma_done(void *p)
+{
+	struct stm32_timers_dma *dma = p;
+	struct dma_tx_state state;
+	enum dma_status status;
+
+	status = dmaengine_tx_status(dma->chan, dma->chan->cookie, &state);
+	if (status == DMA_COMPLETE)
+		complete(&dma->completion);
+}
+
+/**
+ * stm32_timers_dma_burst_read - Read from timers registers using DMA.
+ *
+ * Read from STM32 timers registers using DMA on a single event.
+ * @dev: reference to stm32_timers MFD device
+ * @buf: dma'able destination buffer
+ * @id: stm32_timers_dmas event identifier (ch[1..4], up, trig or com)
+ * @reg: registers start offset for DMA to read from (like CCRx for capture)
+ * @num_reg: number of registers to read upon each dma request, starting @reg.
+ * @bursts: number of bursts to read (e.g. like two for pwm period capture)
+ * @tmo_ms: timeout (milliseconds)
+ */
+int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
+				enum stm32_timers_dmas id, u32 reg,
+				unsigned int num_reg, unsigned int bursts,
+				unsigned long tmo_ms)
+{
+	struct stm32_timers *ddata = dev_get_drvdata(dev);
+	unsigned long timeout = msecs_to_jiffies(tmo_ms);
+	struct regmap *regmap = ddata->regmap;
+	struct stm32_timers_dma *dma = ddata->dma;
+	size_t len = num_reg * bursts * sizeof(u32);
+	struct dma_async_tx_descriptor *desc;
+	struct dma_slave_config config;
+	dma_cookie_t cookie;
+	dma_addr_t dma_buf;
+	u32 dbl, dba;
+	long err;
+	int ret;
+
+	/* sanity check */
+	if (id < STM32_TIMERS_DMA_CH1 || id >= STM32_TIMERS_MAX_DMAS)
+		return -EINVAL;
+
+	if (!num_reg || !bursts || reg > STM32_TIMERS_MAX_REGISTERS ||
+	    (reg + num_reg * sizeof(u32)) > STM32_TIMERS_MAX_REGISTERS)
+		return -EINVAL;
+
+	if (!dma->chans[id])
+		return -ENODEV;
+	mutex_lock(&dma->lock);
+
+	/* select dma channel in use */
+	dma->chan = dma->chans[id];
+	dma_buf = dma_map_single(dev, buf, len, DMA_FROM_DEVICE);
+	ret = dma_mapping_error(dev, dma_buf);
+	if (ret)
+		goto unlock;
+
+	/* Prepare DMA read from timer registers, using DMA burst mode */
+	memset(&config, 0, sizeof(config));
+	config.src_addr = (dma_addr_t)dma->phys_base + TIM_DMAR;
+	config.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+	ret = dmaengine_slave_config(dma->chan, &config);
+	if (ret)
+		goto unmap;
+
+	desc = dmaengine_prep_slave_single(dma->chan, dma_buf, len,
+					   DMA_DEV_TO_MEM, DMA_PREP_INTERRUPT);
+	if (!desc) {
+		ret = -EBUSY;
+		goto unmap;
+	}
+
+	desc->callback = stm32_timers_dma_done;
+	desc->callback_param = dma;
+	cookie = dmaengine_submit(desc);
+	ret = dma_submit_error(cookie);
+	if (ret)
+		goto dma_term;
+
+	reinit_completion(&dma->completion);
+	dma_async_issue_pending(dma->chan);
+
+	/* Setup and enable timer DMA burst mode */
+	dbl = FIELD_PREP(TIM_DCR_DBL, bursts - 1);
+	dba = FIELD_PREP(TIM_DCR_DBA, reg >> 2);
+	ret = regmap_write(regmap, TIM_DCR, dbl | dba);
+	if (ret)
+		goto dma_term;
+
+	/* Clear pending flags before enabling DMA request */
+	ret = regmap_write(regmap, TIM_SR, 0);
+	if (ret)
+		goto dcr_clr;
+
+	ret = regmap_update_bits(regmap, TIM_DIER, stm32_timers_dier_dmaen[id],
+				 stm32_timers_dier_dmaen[id]);
+	if (ret)
+		goto dcr_clr;
+
+	err = wait_for_completion_interruptible_timeout(&dma->completion,
+							timeout);
+	if (err == 0)
+		ret = -ETIMEDOUT;
+	else if (err < 0)
+		ret = err;
+
+	regmap_update_bits(regmap, TIM_DIER, stm32_timers_dier_dmaen[id], 0);
+	regmap_write(regmap, TIM_SR, 0);
+dcr_clr:
+	regmap_write(regmap, TIM_DCR, 0);
+dma_term:
+	dmaengine_terminate_all(dma->chan);
+unmap:
+	dma_unmap_single(dev, dma_buf, len, DMA_FROM_DEVICE);
+unlock:
+	dma->chan = NULL;
+	mutex_unlock(&dma->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(stm32_timers_dma_burst_read);
+
 static const struct regmap_config stm32_timers_regmap_cfg = {
 	.reg_bits = 32,
 	.val_bits = 32,
 	.reg_stride = sizeof(u32),
-	.max_register = 0x3fc,
+	.max_register = STM32_TIMERS_MAX_REGISTERS,
 };
 
 static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
@@ -27,12 +176,53 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
 	regmap_write(ddata->regmap, TIM_ARR, 0x0);
 }
 
+static int stm32_timers_dma_probe(struct device *dev,
+				  struct stm32_timers *ddata)
+{
+	int i;
+	char name[4];
+	struct stm32_timers_dma *dma;
+
+	dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL);
+	if (!dma)
+		return -ENOMEM;
+
+	init_completion(&dma->completion);
+	mutex_init(&dma->lock);
+
+	/* Optional DMA support: get valid dma channel(s) or NULL */
+	for (i = STM32_TIMERS_DMA_CH1; i <= STM32_TIMERS_DMA_CH4; i++) {
+		snprintf(name, ARRAY_SIZE(name), "ch%1d", i + 1);
+		dma->chans[i] = dma_request_slave_channel(dev, name);
+	}
+	dma->chans[STM32_TIMERS_DMA_UP] =
+		dma_request_slave_channel(dev, "up");
+	dma->chans[STM32_TIMERS_DMA_TRIG] =
+		dma_request_slave_channel(dev, "trig");
+	dma->chans[STM32_TIMERS_DMA_COM] =
+		dma_request_slave_channel(dev, "com");
+	ddata->dma = dma;
+
+	return 0;
+}
+
+static void stm32_timers_dma_remove(struct device *dev,
+				    struct stm32_timers *ddata)
+{
+	int i;
+
+	for (i = STM32_TIMERS_DMA_CH1; i < STM32_TIMERS_MAX_DMAS; i++)
+		if (ddata->dma->chans[i])
+			dma_release_channel(ddata->dma->chans[i]);
+}
+
 static int stm32_timers_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct stm32_timers *ddata;
 	struct resource *res;
 	void __iomem *mmio;
+	int ret;
 
 	ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
 	if (!ddata)
@@ -54,9 +244,33 @@ static int stm32_timers_probe(struct platform_device *pdev)
 
 	stm32_timers_get_arr_size(ddata);
 
+	ret = stm32_timers_dma_probe(dev, ddata);
+	if (ret)
+		return ret;
+	/* timers physical addr for dma */
+	ddata->dma->phys_base = res->start;
+
 	platform_set_drvdata(pdev, ddata);
 
-	return devm_of_platform_populate(&pdev->dev);
+	ret = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+	if (ret)
+		stm32_timers_dma_remove(dev, ddata);
+
+	return ret;
+}
+
+static int stm32_timers_remove(struct platform_device *pdev)
+{
+	struct stm32_timers *ddata = platform_get_drvdata(pdev);
+
+	/*
+	 * Don't use devm_ here: enfore of_platform_depopulate() happens before
+	 * dma are released, to avoid race on dma.
+	 */
+	of_platform_depopulate(&pdev->dev);
+	stm32_timers_dma_remove(&pdev->dev, ddata);
+
+	return 0;
 }
 
 static const struct of_device_id stm32_timers_of_match[] = {
@@ -67,6 +281,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
 
 static struct platform_driver stm32_timers_driver = {
 	.probe = stm32_timers_probe,
+	.remove = stm32_timers_remove,
 	.driver	= {
 		.name = "stm32-timers",
 		.of_match_table = stm32_timers_of_match,
diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
index 2aadab6..585a4de 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -8,6 +8,8 @@
 #define _LINUX_STM32_GPTIMER_H_
 
 #include <linux/clk.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
 #include <linux/regmap.h>
 
 #define TIM_CR1		0x00	/* Control Register 1      */
@@ -27,6 +29,8 @@
 #define TIM_CCR3	0x3C	/* Capt/Comp Register 3    */
 #define TIM_CCR4	0x40	/* Capt/Comp Register 4    */
 #define TIM_BDTR	0x44	/* Break and Dead-Time Reg */
+#define TIM_DCR		0x48	/* DMA control register    */
+#define TIM_DMAR	0x4C	/* DMA register for transfer */
 
 #define TIM_CR1_CEN	BIT(0)	/* Counter Enable	   */
 #define TIM_CR1_DIR	BIT(4)  /* Counter Direction	   */
@@ -36,6 +40,13 @@
 #define TIM_SMCR_SMS	(BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
 #define TIM_SMCR_TS	(BIT(4) | BIT(5) | BIT(6)) /* Trigger selection */
 #define TIM_DIER_UIE	BIT(0)	/* Update interrupt	   */
+#define TIM_DIER_UDE	BIT(8)  /* Update DMA request Enable */
+#define TIM_DIER_CC1DE	BIT(9)  /* CC1 DMA request Enable  */
+#define TIM_DIER_CC2DE	BIT(10) /* CC2 DMA request Enable  */
+#define TIM_DIER_CC3DE	BIT(11) /* CC3 DMA request Enable  */
+#define TIM_DIER_CC4DE	BIT(12) /* CC4 DMA request Enable  */
+#define TIM_DIER_COMDE	BIT(13) /* COM DMA request Enable  */
+#define TIM_DIER_TDE	BIT(14) /* Trigger DMA request Enable */
 #define TIM_SR_UIF	BIT(0)	/* Update interrupt flag   */
 #define TIM_EGR_UG	BIT(0)	/* Update Generation       */
 #define TIM_CCMR_PE	BIT(3)	/* Channel Preload Enable  */
@@ -56,6 +67,8 @@
 #define TIM_BDTR_BK2F	(BIT(20) | BIT(21) | BIT(22) | BIT(23))
 #define TIM_BDTR_BK2E	BIT(24) /* Break 2 input enable	   */
 #define TIM_BDTR_BK2P	BIT(25) /* Break 2 input polarity  */
+#define TIM_DCR_DBA	GENMASK(4, 0)	/* DMA base addr */
+#define TIM_DCR_DBL	GENMASK(12, 8)	/* DMA burst len */
 
 #define MAX_TIM_PSC		0xFFFF
 #define TIM_CR2_MMS_SHIFT	4
@@ -65,9 +78,28 @@
 #define TIM_BDTR_BKF_SHIFT	16
 #define TIM_BDTR_BK2F_SHIFT	20
 
+enum stm32_timers_dmas {
+	STM32_TIMERS_DMA_CH1,
+	STM32_TIMERS_DMA_CH2,
+	STM32_TIMERS_DMA_CH3,
+	STM32_TIMERS_DMA_CH4,
+	STM32_TIMERS_DMA_UP,
+	STM32_TIMERS_DMA_TRIG,
+	STM32_TIMERS_DMA_COM,
+	STM32_TIMERS_MAX_DMAS,
+};
+
+struct stm32_timers_dma;
+
 struct stm32_timers {
 	struct clk *clk;
 	struct regmap *regmap;
 	u32 max_arr;
+	struct stm32_timers_dma *dma;
 };
+
+int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
+				enum stm32_timers_dmas id, u32 reg,
+				unsigned int num_reg, unsigned int bursts,
+				unsigned long tmo_ms);
 #endif
-- 
1.9.1

[PATCH v3 2/6] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

STM32 Timers can support up to 7 DMA requests:
- 4 channels, update, compare and trigger.
Optionally request part, or all DMAs from stm32-timers MFD core.

Also add routine to implement burst reads using DMA from timer registers.
This is exported. So, it can be used by child drivers, PWM capture
for instance (but not limited to).

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
Changes in v3:
- Basically Lee's comments:
- rather create a struct stm32_timers_dma, and place a reference to it
  in existing ddata (instead of adding priv struct).
- rather use a struct device in exported routine prototype, and use
  standard helpers instead of ddata. Get rid of to_stm32_timers_priv().
- simplify error handling in probe (remove a goto)
- comment on devm_of_platform_*populate() usage.

Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Add comments on optional dma support
---
 drivers/mfd/stm32-timers.c       | 219 ++++++++++++++++++++++++++++++++++++++-
 include/linux/mfd/stm32-timers.h |  32 ++++++
 2 files changed, 249 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
index 1d347e5..98191ec 100644
--- a/drivers/mfd/stm32-timers.c
+++ b/drivers/mfd/stm32-timers.c
@@ -4,16 +4,165 @@
  * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
  */
 
+#include <linux/bitfield.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
 #include <linux/mfd/stm32-timers.h>
 #include <linux/module.h>
 #include <linux/of_platform.h>
 #include <linux/reset.h>
 
+#define STM32_TIMERS_MAX_REGISTERS	0x3fc
+
+struct stm32_timers_dma {
+	struct completion completion;
+	phys_addr_t phys_base;
+	struct mutex lock;		/* protect dma access */
+	struct dma_chan *chan;
+	struct dma_chan *chans[STM32_TIMERS_MAX_DMAS];
+};
+
+/* DIER register DMA enable bits */
+static const u32 stm32_timers_dier_dmaen[STM32_TIMERS_MAX_DMAS] = {
+	TIM_DIER_CC1DE,
+	TIM_DIER_CC2DE,
+	TIM_DIER_CC3DE,
+	TIM_DIER_CC4DE,
+	TIM_DIER_UIE,
+	TIM_DIER_TDE,
+	TIM_DIER_COMDE
+};
+
+static void stm32_timers_dma_done(void *p)
+{
+	struct stm32_timers_dma *dma = p;
+	struct dma_tx_state state;
+	enum dma_status status;
+
+	status = dmaengine_tx_status(dma->chan, dma->chan->cookie, &state);
+	if (status == DMA_COMPLETE)
+		complete(&dma->completion);
+}
+
+/**
+ * stm32_timers_dma_burst_read - Read from timers registers using DMA.
+ *
+ * Read from STM32 timers registers using DMA on a single event.
+ * @dev: reference to stm32_timers MFD device
+ * @buf: dma'able destination buffer
+ * @id: stm32_timers_dmas event identifier (ch[1..4], up, trig or com)
+ * @reg: registers start offset for DMA to read from (like CCRx for capture)
+ * @num_reg: number of registers to read upon each dma request, starting @reg.
+ * @bursts: number of bursts to read (e.g. like two for pwm period capture)
+ * @tmo_ms: timeout (milliseconds)
+ */
+int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
+				enum stm32_timers_dmas id, u32 reg,
+				unsigned int num_reg, unsigned int bursts,
+				unsigned long tmo_ms)
+{
+	struct stm32_timers *ddata = dev_get_drvdata(dev);
+	unsigned long timeout = msecs_to_jiffies(tmo_ms);
+	struct regmap *regmap = ddata->regmap;
+	struct stm32_timers_dma *dma = ddata->dma;
+	size_t len = num_reg * bursts * sizeof(u32);
+	struct dma_async_tx_descriptor *desc;
+	struct dma_slave_config config;
+	dma_cookie_t cookie;
+	dma_addr_t dma_buf;
+	u32 dbl, dba;
+	long err;
+	int ret;
+
+	/* sanity check */
+	if (id < STM32_TIMERS_DMA_CH1 || id >= STM32_TIMERS_MAX_DMAS)
+		return -EINVAL;
+
+	if (!num_reg || !bursts || reg > STM32_TIMERS_MAX_REGISTERS ||
+	    (reg + num_reg * sizeof(u32)) > STM32_TIMERS_MAX_REGISTERS)
+		return -EINVAL;
+
+	if (!dma->chans[id])
+		return -ENODEV;
+	mutex_lock(&dma->lock);
+
+	/* select dma channel in use */
+	dma->chan = dma->chans[id];
+	dma_buf = dma_map_single(dev, buf, len, DMA_FROM_DEVICE);
+	ret = dma_mapping_error(dev, dma_buf);
+	if (ret)
+		goto unlock;
+
+	/* Prepare DMA read from timer registers, using DMA burst mode */
+	memset(&config, 0, sizeof(config));
+	config.src_addr = (dma_addr_t)dma->phys_base + TIM_DMAR;
+	config.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+	ret = dmaengine_slave_config(dma->chan, &config);
+	if (ret)
+		goto unmap;
+
+	desc = dmaengine_prep_slave_single(dma->chan, dma_buf, len,
+					   DMA_DEV_TO_MEM, DMA_PREP_INTERRUPT);
+	if (!desc) {
+		ret = -EBUSY;
+		goto unmap;
+	}
+
+	desc->callback = stm32_timers_dma_done;
+	desc->callback_param = dma;
+	cookie = dmaengine_submit(desc);
+	ret = dma_submit_error(cookie);
+	if (ret)
+		goto dma_term;
+
+	reinit_completion(&dma->completion);
+	dma_async_issue_pending(dma->chan);
+
+	/* Setup and enable timer DMA burst mode */
+	dbl = FIELD_PREP(TIM_DCR_DBL, bursts - 1);
+	dba = FIELD_PREP(TIM_DCR_DBA, reg >> 2);
+	ret = regmap_write(regmap, TIM_DCR, dbl | dba);
+	if (ret)
+		goto dma_term;
+
+	/* Clear pending flags before enabling DMA request */
+	ret = regmap_write(regmap, TIM_SR, 0);
+	if (ret)
+		goto dcr_clr;
+
+	ret = regmap_update_bits(regmap, TIM_DIER, stm32_timers_dier_dmaen[id],
+				 stm32_timers_dier_dmaen[id]);
+	if (ret)
+		goto dcr_clr;
+
+	err = wait_for_completion_interruptible_timeout(&dma->completion,
+							timeout);
+	if (err == 0)
+		ret = -ETIMEDOUT;
+	else if (err < 0)
+		ret = err;
+
+	regmap_update_bits(regmap, TIM_DIER, stm32_timers_dier_dmaen[id], 0);
+	regmap_write(regmap, TIM_SR, 0);
+dcr_clr:
+	regmap_write(regmap, TIM_DCR, 0);
+dma_term:
+	dmaengine_terminate_all(dma->chan);
+unmap:
+	dma_unmap_single(dev, dma_buf, len, DMA_FROM_DEVICE);
+unlock:
+	dma->chan = NULL;
+	mutex_unlock(&dma->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(stm32_timers_dma_burst_read);
+
 static const struct regmap_config stm32_timers_regmap_cfg = {
 	.reg_bits = 32,
 	.val_bits = 32,
 	.reg_stride = sizeof(u32),
-	.max_register = 0x3fc,
+	.max_register = STM32_TIMERS_MAX_REGISTERS,
 };
 
 static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
@@ -27,12 +176,53 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
 	regmap_write(ddata->regmap, TIM_ARR, 0x0);
 }
 
+static int stm32_timers_dma_probe(struct device *dev,
+				  struct stm32_timers *ddata)
+{
+	int i;
+	char name[4];
+	struct stm32_timers_dma *dma;
+
+	dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL);
+	if (!dma)
+		return -ENOMEM;
+
+	init_completion(&dma->completion);
+	mutex_init(&dma->lock);
+
+	/* Optional DMA support: get valid dma channel(s) or NULL */
+	for (i = STM32_TIMERS_DMA_CH1; i <= STM32_TIMERS_DMA_CH4; i++) {
+		snprintf(name, ARRAY_SIZE(name), "ch%1d", i + 1);
+		dma->chans[i] = dma_request_slave_channel(dev, name);
+	}
+	dma->chans[STM32_TIMERS_DMA_UP] =
+		dma_request_slave_channel(dev, "up");
+	dma->chans[STM32_TIMERS_DMA_TRIG] =
+		dma_request_slave_channel(dev, "trig");
+	dma->chans[STM32_TIMERS_DMA_COM] =
+		dma_request_slave_channel(dev, "com");
+	ddata->dma = dma;
+
+	return 0;
+}
+
+static void stm32_timers_dma_remove(struct device *dev,
+				    struct stm32_timers *ddata)
+{
+	int i;
+
+	for (i = STM32_TIMERS_DMA_CH1; i < STM32_TIMERS_MAX_DMAS; i++)
+		if (ddata->dma->chans[i])
+			dma_release_channel(ddata->dma->chans[i]);
+}
+
 static int stm32_timers_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct stm32_timers *ddata;
 	struct resource *res;
 	void __iomem *mmio;
+	int ret;
 
 	ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
 	if (!ddata)
@@ -54,9 +244,33 @@ static int stm32_timers_probe(struct platform_device *pdev)
 
 	stm32_timers_get_arr_size(ddata);
 
+	ret = stm32_timers_dma_probe(dev, ddata);
+	if (ret)
+		return ret;
+	/* timers physical addr for dma */
+	ddata->dma->phys_base = res->start;
+
 	platform_set_drvdata(pdev, ddata);
 
-	return devm_of_platform_populate(&pdev->dev);
+	ret = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+	if (ret)
+		stm32_timers_dma_remove(dev, ddata);
+
+	return ret;
+}
+
+static int stm32_timers_remove(struct platform_device *pdev)
+{
+	struct stm32_timers *ddata = platform_get_drvdata(pdev);
+
+	/*
+	 * Don't use devm_ here: enfore of_platform_depopulate() happens before
+	 * dma are released, to avoid race on dma.
+	 */
+	of_platform_depopulate(&pdev->dev);
+	stm32_timers_dma_remove(&pdev->dev, ddata);
+
+	return 0;
 }
 
 static const struct of_device_id stm32_timers_of_match[] = {
@@ -67,6 +281,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
 
 static struct platform_driver stm32_timers_driver = {
 	.probe = stm32_timers_probe,
+	.remove = stm32_timers_remove,
 	.driver	= {
 		.name = "stm32-timers",
 		.of_match_table = stm32_timers_of_match,
diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
index 2aadab6..585a4de 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -8,6 +8,8 @@
 #define _LINUX_STM32_GPTIMER_H_
 
 #include <linux/clk.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
 #include <linux/regmap.h>
 
 #define TIM_CR1		0x00	/* Control Register 1      */
@@ -27,6 +29,8 @@
 #define TIM_CCR3	0x3C	/* Capt/Comp Register 3    */
 #define TIM_CCR4	0x40	/* Capt/Comp Register 4    */
 #define TIM_BDTR	0x44	/* Break and Dead-Time Reg */
+#define TIM_DCR		0x48	/* DMA control register    */
+#define TIM_DMAR	0x4C	/* DMA register for transfer */
 
 #define TIM_CR1_CEN	BIT(0)	/* Counter Enable	   */
 #define TIM_CR1_DIR	BIT(4)  /* Counter Direction	   */
@@ -36,6 +40,13 @@
 #define TIM_SMCR_SMS	(BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
 #define TIM_SMCR_TS	(BIT(4) | BIT(5) | BIT(6)) /* Trigger selection */
 #define TIM_DIER_UIE	BIT(0)	/* Update interrupt	   */
+#define TIM_DIER_UDE	BIT(8)  /* Update DMA request Enable */
+#define TIM_DIER_CC1DE	BIT(9)  /* CC1 DMA request Enable  */
+#define TIM_DIER_CC2DE	BIT(10) /* CC2 DMA request Enable  */
+#define TIM_DIER_CC3DE	BIT(11) /* CC3 DMA request Enable  */
+#define TIM_DIER_CC4DE	BIT(12) /* CC4 DMA request Enable  */
+#define TIM_DIER_COMDE	BIT(13) /* COM DMA request Enable  */
+#define TIM_DIER_TDE	BIT(14) /* Trigger DMA request Enable */
 #define TIM_SR_UIF	BIT(0)	/* Update interrupt flag   */
 #define TIM_EGR_UG	BIT(0)	/* Update Generation       */
 #define TIM_CCMR_PE	BIT(3)	/* Channel Preload Enable  */
@@ -56,6 +67,8 @@
 #define TIM_BDTR_BK2F	(BIT(20) | BIT(21) | BIT(22) | BIT(23))
 #define TIM_BDTR_BK2E	BIT(24) /* Break 2 input enable	   */
 #define TIM_BDTR_BK2P	BIT(25) /* Break 2 input polarity  */
+#define TIM_DCR_DBA	GENMASK(4, 0)	/* DMA base addr */
+#define TIM_DCR_DBL	GENMASK(12, 8)	/* DMA burst len */
 
 #define MAX_TIM_PSC		0xFFFF
 #define TIM_CR2_MMS_SHIFT	4
@@ -65,9 +78,28 @@
 #define TIM_BDTR_BKF_SHIFT	16
 #define TIM_BDTR_BK2F_SHIFT	20
 
+enum stm32_timers_dmas {
+	STM32_TIMERS_DMA_CH1,
+	STM32_TIMERS_DMA_CH2,
+	STM32_TIMERS_DMA_CH3,
+	STM32_TIMERS_DMA_CH4,
+	STM32_TIMERS_DMA_UP,
+	STM32_TIMERS_DMA_TRIG,
+	STM32_TIMERS_DMA_COM,
+	STM32_TIMERS_MAX_DMAS,
+};
+
+struct stm32_timers_dma;
+
 struct stm32_timers {
 	struct clk *clk;
 	struct regmap *regmap;
 	u32 max_arr;
+	struct stm32_timers_dma *dma;
 };
+
+int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
+				enum stm32_timers_dmas id, u32 reg,
+				unsigned int num_reg, unsigned int bursts,
+				unsigned long tmo_ms);
 #endif
-- 
1.9.1

[PATCH v3 2/6] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

STM32 Timers can support up to 7 DMA requests:
- 4 channels, update, compare and trigger.
Optionally request part, or all DMAs from stm32-timers MFD core.

Also add routine to implement burst reads using DMA from timer registers.
This is exported. So, it can be used by child drivers, PWM capture
for instance (but not limited to).

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
Changes in v3:
- Basically Lee's comments:
- rather create a struct stm32_timers_dma, and place a reference to it
  in existing ddata (instead of adding priv struct).
- rather use a struct device in exported routine prototype, and use
  standard helpers instead of ddata. Get rid of to_stm32_timers_priv().
- simplify error handling in probe (remove a goto)
- comment on devm_of_platform_*populate() usage.

Changes in v2:
- Abstract DMA handling from child driver: move it to MFD core
- Add comments on optional dma support
---
 drivers/mfd/stm32-timers.c       | 219 ++++++++++++++++++++++++++++++++++++++-
 include/linux/mfd/stm32-timers.h |  32 ++++++
 2 files changed, 249 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
index 1d347e5..98191ec 100644
--- a/drivers/mfd/stm32-timers.c
+++ b/drivers/mfd/stm32-timers.c
@@ -4,16 +4,165 @@
  * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
  */
 
+#include <linux/bitfield.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
 #include <linux/mfd/stm32-timers.h>
 #include <linux/module.h>
 #include <linux/of_platform.h>
 #include <linux/reset.h>
 
+#define STM32_TIMERS_MAX_REGISTERS	0x3fc
+
+struct stm32_timers_dma {
+	struct completion completion;
+	phys_addr_t phys_base;
+	struct mutex lock;		/* protect dma access */
+	struct dma_chan *chan;
+	struct dma_chan *chans[STM32_TIMERS_MAX_DMAS];
+};
+
+/* DIER register DMA enable bits */
+static const u32 stm32_timers_dier_dmaen[STM32_TIMERS_MAX_DMAS] = {
+	TIM_DIER_CC1DE,
+	TIM_DIER_CC2DE,
+	TIM_DIER_CC3DE,
+	TIM_DIER_CC4DE,
+	TIM_DIER_UIE,
+	TIM_DIER_TDE,
+	TIM_DIER_COMDE
+};
+
+static void stm32_timers_dma_done(void *p)
+{
+	struct stm32_timers_dma *dma = p;
+	struct dma_tx_state state;
+	enum dma_status status;
+
+	status = dmaengine_tx_status(dma->chan, dma->chan->cookie, &state);
+	if (status == DMA_COMPLETE)
+		complete(&dma->completion);
+}
+
+/**
+ * stm32_timers_dma_burst_read - Read from timers registers using DMA.
+ *
+ * Read from STM32 timers registers using DMA on a single event.
+ * @dev: reference to stm32_timers MFD device
+ * @buf: dma'able destination buffer
+ * @id: stm32_timers_dmas event identifier (ch[1..4], up, trig or com)
+ * @reg: registers start offset for DMA to read from (like CCRx for capture)
+ * @num_reg: number of registers to read upon each dma request, starting @reg.
+ * @bursts: number of bursts to read (e.g. like two for pwm period capture)
+ * @tmo_ms: timeout (milliseconds)
+ */
+int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
+				enum stm32_timers_dmas id, u32 reg,
+				unsigned int num_reg, unsigned int bursts,
+				unsigned long tmo_ms)
+{
+	struct stm32_timers *ddata = dev_get_drvdata(dev);
+	unsigned long timeout = msecs_to_jiffies(tmo_ms);
+	struct regmap *regmap = ddata->regmap;
+	struct stm32_timers_dma *dma = ddata->dma;
+	size_t len = num_reg * bursts * sizeof(u32);
+	struct dma_async_tx_descriptor *desc;
+	struct dma_slave_config config;
+	dma_cookie_t cookie;
+	dma_addr_t dma_buf;
+	u32 dbl, dba;
+	long err;
+	int ret;
+
+	/* sanity check */
+	if (id < STM32_TIMERS_DMA_CH1 || id >= STM32_TIMERS_MAX_DMAS)
+		return -EINVAL;
+
+	if (!num_reg || !bursts || reg > STM32_TIMERS_MAX_REGISTERS ||
+	    (reg + num_reg * sizeof(u32)) > STM32_TIMERS_MAX_REGISTERS)
+		return -EINVAL;
+
+	if (!dma->chans[id])
+		return -ENODEV;
+	mutex_lock(&dma->lock);
+
+	/* select dma channel in use */
+	dma->chan = dma->chans[id];
+	dma_buf = dma_map_single(dev, buf, len, DMA_FROM_DEVICE);
+	ret = dma_mapping_error(dev, dma_buf);
+	if (ret)
+		goto unlock;
+
+	/* Prepare DMA read from timer registers, using DMA burst mode */
+	memset(&config, 0, sizeof(config));
+	config.src_addr = (dma_addr_t)dma->phys_base + TIM_DMAR;
+	config.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+	ret = dmaengine_slave_config(dma->chan, &config);
+	if (ret)
+		goto unmap;
+
+	desc = dmaengine_prep_slave_single(dma->chan, dma_buf, len,
+					   DMA_DEV_TO_MEM, DMA_PREP_INTERRUPT);
+	if (!desc) {
+		ret = -EBUSY;
+		goto unmap;
+	}
+
+	desc->callback = stm32_timers_dma_done;
+	desc->callback_param = dma;
+	cookie = dmaengine_submit(desc);
+	ret = dma_submit_error(cookie);
+	if (ret)
+		goto dma_term;
+
+	reinit_completion(&dma->completion);
+	dma_async_issue_pending(dma->chan);
+
+	/* Setup and enable timer DMA burst mode */
+	dbl = FIELD_PREP(TIM_DCR_DBL, bursts - 1);
+	dba = FIELD_PREP(TIM_DCR_DBA, reg >> 2);
+	ret = regmap_write(regmap, TIM_DCR, dbl | dba);
+	if (ret)
+		goto dma_term;
+
+	/* Clear pending flags before enabling DMA request */
+	ret = regmap_write(regmap, TIM_SR, 0);
+	if (ret)
+		goto dcr_clr;
+
+	ret = regmap_update_bits(regmap, TIM_DIER, stm32_timers_dier_dmaen[id],
+				 stm32_timers_dier_dmaen[id]);
+	if (ret)
+		goto dcr_clr;
+
+	err = wait_for_completion_interruptible_timeout(&dma->completion,
+							timeout);
+	if (err == 0)
+		ret = -ETIMEDOUT;
+	else if (err < 0)
+		ret = err;
+
+	regmap_update_bits(regmap, TIM_DIER, stm32_timers_dier_dmaen[id], 0);
+	regmap_write(regmap, TIM_SR, 0);
+dcr_clr:
+	regmap_write(regmap, TIM_DCR, 0);
+dma_term:
+	dmaengine_terminate_all(dma->chan);
+unmap:
+	dma_unmap_single(dev, dma_buf, len, DMA_FROM_DEVICE);
+unlock:
+	dma->chan = NULL;
+	mutex_unlock(&dma->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(stm32_timers_dma_burst_read);
+
 static const struct regmap_config stm32_timers_regmap_cfg = {
 	.reg_bits = 32,
 	.val_bits = 32,
 	.reg_stride = sizeof(u32),
-	.max_register = 0x3fc,
+	.max_register = STM32_TIMERS_MAX_REGISTERS,
 };
 
 static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
@@ -27,12 +176,53 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
 	regmap_write(ddata->regmap, TIM_ARR, 0x0);
 }
 
+static int stm32_timers_dma_probe(struct device *dev,
+				  struct stm32_timers *ddata)
+{
+	int i;
+	char name[4];
+	struct stm32_timers_dma *dma;
+
+	dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL);
+	if (!dma)
+		return -ENOMEM;
+
+	init_completion(&dma->completion);
+	mutex_init(&dma->lock);
+
+	/* Optional DMA support: get valid dma channel(s) or NULL */
+	for (i = STM32_TIMERS_DMA_CH1; i <= STM32_TIMERS_DMA_CH4; i++) {
+		snprintf(name, ARRAY_SIZE(name), "ch%1d", i + 1);
+		dma->chans[i] = dma_request_slave_channel(dev, name);
+	}
+	dma->chans[STM32_TIMERS_DMA_UP] =
+		dma_request_slave_channel(dev, "up");
+	dma->chans[STM32_TIMERS_DMA_TRIG] =
+		dma_request_slave_channel(dev, "trig");
+	dma->chans[STM32_TIMERS_DMA_COM] =
+		dma_request_slave_channel(dev, "com");
+	ddata->dma = dma;
+
+	return 0;
+}
+
+static void stm32_timers_dma_remove(struct device *dev,
+				    struct stm32_timers *ddata)
+{
+	int i;
+
+	for (i = STM32_TIMERS_DMA_CH1; i < STM32_TIMERS_MAX_DMAS; i++)
+		if (ddata->dma->chans[i])
+			dma_release_channel(ddata->dma->chans[i]);
+}
+
 static int stm32_timers_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct stm32_timers *ddata;
 	struct resource *res;
 	void __iomem *mmio;
+	int ret;
 
 	ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
 	if (!ddata)
@@ -54,9 +244,33 @@ static int stm32_timers_probe(struct platform_device *pdev)
 
 	stm32_timers_get_arr_size(ddata);
 
+	ret = stm32_timers_dma_probe(dev, ddata);
+	if (ret)
+		return ret;
+	/* timers physical addr for dma */
+	ddata->dma->phys_base = res->start;
+
 	platform_set_drvdata(pdev, ddata);
 
-	return devm_of_platform_populate(&pdev->dev);
+	ret = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+	if (ret)
+		stm32_timers_dma_remove(dev, ddata);
+
+	return ret;
+}
+
+static int stm32_timers_remove(struct platform_device *pdev)
+{
+	struct stm32_timers *ddata = platform_get_drvdata(pdev);
+
+	/*
+	 * Don't use devm_ here: enfore of_platform_depopulate() happens before
+	 * dma are released, to avoid race on dma.
+	 */
+	of_platform_depopulate(&pdev->dev);
+	stm32_timers_dma_remove(&pdev->dev, ddata);
+
+	return 0;
 }
 
 static const struct of_device_id stm32_timers_of_match[] = {
@@ -67,6 +281,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
 
 static struct platform_driver stm32_timers_driver = {
 	.probe = stm32_timers_probe,
+	.remove = stm32_timers_remove,
 	.driver	= {
 		.name = "stm32-timers",
 		.of_match_table = stm32_timers_of_match,
diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
index 2aadab6..585a4de 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -8,6 +8,8 @@
 #define _LINUX_STM32_GPTIMER_H_
 
 #include <linux/clk.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
 #include <linux/regmap.h>
 
 #define TIM_CR1		0x00	/* Control Register 1      */
@@ -27,6 +29,8 @@
 #define TIM_CCR3	0x3C	/* Capt/Comp Register 3    */
 #define TIM_CCR4	0x40	/* Capt/Comp Register 4    */
 #define TIM_BDTR	0x44	/* Break and Dead-Time Reg */
+#define TIM_DCR		0x48	/* DMA control register    */
+#define TIM_DMAR	0x4C	/* DMA register for transfer */
 
 #define TIM_CR1_CEN	BIT(0)	/* Counter Enable	   */
 #define TIM_CR1_DIR	BIT(4)  /* Counter Direction	   */
@@ -36,6 +40,13 @@
 #define TIM_SMCR_SMS	(BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
 #define TIM_SMCR_TS	(BIT(4) | BIT(5) | BIT(6)) /* Trigger selection */
 #define TIM_DIER_UIE	BIT(0)	/* Update interrupt	   */
+#define TIM_DIER_UDE	BIT(8)  /* Update DMA request Enable */
+#define TIM_DIER_CC1DE	BIT(9)  /* CC1 DMA request Enable  */
+#define TIM_DIER_CC2DE	BIT(10) /* CC2 DMA request Enable  */
+#define TIM_DIER_CC3DE	BIT(11) /* CC3 DMA request Enable  */
+#define TIM_DIER_CC4DE	BIT(12) /* CC4 DMA request Enable  */
+#define TIM_DIER_COMDE	BIT(13) /* COM DMA request Enable  */
+#define TIM_DIER_TDE	BIT(14) /* Trigger DMA request Enable */
 #define TIM_SR_UIF	BIT(0)	/* Update interrupt flag   */
 #define TIM_EGR_UG	BIT(0)	/* Update Generation       */
 #define TIM_CCMR_PE	BIT(3)	/* Channel Preload Enable  */
@@ -56,6 +67,8 @@
 #define TIM_BDTR_BK2F	(BIT(20) | BIT(21) | BIT(22) | BIT(23))
 #define TIM_BDTR_BK2E	BIT(24) /* Break 2 input enable	   */
 #define TIM_BDTR_BK2P	BIT(25) /* Break 2 input polarity  */
+#define TIM_DCR_DBA	GENMASK(4, 0)	/* DMA base addr */
+#define TIM_DCR_DBL	GENMASK(12, 8)	/* DMA burst len */
 
 #define MAX_TIM_PSC		0xFFFF
 #define TIM_CR2_MMS_SHIFT	4
@@ -65,9 +78,28 @@
 #define TIM_BDTR_BKF_SHIFT	16
 #define TIM_BDTR_BK2F_SHIFT	20
 
+enum stm32_timers_dmas {
+	STM32_TIMERS_DMA_CH1,
+	STM32_TIMERS_DMA_CH2,
+	STM32_TIMERS_DMA_CH3,
+	STM32_TIMERS_DMA_CH4,
+	STM32_TIMERS_DMA_UP,
+	STM32_TIMERS_DMA_TRIG,
+	STM32_TIMERS_DMA_COM,
+	STM32_TIMERS_MAX_DMAS,
+};
+
+struct stm32_timers_dma;
+
 struct stm32_timers {
 	struct clk *clk;
 	struct regmap *regmap;
 	u32 max_arr;
+	struct stm32_timers_dma *dma;
 };
+
+int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
+				enum stm32_timers_dmas id, u32 reg,
+				unsigned int num_reg, unsigned int bursts,
+				unsigned long tmo_ms);
 #endif
-- 
1.9.1

[PATCH v3 3/6] pwm: stm32: add capture support

[Fabrice Gasnier]

Add support for PMW input mode on pwm-stm32. STM32 timers support
period and duty cycle capture as long as they have at least two PWM
channels. One capture channel is used for period (rising-edge), one
for duty-cycle (falling-edge).
When there's only one channel available, only period can be captured.
Duty-cycle is simply zero'ed in such a case.

Capture requires exclusive access (e.g. no pwm output running at the
same time, to protect common prescaler).
Timer DMA burst mode (from MFD core) is being used, to take two
snapshots of capture registers (upon each period rising edge).

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Acked-by: Thierry Reding <thierry.reding@gmail.com>
---
Changes in v3:
- update stm32_timers_dma_burst_read() call: don't pass ddata structure,
  use MFD parent device structure instead since MFD core update.

Changes in v2:
- DMA handling has been moved to MFD core. Rework capture routines to
  use it.
---
 drivers/pwm/pwm-stm32.c          | 176 +++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/stm32-timers.h |  11 +++
 2 files changed, 187 insertions(+)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 3ac55df..00c6251 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -26,6 +26,7 @@ struct stm32_pwm {
 	struct regmap *regmap;
 	u32 max_arr;
 	bool have_complementary_output;
+	u32 capture[4] ____cacheline_aligned; /* DMA'able buffer */
 };
 
 struct stm32_breakinput {
@@ -63,6 +64,178 @@ static int write_ccrx(struct stm32_pwm *dev, int ch, u32 value)
 	return -EINVAL;
 }
 
+#define TIM_CCER_CC12P (TIM_CCER_CC1P | TIM_CCER_CC2P)
+#define TIM_CCER_CC12E (TIM_CCER_CC1E | TIM_CCER_CC2E)
+#define TIM_CCER_CC34P (TIM_CCER_CC3P | TIM_CCER_CC4P)
+#define TIM_CCER_CC34E (TIM_CCER_CC3E | TIM_CCER_CC4E)
+
+/*
+ * Capture using PWM input mode:
+ *                              ___          ___
+ * TI[1, 2, 3 or 4]: ........._|   |________|
+ *                             ^0  ^1       ^2
+ *                              .   .        .
+ *                              .   .        XXXXX
+ *                              .   .   XXXXX     |
+ *                              .  XXXXX     .    |
+ *                            XXXXX .        .    |
+ * COUNTER:        ______XXXXX  .   .        .    |_XXX
+ *                 start^       .   .        .        ^stop
+ *                      .       .   .        .
+ *                      v       v   .        v
+ *                                  v
+ * CCR1/CCR3:       tx..........t0...........t2
+ * CCR2/CCR4:       tx..............t1.........
+ *
+ * DMA burst transfer:          |            |
+ *                              v            v
+ * DMA buffer:                  { t0, tx }   { t2, t1 }
+ * DMA done:                                 ^
+ *
+ * 0: IC1/3 snapchot on rising edge: counter value -> CCR1/CCR3
+ *    + DMA transfer CCR[1/3] & CCR[2/4] values (t0, tx: doesn't care)
+ * 1: IC2/4 snapchot on falling edge: counter value -> CCR2/CCR4
+ * 2: IC1/3 snapchot on rising edge: counter value -> CCR1/CCR3
+ *    + DMA transfer CCR[1/3] & CCR[2/4] values (t2, t1)
+ *
+ * DMA done, compute:
+ * - Period     = t2 - t0
+ * - Duty cycle = t1 - t0
+ */
+static int stm32_pwm_raw_capture(struct stm32_pwm *priv, struct pwm_device *pwm,
+				 unsigned long tmo_ms, u32 *raw_prd,
+				 u32 *raw_dty)
+{
+	struct device *parent = priv->chip.dev->parent;
+	enum stm32_timers_dmas dma_id;
+	u32 ccen, ccr;
+	int ret;
+
+	/* Ensure registers have been updated, enable counter and capture */
+	regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, TIM_CR1_CEN);
+
+	/* Use cc1 or cc3 DMA resp for PWM input channels 1 & 2 or 3 & 4 */
+	dma_id = pwm->hwpwm < 2 ? STM32_TIMERS_DMA_CH1 : STM32_TIMERS_DMA_CH3;
+	ccen = pwm->hwpwm < 2 ? TIM_CCER_CC12E : TIM_CCER_CC34E;
+	ccr = pwm->hwpwm < 2 ? TIM_CCR1 : TIM_CCR3;
+	regmap_update_bits(priv->regmap, TIM_CCER, ccen, ccen);
+
+	/*
+	 * Timer DMA burst mode. Request 2 registers, 2 bursts, to get both
+	 * CCR1 & CCR2 (or CCR3 & CCR4) on each capture event.
+	 * We'll get two capture snapchots: { CCR1, CCR2 }, { CCR1, CCR2 }
+	 * or { CCR3, CCR4 }, { CCR3, CCR4 }
+	 */
+	ret = stm32_timers_dma_burst_read(parent, priv->capture, dma_id, ccr, 2,
+					  2, tmo_ms);
+	if (ret)
+		goto stop;
+
+	/* Period: t2 - t0 (take care of counter overflow) */
+	if (priv->capture[0] <= priv->capture[2])
+		*raw_prd = priv->capture[2] - priv->capture[0];
+	else
+		*raw_prd = priv->max_arr - priv->capture[0] + priv->capture[2];
+
+	/* Duty cycle capture requires at least two capture units */
+	if (pwm->chip->npwm < 2)
+		*raw_dty = 0;
+	else if (priv->capture[0] <= priv->capture[3])
+		*raw_dty = priv->capture[3] - priv->capture[0];
+	else
+		*raw_dty = priv->max_arr - priv->capture[0] + priv->capture[3];
+
+	if (*raw_dty > *raw_prd) {
+		/*
+		 * Race beetween PWM input and DMA: it may happen
+		 * falling edge triggers new capture on TI2/4 before DMA
+		 * had a chance to read CCR2/4. It means capture[1]
+		 * contains period + duty_cycle. So, subtract period.
+		 */
+		*raw_dty -= *raw_prd;
+	}
+
+stop:
+	regmap_update_bits(priv->regmap, TIM_CCER, ccen, 0);
+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, 0);
+
+	return ret;
+}
+
+static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
+			     struct pwm_capture *result, unsigned long tmo_ms)
+{
+	struct stm32_pwm *priv = to_stm32_pwm_dev(chip);
+	unsigned long long prd, div, dty;
+	unsigned long rate;
+	unsigned int psc = 0;
+	u32 raw_prd, raw_dty;
+	int ret = 0;
+
+	mutex_lock(&priv->lock);
+
+	if (active_channels(priv)) {
+		ret = -EBUSY;
+		goto unlock;
+	}
+
+	ret = clk_enable(priv->clk);
+	if (ret) {
+		dev_err(priv->chip.dev, "failed to enable counter clock\n");
+		goto unlock;
+	}
+
+	rate = clk_get_rate(priv->clk);
+	if (!rate) {
+		ret = -EINVAL;
+		goto clk_dis;
+	}
+
+	/* prescaler: fit timeout window provided by upper layer */
+	div = (unsigned long long)rate * (unsigned long long)tmo_ms;
+	do_div(div, MSEC_PER_SEC);
+	prd = div;
+	while ((div > priv->max_arr) && (psc < MAX_TIM_PSC)) {
+		psc++;
+		div = prd;
+		do_div(div, psc + 1);
+	}
+	regmap_write(priv->regmap, TIM_ARR, priv->max_arr);
+	regmap_write(priv->regmap, TIM_PSC, psc);
+
+	/* Map TI1 or TI2 PWM input to IC1 & IC2 (or TI3/4 to IC3 & IC4) */
+	regmap_update_bits(priv->regmap,
+			   pwm->hwpwm < 2 ? TIM_CCMR1 : TIM_CCMR2,
+			   TIM_CCMR_CC1S | TIM_CCMR_CC2S, pwm->hwpwm & 0x1 ?
+			   TIM_CCMR_CC1S_TI2 | TIM_CCMR_CC2S_TI2 :
+			   TIM_CCMR_CC1S_TI1 | TIM_CCMR_CC2S_TI1);
+
+	/* Capture period on IC1/3 rising edge, duty cycle on IC2/4 falling. */
+	regmap_update_bits(priv->regmap, TIM_CCER, pwm->hwpwm < 2 ?
+			   TIM_CCER_CC12P : TIM_CCER_CC34P, pwm->hwpwm < 2 ?
+			   TIM_CCER_CC2P : TIM_CCER_CC4P);
+
+	ret = stm32_pwm_raw_capture(priv, pwm, tmo_ms, &raw_prd, &raw_dty);
+	if (ret)
+		goto stop;
+
+	prd = (unsigned long long)raw_prd * (psc + 1) * NSEC_PER_SEC;
+	result->period = DIV_ROUND_UP_ULL(prd, rate);
+	dty = (unsigned long long)raw_dty * (psc + 1) * NSEC_PER_SEC;
+	result->duty_cycle = DIV_ROUND_UP_ULL(dty, rate);
+stop:
+	regmap_write(priv->regmap, TIM_CCER, 0);
+	regmap_write(priv->regmap, pwm->hwpwm < 2 ? TIM_CCMR1 : TIM_CCMR2, 0);
+	regmap_write(priv->regmap, TIM_PSC, 0);
+clk_dis:
+	clk_disable(priv->clk);
+unlock:
+	mutex_unlock(&priv->lock);
+
+	return ret;
+}
+
 static int stm32_pwm_config(struct stm32_pwm *priv, int ch,
 			    int duty_ns, int period_ns)
 {
@@ -231,6 +404,9 @@ static int stm32_pwm_apply_locked(struct pwm_chip *chip, struct pwm_device *pwm,
 static const struct pwm_ops stm32pwm_ops = {
 	.owner = THIS_MODULE,
 	.apply = stm32_pwm_apply_locked,
+#if IS_ENABLED(CONFIG_DMA_ENGINE)
+	.capture = stm32_pwm_capture,
+#endif
 };
 
 static int stm32_pwm_set_breakinput(struct stm32_pwm *priv,
diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
index 585a4de..3c72b70 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -51,13 +51,24 @@
 #define TIM_EGR_UG	BIT(0)	/* Update Generation       */
 #define TIM_CCMR_PE	BIT(3)	/* Channel Preload Enable  */
 #define TIM_CCMR_M1	(BIT(6) | BIT(5))  /* Channel PWM Mode 1 */
+#define TIM_CCMR_CC1S		(BIT(0) | BIT(1)) /* Capture/compare 1 sel */
+#define TIM_CCMR_IC1PSC		GENMASK(3, 2)	/* Input capture 1 prescaler */
+#define TIM_CCMR_CC2S		(BIT(8) | BIT(9)) /* Capture/compare 2 sel */
+#define TIM_CCMR_IC2PSC		GENMASK(11, 10)	/* Input capture 2 prescaler */
+#define TIM_CCMR_CC1S_TI1	BIT(0)	/* IC1/IC3 selects TI1/TI3 */
+#define TIM_CCMR_CC1S_TI2	BIT(1)	/* IC1/IC3 selects TI2/TI4 */
+#define TIM_CCMR_CC2S_TI2	BIT(8)	/* IC2/IC4 selects TI2/TI4 */
+#define TIM_CCMR_CC2S_TI1	BIT(9)	/* IC2/IC4 selects TI1/TI3 */
 #define TIM_CCER_CC1E	BIT(0)	/* Capt/Comp 1  out Ena    */
 #define TIM_CCER_CC1P	BIT(1)	/* Capt/Comp 1  Polarity   */
 #define TIM_CCER_CC1NE	BIT(2)	/* Capt/Comp 1N out Ena    */
 #define TIM_CCER_CC1NP	BIT(3)	/* Capt/Comp 1N Polarity   */
 #define TIM_CCER_CC2E	BIT(4)	/* Capt/Comp 2  out Ena    */
+#define TIM_CCER_CC2P	BIT(5)	/* Capt/Comp 2  Polarity   */
 #define TIM_CCER_CC3E	BIT(8)	/* Capt/Comp 3  out Ena    */
+#define TIM_CCER_CC3P	BIT(9)	/* Capt/Comp 3  Polarity   */
 #define TIM_CCER_CC4E	BIT(12)	/* Capt/Comp 4  out Ena    */
+#define TIM_CCER_CC4P	BIT(13)	/* Capt/Comp 4  Polarity   */
 #define TIM_CCER_CCXE	(BIT(0) | BIT(4) | BIT(8) | BIT(12))
 #define TIM_BDTR_BKE	BIT(12) /* Break input enable	   */
 #define TIM_BDTR_BKP	BIT(13) /* Break input polarity	   */
-- 
1.9.1

[PATCH v3 3/6] pwm: stm32: add capture support

[Fabrice Gasnier]

Add support for PMW input mode on pwm-stm32. STM32 timers support
period and duty cycle capture as long as they have at least two PWM
channels. One capture channel is used for period (rising-edge), one
for duty-cycle (falling-edge).
When there's only one channel available, only period can be captured.
Duty-cycle is simply zero'ed in such a case.

Capture requires exclusive access (e.g. no pwm output running at the
same time, to protect common prescaler).
Timer DMA burst mode (from MFD core) is being used, to take two
snapshots of capture registers (upon each period rising edge).

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Acked-by: Thierry Reding <thierry.reding@gmail.com>
---
Changes in v3:
- update stm32_timers_dma_burst_read() call: don't pass ddata structure,
  use MFD parent device structure instead since MFD core update.

Changes in v2:
- DMA handling has been moved to MFD core. Rework capture routines to
  use it.
---
 drivers/pwm/pwm-stm32.c          | 176 +++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/stm32-timers.h |  11 +++
 2 files changed, 187 insertions(+)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 3ac55df..00c6251 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -26,6 +26,7 @@ struct stm32_pwm {
 	struct regmap *regmap;
 	u32 max_arr;
 	bool have_complementary_output;
+	u32 capture[4] ____cacheline_aligned; /* DMA'able buffer */
 };
 
 struct stm32_breakinput {
@@ -63,6 +64,178 @@ static int write_ccrx(struct stm32_pwm *dev, int ch, u32 value)
 	return -EINVAL;
 }
 
+#define TIM_CCER_CC12P (TIM_CCER_CC1P | TIM_CCER_CC2P)
+#define TIM_CCER_CC12E (TIM_CCER_CC1E | TIM_CCER_CC2E)
+#define TIM_CCER_CC34P (TIM_CCER_CC3P | TIM_CCER_CC4P)
+#define TIM_CCER_CC34E (TIM_CCER_CC3E | TIM_CCER_CC4E)
+
+/*
+ * Capture using PWM input mode:
+ *                              ___          ___
+ * TI[1, 2, 3 or 4]: ........._|   |________|
+ *                             ^0  ^1       ^2
+ *                              .   .        .
+ *                              .   .        XXXXX
+ *                              .   .   XXXXX     |
+ *                              .  XXXXX     .    |
+ *                            XXXXX .        .    |
+ * COUNTER:        ______XXXXX  .   .        .    |_XXX
+ *                 start^       .   .        .        ^stop
+ *                      .       .   .        .
+ *                      v       v   .        v
+ *                                  v
+ * CCR1/CCR3:       tx..........t0...........t2
+ * CCR2/CCR4:       tx..............t1.........
+ *
+ * DMA burst transfer:          |            |
+ *                              v            v
+ * DMA buffer:                  { t0, tx }   { t2, t1 }
+ * DMA done:                                 ^
+ *
+ * 0: IC1/3 snapchot on rising edge: counter value -> CCR1/CCR3
+ *    + DMA transfer CCR[1/3] & CCR[2/4] values (t0, tx: doesn't care)
+ * 1: IC2/4 snapchot on falling edge: counter value -> CCR2/CCR4
+ * 2: IC1/3 snapchot on rising edge: counter value -> CCR1/CCR3
+ *    + DMA transfer CCR[1/3] & CCR[2/4] values (t2, t1)
+ *
+ * DMA done, compute:
+ * - Period     = t2 - t0
+ * - Duty cycle = t1 - t0
+ */
+static int stm32_pwm_raw_capture(struct stm32_pwm *priv, struct pwm_device *pwm,
+				 unsigned long tmo_ms, u32 *raw_prd,
+				 u32 *raw_dty)
+{
+	struct device *parent = priv->chip.dev->parent;
+	enum stm32_timers_dmas dma_id;
+	u32 ccen, ccr;
+	int ret;
+
+	/* Ensure registers have been updated, enable counter and capture */
+	regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, TIM_CR1_CEN);
+
+	/* Use cc1 or cc3 DMA resp for PWM input channels 1 & 2 or 3 & 4 */
+	dma_id = pwm->hwpwm < 2 ? STM32_TIMERS_DMA_CH1 : STM32_TIMERS_DMA_CH3;
+	ccen = pwm->hwpwm < 2 ? TIM_CCER_CC12E : TIM_CCER_CC34E;
+	ccr = pwm->hwpwm < 2 ? TIM_CCR1 : TIM_CCR3;
+	regmap_update_bits(priv->regmap, TIM_CCER, ccen, ccen);
+
+	/*
+	 * Timer DMA burst mode. Request 2 registers, 2 bursts, to get both
+	 * CCR1 & CCR2 (or CCR3 & CCR4) on each capture event.
+	 * We'll get two capture snapchots: { CCR1, CCR2 }, { CCR1, CCR2 }
+	 * or { CCR3, CCR4 }, { CCR3, CCR4 }
+	 */
+	ret = stm32_timers_dma_burst_read(parent, priv->capture, dma_id, ccr, 2,
+					  2, tmo_ms);
+	if (ret)
+		goto stop;
+
+	/* Period: t2 - t0 (take care of counter overflow) */
+	if (priv->capture[0] <= priv->capture[2])
+		*raw_prd = priv->capture[2] - priv->capture[0];
+	else
+		*raw_prd = priv->max_arr - priv->capture[0] + priv->capture[2];
+
+	/* Duty cycle capture requires at least two capture units */
+	if (pwm->chip->npwm < 2)
+		*raw_dty = 0;
+	else if (priv->capture[0] <= priv->capture[3])
+		*raw_dty = priv->capture[3] - priv->capture[0];
+	else
+		*raw_dty = priv->max_arr - priv->capture[0] + priv->capture[3];
+
+	if (*raw_dty > *raw_prd) {
+		/*
+		 * Race beetween PWM input and DMA: it may happen
+		 * falling edge triggers new capture on TI2/4 before DMA
+		 * had a chance to read CCR2/4. It means capture[1]
+		 * contains period + duty_cycle. So, subtract period.
+		 */
+		*raw_dty -= *raw_prd;
+	}
+
+stop:
+	regmap_update_bits(priv->regmap, TIM_CCER, ccen, 0);
+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, 0);
+
+	return ret;
+}
+
+static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
+			     struct pwm_capture *result, unsigned long tmo_ms)
+{
+	struct stm32_pwm *priv = to_stm32_pwm_dev(chip);
+	unsigned long long prd, div, dty;
+	unsigned long rate;
+	unsigned int psc = 0;
+	u32 raw_prd, raw_dty;
+	int ret = 0;
+
+	mutex_lock(&priv->lock);
+
+	if (active_channels(priv)) {
+		ret = -EBUSY;
+		goto unlock;
+	}
+
+	ret = clk_enable(priv->clk);
+	if (ret) {
+		dev_err(priv->chip.dev, "failed to enable counter clock\n");
+		goto unlock;
+	}
+
+	rate = clk_get_rate(priv->clk);
+	if (!rate) {
+		ret = -EINVAL;
+		goto clk_dis;
+	}
+
+	/* prescaler: fit timeout window provided by upper layer */
+	div = (unsigned long long)rate * (unsigned long long)tmo_ms;
+	do_div(div, MSEC_PER_SEC);
+	prd = div;
+	while ((div > priv->max_arr) && (psc < MAX_TIM_PSC)) {
+		psc++;
+		div = prd;
+		do_div(div, psc + 1);
+	}
+	regmap_write(priv->regmap, TIM_ARR, priv->max_arr);
+	regmap_write(priv->regmap, TIM_PSC, psc);
+
+	/* Map TI1 or TI2 PWM input to IC1 & IC2 (or TI3/4 to IC3 & IC4) */
+	regmap_update_bits(priv->regmap,
+			   pwm->hwpwm < 2 ? TIM_CCMR1 : TIM_CCMR2,
+			   TIM_CCMR_CC1S | TIM_CCMR_CC2S, pwm->hwpwm & 0x1 ?
+			   TIM_CCMR_CC1S_TI2 | TIM_CCMR_CC2S_TI2 :
+			   TIM_CCMR_CC1S_TI1 | TIM_CCMR_CC2S_TI1);
+
+	/* Capture period on IC1/3 rising edge, duty cycle on IC2/4 falling. */
+	regmap_update_bits(priv->regmap, TIM_CCER, pwm->hwpwm < 2 ?
+			   TIM_CCER_CC12P : TIM_CCER_CC34P, pwm->hwpwm < 2 ?
+			   TIM_CCER_CC2P : TIM_CCER_CC4P);
+
+	ret = stm32_pwm_raw_capture(priv, pwm, tmo_ms, &raw_prd, &raw_dty);
+	if (ret)
+		goto stop;
+
+	prd = (unsigned long long)raw_prd * (psc + 1) * NSEC_PER_SEC;
+	result->period = DIV_ROUND_UP_ULL(prd, rate);
+	dty = (unsigned long long)raw_dty * (psc + 1) * NSEC_PER_SEC;
+	result->duty_cycle = DIV_ROUND_UP_ULL(dty, rate);
+stop:
+	regmap_write(priv->regmap, TIM_CCER, 0);
+	regmap_write(priv->regmap, pwm->hwpwm < 2 ? TIM_CCMR1 : TIM_CCMR2, 0);
+	regmap_write(priv->regmap, TIM_PSC, 0);
+clk_dis:
+	clk_disable(priv->clk);
+unlock:
+	mutex_unlock(&priv->lock);
+
+	return ret;
+}
+
 static int stm32_pwm_config(struct stm32_pwm *priv, int ch,
 			    int duty_ns, int period_ns)
 {
@@ -231,6 +404,9 @@ static int stm32_pwm_apply_locked(struct pwm_chip *chip, struct pwm_device *pwm,
 static const struct pwm_ops stm32pwm_ops = {
 	.owner = THIS_MODULE,
 	.apply = stm32_pwm_apply_locked,
+#if IS_ENABLED(CONFIG_DMA_ENGINE)
+	.capture = stm32_pwm_capture,
+#endif
 };
 
 static int stm32_pwm_set_breakinput(struct stm32_pwm *priv,
diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
index 585a4de..3c72b70 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -51,13 +51,24 @@
 #define TIM_EGR_UG	BIT(0)	/* Update Generation       */
 #define TIM_CCMR_PE	BIT(3)	/* Channel Preload Enable  */
 #define TIM_CCMR_M1	(BIT(6) | BIT(5))  /* Channel PWM Mode 1 */
+#define TIM_CCMR_CC1S		(BIT(0) | BIT(1)) /* Capture/compare 1 sel */
+#define TIM_CCMR_IC1PSC		GENMASK(3, 2)	/* Input capture 1 prescaler */
+#define TIM_CCMR_CC2S		(BIT(8) | BIT(9)) /* Capture/compare 2 sel */
+#define TIM_CCMR_IC2PSC		GENMASK(11, 10)	/* Input capture 2 prescaler */
+#define TIM_CCMR_CC1S_TI1	BIT(0)	/* IC1/IC3 selects TI1/TI3 */
+#define TIM_CCMR_CC1S_TI2	BIT(1)	/* IC1/IC3 selects TI2/TI4 */
+#define TIM_CCMR_CC2S_TI2	BIT(8)	/* IC2/IC4 selects TI2/TI4 */
+#define TIM_CCMR_CC2S_TI1	BIT(9)	/* IC2/IC4 selects TI1/TI3 */
 #define TIM_CCER_CC1E	BIT(0)	/* Capt/Comp 1  out Ena    */
 #define TIM_CCER_CC1P	BIT(1)	/* Capt/Comp 1  Polarity   */
 #define TIM_CCER_CC1NE	BIT(2)	/* Capt/Comp 1N out Ena    */
 #define TIM_CCER_CC1NP	BIT(3)	/* Capt/Comp 1N Polarity   */
 #define TIM_CCER_CC2E	BIT(4)	/* Capt/Comp 2  out Ena    */
+#define TIM_CCER_CC2P	BIT(5)	/* Capt/Comp 2  Polarity   */
 #define TIM_CCER_CC3E	BIT(8)	/* Capt/Comp 3  out Ena    */
+#define TIM_CCER_CC3P	BIT(9)	/* Capt/Comp 3  Polarity   */
 #define TIM_CCER_CC4E	BIT(12)	/* Capt/Comp 4  out Ena    */
+#define TIM_CCER_CC4P	BIT(13)	/* Capt/Comp 4  Polarity   */
 #define TIM_CCER_CCXE	(BIT(0) | BIT(4) | BIT(8) | BIT(12))
 #define TIM_BDTR_BKE	BIT(12) /* Break input enable	   */
 #define TIM_BDTR_BKP	BIT(13) /* Break input polarity	   */
-- 
1.9.1

[PATCH v3 3/6] pwm: stm32: add capture support

[Fabrice Gasnier]

Add support for PMW input mode on pwm-stm32. STM32 timers support
period and duty cycle capture as long as they have at least two PWM
channels. One capture channel is used for period (rising-edge), one
for duty-cycle (falling-edge).
When there's only one channel available, only period can be captured.
Duty-cycle is simply zero'ed in such a case.

Capture requires exclusive access (e.g. no pwm output running at the
same time, to protect common prescaler).
Timer DMA burst mode (from MFD core) is being used, to take two
snapshots of capture registers (upon each period rising edge).

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Acked-by: Thierry Reding <thierry.reding@gmail.com>
---
Changes in v3:
- update stm32_timers_dma_burst_read() call: don't pass ddata structure,
  use MFD parent device structure instead since MFD core update.

Changes in v2:
- DMA handling has been moved to MFD core. Rework capture routines to
  use it.
---
 drivers/pwm/pwm-stm32.c          | 176 +++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/stm32-timers.h |  11 +++
 2 files changed, 187 insertions(+)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 3ac55df..00c6251 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -26,6 +26,7 @@ struct stm32_pwm {
 	struct regmap *regmap;
 	u32 max_arr;
 	bool have_complementary_output;
+	u32 capture[4] ____cacheline_aligned; /* DMA'able buffer */
 };
 
 struct stm32_breakinput {
@@ -63,6 +64,178 @@ static int write_ccrx(struct stm32_pwm *dev, int ch, u32 value)
 	return -EINVAL;
 }
 
+#define TIM_CCER_CC12P (TIM_CCER_CC1P | TIM_CCER_CC2P)
+#define TIM_CCER_CC12E (TIM_CCER_CC1E | TIM_CCER_CC2E)
+#define TIM_CCER_CC34P (TIM_CCER_CC3P | TIM_CCER_CC4P)
+#define TIM_CCER_CC34E (TIM_CCER_CC3E | TIM_CCER_CC4E)
+
+/*
+ * Capture using PWM input mode:
+ *                              ___          ___
+ * TI[1, 2, 3 or 4]: ........._|   |________|
+ *                             ^0  ^1       ^2
+ *                              .   .        .
+ *                              .   .        XXXXX
+ *                              .   .   XXXXX     |
+ *                              .  XXXXX     .    |
+ *                            XXXXX .        .    |
+ * COUNTER:        ______XXXXX  .   .        .    |_XXX
+ *                 start^       .   .        .        ^stop
+ *                      .       .   .        .
+ *                      v       v   .        v
+ *                                  v
+ * CCR1/CCR3:       tx..........t0...........t2
+ * CCR2/CCR4:       tx..............t1.........
+ *
+ * DMA burst transfer:          |            |
+ *                              v            v
+ * DMA buffer:                  { t0, tx }   { t2, t1 }
+ * DMA done:                                 ^
+ *
+ * 0: IC1/3 snapchot on rising edge: counter value -> CCR1/CCR3
+ *    + DMA transfer CCR[1/3] & CCR[2/4] values (t0, tx: doesn't care)
+ * 1: IC2/4 snapchot on falling edge: counter value -> CCR2/CCR4
+ * 2: IC1/3 snapchot on rising edge: counter value -> CCR1/CCR3
+ *    + DMA transfer CCR[1/3] & CCR[2/4] values (t2, t1)
+ *
+ * DMA done, compute:
+ * - Period     = t2 - t0
+ * - Duty cycle = t1 - t0
+ */
+static int stm32_pwm_raw_capture(struct stm32_pwm *priv, struct pwm_device *pwm,
+				 unsigned long tmo_ms, u32 *raw_prd,
+				 u32 *raw_dty)
+{
+	struct device *parent = priv->chip.dev->parent;
+	enum stm32_timers_dmas dma_id;
+	u32 ccen, ccr;
+	int ret;
+
+	/* Ensure registers have been updated, enable counter and capture */
+	regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, TIM_CR1_CEN);
+
+	/* Use cc1 or cc3 DMA resp for PWM input channels 1 & 2 or 3 & 4 */
+	dma_id = pwm->hwpwm < 2 ? STM32_TIMERS_DMA_CH1 : STM32_TIMERS_DMA_CH3;
+	ccen = pwm->hwpwm < 2 ? TIM_CCER_CC12E : TIM_CCER_CC34E;
+	ccr = pwm->hwpwm < 2 ? TIM_CCR1 : TIM_CCR3;
+	regmap_update_bits(priv->regmap, TIM_CCER, ccen, ccen);
+
+	/*
+	 * Timer DMA burst mode. Request 2 registers, 2 bursts, to get both
+	 * CCR1 & CCR2 (or CCR3 & CCR4) on each capture event.
+	 * We'll get two capture snapchots: { CCR1, CCR2 }, { CCR1, CCR2 }
+	 * or { CCR3, CCR4 }, { CCR3, CCR4 }
+	 */
+	ret = stm32_timers_dma_burst_read(parent, priv->capture, dma_id, ccr, 2,
+					  2, tmo_ms);
+	if (ret)
+		goto stop;
+
+	/* Period: t2 - t0 (take care of counter overflow) */
+	if (priv->capture[0] <= priv->capture[2])
+		*raw_prd = priv->capture[2] - priv->capture[0];
+	else
+		*raw_prd = priv->max_arr - priv->capture[0] + priv->capture[2];
+
+	/* Duty cycle capture requires at least two capture units */
+	if (pwm->chip->npwm < 2)
+		*raw_dty = 0;
+	else if (priv->capture[0] <= priv->capture[3])
+		*raw_dty = priv->capture[3] - priv->capture[0];
+	else
+		*raw_dty = priv->max_arr - priv->capture[0] + priv->capture[3];
+
+	if (*raw_dty > *raw_prd) {
+		/*
+		 * Race beetween PWM input and DMA: it may happen
+		 * falling edge triggers new capture on TI2/4 before DMA
+		 * had a chance to read CCR2/4. It means capture[1]
+		 * contains period + duty_cycle. So, subtract period.
+		 */
+		*raw_dty -= *raw_prd;
+	}
+
+stop:
+	regmap_update_bits(priv->regmap, TIM_CCER, ccen, 0);
+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, 0);
+
+	return ret;
+}
+
+static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
+			     struct pwm_capture *result, unsigned long tmo_ms)
+{
+	struct stm32_pwm *priv = to_stm32_pwm_dev(chip);
+	unsigned long long prd, div, dty;
+	unsigned long rate;
+	unsigned int psc = 0;
+	u32 raw_prd, raw_dty;
+	int ret = 0;
+
+	mutex_lock(&priv->lock);
+
+	if (active_channels(priv)) {
+		ret = -EBUSY;
+		goto unlock;
+	}
+
+	ret = clk_enable(priv->clk);
+	if (ret) {
+		dev_err(priv->chip.dev, "failed to enable counter clock\n");
+		goto unlock;
+	}
+
+	rate = clk_get_rate(priv->clk);
+	if (!rate) {
+		ret = -EINVAL;
+		goto clk_dis;
+	}
+
+	/* prescaler: fit timeout window provided by upper layer */
+	div = (unsigned long long)rate * (unsigned long long)tmo_ms;
+	do_div(div, MSEC_PER_SEC);
+	prd = div;
+	while ((div > priv->max_arr) && (psc < MAX_TIM_PSC)) {
+		psc++;
+		div = prd;
+		do_div(div, psc + 1);
+	}
+	regmap_write(priv->regmap, TIM_ARR, priv->max_arr);
+	regmap_write(priv->regmap, TIM_PSC, psc);
+
+	/* Map TI1 or TI2 PWM input to IC1 & IC2 (or TI3/4 to IC3 & IC4) */
+	regmap_update_bits(priv->regmap,
+			   pwm->hwpwm < 2 ? TIM_CCMR1 : TIM_CCMR2,
+			   TIM_CCMR_CC1S | TIM_CCMR_CC2S, pwm->hwpwm & 0x1 ?
+			   TIM_CCMR_CC1S_TI2 | TIM_CCMR_CC2S_TI2 :
+			   TIM_CCMR_CC1S_TI1 | TIM_CCMR_CC2S_TI1);
+
+	/* Capture period on IC1/3 rising edge, duty cycle on IC2/4 falling. */
+	regmap_update_bits(priv->regmap, TIM_CCER, pwm->hwpwm < 2 ?
+			   TIM_CCER_CC12P : TIM_CCER_CC34P, pwm->hwpwm < 2 ?
+			   TIM_CCER_CC2P : TIM_CCER_CC4P);
+
+	ret = stm32_pwm_raw_capture(priv, pwm, tmo_ms, &raw_prd, &raw_dty);
+	if (ret)
+		goto stop;
+
+	prd = (unsigned long long)raw_prd * (psc + 1) * NSEC_PER_SEC;
+	result->period = DIV_ROUND_UP_ULL(prd, rate);
+	dty = (unsigned long long)raw_dty * (psc + 1) * NSEC_PER_SEC;
+	result->duty_cycle = DIV_ROUND_UP_ULL(dty, rate);
+stop:
+	regmap_write(priv->regmap, TIM_CCER, 0);
+	regmap_write(priv->regmap, pwm->hwpwm < 2 ? TIM_CCMR1 : TIM_CCMR2, 0);
+	regmap_write(priv->regmap, TIM_PSC, 0);
+clk_dis:
+	clk_disable(priv->clk);
+unlock:
+	mutex_unlock(&priv->lock);
+
+	return ret;
+}
+
 static int stm32_pwm_config(struct stm32_pwm *priv, int ch,
 			    int duty_ns, int period_ns)
 {
@@ -231,6 +404,9 @@ static int stm32_pwm_apply_locked(struct pwm_chip *chip, struct pwm_device *pwm,
 static const struct pwm_ops stm32pwm_ops = {
 	.owner = THIS_MODULE,
 	.apply = stm32_pwm_apply_locked,
+#if IS_ENABLED(CONFIG_DMA_ENGINE)
+	.capture = stm32_pwm_capture,
+#endif
 };
 
 static int stm32_pwm_set_breakinput(struct stm32_pwm *priv,
diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
index 585a4de..3c72b70 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -51,13 +51,24 @@
 #define TIM_EGR_UG	BIT(0)	/* Update Generation       */
 #define TIM_CCMR_PE	BIT(3)	/* Channel Preload Enable  */
 #define TIM_CCMR_M1	(BIT(6) | BIT(5))  /* Channel PWM Mode 1 */
+#define TIM_CCMR_CC1S		(BIT(0) | BIT(1)) /* Capture/compare 1 sel */
+#define TIM_CCMR_IC1PSC		GENMASK(3, 2)	/* Input capture 1 prescaler */
+#define TIM_CCMR_CC2S		(BIT(8) | BIT(9)) /* Capture/compare 2 sel */
+#define TIM_CCMR_IC2PSC		GENMASK(11, 10)	/* Input capture 2 prescaler */
+#define TIM_CCMR_CC1S_TI1	BIT(0)	/* IC1/IC3 selects TI1/TI3 */
+#define TIM_CCMR_CC1S_TI2	BIT(1)	/* IC1/IC3 selects TI2/TI4 */
+#define TIM_CCMR_CC2S_TI2	BIT(8)	/* IC2/IC4 selects TI2/TI4 */
+#define TIM_CCMR_CC2S_TI1	BIT(9)	/* IC2/IC4 selects TI1/TI3 */
 #define TIM_CCER_CC1E	BIT(0)	/* Capt/Comp 1  out Ena    */
 #define TIM_CCER_CC1P	BIT(1)	/* Capt/Comp 1  Polarity   */
 #define TIM_CCER_CC1NE	BIT(2)	/* Capt/Comp 1N out Ena    */
 #define TIM_CCER_CC1NP	BIT(3)	/* Capt/Comp 1N Polarity   */
 #define TIM_CCER_CC2E	BIT(4)	/* Capt/Comp 2  out Ena    */
+#define TIM_CCER_CC2P	BIT(5)	/* Capt/Comp 2  Polarity   */
 #define TIM_CCER_CC3E	BIT(8)	/* Capt/Comp 3  out Ena    */
+#define TIM_CCER_CC3P	BIT(9)	/* Capt/Comp 3  Polarity   */
 #define TIM_CCER_CC4E	BIT(12)	/* Capt/Comp 4  out Ena    */
+#define TIM_CCER_CC4P	BIT(13)	/* Capt/Comp 4  Polarity   */
 #define TIM_CCER_CCXE	(BIT(0) | BIT(4) | BIT(8) | BIT(12))
 #define TIM_BDTR_BKE	BIT(12) /* Break input enable	   */
 #define TIM_BDTR_BKP	BIT(13) /* Break input polarity	   */
-- 
1.9.1

[PATCH v3 4/6] pwm: stm32: improve capture by tuning counter prescaler

[Fabrice Gasnier]

Currently, capture is based on timeout window to configure prescaler.
PWM capture framework provides 1s window at the time of writing.

There's place for improvement, after input signal has been captured once:
- Finer tune counter clock prescaler, by using 1st capture result (with
arbitrary margin).
- Do a 2nd capture, with scaled capture window.
This increases accuracy, especially at high rates.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Acked-by: Thierry Reding <thierry.reding@gmail.com>
---
Changes in v2:
- Adopt DMA read from MFD core.
---
 drivers/pwm/pwm-stm32.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 00c6251..c6eb0a8 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -169,7 +169,7 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct stm32_pwm *priv = to_stm32_pwm_dev(chip);
 	unsigned long long prd, div, dty;
 	unsigned long rate;
-	unsigned int psc = 0;
+	unsigned int psc = 0, scale;
 	u32 raw_prd, raw_dty;
 	int ret = 0;
 
@@ -220,6 +220,28 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 	if (ret)
 		goto stop;
 
+	/*
+	 * Got a capture. Try to improve accuracy at high rates:
+	 * - decrease counter clock prescaler, scale up to max rate.
+	 */
+	if (raw_prd) {
+		u32 max_arr = priv->max_arr - 0x1000; /* arbitrary margin */
+
+		scale = max_arr / min(max_arr, raw_prd);
+	} else {
+		scale = priv->max_arr; /* bellow resolution, use max scale */
+	}
+
+	if (psc && scale > 1) {
+		/* 2nd measure with new scale */
+		psc /= scale;
+		regmap_write(priv->regmap, TIM_PSC, psc);
+		ret = stm32_pwm_raw_capture(priv, pwm, tmo_ms, &raw_prd,
+					    &raw_dty);
+		if (ret)
+			goto stop;
+	}
+
 	prd = (unsigned long long)raw_prd * (psc + 1) * NSEC_PER_SEC;
 	result->period = DIV_ROUND_UP_ULL(prd, rate);
 	dty = (unsigned long long)raw_dty * (psc + 1) * NSEC_PER_SEC;
-- 
1.9.1

[PATCH v3 4/6] pwm: stm32: improve capture by tuning counter prescaler

[Fabrice Gasnier]

Currently, capture is based on timeout window to configure prescaler.
PWM capture framework provides 1s window at the time of writing.

There's place for improvement, after input signal has been captured once:
- Finer tune counter clock prescaler, by using 1st capture result (with
arbitrary margin).
- Do a 2nd capture, with scaled capture window.
This increases accuracy, especially at high rates.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Acked-by: Thierry Reding <thierry.reding@gmail.com>
---
Changes in v2:
- Adopt DMA read from MFD core.
---
 drivers/pwm/pwm-stm32.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 00c6251..c6eb0a8 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -169,7 +169,7 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct stm32_pwm *priv = to_stm32_pwm_dev(chip);
 	unsigned long long prd, div, dty;
 	unsigned long rate;
-	unsigned int psc = 0;
+	unsigned int psc = 0, scale;
 	u32 raw_prd, raw_dty;
 	int ret = 0;
 
@@ -220,6 +220,28 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 	if (ret)
 		goto stop;
 
+	/*
+	 * Got a capture. Try to improve accuracy at high rates:
+	 * - decrease counter clock prescaler, scale up to max rate.
+	 */
+	if (raw_prd) {
+		u32 max_arr = priv->max_arr - 0x1000; /* arbitrary margin */
+
+		scale = max_arr / min(max_arr, raw_prd);
+	} else {
+		scale = priv->max_arr; /* bellow resolution, use max scale */
+	}
+
+	if (psc && scale > 1) {
+		/* 2nd measure with new scale */
+		psc /= scale;
+		regmap_write(priv->regmap, TIM_PSC, psc);
+		ret = stm32_pwm_raw_capture(priv, pwm, tmo_ms, &raw_prd,
+					    &raw_dty);
+		if (ret)
+			goto stop;
+	}
+
 	prd = (unsigned long long)raw_prd * (psc + 1) * NSEC_PER_SEC;
 	result->period = DIV_ROUND_UP_ULL(prd, rate);
 	dty = (unsigned long long)raw_dty * (psc + 1) * NSEC_PER_SEC;
-- 
1.9.1

[PATCH v3 4/6] pwm: stm32: improve capture by tuning counter prescaler

[Fabrice Gasnier]

Currently, capture is based on timeout window to configure prescaler.
PWM capture framework provides 1s window at the time of writing.

There's place for improvement, after input signal has been captured once:
- Finer tune counter clock prescaler, by using 1st capture result (with
arbitrary margin).
- Do a 2nd capture, with scaled capture window.
This increases accuracy, especially at high rates.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Acked-by: Thierry Reding <thierry.reding@gmail.com>
---
Changes in v2:
- Adopt DMA read from MFD core.
---
 drivers/pwm/pwm-stm32.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 00c6251..c6eb0a8 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -169,7 +169,7 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct stm32_pwm *priv = to_stm32_pwm_dev(chip);
 	unsigned long long prd, div, dty;
 	unsigned long rate;
-	unsigned int psc = 0;
+	unsigned int psc = 0, scale;
 	u32 raw_prd, raw_dty;
 	int ret = 0;
 
@@ -220,6 +220,28 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 	if (ret)
 		goto stop;
 
+	/*
+	 * Got a capture. Try to improve accuracy at high rates:
+	 * - decrease counter clock prescaler, scale up to max rate.
+	 */
+	if (raw_prd) {
+		u32 max_arr = priv->max_arr - 0x1000; /* arbitrary margin */
+
+		scale = max_arr / min(max_arr, raw_prd);
+	} else {
+		scale = priv->max_arr; /* bellow resolution, use max scale */
+	}
+
+	if (psc && scale > 1) {
+		/* 2nd measure with new scale */
+		psc /= scale;
+		regmap_write(priv->regmap, TIM_PSC, psc);
+		ret = stm32_pwm_raw_capture(priv, pwm, tmo_ms, &raw_prd,
+					    &raw_dty);
+		if (ret)
+			goto stop;
+	}
+
 	prd = (unsigned long long)raw_prd * (psc + 1) * NSEC_PER_SEC;
 	result->period = DIV_ROUND_UP_ULL(prd, rate);
 	dty = (unsigned long long)raw_dty * (psc + 1) * NSEC_PER_SEC;
-- 
1.9.1

[PATCH v3 5/6] pwm: stm32: use input prescaler to improve period capture

[Fabrice Gasnier]

Using input prescaler, capture unit will trigger DMA once every
configurable /2, /4 or /8 events (rising edge). This helps improve
period (only) capture accuracy at high rates.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Acked-by: Thierry Reding <thierry.reding@gmail.com>
---
Changes in v2:
- Adopt DMA read from MFD core.
---
 drivers/pwm/pwm-stm32.c          | 63 ++++++++++++++++++++++++++++++++++++++--
 include/linux/mfd/stm32-timers.h |  1 +
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index c6eb0a8..21ea762 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -9,6 +9,7 @@
  *             pwm-atmel.c from Bo Shen
  */
 
+#include <linux/bitfield.h>
 #include <linux/mfd/stm32-timers.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -169,7 +170,7 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct stm32_pwm *priv = to_stm32_pwm_dev(chip);
 	unsigned long long prd, div, dty;
 	unsigned long rate;
-	unsigned int psc = 0, scale;
+	unsigned int psc = 0, icpsc, scale;
 	u32 raw_prd, raw_dty;
 	int ret = 0;
 
@@ -223,6 +224,7 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 	/*
 	 * Got a capture. Try to improve accuracy at high rates:
 	 * - decrease counter clock prescaler, scale up to max rate.
+	 * - use input prescaler, capture once every /2 /4 or /8 edges.
 	 */
 	if (raw_prd) {
 		u32 max_arr = priv->max_arr - 0x1000; /* arbitrary margin */
@@ -242,8 +244,65 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 			goto stop;
 	}
 
+	/* Compute intermediate period not to exceed timeout at low rates */
 	prd = (unsigned long long)raw_prd * (psc + 1) * NSEC_PER_SEC;
-	result->period = DIV_ROUND_UP_ULL(prd, rate);
+	do_div(prd, rate);
+
+	for (icpsc = 0; icpsc < MAX_TIM_ICPSC ; icpsc++) {
+		/* input prescaler: also keep arbitrary margin */
+		if (raw_prd >= (priv->max_arr - 0x1000) >> (icpsc + 1))
+			break;
+		if (prd >= (tmo_ms * NSEC_PER_MSEC) >> (icpsc + 2))
+			break;
+	}
+
+	if (!icpsc)
+		goto done;
+
+	/* Last chance to improve period accuracy, using input prescaler */
+	regmap_update_bits(priv->regmap,
+			   pwm->hwpwm < 2 ? TIM_CCMR1 : TIM_CCMR2,
+			   TIM_CCMR_IC1PSC | TIM_CCMR_IC2PSC,
+			   FIELD_PREP(TIM_CCMR_IC1PSC, icpsc) |
+			   FIELD_PREP(TIM_CCMR_IC2PSC, icpsc));
+
+	ret = stm32_pwm_raw_capture(priv, pwm, tmo_ms, &raw_prd, &raw_dty);
+	if (ret)
+		goto stop;
+
+	if (raw_dty >= (raw_prd >> icpsc)) {
+		/*
+		 * We may fall here using input prescaler, when input
+		 * capture starts on high side (before falling edge).
+		 * Example with icpsc to capture on each 4 events:
+		 *
+		 *       start   1st capture                     2nd capture
+		 *         v     v                               v
+		 *         ___   _____   _____   _____   _____   ____
+		 * TI1..4     |__|    |__|    |__|    |__|    |__|
+		 *            v  v    .  .    .  .    .       v  v
+		 * icpsc1/3:  .  0    .  1    .  2    .  3    .  0
+		 * icpsc2/4:  0       1       2       3       0
+		 *            v  v                            v  v
+		 * CCR1/3  ......t0..............................t2
+		 * CCR2/4  ..t1..............................t1'...
+		 *               .                            .  .
+		 * Capture0:     .<----------------------------->.
+		 * Capture1:     .<-------------------------->.  .
+		 *               .                            .  .
+		 * Period:       .<------>                    .  .
+		 * Low side:                                  .<>.
+		 *
+		 * Result:
+		 * - Period = Capture0 / icpsc
+		 * - Duty = Period - Low side = Period - (Capture0 - Capture1)
+		 */
+		raw_dty = (raw_prd >> icpsc) - (raw_prd - raw_dty);
+	}
+
+done:
+	prd = (unsigned long long)raw_prd * (psc + 1) * NSEC_PER_SEC;
+	result->period = DIV_ROUND_UP_ULL(prd, rate << icpsc);
 	dty = (unsigned long long)raw_dty * (psc + 1) * NSEC_PER_SEC;
 	result->duty_cycle = DIV_ROUND_UP_ULL(dty, rate);
 stop:
diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
index 3c72b70..2f505c2 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -82,6 +82,7 @@
 #define TIM_DCR_DBL	GENMASK(12, 8)	/* DMA burst len */
 
 #define MAX_TIM_PSC		0xFFFF
+#define MAX_TIM_ICPSC		0x3
 #define TIM_CR2_MMS_SHIFT	4
 #define TIM_CR2_MMS2_SHIFT	20
 #define TIM_SMCR_TS_SHIFT	4
-- 
1.9.1

[PATCH v3 5/6] pwm: stm32: use input prescaler to improve period capture

[Fabrice Gasnier]

Using input prescaler, capture unit will trigger DMA once every
configurable /2, /4 or /8 events (rising edge). This helps improve
period (only) capture accuracy at high rates.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Acked-by: Thierry Reding <thierry.reding@gmail.com>
---
Changes in v2:
- Adopt DMA read from MFD core.
---
 drivers/pwm/pwm-stm32.c          | 63 ++++++++++++++++++++++++++++++++++++++--
 include/linux/mfd/stm32-timers.h |  1 +
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index c6eb0a8..21ea762 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -9,6 +9,7 @@
  *             pwm-atmel.c from Bo Shen
  */
 
+#include <linux/bitfield.h>
 #include <linux/mfd/stm32-timers.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -169,7 +170,7 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct stm32_pwm *priv = to_stm32_pwm_dev(chip);
 	unsigned long long prd, div, dty;
 	unsigned long rate;
-	unsigned int psc = 0, scale;
+	unsigned int psc = 0, icpsc, scale;
 	u32 raw_prd, raw_dty;
 	int ret = 0;
 
@@ -223,6 +224,7 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 	/*
 	 * Got a capture. Try to improve accuracy at high rates:
 	 * - decrease counter clock prescaler, scale up to max rate.
+	 * - use input prescaler, capture once every /2 /4 or /8 edges.
 	 */
 	if (raw_prd) {
 		u32 max_arr = priv->max_arr - 0x1000; /* arbitrary margin */
@@ -242,8 +244,65 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 			goto stop;
 	}
 
+	/* Compute intermediate period not to exceed timeout at low rates */
 	prd = (unsigned long long)raw_prd * (psc + 1) * NSEC_PER_SEC;
-	result->period = DIV_ROUND_UP_ULL(prd, rate);
+	do_div(prd, rate);
+
+	for (icpsc = 0; icpsc < MAX_TIM_ICPSC ; icpsc++) {
+		/* input prescaler: also keep arbitrary margin */
+		if (raw_prd >= (priv->max_arr - 0x1000) >> (icpsc + 1))
+			break;
+		if (prd >= (tmo_ms * NSEC_PER_MSEC) >> (icpsc + 2))
+			break;
+	}
+
+	if (!icpsc)
+		goto done;
+
+	/* Last chance to improve period accuracy, using input prescaler */
+	regmap_update_bits(priv->regmap,
+			   pwm->hwpwm < 2 ? TIM_CCMR1 : TIM_CCMR2,
+			   TIM_CCMR_IC1PSC | TIM_CCMR_IC2PSC,
+			   FIELD_PREP(TIM_CCMR_IC1PSC, icpsc) |
+			   FIELD_PREP(TIM_CCMR_IC2PSC, icpsc));
+
+	ret = stm32_pwm_raw_capture(priv, pwm, tmo_ms, &raw_prd, &raw_dty);
+	if (ret)
+		goto stop;
+
+	if (raw_dty >= (raw_prd >> icpsc)) {
+		/*
+		 * We may fall here using input prescaler, when input
+		 * capture starts on high side (before falling edge).
+		 * Example with icpsc to capture on each 4 events:
+		 *
+		 *       start   1st capture                     2nd capture
+		 *         v     v                               v
+		 *         ___   _____   _____   _____   _____   ____
+		 * TI1..4     |__|    |__|    |__|    |__|    |__|
+		 *            v  v    .  .    .  .    .       v  v
+		 * icpsc1/3:  .  0    .  1    .  2    .  3    .  0
+		 * icpsc2/4:  0       1       2       3       0
+		 *            v  v                            v  v
+		 * CCR1/3  ......t0..............................t2
+		 * CCR2/4  ..t1..............................t1'...
+		 *               .                            .  .
+		 * Capture0:     .<----------------------------->.
+		 * Capture1:     .<-------------------------->.  .
+		 *               .                            .  .
+		 * Period:       .<------>                    .  .
+		 * Low side:                                  .<>.
+		 *
+		 * Result:
+		 * - Period = Capture0 / icpsc
+		 * - Duty = Period - Low side = Period - (Capture0 - Capture1)
+		 */
+		raw_dty = (raw_prd >> icpsc) - (raw_prd - raw_dty);
+	}
+
+done:
+	prd = (unsigned long long)raw_prd * (psc + 1) * NSEC_PER_SEC;
+	result->period = DIV_ROUND_UP_ULL(prd, rate << icpsc);
 	dty = (unsigned long long)raw_dty * (psc + 1) * NSEC_PER_SEC;
 	result->duty_cycle = DIV_ROUND_UP_ULL(dty, rate);
 stop:
diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
index 3c72b70..2f505c2 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -82,6 +82,7 @@
 #define TIM_DCR_DBL	GENMASK(12, 8)	/* DMA burst len */
 
 #define MAX_TIM_PSC		0xFFFF
+#define MAX_TIM_ICPSC		0x3
 #define TIM_CR2_MMS_SHIFT	4
 #define TIM_CR2_MMS2_SHIFT	20
 #define TIM_SMCR_TS_SHIFT	4
-- 
1.9.1

[PATCH v3 5/6] pwm: stm32: use input prescaler to improve period capture

[Fabrice Gasnier]

Using input prescaler, capture unit will trigger DMA once every
configurable /2, /4 or /8 events (rising edge). This helps improve
period (only) capture accuracy at high rates.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Acked-by: Thierry Reding <thierry.reding@gmail.com>
---
Changes in v2:
- Adopt DMA read from MFD core.
---
 drivers/pwm/pwm-stm32.c          | 63 ++++++++++++++++++++++++++++++++++++++--
 include/linux/mfd/stm32-timers.h |  1 +
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index c6eb0a8..21ea762 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -9,6 +9,7 @@
  *             pwm-atmel.c from Bo Shen
  */
 
+#include <linux/bitfield.h>
 #include <linux/mfd/stm32-timers.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -169,7 +170,7 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct stm32_pwm *priv = to_stm32_pwm_dev(chip);
 	unsigned long long prd, div, dty;
 	unsigned long rate;
-	unsigned int psc = 0, scale;
+	unsigned int psc = 0, icpsc, scale;
 	u32 raw_prd, raw_dty;
 	int ret = 0;
 
@@ -223,6 +224,7 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 	/*
 	 * Got a capture. Try to improve accuracy at high rates:
 	 * - decrease counter clock prescaler, scale up to max rate.
+	 * - use input prescaler, capture once every /2 /4 or /8 edges.
 	 */
 	if (raw_prd) {
 		u32 max_arr = priv->max_arr - 0x1000; /* arbitrary margin */
@@ -242,8 +244,65 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 			goto stop;
 	}
 
+	/* Compute intermediate period not to exceed timeout at low rates */
 	prd = (unsigned long long)raw_prd * (psc + 1) * NSEC_PER_SEC;
-	result->period = DIV_ROUND_UP_ULL(prd, rate);
+	do_div(prd, rate);
+
+	for (icpsc = 0; icpsc < MAX_TIM_ICPSC ; icpsc++) {
+		/* input prescaler: also keep arbitrary margin */
+		if (raw_prd >= (priv->max_arr - 0x1000) >> (icpsc + 1))
+			break;
+		if (prd >= (tmo_ms * NSEC_PER_MSEC) >> (icpsc + 2))
+			break;
+	}
+
+	if (!icpsc)
+		goto done;
+
+	/* Last chance to improve period accuracy, using input prescaler */
+	regmap_update_bits(priv->regmap,
+			   pwm->hwpwm < 2 ? TIM_CCMR1 : TIM_CCMR2,
+			   TIM_CCMR_IC1PSC | TIM_CCMR_IC2PSC,
+			   FIELD_PREP(TIM_CCMR_IC1PSC, icpsc) |
+			   FIELD_PREP(TIM_CCMR_IC2PSC, icpsc));
+
+	ret = stm32_pwm_raw_capture(priv, pwm, tmo_ms, &raw_prd, &raw_dty);
+	if (ret)
+		goto stop;
+
+	if (raw_dty >= (raw_prd >> icpsc)) {
+		/*
+		 * We may fall here using input prescaler, when input
+		 * capture starts on high side (before falling edge).
+		 * Example with icpsc to capture on each 4 events:
+		 *
+		 *       start   1st capture                     2nd capture
+		 *         v     v                               v
+		 *         ___   _____   _____   _____   _____   ____
+		 * TI1..4     |__|    |__|    |__|    |__|    |__|
+		 *            v  v    .  .    .  .    .       v  v
+		 * icpsc1/3:  .  0    .  1    .  2    .  3    .  0
+		 * icpsc2/4:  0       1       2       3       0
+		 *            v  v                            v  v
+		 * CCR1/3  ......t0..............................t2
+		 * CCR2/4  ..t1..............................t1'...
+		 *               .                            .  .
+		 * Capture0:     .<----------------------------->.
+		 * Capture1:     .<-------------------------->.  .
+		 *               .                            .  .
+		 * Period:       .<------>                    .  .
+		 * Low side:                                  .<>.
+		 *
+		 * Result:
+		 * - Period = Capture0 / icpsc
+		 * - Duty = Period - Low side = Period - (Capture0 - Capture1)
+		 */
+		raw_dty = (raw_prd >> icpsc) - (raw_prd - raw_dty);
+	}
+
+done:
+	prd = (unsigned long long)raw_prd * (psc + 1) * NSEC_PER_SEC;
+	result->period = DIV_ROUND_UP_ULL(prd, rate << icpsc);
 	dty = (unsigned long long)raw_dty * (psc + 1) * NSEC_PER_SEC;
 	result->duty_cycle = DIV_ROUND_UP_ULL(dty, rate);
 stop:
diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
index 3c72b70..2f505c2 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -82,6 +82,7 @@
 #define TIM_DCR_DBL	GENMASK(12, 8)	/* DMA burst len */
 
 #define MAX_TIM_PSC		0xFFFF
+#define MAX_TIM_ICPSC		0x3
 #define TIM_CR2_MMS_SHIFT	4
 #define TIM_CR2_MMS2_SHIFT	20
 #define TIM_SMCR_TS_SHIFT	4
-- 
1.9.1

[PATCH v3 6/6] ARM: dts: stm32: Enable pwm3 input capture on stm32f429i-eval

[Fabrice Gasnier]

Enable pwm3 input capture on stm32f429i-eval, by using DMA.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 arch/arm/boot/dts/stm32429i-eval.dts | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/stm32429i-eval.dts b/arch/arm/boot/dts/stm32429i-eval.dts
index 293ecb9..d5498dd 100644
--- a/arch/arm/boot/dts/stm32429i-eval.dts
+++ b/arch/arm/boot/dts/stm32429i-eval.dts
@@ -271,6 +271,9 @@
 &timers3 {
 	status = "okay";
 
+	/* Enable PWM input capture by using dma */
+	dmas = <&dma1 4 5 0x400 0x0>;
+	dma-names = "ch1";
 	pwm {
 		pinctrl-0 = <&pwm3_pins>;
 		pinctrl-names = "default";
-- 
1.9.1

[PATCH v3 6/6] ARM: dts: stm32: Enable pwm3 input capture on stm32f429i-eval

[Fabrice Gasnier]

Enable pwm3 input capture on stm32f429i-eval, by using DMA.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 arch/arm/boot/dts/stm32429i-eval.dts | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/stm32429i-eval.dts b/arch/arm/boot/dts/stm32429i-eval.dts
index 293ecb9..d5498dd 100644
--- a/arch/arm/boot/dts/stm32429i-eval.dts
+++ b/arch/arm/boot/dts/stm32429i-eval.dts
@@ -271,6 +271,9 @@
 &timers3 {
 	status = "okay";
 
+	/* Enable PWM input capture by using dma */
+	dmas = <&dma1 4 5 0x400 0x0>;
+	dma-names = "ch1";
 	pwm {
 		pinctrl-0 = <&pwm3_pins>;
 		pinctrl-names = "default";
-- 
1.9.1

[PATCH v3 6/6] ARM: dts: stm32: Enable pwm3 input capture on stm32f429i-eval

[Fabrice Gasnier]

Enable pwm3 input capture on stm32f429i-eval, by using DMA.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 arch/arm/boot/dts/stm32429i-eval.dts | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/stm32429i-eval.dts b/arch/arm/boot/dts/stm32429i-eval.dts
index 293ecb9..d5498dd 100644
--- a/arch/arm/boot/dts/stm32429i-eval.dts
+++ b/arch/arm/boot/dts/stm32429i-eval.dts
@@ -271,6 +271,9 @@
 &timers3 {
 	status = "okay";
 
+	/* Enable PWM input capture by using dma */
+	dmas = <&dma1 4 5 0x400 0x0>;
+	dma-names = "ch1";
 	pwm {
 		pinctrl-0 = <&pwm3_pins>;
 		pinctrl-names = "default";
-- 
1.9.1

Re: [PATCH v3 2/6] mfd: stm32-timers: add support for dmas

[Lee Jones]

On Fri, 30 Mar 2018, Fabrice Gasnier wrote:

> STM32 Timers can support up to 7 DMA requests:
> - 4 channels, update, compare and trigger.
> Optionally request part, or all DMAs from stm32-timers MFD core.
> 
> Also add routine to implement burst reads using DMA from timer registers.
> This is exported. So, it can be used by child drivers, PWM capture
> for instance (but not limited to).
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> ---
> Changes in v3:
> - Basically Lee's comments:
> - rather create a struct stm32_timers_dma, and place a reference to it
>   in existing ddata (instead of adding priv struct).
> - rather use a struct device in exported routine prototype, and use
>   standard helpers instead of ddata. Get rid of to_stm32_timers_priv().
> - simplify error handling in probe (remove a goto)
> - comment on devm_of_platform_*populate() usage.
> 
> Changes in v2:
> - Abstract DMA handling from child driver: move it to MFD core
> - Add comments on optional dma support
> ---
>  drivers/mfd/stm32-timers.c       | 219 ++++++++++++++++++++++++++++++++++++++-
>  include/linux/mfd/stm32-timers.h |  32 ++++++
>  2 files changed, 249 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
> index 1d347e5..98191ec 100644
> --- a/drivers/mfd/stm32-timers.c
> +++ b/drivers/mfd/stm32-timers.c
> @@ -4,16 +4,165 @@
>   * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
>   */
>  
> +#include <linux/bitfield.h>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
>  #include <linux/mfd/stm32-timers.h>
>  #include <linux/module.h>
>  #include <linux/of_platform.h>
>  #include <linux/reset.h>
>  
> +#define STM32_TIMERS_MAX_REGISTERS	0x3fc
> +
> +struct stm32_timers_dma {
> +	struct completion completion;
> +	phys_addr_t phys_base;
> +	struct mutex lock;		/* protect dma access */

Nit: I like comments to use good grammar i.e. capital letters to
start a sentence and 's/dma/DMA/'.  Or better still, drop the comment,
we know what the lock is for.

> +	struct dma_chan *chan;
> +	struct dma_chan *chans[STM32_TIMERS_MAX_DMAS];

This requires explanation.  Maybe a kerneldoc header would be good here.

[...]

> +/**
> + * stm32_timers_dma_burst_read - Read from timers registers using DMA.
> + *
> + * Read from STM32 timers registers using DMA on a single event.
> + * @dev: reference to stm32_timers MFD device
> + * @buf: dma'able destination buffer
> + * @id: stm32_timers_dmas event identifier (ch[1..4], up, trig or com)
> + * @reg: registers start offset for DMA to read from (like CCRx for capture)
> + * @num_reg: number of registers to read upon each dma request, starting @reg.
> + * @bursts: number of bursts to read (e.g. like two for pwm period capture)
> + * @tmo_ms: timeout (milliseconds)
> + */
> +int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
> +				enum stm32_timers_dmas id, u32 reg,
> +				unsigned int num_reg, unsigned int bursts,
> +				unsigned long tmo_ms)
> +{
> +	struct stm32_timers *ddata = dev_get_drvdata(dev);
> +	unsigned long timeout = msecs_to_jiffies(tmo_ms);
> +	struct regmap *regmap = ddata->regmap;
> +	struct stm32_timers_dma *dma = ddata->dma;
> +	size_t len = num_reg * bursts * sizeof(u32);
> +	struct dma_async_tx_descriptor *desc;
> +	struct dma_slave_config config;
> +	dma_cookie_t cookie;
> +	dma_addr_t dma_buf;
> +	u32 dbl, dba;
> +	long err;
> +	int ret;
> +
> +	/* sanity check */

Proper grammar in all comments please.

"Sanity check"

[...]

> +	/* select dma channel in use */

Here too.

Etc, etc, etc ...

> +	dma->chan = dma->chans[id];
> +	dma_buf = dma_map_single(dev, buf, len, DMA_FROM_DEVICE);
> +	ret = dma_mapping_error(dev, dma_buf);
> +	if (ret)
> +		goto unlock;
> +
> +	/* Prepare DMA read from timer registers, using DMA burst mode */

This is good.
[...]

[...]

> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
> index 2aadab6..585a4de 100644
> --- a/include/linux/mfd/stm32-timers.h
> +++ b/include/linux/mfd/stm32-timers.h
> @@ -8,6 +8,8 @@
>  #define _LINUX_STM32_GPTIMER_H_
>  
>  #include <linux/clk.h>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
>  #include <linux/regmap.h>

[...]

> +enum stm32_timers_dmas {
> +	STM32_TIMERS_DMA_CH1,
> +	STM32_TIMERS_DMA_CH2,
> +	STM32_TIMERS_DMA_CH3,
> +	STM32_TIMERS_DMA_CH4,
> +	STM32_TIMERS_DMA_UP,
> +	STM32_TIMERS_DMA_TRIG,
> +	STM32_TIMERS_DMA_COM,
> +	STM32_TIMERS_MAX_DMAS,
> +};
> +
> +struct stm32_timers_dma;

Why don't you move the declaration into here?

Then you don't need to forward declare.

>  struct stm32_timers {
>  	struct clk *clk;
>  	struct regmap *regmap;
>  	u32 max_arr;
> +	struct stm32_timers_dma *dma;
>  };
> +
> +int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
> +				enum stm32_timers_dmas id, u32 reg,
> +				unsigned int num_reg, unsigned int bursts,
> +				unsigned long tmo_ms);
>  #endif

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v3 2/6] mfd: stm32-timers: add support for dmas

[Lee Jones]

On Fri, 30 Mar 2018, Fabrice Gasnier wrote:

> STM32 Timers can support up to 7 DMA requests:
> - 4 channels, update, compare and trigger.
> Optionally request part, or all DMAs from stm32-timers MFD core.
> 
> Also add routine to implement burst reads using DMA from timer registers.
> This is exported. So, it can be used by child drivers, PWM capture
> for instance (but not limited to).
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> ---
> Changes in v3:
> - Basically Lee's comments:
> - rather create a struct stm32_timers_dma, and place a reference to it
>   in existing ddata (instead of adding priv struct).
> - rather use a struct device in exported routine prototype, and use
>   standard helpers instead of ddata. Get rid of to_stm32_timers_priv().
> - simplify error handling in probe (remove a goto)
> - comment on devm_of_platform_*populate() usage.
> 
> Changes in v2:
> - Abstract DMA handling from child driver: move it to MFD core
> - Add comments on optional dma support
> ---
>  drivers/mfd/stm32-timers.c       | 219 ++++++++++++++++++++++++++++++++++++++-
>  include/linux/mfd/stm32-timers.h |  32 ++++++
>  2 files changed, 249 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
> index 1d347e5..98191ec 100644
> --- a/drivers/mfd/stm32-timers.c
> +++ b/drivers/mfd/stm32-timers.c
> @@ -4,16 +4,165 @@
>   * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
>   */
>  
> +#include <linux/bitfield.h>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
>  #include <linux/mfd/stm32-timers.h>
>  #include <linux/module.h>
>  #include <linux/of_platform.h>
>  #include <linux/reset.h>
>  
> +#define STM32_TIMERS_MAX_REGISTERS	0x3fc
> +
> +struct stm32_timers_dma {
> +	struct completion completion;
> +	phys_addr_t phys_base;
> +	struct mutex lock;		/* protect dma access */

Nit: I like comments to use good grammar i.e. capital letters to
start a sentence and 's/dma/DMA/'.  Or better still, drop the comment,
we know what the lock is for.

> +	struct dma_chan *chan;
> +	struct dma_chan *chans[STM32_TIMERS_MAX_DMAS];

This requires explanation.  Maybe a kerneldoc header would be good here.

[...]

> +/**
> + * stm32_timers_dma_burst_read - Read from timers registers using DMA.
> + *
> + * Read from STM32 timers registers using DMA on a single event.
> + * @dev: reference to stm32_timers MFD device
> + * @buf: dma'able destination buffer
> + * @id: stm32_timers_dmas event identifier (ch[1..4], up, trig or com)
> + * @reg: registers start offset for DMA to read from (like CCRx for capture)
> + * @num_reg: number of registers to read upon each dma request, starting @reg.
> + * @bursts: number of bursts to read (e.g. like two for pwm period capture)
> + * @tmo_ms: timeout (milliseconds)
> + */
> +int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
> +				enum stm32_timers_dmas id, u32 reg,
> +				unsigned int num_reg, unsigned int bursts,
> +				unsigned long tmo_ms)
> +{
> +	struct stm32_timers *ddata = dev_get_drvdata(dev);
> +	unsigned long timeout = msecs_to_jiffies(tmo_ms);
> +	struct regmap *regmap = ddata->regmap;
> +	struct stm32_timers_dma *dma = ddata->dma;
> +	size_t len = num_reg * bursts * sizeof(u32);
> +	struct dma_async_tx_descriptor *desc;
> +	struct dma_slave_config config;
> +	dma_cookie_t cookie;
> +	dma_addr_t dma_buf;
> +	u32 dbl, dba;
> +	long err;
> +	int ret;
> +
> +	/* sanity check */

Proper grammar in all comments please.

"Sanity check"

[...]

> +	/* select dma channel in use */

Here too.

Etc, etc, etc ...

> +	dma->chan = dma->chans[id];
> +	dma_buf = dma_map_single(dev, buf, len, DMA_FROM_DEVICE);
> +	ret = dma_mapping_error(dev, dma_buf);
> +	if (ret)
> +		goto unlock;
> +
> +	/* Prepare DMA read from timer registers, using DMA burst mode */

This is good.
[...]

[...]

> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
> index 2aadab6..585a4de 100644
> --- a/include/linux/mfd/stm32-timers.h
> +++ b/include/linux/mfd/stm32-timers.h
> @@ -8,6 +8,8 @@
>  #define _LINUX_STM32_GPTIMER_H_
>  
>  #include <linux/clk.h>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
>  #include <linux/regmap.h>

[...]

> +enum stm32_timers_dmas {
> +	STM32_TIMERS_DMA_CH1,
> +	STM32_TIMERS_DMA_CH2,
> +	STM32_TIMERS_DMA_CH3,
> +	STM32_TIMERS_DMA_CH4,
> +	STM32_TIMERS_DMA_UP,
> +	STM32_TIMERS_DMA_TRIG,
> +	STM32_TIMERS_DMA_COM,
> +	STM32_TIMERS_MAX_DMAS,
> +};
> +
> +struct stm32_timers_dma;

Why don't you move the declaration into here?

Then you don't need to forward declare.

>  struct stm32_timers {
>  	struct clk *clk;
>  	struct regmap *regmap;
>  	u32 max_arr;
> +	struct stm32_timers_dma *dma;
>  };
> +
> +int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
> +				enum stm32_timers_dmas id, u32 reg,
> +				unsigned int num_reg, unsigned int bursts,
> +				unsigned long tmo_ms);
>  #endif

-- 
Lee Jones [???]
Linaro Services Technical Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 1/6] dt-bindings: mfd: stm32-timers: add support for dmas

[Lee Jones]

On Fri, 30 Mar 2018, Fabrice Gasnier wrote:

> Add support for DMAs to STM32 timers. STM32 Timers can support up to 7
> dma requests: up to 4 channels, update, compare and trigger.
> DMAs may be used to transfer data from pwm capture for instance.
> DMA support is made optional, PWM capture support is also an option.
> This is much more wise system-wide to avoid shortage on DMA request
> lines as there's significant amount of timer instances that can
> request up to 7 channels.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> ---
>  .../devicetree/bindings/mfd/stm32-timers.txt         | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)

Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v3 1/6] dt-bindings: mfd: stm32-timers: add support for dmas

[Lee Jones]

On Fri, 30 Mar 2018, Fabrice Gasnier wrote:

> Add support for DMAs to STM32 timers. STM32 Timers can support up to 7
> dma requests: up to 4 channels, update, compare and trigger.
> DMAs may be used to transfer data from pwm capture for instance.
> DMA support is made optional, PWM capture support is also an option.
> This is much more wise system-wide to avoid shortage on DMA request
> lines as there's significant amount of timer instances that can
> request up to 7 channels.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> ---
>  .../devicetree/bindings/mfd/stm32-timers.txt         | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)

Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [???]
Linaro Services Technical Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 1/6] dt-bindings: mfd: stm32-timers: add support for dmas

[Lee Jones]

On Mon, 16 Apr 2018, Lee Jones wrote:

> On Fri, 30 Mar 2018, Fabrice Gasnier wrote:
> 
> > Add support for DMAs to STM32 timers. STM32 Timers can support up to 7
> > dma requests: up to 4 channels, update, compare and trigger.
> > DMAs may be used to transfer data from pwm capture for instance.
> > DMA support is made optional, PWM capture support is also an option.
> > This is much more wise system-wide to avoid shortage on DMA request
> > lines as there's significant amount of timer instances that can
> > request up to 7 channels.
> > 
> > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> > ---
> >  .../devicetree/bindings/mfd/stm32-timers.txt         | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> 
> Acked-by: Lee Jones <lee.jones@linaro.org>

Actually:

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v3 1/6] dt-bindings: mfd: stm32-timers: add support for dmas

[Lee Jones]

On Mon, 16 Apr 2018, Lee Jones wrote:

> On Fri, 30 Mar 2018, Fabrice Gasnier wrote:
> 
> > Add support for DMAs to STM32 timers. STM32 Timers can support up to 7
> > dma requests: up to 4 channels, update, compare and trigger.
> > DMAs may be used to transfer data from pwm capture for instance.
> > DMA support is made optional, PWM capture support is also an option.
> > This is much more wise system-wide to avoid shortage on DMA request
> > lines as there's significant amount of timer instances that can
> > request up to 7 channels.
> > 
> > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> > ---
> >  .../devicetree/bindings/mfd/stm32-timers.txt         | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> 
> Acked-by: Lee Jones <lee.jones@linaro.org>

Actually:

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [???]
Linaro Services Technical Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 3/6] pwm: stm32: add capture support

[Lee Jones]

On Fri, 30 Mar 2018, Fabrice Gasnier wrote:

> Add support for PMW input mode on pwm-stm32. STM32 timers support
> period and duty cycle capture as long as they have at least two PWM
> channels. One capture channel is used for period (rising-edge), one
> for duty-cycle (falling-edge).
> When there's only one channel available, only period can be captured.
> Duty-cycle is simply zero'ed in such a case.
> 
> Capture requires exclusive access (e.g. no pwm output running at the
> same time, to protect common prescaler).
> Timer DMA burst mode (from MFD core) is being used, to take two
> snapshots of capture registers (upon each period rising edge).
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Acked-by: Thierry Reding <thierry.reding@gmail.com>
> ---
> Changes in v3:
> - update stm32_timers_dma_burst_read() call: don't pass ddata structure,
>   use MFD parent device structure instead since MFD core update.
> 
> Changes in v2:
> - DMA handling has been moved to MFD core. Rework capture routines to
>   use it.
> ---
>  drivers/pwm/pwm-stm32.c          | 176 +++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/stm32-timers.h |  11 +++

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

>  2 files changed, 187 insertions(+)

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v3 3/6] pwm: stm32: add capture support

[Lee Jones]

On Fri, 30 Mar 2018, Fabrice Gasnier wrote:

> Add support for PMW input mode on pwm-stm32. STM32 timers support
> period and duty cycle capture as long as they have at least two PWM
> channels. One capture channel is used for period (rising-edge), one
> for duty-cycle (falling-edge).
> When there's only one channel available, only period can be captured.
> Duty-cycle is simply zero'ed in such a case.
> 
> Capture requires exclusive access (e.g. no pwm output running at the
> same time, to protect common prescaler).
> Timer DMA burst mode (from MFD core) is being used, to take two
> snapshots of capture registers (upon each period rising edge).
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Acked-by: Thierry Reding <thierry.reding@gmail.com>
> ---
> Changes in v3:
> - update stm32_timers_dma_burst_read() call: don't pass ddata structure,
>   use MFD parent device structure instead since MFD core update.
> 
> Changes in v2:
> - DMA handling has been moved to MFD core. Rework capture routines to
>   use it.
> ---
>  drivers/pwm/pwm-stm32.c          | 176 +++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/stm32-timers.h |  11 +++

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

>  2 files changed, 187 insertions(+)

-- 
Lee Jones [???]
Linaro Services Technical Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 5/6] pwm: stm32: use input prescaler to improve period capture

[Lee Jones]

On Fri, 30 Mar 2018, Fabrice Gasnier wrote:

> Using input prescaler, capture unit will trigger DMA once every
> configurable /2, /4 or /8 events (rising edge). This helps improve
> period (only) capture accuracy at high rates.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Acked-by: Thierry Reding <thierry.reding@gmail.com>
> ---
> Changes in v2:
> - Adopt DMA read from MFD core.
> ---
>  drivers/pwm/pwm-stm32.c          | 63 ++++++++++++++++++++++++++++++++++++++--
>  include/linux/mfd/stm32-timers.h |  1 +

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

>  2 files changed, 62 insertions(+), 2 deletions(-)

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v3 5/6] pwm: stm32: use input prescaler to improve period capture

[Lee Jones]

On Fri, 30 Mar 2018, Fabrice Gasnier wrote:

> Using input prescaler, capture unit will trigger DMA once every
> configurable /2, /4 or /8 events (rising edge). This helps improve
> period (only) capture accuracy at high rates.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Acked-by: Thierry Reding <thierry.reding@gmail.com>
> ---
> Changes in v2:
> - Adopt DMA read from MFD core.
> ---
>  drivers/pwm/pwm-stm32.c          | 63 ++++++++++++++++++++++++++++++++++++++--
>  include/linux/mfd/stm32-timers.h |  1 +

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

>  2 files changed, 62 insertions(+), 2 deletions(-)

-- 
Lee Jones [???]
Linaro Services Technical Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 2/6] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

On 04/16/2018 02:22 PM, Lee Jones wrote:
> On Fri, 30 Mar 2018, Fabrice Gasnier wrote:
> 
>> STM32 Timers can support up to 7 DMA requests:
>> - 4 channels, update, compare and trigger.
>> Optionally request part, or all DMAs from stm32-timers MFD core.
>>
>> Also add routine to implement burst reads using DMA from timer registers.
>> This is exported. So, it can be used by child drivers, PWM capture
>> for instance (but not limited to).
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>> ---
>> Changes in v3:
>> - Basically Lee's comments:
>> - rather create a struct stm32_timers_dma, and place a reference to it
>>   in existing ddata (instead of adding priv struct).
>> - rather use a struct device in exported routine prototype, and use
>>   standard helpers instead of ddata. Get rid of to_stm32_timers_priv().
>> - simplify error handling in probe (remove a goto)
>> - comment on devm_of_platform_*populate() usage.
>>
>> Changes in v2:
>> - Abstract DMA handling from child driver: move it to MFD core
>> - Add comments on optional dma support
>> ---
>>  drivers/mfd/stm32-timers.c       | 219 ++++++++++++++++++++++++++++++++++++++-
>>  include/linux/mfd/stm32-timers.h |  32 ++++++
>>  2 files changed, 249 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
>> index 1d347e5..98191ec 100644
>> --- a/drivers/mfd/stm32-timers.c
>> +++ b/drivers/mfd/stm32-timers.c
>> @@ -4,16 +4,165 @@
>>   * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
>>   */
>>  
>> +#include <linux/bitfield.h>
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>>  #include <linux/mfd/stm32-timers.h>
>>  #include <linux/module.h>
>>  #include <linux/of_platform.h>
>>  #include <linux/reset.h>
>>  
>> +#define STM32_TIMERS_MAX_REGISTERS	0x3fc
>> +
>> +struct stm32_timers_dma {
>> +	struct completion completion;
>> +	phys_addr_t phys_base;
>> +	struct mutex lock;		/* protect dma access */
> 
> Nit: I like comments to use good grammar i.e. capital letters to
> start a sentence and 's/dma/DMA/'.  Or better still, drop the comment,
> we know what the lock is for.
> 
>> +	struct dma_chan *chan;
>> +	struct dma_chan *chans[STM32_TIMERS_MAX_DMAS];
> 
> This requires explanation.  Maybe a kerneldoc header would be good here.

Hi Lee,

I'll add kerneldoc in next version.

> 
> [...]
> 
>> +/**
>> + * stm32_timers_dma_burst_read - Read from timers registers using DMA.
>> + *
>> + * Read from STM32 timers registers using DMA on a single event.
>> + * @dev: reference to stm32_timers MFD device
>> + * @buf: dma'able destination buffer
>> + * @id: stm32_timers_dmas event identifier (ch[1..4], up, trig or com)
>> + * @reg: registers start offset for DMA to read from (like CCRx for capture)
>> + * @num_reg: number of registers to read upon each dma request, starting @reg.
>> + * @bursts: number of bursts to read (e.g. like two for pwm period capture)
>> + * @tmo_ms: timeout (milliseconds)
>> + */
>> +int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
>> +				enum stm32_timers_dmas id, u32 reg,
>> +				unsigned int num_reg, unsigned int bursts,
>> +				unsigned long tmo_ms)
>> +{
>> +	struct stm32_timers *ddata = dev_get_drvdata(dev);
>> +	unsigned long timeout = msecs_to_jiffies(tmo_ms);
>> +	struct regmap *regmap = ddata->regmap;
>> +	struct stm32_timers_dma *dma = ddata->dma;
>> +	size_t len = num_reg * bursts * sizeof(u32);
>> +	struct dma_async_tx_descriptor *desc;
>> +	struct dma_slave_config config;
>> +	dma_cookie_t cookie;
>> +	dma_addr_t dma_buf;
>> +	u32 dbl, dba;
>> +	long err;
>> +	int ret;
>> +
>> +	/* sanity check */
> 
> Proper grammar in all comments please.
> 
> "Sanity check"
> 
> [...]
> 
>> +	/* select dma channel in use */
> 
> Here too.
> 
> Etc, etc, etc ...

Okay, will take care of this.

> 
>> +	dma->chan = dma->chans[id];
>> +	dma_buf = dma_map_single(dev, buf, len, DMA_FROM_DEVICE);
>> +	ret = dma_mapping_error(dev, dma_buf);
>> +	if (ret)
>> +		goto unlock;
>> +
>> +	/* Prepare DMA read from timer registers, using DMA burst mode */
> 
> This is good.
> [...]
> 
> [...]
> 
>> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
>> index 2aadab6..585a4de 100644
>> --- a/include/linux/mfd/stm32-timers.h
>> +++ b/include/linux/mfd/stm32-timers.h
>> @@ -8,6 +8,8 @@
>>  #define _LINUX_STM32_GPTIMER_H_
>>  
>>  #include <linux/clk.h>
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>>  #include <linux/regmap.h>
> 
> [...]
> 
>> +enum stm32_timers_dmas {
>> +	STM32_TIMERS_DMA_CH1,
>> +	STM32_TIMERS_DMA_CH2,
>> +	STM32_TIMERS_DMA_CH3,
>> +	STM32_TIMERS_DMA_CH4,
>> +	STM32_TIMERS_DMA_UP,
>> +	STM32_TIMERS_DMA_TRIG,
>> +	STM32_TIMERS_DMA_COM,
>> +	STM32_TIMERS_MAX_DMAS,
>> +};
>> +
>> +struct stm32_timers_dma;
> 
> Why don't you move the declaration into here?

To follow previous discussions we had in V1 and V2, this is to avoid
sharing all the information with child drivers, e.g. passing physical
address of parent MFD into child devices.

I should probably add a comment there ? Something like:

/* STM32 timers MFD parent internal struct to handle DMA transfers */
struct stm32_timers_dma;

Do you agree with this ?

Thanks for reviewing,
BR,
Fabrice

> 
> Then you don't need to forward declare.
> 
>>  struct stm32_timers {
>>  	struct clk *clk;
>>  	struct regmap *regmap;
>>  	u32 max_arr;
>> +	struct stm32_timers_dma *dma;
>>  };
>> +
>> +int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
>> +				enum stm32_timers_dmas id, u32 reg,
>> +				unsigned int num_reg, unsigned int bursts,
>> +				unsigned long tmo_ms);
>>  #endif
> 

Re: [PATCH v3 2/6] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

On 04/16/2018 02:22 PM, Lee Jones wrote:
> On Fri, 30 Mar 2018, Fabrice Gasnier wrote:
> 
>> STM32 Timers can support up to 7 DMA requests:
>> - 4 channels, update, compare and trigger.
>> Optionally request part, or all DMAs from stm32-timers MFD core.
>>
>> Also add routine to implement burst reads using DMA from timer registers.
>> This is exported. So, it can be used by child drivers, PWM capture
>> for instance (but not limited to).
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>> ---
>> Changes in v3:
>> - Basically Lee's comments:
>> - rather create a struct stm32_timers_dma, and place a reference to it
>>   in existing ddata (instead of adding priv struct).
>> - rather use a struct device in exported routine prototype, and use
>>   standard helpers instead of ddata. Get rid of to_stm32_timers_priv().
>> - simplify error handling in probe (remove a goto)
>> - comment on devm_of_platform_*populate() usage.
>>
>> Changes in v2:
>> - Abstract DMA handling from child driver: move it to MFD core
>> - Add comments on optional dma support
>> ---
>>  drivers/mfd/stm32-timers.c       | 219 ++++++++++++++++++++++++++++++++++++++-
>>  include/linux/mfd/stm32-timers.h |  32 ++++++
>>  2 files changed, 249 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
>> index 1d347e5..98191ec 100644
>> --- a/drivers/mfd/stm32-timers.c
>> +++ b/drivers/mfd/stm32-timers.c
>> @@ -4,16 +4,165 @@
>>   * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
>>   */
>>  
>> +#include <linux/bitfield.h>
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>>  #include <linux/mfd/stm32-timers.h>
>>  #include <linux/module.h>
>>  #include <linux/of_platform.h>
>>  #include <linux/reset.h>
>>  
>> +#define STM32_TIMERS_MAX_REGISTERS	0x3fc
>> +
>> +struct stm32_timers_dma {
>> +	struct completion completion;
>> +	phys_addr_t phys_base;
>> +	struct mutex lock;		/* protect dma access */
> 
> Nit: I like comments to use good grammar i.e. capital letters to
> start a sentence and 's/dma/DMA/'.  Or better still, drop the comment,
> we know what the lock is for.
> 
>> +	struct dma_chan *chan;
>> +	struct dma_chan *chans[STM32_TIMERS_MAX_DMAS];
> 
> This requires explanation.  Maybe a kerneldoc header would be good here.

Hi Lee,

I'll add kerneldoc in next version.

> 
> [...]
> 
>> +/**
>> + * stm32_timers_dma_burst_read - Read from timers registers using DMA.
>> + *
>> + * Read from STM32 timers registers using DMA on a single event.
>> + * @dev: reference to stm32_timers MFD device
>> + * @buf: dma'able destination buffer
>> + * @id: stm32_timers_dmas event identifier (ch[1..4], up, trig or com)
>> + * @reg: registers start offset for DMA to read from (like CCRx for capture)
>> + * @num_reg: number of registers to read upon each dma request, starting @reg.
>> + * @bursts: number of bursts to read (e.g. like two for pwm period capture)
>> + * @tmo_ms: timeout (milliseconds)
>> + */
>> +int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
>> +				enum stm32_timers_dmas id, u32 reg,
>> +				unsigned int num_reg, unsigned int bursts,
>> +				unsigned long tmo_ms)
>> +{
>> +	struct stm32_timers *ddata = dev_get_drvdata(dev);
>> +	unsigned long timeout = msecs_to_jiffies(tmo_ms);
>> +	struct regmap *regmap = ddata->regmap;
>> +	struct stm32_timers_dma *dma = ddata->dma;
>> +	size_t len = num_reg * bursts * sizeof(u32);
>> +	struct dma_async_tx_descriptor *desc;
>> +	struct dma_slave_config config;
>> +	dma_cookie_t cookie;
>> +	dma_addr_t dma_buf;
>> +	u32 dbl, dba;
>> +	long err;
>> +	int ret;
>> +
>> +	/* sanity check */
> 
> Proper grammar in all comments please.
> 
> "Sanity check"
> 
> [...]
> 
>> +	/* select dma channel in use */
> 
> Here too.
> 
> Etc, etc, etc ...

Okay, will take care of this.

> 
>> +	dma->chan = dma->chans[id];
>> +	dma_buf = dma_map_single(dev, buf, len, DMA_FROM_DEVICE);
>> +	ret = dma_mapping_error(dev, dma_buf);
>> +	if (ret)
>> +		goto unlock;
>> +
>> +	/* Prepare DMA read from timer registers, using DMA burst mode */
> 
> This is good.
> [...]
> 
> [...]
> 
>> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
>> index 2aadab6..585a4de 100644
>> --- a/include/linux/mfd/stm32-timers.h
>> +++ b/include/linux/mfd/stm32-timers.h
>> @@ -8,6 +8,8 @@
>>  #define _LINUX_STM32_GPTIMER_H_
>>  
>>  #include <linux/clk.h>
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>>  #include <linux/regmap.h>
> 
> [...]
> 
>> +enum stm32_timers_dmas {
>> +	STM32_TIMERS_DMA_CH1,
>> +	STM32_TIMERS_DMA_CH2,
>> +	STM32_TIMERS_DMA_CH3,
>> +	STM32_TIMERS_DMA_CH4,
>> +	STM32_TIMERS_DMA_UP,
>> +	STM32_TIMERS_DMA_TRIG,
>> +	STM32_TIMERS_DMA_COM,
>> +	STM32_TIMERS_MAX_DMAS,
>> +};
>> +
>> +struct stm32_timers_dma;
> 
> Why don't you move the declaration into here?

To follow previous discussions we had in V1 and V2, this is to avoid
sharing all the information with child drivers, e.g. passing physical
address of parent MFD into child devices.

I should probably add a comment there ? Something like:

/* STM32 timers MFD parent internal struct to handle DMA transfers */
struct stm32_timers_dma;

Do you agree with this ?

Thanks for reviewing,
BR,
Fabrice

> 
> Then you don't need to forward declare.
> 
>>  struct stm32_timers {
>>  	struct clk *clk;
>>  	struct regmap *regmap;
>>  	u32 max_arr;
>> +	struct stm32_timers_dma *dma;
>>  };
>> +
>> +int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
>> +				enum stm32_timers_dmas id, u32 reg,
>> +				unsigned int num_reg, unsigned int bursts,
>> +				unsigned long tmo_ms);
>>  #endif
> 

[PATCH v3 2/6] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

On 04/16/2018 02:22 PM, Lee Jones wrote:
> On Fri, 30 Mar 2018, Fabrice Gasnier wrote:
> 
>> STM32 Timers can support up to 7 DMA requests:
>> - 4 channels, update, compare and trigger.
>> Optionally request part, or all DMAs from stm32-timers MFD core.
>>
>> Also add routine to implement burst reads using DMA from timer registers.
>> This is exported. So, it can be used by child drivers, PWM capture
>> for instance (but not limited to).
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>> ---
>> Changes in v3:
>> - Basically Lee's comments:
>> - rather create a struct stm32_timers_dma, and place a reference to it
>>   in existing ddata (instead of adding priv struct).
>> - rather use a struct device in exported routine prototype, and use
>>   standard helpers instead of ddata. Get rid of to_stm32_timers_priv().
>> - simplify error handling in probe (remove a goto)
>> - comment on devm_of_platform_*populate() usage.
>>
>> Changes in v2:
>> - Abstract DMA handling from child driver: move it to MFD core
>> - Add comments on optional dma support
>> ---
>>  drivers/mfd/stm32-timers.c       | 219 ++++++++++++++++++++++++++++++++++++++-
>>  include/linux/mfd/stm32-timers.h |  32 ++++++
>>  2 files changed, 249 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
>> index 1d347e5..98191ec 100644
>> --- a/drivers/mfd/stm32-timers.c
>> +++ b/drivers/mfd/stm32-timers.c
>> @@ -4,16 +4,165 @@
>>   * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
>>   */
>>  
>> +#include <linux/bitfield.h>
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>>  #include <linux/mfd/stm32-timers.h>
>>  #include <linux/module.h>
>>  #include <linux/of_platform.h>
>>  #include <linux/reset.h>
>>  
>> +#define STM32_TIMERS_MAX_REGISTERS	0x3fc
>> +
>> +struct stm32_timers_dma {
>> +	struct completion completion;
>> +	phys_addr_t phys_base;
>> +	struct mutex lock;		/* protect dma access */
> 
> Nit: I like comments to use good grammar i.e. capital letters to
> start a sentence and 's/dma/DMA/'.  Or better still, drop the comment,
> we know what the lock is for.
> 
>> +	struct dma_chan *chan;
>> +	struct dma_chan *chans[STM32_TIMERS_MAX_DMAS];
> 
> This requires explanation.  Maybe a kerneldoc header would be good here.

Hi Lee,

I'll add kerneldoc in next version.

> 
> [...]
> 
>> +/**
>> + * stm32_timers_dma_burst_read - Read from timers registers using DMA.
>> + *
>> + * Read from STM32 timers registers using DMA on a single event.
>> + * @dev: reference to stm32_timers MFD device
>> + * @buf: dma'able destination buffer
>> + * @id: stm32_timers_dmas event identifier (ch[1..4], up, trig or com)
>> + * @reg: registers start offset for DMA to read from (like CCRx for capture)
>> + * @num_reg: number of registers to read upon each dma request, starting @reg.
>> + * @bursts: number of bursts to read (e.g. like two for pwm period capture)
>> + * @tmo_ms: timeout (milliseconds)
>> + */
>> +int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
>> +				enum stm32_timers_dmas id, u32 reg,
>> +				unsigned int num_reg, unsigned int bursts,
>> +				unsigned long tmo_ms)
>> +{
>> +	struct stm32_timers *ddata = dev_get_drvdata(dev);
>> +	unsigned long timeout = msecs_to_jiffies(tmo_ms);
>> +	struct regmap *regmap = ddata->regmap;
>> +	struct stm32_timers_dma *dma = ddata->dma;
>> +	size_t len = num_reg * bursts * sizeof(u32);
>> +	struct dma_async_tx_descriptor *desc;
>> +	struct dma_slave_config config;
>> +	dma_cookie_t cookie;
>> +	dma_addr_t dma_buf;
>> +	u32 dbl, dba;
>> +	long err;
>> +	int ret;
>> +
>> +	/* sanity check */
> 
> Proper grammar in all comments please.
> 
> "Sanity check"
> 
> [...]
> 
>> +	/* select dma channel in use */
> 
> Here too.
> 
> Etc, etc, etc ...

Okay, will take care of this.

> 
>> +	dma->chan = dma->chans[id];
>> +	dma_buf = dma_map_single(dev, buf, len, DMA_FROM_DEVICE);
>> +	ret = dma_mapping_error(dev, dma_buf);
>> +	if (ret)
>> +		goto unlock;
>> +
>> +	/* Prepare DMA read from timer registers, using DMA burst mode */
> 
> This is good.
> [...]
> 
> [...]
> 
>> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
>> index 2aadab6..585a4de 100644
>> --- a/include/linux/mfd/stm32-timers.h
>> +++ b/include/linux/mfd/stm32-timers.h
>> @@ -8,6 +8,8 @@
>>  #define _LINUX_STM32_GPTIMER_H_
>>  
>>  #include <linux/clk.h>
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>>  #include <linux/regmap.h>
> 
> [...]
> 
>> +enum stm32_timers_dmas {
>> +	STM32_TIMERS_DMA_CH1,
>> +	STM32_TIMERS_DMA_CH2,
>> +	STM32_TIMERS_DMA_CH3,
>> +	STM32_TIMERS_DMA_CH4,
>> +	STM32_TIMERS_DMA_UP,
>> +	STM32_TIMERS_DMA_TRIG,
>> +	STM32_TIMERS_DMA_COM,
>> +	STM32_TIMERS_MAX_DMAS,
>> +};
>> +
>> +struct stm32_timers_dma;
> 
> Why don't you move the declaration into here?

To follow previous discussions we had in V1 and V2, this is to avoid
sharing all the information with child drivers, e.g. passing physical
address of parent MFD into child devices.

I should probably add a comment there ? Something like:

/* STM32 timers MFD parent internal struct to handle DMA transfers */
struct stm32_timers_dma;

Do you agree with this ?

Thanks for reviewing,
BR,
Fabrice

> 
> Then you don't need to forward declare.
> 
>>  struct stm32_timers {
>>  	struct clk *clk;
>>  	struct regmap *regmap;
>>  	u32 max_arr;
>> +	struct stm32_timers_dma *dma;
>>  };
>> +
>> +int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
>> +				enum stm32_timers_dmas id, u32 reg,
>> +				unsigned int num_reg, unsigned int bursts,
>> +				unsigned long tmo_ms);
>>  #endif
> 

Re: [PATCH v3 2/6] mfd: stm32-timers: add support for dmas

[Lee Jones]

On Mon, 16 Apr 2018, Fabrice Gasnier wrote:

> On 04/16/2018 02:22 PM, Lee Jones wrote:
> > On Fri, 30 Mar 2018, Fabrice Gasnier wrote:
> > 
> >> STM32 Timers can support up to 7 DMA requests:
> >> - 4 channels, update, compare and trigger.
> >> Optionally request part, or all DMAs from stm32-timers MFD core.
> >>
> >> Also add routine to implement burst reads using DMA from timer registers.
> >> This is exported. So, it can be used by child drivers, PWM capture
> >> for instance (but not limited to).
> >>
> >> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> >> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> >> ---
> >> Changes in v3:
> >> - Basically Lee's comments:
> >> - rather create a struct stm32_timers_dma, and place a reference to it
> >>   in existing ddata (instead of adding priv struct).
> >> - rather use a struct device in exported routine prototype, and use
> >>   standard helpers instead of ddata. Get rid of to_stm32_timers_priv().
> >> - simplify error handling in probe (remove a goto)
> >> - comment on devm_of_platform_*populate() usage.
> >>
> >> Changes in v2:
> >> - Abstract DMA handling from child driver: move it to MFD core
> >> - Add comments on optional dma support
> >> ---
> >>  drivers/mfd/stm32-timers.c       | 219 ++++++++++++++++++++++++++++++++++++++-
> >>  include/linux/mfd/stm32-timers.h |  32 ++++++
> >>  2 files changed, 249 insertions(+), 2 deletions(-)

[...]

> >> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
> >> index 2aadab6..585a4de 100644
> >> --- a/include/linux/mfd/stm32-timers.h
> >> +++ b/include/linux/mfd/stm32-timers.h
> >> @@ -8,6 +8,8 @@
> >>  #define _LINUX_STM32_GPTIMER_H_
> >>  
> >>  #include <linux/clk.h>
> >> +#include <linux/dmaengine.h>
> >> +#include <linux/dma-mapping.h>
> >>  #include <linux/regmap.h>
> > 
> > [...]
> > 
> >> +enum stm32_timers_dmas {
> >> +	STM32_TIMERS_DMA_CH1,
> >> +	STM32_TIMERS_DMA_CH2,
> >> +	STM32_TIMERS_DMA_CH3,
> >> +	STM32_TIMERS_DMA_CH4,
> >> +	STM32_TIMERS_DMA_UP,
> >> +	STM32_TIMERS_DMA_TRIG,
> >> +	STM32_TIMERS_DMA_COM,
> >> +	STM32_TIMERS_MAX_DMAS,
> >> +};
> >> +
> >> +struct stm32_timers_dma;
> > 
> > Why don't you move the declaration into here?
> 
> To follow previous discussions we had in V1 and V2, this is to avoid
> sharing all the information with child drivers, e.g. passing physical
> address of parent MFD into child devices.
> 
> I should probably add a comment there ? Something like:
> 
> /* STM32 timers MFD parent internal struct to handle DMA transfers */
> struct stm32_timers_dma;
> 
> Do you agree with this ?
> 
> Thanks for reviewing,
> BR,
> Fabrice
> 
> > 
> > Then you don't need to forward declare.

Yes, I remember our previous conversation.

Perhaps you could always put a comment like:

> >>  struct stm32_timers {
> >>  	struct clk *clk;
> >>  	struct regmap *regmap;
> >>  	u32 max_arr;
> >> +	struct stm32_timers_dma *dma; /* Only to be used by the parent */
> >>  };

If this is not acceptable, then the current solution will do.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v3 2/6] mfd: stm32-timers: add support for dmas

[Lee Jones]

On Mon, 16 Apr 2018, Fabrice Gasnier wrote:

> On 04/16/2018 02:22 PM, Lee Jones wrote:
> > On Fri, 30 Mar 2018, Fabrice Gasnier wrote:
> > 
> >> STM32 Timers can support up to 7 DMA requests:
> >> - 4 channels, update, compare and trigger.
> >> Optionally request part, or all DMAs from stm32-timers MFD core.
> >>
> >> Also add routine to implement burst reads using DMA from timer registers.
> >> This is exported. So, it can be used by child drivers, PWM capture
> >> for instance (but not limited to).
> >>
> >> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> >> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> >> ---
> >> Changes in v3:
> >> - Basically Lee's comments:
> >> - rather create a struct stm32_timers_dma, and place a reference to it
> >>   in existing ddata (instead of adding priv struct).
> >> - rather use a struct device in exported routine prototype, and use
> >>   standard helpers instead of ddata. Get rid of to_stm32_timers_priv().
> >> - simplify error handling in probe (remove a goto)
> >> - comment on devm_of_platform_*populate() usage.
> >>
> >> Changes in v2:
> >> - Abstract DMA handling from child driver: move it to MFD core
> >> - Add comments on optional dma support
> >> ---
> >>  drivers/mfd/stm32-timers.c       | 219 ++++++++++++++++++++++++++++++++++++++-
> >>  include/linux/mfd/stm32-timers.h |  32 ++++++
> >>  2 files changed, 249 insertions(+), 2 deletions(-)

[...]

> >> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
> >> index 2aadab6..585a4de 100644
> >> --- a/include/linux/mfd/stm32-timers.h
> >> +++ b/include/linux/mfd/stm32-timers.h
> >> @@ -8,6 +8,8 @@
> >>  #define _LINUX_STM32_GPTIMER_H_
> >>  
> >>  #include <linux/clk.h>
> >> +#include <linux/dmaengine.h>
> >> +#include <linux/dma-mapping.h>
> >>  #include <linux/regmap.h>
> > 
> > [...]
> > 
> >> +enum stm32_timers_dmas {
> >> +	STM32_TIMERS_DMA_CH1,
> >> +	STM32_TIMERS_DMA_CH2,
> >> +	STM32_TIMERS_DMA_CH3,
> >> +	STM32_TIMERS_DMA_CH4,
> >> +	STM32_TIMERS_DMA_UP,
> >> +	STM32_TIMERS_DMA_TRIG,
> >> +	STM32_TIMERS_DMA_COM,
> >> +	STM32_TIMERS_MAX_DMAS,
> >> +};
> >> +
> >> +struct stm32_timers_dma;
> > 
> > Why don't you move the declaration into here?
> 
> To follow previous discussions we had in V1 and V2, this is to avoid
> sharing all the information with child drivers, e.g. passing physical
> address of parent MFD into child devices.
> 
> I should probably add a comment there ? Something like:
> 
> /* STM32 timers MFD parent internal struct to handle DMA transfers */
> struct stm32_timers_dma;
> 
> Do you agree with this ?
> 
> Thanks for reviewing,
> BR,
> Fabrice
> 
> > 
> > Then you don't need to forward declare.

Yes, I remember our previous conversation.

Perhaps you could always put a comment like:

> >>  struct stm32_timers {
> >>  	struct clk *clk;
> >>  	struct regmap *regmap;
> >>  	u32 max_arr;
> >> +	struct stm32_timers_dma *dma; /* Only to be used by the parent */
> >>  };

If this is not acceptable, then the current solution will do.

-- 
Lee Jones [???]
Linaro Services Technical Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 2/6] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

On 04/16/2018 04:47 PM, Lee Jones wrote:
> On Mon, 16 Apr 2018, Fabrice Gasnier wrote:
> 
>> On 04/16/2018 02:22 PM, Lee Jones wrote:
>>> On Fri, 30 Mar 2018, Fabrice Gasnier wrote:
>>>
>>>> STM32 Timers can support up to 7 DMA requests:
>>>> - 4 channels, update, compare and trigger.
>>>> Optionally request part, or all DMAs from stm32-timers MFD core.
>>>>
>>>> Also add routine to implement burst reads using DMA from timer registers.
>>>> This is exported. So, it can be used by child drivers, PWM capture
>>>> for instance (but not limited to).
>>>>
>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>>> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>>>> ---
>>>> Changes in v3:
>>>> - Basically Lee's comments:
>>>> - rather create a struct stm32_timers_dma, and place a reference to it
>>>>   in existing ddata (instead of adding priv struct).
>>>> - rather use a struct device in exported routine prototype, and use
>>>>   standard helpers instead of ddata. Get rid of to_stm32_timers_priv().
>>>> - simplify error handling in probe (remove a goto)
>>>> - comment on devm_of_platform_*populate() usage.
>>>>
>>>> Changes in v2:
>>>> - Abstract DMA handling from child driver: move it to MFD core
>>>> - Add comments on optional dma support
>>>> ---
>>>>  drivers/mfd/stm32-timers.c       | 219 ++++++++++++++++++++++++++++++++++++++-
>>>>  include/linux/mfd/stm32-timers.h |  32 ++++++
>>>>  2 files changed, 249 insertions(+), 2 deletions(-)
> 
> [...]
> 
>>>> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
>>>> index 2aadab6..585a4de 100644
>>>> --- a/include/linux/mfd/stm32-timers.h
>>>> +++ b/include/linux/mfd/stm32-timers.h
>>>> @@ -8,6 +8,8 @@
>>>>  #define _LINUX_STM32_GPTIMER_H_
>>>>  
>>>>  #include <linux/clk.h>
>>>> +#include <linux/dmaengine.h>
>>>> +#include <linux/dma-mapping.h>
>>>>  #include <linux/regmap.h>
>>>
>>> [...]
>>>
>>>> +enum stm32_timers_dmas {
>>>> +	STM32_TIMERS_DMA_CH1,
>>>> +	STM32_TIMERS_DMA_CH2,
>>>> +	STM32_TIMERS_DMA_CH3,
>>>> +	STM32_TIMERS_DMA_CH4,
>>>> +	STM32_TIMERS_DMA_UP,
>>>> +	STM32_TIMERS_DMA_TRIG,
>>>> +	STM32_TIMERS_DMA_COM,
>>>> +	STM32_TIMERS_MAX_DMAS,
>>>> +};
>>>> +
>>>> +struct stm32_timers_dma;
>>>
>>> Why don't you move the declaration into here?
>>
>> To follow previous discussions we had in V1 and V2, this is to avoid
>> sharing all the information with child drivers, e.g. passing physical
>> address of parent MFD into child devices.
>>
>> I should probably add a comment there ? Something like:
>>
>> /* STM32 timers MFD parent internal struct to handle DMA transfers */
>> struct stm32_timers_dma;
>>
>> Do you agree with this ?
>>
>> Thanks for reviewing,
>> BR,
>> Fabrice
>>
>>>
>>> Then you don't need to forward declare.
> 
> Yes, I remember our previous conversation.
> 
> Perhaps you could always put a comment like:
> 
>>>>  struct stm32_timers {
>>>>  	struct clk *clk;
>>>>  	struct regmap *regmap;
>>>>  	u32 max_arr;
>>>> +	struct stm32_timers_dma *dma; /* Only to be used by the parent */
>>>>  };
> 
> If this is not acceptable, then the current solution will do.

Hi Lee,

I'll update it with your proposal,

Many thanks,
Fabrice

> 

Re: [PATCH v3 2/6] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

On 04/16/2018 04:47 PM, Lee Jones wrote:
> On Mon, 16 Apr 2018, Fabrice Gasnier wrote:
> 
>> On 04/16/2018 02:22 PM, Lee Jones wrote:
>>> On Fri, 30 Mar 2018, Fabrice Gasnier wrote:
>>>
>>>> STM32 Timers can support up to 7 DMA requests:
>>>> - 4 channels, update, compare and trigger.
>>>> Optionally request part, or all DMAs from stm32-timers MFD core.
>>>>
>>>> Also add routine to implement burst reads using DMA from timer registers.
>>>> This is exported. So, it can be used by child drivers, PWM capture
>>>> for instance (but not limited to).
>>>>
>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>>> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>>>> ---
>>>> Changes in v3:
>>>> - Basically Lee's comments:
>>>> - rather create a struct stm32_timers_dma, and place a reference to it
>>>>   in existing ddata (instead of adding priv struct).
>>>> - rather use a struct device in exported routine prototype, and use
>>>>   standard helpers instead of ddata. Get rid of to_stm32_timers_priv().
>>>> - simplify error handling in probe (remove a goto)
>>>> - comment on devm_of_platform_*populate() usage.
>>>>
>>>> Changes in v2:
>>>> - Abstract DMA handling from child driver: move it to MFD core
>>>> - Add comments on optional dma support
>>>> ---
>>>>  drivers/mfd/stm32-timers.c       | 219 ++++++++++++++++++++++++++++++++++++++-
>>>>  include/linux/mfd/stm32-timers.h |  32 ++++++
>>>>  2 files changed, 249 insertions(+), 2 deletions(-)
> 
> [...]
> 
>>>> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
>>>> index 2aadab6..585a4de 100644
>>>> --- a/include/linux/mfd/stm32-timers.h
>>>> +++ b/include/linux/mfd/stm32-timers.h
>>>> @@ -8,6 +8,8 @@
>>>>  #define _LINUX_STM32_GPTIMER_H_
>>>>  
>>>>  #include <linux/clk.h>
>>>> +#include <linux/dmaengine.h>
>>>> +#include <linux/dma-mapping.h>
>>>>  #include <linux/regmap.h>
>>>
>>> [...]
>>>
>>>> +enum stm32_timers_dmas {
>>>> +	STM32_TIMERS_DMA_CH1,
>>>> +	STM32_TIMERS_DMA_CH2,
>>>> +	STM32_TIMERS_DMA_CH3,
>>>> +	STM32_TIMERS_DMA_CH4,
>>>> +	STM32_TIMERS_DMA_UP,
>>>> +	STM32_TIMERS_DMA_TRIG,
>>>> +	STM32_TIMERS_DMA_COM,
>>>> +	STM32_TIMERS_MAX_DMAS,
>>>> +};
>>>> +
>>>> +struct stm32_timers_dma;
>>>
>>> Why don't you move the declaration into here?
>>
>> To follow previous discussions we had in V1 and V2, this is to avoid
>> sharing all the information with child drivers, e.g. passing physical
>> address of parent MFD into child devices.
>>
>> I should probably add a comment there ? Something like:
>>
>> /* STM32 timers MFD parent internal struct to handle DMA transfers */
>> struct stm32_timers_dma;
>>
>> Do you agree with this ?
>>
>> Thanks for reviewing,
>> BR,
>> Fabrice
>>
>>>
>>> Then you don't need to forward declare.
> 
> Yes, I remember our previous conversation.
> 
> Perhaps you could always put a comment like:
> 
>>>>  struct stm32_timers {
>>>>  	struct clk *clk;
>>>>  	struct regmap *regmap;
>>>>  	u32 max_arr;
>>>> +	struct stm32_timers_dma *dma; /* Only to be used by the parent */
>>>>  };
> 
> If this is not acceptable, then the current solution will do.

Hi Lee,

I'll update it with your proposal,

Many thanks,
Fabrice

> 

[PATCH v3 2/6] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

On 04/16/2018 04:47 PM, Lee Jones wrote:
> On Mon, 16 Apr 2018, Fabrice Gasnier wrote:
> 
>> On 04/16/2018 02:22 PM, Lee Jones wrote:
>>> On Fri, 30 Mar 2018, Fabrice Gasnier wrote:
>>>
>>>> STM32 Timers can support up to 7 DMA requests:
>>>> - 4 channels, update, compare and trigger.
>>>> Optionally request part, or all DMAs from stm32-timers MFD core.
>>>>
>>>> Also add routine to implement burst reads using DMA from timer registers.
>>>> This is exported. So, it can be used by child drivers, PWM capture
>>>> for instance (but not limited to).
>>>>
>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>>> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>>>> ---
>>>> Changes in v3:
>>>> - Basically Lee's comments:
>>>> - rather create a struct stm32_timers_dma, and place a reference to it
>>>>   in existing ddata (instead of adding priv struct).
>>>> - rather use a struct device in exported routine prototype, and use
>>>>   standard helpers instead of ddata. Get rid of to_stm32_timers_priv().
>>>> - simplify error handling in probe (remove a goto)
>>>> - comment on devm_of_platform_*populate() usage.
>>>>
>>>> Changes in v2:
>>>> - Abstract DMA handling from child driver: move it to MFD core
>>>> - Add comments on optional dma support
>>>> ---
>>>>  drivers/mfd/stm32-timers.c       | 219 ++++++++++++++++++++++++++++++++++++++-
>>>>  include/linux/mfd/stm32-timers.h |  32 ++++++
>>>>  2 files changed, 249 insertions(+), 2 deletions(-)
> 
> [...]
> 
>>>> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
>>>> index 2aadab6..585a4de 100644
>>>> --- a/include/linux/mfd/stm32-timers.h
>>>> +++ b/include/linux/mfd/stm32-timers.h
>>>> @@ -8,6 +8,8 @@
>>>>  #define _LINUX_STM32_GPTIMER_H_
>>>>  
>>>>  #include <linux/clk.h>
>>>> +#include <linux/dmaengine.h>
>>>> +#include <linux/dma-mapping.h>
>>>>  #include <linux/regmap.h>
>>>
>>> [...]
>>>
>>>> +enum stm32_timers_dmas {
>>>> +	STM32_TIMERS_DMA_CH1,
>>>> +	STM32_TIMERS_DMA_CH2,
>>>> +	STM32_TIMERS_DMA_CH3,
>>>> +	STM32_TIMERS_DMA_CH4,
>>>> +	STM32_TIMERS_DMA_UP,
>>>> +	STM32_TIMERS_DMA_TRIG,
>>>> +	STM32_TIMERS_DMA_COM,
>>>> +	STM32_TIMERS_MAX_DMAS,
>>>> +};
>>>> +
>>>> +struct stm32_timers_dma;
>>>
>>> Why don't you move the declaration into here?
>>
>> To follow previous discussions we had in V1 and V2, this is to avoid
>> sharing all the information with child drivers, e.g. passing physical
>> address of parent MFD into child devices.
>>
>> I should probably add a comment there ? Something like:
>>
>> /* STM32 timers MFD parent internal struct to handle DMA transfers */
>> struct stm32_timers_dma;
>>
>> Do you agree with this ?
>>
>> Thanks for reviewing,
>> BR,
>> Fabrice
>>
>>>
>>> Then you don't need to forward declare.
> 
> Yes, I remember our previous conversation.
> 
> Perhaps you could always put a comment like:
> 
>>>>  struct stm32_timers {
>>>>  	struct clk *clk;
>>>>  	struct regmap *regmap;
>>>>  	u32 max_arr;
>>>> +	struct stm32_timers_dma *dma; /* Only to be used by the parent */
>>>>  };
> 
> If this is not acceptable, then the current solution will do.

Hi Lee,

I'll update it with your proposal,

Many thanks,
Fabrice

>