i.MX pwm patches

i.MX pwm patches

[Sascha Hauer]

Hi All,

The following patches are an overdue work on the i.MX pwm driver.

- introduce SoC specific functions to make the driver easier to maintain
- use peripheral clock for pwm output unconditionally
- separate the two clocks this module has
- enable ipg clock for register accesses, peripheral clock for enabling
  the pwm
- make the driver safe for calling pwm_config before pwm_enable
- Add devicetree support for i.MX53

The platform device support is still implemented using cpu_is_*, I think
this can be dropped completely soon as this driver has no in kernel users
currently, so there should be no need to keep compatibility for platform
based boards.

Hopefully this series fixes the issues mentioned by Benoît. It has been
tested on an i.MX53 only.

Sascha

----------------------------------------------------------------
Philipp Zabel (3):
      pwm i.MX: add devicetree support
      pwm i.MX: fix clock lookup
      pwm i.MX: add devicetree support

Sascha Hauer (6):
      pwm i.MX: factor out SoC specific functions
      pwm i.MX: remove unnecessary if in pwm_[en|dis]able
      pwm i.MX: add functions to enable/disable pwm.
      pwm i.MX: Use module_platform_driver
      pwm i.MX: use per clock unconditionally
      ARM i.MX53: Add pwms to dtsi

 arch/arm/boot/dts/imx53.dtsi        |   14 ++
 arch/arm/mach-imx/clk-imx51-imx53.c |    4 +
 drivers/pwm/pwm-imx.c               |  278 +++++++++++++++++++++++------------
 3 files changed, 202 insertions(+), 94 deletions(-)

[PATCH 1/9] pwm i.MX: factor out SoC specific functions

[Sascha Hauer]

To make the code more flexible.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/pwm/pwm-imx.c |  145 ++++++++++++++++++++++++++++---------------------
 1 file changed, 82 insertions(+), 63 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index 2a0b353..38270f4 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -46,81 +46,95 @@ 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);
 };
 
 #define to_imx_chip(chip)	container_of(chip, struct imx_chip, chip)
 
-static int imx_pwm_config(struct pwm_chip *chip,
+static int imx_pwm_config_v1(struct pwm_chip *chip,
 		struct pwm_device *pwm, int duty_ns, int period_ns)
 {
 	struct imx_chip *imx = to_imx_chip(chip);
 
-	if (!(cpu_is_mx1() || cpu_is_mx21())) {
-		unsigned long long c;
-		unsigned long period_cycles, duty_cycles, prescale;
-		u32 cr;
-
-		c = clk_get_rate(imx->clk);
-		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_EN;
-
-		if (cpu_is_mx25())
-			cr |= MX3_PWMCR_CLKSRC_IPG;
-		else
-			cr |= MX3_PWMCR_CLKSRC_IPG_HIGH;
-
-		writel(cr, imx->mmio_base + MX3_PWMCR);
-	} else if (cpu_is_mx1() || cpu_is_mx21()) {
-		/* The PWM subsystem allows for exact frequencies. However,
-		 * I cannot connect a scope on my device to the PWM line and
-		 * thus cannot provide the program the PWM controller
-		 * exactly. Instead, I'm relying on the fact that the
-		 * Bootloader (u-boot or WinCE+haret) has programmed the PWM
-		 * function group already. So I'll just modify the PWM sample
-		 * register to follow the ratio of duty_ns vs. period_ns
-		 * accordingly.
-		 *
-		 * This is good enough for programming the brightness of
-		 * the LCD backlight.
-		 *
-		 * The real implementation would divide PERCLK[0] first by
-		 * both the prescaler (/1 .. /128) and then by CLKSEL
-		 * (/2 .. /16).
-		 */
-		u32 max = readl(imx->mmio_base + MX1_PWMP);
-		u32 p = max * duty_ns / period_ns;
-		writel(max - p, imx->mmio_base + MX1_PWMS);
-	} else {
-		BUG();
-	}
+	/* The PWM subsystem allows for exact frequencies. However,
+	 * I cannot connect a scope on my device to the PWM line and
+	 * thus cannot provide the program the PWM controller
+	 * exactly. Instead, I'm relying on the fact that the
+	 * Bootloader (u-boot or WinCE+haret) has programmed the PWM
+	 * function group already. So I'll just modify the PWM sample
+	 * register to follow the ratio of duty_ns vs. period_ns
+	 * accordingly.
+	 *
+	 * This is good enough for programming the brightness of
+	 * the LCD backlight.
+	 *
+	 * The real implementation would divide PERCLK[0] first by
+	 * both the prescaler (/1 .. /128) and then by CLKSEL
+	 * (/2 .. /16).
+	 */
+	u32 max = readl(imx->mmio_base + MX1_PWMP);
+	u32 p = max * duty_ns / period_ns;
+	writel(max - p, imx->mmio_base + MX1_PWMS);
+
+	return 0;
+}
+
+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;
+	u32 cr;
+
+	c = clk_get_rate(imx->clk);
+	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_EN;
+
+	if (cpu_is_mx25())
+		cr |= MX3_PWMCR_CLKSRC_IPG;
+	else
+		cr |= MX3_PWMCR_CLKSRC_IPG_HIGH;
+
+	writel(cr, imx->mmio_base + MX3_PWMCR);
 
 	return 0;
 }
 
+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);
+
+	return imx->config(chip, pwm, duty_ns, period_ns);
+}
+
 static int imx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct imx_chip *imx = to_imx_chip(chip);
@@ -187,6 +201,11 @@ static int __devinit imx_pwm_probe(struct platform_device *pdev)
 	if (imx->mmio_base == NULL)
 		return -EADDRNOTAVAIL;
 
+	if (cpu_is_mx1() || cpu_is_mx21())
+		imx->config = imx_pwm_config_v1;
+	else
+		imx->config = imx_pwm_config_v2;
+
 	ret = pwmchip_add(&imx->chip);
 	if (ret < 0)
 		return ret;
