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

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

[Sascha Hauer]

The following is the third version of the i.MX pwm series. I integrated
the remaining comments from Shawn and Benoît and added their tags.

So Thierry, please pull the attached patches. The pull request only
contains the pwm framework specific patches, the remaining two I'd
like to push via the arm-soc tree in case the i.MX5 also gets devicetree
clock lookups.

Sascha


Changes since v2:

- check return value of clk_prepare_enable
- remove platform based probing

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


The following changes since commit fea7a08acb13524b47711625eebea40a0ede69a0:

  Linux 3.6-rc3 (2012-08-22 13:29:06 -0700)

are available in the git repository at:

  git://git.pengutronix.de/git/imx/linux-2.6.git tags/imx-pwm-oftree

for you to fetch changes up to 8424520ae5f2ad2c8c6cd5e990054cf06f479006:

  pwm i.MX: fix clock lookup (2012-09-06 12:35:58 +0200)

----------------------------------------------------------------
This series cleans up the i.MX PWM driver and converts it to
devicetree probing.

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

Sascha Hauer (5):
      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

 Documentation/devicetree/bindings/pwm/imx-pwm.txt |   17 ++
 drivers/pwm/pwm-imx.c                             |  277 ++++++++++++++-------
 2 files changed, 198 insertions(+), 96 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pwm/imx-pwm.txt

[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>
Reviewed-by: Shawn Guo <shawn.guo@linaro.org>
Reviewed-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com>
---
 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

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

[Thierry Reding]

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

On Thu, Sep 06, 2012 at 02:48:07PM +0200, Sascha Hauer wrote:
> To make the code more flexible.

The code isn't made much more flexible, but it is cleaned up quite a
bit. Maybe this should be mentioned instead. Or something along the
lines that these changes make it easier to support multiple variants
in the driver.

Also, can we please try to stick to the "pwm:" prefix for the subject. I
like to keep for consistency and because it makes filtering easier.
Furthermore I've been getting on people's nerves by telling them to use
the all uppercase "PWM" when referring to PWM devices (note the prefix
subject is still lowercase), so it would be unfair not to get on your
nerves as well. =)

One other comment inline.

> 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>
> ---
>  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,

According to CodingStyle, this should be:

	/*
	 * The PWM subsystem...
	 * ...
	 */

I know the original had it wrong as well, but since you're changing the
lines anyway, would you mind fixing it anyway?

Thierry

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

[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>
Reviewed-by: Shawn Guo <shawn.guo@linaro.org>
Reviewed-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com>
---
 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..609b540 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 ret;
 
-	if (!imx->clk_enabled) {
-		rc = clk_prepare_enable(imx->clk);
-		if (!rc)
-			imx->clk_enabled = 1;
-	}
-	return rc;
+	ret = clk_prepare_enable(imx->clk);
+	if (ret)
+		return ret;
+
+	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>
Reviewed-by: Shawn Guo <shawn.guo@linaro.org>
Reviewed-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com>
---
 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 609b540..242a30d 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 (ret)
 		return ret;
 
+	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>
Reviewed-by: Shawn Guo <shawn.guo@linaro.org>
Reviewed-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com>
---
 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 242a30d..97f385e 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>

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

[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>
Reviewed-by: Shawn Guo <shawn.guo@linaro.org>
Reviewed-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com>
---
 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 675e2bb..ea4ab2c 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/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>
Reviewed-by: Shawn Guo <shawn.guo@linaro.org>
Reviewed-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com>
---
 drivers/pwm/pwm-imx.c |   35 +++++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index ea4ab2c..d45d44f 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,17 @@ 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_ipg);
+	if (ret)
+		return ret;
 
-	return imx->config(chip, pwm, duty_ns, period_ns);
+	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 +179,7 @@ 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);
+	ret = clk_prepare_enable(imx->clk_per);
 	if (ret)
 		return ret;
 
@@ -186,7 +196,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 +248,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/9] pwm i.MX: fix clock lookup

[Benoît Thébaudeau]

On Thursday, September 6, 2012 2:48:13 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>
> Reviewed-by: Shawn Guo <shawn.guo@linaro.org>
> Reviewed-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com>
> ---
>  drivers/pwm/pwm-imx.c |   35 +++++++++++++++++++++++++++--------
>  1 file changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> index ea4ab2c..d45d44f 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,17 @@ 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_ipg);
> +	if (ret)
> +		return ret;
>  
> -	return imx->config(chip, pwm, duty_ns, period_ns);
> +	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 +179,7 @@ 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);
> +	ret = clk_prepare_enable(imx->clk_per);
>  	if (ret)
>  		return ret;

