linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework
@ 2012-09-10 12:05 Thierry Reding
  2012-09-10 12:05 ` [PATCH v2 1/3] MIPS: JZ4740: Break circular header dependency Thierry Reding
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Thierry Reding @ 2012-09-10 12:05 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: linux-mips, linux-kernel, Antony Pavlov, Lars-Peter Clausen,
	Maarten ter Huurne

Hi,

This small series fixes a build error due to a circular header
dependency, exports the timer API so it can be used outside of
the arch/mips/jz4740 tree and finally moves and converts the
JZ4740 PWM driver to the PWM framework.

Note that I don't have any hardware to test this on, so I had to
rely on compile tests only. Patches 1 and 2 should probably go
through the MIPS tree, while I can take patch 3 through the PWM
tree. It touches a couple of files in arch/mips but the changes
are unlikely to cause conflicts.

Thierry

Thierry Reding (3):
  MIPS: JZ4740: Break circular header dependency
  MIPS: JZ4740: Export timer API
  pwm: Add Ingenic JZ4740 support

 arch/mips/include/asm/mach-jz4740/irq.h      |   5 +
 arch/mips/include/asm/mach-jz4740/platform.h |   1 +
 arch/mips/include/asm/mach-jz4740/timer.h    | 113 ++++++++++++++
 arch/mips/jz4740/Kconfig                     |   3 -
 arch/mips/jz4740/Makefile                    |   2 +-
 arch/mips/jz4740/board-qi_lb60.c             |   1 +
 arch/mips/jz4740/irq.h                       |  23 ---
 arch/mips/jz4740/platform.c                  |   6 +
 arch/mips/jz4740/pwm.c                       | 177 ---------------------
 arch/mips/jz4740/time.c                      |   2 +-
 arch/mips/jz4740/timer.c                     |   4 +-
 arch/mips/jz4740/timer.h                     | 136 -----------------
 drivers/pwm/Kconfig                          |  12 +-
 drivers/pwm/Makefile                         |   1 +
 drivers/pwm/pwm-jz4740.c                     | 221 +++++++++++++++++++++++++++
 15 files changed, 363 insertions(+), 344 deletions(-)
 delete mode 100644 arch/mips/jz4740/irq.h
 delete mode 100644 arch/mips/jz4740/pwm.c
 delete mode 100644 arch/mips/jz4740/timer.h
 create mode 100644 drivers/pwm/pwm-jz4740.c

-- 
1.7.12


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

* [PATCH v2 1/3] MIPS: JZ4740: Break circular header dependency
  2012-09-10 12:05 [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework Thierry Reding
@ 2012-09-10 12:05 ` Thierry Reding
  2012-09-10 12:05 ` [PATCH v2 2/3] MIPS: JZ4740: Export timer API Thierry Reding
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Thierry Reding @ 2012-09-10 12:05 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: linux-mips, linux-kernel, Antony Pavlov, Lars-Peter Clausen,
	Maarten ter Huurne

When including irq.h, arch/mips/jz4740/irq.h will be selected as the
first candidate. This header does not include the proper definitions
(most notably NR_IRQS) required by subsequent headers. To solve this
arch/mips/jz4740/irq.h can be deleted and its contents can be moved
into arch/mips/include/asm/mach-jz4740/irq.h, which will then be
correctly included.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
---
 arch/mips/include/asm/mach-jz4740/irq.h |  5 +++++
 arch/mips/jz4740/irq.h                  | 23 -----------------------
 2 files changed, 5 insertions(+), 23 deletions(-)
 delete mode 100644 arch/mips/jz4740/irq.h

diff --git a/arch/mips/include/asm/mach-jz4740/irq.h b/arch/mips/include/asm/mach-jz4740/irq.h
index 5ad1a9c..aa6fd90 100644
--- a/arch/mips/include/asm/mach-jz4740/irq.h
+++ b/arch/mips/include/asm/mach-jz4740/irq.h
@@ -54,4 +54,9 @@
 
 #define NR_IRQS (JZ4740_IRQ_ADC_BASE + 6)
 
+struct irq_data;
+
+extern void jz4740_irq_suspend(struct irq_data *data);
+extern void jz4740_irq_resume(struct irq_data *data);
+
 #endif
diff --git a/arch/mips/jz4740/irq.h b/arch/mips/jz4740/irq.h
deleted file mode 100644
index f75e39d..0000000
--- a/arch/mips/jz4740/irq.h
+++ /dev/null
@@ -1,23 +0,0 @@
-/*
- *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
- *
- *  This program is free software; you can redistribute it and/or modify it
- *  under  the terms of the GNU General  Public License as published by the
- *  Free Software Foundation;  either version 2 of the License, or (at your
- *  option) any later version.
- *
- *  You should have received a copy of the GNU General Public License along
- *  with this program; if not, write to the Free Software Foundation, Inc.,
- *  675 Mass Ave, Cambridge, MA 02139, USA.
- *
- */
-
-#ifndef __MIPS_JZ4740_IRQ_H__
-#define __MIPS_JZ4740_IRQ_H__
-
-#include <linux/irq.h>
-
-extern void jz4740_irq_suspend(struct irq_data *data);
-extern void jz4740_irq_resume(struct irq_data *data);
-
-#endif
-- 
1.7.12


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

* [PATCH v2 2/3] MIPS: JZ4740: Export timer API
  2012-09-10 12:05 [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework Thierry Reding
  2012-09-10 12:05 ` [PATCH v2 1/3] MIPS: JZ4740: Break circular header dependency Thierry Reding
@ 2012-09-10 12:05 ` Thierry Reding
  2012-09-10 12:05 ` [PATCH v2 3/3] pwm: Add Ingenic JZ4740 support Thierry Reding
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Thierry Reding @ 2012-09-10 12:05 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: linux-mips, linux-kernel, Antony Pavlov, Lars-Peter Clausen,
	Maarten ter Huurne

This is a prerequisite for allowing the PWM driver to be converted to
the PWM framework.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
---
Changes in v2:
- keep timer API as static inline, move it to public header instead

 arch/mips/include/asm/mach-jz4740/timer.h | 113 +++++++++++++++++++++++++
 arch/mips/jz4740/time.c                   |   2 +-
 arch/mips/jz4740/timer.c                  |   4 +-
 arch/mips/jz4740/timer.h                  | 136 ------------------------------
 4 files changed, 116 insertions(+), 139 deletions(-)
 delete mode 100644 arch/mips/jz4740/timer.h

diff --git a/arch/mips/include/asm/mach-jz4740/timer.h b/arch/mips/include/asm/mach-jz4740/timer.h
index 9baa03c..a7759fb 100644
--- a/arch/mips/include/asm/mach-jz4740/timer.h
+++ b/arch/mips/include/asm/mach-jz4740/timer.h
@@ -16,7 +16,120 @@
 #ifndef __ASM_MACH_JZ4740_TIMER
 #define __ASM_MACH_JZ4740_TIMER
 
+#define JZ_REG_TIMER_STOP		0x0C
+#define JZ_REG_TIMER_STOP_SET		0x1C
+#define JZ_REG_TIMER_STOP_CLEAR		0x2C
+#define JZ_REG_TIMER_ENABLE		0x00
+#define JZ_REG_TIMER_ENABLE_SET		0x04
+#define JZ_REG_TIMER_ENABLE_CLEAR	0x08
+#define JZ_REG_TIMER_FLAG		0x10
+#define JZ_REG_TIMER_FLAG_SET		0x14
+#define JZ_REG_TIMER_FLAG_CLEAR		0x18
+#define JZ_REG_TIMER_MASK		0x20
+#define JZ_REG_TIMER_MASK_SET		0x24
+#define JZ_REG_TIMER_MASK_CLEAR		0x28
+
+#define JZ_REG_TIMER_DFR(x) (((x) * 0x10) + 0x30)
+#define JZ_REG_TIMER_DHR(x) (((x) * 0x10) + 0x34)
+#define JZ_REG_TIMER_CNT(x) (((x) * 0x10) + 0x38)
+#define JZ_REG_TIMER_CTRL(x) (((x) * 0x10) + 0x3C)
+
+#define JZ_TIMER_IRQ_HALF(x) BIT((x) + 0x10)
+#define JZ_TIMER_IRQ_FULL(x) BIT(x)
+
+#define JZ_TIMER_CTRL_PWM_ABBRUPT_SHUTDOWN	BIT(9)
+#define JZ_TIMER_CTRL_PWM_ACTIVE_LOW		BIT(8)
+#define JZ_TIMER_CTRL_PWM_ENABLE		BIT(7)
+#define JZ_TIMER_CTRL_PRESCALE_MASK		0x1c
+#define JZ_TIMER_CTRL_PRESCALE_OFFSET		0x3
+#define JZ_TIMER_CTRL_PRESCALE_1		(0 << 3)
+#define JZ_TIMER_CTRL_PRESCALE_4		(1 << 3)
+#define JZ_TIMER_CTRL_PRESCALE_16		(2 << 3)
+#define JZ_TIMER_CTRL_PRESCALE_64		(3 << 3)
+#define JZ_TIMER_CTRL_PRESCALE_256		(4 << 3)
+#define JZ_TIMER_CTRL_PRESCALE_1024		(5 << 3)
+
+#define JZ_TIMER_CTRL_PRESCALER(x) ((x) << JZ_TIMER_CTRL_PRESCALE_OFFSET)
+
+#define JZ_TIMER_CTRL_SRC_EXT		BIT(2)
+#define JZ_TIMER_CTRL_SRC_RTC		BIT(1)
+#define JZ_TIMER_CTRL_SRC_PCLK		BIT(0)
+
+extern void __iomem *jz4740_timer_base;
+void __init jz4740_timer_init(void);
+
 void jz4740_timer_enable_watchdog(void);
 void jz4740_timer_disable_watchdog(void);
 
+static inline void jz4740_timer_stop(unsigned int timer)
+{
+	writel(BIT(timer), jz4740_timer_base + JZ_REG_TIMER_STOP_SET);
+}
+
+static inline void jz4740_timer_start(unsigned int timer)
+{
+	writel(BIT(timer), jz4740_timer_base + JZ_REG_TIMER_STOP_CLEAR);
+}
+
+static inline bool jz4740_timer_is_enabled(unsigned int timer)
+{
+	return readb(jz4740_timer_base + JZ_REG_TIMER_ENABLE) & BIT(timer);
+}
+
+static inline void jz4740_timer_enable(unsigned int timer)
+{
+	writeb(BIT(timer), jz4740_timer_base + JZ_REG_TIMER_ENABLE_SET);
+}
+
+static inline void jz4740_timer_disable(unsigned int timer)
+{
+	writeb(BIT(timer), jz4740_timer_base + JZ_REG_TIMER_ENABLE_CLEAR);
+}
+
+static inline void jz4740_timer_set_period(unsigned int timer, uint16_t period)
+{
+	writew(period, jz4740_timer_base + JZ_REG_TIMER_DFR(timer));
+}
+
+static inline void jz4740_timer_set_duty(unsigned int timer, uint16_t duty)
+{
+	writew(duty, jz4740_timer_base + JZ_REG_TIMER_DHR(timer));
+}
+
+static inline void jz4740_timer_set_count(unsigned int timer, uint16_t count)
+{
+	writew(count, jz4740_timer_base + JZ_REG_TIMER_CNT(timer));
+}
+
+static inline uint16_t jz4740_timer_get_count(unsigned int timer)
+{
+	return readw(jz4740_timer_base + JZ_REG_TIMER_CNT(timer));
+}
+
+static inline void jz4740_timer_ack_full(unsigned int timer)
+{
+	writel(JZ_TIMER_IRQ_FULL(timer), jz4740_timer_base + JZ_REG_TIMER_FLAG_CLEAR);
+}
+
+static inline void jz4740_timer_irq_full_enable(unsigned int timer)
+{
+	writel(JZ_TIMER_IRQ_FULL(timer), jz4740_timer_base + JZ_REG_TIMER_FLAG_CLEAR);
+	writel(JZ_TIMER_IRQ_FULL(timer), jz4740_timer_base + JZ_REG_TIMER_MASK_CLEAR);
+}
+
+static inline void jz4740_timer_irq_full_disable(unsigned int timer)
+{
+	writel(JZ_TIMER_IRQ_FULL(timer), jz4740_timer_base + JZ_REG_TIMER_MASK_SET);
+}
+
+static inline void jz4740_timer_set_ctrl(unsigned int timer, uint16_t ctrl)
+{
+	writew(ctrl, jz4740_timer_base + JZ_REG_TIMER_CTRL(timer));
+}
+
+static inline uint16_t jz4740_timer_get_ctrl(unsigned int timer)
+{
+	return readw(jz4740_timer_base + JZ_REG_TIMER_CTRL(timer));
+}
+
 #endif
