Final Samsung PWM support cleanup

This patch moves all Samsung platforms using PWM clocksource from legacy samsung-time to new samsung-pwm-timer driver.

Signed-off-by Tomasz Figa<[email protected]>

---
  arch/arm/mach-s3c24xx/Kconfig  | 12 ++++++------
  arch/arm/mach-s3c24xx/common.c | 16 ++++++++++++++++
  arch/arm/mach-s3c64xx/Kconfig  |  4 ++--
  arch/arm/mach-s3c64xx/common.c | 17 +++++++++++++++++
  arch/arm/mach-s5p64x0/Kconfig  |  4 ++--
  arch/arm/mach-s5p64x0/common.c | 17 +++++++++++++++++
  arch/arm/mach-s5pc100/Kconfig  |  2 +-
  arch/arm/mach-s5pc100/common.c | 17 +++++++++++++++++
  arch/arm/mach-s5pv210/Kconfig  |  2 +-
  arch/arm/mach-s5pv210/common.c | 17 +++++++++++++++++
  10 files changed, 96 insertions(+), 12 deletions(-)

arch/arm/mach-s3c24xx/Kconfig +0 -0

index 6d9252e..46f9a9c 100644

bool
help
Enable S3C6410 CPU support

Quoted 24 lines · Tomasz Figa

This patch moves all Samsung platforms using PWM clocksource from legacy samsung-time to new samsung-pwm-timer driver.

Signed-off-by Tomasz Figa<[email protected]>

---

  arch/arm/mach-s3c24xx/Kconfig  | 12 ++++++------
  arch/arm/mach-s3c24xx/common.c | 16 ++++++++++++++++
  arch/arm/mach-s3c64xx/Kconfig  |  4 ++--
  arch/arm/mach-s3c64xx/common.c | 17 +++++++++++++++++
  arch/arm/mach-s5p64x0/Kconfig  |  4 ++--
  arch/arm/mach-s5p64x0/common.c | 17 +++++++++++++++++
  arch/arm/mach-s5pc100/Kconfig  |  2 +-
  arch/arm/mach-s5pc100/common.c | 17 +++++++++++++++++
  arch/arm/mach-s5pv210/Kconfig  |  2 +-
  arch/arm/mach-s5pv210/common.c | 17 +++++++++++++++++
  10 files changed, 96 insertions(+), 12 deletions(-)

(quoted diff) +0 -0

b/arch/arm/mach-s3c24xx/Kconfig index 6d9252e..46f9a9c 100644

arch/arm/mach-s3c24xx/Kconfig +0 -0

@@ -30,7 +30,7 @@ config CPU_S3C2410
 

arch/arm/mach-s3c24xx/Kconfig config CPU_S3C2410 · line 33

Quoted 6 lines

arch/arm/mach-s3c24xx/Kconfig +1 -1

  	select S3C2410_CLOCK
  	select ARM_S3C2410_CPUFREQ if ARM_S3C24XX_CPUFREQ
  	select S3C2410_PM if PM
 
-	select SAMSUNG_HRT
+	select CLKSRC_SAMSUNG_PWM

Please put this in alphabetical ordering, this is just replacing though.

arch/arm/mach-s3c24xx/Kconfig config CPU_S3C2412 · line 47

Quoted 13 lines

arch/arm/mach-s3c24xx/Kconfig +0 -0

  	select SAMSUNG_WDT_RESET

help

Support for S3C2410 and S3C2410A family from the S3C24XX line

arch/arm/mach-s3c24xx/Kconfig +1 -1

@@ -42,7 +42,7 @@ config CPU_S3C2412
 
  	select CPU_LLSERIAL_S3C2440
  	select S3C2412_DMA if S3C24XX_DMA
  	select S3C2412_PM if PM
 
-	select SAMSUNG_HRT
+	select CLKSRC_SAMSUNG_PWM

Same as above.

arch/arm/mach-s3c24xx/Kconfig config CPU_S3C2416 · line 59

Quoted 12 lines

arch/arm/mach-s3c24xx/Kconfig +0 -0

  	help

Support for the S3C2412 and S3C2413 SoCs from the S3C24XX line

arch/arm/mach-s3c24xx/Kconfig +1 -1

@@ -54,7 +54,7 @@ config CPU_S3C2416
 
  	select S3C2443_COMMON
  	select S3C2443_DMA if S3C24XX_DMA
  	select SAMSUNG_CLKSRC
 
-	select SAMSUNG_HRT
+	select CLKSRC_SAMSUNG_PWM

Same as above.

arch/arm/mach-s3c24xx/Kconfig config CPU_S3C2440 · line 70

Quoted 12 lines

arch/arm/mach-s3c24xx/Kconfig +0 -0

  	help

Support for the S3C2416 SoC from the S3C24XX line

arch/arm/mach-s3c24xx/Kconfig +1 -1

@@ -65,7 +65,7 @@ config CPU_S3C2440
 
  	select S3C2410_CLOCK
  	select S3C2410_PM if PM
  	select S3C2440_DMA if S3C24XX_DMA
 
-	select SAMSUNG_HRT
+	select CLKSRC_SAMSUNG_PWM

Same as above.

arch/arm/mach-s3c24xx/Kconfig config CPU_S3C2442 · line 80

Quoted 12 lines

arch/arm/mach-s3c24xx/Kconfig +0 -0

  	help

Support for S3C2440 Samsung Mobile CPU based systems.

arch/arm/mach-s3c24xx/Kconfig +1 -1

@@ -75,7 +75,7 @@ config CPU_S3C2442
 
  	select CPU_LLSERIAL_S3C2440
  	select S3C2410_CLOCK
  	select S3C2410_PM if PM
 
-	select SAMSUNG_HRT
+	select CLKSRC_SAMSUNG_PWM

Same.

arch/arm/mach-s3c24xx/Kconfig config CPU_S3C2443 · line 96

Quoted 12 lines

arch/arm/mach-s3c24xx/Kconfig +0 -0

  	help

Support for S3C2442 Samsung Mobile CPU based systems.

arch/arm/mach-s3c24xx/Kconfig +1 -1

@@ -91,7 +91,7 @@ config CPU_S3C2443
 
  	select S3C2443_COMMON
  	select S3C2443_DMA if S3C24XX_DMA
  	select SAMSUNG_CLKSRC
 
-	select SAMSUNG_HRT
+	select CLKSRC_SAMSUNG_PWM

Same.

[...]

arch/arm/mach-s3c24xx/Kconfig config CPU_S3C2443

Quoted 4 lines

arch/arm/mach-s3c24xx/Kconfig +1 -1

  config CPU_S3C6400
 
-	select SAMSUNG_HRT
+	select CLKSRC_SAMSUNG_PWM

Same.

arch/arm/mach-s3c24xx/Kconfig

Quoted 9 lines

bool
help

Enable S3C6400 CPU support

config CPU_S3C6410

arch/arm/mach-s3c24xx/Kconfig +1 -1

-	select SAMSUNG_HRT
+	select CLKSRC_SAMSUNG_PWM

Same.

Quoted 4 lines

bool
help

Enable S3C6410 CPU support

[...]

arch/arm/mach-s3c24xx/Kconfig config CPU_S5P6440 · line 16

Quoted 8 lines

arch/arm/mach-s3c24xx/Kconfig +1 -1

@@ -11,14 +11,14 @@ config CPU_S5P6440
 
  	bool
  	select S5P_SLEEP if PM
  	select SAMSUNG_DMADEV
 
-	select SAMSUNG_HRT
+	select CLKSRC_SAMSUNG_PWM

Same.

arch/arm/mach-s3c24xx/Kconfig config CPU_S5P6440

Quoted 11 lines

arch/arm/mach-s3c24xx/Kconfig +1 -1

  	select SAMSUNG_WAKEMASK if PM
  	help
 
  	  Enable S5P6440 CPU support
 
  config CPU_S5P6450
 
  	bool
 