Have you tested that this actually works on i.MX53?

I have tested it successfully on i.MX35 (with a few additions to platform code).
But i.MX35 has a single bit controlling both PWM IPG and PER clock gates.

On i.MX53, there are 2 separate control bits for these. So, if ipg clk is
strictly required to access PWM registers, even if per clk is enabled, this code
should not work without adding

+	ret = clk_prepare_enable(imx->clk_ipg);
+	if (ret)
+		return ret;

and

+	clk_disable_unprepare(imx->clk_ipg);

around each call to set_enable().

>  
> @@ -186,7 +196,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 +248,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;

Best regards,
Benoît

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

[Sascha Hauer]

On Thu, Sep 06, 2012 at 08:31:58PM +0200, Benoît Thébaudeau wrote:
> On Thursday, September 6, 2012 2:48:13 PM, Sascha Hauer wrote:
> > 
> > +	int ret;
> > +
> > +	ret = clk_prepare_enable(imx->clk_ipg);
> > +	if (ret)
> > +		return ret;
> >  
> > -	return imx->config(chip, pwm, duty_ns, period_ns);
> > +	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 +179,7 @@ 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);
> > +	ret = clk_prepare_enable(imx->clk_per);
> >  	if (ret)
> >  		return ret;
> 
> Have you tested that this actually works on i.MX53?
> 
> I have tested it successfully on i.MX35 (with a few additions to platform code).
> But i.MX35 has a single bit controlling both PWM IPG and PER clock gates.
> 
> On i.MX53, there are 2 separate control bits for these. So, if ipg clk is
> strictly required to access PWM registers, even if per clk is enabled, this code
> should not work without adding

I tested this on i.MX53, but you're right, this seems to be wrong. I'll
recheck tomorrow.

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 7/9] pwm i.MX: fix clock lookup

[Benoît Thébaudeau]

On Thursday, September 6, 2012 8:42:56 PM, Sascha Hauer wrote:
> On Thu, Sep 06, 2012 at 08:31:58PM +0200, Benoît Thébaudeau wrote:
> > On Thursday, September 6, 2012 2:48:13 PM, Sascha Hauer wrote:
> > > 
> > > +	int ret;
> > > +
> > > +	ret = clk_prepare_enable(imx->clk_ipg);
> > > +	if (ret)
> > > +		return ret;
> > >  
> > > -	return imx->config(chip, pwm, duty_ns, period_ns);
> > > +	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 +179,7 @@ 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);
> > > +	ret = clk_prepare_enable(imx->clk_per);
> > >  	if (ret)
> > >  		return ret;
> > 
> > Have you tested that this actually works on i.MX53?
> > 
> > I have tested it successfully on i.MX35 (with a few additions to
> > platform code).
> > But i.MX35 has a single bit controlling both PWM IPG and PER clock
> > gates.
> > 
> > On i.MX53, there are 2 separate control bits for these. So, if ipg
> > clk is
> > strictly required to access PWM registers, even if per clk is
> > enabled, this code
> > should not work without adding
> 
> I tested this on i.MX53, but you're right, this seems to be wrong.
> I'll
> recheck tomorrow.

I've performed a few more tests with bare register accesses in the bootloader to
see how the PWM IP behaves.

On i.MX25, i.MX35 and i.MX51, read accesses always work, whatever the state of
the IPG and PER clock gates.

On i.MX25 and i.MX51, write accesses always work, whatever the state of the IPG
and PER clock gates.

On i.MX35, write accesses work if and only if the IPG and PER clocks are not
gated off (single control bit for both).

I don't have i.MX53 or i.MX6Q platforms to test that.

Best regards,
Benoît

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

[Sascha Hauer]

