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

[PATCH 0/8] 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 are precursor patches
- Subsequent 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.

Fabrice Gasnier (8):
  pwm: stm32: fix, remove unused struct device
  pwm: stm32: protect common prescaler for all channels
  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                         |  37 +-
 drivers/pwm/pwm-stm32.c                            | 407 ++++++++++++++++++++-
 include/linux/mfd/stm32-timers.h                   |  31 ++
 5 files changed, 492 insertions(+), 6 deletions(-)

-- 
1.9.1

[PATCH 0/8] 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 are precursor patches
- Subsequent 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.

Fabrice Gasnier (8):
  pwm: stm32: fix, remove unused struct device
  pwm: stm32: protect common prescaler for all channels
  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                         |  37 +-
 drivers/pwm/pwm-stm32.c                            | 407 ++++++++++++++++++++-
 include/linux/mfd/stm32-timers.h                   |  31 ++
 5 files changed, 492 insertions(+), 6 deletions(-)

-- 
1.9.1

[PATCH 1/8] pwm: stm32: fix, remove unused struct device

[Fabrice Gasnier]

dev is never assigned nor used. remove it.

Fixes: 7edf7369205b ("pwm: Add driver for STM32 plaftorm")

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/pwm/pwm-stm32.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 6139512..59524f9 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -21,7 +21,6 @@
 
 struct stm32_pwm {
 	struct pwm_chip chip;
-	struct device *dev;
 	struct clk *clk;
 	struct regmap *regmap;
 	u32 max_arr;
-- 
1.9.1

[PATCH 1/8] pwm: stm32: fix, remove unused struct device

[Fabrice Gasnier]

dev is never assigned nor used. remove it.

Fixes: 7edf7369205b ("pwm: Add driver for STM32 plaftorm")

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/pwm/pwm-stm32.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 6139512..59524f9 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -21,7 +21,6 @@
 
 struct stm32_pwm {
 	struct pwm_chip chip;
-	struct device *dev;
 	struct clk *clk;
 	struct regmap *regmap;
 	u32 max_arr;
-- 
1.9.1

[PATCH 2/8] pwm: stm32: protect common prescaler for all channels

[Fabrice Gasnier]

There may be a race, when configuring two pwm channels, with different
prescaler values, when there's no active channel yet.
Add mutex lock to avoid concurrent access on pwm apply state.
This is also precursor patch for pwm capture support.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/pwm/pwm-stm32.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 59524f9..3ac55df 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -21,6 +21,7 @@
 
 struct stm32_pwm {
 	struct pwm_chip chip;
+	struct mutex lock; /* protect pwm config/enable */
 	struct clk *clk;
 	struct regmap *regmap;
 	u32 max_arr;
@@ -213,9 +214,23 @@ static int stm32_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	return ret;
 }
 
+static int stm32_pwm_apply_locked(struct pwm_chip *chip, struct pwm_device *pwm,
+				  struct pwm_state *state)
+{
+	struct stm32_pwm *priv = to_stm32_pwm_dev(chip);
+	int ret;
+
+	/* protect common prescaler for all active channels */
+	mutex_lock(&priv->lock);
+	ret = stm32_pwm_apply(chip, pwm, state);
+	mutex_unlock(&priv->lock);
+
+	return ret;
+}
+
 static const struct pwm_ops stm32pwm_ops = {
 	.owner = THIS_MODULE,
-	.apply = stm32_pwm_apply,
+	.apply = stm32_pwm_apply_locked,
 };
 
 static int stm32_pwm_set_breakinput(struct stm32_pwm *priv,
@@ -335,6 +350,7 @@ static int stm32_pwm_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
+	mutex_init(&priv->lock);
 	priv->regmap = ddata->regmap;
 	priv->clk = ddata->clk;
 	priv->max_arr = ddata->max_arr;
-- 
1.9.1

[PATCH 2/8] pwm: stm32: protect common prescaler for all channels

[Fabrice Gasnier]

There may be a race, when configuring two pwm channels, with different
prescaler values, when there's no active channel yet.
Add mutex lock to avoid concurrent access on pwm apply state.
This is also precursor patch for pwm capture support.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/pwm/pwm-stm32.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 59524f9..3ac55df 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -21,6 +21,7 @@
 
 struct stm32_pwm {
 	struct pwm_chip chip;
+	struct mutex lock; /* protect pwm config/enable */
 	struct clk *clk;
 	struct regmap *regmap;
 	u32 max_arr;
@@ -213,9 +214,23 @@ static int stm32_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	return ret;
 }
 
+static int stm32_pwm_apply_locked(struct pwm_chip *chip, struct pwm_device *pwm,
+				  struct pwm_state *state)
+{
+	struct stm32_pwm *priv = to_stm32_pwm_dev(chip);
+	int ret;
+
+	/* protect common prescaler for all active channels */
+	mutex_lock(&priv->lock);
+	ret = stm32_pwm_apply(chip, pwm, state);
+	mutex_unlock(&priv->lock);
+
+	return ret;
+}
+
 static const struct pwm_ops stm32pwm_ops = {
 	.owner = THIS_MODULE,
-	.apply = stm32_pwm_apply,
+	.apply = stm32_pwm_apply_locked,
 };
 
 static int stm32_pwm_set_breakinput(struct stm32_pwm *priv,
@@ -335,6 +350,7 @@ static int stm32_pwm_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
+	mutex_init(&priv->lock);
 	priv->regmap = ddata->regmap;
 	priv->clk = ddata->clk;
 	priv->max_arr = ddata->max_arr;
-- 
1.9.1

[PATCH 3/8] 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>
---
 .../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 3/8] 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>
---
 .../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 4/8] 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, keep reference of device's bus address to allow child drivers to
transfer data from/to device by using dma.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
 include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
index a6675a4..372b51e 100644
--- a/drivers/mfd/stm32-timers.c
+++ b/drivers/mfd/stm32-timers.c
@@ -29,6 +29,23 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
 	regmap_write(ddata->regmap, TIM_ARR, 0x0);
 }
 
+static void stm32_timers_dma_probe(struct device *dev,
+				   struct stm32_timers *ddata)
+{
+	int i;
+	char name[4];
+
+	for (i = STM32_TIMERS_DMA_CH1; i <= STM32_TIMERS_DMA_CH4; i++) {
+		snprintf(name, ARRAY_SIZE(name), "ch%1d", i + 1);
+		ddata->dmas[i] = dma_request_slave_channel(dev, name);
+	}
+	ddata->dmas[STM32_TIMERS_DMA_UP] = dma_request_slave_channel(dev, "up");
+	ddata->dmas[STM32_TIMERS_DMA_TRIG] =
+		dma_request_slave_channel(dev, "trig");
+	ddata->dmas[STM32_TIMERS_DMA_COM] =
+		dma_request_slave_channel(dev, "com");
+}
+
 static int stm32_timers_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
 	mmio = devm_ioremap_resource(dev, res);
 	if (IS_ERR(mmio))
 		return PTR_ERR(mmio);
+	ddata->phys_base = res->start;
 
 	ddata->regmap = devm_regmap_init_mmio_clk(dev, "int", mmio,
 						  &stm32_timers_regmap_cfg);
@@ -56,9 +74,25 @@ static int stm32_timers_probe(struct platform_device *pdev)
 
 	stm32_timers_get_arr_size(ddata);
 
+	stm32_timers_dma_probe(dev, ddata);
+
 	platform_set_drvdata(pdev, ddata);
 
-	return devm_of_platform_populate(&pdev->dev);
+	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+}
+
+static int stm32_timers_remove(struct platform_device *pdev)
+{
+	struct stm32_timers *ddata = platform_get_drvdata(pdev);
+	int i;
+
+	of_platform_depopulate(&pdev->dev);
+
+	for (i = 0; i < STM32_TIMERS_MAX_DMAS; i++)
+		if (ddata->dmas[i])
+			dma_release_channel(ddata->dmas[i]);
+
+	return 0;
 }
 
 static const struct of_device_id stm32_timers_of_match[] = {
@@ -69,6 +103,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 ce7346e..2b4ffb9 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -10,6 +10,7 @@
 #define _LINUX_STM32_GPTIMER_H_
 
 #include <linux/clk.h>
+#include <linux/dmaengine.h>
 #include <linux/regmap.h>
 
 #define TIM_CR1		0x00	/* Control Register 1      */
@@ -67,9 +68,22 @@
 #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 {
 	struct clk *clk;
 	struct regmap *regmap;
+	phys_addr_t phys_base;
 	u32 max_arr;
+	struct dma_chan *dmas[STM32_TIMERS_MAX_DMAS];
 };
 #endif
-- 
1.9.1

[PATCH 4/8] 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, keep reference of device's bus address to allow child drivers to
transfer data from/to device by using dma.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
 include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
index a6675a4..372b51e 100644
--- a/drivers/mfd/stm32-timers.c
+++ b/drivers/mfd/stm32-timers.c
@@ -29,6 +29,23 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
 	regmap_write(ddata->regmap, TIM_ARR, 0x0);
 }
 
+static void stm32_timers_dma_probe(struct device *dev,
+				   struct stm32_timers *ddata)
+{
+	int i;
+	char name[4];
+
+	for (i = STM32_TIMERS_DMA_CH1; i <= STM32_TIMERS_DMA_CH4; i++) {
+		snprintf(name, ARRAY_SIZE(name), "ch%1d", i + 1);
+		ddata->dmas[i] = dma_request_slave_channel(dev, name);
+	}
+	ddata->dmas[STM32_TIMERS_DMA_UP] = dma_request_slave_channel(dev, "up");
+	ddata->dmas[STM32_TIMERS_DMA_TRIG] =
+		dma_request_slave_channel(dev, "trig");
+	ddata->dmas[STM32_TIMERS_DMA_COM] =
+		dma_request_slave_channel(dev, "com");
+}
+
 static int stm32_timers_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
 	mmio = devm_ioremap_resource(dev, res);
 	if (IS_ERR(mmio))
 		return PTR_ERR(mmio);
+	ddata->phys_base = res->start;
 
 	ddata->regmap = devm_regmap_init_mmio_clk(dev, "int", mmio,
 						  &stm32_timers_regmap_cfg);
@@ -56,9 +74,25 @@ static int stm32_timers_probe(struct platform_device *pdev)
 
 	stm32_timers_get_arr_size(ddata);
 
+	stm32_timers_dma_probe(dev, ddata);
+
 	platform_set_drvdata(pdev, ddata);
 
-	return devm_of_platform_populate(&pdev->dev);
+	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+}
+
+static int stm32_timers_remove(struct platform_device *pdev)
+{
+	struct stm32_timers *ddata = platform_get_drvdata(pdev);
+	int i;
+
+	of_platform_depopulate(&pdev->dev);
+
+	for (i = 0; i < STM32_TIMERS_MAX_DMAS; i++)
+		if (ddata->dmas[i])
+			dma_release_channel(ddata->dmas[i]);
+
+	return 0;
 }
 
 static const struct of_device_id stm32_timers_of_match[] = {
@@ -69,6 +103,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 ce7346e..2b4ffb9 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -10,6 +10,7 @@
 #define _LINUX_STM32_GPTIMER_H_
 
 #include <linux/clk.h>
+#include <linux/dmaengine.h>
 #include <linux/regmap.h>
 
 #define TIM_CR1		0x00	/* Control Register 1      */
@@ -67,9 +68,22 @@
 #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 {
 	struct clk *clk;
 	struct regmap *regmap;
+	phys_addr_t phys_base;
 	u32 max_arr;
+	struct dma_chan *dmas[STM32_TIMERS_MAX_DMAS];
 };
 #endif
-- 
1.9.1

Re: [PATCH 4/8] mfd: stm32-timers: add support for dmas

[Lee Jones]

On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
> transfer data from/to device by using dma.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> ---
>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
>  2 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
> index a6675a4..372b51e 100644
> --- a/drivers/mfd/stm32-timers.c
> +++ b/drivers/mfd/stm32-timers.c
> @@ -29,6 +29,23 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
>  	regmap_write(ddata->regmap, TIM_ARR, 0x0);
>  }
>  
> +static void stm32_timers_dma_probe(struct device *dev,
> +				   struct stm32_timers *ddata)
> +{
> +	int i;
> +	char name[4];
> +
> +	for (i = STM32_TIMERS_DMA_CH1; i <= STM32_TIMERS_DMA_CH4; i++) {
> +		snprintf(name, ARRAY_SIZE(name), "ch%1d", i + 1);
> +		ddata->dmas[i] = dma_request_slave_channel(dev, name);

And if any of them fail?

> +	}
> +	ddata->dmas[STM32_TIMERS_DMA_UP] = dma_request_slave_channel(dev, "up");
> +	ddata->dmas[STM32_TIMERS_DMA_TRIG] =
> +		dma_request_slave_channel(dev, "trig");
> +	ddata->dmas[STM32_TIMERS_DMA_COM] =
> +		dma_request_slave_channel(dev, "com");

Can you format these in the same why.  This hurts my eyes.

> +}
> +
>  static int stm32_timers_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
>  	mmio = devm_ioremap_resource(dev, res);
>  	if (IS_ERR(mmio))
>  		return PTR_ERR(mmio);
> +	ddata->phys_base = res->start;

What do you use this for?

>  	ddata->regmap = devm_regmap_init_mmio_clk(dev, "int", mmio,
>  						  &stm32_timers_regmap_cfg);
> @@ -56,9 +74,25 @@ static int stm32_timers_probe(struct platform_device *pdev)
>  
>  	stm32_timers_get_arr_size(ddata);
>  
> +	stm32_timers_dma_probe(dev, ddata);
> +

Surely this can fail?

>  	platform_set_drvdata(pdev, ddata);
>  
> -	return devm_of_platform_populate(&pdev->dev);
> +	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
> +}
> +
> +static int stm32_timers_remove(struct platform_device *pdev)
> +{
> +	struct stm32_timers *ddata = platform_get_drvdata(pdev);
> +	int i;
> +
> +	of_platform_depopulate(&pdev->dev);
> +
> +	for (i = 0; i < STM32_TIMERS_MAX_DMAS; i++)
> +		if (ddata->dmas[i])
> +			dma_release_channel(ddata->dmas[i]);
> +
> +	return 0;
>  }
>  
>  static const struct of_device_id stm32_timers_of_match[] = {
> @@ -69,6 +103,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 ce7346e..2b4ffb9 100644
> --- a/include/linux/mfd/stm32-timers.h
> +++ b/include/linux/mfd/stm32-timers.h
> @@ -10,6 +10,7 @@
>  #define _LINUX_STM32_GPTIMER_H_
>  
>  #include <linux/clk.h>
> +#include <linux/dmaengine.h>
>  #include <linux/regmap.h>
>  
>  #define TIM_CR1		0x00	/* Control Register 1      */
> @@ -67,9 +68,22 @@
>  #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 {
>  	struct clk *clk;
>  	struct regmap *regmap;
> +	phys_addr_t phys_base;
>  	u32 max_arr;
> +	struct dma_chan *dmas[STM32_TIMERS_MAX_DMAS];
>  };
>  #endif

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

