* [PATCH 1/3] ARM: move timer-sp.c from versatile to common
@ 2010-09-30 22:49 Rob Herring
2010-09-30 22:49 ` [PATCH 3/3] ARM: twd_smp: add clock api support Rob Herring
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Rob Herring @ 2010-09-30 22:49 UTC (permalink / raw)
To: linux-arm-kernel
From: Rob Herring <rob.herring@smooth-stone.com>
The timer-sp h/w used on versatile platforms can also be used for other
platforms, so move it to a common location.
Rename timer names for clocksource and clockevent to generic name.
Which timer instance is which is platform dependent.
Signed-off-by: Rob Herring <rob.herring@smooth-stone.com>
---
arch/arm/common/Makefile | 1 +
arch/arm/common/timer-sp.c | 154 ++++++++++++++++++++++
arch/arm/include/asm/hardware/timer-sp.h | 2 +
arch/arm/mach-integrator/integrator_cp.c | 2 +-
arch/arm/mach-realview/core.c | 2 +-
arch/arm/mach-versatile/core.c | 2 +-
arch/arm/mach-vexpress/ct-ca9x4.c | 2 +-
arch/arm/mach-vexpress/v2m.c | 2 +-
arch/arm/plat-versatile/Makefile | 1 -
arch/arm/plat-versatile/include/plat/timer-sp.h | 2 -
arch/arm/plat-versatile/timer-sp.c | 156 -----------------------
11 files changed, 162 insertions(+), 164 deletions(-)
create mode 100644 arch/arm/common/timer-sp.c
create mode 100644 arch/arm/include/asm/hardware/timer-sp.h
delete mode 100644 arch/arm/plat-versatile/include/plat/timer-sp.h
delete mode 100644 arch/arm/plat-versatile/timer-sp.c
diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
index e6e8664..e7521bc 100644
--- a/arch/arm/common/Makefile
+++ b/arch/arm/common/Makefile
@@ -17,3 +17,4 @@ obj-$(CONFIG_ARCH_IXP2000) += uengine.o
obj-$(CONFIG_ARCH_IXP23XX) += uengine.o
obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o
obj-$(CONFIG_COMMON_CLKDEV) += clkdev.o
+obj-$(CONFIG_ARM_TIMER_SP804) += timer-sp.o
diff --git a/arch/arm/common/timer-sp.c b/arch/arm/common/timer-sp.c
new file mode 100644
index 0000000..b552358
--- /dev/null
+++ b/arch/arm/common/timer-sp.c
@@ -0,0 +1,154 @@
+/*
+ * linux/arch/arm/common/timer-sp.c
+ *
+ * Copyright (C) 1999 - 2003 ARM Limited
+ * Copyright (C) 2000 Deep Blue Solutions Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+#include <linux/clocksource.h>
+#include <linux/clockchips.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/io.h>
+
+#include <asm/hardware/arm_timer.h>
+
+/*
+ * These timers are currently always setup to be clocked at 1MHz.
+ */
+#define TIMER_FREQ_KHZ (1000)
+#define TIMER_RELOAD (TIMER_FREQ_KHZ * 1000 / HZ)
+
+static void __iomem *clksrc_base;
+
+static cycle_t sp804_read(struct clocksource *cs)
+{
+ return ~readl(clksrc_base + TIMER_VALUE);
+}
+
+static struct clocksource clocksource_sp804 = {
+ .name = "timer-sp",
+ .rating = 200,
+ .read = sp804_read,
+ .mask = CLOCKSOURCE_MASK(32),
+ .shift = 20,
+ .flags = CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
+void __init sp804_clocksource_init(void __iomem *base)
+{
+ struct clocksource *cs = &clocksource_sp804;
+
+ clksrc_base = base;
+
+ /* setup timer 0 as free-running clocksource */
+ writel(0, clksrc_base + TIMER_CTRL);
+ writel(0xffffffff, clksrc_base + TIMER_LOAD);
+ writel(0xffffffff, clksrc_base + TIMER_VALUE);
+ writel(TIMER_CTRL_32BIT | TIMER_CTRL_ENABLE | TIMER_CTRL_PERIODIC,
+ clksrc_base + TIMER_CTRL);
+
+ cs->mult = clocksource_khz2mult(TIMER_FREQ_KHZ, cs->shift);
+ clocksource_register(cs);
+}
+
+
+static void __iomem *clkevt_base;
+
+/*
+ * IRQ handler for the timer
+ */
+static irqreturn_t sp804_timer_interrupt(int irq, void *dev_id)
+{
+ struct clock_event_device *evt = dev_id;
+
+ /* clear the interrupt */
+ writel(1, clkevt_base + TIMER_INTCLR);
+
+ evt->event_handler(evt);
+
+ return IRQ_HANDLED;
+}
+
+static void sp804_set_mode(enum clock_event_mode mode,
+ struct clock_event_device *evt)
+{
+ unsigned long ctrl = TIMER_CTRL_32BIT | TIMER_CTRL_IE;
+
+ writel(ctrl, clkevt_base + TIMER_CTRL);
+
+ switch (mode) {
+ case CLOCK_EVT_MODE_PERIODIC:
+ writel(TIMER_RELOAD, clkevt_base + TIMER_LOAD);
+ ctrl |= TIMER_CTRL_PERIODIC | TIMER_CTRL_ENABLE;
+ break;
+
+ case CLOCK_EVT_MODE_ONESHOT:
+ /* period set, and timer enabled in 'next_event' hook */
+ ctrl |= TIMER_CTRL_ONESHOT;
+ break;
+
+ case CLOCK_EVT_MODE_UNUSED:
+ case CLOCK_EVT_MODE_SHUTDOWN:
+ default:
+ break;
+ }
+
+ writel(ctrl, clkevt_base + TIMER_CTRL);
+}
+
+static int sp804_set_next_event(unsigned long next,
+ struct clock_event_device *evt)
+{
+ unsigned long ctrl = readl(clkevt_base + TIMER_CTRL);
+
+ writel(next, clkevt_base + TIMER_LOAD);
+ writel(ctrl | TIMER_CTRL_ENABLE, clkevt_base + TIMER_CTRL);
+
+ return 0;
+}
+
+static struct clock_event_device sp804_clockevent = {
+ .name = "timer-sp",
+ .shift = 32,
+ .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
+ .set_mode = sp804_set_mode,
+ .set_next_event = sp804_set_next_event,
+ .rating = 300,
+ .cpumask = cpu_all_mask,
+};
+
+static struct irqaction sp804_timer_irq = {
+ .name = "timer",
+ .flags = IRQF_DISABLED | IRQF_TIMER | IRQF_IRQPOLL,
+ .handler = sp804_timer_interrupt,
+ .dev_id = &sp804_clockevent,
+};
+
+void __init sp804_clockevents_init(void __iomem *base, unsigned int timer_irq)
+{
+ struct clock_event_device *evt = &sp804_clockevent;
+
+ clkevt_base = base;
+
+ evt->irq = timer_irq;
+ evt->mult = div_sc(TIMER_FREQ_KHZ, NSEC_PER_MSEC, evt->shift);
+ evt->max_delta_ns = clockevent_delta2ns(0xffffffff, evt);
+ evt->min_delta_ns = clockevent_delta2ns(0xf, evt);
+
+ setup_irq(timer_irq, &sp804_timer_irq);
+ clockevents_register_device(evt);
+}
diff --git a/arch/arm/include/asm/hardware/timer-sp.h b/arch/arm/include/asm/hardware/timer-sp.h
new file mode 100644
index 0000000..21e75e3
--- /dev/null
+++ b/arch/arm/include/asm/hardware/timer-sp.h
@@ -0,0 +1,2 @@
+void sp804_clocksource_init(void __iomem *);
+void sp804_clockevents_init(void __iomem *, unsigned int);
diff --git a/arch/arm/mach-integrator/integrator_cp.c b/arch/arm/mach-integrator/integrator_cp.c
index 05db40e..6c432a0 100644
--- a/arch/arm/mach-integrator/integrator_cp.c
+++ b/arch/arm/mach-integrator/integrator_cp.c
@@ -41,7 +41,7 @@
#include <asm/mach/map.h>
#include <asm/mach/time.h>
-#include <plat/timer-sp.h>
+#include <asm/hardware/timer-sp.h>
#include "common.h"
diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c
index 2fa38df..121c6d6 100644
--- a/arch/arm/mach-realview/core.c
+++ b/arch/arm/mach-realview/core.c
@@ -50,7 +50,7 @@
#include <mach/clkdev.h>
#include <mach/platform.h>
#include <mach/irqs.h>
-#include <plat/timer-sp.h>
+#include <asm/hardware/timer-sp.h>
#include "core.h"
diff --git a/arch/arm/mach-versatile/core.c b/arch/arm/mach-versatile/core.c
index e38acb0..6b93bd6 100644
--- a/arch/arm/mach-versatile/core.c
+++ b/arch/arm/mach-versatile/core.c
@@ -49,7 +49,7 @@
#include <mach/clkdev.h>
#include <mach/hardware.h>
#include <mach/platform.h>
-#include <plat/timer-sp.h>
+#include <asm/hardware/timer-sp.h>
#include "core.h"
diff --git a/arch/arm/mach-vexpress/ct-ca9x4.c b/arch/arm/mach-vexpress/ct-ca9x4.c
index efb1270..685e6ee 100644
--- a/arch/arm/mach-vexpress/ct-ca9x4.c
+++ b/arch/arm/mach-vexpress/ct-ca9x4.c
@@ -21,7 +21,7 @@
#include <mach/clkdev.h>
#include <mach/ct-ca9x4.h>
-#include <plat/timer-sp.h>
+#include <asm/hardware/timer-sp.h>
#include <asm/mach/arch.h>
#include <asm/mach/map.h>
diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
index 817f0ad..5bc853d 100644
--- a/arch/arm/mach-vexpress/v2m.c
+++ b/arch/arm/mach-vexpress/v2m.c
@@ -22,7 +22,7 @@
#include <mach/clkdev.h>
#include <mach/motherboard.h>
-#include <plat/timer-sp.h>
+#include <asm/hardware/timer-sp.h>
#include "core.h"
diff --git a/arch/arm/plat-versatile/Makefile b/arch/arm/plat-versatile/Makefile
index 5cf88e8..aaa571d 100644
--- a/arch/arm/plat-versatile/Makefile
+++ b/arch/arm/plat-versatile/Makefile
@@ -1,5 +1,4 @@
obj-y := clock.o
-obj-$(CONFIG_ARM_TIMER_SP804) += timer-sp.o
obj-$(CONFIG_ARCH_REALVIEW) += sched-clock.o
obj-$(CONFIG_ARCH_VERSATILE) += sched-clock.o
ifeq ($(CONFIG_LEDS_CLASS),y)
diff --git a/arch/arm/plat-versatile/include/plat/timer-sp.h b/arch/arm/plat-versatile/include/plat/timer-sp.h
deleted file mode 100644
index 21e75e3..0000000
--- a/arch/arm/plat-versatile/include/plat/timer-sp.h
+++ /dev/null
@@ -1,2 +0,0 @@
-void sp804_clocksource_init(void __iomem *);
-void sp804_clockevents_init(void __iomem *, unsigned int);
diff --git a/arch/arm/plat-versatile/timer-sp.c b/arch/arm/plat-versatile/timer-sp.c
deleted file mode 100644
index fb0d1c2..0000000
--- a/arch/arm/plat-versatile/timer-sp.c
+++ /dev/null
@@ -1,156 +0,0 @@
-/*
- * linux/arch/arm/plat-versatile/timer-sp.c
- *
- * Copyright (C) 1999 - 2003 ARM Limited
- * Copyright (C) 2000 Deep Blue Solutions Ltd
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- */
-#include <linux/clocksource.h>
-#include <linux/clockchips.h>
-#include <linux/interrupt.h>
-#include <linux/irq.h>
-#include <linux/io.h>
-
-#include <asm/hardware/arm_timer.h>
-
-#include <plat/timer-sp.h>
-
-/*
- * These timers are currently always setup to be clocked at 1MHz.
- */
-#define TIMER_FREQ_KHZ (1000)
-#define TIMER_RELOAD (TIMER_FREQ_KHZ * 1000 / HZ)
-
-static void __iomem *clksrc_base;
-
-static cycle_t sp804_read(struct clocksource *cs)
-{
- return ~readl(clksrc_base + TIMER_VALUE);
-}
-
-static struct clocksource clocksource_sp804 = {
- .name = "timer3",
- .rating = 200,
- .read = sp804_read,
- .mask = CLOCKSOURCE_MASK(32),
- .shift = 20,
- .flags = CLOCK_SOURCE_IS_CONTINUOUS,
-};
-
-void __init sp804_clocksource_init(void __iomem *base)
-{
- struct clocksource *cs = &clocksource_sp804;
-
- clksrc_base = base;
-
- /* setup timer 0 as free-running clocksource */
- writel(0, clksrc_base + TIMER_CTRL);
- writel(0xffffffff, clksrc_base + TIMER_LOAD);
- writel(0xffffffff, clksrc_base + TIMER_VALUE);
- writel(TIMER_CTRL_32BIT | TIMER_CTRL_ENABLE | TIMER_CTRL_PERIODIC,
- clksrc_base + TIMER_CTRL);
-
- cs->mult = clocksource_khz2mult(TIMER_FREQ_KHZ, cs->shift);
- clocksource_register(cs);
-}
-
-
-static void __iomem *clkevt_base;
-
-/*
- * IRQ handler for the timer
- */
-static irqreturn_t sp804_timer_interrupt(int irq, void *dev_id)
-{
- struct clock_event_device *evt = dev_id;
-
- /* clear the interrupt */
- writel(1, clkevt_base + TIMER_INTCLR);
-
- evt->event_handler(evt);
-
- return IRQ_HANDLED;
-}
-
-static void sp804_set_mode(enum clock_event_mode mode,
- struct clock_event_device *evt)
-{
- unsigned long ctrl = TIMER_CTRL_32BIT | TIMER_CTRL_IE;
-
- writel(ctrl, clkevt_base + TIMER_CTRL);
-
- switch (mode) {
- case CLOCK_EVT_MODE_PERIODIC:
- writel(TIMER_RELOAD, clkevt_base + TIMER_LOAD);
- ctrl |= TIMER_CTRL_PERIODIC | TIMER_CTRL_ENABLE;
- break;
-
- case CLOCK_EVT_MODE_ONESHOT:
- /* period set, and timer enabled in 'next_event' hook */
- ctrl |= TIMER_CTRL_ONESHOT;
- break;
-
- case CLOCK_EVT_MODE_UNUSED:
- case CLOCK_EVT_MODE_SHUTDOWN:
- default:
- break;
- }
-
- writel(ctrl, clkevt_base + TIMER_CTRL);
-}
-
-static int sp804_set_next_event(unsigned long next,
- struct clock_event_device *evt)
-{
- unsigned long ctrl = readl(clkevt_base + TIMER_CTRL);
-
- writel(next, clkevt_base + TIMER_LOAD);
- writel(ctrl | TIMER_CTRL_ENABLE, clkevt_base + TIMER_CTRL);
-
- return 0;
-}
-
-static struct clock_event_device sp804_clockevent = {
- .name = "timer0",
- .shift = 32,
- .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
- .set_mode = sp804_set_mode,
- .set_next_event = sp804_set_next_event,
- .rating = 300,
- .cpumask = cpu_all_mask,
-};
-
-static struct irqaction sp804_timer_irq = {
- .name = "timer",
- .flags = IRQF_DISABLED | IRQF_TIMER | IRQF_IRQPOLL,
- .handler = sp804_timer_interrupt,
- .dev_id = &sp804_clockevent,
-};
-
-void __init sp804_clockevents_init(void __iomem *base, unsigned int timer_irq)
-{
- struct clock_event_device *evt = &sp804_clockevent;
-
- clkevt_base = base;
-
- evt->irq = timer_irq;
- evt->mult = div_sc(TIMER_FREQ_KHZ, NSEC_PER_MSEC, evt->shift);
- evt->max_delta_ns = clockevent_delta2ns(0xffffffff, evt);
- evt->min_delta_ns = clockevent_delta2ns(0xf, evt);
-
- setup_irq(timer_irq, &sp804_timer_irq);
- clockevents_register_device(evt);
-}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] ARM: twd_smp: add clock api support
2010-09-30 22:49 [PATCH 1/3] ARM: move timer-sp.c from versatile to common Rob Herring
@ 2010-09-30 22:49 ` Rob Herring
2010-10-01 0:49 ` Colin Cross
2010-10-01 2:39 ` [PATCH 1/3] ARM: move timer-sp.c from versatile to common Jean-Christophe PLAGNIOL-VILLARD
2010-10-01 8:24 ` Russell King - ARM Linux
2 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2010-09-30 22:49 UTC (permalink / raw)
To: linux-arm-kernel
From: Rob Herring <rob.herring@smooth-stone.com>
The private timer freq is currently dynamically detected
using jiffies count to determine the rate. This method adds
a delay to boot-up, so use the clock api instead to get the
clock rate.
Signed-off-by: Rob Herring <rob.herring@smooth-stone.com>
---
arch/arm/include/asm/smp_twd.h | 2 ++
arch/arm/kernel/smp_twd.c | 7 +++++++
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/arch/arm/include/asm/smp_twd.h b/arch/arm/include/asm/smp_twd.h
index 634f357..bafad52 100644
--- a/arch/arm/include/asm/smp_twd.h
+++ b/arch/arm/include/asm/smp_twd.h
@@ -19,11 +19,13 @@
#define TWD_TIMER_CONTROL_IT_ENABLE (1 << 2)
struct clock_event_device;
+struct clk;
extern void __iomem *twd_base;
void twd_timer_stop(void);
int twd_timer_ack(void);
void twd_timer_setup(struct clock_event_device *);
+void twd_timer_init(void __iomem *base, struct clk *clk);
#endif
diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index 35882fb..1a3c959 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -17,6 +17,7 @@
#include <linux/clockchips.h>
#include <linux/irq.h>
#include <linux/io.h>
+#include <linux/clk.h>
#include <asm/smp_twd.h>
#include <asm/hardware/gic.h>
@@ -151,6 +152,12 @@ void __cpuinit twd_timer_setup(struct clock_event_device *clk)
clockevents_register_device(clk);
}
+void __init twd_timer_init(void __iomem *base, struct clk *clk)
+{
+ twd_base = base;
+ twd_timer_rate = clk_get_rate(clk);
+}
+
#ifdef CONFIG_HOTPLUG_CPU
/*
* take a local timer down
--
1.7.0.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] ARM: twd_smp: add clock api support
2010-09-30 22:49 ` [PATCH 3/3] ARM: twd_smp: add clock api support Rob Herring
@ 2010-10-01 0:49 ` Colin Cross
2010-10-01 2:03 ` Rob Herring
0 siblings, 1 reply; 14+ messages in thread
From: Colin Cross @ 2010-10-01 0:49 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 30, 2010 at 3:49 PM, Rob Herring <robherring2@gmail.com> wrote:
> From: Rob Herring <rob.herring@smooth-stone.com>
>
> The private timer freq is currently dynamically detected
> using jiffies count to determine the rate. This method adds
> a delay to boot-up, so use the clock api instead to get the
> clock rate.
>
> Signed-off-by: Rob Herring <rob.herring@smooth-stone.com>
> ---
> ?arch/arm/include/asm/smp_twd.h | ? ?2 ++
> ?arch/arm/kernel/smp_twd.c ? ? ?| ? ?7 +++++++
> ?2 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/include/asm/smp_twd.h b/arch/arm/include/asm/smp_twd.h
> index 634f357..bafad52 100644
> --- a/arch/arm/include/asm/smp_twd.h
> +++ b/arch/arm/include/asm/smp_twd.h
> @@ -19,11 +19,13 @@
> ?#define TWD_TIMER_CONTROL_IT_ENABLE ? ?(1 << 2)
>
> ?struct clock_event_device;
> +struct clk;
>
> ?extern void __iomem *twd_base;
>
> ?void twd_timer_stop(void);
> ?int twd_timer_ack(void);
> ?void twd_timer_setup(struct clock_event_device *);
> +void twd_timer_init(void __iomem *base, struct clk *clk);
>
> ?#endif
> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
> index 35882fb..1a3c959 100644
> --- a/arch/arm/kernel/smp_twd.c
> +++ b/arch/arm/kernel/smp_twd.c
> @@ -17,6 +17,7 @@
> ?#include <linux/clockchips.h>
> ?#include <linux/irq.h>
> ?#include <linux/io.h>
> +#include <linux/clk.h>
>
> ?#include <asm/smp_twd.h>
> ?#include <asm/hardware/gic.h>
> @@ -151,6 +152,12 @@ void __cpuinit twd_timer_setup(struct clock_event_device *clk)
> ? ? ? ?clockevents_register_device(clk);
> ?}
>
> +void __init twd_timer_init(void __iomem *base, struct clk *clk)
> +{
> + ? ? ? twd_base = base;
> + ? ? ? twd_timer_rate = clk_get_rate(clk);
> +}
> +
> ?#ifdef CONFIG_HOTPLUG_CPU
> ?/*
> ?* take a local timer down
The local timers run off the PERIPHCLK in the CPU, which is specified
as the CPU clock divided by an implementation defined integer >= 2.
If you take the divider value as a parameter to twd_timer_init, the
clock that is passed in can be the cpu's clock.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] ARM: twd_smp: add clock api support
2010-10-01 0:49 ` Colin Cross
@ 2010-10-01 2:03 ` Rob Herring
2010-10-01 2:30 ` Colin Cross
0 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2010-10-01 2:03 UTC (permalink / raw)
To: linux-arm-kernel
On 09/30/2010 07:49 PM, Colin Cross wrote:
> On Thu, Sep 30, 2010 at 3:49 PM, Rob Herring<robherring2@gmail.com> wrote:
>> From: Rob Herring<rob.herring@smooth-stone.com>
>>
>> The private timer freq is currently dynamically detected
>> using jiffies count to determine the rate. This method adds
>> a delay to boot-up, so use the clock api instead to get the
>> clock rate.
>>
>> Signed-off-by: Rob Herring<rob.herring@smooth-stone.com>
>> ---
>> arch/arm/include/asm/smp_twd.h | 2 ++
>> arch/arm/kernel/smp_twd.c | 7 +++++++
>> 2 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/smp_twd.h b/arch/arm/include/asm/smp_twd.h
>> index 634f357..bafad52 100644
>> --- a/arch/arm/include/asm/smp_twd.h
>> +++ b/arch/arm/include/asm/smp_twd.h
>> @@ -19,11 +19,13 @@
>> #define TWD_TIMER_CONTROL_IT_ENABLE (1<< 2)
>>
>> struct clock_event_device;
>> +struct clk;
>>
>> extern void __iomem *twd_base;
>>
>> void twd_timer_stop(void);
>> int twd_timer_ack(void);
>> void twd_timer_setup(struct clock_event_device *);
>> +void twd_timer_init(void __iomem *base, struct clk *clk);
>>
>> #endif
>> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
>> index 35882fb..1a3c959 100644
>> --- a/arch/arm/kernel/smp_twd.c
>> +++ b/arch/arm/kernel/smp_twd.c
>> @@ -17,6 +17,7 @@
>> #include<linux/clockchips.h>
>> #include<linux/irq.h>
>> #include<linux/io.h>
>> +#include<linux/clk.h>
>>
>> #include<asm/smp_twd.h>
>> #include<asm/hardware/gic.h>
>> @@ -151,6 +152,12 @@ void __cpuinit twd_timer_setup(struct clock_event_device *clk)
>> clockevents_register_device(clk);
>> }
>>
>> +void __init twd_timer_init(void __iomem *base, struct clk *clk)
>> +{
>> + twd_base = base;
>> + twd_timer_rate = clk_get_rate(clk);
>> +}
>> +
>> #ifdef CONFIG_HOTPLUG_CPU
>> /*
>> * take a local timer down
>
> The local timers run off the PERIPHCLK in the CPU, which is specified
> as the CPU clock divided by an implementation defined integer>= 2.
> If you take the divider value as a parameter to twd_timer_init, the
> clock that is passed in can be the cpu's clock.
That is an implementation detail of the A9. Future designs could be
completely asynchronous. Using the clock api works either way.
Rob
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] ARM: twd_smp: add clock api support
2010-10-01 2:03 ` Rob Herring
@ 2010-10-01 2:30 ` Colin Cross
2010-10-01 3:14 ` Rob Herring
0 siblings, 1 reply; 14+ messages in thread
From: Colin Cross @ 2010-10-01 2:30 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 30, 2010 at 7:03 PM, Rob Herring <robherring2@gmail.com> wrote:
> On 09/30/2010 07:49 PM, Colin Cross wrote:
>>
>> On Thu, Sep 30, 2010 at 3:49 PM, Rob Herring<robherring2@gmail.com>
>> ?wrote:
>>>
>>> From: Rob Herring<rob.herring@smooth-stone.com>
>>>
>>> The private timer freq is currently dynamically detected
>>> using jiffies count to determine the rate. This method adds
>>> a delay to boot-up, so use the clock api instead to get the
>>> clock rate.
>>>
>>> Signed-off-by: Rob Herring<rob.herring@smooth-stone.com>
>>> ---
>>> ?arch/arm/include/asm/smp_twd.h | ? ?2 ++
>>> ?arch/arm/kernel/smp_twd.c ? ? ?| ? ?7 +++++++
>>> ?2 files changed, 9 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/include/asm/smp_twd.h
>>> b/arch/arm/include/asm/smp_twd.h
>>> index 634f357..bafad52 100644
>>> --- a/arch/arm/include/asm/smp_twd.h
>>> +++ b/arch/arm/include/asm/smp_twd.h
>>> @@ -19,11 +19,13 @@
>>> ?#define TWD_TIMER_CONTROL_IT_ENABLE ? ?(1<< ?2)
>>>
>>> ?struct clock_event_device;
>>> +struct clk;
>>>
>>> ?extern void __iomem *twd_base;
>>>
>>> ?void twd_timer_stop(void);
>>> ?int twd_timer_ack(void);
>>> ?void twd_timer_setup(struct clock_event_device *);
>>> +void twd_timer_init(void __iomem *base, struct clk *clk);
>>>
>>> ?#endif
>>> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
>>> index 35882fb..1a3c959 100644
>>> --- a/arch/arm/kernel/smp_twd.c
>>> +++ b/arch/arm/kernel/smp_twd.c
>>> @@ -17,6 +17,7 @@
>>> ?#include<linux/clockchips.h>
>>> ?#include<linux/irq.h>
>>> ?#include<linux/io.h>
>>> +#include<linux/clk.h>
>>>
>>> ?#include<asm/smp_twd.h>
>>> ?#include<asm/hardware/gic.h>
>>> @@ -151,6 +152,12 @@ void __cpuinit twd_timer_setup(struct
>>> clock_event_device *clk)
>>> ? ? ? ?clockevents_register_device(clk);
>>> ?}
>>>
>>> +void __init twd_timer_init(void __iomem *base, struct clk *clk)
>>> +{
>>> + ? ? ? twd_base = base;
>>> + ? ? ? twd_timer_rate = clk_get_rate(clk);
>>> +}
>>> +
>>> ?#ifdef CONFIG_HOTPLUG_CPU
>>> ?/*
>>> ?* take a local timer down
>>
>> The local timers run off the PERIPHCLK in the CPU, which is specified
>> as the CPU clock divided by an implementation defined integer>= 2.
>> If you take the divider value as a parameter to twd_timer_init, the
>> clock that is passed in can be the cpu's clock.
>
> That is an implementation detail of the A9. Future designs could be
> completely asynchronous. Using the clock api works either way.
True. That could still be handled by passing a divider of 1. For all
current implementations, this api will be used with clock that is a
constant divider of an existing clock. Taking a divider value would
avoid having to create a new clock that may not be similar to any
other clock in the device.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] ARM: move timer-sp.c from versatile to common
2010-09-30 22:49 [PATCH 1/3] ARM: move timer-sp.c from versatile to common Rob Herring
2010-09-30 22:49 ` [PATCH 3/3] ARM: twd_smp: add clock api support Rob Herring
@ 2010-10-01 2:39 ` Jean-Christophe PLAGNIOL-VILLARD
2010-10-01 3:23 ` Rob Herring
2010-10-01 8:24 ` Russell King - ARM Linux
2 siblings, 1 reply; 14+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2010-10-01 2:39 UTC (permalink / raw)
To: linux-arm-kernel
On 17:49 Thu 30 Sep , Rob Herring wrote:
> From: Rob Herring <rob.herring@smooth-stone.com>
>
> The timer-sp h/w used on versatile platforms can also be used for other
> platforms, so move it to a common location.
>
> Rename timer names for clocksource and clockevent to generic name.
> Which timer instance is which is platform dependent.
>
How about move it to drivers/clocksource ?
Best Regards,
J.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] ARM: twd_smp: add clock api support
2010-10-01 2:30 ` Colin Cross
@ 2010-10-01 3:14 ` Rob Herring
2010-10-01 3:27 ` Colin Cross
0 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2010-10-01 3:14 UTC (permalink / raw)
To: linux-arm-kernel
On 09/30/2010 09:30 PM, Colin Cross wrote:
> On Thu, Sep 30, 2010 at 7:03 PM, Rob Herring<robherring2@gmail.com> wrote:
>> On 09/30/2010 07:49 PM, Colin Cross wrote:
>>>
>>> On Thu, Sep 30, 2010 at 3:49 PM, Rob Herring<robherring2@gmail.com>
>>> wrote:
>>>>
>>>> From: Rob Herring<rob.herring@smooth-stone.com>
>>>>
>>>> The private timer freq is currently dynamically detected
>>>> using jiffies count to determine the rate. This method adds
>>>> a delay to boot-up, so use the clock api instead to get the
>>>> clock rate.
>>>>
>>>> Signed-off-by: Rob Herring<rob.herring@smooth-stone.com>
>>>> ---
>>>> arch/arm/include/asm/smp_twd.h | 2 ++
>>>> arch/arm/kernel/smp_twd.c | 7 +++++++
>>>> 2 files changed, 9 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/arch/arm/include/asm/smp_twd.h
>>>> b/arch/arm/include/asm/smp_twd.h
>>>> index 634f357..bafad52 100644
>>>> --- a/arch/arm/include/asm/smp_twd.h
>>>> +++ b/arch/arm/include/asm/smp_twd.h
>>>> @@ -19,11 +19,13 @@
>>>> #define TWD_TIMER_CONTROL_IT_ENABLE (1<< 2)
>>>>
>>>> struct clock_event_device;
>>>> +struct clk;
>>>>
>>>> extern void __iomem *twd_base;
>>>>
>>>> void twd_timer_stop(void);
>>>> int twd_timer_ack(void);
>>>> void twd_timer_setup(struct clock_event_device *);
>>>> +void twd_timer_init(void __iomem *base, struct clk *clk);
>>>>
>>>> #endif
>>>> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
>>>> index 35882fb..1a3c959 100644
>>>> --- a/arch/arm/kernel/smp_twd.c
>>>> +++ b/arch/arm/kernel/smp_twd.c
>>>> @@ -17,6 +17,7 @@
>>>> #include<linux/clockchips.h>
>>>> #include<linux/irq.h>
>>>> #include<linux/io.h>
>>>> +#include<linux/clk.h>
>>>>
>>>> #include<asm/smp_twd.h>
>>>> #include<asm/hardware/gic.h>
>>>> @@ -151,6 +152,12 @@ void __cpuinit twd_timer_setup(struct
>>>> clock_event_device *clk)
>>>> clockevents_register_device(clk);
>>>> }
>>>>
>>>> +void __init twd_timer_init(void __iomem *base, struct clk *clk)
>>>> +{
>>>> + twd_base = base;
>>>> + twd_timer_rate = clk_get_rate(clk);
>>>> +}
>>>> +
>>>> #ifdef CONFIG_HOTPLUG_CPU
>>>> /*
>>>> * take a local timer down
>>>
>>> The local timers run off the PERIPHCLK in the CPU, which is specified
>>> as the CPU clock divided by an implementation defined integer>= 2.
>>> If you take the divider value as a parameter to twd_timer_init, the
>>> clock that is passed in can be the cpu's clock.
>>
>> That is an implementation detail of the A9. Future designs could be
>> completely asynchronous. Using the clock api works either way.
>
> True. That could still be handled by passing a divider of 1. For all
> current implementations, this api will be used with clock that is a
> constant divider of an existing clock. Taking a divider value would
> avoid having to create a new clock that may not be similar to any
> other clock in the device.
If I pass in a divider, what clock rate do I divide with it? If I only
have the divider, then I'm dependent on knowing CLK freq (the cpu freq
in A9 case). Ultimately I have to know the timer clock rate to setup a
clock_event device. I'm open to passing in the clock rate directly, but
having the clk ptr is more flexible. Other examples of timer init like
i.MX use the same arguments.
The point of this api is to avoid spending 5 jiffies on the primary cpu
to calculate the rate. That is a significant chunk of the kernel boot
time. Calling this function is entirely optional and the old way still
works.
Rob
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] ARM: move timer-sp.c from versatile to common
2010-10-01 2:39 ` [PATCH 1/3] ARM: move timer-sp.c from versatile to common Jean-Christophe PLAGNIOL-VILLARD
@ 2010-10-01 3:23 ` Rob Herring
0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2010-10-01 3:23 UTC (permalink / raw)
To: linux-arm-kernel
On 09/30/2010 09:39 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 17:49 Thu 30 Sep , Rob Herring wrote:
>> From: Rob Herring<rob.herring@smooth-stone.com>
>>
>> The timer-sp h/w used on versatile platforms can also be used for other
>> platforms, so move it to a common location.
>>
>> Rename timer names for clocksource and clockevent to generic name.
>> Which timer instance is which is platform dependent.
>>
> How about move it to drivers/clocksource ?
>
> Best Regards,
> J.
I'll defer to Russell. It's not exactly common practice to put timer
code there. The ones that are there don't appear to be the primary
system timers as they are initialized by initcalls which is not early
enough.
Rob
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] ARM: twd_smp: add clock api support
2010-10-01 3:14 ` Rob Herring
@ 2010-10-01 3:27 ` Colin Cross
2010-10-01 17:04 ` Rob Herring
0 siblings, 1 reply; 14+ messages in thread
From: Colin Cross @ 2010-10-01 3:27 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 30, 2010 at 8:14 PM, Rob Herring <robherring2@gmail.com> wrote:
> On 09/30/2010 09:30 PM, Colin Cross wrote:
>>
>> On Thu, Sep 30, 2010 at 7:03 PM, Rob Herring<robherring2@gmail.com>
>> ?wrote:
>>>
>>> On 09/30/2010 07:49 PM, Colin Cross wrote:
>>>>
>>>> On Thu, Sep 30, 2010 at 3:49 PM, Rob Herring<robherring2@gmail.com>
>>>> ?wrote:
>>>>>
>>>>> From: Rob Herring<rob.herring@smooth-stone.com>
>>>>>
>>>>> The private timer freq is currently dynamically detected
>>>>> using jiffies count to determine the rate. This method adds
>>>>> a delay to boot-up, so use the clock api instead to get the
>>>>> clock rate.
>>>>>
>>>>> Signed-off-by: Rob Herring<rob.herring@smooth-stone.com>
>>>>> ---
>>>>> ?arch/arm/include/asm/smp_twd.h | ? ?2 ++
>>>>> ?arch/arm/kernel/smp_twd.c ? ? ?| ? ?7 +++++++
>>>>> ?2 files changed, 9 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/include/asm/smp_twd.h
>>>>> b/arch/arm/include/asm/smp_twd.h
>>>>> index 634f357..bafad52 100644
>>>>> --- a/arch/arm/include/asm/smp_twd.h
>>>>> +++ b/arch/arm/include/asm/smp_twd.h
>>>>> @@ -19,11 +19,13 @@
>>>>> ?#define TWD_TIMER_CONTROL_IT_ENABLE ? ?(1<< ? ?2)
>>>>>
>>>>> ?struct clock_event_device;
>>>>> +struct clk;
>>>>>
>>>>> ?extern void __iomem *twd_base;
>>>>>
>>>>> ?void twd_timer_stop(void);
>>>>> ?int twd_timer_ack(void);
>>>>> ?void twd_timer_setup(struct clock_event_device *);
>>>>> +void twd_timer_init(void __iomem *base, struct clk *clk);
>>>>>
>>>>> ?#endif
>>>>> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
>>>>> index 35882fb..1a3c959 100644
>>>>> --- a/arch/arm/kernel/smp_twd.c
>>>>> +++ b/arch/arm/kernel/smp_twd.c
>>>>> @@ -17,6 +17,7 @@
>>>>> ?#include<linux/clockchips.h>
>>>>> ?#include<linux/irq.h>
>>>>> ?#include<linux/io.h>
>>>>> +#include<linux/clk.h>
>>>>>
>>>>> ?#include<asm/smp_twd.h>
>>>>> ?#include<asm/hardware/gic.h>
>>>>> @@ -151,6 +152,12 @@ void __cpuinit twd_timer_setup(struct
>>>>> clock_event_device *clk)
>>>>> ? ? ? ?clockevents_register_device(clk);
>>>>> ?}
>>>>>
>>>>> +void __init twd_timer_init(void __iomem *base, struct clk *clk)
>>>>> +{
>>>>> + ? ? ? twd_base = base;
>>>>> + ? ? ? twd_timer_rate = clk_get_rate(clk);
>>>>> +}
>>>>> +
>>>>> ?#ifdef CONFIG_HOTPLUG_CPU
>>>>> ?/*
>>>>> ?* take a local timer down
>>>>
>>>> The local timers run off the PERIPHCLK in the CPU, which is specified
>>>> as the CPU clock divided by an implementation defined integer>= 2.
>>>> If you take the divider value as a parameter to twd_timer_init, the
>>>> clock that is passed in can be the cpu's clock.
>>>
>>> That is an implementation detail of the A9. Future designs could be
>>> completely asynchronous. Using the clock api works either way.
>>
>> True. ?That could still be handled by passing a divider of 1. ?For all
>> current implementations, this api will be used with clock that is a
>> constant divider of an existing clock. ?Taking a divider value would
>> avoid having to create a new clock that may not be similar to any
>> other clock in the device.
>
> If I pass in a divider, what clock rate do I divide with it? If I only have
> the divider, then I'm dependent on knowing CLK freq (the cpu freq in A9
> case). Ultimately I have to know the timer clock rate to setup a clock_event
> device. I'm open to passing in the clock rate directly, but having the clk
> ptr is more flexible. Other examples of timer init like i.MX use the same
> arguments.
>
> The point of this api is to avoid spending 5 jiffies on the primary cpu to
> calculate the rate. That is a significant chunk of the kernel boot time.
> Calling this function is entirely optional and the old way still works.
I like the idea of passing in the clk pointer - it would simplify
problems I've been having with rescaling the localtimer when the cpu
frequency changes. However, it would reduce the amount of
machine-specific code necessary if I could pass in the
already-existing clock (cpu clock, for A9), as well as the value of
the divider that is present inside cpu (4 for Tegra2).
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] ARM: move timer-sp.c from versatile to common
2010-09-30 22:49 [PATCH 1/3] ARM: move timer-sp.c from versatile to common Rob Herring
2010-09-30 22:49 ` [PATCH 3/3] ARM: twd_smp: add clock api support Rob Herring
2010-10-01 2:39 ` [PATCH 1/3] ARM: move timer-sp.c from versatile to common Jean-Christophe PLAGNIOL-VILLARD
@ 2010-10-01 8:24 ` Russell King - ARM Linux
2 siblings, 0 replies; 14+ messages in thread
From: Russell King - ARM Linux @ 2010-10-01 8:24 UTC (permalink / raw)
To: linux-arm-kernel
What happened to 2/3 ?
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] ARM: twd_smp: add clock api support
2010-10-01 3:27 ` Colin Cross
@ 2010-10-01 17:04 ` Rob Herring
2010-10-01 18:22 ` Colin Cross
0 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2010-10-01 17:04 UTC (permalink / raw)
To: linux-arm-kernel
On 09/30/2010 10:27 PM, Colin Cross wrote:
> On Thu, Sep 30, 2010 at 8:14 PM, Rob Herring<robherring2@gmail.com> wrote:
>> On 09/30/2010 09:30 PM, Colin Cross wrote:
>>>
>>> On Thu, Sep 30, 2010 at 7:03 PM, Rob Herring<robherring2@gmail.com>
>>> wrote:
>>>>
>>>> On 09/30/2010 07:49 PM, Colin Cross wrote:
>>>>>
>>>>> On Thu, Sep 30, 2010 at 3:49 PM, Rob Herring<robherring2@gmail.com>
>>>>> wrote:
>>>>>>
>>>>>> From: Rob Herring<rob.herring@smooth-stone.com>
>>>>>>
>>>>>> The private timer freq is currently dynamically detected
>>>>>> using jiffies count to determine the rate. This method adds
>>>>>> a delay to boot-up, so use the clock api instead to get the
>>>>>> clock rate.
>>>>>>
>>>>>> Signed-off-by: Rob Herring<rob.herring@smooth-stone.com>
>>>>>> ---
>>>>>> arch/arm/include/asm/smp_twd.h | 2 ++
>>>>>> arch/arm/kernel/smp_twd.c | 7 +++++++
>>>>>> 2 files changed, 9 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/include/asm/smp_twd.h
>>>>>> b/arch/arm/include/asm/smp_twd.h
>>>>>> index 634f357..bafad52 100644
>>>>>> --- a/arch/arm/include/asm/smp_twd.h
>>>>>> +++ b/arch/arm/include/asm/smp_twd.h
>>>>>> @@ -19,11 +19,13 @@
>>>>>> #define TWD_TIMER_CONTROL_IT_ENABLE (1<< 2)
>>>>>>
>>>>>> struct clock_event_device;
>>>>>> +struct clk;
>>>>>>
>>>>>> extern void __iomem *twd_base;
>>>>>>
>>>>>> void twd_timer_stop(void);
>>>>>> int twd_timer_ack(void);
>>>>>> void twd_timer_setup(struct clock_event_device *);
>>>>>> +void twd_timer_init(void __iomem *base, struct clk *clk);
>>>>>>
>>>>>> #endif
>>>>>> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
>>>>>> index 35882fb..1a3c959 100644
>>>>>> --- a/arch/arm/kernel/smp_twd.c
>>>>>> +++ b/arch/arm/kernel/smp_twd.c
>>>>>> @@ -17,6 +17,7 @@
>>>>>> #include<linux/clockchips.h>
>>>>>> #include<linux/irq.h>
>>>>>> #include<linux/io.h>
>>>>>> +#include<linux/clk.h>
>>>>>>
>>>>>> #include<asm/smp_twd.h>
>>>>>> #include<asm/hardware/gic.h>
>>>>>> @@ -151,6 +152,12 @@ void __cpuinit twd_timer_setup(struct
>>>>>> clock_event_device *clk)
>>>>>> clockevents_register_device(clk);
>>>>>> }
>>>>>>
>>>>>> +void __init twd_timer_init(void __iomem *base, struct clk *clk)
>>>>>> +{
>>>>>> + twd_base = base;
>>>>>> + twd_timer_rate = clk_get_rate(clk);
>>>>>> +}
>>>>>> +
>>>>>> #ifdef CONFIG_HOTPLUG_CPU
>>>>>> /*
>>>>>> * take a local timer down
>>>>>
>>>>> The local timers run off the PERIPHCLK in the CPU, which is specified
>>>>> as the CPU clock divided by an implementation defined integer>= 2.
>>>>> If you take the divider value as a parameter to twd_timer_init, the
>>>>> clock that is passed in can be the cpu's clock.
>>>>
>>>> That is an implementation detail of the A9. Future designs could be
>>>> completely asynchronous. Using the clock api works either way.
>>>
>>> True. That could still be handled by passing a divider of 1. For all
>>> current implementations, this api will be used with clock that is a
>>> constant divider of an existing clock. Taking a divider value would
>>> avoid having to create a new clock that may not be similar to any
>>> other clock in the device.
>>
>> If I pass in a divider, what clock rate do I divide with it? If I only have
>> the divider, then I'm dependent on knowing CLK freq (the cpu freq in A9
>> case). Ultimately I have to know the timer clock rate to setup a clock_event
>> device. I'm open to passing in the clock rate directly, but having the clk
>> ptr is more flexible. Other examples of timer init like i.MX use the same
>> arguments.
>>
>> The point of this api is to avoid spending 5 jiffies on the primary cpu to
>> calculate the rate. That is a significant chunk of the kernel boot time.
>> Calling this function is entirely optional and the old way still works.
>
> I like the idea of passing in the clk pointer - it would simplify
> problems I've been having with rescaling the localtimer when the cpu
> frequency changes. However, it would reduce the amount of
> machine-specific code necessary if I could pass in the
> already-existing clock (cpu clock, for A9), as well as the value of
> the divider that is present inside cpu (4 for Tegra2).
You are assuming all hardware would have a fixed divider. There is no
requirement that the CLK:PERIPHCLK ratio is fixed. Well designed clock
controller h/w would support CLK changing while maintaining PERIPHCLK
rate or at least support changing the ratio.
You will need to get the clock rate every set_next_event and somehow
sync cpu freq changes to when no timers are active on all cores. Or just
accept the timer inaccuracy for 1 time. Good luck with that... ;)
What is the problem of defining another clock in the platform? The
watchdogs if implemented also need that clock.
Rob
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] ARM: twd_smp: add clock api support
2010-10-01 17:04 ` Rob Herring
@ 2010-10-01 18:22 ` Colin Cross
2010-10-01 18:34 ` Russell King - ARM Linux
0 siblings, 1 reply; 14+ messages in thread
From: Colin Cross @ 2010-10-01 18:22 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 1, 2010 at 10:04 AM, Rob Herring <robherring2@gmail.com> wrote:
> On 09/30/2010 10:27 PM, Colin Cross wrote:
>>
>> On Thu, Sep 30, 2010 at 8:14 PM, Rob Herring<robherring2@gmail.com>
>> ?wrote:
>>>
>>> On 09/30/2010 09:30 PM, Colin Cross wrote:
>>>>
>>>> On Thu, Sep 30, 2010 at 7:03 PM, Rob Herring<robherring2@gmail.com>
>>>> ?wrote:
>>>>>
>>>>> On 09/30/2010 07:49 PM, Colin Cross wrote:
snip
>>>>>> The local timers run off the PERIPHCLK in the CPU, which is specified
>>>>>> as the CPU clock divided by an implementation defined integer>= 2.
>>>>>> If you take the divider value as a parameter to twd_timer_init, the
>>>>>> clock that is passed in can be the cpu's clock.
>>>>>
>>>>> That is an implementation detail of the A9. Future designs could be
>>>>> completely asynchronous. Using the clock api works either way.
>>>>
>>>> True. ?That could still be handled by passing a divider of 1. ?For all
>>>> current implementations, this api will be used with clock that is a
>>>> constant divider of an existing clock. ?Taking a divider value would
>>>> avoid having to create a new clock that may not be similar to any
>>>> other clock in the device.
>>>
>>> If I pass in a divider, what clock rate do I divide with it? If I only
>>> have
>>> the divider, then I'm dependent on knowing CLK freq (the cpu freq in A9
>>> case). Ultimately I have to know the timer clock rate to setup a
>>> clock_event
>>> device. I'm open to passing in the clock rate directly, but having the
>>> clk
>>> ptr is more flexible. Other examples of timer init like i.MX use the same
>>> arguments.
>>>
>>> The point of this api is to avoid spending 5 jiffies on the primary cpu
>>> to
>>> calculate the rate. That is a significant chunk of the kernel boot time.
>>> Calling this function is entirely optional and the old way still works.
>>
>> I like the idea of passing in the clk pointer - it would simplify
>> problems I've been having with rescaling the localtimer when the cpu
>> frequency changes. ?However, it would reduce the amount of
>> machine-specific code necessary if I could pass in the
>> already-existing clock (cpu clock, for A9), as well as the value of
>> the divider that is present inside cpu (4 for Tegra2).
>
> You are assuming all hardware would have a fixed divider. There is no
> requirement that the CLK:PERIPHCLK ratio is fixed. Well designed clock
> controller h/w would support CLK changing while maintaining PERIPHCLK rate
> or at least support changing the ratio.
The ARM specification for Cortex A9 requires a fixed CLK:PERIPHCLK
ratio, which AFAICT covers at least 5 out of the 6 current users of
the twd driver.
> You will need to get the clock rate every set_next_event and somehow sync
> cpu freq changes to when no timers are active on all cores. Or just accept
> the timer inaccuracy for 1 time. Good luck with that... ;)
The timer APIs require that a timer take at least as long as requested
to fire. If you change the TWD prescaler before the CPU frequency
increases, or after the CPU frequency decreases, the timer interrupt
will not fire until a small time after the requested time.
> What is the problem of defining another clock in the platform? The watchdogs
> if implemented also need that clock.
It would be a convenient shortcut. On Tegra, we don't use the A9
watchdog, because it gets powered down in idle. Defining a new clock
is not a trivial task - on Tegra we have no fixed dividers like this,
so we would need a new clock type. An extra argument, which can be
set to 1 if your local timer block has no divider, would save ~20 LoC
on each existing platform, but either way would work.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] ARM: twd_smp: add clock api support
2010-10-01 18:22 ` Colin Cross
@ 2010-10-01 18:34 ` Russell King - ARM Linux
2010-10-01 18:47 ` Rob Herring
0 siblings, 1 reply; 14+ messages in thread
From: Russell King - ARM Linux @ 2010-10-01 18:34 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 01, 2010 at 11:22:31AM -0700, Colin Cross wrote:
> The timer APIs require that a timer take at least as long as requested
> to fire. If you change the TWD prescaler before the CPU frequency
> increases, or after the CPU frequency decreases, the timer interrupt
> will not fire until a small time after the requested time.
That maybe - and for TWD which is used for local timer purposes only,
that's fine. What wouldn't be fine is if your clocksource counter
changes frequency...
In any case, I'm not sure that passing in a struct clk pointer is
really the right way to go - the point of the clk API is to remove
such passing of static structures.
How about instead using clk_get_sys() to obtain a clock for the TWD?
Do we need separate connection IDs for each TWD, or are all TWDs in
a system always clocked together?
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] ARM: twd_smp: add clock api support
2010-10-01 18:34 ` Russell King - ARM Linux
@ 2010-10-01 18:47 ` Rob Herring
0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2010-10-01 18:47 UTC (permalink / raw)
To: linux-arm-kernel
On 10/01/2010 01:34 PM, Russell King - ARM Linux wrote:
> On Fri, Oct 01, 2010 at 11:22:31AM -0700, Colin Cross wrote:
>> The timer APIs require that a timer take at least as long as requested
>> to fire. If you change the TWD prescaler before the CPU frequency
>> increases, or after the CPU frequency decreases, the timer interrupt
>> will not fire until a small time after the requested time.
>
> That maybe - and for TWD which is used for local timer purposes only,
> that's fine. What wouldn't be fine is if your clocksource counter
> changes frequency...
>
> In any case, I'm not sure that passing in a struct clk pointer is
> really the right way to go - the point of the clk API is to remove
> such passing of static structures.
>
> How about instead using clk_get_sys() to obtain a clock for the TWD?
Okay. Opinions on the name: periphclk, arm_twd, twd?
>
> Do we need separate connection IDs for each TWD, or are all TWDs in
> a system always clocked together?
A9 is 1 clock for all. Probably is likely that they will always be
symmetric.
Rob
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2010-10-01 18:47 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-30 22:49 [PATCH 1/3] ARM: move timer-sp.c from versatile to common Rob Herring
2010-09-30 22:49 ` [PATCH 3/3] ARM: twd_smp: add clock api support Rob Herring
2010-10-01 0:49 ` Colin Cross
2010-10-01 2:03 ` Rob Herring
2010-10-01 2:30 ` Colin Cross
2010-10-01 3:14 ` Rob Herring
2010-10-01 3:27 ` Colin Cross
2010-10-01 17:04 ` Rob Herring
2010-10-01 18:22 ` Colin Cross
2010-10-01 18:34 ` Russell King - ARM Linux
2010-10-01 18:47 ` Rob Herring
2010-10-01 2:39 ` [PATCH 1/3] ARM: move timer-sp.c from versatile to common Jean-Christophe PLAGNIOL-VILLARD
2010-10-01 3:23 ` Rob Herring
2010-10-01 8:24 ` Russell King - ARM Linux
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).