-	select SAMSUNG_HRT
+	select CLKSRC_SAMSUNG_PWM

Same.

[...]

arch/arm/mach-s3c24xx/Kconfig config CPU_S5PC100 · line 16

Quoted 8 lines

arch/arm/mach-s3c24xx/Kconfig +1 -1

@@ -11,7 +11,7 @@ config CPU_S5PC100
 
  	bool
  	select S5P_EXT_INT
  	select SAMSUNG_DMADEV
 
-	select SAMSUNG_HRT
+	select CLKSRC_SAMSUNG_PWM

Same.

[...]

Quoted 6 lines

arch/arm/mach-s3c24xx/Kconfig +0 -0

@@ -15,7 +15,7 @@ config CPU_S5PV210
 
  	select S5P_PM if PM
  	select S5P_SLEEP if PM
  	select SAMSUNG_DMADEV
 

arch/arm/mach-s3c24xx/Kconfig config CPU_S5PV210 · line 20

Quoted 2 lines

arch/arm/mach-s3c24xx/Kconfig +1 -1

-	select SAMSUNG_HRT
+	select CLKSRC_SAMSUNG_PWM

Same.

[...]

Let me fix it...

Quoted 4 lines

Review

Quoted 2 lines

(quoted diff) +1 -1

-	select SAMSUNG_HRT
+	select CLKSRC_SAMSUNG_PWM

Please put this in alphabetical ordering, this is just replacing though.

Argh, I always forget about this ordering, sorry.

Quoted 1 line

Let me fix it...

On 06/25/13 00:38, Tomasz Figa wrote:

[...]

Quoted 6 lines

Quoted 5 lines

Review

Quoted 2 lines

(quoted diff) +1 -1

-	select SAMSUNG_HRT
+	select CLKSRC_SAMSUNG_PWM

Please put this in alphabetical ordering, this is just replacing though.>

Argh, I always forget about this ordering, sorry.

no problem ;-)

[...]

Quoted 1 line

Quoted 1 line

Let me fix it...

like following :-)

8<----------------------------------------------

arch/arm/Kconfig +5 -0

index 49d993c..734d85b 100644
@@ -694,6 +694,7 @@ config ARCH_S3C24XX
  	select ARCH_REQUIRE_GPIOLIB
  	select CLKDEV_LOOKUP
  	select CLKSRC_MMIO
+	select CLKSRC_SAMSUNG_PWM
  	select GENERIC_CLOCKEVENTS
  	select HAVE_CLK
  	select HAVE_S3C2410_I2C if I2C

@@ -715,6 +716,7 @@ config ARCH_S3C64XX
  	select ARM_VIC
  	select CLKDEV_LOOKUP
  	select CLKSRC_MMIO
+	select CLKSRC_SAMSUNG_PWM
  	select CPU_V6
  	select GENERIC_CLOCKEVENTS
  	select HAVE_CLK

@@ -737,6 +739,7 @@ config ARCH_S5P64X0
  	bool "Samsung S5P6440 S5P6450"
  	select CLKDEV_LOOKUP
  	select CLKSRC_MMIO
+	select CLKSRC_SAMSUNG_PWM
  	select CPU_V6
  	select GENERIC_CLOCKEVENTS
  	select HAVE_CLK

@@ -753,6 +756,7 @@ config ARCH_S5PC100
  	select ARCH_REQUIRE_GPIOLIB
  	select CLKDEV_LOOKUP
  	select CLKSRC_MMIO
+	select CLKSRC_SAMSUNG_PWM
  	select CPU_V7
  	select GENERIC_CLOCKEVENTS
  	select HAVE_CLK

@@ -770,6 +774,7 @@ config ARCH_S5PV210
  	select ARCH_SPARSEMEM_ENABLE
  	select CLKDEV_LOOKUP
  	select CLKSRC_MMIO
+	select CLKSRC_SAMSUNG_PWM
  	select CPU_V7
  	select GENERIC_CLOCKEVENTS
  	select HAVE_CLK

(quoted diff) +0 -0

b/arch/arm/mach-s3c24xx/Kconfig index f2f7088..8124ec7 100644

arch/arm/mach-s3c24xx/Kconfig +0 -6

@@ -30,7 +30,6 @@ config CPU_S3C2410
  	select S3C2410_CLOCK
  	select S3C2410_CPUFREQ if CPU_FREQ_S3C24XX
  	select S3C2410_PM if PM
-	select SAMSUNG_HRT
  	help
  	  Support for S3C2410 and S3C2410A family from the S3C24XX line
  	  of Samsung Mobile CPUs.

@@ -41,7 +40,6 @@ config CPU_S3C2412
  	select CPU_LLSERIAL_S3C2440
  	select S3C2412_DMA if S3C24XX_DMA
  	select S3C2412_PM if PM
-	select SAMSUNG_HRT
  	help
  	  Support for the S3C2412 and S3C2413 SoCs from the S3C24XX line
 

@@ -53,7 +51,6 @@ config CPU_S3C2416
  	select S3C2443_COMMON
  	select S3C2443_DMA if S3C24XX_DMA
  	select SAMSUNG_CLKSRC
-	select SAMSUNG_HRT
  	help
  	  Support for the S3C2416 SoC from the S3C24XX line
 

@@ -64,7 +61,6 @@ config CPU_S3C2440
  	select S3C2410_CLOCK
  	select S3C2410_PM if PM
  	select S3C2440_DMA if S3C24XX_DMA
-	select SAMSUNG_HRT
  	help
  	  Support for S3C2440 Samsung Mobile CPU based systems.
 

@@ -74,7 +70,6 @@ config CPU_S3C2442
  	select CPU_LLSERIAL_S3C2440
  	select S3C2410_CLOCK
  	select S3C2410_PM if PM
-	select SAMSUNG_HRT
  	help
  	  Support for S3C2442 Samsung Mobile CPU based systems.
 

@@ -89,7 +84,6 @@ config CPU_S3C2443
  	select S3C2443_COMMON
  	select S3C2443_DMA if S3C24XX_DMA
  	select SAMSUNG_CLKSRC
-	select SAMSUNG_HRT
  	help
  	  Support for the S3C2443 SoC from the S3C24XX line
 

(quoted diff) +0 -0

b/arch/arm/mach-s3c64xx/Kconfig index 2057853..041da51 100644

arch/arm/mach-s3c64xx/Kconfig +0 -2

@@ -17,13 +17,11 @@ config PLAT_S3C64XX
  # Configuration options for the S3C6410 CPU
 
  config CPU_S3C6400
-	select SAMSUNG_HRT
  	bool
  	help
  	  Enable S3C6400 CPU support
 
  config CPU_S3C6410
-	select SAMSUNG_HRT
  	bool
  	help
  	  Enable S3C6410 CPU support

(quoted diff) +0 -0

b/arch/arm/mach-s5p64x0/Kconfig index 5a707bd..bb2111b 100644

arch/arm/mach-s5p64x0/Kconfig +0 -2

@@ -11,14 +11,12 @@ config CPU_S5P6440
  	bool
  	select S5P_SLEEP if PM
  	select SAMSUNG_DMADEV
-	select SAMSUNG_HRT
  	select SAMSUNG_WAKEMASK if PM
  	help
  	  Enable S5P6440 CPU support
 
  config CPU_S5P6450
  	bool
-	select SAMSUNG_HRT
  	select S5P_SLEEP if PM
  	select SAMSUNG_DMADEV
  	select SAMSUNG_WAKEMASK if PM

(quoted diff) +0 -0

b/arch/arm/mach-s5pc100/Kconfig index 2f456a4..15170be 100644

arch/arm/mach-s5pc100/Kconfig +0 -1

@@ -11,7 +11,6 @@ config CPU_S5PC100
  	bool
  	select S5P_EXT_INT
  	select SAMSUNG_DMADEV
-	select SAMSUNG_HRT
  	help
  	  Enable S5PC100 CPU support
 

(quoted diff) +0 -0

