[PATCH v2] pwm i.MX: add devicetree support

[PATCH v2] pwm i.MX: add devicetree support

[Sascha Hauer]

Changes since v1:

- Add devicetree binding documentation
- Merge 5/9 and 9/9
- fix #pwm-cells (must be 2 instead of 3)
- fix wrong name in MODULE_DEVICE_TABLE
- drop platform based probing while introducing devicetree based probe

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

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 pwm support

 Documentation/devicetree/bindings/pwm/imx-pwm.txt |   17 ++
 arch/arm/boot/dts/imx53.dtsi                      |   14 ++
 arch/arm/mach-imx/clk-imx51-imx53.c               |    4 +
 drivers/pwm/pwm-imx.c                             |  275 ++++++++++++++-------
 4 files changed, 214 insertions(+), 96 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pwm/imx-pwm.txt

[PATCH 1/8] 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/8] 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/8] 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/8] 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/8] 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>
---
 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 a689144..af77045 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

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

[Benoît Thébaudeau]

On Wednesday, September 5, 2012 3:35:24 PM, Sascha Hauer wrote:
> 
> 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>
> ---
>  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 a689144..af77045 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),

Since this is supposed to remove platform-based support, why don't you also
remove the PWM stuff from arch/arm/plat-mxc and
arch/arm/mach-imx/devices-imx*.h?

Best regards,
Benoît

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

[Sascha Hauer]

On Wed, Sep 05, 2012 at 11:42:34PM +0200, Benoît Thébaudeau wrote:
> 
> Since this is supposed to remove platform-based support, why don't you also
> remove the PWM stuff from arch/arm/plat-mxc and
> arch/arm/mach-imx/devices-imx*.h?

Simple answer: I was too lazy yesterday. I will include it in the next
round.

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 |

[PATCH 6/8] 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 af77045..b234288 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -131,16 +131,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/8] 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 b234288..5b03ace 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -40,7 +40,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;
@@ -105,7 +106,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;
@@ -160,8 +161,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)
@@ -169,7 +177,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;
 
@@ -186,7 +194,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;
 }
 
@@ -238,10 +246,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

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

[Benoît Thébaudeau]

On Wednesday, September 5, 2012 3:35:26 PM, Sascha Hauer wrote:
> 
> 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 b234288..5b03ace 100644
> --- a/drivers/pwm/pwm-imx.c
> +++ b/drivers/pwm/pwm-imx.c
> @@ -40,7 +40,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;
> @@ -105,7 +106,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;
> @@ -160,8 +161,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);

Why don't you test the return value like in imx_pwm_enable()?

> +
> +	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)
> @@ -169,7 +177,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;
>  
> @@ -186,7 +194,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;
>  }
>  
> @@ -238,10 +246,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;

I have reviewed the whole series. Apart from the comments I made, it looks good
to me.

Best regards,
Benoît

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

[Sascha Hauer]

On Wed, Sep 05, 2012 at 11:48:51PM +0200, Benoît Thébaudeau wrote:
> >  
> > -	c = clk_get_rate(imx->clk);
> > +	c = clk_get_rate(imx->clk_per);
> >  	c = c * period_ns;
> >  	do_div(c, 1000000000);
> >  	period_cycles = c;
> > @@ -160,8 +161,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);
> 
> Why don't you test the return value like in imx_pwm_enable()?

Will do next time.

Sascha

> 
> I have reviewed the whole series. Apart from the comments I made, it looks good
> to me.

Thanks. I can take this as a reviewed-by, right?

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 |

[PATCH 8/8] ARM i.MX53: Add pwm support

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 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..18b9fc3 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 = <2>;
+				compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
+				reg = <0x53fb4000 0x4000>;
+				interrupts = <61>;
+			};
+
+			pwm2: pwm@53fb8000 {
+				#pwm-cells = <2>;
+				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

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

[Shawn Guo]

On Wed, Sep 05, 2012 at 03:35:19PM +0200, Sascha Hauer wrote:
> 
> Changes since v1:
> 
> - Add devicetree binding documentation
> - Merge 5/9 and 9/9
> - fix #pwm-cells (must be 2 instead of 3)
> - fix wrong name in MODULE_DEVICE_TABLE
> - drop platform based probing while introducing devicetree based probe
> 
> ----------------------------------------------------------------
> Philipp Zabel (2):
>       pwm i.MX: add devicetree support
>       pwm i.MX: fix clock lookup
> 
> 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 pwm support
> 
For the series,

Reviewed-by: Shawn Guo <shawn.guo@linaro.org>

>  Documentation/devicetree/bindings/pwm/imx-pwm.txt |   17 ++
>  arch/arm/boot/dts/imx53.dtsi                      |   14 ++
>  arch/arm/mach-imx/clk-imx51-imx53.c               |    4 +
>  drivers/pwm/pwm-imx.c                             |  275 ++++++++++++++-------
>  4 files changed, 214 insertions(+), 96 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pwm/imx-pwm.txt
>