linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* i.MX pwm patches
@ 2012-08-28 11:48 Sascha Hauer
  2012-08-28 11:48 ` [PATCH 1/9] pwm i.MX: factor out SoC specific functions Sascha Hauer
                   ` (9 more replies)
  0 siblings, 10 replies; 20+ messages in thread
From: Sascha Hauer @ 2012-08-28 11:48 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: HACHIMI Samir, shawn.guo, thierry.reding, linux-kernel,
	Benoît Thébaudeau

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

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

* [PATCH 1/9] pwm i.MX: factor out SoC specific functions
  2012-08-28 11:48 i.MX pwm patches Sascha Hauer
@ 2012-08-28 11:48 ` Sascha Hauer
  2012-08-28 11:48 ` [PATCH 2/9] pwm i.MX: remove unnecessary if in pwm_[en|dis]able Sascha Hauer
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Sascha Hauer @ 2012-08-28 11:48 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: HACHIMI Samir, shawn.guo, thierry.reding, linux-kernel,
	Benoît Thébaudeau, 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


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

* [PATCH 2/9] pwm i.MX: remove unnecessary if in pwm_[en|dis]able
  2012-08-28 11:48 i.MX pwm patches Sascha Hauer
  2012-08-28 11:48 ` [PATCH 1/9] pwm i.MX: factor out SoC specific functions Sascha Hauer
@ 2012-08-28 11:48 ` Sascha Hauer
  2012-08-28 11:48 ` [PATCH 3/9] pwm i.MX: add functions to enable/disable pwm Sascha Hauer
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Sascha Hauer @ 2012-08-28 11:48 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: HACHIMI Samir, shawn.guo, thierry.reding, linux-kernel,
	Benoît Thébaudeau, 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


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

* [PATCH 3/9] pwm i.MX: add functions to enable/disable pwm.
  2012-08-28 11:48 i.MX pwm patches Sascha Hauer
  2012-08-28 11:48 ` [PATCH 1/9] pwm i.MX: factor out SoC specific functions Sascha Hauer
  2012-08-28 11:48 ` [PATCH 2/9] pwm i.MX: remove unnecessary if in pwm_[en|dis]able Sascha Hauer
@ 2012-08-28 11:48 ` Sascha Hauer
  2012-08-28 11:48 ` [PATCH 4/9] pwm i.MX: Use module_platform_driver Sascha Hauer
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Sascha Hauer @ 2012-08-28 11:48 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: HACHIMI Samir, shawn.guo, thierry.reding, linux-kernel,
	Benoît Thébaudeau, 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


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

* [PATCH 4/9] pwm i.MX: Use module_platform_driver
  2012-08-28 11:48 i.MX pwm patches Sascha Hauer
                   ` (2 preceding siblings ...)
  2012-08-28 11:48 ` [PATCH 3/9] pwm i.MX: add functions to enable/disable pwm Sascha Hauer
@ 2012-08-28 11:48 ` Sascha Hauer
  2012-08-28 11:48 ` [PATCH 5/9] pwm i.MX: add devicetree support Sascha Hauer
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Sascha Hauer @ 2012-08-28 11:48 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: HACHIMI Samir, shawn.guo, thierry.reding, linux-kernel,
	Benoît Thébaudeau, 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


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

* [PATCH 5/9] pwm i.MX: add devicetree support
  2012-08-28 11:48 i.MX pwm patches Sascha Hauer
                   ` (3 preceding siblings ...)
  2012-08-28 11:48 ` [PATCH 4/9] pwm i.MX: Use module_platform_driver Sascha Hauer
@ 2012-08-28 11:48 ` Sascha Hauer
  2012-08-30 22:26   ` Shawn Guo
  2012-08-28 11:48 ` [PATCH 6/9] pwm i.MX: use per clock unconditionally Sascha Hauer
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Sascha Hauer @ 2012-08-28 11:48 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: HACHIMI Samir, shawn.guo, thierry.reding, linux-kernel,
	Benoît Thébaudeau, Philipp Zabel, 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


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

* [PATCH 6/9] pwm i.MX: use per clock unconditionally
  2012-08-28 11:48 i.MX pwm patches Sascha Hauer
                   ` (4 preceding siblings ...)
  2012-08-28 11:48 ` [PATCH 5/9] pwm i.MX: add devicetree support Sascha Hauer
@ 2012-08-28 11:48 ` Sascha Hauer
  2012-08-28 11:48 ` [PATCH 7/9] pwm i.MX: fix clock lookup Sascha Hauer
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Sascha Hauer @ 2012-08-28 11:48 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: HACHIMI Samir, shawn.guo, thierry.reding, linux-kernel,
	Benoît Thébaudeau, 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


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