b/arch/arm/mach-s5pv210/Kconfig index 0963283..caaedaf 100644

This patch introduces new Samsung PWM driver, which is heavily cleaned, multiplatform aware and supports DeviceTree based instantiation.

Since on historical hardware PWM block can be shared with clocksource driver, a shared spinlock is used to protect access to shared registers, already exported from the clocksource driver.

Signed-off-by Tomasz Figa<[email protected]>

---
  drivers/pwm/Makefile      |   1 +
  drivers/pwm/pwm-samsung.c | 601 ++++++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 602 insertions(+)

create mode 100644 drivers/pwm/pwm-samsung.c

Changes since v2:
- Replaced __raw_{readl,writel} with {readl,writel}.
- Corrected commit message.

Quoted 17 lines · Tomasz Figa

This patch introduces new Samsung PWM driver, which is heavily cleaned,
multiplatform aware and supports DeviceTree based instantiation.

Since on historical hardware PWM block can be shared with clocksource driver, a shared spinlock is used to protect access to shared registers, already exported from the clocksource driver.

Signed-off-by Tomasz Figa<[email protected]>

---

  drivers/pwm/Makefile      |   1 +
  drivers/pwm/pwm-samsung.c | 601

++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 602 insertions(+)
create mode 100644 drivers/pwm/pwm-samsung.c

Quoted 3 lines

Changes since v2:
- Replaced __raw_{readl,writel} with {readl,writel}.
- Corrected commit message.

[...]

Review

Quoted 1 line

(quoted diff) +1 -0

