Skip to contentSkip to conversation

Add high resolution generic time support for S3C64xx

PATCH New Tomasz Figa 5 participants 21 messages Aug 31, 2011 – Oct 13, 2011

lore
Share & tools
lore
#0 lore raw

cover Add high resolution generic time support for S3C64xx

#1 lore raw

PATCH 1/5 ARM: Samsung: PWM: Allow to differentiate SoCs based on platform device name.

#2 lore raw

PATCH 1/5 ARM: Samsung: PWM: Allow to differentiate SoCs based on platform device name.

Quoted 3 lines · Tomasz Figa

This patch is a prerequisite to adding generic time support for S3C64xx. It makes possible to differentiate SoCs, required to exclude timers 3 and 4 from PWM driver only on S3C64xx.

Now we have the cpu_is_foo() support for Samsung CPUs can we use that instead?

#3 lore raw

PATCH 1/5 ARM: Samsung: PWM: Allow to differentiate SoCs based on platform device name.

2011/8/31 Mark Brown <[email protected]>:

Quoted 9 lines · Tomasz Figa
Quoted 4 lines

This patch is a prerequisite to adding generic time support for S3C64xx. It makes possible to differentiate SoCs, required to exclude timers 3 and 4 from PWM driver only on S3C64xx.

Now we have the cpu_is_foo() support for Samsung CPUs can we use that instead?

Well, I thought about it, but I based my patches on latest stable sources and cpu_is_s3c64xx wasn't there yet. Should I rebase them to Linus' tree?

I'm not yet fully used to submitting patches here, so things like choosing the base for patches are sometimes a bit confusing to me and I couldn't find any guide. Should fixes be based on stable tree and new features implemented on top of current Linus' tree? Could you give me some advice on this?

#4 lore raw

PATCH 1/5 ARM: Samsung: PWM: Allow to differentiate SoCs based on platform device name.

Quoted 5 lines · Tomasz Figa

2011/8/31 Mark Brown <[email protected]>:

Quoted 3 lines · Tomasz Figa
Quoted 2 lines

This patch is a prerequisite to adding generic time support for

S3C64xx.

Quoted 1 line
Quoted 1 line
Quoted 1 line

It makes possible to differentiate SoCs, required to exclude timers 3

and 4

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

from PWM driver only on S3C64xx.

Now we have the cpu_is_foo() support for Samsung CPUs can we use that instead?

Well, I thought about it, but I based my patches on latest stable sources

and

Quoted 3 lines

cpu_is_s3c64xx wasn't there yet. Should I rebase them to Linus' tree?

I'm not yet fully used to submitting patches here, so things like choosing

the

Quoted 1 line

base for patches are sometimes a bit confusing to me and I couldn't find

any

Quoted 1 line

guide. Should fixes be based on stable tree and new features implemented

on

Quoted 1 line

top of current Linus' tree? Could you give me some advice on this?

Hi Tomasz,

You can use "soc_is_s3c64xx()" based on Samsung -next tree.
git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
for-next

In addition, would be better to me if you could work about Samsung stuff based on it.

As a note, your previous patches are in there :)

If any problems, please let me know...

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

#5 lore raw

PATCH 1/5 ARM: Samsung: PWM: Allow to differentiate SoCs based on platform device name.

Quoted 24 lines · Kukjin Kim
Quoted 5 lines · Tomasz Figa

2011/8/31 Mark Brown <[email protected]>:

Quoted 3 lines · Tomasz Figa
Quoted 2 lines

This patch is a prerequisite to adding generic time support for

S3C64xx.

Quoted 1 line
Quoted 1 line
Quoted 1 line

It makes possible to differentiate SoCs, required to exclude timers 3

and 4

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

from PWM driver only on S3C64xx.

Now we have the cpu_is_foo() support for Samsung CPUs can we use that instead?

Well, I thought about it, but I based my patches on latest stable sources

and

Quoted 3 lines

cpu_is_s3c64xx wasn't there yet. Should I rebase them to Linus' tree?

I'm not yet fully used to submitting patches here, so things like choosing

the

Quoted 1 line

base for patches are sometimes a bit confusing to me and I couldn't find

any

Quoted 1 line

guide. Should fixes be based on stable tree and new features implemented

Quoted 8 lines

on

Quoted 1 line

top of current Linus' tree? Could you give me some advice on this?

Hi Tomasz,

You can use "soc_is_s3c64xx()" based on Samsung -next tree.
git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
for-next

+static struct platform_device_id s3c_pwm_driver_ids[] = {
+ {

+               .name           = "s3c24xx-pwm",
+               .driver_data    = TYPE_GENERIC,
+       }, {
+               .name           = "s3c64xx-pwm",
+               .driver_data    = TYPE_S3C64XX,

+ },
+};
+MODULE_DEVICE_TABLE(platform, s3c_pwm_driver_ids);

doesn't it right way?
do you want to use the soc_is_* at device?

Quoted 22 lines

In addition, would be better to me if you could work about Samsung stuff based on it.

As a note, your previous patches are in there :)

If any problems, please let me know...

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#6 lore raw

PATCH 1/5 ARM: Samsung: PWM: Allow to differentiate SoCs based on platform device name.

Quoted 34 lines · Kyungmin Park
Quoted 21 lines · Kukjin Kim
Quoted 5 lines · Tomasz Figa

2011/8/31 Mark Brown <[email protected]>:

Quoted 3 lines · Tomasz Figa
Quoted 2 lines

This patch is a prerequisite to adding generic time support for

S3C64xx.

Quoted 1 line
Quoted 1 line
Quoted 1 line

It makes possible to differentiate SoCs, required to exclude timers 3

and 4

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

from PWM driver only on S3C64xx.

Now we have the cpu_is_foo() support for Samsung CPUs can we use that instead?

[snip]

Hi Tomasz,

You can use "soc_is_s3c64xx()" based on Samsung -next tree.
git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
for-next

(quoted diff) +10 -0
+static struct platform_device_id s3c_pwm_driver_ids[] = {
+       {
+               .name           = "s3c24xx-pwm",
+               .driver_data    = TYPE_GENERIC,
+       }, {
+               .name           = "s3c64xx-pwm",
+               .driver_data    = TYPE_S3C64XX,
+       },
+};
+MODULE_DEVICE_TABLE(platform, s3c_pwm_driver_ids);
Quoted 3 lines

doesn't it right way?
do you want to use the soc_is_* at device?

Well, now we have three alternatives:
- to leave it as is (i.e. changing platform device names)
- to use cpu_is_* (does it ever exist for S3C64xx or soc_is_s3c64xx is its equivalent?)
- to use soc_is_*

Waiting for more comments then.

Best regards,
Tom

#7 lore raw

