linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 00/11] pwm: imx: Provide atomic operation for IMX PWM driver
@ 2017-01-29 21:54 Lukasz Majewski
  2017-01-29 21:54 ` [PATCH v5 01/11] pwm: print error messages with pr_err() instead of pr_debug() Lukasz Majewski
                   ` (10 more replies)
  0 siblings, 11 replies; 21+ messages in thread
From: Lukasz Majewski @ 2017-01-29 21:54 UTC (permalink / raw)
  To: Thierry Reding, Sascha Hauer, Stefan Agner, Boris Brezillon,
	linux-pwm, Bhuvanchandra DV, linux-kernel
  Cc: Lothar Wassmann, kernel, Fabio Estevam, Lukasz Majewski

This patch set brings atomic operation to i.MX's PWMv2 driver.

This work has been supported and suggested by Boris Brezillon [1] and 
Stefan Agner, by showing how simple the transition could be :-) (in theory).

It has been divided into several steps:

- Remove ipg clock and enable per clock when required (as proposed by Boris 
  Brezillon)

- Provide different pwm ops for PWMv1 and PWMv2

- Separate PWMv1 commits from "generic" and non atomic PWM code

  NOTE: Since I do _not_ have board with PWMv1, I would like to ask somebody
        for testing

- Move some imx_config_v2 code to separate functions

- Provide PWM atomic implementation (the ->apply() driver) for PWMv2 in a
  single patch for better readability. This patch also includes some code
  to address potential issues on i.MX7 (lack of peripheral clock when accessing
  PWM registers).

- Remove redundant PWM code (disable, enable, config callbacks)

- Update proper documentation entries

- Provide support for polarity inversion on top of atomic PWM rework


Test HW:
--------
This patch set has been tested on i.MX6q board with
v4.10-rc5 kernel SHA1: 1b1bc42c1692e9b62756323c675a44cb1a1f9dbd

The PWM operation has been tested with pwm_bl backlight driver by changing
its brightness.

[1]: http://patchwork.ozlabs.org/patch/685402/

Lothar Wassmann (2):
  pwm: print error messages with pr_err() instead of pr_debug()
  pwm: core: make the PWM_POLARITY flag in DTB optional

Lukasz Majewski (8):
  pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2
  pwm: imx: Rewrite imx_pwm_*_v1 code to facilitate switch to atomic pwm
    operation
  pwm: imx: Move PWMv2 software reset code to a separate function
  pwm: imx: Move PWMv2 wait for fifo slot code to a separate function
  pwm: imx: Provide atomic PWM support for i.MX PWMv2
  pwm: imx: Remove redundant i.MX PWMv2 code
  pwm: imx: doc: Update imx-pwm.txt documentation entry
  pwm: imx: Add polarity inversion support to i.MX's PWMv2

Sascha Hauer (1):
  pwm: imx: remove ipg clock and enable per clock when required

 Documentation/devicetree/bindings/pwm/imx-pwm.txt |   6 +-
 drivers/pwm/core.c                                |  26 ++-
 drivers/pwm/pwm-imx.c                             | 269 ++++++++++------------
 3 files changed, 145 insertions(+), 156 deletions(-)

-- 
2.1.4

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

* [PATCH v5 01/11] pwm: print error messages with pr_err() instead of pr_debug()
  2017-01-29 21:54 [PATCH v5 00/11] pwm: imx: Provide atomic operation for IMX PWM driver Lukasz Majewski
@ 2017-01-29 21:54 ` Lukasz Majewski
  2017-01-29 21:54 ` [PATCH v5 02/11] pwm: imx: remove ipg clock and enable per clock when required Lukasz Majewski
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Lukasz Majewski @ 2017-01-29 21:54 UTC (permalink / raw)
  To: Thierry Reding, Sascha Hauer, Stefan Agner, Boris Brezillon,
	linux-pwm, Bhuvanchandra DV, linux-kernel
  Cc: Lothar Wassmann, kernel, Fabio Estevam, Lothar Wassmann

From: Lothar Wassmann <LW@KARO-electronics.de>

Make the messages that are printed in case of fatal errors actually
visible to the user without having to recompile the driver with
debugging enabled.

Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
---
Changes for v5:
- None

Changes for v4:
- None

Changes for v3:
- None
---
 drivers/pwm/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 172ef82..ec7179f 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -663,13 +663,13 @@ struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id)
 	err = of_parse_phandle_with_args(np, "pwms", "#pwm-cells", index,
 					 &args);
 	if (err) {
-		pr_debug("%s(): can't parse \"pwms\" property\n", __func__);
+		pr_err("%s(): can't parse \"pwms\" property\n", __func__);
 		return ERR_PTR(err);
 	}
 
 	pc = of_node_to_pwmchip(args.np);
 	if (IS_ERR(pc)) {
-		pr_debug("%s(): PWM chip not found\n", __func__);
+		pr_err("%s(): PWM chip not found\n", __func__);
 		pwm = ERR_CAST(pc);
 		goto put;
 	}
-- 
2.1.4

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

* [PATCH v5 02/11] pwm: imx: remove ipg clock and enable per clock when required
  2017-01-29 21:54 [PATCH v5 00/11] pwm: imx: Provide atomic operation for IMX PWM driver Lukasz Majewski
  2017-01-29 21:54 ` [PATCH v5 01/11] pwm: print error messages with pr_err() instead of pr_debug() Lukasz Majewski
@ 2017-01-29 21:54 ` Lukasz Majewski
  2017-01-29 21:54 ` [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2 Lukasz Majewski
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Lukasz Majewski @ 2017-01-29 21:54 UTC (permalink / raw)
  To: Thierry Reding, Sascha Hauer, Stefan Agner, Boris Brezillon,
	linux-pwm, Bhuvanchandra DV, linux-kernel
  Cc: Lothar Wassmann, kernel, Fabio Estevam, Philipp Zabel

From: Sascha Hauer <s.hauer@pengutronix.de>

The use of the ipg clock was introduced with commit 7b27c160c681
("pwm: i.MX: fix clock lookup").
In the commit message it was claimed that the ipg clock is enabled for
register accesses. This is true for the ->config() callback, but not
for the ->set_enable() callback. Given that the ipg clock is not
consistently enabled for all register accesses we can assume that either
it is not required at all or that the current code does not work.
Remove the ipg clock code for now so that it's no longer in the way of
refactoring the driver.

On the other hand, the imx7 IP requires the peripheral clock to be
enabled before accessing its registers. Since ->config() can be called
when the PWM is disabled (in which case, the peripheral clock is also
disabled), we need to surround the imx->config() with
clk_prepare_enable(per_clk)/clk_disable_unprepare(per_clk) calls.

Note that the driver was working fine for the i.MX 7 IP so far because
the ipg and peripheral clock use the same hardware clock gate, which
guaranteed peripheral clock activation even when ->config() was called
when the PWM was disabled.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Stefan Agner <stefan@agner.ch>
Tested-by: Stefan Agner <stefan@agner.ch>
---
Changes in v5:
- Minor edit of commmit message

Changes in v4:
- Enable per clk before calling imx->config()

Changes in v3:
- New patch
---
 drivers/pwm/pwm-imx.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index d600fd5..b1d1e50 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -49,7 +49,6 @@
 
 struct imx_chip {
 	struct clk	*clk_per;
-	struct clk	*clk_ipg;
 
 	void __iomem	*mmio_base;
 
@@ -206,13 +205,13 @@ static int imx_pwm_config(struct pwm_chip *chip,
 	struct imx_chip *imx = to_imx_chip(chip);
 	int ret;
 
-	ret = clk_prepare_enable(imx->clk_ipg);
+	ret = clk_prepare_enable(imx->clk_per);
 	if (ret)
 		return ret;
 
 	ret = imx->config(chip, pwm, duty_ns, period_ns);
 
-	clk_disable_unprepare(imx->clk_ipg);
+	clk_disable_unprepare(imx->clk_per);
 
 	return ret;
 }
@@ -293,13 +292,6 @@ static int imx_pwm_probe(struct platform_device *pdev)
 		return PTR_ERR(imx->clk_per);
 	}
 
-	imx->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
-	if (IS_ERR(imx->clk_ipg)) {
-		dev_err(&pdev->dev, "getting ipg clock failed with %ld\n",
-				PTR_ERR(imx->clk_ipg));
-		return PTR_ERR(imx->clk_ipg);
-	}
-
 	imx->chip.ops = &imx_pwm_ops;
 	imx->chip.dev = &pdev->dev;
 	imx->chip.base = -1;
-- 
2.1.4

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

* [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2
  2017-01-29 21:54 [PATCH v5 00/11] pwm: imx: Provide atomic operation for IMX PWM driver Lukasz Majewski
  2017-01-29 21:54 ` [PATCH v5 01/11] pwm: print error messages with pr_err() instead of pr_debug() Lukasz Majewski
  2017-01-29 21:54 ` [PATCH v5 02/11] pwm: imx: remove ipg clock and enable per clock when required Lukasz Majewski
@ 2017-01-29 21:54 ` Lukasz Majewski
  2017-01-30  7:23   ` Thierry Reding
  2017-01-29 21:54 ` [PATCH v5 04/11] pwm: imx: Rewrite imx_pwm_*_v1 code to facilitate switch to atomic pwm operation Lukasz Majewski
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 21+ messages in thread
From: Lukasz Majewski @ 2017-01-29 21:54 UTC (permalink / raw)
  To: Thierry Reding, Sascha Hauer, Stefan Agner, Boris Brezillon,
	linux-pwm, Bhuvanchandra DV, linux-kernel
  Cc: Lothar Wassmann, kernel, Fabio Estevam, Lukasz Majewski,
	Lothar Waßmann

From: Lukasz Majewski <l.majewski@majess.pl>

This patch provides separate set of pwm ops utilized by
i.MX's PWMv1 and PWMv2.

Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
Acked-by: Shawn Guo <shawn.guo@linaro.org>
Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Changes for v5:
- None

Changes for v4:
- None