+	of_property_for_each_u32(np, "samsung,pwm-outputs", prop, cur, 

Review

Quoted 3 lines

{

(quoted diff) +2 -0

+		if (val>= SAMSUNG_PWM_NUM) {
+			pr_warning("%s: invalid channel index in 

property\n",

[...]

drivers/pwm/pwm-samsung.c line 583

Quoted 2 lines

drivers/pwm/pwm-samsung.c new file +2 -0

+
+static struct dev_pm_ops pwm_samsung_pm_ops = {

WARNING: struct dev_pm_ops should normally be const #622: FILE: drivers/pwm/pwm-samsung.c:583:
+static struct dev_pm_ops pwm_samsung_pm_ops = {

fixed to add const when I applied.

Quoted 14 lines · Tomasz Figa

This patch introduces new Samsung PWM driver, which is heavily cleaned, multiplatform aware and supports DeviceTree based instantiation.

Since on historical hardware PWM block can be shared with clocksource driver, a shared spinlock is used to protect access to shared registers, already exported from the clocksource driver.

Signed-off-by Tomasz Figa<[email protected]>

---
 drivers/pwm/Makefile      |   1 +
 drivers/pwm/pwm-samsung.c | 601 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 602 insertions(+)
 create mode 100644 drivers/pwm/pwm-samsung.c

Quoted 4 lines

Changes since v2:
- Replaced __raw_{readl,writel} with {readl,writel}.
- Corrected commit message.

[...]

drivers/pwm/pwm-samsung.c line 432

Quoted 3 lines

drivers/pwm/pwm-samsung.c new file +3 -0

+	of_property_for_each_u32(np, "samsung,pwm-outputs", prop, cur, val) {
+		if (val>= SAMSUNG_PWM_NUM) {
+			pr_warning("%s: invalid channel index in samsung,pwm-outputs property\n",

Just note, checkpatch complains following, so fixed to use pr_warn() when I applied.

Quoted 18 lines · Kukjin Kim

Quoted 17 lines · Tomasz Figa

This patch introduces new Samsung PWM driver, which is heavily cleaned,
multiplatform aware and supports DeviceTree based instantiation.

Since on historical hardware PWM block can be shared with clocksource driver, a shared spinlock is used to protect access to shared registers, already exported from the clocksource driver.

Signed-off-by Tomasz Figa<[email protected]>

---

 drivers/pwm/Makefile      |   1 +
 drivers/pwm/pwm-samsung.c | 601
 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed,
 602 insertions(+)
 create mode 100644 drivers/pwm/pwm-samsung.c

Quoted 7 lines

Quoted 3 lines

Changes since v2:
- Replaced __raw_{readl,writel} with {readl,writel}.
- Corrected commit message.

[...]

Review

Quoted 1 line

(quoted diff) +1 -0

+	of_property_for_each_u32(np, "samsung,pwm-outputs", prop, cur, 

Quoted 3 lines

Review

Quoted 3 lines

{

(quoted diff) +2 -0

+		if (val>= SAMSUNG_PWM_NUM) {
+			pr_warning("%s: invalid channel index in 

Quoted 3 lines

Quoted 1 line

property\n",>

Just note, checkpatch complains following, so fixed to use pr_warn() when I applied.

Note that you can't apply patches that touch the PWM tree without my Ack and I already mentioned that the current way this driver is written isn't acceptable.

So either you fix it properly, or if everybody except me thinks we don't need a proper design for drivers anymore, then the only way I'll accept this driver into the PWM tree is if you put a really big comment at the top of the file saying that the driver is badly designed on purpose and that people shouldn't be using it as a reference.

Quoted 19 lines · On Monday 24 of June 2013 19:49:04 Thierry Reding wrote:

Quoted 18 lines · Kukjin Kim

Quoted 17 lines · Tomasz Figa

This patch introduces new Samsung PWM driver, which is heavily cleaned,
multiplatform aware and supports DeviceTree based instantiation.

Since on historical hardware PWM block can be shared with clocksource driver, a shared spinlock is used to protect access to shared registers, already exported from the clocksource driver.

Signed-off-by Tomasz Figa<[email protected]>

---

 drivers/pwm/Makefile      |   1 +
 drivers/pwm/pwm-samsung.c | 601
 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed,
 602 insertions(+)
 create mode 100644 drivers/pwm/pwm-samsung.c

Quoted 7 lines

Quoted 7 lines

Quoted 3 lines

Changes since v2:
- Replaced __raw_{readl,writel} with {readl,writel}.
- Corrected commit message.

[...]

Review

Quoted 1 line

(quoted diff) +1 -0

+	of_property_for_each_u32(np, "samsung,pwm-outputs", prop, cur, 

val)

Quoted 3 lines

Quoted 3 lines

Review

Quoted 3 lines

{

(quoted diff) +2 -0

+		if (val>= SAMSUNG_PWM_NUM) {
+			pr_warning("%s: invalid channel index in 

samsung,pwm-outputs

Quoted 8 lines

Quoted 3 lines

Quoted 1 line

property\n",>

Just note, checkpatch complains following, so fixed to use pr_warn() when I applied.

Note that you can't apply patches that touch the PWM tree without my Ack and I already mentioned that the current way this driver is written isn't acceptable.

Quoted 5 lines

So either you fix it properly, or if everybody except me thinks we don't need a proper design for drivers anymore, then the only way I'll accept this driver into the PWM tree is if you put a really big comment at the top of the file saying that the driver is badly designed on purpose and that people shouldn't be using it as a reference.

Sorry, I don't understand what problem you have with this design. It completely meets all the requirements applicable on hardware platforms it is (and going to be) used on.

The only thing it would do in a suboptimal way would be synchronization of register accesses for multiple instances of the driver - one spinlock would be used for all of them. This is insignificant because there is no time critical code in this driver and it is really unlikely that a SoC with multiple instances of this IP block shows up.

Channel reservation between clocksource and PWM drivers is completely correct, relying on the fact that the former can only use channels _without_ outputs and the latter can only use channels _with_ outputs. Do I have to add that there can't be a channel both with and without output?

So the only place for improvement here, without starting overengineering things, is a comment about the purpose of the spinlock and why it can be used in our case.

Quoted 2 lines · Tomasz Figa

Quoted 1 line · On Monday 24 of June 2013 19:49:04 Thierry Reding wrote:

On Tue, Jun 25, 2013 at 12:22:42AM +0900, Kukjin Kim wrote:

Quoted 16 lines

Quoted 15 lines

Quoted 4 lines

Just note, checkpatch complains following, so fixed to use pr_warn()
when I applied.

Note that you can't apply patches that touch the PWM tree without my Ack and I already mentioned that the current way this driver is written isn't acceptable.

So either you fix it properly, or if everybody except me thinks we don't need a proper design for drivers anymore, then the only way I'll accept this driver into the PWM tree is if you put a really big comment at the top of the file saying that the driver is badly designed on purpose and that people shouldn't be using it as a reference.

Quoted 3 lines

Sorry, I don't understand what problem you have with this design. It completely meets all the requirements applicable on hardware platforms it is (and going to be) used on.

My main problem with it is that there is no design. And as such it sets a bad example. If I accept this into the PWM tree as is then what am I supposed to tell the next person that comes up with a similarly broken driver?

Quoted 6 lines

The only thing it would do in a suboptimal way would be synchronization of register accesses for multiple instances of the driver - one spinlock would be used for all of them. This is insignificant because there is no time critical code in this driver and it is really unlikely that a SoC with multiple instances of this IP block shows up.

I've had people give me guarantees that this and that would *never* happen only to change their minds 6 months down the road. And again, even if it was actually true in this case, it isn't a valid excuse for setting a bad example.

Quoted 5 lines

Channel reservation between clocksource and PWM drivers is completely correct, relying on the fact that the former can only use channels _without_ outputs and the latter can only use channels _with_ outputs. Do I have to add that there can't be a channel both with and without output?

The issue is that you can't really ensure that both the clocksource and PWM drivers use the same variant and therefore might be using different masks.

Quoted 3 lines

So the only place for improvement here, without starting overengineering things, is a comment about the purpose of the spinlock and why it can be used in our case.

I disagree. You actually even had a version with a halfway decent design at some point and it was discarded. I don't think I'm asking for all that much here. I even gave you the option of admitting that the driver was suboptimal. All I request in return is that you mention that in the code so that either somebody else might go and clean things up or at least that nobody will copy from a bad example.

Quoted 1 line · On Monday 24 of June 2013 22:13:27 Thierry Reding wrote:

On Mon, Jun 24, 2013 at 08:31:43PM +0200, Tomasz Figa wrote:

Quoted 8 lines

Quoted 5 lines

Channel reservation between clocksource and PWM drivers is completely correct, relying on the fact that the former can only use channels _without_ outputs and the latter can only use channels _with_ outputs. Do I have to add that there can't be a channel both with and without output?

The issue is that you can't really ensure that both the clocksource and PWM drivers use the same variant and therefore might be using different masks.

Output mask is _not_ a part of the constant variant data. It is stored in the same struct, which is to simplify passing SoC-specific parameters from platform code to both drivers, but it is not defined as a constant anywhere in drivers.

As I already explained in one of my previous replies, it is either parsed from DT (samsung,pwm-outputs property) or passed through the variant struct as platform_data from board files. It isn't possible for both drivers to get different mask values.

Quoted 10 lines

Quoted 3 lines

So the only place for improvement here, without starting overengineering things, is a comment about the purpose of the spinlock and why it can be used in our case.

I disagree. You actually even had a version with a halfway decent design at some point and it was discarded. I don't think I'm asking for all that much here. I even gave you the option of admitting that the driver was suboptimal. All I request in return is that you mention that in the code so that either somebody else might go and clean things up or at least that nobody will copy from a bad example.

What about:

/*
* PWM block is shared between pwm-samsung and samsung_pwm_timer drivers
* and some registers need access synchronization. If both drivers are
* compiled in, the spinlock is defined in the clocksource driver,
* otherwise following definition is used.
*
* Currently we do not need any more complex synchronization method
* because all the supported SoCs contain only one instance of the PWM
* IP. Should this change, both drivers will need to be modified to
* properly synchronize accesses to particular instances.
*/

Quoted 2 lines · Tomasz Figa

Quoted 1 line · On Monday 24 of June 2013 22:13:27 Thierry Reding wrote:

On Mon, Jun 24, 2013 at 08:31:43PM +0200, Tomasz Figa wrote:

[...]

Quoted 20 lines

Quoted 9 lines

Quoted 8 lines

Channel reservation between clocksource and PWM drivers is completely
correct, relying on the fact that the former can only use channels _without_ outputs and the latter can only use channels _with_ outputs.
Do I have to add that there can't be a channel both with and without
output?

Quoted 5 lines

The issue is that you can't really ensure that both the clocksource and
PWM drivers use the same variant and therefore might be using different
masks.

Output mask is _not_ a part of the constant variant data. It is stored in the same struct, which is to simplify passing SoC-specific parameters from platform code to both drivers, but it is not defined as a constant anywhere in drivers.

Quoted 4 lines

As I already explained in one of my previous replies, it is either parsed from DT (samsung,pwm-outputs property) or passed through the variant struct as platform_data from board files. It isn't possible for both drivers to get different mask values.

But samsung_pwm_set_platdata() and samsung_pwm_clocksource_init() can still be called with different variants, can't they?

Furthermore the
variants aren't read-only and therefore can be modified at any point in time. So it is indeed possible for them to get different mask values.

Granted, it may be unlikely that they will but it wouldn't be all that difficult to write the driver in a way to make it really impossible and at the same time make sure that both driver share the same variant information.

Quoted 16 lines

Quoted 11 lines

Quoted 3 lines

So the only place for improvement here, without starting overengineering things, is a comment about the purpose of the spinlock and why it can be used in our case.

I disagree. You actually even had a version with a halfway decent design at some point and it was discarded. I don't think I'm asking for all that much here. I even gave you the option of admitting that the driver was suboptimal. All I request in return is that you mention that in the code so that either somebody else might go and clean things up or at least that nobody will copy from a bad example.

What about:

/*

Quoted 11 lines

* PWM block is shared between pwm-samsung and samsung_pwm_timer drivers
* and some registers need access synchronization. If both drivers are
* compiled in, the spinlock is defined in the clocksource driver,
* otherwise following definition is used.
*
* Currently we do not need any more complex synchronization method
* because all the supported SoCs contain only one instance of the PWM
* IP. Should this change, both drivers will need to be modified to
* properly synchronize accesses to particular instances. */

I see that you can't be persuaded. And everybody else seems to be okay with it so... have it your way. I'm probably going to regret this.

Quoted 1 line · On Monday 24 of June 2013 22:53:42 Thierry Reding wrote:

On Mon, Jun 24, 2013 at 10:32:55PM +0200, Tomasz Figa wrote:

Quoted 16 lines

Quoted 15 lines

What about:

/*

* PWM block is shared between pwm-samsung and samsung_pwm_timer drivers
* and some registers need access synchronization. If both drivers are
* compiled in, the spinlock is defined in the clocksource driver,
* otherwise following definition is used.
*
* Currently we do not need any more complex synchronization method
* because all the supported SoCs contain only one instance of the PWM
* IP. Should this change, both drivers will need to be modified to
* properly synchronize accesses to particular instances. */

Quoted 2 lines

I see that you can't be persuaded. And everybody else seems to be okay with it so... have it your way. I'm probably going to regret this.

This was just a proposal. Do you want anything else to be added to the comment or anything to be changed?

Quoted 2 lines · Tomasz Figa

Quoted 1 line · On Monday 24 of June 2013 22:53:42 Thierry Reding wrote:

On Mon, Jun 24, 2013 at 10:32:55PM +0200, Tomasz Figa wrote:

[...]

Quoted 22 lines

Quoted 21 lines

Quoted 4 lines

What about:

/*

Quoted 13 lines

* PWM block is shared between pwm-samsung and samsung_pwm_timer drivers
* and some registers need access synchronization. If both drivers are
* compiled in, the spinlock is defined in the clocksource driver,
* otherwise following definition is used.
*
* Currently we do not need any more complex synchronization method
* because all the supported SoCs contain only one instance of the PWM
* IP. Should this change, both drivers will need to be modified to
* properly synchronize accesses to particular instances. */

I see that you can't be persuaded. And everybody else seems to be okay
with it so... have it your way. I'm probably going to regret this.

Quoted 2 lines

This was just a proposal. Do you want anything else to be added to the comment or anything to be changed?

For reference, I still don't like this but since I'm the only one complaining, go ahead. With that comment added to the driver:

Acked-by Thierry Reding <[email protected]>

Kukjin, can you amend the comment I mentioned to this patch above the line with DEFINE_SPINLOCK or I should send fixed version?

Quoted 26 lines

Quoted 16 lines

Quoted 16 lines

Quoted 15 lines

/*

* PWM block is shared between pwm-samsung and samsung_pwm_timer drivers
* and some registers need access synchronization. If both drivers are
* compiled in, the spinlock is defined in the clocksource driver,
* otherwise following definition is used.
*
* Currently we do not need any more complex synchronization method
* because all the supported SoCs contain only one instance of the PWM
* IP. Should this change, both drivers will need to be modified to
* properly synchronize accesses to particular instances. */

Quoted 6 lines

Quoted 3 lines

I see that you can't be persuaded. And everybody else seems to be okay
with it so... have it your way. I'm probably going to regret this.

This was just a proposal. Do you want anything else to be added to the comment or anything to be changed?

For reference, I still don't like this but since I'm the only one complaining, go ahead. With that comment added to the driver:

Quoted 1 line

Acked-by Thierry Reding<[email protected]>

OK. Thank you.

Kukjin, can you amend the comment I mentioned to this patch above the line with DEFINE_SPINLOCK or I should send fixed version?

Note that you can't apply patches that touch the PWM tree without my Ack and I already mentioned that the current way this driver is written isn't acceptable.

So either you fix it properly, or if everybody except me thinks we don't need a proper design for drivers anymore, then the only way I'll accept this driver into the PWM tree is if you put a really big comment at the top of the file saying that the driver is badly designed on purpose and that people shouldn't be using it as a reference.

Since we now have a proper Samsung PWM clocksource driver in place, we can proceed with further cleanup of PWM timers support on Samsung SoCs.

This series attempts to achieve this goal by:
1) moving remaining Samsung platforms to the new clocksource driver
2) removing old clocksource driver
3) adding new multiplatform- and DT-aware PWM driver
4) moving all Samsung platforms to use the new PWM driver
5) removing old PWM driver
6) removing all PWM-related code that is not used anymore

Cleaning up the PWM driver is a bit tricky, because the design of current driver makes it completely unsuitable for DT and multiplatform and would require a heavy rework to make it usable, breaking any existing Samsung PWM users by the way. To avoid any breakage I decided to keep the old driver, add new one, move all platforms to it and then remove the old one.

See particular patches for more detailed descriptions.

On S3C6410-based Tiny6410 (Mini6410-compatible), after enabling pwm-beeper and Exynos4210-based Origen board (with PWM0 attached to a scope):

Tested-by Tomasz Figa <[email protected]>

Changes since v1:
- made sure that suspend/resume is handled correctly in both clocksource and PWM drivers,
- fixed incorrect definition of AUTORELOAD bit for channel 4,
- fixed order of registering PWM device and calling samsung_bl_set(), which assumes that PWM device has been already registered,
- corrected commit messages of several patches,
- addressed all the valid comments from reviewers, including cleanup of most code originally copied from previous PWM driver,
- rebased on top of current Kgene's for-next,
- tested on Exynos4210, verifying correct PWM output with a scope.

Needs testing on other platforms as I could only compile test for them. Tomasz Figa (18):
ARM: SAMSUNG: Unify base address definitions of timer block
ARM: SAMSUNG: Add new PWM platform device
ARM: SAMSUNG: Set PWM platform data
clocksource: samsung_pwm_timer: Cache clocksource register address
clocksource: samsung_pwm_timer: Do not use clocksource_mmio
clocksource: samsung_pwm_timer: Handle suspend/resume correctly
ARM: SAMSUNG: Move all platforms to new clocksource driver
ARM: SAMSUNG: Remove old samsung-time driver
ARM: SAMSUNG: Remove unused PWM timer IRQ chip code
pwm: samsung: Rename to pwm-samsung-legacy
pwm: Add new pwm-samsung driver
ARM: SAMSUNG: Rework private data handling in dev-backlight
ARM: SAMSUNG: Modify board files to use new PWM platform device
pwm: Remove superseded pwm-samsung-legacy driver
ARM: SAMSUNG: Remove old PWM timer platform devices
ARM: SAMSUNG: Remove pwm-clock infrastructure
ARM: SAMSUNG: Remove remaining uses of plat/regs-timer.h header
ARM: SAMSUNG: Remove plat/regs-timer.h header

 arch/arm/Kconfig                                   |   1 -
 arch/arm/mach-s3c24xx/Kconfig                      |  12 +-
 arch/arm/mach-s3c24xx/clock-s3c2410.c              |   1 -
 arch/arm/mach-s3c24xx/clock-s3c2412.c              |   1 -
 arch/arm/mach-s3c24xx/clock-s3c2416.c              |   2 -
 arch/arm/mach-s3c24xx/clock-s3c2443.c              |   2 -
 arch/arm/mach-s3c24xx/common.c                     |  27 +
 arch/arm/mach-s3c24xx/include/mach/map.h           |   2 +
 arch/arm/mach-s3c24xx/mach-h1940.c                 |   4 +-
 arch/arm/mach-s3c24xx/mach-rx1950.c                |   5 +-
 arch/arm/mach-s3c64xx/Kconfig                      |   4 +-
 arch/arm/mach-s3c64xx/clock.c                      |   2 -
 arch/arm/mach-s3c64xx/common.c                     |  32 +-
 arch/arm/mach-s3c64xx/include/mach/irqs.h          |   8 -
 arch/arm/mach-s3c64xx/include/mach/map.h           |   1 +
 arch/arm/mach-s3c64xx/irq-pm.c                     |   2 -
 arch/arm/mach-s3c64xx/mach-crag6410.c              |   4 +-
 arch/arm/mach-s3c64xx/mach-hmt.c                   |   4 +-
 arch/arm/mach-s3c64xx/mach-smartq.c                |   4 +-
 arch/arm/mach-s3c64xx/mach-smdk6410.c              |   5 +-
 arch/arm/mach-s5p64x0/Kconfig                      |   4 +-
 arch/arm/mach-s5p64x0/clock-s5p6440.c              |   2 -
 arch/arm/mach-s5p64x0/clock-s5p6450.c              |   2 -
 arch/arm/mach-s5p64x0/common.c                     |  27 +
 arch/arm/mach-s5p64x0/include/mach/irqs.h          |   2 -
 arch/arm/mach-s5p64x0/include/mach/map.h           |   1 +
 arch/arm/mach-s5p64x0/mach-smdk6440.c              |   5 +-
 arch/arm/mach-s5p64x0/mach-smdk6450.c              |   5 +-
 arch/arm/mach-s5p64x0/pm.c                         |   3 -
 arch/arm/mach-s5pc100/Kconfig                      |   2 +-
 arch/arm/mach-s5pc100/clock.c                      |   2 -
 arch/arm/mach-s5pc100/common.c                     |  28 +
 arch/arm/mach-s5pc100/include/mach/irqs.h          |   2 -
 arch/arm/mach-s5pc100/include/mach/map.h           |   1 +
 arch/arm/mach-s5pc100/mach-smdkc100.c              |   5 +-
 arch/arm/mach-s5pv210/Kconfig                      |   2 +-
 arch/arm/mach-s5pv210/clock.c                      |   1 -
 arch/arm/mach-s5pv210/common.c                     |  28 +
 arch/arm/mach-s5pv210/include/mach/irqs.h          |   2 -
 arch/arm/mach-s5pv210/include/mach/map.h           |   1 +
 arch/arm/mach-s5pv210/mach-smdkv210.c              |   5 +-
 arch/arm/mach-s5pv210/pm.c                         |  10 -
 arch/arm/plat-samsung/Kconfig                      |  14 -
 arch/arm/plat-samsung/Makefile                     |   3 -
 arch/arm/plat-samsung/dev-backlight.c              |  61 +-
 arch/arm/plat-samsung/devs.c                       |  42 +-
 arch/arm/plat-samsung/include/plat/clock.h         |   4 -
 arch/arm/plat-samsung/include/plat/devs.h          |   1 +
 arch/arm/plat-samsung/include/plat/irq-vic-timer.h |  13 -
 arch/arm/plat-samsung/include/plat/irqs.h          |   9 -
 arch/arm/plat-samsung/include/plat/pwm-clock.h     |  81 ---
 arch/arm/plat-samsung/include/plat/pwm-core.h      |  24 +
 arch/arm/plat-samsung/include/plat/regs-timer.h    | 124 ----
 arch/arm/plat-samsung/include/plat/samsung-time.h  |  23 -
 arch/arm/plat-samsung/irq-vic-timer.c              |  98 ---
 arch/arm/plat-samsung/pwm-clock.c                  | 474 --------------
arch/arm/plat-samsung/s5p-irq.c                    |   4 -
 arch/arm/plat-samsung/samsung-time.c               | 394 ------------
 drivers/clocksource/samsung_pwm_timer.c            |  84 +--
 drivers/pwm/pwm-samsung.c                          | 694
++++++++++++++------- 60 files changed, 766 insertions(+), 1644
deletions(-)
 delete mode 100644 arch/arm/plat-samsung/include/plat/irq-vic-timer.h
 delete mode 100644 arch/arm/plat-samsung/include/plat/pwm-clock.h
 create mode 100644 arch/arm/plat-samsung/include/plat/pwm-core.h
 delete mode 100644 arch/arm/plat-samsung/include/plat/regs-timer.h
 delete mode 100644 arch/arm/plat-samsung/irq-vic-timer.c
 delete mode 100644 arch/arm/plat-samsung/pwm-clock.c
 delete mode 100644 arch/arm/plat-samsung/samsung-time.c

Since we now have a proper Samsung PWM clocksource driver in place, we can proceed with further cleanup of PWM timers support on Samsung SoCs.

This series attempts to achieve this goal by:
1) moving remaining Samsung platforms to the new clocksource driver
2) removing old clocksource driver
3) adding new multiplatform- and DT-aware PWM driver
4) moving all Samsung platforms to use the new PWM driver
5) removing old PWM driver
6) removing all PWM-related code that is not used anymore

Cleaning up the PWM driver is a bit tricky, because the design of current driver makes it completely unsuitable for DT and multiplatform and would require a heavy rework to make it usable, breaking any existing Samsung PWM users by the way. To avoid any breakage I decided to keep the old driver, add new one, move all platforms to it and then remove the old one.

See particular patches for more detailed descriptions.

On S3C6410-based Tiny6410 (Mini6410-compatible), after enabling pwm-beeper and Exynos4210-based Origen board (with PWM0 attached to a scope):

Tested-by Tomasz Figa <[email protected]>

Quoted 24 lines · Tomasz Figa

Since we now have a proper Samsung PWM clocksource driver in place, we can proceed with further cleanup of PWM timers support on Samsung SoCs.>
This series attempts to achieve this goal by:
1) moving remaining Samsung platforms to the new clocksource driver
2) removing old clocksource driver
3) adding new multiplatform- and DT-aware PWM driver
4) moving all Samsung platforms to use the new PWM driver
5) removing old PWM driver
6) removing all PWM-related code that is not used anymore