diff --git a/arch/mips/jz4740/time.c b/arch/mips/jz4740/time.c
index f83c2dd..39bb4bb 100644
--- a/arch/mips/jz4740/time.c
+++ b/arch/mips/jz4740/time.c
@@ -20,10 +20,10 @@
 #include <linux/clockchips.h>
 
 #include <asm/mach-jz4740/irq.h>
+#include <asm/mach-jz4740/timer.h>
 #include <asm/time.h>
 
 #include "clock.h"
-#include "timer.h"
 
 #define TIMER_CLOCKEVENT 0
 #define TIMER_CLOCKSOURCE 1
diff --git a/arch/mips/jz4740/timer.c b/arch/mips/jz4740/timer.c
index 654d5c3..22f11d7 100644
--- a/arch/mips/jz4740/timer.c
+++ b/arch/mips/jz4740/timer.c
@@ -17,11 +17,11 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 
-#include "timer.h"
-
 #include <asm/mach-jz4740/base.h>
+#include <asm/mach-jz4740/timer.h>
 
 void __iomem *jz4740_timer_base;
+EXPORT_SYMBOL_GPL(jz4740_timer_base);
 
 void jz4740_timer_enable_watchdog(void)
 {
diff --git a/arch/mips/jz4740/timer.h b/arch/mips/jz4740/timer.h
deleted file mode 100644
index fca3994..0000000
--- a/arch/mips/jz4740/timer.h
+++ /dev/null
@@ -1,136 +0,0 @@
-/*
- *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
- *  JZ4740 platform timer support
- *
- *  This program is free software; you can redistribute it and/or modify it
- *  under  the terms of the GNU General  Public License as published by the
- *  Free Software Foundation;  either version 2 of the License, or (at your
- *  option) any later version.
- *
- *  You should have received a copy of the GNU General Public License along
- *  with this program; if not, write to the Free Software Foundation, Inc.,
- *  675 Mass Ave, Cambridge, MA 02139, USA.
- *
- */
-
-#ifndef __MIPS_JZ4740_TIMER_H__
-#define __MIPS_JZ4740_TIMER_H__
-
-#include <linux/module.h>
-#include <linux/io.h>
-
-#define JZ_REG_TIMER_STOP		0x0C
-#define JZ_REG_TIMER_STOP_SET		0x1C
-#define JZ_REG_TIMER_STOP_CLEAR		0x2C
-#define JZ_REG_TIMER_ENABLE		0x00
-#define JZ_REG_TIMER_ENABLE_SET		0x04
-#define JZ_REG_TIMER_ENABLE_CLEAR	0x08
-#define JZ_REG_TIMER_FLAG		0x10
-#define JZ_REG_TIMER_FLAG_SET		0x14
-#define JZ_REG_TIMER_FLAG_CLEAR		0x18
-#define JZ_REG_TIMER_MASK		0x20
-#define JZ_REG_TIMER_MASK_SET		0x24
-#define JZ_REG_TIMER_MASK_CLEAR		0x28
-
-#define JZ_REG_TIMER_DFR(x) (((x) * 0x10) + 0x30)
-#define JZ_REG_TIMER_DHR(x) (((x) * 0x10) + 0x34)
-#define JZ_REG_TIMER_CNT(x) (((x) * 0x10) + 0x38)
-#define JZ_REG_TIMER_CTRL(x) (((x) * 0x10) + 0x3C)
-
-#define JZ_TIMER_IRQ_HALF(x) BIT((x) + 0x10)
-#define JZ_TIMER_IRQ_FULL(x) BIT(x)
-
-#define JZ_TIMER_CTRL_PWM_ABBRUPT_SHUTDOWN	BIT(9)
-#define JZ_TIMER_CTRL_PWM_ACTIVE_LOW		BIT(8)
-#define JZ_TIMER_CTRL_PWM_ENABLE		BIT(7)
-#define JZ_TIMER_CTRL_PRESCALE_MASK		0x1c
-#define JZ_TIMER_CTRL_PRESCALE_OFFSET		0x3
-#define JZ_TIMER_CTRL_PRESCALE_1		(0 << 3)
-#define JZ_TIMER_CTRL_PRESCALE_4		(1 << 3)
-#define JZ_TIMER_CTRL_PRESCALE_16		(2 << 3)
-#define JZ_TIMER_CTRL_PRESCALE_64		(3 << 3)
-#define JZ_TIMER_CTRL_PRESCALE_256		(4 << 3)
-#define JZ_TIMER_CTRL_PRESCALE_1024		(5 << 3)
-
-#define JZ_TIMER_CTRL_PRESCALER(x) ((x) << JZ_TIMER_CTRL_PRESCALE_OFFSET)
-
-#define JZ_TIMER_CTRL_SRC_EXT		BIT(2)
-#define JZ_TIMER_CTRL_SRC_RTC		BIT(1)
-#define JZ_TIMER_CTRL_SRC_PCLK		BIT(0)
-
-extern void __iomem *jz4740_timer_base;
-void __init jz4740_timer_init(void);
-
-static inline void jz4740_timer_stop(unsigned int timer)
-{
-	writel(BIT(timer), jz4740_timer_base + JZ_REG_TIMER_STOP_SET);
-}
-
-static inline void jz4740_timer_start(unsigned int timer)
-{
-	writel(BIT(timer), jz4740_timer_base + JZ_REG_TIMER_STOP_CLEAR);
-}
-
-static inline bool jz4740_timer_is_enabled(unsigned int timer)
-{
-	return readb(jz4740_timer_base + JZ_REG_TIMER_ENABLE) & BIT(timer);
-}
-
-static inline void jz4740_timer_enable(unsigned int timer)
-{
-	writeb(BIT(timer), jz4740_timer_base + JZ_REG_TIMER_ENABLE_SET);
-}
-
-static inline void jz4740_timer_disable(unsigned int timer)
-{
-	writeb(BIT(timer), jz4740_timer_base + JZ_REG_TIMER_ENABLE_CLEAR);
-}
-
-
-static inline void jz4740_timer_set_period(unsigned int timer, uint16_t period)
-{
-	writew(period, jz4740_timer_base + JZ_REG_TIMER_DFR(timer));
-}
-
-static inline void jz4740_timer_set_duty(unsigned int timer, uint16_t duty)
-{
-	writew(duty, jz4740_timer_base + JZ_REG_TIMER_DHR(timer));
-}
-
-static inline void jz4740_timer_set_count(unsigned int timer, uint16_t count)
-{
-	writew(count, jz4740_timer_base + JZ_REG_TIMER_CNT(timer));
-}
-
-static inline uint16_t jz4740_timer_get_count(unsigned int timer)
-{
-	return readw(jz4740_timer_base + JZ_REG_TIMER_CNT(timer));
-}
-
-static inline void jz4740_timer_ack_full(unsigned int timer)
-{
-	writel(JZ_TIMER_IRQ_FULL(timer), jz4740_timer_base + JZ_REG_TIMER_FLAG_CLEAR);
-}
-
-static inline void jz4740_timer_irq_full_enable(unsigned int timer)
-{
-	writel(JZ_TIMER_IRQ_FULL(timer), jz4740_timer_base + JZ_REG_TIMER_FLAG_CLEAR);
-	writel(JZ_TIMER_IRQ_FULL(timer), jz4740_timer_base + JZ_REG_TIMER_MASK_CLEAR);
-}
-
-static inline void jz4740_timer_irq_full_disable(unsigned int timer)
-{
-	writel(JZ_TIMER_IRQ_FULL(timer), jz4740_timer_base + JZ_REG_TIMER_MASK_SET);
-}
-
-static inline void jz4740_timer_set_ctrl(unsigned int timer, uint16_t ctrl)
-{
-	writew(ctrl, jz4740_timer_base + JZ_REG_TIMER_CTRL(timer));
-}
-
-static inline uint16_t jz4740_timer_get_ctrl(unsigned int timer)
-{
-	return readw(jz4740_timer_base + JZ_REG_TIMER_CTRL(timer));
-}
-
-#endif
-- 
1.7.12


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

* [PATCH v2 3/3] pwm: Add Ingenic JZ4740 support
  2012-09-10 12:05 [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework Thierry Reding
  2012-09-10 12:05 ` [PATCH v2 1/3] MIPS: JZ4740: Break circular header dependency Thierry Reding
  2012-09-10 12:05 ` [PATCH v2 2/3] MIPS: JZ4740: Export timer API Thierry Reding
@ 2012-09-10 12:05 ` Thierry Reding
  2012-09-10 21:51   ` Lars-Peter Clausen
  2012-09-10 15:20 ` [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework Lars-Peter Clausen
  2012-09-22  7:41 ` Thierry Reding
  4 siblings, 1 reply; 15+ messages in thread
From: Thierry Reding @ 2012-09-10 12:05 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: linux-mips, linux-kernel, Antony Pavlov, Lars-Peter Clausen,
	Maarten ter Huurne

This commit moves the driver to drivers/pwm and converts it to the new
PWM framework.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
---
Changes in v2:
- remove PWM core hunk that slipped in by mistake
- change pwm-beeper platform data back to 4
- refer to timer.h by full path
- register PWMs 0 and 1 but reserve them for system tasks
- replace printk by dev_err now that a proper device is available
- use jz4740_pwm_{enable,disable}() instead of pwm_{enable,disable}()
  internally
- make THIS_MODULE the owner of jz4740_pwm_ops and jz4740_pwm_driver
- add __devinit and __devexit annotations to jz4740_pwm_probe() and
  jz4740_pwm_remove() respectively
- add MODULE_AUTHOR(), MODULE_DESCRIPTION(), MODULE_ALIAS() and
  MODULE_LICENSE()

 arch/mips/include/asm/mach-jz4740/platform.h |   1 +
 arch/mips/jz4740/Kconfig                     |   3 -
 arch/mips/jz4740/Makefile                    |   2 +-
 arch/mips/jz4740/board-qi_lb60.c             |   1 +
 arch/mips/jz4740/platform.c                  |   6 +
 arch/mips/jz4740/pwm.c                       | 177 ---------------------
 drivers/pwm/Kconfig                          |  12 +-
 drivers/pwm/Makefile                         |   1 +
 drivers/pwm/pwm-jz4740.c                     | 221 +++++++++++++++++++++++++++
 9 files changed, 242 insertions(+), 182 deletions(-)
 delete mode 100644 arch/mips/jz4740/pwm.c
 create mode 100644 drivers/pwm/pwm-jz4740.c

diff --git a/arch/mips/include/asm/mach-jz4740/platform.h b/arch/mips/include/asm/mach-jz4740/platform.h
index 564ab81..163e81d 100644
--- a/arch/mips/include/asm/mach-jz4740/platform.h
+++ b/arch/mips/include/asm/mach-jz4740/platform.h
@@ -31,6 +31,7 @@ extern struct platform_device jz4740_pcm_device;
 extern struct platform_device jz4740_codec_device;
 extern struct platform_device jz4740_adc_device;
 extern struct platform_device jz4740_wdt_device;
+extern struct platform_device jz4740_pwm_device;
 
 void jz4740_serial_device_register(void);
 
diff --git a/arch/mips/jz4740/Kconfig b/arch/mips/jz4740/Kconfig
index 3e7141f..4689030 100644
--- a/arch/mips/jz4740/Kconfig
+++ b/arch/mips/jz4740/Kconfig
@@ -7,6 +7,3 @@ config JZ4740_QI_LB60
 	bool "Qi Hardware Ben NanoNote"
 
 endchoice
-
-config HAVE_PWM
-	bool
diff --git a/arch/mips/jz4740/Makefile b/arch/mips/jz4740/Makefile
index e44abea..63bad0e 100644
--- a/arch/mips/jz4740/Makefile
+++ b/arch/mips/jz4740/Makefile
@@ -5,7 +5,7 @@
 # Object file lists.
 
 obj-y += prom.o irq.o time.o reset.o setup.o dma.o \
-	gpio.o clock.o platform.o timer.o pwm.o serial.o
+	gpio.o clock.o platform.o timer.o serial.o
 
 obj-$(CONFIG_DEBUG_FS) += clock-debugfs.o
 
diff --git a/arch/mips/jz4740/board-qi_lb60.c b/arch/mips/jz4740/board-qi_lb60.c
index 9a3d9de..43d964d 100644
--- a/arch/mips/jz4740/board-qi_lb60.c
+++ b/arch/mips/jz4740/board-qi_lb60.c
@@ -437,6 +437,7 @@ static struct platform_device *jz_platform_devices[] __initdata = {
 	&jz4740_codec_device,
 	&jz4740_rtc_device,
 	&jz4740_adc_device,
+	&jz4740_pwm_device,
 	&qi_lb60_gpio_keys,
 	&qi_lb60_pwm_beeper,
 	&qi_lb60_charger_device,
diff --git a/arch/mips/jz4740/platform.c b/arch/mips/jz4740/platform.c
index e342ed4..6d14dcd 100644
--- a/arch/mips/jz4740/platform.c
+++ b/arch/mips/jz4740/platform.c
@@ -323,3 +323,9 @@ struct platform_device jz4740_wdt_device = {
 	.num_resources = ARRAY_SIZE(jz4740_wdt_resources),
 	.resource      = jz4740_wdt_resources,
 };
+
+/* PWM */
+struct platform_device jz4740_pwm_device = {
+	.name = "jz4740-pwm",
+	.id   = -1,
+};
diff --git a/arch/mips/jz4740/pwm.c b/arch/mips/jz4740/pwm.c
deleted file mode 100644
index a26a6fa..0000000
--- a/arch/mips/jz4740/pwm.c
+++ /dev/null
@@ -1,177 +0,0 @@
-/*
- *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
- *  JZ4740 platform PWM support
- *
- *  This program is free software; you can redistribute it and/or modify it
- *  under  the terms of the GNU General  Public License as published by the
- *  Free Software Foundation;  either version 2 of the License, or (at your
- *  option) any later version.
- *
- *  You should have received a copy of the GNU General Public License along
- *  with this program; if not, write to the Free Software Foundation, Inc.,
- *  675 Mass Ave, Cambridge, MA 02139, USA.
- *
- */
-
-#include <linux/kernel.h>
-
-#include <linux/clk.h>
-#include <linux/err.h>
-#include <linux/pwm.h>
-#include <linux/gpio.h>
-
-#include <asm/mach-jz4740/gpio.h>
-#include "timer.h"
-
-static struct clk *jz4740_pwm_clk;
-
-DEFINE_MUTEX(jz4740_pwm_mutex);
-
-struct pwm_device {
-	unsigned int id;
-	unsigned int gpio;
-	bool used;
-};
-
-static struct pwm_device jz4740_pwm_list[] = {
-	{ 2, JZ_GPIO_PWM2, false },
-	{ 3, JZ_GPIO_PWM3, false },
-	{ 4, JZ_GPIO_PWM4, false },
-	{ 5, JZ_GPIO_PWM5, false },
-	{ 6, JZ_GPIO_PWM6, false },
-	{ 7, JZ_GPIO_PWM7, false },
-};
-
-struct pwm_device *pwm_request(int id, const char *label)
-{
-	int ret = 0;
-	struct pwm_device *pwm;
-
-	if (id < 2 || id > 7 || !jz4740_pwm_clk)
-		return ERR_PTR(-ENODEV);
-
-	mutex_lock(&jz4740_pwm_mutex);
-
-	pwm = &jz4740_pwm_list[id - 2];
-	if (pwm->used)
-		ret = -EBUSY;
-	else
-		pwm->used = true;
-
-	mutex_unlock(&jz4740_pwm_mutex);
-
-	if (ret)
-		return ERR_PTR(ret);
-
-	ret = gpio_request(pwm->gpio, label);
-
-	if (ret) {
-		printk(KERN_ERR "Failed to request pwm gpio: %d\n", ret);
-		pwm->used = false;
-		return ERR_PTR(ret);
-	}
-
-	jz_gpio_set_function(pwm->gpio, JZ_GPIO_FUNC_PWM);
-
-	jz4740_timer_start(id);
-
-	return pwm;
-}
-
-void pwm_free(struct pwm_device *pwm)
-{
-	pwm_disable(pwm);
-	jz4740_timer_set_ctrl(pwm->id, 0);
-
-	jz_gpio_set_function(pwm->gpio, JZ_GPIO_FUNC_NONE);
-	gpio_free(pwm->gpio);
-
-	jz4740_timer_stop(pwm->id);
-
-	pwm->used = false;
-}
-
-int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
-{
-	unsigned long long tmp;
-	unsigned long period, duty;
-	unsigned int prescaler = 0;
-	unsigned int id = pwm->id;
-	uint16_t ctrl;
-	bool is_enabled;
-
-	if (duty_ns < 0 || duty_ns > period_ns)
-		return -EINVAL;
-
-	tmp = (unsigned long long)clk_get_rate(jz4740_pwm_clk) * period_ns;
-	do_div(tmp, 1000000000);
-	period = tmp;
-
-	while (period > 0xffff && prescaler < 6) {
-		period >>= 2;
-		++prescaler;
-	}
-
-	if (prescaler == 6)
-		return -EINVAL;
-
-	tmp = (unsigned long long)period * duty_ns;
-	do_div(tmp, period_ns);
-	duty = period - tmp;
-
-	if (duty >= period)
-		duty = period - 1;
-
-	is_enabled = jz4740_timer_is_enabled(id);
-	if (is_enabled)
-		pwm_disable(pwm);
-
-	jz4740_timer_set_count(id, 0);
-	jz4740_timer_set_duty(id, duty);
-	jz4740_timer_set_period(id, period);
-
-	ctrl = JZ_TIMER_CTRL_PRESCALER(prescaler) | JZ_TIMER_CTRL_SRC_EXT |
-		JZ_TIMER_CTRL_PWM_ABBRUPT_SHUTDOWN;
-
-	jz4740_timer_set_ctrl(id, ctrl);
-
-	if (is_enabled)
-		pwm_enable(pwm);
-
-	return 0;
-}
-
-int pwm_enable(struct pwm_device *pwm)
-{
-	uint32_t ctrl = jz4740_timer_get_ctrl(pwm->id);
-
-	ctrl |= JZ_TIMER_CTRL_PWM_ENABLE;
-	jz4740_timer_set_ctrl(pwm->id, ctrl);
-	jz4740_timer_enable(pwm->id);
-
-	return 0;
-}
-
-void pwm_disable(struct pwm_device *pwm)
-{
-	uint32_t ctrl = jz4740_timer_get_ctrl(pwm->id);
-
-	ctrl &= ~JZ_TIMER_CTRL_PWM_ENABLE;
-	jz4740_timer_disable(pwm->id);
-	jz4740_timer_set_ctrl(pwm->id, ctrl);
-}
-
-static int __init jz4740_pwm_init(void)
-{
-	int ret = 0;
-
-	jz4740_pwm_clk = clk_get(NULL, "ext");
-
-	if (IS_ERR(jz4740_pwm_clk)) {
-		ret = PTR_ERR(jz4740_pwm_clk);
-		jz4740_pwm_clk = NULL;
-	}
-
-	return ret;
-}
-subsys_initcall(jz4740_pwm_init);
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 90c5c73..5c663df 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -1,6 +1,6 @@
 menuconfig PWM
 	bool "Pulse-Width Modulation (PWM) Support"
-	depends on !MACH_JZ4740 && !PUV3_PWM
+	depends on !PUV3_PWM
 	help
 	  Generic Pulse-Width Modulation (PWM) support.
 
@@ -47,6 +47,16 @@ config PWM_IMX
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-imx.
 
+config PWM_JZ4740
+	tristate "Ingenic JZ4740 PWM support"
+	depends on MACH_JZ4740
+	help
+	  Generic PWM framework driver for Ingenic JZ4740 based
+	  machines.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-jz4740.
+
 config PWM_LPC32XX
 	tristate "LPC32XX PWM support"
 	depends on ARCH_LPC32XX
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index e4b2c89..a1d6169 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -1,6 +1,7 @@
 obj-$(CONFIG_PWM)		+= core.o
 obj-$(CONFIG_PWM_BFIN)		+= pwm-bfin.o
 obj-$(CONFIG_PWM_IMX)		+= pwm-imx.o
+obj-$(CONFIG_PWM_JZ4740)	+= pwm-jz4740.o
 obj-$(CONFIG_PWM_LPC32XX)	+= pwm-lpc32xx.o
 obj-$(CONFIG_PWM_MXS)		+= pwm-mxs.o
 obj-$(CONFIG_PWM_PXA)		+= pwm-pxa.o
diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
new file mode 100644
index 0000000..10250fc
--- /dev/null
+++ b/drivers/pwm/pwm-jz4740.c
@@ -0,0 +1,221 @@
+/*
+ *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
+ *  JZ4740 platform PWM support
+ *
+ *  This program is free software; you can redistribute it and/or modify it
+ *  under  the terms of the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the License, or (at your
+ *  option) any later version.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+
+#include <asm/mach-jz4740/gpio.h>
+#include <asm/mach-jz4740/timer.h>
+
+#define NUM_PWM 8
+
+static const unsigned int jz4740_pwm_gpio_list[NUM_PWM] = {
+	JZ_GPIO_PWM0,
+	JZ_GPIO_PWM1,
+	JZ_GPIO_PWM2,
+	JZ_GPIO_PWM3,
+	JZ_GPIO_PWM4,
+	JZ_GPIO_PWM5,
+	JZ_GPIO_PWM6,
+	JZ_GPIO_PWM7,
+};
+
+struct jz4740_pwm_chip {
+	struct pwm_chip chip;
+	struct clk *clk;
+};
+
+static inline struct jz4740_pwm_chip *to_jz4740(struct pwm_chip *chip)
+{
+	return container_of(chip, struct jz4740_pwm_chip, chip);
+}
+
+static int jz4740_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	unsigned int gpio = jz4740_pwm_gpio_list[pwm->hwpwm];
+	int ret;
+
+	/*
+	 * Timers 0 and 1 are used for system tasks, so they are unavailable
+	 * for use as PWMs.
+	 */
+	if (pwm->hwpwm < 2)
+		return -EBUSY;
+
+	ret = gpio_request(gpio, pwm->label);
+	if (ret) {
+		dev_err(chip->dev, "Failed to request GPIO#%u for PWM: %d\n",
+			gpio, ret);
+		return ret;
+	}
+
+	jz_gpio_set_function(gpio, JZ_GPIO_FUNC_PWM);
+
+	jz4740_timer_start(pwm->hwpwm);
+
+	return 0;
+}
+
+static void jz4740_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	unsigned int gpio = jz4740_pwm_gpio_list[pwm->hwpwm];
+
+	jz4740_timer_set_ctrl(pwm->hwpwm, 0);
+
+	jz_gpio_set_function(gpio, JZ_GPIO_FUNC_NONE);
+	gpio_free(gpio);
+
+	jz4740_timer_stop(pwm->hwpwm);
+}
+
+static int jz4740_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	uint32_t ctrl = jz4740_timer_get_ctrl(pwm->pwm);
+
+	ctrl |= JZ_TIMER_CTRL_PWM_ENABLE;
+	jz4740_timer_set_ctrl(pwm->hwpwm, ctrl);
+	jz4740_timer_enable(pwm->hwpwm);
+
+	return 0;
+}
+
+static void jz4740_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	uint32_t ctrl = jz4740_timer_get_ctrl(pwm->hwpwm);
+
+	ctrl &= ~JZ_TIMER_CTRL_PWM_ENABLE;
+	jz4740_timer_disable(pwm->hwpwm);
+	jz4740_timer_set_ctrl(pwm->hwpwm, ctrl);
+}
+
+static int jz4740_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+			     int duty_ns, int period_ns)
+{
+	struct jz4740_pwm_chip *jz4740 = to_jz4740(pwm->chip);
+	unsigned long long tmp;
+	unsigned long period, duty;
+	unsigned int prescaler = 0;
+	uint16_t ctrl;
+	bool is_enabled;
+
+	tmp = (unsigned long long)clk_get_rate(jz4740->clk) * period_ns;
+	do_div(tmp, 1000000000);
+	period = tmp;
+
+	while (period > 0xffff && prescaler < 6) {
+		period >>= 2;
+		++prescaler;
+	}
+
+	if (prescaler == 6)
+		return -EINVAL;
+
+	tmp = (unsigned long long)period * duty_ns;
+	do_div(tmp, period_ns);
+	duty = period - tmp;
+
+	if (duty >= period)
+		duty = period - 1;
+
+	is_enabled = jz4740_timer_is_enabled(pwm->hwpwm);
+	if (is_enabled)
+		jz4740_pwm_disable(chip, pwm);
+
+	jz4740_timer_set_count(pwm->hwpwm, 0);
+	jz4740_timer_set_duty(pwm->hwpwm, duty);
+	jz4740_timer_set_period(pwm->hwpwm, period);
+
+	ctrl = JZ_TIMER_CTRL_PRESCALER(prescaler) | JZ_TIMER_CTRL_SRC_EXT |
+		JZ_TIMER_CTRL_PWM_ABBRUPT_SHUTDOWN;
+
+	jz4740_timer_set_ctrl(pwm->hwpwm, ctrl);
+
+	if (is_enabled)
+		jz4740_pwm_enable(chip, pwm);
+
+	return 0;
+}
+
+static const struct pwm_ops jz4740_pwm_ops = {
+	.request = jz4740_pwm_request,
+	.free = jz4740_pwm_free,
+	.config = jz4740_pwm_config,
+	.enable = jz4740_pwm_enable,
+	.disable = jz4740_pwm_disable,
+	.owner = THIS_MODULE,
+};
+
+static int __devinit jz4740_pwm_probe(struct platform_device *pdev)
+{
+	struct jz4740_pwm_chip *jz4740;
+	int ret;
+
+	jz4740 = devm_kzalloc(&pdev->dev, sizeof(*jz4740), GFP_KERNEL);
+	if (!jz4740)
+		return -ENOMEM;
+
+	jz4740->clk = clk_get(NULL, "ext");
+	if (IS_ERR(jz4740->clk))
+		return PTR_ERR(jz4740->clk);
+
+	jz4740->chip.dev = &pdev->dev;
+	jz4740->chip.ops = &jz4740_pwm_ops;
+	jz4740->chip.npwm = NUM_PWM;
+	jz4740->chip.base = -1;
+
+	ret = pwmchip_add(&jz4740->chip);
+	if (ret < 0) {
+		clk_put(jz4740->clk);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, jz4740);
+
+	return 0;
+}
+
+static int __devexit jz4740_pwm_remove(struct platform_device *pdev)
+{
+	struct jz4740_pwm_chip *jz4740 = platform_get_drvdata(pdev);
+	int ret;
+
+	ret = pwmchip_remove(&jz4740->chip);
+	if (ret < 0)
+		return ret;
+
+	clk_put(jz4740->clk);
+
+	return 0;
+}
+
+static struct platform_driver jz4740_pwm_driver = {
+	.driver = {
+		.name = "jz4740-pwm",
+		.owner = THIS_MODULE,
+	},
+	.probe = jz4740_pwm_probe,
+	.remove = __devexit_p(jz4740_pwm_remove),
+};
+module_platform_driver(jz4740_pwm_driver);
+
+MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
+MODULE_DESCRIPTION("Ingenic JZ4740 PWM driver");
+MODULE_ALIAS("platform:jz4740-pwm");
+MODULE_LICENSE("GPL");
-- 
1.7.12


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