* [PATCH 7/9] pwm i.MX: fix clock lookup
  2012-08-28 11:48 i.MX pwm patches Sascha Hauer
                   ` (5 preceding siblings ...)
  2012-08-28 11:48 ` [PATCH 6/9] pwm i.MX: use per clock unconditionally Sascha Hauer
@ 2012-08-28 11:48 ` Sascha Hauer
  2012-08-28 11:48 ` [PATCH 8/9] ARM i.MX53: Add pwms to dtsi Sascha Hauer
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Sascha Hauer @ 2012-08-28 11:48 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: HACHIMI Samir, shawn.guo, thierry.reding, linux-kernel,
	Benoît Thébaudeau, Philipp Zabel, 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


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

* [PATCH 8/9] ARM i.MX53: Add pwms to dtsi
  2012-08-28 11:48 i.MX pwm patches Sascha Hauer
                   ` (6 preceding siblings ...)
  2012-08-28 11:48 ` [PATCH 7/9] pwm i.MX: fix clock lookup Sascha Hauer
@ 2012-08-28 11:48 ` Sascha Hauer
  2012-08-30 22:32   ` Shawn Guo
  2012-08-28 11:48 ` [PATCH 9/9] pwm i.MX: add devicetree support Sascha Hauer
  2012-08-30 21:45 ` i.MX pwm patches Shawn Guo
  9 siblings, 1 reply; 20+ messages in thread
From: Sascha Hauer @ 2012-08-28 11:48 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: HACHIMI Samir, shawn.guo, thierry.reding, linux-kernel,
	Benoît Thébaudeau, 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


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

* [PATCH 9/9] pwm i.MX: add devicetree support
  2012-08-28 11:48 i.MX pwm patches Sascha Hauer
                   ` (7 preceding siblings ...)
  2012-08-28 11:48 ` [PATCH 8/9] ARM i.MX53: Add pwms to dtsi Sascha Hauer
@ 2012-08-28 11:48 ` Sascha Hauer
  2012-08-30 22:28   ` Shawn Guo
  2012-08-30 21:45 ` i.MX pwm patches Shawn Guo
  9 siblings, 1 reply; 20+ messages in thread
From: Sascha Hauer @ 2012-08-28 11:48 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: HACHIMI Samir, shawn.guo, thierry.reding, linux-kernel,
	Benoît Thébaudeau, Philipp Zabel, 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


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

* Re: i.MX pwm patches
  2012-08-28 11:48 i.MX pwm patches Sascha Hauer
                   ` (8 preceding siblings ...)
  2012-08-28 11:48 ` [PATCH 9/9] pwm i.MX: add devicetree support Sascha Hauer
@ 2012-08-30 21:45 ` Shawn Guo
  2012-08-31 13:05   ` Sascha Hauer
  9 siblings, 1 reply; 20+ messages in thread
From: Shawn Guo @ 2012-08-30 21:45 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-arm-kernel, HACHIMI Samir, thierry.reding, linux-kernel,
	Benoît Thébaudeau

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

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

* Re: [PATCH 5/9] pwm i.MX: add devicetree support
  2012-08-28 11:48 ` [PATCH 5/9] pwm i.MX: add devicetree support Sascha Hauer
@ 2012-08-30 22:26   ` Shawn Guo
  0 siblings, 0 replies; 20+ messages in thread
From: Shawn Guo @ 2012-08-30 22:26 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-arm-kernel, HACHIMI Samir, thierry.reding, linux-kernel,
	Benoît Thébaudeau, Philipp Zabel

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
> 

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

* Re: [PATCH 9/9] pwm i.MX: add devicetree support
  2012-08-28 11:48 ` [PATCH 9/9] pwm i.MX: add devicetree support Sascha Hauer
@ 2012-08-30 22:28   ` Shawn Guo
  0 siblings, 0 replies; 20+ messages in thread
From: Shawn Guo @ 2012-08-30 22:28 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-arm-kernel, HACHIMI Samir, thierry.reding, linux-kernel,
	Benoît Thébaudeau, Philipp Zabel

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
> 

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

* Re: [PATCH 8/9] ARM i.MX53: Add pwms to dtsi
  2012-08-28 11:48 ` [PATCH 8/9] ARM i.MX53: Add pwms to dtsi Sascha Hauer
@ 2012-08-30 22:32   ` Shawn Guo
  2012-08-31 13:07     ` Sascha Hauer
  0 siblings, 1 reply; 20+ messages in thread
From: Shawn Guo @ 2012-08-30 22:32 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-arm-kernel, HACHIMI Samir, thierry.reding, linux-kernel,
	Benoît Thébaudeau

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

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