Cleaning up the PWM driver is a bit tricky, because the design of current driver makes it completely unsuitable for DT and multiplatform and would require a heavy rework to make it usable, breaking any existing Samsung PWM users by the way. To avoid any breakage I decided to keep the old driver, add new one, move all platforms to it and then remove the old one.

See particular patches for more detailed descriptions.

On S3C6410-based Tiny6410 (Mini6410-compatible), after enabling pwm-beeper and Exynos4210-based Origen board (with PWM0 attached to a scope):

Quoted 1 line

Tested-by Tomasz Figa <[email protected]>

Looks good to me,

Acked-by Arnd Bergmann <[email protected]>

Two small comments:

* always use 'git format-patch -M' to detect renames (patch 10)

* for new code, don't use __raw_readl and similar functions but use readl_relaxed or plain readl.

Since we now have a proper Samsung PWM clocksource driver in place, we can proceed with further cleanup of PWM timers support on Samsung SoCs.

This series attempts to achieve this goal by:
1) moving remaining Samsung platforms to the new clocksource driver
2) removing old clocksource driver
3) adding new multiplatform- and DT-aware PWM driver
4) moving all Samsung platforms to use the new PWM driver
5) removing old PWM driver
6) removing all PWM-related code that is not used anymore

Cleaning up the PWM driver is a bit tricky, because the design of current driver makes it completely unsuitable for DT and multiplatform and would require a heavy rework to make it usable, breaking any existing Samsung PWM users by the way. To avoid any breakage I decided to keep the old driver, add new one, move all platforms to it and then remove the old one.