* Re: [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework
  2012-09-10 12:05 [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework Thierry Reding
                   ` (2 preceding siblings ...)
  2012-09-10 12:05 ` [PATCH v2 3/3] pwm: Add Ingenic JZ4740 support Thierry Reding
@ 2012-09-10 15:20 ` Lars-Peter Clausen
  2012-09-10 17:30   ` Thierry Reding
  2012-09-22  7:41 ` Thierry Reding
  4 siblings, 1 reply; 15+ messages in thread
From: Lars-Peter Clausen @ 2012-09-10 15:20 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Ralf Baechle, linux-mips, linux-kernel, Antony Pavlov,
	Maarten ter Huurne

On 09/10/2012 02:05 PM, Thierry Reding wrote:
> Hi,
> 

I think v2 looks, good. Will give it some testing later.

> This small series fixes a build error due to a circular header
> dependency, exports the timer API so it can be used outside of
> the arch/mips/jz4740 tree and finally moves and converts the
> JZ4740 PWM driver to the PWM framework.
> 
> Note that I don't have any hardware to test this on, so I had to
> rely on compile tests only. Patches 1 and 2 should probably go
> through the MIPS tree, while I can take patch 3 through the PWM
> tree. It touches a couple of files in arch/mips but the changes
> are unlikely to cause conflicts.

Patch 2 and 3 should probably go through the same tree since patch 3 depends
on patch 2. I'd like to see them both go through the PWM tree.

Patch 1 should go through the MIPS tree, but I still can't see why the issue
should occur nor does it happen for anybody else except for you. Instead of
moving the content over to the public irq.h I'd rather like to see the
private irq.h being renamed.

Thanks,
- Lars

> 
> Thierry
> 
> Thierry Reding (3):
>   MIPS: JZ4740: Break circular header dependency
>   MIPS: JZ4740: Export timer API
>   pwm: Add Ingenic JZ4740 support
> 
>  arch/mips/include/asm/mach-jz4740/irq.h      |   5 +
>  arch/mips/include/asm/mach-jz4740/platform.h |   1 +
>  arch/mips/include/asm/mach-jz4740/timer.h    | 113 ++++++++++++++
>  arch/mips/jz4740/Kconfig                     |   3 -
>  arch/mips/jz4740/Makefile                    |   2 +-
>  arch/mips/jz4740/board-qi_lb60.c             |   1 +
>  arch/mips/jz4740/irq.h                       |  23 ---
>  arch/mips/jz4740/platform.c                  |   6 +
>  arch/mips/jz4740/pwm.c                       | 177 ---------------------
>  arch/mips/jz4740/time.c                      |   2 +-
>  arch/mips/jz4740/timer.c                     |   4 +-
>  arch/mips/jz4740/timer.h                     | 136 -----------------
>  drivers/pwm/Kconfig                          |  12 +-
>  drivers/pwm/Makefile                         |   1 +
>  drivers/pwm/pwm-jz4740.c                     | 221 +++++++++++++++++++++++++++
>  15 files changed, 363 insertions(+), 344 deletions(-)
>  delete mode 100644 arch/mips/jz4740/irq.h
>  delete mode 100644 arch/mips/jz4740/pwm.c
>  delete mode 100644 arch/mips/jz4740/timer.h
>  create mode 100644 drivers/pwm/pwm-jz4740.c
> 


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

* Re: [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework
  2012-09-10 15:20 ` [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework Lars-Peter Clausen
@ 2012-09-10 17:30   ` Thierry Reding
  2012-09-11 17:56     ` Lars-Peter Clausen
  0 siblings, 1 reply; 15+ messages in thread
From: Thierry Reding @ 2012-09-10 17:30 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Ralf Baechle, linux-mips, linux-kernel, Antony Pavlov,
	Maarten ter Huurne

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

On Mon, Sep 10, 2012 at 05:20:34PM +0200, Lars-Peter Clausen wrote:
> On 09/10/2012 02:05 PM, Thierry Reding wrote:
> > Hi,
> > 
> 
> I think v2 looks, good. Will give it some testing later.
> 
> > This small series fixes a build error due to a circular header
> > dependency, exports the timer API so it can be used outside of
> > the arch/mips/jz4740 tree and finally moves and converts the
> > JZ4740 PWM driver to the PWM framework.
> > 
> > Note that I don't have any hardware to test this on, so I had to
> > rely on compile tests only. Patches 1 and 2 should probably go
> > through the MIPS tree, while I can take patch 3 through the PWM
> > tree. It touches a couple of files in arch/mips but the changes
> > are unlikely to cause conflicts.
> 
> Patch 2 and 3 should probably go through the same tree since patch 3 depends
> on patch 2. I'd like to see them both go through the PWM tree.

That's fine with me. I'll probably need an Acked-by from Ralf just to be
safe.

> Patch 1 should go through the MIPS tree, but I still can't see why the issue
> should occur nor does it happen for anybody else except for you. Instead of
> moving the content over to the public irq.h I'd rather like to see the
> private irq.h being renamed.

If we can solve this some other way I'm all for it. Maybe you can share
the defconfig or .config that you use so I can test under the same
conditions.

Thierry

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

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

* Re: [PATCH v2 3/3] pwm: Add Ingenic JZ4740 support
  2012-09-10 12:05 ` [PATCH v2 3/3] pwm: Add Ingenic JZ4740 support Thierry Reding
@ 2012-09-10 21:51   ` Lars-Peter Clausen
  2012-09-11  5:02     ` Thierry Reding
  0 siblings, 1 reply; 15+ messages in thread
From: Lars-Peter Clausen @ 2012-09-10 21:51 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Ralf Baechle, linux-mips, linux-kernel, Antony Pavlov,
	Maarten ter Huurne

On 09/10/2012 02:05 PM, Thierry Reding wrote:
> This commit moves the driver to drivers/pwm and converts it to the new
> PWM framework.
> 
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>

Seems to work, thanks a lot. This one and patch 2:

Acked-by: Lars-Peter Clausen <lars@metafoo.de>
Tested-by: Lars-Peter Clausen <lars@metafoo.de>

But I noticed a different problem. Some drivers using the pwm API depend on
HAVE_PWM (e.g. the pwm beeper driver), but the generic PWM framework does not
select HAVE_PWM, so I couldn't select the pwm beeper driver. Imo the generic
PWM framework should select HAVE_PWM

- Lars

> ---
> Changes in v2:
> - remove PWM core hunk that slipped in by mistake
> - change pwm-beeper platform data back to 4
> - refer to timer.h by full path
> - register PWMs 0 and 1 but reserve them for system tasks
> - replace printk by dev_err now that a proper device is available
> - use jz4740_pwm_{enable,disable}() instead of pwm_{enable,disable}()
>   internally
> - make THIS_MODULE the owner of jz4740_pwm_ops and jz4740_pwm_driver
> - add __devinit and __devexit annotations to jz4740_pwm_probe() and
>   jz4740_pwm_remove() respectively
> - add MODULE_AUTHOR(), MODULE_DESCRIPTION(), MODULE_ALIAS() and
>   MODULE_LICENSE()
> 
>  arch/mips/include/asm/mach-jz4740/platform.h |   1 +
>  arch/mips/jz4740/Kconfig                     |   3 -
>  arch/mips/jz4740/Makefile                    |   2 +-
>  arch/mips/jz4740/board-qi_lb60.c             |   1 +
>  arch/mips/jz4740/platform.c                  |   6 +
>  arch/mips/jz4740/pwm.c                       | 177 ---------------------
>  drivers/pwm/Kconfig                          |  12 +-
>  drivers/pwm/Makefile                         |   1 +
>  drivers/pwm/pwm-jz4740.c                     | 221 +++++++++++++++++++++++++++
>  9 files changed, 242 insertions(+), 182 deletions(-)
>  delete mode 100644 arch/mips/jz4740/pwm.c
>  create mode 100644 drivers/pwm/pwm-jz4740.c
> 
> diff --git a/arch/mips/include/asm/mach-jz4740/platform.h b/arch/mips/include/asm/mach-jz4740/platform.h
> index 564ab81..163e81d 100644
> --- a/arch/mips/include/asm/mach-jz4740/platform.h
> +++ b/arch/mips/include/asm/mach-jz4740/platform.h
> @@ -31,6 +31,7 @@ extern struct platform_device jz4740_pcm_device;
>  extern struct platform_device jz4740_codec_device;
>  extern struct platform_device jz4740_adc_device;
>  extern struct platform_device jz4740_wdt_device;
> +extern struct platform_device jz4740_pwm_device;
>  
>  void jz4740_serial_device_register(void);
>  
> diff --git a/arch/mips/jz4740/Kconfig b/arch/mips/jz4740/Kconfig
> index 3e7141f..4689030 100644
> --- a/arch/mips/jz4740/Kconfig
> +++ b/arch/mips/jz4740/Kconfig
> @@ -7,6 +7,3 @@ config JZ4740_QI_LB60
>  	bool "Qi Hardware Ben NanoNote"
>  
>  endchoice
> -
> -config HAVE_PWM
> -	bool
> diff --git a/arch/mips/jz4740/Makefile b/arch/mips/jz4740/Makefile
> index e44abea..63bad0e 100644
> --- a/arch/mips/jz4740/Makefile
> +++ b/arch/mips/jz4740/Makefile
> @@ -5,7 +5,7 @@
>  # Object file lists.
>  
>  obj-y += prom.o irq.o time.o reset.o setup.o dma.o \
> -	gpio.o clock.o platform.o timer.o pwm.o serial.o
> +	gpio.o clock.o platform.o timer.o serial.o
>  
>  obj-$(CONFIG_DEBUG_FS) += clock-debugfs.o
>  
> diff --git a/arch/mips/jz4740/board-qi_lb60.c b/arch/mips/jz4740/board-qi_lb60.c
> index 9a3d9de..43d964d 100644
> --- a/arch/mips/jz4740/board-qi_lb60.c
> +++ b/arch/mips/jz4740/board-qi_lb60.c
> @@ -437,6 +437,7 @@ static struct platform_device *jz_platform_devices[] __initdata = {
>  	&jz4740_codec_device,
>  	&jz4740_rtc_device,
>  	&jz4740_adc_device,
> +	&jz4740_pwm_device,
>  	&qi_lb60_gpio_keys,
>  	&qi_lb60_pwm_beeper,
>  	&qi_lb60_charger_device,
> diff --git a/arch/mips/jz4740/platform.c b/arch/mips/jz4740/platform.c
> index e342ed4..6d14dcd 100644
> --- a/arch/mips/jz4740/platform.c
> +++ b/arch/mips/jz4740/platform.c
> @@ -323,3 +323,9 @@ struct platform_device jz4740_wdt_device = {
>  	.num_resources = ARRAY_SIZE(jz4740_wdt_resources),
>  	.resource      = jz4740_wdt_resources,
>  };
> +
> +/* PWM */
> +struct platform_device jz4740_pwm_device = {
> +	.name = "jz4740-pwm",
> +	.id   = -1,
> +};
> diff --git a/arch/mips/jz4740/pwm.c b/arch/mips/jz4740/pwm.c
> deleted file mode 100644
> index a26a6fa..0000000
> --- a/arch/mips/jz4740/pwm.c
> +++ /dev/null
> @@ -1,177 +0,0 @@
> -/*
> - *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
> - *  JZ4740 platform PWM support
> - *
> - *  This program is free software; you can redistribute it and/or modify it
> - *  under  the terms of the GNU General  Public License as published by the
> - *  Free Software Foundation;  either version 2 of the License, or (at your
> - *  option) any later version.
> - *
> - *  You should have received a copy of the GNU General Public License along
> - *  with this program; if not, write to the Free Software Foundation, Inc.,
> - *  675 Mass Ave, Cambridge, MA 02139, USA.
> - *
> - */
> -
> -#include <linux/kernel.h>
> -
> -#include <linux/clk.h>
> -#include <linux/err.h>
> -#include <linux/pwm.h>
> -#include <linux/gpio.h>
> -
> -#include <asm/mach-jz4740/gpio.h>
> -#include "timer.h"
> -
> -static struct clk *jz4740_pwm_clk;
> -
> -DEFINE_MUTEX(jz4740_pwm_mutex);
> -
> -struct pwm_device {
> -	unsigned int id;
> -	unsigned int gpio;
> -	bool used;
> -};
> -
> -static struct pwm_device jz4740_pwm_list[] = {
> -	{ 2, JZ_GPIO_PWM2, false },
> -	{ 3, JZ_GPIO_PWM3, false },
> -	{ 4, JZ_GPIO_PWM4, false },
> -	{ 5, JZ_GPIO_PWM5, false },
> -	{ 6, JZ_GPIO_PWM6, false },
> -	{ 7, JZ_GPIO_PWM7, false },
> -};
> -
> -struct pwm_device *pwm_request(int id, const char *label)
> -{
> -	int ret = 0;
> -	struct pwm_device *pwm;
> -
> -	if (id < 2 || id > 7 || !jz4740_pwm_clk)
> -		return ERR_PTR(-ENODEV);
> -
> -	mutex_lock(&jz4740_pwm_mutex);
> -
> -	pwm = &jz4740_pwm_list[id - 2];
> -	if (pwm->used)
> -		ret = -EBUSY;
> -	else
> -		pwm->used = true;
> -
> -	mutex_unlock(&jz4740_pwm_mutex);
> -
> -	if (ret)
> -		return ERR_PTR(ret);
> -
> -	ret = gpio_request(pwm->gpio, label);
> -
> -	if (ret) {
> -		printk(KERN_ERR "Failed to request pwm gpio: %d\n", ret);
> -		pwm->used = false;
> -		return ERR_PTR(ret);
> -	}
> -
> -	jz_gpio_set_function(pwm->gpio, JZ_GPIO_FUNC_PWM);
> -
> -	jz4740_timer_start(id);
> -
> -	return pwm;
> -}
> -
> -void pwm_free(struct pwm_device *pwm)
> -{
> -	pwm_disable(pwm);
> -	jz4740_timer_set_ctrl(pwm->id, 0);
> -
> -	jz_gpio_set_function(pwm->gpio, JZ_GPIO_FUNC_NONE);
> -	gpio_free(pwm->gpio);
> -
> -	jz4740_timer_stop(pwm->id);
> -
> -	pwm->used = false;
> -}
> -
> -int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> -{
> -	unsigned long long tmp;
> -	unsigned long period, duty;
> -	unsigned int prescaler = 0;
> -	unsigned int id = pwm->id;
> -	uint16_t ctrl;
> -	bool is_enabled;
> -
> -	if (duty_ns < 0 || duty_ns > period_ns)
> -		return -EINVAL;
> -
> -	tmp = (unsigned long long)clk_get_rate(jz4740_pwm_clk) * period_ns;
> -	do_div(tmp, 1000000000);
> -	period = tmp;
> -
> -	while (period > 0xffff && prescaler < 6) {
> -		period >>= 2;
> -		++prescaler;
> -	}
> -
> -	if (prescaler == 6)
> -		return -EINVAL;
> -
> -	tmp = (unsigned long long)period * duty_ns;
> -	do_div(tmp, period_ns);
> -	duty = period - tmp;
> -
> -	if (duty >= period)
> -		duty = period - 1;
> -
> -	is_enabled = jz4740_timer_is_enabled(id);
> -	if (is_enabled)
> -		pwm_disable(pwm);
> -
> -	jz4740_timer_set_count(id, 0);
> -	jz4740_timer_set_duty(id, duty);
> -	jz4740_timer_set_period(id, period);
> -
> -	ctrl = JZ_TIMER_CTRL_PRESCALER(prescaler) | JZ_TIMER_CTRL_SRC_EXT |
> -		JZ_TIMER_CTRL_PWM_ABBRUPT_SHUTDOWN;
> -
> -	jz4740_timer_set_ctrl(id, ctrl);
> -
> -	if (is_enabled)
> -		pwm_enable(pwm);
> -
> -	return 0;
> -}
> -
> -int pwm_enable(struct pwm_device *pwm)
> -{
> -	uint32_t ctrl = jz4740_timer_get_ctrl(pwm->id);
> -
> -	ctrl |= JZ_TIMER_CTRL_PWM_ENABLE;
> -	jz4740_timer_set_ctrl(pwm->id, ctrl);
> -	jz4740_timer_enable(pwm->id);
> -
> -	return 0;
> -}
> -
> -void pwm_disable(struct pwm_device *pwm)
> -{
> -	uint32_t ctrl = jz4740_timer_get_ctrl(pwm->id);
> -
> -	ctrl &= ~JZ_TIMER_CTRL_PWM_ENABLE;
> -	jz4740_timer_disable(pwm->id);
> -	jz4740_timer_set_ctrl(pwm->id, ctrl);
> -}
> -
> -static int __init jz4740_pwm_init(void)
> -{
> -	int ret = 0;
> -
> -	jz4740_pwm_clk = clk_get(NULL, "ext");
> -
> -	if (IS_ERR(jz4740_pwm_clk)) {
> -		ret = PTR_ERR(jz4740_pwm_clk);
> -		jz4740_pwm_clk = NULL;
> -	}
> -
> -	return ret;
> -}
> -subsys_initcall(jz4740_pwm_init);
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 90c5c73..5c663df 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -1,6 +1,6 @@
>  menuconfig PWM
>  	bool "Pulse-Width Modulation (PWM) Support"
> -	depends on !MACH_JZ4740 && !PUV3_PWM
> +	depends on !PUV3_PWM
>  	help
>  	  Generic Pulse-Width Modulation (PWM) support.
>  
> @@ -47,6 +47,16 @@ config PWM_IMX
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called pwm-imx.
>  
> +config PWM_JZ4740
> +	tristate "Ingenic JZ4740 PWM support"
> +	depends on MACH_JZ4740
> +	help
> +	  Generic PWM framework driver for Ingenic JZ4740 based
> +	  machines.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called pwm-jz4740.
> +
>  config PWM_LPC32XX
>  	tristate "LPC32XX PWM support"
>  	depends on ARCH_LPC32XX
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index e4b2c89..a1d6169 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -1,6 +1,7 @@
>  obj-$(CONFIG_PWM)		+= core.o
>  obj-$(CONFIG_PWM_BFIN)		+= pwm-bfin.o
>  obj-$(CONFIG_PWM_IMX)		+= pwm-imx.o
> +obj-$(CONFIG_PWM_JZ4740)	+= pwm-jz4740.o
>  obj-$(CONFIG_PWM_LPC32XX)	+= pwm-lpc32xx.o
>  obj-$(CONFIG_PWM_MXS)		+= pwm-mxs.o
>  obj-$(CONFIG_PWM_PXA)		+= pwm-pxa.o
> diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
> new file mode 100644
> index 0000000..10250fc
> --- /dev/null
> +++ b/drivers/pwm/pwm-jz4740.c
> @@ -0,0 +1,221 @@
> +/*
> + *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
> + *  JZ4740 platform PWM support
> + *
> + *  This program is free software; you can redistribute it and/or modify it
> + *  under  the terms of the GNU General  Public License as published by the
> + *  Free Software Foundation;  either version 2 of the License, or (at your
> + *  option) any later version.
> + *
> + *  You should have received a copy of the GNU General Public License along
> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> + *  675 Mass Ave, Cambridge, MA 02139, USA.
> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/gpio.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +
> +#include <asm/mach-jz4740/gpio.h>
> +#include <asm/mach-jz4740/timer.h>
> +
> +#define NUM_PWM 8
> +
> +static const unsigned int jz4740_pwm_gpio_list[NUM_PWM] = {
> +	JZ_GPIO_PWM0,
> +	JZ_GPIO_PWM1,
> +	JZ_GPIO_PWM2,
> +	JZ_GPIO_PWM3,
> +	JZ_GPIO_PWM4,
> +	JZ_GPIO_PWM5,
> +	JZ_GPIO_PWM6,
> +	JZ_GPIO_PWM7,
> +};
> +
> +struct jz4740_pwm_chip {
> +	struct pwm_chip chip;
> +	struct clk *clk;
> +};
> +
> +static inline struct jz4740_pwm_chip *to_jz4740(struct pwm_chip *chip)
> +{
> +	return container_of(chip, struct jz4740_pwm_chip, chip);
> +}
> +
> +static int jz4740_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	unsigned int gpio = jz4740_pwm_gpio_list[pwm->hwpwm];
> +	int ret;
> +
> +	/*
> +	 * Timers 0 and 1 are used for system tasks, so they are unavailable
> +	 * for use as PWMs.
> +	 */
> +	if (pwm->hwpwm < 2)
> +		return -EBUSY;
> +
> +	ret = gpio_request(gpio, pwm->label);
> +	if (ret) {
> +		dev_err(chip->dev, "Failed to request GPIO#%u for PWM: %d\n",
> +			gpio, ret);
> +		return ret;
> +	}
> +
> +	jz_gpio_set_function(gpio, JZ_GPIO_FUNC_PWM);
> +
> +	jz4740_timer_start(pwm->hwpwm);
> +
> +	return 0;
> +}
> +
> +static void jz4740_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	unsigned int gpio = jz4740_pwm_gpio_list[pwm->hwpwm];
> +
> +	jz4740_timer_set_ctrl(pwm->hwpwm, 0);
> +
> +	jz_gpio_set_function(gpio, JZ_GPIO_FUNC_NONE);
> +	gpio_free(gpio);
> +
> +	jz4740_timer_stop(pwm->hwpwm);
> +}
> +
> +static int jz4740_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	uint32_t ctrl = jz4740_timer_get_ctrl(pwm->pwm);
> +
> +	ctrl |= JZ_TIMER_CTRL_PWM_ENABLE;
> +	jz4740_timer_set_ctrl(pwm->hwpwm, ctrl);
> +	jz4740_timer_enable(pwm->hwpwm);
> +
> +	return 0;
> +}
> +
> +static void jz4740_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	uint32_t ctrl = jz4740_timer_get_ctrl(pwm->hwpwm);
> +
> +	ctrl &= ~JZ_TIMER_CTRL_PWM_ENABLE;
> +	jz4740_timer_disable(pwm->hwpwm);
> +	jz4740_timer_set_ctrl(pwm->hwpwm, ctrl);
> +}
> +
> +static int jz4740_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> +			     int duty_ns, int period_ns)
> +{
> +	struct jz4740_pwm_chip *jz4740 = to_jz4740(pwm->chip);
> +	unsigned long long tmp;
> +	unsigned long period, duty;
> +	unsigned int prescaler = 0;
> +	uint16_t ctrl;
> +	bool is_enabled;
> +
> +	tmp = (unsigned long long)clk_get_rate(jz4740->clk) * period_ns;
> +	do_div(tmp, 1000000000);
> +	period = tmp;
> +
> +	while (period > 0xffff && prescaler < 6) {
> +		period >>= 2;
> +		++prescaler;
> +	}
> +
> +	if (prescaler == 6)
> +		return -EINVAL;
> +
> +	tmp = (unsigned long long)period * duty_ns;
> +	do_div(tmp, period_ns);
> +	duty = period - tmp;
> +
> +	if (duty >= period)
> +		duty = period - 1;
> +
> +	is_enabled = jz4740_timer_is_enabled(pwm->hwpwm);
> +	if (is_enabled)
> +		jz4740_pwm_disable(chip, pwm);
> +
> +	jz4740_timer_set_count(pwm->hwpwm, 0);
> +	jz4740_timer_set_duty(pwm->hwpwm, duty);
> +	jz4740_timer_set_period(pwm->hwpwm, period);
> +
> +	ctrl = JZ_TIMER_CTRL_PRESCALER(prescaler) | JZ_TIMER_CTRL_SRC_EXT |
> +		JZ_TIMER_CTRL_PWM_ABBRUPT_SHUTDOWN;
> +
> +	jz4740_timer_set_ctrl(pwm->hwpwm, ctrl);
> +
> +	if (is_enabled)
> +		jz4740_pwm_enable(chip, pwm);
> +
> +	return 0;
> +}
> +
> +static const struct pwm_ops jz4740_pwm_ops = {
> +	.request = jz4740_pwm_request,
> +	.free = jz4740_pwm_free,
> +	.config = jz4740_pwm_config,
> +	.enable = jz4740_pwm_enable,
> +	.disable = jz4740_pwm_disable,
> +	.owner = THIS_MODULE,
> +};
> +
> +static int __devinit jz4740_pwm_probe(struct platform_device *pdev)
> +{
> +	struct jz4740_pwm_chip *jz4740;
> +	int ret;
> +
> +	jz4740 = devm_kzalloc(&pdev->dev, sizeof(*jz4740), GFP_KERNEL);
> +	if (!jz4740)
> +		return -ENOMEM;
> +
> +	jz4740->clk = clk_get(NULL, "ext");
> +	if (IS_ERR(jz4740->clk))
> +		return PTR_ERR(jz4740->clk);
> +
> +	jz4740->chip.dev = &pdev->dev;
> +	jz4740->chip.ops = &jz4740_pwm_ops;
> +	jz4740->chip.npwm = NUM_PWM;
> +	jz4740->chip.base = -1;
> +
> +	ret = pwmchip_add(&jz4740->chip);
> +	if (ret < 0) {
> +		clk_put(jz4740->clk);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, jz4740);
> +
> +	return 0;
> +}
> +
> +static int __devexit jz4740_pwm_remove(struct platform_device *pdev)
> +{
> +	struct jz4740_pwm_chip *jz4740 = platform_get_drvdata(pdev);
> +	int ret;
> +
> +	ret = pwmchip_remove(&jz4740->chip);
> +	if (ret < 0)
> +		return ret;
> +
> +	clk_put(jz4740->clk);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver jz4740_pwm_driver = {
> +	.driver = {
> +		.name = "jz4740-pwm",
> +		.owner = THIS_MODULE,
> +	},
> +	.probe = jz4740_pwm_probe,
> +	.remove = __devexit_p(jz4740_pwm_remove),
> +};
> +module_platform_driver(jz4740_pwm_driver);
> +
> +MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
> +MODULE_DESCRIPTION("Ingenic JZ4740 PWM driver");
> +MODULE_ALIAS("platform:jz4740-pwm");
> +MODULE_LICENSE("GPL");


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

* Re: [PATCH v2 3/3] pwm: Add Ingenic JZ4740 support
  2012-09-10 21:51   ` Lars-Peter Clausen
@ 2012-09-11  5:02     ` Thierry Reding
  2012-09-11 17:54       ` Lars-Peter Clausen
  0 siblings, 1 reply; 15+ messages in thread
From: Thierry Reding @ 2012-09-11  5:02 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Ralf Baechle, linux-mips, linux-kernel, Antony Pavlov,
	Maarten ter Huurne

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

On Mon, Sep 10, 2012 at 11:51:48PM +0200, Lars-Peter Clausen wrote:
> On 09/10/2012 02:05 PM, Thierry Reding wrote:
> > This commit moves the driver to drivers/pwm and converts it to the new
> > PWM framework.
> > 
> > Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> 
> Seems to work, thanks a lot. This one and patch 2:
> 
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
> Tested-by: Lars-Peter Clausen <lars@metafoo.de>
> 
> But I noticed a different problem. Some drivers using the pwm API depend on
> HAVE_PWM (e.g. the pwm beeper driver), but the generic PWM framework does not
> select HAVE_PWM, so I couldn't select the pwm beeper driver. Imo the generic
> PWM framework should select HAVE_PWM

Does it also work if you add || PWM to the PWM beeper driver's depends?
I thought I had done something similar for pwm-backlight, but looking at
the logs I didn't. The reason was that it also uses the new APIs
introduced by the PWM subsystem. For pwm-beeper the situation is
different because it only uses the legacy API and therefore can work
with both the legacy and new frameworks.

I think selecting HAVE_PWM won't work properly because it isn't provided
by all architectures. So you might end up with PWM enabled on PowerPC,
which doesn't define HAVE_PWM and will probably give you Kconfig
warnings and will still not let you select pwm-beeper.

Thierry

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

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

* Re: [PATCH v2 3/3] pwm: Add Ingenic JZ4740 support
  2012-09-11  5:02     ` Thierry Reding
@ 2012-09-11 17:54       ` Lars-Peter Clausen
  0 siblings, 0 replies; 15+ messages in thread
From: Lars-Peter Clausen @ 2012-09-11 17:54 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Ralf Baechle, linux-mips, linux-kernel, Antony Pavlov,
	Maarten ter Huurne

On 09/11/2012 07:02 AM, Thierry Reding wrote:
> On Mon, Sep 10, 2012 at 11:51:48PM +0200, Lars-Peter Clausen wrote:
>> On 09/10/2012 02:05 PM, Thierry Reding wrote:
>>> This commit moves the driver to drivers/pwm and converts it to the new
>>> PWM framework.
>>>
>>> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
>>
>> Seems to work, thanks a lot. This one and patch 2:
>>
>> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
>> Tested-by: Lars-Peter Clausen <lars@metafoo.de>
>>
>> But I noticed a different problem. Some drivers using the pwm API depend on
>> HAVE_PWM (e.g. the pwm beeper driver), but the generic PWM framework does not
>> select HAVE_PWM, so I couldn't select the pwm beeper driver. Imo the generic
>> PWM framework should select HAVE_PWM
> 
> Does it also work if you add || PWM to the PWM beeper driver's depends?

Should work, but to select HAVE_PWM would in my opinion have been cleaner. But
since the custom implementations of the PWM API should be gone soon anyway it
probably does not matter that much.

- Lars

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

* Re: [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework
  2012-09-10 17:30   ` Thierry Reding
@ 2012-09-11 17:56     ` Lars-Peter Clausen
  2012-09-12 15:02       ` Thierry Reding
  0 siblings, 1 reply; 15+ messages in thread
From: Lars-Peter Clausen @ 2012-09-11 17:56 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Ralf Baechle, linux-mips, linux-kernel, Antony Pavlov,
	Maarten ter Huurne

On 09/10/2012 07:30 PM, Thierry Reding wrote:
> On Mon, Sep 10, 2012 at 05:20:34PM +0200, Lars-Peter Clausen wrote:
>> On 09/10/2012 02:05 PM, Thierry Reding wrote:
>>> Hi,
>>>
>>
>> [...]
> 
>> Patch 1 should go through the MIPS tree, but I still can't see why the issue
>> should occur nor does it happen for anybody else except for you. Instead of
>> moving the content over to the public irq.h I'd rather like to see the
>> private irq.h being renamed.
> 
> If we can solve this some other way I'm all for it. Maybe you can share
> the defconfig or .config that you use so I can test under the same
> conditions.
> 
> Thierry

This is the config I'm using:
http://projects.qi-hardware.com/index.php/p/qi-kernel/source/tree/jz-3.4/arch/mips/configs/qi_lb60_defconfig

- Lars


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

* Re: [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework
  2012-09-11 17:56     ` Lars-Peter Clausen
@ 2012-09-12 15:02       ` Thierry Reding
  0 siblings, 0 replies; 15+ messages in thread
From: Thierry Reding @ 2012-09-12 15:02 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Ralf Baechle, linux-mips, linux-kernel, Antony Pavlov,
	Maarten ter Huurne

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

On Tue, Sep 11, 2012 at 07:56:29PM +0200, Lars-Peter Clausen wrote:
> On 09/10/2012 07:30 PM, Thierry Reding wrote:
> > On Mon, Sep 10, 2012 at 05:20:34PM +0200, Lars-Peter Clausen wrote:
> >> On 09/10/2012 02:05 PM, Thierry Reding wrote:
> >>> Hi,
> >>>
> >>
> >> [...]
> > 
> >> Patch 1 should go through the MIPS tree, but I still can't see why the issue
> >> should occur nor does it happen for anybody else except for you. Instead of
> >> moving the content over to the public irq.h I'd rather like to see the
> >> private irq.h being renamed.
> > 
> > If we can solve this some other way I'm all for it. Maybe you can share
> > the defconfig or .config that you use so I can test under the same
> > conditions.
> > 
> > Thierry
> 
> This is the config I'm using:
> http://projects.qi-hardware.com/index.php/p/qi-kernel/source/tree/jz-3.4/arch/mips/configs/qi_lb60_defconfig

I can still reproduce the error with that configuration. Perhaps related
to the toolchain? I use a vanilla gcc 4.7.1 cross-compiler. This is all
on top of Linux v3.6-rc2.

Here's the complete error message:

$ nice make ARCH=mips CROSS_COMPILE=/home/thierry.reding/pbs-stage1/bin/mips-linux-gnu- O=build/jz4740
  Using /home/thierry.reding/src/kernel/linux-pwm.git as source for kernel
  GEN     /home/thierry.reding/src/kernel/linux-pwm.git/build/jz4740/Makefile
  CHK     include/linux/version.h
  CHK     include/generated/utsrelease.h
  CALL    /home/thierry.reding/src/kernel/linux-pwm.git/scripts/checksyscalls.sh
  CHK     include/generated/compile.h
  CC      arch/mips/jz4740/irq.o
In file included from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/include/asm/irq.h:18:0,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/irq.h:27,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/asm-generic/hardirq.h:12,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/include/asm/hardirq.h:16,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/hardirq.h:7,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/interrupt.h:12,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c:19:
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.h:20:39: error: 'struct irq_data' declared inside parameter list [-Werror]
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.h:20:39: error: its scope is only this definition or declaration, which is probably not what you want [-Werror]
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.h:21:38: error: 'struct irq_data' declared inside parameter list [-Werror]
In file included from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/irq.h:356:0,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/asm-generic/hardirq.h:12,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/include/asm/hardirq.h:16,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/hardirq.h:7,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/interrupt.h:12,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c:19:
/home/thierry.reding/src/kernel/linux-pwm.git/include/linux/irqdesc.h:75:33: error: 'NR_IRQS' undeclared here (not in a function)
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c: In function 'jz4740_cascade':
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c:49:39: error: 'JZ4740_IRQ_BASE' undeclared (first use in this function)
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c:49:39: note: each undeclared identifier is reported only once for each function it appears in
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c: At top level:
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c:62:6: error: conflicting types for 'jz4740_irq_suspend'
In file included from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/include/asm/irq.h:18:0,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/irq.h:27,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/asm-generic/hardirq.h:12,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/include/asm/hardirq.h:16,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/hardirq.h:7,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/interrupt.h:12,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c:19:
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.h:20:13: note: previous declaration of 'jz4740_irq_suspend' was here
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c:68:6: error: conflicting types for 'jz4740_irq_resume'
In file included from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/include/asm/irq.h:18:0,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/irq.h:27,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/asm-generic/hardirq.h:12,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/include/asm/hardirq.h:16,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/hardirq.h:7,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/interrupt.h:12,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c:19:
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.h:21:13: note: previous declaration of 'jz4740_irq_resume' was here
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c: In function 'arch_init_irq':
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c:91:41: error: 'JZ4740_IRQ_BASE' undeclared (first use in this function)
cc1: all warnings being treated as errors
make[3]: *** [arch/mips/jz4740/irq.o] Error 1
make[2]: *** [arch/mips/jz4740] Error 2
make[1]: *** [arch/mips] Error 2
make: *** [sub-make] Error 2
  Using /home/thierry.reding/src/kernel/linux-pwm.git as source for kernel
  GEN     /home/thierry.reding/src/kernel/linux-pwm.git/build/jz4740/Makefile
  CHK     include/linux/version.h
  CHK     include/generated/utsrelease.h
  CALL    /home/thierry.reding/src/kernel/linux-pwm.git/scripts/checksyscalls.sh
  CHK     include/generated/compile.h
  CC      arch/mips/jz4740/irq.o
In file included from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/include/asm/irq.h:18:0,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/irq.h:27,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/asm-generic/hardirq.h:12,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/include/asm/hardirq.h:16,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/hardirq.h:7,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/interrupt.h:12,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c:19:
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.h:20:39: error: 'struct irq_data' declared inside parameter list [-Werror]
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.h:20:39: error: its scope is only this definition or declaration, which is probably not what you want [-Werror]
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.h:21:38: error: 'struct irq_data' declared inside parameter list [-Werror]
In file included from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/irq.h:356:0,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/asm-generic/hardirq.h:12,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/include/asm/hardirq.h:16,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/hardirq.h:7,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/interrupt.h:12,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c:19:
/home/thierry.reding/src/kernel/linux-pwm.git/include/linux/irqdesc.h:75:33: error: 'NR_IRQS' undeclared here (not in a function)
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c: In function 'jz4740_cascade':
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c:49:39: error: 'JZ4740_IRQ_BASE' undeclared (first use in this function)
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c:49:39: note: each undeclared identifier is reported only once for each function it appears in
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c: At top level:
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c:62:6: error: conflicting types for 'jz4740_irq_suspend'
In file included from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/include/asm/irq.h:18:0,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/irq.h:27,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/asm-generic/hardirq.h:12,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/include/asm/hardirq.h:16,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/hardirq.h:7,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/interrupt.h:12,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c:19:
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.h:20:13: note: previous declaration of 'jz4740_irq_suspend' was here
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c:68:6: error: conflicting types for 'jz4740_irq_resume'
In file included from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/include/asm/irq.h:18:0,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/irq.h:27,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/asm-generic/hardirq.h:12,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/include/asm/hardirq.h:16,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/hardirq.h:7,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/include/linux/interrupt.h:12,
                 from /home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c:19:
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.h:21:13: note: previous declaration of 'jz4740_irq_resume' was here
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c: In function 'arch_init_irq':
/home/thierry.reding/src/kernel/linux-pwm.git/arch/mips/jz4740/irq.c:91:41: error: 'JZ4740_IRQ_BASE' undeclared (first use in this function)
cc1: all warnings being treated as errors
make[3]: *** [arch/mips/jz4740/irq.o] Error 1
make[2]: *** [arch/mips/jz4740] Error 2
make[1]: *** [arch/mips] Error 2
make: *** [sub-make] Error 2

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

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

* Re: [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework
  2012-09-10 12:05 [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework Thierry Reding
                   ` (3 preceding siblings ...)
  2012-09-10 15:20 ` [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework Lars-Peter Clausen
@ 2012-09-22  7:41 ` Thierry Reding
  2012-09-23 13:56   ` Ralf Baechle
  4 siblings, 1 reply; 15+ messages in thread
From: Thierry Reding @ 2012-09-22  7:41 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: linux-mips, linux-kernel, Antony Pavlov, Lars-Peter Clausen,
	Maarten ter Huurne

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

On Mon, Sep 10, 2012 at 02:05:16PM +0200, Thierry Reding wrote:
> Hi,
> 
> This small series fixes a build error due to a circular header
> dependency, exports the timer API so it can be used outside of
> the arch/mips/jz4740 tree and finally moves and converts the
> JZ4740 PWM driver to the PWM framework.
> 
> Note that I don't have any hardware to test this on, so I had to
> rely on compile tests only. Patches 1 and 2 should probably go
> through the MIPS tree, while I can take patch 3 through the PWM
> tree. It touches a couple of files in arch/mips but the changes
> are unlikely to cause conflicts.
> 
> Thierry
> 
> Thierry Reding (3):
>   MIPS: JZ4740: Break circular header dependency
>   MIPS: JZ4740: Export timer API
>   pwm: Add Ingenic JZ4740 support
> 
>  arch/mips/include/asm/mach-jz4740/irq.h      |   5 +
>  arch/mips/include/asm/mach-jz4740/platform.h |   1 +
>  arch/mips/include/asm/mach-jz4740/timer.h    | 113 ++++++++++++++
>  arch/mips/jz4740/Kconfig                     |   3 -
>  arch/mips/jz4740/Makefile                    |   2 +-
>  arch/mips/jz4740/board-qi_lb60.c             |   1 +
>  arch/mips/jz4740/irq.h                       |  23 ---
>  arch/mips/jz4740/platform.c                  |   6 +
>  arch/mips/jz4740/pwm.c                       | 177 ---------------------
>  arch/mips/jz4740/time.c                      |   2 +-
>  arch/mips/jz4740/timer.c                     |   4 +-
>  arch/mips/jz4740/timer.h                     | 136 -----------------
>  drivers/pwm/Kconfig                          |  12 +-
>  drivers/pwm/Makefile                         |   1 +
>  drivers/pwm/pwm-jz4740.c                     | 221 +++++++++++++++++++++++++++
>  15 files changed, 363 insertions(+), 344 deletions(-)
>  delete mode 100644 arch/mips/jz4740/irq.h
>  delete mode 100644 arch/mips/jz4740/pwm.c
>  delete mode 100644 arch/mips/jz4740/timer.h
>  create mode 100644 drivers/pwm/pwm-jz4740.c

Hi Ralf,

Have you had a chance to look at this? It is the last remaining PWM
driver that isn't moved to the PWM framework yet. All the others are
either in linux-next already and queued for 3.7 or have recently got
Acked-by the respective maintainers (Unicore32). Patches 2 and 3 were
already acked and tested by Lars-Peter who did the initial porting.
Patch 1 can probably be dropped since I seem to be the only one running
into that issue.

I really want to take this in for 3.7 so I can use the 3.7 cycle to
transition from the legacy API to the new API and possibly even get rid
of the legacy parts altogether. However I don't want to do this without
the Acked-by from the MIPS maintainer.

Thierry

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

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

* Re: [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework
  2012-09-22  7:41 ` Thierry Reding
@ 2012-09-23 13:56   ` Ralf Baechle
  2012-09-23 17:12     ` Thierry Reding
  2012-09-27 19:48     ` Thierry Reding
  0 siblings, 2 replies; 15+ messages in thread
From: Ralf Baechle @ 2012-09-23 13:56 UTC (permalink / raw)
  To: Thierry Reding, Michal Marek, linux-kbuild
  Cc: linux-mips, linux-kernel, Antony Pavlov, Lars-Peter Clausen,
	Maarten ter Huurne

On Sat, Sep 22, 2012 at 09:41:45AM +0200, Thierry Reding wrote:

> Have you had a chance to look at this? It is the last remaining PWM
> driver that isn't moved to the PWM framework yet. All the others are
> either in linux-next already and queued for 3.7 or have recently got
> Acked-by the respective maintainers (Unicore32). Patches 2 and 3 were
> already acked and tested by Lars-Peter who did the initial porting.
> Patch 1 can probably be dropped since I seem to be the only one running
> into that issue.
> 
> I really want to take this in for 3.7 so I can use the 3.7 cycle to
> transition from the legacy API to the new API and possibly even get rid
> of the legacy parts altogether. However I don't want to do this without
> the Acked-by from the MIPS maintainer.

Acked-by: Ralf Baechle <ralf@linux-mips.org>

for 2/3 and 3/3.

I now can reproduce the build error that 1/3 is supposed to fix.  The issue
is not as first suspected an odd bug in just your compiler.  The tree
(I was testing on today's -next) is building fine when compiling in-tree
but fails out of tree:

  CC      arch/mips/jz4740/irq.o
In file included from /home/ralf/src/linux/linux-jz4740/arch/mips/include/asm/irq.h:18:0,
                 from /home/ralf/src/linux/linux-jz4740/include/linux/irq.h:27,
                 from /home/ralf/src/linux/linux-jz4740/include/asm-generic/hardirq.h:12,
                 from /home/ralf/src/linux/linux-jz4740/arch/mips/include/asm/hardirq.h:16,
                 from /home/ralf/src/linux/linux-jz4740/include/linux/hardirq.h:7,
                 from /home/ralf/src/linux/linux-jz4740/include/linux/interrupt.h:12,
                 from /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c:19:
/home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.h:20:39: error: ‘struct irq_data’ declared inside parameter list [-Werror]
/home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.h:20:39: error: its scope is only this definition or declaration, which is probably not what you want [-Werror]
/home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.h:21:38: error: ‘struct irq_data’ declared inside parameter list [-Werror]
In file included from /home/ralf/src/linux/linux-jz4740/include/linux/irq.h:356:0,
                 from /home/ralf/src/linux/linux-jz4740/include/asm-generic/hardirq.h:12,
                 from /home/ralf/src/linux/linux-jz4740/arch/mips/include/asm/hardirq.h:16,
                 from /home/ralf/src/linux/linux-jz4740/include/linux/hardirq.h:7,
                 from /home/ralf/src/linux/linux-jz4740/include/linux/interrupt.h:12,
                 from /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c:19:
/home/ralf/src/linux/linux-jz4740/include/linux/irqdesc.h:73:33: error: ‘NR_IRQS’ undeclared here (not in a function)
/home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c: In function ‘jz4740_cascade’:
/home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c:49:39: error: ‘JZ4740_IRQ_BASE’ undeclared (first use in this function)
/home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c:49:39: note: each undeclared identifier is reported only once for each function it appears in
/home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c: At top level:
/home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c:62:6: error: conflicting types for ‘jz4740_irq_suspend’
In file included from /home/ralf/src/linux/linux-jz4740/arch/mips/include/asm/irq.h:18:0,
                 from /home/ralf/src/linux/linux-jz4740/include/linux/irq.h:27,
                 from /home/ralf/src/linux/linux-jz4740/include/asm-generic/hardirq.h:12,
                 from /home/ralf/src/linux/linux-jz4740/arch/mips/include/asm/hardirq.h:16,
                 from /home/ralf/src/linux/linux-jz4740/include/linux/hardirq.h:7,
                 from /home/ralf/src/linux/linux-jz4740/include/linux/interrupt.h:12,
                 from /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c:19:
/home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.h:20:13: note: previous declaration of ‘jz4740_irq_suspend’ was here
/home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c:68:6: error: conflicting types for ‘jz4740_irq_resume’
In file included from /home/ralf/src/linux/linux-jz4740/arch/mips/include/asm/irq.h:18:0,
                 from /home/ralf/src/linux/linux-jz4740/include/linux/irq.h:27,
                 from /home/ralf/src/linux/linux-jz4740/include/asm-generic/hardirq.h:12,
                 from /home/ralf/src/linux/linux-jz4740/arch/mips/include/asm/hardirq.h:16,
                 from /home/ralf/src/linux/linux-jz4740/include/linux/hardirq.h:7,
                 from /home/ralf/src/linux/linux-jz4740/include/linux/interrupt.h:12,
                 from /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c:19:
/home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.h:21:13: note: previous declaration of ‘jz4740_irq_resume’ was here
/home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c: In function ‘arch_init_irq’:
/home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c:91:41: error: ‘JZ4740_IRQ_BASE’ undeclared (first use in this function)

Which (while your patch is probably fine, I haven't tested) this seems to
be a build system issue, so should be preferably be fixed there.

Marek, the whole email exchange is archived at
http://www.linux-mips.org/archives/linux-mips/2012-09/threads.html

  Ralf

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

* Re: [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework
  2012-09-23 13:56   ` Ralf Baechle
@ 2012-09-23 17:12     ` Thierry Reding
  2012-09-27 19:48     ` Thierry Reding
  1 sibling, 0 replies; 15+ messages in thread
From: Thierry Reding @ 2012-09-23 17:12 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Michal Marek, linux-kbuild, linux-mips, linux-kernel,
	Antony Pavlov, Lars-Peter Clausen, Maarten ter Huurne

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

On Sun, Sep 23, 2012 at 03:56:35PM +0200, Ralf Baechle wrote:
> On Sat, Sep 22, 2012 at 09:41:45AM +0200, Thierry Reding wrote:
> 
> > Have you had a chance to look at this? It is the last remaining PWM
> > driver that isn't moved to the PWM framework yet. All the others are
> > either in linux-next already and queued for 3.7 or have recently got
> > Acked-by the respective maintainers (Unicore32). Patches 2 and 3 were
> > already acked and tested by Lars-Peter who did the initial porting.
> > Patch 1 can probably be dropped since I seem to be the only one running
> > into that issue.
> > 
> > I really want to take this in for 3.7 so I can use the 3.7 cycle to
> > transition from the legacy API to the new API and possibly even get rid
> > of the legacy parts altogether. However I don't want to do this without
> > the Acked-by from the MIPS maintainer.
> 
> Acked-by: Ralf Baechle <ralf@linux-mips.org>
> 
> for 2/3 and 3/3.

Alright, thanks. I'll take those through the PWM tree.

> I now can reproduce the build error that 1/3 is supposed to fix.  The issue
> is not as first suspected an odd bug in just your compiler.  The tree
> (I was testing on today's -next) is building fine when compiling in-tree
> but fails out of tree:

Okay, that makes sense. I rarely if even build in-tree nowadays.

>   CC      arch/mips/jz4740/irq.o
> In file included from /home/ralf/src/linux/linux-jz4740/arch/mips/include/asm/irq.h:18:0,
>                  from /home/ralf/src/linux/linux-jz4740/include/linux/irq.h:27,
>                  from /home/ralf/src/linux/linux-jz4740/include/asm-generic/hardirq.h:12,
>                  from /home/ralf/src/linux/linux-jz4740/arch/mips/include/asm/hardirq.h:16,
>                  from /home/ralf/src/linux/linux-jz4740/include/linux/hardirq.h:7,
>                  from /home/ralf/src/linux/linux-jz4740/include/linux/interrupt.h:12,
>                  from /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c:19:
> /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.h:20:39: error: ‘struct irq_data’ declared inside parameter list [-Werror]
> /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.h:20:39: error: its scope is only this definition or declaration, which is probably not what you want [-Werror]
> /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.h:21:38: error: ‘struct irq_data’ declared inside parameter list [-Werror]
> In file included from /home/ralf/src/linux/linux-jz4740/include/linux/irq.h:356:0,
>                  from /home/ralf/src/linux/linux-jz4740/include/asm-generic/hardirq.h:12,
>                  from /home/ralf/src/linux/linux-jz4740/arch/mips/include/asm/hardirq.h:16,
>                  from /home/ralf/src/linux/linux-jz4740/include/linux/hardirq.h:7,
>                  from /home/ralf/src/linux/linux-jz4740/include/linux/interrupt.h:12,
>                  from /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c:19:
> /home/ralf/src/linux/linux-jz4740/include/linux/irqdesc.h:73:33: error: ‘NR_IRQS’ undeclared here (not in a function)
> /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c: In function ‘jz4740_cascade’:
> /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c:49:39: error: ‘JZ4740_IRQ_BASE’ undeclared (first use in this function)
> /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c:49:39: note: each undeclared identifier is reported only once for each function it appears in
> /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c: At top level:
> /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c:62:6: error: conflicting types for ‘jz4740_irq_suspend’
> In file included from /home/ralf/src/linux/linux-jz4740/arch/mips/include/asm/irq.h:18:0,
>                  from /home/ralf/src/linux/linux-jz4740/include/linux/irq.h:27,
>                  from /home/ralf/src/linux/linux-jz4740/include/asm-generic/hardirq.h:12,
>                  from /home/ralf/src/linux/linux-jz4740/arch/mips/include/asm/hardirq.h:16,
>                  from /home/ralf/src/linux/linux-jz4740/include/linux/hardirq.h:7,
>                  from /home/ralf/src/linux/linux-jz4740/include/linux/interrupt.h:12,
>                  from /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c:19:
> /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.h:20:13: note: previous declaration of ‘jz4740_irq_suspend’ was here
> /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c:68:6: error: conflicting types for ‘jz4740_irq_resume’
> In file included from /home/ralf/src/linux/linux-jz4740/arch/mips/include/asm/irq.h:18:0,
>                  from /home/ralf/src/linux/linux-jz4740/include/linux/irq.h:27,
>                  from /home/ralf/src/linux/linux-jz4740/include/asm-generic/hardirq.h:12,
>                  from /home/ralf/src/linux/linux-jz4740/arch/mips/include/asm/hardirq.h:16,
>                  from /home/ralf/src/linux/linux-jz4740/include/linux/hardirq.h:7,
>                  from /home/ralf/src/linux/linux-jz4740/include/linux/interrupt.h:12,
>                  from /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c:19:
> /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.h:21:13: note: previous declaration of ‘jz4740_irq_resume’ was here
> /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c: In function ‘arch_init_irq’:
> /home/ralf/src/linux/linux-jz4740/arch/mips/jz4740/irq.c:91:41: error: ‘JZ4740_IRQ_BASE’ undeclared (first use in this function)
> 
> Which (while your patch is probably fine, I haven't tested) this seems to
> be a build system issue, so should be preferably be fixed there.
> 
> Marek, the whole email exchange is archived at
> http://www.linux-mips.org/archives/linux-mips/2012-09/threads.html

Okay. I think I mentioned in the commit message for patch 1 that the
include order seems to cause this. Maybe it's just a matter of
rearranging those.

Thierry

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

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

* Re: [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework
  2012-09-23 13:56   ` Ralf Baechle
  2012-09-23 17:12     ` Thierry Reding
@ 2012-09-27 19:48     ` Thierry Reding
  1 sibling, 0 replies; 15+ messages in thread
From: Thierry Reding @ 2012-09-27 19:48 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Michal Marek, linux-kbuild, linux-mips, linux-kernel,
	Antony Pavlov, Lars-Peter Clausen, Maarten ter Huurne

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

On Sun, Sep 23, 2012 at 03:56:35PM +0200, Ralf Baechle wrote:
> On Sat, Sep 22, 2012 at 09:41:45AM +0200, Thierry Reding wrote:
> 
> > Have you had a chance to look at this? It is the last remaining PWM
> > driver that isn't moved to the PWM framework yet. All the others are
> > either in linux-next already and queued for 3.7 or have recently got
> > Acked-by the respective maintainers (Unicore32). Patches 2 and 3 were
> > already acked and tested by Lars-Peter who did the initial porting.
> > Patch 1 can probably be dropped since I seem to be the only one running
> > into that issue.
> > 
> > I really want to take this in for 3.7 so I can use the 3.7 cycle to
> > transition from the legacy API to the new API and possibly even get rid
> > of the legacy parts altogether. However I don't want to do this without
> > the Acked-by from the MIPS maintainer.
> 
> Acked-by: Ralf Baechle <ralf@linux-mips.org>
> 
> for 2/3 and 3/3.

Applied both patches, thanks.

Thierry

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

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

end of thread, other threads:[~2012-09-27 19:49 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-10 12:05 [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework Thierry Reding
2012-09-10 12:05 ` [PATCH v2 1/3] MIPS: JZ4740: Break circular header dependency Thierry Reding
2012-09-10 12:05 ` [PATCH v2 2/3] MIPS: JZ4740: Export timer API Thierry Reding
2012-09-10 12:05 ` [PATCH v2 3/3] pwm: Add Ingenic JZ4740 support Thierry Reding
2012-09-10 21:51   ` Lars-Peter Clausen
2012-09-11  5:02     ` Thierry Reding
2012-09-11 17:54       ` Lars-Peter Clausen
2012-09-10 15:20 ` [PATCH v2 0/3] MIPS: JZ4740: Move PWM driver to PWM framework Lars-Peter Clausen
2012-09-10 17:30   ` Thierry Reding
2012-09-11 17:56     ` Lars-Peter Clausen
2012-09-12 15:02       ` Thierry Reding
2012-09-22  7:41 ` Thierry Reding
2012-09-23 13:56   ` Ralf Baechle
2012-09-23 17:12     ` Thierry Reding
2012-09-27 19:48     ` Thierry Reding

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