PATCH 1/5 ARM: Samsung: PWM: Allow to differentiate SoCs based on platform device name.

Quoted 6 lines · Tomasz Figa

On Thursday 01 of September 2011 at 12:33:10, Kyungmin Park wrote:

Well, now we have three alternatives:
- to leave it as is (i.e. changing platform device names)
- to use cpu_is_* (does it ever exist for S3C64xx or soc_is_s3c64xx is its equivalent?)
- to use soc_is_*

soc_is is the same thing.

#8 lore raw

PATCH 1/5 ARM: Samsung: PWM: Allow to differentiate SoCs based on platform device name.

In reply to Tomasz Figa
Quoted 44 lines · Tomasz Figa
Quoted 34 lines · Kyungmin Park
Quoted 21 lines · Kukjin Kim
Quoted 5 lines · Tomasz Figa

2011/8/31 Mark Brown <[email protected]>:

Quoted 3 lines · Tomasz Figa
Quoted 2 lines

This patch is a prerequisite to adding generic time support for

S3C64xx.

Quoted 1 line
Quoted 1 line
Quoted 1 line

It makes possible to differentiate SoCs, required to exclude timers 3

and 4

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

from PWM driver only on S3C64xx.

Now we have the cpu_is_foo() support for Samsung CPUs can we use that instead?

[snip]

Hi Tomasz,

You can use "soc_is_s3c64xx()" based on Samsung -next tree.
git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
for-next

(quoted diff) +10 -0
+static struct platform_device_id s3c_pwm_driver_ids[] = {
+       {
+               .name           = "s3c24xx-pwm",
+               .driver_data    = TYPE_GENERIC,
+       }, {
+               .name           = "s3c64xx-pwm",
+               .driver_data    = TYPE_S3C64XX,
+       },
+};
+MODULE_DEVICE_TABLE(platform, s3c_pwm_driver_ids);
Quoted 3 lines

doesn't it right way?
do you want to use the soc_is_* at device?

Well, now we have three alternatives:
- to leave it as is (i.e. changing platform device names)
- to use cpu_is_* (does it ever exist for S3C64xx or soc_is_s3c64xx is its equivalent?)
- to use soc_is_*

Quoted 2 lines

Waiting for more comments then.

Ping.

Should I keep it as is or rather change it to use soc_is_s3c64xx instead?

Best regards,
Tom

#9 lore raw

PATCH 1/5 ARM: Samsung: PWM: Allow to differentiate SoCs based on platform device name.

Quoted 11 lines · Tomasz Figa
Quoted 7 lines · Tomasz Figa

Well, now we have three alternatives:
- to leave it as is (i.e. changing platform device names)
- to use cpu_is_* (does it ever exist for S3C64xx or soc_is_s3c64xx is its equivalent?)
- to use soc_is_*

Waiting for more comments then.

Ping.

Should I keep it as is or rather change it to use soc_is_s3c64xx instead?