[PATCH 4/8] mfd: stm32-timers: add support for dmas

[Lee Jones]

On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
> transfer data from/to device by using dma.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> ---
>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
>  2 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
> index a6675a4..372b51e 100644
> --- a/drivers/mfd/stm32-timers.c
> +++ b/drivers/mfd/stm32-timers.c
> @@ -29,6 +29,23 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
>  	regmap_write(ddata->regmap, TIM_ARR, 0x0);
>  }
>  
> +static void stm32_timers_dma_probe(struct device *dev,
> +				   struct stm32_timers *ddata)
> +{
> +	int i;
> +	char name[4];
> +
> +	for (i = STM32_TIMERS_DMA_CH1; i <= STM32_TIMERS_DMA_CH4; i++) {
> +		snprintf(name, ARRAY_SIZE(name), "ch%1d", i + 1);
> +		ddata->dmas[i] = dma_request_slave_channel(dev, name);

And if any of them fail?

> +	}
> +	ddata->dmas[STM32_TIMERS_DMA_UP] = dma_request_slave_channel(dev, "up");
> +	ddata->dmas[STM32_TIMERS_DMA_TRIG] =
> +		dma_request_slave_channel(dev, "trig");
> +	ddata->dmas[STM32_TIMERS_DMA_COM] =
> +		dma_request_slave_channel(dev, "com");

Can you format these in the same why.  This hurts my eyes.

> +}
> +
>  static int stm32_timers_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
>  	mmio = devm_ioremap_resource(dev, res);
>  	if (IS_ERR(mmio))
>  		return PTR_ERR(mmio);
> +	ddata->phys_base = res->start;

What do you use this for?

>  	ddata->regmap = devm_regmap_init_mmio_clk(dev, "int", mmio,
>  						  &stm32_timers_regmap_cfg);
> @@ -56,9 +74,25 @@ static int stm32_timers_probe(struct platform_device *pdev)
>  
>  	stm32_timers_get_arr_size(ddata);
>  
> +	stm32_timers_dma_probe(dev, ddata);
> +

Surely this can fail?

>  	platform_set_drvdata(pdev, ddata);
>  
> -	return devm_of_platform_populate(&pdev->dev);
> +	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
> +}
> +
> +static int stm32_timers_remove(struct platform_device *pdev)
> +{
> +	struct stm32_timers *ddata = platform_get_drvdata(pdev);
> +	int i;
> +
> +	of_platform_depopulate(&pdev->dev);
> +
> +	for (i = 0; i < STM32_TIMERS_MAX_DMAS; i++)
> +		if (ddata->dmas[i])
> +			dma_release_channel(ddata->dmas[i]);
> +
> +	return 0;
>  }
>  
>  static const struct of_device_id stm32_timers_of_match[] = {
> @@ -69,6 +103,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 ce7346e..2b4ffb9 100644
> --- a/include/linux/mfd/stm32-timers.h
> +++ b/include/linux/mfd/stm32-timers.h
> @@ -10,6 +10,7 @@
>  #define _LINUX_STM32_GPTIMER_H_
>  
>  #include <linux/clk.h>
> +#include <linux/dmaengine.h>
>  #include <linux/regmap.h>
>  
>  #define TIM_CR1		0x00	/* Control Register 1      */
> @@ -67,9 +68,22 @@
>  #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 {
>  	struct clk *clk;
>  	struct regmap *regmap;
> +	phys_addr_t phys_base;
>  	u32 max_arr;
> +	struct dma_chan *dmas[STM32_TIMERS_MAX_DMAS];
>  };
>  #endif

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

Re: [PATCH 4/8] mfd: stm32-timers: add support for dmas

[Lee Jones]

On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
> transfer data from/to device by using dma.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
> ---
>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
>  2 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
> index a6675a4..372b51e 100644
> --- a/drivers/mfd/stm32-timers.c
> +++ b/drivers/mfd/stm32-timers.c
> @@ -29,6 +29,23 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
>  	regmap_write(ddata->regmap, TIM_ARR, 0x0);
>  }
>  
> +static void stm32_timers_dma_probe(struct device *dev,
> +				   struct stm32_timers *ddata)
> +{
> +	int i;
> +	char name[4];
> +
> +	for (i = STM32_TIMERS_DMA_CH1; i <= STM32_TIMERS_DMA_CH4; i++) {
> +		snprintf(name, ARRAY_SIZE(name), "ch%1d", i + 1);
> +		ddata->dmas[i] = dma_request_slave_channel(dev, name);

And if any of them fail?

> +	}
> +	ddata->dmas[STM32_TIMERS_DMA_UP] = dma_request_slave_channel(dev, "up");
> +	ddata->dmas[STM32_TIMERS_DMA_TRIG] =
> +		dma_request_slave_channel(dev, "trig");
> +	ddata->dmas[STM32_TIMERS_DMA_COM] =
> +		dma_request_slave_channel(dev, "com");

Can you format these in the same why.  This hurts my eyes.

> +}
> +
>  static int stm32_timers_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
>  	mmio = devm_ioremap_resource(dev, res);
>  	if (IS_ERR(mmio))
>  		return PTR_ERR(mmio);
> +	ddata->phys_base = res->start;

What do you use this for?

>  	ddata->regmap = devm_regmap_init_mmio_clk(dev, "int", mmio,
>  						  &stm32_timers_regmap_cfg);
> @@ -56,9 +74,25 @@ static int stm32_timers_probe(struct platform_device *pdev)
>  
>  	stm32_timers_get_arr_size(ddata);
>  
> +	stm32_timers_dma_probe(dev, ddata);
> +

Surely this can fail?

>  	platform_set_drvdata(pdev, ddata);
>  
> -	return devm_of_platform_populate(&pdev->dev);
> +	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
> +}
> +
> +static int stm32_timers_remove(struct platform_device *pdev)
> +{
> +	struct stm32_timers *ddata = platform_get_drvdata(pdev);
> +	int i;
> +
> +	of_platform_depopulate(&pdev->dev);
> +
> +	for (i = 0; i < STM32_TIMERS_MAX_DMAS; i++)
> +		if (ddata->dmas[i])
> +			dma_release_channel(ddata->dmas[i]);
> +
> +	return 0;
>  }
>  
>  static const struct of_device_id stm32_timers_of_match[] = {
> @@ -69,6 +103,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 ce7346e..2b4ffb9 100644
> --- a/include/linux/mfd/stm32-timers.h
> +++ b/include/linux/mfd/stm32-timers.h
> @@ -10,6 +10,7 @@
>  #define _LINUX_STM32_GPTIMER_H_
>  
>  #include <linux/clk.h>
> +#include <linux/dmaengine.h>
>  #include <linux/regmap.h>
>  
>  #define TIM_CR1		0x00	/* Control Register 1      */
> @@ -67,9 +68,22 @@
>  #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 {
>  	struct clk *clk;
>  	struct regmap *regmap;
> +	phys_addr_t phys_base;
>  	u32 max_arr;
> +	struct dma_chan *dmas[STM32_TIMERS_MAX_DMAS];
>  };
>  #endif

-- 
Lee Jones
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

On 01/23/2018 02:32 PM, Lee Jones wrote:
> On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
>> transfer data from/to device by using dma.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>> ---
>>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
>>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
>>  2 files changed, 50 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
>> index a6675a4..372b51e 100644
>> --- a/drivers/mfd/stm32-timers.c
>> +++ b/drivers/mfd/stm32-timers.c
>> @@ -29,6 +29,23 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
>>  	regmap_write(ddata->regmap, TIM_ARR, 0x0);
>>  }
>>  
>> +static void stm32_timers_dma_probe(struct device *dev,
>> +				   struct stm32_timers *ddata)
>> +{
>> +	int i;
>> +	char name[4];
>> +
>> +	for (i = STM32_TIMERS_DMA_CH1; i <= STM32_TIMERS_DMA_CH4; i++) {
>> +		snprintf(name, ARRAY_SIZE(name), "ch%1d", i + 1);
>> +		ddata->dmas[i] = dma_request_slave_channel(dev, name);
> 
> And if any of them fail?

Hi Lee,

If some of these fails, reference will be NULL. It is checked in child
driver (pwm for instance) at runtime. Support is being added as an
option: pwm capture will simply be unavailable in this case (fail with
error).

> 
>> +	}
>> +	ddata->dmas[STM32_TIMERS_DMA_UP] = dma_request_slave_channel(dev, "up");
>> +	ddata->dmas[STM32_TIMERS_DMA_TRIG] =
>> +		dma_request_slave_channel(dev, "trig");
>> +	ddata->dmas[STM32_TIMERS_DMA_COM] =
>> +		dma_request_slave_channel(dev, "com");
> 
> Can you format these in the same why.  This hurts my eyes.

I use enum values and try to match as possible with reference manual for
"up", "trig" & "com" names.
Would have some suggestion to beautify this?

> 
>> +}
>> +
>>  static int stm32_timers_probe(struct platform_device *pdev)
>>  {
>>  	struct device *dev = &pdev->dev;
>> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
>>  	mmio = devm_ioremap_resource(dev, res);
>>  	if (IS_ERR(mmio))
>>  		return PTR_ERR(mmio);
>> +	ddata->phys_base = res->start;
> 
> What do you use this for?

This is used in in child driver (pwm) for capture data transfer by dma.

> 
>>  	ddata->regmap = devm_regmap_init_mmio_clk(dev, "int", mmio,
>>  						  &stm32_timers_regmap_cfg);
>> @@ -56,9 +74,25 @@ static int stm32_timers_probe(struct platform_device *pdev)
>>  
>>  	stm32_timers_get_arr_size(ddata);
>>  
>> +	stm32_timers_dma_probe(dev, ddata);
>> +
> 
> Surely this can fail?
Yes. Same as above.

Thanks for reviewing,
Best Regards,
Fabrice

> 
>>  	platform_set_drvdata(pdev, ddata);
>>  
>> -	return devm_of_platform_populate(&pdev->dev);
>> +	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
>> +}
>> +
>> +static int stm32_timers_remove(struct platform_device *pdev)
>> +{
>> +	struct stm32_timers *ddata = platform_get_drvdata(pdev);
>> +	int i;
>> +
>> +	of_platform_depopulate(&pdev->dev);
>> +
>> +	for (i = 0; i < STM32_TIMERS_MAX_DMAS; i++)
>> +		if (ddata->dmas[i])
>> +			dma_release_channel(ddata->dmas[i]);
>> +
>> +	return 0;
>>  }
>>  
>>  static const struct of_device_id stm32_timers_of_match[] = {
>> @@ -69,6 +103,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 ce7346e..2b4ffb9 100644
>> --- a/include/linux/mfd/stm32-timers.h
>> +++ b/include/linux/mfd/stm32-timers.h
>> @@ -10,6 +10,7 @@
>>  #define _LINUX_STM32_GPTIMER_H_
>>  
>>  #include <linux/clk.h>
>> +#include <linux/dmaengine.h>
>>  #include <linux/regmap.h>
>>  
>>  #define TIM_CR1		0x00	/* Control Register 1      */
>> @@ -67,9 +68,22 @@
>>  #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 {
>>  	struct clk *clk;
>>  	struct regmap *regmap;
>> +	phys_addr_t phys_base;
>>  	u32 max_arr;
>> +	struct dma_chan *dmas[STM32_TIMERS_MAX_DMAS];
>>  };
>>  #endif
> 

[PATCH 4/8] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

On 01/23/2018 02:32 PM, Lee Jones wrote:
> On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
>> transfer data from/to device by using dma.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>> ---
>>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
>>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
>>  2 files changed, 50 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
>> index a6675a4..372b51e 100644
>> --- a/drivers/mfd/stm32-timers.c
>> +++ b/drivers/mfd/stm32-timers.c
>> @@ -29,6 +29,23 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
>>  	regmap_write(ddata->regmap, TIM_ARR, 0x0);
>>  }
>>  
>> +static void stm32_timers_dma_probe(struct device *dev,
>> +				   struct stm32_timers *ddata)
>> +{
>> +	int i;
>> +	char name[4];
>> +
>> +	for (i = STM32_TIMERS_DMA_CH1; i <= STM32_TIMERS_DMA_CH4; i++) {
>> +		snprintf(name, ARRAY_SIZE(name), "ch%1d", i + 1);
>> +		ddata->dmas[i] = dma_request_slave_channel(dev, name);
> 
> And if any of them fail?

Hi Lee,

If some of these fails, reference will be NULL. It is checked in child
driver (pwm for instance) at runtime. Support is being added as an
option: pwm capture will simply be unavailable in this case (fail with
error).

> 
>> +	}
>> +	ddata->dmas[STM32_TIMERS_DMA_UP] = dma_request_slave_channel(dev, "up");
>> +	ddata->dmas[STM32_TIMERS_DMA_TRIG] =
>> +		dma_request_slave_channel(dev, "trig");
>> +	ddata->dmas[STM32_TIMERS_DMA_COM] =
>> +		dma_request_slave_channel(dev, "com");
> 
> Can you format these in the same why.  This hurts my eyes.

I use enum values and try to match as possible with reference manual for
"up", "trig" & "com" names.
Would have some suggestion to beautify this?

> 
>> +}
>> +
>>  static int stm32_timers_probe(struct platform_device *pdev)
>>  {
>>  	struct device *dev = &pdev->dev;
>> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
>>  	mmio = devm_ioremap_resource(dev, res);
>>  	if (IS_ERR(mmio))
>>  		return PTR_ERR(mmio);
>> +	ddata->phys_base = res->start;
> 
> What do you use this for?

This is used in in child driver (pwm) for capture data transfer by dma.

> 
>>  	ddata->regmap = devm_regmap_init_mmio_clk(dev, "int", mmio,
>>  						  &stm32_timers_regmap_cfg);
>> @@ -56,9 +74,25 @@ static int stm32_timers_probe(struct platform_device *pdev)
>>  
>>  	stm32_timers_get_arr_size(ddata);
>>  
>> +	stm32_timers_dma_probe(dev, ddata);
>> +
> 
> Surely this can fail?
Yes. Same as above.

Thanks for reviewing,
Best Regards,
Fabrice

> 
>>  	platform_set_drvdata(pdev, ddata);
>>  
>> -	return devm_of_platform_populate(&pdev->dev);
>> +	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
>> +}
>> +
>> +static int stm32_timers_remove(struct platform_device *pdev)
>> +{
>> +	struct stm32_timers *ddata = platform_get_drvdata(pdev);
>> +	int i;
>> +
>> +	of_platform_depopulate(&pdev->dev);
>> +
>> +	for (i = 0; i < STM32_TIMERS_MAX_DMAS; i++)
>> +		if (ddata->dmas[i])
>> +			dma_release_channel(ddata->dmas[i]);
>> +
>> +	return 0;
>>  }
>>  
>>  static const struct of_device_id stm32_timers_of_match[] = {
>> @@ -69,6 +103,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 ce7346e..2b4ffb9 100644
>> --- a/include/linux/mfd/stm32-timers.h
>> +++ b/include/linux/mfd/stm32-timers.h
>> @@ -10,6 +10,7 @@
>>  #define _LINUX_STM32_GPTIMER_H_
>>  
>>  #include <linux/clk.h>
>> +#include <linux/dmaengine.h>
>>  #include <linux/regmap.h>
>>  
>>  #define TIM_CR1		0x00	/* Control Register 1      */
>> @@ -67,9 +68,22 @@
>>  #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 {
>>  	struct clk *clk;
>>  	struct regmap *regmap;
>> +	phys_addr_t phys_base;
>>  	u32 max_arr;
>> +	struct dma_chan *dmas[STM32_TIMERS_MAX_DMAS];
>>  };
>>  #endif
> 