On Thu, Sep 06, 2012 at 09:09:42PM +0200, Benoît Thébaudeau wrote:
> On Thursday, September 6, 2012 8:42:56 PM, Sascha Hauer wrote:
> > On Thu, Sep 06, 2012 at 08:31:58PM +0200, Benoît Thébaudeau wrote:
> > > On Thursday, September 6, 2012 2:48:13 PM, Sascha Hauer wrote:
> > > > 
> > > > +	int ret;
> > > > +
> > > > +	ret = clk_prepare_enable(imx->clk_ipg);
> > > > +	if (ret)
> > > > +		return ret;
> > > >  
> > > > -	return imx->config(chip, pwm, duty_ns, period_ns);
> > > > +	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 +179,7 @@ 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);
> > > > +	ret = clk_prepare_enable(imx->clk_per);
> > > >  	if (ret)
> > > >  		return ret;
> > > 
> > > Have you tested that this actually works on i.MX53?
> > > 
> > > I have tested it successfully on i.MX35 (with a few additions to
> > > platform code).
> > > But i.MX35 has a single bit controlling both PWM IPG and PER clock
> > > gates.
> > > 
> > > On i.MX53, there are 2 separate control bits for these. So, if ipg
> > > clk is
> > > strictly required to access PWM registers, even if per clk is
> > > enabled, this code
> > > should not work without adding
> > 
> > I tested this on i.MX53, but you're right, this seems to be wrong.
> > I'll
> > recheck tomorrow.
> 
> I've performed a few more tests with bare register accesses in the bootloader to
> see how the PWM IP behaves.
> 
> On i.MX25, i.MX35 and i.MX51, read accesses always work, whatever the state of
> the IPG and PER clock gates.
> 
> On i.MX25 and i.MX51, write accesses always work, whatever the state of the IPG
> and PER clock gates.
> 
> On i.MX35, write accesses work if and only if the IPG and PER clocks are not
> gated off (single control bit for both).

Ok, I tested on i.MX53 and i.MX27.

On i.MX53 the registers are always accessible. I also measured with an
oscilloscope that the ipg/per clock gates en/disable the PWM when it's
configured for the corresponding clock.

On i.MX27 register accesses also work regardless of the clock gates.
Here we have a single clock gate which only gates the ipg clock.

btw the i.MX6 has a single gate per pwm which are described as "PWMx clocks"

So it seems while not 100% correct the current code seems to be safe.

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 7/9] pwm i.MX: fix clock lookup

[Benoît Thébaudeau]

On Friday, September 7, 2012 9:41:38 AM, Sascha Hauer wrote:
> On Thu, Sep 06, 2012 at 09:09:42PM +0200, Benoît Thébaudeau wrote:
> > On Thursday, September 6, 2012 8:42:56 PM, Sascha Hauer wrote:
> > > On Thu, Sep 06, 2012 at 08:31:58PM +0200, Benoît Thébaudeau
> > > wrote:
> > > > On Thursday, September 6, 2012 2:48:13 PM, Sascha Hauer wrote:
> > > > > 
> > > > > +	int ret;
> > > > > +
> > > > > +	ret = clk_prepare_enable(imx->clk_ipg);
> > > > > +	if (ret)
> > > > > +		return ret;
> > > > >  
> > > > > -	return imx->config(chip, pwm, duty_ns, period_ns);
> > > > > +	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 +179,7 @@ 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);
> > > > > +	ret = clk_prepare_enable(imx->clk_per);
> > > > >  	if (ret)
> > > > >  		return ret;
> > > > 
> > > > Have you tested that this actually works on i.MX53?
> > > > 
> > > > I have tested it successfully on i.MX35 (with a few additions
> > > > to
> > > > platform code).
> > > > But i.MX35 has a single bit controlling both PWM IPG and PER
> > > > clock
> > > > gates.
> > > > 
> > > > On i.MX53, there are 2 separate control bits for these. So, if
> > > > ipg
> > > > clk is
> > > > strictly required to access PWM registers, even if per clk is
> > > > enabled, this code
> > > > should not work without adding
> > > 
> > > I tested this on i.MX53, but you're right, this seems to be
> > > wrong.
> > > I'll
> > > recheck tomorrow.
> > 
> > I've performed a few more tests with bare register accesses in the
> > bootloader to
> > see how the PWM IP behaves.
> > 
> > On i.MX25, i.MX35 and i.MX51, read accesses always work, whatever
> > the state of
> > the IPG and PER clock gates.
> > 
> > On i.MX25 and i.MX51, write accesses always work, whatever the
> > state of the IPG
> > and PER clock gates.
> > 
> > On i.MX35, write accesses work if and only if the IPG and PER
> > clocks are not
> > gated off (single control bit for both).
> 
> Ok, I tested on i.MX53 and i.MX27.
> 
> On i.MX53 the registers are always accessible. I also measured with
> an
> oscilloscope that the ipg/per clock gates en/disable the PWM when
> it's
> configured for the corresponding clock.
> 
> On i.MX27 register accesses also work regardless of the clock gates.
> Here we have a single clock gate which only gates the ipg clock.
> 
> btw the i.MX6 has a single gate per pwm which are described as "PWMx
> clocks"
> 
> So it seems while not 100% correct the current code seems to be safe.