See particular patches for more detailed descriptions.

On S3C6410-based Tiny6410 (Mini6410-compatible), after enabling pwm-beeper and Exynos4210-based Origen board (with PWM0 attached to a scope):

Tested-by Tomasz Figa <[email protected]>

Changes since v1:
- made sure that suspend/resume is handled correctly in both clocksource and PWM drivers,
- fixed incorrect definition of AUTORELOAD bit for channel 4,
- fixed order of registering PWM device and calling samsung_bl_set(), which assumes that PWM device has been already registered,
- corrected commit messages of several patches,
- addressed all the valid comments from reviewers, including cleanup of most code originally copied from previous PWM driver,
- rebased on top of current Kgene's for-next,
- tested on Exynos4210, verifying correct PWM output with a scope.

Needs testing on other platforms as I could only compile test for them. Tomasz Figa (18):
ARM: SAMSUNG: Unify base address definitions of timer block
ARM: SAMSUNG: Add new PWM platform device
ARM: SAMSUNG: Set PWM platform data
clocksource: samsung_pwm_timer: Cache clocksource register address
clocksource: samsung_pwm_timer: Do not use clocksource_mmio
clocksource: samsung_pwm_timer: Handle suspend/resume correctly
ARM: SAMSUNG: Move all platforms to new clocksource driver
ARM: SAMSUNG: Remove old samsung-time driver
ARM: SAMSUNG: Remove unused PWM timer IRQ chip code
pwm: samsung: Rename to pwm-samsung-legacy
pwm: Add new pwm-samsung driver
ARM: SAMSUNG: Rework private data handling in dev-backlight
ARM: SAMSUNG: Modify board files to use new PWM platform device
pwm: Remove superseded pwm-samsung-legacy driver
ARM: SAMSUNG: Remove old PWM timer platform devices
ARM: SAMSUNG: Remove pwm-clock infrastructure
ARM: SAMSUNG: Remove remaining uses of plat/regs-timer.h header
ARM: SAMSUNG: Remove plat/regs-timer.h header

 arch/arm/Kconfig                                   |   1 -
 arch/arm/mach-s3c24xx/Kconfig                      |  12 +-
 arch/arm/mach-s3c24xx/clock-s3c2410.c              |   1 -
 arch/arm/mach-s3c24xx/clock-s3c2412.c              |   1 -
 arch/arm/mach-s3c24xx/clock-s3c2416.c              |   2 -
 arch/arm/mach-s3c24xx/clock-s3c2443.c              |   2 -
 arch/arm/mach-s3c24xx/common.c                     |  27 +
 arch/arm/mach-s3c24xx/include/mach/map.h           |   2 +
 arch/arm/mach-s3c24xx/mach-h1940.c                 |   4 +-
 arch/arm/mach-s3c24xx/mach-rx1950.c                |   5 +-
 arch/arm/mach-s3c64xx/Kconfig                      |   4 +-
 arch/arm/mach-s3c64xx/clock.c                      |   2 -
 arch/arm/mach-s3c64xx/common.c                     |  32 +-
 arch/arm/mach-s3c64xx/include/mach/irqs.h          |   8 -
 arch/arm/mach-s3c64xx/include/mach/map.h           |   1 +
 arch/arm/mach-s3c64xx/irq-pm.c                     |   2 -
 arch/arm/mach-s3c64xx/mach-crag6410.c              |   4 +-
 arch/arm/mach-s3c64xx/mach-hmt.c                   |   4 +-
 arch/arm/mach-s3c64xx/mach-smartq.c                |   4 +-
 arch/arm/mach-s3c64xx/mach-smdk6410.c              |   5 +-
 arch/arm/mach-s5p64x0/Kconfig                      |   4 +-
 arch/arm/mach-s5p64x0/clock-s5p6440.c              |   2 -
 arch/arm/mach-s5p64x0/clock-s5p6450.c              |   2 -
 arch/arm/mach-s5p64x0/common.c                     |  27 +
 arch/arm/mach-s5p64x0/include/mach/irqs.h          |   2 -
 arch/arm/mach-s5p64x0/include/mach/map.h           |   1 +
 arch/arm/mach-s5p64x0/mach-smdk6440.c              |   5 +-
 arch/arm/mach-s5p64x0/mach-smdk6450.c              |   5 +-
 arch/arm/mach-s5p64x0/pm.c                         |   3 -
 arch/arm/mach-s5pc100/Kconfig                      |   2 +-
 arch/arm/mach-s5pc100/clock.c                      |   2 -
 arch/arm/mach-s5pc100/common.c                     |  28 +
 arch/arm/mach-s5pc100/include/mach/irqs.h          |   2 -
 arch/arm/mach-s5pc100/include/mach/map.h           |   1 +
 arch/arm/mach-s5pc100/mach-smdkc100.c              |   5 +-
 arch/arm/mach-s5pv210/Kconfig                      |   2 +-
 arch/arm/mach-s5pv210/clock.c                      |   1 -
 arch/arm/mach-s5pv210/common.c                     |  28 +
 arch/arm/mach-s5pv210/include/mach/irqs.h          |   2 -
 arch/arm/mach-s5pv210/include/mach/map.h           |   1 +
 arch/arm/mach-s5pv210/mach-smdkv210.c              |   5 +-
 arch/arm/mach-s5pv210/pm.c                         |  10 -
 arch/arm/plat-samsung/Kconfig                      |  14 -
 arch/arm/plat-samsung/Makefile                     |   3 -
 arch/arm/plat-samsung/dev-backlight.c              |  61 +-
 arch/arm/plat-samsung/devs.c                       |  42 +-
 arch/arm/plat-samsung/include/plat/clock.h         |   4 -
 arch/arm/plat-samsung/include/plat/devs.h          |   1 +
 arch/arm/plat-samsung/include/plat/irq-vic-timer.h |  13 -
 arch/arm/plat-samsung/include/plat/irqs.h          |   9 -
 arch/arm/plat-samsung/include/plat/pwm-clock.h     |  81 ---
 arch/arm/plat-samsung/include/plat/pwm-core.h      |  24 +
 arch/arm/plat-samsung/include/plat/regs-timer.h    | 124 ----
 arch/arm/plat-samsung/include/plat/samsung-time.h  |  23 -
 arch/arm/plat-samsung/irq-vic-timer.c              |  98 ---
 arch/arm/plat-samsung/pwm-clock.c                  | 474 --------------
 arch/arm/plat-samsung/s5p-irq.c                    |   4 -
 arch/arm/plat-samsung/samsung-time.c               | 394 ------------
 drivers/clocksource/samsung_pwm_timer.c            |  84 +--
 drivers/pwm/pwm-samsung.c                          | 694