Changes for v3:
- Adjust the code to work with ipg clock removed

Changes for v2:
- New patch
---
 drivers/pwm/pwm-imx.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index b1d1e50..0fa480d 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -239,7 +239,14 @@ static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 	clk_disable_unprepare(imx->clk_per);
 }
 
-static struct pwm_ops imx_pwm_ops = {
+static struct pwm_ops imx_pwm_ops_v1 = {
+	.enable = imx_pwm_enable,
+	.disable = imx_pwm_disable,
+	.config = imx_pwm_config,
+	.owner = THIS_MODULE,
+};
+
+static struct pwm_ops imx_pwm_ops_v2 = {
 	.enable = imx_pwm_enable,
 	.disable = imx_pwm_disable,
 	.config = imx_pwm_config,
@@ -250,16 +257,19 @@ struct imx_pwm_data {
 	int (*config)(struct pwm_chip *chip,
 		struct pwm_device *pwm, int duty_ns, int period_ns);
 	void (*set_enable)(struct pwm_chip *chip, bool enable);
+	struct pwm_ops *pwm_ops;
 };
 
 static struct imx_pwm_data imx_pwm_data_v1 = {
 	.config = imx_pwm_config_v1,
 	.set_enable = imx_pwm_set_enable_v1,
+	.pwm_ops = &imx_pwm_ops_v1,
 };
 
 static struct imx_pwm_data imx_pwm_data_v2 = {
 	.config = imx_pwm_config_v2,
 	.set_enable = imx_pwm_set_enable_v2,
+	.pwm_ops = &imx_pwm_ops_v2,
 };
 
 static const struct of_device_id imx_pwm_dt_ids[] = {
@@ -281,6 +291,8 @@ static int imx_pwm_probe(struct platform_device *pdev)
 	if (!of_id)
 		return -ENODEV;
 
+	data = of_id->data;
+
 	imx = devm_kzalloc(&pdev->dev, sizeof(*imx), GFP_KERNEL);
 	if (imx == NULL)
 		return -ENOMEM;
@@ -292,7 +304,7 @@ static int imx_pwm_probe(struct platform_device *pdev)
 		return PTR_ERR(imx->clk_per);
 	}
 
-	imx->chip.ops = &imx_pwm_ops;
+	imx->chip.ops = data->pwm_ops;
 	imx->chip.dev = &pdev->dev;
 	imx->chip.base = -1;
 	imx->chip.npwm = 1;
@@ -303,7 +315,6 @@ static int imx_pwm_probe(struct platform_device *pdev)
 	if (IS_ERR(imx->mmio_base))
 		return PTR_ERR(imx->mmio_base);
 
-	data = of_id->data;
 	imx->config = data->config;
 	imx->set_enable = data->set_enable;
 
-- 
2.1.4

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

* [PATCH v5 04/11] pwm: imx: Rewrite imx_pwm_*_v1 code to facilitate switch to atomic pwm operation
  2017-01-29 21:54 [PATCH v5 00/11] pwm: imx: Provide atomic operation for IMX PWM driver Lukasz Majewski
                   ` (2 preceding siblings ...)
  2017-01-29 21:54 ` [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2 Lukasz Majewski
@ 2017-01-29 21:54 ` Lukasz Majewski
  2017-01-29 21:54 ` [PATCH v5 05/11] pwm: imx: Move PWMv2 software reset code to a separate function Lukasz Majewski
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Lukasz Majewski @ 2017-01-29 21:54 UTC (permalink / raw)
  To: Thierry Reding, Sascha Hauer, Stefan Agner, Boris Brezillon,
	linux-pwm, Bhuvanchandra DV, linux-kernel
  Cc: Lothar Wassmann, kernel, Fabio Estevam, Lukasz Majewski

From: Lukasz Majewski <l.majewski@majess.pl>

The code has been rewritten to remove "generic" calls to
imx_pwm_{enable|disable|config}.

Such approach would facilitate switch to atomic PWM (a.k.a ->apply())
implementation.

Suggested-by: Stefan Agner <stefan@agner.ch>
Suggested-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Reviewed-by: Stefan Agner <stefan@agner.ch>
Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
---
Changes for v5:
- None

Changes for v4:
- None

[Minor code adjustment according to Stefan Agner comment]

Changes for v3:
- Remove ipg clock

Changes for v2:
- Add missing clock unprepare for clk_ipg
- Enable peripheral PWM clock (clk_per)
---
 drivers/pwm/pwm-imx.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index 0fa480d..11e3f3e 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -90,19 +90,34 @@ static int imx_pwm_config_v1(struct pwm_chip *chip,
 	return 0;
 }
 
-static void imx_pwm_set_enable_v1(struct pwm_chip *chip, bool enable)
+static int imx_pwm_enable_v1(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct imx_chip *imx = to_imx_chip(chip);
+	int ret;
 	u32 val;
 
+	ret = clk_prepare_enable(imx->clk_per);
+	if (ret)
+		return ret;
+
 	val = readl(imx->mmio_base + MX1_PWMC);
+	val |= MX1_PWMC_EN;
+	writel(val, imx->mmio_base + MX1_PWMC);
 
-	if (enable)
-		val |= MX1_PWMC_EN;
-	else
-		val &= ~MX1_PWMC_EN;
+	return 0;
+}
+
+static void imx_pwm_disable_v1(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct imx_chip *imx = to_imx_chip(chip);
+	u32 val;
+
+	val = readl(imx->mmio_base + MX1_PWMC);
+	val &= ~MX1_PWMC_EN;
 
 	writel(val, imx->mmio_base + MX1_PWMC);
+
+	clk_disable_unprepare(imx->clk_per);
 }
 
 static int imx_pwm_config_v2(struct pwm_chip *chip,
@@ -240,9 +255,9 @@ static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 }
 
 static struct pwm_ops imx_pwm_ops_v1 = {
-	.enable = imx_pwm_enable,
-	.disable = imx_pwm_disable,
-	.config = imx_pwm_config,
+	.enable = imx_pwm_enable_v1,
+	.disable = imx_pwm_disable_v1,
+	.config = imx_pwm_config_v1,
 	.owner = THIS_MODULE,
 };
 
@@ -261,8 +276,6 @@ struct imx_pwm_data {
 };
 
 static struct imx_pwm_data imx_pwm_data_v1 = {
-	.config = imx_pwm_config_v1,
-	.set_enable = imx_pwm_set_enable_v1,
 	.pwm_ops = &imx_pwm_ops_v1,
 };
 
-- 
2.1.4

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

* [PATCH v5 05/11] pwm: imx: Move PWMv2 software reset code to a separate function
  2017-01-29 21:54 [PATCH v5 00/11] pwm: imx: Provide atomic operation for IMX PWM driver Lukasz Majewski
                   ` (3 preceding siblings ...)
  2017-01-29 21:54 ` [PATCH v5 04/11] pwm: imx: Rewrite imx_pwm_*_v1 code to facilitate switch to atomic pwm operation Lukasz Majewski
@ 2017-01-29 21:54 ` Lukasz Majewski
  2017-01-30  7:26   ` Thierry Reding
  2017-01-29 21:54 ` [PATCH v5 06/11] pwm: imx: Move PWMv2 wait for fifo slot " Lukasz Majewski
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 21+ messages in thread
From: Lukasz Majewski @ 2017-01-29 21:54 UTC (permalink / raw)
  To: Thierry Reding, Sascha Hauer, Stefan Agner, Boris Brezillon,
	linux-pwm, Bhuvanchandra DV, linux-kernel
  Cc: Lothar Wassmann, kernel, Fabio Estevam, Lukasz Majewski

From: Lukasz Majewski <l.majewski@majess.pl>

The software reset code has been extracted from imx_pwm_config_v2 function
and moved to new one - imx_pwm_sw_reset().

This change reduces the overall size of imx_pwm_config_v2() and prepares
it for atomic PWM operation.

Suggested-by: Stefan Agner <stefan@agner.ch>
Suggested-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>

---
Changes for v5:
- None

Changes for v4:
- None

Changes for v3:
- None

Changes for v2:
- Add missing parenthesis
---
 drivers/pwm/pwm-imx.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index 11e3f3e..f0d78f3 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -120,6 +120,25 @@ static void imx_pwm_disable_v1(struct pwm_chip *chip, struct pwm_device *pwm)
 	clk_disable_unprepare(imx->clk_per);
 }
 
+static void imx_pwm_sw_reset(struct pwm_chip *chip)
+{
+	struct imx_chip *imx = to_imx_chip(chip);
+	struct device *dev = chip->dev;
+	int wait_count = 0;
+	u32 cr;
+
+	writel(MX3_PWMCR_SWR, imx->mmio_base + MX3_PWMCR);
+	do {
+		usleep_range(200, 1000);
+		cr = readl(imx->mmio_base + MX3_PWMCR);
+	} while ((cr & MX3_PWMCR_SWR) &&
+		 (wait_count++ < MX3_PWM_SWR_LOOP));
+
+	if (cr & MX3_PWMCR_SWR)
+		dev_warn(dev, "software reset timeout\n");
+}
+
+
 static int imx_pwm_config_v2(struct pwm_chip *chip,
 		struct pwm_device *pwm, int duty_ns, int period_ns)
 {
@@ -129,7 +148,7 @@ static int imx_pwm_config_v2(struct pwm_chip *chip,
 	unsigned long period_cycles, duty_cycles, prescale;
 	unsigned int period_ms;
 	bool enable = pwm_is_enabled(pwm);
-	int wait_count = 0, fifoav;
+	int fifoav;
 	u32 cr, sr;
 
 	/*
@@ -152,15 +171,7 @@ static int imx_pwm_config_v2(struct pwm_chip *chip,
 				dev_warn(dev, "there is no free FIFO slot\n");
 		}
 	} else {
-		writel(MX3_PWMCR_SWR, imx->mmio_base + MX3_PWMCR);
-		do {
-			usleep_range(200, 1000);
-			cr = readl(imx->mmio_base + MX3_PWMCR);
-		} while ((cr & MX3_PWMCR_SWR) &&
-			 (wait_count++ < MX3_PWM_SWR_LOOP));
-
-		if (cr & MX3_PWMCR_SWR)
-			dev_warn(dev, "software reset timeout\n");
+		imx_pwm_sw_reset(chip);
 	}
 
 	c = clk_get_rate(imx->clk_per);
-- 
2.1.4

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

* [PATCH v5 06/11] pwm: imx: Move PWMv2 wait for fifo slot code to a separate function
  2017-01-29 21:54 [PATCH v5 00/11] pwm: imx: Provide atomic operation for IMX PWM driver Lukasz Majewski
                   ` (4 preceding siblings ...)
  2017-01-29 21:54 ` [PATCH v5 05/11] pwm: imx: Move PWMv2 software reset code to a separate function Lukasz Majewski
@ 2017-01-29 21:54 ` Lukasz Majewski
  2017-01-29 21:54 ` [PATCH v5 07/11] pwm: imx: Provide atomic PWM support for i.MX PWMv2 Lukasz Majewski
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Lukasz Majewski @ 2017-01-29 21:54 UTC (permalink / raw)
  To: Thierry Reding, Sascha Hauer, Stefan Agner, Boris Brezillon,
	linux-pwm, Bhuvanchandra DV, linux-kernel
  Cc: Lothar Wassmann, kernel, Fabio Estevam, Lukasz Majewski

From: Lukasz Majewski <l.majewski@majess.pl>

The code, which waits for fifo slot, has been extracted from
imx_pwm_config_v2 function and moved to new one - imx_pwm_wait_fifo_slot().

This change reduces the overall size of imx_pwm_config_v2() and prepares
it for atomic PWM operation.

Suggested-by: Stefan Agner <stefan@agner.ch>
Suggested-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
---
Changes for v5:
- None

Changes for v4:
- None

Changes for v3:
- None

Changes for v2:
- None
---
 drivers/pwm/pwm-imx.c | 43 +++++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index f0d78f3..60cdc5c 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -138,18 +138,36 @@ static void imx_pwm_sw_reset(struct pwm_chip *chip)
 		dev_warn(dev, "software reset timeout\n");
 }
 
+static void imx_pwm_wait_fifo_slot(struct pwm_chip *chip,
+				   struct pwm_device *pwm)
+{
+	struct imx_chip *imx = to_imx_chip(chip);
+	struct device *dev = chip->dev;
+	unsigned int period_ms;
+	int fifoav;
+	u32 sr;
+
+	sr = readl(imx->mmio_base + MX3_PWMSR);
+	fifoav = sr & MX3_PWMSR_FIFOAV_MASK;
+	if (fifoav == MX3_PWMSR_FIFOAV_4WORDS) {
+		period_ms = DIV_ROUND_UP(pwm_get_period(pwm),
+					 NSEC_PER_MSEC);
+		msleep(period_ms);
+
+		sr = readl(imx->mmio_base + MX3_PWMSR);
+		if (fifoav == (sr & MX3_PWMSR_FIFOAV_MASK))
+			dev_warn(dev, "there is no free FIFO slot\n");
+	}
+}
 
 static int imx_pwm_config_v2(struct pwm_chip *chip,
 		struct pwm_device *pwm, int duty_ns, int period_ns)
 {
 	struct imx_chip *imx = to_imx_chip(chip);
-	struct device *dev = chip->dev;
 	unsigned long long c;
 	unsigned long period_cycles, duty_cycles, prescale;
-	unsigned int period_ms;
 	bool enable = pwm_is_enabled(pwm);
-	int fifoav;
-	u32 cr, sr;
+	u32 cr;
 
 	/*
 	 * i.MX PWMv2 has a 4-word sample FIFO.
@@ -158,21 +176,10 @@ static int imx_pwm_config_v2(struct pwm_chip *chip,
 	 * wait for a full PWM cycle to get a relinquished FIFO slot
 	 * when the controller is enabled and the FIFO is fully loaded.
 	 */
-	if (enable) {
-		sr = readl(imx->mmio_base + MX3_PWMSR);
-		fifoav = sr & MX3_PWMSR_FIFOAV_MASK;
-		if (fifoav == MX3_PWMSR_FIFOAV_4WORDS) {
-			period_ms = DIV_ROUND_UP(pwm_get_period(pwm),
-						 NSEC_PER_MSEC);
-			msleep(period_ms);
-
-			sr = readl(imx->mmio_base + MX3_PWMSR);
-			if (fifoav == (sr & MX3_PWMSR_FIFOAV_MASK))
-				dev_warn(dev, "there is no free FIFO slot\n");
-		}
-	} else {
+	if (enable)
+		imx_pwm_wait_fifo_slot(chip, pwm);
+	else
 		imx_pwm_sw_reset(chip);
-	}
 
 	c = clk_get_rate(imx->clk_per);
 	c = c * period_ns;
-- 
2.1.4

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

* [PATCH v5 07/11] pwm: imx: Provide atomic PWM support for i.MX PWMv2
  2017-01-29 21:54 [PATCH v5 00/11] pwm: imx: Provide atomic operation for IMX PWM driver Lukasz Majewski
                   ` (5 preceding siblings ...)
  2017-01-29 21:54 ` [PATCH v5 06/11] pwm: imx: Move PWMv2 wait for fifo slot " Lukasz Majewski
@ 2017-01-29 21:54 ` Lukasz Majewski
  2017-01-30  7:49   ` Boris Brezillon
  2017-01-29 21:54 ` [PATCH v5 08/11] pwm: imx: Remove redundant i.MX PWMv2 code Lukasz Majewski
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 21+ messages in thread
From: Lukasz Majewski @ 2017-01-29 21:54 UTC (permalink / raw)
  To: Thierry Reding, Sascha Hauer, Stefan Agner, Boris Brezillon,
	linux-pwm, Bhuvanchandra DV, linux-kernel
  Cc: Lothar Wassmann, kernel, Fabio Estevam, Lukasz Majewski, Lukasz Majewski

This commit provides apply() callback implementation for i.MX's PWMv2.

Suggested-by: Stefan Agner <stefan@agner.ch>
Suggested-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
Changes for v5:
- Modify ->apply() function to avoid unbalanced clock enabling/disabling
- Fix preventing iMX7 from hanging

Changes for v4:
- Avoid recalculation of PWM parameters when disabling PWM signal
- Unconditionally call clk_prepare_enable(imx->clk_per) and
  clk_disable_unprepare(imx->clk_per)

Changes for v3:
- Remove ipg clock enable/disable functions

Changes for v2:
- None
---
 drivers/pwm/pwm-imx.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index 60cdc5c..fdaa11b 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -249,6 +249,73 @@ static int imx_pwm_config(struct pwm_chip *chip,
 	return ret;
 }
 
+static int imx_pwm_apply_v2(struct pwm_chip *chip, struct pwm_device *pwm,
+			    struct pwm_state *state)
+{
+	unsigned long period_cycles, duty_cycles, prescale;
+	struct imx_chip *imx = to_imx_chip(chip);
+	struct pwm_state cstate;
+	unsigned long long c;
+	int ret;
+
+	pwm_get_state(pwm, &cstate);
+
+	if (state->enabled) {
+		c = clk_get_rate(imx->clk_per);
+		c *= state->period;
+
+		do_div(c, 1000000000);
+		period_cycles = c;
+
+		prescale = period_cycles / 0x10000 + 1;
+
+		period_cycles /= prescale;
+		c = (unsigned long long)period_cycles * state->duty_cycle;
+		do_div(c, state->period);
+		duty_cycles = c;
+
+		/*
+		 * according to imx pwm RM, the real period value should be
+		 * PERIOD value in PWMPR plus 2.
+		 */
+		if (period_cycles > 2)
+			period_cycles -= 2;
+		else
+			period_cycles = 0;
+
+		/*
+		 * Wait for a free FIFO slot if the PWM is already enabled, and
+		 * flush the FIFO if the PWM was disabled and is about to be
+		 * enabled.
+		 */
+		if (cstate.enabled) {
+			imx_pwm_wait_fifo_slot(chip, pwm);
+		} else if (state->enabled) {
+			ret = clk_prepare_enable(imx->clk_per);
+			if (ret)
+				return ret;
+
+			imx_pwm_sw_reset(chip);
+		}
+
+		writel(duty_cycles, imx->mmio_base + MX3_PWMSAR);
+		writel(period_cycles, imx->mmio_base + MX3_PWMPR);
+
+		writel(MX3_PWMCR_PRESCALER(prescale) |
+		       MX3_PWMCR_DOZEEN | MX3_PWMCR_WAITEN |
+		       MX3_PWMCR_DBGEN | MX3_PWMCR_CLKSRC_IPG_HIGH |
+		       MX3_PWMCR_EN,
+		       imx->mmio_base + MX3_PWMCR);
+
+	} else if (cstate.enabled) {
+		writel(0, imx->mmio_base + MX3_PWMCR);
+
+		clk_disable_unprepare(imx->clk_per);
+	}
+
+	return 0;
+}
+
 static int imx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct imx_chip *imx = to_imx_chip(chip);
@@ -280,6 +347,7 @@ static struct pwm_ops imx_pwm_ops_v1 = {
 };
 
 static struct pwm_ops imx_pwm_ops_v2 = {
+	.apply = imx_pwm_apply_v2,
 	.enable = imx_pwm_enable,
 	.disable = imx_pwm_disable,
 	.config = imx_pwm_config,
-- 
2.1.4

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

* [PATCH v5 08/11] pwm: imx: Remove redundant i.MX PWMv2 code
  2017-01-29 21:54 [PATCH v5 00/11] pwm: imx: Provide atomic operation for IMX PWM driver Lukasz Majewski
                   ` (6 preceding siblings ...)
  2017-01-29 21:54 ` [PATCH v5 07/11] pwm: imx: Provide atomic PWM support for i.MX PWMv2 Lukasz Majewski
@ 2017-01-29 21:54 ` Lukasz Majewski
  2017-01-29 21:54 ` [PATCH v5 09/11] pwm: core: make the PWM_POLARITY flag in DTB optional Lukasz Majewski
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Lukasz Majewski @ 2017-01-29 21:54 UTC (permalink / raw)
  To: Thierry Reding, Sascha Hauer, Stefan Agner, Boris Brezillon,
	linux-pwm, Bhuvanchandra DV, linux-kernel
  Cc: Lothar Wassmann, kernel, Fabio Estevam, Lukasz Majewski, Lukasz Majewski

The code providing functionality surpassed by the atomic PWM is not needed
anymore and hence can be removed.

Suggested-by: Stefan Agner <stefan@agner.ch>
Suggested-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
Changes for v5:
- None

Changes for v4:
- None

Changes for v3:
- None

Changes for v2:
- None
---
 drivers/pwm/pwm-imx.c | 127 --------------------------------------------------
 1 file changed, 127 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index fdaa11b..e037023 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -53,10 +53,6 @@ struct imx_chip {
 	void __iomem	*mmio_base;
 
 	struct pwm_chip	chip;
-
-	int (*config)(struct pwm_chip *chip,
-		struct pwm_device *pwm, int duty_ns, int period_ns);
-	void (*set_enable)(struct pwm_chip *chip, bool enable);
 };
 
 #define to_imx_chip(chip)	container_of(chip, struct imx_chip, chip)
@@ -160,95 +156,6 @@ static void imx_pwm_wait_fifo_slot(struct pwm_chip *chip,
 	}
 }
 
-static int imx_pwm_config_v2(struct pwm_chip *chip,
-		struct pwm_device *pwm, int duty_ns, int period_ns)
-{
-	struct imx_chip *imx = to_imx_chip(chip);
-	unsigned long long c;
-	unsigned long period_cycles, duty_cycles, prescale;
-	bool enable = pwm_is_enabled(pwm);
-	u32 cr;
-
-	/*
-	 * i.MX PWMv2 has a 4-word sample FIFO.
-	 * In order to avoid FIFO overflow issue, we do software reset
-	 * to clear all sample FIFO if the controller is disabled or
-	 * wait for a full PWM cycle to get a relinquished FIFO slot
-	 * when the controller is enabled and the FIFO is fully loaded.
-	 */
-	if (enable)
-		imx_pwm_wait_fifo_slot(chip, pwm);
-	else
-		imx_pwm_sw_reset(chip);
-
-	c = clk_get_rate(imx->clk_per);
-	c = c * period_ns;
-	do_div(c, 1000000000);
-	period_cycles = c;
-
-	prescale = period_cycles / 0x10000 + 1;
-
-	period_cycles /= prescale;
-	c = (unsigned long long)period_cycles * duty_ns;
-	do_div(c, period_ns);
-	duty_cycles = c;
-
-	/*
-	 * according to imx pwm RM, the real period value should be
-	 * PERIOD value in PWMPR plus 2.
-	 */
-	if (period_cycles > 2)
-		period_cycles -= 2;
-	else
-		period_cycles = 0;
-
-	writel(duty_cycles, imx->mmio_base + MX3_PWMSAR);
-	writel(period_cycles, imx->mmio_base + MX3_PWMPR);
-
-	cr = MX3_PWMCR_PRESCALER(prescale) |
-		MX3_PWMCR_DOZEEN | MX3_PWMCR_WAITEN |
-		MX3_PWMCR_DBGEN | MX3_PWMCR_CLKSRC_IPG_HIGH;
-
-	if (enable)
-		cr |= MX3_PWMCR_EN;
-
-	writel(cr, imx->mmio_base + MX3_PWMCR);
-
-	return 0;
-}
-
-static void imx_pwm_set_enable_v2(struct pwm_chip *chip, bool enable)
-{
-	struct imx_chip *imx = to_imx_chip(chip);
-	u32 val;
-
-	val = readl(imx->mmio_base + MX3_PWMCR);
-
-	if (enable)
-		val |= MX3_PWMCR_EN;
-	else
-		val &= ~MX3_PWMCR_EN;
-
-	writel(val, imx->mmio_base + MX3_PWMCR);
-}
-
-static int imx_pwm_config(struct pwm_chip *chip,
-		struct pwm_device *pwm, int duty_ns, int period_ns)
-{
-	struct imx_chip *imx = to_imx_chip(chip);
-	int ret;
-
-	ret = clk_prepare_enable(imx->clk_per);
-	if (ret)
-		return ret;
-
-	ret = imx->config(chip, pwm, duty_ns, period_ns);
-
-	clk_disable_unprepare(imx->clk_per);
-
-	return ret;
-}
-
 static int imx_pwm_apply_v2(struct pwm_chip *chip, struct pwm_device *pwm,
 			    struct pwm_state *state)
 {
@@ -316,29 +223,6 @@ static int imx_pwm_apply_v2(struct pwm_chip *chip, struct pwm_device *pwm,
 	return 0;
 }
 
-static int imx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
-{
-	struct imx_chip *imx = to_imx_chip(chip);
-	int ret;
-
-	ret = clk_prepare_enable(imx->clk_per);
-	if (ret)
-		return ret;
-
-	imx->set_enable(chip, true);
-
-	return 0;
-}
-
-static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
-{
-	struct imx_chip *imx = to_imx_chip(chip);
-
-	imx->set_enable(chip, false);
-
-	clk_disable_unprepare(imx->clk_per);
-}
-
 static struct pwm_ops imx_pwm_ops_v1 = {
 	.enable = imx_pwm_enable_v1,
 	.disable = imx_pwm_disable_v1,
@@ -348,16 +232,10 @@ static struct pwm_ops imx_pwm_ops_v1 = {
 
 static struct pwm_ops imx_pwm_ops_v2 = {
 	.apply = imx_pwm_apply_v2,
-	.enable = imx_pwm_enable,
-	.disable = imx_pwm_disable,
-	.config = imx_pwm_config,
 	.owner = THIS_MODULE,
 };
 
 struct imx_pwm_data {
-	int (*config)(struct pwm_chip *chip,
-		struct pwm_device *pwm, int duty_ns, int period_ns);
-	void (*set_enable)(struct pwm_chip *chip, bool enable);
 	struct pwm_ops *pwm_ops;
 };
 
@@ -366,8 +244,6 @@ static struct imx_pwm_data imx_pwm_data_v1 = {
 };
 
 static struct imx_pwm_data imx_pwm_data_v2 = {
-	.config = imx_pwm_config_v2,
-	.set_enable = imx_pwm_set_enable_v2,
 	.pwm_ops = &imx_pwm_ops_v2,
 };
 
@@ -414,9 +290,6 @@ static int imx_pwm_probe(struct platform_device *pdev)
 	if (IS_ERR(imx->mmio_base))
 		return PTR_ERR(imx->mmio_base);
 
-	imx->config = data->config;
-	imx->set_enable = data->set_enable;
-
 	ret = pwmchip_add(&imx->chip);
 	if (ret < 0)
 		return ret;
-- 
2.1.4

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

* [PATCH v5 09/11] pwm: core: make the PWM_POLARITY flag in DTB optional
  2017-01-29 21:54 [PATCH v5 00/11] pwm: imx: Provide atomic operation for IMX PWM driver Lukasz Majewski
                   ` (7 preceding siblings ...)
  2017-01-29 21:54 ` [PATCH v5 08/11] pwm: imx: Remove redundant i.MX PWMv2 code Lukasz Majewski
@ 2017-01-29 21:54 ` Lukasz Majewski
  2017-01-29 21:54 ` [PATCH v5 10/11] pwm: imx: doc: Update imx-pwm.txt documentation entry Lukasz Majewski
  2017-01-29 21:54 ` [PATCH v5 11/11] pwm: imx: Add polarity inversion support to i.MX's PWMv2 Lukasz Majewski
  10 siblings, 0 replies; 21+ messages in thread
From: Lukasz Majewski @ 2017-01-29 21:54 UTC (permalink / raw)
  To: Thierry Reding, Sascha Hauer, Stefan Agner, Boris Brezillon,
	linux-pwm, Bhuvanchandra DV, linux-kernel
  Cc: Lothar Wassmann, kernel, Fabio Estevam, Lothar Wassmann, Lukasz Majewski

From: Lothar Wassmann <LW@KARO-electronics.de>

Change the pwm chip driver registration, so that a chip driver that
supports polarity inversion can still be used with DTBs that don't
provide the 'PWM_POLARITY' flag.

This is done to provide polarity inversion support for the pwm-imx
driver without having to modify all existing DTS files.

Signed-off-by: Lothar Wassmann <LW@KARO-electronics.de>
Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
Suggested-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
---
Changes for v5:
- None

Changes for v4:
- None

Changes for v3:
- None

Changes for v2:
- None
---
 drivers/pwm/core.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index ec7179f..d80e5c5 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -137,9 +137,14 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
 {
 	struct pwm_device *pwm;
 
+	/* check, whether the driver supports a third cell for flags */
 	if (pc->of_pwm_n_cells < 3)
 		return ERR_PTR(-EINVAL);
 
+	/* flags in the third cell are optional */
+	if (args->args_count < 2)
+		return ERR_PTR(-EINVAL);
+
 	if (args->args[0] >= pc->npwm)
 		return ERR_PTR(-EINVAL);
 
@@ -148,11 +153,10 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
 		return pwm;
 
 	pwm->args.period = args->args[1];
+	pwm->args.polarity = PWM_POLARITY_NORMAL;
 
-	if (args->args[2] & PWM_POLARITY_INVERTED)
+	if (args->args_count > 2 && args->args[2] & PWM_POLARITY_INVERTED)
 		pwm->args.polarity = PWM_POLARITY_INVERSED;
-	else
-		pwm->args.polarity = PWM_POLARITY_NORMAL;
 
 	return pwm;
 }
@@ -163,9 +167,14 @@ of_pwm_simple_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
 {
 	struct pwm_device *pwm;
 
+	/* sanity check driver support */
 	if (pc->of_pwm_n_cells < 2)
 		return ERR_PTR(-EINVAL);
 
+	/* all cells are required */
+	if (args->args_count != pc->of_pwm_n_cells)
+		return ERR_PTR(-EINVAL);
+
 	if (args->args[0] >= pc->npwm)
 		return ERR_PTR(-EINVAL);
 
@@ -674,13 +683,6 @@ struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id)
 		goto put;
 	}
 
-	if (args.args_count != pc->of_pwm_n_cells) {
-		pr_debug("%s: wrong #pwm-cells for %s\n", np->full_name,
-			 args.np->full_name);
-		pwm = ERR_PTR(-EINVAL);
-		goto put;
-	}
-
 	pwm = pc->of_xlate(pc, &args);
 	if (IS_ERR(pwm))
 		goto put;
-- 
2.1.4

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

* [PATCH v5 10/11] pwm: imx: doc: Update imx-pwm.txt documentation entry
  2017-01-29 21:54 [PATCH v5 00/11] pwm: imx: Provide atomic operation for IMX PWM driver Lukasz Majewski
                   ` (8 preceding siblings ...)
  2017-01-29 21:54 ` [PATCH v5 09/11] pwm: core: make the PWM_POLARITY flag in DTB optional Lukasz Majewski
@ 2017-01-29 21:54 ` Lukasz Majewski
  2017-01-29 21:54 ` [PATCH v5 11/11] pwm: imx: Add polarity inversion support to i.MX's PWMv2 Lukasz Majewski
  10 siblings, 0 replies; 21+ messages in thread
From: Lukasz Majewski @ 2017-01-29 21:54 UTC (permalink / raw)
  To: Thierry Reding, Sascha Hauer, Stefan Agner, Boris Brezillon,
	linux-pwm, Bhuvanchandra DV, linux-kernel
  Cc: Lothar Wassmann, kernel, Fabio Estevam, Lukasz Majewski

From: Lukasz Majewski <l.majewski@majess.pl>

The imx-pwm.txt documentation update as a preparation for polarity
support.

Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
Acked-by: Rob Herring <robh@kernel.org>
---
Changes for v5:
- None

Changes for v4:
- None

Changes for v3:
- None

Changes for v2:
- New patch
---
 Documentation/devicetree/bindings/pwm/imx-pwm.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
index e00c2e9..c61bdf8 100644
--- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
@@ -6,8 +6,8 @@ Required properties:
   - "fsl,imx1-pwm" for PWM compatible with the one integrated on i.MX1
   - "fsl,imx27-pwm" for PWM compatible with the one integrated on i.MX27
 - reg: physical base address and length of the controller's registers
-- #pwm-cells: should be 2. See pwm.txt in this directory for a description of
-  the cells format.
+- #pwm-cells: 2 for i.MX1 and 3 for i.MX27 and newer SoCs. See pwm.txt
+  in this directory for a description of the cells format.
 - clocks : Clock specifiers for both ipg and per clocks.
 - clock-names : Clock names should include both "ipg" and "per"
 See the clock consumer binding,
@@ -17,7 +17,7 @@ See the clock consumer binding,
 Example:
 
 pwm1: pwm@53fb4000 {
-	#pwm-cells = <2>;
+	#pwm-cells = <3>;
 	compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
 	reg = <0x53fb4000 0x4000>;
 	clocks = <&clks IMX5_CLK_PWM1_IPG_GATE>,
-- 
2.1.4

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

* [PATCH v5 11/11] pwm: imx: Add polarity inversion support to i.MX's PWMv2
  2017-01-29 21:54 [PATCH v5 00/11] pwm: imx: Provide atomic operation for IMX PWM driver Lukasz Majewski
                   ` (9 preceding siblings ...)
  2017-01-29 21:54 ` [PATCH v5 10/11] pwm: imx: doc: Update imx-pwm.txt documentation entry Lukasz Majewski
@ 2017-01-29 21:54 ` Lukasz Majewski
  10 siblings, 0 replies; 21+ messages in thread
From: Lukasz Majewski @ 2017-01-29 21:54 UTC (permalink / raw)
  To: Thierry Reding, Sascha Hauer, Stefan Agner, Boris Brezillon,
	linux-pwm, Bhuvanchandra DV, linux-kernel
  Cc: Lothar Wassmann, kernel, Fabio Estevam, Lukasz Majewski, Lukasz Majewski

With this patch the polarity settings for i.MX's PWMv2 is now supported
on top of atomic PWM setting

Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
---
Changes for v5:
- Adjust to corrected ->apply_v2() code

Changes for v4:
- MX3_PWMCR_POUTC setting adjustements after apply_v2() previous changes

Changes for v3:
- None

Changes for v2:
- New patch
---
 drivers/pwm/pwm-imx.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index e037023..9f7e787 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -38,6 +38,7 @@
 #define MX3_PWMCR_DOZEEN		(1 << 24)
 #define MX3_PWMCR_WAITEN		(1 << 23)
 #define MX3_PWMCR_DBGEN			(1 << 22)
+#define MX3_PWMCR_POUTC			(1 << 18)
 #define MX3_PWMCR_CLKSRC_IPG_HIGH	(2 << 16)
 #define MX3_PWMCR_CLKSRC_IPG		(1 << 16)
 #define MX3_PWMCR_SWR			(1 << 3)
@@ -164,6 +165,7 @@ static int imx_pwm_apply_v2(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct pwm_state cstate;
 	unsigned long long c;
 	int ret;
+	u32 cr;
 
 	pwm_get_state(pwm, &cstate);
 
@@ -208,12 +210,15 @@ static int imx_pwm_apply_v2(struct pwm_chip *chip, struct pwm_device *pwm,
 		writel(duty_cycles, imx->mmio_base + MX3_PWMSAR);
 		writel(period_cycles, imx->mmio_base + MX3_PWMPR);
 
-		writel(MX3_PWMCR_PRESCALER(prescale) |
-		       MX3_PWMCR_DOZEEN | MX3_PWMCR_WAITEN |
-		       MX3_PWMCR_DBGEN | MX3_PWMCR_CLKSRC_IPG_HIGH |
-		       MX3_PWMCR_EN,
-		       imx->mmio_base + MX3_PWMCR);
+		cr = MX3_PWMCR_PRESCALER(prescale) |
+			MX3_PWMCR_DOZEEN | MX3_PWMCR_WAITEN |
+			MX3_PWMCR_DBGEN | MX3_PWMCR_CLKSRC_IPG_HIGH |
+			MX3_PWMCR_EN;
+
+		if (state->polarity == PWM_POLARITY_INVERSED)
+			cr |= MX3_PWMCR_POUTC;
 
+		writel(cr, imx->mmio_base + MX3_PWMCR);
 	} else if (cstate.enabled) {
 		writel(0, imx->mmio_base + MX3_PWMCR);
 
@@ -236,6 +241,7 @@ static struct pwm_ops imx_pwm_ops_v2 = {
 };
 
 struct imx_pwm_data {
+	bool polarity_supported;
 	struct pwm_ops *pwm_ops;
 };
 
@@ -244,6 +250,7 @@ static struct imx_pwm_data imx_pwm_data_v1 = {
 };
 
 static struct imx_pwm_data imx_pwm_data_v2 = {
+	.polarity_supported = true,
 	.pwm_ops = &imx_pwm_ops_v2,
 };
 
@@ -284,6 +291,11 @@ static int imx_pwm_probe(struct platform_device *pdev)
 	imx->chip.base = -1;
 	imx->chip.npwm = 1;
 	imx->chip.can_sleep = true;
+	if (data->polarity_supported) {
+		dev_dbg(&pdev->dev, "PWM supports output inversion\n");
+		imx->chip.of_xlate = of_pwm_xlate_with_flags;
+		imx->chip.of_pwm_n_cells = 3;
+	}
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	imx->mmio_base = devm_ioremap_resource(&pdev->dev, r);
-- 
2.1.4

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

* Re: [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2
  2017-01-29 21:54 ` [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2 Lukasz Majewski
@ 2017-01-30  7:23   ` Thierry Reding
  2017-01-30  7:43     ` Thierry Reding
  0 siblings, 1 reply; 21+ messages in thread
From: Thierry Reding @ 2017-01-30  7:23 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Sascha Hauer, Stefan Agner, Boris Brezillon, linux-pwm,
	Bhuvanchandra DV, linux-kernel, Lothar Wassmann, kernel,
	Fabio Estevam, Lukasz Majewski

[-- Attachment #1: Type: text/plain, Size: 1493 bytes --]

On Sun, Jan 29, 2017 at 10:54:07PM +0100, Lukasz Majewski wrote:
> From: Lukasz Majewski <l.majewski@majess.pl>
> 
> This patch provides separate set of pwm ops utilized by
> i.MX's PWMv1 and PWMv2.
> 
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> Acked-by: Shawn Guo <shawn.guo@linaro.org>
> Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> Changes for v5:
> - None
> 
> Changes for v4:
> - None
> 
> Changes for v3:
> - Adjust the code to work with ipg clock removed
> 
> Changes for v2:
> - New patch
> ---
>  drivers/pwm/pwm-imx.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> index b1d1e50..0fa480d 100644
> --- a/drivers/pwm/pwm-imx.c
> +++ b/drivers/pwm/pwm-imx.c
> @@ -239,7 +239,14 @@ static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
>  	clk_disable_unprepare(imx->clk_per);
>  }
>  
> -static struct pwm_ops imx_pwm_ops = {
> +static struct pwm_ops imx_pwm_ops_v1 = {
> +	.enable = imx_pwm_enable,
> +	.disable = imx_pwm_disable,
> +	.config = imx_pwm_config,
> +	.owner = THIS_MODULE,
> +};
> +
> +static struct pwm_ops imx_pwm_ops_v2 = {

Can't these two be const? No need to respin for only this, just let me
know and I can make the change while applying.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v5 05/11] pwm: imx: Move PWMv2 software reset code to a separate function
  2017-01-29 21:54 ` [PATCH v5 05/11] pwm: imx: Move PWMv2 software reset code to a separate function Lukasz Majewski
@ 2017-01-30  7:26   ` Thierry Reding
  0 siblings, 0 replies; 21+ messages in thread
From: Thierry Reding @ 2017-01-30  7:26 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Sascha Hauer, Stefan Agner, Boris Brezillon, linux-pwm,
	Bhuvanchandra DV, linux-kernel, Lothar Wassmann, kernel,
	Fabio Estevam, Lukasz Majewski

[-- Attachment #1: Type: text/plain, Size: 1669 bytes --]

On Sun, Jan 29, 2017 at 10:54:09PM +0100, Lukasz Majewski wrote:
> From: Lukasz Majewski <l.majewski@majess.pl>
> 
> The software reset code has been extracted from imx_pwm_config_v2 function
> and moved to new one - imx_pwm_sw_reset().
> 
> This change reduces the overall size of imx_pwm_config_v2() and prepares
> it for atomic PWM operation.
> 
> Suggested-by: Stefan Agner <stefan@agner.ch>
> Suggested-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> 
> ---
> Changes for v5:
> - None
> 
> Changes for v4:
> - None
> 
> Changes for v3:
> - None
> 
> Changes for v2:
> - Add missing parenthesis
> ---
>  drivers/pwm/pwm-imx.c | 31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> index 11e3f3e..f0d78f3 100644
> --- a/drivers/pwm/pwm-imx.c
> +++ b/drivers/pwm/pwm-imx.c
> @@ -120,6 +120,25 @@ static void imx_pwm_disable_v1(struct pwm_chip *chip, struct pwm_device *pwm)
>  	clk_disable_unprepare(imx->clk_per);
>  }
>  
> +static void imx_pwm_sw_reset(struct pwm_chip *chip)
> +{
> +	struct imx_chip *imx = to_imx_chip(chip);
> +	struct device *dev = chip->dev;
> +	int wait_count = 0;
> +	u32 cr;
> +
> +	writel(MX3_PWMCR_SWR, imx->mmio_base + MX3_PWMCR);
> +	do {
> +		usleep_range(200, 1000);
> +		cr = readl(imx->mmio_base + MX3_PWMCR);
> +	} while ((cr & MX3_PWMCR_SWR) &&
> +		 (wait_count++ < MX3_PWM_SWR_LOOP));

I think you could replace this by one of the accessors from
linux/iopoll.h, but that can be a separate patch.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2
  2017-01-30  7:23   ` Thierry Reding
@ 2017-01-30  7:43     ` Thierry Reding
  2017-01-30  8:36       ` Lukasz Majewski
  0 siblings, 1 reply; 21+ messages in thread
From: Thierry Reding @ 2017-01-30  7:43 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Sascha Hauer, Stefan Agner, Boris Brezillon, linux-pwm,
	Bhuvanchandra DV, linux-kernel, Lothar Wassmann, kernel,
	Fabio Estevam, Lukasz Majewski

[-- Attachment #1: Type: text/plain, Size: 2034 bytes --]

On Mon, Jan 30, 2017 at 08:23:12AM +0100, Thierry Reding wrote:
> On Sun, Jan 29, 2017 at 10:54:07PM +0100, Lukasz Majewski wrote:
> > From: Lukasz Majewski <l.majewski@majess.pl>
> > 
> > This patch provides separate set of pwm ops utilized by
> > i.MX's PWMv1 and PWMv2.
> > 
> > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> > Acked-by: Shawn Guo <shawn.guo@linaro.org>
> > Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> > Changes for v5:
> > - None
> > 
> > Changes for v4:
> > - None
> > 
> > Changes for v3:
> > - Adjust the code to work with ipg clock removed
> > 
> > Changes for v2:
> > - New patch
> > ---
> >  drivers/pwm/pwm-imx.c | 17 ++++++++++++++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> > index b1d1e50..0fa480d 100644
> > --- a/drivers/pwm/pwm-imx.c
> > +++ b/drivers/pwm/pwm-imx.c
> > @@ -239,7 +239,14 @@ static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> >  	clk_disable_unprepare(imx->clk_per);
> >  }
> >  
> > -static struct pwm_ops imx_pwm_ops = {
> > +static struct pwm_ops imx_pwm_ops_v1 = {
> > +	.enable = imx_pwm_enable,
> > +	.disable = imx_pwm_disable,
> > +	.config = imx_pwm_config,
> > +	.owner = THIS_MODULE,
> > +};
> > +
> > +static struct pwm_ops imx_pwm_ops_v2 = {
> 
> Can't these two be const? No need to respin for only this, just let me
> know and I can make the change while applying.

Nevermind that. I just remembered that I had picked up a patch to make
the original imx_pwm_ops a const and things still work fine if I make
both of the above const, so I just had to manually apply your patch, but
other than that it seems fine. Let me apply the rest of this set and
push out. It'd be great if you could check afterwards that it's all
still what you expect.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v5 07/11] pwm: imx: Provide atomic PWM support for i.MX PWMv2
  2017-01-29 21:54 ` [PATCH v5 07/11] pwm: imx: Provide atomic PWM support for i.MX PWMv2 Lukasz Majewski
@ 2017-01-30  7:49   ` Boris Brezillon
  2017-01-30  7:54     ` Thierry Reding
  0 siblings, 1 reply; 21+ messages in thread
From: Boris Brezillon @ 2017-01-30  7:49 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Thierry Reding, Sascha Hauer, Stefan Agner, linux-pwm,
	Bhuvanchandra DV, linux-kernel, Lothar Wassmann, kernel,
	Fabio Estevam, Lukasz Majewski

On Sun, 29 Jan 2017 22:54:11 +0100
Lukasz Majewski <lukma@denx.de> wrote:

> This commit provides apply() callback implementation for i.MX's PWMv2.
> 
> Suggested-by: Stefan Agner <stefan@agner.ch>
> Suggested-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> Changes for v5:
> - Modify ->apply() function to avoid unbalanced clock enabling/disabling
> - Fix preventing iMX7 from hanging
> 
> Changes for v4:
> - Avoid recalculation of PWM parameters when disabling PWM signal
> - Unconditionally call clk_prepare_enable(imx->clk_per) and
>   clk_disable_unprepare(imx->clk_per)
> 
> Changes for v3:
> - Remove ipg clock enable/disable functions
> 
> Changes for v2:
> - None
> ---
>  drivers/pwm/pwm-imx.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> index 60cdc5c..fdaa11b 100644
> --- a/drivers/pwm/pwm-imx.c
> +++ b/drivers/pwm/pwm-imx.c
> @@ -249,6 +249,73 @@ static int imx_pwm_config(struct pwm_chip *chip,
>  	return ret;
>  }
>  
> +static int imx_pwm_apply_v2(struct pwm_chip *chip, struct pwm_device *pwm,
> +			    struct pwm_state *state)
> +{
> +	unsigned long period_cycles, duty_cycles, prescale;
> +	struct imx_chip *imx = to_imx_chip(chip);
> +	struct pwm_state cstate;
> +	unsigned long long c;
> +	int ret;
> +
> +	pwm_get_state(pwm, &cstate);
> +
> +	if (state->enabled) {
> +		c = clk_get_rate(imx->clk_per);
> +		c *= state->period;
> +
> +		do_div(c, 1000000000);
> +		period_cycles = c;
> +
> +		prescale = period_cycles / 0x10000 + 1;
> +
> +		period_cycles /= prescale;
> +		c = (unsigned long long)period_cycles * state->duty_cycle;
> +		do_div(c, state->period);
> +		duty_cycles = c;
> +
> +		/*
> +		 * according to imx pwm RM, the real period value should be
> +		 * PERIOD value in PWMPR plus 2.
> +		 */
> +		if (period_cycles > 2)
> +			period_cycles -= 2;
> +		else
> +			period_cycles = 0;
> +
> +		/*
> +		 * Wait for a free FIFO slot if the PWM is already enabled, and
> +		 * flush the FIFO if the PWM was disabled and is about to be
> +		 * enabled.
> +		 */
> +		if (cstate.enabled) {
> +			imx_pwm_wait_fifo_slot(chip, pwm);
> +		} else if (state->enabled) {

Should just be

		} else {

since we're already in the 'if (state->enabled)' block (see above).

I see that Thierry already applied the series, so, just for the record,
with this fixed, the whole series is

Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>

Thanks,

Boris

> +			ret = clk_prepare_enable(imx->clk_per);
> +			if (ret)
> +				return ret;
> +
> +			imx_pwm_sw_reset(chip);
> +		}
> +
> +		writel(duty_cycles, imx->mmio_base + MX3_PWMSAR);
> +		writel(period_cycles, imx->mmio_base + MX3_PWMPR);
> +
> +		writel(MX3_PWMCR_PRESCALER(prescale) |
> +		       MX3_PWMCR_DOZEEN | MX3_PWMCR_WAITEN |
> +		       MX3_PWMCR_DBGEN | MX3_PWMCR_CLKSRC_IPG_HIGH |
> +		       MX3_PWMCR_EN,
> +		       imx->mmio_base + MX3_PWMCR);
> +
> +	} else if (cstate.enabled) {
> +		writel(0, imx->mmio_base + MX3_PWMCR);
> +
> +		clk_disable_unprepare(imx->clk_per);
> +	}
> +
> +	return 0;
> +}

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

* Re: [PATCH v5 07/11] pwm: imx: Provide atomic PWM support for i.MX PWMv2
  2017-01-30  7:49   ` Boris Brezillon
@ 2017-01-30  7:54     ` Thierry Reding
  0 siblings, 0 replies; 21+ messages in thread
From: Thierry Reding @ 2017-01-30  7:54 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Lukasz Majewski, Sascha Hauer, Stefan Agner, linux-pwm,
	Bhuvanchandra DV, linux-kernel, Lothar Wassmann, kernel,
	Fabio Estevam, Lukasz Majewski

[-- Attachment #1: Type: text/plain, Size: 3171 bytes --]

On Mon, Jan 30, 2017 at 08:49:14AM +0100, Boris Brezillon wrote:
> On Sun, 29 Jan 2017 22:54:11 +0100
> Lukasz Majewski <lukma@denx.de> wrote:
> 
> > This commit provides apply() callback implementation for i.MX's PWMv2.
> > 
> > Suggested-by: Stefan Agner <stefan@agner.ch>
> > Suggested-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> > Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > ---
> > Changes for v5:
> > - Modify ->apply() function to avoid unbalanced clock enabling/disabling
> > - Fix preventing iMX7 from hanging
> > 
> > Changes for v4:
> > - Avoid recalculation of PWM parameters when disabling PWM signal
> > - Unconditionally call clk_prepare_enable(imx->clk_per) and
> >   clk_disable_unprepare(imx->clk_per)
> > 
> > Changes for v3:
> > - Remove ipg clock enable/disable functions
> > 
> > Changes for v2:
> > - None
> > ---
> >  drivers/pwm/pwm-imx.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 68 insertions(+)
> > 
> > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> > index 60cdc5c..fdaa11b 100644
> > --- a/drivers/pwm/pwm-imx.c
> > +++ b/drivers/pwm/pwm-imx.c
> > @@ -249,6 +249,73 @@ static int imx_pwm_config(struct pwm_chip *chip,
> >  	return ret;
> >  }
> >  
> > +static int imx_pwm_apply_v2(struct pwm_chip *chip, struct pwm_device *pwm,
> > +			    struct pwm_state *state)
> > +{
> > +	unsigned long period_cycles, duty_cycles, prescale;
> > +	struct imx_chip *imx = to_imx_chip(chip);
> > +	struct pwm_state cstate;
> > +	unsigned long long c;
> > +	int ret;
> > +
> > +	pwm_get_state(pwm, &cstate);
> > +
> > +	if (state->enabled) {
> > +		c = clk_get_rate(imx->clk_per);
> > +		c *= state->period;
> > +
> > +		do_div(c, 1000000000);
> > +		period_cycles = c;
> > +
> > +		prescale = period_cycles / 0x10000 + 1;
> > +
> > +		period_cycles /= prescale;
> > +		c = (unsigned long long)period_cycles * state->duty_cycle;
> > +		do_div(c, state->period);
> > +		duty_cycles = c;
> > +
> > +		/*
> > +		 * according to imx pwm RM, the real period value should be
> > +		 * PERIOD value in PWMPR plus 2.
> > +		 */
> > +		if (period_cycles > 2)
> > +			period_cycles -= 2;
> > +		else
> > +			period_cycles = 0;
> > +
> > +		/*
> > +		 * Wait for a free FIFO slot if the PWM is already enabled, and
> > +		 * flush the FIFO if the PWM was disabled and is about to be
> > +		 * enabled.
> > +		 */
> > +		if (cstate.enabled) {
> > +			imx_pwm_wait_fifo_slot(chip, pwm);
> > +		} else if (state->enabled) {
> 
> Should just be
> 
> 		} else {
> 
> since we're already in the 'if (state->enabled)' block (see above).
> 
> I see that Thierry already applied the series, so, just for the record,
> with this fixed, the whole series is
> 
> Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>

I haven't gotten to that patch yet because I need to manually apply a
bunch of these because of the const pwm_ops patch. I'll apply the change
you mentioned and will add your R-b tag.

Thanks,
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2
  2017-01-30  7:43     ` Thierry Reding
@ 2017-01-30  8:36       ` Lukasz Majewski
  2017-01-30  8:45         ` Thierry Reding
  0 siblings, 1 reply; 21+ messages in thread
From: Lukasz Majewski @ 2017-01-30  8:36 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Sascha Hauer, Stefan Agner, Boris Brezillon, linux-pwm,
	Bhuvanchandra DV, linux-kernel, Lothar Wassmann, kernel,
	Fabio Estevam, Lukasz Majewski

[-- Attachment #1: Type: text/plain, Size: 2485 bytes --]

Hi Thierry,

> On Mon, Jan 30, 2017 at 08:23:12AM +0100, Thierry Reding wrote:
> > On Sun, Jan 29, 2017 at 10:54:07PM +0100, Lukasz Majewski wrote:
> > > From: Lukasz Majewski <l.majewski@majess.pl>
> > > 
> > > This patch provides separate set of pwm ops utilized by
> > > i.MX's PWMv1 and PWMv2.
> > > 
> > > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > > Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> > > Acked-by: Shawn Guo <shawn.guo@linaro.org>
> > > Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > ---
> > > Changes for v5:
> > > - None
> > > 
> > > Changes for v4:
> > > - None
> > > 
> > > Changes for v3:
> > > - Adjust the code to work with ipg clock removed
> > > 
> > > Changes for v2:
> > > - New patch
> > > ---
> > >  drivers/pwm/pwm-imx.c | 17 ++++++++++++++---
> > >  1 file changed, 14 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> > > index b1d1e50..0fa480d 100644
> > > --- a/drivers/pwm/pwm-imx.c
> > > +++ b/drivers/pwm/pwm-imx.c
> > > @@ -239,7 +239,14 @@ static void imx_pwm_disable(struct pwm_chip
> > > *chip, struct pwm_device *pwm)
> > > clk_disable_unprepare(imx->clk_per); }
> > >  
> > > -static struct pwm_ops imx_pwm_ops = {
> > > +static struct pwm_ops imx_pwm_ops_v1 = {
> > > +	.enable = imx_pwm_enable,
> > > +	.disable = imx_pwm_disable,
> > > +	.config = imx_pwm_config,
> > > +	.owner = THIS_MODULE,
> > > +};
> > > +
> > > +static struct pwm_ops imx_pwm_ops_v2 = {
> > 
> > Can't these two be const? No need to respin for only this, just let
> > me know and I can make the change while applying.
> 
> Nevermind that. I just remembered that I had picked up a patch to make
> the original imx_pwm_ops a const and things still work fine if I make
> both of the above const, so I just had to manually apply your patch,
> but other than that it seems fine. Let me apply the rest of this set
> and push out. It'd be great if you could check afterwards that it's
> all still what you expect.

I will do that. Thanks for integrating the patch series :-).

> 
> Thierry


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2
  2017-01-30  8:36       ` Lukasz Majewski
@ 2017-01-30  8:45         ` Thierry Reding
  2017-01-30  8:55           ` Lukasz Majewski
  0 siblings, 1 reply; 21+ messages in thread
From: Thierry Reding @ 2017-01-30  8:45 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Sascha Hauer, Stefan Agner, Boris Brezillon, linux-pwm,
	Bhuvanchandra DV, linux-kernel, Lothar Wassmann, kernel,
	Fabio Estevam, Lukasz Majewski

[-- Attachment #1: Type: text/plain, Size: 2858 bytes --]

On Mon, Jan 30, 2017 at 09:36:49AM +0100, Lukasz Majewski wrote:
> Hi Thierry,
> 
> > On Mon, Jan 30, 2017 at 08:23:12AM +0100, Thierry Reding wrote:
> > > On Sun, Jan 29, 2017 at 10:54:07PM +0100, Lukasz Majewski wrote:
> > > > From: Lukasz Majewski <l.majewski@majess.pl>
> > > > 
> > > > This patch provides separate set of pwm ops utilized by
> > > > i.MX's PWMv1 and PWMv2.
> > > > 
> > > > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > > > Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> > > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> > > > Acked-by: Shawn Guo <shawn.guo@linaro.org>
> > > > Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > ---
> > > > Changes for v5:
> > > > - None
> > > > 
> > > > Changes for v4:
> > > > - None
> > > > 
> > > > Changes for v3:
> > > > - Adjust the code to work with ipg clock removed
> > > > 
> > > > Changes for v2:
> > > > - New patch
> > > > ---
> > > >  drivers/pwm/pwm-imx.c | 17 ++++++++++++++---
> > > >  1 file changed, 14 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> > > > index b1d1e50..0fa480d 100644
> > > > --- a/drivers/pwm/pwm-imx.c
> > > > +++ b/drivers/pwm/pwm-imx.c
> > > > @@ -239,7 +239,14 @@ static void imx_pwm_disable(struct pwm_chip
> > > > *chip, struct pwm_device *pwm)
> > > > clk_disable_unprepare(imx->clk_per); }
> > > >  
> > > > -static struct pwm_ops imx_pwm_ops = {
> > > > +static struct pwm_ops imx_pwm_ops_v1 = {
> > > > +	.enable = imx_pwm_enable,
> > > > +	.disable = imx_pwm_disable,
> > > > +	.config = imx_pwm_config,
> > > > +	.owner = THIS_MODULE,
> > > > +};
> > > > +
> > > > +static struct pwm_ops imx_pwm_ops_v2 = {
> > > 
> > > Can't these two be const? No need to respin for only this, just let
> > > me know and I can make the change while applying.
> > 
> > Nevermind that. I just remembered that I had picked up a patch to make
> > the original imx_pwm_ops a const and things still work fine if I make
> > both of the above const, so I just had to manually apply your patch,
> > but other than that it seems fine. Let me apply the rest of this set
> > and push out. It'd be great if you could check afterwards that it's
> > all still what you expect.
> 
> I will do that. Thanks for integrating the patch series :-).

Thanks for sticking with it. I know the initial patches for optional
polarity support have been around for years, and it took a really long
time for this all to come together.

But I think the end result is sound and looks really good.

The one remaining bit that I'm not 100% happy about is that the v1
support is not atomic while the v2 support is. Not a blocker, but it
looks as if it should be easy to convert over v1 as well. Any takers?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2
  2017-01-30  8:45         ` Thierry Reding
@ 2017-01-30  8:55           ` Lukasz Majewski
  2017-01-30  9:04             ` Thierry Reding
  0 siblings, 1 reply; 21+ messages in thread
From: Lukasz Majewski @ 2017-01-30  8:55 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Sascha Hauer, Stefan Agner, Boris Brezillon, linux-pwm,
	Bhuvanchandra DV, linux-kernel, Lothar Wassmann, kernel,
	Fabio Estevam, Lukasz Majewski

[-- Attachment #1: Type: text/plain, Size: 3356 bytes --]

Hi Thierry,

> On Mon, Jan 30, 2017 at 09:36:49AM +0100, Lukasz Majewski wrote:
> > Hi Thierry,
> > 
> > > On Mon, Jan 30, 2017 at 08:23:12AM +0100, Thierry Reding wrote:
> > > > On Sun, Jan 29, 2017 at 10:54:07PM +0100, Lukasz Majewski wrote:
> > > > > From: Lukasz Majewski <l.majewski@majess.pl>
> > > > > 
> > > > > This patch provides separate set of pwm ops utilized by
> > > > > i.MX's PWMv1 and PWMv2.
> > > > > 
> > > > > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > > > > Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> > > > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> > > > > Acked-by: Shawn Guo <shawn.guo@linaro.org>
> > > > > Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > > ---
> > > > > Changes for v5:
> > > > > - None
> > > > > 
> > > > > Changes for v4:
> > > > > - None
> > > > > 
> > > > > Changes for v3:
> > > > > - Adjust the code to work with ipg clock removed
> > > > > 
> > > > > Changes for v2:
> > > > > - New patch
> > > > > ---
> > > > >  drivers/pwm/pwm-imx.c | 17 ++++++++++++++---
> > > > >  1 file changed, 14 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> > > > > index b1d1e50..0fa480d 100644
> > > > > --- a/drivers/pwm/pwm-imx.c
> > > > > +++ b/drivers/pwm/pwm-imx.c
> > > > > @@ -239,7 +239,14 @@ static void imx_pwm_disable(struct
> > > > > pwm_chip *chip, struct pwm_device *pwm)
> > > > > clk_disable_unprepare(imx->clk_per); }
> > > > >  
> > > > > -static struct pwm_ops imx_pwm_ops = {
> > > > > +static struct pwm_ops imx_pwm_ops_v1 = {
> > > > > +	.enable = imx_pwm_enable,
> > > > > +	.disable = imx_pwm_disable,
> > > > > +	.config = imx_pwm_config,
> > > > > +	.owner = THIS_MODULE,
> > > > > +};
> > > > > +
> > > > > +static struct pwm_ops imx_pwm_ops_v2 = {
> > > > 
> > > > Can't these two be const? No need to respin for only this, just
> > > > let me know and I can make the change while applying.
> > > 
> > > Nevermind that. I just remembered that I had picked up a patch to
> > > make the original imx_pwm_ops a const and things still work fine
> > > if I make both of the above const, so I just had to manually
> > > apply your patch, but other than that it seems fine. Let me apply
> > > the rest of this set and push out. It'd be great if you could
> > > check afterwards that it's all still what you expect.
> > 
> > I will do that. Thanks for integrating the patch series :-).
> 
> Thanks for sticking with it. I know the initial patches for optional
> polarity support have been around for years, and it took a really long
> time for this all to come together.
> 
> But I think the end result is sound and looks really good.

You are welcome :-)

> 
> The one remaining bit that I'm not 100% happy about is that the v1
> support is not atomic while the v2 support is.

Here the only limitation is the lack of v1 HW.

> Not a blocker, but it
> looks as if it should be easy to convert over v1 as well. Any takers?
> 
> Thierry


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2
  2017-01-30  8:55           ` Lukasz Majewski
@ 2017-01-30  9:04             ` Thierry Reding
  0 siblings, 0 replies; 21+ messages in thread
From: Thierry Reding @ 2017-01-30  9:04 UTC (permalink / raw)
  To: Lukasz Majewski, Sascha Hauer, Lothar Wassmann, Fabio Estevam, Shawn Guo
  Cc: Stefan Agner, Boris Brezillon, linux-pwm, Bhuvanchandra DV,
	linux-kernel, kernel, Lukasz Majewski

[-- Attachment #1: Type: text/plain, Size: 3567 bytes --]

On Mon, Jan 30, 2017 at 09:55:04AM +0100, Lukasz Majewski wrote:
> Hi Thierry,
> 
> > On Mon, Jan 30, 2017 at 09:36:49AM +0100, Lukasz Majewski wrote:
> > > Hi Thierry,
> > > 
> > > > On Mon, Jan 30, 2017 at 08:23:12AM +0100, Thierry Reding wrote:
> > > > > On Sun, Jan 29, 2017 at 10:54:07PM +0100, Lukasz Majewski wrote:
> > > > > > From: Lukasz Majewski <l.majewski@majess.pl>
> > > > > > 
> > > > > > This patch provides separate set of pwm ops utilized by
> > > > > > i.MX's PWMv1 and PWMv2.
> > > > > > 
> > > > > > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > > > > > Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> > > > > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> > > > > > Acked-by: Shawn Guo <shawn.guo@linaro.org>
> > > > > > Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > > > ---
> > > > > > Changes for v5:
> > > > > > - None
> > > > > > 
> > > > > > Changes for v4:
> > > > > > - None
> > > > > > 
> > > > > > Changes for v3:
> > > > > > - Adjust the code to work with ipg clock removed
> > > > > > 
> > > > > > Changes for v2:
> > > > > > - New patch
> > > > > > ---
> > > > > >  drivers/pwm/pwm-imx.c | 17 ++++++++++++++---
> > > > > >  1 file changed, 14 insertions(+), 3 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> > > > > > index b1d1e50..0fa480d 100644
> > > > > > --- a/drivers/pwm/pwm-imx.c
> > > > > > +++ b/drivers/pwm/pwm-imx.c
> > > > > > @@ -239,7 +239,14 @@ static void imx_pwm_disable(struct
> > > > > > pwm_chip *chip, struct pwm_device *pwm)
> > > > > > clk_disable_unprepare(imx->clk_per); }
> > > > > >  
> > > > > > -static struct pwm_ops imx_pwm_ops = {
> > > > > > +static struct pwm_ops imx_pwm_ops_v1 = {
> > > > > > +	.enable = imx_pwm_enable,
> > > > > > +	.disable = imx_pwm_disable,
> > > > > > +	.config = imx_pwm_config,
> > > > > > +	.owner = THIS_MODULE,
> > > > > > +};
> > > > > > +
> > > > > > +static struct pwm_ops imx_pwm_ops_v2 = {
> > > > > 
> > > > > Can't these two be const? No need to respin for only this, just
> > > > > let me know and I can make the change while applying.
> > > > 
> > > > Nevermind that. I just remembered that I had picked up a patch to
> > > > make the original imx_pwm_ops a const and things still work fine
> > > > if I make both of the above const, so I just had to manually
> > > > apply your patch, but other than that it seems fine. Let me apply
> > > > the rest of this set and push out. It'd be great if you could
> > > > check afterwards that it's all still what you expect.
> > > 
> > > I will do that. Thanks for integrating the patch series :-).
> > 
> > Thanks for sticking with it. I know the initial patches for optional
> > polarity support have been around for years, and it took a really long
> > time for this all to come together.
> > 
> > But I think the end result is sound and looks really good.
> 
> You are welcome :-)
> 
> > 
> > The one remaining bit that I'm not 100% happy about is that the v1
> > support is not atomic while the v2 support is.
> 
> Here the only limitation is the lack of v1 HW.

That doesn't have to be a blocker. If you're willing to invest some more
work to do the additional conversion (I think it would be a fairly minor
change, looking at the existing v1 code), I'm sure we can find someone
with the hardware to test it.

Sascha, Lothar, Fabio, Shawn: do you guys have access to v1 hardware, or
know of anyone who might?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

end of thread, other threads:[~2017-01-30  9:04 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-29 21:54 [PATCH v5 00/11] pwm: imx: Provide atomic operation for IMX PWM driver Lukasz Majewski
2017-01-29 21:54 ` [PATCH v5 01/11] pwm: print error messages with pr_err() instead of pr_debug() Lukasz Majewski
2017-01-29 21:54 ` [PATCH v5 02/11] pwm: imx: remove ipg clock and enable per clock when required Lukasz Majewski
2017-01-29 21:54 ` [PATCH v5 03/11] pwm: imx: Add separate set of pwm ops for PWMv1 and PWMv2 Lukasz Majewski
2017-01-30  7:23   ` Thierry Reding
2017-01-30  7:43     ` Thierry Reding
2017-01-30  8:36       ` Lukasz Majewski
2017-01-30  8:45         ` Thierry Reding
2017-01-30  8:55           ` Lukasz Majewski
2017-01-30  9:04             ` Thierry Reding
2017-01-29 21:54 ` [PATCH v5 04/11] pwm: imx: Rewrite imx_pwm_*_v1 code to facilitate switch to atomic pwm operation Lukasz Majewski
2017-01-29 21:54 ` [PATCH v5 05/11] pwm: imx: Move PWMv2 software reset code to a separate function Lukasz Majewski
2017-01-30  7:26   ` Thierry Reding
2017-01-29 21:54 ` [PATCH v5 06/11] pwm: imx: Move PWMv2 wait for fifo slot " Lukasz Majewski
2017-01-29 21:54 ` [PATCH v5 07/11] pwm: imx: Provide atomic PWM support for i.MX PWMv2 Lukasz Majewski
2017-01-30  7:49   ` Boris Brezillon
2017-01-30  7:54     ` Thierry Reding
2017-01-29 21:54 ` [PATCH v5 08/11] pwm: imx: Remove redundant i.MX PWMv2 code Lukasz Majewski
2017-01-29 21:54 ` [PATCH v5 09/11] pwm: core: make the PWM_POLARITY flag in DTB optional Lukasz Majewski
2017-01-29 21:54 ` [PATCH v5 10/11] pwm: imx: doc: Update imx-pwm.txt documentation entry Lukasz Majewski
2017-01-29 21:54 ` [PATCH v5 11/11] pwm: imx: Add polarity inversion support to i.MX's PWMv2 Lukasz Majewski

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