Agreed.

Best regards,
Benoît

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

[Sascha Hauer]

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

[PATCH 9/9] ARM i.MX: remove PWM platform support

[Sascha Hauer]

As the pwm driver now is devicetree only, remove the platform
support for this device.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/mach-imx/clk-imx21.c                   |    1 -
 arch/arm/mach-imx/clk-imx25.c                   |    8 ---
 arch/arm/mach-imx/clk-imx27.c                   |    1 -
 arch/arm/mach-imx/clk-imx51-imx53.c             |    2 -
 arch/arm/mach-imx/devices-imx25.h               |    4 --
 arch/arm/mach-imx/devices-imx51.h               |    4 --
 arch/arm/plat-mxc/devices/Kconfig               |    3 -
 arch/arm/plat-mxc/devices/Makefile              |    1 -
 arch/arm/plat-mxc/devices/platform-mxc_pwm.c    |   69 -----------------------
 arch/arm/plat-mxc/include/mach/devices-common.h |    9 ---
 10 files changed, 102 deletions(-)
 delete mode 100644 arch/arm/plat-mxc/devices/platform-mxc_pwm.c

diff --git a/arch/arm/mach-imx/clk-imx21.c b/arch/arm/mach-imx/clk-imx21.c
index ea13e61..63d5724 100644
--- a/arch/arm/mach-imx/clk-imx21.c
+++ b/arch/arm/mach-imx/clk-imx21.c
@@ -150,7 +150,6 @@ int __init mx21_clocks_init(unsigned long lref, unsigned long href)
 	clk_register_clkdev(clk[per1], "per", "imx-gpt.1");
 	clk_register_clkdev(clk[gpt3_ipg_gate], "ipg", "imx-gpt.2");
 	clk_register_clkdev(clk[per1], "per", "imx-gpt.2");
-	clk_register_clkdev(clk[pwm_ipg_gate], "pwm", "mxc_pwm.0");
 	clk_register_clkdev(clk[per2], "per", "imx21-cspi.0");
 	clk_register_clkdev(clk[cspi1_ipg_gate], "ipg", "imx21-cspi.0");
 	clk_register_clkdev(clk[per2], "per", "imx21-cspi.1");
diff --git a/arch/arm/mach-imx/clk-imx25.c b/arch/arm/mach-imx/clk-imx25.c
index fdd8cc8..1d40c6e 100644
--- a/arch/arm/mach-imx/clk-imx25.c
+++ b/arch/arm/mach-imx/clk-imx25.c
@@ -202,14 +202,6 @@ int __init mx25_clocks_init(void)
 	clk_register_clkdev(clk[cspi1_ipg], NULL, "imx35-cspi.0");
 	clk_register_clkdev(clk[cspi2_ipg], NULL, "imx35-cspi.1");
 	clk_register_clkdev(clk[cspi3_ipg], NULL, "imx35-cspi.2");
-	clk_register_clkdev(clk[pwm1_ipg], "ipg", "mxc_pwm.0");
-	clk_register_clkdev(clk[per10], "per", "mxc_pwm.0");
-	clk_register_clkdev(clk[pwm1_ipg], "ipg", "mxc_pwm.1");
-	clk_register_clkdev(clk[per10], "per", "mxc_pwm.1");
-	clk_register_clkdev(clk[pwm1_ipg], "ipg", "mxc_pwm.2");
-	clk_register_clkdev(clk[per10], "per", "mxc_pwm.2");
-	clk_register_clkdev(clk[pwm1_ipg], "ipg", "mxc_pwm.3");
-	clk_register_clkdev(clk[per10], "per", "mxc_pwm.3");
 	clk_register_clkdev(clk[kpp_ipg], NULL, "imx-keypad");
 	clk_register_clkdev(clk[tsc_ipg], NULL, "mx25-adc");
 	clk_register_clkdev(clk[i2c_ipg_per], NULL, "imx-i2c.0");