++++++++++++++------- 60 files changed, 766 insertions(+), 1644
deletions(-)
 delete mode 100644 arch/arm/plat-samsung/include/plat/irq-vic-timer.h
 delete mode 100644 arch/arm/plat-samsung/include/plat/pwm-clock.h
 create mode 100644 arch/arm/plat-samsung/include/plat/pwm-core.h
 delete mode 100644 arch/arm/plat-samsung/include/plat/regs-timer.h
 delete mode 100644 arch/arm/plat-samsung/irq-vic-timer.c
 delete mode 100644 arch/arm/plat-samsung/pwm-clock.c
 delete mode 100644 arch/arm/plat-samsung/samsung-time.c

Am Freitag, 21. Juni 2013, 00:12:45 schrieb Tomasz Figa:

Quoted 24 lines

Since we now have a proper Samsung PWM clocksource driver in place, we can proceed with further cleanup of PWM timers support on Samsung SoCs.>
This series attempts to achieve this goal by:
1) moving remaining Samsung platforms to the new clocksource driver
2) removing old clocksource driver
3) adding new multiplatform- and DT-aware PWM driver
4) moving all Samsung platforms to use the new PWM driver
5) removing old PWM driver
6) removing all PWM-related code that is not used anymore

Cleaning up the PWM driver is a bit tricky, because the design of current driver makes it completely unsuitable for DT and multiplatform and would require a heavy rework to make it usable, breaking any existing Samsung PWM users by the way. To avoid any breakage I decided to keep the old driver, add new one, move all platforms to it and then remove the old one.

See particular patches for more detailed descriptions.

On S3C6410-based Tiny6410 (Mini6410-compatible), after enabling pwm-beeper and Exynos4210-based Origen board (with PWM0 attached to a scope):

Quoted 1 line

Tested-by Tomasz Figa <[email protected]>

On a s3c2416 based machine (with the additional two v3 patches)

Tested-by Heiko Stuebner <[email protected]>

and again thanks for working on this :-)

Quoted 113 lines

Changes since v1:
- made sure that suspend/resume is handled correctly in both

clocksource and PWM drivers,

- fixed incorrect definition of AUTORELOAD bit for channel 4,
- fixed order of registering PWM device and calling samsung_bl_set(),

which assumes that PWM device has been already registered,

- corrected commit messages of several patches,
- addressed all the valid comments from reviewers, including cleanup

of most code originally copied from previous PWM driver,

- rebased on top of current Kgene's for-next,
- tested on Exynos4210, verifying correct PWM output with a scope.

Needs testing on other platforms as I could only compile test for them.