Re: [PATCH 4/8] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

On 01/23/2018 02:32 PM, Lee Jones wrote:
> On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
>> transfer data from/to device by using dma.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>> ---
>>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
>>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
>>  2 files changed, 50 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
>> index a6675a4..372b51e 100644
>> --- a/drivers/mfd/stm32-timers.c
>> +++ b/drivers/mfd/stm32-timers.c
>> @@ -29,6 +29,23 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
>>  	regmap_write(ddata->regmap, TIM_ARR, 0x0);
>>  }
>>  
>> +static void stm32_timers_dma_probe(struct device *dev,
>> +				   struct stm32_timers *ddata)
>> +{
>> +	int i;
>> +	char name[4];
>> +
>> +	for (i = STM32_TIMERS_DMA_CH1; i <= STM32_TIMERS_DMA_CH4; i++) {
>> +		snprintf(name, ARRAY_SIZE(name), "ch%1d", i + 1);
>> +		ddata->dmas[i] = dma_request_slave_channel(dev, name);
> 
> And if any of them fail?

Hi Lee,

If some of these fails, reference will be NULL. It is checked in child
driver (pwm for instance) at runtime. Support is being added as an
option: pwm capture will simply be unavailable in this case (fail with
error).

> 
>> +	}
>> +	ddata->dmas[STM32_TIMERS_DMA_UP] = dma_request_slave_channel(dev, "up");
>> +	ddata->dmas[STM32_TIMERS_DMA_TRIG] =
>> +		dma_request_slave_channel(dev, "trig");
>> +	ddata->dmas[STM32_TIMERS_DMA_COM] =
>> +		dma_request_slave_channel(dev, "com");
> 
> Can you format these in the same why.  This hurts my eyes.

I use enum values and try to match as possible with reference manual for
"up", "trig" & "com" names.
Would have some suggestion to beautify this?

> 
>> +}
>> +
>>  static int stm32_timers_probe(struct platform_device *pdev)
>>  {
>>  	struct device *dev = &pdev->dev;
>> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
>>  	mmio = devm_ioremap_resource(dev, res);
>>  	if (IS_ERR(mmio))
>>  		return PTR_ERR(mmio);
>> +	ddata->phys_base = res->start;
> 
> What do you use this for?

This is used in in child driver (pwm) for capture data transfer by dma.

> 
>>  	ddata->regmap = devm_regmap_init_mmio_clk(dev, "int", mmio,
>>  						  &stm32_timers_regmap_cfg);
>> @@ -56,9 +74,25 @@ static int stm32_timers_probe(struct platform_device *pdev)
>>  
>>  	stm32_timers_get_arr_size(ddata);
>>  
>> +	stm32_timers_dma_probe(dev, ddata);
>> +
> 
> Surely this can fail?
Yes. Same as above.

Thanks for reviewing,
Best Regards,
Fabrice

> 
>>  	platform_set_drvdata(pdev, ddata);
>>  
>> -	return devm_of_platform_populate(&pdev->dev);
>> +	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
>> +}
>> +
>> +static int stm32_timers_remove(struct platform_device *pdev)
>> +{
>> +	struct stm32_timers *ddata = platform_get_drvdata(pdev);
>> +	int i;
>> +
>> +	of_platform_depopulate(&pdev->dev);
>> +
>> +	for (i = 0; i < STM32_TIMERS_MAX_DMAS; i++)
>> +		if (ddata->dmas[i])
>> +			dma_release_channel(ddata->dmas[i]);
>> +
>> +	return 0;
>>  }
>>  
>>  static const struct of_device_id stm32_timers_of_match[] = {
>> @@ -69,6 +103,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 ce7346e..2b4ffb9 100644
>> --- a/include/linux/mfd/stm32-timers.h
>> +++ b/include/linux/mfd/stm32-timers.h
>> @@ -10,6 +10,7 @@
>>  #define _LINUX_STM32_GPTIMER_H_
>>  
>>  #include <linux/clk.h>
>> +#include <linux/dmaengine.h>
>>  #include <linux/regmap.h>
>>  
>>  #define TIM_CR1		0x00	/* Control Register 1      */
>> @@ -67,9 +68,22 @@
>>  #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 {
>>  	struct clk *clk;
>>  	struct regmap *regmap;
>> +	phys_addr_t phys_base;
>>  	u32 max_arr;
>> +	struct dma_chan *dmas[STM32_TIMERS_MAX_DMAS];
>>  };
>>  #endif
> 

Re: [PATCH 4/8] mfd: stm32-timers: add support for dmas

[Lee Jones]

On Tue, 23 Jan 2018, Fabrice Gasnier wrote:

> On 01/23/2018 02:32 PM, Lee Jones wrote:
> > On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
> >> transfer data from/to device by using dma.
> >>
> >> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> >> ---
> >>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
> >>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
> >>  2 files changed, 50 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
> >> index a6675a4..372b51e 100644
> >> --- a/drivers/mfd/stm32-timers.c
> >> +++ b/drivers/mfd/stm32-timers.c
> >> @@ -29,6 +29,23 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
> >>  	regmap_write(ddata->regmap, TIM_ARR, 0x0);
> >>  }
> >>  
> >> +static void stm32_timers_dma_probe(struct device *dev,
> >> +				   struct stm32_timers *ddata)
> >> +{
> >> +	int i;
> >> +	char name[4];
> >> +
> >> +	for (i = STM32_TIMERS_DMA_CH1; i <= STM32_TIMERS_DMA_CH4; i++) {
> >> +		snprintf(name, ARRAY_SIZE(name), "ch%1d", i + 1);
> >> +		ddata->dmas[i] = dma_request_slave_channel(dev, name);
> > 
> > And if any of them fail?
> 
> Hi Lee,
> 
> If some of these fails, reference will be NULL. It is checked in child
> driver (pwm for instance) at runtime. Support is being added as an
> option: pwm capture will simply be unavailable in this case (fail with
> error).

Can you place a simple one-line comment in here please.  Or else
someone will come along and add error handling for you.

> >> +	}
> >> +	ddata->dmas[STM32_TIMERS_DMA_UP] = dma_request_slave_channel(dev, "up");
> >> +	ddata->dmas[STM32_TIMERS_DMA_TRIG] =
> >> +		dma_request_slave_channel(dev, "trig");
> >> +	ddata->dmas[STM32_TIMERS_DMA_COM] =
> >> +		dma_request_slave_channel(dev, "com");
> > 
> > Can you format these in the same why.  This hurts my eyes.
> 
> I use enum values and try to match as possible with reference manual for
> "up", "trig" & "com" names.
> Would have some suggestion to beautify this?

I just mean, can you push the first  dma_request_slave_channel() call
on to the next line, so each of the calls are formatted in the same
manner please?

> >> +}
> >> +
> >>  static int stm32_timers_probe(struct platform_device *pdev)
> >>  {
> >>  	struct device *dev = &pdev->dev;
> >> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
> >>  	mmio = devm_ioremap_resource(dev, res);
> >>  	if (IS_ERR(mmio))
> >>  		return PTR_ERR(mmio);
> >> +	ddata->phys_base = res->start;
> > 
> > What do you use this for?
> 
> This is used in in child driver (pwm) for capture data transfer by dma.

Might be worth being clear about that.

Perhaps pass in 'dma_base' (phys_base + offset) instead?

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

[PATCH 4/8] mfd: stm32-timers: add support for dmas

[Lee Jones]

On Tue, 23 Jan 2018, Fabrice Gasnier wrote:

> On 01/23/2018 02:32 PM, Lee Jones wrote:
> > On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
> >> transfer data from/to device by using dma.
> >>
> >> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> >> ---
> >>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
> >>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
> >>  2 files changed, 50 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
> >> index a6675a4..372b51e 100644
> >> --- a/drivers/mfd/stm32-timers.c
> >> +++ b/drivers/mfd/stm32-timers.c
> >> @@ -29,6 +29,23 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
> >>  	regmap_write(ddata->regmap, TIM_ARR, 0x0);
> >>  }
> >>  
> >> +static void stm32_timers_dma_probe(struct device *dev,
> >> +				   struct stm32_timers *ddata)
> >> +{
> >> +	int i;
> >> +	char name[4];
> >> +
> >> +	for (i = STM32_TIMERS_DMA_CH1; i <= STM32_TIMERS_DMA_CH4; i++) {
> >> +		snprintf(name, ARRAY_SIZE(name), "ch%1d", i + 1);
> >> +		ddata->dmas[i] = dma_request_slave_channel(dev, name);
> > 
> > And if any of them fail?
> 
> Hi Lee,
> 
> If some of these fails, reference will be NULL. It is checked in child
> driver (pwm for instance) at runtime. Support is being added as an
> option: pwm capture will simply be unavailable in this case (fail with
> error).

Can you place a simple one-line comment in here please.  Or else
someone will come along and add error handling for you.

> >> +	}
> >> +	ddata->dmas[STM32_TIMERS_DMA_UP] = dma_request_slave_channel(dev, "up");
> >> +	ddata->dmas[STM32_TIMERS_DMA_TRIG] =
> >> +		dma_request_slave_channel(dev, "trig");
> >> +	ddata->dmas[STM32_TIMERS_DMA_COM] =
> >> +		dma_request_slave_channel(dev, "com");
> > 
> > Can you format these in the same why.  This hurts my eyes.
> 
> I use enum values and try to match as possible with reference manual for
> "up", "trig" & "com" names.
> Would have some suggestion to beautify this?

I just mean, can you push the first  dma_request_slave_channel() call
on to the next line, so each of the calls are formatted in the same
manner please?

> >> +}
> >> +
> >>  static int stm32_timers_probe(struct platform_device *pdev)
> >>  {
> >>  	struct device *dev = &pdev->dev;
> >> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
> >>  	mmio = devm_ioremap_resource(dev, res);
> >>  	if (IS_ERR(mmio))
> >>  		return PTR_ERR(mmio);
> >> +	ddata->phys_base = res->start;
> > 
> > What do you use this for?
> 
> This is used in in child driver (pwm) for capture data transfer by dma.

Might be worth being clear about that.

Perhaps pass in 'dma_base' (phys_base + offset) instead?

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

Re: [PATCH 4/8] mfd: stm32-timers: add support for dmas

[Lee Jones]

On Tue, 23 Jan 2018, Fabrice Gasnier wrote:

> On 01/23/2018 02:32 PM, Lee Jones wrote:
> > On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
> >> transfer data from/to device by using dma.
> >>
> >> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
> >> ---
> >>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
> >>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
> >>  2 files changed, 50 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
> >> index a6675a4..372b51e 100644
> >> --- a/drivers/mfd/stm32-timers.c
> >> +++ b/drivers/mfd/stm32-timers.c
> >> @@ -29,6 +29,23 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
> >>  	regmap_write(ddata->regmap, TIM_ARR, 0x0);
> >>  }
> >>  
> >> +static void stm32_timers_dma_probe(struct device *dev,
> >> +				   struct stm32_timers *ddata)
> >> +{
> >> +	int i;
> >> +	char name[4];
> >> +
> >> +	for (i = STM32_TIMERS_DMA_CH1; i <= STM32_TIMERS_DMA_CH4; i++) {
> >> +		snprintf(name, ARRAY_SIZE(name), "ch%1d", i + 1);
> >> +		ddata->dmas[i] = dma_request_slave_channel(dev, name);
> > 
> > And if any of them fail?
> 
> Hi Lee,
> 
> If some of these fails, reference will be NULL. It is checked in child
> driver (pwm for instance) at runtime. Support is being added as an
> option: pwm capture will simply be unavailable in this case (fail with
> error).

Can you place a simple one-line comment in here please.  Or else
someone will come along and add error handling for you.

> >> +	}
> >> +	ddata->dmas[STM32_TIMERS_DMA_UP] = dma_request_slave_channel(dev, "up");
> >> +	ddata->dmas[STM32_TIMERS_DMA_TRIG] =
> >> +		dma_request_slave_channel(dev, "trig");
> >> +	ddata->dmas[STM32_TIMERS_DMA_COM] =
> >> +		dma_request_slave_channel(dev, "com");
> > 
> > Can you format these in the same why.  This hurts my eyes.
> 
> I use enum values and try to match as possible with reference manual for
> "up", "trig" & "com" names.
> Would have some suggestion to beautify this?

I just mean, can you push the first  dma_request_slave_channel() call
on to the next line, so each of the calls are formatted in the same
manner please?

> >> +}
> >> +
> >>  static int stm32_timers_probe(struct platform_device *pdev)
> >>  {
> >>  	struct device *dev = &pdev->dev;
> >> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
> >>  	mmio = devm_ioremap_resource(dev, res);
> >>  	if (IS_ERR(mmio))
> >>  		return PTR_ERR(mmio);
> >> +	ddata->phys_base = res->start;
> > 
> > What do you use this for?
> 
> This is used in in child driver (pwm) for capture data transfer by dma.

Might be worth being clear about that.

Perhaps pass in 'dma_base' (phys_base + offset) instead?

-- 
Lee Jones
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

On 01/23/2018 04:30 PM, Lee Jones wrote:
> On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
> 
>> On 01/23/2018 02:32 PM, Lee Jones wrote:
>>> On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
>>>> transfer data from/to device by using dma.
>>>>
>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>>> ---
>>>>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
>>>>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
>>>>  2 files changed, 50 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
>>>> index a6675a4..372b51e 100644
>>>> --- a/drivers/mfd/stm32-timers.c
>>>> +++ b/drivers/mfd/stm32-timers.c
>>>> @@ -29,6 +29,23 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
>>>>  	regmap_write(ddata->regmap, TIM_ARR, 0x0);
>>>>  }
>>>>  
>>>> +static void stm32_timers_dma_probe(struct device *dev,
>>>> +				   struct stm32_timers *ddata)
>>>> +{
>>>> +	int i;
>>>> +	char name[4];
>>>> +
>>>> +	for (i = STM32_TIMERS_DMA_CH1; i <= STM32_TIMERS_DMA_CH4; i++) {
>>>> +		snprintf(name, ARRAY_SIZE(name), "ch%1d", i + 1);
>>>> +		ddata->dmas[i] = dma_request_slave_channel(dev, name);
>>>
>>> And if any of them fail?
>>
>> Hi Lee,
>>
>> If some of these fails, reference will be NULL. It is checked in child
>> driver (pwm for instance) at runtime. Support is being added as an
>> option: pwm capture will simply be unavailable in this case (fail with
>> error).
> 
> Can you place a simple one-line comment in here please.  Or else
> someone will come along and add error handling for you.

Hi Lee,

Ok, I'll add a comment in v2.

> 
>>>> +	}
>>>> +	ddata->dmas[STM32_TIMERS_DMA_UP] = dma_request_slave_channel(dev, "up");
>>>> +	ddata->dmas[STM32_TIMERS_DMA_TRIG] =
>>>> +		dma_request_slave_channel(dev, "trig");
>>>> +	ddata->dmas[STM32_TIMERS_DMA_COM] =
>>>> +		dma_request_slave_channel(dev, "com");
>>>
>>> Can you format these in the same why.  This hurts my eyes.
>>
>> I use enum values and try to match as possible with reference manual for
>> "up", "trig" & "com" names.
>> Would have some suggestion to beautify this?
> 
> I just mean, can you push the first  dma_request_slave_channel() call
> on to the next line, so each of the calls are formatted in the same
> manner please?

Ok, got it now. I'll do this in v2.

> 
>>>> +}
>>>> +
>>>>  static int stm32_timers_probe(struct platform_device *pdev)
>>>>  {
>>>>  	struct device *dev = &pdev->dev;
>>>> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
>>>>  	mmio = devm_ioremap_resource(dev, res);
>>>>  	if (IS_ERR(mmio))
>>>>  		return PTR_ERR(mmio);
>>>> +	ddata->phys_base = res->start;
>>>
>>> What do you use this for?
>>
>> This is used in in child driver (pwm) for capture data transfer by dma.
> 
> Might be worth being clear about that.
> 
> Perhaps pass in 'dma_base' (phys_base + offset) instead?
> 

I guess you've had a look at [PATCH 5/8] pwm: stm32: add capture
support. Are you talking about passing phys_base + TIM_DMAR ?

If this is the case, I'd prefer to keep phys base only if you don't
mind, and handle TIM_DMAR offset in pwm driver. This way, all dma slave
config is kept locally at one place.
Or do you mean something else ?

Maybe I can add a comment here about this ?
Something like:
/* phys_base to be used by child driver, e.g. DMA burst mode */

Please advise,
Thanks,
Fabrice

[PATCH 4/8] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

On 01/23/2018 04:30 PM, Lee Jones wrote:
> On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
> 
>> On 01/23/2018 02:32 PM, Lee Jones wrote:
>>> On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
>>>> transfer data from/to device by using dma.
>>>>
>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>>> ---
>>>>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
>>>>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
>>>>  2 files changed, 50 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
>>>> index a6675a4..372b51e 100644
>>>> --- a/drivers/mfd/stm32-timers.c
>>>> +++ b/drivers/mfd/stm32-timers.c
>>>> @@ -29,6 +29,23 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
>>>>  	regmap_write(ddata->regmap, TIM_ARR, 0x0);
>>>>  }
>>>>  
>>>> +static void stm32_timers_dma_probe(struct device *dev,
>>>> +				   struct stm32_timers *ddata)
>>>> +{
>>>> +	int i;
>>>> +	char name[4];
>>>> +
>>>> +	for (i = STM32_TIMERS_DMA_CH1; i <= STM32_TIMERS_DMA_CH4; i++) {
>>>> +		snprintf(name, ARRAY_SIZE(name), "ch%1d", i + 1);
>>>> +		ddata->dmas[i] = dma_request_slave_channel(dev, name);
>>>
>>> And if any of them fail?
>>
>> Hi Lee,
>>
>> If some of these fails, reference will be NULL. It is checked in child
>> driver (pwm for instance) at runtime. Support is being added as an
>> option: pwm capture will simply be unavailable in this case (fail with
>> error).
> 
> Can you place a simple one-line comment in here please.  Or else
> someone will come along and add error handling for you.

Hi Lee,

Ok, I'll add a comment in v2.

> 
>>>> +	}
>>>> +	ddata->dmas[STM32_TIMERS_DMA_UP] = dma_request_slave_channel(dev, "up");
>>>> +	ddata->dmas[STM32_TIMERS_DMA_TRIG] =
>>>> +		dma_request_slave_channel(dev, "trig");
>>>> +	ddata->dmas[STM32_TIMERS_DMA_COM] =
>>>> +		dma_request_slave_channel(dev, "com");
>>>
>>> Can you format these in the same why.  This hurts my eyes.
>>
>> I use enum values and try to match as possible with reference manual for
>> "up", "trig" & "com" names.
>> Would have some suggestion to beautify this?
> 
> I just mean, can you push the first  dma_request_slave_channel() call
> on to the next line, so each of the calls are formatted in the same
> manner please?

Ok, got it now. I'll do this in v2.

> 
>>>> +}
>>>> +
>>>>  static int stm32_timers_probe(struct platform_device *pdev)
>>>>  {
>>>>  	struct device *dev = &pdev->dev;
>>>> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
>>>>  	mmio = devm_ioremap_resource(dev, res);
>>>>  	if (IS_ERR(mmio))
>>>>  		return PTR_ERR(mmio);
>>>> +	ddata->phys_base = res->start;
>>>
>>> What do you use this for?
>>
>> This is used in in child driver (pwm) for capture data transfer by dma.
> 
> Might be worth being clear about that.
> 
> Perhaps pass in 'dma_base' (phys_base + offset) instead?
> 

I guess you've had a look at [PATCH 5/8] pwm: stm32: add capture
support. Are you talking about passing phys_base + TIM_DMAR ?

If this is the case, I'd prefer to keep phys base only if you don't
mind, and handle TIM_DMAR offset in pwm driver. This way, all dma slave
config is kept locally at one place.
Or do you mean something else ?

Maybe I can add a comment here about this ?
Something like:
/* phys_base to be used by child driver, e.g. DMA burst mode */

Please advise,
Thanks,
Fabrice

Re: [PATCH 4/8] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

On 01/23/2018 04:30 PM, Lee Jones wrote:
> On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
> 
>> On 01/23/2018 02:32 PM, Lee Jones wrote:
>>> On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
>>>> transfer data from/to device by using dma.
>>>>
>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>>> ---
>>>>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
>>>>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
>>>>  2 files changed, 50 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
>>>> index a6675a4..372b51e 100644
>>>> --- a/drivers/mfd/stm32-timers.c
>>>> +++ b/drivers/mfd/stm32-timers.c
>>>> @@ -29,6 +29,23 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
>>>>  	regmap_write(ddata->regmap, TIM_ARR, 0x0);
>>>>  }
>>>>  
>>>> +static void stm32_timers_dma_probe(struct device *dev,
>>>> +				   struct stm32_timers *ddata)
>>>> +{
>>>> +	int i;
>>>> +	char name[4];
>>>> +
>>>> +	for (i = STM32_TIMERS_DMA_CH1; i <= STM32_TIMERS_DMA_CH4; i++) {
>>>> +		snprintf(name, ARRAY_SIZE(name), "ch%1d", i + 1);
>>>> +		ddata->dmas[i] = dma_request_slave_channel(dev, name);
>>>
>>> And if any of them fail?
>>
>> Hi Lee,
>>
>> If some of these fails, reference will be NULL. It is checked in child
>> driver (pwm for instance) at runtime. Support is being added as an
>> option: pwm capture will simply be unavailable in this case (fail with
>> error).
> 
> Can you place a simple one-line comment in here please.  Or else
> someone will come along and add error handling for you.

Hi Lee,

Ok, I'll add a comment in v2.

> 
>>>> +	}
>>>> +	ddata->dmas[STM32_TIMERS_DMA_UP] = dma_request_slave_channel(dev, "up");
>>>> +	ddata->dmas[STM32_TIMERS_DMA_TRIG] =
>>>> +		dma_request_slave_channel(dev, "trig");
>>>> +	ddata->dmas[STM32_TIMERS_DMA_COM] =
>>>> +		dma_request_slave_channel(dev, "com");
>>>
>>> Can you format these in the same why.  This hurts my eyes.
>>
>> I use enum values and try to match as possible with reference manual for
>> "up", "trig" & "com" names.
>> Would have some suggestion to beautify this?
> 
> I just mean, can you push the first  dma_request_slave_channel() call
> on to the next line, so each of the calls are formatted in the same
> manner please?

Ok, got it now. I'll do this in v2.

> 
>>>> +}
>>>> +
>>>>  static int stm32_timers_probe(struct platform_device *pdev)
>>>>  {
>>>>  	struct device *dev = &pdev->dev;
>>>> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
>>>>  	mmio = devm_ioremap_resource(dev, res);
>>>>  	if (IS_ERR(mmio))
>>>>  		return PTR_ERR(mmio);
>>>> +	ddata->phys_base = res->start;
>>>
>>> What do you use this for?
>>
>> This is used in in child driver (pwm) for capture data transfer by dma.
> 
> Might be worth being clear about that.
> 
> Perhaps pass in 'dma_base' (phys_base + offset) instead?
> 

I guess you've had a look at [PATCH 5/8] pwm: stm32: add capture
support. Are you talking about passing phys_base + TIM_DMAR ?

If this is the case, I'd prefer to keep phys base only if you don't
mind, and handle TIM_DMAR offset in pwm driver. This way, all dma slave
config is kept locally at one place.
Or do you mean something else ?

Maybe I can add a comment here about this ?
Something like:
/* phys_base to be used by child driver, e.g. DMA burst mode */

Please advise,
Thanks,
Fabrice

Re: [PATCH 4/8] mfd: stm32-timers: add support for dmas

[Lee Jones]

On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
> On 01/23/2018 04:30 PM, Lee Jones wrote:
> > On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
> > 
> >> On 01/23/2018 02:32 PM, Lee Jones wrote:
> >>> On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
> >>>> transfer data from/to device by using dma.
> >>>>
> >>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> >>>> ---
> >>>>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
> >>>>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
> >>>>  2 files changed, 50 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
> >>>>  static int stm32_timers_probe(struct platform_device *pdev)
> >>>>  {
> >>>>  	struct device *dev = &pdev->dev;
> >>>> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
> >>>>  	mmio = devm_ioremap_resource(dev, res);
> >>>>  	if (IS_ERR(mmio))
> >>>>  		return PTR_ERR(mmio);
> >>>> +	ddata->phys_base = res->start;
> >>>
> >>> What do you use this for?
> >>
> >> This is used in in child driver (pwm) for capture data transfer by dma.
> > 
> > Might be worth being clear about that.
> > 
> > Perhaps pass in 'dma_base' (phys_base + offset) instead?
> 
> I guess you've had a look at [PATCH 5/8] pwm: stm32: add capture
> support. Are you talking about passing phys_base + TIM_DMAR ?

I have and I am.

> If this is the case, I'd prefer to keep phys base only if you don't
> mind, and handle TIM_DMAR offset in pwm driver. This way, all dma slave
> config is kept locally at one place.
> Or do you mean something else ?
> 
> Maybe I can add a comment here about this ?
> Something like:
> /* phys_base to be used by child driver, e.g. DMA burst mode */

I haven't seen the memory map for this device, so it's not easy for me
to comment, but passing in the physical address of the parent MFD into
a child device doesn't quite sit right with me.

At what level does TIM_DMAR sit?  Is it a child (PWM) specific
property, or is it described at parent (Timer) level?

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

[PATCH 4/8] mfd: stm32-timers: add support for dmas

[Lee Jones]

On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
> On 01/23/2018 04:30 PM, Lee Jones wrote:
> > On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
> > 
> >> On 01/23/2018 02:32 PM, Lee Jones wrote:
> >>> On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
> >>>> transfer data from/to device by using dma.
> >>>>
> >>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> >>>> ---
> >>>>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
> >>>>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
> >>>>  2 files changed, 50 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
> >>>>  static int stm32_timers_probe(struct platform_device *pdev)
> >>>>  {
> >>>>  	struct device *dev = &pdev->dev;
> >>>> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
> >>>>  	mmio = devm_ioremap_resource(dev, res);
> >>>>  	if (IS_ERR(mmio))
> >>>>  		return PTR_ERR(mmio);
> >>>> +	ddata->phys_base = res->start;
> >>>
> >>> What do you use this for?
> >>
> >> This is used in in child driver (pwm) for capture data transfer by dma.
> > 
> > Might be worth being clear about that.
> > 
> > Perhaps pass in 'dma_base' (phys_base + offset) instead?
> 
> I guess you've had a look at [PATCH 5/8] pwm: stm32: add capture
> support. Are you talking about passing phys_base + TIM_DMAR ?

I have and I am.

> If this is the case, I'd prefer to keep phys base only if you don't
> mind, and handle TIM_DMAR offset in pwm driver. This way, all dma slave
> config is kept locally at one place.
> Or do you mean something else ?
> 
> Maybe I can add a comment here about this ?
> Something like:
> /* phys_base to be used by child driver, e.g. DMA burst mode */

I haven't seen the memory map for this device, so it's not easy for me
to comment, but passing in the physical address of the parent MFD into
a child device doesn't quite sit right with me.

At what level does TIM_DMAR sit?  Is it a child (PWM) specific
property, or is it described at parent (Timer) level?

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

Re: [PATCH 4/8] mfd: stm32-timers: add support for dmas

[Lee Jones]

On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
> On 01/23/2018 04:30 PM, Lee Jones wrote:
> > On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
> > 
> >> On 01/23/2018 02:32 PM, Lee Jones wrote:
> >>> On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
> >>>> transfer data from/to device by using dma.
> >>>>
> >>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
> >>>> ---
> >>>>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
> >>>>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
> >>>>  2 files changed, 50 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
> >>>>  static int stm32_timers_probe(struct platform_device *pdev)
> >>>>  {
> >>>>  	struct device *dev = &pdev->dev;
> >>>> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
> >>>>  	mmio = devm_ioremap_resource(dev, res);
> >>>>  	if (IS_ERR(mmio))
> >>>>  		return PTR_ERR(mmio);
> >>>> +	ddata->phys_base = res->start;
> >>>
> >>> What do you use this for?
> >>
> >> This is used in in child driver (pwm) for capture data transfer by dma.
> > 
> > Might be worth being clear about that.
> > 
> > Perhaps pass in 'dma_base' (phys_base + offset) instead?
> 
> I guess you've had a look at [PATCH 5/8] pwm: stm32: add capture
> support. Are you talking about passing phys_base + TIM_DMAR ?

I have and I am.

> If this is the case, I'd prefer to keep phys base only if you don't
> mind, and handle TIM_DMAR offset in pwm driver. This way, all dma slave
> config is kept locally at one place.
> Or do you mean something else ?
> 
> Maybe I can add a comment here about this ?
> Something like:
> /* phys_base to be used by child driver, e.g. DMA burst mode */

I haven't seen the memory map for this device, so it's not easy for me
to comment, but passing in the physical address of the parent MFD into
a child device doesn't quite sit right with me.

At what level does TIM_DMAR sit?  Is it a child (PWM) specific
property, or is it described at parent (Timer) level?

-- 
Lee Jones
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

On 01/23/2018 05:41 PM, Lee Jones wrote:
> On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
>> On 01/23/2018 04:30 PM, Lee Jones wrote:
>>> On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
>>>
>>>> On 01/23/2018 02:32 PM, Lee Jones wrote:
>>>>> On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
>>>>>> transfer data from/to device by using dma.
>>>>>>
>>>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>>>>> ---
>>>>>>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
>>>>>>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
>>>>>>  2 files changed, 50 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
>>>>>>  static int stm32_timers_probe(struct platform_device *pdev)
>>>>>>  {
>>>>>>  	struct device *dev = &pdev->dev;
>>>>>> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
>>>>>>  	mmio = devm_ioremap_resource(dev, res);
>>>>>>  	if (IS_ERR(mmio))
>>>>>>  		return PTR_ERR(mmio);
>>>>>> +	ddata->phys_base = res->start;
>>>>>
>>>>> What do you use this for?
>>>>
>>>> This is used in in child driver (pwm) for capture data transfer by dma.
>>>
>>> Might be worth being clear about that.
>>>
>>> Perhaps pass in 'dma_base' (phys_base + offset) instead?
>>
>> I guess you've had a look at [PATCH 5/8] pwm: stm32: add capture
>> support. Are you talking about passing phys_base + TIM_DMAR ?
> 
> I have and I am.
> 
>> If this is the case, I'd prefer to keep phys base only if you don't
>> mind, and handle TIM_DMAR offset in pwm driver. This way, all dma slave
>> config is kept locally at one place.
>> Or do you mean something else ?
>>
>> Maybe I can add a comment here about this ?
>> Something like:
>> /* phys_base to be used by child driver, e.g. DMA burst mode */
> 
> I haven't seen the memory map for this device, so it's not easy for me
> to comment, but passing in the physical address of the parent MFD into
> a child device doesn't quite sit right with me.
> 
> At what level does TIM_DMAR sit?  Is it a child (PWM) specific
> property, or is it described at parent (Timer) level?
> 
Hi Lee,

This isn't child (PWM) specific. TIM_DMAR is described at timer level as
well as all timers DMA requests lines. Current patchset make it useful
for PWM capture. Basically, I think this can be seen as interrupts, as
each (0..7) dma request has an enable bit (in DIER: interrupt enable
register). This is similar as interrupts at timer level.

So, I understand your point regarding passing physical address of the
parent MFD... Speaking of interrupts, I'd probably have looked at
irq_chip. Regarding dma, i'm not sure what is preferred way ?

Another way maybe to export a routine (export symbol) from MFD core, to
handle dma transfer from there?
By looking into drivers/mfd, I found similar approach, e.g.
rtsx_pci_dma_transfer(). Do you think this is better approach ?

Please let me know your opinion.

Best Regards,
Fabrice

[PATCH 4/8] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

On 01/23/2018 05:41 PM, Lee Jones wrote:
> On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
>> On 01/23/2018 04:30 PM, Lee Jones wrote:
>>> On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
>>>
>>>> On 01/23/2018 02:32 PM, Lee Jones wrote:
>>>>> On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
>>>>>> transfer data from/to device by using dma.
>>>>>>
>>>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>>>>> ---
>>>>>>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
>>>>>>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
>>>>>>  2 files changed, 50 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
>>>>>>  static int stm32_timers_probe(struct platform_device *pdev)
>>>>>>  {
>>>>>>  	struct device *dev = &pdev->dev;
>>>>>> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
>>>>>>  	mmio = devm_ioremap_resource(dev, res);
>>>>>>  	if (IS_ERR(mmio))
>>>>>>  		return PTR_ERR(mmio);
>>>>>> +	ddata->phys_base = res->start;
>>>>>
>>>>> What do you use this for?
>>>>
>>>> This is used in in child driver (pwm) for capture data transfer by dma.
>>>
>>> Might be worth being clear about that.
>>>
>>> Perhaps pass in 'dma_base' (phys_base + offset) instead?
>>
>> I guess you've had a look at [PATCH 5/8] pwm: stm32: add capture
>> support. Are you talking about passing phys_base + TIM_DMAR ?
> 
> I have and I am.
> 
>> If this is the case, I'd prefer to keep phys base only if you don't
>> mind, and handle TIM_DMAR offset in pwm driver. This way, all dma slave
>> config is kept locally at one place.
>> Or do you mean something else ?
>>
>> Maybe I can add a comment here about this ?
>> Something like:
>> /* phys_base to be used by child driver, e.g. DMA burst mode */
> 
> I haven't seen the memory map for this device, so it's not easy for me
> to comment, but passing in the physical address of the parent MFD into
> a child device doesn't quite sit right with me.
> 
> At what level does TIM_DMAR sit?  Is it a child (PWM) specific
> property, or is it described at parent (Timer) level?
> 
Hi Lee,

This isn't child (PWM) specific. TIM_DMAR is described at timer level as
well as all timers DMA requests lines. Current patchset make it useful
for PWM capture. Basically, I think this can be seen as interrupts, as
each (0..7) dma request has an enable bit (in DIER: interrupt enable
register). This is similar as interrupts at timer level.

So, I understand your point regarding passing physical address of the
parent MFD... Speaking of interrupts, I'd probably have looked at
irq_chip. Regarding dma, i'm not sure what is preferred way ?

Another way maybe to export a routine (export symbol) from MFD core, to
handle dma transfer from there?
By looking into drivers/mfd, I found similar approach, e.g.
rtsx_pci_dma_transfer(). Do you think this is better approach ?

Please let me know your opinion.

Best Regards,
Fabrice

Re: [PATCH 4/8] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

On 01/23/2018 05:41 PM, Lee Jones wrote:
> On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
>> On 01/23/2018 04:30 PM, Lee Jones wrote:
>>> On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
>>>
>>>> On 01/23/2018 02:32 PM, Lee Jones wrote:
>>>>> On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
>>>>>> transfer data from/to device by using dma.
>>>>>>
>>>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>>>>> ---
>>>>>>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
>>>>>>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
>>>>>>  2 files changed, 50 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
>>>>>>  static int stm32_timers_probe(struct platform_device *pdev)
>>>>>>  {
>>>>>>  	struct device *dev = &pdev->dev;
>>>>>> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
>>>>>>  	mmio = devm_ioremap_resource(dev, res);
>>>>>>  	if (IS_ERR(mmio))
>>>>>>  		return PTR_ERR(mmio);
>>>>>> +	ddata->phys_base = res->start;
>>>>>
>>>>> What do you use this for?
>>>>
>>>> This is used in in child driver (pwm) for capture data transfer by dma.
>>>
>>> Might be worth being clear about that.
>>>
>>> Perhaps pass in 'dma_base' (phys_base + offset) instead?
>>
>> I guess you've had a look at [PATCH 5/8] pwm: stm32: add capture
>> support. Are you talking about passing phys_base + TIM_DMAR ?
> 
> I have and I am.
> 
>> If this is the case, I'd prefer to keep phys base only if you don't
>> mind, and handle TIM_DMAR offset in pwm driver. This way, all dma slave
>> config is kept locally at one place.
>> Or do you mean something else ?
>>
>> Maybe I can add a comment here about this ?
>> Something like:
>> /* phys_base to be used by child driver, e.g. DMA burst mode */
> 
> I haven't seen the memory map for this device, so it's not easy for me
> to comment, but passing in the physical address of the parent MFD into
> a child device doesn't quite sit right with me.
> 
> At what level does TIM_DMAR sit?  Is it a child (PWM) specific
> property, or is it described at parent (Timer) level?
> 
Hi Lee,

This isn't child (PWM) specific. TIM_DMAR is described at timer level as
well as all timers DMA requests lines. Current patchset make it useful
for PWM capture. Basically, I think this can be seen as interrupts, as
each (0..7) dma request has an enable bit (in DIER: interrupt enable
register). This is similar as interrupts at timer level.

So, I understand your point regarding passing physical address of the
parent MFD... Speaking of interrupts, I'd probably have looked at
irq_chip. Regarding dma, i'm not sure what is preferred way ?

Another way maybe to export a routine (export symbol) from MFD core, to
handle dma transfer from there?
By looking into drivers/mfd, I found similar approach, e.g.
rtsx_pci_dma_transfer(). Do you think this is better approach ?

Please let me know your opinion.

Best Regards,
Fabrice

Re: [PATCH 4/8] mfd: stm32-timers: add support for dmas

[Lee Jones]

On Wed, 24 Jan 2018, Fabrice Gasnier wrote:
> On 01/23/2018 05:41 PM, Lee Jones wrote:
> > On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
> >> On 01/23/2018 04:30 PM, Lee Jones wrote:
> >>> On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
> >>>
> >>>> On 01/23/2018 02:32 PM, Lee Jones wrote:
> >>>>> On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
> >>>>>> transfer data from/to device by using dma.
> >>>>>>
> >>>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> >>>>>> ---
> >>>>>>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
> >>>>>>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
> >>>>>>  2 files changed, 50 insertions(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
> >>>>>>  static int stm32_timers_probe(struct platform_device *pdev)
> >>>>>>  {
> >>>>>>  	struct device *dev = &pdev->dev;
> >>>>>> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
> >>>>>>  	mmio = devm_ioremap_resource(dev, res);
> >>>>>>  	if (IS_ERR(mmio))
> >>>>>>  		return PTR_ERR(mmio);
> >>>>>> +	ddata->phys_base = res->start;
> >>>>>
> >>>>> What do you use this for?
> >>>>
> >>>> This is used in in child driver (pwm) for capture data transfer by dma.
> >>>
> >>> Might be worth being clear about that.
> >>>
> >>> Perhaps pass in 'dma_base' (phys_base + offset) instead?
> >>
> >> I guess you've had a look at [PATCH 5/8] pwm: stm32: add capture
> >> support. Are you talking about passing phys_base + TIM_DMAR ?
> > 
> > I have and I am.
> > 
> >> If this is the case, I'd prefer to keep phys base only if you don't
> >> mind, and handle TIM_DMAR offset in pwm driver. This way, all dma slave
> >> config is kept locally at one place.
> >> Or do you mean something else ?
> >>
> >> Maybe I can add a comment here about this ?
> >> Something like:
> >> /* phys_base to be used by child driver, e.g. DMA burst mode */
> > 
> > I haven't seen the memory map for this device, so it's not easy for me
> > to comment, but passing in the physical address of the parent MFD into
> > a child device doesn't quite sit right with me.
> > 
> > At what level does TIM_DMAR sit?  Is it a child (PWM) specific
> > property, or is it described at parent (Timer) level?
> > 
> Hi Lee,
> 
> This isn't child (PWM) specific. TIM_DMAR is described at timer level as
> well as all timers DMA requests lines. Current patchset make it useful
> for PWM capture. Basically, I think this can be seen as interrupts, as
> each (0..7) dma request has an enable bit (in DIER: interrupt enable
> register). This is similar as interrupts at timer level.
> 
> So, I understand your point regarding passing physical address of the
> parent MFD... Speaking of interrupts, I'd probably have looked at
> irq_chip. Regarding dma, i'm not sure what is preferred way ?
> 
> Another way maybe to export a routine (export symbol) from MFD core, to
> handle dma transfer from there?
> By looking into drivers/mfd, I found similar approach, e.g.
> rtsx_pci_dma_transfer(). Do you think this is better approach ?
> 
> Please let me know your opinion.

It sounds fine in principle.  You are in a better position to make
that decision as you know the H/W more intimately than I, however it
does sound like a good idea to abstract the DMA handling from the
device if these aren't device-{level|specific} operations.

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

[PATCH 4/8] mfd: stm32-timers: add support for dmas

[Lee Jones]

On Wed, 24 Jan 2018, Fabrice Gasnier wrote:
> On 01/23/2018 05:41 PM, Lee Jones wrote:
> > On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
> >> On 01/23/2018 04:30 PM, Lee Jones wrote:
> >>> On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
> >>>
> >>>> On 01/23/2018 02:32 PM, Lee Jones wrote:
> >>>>> On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
> >>>>>> transfer data from/to device by using dma.
> >>>>>>
> >>>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> >>>>>> ---
> >>>>>>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
> >>>>>>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
> >>>>>>  2 files changed, 50 insertions(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
> >>>>>>  static int stm32_timers_probe(struct platform_device *pdev)
> >>>>>>  {
> >>>>>>  	struct device *dev = &pdev->dev;
> >>>>>> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
> >>>>>>  	mmio = devm_ioremap_resource(dev, res);
> >>>>>>  	if (IS_ERR(mmio))
> >>>>>>  		return PTR_ERR(mmio);
> >>>>>> +	ddata->phys_base = res->start;
> >>>>>
> >>>>> What do you use this for?
> >>>>
> >>>> This is used in in child driver (pwm) for capture data transfer by dma.
> >>>
> >>> Might be worth being clear about that.
> >>>
> >>> Perhaps pass in 'dma_base' (phys_base + offset) instead?
> >>
> >> I guess you've had a look at [PATCH 5/8] pwm: stm32: add capture
> >> support. Are you talking about passing phys_base + TIM_DMAR ?
> > 
> > I have and I am.
> > 
> >> If this is the case, I'd prefer to keep phys base only if you don't
> >> mind, and handle TIM_DMAR offset in pwm driver. This way, all dma slave
> >> config is kept locally at one place.
> >> Or do you mean something else ?
> >>
> >> Maybe I can add a comment here about this ?
> >> Something like:
> >> /* phys_base to be used by child driver, e.g. DMA burst mode */
> > 
> > I haven't seen the memory map for this device, so it's not easy for me
> > to comment, but passing in the physical address of the parent MFD into
> > a child device doesn't quite sit right with me.
> > 
> > At what level does TIM_DMAR sit?  Is it a child (PWM) specific
> > property, or is it described at parent (Timer) level?
> > 
> Hi Lee,
> 
> This isn't child (PWM) specific. TIM_DMAR is described at timer level as
> well as all timers DMA requests lines. Current patchset make it useful
> for PWM capture. Basically, I think this can be seen as interrupts, as
> each (0..7) dma request has an enable bit (in DIER: interrupt enable
> register). This is similar as interrupts at timer level.
> 
> So, I understand your point regarding passing physical address of the
> parent MFD... Speaking of interrupts, I'd probably have looked at
> irq_chip. Regarding dma, i'm not sure what is preferred way ?
> 
> Another way maybe to export a routine (export symbol) from MFD core, to
> handle dma transfer from there?
> By looking into drivers/mfd, I found similar approach, e.g.
> rtsx_pci_dma_transfer(). Do you think this is better approach ?
> 
> Please let me know your opinion.

It sounds fine in principle.  You are in a better position to make
that decision as you know the H/W more intimately than I, however it
does sound like a good idea to abstract the DMA handling from the
device if these aren't device-{level|specific} operations.

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

Re: [PATCH 4/8] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

On 01/24/2018 03:56 PM, Lee Jones wrote:
> On Wed, 24 Jan 2018, Fabrice Gasnier wrote:
>> On 01/23/2018 05:41 PM, Lee Jones wrote:
>>> On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
>>>> On 01/23/2018 04:30 PM, Lee Jones wrote:
>>>>> On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
>>>>>
>>>>>> On 01/23/2018 02:32 PM, Lee Jones wrote:
>>>>>>> On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
>>>>>>>> transfer data from/to device by using dma.
>>>>>>>>
>>>>>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>>>>>>> ---
>>>>>>>>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
>>>>>>>>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
>>>>>>>>  2 files changed, 50 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
>>>>>>>>  static int stm32_timers_probe(struct platform_device *pdev)
>>>>>>>>  {
>>>>>>>>  	struct device *dev = &pdev->dev;
>>>>>>>> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
>>>>>>>>  	mmio = devm_ioremap_resource(dev, res);
>>>>>>>>  	if (IS_ERR(mmio))
>>>>>>>>  		return PTR_ERR(mmio);
>>>>>>>> +	ddata->phys_base = res->start;
>>>>>>>
>>>>>>> What do you use this for?
>>>>>>
>>>>>> This is used in in child driver (pwm) for capture data transfer by dma.
>>>>>
>>>>> Might be worth being clear about that.
>>>>>
>>>>> Perhaps pass in 'dma_base' (phys_base + offset) instead?
>>>>
>>>> I guess you've had a look at [PATCH 5/8] pwm: stm32: add capture
>>>> support. Are you talking about passing phys_base + TIM_DMAR ?
>>>
>>> I have and I am.
>>>
>>>> If this is the case, I'd prefer to keep phys base only if you don't
>>>> mind, and handle TIM_DMAR offset in pwm driver. This way, all dma slave
>>>> config is kept locally at one place.
>>>> Or do you mean something else ?
>>>>
>>>> Maybe I can add a comment here about this ?
>>>> Something like:
>>>> /* phys_base to be used by child driver, e.g. DMA burst mode */
>>>
>>> I haven't seen the memory map for this device, so it's not easy for me
>>> to comment, but passing in the physical address of the parent MFD into
>>> a child device doesn't quite sit right with me.
>>>
>>> At what level does TIM_DMAR sit?  Is it a child (PWM) specific
>>> property, or is it described at parent (Timer) level?
>>>
>> Hi Lee,
>>
>> This isn't child (PWM) specific. TIM_DMAR is described at timer level as
>> well as all timers DMA requests lines. Current patchset make it useful
>> for PWM capture. Basically, I think this can be seen as interrupts, as
>> each (0..7) dma request has an enable bit (in DIER: interrupt enable
>> register). This is similar as interrupts at timer level.
>>
>> So, I understand your point regarding passing physical address of the
>> parent MFD... Speaking of interrupts, I'd probably have looked at
>> irq_chip. Regarding dma, i'm not sure what is preferred way ?
>>
>> Another way maybe to export a routine (export symbol) from MFD core, to
>> handle dma transfer from there?
>> By looking into drivers/mfd, I found similar approach, e.g.
>> rtsx_pci_dma_transfer(). Do you think this is better approach ?
>>
>> Please let me know your opinion.
> 
> It sounds fine in principle.  You are in a better position to make
> that decision as you know the H/W more intimately than I, however it
> does sound like a good idea to abstract the DMA handling from the
> device if these aren't device-{level|specific} operations.
> 

Hi Lee,

Thanks, I'll rework this in v2 to follow your advice.

Best Regards,
Fabrice

[PATCH 4/8] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

On 01/24/2018 03:56 PM, Lee Jones wrote:
> On Wed, 24 Jan 2018, Fabrice Gasnier wrote:
>> On 01/23/2018 05:41 PM, Lee Jones wrote:
>>> On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
>>>> On 01/23/2018 04:30 PM, Lee Jones wrote:
>>>>> On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
>>>>>
>>>>>> On 01/23/2018 02:32 PM, Lee Jones wrote:
>>>>>>> On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
>>>>>>>> transfer data from/to device by using dma.
>>>>>>>>
>>>>>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>>>>>>> ---
>>>>>>>>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
>>>>>>>>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
>>>>>>>>  2 files changed, 50 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
>>>>>>>>  static int stm32_timers_probe(struct platform_device *pdev)
>>>>>>>>  {
>>>>>>>>  	struct device *dev = &pdev->dev;
>>>>>>>> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
>>>>>>>>  	mmio = devm_ioremap_resource(dev, res);
>>>>>>>>  	if (IS_ERR(mmio))
>>>>>>>>  		return PTR_ERR(mmio);
>>>>>>>> +	ddata->phys_base = res->start;
>>>>>>>
>>>>>>> What do you use this for?
>>>>>>
>>>>>> This is used in in child driver (pwm) for capture data transfer by dma.
>>>>>
>>>>> Might be worth being clear about that.
>>>>>
>>>>> Perhaps pass in 'dma_base' (phys_base + offset) instead?
>>>>
>>>> I guess you've had a look at [PATCH 5/8] pwm: stm32: add capture
>>>> support. Are you talking about passing phys_base + TIM_DMAR ?
>>>
>>> I have and I am.
>>>
>>>> If this is the case, I'd prefer to keep phys base only if you don't
>>>> mind, and handle TIM_DMAR offset in pwm driver. This way, all dma slave
>>>> config is kept locally at one place.
>>>> Or do you mean something else ?
>>>>
>>>> Maybe I can add a comment here about this ?
>>>> Something like:
>>>> /* phys_base to be used by child driver, e.g. DMA burst mode */
>>>
>>> I haven't seen the memory map for this device, so it's not easy for me
>>> to comment, but passing in the physical address of the parent MFD into
>>> a child device doesn't quite sit right with me.
>>>
>>> At what level does TIM_DMAR sit?  Is it a child (PWM) specific
>>> property, or is it described at parent (Timer) level?
>>>
>> Hi Lee,
>>
>> This isn't child (PWM) specific. TIM_DMAR is described at timer level as
>> well as all timers DMA requests lines. Current patchset make it useful
>> for PWM capture. Basically, I think this can be seen as interrupts, as
>> each (0..7) dma request has an enable bit (in DIER: interrupt enable
>> register). This is similar as interrupts at timer level.
>>
>> So, I understand your point regarding passing physical address of the
>> parent MFD... Speaking of interrupts, I'd probably have looked at
>> irq_chip. Regarding dma, i'm not sure what is preferred way ?
>>
>> Another way maybe to export a routine (export symbol) from MFD core, to
>> handle dma transfer from there?
>> By looking into drivers/mfd, I found similar approach, e.g.
>> rtsx_pci_dma_transfer(). Do you think this is better approach ?
>>
>> Please let me know your opinion.
> 
> It sounds fine in principle.  You are in a better position to make
> that decision as you know the H/W more intimately than I, however it
> does sound like a good idea to abstract the DMA handling from the
> device if these aren't device-{level|specific} operations.
> 

Hi Lee,

Thanks, I'll rework this in v2 to follow your advice.

Best Regards,
Fabrice

Re: [PATCH 4/8] mfd: stm32-timers: add support for dmas

[Fabrice Gasnier]

On 01/24/2018 03:56 PM, Lee Jones wrote:
> On Wed, 24 Jan 2018, Fabrice Gasnier wrote:
>> On 01/23/2018 05:41 PM, Lee Jones wrote:
>>> On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
>>>> On 01/23/2018 04:30 PM, Lee Jones wrote:
>>>>> On Tue, 23 Jan 2018, Fabrice Gasnier wrote:
>>>>>
>>>>>> On 01/23/2018 02:32 PM, Lee Jones wrote:
>>>>>>> On Tue, 16 Jan 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, keep reference of device's bus address to allow child drivers to
>>>>>>>> transfer data from/to device by using dma.
>>>>>>>>
>>>>>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>>>>>>> ---
>>>>>>>>  drivers/mfd/stm32-timers.c       | 37 ++++++++++++++++++++++++++++++++++++-
>>>>>>>>  include/linux/mfd/stm32-timers.h | 14 ++++++++++++++
>>>>>>>>  2 files changed, 50 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
>>>>>>>>  static int stm32_timers_probe(struct platform_device *pdev)
>>>>>>>>  {
>>>>>>>>  	struct device *dev = &pdev->dev;
>>>>>>>> @@ -44,6 +61,7 @@ static int stm32_timers_probe(struct platform_device *pdev)
>>>>>>>>  	mmio = devm_ioremap_resource(dev, res);
>>>>>>>>  	if (IS_ERR(mmio))
>>>>>>>>  		return PTR_ERR(mmio);
>>>>>>>> +	ddata->phys_base = res->start;
>>>>>>>
>>>>>>> What do you use this for?
>>>>>>
>>>>>> This is used in in child driver (pwm) for capture data transfer by dma.
>>>>>
>>>>> Might be worth being clear about that.
>>>>>
>>>>> Perhaps pass in 'dma_base' (phys_base + offset) instead?
>>>>
>>>> I guess you've had a look at [PATCH 5/8] pwm: stm32: add capture
>>>> support. Are you talking about passing phys_base + TIM_DMAR ?
>>>
>>> I have and I am.
>>>
>>>> If this is the case, I'd prefer to keep phys base only if you don't
>>>> mind, and handle TIM_DMAR offset in pwm driver. This way, all dma slave
>>>> config is kept locally at one place.
>>>> Or do you mean something else ?
>>>>
>>>> Maybe I can add a comment here about this ?
>>>> Something like:
>>>> /* phys_base to be used by child driver, e.g. DMA burst mode */
>>>
>>> I haven't seen the memory map for this device, so it's not easy for me
>>> to comment, but passing in the physical address of the parent MFD into
>>> a child device doesn't quite sit right with me.
>>>
>>> At what level does TIM_DMAR sit?  Is it a child (PWM) specific
>>> property, or is it described at parent (Timer) level?
>>>
>> Hi Lee,
>>
>> This isn't child (PWM) specific. TIM_DMAR is described at timer level as
>> well as all timers DMA requests lines. Current patchset make it useful
>> for PWM capture. Basically, I think this can be seen as interrupts, as
>> each (0..7) dma request has an enable bit (in DIER: interrupt enable
>> register). This is similar as interrupts at timer level.
>>
>> So, I understand your point regarding passing physical address of the
>> parent MFD... Speaking of interrupts, I'd probably have looked at
>> irq_chip. Regarding dma, i'm not sure what is preferred way ?
>>
>> Another way maybe to export a routine (export symbol) from MFD core, to
>> handle dma transfer from there?
>> By looking into drivers/mfd, I found similar approach, e.g.
>> rtsx_pci_dma_transfer(). Do you think this is better approach ?
>>
>> Please let me know your opinion.
> 
> It sounds fine in principle.  You are in a better position to make
> that decision as you know the H/W more intimately than I, however it
> does sound like a good idea to abstract the DMA handling from the
> device if these aren't device-{level|specific} operations.
> 

Hi Lee,

Thanks, I'll rework this in v2 to follow your advice.

Best Regards,
Fabrice

Re: [PATCH 4/8] mfd: stm32-timers: add support for dmas

[cas]

Apologies for interrupting any work/messages being undertaken. I am a newbie 
here (1st post) and felt this was the only way to get this information to 
Linus Torvalds.
____
The following was posted on a forum on the 22nd March 2017:

'I've been working at Intel for the last 15 years as an electrical engineer. 
3 years ago I [got] bored with my job and wanted to move to a different 
department. Intel is pretty good about letting employees move around within 
the company. An opening came up on the Management Engine team, and since I 
have experience with micro-controllers I figured I'd give it a shot. The 
interview went great and they wanted me on board, but the final step was to 
get securuty clearance.

I asked them WTF I need a security clearance for, and they said they 
couldn't tell me unitl I was on the team. Thus began a 3 year trip down the 
fucking rabbit hole.

I'm coming forward today because of the news that Trump was spied on. I know 
exactly how it was done because I spent the last three years adding 
backdoors into the ME. For those of you that are unaware, the ME is ona 
separate CPU and cannot be disabled, and it exists at a level below the 
operating system. Intel has been working with the intelligence community for 
years to get backdoors into physical hardware so they don't have to worry 
about finding OS exploits. If the hardware itself is compromised then the 
whole machine is compromised.

----Next post----

The ME has full access to memory without the parent CPU having any 
knowledge, it has full access to the TCP/IP stacl, and full access to every 
peripheral connected to the computer. It also runs when the computer is 
hibernating. It doesn't matter if you're using Windows, Linux, Mac OS X, 
Whonix, Tails, Qubes OS, or Subgraph. If you have an i3, i5, or i7 hen 
you're completely owned by the intelligence community.

We added similar finctionality to Samsung's WEEPING ANGEL, except the 
intelligence community calls our project ODIN'S EYE. Through the ME we can 
activate the mic and webcam even though the computer appears to be off or 
sleeping.

Evidence of surveillance on Trump, his family, and key people in his 
campaign will come out eventually. I know the surveillance happened for a 
fact. Future leaks are coming, watch out for ODIN'S EYE.'

---END POST---

Jim Stone over at

82.221.129.208/.zj3.html

has the image of the text of the above (you need to scroll down a bit). He 
also mentioned this stuff, and more, over 7 years ago, though not in the 
same exact terms as the above, and states that this has actually occured 
since the Centrino processor, which is where the first backdoors started to 
be placed.

I am here to pass on the information to all of you, if you do not already 
have it (and since I have just joined to explicitly to send this I have no 
idea if any of you know this or not), and primarily to get this information 
to Linus Torvalds who has been quoted by RT very recently as saying that the 
patches announced by intel (for the Spectre and Meltdown bug) are 'garbage'.

I hope that the above information can now be used to understand the real 
extent of the problem.

[Note: if one is not able to find the image of the text at the above site 
you can find it here:

liferepairspecialist.com/Memes/

look for the image called 'inteladmission' - it is in this folder and not in 
any subfolders.] 

[PATCH 4/8] mfd: stm32-timers: add support for dmas

[cas]

Apologies for interrupting any work/messages being undertaken. I am a newbie 
here (1st post) and felt this was the only way to get this information to 
Linus Torvalds.
____
The following was posted on a forum on the 22nd March 2017:

'I've been working at Intel for the last 15 years as an electrical engineer. 
3 years ago I [got] bored with my job and wanted to move to a different 
department. Intel is pretty good about letting employees move around within 
the company. An opening came up on the Management Engine team, and since I 
have experience with micro-controllers I figured I'd give it a shot. The 
interview went great and they wanted me on board, but the final step was to 
get securuty clearance.

I asked them WTF I need a security clearance for, and they said they 
couldn't tell me unitl I was on the team. Thus began a 3 year trip down the 
fucking rabbit hole.

I'm coming forward today because of the news that Trump was spied on. I know 
exactly how it was done because I spent the last three years adding 
backdoors into the ME. For those of you that are unaware, the ME is ona 
separate CPU and cannot be disabled, and it exists at a level below the 
operating system. Intel has been working with the intelligence community for 
years to get backdoors into physical hardware so they don't have to worry 
about finding OS exploits. If the hardware itself is compromised then the 
whole machine is compromised.

----Next post----

The ME has full access to memory without the parent CPU having any 
knowledge, it has full access to the TCP/IP stacl, and full access to every 
peripheral connected to the computer. It also runs when the computer is 
hibernating. It doesn't matter if you're using Windows, Linux, Mac OS X, 
Whonix, Tails, Qubes OS, or Subgraph. If you have an i3, i5, or i7 hen 
you're completely owned by the intelligence community.

We added similar finctionality to Samsung's WEEPING ANGEL, except the 
intelligence community calls our project ODIN'S EYE. Through the ME we can 
activate the mic and webcam even though the computer appears to be off or 
sleeping.

Evidence of surveillance on Trump, his family, and key people in his 
campaign will come out eventually. I know the surveillance happened for a 
fact. Future leaks are coming, watch out for ODIN'S EYE.'

---END POST---

Jim Stone over at

82.221.129.208/.zj3.html

has the image of the text of the above (you need to scroll down a bit). He 
also mentioned this stuff, and more, over 7 years ago, though not in the 
same exact terms as the above, and states that this has actually occured 
since the Centrino processor, which is where the first backdoors started to 
be placed.

I am here to pass on the information to all of you, if you do not already 
have it (and since I have just joined to explicitly to send this I have no 
idea if any of you know this or not), and primarily to get this information 
to Linus Torvalds who has been quoted by RT very recently as saying that the 
patches announced by intel (for the Spectre and Meltdown bug) are 'garbage'.

I hope that the above information can now be used to understand the real 
extent of the problem.

[Note: if one is not able to find the image of the text at the above site 
you can find it here:

liferepairspecialist.com/Memes/

look for the image called 'inteladmission' - it is in this folder and not in 
any subfolders.] 

Re: [PATCH 4/8] mfd: stm32-timers: add support for dmas

[cas]

Apologies for interrupting any work/messages being undertaken. I am a newbie 
here (1st post) and felt this was the only way to get this information to 
Linus Torvalds.
____
The following was posted on a forum on the 22nd March 2017:

'I've been working at Intel for the last 15 years as an electrical engineer. 
3 years ago I [got] bored with my job and wanted to move to a different 
department. Intel is pretty good about letting employees move around within 
the company. An opening came up on the Management Engine team, and since I 
have experience with micro-controllers I figured I'd give it a shot. The 
interview went great and they wanted me on board, but the final step was to 
get securuty clearance.

I asked them WTF I need a security clearance for, and they said they 
couldn't tell me unitl I was on the team. Thus began a 3 year trip down the 
fucking rabbit hole.

I'm coming forward today because of the news that Trump was spied on. I know 
exactly how it was done because I spent the last three years adding 
backdoors into the ME. For those of you that are unaware, the ME is ona 
separate CPU and cannot be disabled, and it exists at a level below the 
operating system. Intel has been working with the intelligence community for 
years to get backdoors into physical hardware so they don't have to worry 
about finding OS exploits. If the hardware itself is compromised then the 
whole machine is compromised.

----Next post----

The ME has full access to memory without the parent CPU having any 
knowledge, it has full access to the TCP/IP stacl, and full access to every 
peripheral connected to the computer. It also runs when the computer is 
hibernating. It doesn't matter if you're using Windows, Linux, Mac OS X, 
Whonix, Tails, Qubes OS, or Subgraph. If you have an i3, i5, or i7 hen 
you're completely owned by the intelligence community.

We added similar finctionality to Samsung's WEEPING ANGEL, except the 
intelligence community calls our project ODIN'S EYE. Through the ME we can 
activate the mic and webcam even though the computer appears to be off or 
sleeping.

Evidence of surveillance on Trump, his family, and key people in his 
campaign will come out eventually. I know the surveillance happened for a 
fact. Future leaks are coming, watch out for ODIN'S EYE.'

---END POST---

Jim Stone over at

82.221.129.208/.zj3.html

has the image of the text of the above (you need to scroll down a bit). He 
also mentioned this stuff, and more, over 7 years ago, though not in the 
same exact terms as the above, and states that this has actually occured 
since the Centrino processor, which is where the first backdoors started to 
be placed.

I am here to pass on the information to all of you, if you do not already 
have it (and since I have just joined to explicitly to send this I have no 
idea if any of you know this or not), and primarily to get this information 
to Linus Torvalds who has been quoted by RT very recently as saying that the 
patches announced by intel (for the Spectre and Meltdown bug) are 'garbage'.

I hope that the above information can now be used to understand the real 
extent of the problem.

[Note: if one is not able to find the image of the text at the above site 
you can find it here:

liferepairspecialist.com/Memes/

look for the image called 'inteladmission' - it is in this folder and not in 
any subfolders.] 

[PATCH 5/8] 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, one for duty-cycle.
When there's only one channel available, only period can be captured.
It's zero'ed in such a case.
This requires exclusive access (e.g. no pwm output running at the same
time, to protect common prescaler).
Timer DMA burst mode is being used, to take two snapshots of capture
registers (upon each period rising edge).
Falling edge captures duty cycle to capture registers (only).

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/pwm/pwm-stm32.c          | 301 ++++++++++++++++++++++++++++++++++++++-
 include/linux/mfd/stm32-timers.h |  16 +++
 2 files changed, 314 insertions(+), 3 deletions(-)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 3ac55df..b9c7e878 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/dma-mapping.h>
 #include <linux/mfd/stm32-timers.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -21,11 +22,17 @@
 
 struct stm32_pwm {
 	struct pwm_chip chip;
+	struct completion completion;
 	struct mutex lock; /* protect pwm config/enable */
 	struct clk *clk;
 	struct regmap *regmap;
+	phys_addr_t phys_base;
 	u32 max_arr;
 	bool have_complementary_output;
+	u32 capture[2];
+	struct dma_chan *dmas[STM32_TIMERS_MAX_DMAS];
+	dma_addr_t dma_buf_phys;	/* dma buffer bus address (phys) */
+	u32 *dma_buf;			/* dma buffer cpu address */
 };
 
 struct stm32_breakinput {
@@ -63,6 +70,263 @@ static int write_ccrx(struct stm32_pwm *dev, int ch, u32 value)
 	return -EINVAL;
 }
 
+/*
+ * 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 void stm32_pwm_dma_done(void *p)
+{
+	struct pwm_device *pwm = p;
+	struct stm32_pwm *priv = to_stm32_pwm_dev(pwm->chip);
+	/* Use cc1 / cc3 DMA resp for PWM input channels 1 & 2 / 3 & 4 */
+	struct dma_chan *dma_chan = priv->dmas[pwm->hwpwm < 2 ? 0 : 2];
+	struct dma_tx_state state;
+	enum dma_status status;
+
+	status = dmaengine_tx_status(dma_chan, dma_chan->cookie, &state);
+	if (status == DMA_COMPLETE) {
+		/* Period: t2 - t0 (take care of counter overflow) */
+		if (priv->dma_buf[0] <= priv->dma_buf[2])
+			priv->capture[0] = priv->dma_buf[2] - priv->dma_buf[0];
+		else
+			priv->capture[0] = priv->max_arr - priv->dma_buf[0] +
+				priv->dma_buf[2];
+
+		/* Duty cycle capture requires at least two capture units */
+		if (pwm->chip->npwm < 2)
+			priv->capture[1] = 0;
+		else if (priv->dma_buf[0] <= priv->dma_buf[3])
+			priv->capture[1] = priv->dma_buf[3] - priv->dma_buf[0];
+		else
+			priv->capture[1] = priv->max_arr - priv->dma_buf[0] +
+				priv->dma_buf[3];
+
+		if (priv->capture[1] > priv->capture[0]) {
+			/*
+			 * 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.
+			 */
+			priv->capture[1] -= priv->capture[0];
+		}
+		complete(&priv->completion);
+	}
+}
+
+#define TIM_DCR_DBL_2_TRANSFERS		BIT(8)
+
+static int stm32_pwm_dma_start(struct stm32_pwm *priv, struct pwm_device *pwm)
+{
+	struct dma_chan *dma_chan = priv->dmas[pwm->hwpwm < 2 ? 0 : 2];
+	struct dma_async_tx_descriptor *desc;
+	struct dma_slave_config config;
+	dma_cookie_t cookie;
+	int ret;
+
+	if (!dma_chan || !priv->dma_buf)
+		return -ENODEV;
+
+	memset(&config, 0, sizeof(config));
+	config.src_addr = (dma_addr_t)priv->phys_base + TIM_DMAR;
+	config.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+	ret = dmaengine_slave_config(dma_chan, &config);
+	if (ret)
+		return ret;
+
+	/* Prepare DMA transaction (4 32-bits words, twice bellow transfer) */
+	desc = dmaengine_prep_slave_single(dma_chan, priv->dma_buf_phys,
+					   4 * sizeof(u32), DMA_DEV_TO_MEM,
+					   DMA_PREP_INTERRUPT);
+	if (!desc)
+		return -EBUSY;
+
+	desc->callback = stm32_pwm_dma_done;
+	desc->callback_param = pwm;
+	cookie = dmaengine_submit(desc);
+	ret = dma_submit_error(cookie);
+	if (ret) {
+		dmaengine_terminate_all(dma_chan);
+		return ret;
+	}
+	dma_async_issue_pending(dma_chan);
+
+	/*
+	 * Timer DMA burst mode. Request 2 transfers to get both CCR1 & CCR2
+	 * (or CCR3 & CCR4) on each capture event from DMAR register:
+	 * - DBL (transfer len): 2
+	 * - DBA (start offset): CCR1 or CCR3 (offset / 4)
+	 * As 4 32-bits words has been requested above, we'll get two
+	 * snapchots in 'dma_buf': { CCR1, CCR2 }, { CCR1, CCR2 }
+	 * or { CCR3, CCR4 }, { CCR3, CCR4 }
+	 */
+	regmap_write(priv->regmap, TIM_DCR, TIM_DCR_DBL_2_TRANSFERS |
+		     (pwm->hwpwm < 2 ? (TIM_CCR1 / 4) : (TIM_CCR3 / 4)));
+	regmap_update_bits(priv->regmap, TIM_DIER, TIM_DIER_XDE,
+			   pwm->hwpwm < 2 ? TIM_DIER_CC1DE : TIM_DIER_CC3DE);
+
+	return 0;
+}
+
+static void stm32_pwm_dma_stop(struct stm32_pwm *priv, struct pwm_device *pwm)
+{
+	struct dma_chan *dma_chan = priv->dmas[pwm->hwpwm < 2 ? 0 : 2];
+
+	regmap_update_bits(priv->regmap, TIM_DIER, TIM_DIER_XDE, 0);
+	regmap_write(priv->regmap, TIM_DCR, 0);
+	dmaengine_terminate_all(dma_chan);
+}
+
+#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)
+
+static int stm32_pwm_do_capture(struct stm32_pwm *priv, struct pwm_device *pwm,
+				unsigned long tmo_ms)
+{
+	unsigned long timeout = msecs_to_jiffies(tmo_ms);
+	long err;
+	int ret;
+
+	reinit_completion(&priv->completion);
+
+	/* Ensure registers have been updated */
+	regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
+
+	/* Clear pending flags, start counter, enable DMA, then capture */
+	regmap_write(priv->regmap, TIM_SR, 0);
+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, TIM_CR1_CEN);
+	ret = stm32_pwm_dma_start(priv, pwm);
+	if (ret)
+		goto stop;
+	regmap_update_bits(priv->regmap, TIM_CCER, pwm->hwpwm < 2 ?
+			   TIM_CCER_CC12E : TIM_CCER_CC34E,
+			   TIM_CCER_CC12E | TIM_CCER_CC34E);
+
+	err = wait_for_completion_interruptible_timeout(&priv->completion,
+							timeout);
+
+	regmap_update_bits(priv->regmap, TIM_CCER, pwm->hwpwm < 2 ?
+			   TIM_CCER_CC12E : TIM_CCER_CC34E, 0);
+	stm32_pwm_dma_stop(priv, pwm);
+	if (err == 0)
+		ret = -ETIMEDOUT;
+	else if (err < 0)
+		ret = err;
+
+stop:
+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, 0);
+	regmap_write(priv->regmap, TIM_SR, 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_do_capture(priv, pwm, tmo_ms);
+	if (ret)
+		goto stop;
+
+	raw_prd = priv->capture[0];
+	raw_dty = priv->capture[1];
+
+	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 +495,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,
@@ -344,23 +611,40 @@ static int stm32_pwm_probe(struct platform_device *pdev)
 	struct device_node *np = dev->of_node;
 	struct stm32_timers *ddata = dev_get_drvdata(pdev->dev.parent);
 	struct stm32_pwm *priv;
-	int ret;
+	bool dma = false;
+	int i, ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
 	mutex_init(&priv->lock);
+	init_completion(&priv->completion);
 	priv->regmap = ddata->regmap;
+	priv->phys_base = ddata->phys_base;
 	priv->clk = ddata->clk;
 	priv->max_arr = ddata->max_arr;
 
 	if (!priv->regmap || !priv->clk)
 		return -EINVAL;
 
+	for (i = 0; i < STM32_TIMERS_MAX_DMAS; i++) {
+		priv->dmas[i] = ddata->dmas[i];
+		if (priv->dmas[i])
+			dma = true;
+	}
+
+	if (dma) {
+		priv->dma_buf = dma_alloc_coherent(dev->parent, PAGE_SIZE,
+						   &priv->dma_buf_phys,
+						   GFP_KERNEL);
+		if (!priv->dma_buf)
+			dev_dbg(dev, "can't allocate DMA buffer\n");
+	}
+
 	ret = stm32_pwm_apply_breakinputs(priv, np);
 	if (ret)
-		return ret;
+		goto dma_free;
 
 	stm32_pwm_detect_complementary(priv);
 
@@ -371,11 +655,18 @@ static int stm32_pwm_probe(struct platform_device *pdev)
 
 	ret = pwmchip_add(&priv->chip);
 	if (ret < 0)
-		return ret;
+		goto dma_free;
 
 	platform_set_drvdata(pdev, priv);
 
 	return 0;
+
+dma_free:
+	if (priv->dma_buf)
+		dma_free_coherent(priv->chip.dev, PAGE_SIZE,
+				  priv->dma_buf, priv->dma_buf_phys);
+
+	return ret;
 }
 
 static int stm32_pwm_remove(struct platform_device *pdev)
@@ -388,6 +679,10 @@ static int stm32_pwm_remove(struct platform_device *pdev)
 
 	pwmchip_remove(&priv->chip);
 
+	if (priv->dma_buf)
+		dma_free_coherent(priv->chip.dev, PAGE_SIZE,
+				  priv->dma_buf, priv->dma_buf_phys);
+
 	return 0;
 }
 
diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
index 2b4ffb9..219eccc 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -30,6 +30,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	   */
@@ -39,17 +41,31 @@
 #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_CC1DE	BIT(9)  /* CC1 DMA request Enable  */
+#define TIM_DIER_CC3DE	BIT(11) /* CC3 DMA request Enable  */
+#define TIM_DIER_XDE	GENMASK(14, 8)
 #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  */
 #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 5/8] 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, one for duty-cycle.
When there's only one channel available, only period can be captured.
It's zero'ed in such a case.
This requires exclusive access (e.g. no pwm output running at the same
time, to protect common prescaler).
Timer DMA burst mode is being used, to take two snapshots of capture
registers (upon each period rising edge).
Falling edge captures duty cycle to capture registers (only).

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/pwm/pwm-stm32.c          | 301 ++++++++++++++++++++++++++++++++++++++-
 include/linux/mfd/stm32-timers.h |  16 +++
 2 files changed, 314 insertions(+), 3 deletions(-)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 3ac55df..b9c7e878 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/dma-mapping.h>
 #include <linux/mfd/stm32-timers.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -21,11 +22,17 @@
 
 struct stm32_pwm {
 	struct pwm_chip chip;
+	struct completion completion;
 	struct mutex lock; /* protect pwm config/enable */
 	struct clk *clk;
 	struct regmap *regmap;
+	phys_addr_t phys_base;
 	u32 max_arr;
 	bool have_complementary_output;
+	u32 capture[2];
+	struct dma_chan *dmas[STM32_TIMERS_MAX_DMAS];
+	dma_addr_t dma_buf_phys;	/* dma buffer bus address (phys) */
+	u32 *dma_buf;			/* dma buffer cpu address */
 };
 
 struct stm32_breakinput {
@@ -63,6 +70,263 @@ static int write_ccrx(struct stm32_pwm *dev, int ch, u32 value)
 	return -EINVAL;
 }
 
+/*
+ * 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 void stm32_pwm_dma_done(void *p)
+{
+	struct pwm_device *pwm = p;
+	struct stm32_pwm *priv = to_stm32_pwm_dev(pwm->chip);
+	/* Use cc1 / cc3 DMA resp for PWM input channels 1 & 2 / 3 & 4 */
+	struct dma_chan *dma_chan = priv->dmas[pwm->hwpwm < 2 ? 0 : 2];
+	struct dma_tx_state state;
+	enum dma_status status;
+
+	status = dmaengine_tx_status(dma_chan, dma_chan->cookie, &state);
+	if (status == DMA_COMPLETE) {
+		/* Period: t2 - t0 (take care of counter overflow) */
+		if (priv->dma_buf[0] <= priv->dma_buf[2])
+			priv->capture[0] = priv->dma_buf[2] - priv->dma_buf[0];
+		else
+			priv->capture[0] = priv->max_arr - priv->dma_buf[0] +
+				priv->dma_buf[2];
+
+		/* Duty cycle capture requires at least two capture units */
+		if (pwm->chip->npwm < 2)
+			priv->capture[1] = 0;
+		else if (priv->dma_buf[0] <= priv->dma_buf[3])
+			priv->capture[1] = priv->dma_buf[3] - priv->dma_buf[0];
+		else
+			priv->capture[1] = priv->max_arr - priv->dma_buf[0] +
+				priv->dma_buf[3];
+
+		if (priv->capture[1] > priv->capture[0]) {
+			/*
+			 * 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.
+			 */
+			priv->capture[1] -= priv->capture[0];
+		}
+		complete(&priv->completion);
+	}
+}
+
+#define TIM_DCR_DBL_2_TRANSFERS		BIT(8)
+
+static int stm32_pwm_dma_start(struct stm32_pwm *priv, struct pwm_device *pwm)
+{
+	struct dma_chan *dma_chan = priv->dmas[pwm->hwpwm < 2 ? 0 : 2];
+	struct dma_async_tx_descriptor *desc;
+	struct dma_slave_config config;
+	dma_cookie_t cookie;
+	int ret;
+
+	if (!dma_chan || !priv->dma_buf)
+		return -ENODEV;
+
+	memset(&config, 0, sizeof(config));
+	config.src_addr = (dma_addr_t)priv->phys_base + TIM_DMAR;
+	config.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+	ret = dmaengine_slave_config(dma_chan, &config);
+	if (ret)
+		return ret;
+
+	/* Prepare DMA transaction (4 32-bits words, twice bellow transfer) */
+	desc = dmaengine_prep_slave_single(dma_chan, priv->dma_buf_phys,
+					   4 * sizeof(u32), DMA_DEV_TO_MEM,
+					   DMA_PREP_INTERRUPT);
+	if (!desc)
+		return -EBUSY;
+
+	desc->callback = stm32_pwm_dma_done;
+	desc->callback_param = pwm;
+	cookie = dmaengine_submit(desc);
+	ret = dma_submit_error(cookie);
+	if (ret) {
+		dmaengine_terminate_all(dma_chan);
+		return ret;
+	}
+	dma_async_issue_pending(dma_chan);
+
+	/*
+	 * Timer DMA burst mode. Request 2 transfers to get both CCR1 & CCR2
+	 * (or CCR3 & CCR4) on each capture event from DMAR register:
+	 * - DBL (transfer len): 2
+	 * - DBA (start offset): CCR1 or CCR3 (offset / 4)
+	 * As 4 32-bits words has been requested above, we'll get two
+	 * snapchots in 'dma_buf': { CCR1, CCR2 }, { CCR1, CCR2 }
+	 * or { CCR3, CCR4 }, { CCR3, CCR4 }
+	 */
+	regmap_write(priv->regmap, TIM_DCR, TIM_DCR_DBL_2_TRANSFERS |
+		     (pwm->hwpwm < 2 ? (TIM_CCR1 / 4) : (TIM_CCR3 / 4)));
+	regmap_update_bits(priv->regmap, TIM_DIER, TIM_DIER_XDE,
+			   pwm->hwpwm < 2 ? TIM_DIER_CC1DE : TIM_DIER_CC3DE);
+
+	return 0;
+}
+
+static void stm32_pwm_dma_stop(struct stm32_pwm *priv, struct pwm_device *pwm)
+{
+	struct dma_chan *dma_chan = priv->dmas[pwm->hwpwm < 2 ? 0 : 2];
+
+	regmap_update_bits(priv->regmap, TIM_DIER, TIM_DIER_XDE, 0);
+	regmap_write(priv->regmap, TIM_DCR, 0);
+	dmaengine_terminate_all(dma_chan);
+}
+
+#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)
+
+static int stm32_pwm_do_capture(struct stm32_pwm *priv, struct pwm_device *pwm,
+				unsigned long tmo_ms)
+{
+	unsigned long timeout = msecs_to_jiffies(tmo_ms);
+	long err;
+	int ret;
+
+	reinit_completion(&priv->completion);
+
+	/* Ensure registers have been updated */
+	regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
+
+	/* Clear pending flags, start counter, enable DMA, then capture */
+	regmap_write(priv->regmap, TIM_SR, 0);
+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, TIM_CR1_CEN);
+	ret = stm32_pwm_dma_start(priv, pwm);
+	if (ret)
+		goto stop;
+	regmap_update_bits(priv->regmap, TIM_CCER, pwm->hwpwm < 2 ?
+			   TIM_CCER_CC12E : TIM_CCER_CC34E,
+			   TIM_CCER_CC12E | TIM_CCER_CC34E);
+
+	err = wait_for_completion_interruptible_timeout(&priv->completion,
+							timeout);
+
+	regmap_update_bits(priv->regmap, TIM_CCER, pwm->hwpwm < 2 ?
+			   TIM_CCER_CC12E : TIM_CCER_CC34E, 0);
+	stm32_pwm_dma_stop(priv, pwm);
+	if (err == 0)
+		ret = -ETIMEDOUT;
+	else if (err < 0)
+		ret = err;
+
+stop:
+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, 0);
+	regmap_write(priv->regmap, TIM_SR, 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_do_capture(priv, pwm, tmo_ms);
+	if (ret)
+		goto stop;
+
+	raw_prd = priv->capture[0];
+	raw_dty = priv->capture[1];
+
+	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 +495,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,
@@ -344,23 +611,40 @@ static int stm32_pwm_probe(struct platform_device *pdev)
 	struct device_node *np = dev->of_node;
 	struct stm32_timers *ddata = dev_get_drvdata(pdev->dev.parent);
 	struct stm32_pwm *priv;
-	int ret;
+	bool dma = false;
+	int i, ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
 	mutex_init(&priv->lock);
+	init_completion(&priv->completion);
 	priv->regmap = ddata->regmap;
+	priv->phys_base = ddata->phys_base;
 	priv->clk = ddata->clk;
 	priv->max_arr = ddata->max_arr;
 
 	if (!priv->regmap || !priv->clk)
 		return -EINVAL;
 
+	for (i = 0; i < STM32_TIMERS_MAX_DMAS; i++) {
+		priv->dmas[i] = ddata->dmas[i];
+		if (priv->dmas[i])
+			dma = true;
+	}
+
+	if (dma) {
+		priv->dma_buf = dma_alloc_coherent(dev->parent, PAGE_SIZE,
+						   &priv->dma_buf_phys,
+						   GFP_KERNEL);
+		if (!priv->dma_buf)
+			dev_dbg(dev, "can't allocate DMA buffer\n");
+	}
+
 	ret = stm32_pwm_apply_breakinputs(priv, np);
 	if (ret)
-		return ret;
+		goto dma_free;
 
 	stm32_pwm_detect_complementary(priv);
 
@@ -371,11 +655,18 @@ static int stm32_pwm_probe(struct platform_device *pdev)
 
 	ret = pwmchip_add(&priv->chip);
 	if (ret < 0)
-		return ret;
+		goto dma_free;
 
 	platform_set_drvdata(pdev, priv);
 
 	return 0;
+
+dma_free:
+	if (priv->dma_buf)
+		dma_free_coherent(priv->chip.dev, PAGE_SIZE,
+				  priv->dma_buf, priv->dma_buf_phys);
+
+	return ret;
 }
 
 static int stm32_pwm_remove(struct platform_device *pdev)
@@ -388,6 +679,10 @@ static int stm32_pwm_remove(struct platform_device *pdev)
 
 	pwmchip_remove(&priv->chip);
 
+	if (priv->dma_buf)
+		dma_free_coherent(priv->chip.dev, PAGE_SIZE,
+				  priv->dma_buf, priv->dma_buf_phys);
+
 	return 0;
 }
 
diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
index 2b4ffb9..219eccc 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -30,6 +30,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	   */
@@ -39,17 +41,31 @@
 #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_CC1DE	BIT(9)  /* CC1 DMA request Enable  */
+#define TIM_DIER_CC3DE	BIT(11) /* CC3 DMA request Enable  */
+#define TIM_DIER_XDE	GENMASK(14, 8)
 #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  */
 #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 6/8] 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>
---
 drivers/pwm/pwm-stm32.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index b9c7e878..c890404 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -257,7 +257,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;
 
@@ -311,6 +311,30 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 	raw_prd = priv->capture[0];
 	raw_dty = priv->capture[1];
 
+	/*
+	 * 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 */
+		regmap_write(priv->regmap, TIM_PSC, psc / scale);
+		ret = stm32_pwm_do_capture(priv, pwm, tmo_ms);
+		if (ret)
+			goto stop;
+
+		psc /= scale;
+		raw_prd = priv->capture[0];
+		raw_dty = priv->capture[1];
+	}
+
 	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 6/8] 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>
---
 drivers/pwm/pwm-stm32.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index b9c7e878..c890404 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -257,7 +257,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;
 
@@ -311,6 +311,30 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 	raw_prd = priv->capture[0];
 	raw_dty = priv->capture[1];
 
+	/*
+	 * 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 */
+		regmap_write(priv->regmap, TIM_PSC, psc / scale);
+		ret = stm32_pwm_do_capture(priv, pwm, tmo_ms);
+		if (ret)
+			goto stop;
+
+		psc /= scale;
+		raw_prd = priv->capture[0];
+		raw_dty = priv->capture[1];
+	}
+
 	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 7/8] 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 can help improve
period (only) capture accuracy.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/pwm/pwm-stm32.c          | 67 ++++++++++++++++++++++++++++++++++++++--
 include/linux/mfd/stm32-timers.h |  1 +
 2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index c890404..6b99d92 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/dma-mapping.h>
 #include <linux/mfd/stm32-timers.h>
 #include <linux/module.h>
@@ -257,7 +258,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;
 
@@ -314,6 +315,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 */
@@ -335,8 +337,69 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 		raw_dty = priv->capture[1];
 	}
 
+	/* 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_do_capture(priv, pwm, tmo_ms);
+	if (ret)
+		goto stop;
+
+	raw_prd = priv->capture[0];
+	raw_dty = priv->capture[1];
+
+	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 = (t2 - t0) / icpsc
+		 * - Duty = Period - Low side = Period - (Capture0 - Capture1)
+		 */
+		raw_dty = priv->capture[0] - priv->capture[1];
+		raw_dty = (raw_prd >> icpsc) - 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 219eccc..b67cb28 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -77,6 +77,7 @@
 #define TIM_BDTR_BK2P	BIT(25) /* Break 2 input polarity  */
 
 #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 7/8] 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 can help improve
period (only) capture accuracy.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
---
 drivers/pwm/pwm-stm32.c          | 67 ++++++++++++++++++++++++++++++++++++++--
 include/linux/mfd/stm32-timers.h |  1 +
 2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index c890404..6b99d92 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/dma-mapping.h>
 #include <linux/mfd/stm32-timers.h>
 #include <linux/module.h>
@@ -257,7 +258,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;
 
@@ -314,6 +315,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 */
@@ -335,8 +337,69 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
 		raw_dty = priv->capture[1];
 	}
 
+	/* 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_do_capture(priv, pwm, tmo_ms);
+	if (ret)
+		goto stop;
+
+	raw_prd = priv->capture[0];
+	raw_dty = priv->capture[1];
+
+	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 = (t2 - t0) / icpsc
+		 * - Duty = Period - Low side = Period - (Capture0 - Capture1)
+		 */
+		raw_dty = priv->capture[0] - priv->capture[1];
+		raw_dty = (raw_prd >> icpsc) - 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 219eccc..b67cb28 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -77,6 +77,7 @@
 #define TIM_BDTR_BK2P	BIT(25) /* Break 2 input polarity  */
 
 #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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 8/8] 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>
---
 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 8/8] 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>
---
 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