I don't know what question you're asking (the questions don't make sense in terms of the quoted context.)

If you're asking whether you should use a cpu_ prefix instead of a soc_ prefix for identifying s3c64xx, then consider the question.

What is the CPU?

What are S3C64xx / OMAP / PXA310 / SA-11x0 / AT91RM9200? CPU or SoC ? What are Cortex A8/A9 / Xscale / StrongARM / ARM920? CPU or SoC ?

#10 lore raw

PATCH 1/5 ARM: Samsung: PWM: Allow to differentiate SoCs based on platform device name.

Quoted 16 lines · Russell King - ARM Linux
Quoted 11 lines · Tomasz Figa
Quoted 7 lines · Tomasz Figa

Well, now we have three alternatives:
- to leave it as is (i.e. changing platform device names)
- to use cpu_is_* (does it ever exist for S3C64xx or soc_is_s3c64xx is its equivalent?)
- to use soc_is_*

Waiting for more comments then.

Ping.

Should I keep it as is or rather change it to use soc_is_s3c64xx instead?

I don't know what question you're asking (the questions don't make sense in terms of the quoted context.)

Quoted 7 lines

If you're asking whether you should use a cpu_ prefix instead of a soc_ prefix for identifying s3c64xx, then consider the question.

What is the CPU?

What are S3C64xx / OMAP / PXA310 / SA-11x0 / AT91RM9200? CPU or SoC ? What are Cortex A8/A9 / Xscale / StrongARM / ARM920? CPU or SoC ?

Well, I believe that the context I quoted explains everything, but let me explain that again:

The patch is about a driver being able to check to what SoC a device belongs, but there are at least two ways to achieve that.

I can see many drivers using the way I used, i.e. distinct platform device names for each SoC that has to be differentiated plus a generic one. However, I was suggested that using cpu_is_*/soc_is_* might be a better approach, while at the same time I received an opinion that the original method is the preferred one.

Best regards,
Tom

#11 lore raw

PATCH 1/5 ARM: Samsung: PWM: Allow to differentiate SoCs based on platform device name.

Quoted 10 lines · Tomasz Figa

Well, I believe that the context I quoted explains everything, but let me explain that again:

The patch is about a driver being able to check to what SoC a device belongs, but there are at least two ways to achieve that.

I can see many drivers using the way I used, i.e. distinct platform device names for each SoC that has to be differentiated plus a generic one. However, I was suggested that using cpu_is_*/soc_is_* might be a better approach, while at the same time I received an opinion that the original method is the preferred one.

Ok. There's pros and cons here:

Pros to soc_is_*:
1. Doesn't require additional storage in every driver but does require code.
2. Obvious what's going on for a specific SoC when reading the code.

Cons:
1. Needs to be updated each time a new SoC is added.
2. Doesn't scale well - number of soc_is_* will grow to unmanageable levels over time.

Pros to platform driver name:
1. Can re-use existing names if feature compatible without driver mods.
2. Scales well with an increasing number of SoCs.

Cons:
1. People will hate having SoC names which don't refer to their exact SoC.
2. Probably requires storage of a set of flags in driver private data to identify SoC specific features.
3. Requires additional string space to identify each driver name.

However, thinking about this more wrt DT, there's another aspect to this. Rather than encoding into the driver "this SoC has features and quirks X,Y,Z" maybe that information should be in the device tree itself. For example, SoC 1 has X and Z, SoC 2 has Y. Then a new SoC 3 comes along with X and Y but not Z. If X, Y, Z are encoded into DT then there's no need to touch the kernel to support SoC 3, not even to change driver names or soc_is_xxx macros etc.

So maybe the answer is: platform driver name determines a set of feature flags for the driver in lieu of DT support, and when DT support comes along the flags are controlled by DT properties instead.

#12 lore raw

PATCH 1/5 ARM: Samsung: PWM: Allow to differentiate SoCs based on platform device name.

Quoted 9 lines · Russell King - ARM Linux

Pros to platform driver name:
1. Can re-use existing names if feature compatible without driver mods.
2. Scales well with an increasing number of SoCs.

Cons:
1. People will hate having SoC names which don't refer to their exact SoC.
2. Probably requires storage of a set of flags in driver private data to identify SoC specific features.
3. Requires additional string space to identify each driver name.

There's also an issue with actually getting the devices together to register which causes fragility here.

Quoted 7 lines

However, thinking about this more wrt DT, there's another aspect to this. Rather than encoding into the driver "this SoC has features and quirks X,Y,Z" maybe that information should be in the device tree itself. For example, SoC 1 has X and Z, SoC 2 has Y. Then a new SoC 3 comes along with X and Y but not Z. If X, Y, Z are encoded into DT then there's no need to touch the kernel to support SoC 3, not even to change driver names or soc_is_xxx macros etc.

This does depend pretty strongly on making sure that the SoC DT is distributed separately to the board DT - that should be solved now.

#13 lore raw

PATCH 1/5 ARM: Samsung: PWM: Allow to differentiate SoCs based on platform device name.

Quoted 9 lines · Mark Brown
Quoted 6 lines · Russell King - ARM Linux

Pros to platform driver name:
1. Can re-use existing names if feature compatible without driver mods.
2. Scales well with an increasing number of SoCs.

Cons:
1. People will hate having SoC names which don't refer to their exact

SoC.

Quoted 8 lines
Quoted 3 lines

2. Probably requires storage of a set of flags in driver private data to identify SoC specific features.
3. Requires additional string space to identify each driver name.

There's also an issue with actually getting the devices together to register which causes fragility here.

Quoted 1 line

However, thinking about this more wrt DT, there's another aspect to

this.

Quoted 9 lines
Quoted 6 lines

Rather than encoding into the driver "this SoC has features and quirks X,Y,Z" maybe that information should be in the device tree itself. For example, SoC 1 has X and Z, SoC 2 has Y. Then a new SoC 3 comes along with X and Y but not Z. If X, Y, Z are encoded into DT then there's no need to touch the kernel to support SoC 3, not even to change driver names or soc_is_xxx macros etc.

This does depend pretty strongly on making sure that the SoC DT is distributed separately to the board DT - that should be solved now.

Yes, I agree. So we can keep the current way to support each SoCs' device with one driver until DT world.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

#15 lore raw

PATCH 2/5 ARM: s3c64xx: Add generic high resolution time support using PWM timers.

Quoted 5 lines · Tomasz Figa

From d1fa581f09d8f8a3cc24d05420e9e65112fd8c12 Mon Sep 17 00:00:00 2001
From: Tomasz Figa <[email protected]>
Date: Sun, 28 Aug 2011 02:45:06 +0200
Subject: [PATCH 2/5] ARM: s3c64xx: Add generic high resolution time

support

Quoted 3 lines

using PWM timers.

This patch adds a sys_timer implementing generic clock source and clock

event

Quoted 2 lines

device on S3C64xx using PWM timers 3 and 4. It can be enabled by a Kconfig option, based on which either the new sys_timer is used as s3c64xx_timer

or

Review

Quoted 49 lines

s3c64xx_timer is defined to s3c24xx_timer which is the old tick timer.

---
 arch/arm/Kconfig                         |    1 -
 arch/arm/mach-s3c64xx/Kconfig            |   12 ++
 arch/arm/mach-s3c64xx/Makefile           |    3 +
 arch/arm/mach-s3c64xx/time.c             |  320
++++++++++++++++++++++++++++++
 arch/arm/plat-samsung/include/plat/cpu.h |    7 +
 5 files changed, 342 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-s3c64xx/time.c
arch/arm/Kconfig +0 -1
index 9adc278..05941f6 100644
@@ -700,7 +700,6 @@ config ARCH_S3C64XX
 	select ARM_VIC
 	select HAVE_CLK
 	select NO_IOPORT
-	select ARCH_USES_GETTIMEOFFSET
 	select ARCH_HAS_CPUFREQ
 	select ARCH_REQUIRE_GPIOLIB
 	select SAMSUNG_CLKSRC
arch/arm/mach-s3c64xx/Kconfig +12 -0
index e4177e2..2db76fd 100644
@@ -25,6 +25,18 @@ config CPU_S3C6410
 	help
 	  Enable S3C6410 CPU support
 
+config S3C64XX_GENERIC_CLOCKEVENTS
+	bool "Enable high resolution generic time support using PWM timers"
+	select GENERIC_CLOCKEVENTS
+	help
+	  This option enables high resolution generic time support
+	  on S3C6400/S3C6410 SoCs using PWM timers 3 and 4 instead
+	  of standard periodic tick using PWM timer 4.
+
+config S3C64XX_ARCH_USES_GETTIMEOFFSET
+	def_bool y if !S3C64XX_GENERIC_CLOCKEVENTS
+	select ARCH_USES_GETTIMEOFFSET
+
 config S3C64XX_DMA
 	bool "S3C64XX DMA"
 	select S3C_DMA
(quoted diff) +0 -0

b/arch/arm/mach-s3c64xx/Makefile

Quoted 19 lines

index 4657363..a152002 100644

arch/arm/mach-s3c64xx/Makefile +3 -0
@@ -15,6 +15,9 @@ obj-y				+= cpu.o
 obj-y				+= clock.o
 obj-y				+= gpiolib.o
 
+# Generic clockevents
+obj-$(CONFIG_S3C64XX_GENERIC_CLOCKEVENTS) += time.o
+
 # Core support for S3C6400 system
 
 obj-$(CONFIG_CPU_S3C6400)	+= s3c6400.o
arch/arm/mach-s3c64xx/time.c new file +0 -0
new file mode 100644
index 0000000..4fa57f0

arch/arm/mach-s3c64xx/time.c line 7

Quoted 7 lines
arch/arm/mach-s3c64xx/time.c new file +7 -0
@@ -0,0 +1,320 @@
+/* linux/arch/arm/mach-s3c64xx/time.c
+ *
+ * Copyright (c) 2011 Tomasz Figa <[email protected]>
+ *
+ * based on linux/arch/arm/plat-samsung/s5p-time.c
+ *
+ * S3C64XX generic high resolution time support using PWM 3 and PWM 4

timers.

arch/arm/mach-s3c64xx/time.c line 14

Quoted 7 lines
arch/arm/mach-s3c64xx/time.c new file +7 -0
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Notes:
+ * - Frequency divisors are currently hardcoded to make both timers

operate

arch/arm/mach-s3c64xx/time.c line 15

Quoted 1 line
arch/arm/mach-s3c64xx/time.c new file +1 -0
+ *   at 1/6 the frequency of PCLK, so with the usual PCLK frequency of 66

MHz,

arch/arm/mach-s3c64xx/time.c line 18

Quoted 3 lines
arch/arm/mach-s3c64xx/time.c new file +3 -0
+ *   the timers are clocked at 11 MHz giving us the operating range from
+ *   90 nsec to 386 sec.
+ * - PWM registers, especially TCON, are shared with PWM driver, but

since

arch/arm/mach-s3c64xx/time.c line 19

Quoted 1 line
arch/arm/mach-s3c64xx/time.c new file +1 -0
+ *   all the clock event callbacks are run in atomic context

synchronization

Quoted 36 lines
arch/arm/mach-s3c64xx/time.c new file +36 -0
+ *   is not needed.
+ */
+
+#include <linux/sched.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/platform_device.h>
+
+#include <mach/map.h>
+#include <plat/regs-timer.h>
+#include <asm/mach/time.h>
+
+#define PWM_EVENT	(3)
+#define PWM_SOURCE	(4)
+
+#define ONESHOT		(0)
+#define PERIODIC	(1)
+
+#define TCNT_MAX	(0xffffffff)
+
+static struct clk *timerclk;
+
+/*
+ * PWM timers setup code
+ */
+
+static inline void s3c64xx_pwm_stop(unsigned int pwm_id)
+{
+	unsigned long tcon = __raw_readl(S3C2410_TCON);
+
+	switch (pwm_id) {
+	case 3:
+		tcon &= ~S3C2410_TCON_T3START;

arch/arm/mach-s3c64xx/time.c line 65

Quoted 10 lines
arch/arm/mach-s3c64xx/time.c new file +10 -0
+		break;
+	case 4:
+		tcon &= ~S3C2410_TCON_T4START;
+		break;
+	}
+
+	__raw_writel(tcon, S3C2410_TCON);
+}
+
+static inline void s3c64xx_pwm_init(unsigned int pwm_id, unsigned long

tcnt)

Quoted 134 lines
arch/arm/mach-s3c64xx/time.c new file +62 -0
+{
+	unsigned long tcon = __raw_readl(S3C2410_TCON);
+
+	/* timers reload after counting zero, so reduce the count by 1 */
+	--tcnt;
+
+	/* stop the timer and ask it to load the new value */
+	switch (pwm_id) {
+	case 3:
+		tcon &= ~(0xf<<16);
+		tcon |= S3C2410_TCON_T3MANUALUPD;
+		break;
+	case 4:
+		tcon &= ~(7<<20);
+		tcon |= S3C2410_TCON_T4MANUALUPD;
+		break;
+	}
+
+	__raw_writel(tcnt, S3C2410_TCNTB(pwm_id));
+	__raw_writel(tcnt, S3C2410_TCMPB(pwm_id));
+	__raw_writel(tcon, S3C2410_TCON);
+}
+
+static inline void s3c64xx_pwm_start(unsigned int pwm_id, bool periodic)
+{
+	unsigned long tcon = __raw_readl(S3C2410_TCON);
+
+	switch (pwm_id) {
+	case 3:
+		tcon |= S3C2410_TCON_T3START;
+		tcon &= ~S3C2410_TCON_T3MANUALUPD;
+
+		if (periodic)
+			tcon |= S3C2410_TCON_T3RELOAD;
+		else
+			tcon &= ~S3C2410_TCON_T3RELOAD;
+		break;
+	case 4:
+		tcon |= S3C2410_TCON_T4START;
+		tcon &= ~S3C2410_TCON_T4MANUALUPD;
+
+		if (periodic)
+			tcon |= S3C2410_TCON_T4RELOAD;
+		else
+			tcon &= ~S3C2410_TCON_T4RELOAD;
+		break;
+	}
+
+	__raw_writel(tcon, S3C2410_TCON);
+}
+
+/*
+ * Clock event
+ */
+
+static struct clk *event_in;
+static struct clk *event_div;
+
+static unsigned long clock_count_per_tick;
+
+static int s3c64xx_clock_event_set_next_event(unsigned long cycles,
+						struct clock_event_device

*evt)

arch/arm/mach-s3c64xx/time.c new file +24 -0
+{
+	s3c64xx_pwm_init(PWM_EVENT, cycles);
+	s3c64xx_pwm_start(PWM_EVENT, ONESHOT);
+	return 0;
+}
+
+static void s3c64xx_clock_event_resume(void)
+{
+	unsigned long pclk;
+	struct clk *tscaler;
+
+	pclk = clk_get_rate(timerclk);
+	tscaler = clk_get_parent(event_div);
+	clk_set_rate(tscaler, pclk / 3);
+
+	clk_set_rate(event_div, pclk / 6);
+	clk_set_parent(event_in, event_div);
+
+	s3c64xx_pwm_init(PWM_EVENT, clock_count_per_tick);
+	s3c64xx_pwm_start(PWM_EVENT, PERIODIC);
+}
+
+static void s3c64xx_clock_event_set_mode(enum clock_event_mode mode,
+						struct clock_event_device

*evt)

arch/arm/mach-s3c64xx/time.c new file +22 -0
+{
+	s3c64xx_pwm_stop(PWM_EVENT);
+
+	switch (mode) {
+	case CLOCK_EVT_MODE_PERIODIC:
+		s3c64xx_pwm_init(PWM_EVENT, clock_count_per_tick);
+		s3c64xx_pwm_start(PWM_EVENT, PERIODIC);
+		break;
+	case CLOCK_EVT_MODE_ONESHOT:
+		break;
+	case CLOCK_EVT_MODE_UNUSED:
+	case CLOCK_EVT_MODE_SHUTDOWN:
+		break;
+	case CLOCK_EVT_MODE_RESUME:
+		s3c64xx_clock_event_resume();
+		break;
+	}
+}
+
+static struct clock_event_device s3c64xx_clock_event_device = {
+	.name		= "s3c64xx_clkevt",
+	.features       = CLOCK_EVT_FEAT_PERIODIC |

CLOCK_EVT_FEAT_ONESHOT,

arch/arm/mach-s3c64xx/time.c new file +23 -0
+	.rating		= 200,
+	.shift		= 32,
+	.set_next_event	= s3c64xx_clock_event_set_next_event,
+	.set_mode	= s3c64xx_clock_event_set_mode,
+};
+
+static irqreturn_t s3c64xx_clock_event_isr(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = &s3c64xx_clock_event_device;
+
+	evt->event_handler(evt);
+
+	return IRQ_HANDLED;
+}
+
+static struct irqaction s3c64xx_clock_event_irq = {
+	.name		= "s3c64xx_clkevt_irq",
+	.flags		= IRQF_DISABLED | IRQF_TIMER | IRQF_IRQPOLL,
+	.handler	= s3c64xx_clock_event_isr,
+};
+
+static void __init s3c64xx_clock_event_init(void)
+{

arch/arm/mach-s3c64xx/time.c

Quoted 10 lines
arch/arm/mach-s3c64xx/time.c new file +10 -0
+	unsigned long clock_rate;
+
+	clock_rate = clk_get_rate(event_in);
+
+	clock_count_per_tick = clock_rate / HZ;
+
+	s3c64xx_clock_event_device.mult = div_sc(clock_rate,
+			NSEC_PER_SEC, s3c64xx_clock_event_device.shift);
+	s3c64xx_clock_event_device.max_delta_ns =
+			clockevent_delta2ns(-1,

&s3c64xx_clock_event_device);

Quoted 79 lines
arch/arm/mach-s3c64xx/time.c new file +57 -0
+	s3c64xx_clock_event_device.min_delta_ns =
+			clockevent_delta2ns(1, &s3c64xx_clock_event_device);
+
+	s3c64xx_clock_event_device.cpumask = cpumask_of(0);
+	clockevents_register_device(&s3c64xx_clock_event_device);
+
+	setup_irq(IRQ_TIMER3, &s3c64xx_clock_event_irq);
+}
+
+/*
+ * Clock source
+ */
+
+static struct clk *source_in;
+static struct clk *source_div;
+
+static cycle_t s3c64xx_clocksource_read(struct clocksource *cs)
+{
+	return (cycle_t) ~__raw_readl(S3C2410_TCNTO(PWM_SOURCE));
+}
+
+static void s3c64xx_clocksource_resume(struct clocksource *cs)
+{
+	unsigned long pclk;
+	struct clk *tscaler;
+
+	pclk = clk_get_rate(timerclk);
+	tscaler = clk_get_parent(source_div);
+	clk_set_rate(tscaler, pclk / 3);
+
+	clk_set_rate(source_div, pclk / 6);
+	clk_set_parent(source_in, source_div);
+
+	s3c64xx_pwm_init(PWM_SOURCE, TCNT_MAX);
+	s3c64xx_pwm_start(PWM_SOURCE, PERIODIC);
+}
+
+static struct clocksource pwm_clocksource = {
+	.name		= "s3c64xx_clksrc",
+	.rating		= 250,
+	.read		= s3c64xx_clocksource_read,
+	.mask		= CLOCKSOURCE_MASK(32),
+	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
+	.resume		= s3c64xx_clocksource_resume,
+};
+
+static void __init s3c64xx_clocksource_init(void)
+{
+	unsigned long clock_rate;
+
+	clock_rate = clk_get_rate(source_in);
+
+	s3c64xx_pwm_init(PWM_SOURCE, TCNT_MAX);
+	s3c64xx_pwm_start(PWM_SOURCE, PERIODIC);
+
+	if (clocksource_register_hz(&pwm_clocksource, clock_rate))
+		panic("%s: can't register clocksource\n",

pwm_clocksource.name);

arch/arm/mach-s3c64xx/time.c new file +21 -0
+}
+
+static void __init s3c64xx_timer_init_common(void)
+{
+	struct platform_device tmpdev;
+	unsigned long pclk;
+	struct clk *tscaler;
+
+	tmpdev.dev.bus = &platform_bus_type;
+
+	timerclk = clk_get(NULL, "timers");
+	if (IS_ERR(timerclk))
+		panic("failed to get timers clock for system timer");
+
+	tmpdev.id = PWM_EVENT;
+	event_in = clk_get(&tmpdev.dev, "pwm-tin");
+	if (IS_ERR(event_in))
+		panic("failed to get pwm-event_in clock for system timer");
+
+	event_div = clk_get(&tmpdev.dev, "pwm-tdiv");
+	if (IS_ERR(event_div))

arch/arm/mach-s3c64xx/time.c

Quoted 10 lines
arch/arm/mach-s3c64xx/time.c new file +10 -0
+		panic("failed to get pwm-event_div clock for system timer");
+
+	tmpdev.id = PWM_SOURCE;
+	source_in = clk_get(&tmpdev.dev, "pwm-tin");
+	if (IS_ERR(source_in))
+		panic("failed to get pwm-source_in clock for system timer");
+
+	source_div = clk_get(&tmpdev.dev, "pwm-tdiv");
+	if (IS_ERR(source_div))
+		panic("failed to get pwm-source_div clock for system

timer");

Quoted 46 lines
arch/arm/mach-s3c64xx/time.c new file +26 -0
+
+	pclk = clk_get_rate(timerclk);
+	tscaler = clk_get_parent(event_div);
+	clk_set_rate(tscaler, pclk / 3);
+
+	clk_set_rate(event_div, pclk / 6);
+	clk_set_parent(event_in, event_div);
+
+	clk_set_rate(source_div, pclk / 6);
+	clk_set_parent(source_in, source_div);
+
+	clk_enable(timerclk);
+	clk_enable(event_in);
+	clk_enable(source_in);
+}
+
+static void __init s3c64xx_timer_init(void)
+{
+	s3c64xx_timer_init_common();
+	s3c64xx_clock_event_init();
+	s3c64xx_clocksource_init();
+}
+
+struct sys_timer s3c64xx_timer = {
+	.init		= s3c64xx_timer_init,
+};
arch/arm/plat- renamed from arch/arm/plat-samsung/include/plat/cpu.h +0 -0

samsung/include/plat/cpu.h
index c0a5741..e1fcad0 100644

arch/arm/plat-samsung/include/plat/cpu.h +7 -1
@@ -74,6 +74,13 @@ extern struct syscore_ops s3c2416_pm_syscore_ops;
 extern struct syscore_ops s3c244x_pm_syscore_ops;
 extern struct syscore_ops s3c64xx_irq_syscore_ops;
 
+/* timer for 64xx */
+#ifdef CONFIG_S3C64XX_GENERIC_CLOCKEVENTS
+extern struct sys_timer s3c64xx_timer;
+#else
+#define s3c64xx_timer s3c24xx_timer
+#endif
+
 /* system device classes */
 
 extern struct sysdev_class s3c2410_sysclass;
--
Quoted 1 line

1.7.6.1

Hi Tomasz Figa,

Sorry for late response, probably I'm missing this......and I couldn't review this in detail :(

But as you know above mach-s3c64xx/time.c is similar with plat-s5p/s5p-time.c so I'd prefer that you could consolidate them, samsung_timer into plat-samsung directory. In addition, if you need to implement SoC specific feature, you can use soc_is_xxxx() like soc_is_s3c64xx() there.

If any problems, please let me know.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

#16 lore raw

PATCH 2/5 ARM: s3c64xx: Add generic high resolution time support using PWM timers.

Quoted 433 lines · Kukjin Kim
Quoted 5 lines · Tomasz Figa

From d1fa581f09d8f8a3cc24d05420e9e65112fd8c12 Mon Sep 17 00:00:00 2001
From: Tomasz Figa <[email protected]>
Date: Sun, 28 Aug 2011 02:45:06 +0200
Subject: [PATCH 2/5] ARM: s3c64xx: Add generic high resolution time

support

Quoted 3 lines

using PWM timers.

This patch adds a sys_timer implementing generic clock source and clock

event

Quoted 2 lines

device on S3C64xx using PWM timers 3 and 4. It can be enabled by a Kconfig option, based on which either the new sys_timer is used as s3c64xx_timer

or

Review

Quoted 49 lines

s3c64xx_timer is defined to s3c24xx_timer which is the old tick timer.

---
 arch/arm/Kconfig                         |    1 -
 arch/arm/mach-s3c64xx/Kconfig            |   12 ++
 arch/arm/mach-s3c64xx/Makefile           |    3 +
 arch/arm/mach-s3c64xx/time.c             |  320
++++++++++++++++++++++++++++++
 arch/arm/plat-samsung/include/plat/cpu.h |    7 +
 5 files changed, 342 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-s3c64xx/time.c
arch/arm/Kconfig +0 -1
index 9adc278..05941f6 100644
@@ -700,7 +700,6 @@ config ARCH_S3C64XX
 	select ARM_VIC
 	select HAVE_CLK
 	select NO_IOPORT
-	select ARCH_USES_GETTIMEOFFSET
 	select ARCH_HAS_CPUFREQ
 	select ARCH_REQUIRE_GPIOLIB
 	select SAMSUNG_CLKSRC
arch/arm/mach-s3c64xx/Kconfig +12 -0
index e4177e2..2db76fd 100644
@@ -25,6 +25,18 @@ config CPU_S3C6410
 	help
 	  Enable S3C6410 CPU support
 
+config S3C64XX_GENERIC_CLOCKEVENTS
+	bool "Enable high resolution generic time support using PWM timers"
+	select GENERIC_CLOCKEVENTS
+	help
+	  This option enables high resolution generic time support
+	  on S3C6400/S3C6410 SoCs using PWM timers 3 and 4 instead
+	  of standard periodic tick using PWM timer 4.
+
+config S3C64XX_ARCH_USES_GETTIMEOFFSET
+	def_bool y if !S3C64XX_GENERIC_CLOCKEVENTS
+	select ARCH_USES_GETTIMEOFFSET
+
 config S3C64XX_DMA
 	bool "S3C64XX DMA"
 	select S3C_DMA
(quoted diff) +0 -0

b/arch/arm/mach-s3c64xx/Makefile

Quoted 19 lines

index 4657363..a152002 100644

arch/arm/mach-s3c64xx/Makefile +3 -0
@@ -15,6 +15,9 @@ obj-y				+= cpu.o
 obj-y				+= clock.o
 obj-y				+= gpiolib.o
 
+# Generic clockevents
+obj-$(CONFIG_S3C64XX_GENERIC_CLOCKEVENTS) += time.o
+
 # Core support for S3C6400 system
 
 obj-$(CONFIG_CPU_S3C6400)	+= s3c6400.o
arch/arm/mach-s3c64xx/time.c new file +0 -0
new file mode 100644
index 0000000..4fa57f0

arch/arm/mach-s3c64xx/time.c line 7

Quoted 7 lines
arch/arm/mach-s3c64xx/time.c new file +7 -0
@@ -0,0 +1,320 @@
+/* linux/arch/arm/mach-s3c64xx/time.c
+ *
+ * Copyright (c) 2011 Tomasz Figa <[email protected]>
+ *
+ * based on linux/arch/arm/plat-samsung/s5p-time.c
+ *
+ * S3C64XX generic high resolution time support using PWM 3 and PWM 4

timers.

arch/arm/mach-s3c64xx/time.c line 14

Quoted 7 lines
arch/arm/mach-s3c64xx/time.c new file +7 -0
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Notes:
+ * - Frequency divisors are currently hardcoded to make both timers

operate

arch/arm/mach-s3c64xx/time.c line 15

Quoted 1 line
arch/arm/mach-s3c64xx/time.c new file +1 -0
+ *   at 1/6 the frequency of PCLK, so with the usual PCLK frequency of 66

MHz,

arch/arm/mach-s3c64xx/time.c line 18

Quoted 3 lines
arch/arm/mach-s3c64xx/time.c new file +3 -0
+ *   the timers are clocked at 11 MHz giving us the operating range from
+ *   90 nsec to 386 sec.
+ * - PWM registers, especially TCON, are shared with PWM driver, but

since

arch/arm/mach-s3c64xx/time.c line 19

Quoted 1 line
arch/arm/mach-s3c64xx/time.c new file +1 -0
+ *   all the clock event callbacks are run in atomic context

synchronization

Quoted 36 lines
arch/arm/mach-s3c64xx/time.c new file +36 -0
+ *   is not needed.
+ */
+
+#include <linux/sched.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/platform_device.h>
+
+#include <mach/map.h>
+#include <plat/regs-timer.h>
+#include <asm/mach/time.h>
+
+#define PWM_EVENT	(3)
+#define PWM_SOURCE	(4)
+
+#define ONESHOT		(0)
+#define PERIODIC	(1)
+
+#define TCNT_MAX	(0xffffffff)
+
+static struct clk *timerclk;
+
+/*
+ * PWM timers setup code
+ */
+
+static inline void s3c64xx_pwm_stop(unsigned int pwm_id)
+{
+	unsigned long tcon = __raw_readl(S3C2410_TCON);
+
+	switch (pwm_id) {
+	case 3:
+		tcon &= ~S3C2410_TCON_T3START;

arch/arm/mach-s3c64xx/time.c line 65

Quoted 10 lines
arch/arm/mach-s3c64xx/time.c new file +10 -0
+		break;
+	case 4:
+		tcon &= ~S3C2410_TCON_T4START;
+		break;
+	}
+
+	__raw_writel(tcon, S3C2410_TCON);
+}
+
+static inline void s3c64xx_pwm_init(unsigned int pwm_id, unsigned long

tcnt)

Quoted 134 lines
arch/arm/mach-s3c64xx/time.c new file +62 -0
+{
+	unsigned long tcon = __raw_readl(S3C2410_TCON);
+
+	/* timers reload after counting zero, so reduce the count by 1 */
+	--tcnt;
+
+	/* stop the timer and ask it to load the new value */
+	switch (pwm_id) {
+	case 3:
+		tcon &= ~(0xf<<16);
+		tcon |= S3C2410_TCON_T3MANUALUPD;
+		break;
+	case 4:
+		tcon &= ~(7<<20);
+		tcon |= S3C2410_TCON_T4MANUALUPD;
+		break;
+	}
+
+	__raw_writel(tcnt, S3C2410_TCNTB(pwm_id));
+	__raw_writel(tcnt, S3C2410_TCMPB(pwm_id));
+	__raw_writel(tcon, S3C2410_TCON);
+}
+
+static inline void s3c64xx_pwm_start(unsigned int pwm_id, bool periodic)
+{
+	unsigned long tcon = __raw_readl(S3C2410_TCON);
+
+	switch (pwm_id) {
+	case 3:
+		tcon |= S3C2410_TCON_T3START;
+		tcon &= ~S3C2410_TCON_T3MANUALUPD;
+
+		if (periodic)
+			tcon |= S3C2410_TCON_T3RELOAD;
+		else
+			tcon &= ~S3C2410_TCON_T3RELOAD;
+		break;
+	case 4:
+		tcon |= S3C2410_TCON_T4START;
+		tcon &= ~S3C2410_TCON_T4MANUALUPD;
+
+		if (periodic)
+			tcon |= S3C2410_TCON_T4RELOAD;
+		else
+			tcon &= ~S3C2410_TCON_T4RELOAD;
+		break;
+	}
+
+	__raw_writel(tcon, S3C2410_TCON);
+}
+
+/*
+ * Clock event
+ */
+
+static struct clk *event_in;
+static struct clk *event_div;
+
+static unsigned long clock_count_per_tick;
+
+static int s3c64xx_clock_event_set_next_event(unsigned long cycles,
+						struct clock_event_device

*evt)

arch/arm/mach-s3c64xx/time.c new file +24 -0
+{
+	s3c64xx_pwm_init(PWM_EVENT, cycles);
+	s3c64xx_pwm_start(PWM_EVENT, ONESHOT);
+	return 0;
+}
+
+static void s3c64xx_clock_event_resume(void)
+{
+	unsigned long pclk;
+	struct clk *tscaler;
+
+	pclk = clk_get_rate(timerclk);
+	tscaler = clk_get_parent(event_div);
+	clk_set_rate(tscaler, pclk / 3);
+
+	clk_set_rate(event_div, pclk / 6);
+	clk_set_parent(event_in, event_div);
+
+	s3c64xx_pwm_init(PWM_EVENT, clock_count_per_tick);
+	s3c64xx_pwm_start(PWM_EVENT, PERIODIC);
+}
+
+static void s3c64xx_clock_event_set_mode(enum clock_event_mode mode,
+						struct clock_event_device

*evt)

arch/arm/mach-s3c64xx/time.c new file +22 -0
+{
+	s3c64xx_pwm_stop(PWM_EVENT);
+
+	switch (mode) {
+	case CLOCK_EVT_MODE_PERIODIC:
+		s3c64xx_pwm_init(PWM_EVENT, clock_count_per_tick);
+		s3c64xx_pwm_start(PWM_EVENT, PERIODIC);
+		break;
+	case CLOCK_EVT_MODE_ONESHOT:
+		break;
+	case CLOCK_EVT_MODE_UNUSED:
+	case CLOCK_EVT_MODE_SHUTDOWN:
+		break;
+	case CLOCK_EVT_MODE_RESUME:
+		s3c64xx_clock_event_resume();
+		break;
+	}
+}
+
+static struct clock_event_device s3c64xx_clock_event_device = {
+	.name		= "s3c64xx_clkevt",
+	.features       = CLOCK_EVT_FEAT_PERIODIC |

CLOCK_EVT_FEAT_ONESHOT,

arch/arm/mach-s3c64xx/time.c new file +23 -0
+	.rating		= 200,
+	.shift		= 32,
+	.set_next_event	= s3c64xx_clock_event_set_next_event,
+	.set_mode	= s3c64xx_clock_event_set_mode,
+};
+
+static irqreturn_t s3c64xx_clock_event_isr(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = &s3c64xx_clock_event_device;
+
+	evt->event_handler(evt);
+
+	return IRQ_HANDLED;
+}
+
+static struct irqaction s3c64xx_clock_event_irq = {
+	.name		= "s3c64xx_clkevt_irq",
+	.flags		= IRQF_DISABLED | IRQF_TIMER | IRQF_IRQPOLL,
+	.handler	= s3c64xx_clock_event_isr,
+};
+
+static void __init s3c64xx_clock_event_init(void)
+{

arch/arm/mach-s3c64xx/time.c

Quoted 10 lines
arch/arm/mach-s3c64xx/time.c new file +10 -0
+	unsigned long clock_rate;
+
+	clock_rate = clk_get_rate(event_in);
+
+	clock_count_per_tick = clock_rate / HZ;
+
+	s3c64xx_clock_event_device.mult = div_sc(clock_rate,
+			NSEC_PER_SEC, s3c64xx_clock_event_device.shift);
+	s3c64xx_clock_event_device.max_delta_ns =
+			clockevent_delta2ns(-1,

&s3c64xx_clock_event_device);

Quoted 79 lines
arch/arm/mach-s3c64xx/time.c new file +57 -0
+	s3c64xx_clock_event_device.min_delta_ns =
+			clockevent_delta2ns(1, &s3c64xx_clock_event_device);
+
+	s3c64xx_clock_event_device.cpumask = cpumask_of(0);
+	clockevents_register_device(&s3c64xx_clock_event_device);
+
+	setup_irq(IRQ_TIMER3, &s3c64xx_clock_event_irq);
+}
+
+/*
+ * Clock source
+ */
+
+static struct clk *source_in;
+static struct clk *source_div;
+
+static cycle_t s3c64xx_clocksource_read(struct clocksource *cs)
+{
+	return (cycle_t) ~__raw_readl(S3C2410_TCNTO(PWM_SOURCE));
+}
+
+static void s3c64xx_clocksource_resume(struct clocksource *cs)
+{
+	unsigned long pclk;
+	struct clk *tscaler;
+
+	pclk = clk_get_rate(timerclk);
+	tscaler = clk_get_parent(source_div);
+	clk_set_rate(tscaler, pclk / 3);
+
+	clk_set_rate(source_div, pclk / 6);
+	clk_set_parent(source_in, source_div);
+
+	s3c64xx_pwm_init(PWM_SOURCE, TCNT_MAX);
+	s3c64xx_pwm_start(PWM_SOURCE, PERIODIC);
+}
+
+static struct clocksource pwm_clocksource = {
+	.name		= "s3c64xx_clksrc",
+	.rating		= 250,
+	.read		= s3c64xx_clocksource_read,
+	.mask		= CLOCKSOURCE_MASK(32),
+	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
+	.resume		= s3c64xx_clocksource_resume,
+};
+
+static void __init s3c64xx_clocksource_init(void)
+{
+	unsigned long clock_rate;
+
+	clock_rate = clk_get_rate(source_in);
+
+	s3c64xx_pwm_init(PWM_SOURCE, TCNT_MAX);
+	s3c64xx_pwm_start(PWM_SOURCE, PERIODIC);
+
+	if (clocksource_register_hz(&pwm_clocksource, clock_rate))
+		panic("%s: can't register clocksource\n",

pwm_clocksource.name);

arch/arm/mach-s3c64xx/time.c new file +21 -0
+}
+
+static void __init s3c64xx_timer_init_common(void)
+{
+	struct platform_device tmpdev;
+	unsigned long pclk;
+	struct clk *tscaler;
+
+	tmpdev.dev.bus = &platform_bus_type;
+
+	timerclk = clk_get(NULL, "timers");
+	if (IS_ERR(timerclk))
+		panic("failed to get timers clock for system timer");
+
+	tmpdev.id = PWM_EVENT;
+	event_in = clk_get(&tmpdev.dev, "pwm-tin");
+	if (IS_ERR(event_in))
+		panic("failed to get pwm-event_in clock for system timer");
+
+	event_div = clk_get(&tmpdev.dev, "pwm-tdiv");
+	if (IS_ERR(event_div))

arch/arm/mach-s3c64xx/time.c

Quoted 10 lines
arch/arm/mach-s3c64xx/time.c new file +10 -0
+		panic("failed to get pwm-event_div clock for system timer");
+
+	tmpdev.id = PWM_SOURCE;
+	source_in = clk_get(&tmpdev.dev, "pwm-tin");
+	if (IS_ERR(source_in))
+		panic("failed to get pwm-source_in clock for system timer");
+
+	source_div = clk_get(&tmpdev.dev, "pwm-tdiv");
+	if (IS_ERR(source_div))
+		panic("failed to get pwm-source_div clock for system

timer");

Quoted 44 lines
arch/arm/mach-s3c64xx/time.c new file +26 -0
+
+	pclk = clk_get_rate(timerclk);
+	tscaler = clk_get_parent(event_div);
+	clk_set_rate(tscaler, pclk / 3);
+
+	clk_set_rate(event_div, pclk / 6);
+	clk_set_parent(event_in, event_div);
+
+	clk_set_rate(source_div, pclk / 6);
+	clk_set_parent(source_in, source_div);
+
+	clk_enable(timerclk);
+	clk_enable(event_in);
+	clk_enable(source_in);
+}
+
+static void __init s3c64xx_timer_init(void)
+{
+	s3c64xx_timer_init_common();
+	s3c64xx_clock_event_init();
+	s3c64xx_clocksource_init();
+}
+
+struct sys_timer s3c64xx_timer = {
+	.init		= s3c64xx_timer_init,
+};
arch/arm/plat- renamed from arch/arm/plat-samsung/include/plat/cpu.h +0 -0

samsung/include/plat/cpu.h
index c0a5741..e1fcad0 100644

arch/arm/plat-samsung/include/plat/cpu.h +7 -0
@@ -74,6 +74,13 @@ extern struct syscore_ops s3c2416_pm_syscore_ops;
 extern struct syscore_ops s3c244x_pm_syscore_ops;
 extern struct syscore_ops s3c64xx_irq_syscore_ops;
 
+/* timer for 64xx */
+#ifdef CONFIG_S3C64XX_GENERIC_CLOCKEVENTS
+extern struct sys_timer s3c64xx_timer;
+#else
+#define s3c64xx_timer s3c24xx_timer
+#endif
+
 /* system device classes */
 
Quoted 8 lines
Quoted 3 lines
arch/arm/plat-samsung/include/plat/cpu.h +0 -1
 extern struct sysdev_class s3c2410_sysclass;
--

1.7.6.1

Hi Tomasz Figa,

Sorry for late response, probably I'm missing this......and I couldn't review this in detail :(

Hi,

Same on my side, sorry for my absence on the list. I've been very busy, with no time for mainline submissions.

Quoted 5 lines

But as you know above mach-s3c64xx/time.c is similar with plat-s5p/s5p-time.c so I'd prefer that you could consolidate them, samsung_timer into plat-samsung directory. In addition, if you need to implement SoC specific feature, you can use soc_is_xxxx() like soc_is_s3c64xx() there.

OK, I will do that.

Quoted 1 line

If any problems, please let me know.

OK, thanks.

Best regards,
Tom

Quoted 8 lines

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

#17 lore raw

PATCH 2/5 ARM: s3c64xx: Add generic high resolution time support using PWM timers.

In reply to Tomasz Figa

Review

Quoted 11 lines · Tomasz Figa
(quoted diff) +11 -0
+/*
+ * Clock source
+ */
+
+static struct clk *source_in;
+static struct clk *source_div;
+
+static cycle_t s3c64xx_clocksource_read(struct clocksource *cs)
+{
+	return (cycle_t) ~__raw_readl(S3C2410_TCNTO(PWM_SOURCE));
+}

This looks like a very simple MMIO clocksource... This looks the same as clocksource_mmio_readl_down().

Quoted 27 lines
(quoted diff) +27 -0
+
+static void s3c64xx_clocksource_resume(struct clocksource *cs)
+{
+	unsigned long pclk;
+	struct clk *tscaler;
+
+	pclk = clk_get_rate(timerclk);
+	tscaler = clk_get_parent(source_div);
+	clk_set_rate(tscaler, pclk / 3);
+
+	clk_set_rate(source_div, pclk / 6);
+	clk_set_parent(source_in, source_div);
+
+	s3c64xx_pwm_init(PWM_SOURCE, TCNT_MAX);
+	s3c64xx_pwm_start(PWM_SOURCE, PERIODIC);
+}
+
+static struct clocksource pwm_clocksource = {
+	.name		= "s3c64xx_clksrc",
+	.rating		= 250,
+	.read		= s3c64xx_clocksource_read,
+	.mask		= CLOCKSOURCE_MASK(32),
+	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
+	.resume		= s3c64xx_clocksource_resume,
+};
+
+static void __init s3c64xx_clocksource_init(void)

Review

Quoted 10 lines
(quoted diff) +10 -0
+{
+	unsigned long clock_rate;
+
+	clock_rate = clk_get_rate(source_in);
+
+	s3c64xx_pwm_init(PWM_SOURCE, TCNT_MAX);
+	s3c64xx_pwm_start(PWM_SOURCE, PERIODIC);
+
+	if (clocksource_register_hz(&pwm_clocksource, clock_rate))
+		panic("%s: can't register clocksource\n", pwm_clocksource.name);

Apart from the resume entry, this looks like it could use drivers/clocksource/mmio.c - and so avoid yet another clocksource creation.

Maybe adding some kind of resume support to mmio.c would be a good idea?