-- 
1.7.10.4

[PATCH 2/9] pwm i.MX: remove unnecessary if in pwm_[en|dis]able

[Sascha Hauer]

The pwm core makes sure that pwm_enable/disable are called only
once. Still keep the enabled state since we will need it in
pwm_config.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/pwm/pwm-imx.c |   25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index 38270f4..0f244e6 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -42,7 +42,7 @@
 struct imx_chip {
 	struct clk	*clk;
 
-	int		clk_enabled;
+	int		enabled;
 	void __iomem	*mmio_base;
 
 	struct pwm_chip	chip;
@@ -138,14 +138,15 @@ static int imx_pwm_config(struct pwm_chip *chip,
 static int imx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct imx_chip *imx = to_imx_chip(chip);
-	int rc = 0;
+	int rc;
 
-	if (!imx->clk_enabled) {
-		rc = clk_prepare_enable(imx->clk);
-		if (!rc)
-			imx->clk_enabled = 1;
-	}
-	return rc;
+	rc = clk_prepare_enable(imx->clk);
+	if (rc)
+		return rc;
+
+	imx->enabled = 1;
+
+	return 0;
 }
 
 static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
@@ -154,10 +155,8 @@ static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 
 	writel(0, imx->mmio_base + MX3_PWMCR);
 
-	if (imx->clk_enabled) {
-		clk_disable_unprepare(imx->clk);
-		imx->clk_enabled = 0;
-	}
+	clk_disable_unprepare(imx->clk);
+	imx->enabled = 0;
 }
 
 static struct pwm_ops imx_pwm_ops = {
@@ -189,8 +188,6 @@ static int __devinit imx_pwm_probe(struct platform_device *pdev)
 	imx->chip.base = -1;
 	imx->chip.npwm = 1;
 
-	imx->clk_enabled = 0;
-
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (r == NULL) {
 		dev_err(&pdev->dev, "no memory resource defined\n");
-- 
1.7.10.4

[PATCH 3/9] pwm i.MX: add functions to enable/disable pwm.

[Sascha Hauer]

We used to enable/disable the pwm only by switching the
clock on or off. Instead, use the dedicated register bits.
These differ on different SoCs, so introduce a SoC specific
function for this.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/pwm/pwm-imx.c |   48 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index 0f244e6..3fc4ae8 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -25,6 +25,7 @@
 #define MX1_PWMS    0x04   /* PWM Sample Register */
 #define MX1_PWMP    0x08   /* PWM Period Register */
 
+#define MX1_PWMC_EN		(1 << 4)
 
 /* i.MX27, i.MX31, i.MX35 share the same PWM function block: */
 
@@ -49,6 +50,7 @@ struct imx_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)
@@ -81,6 +83,21 @@ 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)
+{
+	struct imx_chip *imx = to_imx_chip(chip);
+	u32 val;
+
+	val = readl(imx->mmio_base + MX1_PWMC);
+
+	if (enable)
+		val |= MX1_PWMC_EN;
+	else
+		val &= ~MX1_PWMC_EN;
+
+	writel(val, imx->mmio_base + MX1_PWMC);
+}
+
 static int imx_pwm_config_v2(struct pwm_chip *chip,
 		struct pwm_device *pwm, int duty_ns, int period_ns)
 {
@@ -115,7 +132,10 @@ static int imx_pwm_config_v2(struct pwm_chip *chip,
 
 	cr = MX3_PWMCR_PRESCALER(prescale) |
 		MX3_PWMCR_DOZEEN | MX3_PWMCR_WAITEN |
-		MX3_PWMCR_DBGEN | MX3_PWMCR_EN;
+		MX3_PWMCR_DBGEN;
+
+	if (imx->enabled)
+		cr |= MX3_PWMCR_EN;
 
 	if (cpu_is_mx25())
 		cr |= MX3_PWMCR_CLKSRC_IPG;
@@ -127,6 +147,21 @@ static int imx_pwm_config_v2(struct pwm_chip *chip,
 	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)
 {
@@ -144,6 +179,8 @@ static int imx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 	if (rc)
 		return rc;
 
+	imx->set_enable(chip, true);
+
 	imx->enabled = 1;
 
 	return 0;
@@ -153,7 +190,7 @@ static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct imx_chip *imx = to_imx_chip(chip);
 
-	writel(0, imx->mmio_base + MX3_PWMCR);
+	imx->set_enable(chip, false);
 
 	clk_disable_unprepare(imx->clk);
 	imx->enabled = 0;
@@ -198,10 +235,13 @@ static int __devinit imx_pwm_probe(struct platform_device *pdev)
 	if (imx->mmio_base == NULL)
 		return -EADDRNOTAVAIL;
 
-	if (cpu_is_mx1() || cpu_is_mx21())
+	if (cpu_is_mx1() || cpu_is_mx21()) {
 		imx->config = imx_pwm_config_v1;
-	else
+		imx->set_enable = imx_pwm_set_enable_v1;
+	} else {
 		imx->config = imx_pwm_config_v2;
+		imx->set_enable = imx_pwm_set_enable_v2;
+	}
 
 	ret = pwmchip_add(&imx->chip);
 	if (ret < 0)
-- 
1.7.10.4

[PATCH 4/9] pwm i.MX: Use module_platform_driver

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/pwm/pwm-imx.c |   12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index 3fc4ae8..a689144 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -270,17 +270,7 @@ static struct platform_driver imx_pwm_driver = {
 	.remove		= __devexit_p(imx_pwm_remove),
 };
 
-static int __init imx_pwm_init(void)
-{
-	return platform_driver_register(&imx_pwm_driver);
-}
-arch_initcall(imx_pwm_init);
-
-static void __exit imx_pwm_exit(void)
-{
-	platform_driver_unregister(&imx_pwm_driver);
-}
-module_exit(imx_pwm_exit);
+module_platform_driver(imx_pwm_driver);
 
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
-- 
1.7.10.4

[PATCH 5/9] pwm i.MX: add devicetree support

[Sascha Hauer]

From: Philipp Zabel <p.zabel@pengutronix.de>

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/pwm/pwm-imx.c |   25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index a689144..dfc1bae 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -16,9 +16,9 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/pwm.h>
+#include <linux/of_device.h>
 #include <mach/hardware.h>
 
-
 /* i.MX1 and i.MX21 share the same PWM function block: */
 
 #define MX1_PWMC    0x00   /* PWM Control Register */
@@ -203,8 +203,17 @@ static struct pwm_ops imx_pwm_ops = {
 	.owner = THIS_MODULE,
 };
 
+static const struct of_device_id imx_pwm_dt_ids[] = {
+	{ .compatible = "fsl,imx1-pwm", .data = &imx_pwm_config_v1, },
+	{ .compatible = "fsl,imx27-pwm", .data = &imx_pwm_config_v2, },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx_pwm_of_match);
+
 static int __devinit imx_pwm_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id =
+			of_match_device(imx_pwm_dt_ids, &pdev->dev);
 	struct imx_chip *imx;
 	struct resource *r;
 	int ret = 0;
@@ -235,12 +244,16 @@ static int __devinit imx_pwm_probe(struct platform_device *pdev)
 	if (imx->mmio_base == NULL)
 		return -EADDRNOTAVAIL;
 
-	if (cpu_is_mx1() || cpu_is_mx21()) {
-		imx->config = imx_pwm_config_v1;
-		imx->set_enable = imx_pwm_set_enable_v1;
+	if (of_id) {
+		imx->config = of_id->data;
 	} else {
-		imx->config = imx_pwm_config_v2;
-		imx->set_enable = imx_pwm_set_enable_v2;
+		if (cpu_is_mx1() || cpu_is_mx21()) {
+			imx->config = imx_pwm_config_v1;
+			imx->set_enable = imx_pwm_set_enable_v1;
+		} else {
+			imx->config = imx_pwm_config_v2;
+			imx->set_enable = imx_pwm_set_enable_v2;
+		}
 	}
 
 	ret = pwmchip_add(&imx->chip);
-- 
1.7.10.4

[PATCH 6/9] pwm i.MX: use per clock unconditionally

[Sascha Hauer]

The i.MX pwm core has two clocks: The ipg clock and the ipg highfreq
(peripheral) clock. The ipg clock has to be enabled for this hardware
to work. The actual pwm output can either be driven by the ipg clock
or the ipg highfreq. The ipg highfreq has the advantage that it runs
even when the SoC is in low power modes.
Use the always running clock also on i.MX25.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/pwm/pwm-imx.c |    7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index dfc1bae..3834f475 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -132,16 +132,11 @@ static int imx_pwm_config_v2(struct pwm_chip *chip,
 
 	cr = MX3_PWMCR_PRESCALER(prescale) |
 		MX3_PWMCR_DOZEEN | MX3_PWMCR_WAITEN |
-		MX3_PWMCR_DBGEN;
+		MX3_PWMCR_DBGEN | MX3_PWMCR_CLKSRC_IPG_HIGH;
 
 	if (imx->enabled)
 		cr |= MX3_PWMCR_EN;
 
-	if (cpu_is_mx25())
-		cr |= MX3_PWMCR_CLKSRC_IPG;
-	else
-		cr |= MX3_PWMCR_CLKSRC_IPG_HIGH;
-
 	writel(cr, imx->mmio_base + MX3_PWMCR);
 
 	return 0;
-- 
1.7.10.4

[PATCH 7/9] pwm i.MX: fix clock lookup

[Sascha Hauer]

From: Philipp Zabel <p.zabel@pengutronix.de>

The i.MX pwm core has two clocks: The ipg clock and the ipg highfreq
(peripheral) clock. The ipg clock has to be enabled for this hardware
to work. The actual pwm output can either be driven by the ipg clock
or the ipg highfreq. The ipg highfreq has the advantage that it runs
even when the SoC is in low power modes.
This patch requests both clocks and enables the ipg clock for accessing
registers and the peripheral clock to actually turn on the pwm.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/pwm/pwm-imx.c |   33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index 3834f475..308ab51 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -41,7 +41,8 @@
 #define MX3_PWMCR_EN              (1 << 0)
 
 struct imx_chip {
-	struct clk	*clk;
+	struct clk	*clk_per;
+	struct clk	*clk_ipg;
 
 	int		enabled;
 	void __iomem	*mmio_base;
@@ -106,7 +107,7 @@ static int imx_pwm_config_v2(struct pwm_chip *chip,
 	unsigned long period_cycles, duty_cycles, prescale;
 	u32 cr;
 
-	c = clk_get_rate(imx->clk);
+	c = clk_get_rate(imx->clk_per);
 	c = c * period_ns;
 	do_div(c, 1000000000);
 	period_cycles = c;
@@ -161,8 +162,15 @@ 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;
 
-	return imx->config(chip, pwm, duty_ns, period_ns);
+	clk_prepare_enable(imx->clk_ipg);
+
+	ret = imx->config(chip, pwm, duty_ns, period_ns);
+
+	clk_disable_unprepare(imx->clk_ipg);
+
+	return ret;
 }
 
 static int imx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
@@ -170,7 +178,7 @@ static int imx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 	struct imx_chip *imx = to_imx_chip(chip);
 	int rc;
 
-	rc = clk_prepare_enable(imx->clk);
+	rc = clk_prepare_enable(imx->clk_per);
 	if (rc)
 		return rc;
 
@@ -187,7 +195,7 @@ static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 
 	imx->set_enable(chip, false);
 
-	clk_disable_unprepare(imx->clk);
+	clk_disable_unprepare(imx->clk_per);
 	imx->enabled = 0;
 }
 
@@ -219,10 +227,19 @@ static int __devinit imx_pwm_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	imx->clk = devm_clk_get(&pdev->dev, "pwm");
+	imx->clk_per = devm_clk_get(&pdev->dev, "per");
+	if (IS_ERR(imx->clk_per)) {
+		dev_err(&pdev->dev, "getting per clock failed with %ld\n",
+				PTR_ERR(imx->clk_per));
+		return PTR_ERR(imx->clk_per);
+	}
 
-	if (IS_ERR(imx->clk))
-		return PTR_ERR(imx->clk);
+	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;
-- 
1.7.10.4

[PATCH 8/9] ARM i.MX53: Add pwms to dtsi

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Conflicts:
	arch/arm/mach-imx/clk-imx51-imx53.c
---
 arch/arm/boot/dts/imx53.dtsi        |   14 ++++++++++++++
 arch/arm/mach-imx/clk-imx51-imx53.c |    4 ++++
 2 files changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index cd37165..7ec17e4 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -189,6 +189,20 @@
 				status = "disabled";
 			};
 
+			pwm1: pwm@53fb4000 {
+				#pwm-cells = <3>;
+				compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
+				reg = <0x53fb4000 0x4000>;
+				interrupts = <61>;
+			};
+
+			pwm2: pwm@53fb8000 {
+				#pwm-cells = <3>;
+				compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
+				reg = <0x53fb8000 0x4000>;
+				interrupts = <94>;
+			};
+
 			uart1: serial@53fbc000 {
 				compatible = "fsl,imx53-uart", "fsl,imx21-uart";
 				reg = <0x53fbc000 0x4000>;
diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c b/arch/arm/mach-imx/clk-imx51-imx53.c
index 4bdcaa9..b522411 100644
--- a/arch/arm/mach-imx/clk-imx51-imx53.c
+++ b/arch/arm/mach-imx/clk-imx51-imx53.c
@@ -455,6 +455,10 @@ int __init mx53_clocks_init(unsigned long rate_ckil, unsigned long rate_osc,
 	clk_register_clkdev(clk[ssi1_ipg_gate], NULL, "63fcc000.ssi");
 	clk_register_clkdev(clk[ssi2_ipg_gate], NULL, "50014000.ssi");
 	clk_register_clkdev(clk[ssi3_ipg_gate], NULL, "63fd0000.ssi");
+	clk_register_clkdev(clk[pwm1_ipg_gate], "ipg", "53fb4000.pwm");
+	clk_register_clkdev(clk[pwm1_hf_gate], "per", "53fb4000.pwm");
+	clk_register_clkdev(clk[pwm2_ipg_gate], "ipg", "53fb8000.pwm");
+	clk_register_clkdev(clk[pwm2_hf_gate], "per", "53fb8000.pwm");
 
 	/* set SDHC root clock to 200MHZ*/
 	clk_set_rate(clk[esdhc_a_podf], 200000000);
-- 
1.7.10.4

[PATCH 9/9] pwm i.MX: add devicetree support

[Sascha Hauer]

From: Philipp Zabel <p.zabel@pengutronix.de>

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/pwm/pwm-imx.c |   25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index 308ab51..dce2acb 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -206,9 +206,25 @@ static struct pwm_ops imx_pwm_ops = {
 	.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);
+};
+
+static struct imx_pwm_data imx_pwm_data_v1 = {
+	.config = imx_pwm_config_v1,
+	.set_enable = imx_pwm_set_enable_v1,
+};
+
+static struct imx_pwm_data imx_pwm_data_v2 = {
+	.config = imx_pwm_config_v2,
+	.set_enable = imx_pwm_set_enable_v2,
+};
+
 static const struct of_device_id imx_pwm_dt_ids[] = {
-	{ .compatible = "fsl,imx1-pwm", .data = &imx_pwm_config_v1, },
-	{ .compatible = "fsl,imx27-pwm", .data = &imx_pwm_config_v2, },
+	{ .compatible = "fsl,imx1-pwm", .data = &imx_pwm_data_v1, },
+	{ .compatible = "fsl,imx27-pwm", .data = &imx_pwm_data_v2, },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, imx_pwm_of_match);
@@ -257,7 +273,9 @@ static int __devinit imx_pwm_probe(struct platform_device *pdev)
 		return -EADDRNOTAVAIL;
 
 	if (of_id) {
-		imx->config = of_id->data;
+		struct imx_pwm_data *data = of_id->data;
+		imx->config = data->config;
+		imx->set_enable = data->set_enable;
 	} else {
 		if (cpu_is_mx1() || cpu_is_mx21()) {
 			imx->config = imx_pwm_config_v1;
@@ -290,6 +308,7 @@ static int __devexit imx_pwm_remove(struct platform_device *pdev)
 static struct platform_driver imx_pwm_driver = {
 	.driver		= {
 		.name	= "mxc_pwm",
+		.of_match_table = of_match_ptr(imx_pwm_dt_ids),
 	},
 	.probe		= imx_pwm_probe,
 	.remove		= __devexit_p(imx_pwm_remove),
-- 
1.7.10.4

Re: i.MX pwm patches

[Shawn Guo]

On Tue, Aug 28, 2012 at 01:48:15PM +0200, Sascha Hauer wrote:
> Hi All,
> 
> The following patches are an overdue work on the i.MX pwm driver.
> 
> - introduce SoC specific functions to make the driver easier to maintain
> - use peripheral clock for pwm output unconditionally
> - separate the two clocks this module has
> - enable ipg clock for register accesses, peripheral clock for enabling
>   the pwm
> - make the driver safe for calling pwm_config before pwm_enable
> - Add devicetree support for i.MX53
> 
> The platform device support is still implemented using cpu_is_*, I think
> this can be dropped completely soon

Anything stops us from doing this right now?  The bonus point of
cleaning this is that we can remove the #include <mach/hardware.h>
from the driver, which is helpful for single-kernel project.

Regards,
Shawn

> as this driver has no in kernel users
> currently, so there should be no need to keep compatibility for platform
> based boards.
> 
> Hopefully this series fixes the issues mentioned by Benoît. It has been
> tested on an i.MX53 only.
> 
> Sascha
> 
> ----------------------------------------------------------------
> Philipp Zabel (3):
>       pwm i.MX: add devicetree support
>       pwm i.MX: fix clock lookup
>       pwm i.MX: add devicetree support
> 
> Sascha Hauer (6):
>       pwm i.MX: factor out SoC specific functions
>       pwm i.MX: remove unnecessary if in pwm_[en|dis]able
>       pwm i.MX: add functions to enable/disable pwm.
>       pwm i.MX: Use module_platform_driver
>       pwm i.MX: use per clock unconditionally
>       ARM i.MX53: Add pwms to dtsi
> 
>  arch/arm/boot/dts/imx53.dtsi        |   14 ++
>  arch/arm/mach-imx/clk-imx51-imx53.c |    4 +
>  drivers/pwm/pwm-imx.c               |  278 +++++++++++++++++++++++------------
>  3 files changed, 202 insertions(+), 94 deletions(-)

Re: [PATCH 5/9] pwm i.MX: add devicetree support

[Shawn Guo]

On Tue, Aug 28, 2012 at 01:48:20PM +0200, Sascha Hauer wrote:
> From: Philipp Zabel <p.zabel@pengutronix.de>
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/pwm/pwm-imx.c |   25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> index a689144..dfc1bae 100644
> --- a/drivers/pwm/pwm-imx.c
> +++ b/drivers/pwm/pwm-imx.c
> @@ -16,9 +16,9 @@
>  #include <linux/clk.h>
>  #include <linux/io.h>
>  #include <linux/pwm.h>
> +#include <linux/of_device.h>
>  #include <mach/hardware.h>
>  
> -
>  /* i.MX1 and i.MX21 share the same PWM function block: */
>  
>  #define MX1_PWMC    0x00   /* PWM Control Register */
> @@ -203,8 +203,17 @@ static struct pwm_ops imx_pwm_ops = {
>  	.owner = THIS_MODULE,
>  };
>  
> +static const struct of_device_id imx_pwm_dt_ids[] = {
> +	{ .compatible = "fsl,imx1-pwm", .data = &imx_pwm_config_v1, },
> +	{ .compatible = "fsl,imx27-pwm", .data = &imx_pwm_config_v2, },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imx_pwm_of_match);

s/imx_pwm_of_match/imx_pwm_dt_ids

Regards,
Shawn

> +
>  static int __devinit imx_pwm_probe(struct platform_device *pdev)
>  {
> +	const struct of_device_id *of_id =
> +			of_match_device(imx_pwm_dt_ids, &pdev->dev);
>  	struct imx_chip *imx;
>  	struct resource *r;
>  	int ret = 0;
> @@ -235,12 +244,16 @@ static int __devinit imx_pwm_probe(struct platform_device *pdev)
>  	if (imx->mmio_base == NULL)
>  		return -EADDRNOTAVAIL;
>  
> -	if (cpu_is_mx1() || cpu_is_mx21()) {
> -		imx->config = imx_pwm_config_v1;
> -		imx->set_enable = imx_pwm_set_enable_v1;
> +	if (of_id) {
> +		imx->config = of_id->data;
>  	} else {
> -		imx->config = imx_pwm_config_v2;
> -		imx->set_enable = imx_pwm_set_enable_v2;
> +		if (cpu_is_mx1() || cpu_is_mx21()) {
> +			imx->config = imx_pwm_config_v1;
> +			imx->set_enable = imx_pwm_set_enable_v1;
> +		} else {
> +			imx->config = imx_pwm_config_v2;
> +			imx->set_enable = imx_pwm_set_enable_v2;
> +		}
>  	}
>  
>  	ret = pwmchip_add(&imx->chip);
> -- 
> 1.7.10.4
> 

Re: [PATCH 9/9] pwm i.MX: add devicetree support

[Shawn Guo]

Shouldn't this patch be merged into #5?  Also, a binding doc should
be added.

Regards,
Shawn

On Tue, Aug 28, 2012 at 01:48:24PM +0200, Sascha Hauer wrote:
> From: Philipp Zabel <p.zabel@pengutronix.de>
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/pwm/pwm-imx.c |   25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> index 308ab51..dce2acb 100644
> --- a/drivers/pwm/pwm-imx.c
> +++ b/drivers/pwm/pwm-imx.c
> @@ -206,9 +206,25 @@ static struct pwm_ops imx_pwm_ops = {
>  	.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);
> +};
> +
> +static struct imx_pwm_data imx_pwm_data_v1 = {
> +	.config = imx_pwm_config_v1,
> +	.set_enable = imx_pwm_set_enable_v1,
> +};
> +
> +static struct imx_pwm_data imx_pwm_data_v2 = {
> +	.config = imx_pwm_config_v2,
> +	.set_enable = imx_pwm_set_enable_v2,
> +};
> +
>  static const struct of_device_id imx_pwm_dt_ids[] = {
> -	{ .compatible = "fsl,imx1-pwm", .data = &imx_pwm_config_v1, },
> -	{ .compatible = "fsl,imx27-pwm", .data = &imx_pwm_config_v2, },
> +	{ .compatible = "fsl,imx1-pwm", .data = &imx_pwm_data_v1, },
> +	{ .compatible = "fsl,imx27-pwm", .data = &imx_pwm_data_v2, },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, imx_pwm_of_match);
> @@ -257,7 +273,9 @@ static int __devinit imx_pwm_probe(struct platform_device *pdev)
>  		return -EADDRNOTAVAIL;
>  
>  	if (of_id) {
> -		imx->config = of_id->data;
> +		struct imx_pwm_data *data = of_id->data;
> +		imx->config = data->config;
> +		imx->set_enable = data->set_enable;
>  	} else {
>  		if (cpu_is_mx1() || cpu_is_mx21()) {
>  			imx->config = imx_pwm_config_v1;
> @@ -290,6 +308,7 @@ static int __devexit imx_pwm_remove(struct platform_device *pdev)
>  static struct platform_driver imx_pwm_driver = {
>  	.driver		= {
>  		.name	= "mxc_pwm",
> +		.of_match_table = of_match_ptr(imx_pwm_dt_ids),
>  	},
>  	.probe		= imx_pwm_probe,
>  	.remove		= __devexit_p(imx_pwm_remove),
> -- 
> 1.7.10.4
> 

Re: [PATCH 8/9] ARM i.MX53: Add pwms to dtsi

[Shawn Guo]

On Tue, Aug 28, 2012 at 01:48:23PM +0200, Sascha Hauer wrote:
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> 
> Conflicts:
> 	arch/arm/mach-imx/clk-imx51-imx53.c

Yeah, I know you have sorted out conflicts :)

> ---
>  arch/arm/boot/dts/imx53.dtsi        |   14 ++++++++++++++
>  arch/arm/mach-imx/clk-imx51-imx53.c |    4 ++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
> index cd37165..7ec17e4 100644
> --- a/arch/arm/boot/dts/imx53.dtsi
> +++ b/arch/arm/boot/dts/imx53.dtsi
> @@ -189,6 +189,20 @@
>  				status = "disabled";
>  			};
>  
> +			pwm1: pwm@53fb4000 {
> +				#pwm-cells = <3>;

pwm-cells should be 2?

> +				compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
> +				reg = <0x53fb4000 0x4000>;
> +				interrupts = <61>;
> +			};
> +
> +			pwm2: pwm@53fb8000 {
> +				#pwm-cells = <3>;
> +				compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
> +				reg = <0x53fb8000 0x4000>;
> +				interrupts = <94>;
> +			};
> +
>  			uart1: serial@53fbc000 {
>  				compatible = "fsl,imx53-uart", "fsl,imx21-uart";
>  				reg = <0x53fbc000 0x4000>;
> diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c b/arch/arm/mach-imx/clk-imx51-imx53.c
> index 4bdcaa9..b522411 100644
> --- a/arch/arm/mach-imx/clk-imx51-imx53.c
> +++ b/arch/arm/mach-imx/clk-imx51-imx53.c
> @@ -455,6 +455,10 @@ int __init mx53_clocks_init(unsigned long rate_ckil, unsigned long rate_osc,
>  	clk_register_clkdev(clk[ssi1_ipg_gate], NULL, "63fcc000.ssi");
>  	clk_register_clkdev(clk[ssi2_ipg_gate], NULL, "50014000.ssi");
>  	clk_register_clkdev(clk[ssi3_ipg_gate], NULL, "63fd0000.ssi");
> +	clk_register_clkdev(clk[pwm1_ipg_gate], "ipg", "53fb4000.pwm");
> +	clk_register_clkdev(clk[pwm1_hf_gate], "per", "53fb4000.pwm");
> +	clk_register_clkdev(clk[pwm2_ipg_gate], "ipg", "53fb8000.pwm");
> +	clk_register_clkdev(clk[pwm2_hf_gate], "per", "53fb8000.pwm");

It should be in a separate patch?

>  
>  	/* set SDHC root clock to 200MHZ*/
>  	clk_set_rate(clk[esdhc_a_podf], 200000000);
> -- 
> 1.7.10.4
> 

-- 
Regards,
Shawn

Re: [PATCH 8/9] ARM i.MX53: Add pwms to dtsi

[Shawn Guo]

On Fri, Aug 31, 2012 at 03:07:23PM +0200, Sascha Hauer wrote:
> > >  	clk_register_clkdev(clk[ssi1_ipg_gate], NULL, "63fcc000.ssi");
> > >  	clk_register_clkdev(clk[ssi2_ipg_gate], NULL, "50014000.ssi");
> > >  	clk_register_clkdev(clk[ssi3_ipg_gate], NULL, "63fd0000.ssi");
> > > +	clk_register_clkdev(clk[pwm1_ipg_gate], "ipg", "53fb4000.pwm");
> > > +	clk_register_clkdev(clk[pwm1_hf_gate], "per", "53fb4000.pwm");
> > > +	clk_register_clkdev(clk[pwm2_ipg_gate], "ipg", "53fb8000.pwm");
> > > +	clk_register_clkdev(clk[pwm2_hf_gate], "per", "53fb8000.pwm");
> > 
> > It should be in a separate patch?
> 
> Should it? Surely I can do this, I had the feeling though that it
> belongs together.
> 
>From patch subject, I do not expect these changes in the patch.  It's
okay to have it but we need a more proper patch subject then.  

-- 
Regards,
Shawn

Re: i.MX pwm patches

[Sascha Hauer]

On Fri, Aug 31, 2012 at 05:45:49AM +0800, Shawn Guo wrote:
> On Tue, Aug 28, 2012 at 01:48:15PM +0200, Sascha Hauer wrote:
> > Hi All,
> > 
> > The following patches are an overdue work on the i.MX pwm driver.
> > 
> > - introduce SoC specific functions to make the driver easier to maintain
> > - use peripheral clock for pwm output unconditionally
> > - separate the two clocks this module has
> > - enable ipg clock for register accesses, peripheral clock for enabling
> >   the pwm
> > - make the driver safe for calling pwm_config before pwm_enable
> > - Add devicetree support for i.MX53
> > 
> > The platform device support is still implemented using cpu_is_*, I think
> > this can be dropped completely soon
> 
> Anything stops us from doing this right now?  The bonus point of
> cleaning this is that we can remove the #include <mach/hardware.h>
> from the driver, which is helpful for single-kernel project.

The pwm driver currently has no in kernel user, so I'd say yes.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 8/9] ARM i.MX53: Add pwms to dtsi

[Sascha Hauer]

On Fri, Aug 31, 2012 at 06:32:20AM +0800, Shawn Guo wrote:
> On Tue, Aug 28, 2012 at 01:48:23PM +0200, Sascha Hauer wrote:
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > 
> > Conflicts:
> > 	arch/arm/mach-imx/clk-imx51-imx53.c
> 
> Yeah, I know you have sorted out conflicts :)

:)

> 
> > ---
> >  arch/arm/boot/dts/imx53.dtsi        |   14 ++++++++++++++
> >  arch/arm/mach-imx/clk-imx51-imx53.c |    4 ++++
> >  2 files changed, 18 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
> > index cd37165..7ec17e4 100644
> > --- a/arch/arm/boot/dts/imx53.dtsi
> > +++ b/arch/arm/boot/dts/imx53.dtsi
> > @@ -189,6 +189,20 @@
> >  				status = "disabled";
> >  			};
> >  
> > +			pwm1: pwm@53fb4000 {
> > +				#pwm-cells = <3>;
> 
> pwm-cells should be 2?

Yes, right. We have a patch internally that allows us to pass a
'inverted' flag to the pwm, hence I accidently have 3 here.

> >  	clk_register_clkdev(clk[ssi1_ipg_gate], NULL, "63fcc000.ssi");
> >  	clk_register_clkdev(clk[ssi2_ipg_gate], NULL, "50014000.ssi");
> >  	clk_register_clkdev(clk[ssi3_ipg_gate], NULL, "63fd0000.ssi");
> > +	clk_register_clkdev(clk[pwm1_ipg_gate], "ipg", "53fb4000.pwm");
> > +	clk_register_clkdev(clk[pwm1_hf_gate], "per", "53fb4000.pwm");
> > +	clk_register_clkdev(clk[pwm2_ipg_gate], "ipg", "53fb8000.pwm");
> > +	clk_register_clkdev(clk[pwm2_hf_gate], "per", "53fb8000.pwm");
> 
> It should be in a separate patch?

Should it? Surely I can do this, I had the feeling though that it
belongs together.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 8/9] ARM i.MX53: Add pwms to dtsi

[Thierry Reding]

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

On Fri, Aug 31, 2012 at 03:07:23PM +0200, Sascha Hauer wrote:
> On Fri, Aug 31, 2012 at 06:32:20AM +0800, Shawn Guo wrote:
> > On Tue, Aug 28, 2012 at 01:48:23PM +0200, Sascha Hauer wrote:
[...]
> > >  arch/arm/boot/dts/imx53.dtsi        |   14 ++++++++++++++
> > >  arch/arm/mach-imx/clk-imx51-imx53.c |    4 ++++
> > >  2 files changed, 18 insertions(+)
> > > 
> > > diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
> > > index cd37165..7ec17e4 100644
> > > --- a/arch/arm/boot/dts/imx53.dtsi
> > > +++ b/arch/arm/boot/dts/imx53.dtsi
> > > @@ -189,6 +189,20 @@
> > >  				status = "disabled";
> > >  			};
> > >  
> > > +			pwm1: pwm@53fb4000 {
> > > +				#pwm-cells = <3>;
> > 
> > pwm-cells should be 2?
> 
> Yes, right. We have a patch internally that allows us to pass a
> 'inverted' flag to the pwm, hence I accidently have 3 here.

There are patches in for-next that add support for setting the PWM
polarity, though there's currently no support for specifying it via a
third cell in the specifier. Would you mind sharing the patches that add
this?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 8/9] ARM i.MX53: Add pwms to dtsi

[Sascha Hauer]

On Fri, Sep 07, 2012 at 03:29:55PM +0200, Thierry Reding wrote:
> On Fri, Aug 31, 2012 at 03:07:23PM +0200, Sascha Hauer wrote:
> > On Fri, Aug 31, 2012 at 06:32:20AM +0800, Shawn Guo wrote:
> > > On Tue, Aug 28, 2012 at 01:48:23PM +0200, Sascha Hauer wrote:
> [...]
> > > >  arch/arm/boot/dts/imx53.dtsi        |   14 ++++++++++++++
> > > >  arch/arm/mach-imx/clk-imx51-imx53.c |    4 ++++
> > > >  2 files changed, 18 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
> > > > index cd37165..7ec17e4 100644
> > > > --- a/arch/arm/boot/dts/imx53.dtsi
> > > > +++ b/arch/arm/boot/dts/imx53.dtsi
> > > > @@ -189,6 +189,20 @@
> > > >  				status = "disabled";
> > > >  			};
> > > >  
> > > > +			pwm1: pwm@53fb4000 {
> > > > +				#pwm-cells = <3>;
> > > 
> > > pwm-cells should be 2?
> > 
> > Yes, right. We have a patch internally that allows us to pass a
> > 'inverted' flag to the pwm, hence I accidently have 3 here.
> 
> There are patches in for-next that add support for setting the PWM
> polarity, though there's currently no support for specifying it via a
> third cell in the specifier. Would you mind sharing the patches that add
> this?

Yes, will do. I was afraid this leads to some discussion, so I skipped
them so far.

The basic idea was that the third cell is for flags from which bit0 set
means 'inverted'. We currently implemented this i.MX specific, but if
you think this is acceptable it's propably a good idea to implement this
in a generic manner.

Sascha



-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 8/9] ARM i.MX53: Add pwms to dtsi

[Thierry Reding]

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

On Fri, Sep 07, 2012 at 07:26:12PM +0200, Sascha Hauer wrote:
> On Fri, Sep 07, 2012 at 03:29:55PM +0200, Thierry Reding wrote:
> > On Fri, Aug 31, 2012 at 03:07:23PM +0200, Sascha Hauer wrote:
> > > On Fri, Aug 31, 2012 at 06:32:20AM +0800, Shawn Guo wrote:
> > > > On Tue, Aug 28, 2012 at 01:48:23PM +0200, Sascha Hauer wrote:
> > [...]
> > > > >  arch/arm/boot/dts/imx53.dtsi        |   14 ++++++++++++++
> > > > >  arch/arm/mach-imx/clk-imx51-imx53.c |    4 ++++
> > > > >  2 files changed, 18 insertions(+)
> > > > > 
> > > > > diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
> > > > > index cd37165..7ec17e4 100644
> > > > > --- a/arch/arm/boot/dts/imx53.dtsi
> > > > > +++ b/arch/arm/boot/dts/imx53.dtsi
> > > > > @@ -189,6 +189,20 @@
> > > > >  				status = "disabled";
> > > > >  			};
> > > > >  
> > > > > +			pwm1: pwm@53fb4000 {
> > > > > +				#pwm-cells = <3>;
> > > > 
> > > > pwm-cells should be 2?
> > > 
> > > Yes, right. We have a patch internally that allows us to pass a
> > > 'inverted' flag to the pwm, hence I accidently have 3 here.
> > 
> > There are patches in for-next that add support for setting the PWM
> > polarity, though there's currently no support for specifying it via a
> > third cell in the specifier. Would you mind sharing the patches that add
> > this?
> 
> Yes, will do. I was afraid this leads to some discussion, so I skipped
> them so far.
> 
> The basic idea was that the third cell is for flags from which bit0 set
> means 'inverted'. We currently implemented this i.MX specific, but if
> you think this is acceptable it's propably a good idea to implement this
> in a generic manner.

Absolutely. That's exactly what I had in mind as well, so if you have
the code ready I don't have to write it myself.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 5/9] pwm i.MX: add devicetree support

[Sascha Hauer]

From: Philipp Zabel <p.zabel@pengutronix.de>

At the same time remove platform based support. No user for
this driver has made it into mainline so far, so all we break
is out of tree stuff.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Shawn Guo <shawn.guo@linaro.org>
Reviewed-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com>
---
 Documentation/devicetree/bindings/pwm/imx-pwm.txt |   17 ++++++++
 drivers/pwm/pwm-imx.c                             |   45 ++++++++++++++++-----
 2 files changed, 52 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pwm/imx-pwm.txt

diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
new file mode 100644
index 0000000..9b9b185
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
@@ -0,0 +1,17 @@
+Freescale i.MX PWM controller
+
+Required properties:
+- compatible: should be "fsl,<soc>-pwm"
+- reg: physical base address and length of the controller's registers
+- #pwm-cells: should be 2.  The first cell specifies the per-chip index
+  of the PWM to use and the second cell is the duty cycle in nanoseconds.
+- interrupts: The interrupt for the pwm controller
+
+Example:
+
+pwm1: pwm@53fb4000 {
+	#pwm-cells = <2>;
+	compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
+	reg = <0x53fb4000 0x4000>;
+	interrupts = <61>;
+};
diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index 97f385e..675e2bb 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -16,8 +16,7 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/pwm.h>
-#include <mach/hardware.h>
-
+#include <linux/of_device.h>
 
 /* i.MX1 and i.MX21 share the same PWM function block: */
 
@@ -203,12 +202,41 @@ static struct pwm_ops imx_pwm_ops = {
 	.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);
+};
+
+static struct imx_pwm_data imx_pwm_data_v1 = {
+	.config = imx_pwm_config_v1,
+	.set_enable = imx_pwm_set_enable_v1,
+};
+
+static struct imx_pwm_data imx_pwm_data_v2 = {
+	.config = imx_pwm_config_v2,
+	.set_enable = imx_pwm_set_enable_v2,
+};
+
+static const struct of_device_id imx_pwm_dt_ids[] = {
+	{ .compatible = "fsl,imx1-pwm", .data = &imx_pwm_data_v1, },
+	{ .compatible = "fsl,imx27-pwm", .data = &imx_pwm_data_v2, },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx_pwm_dt_ids);
+
 static int __devinit imx_pwm_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id =
+			of_match_device(imx_pwm_dt_ids, &pdev->dev);
+	struct imx_pwm_data *data;
 	struct imx_chip *imx;
 	struct resource *r;
 	int ret = 0;
 
+	if (!of_id)
+		return -ENODEV;
+
 	imx = devm_kzalloc(&pdev->dev, sizeof(*imx), GFP_KERNEL);
 	if (imx == NULL) {
 		dev_err(&pdev->dev, "failed to allocate memory\n");
@@ -235,13 +263,9 @@ static int __devinit imx_pwm_probe(struct platform_device *pdev)
 	if (imx->mmio_base == NULL)
 		return -EADDRNOTAVAIL;
 
-	if (cpu_is_mx1() || cpu_is_mx21()) {
-		imx->config = imx_pwm_config_v1;
-		imx->set_enable = imx_pwm_set_enable_v1;
-	} else {
-		imx->config = imx_pwm_config_v2;
-		imx->set_enable = imx_pwm_set_enable_v2;
-	}
+	data = of_id->data;
+	imx->config = data->config;
+	imx->set_enable = data->set_enable;
 
 	ret = pwmchip_add(&imx->chip);
 	if (ret < 0)
@@ -264,7 +288,8 @@ static int __devexit imx_pwm_remove(struct platform_device *pdev)
 
 static struct platform_driver imx_pwm_driver = {
 	.driver		= {
-		.name	= "mxc_pwm",
+		.name	= "imx-pwm",
+		.of_match_table = of_match_ptr(imx_pwm_dt_ids),
 	},
 	.probe		= imx_pwm_probe,
 	.remove		= __devexit_p(imx_pwm_remove),
-- 
1.7.10.4