Tomasz Figa (18):
ARM: SAMSUNG: Unify base address definitions of timer block
ARM: SAMSUNG: Add new PWM platform device
ARM: SAMSUNG: Set PWM platform data
clocksource: samsung_pwm_timer: Cache clocksource register address
clocksource: samsung_pwm_timer: Do not use clocksource_mmio
clocksource: samsung_pwm_timer: Handle suspend/resume correctly
ARM: SAMSUNG: Move all platforms to new clocksource driver
ARM: SAMSUNG: Remove old samsung-time driver
ARM: SAMSUNG: Remove unused PWM timer IRQ chip code
pwm: samsung: Rename to pwm-samsung-legacy
pwm: Add new pwm-samsung driver
ARM: SAMSUNG: Rework private data handling in dev-backlight
ARM: SAMSUNG: Modify board files to use new PWM platform device
pwm: Remove superseded pwm-samsung-legacy driver
ARM: SAMSUNG: Remove old PWM timer platform devices
ARM: SAMSUNG: Remove pwm-clock infrastructure
ARM: SAMSUNG: Remove remaining uses of plat/regs-timer.h header
ARM: SAMSUNG: Remove plat/regs-timer.h header

 arch/arm/Kconfig                                   |   1 -
 arch/arm/mach-s3c24xx/Kconfig                      |  12 +-
 arch/arm/mach-s3c24xx/clock-s3c2410.c              |   1 -
 arch/arm/mach-s3c24xx/clock-s3c2412.c              |   1 -
 arch/arm/mach-s3c24xx/clock-s3c2416.c              |   2 -
 arch/arm/mach-s3c24xx/clock-s3c2443.c              |   2 -
 arch/arm/mach-s3c24xx/common.c                     |  27 +
 arch/arm/mach-s3c24xx/include/mach/map.h           |   2 +
 arch/arm/mach-s3c24xx/mach-h1940.c                 |   4 +-
 arch/arm/mach-s3c24xx/mach-rx1950.c                |   5 +-
 arch/arm/mach-s3c64xx/Kconfig                      |   4 +-
 arch/arm/mach-s3c64xx/clock.c                      |   2 -
 arch/arm/mach-s3c64xx/common.c                     |  32 +-
 arch/arm/mach-s3c64xx/include/mach/irqs.h          |   8 -
 arch/arm/mach-s3c64xx/include/mach/map.h           |   1 +
 arch/arm/mach-s3c64xx/irq-pm.c                     |   2 -
 arch/arm/mach-s3c64xx/mach-crag6410.c              |   4 +-
 arch/arm/mach-s3c64xx/mach-hmt.c                   |   4 +-
 arch/arm/mach-s3c64xx/mach-smartq.c                |   4 +-
 arch/arm/mach-s3c64xx/mach-smdk6410.c              |   5 +-
 arch/arm/mach-s5p64x0/Kconfig                      |   4 +-
 arch/arm/mach-s5p64x0/clock-s5p6440.c              |   2 -
 arch/arm/mach-s5p64x0/clock-s5p6450.c              |   2 -
 arch/arm/mach-s5p64x0/common.c                     |  27 +
 arch/arm/mach-s5p64x0/include/mach/irqs.h          |   2 -
 arch/arm/mach-s5p64x0/include/mach/map.h           |   1 +
 arch/arm/mach-s5p64x0/mach-smdk6440.c              |   5 +-
 arch/arm/mach-s5p64x0/mach-smdk6450.c              |   5 +-
 arch/arm/mach-s5p64x0/pm.c                         |   3 -
 arch/arm/mach-s5pc100/Kconfig                      |   2 +-
 arch/arm/mach-s5pc100/clock.c                      |   2 -
 arch/arm/mach-s5pc100/common.c                     |  28 +
 arch/arm/mach-s5pc100/include/mach/irqs.h          |   2 -
 arch/arm/mach-s5pc100/include/mach/map.h           |   1 +
 arch/arm/mach-s5pc100/mach-smdkc100.c              |   5 +-
 arch/arm/mach-s5pv210/Kconfig                      |   2 +-
 arch/arm/mach-s5pv210/clock.c                      |   1 -
 arch/arm/mach-s5pv210/common.c                     |  28 +
 arch/arm/mach-s5pv210/include/mach/irqs.h          |   2 -
 arch/arm/mach-s5pv210/include/mach/map.h           |   1 +
 arch/arm/mach-s5pv210/mach-smdkv210.c              |   5 +-
 arch/arm/mach-s5pv210/pm.c                         |  10 -
 arch/arm/plat-samsung/Kconfig                      |  14 -
 arch/arm/plat-samsung/Makefile                     |   3 -
 arch/arm/plat-samsung/dev-backlight.c              |  61 +-
 arch/arm/plat-samsung/devs.c                       |  42 +-
 arch/arm/plat-samsung/include/plat/clock.h         |   4 -
 arch/arm/plat-samsung/include/plat/devs.h          |   1 +
 arch/arm/plat-samsung/include/plat/irq-vic-timer.h |  13 -
 arch/arm/plat-samsung/include/plat/irqs.h          |   9 -
 arch/arm/plat-samsung/include/plat/pwm-clock.h     |  81 ---
 arch/arm/plat-samsung/include/plat/pwm-core.h      |  24 +
 arch/arm/plat-samsung/include/plat/regs-timer.h    | 124 ----
 arch/arm/plat-samsung/include/plat/samsung-time.h  |  23 -
 arch/arm/plat-samsung/irq-vic-timer.c              |  98 ---
 arch/arm/plat-samsung/pwm-clock.c                  | 474
 --------------
 arch/arm/plat-samsung/s5p-irq.c                    |   4 -
 arch/arm/plat-samsung/samsung-time.c               | 394 ------------
 drivers/clocksource/samsung_pwm_timer.c            |  84 +--
 drivers/pwm/pwm-samsung.c                          | 694

++++++++++++++------- 60 files changed, 766 insertions(+), 1644 deletions(-)

 delete mode 100644 arch/arm/plat-samsung/include/plat/irq-vic-timer.h
 delete mode 100644 arch/arm/plat-samsung/include/plat/pwm-clock.h
 create mode 100644 arch/arm/plat-samsung/include/plat/pwm-core.h
 delete mode 100644 arch/arm/plat-samsung/include/plat/regs-timer.h
 delete mode 100644 arch/arm/plat-samsung/irq-vic-timer.c
 delete mode 100644 arch/arm/plat-samsung/pwm-clock.c
 delete mode 100644 arch/arm/plat-samsung/samsung-time.c

Hi Heiko,

Quoted 27 lines · On Saturday 22 of June 2013 20:01:49 Heiko St?bner wrote:

Am Freitag, 21. Juni 2013, 00:12:45 schrieb Tomasz Figa:

Quoted 25 lines

Since we now have a proper Samsung PWM clocksource driver in place, we can proceed with further cleanup of PWM timers support on Samsung SoCs.>
This series attempts to achieve this goal by:
1) moving remaining Samsung platforms to the new clocksource driver
2) removing old clocksource driver
3) adding new multiplatform- and DT-aware PWM driver
4) moving all Samsung platforms to use the new PWM driver
5) removing old PWM driver
6) removing all PWM-related code that is not used anymore

Cleaning up the PWM driver is a bit tricky, because the design of current driver makes it completely unsuitable for DT and multiplatform and would require a heavy rework to make it usable, breaking any existing Samsung PWM users by the way. To avoid any breakage I decided to keep the old driver, add new one, move all platforms to it and then remove the old one.

See particular patches for more detailed descriptions.

On S3C6410-based Tiny6410 (Mini6410-compatible), after enabling pwm-beeper and Exynos4210-based Origen board (with PWM0 attached to a scope):

Tested-by Tomasz Figa <[email protected]>

Quoted 4 lines

On a s3c2416 based machine (with the additional two v3 patches)

Tested-by Heiko Stuebner <[email protected]>

Quoted 2 lines

and again thanks for working on this :-)

Thanks for testing!

I hope we can get this merged ASAP, as this is an important prerequisite for further work on S3C24xx and S3C64xx...

Am Freitag, 21. Juni 2013, 00:12:45 schrieb Tomasz Figa:

Quoted 19 lines

Since we now have a proper Samsung PWM clocksource driver in place, we can proceed with further cleanup of PWM timers support on Samsung SoCs.

This series attempts to achieve this goal by:
1) moving remaining Samsung platforms to the new clocksource driver
2) removing old clocksource driver
3) adding new multiplatform- and DT-aware PWM driver
4) moving all Samsung platforms to use the new PWM driver
5) removing old PWM driver
6) removing all PWM-related code that is not used anymore

Cleaning up the PWM driver is a bit tricky, because the design of current driver makes it completely unsuitable for DT and multiplatform and would require a heavy rework to make it usable, breaking any existing Samsung PWM users by the way. To avoid any breakage I decided to keep the old driver, add new one, move all platforms to it and then remove the old one.

See particular patches for more detailed descriptions.

Quoted 4 lines

On S3C6410-based Tiny6410 (Mini6410-compatible), after enabling pwm-beeper and Exynos4210-based Origen board (with PWM0 attached to a scope):

Tested-by Tomasz Figa<[email protected]>

On a s3c2416 based machine (with the additional two v3 patches)

Tested-by Heiko Stuebner<[email protected]>