* Re: [PATCH 8/9] ARM i.MX53: Add pwms to dtsi
  2012-08-31 13:07     ` Sascha Hauer
@ 2012-08-31  0:16       ` Shawn Guo
  2012-09-07 13:29       ` Thierry Reding
  1 sibling, 0 replies; 20+ messages in thread
From: Shawn Guo @ 2012-08-31  0:16 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-arm-kernel, HACHIMI Samir, thierry.reding, linux-kernel,
	Benoît Thébaudeau

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

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

* Re: i.MX pwm patches
  2012-08-30 21:45 ` i.MX pwm patches Shawn Guo
@ 2012-08-31 13:05   ` Sascha Hauer
  0 siblings, 0 replies; 20+ messages in thread
From: Sascha Hauer @ 2012-08-31 13:05 UTC (permalink / raw)
  To: Shawn Guo
  Cc: linux-arm-kernel, HACHIMI Samir, thierry.reding, linux-kernel,
	Benoît Thébaudeau

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 |

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

* Re: [PATCH 8/9] ARM i.MX53: Add pwms to dtsi
  2012-08-30 22:32   ` Shawn Guo
@ 2012-08-31 13:07     ` Sascha Hauer
  2012-08-31  0:16       ` Shawn Guo
  2012-09-07 13:29       ` Thierry Reding
  0 siblings, 2 replies; 20+ messages in thread
From: Sascha Hauer @ 2012-08-31 13:07 UTC (permalink / raw)
  To: Shawn Guo
  Cc: linux-arm-kernel, HACHIMI Samir, thierry.reding, linux-kernel,
	Benoît Thébaudeau

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 |

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

* Re: [PATCH 8/9] ARM i.MX53: Add pwms to dtsi
  2012-08-31 13:07     ` Sascha Hauer
  2012-08-31  0:16       ` Shawn Guo
@ 2012-09-07 13:29       ` Thierry Reding
  2012-09-07 17:26         ` Sascha Hauer
  1 sibling, 1 reply; 20+ messages in thread
From: Thierry Reding @ 2012-09-07 13:29 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Shawn Guo, linux-arm-kernel, HACHIMI Samir, linux-kernel,
	Benoît Thébaudeau

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

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

* Re: [PATCH 8/9] ARM i.MX53: Add pwms to dtsi
  2012-09-07 13:29       ` Thierry Reding
@ 2012-09-07 17:26         ` Sascha Hauer
  2012-09-07 20:10           ` Thierry Reding
  0 siblings, 1 reply; 20+ messages in thread
From: Sascha Hauer @ 2012-09-07 17:26 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Shawn Guo, linux-arm-kernel, HACHIMI Samir, linux-kernel,
	Benoît Thébaudeau

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 |

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

* Re: [PATCH 8/9] ARM i.MX53: Add pwms to dtsi
  2012-09-07 17:26         ` Sascha Hauer
@ 2012-09-07 20:10           ` Thierry Reding
  0 siblings, 0 replies; 20+ messages in thread
From: Thierry Reding @ 2012-09-07 20:10 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Shawn Guo, linux-arm-kernel, HACHIMI Samir, linux-kernel,
	Benoît Thébaudeau

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

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

end of thread, other threads:[~2012-09-07 20:10 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-28 11:48 i.MX pwm patches Sascha Hauer
2012-08-28 11:48 ` [PATCH 1/9] pwm i.MX: factor out SoC specific functions Sascha Hauer
2012-08-28 11:48 ` [PATCH 2/9] pwm i.MX: remove unnecessary if in pwm_[en|dis]able Sascha Hauer
2012-08-28 11:48 ` [PATCH 3/9] pwm i.MX: add functions to enable/disable pwm Sascha Hauer
2012-08-28 11:48 ` [PATCH 4/9] pwm i.MX: Use module_platform_driver Sascha Hauer
2012-08-28 11:48 ` [PATCH 5/9] pwm i.MX: add devicetree support Sascha Hauer
2012-08-30 22:26   ` Shawn Guo
2012-08-28 11:48 ` [PATCH 6/9] pwm i.MX: use per clock unconditionally Sascha Hauer
2012-08-28 11:48 ` [PATCH 7/9] pwm i.MX: fix clock lookup Sascha Hauer
2012-08-28 11:48 ` [PATCH 8/9] ARM i.MX53: Add pwms to dtsi Sascha Hauer
2012-08-30 22:32   ` Shawn Guo
2012-08-31 13:07     ` Sascha Hauer
2012-08-31  0:16       ` Shawn Guo
2012-09-07 13:29       ` Thierry Reding
2012-09-07 17:26         ` Sascha Hauer
2012-09-07 20:10           ` Thierry Reding
2012-08-28 11:48 ` [PATCH 9/9] pwm i.MX: add devicetree support Sascha Hauer
2012-08-30 22:28   ` Shawn Guo
2012-08-30 21:45 ` i.MX pwm patches Shawn Guo
2012-08-31 13:05   ` Sascha Hauer

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