diff --git a/arch/arm/mach-imx/clk-imx27.c b/arch/arm/mach-imx/clk-imx27.c
index f69ca46..4043222 100644
--- a/arch/arm/mach-imx/clk-imx27.c
+++ b/arch/arm/mach-imx/clk-imx27.c
@@ -210,7 +210,6 @@ int __init mx27_clocks_init(unsigned long fref)
 	clk_register_clkdev(clk[per1_gate], "per", "imx-gpt.4");
 	clk_register_clkdev(clk[gpt6_ipg_gate], "ipg", "imx-gpt.5");
 	clk_register_clkdev(clk[per1_gate], "per", "imx-gpt.5");
-	clk_register_clkdev(clk[pwm_ipg_gate], NULL, "mxc_pwm.0");
 	clk_register_clkdev(clk[per2_gate], "per", "mxc-mmc.0");
 	clk_register_clkdev(clk[sdhc1_ipg_gate], "ipg", "mxc-mmc.0");
 	clk_register_clkdev(clk[per2_gate], "per", "mxc-mmc.1");
diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c b/arch/arm/mach-imx/clk-imx51-imx53.c
index b522411..cafe20f 100644
--- a/arch/arm/mach-imx/clk-imx51-imx53.c
+++ b/arch/arm/mach-imx/clk-imx51-imx53.c
@@ -254,8 +254,6 @@ static void __init mx5_clocks_common_init(unsigned long rate_ckil,
 	clk_register_clkdev(clk[ecspi2_per_gate], "per", "imx51-ecspi.1");
 	clk_register_clkdev(clk[ecspi2_ipg_gate], "ipg", "imx51-ecspi.1");
 	clk_register_clkdev(clk[cspi_ipg_gate], NULL, "imx35-cspi.2");
-	clk_register_clkdev(clk[pwm1_ipg_gate], "pwm", "mxc_pwm.0");
-	clk_register_clkdev(clk[pwm2_ipg_gate], "pwm", "mxc_pwm.1");
 	clk_register_clkdev(clk[i2c1_gate], NULL, "imx-i2c.0");
 	clk_register_clkdev(clk[i2c2_gate], NULL, "imx-i2c.1");
 	clk_register_clkdev(clk[usboh3_per_gate], "per", "mxc-ehci.0");
diff --git a/arch/arm/mach-imx/devices-imx25.h b/arch/arm/mach-imx/devices-imx25.h
index f8e03dd..441c422 100644
--- a/arch/arm/mach-imx/devices-imx25.h
+++ b/arch/arm/mach-imx/devices-imx25.h
@@ -84,7 +84,3 @@ extern const struct imx_spi_imx_data imx25_cspi_data[];
 #define imx25_add_spi_imx0(pdata)	imx25_add_spi_imx(0, pdata)
 #define imx25_add_spi_imx1(pdata)	imx25_add_spi_imx(1, pdata)
 #define imx25_add_spi_imx2(pdata)	imx25_add_spi_imx(2, pdata)
-
-extern struct imx_mxc_pwm_data imx25_mxc_pwm_data[];
-#define imx25_add_mxc_pwm(id)	\
-	imx_add_mxc_pwm(&imx25_mxc_pwm_data[id])
diff --git a/arch/arm/mach-imx/devices-imx51.h b/arch/arm/mach-imx/devices-imx51.h
index 9f17187..18643f7 100644
--- a/arch/arm/mach-imx/devices-imx51.h
+++ b/arch/arm/mach-imx/devices-imx51.h
@@ -58,10 +58,6 @@ extern const struct imx_imx2_wdt_data imx51_imx2_wdt_data[];
 #define imx51_add_imx2_wdt(id)	\
 	imx_add_imx2_wdt(&imx51_imx2_wdt_data[id])
 
-extern const struct imx_mxc_pwm_data imx51_mxc_pwm_data[];
-#define imx51_add_mxc_pwm(id)	\
-	imx_add_mxc_pwm(&imx51_mxc_pwm_data[id])
-
 extern const struct imx_imx_keypad_data imx51_imx_keypad_data;
 #define imx51_add_imx_keypad(pdata)	\
 	imx_add_imx_keypad(&imx51_imx_keypad_data, pdata)
diff --git a/arch/arm/plat-mxc/devices/Kconfig b/arch/arm/plat-mxc/devices/Kconfig
index cb3e3ee..478b014 100644
--- a/arch/arm/plat-mxc/devices/Kconfig
+++ b/arch/arm/plat-mxc/devices/Kconfig
@@ -61,9 +61,6 @@ config IMX_HAVE_PLATFORM_MXC_MMC
 config IMX_HAVE_PLATFORM_MXC_NAND
 	bool
 
-config IMX_HAVE_PLATFORM_MXC_PWM
-	bool
-
 config IMX_HAVE_PLATFORM_MXC_RNGA
 	bool
 	select ARCH_HAS_RNGA
diff --git a/arch/arm/plat-mxc/devices/Makefile b/arch/arm/plat-mxc/devices/Makefile
index c11ac84..b3bddeb 100644
--- a/arch/arm/plat-mxc/devices/Makefile
+++ b/arch/arm/plat-mxc/devices/Makefile
@@ -20,7 +20,6 @@ obj-$(CONFIG_IMX_HAVE_PLATFORM_MX2_CAMERA) += platform-mx2-camera.o
 obj-$(CONFIG_IMX_HAVE_PLATFORM_MXC_EHCI) += platform-mxc-ehci.o
 obj-$(CONFIG_IMX_HAVE_PLATFORM_MXC_MMC) += platform-mxc-mmc.o
 obj-$(CONFIG_IMX_HAVE_PLATFORM_MXC_NAND) += platform-mxc_nand.o
-obj-$(CONFIG_IMX_HAVE_PLATFORM_MXC_PWM) += platform-mxc_pwm.o
 obj-$(CONFIG_IMX_HAVE_PLATFORM_MXC_RNGA) += platform-mxc_rnga.o
 obj-$(CONFIG_IMX_HAVE_PLATFORM_MXC_RTC) += platform-mxc_rtc.o
 obj-$(CONFIG_IMX_HAVE_PLATFORM_MXC_W1) += platform-mxc_w1.o
diff --git a/arch/arm/plat-mxc/devices/platform-mxc_pwm.c b/arch/arm/plat-mxc/devices/platform-mxc_pwm.c
deleted file mode 100644
index b0c4ae2..0000000
--- a/arch/arm/plat-mxc/devices/platform-mxc_pwm.c
+++ /dev/null
@@ -1,69 +0,0 @@
-/*
- * Copyright (C) 2009-2010 Pengutronix
- * Uwe Kleine-Koenig <u.kleine-koenig@pengutronix.de>
- *
- * This program is free software; you can redistribute it and/or modify it under
- * the terms of the GNU General Public License version 2 as published by the
- * Free Software Foundation.
- */
-#include <mach/hardware.h>
-#include <mach/devices-common.h>
-
-#define imx_mxc_pwm_data_entry_single(soc, _id, _hwid, _size)		\
-	{								\
-		.id = _id,						\
-		.iobase = soc ## _PWM ## _hwid ## _BASE_ADDR,		\
-		.iosize = _size,					\
-		.irq = soc ## _INT_PWM ## _hwid,			\
-	}
-#define imx_mxc_pwm_data_entry(soc, _id, _hwid, _size)			\
-	[_id] = imx_mxc_pwm_data_entry_single(soc, _id, _hwid, _size)
-
-#ifdef CONFIG_SOC_IMX21
-const struct imx_mxc_pwm_data imx21_mxc_pwm_data __initconst =
-	imx_mxc_pwm_data_entry_single(MX21, 0, , SZ_4K);
-#endif /* ifdef CONFIG_SOC_IMX21 */
-
-#ifdef CONFIG_SOC_IMX25
-const struct imx_mxc_pwm_data imx25_mxc_pwm_data[] __initconst = {
-#define imx25_mxc_pwm_data_entry(_id, _hwid)				\
-	imx_mxc_pwm_data_entry(MX25, _id, _hwid, SZ_16K)
-	imx25_mxc_pwm_data_entry(0, 1),
-	imx25_mxc_pwm_data_entry(1, 2),
-	imx25_mxc_pwm_data_entry(2, 3),
-	imx25_mxc_pwm_data_entry(3, 4),
-};
-#endif /* ifdef CONFIG_SOC_IMX25 */
-
-#ifdef CONFIG_SOC_IMX27
-const struct imx_mxc_pwm_data imx27_mxc_pwm_data __initconst =
-	imx_mxc_pwm_data_entry_single(MX27, 0, , SZ_4K);
-#endif /* ifdef CONFIG_SOC_IMX27 */
-
-#ifdef CONFIG_SOC_IMX51
-const struct imx_mxc_pwm_data imx51_mxc_pwm_data[] __initconst = {
-#define imx51_mxc_pwm_data_entry(_id, _hwid)				\
-	imx_mxc_pwm_data_entry(MX51, _id, _hwid, SZ_16K)
-	imx51_mxc_pwm_data_entry(0, 1),
-	imx51_mxc_pwm_data_entry(1, 2),
-};
-#endif /* ifdef CONFIG_SOC_IMX51 */
-
-struct platform_device *__init imx_add_mxc_pwm(
-		const struct imx_mxc_pwm_data *data)
-{
-	struct resource res[] = {
-		{
-			.start = data->iobase,
-			.end = data->iobase + data->iosize - 1,
-			.flags = IORESOURCE_MEM,
-		}, {
-			.start = data->irq,
-			.end = data->irq,
-			.flags = IORESOURCE_IRQ,
-		},
-	};
-
-	return imx_add_platform_device("mxc_pwm", data->id,
-			res, ARRAY_SIZE(res), NULL, 0);
-}
diff --git a/arch/arm/plat-mxc/include/mach/devices-common.h b/arch/arm/plat-mxc/include/mach/devices-common.h
index a7f5bb1..e8c5f84 100644
--- a/arch/arm/plat-mxc/include/mach/devices-common.h
+++ b/arch/arm/plat-mxc/include/mach/devices-common.h
@@ -271,15 +271,6 @@ struct imx_pata_imx_data {
 struct platform_device *__init imx_add_pata_imx(
 		const struct imx_pata_imx_data *data);
 
-struct imx_mxc_pwm_data {
-	int id;
-	resource_size_t iobase;
-	resource_size_t iosize;
-	resource_size_t irq;
-};
-struct platform_device *__init imx_add_mxc_pwm(
-		const struct imx_mxc_pwm_data *data);
-
 /* mxc_rtc */
 struct imx_mxc_rtc_data {
 	resource_size_t iobase;
-- 
1.7.10.4

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

[Thierry Reding]

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

On Thu, Sep 06, 2012 at 02:48:06PM +0200, Sascha Hauer wrote:
> The following is the third version of the i.MX pwm series. I integrated
> the remaining comments from Shawn and Benoît and added their tags.
> 
> So Thierry, please pull the attached patches. The pull request only
> contains the pwm framework specific patches, the remaining two I'd
> like to push via the arm-soc tree in case the i.MX5 also gets devicetree
> clock lookups.

Hi Sascha,

Sorry for taking so long to get back to you. Apart from my comments to
patch 1, I really like how this series cleans up the i.MX driver. It's
already a bit late in the 3.6 release cycle for these kinds of changes
and none of them fix any regressions, so I'd like to queue them for
3.7 after my comments have been addressed.

Thierry

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

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

[Sascha Hauer]

On Fri, Sep 07, 2012 at 03:10:37PM +0200, Thierry Reding wrote:
> On Thu, Sep 06, 2012 at 02:48:06PM +0200, Sascha Hauer wrote:
> > The following is the third version of the i.MX pwm series. I integrated
> > the remaining comments from Shawn and Benoît and added their tags.
> > 
> > So Thierry, please pull the attached patches. The pull request only
> > contains the pwm framework specific patches, the remaining two I'd
> > like to push via the arm-soc tree in case the i.MX5 also gets devicetree
> > clock lookups.
> 
> Hi Sascha,
> 
> Sorry for taking so long to get back to you. Apart from my comments to
> patch 1, I really like how this series cleans up the i.MX driver. It's
> already a bit late in the 3.6 release cycle for these kinds of changes
> and none of them fix any regressions, so I'd like to queue them for
> 3.7 after my comments have been addressed.

Sure, no problem. The patches were never intended to go in for 3.6. I
will address your comments and resend on monday.

Thanks
 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 |