From: Rob Herring <[email protected]>
This converts arm and arm64 to use CLKSRC_OF DT based initialization for
the arch timer. A new function arch_timer_arch_init is added to allow for
arch specific setup.
This has a side effect of enabling sched_clock on omap5 and exynos5. There
should not be any reason not to use the arch timers for sched_clock.
Signed-off-by: Rob Herring <[email protected]>
Cc: Russell King <[email protected]>
Cc: Kukjin Kim <[email protected]>
Cc: Tony Lindgren <[email protected]>
Cc: Simon Horman <[email protected]>
Cc: Magnus Damm <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
This is dependent on my CLKSRC_OF clean-up in arm-soc, my 64-bit sched_clock
support series, and Arnd's default machine descriptor patch (for default
clocksource_of_init call). This is only compile tested on arm.
The full series (including sp804 work) is available here:
git://sources.calxeda.com/kernel/linux.git arm-timers
Rob
arch/arm/include/asm/arch_timer.h | 13 +------------
arch/arm/kernel/arch_timer.c | 17 +++--------------
arch/arm/mach-exynos/mach-exynos5-dt.c | 1 -
arch/arm/mach-exynos/mct.c | 6 ------
arch/arm/mach-highbank/highbank.c | 5 +----
arch/arm/mach-omap2/timer.c | 5 +----
arch/arm/mach-shmobile/board-kzm9d.c | 1 -
arch/arm/mach-shmobile/include/mach/common.h | 1 -
arch/arm/mach-shmobile/setup-emev2.c | 1 -
arch/arm/mach-shmobile/setup-r8a7740.c | 1 -
arch/arm/mach-shmobile/setup-sh7372.c | 1 -
arch/arm/mach-shmobile/setup-sh73a0.c | 1 -
arch/arm/mach-shmobile/timer.c | 6 ------
arch/arm/mach-vexpress/v2m.c | 7 ++-----
arch/arm/mach-virt/virt.c | 9 ---------
arch/arm64/include/asm/arch_timer.h | 5 +++++
arch/arm64/kernel/time.c | 6 ++++--
drivers/clocksource/Kconfig | 1 +
drivers/clocksource/arm_arch_timer.c | 22 ++++++----------------
include/clocksource/arm_arch_timer.h | 6 ------
20 files changed, 24 insertions(+), 91 deletions(-)
diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index 7ade91d..7c1bfc0 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -10,8 +10,7 @@
#include <clocksource/arm_arch_timer.h>
#ifdef CONFIG_ARM_ARCH_TIMER
-int arch_timer_of_register(void);
-int arch_timer_sched_clock_init(void);
+int arch_timer_arch_init(void);
/*
* These register accessors are marked inline so the compiler can
@@ -110,16 +109,6 @@ static inline void __cpuinit arch_counter_set_user_access(void)
asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
}
-#else
-static inline int arch_timer_of_register(void)
-{
- return -ENXIO;
-}
-
-static inline int arch_timer_sched_clock_init(void)
-{
- return -ENXIO;
-}
#endif
#endif
diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index d813ae6..d0b59bc 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -32,24 +32,13 @@ static void __init arch_timer_delay_timer_register(void)
register_current_timer_delay(&arch_delay_timer);
}
-int __init arch_timer_of_register(void)
-{
- int ret;
-
- ret = arch_timer_init();
- if (ret)
- return ret;
-
- arch_timer_delay_timer_register();
-
- return 0;
-}
-
-int __init arch_timer_sched_clock_init(void)
+int __init arch_timer_arch_init(void)
{
if (arch_timer_get_rate() == 0)
return -ENXIO;
+ arch_timer_delay_timer_register();
+
setup_sched_clock_64(arch_timer_read_counter, arch_timer_get_rate());
return 0;
}
diff --git a/arch/arm/mach-exynos/mach-exynos5-dt.c b/arch/arm/mach-exynos/mach-exynos5-dt.c
index acaeb14..4d97b43 100644
--- a/arch/arm/mach-exynos/mach-exynos5-dt.c
+++ b/arch/arm/mach-exynos/mach-exynos5-dt.c
@@ -216,7 +216,6 @@ DT_MACHINE_START(EXYNOS5_DT, "SAMSUNG EXYNOS5 (Flattened Device Tree)")
.map_io = exynos5_dt_map_io,
.init_machine = exynos5_dt_machine_init,
.init_late = exynos_init_late,
- .init_time = exynos4_timer_init,
.dt_compat = exynos5_dt_compat,
.restart = exynos5_restart,
.reserve = exynos5_reserve,
diff --git a/arch/arm/mach-exynos/mct.c b/arch/arm/mach-exynos/mct.c
index c9d6650..04aff6a 100644
--- a/arch/arm/mach-exynos/mct.c
+++ b/arch/arm/mach-exynos/mct.c
@@ -21,7 +21,6 @@
#include <linux/percpu.h>
#include <linux/of.h>
-#include <asm/arch_timer.h>
#include <asm/localtimer.h>
#include <plat/cpu.h>
@@ -469,11 +468,6 @@ static void __init exynos4_timer_resources(void)
void __init exynos4_timer_init(void)
{
- if (soc_is_exynos5440()) {
- arch_timer_of_register();
- return;
- }
-
if ((soc_is_exynos4210()) || (soc_is_exynos5250()))
mct_int_type = MCT_INT_SPI;
else
diff --git a/arch/arm/mach-highbank/highbank.c b/arch/arm/mach-highbank/highbank.c
index 76c1170..758150e 100644
--- a/arch/arm/mach-highbank/highbank.c
+++ b/arch/arm/mach-highbank/highbank.c
@@ -15,6 +15,7 @@
*/
#include <linux/clk.h>
#include <linux/clkdev.h>
+#include <linux/clocksource.h>
#include <linux/dma-mapping.h>
#include <linux/io.h>
#include <linux/irq.h>
@@ -28,7 +29,6 @@
#include <linux/amba/bus.h>
#include <linux/clk-provider.h>
-#include <asm/arch_timer.h>
#include <asm/cacheflush.h>
#include <asm/cputype.h>
#include <asm/smp_plat.h>
@@ -118,9 +118,6 @@ static void __init highbank_timer_init(void)
sp804_clocksource_and_sched_clock_init(timer_base + 0x20, "timer1");
sp804_clockevents_init(timer_base, irq, "timer0");
- arch_timer_of_register();
- arch_timer_sched_clock_init();
-
clocksource_of_init();
}
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 4fd8025..7dd6453 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -46,7 +46,6 @@
#include <asm/smp_twd.h>
#include <asm/sched_clock.h>
-#include <asm/arch_timer.h>
#include "omap_hwmod.h"
#include "omap_device.h"
#include <plat/counter-32k.h>
@@ -624,9 +623,7 @@ void __init omap5_realtime_timer_init(void)
omap5_sync32k_timer_init();
realtime_counter_init();
- err = arch_timer_of_register();
- if (err)
- pr_err("%s: arch_timer_register failed %d\n", __func__, err);
+ clocksource_of_init();
}
#endif /* CONFIG_SOC_OMAP5 */
diff --git a/arch/arm/mach-shmobile/board-kzm9d.c b/arch/arm/mach-shmobile/board-kzm9d.c
index c254782..c016ccd 100644
--- a/arch/arm/mach-shmobile/board-kzm9d.c
+++ b/arch/arm/mach-shmobile/board-kzm9d.c
@@ -90,6 +90,5 @@ DT_MACHINE_START(KZM9D_DT, "kzm9d")
.init_irq = emev2_init_irq,
.init_machine = kzm9d_add_standard_devices,
.init_late = shmobile_init_late,
- .init_time = shmobile_timer_init,
.dt_compat = kzm9d_boards_compat_dt,
MACHINE_END
diff --git a/arch/arm/mach-shmobile/include/mach/common.h b/arch/arm/mach-shmobile/include/mach/common.h
index e48606d..5efc0f0 100644
--- a/arch/arm/mach-shmobile/include/mach/common.h
+++ b/arch/arm/mach-shmobile/include/mach/common.h
@@ -2,7 +2,6 @@
#define __ARCH_MACH_COMMON_H
extern void shmobile_earlytimer_init(void);
-extern void shmobile_timer_init(void);
extern void shmobile_setup_delay(unsigned int max_cpu_core_mhz,
unsigned int mult, unsigned int div);
struct twd_local_timer;
diff --git a/arch/arm/mach-shmobile/setup-emev2.c b/arch/arm/mach-shmobile/setup-emev2.c
index 47662a5..4e38a66 100644
--- a/arch/arm/mach-shmobile/setup-emev2.c
+++ b/arch/arm/mach-shmobile/setup-emev2.c
@@ -456,7 +456,6 @@ DT_MACHINE_START(EMEV2_DT, "Generic Emma Mobile EV2 (Flattened Device Tree)")
.nr_irqs = NR_IRQS_LEGACY,
.init_irq = irqchip_init,
.init_machine = emev2_add_standard_devices_dt,
- .init_time = shmobile_timer_init,
.dt_compat = emev2_boards_compat_dt,
MACHINE_END
diff --git a/arch/arm/mach-shmobile/setup-r8a7740.c b/arch/arm/mach-shmobile/setup-r8a7740.c
index 8b85d4d..104b474 100644
--- a/arch/arm/mach-shmobile/setup-r8a7740.c
+++ b/arch/arm/mach-shmobile/setup-r8a7740.c
@@ -906,7 +906,6 @@ DT_MACHINE_START(R8A7740_DT, "Generic R8A7740 (Flattened Device Tree)")
.init_irq = r8a7740_init_irq,
.handle_irq = shmobile_handle_irq_intc,
.init_machine = r8a7740_add_standard_devices_dt,
- .init_time = shmobile_timer_init,
.dt_compat = r8a7740_boards_compat_dt,
MACHINE_END
diff --git a/arch/arm/mach-shmobile/setup-sh7372.c b/arch/arm/mach-shmobile/setup-sh7372.c
index 59c7146..5502d62 100644
--- a/arch/arm/mach-shmobile/setup-sh7372.c
+++ b/arch/arm/mach-shmobile/setup-sh7372.c
@@ -1175,7 +1175,6 @@ DT_MACHINE_START(SH7372_DT, "Generic SH7372 (Flattened Device Tree)")
.init_irq = sh7372_init_irq,
.handle_irq = shmobile_handle_irq_intc,
.init_machine = sh7372_add_standard_devices_dt,
- .init_time = shmobile_timer_init,
.dt_compat = sh7372_boards_compat_dt,
MACHINE_END
diff --git a/arch/arm/mach-shmobile/setup-sh73a0.c b/arch/arm/mach-shmobile/setup-sh73a0.c
index bdab575..ea66316 100644
--- a/arch/arm/mach-shmobile/setup-sh73a0.c
+++ b/arch/arm/mach-shmobile/setup-sh73a0.c
@@ -923,7 +923,6 @@ DT_MACHINE_START(SH73A0_DT, "Generic SH73A0 (Flattened Device Tree)")
.nr_irqs = NR_IRQS_LEGACY,
.init_irq = sh73a0_init_irq_dt,
.init_machine = sh73a0_add_standard_devices_dt,
- .init_time = shmobile_timer_init,
.dt_compat = sh73a0_boards_compat_dt,
MACHINE_END
#endif /* CONFIG_USE_OF */
diff --git a/arch/arm/mach-shmobile/timer.c b/arch/arm/mach-shmobile/timer.c
index 3d16d4d..add2f15 100644
--- a/arch/arm/mach-shmobile/timer.c
+++ b/arch/arm/mach-shmobile/timer.c
@@ -60,9 +60,3 @@ void __init shmobile_earlytimer_init(void)
{
late_time_init = shmobile_late_time_init;
}
-
-void __init shmobile_timer_init(void)
-{
- arch_timer_of_register();
- arch_timer_sched_clock_init();
-}
diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
index d0ad789..6215717 100644
--- a/arch/arm/mach-vexpress/v2m.c
+++ b/arch/arm/mach-vexpress/v2m.c
@@ -1,6 +1,7 @@
/*
* Versatile Express V2M Motherboard Support
*/
+#include <linux/clocksource.h>
#include <linux/device.h>
#include <linux/amba/bus.h>
#include <linux/amba/mmci.h>
@@ -23,7 +24,6 @@
#include <linux/regulator/machine.h>
#include <linux/vexpress.h>
-#include <asm/arch_timer.h>
#include <asm/mach-types.h>
#include <asm/sizes.h>
#include <asm/mach/arch.h>
@@ -446,10 +446,7 @@ static void __init v2m_dt_timer_init(void)
irq_of_parse_and_map(node, 0));
}
- arch_timer_of_register();
-
- if (arch_timer_sched_clock_init() != 0)
- versatile_sched_clock_init(vexpress_get_24mhz_clock_base(),
+ versatile_sched_clock_init(vexpress_get_24mhz_clock_base(),
24000000);
}
diff --git a/arch/arm/mach-virt/virt.c b/arch/arm/mach-virt/virt.c
index 31666f6..adc0945 100644
--- a/arch/arm/mach-virt/virt.c
+++ b/arch/arm/mach-virt/virt.c
@@ -23,21 +23,13 @@
#include <linux/of_platform.h>
#include <linux/smp.h>
-#include <asm/arch_timer.h>
#include <asm/mach/arch.h>
-#include <asm/mach/time.h>
static void __init virt_init(void)
{
of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
}
-static void __init virt_timer_init(void)
-{
- WARN_ON(arch_timer_of_register() != 0);
- WARN_ON(arch_timer_sched_clock_init() != 0);
-}
-
static const char *virt_dt_match[] = {
"linux,dummy-virt",
NULL
@@ -47,7 +39,6 @@ extern struct smp_operations virt_smp_ops;
DT_MACHINE_START(VIRT, "Dummy Virtual Machine")
.init_irq = irqchip_init,
- .init_time = virt_timer_init,
.init_machine = virt_init,
.smp = smp_ops(virt_smp_ops),
.dt_compat = virt_dt_match,
diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 91e2a6a..bf6ab242 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -130,4 +130,9 @@ static inline u64 arch_counter_get_cntvct(void)
return cval;
}
+static inline int arch_timer_arch_init(void)
+{
+ return 0;
+}
+
#endif
diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c
index b0ef18d..a551f88 100644
--- a/arch/arm64/kernel/time.c
+++ b/arch/arm64/kernel/time.c
@@ -32,6 +32,7 @@
#include <linux/timer.h>
#include <linux/irq.h>
#include <linux/delay.h>
+#include <linux/clocksource.h>
#include <clocksource/arm_arch_timer.h>
@@ -77,10 +78,11 @@ void __init time_init(void)
{
u32 arch_timer_rate;
- if (arch_timer_init())
- panic("Unable to initialise architected timer.\n");
+ clocksource_of_init();
arch_timer_rate = arch_timer_get_rate();
+ if (!arch_timer_rate)
+ panic("Unable to initialise architected timer.\n");
/* Cache the sched_clock multiplier to save a divide in the hot path. */
sched_clock_mult = NSEC_PER_SEC / arch_timer_rate;
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index e507ab7..d98e7e1 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -62,6 +62,7 @@ config CLKSRC_DBX500_PRCMU_SCHED_CLOCK
config ARM_ARCH_TIMER
bool
+ select CLKSRC_OF if OF
config CLKSRC_METAG_GENERIC
def_bool y if METAG
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index d7ad425..afb70aa 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -337,24 +337,11 @@ out:
return err;
}
-static const struct of_device_id arch_timer_of_match[] __initconst = {
- { .compatible = "arm,armv7-timer", },
- { .compatible = "arm,armv8-timer", },
- {},
-};
-
-int __init arch_timer_init(void)
+static void __init arch_timer_init(struct device_node *np)
{
- struct device_node *np;
u32 freq;
int i;
- np = of_find_matching_node(NULL, arch_timer_of_match);
- if (!np) {
- pr_err("arch_timer: can't find DT node\n");
- return -ENODEV;
- }
-
/* Try to determine the frequency from the device tree or CNTFRQ */
if (!of_property_read_u32(np, "clock-frequency", &freq))
arch_timer_rate = freq;
@@ -378,7 +365,7 @@ int __init arch_timer_init(void)
if (!arch_timer_ppi[PHYS_SECURE_PPI] ||
!arch_timer_ppi[PHYS_NONSECURE_PPI]) {
pr_warn("arch_timer: No interrupt available, giving up\n");
- return -EINVAL;
+ return;
}
}
@@ -387,5 +374,8 @@ int __init arch_timer_init(void)
else
arch_timer_read_counter = arch_counter_get_cntpct;
- return arch_timer_register();
+ arch_timer_register();
+ arch_timer_arch_init();
}
+CLOCKSOURCE_OF_DECLARE(armv7_arch_timer, "arm,armv7-timer", arch_timer_init);
+CLOCKSOURCE_OF_DECLARE(armv8_arch_timer, "arm,armv8-timer", arch_timer_init);
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index 2603267..e6c9c4c 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -31,18 +31,12 @@
#ifdef CONFIG_ARM_ARCH_TIMER
-extern int arch_timer_init(void);
extern u32 arch_timer_get_rate(void);
extern u64 (*arch_timer_read_counter)(void);
extern struct timecounter *arch_timer_get_timecounter(void);
#else
-static inline int arch_timer_init(void)
-{
- return -ENXIO;
-}
-
static inline u32 arch_timer_get_rate(void)
{
return 0;
--
1.7.10.4
Hi Rob,
(adding Marc to Cc as he may have comments).
On Wed, Mar 20, 2013 at 10:34:35PM +0000, Rob Herring wrote:
> From: Rob Herring <[email protected]>
>
> This converts arm and arm64 to use CLKSRC_OF DT based initialization for
> the arch timer. A new function arch_timer_arch_init is added to allow for
> arch specific setup.
>
> This has a side effect of enabling sched_clock on omap5 and exynos5. There
> should not be any reason not to use the arch timers for sched_clock.
Nice! I was just about to post a (slightly updated) version of Thomas Abraham's
arch_timer clocksource_of_init patch, but this seems much more comprehensive.
I have some other arch_timer patches which may clash, but they could be rebased
atop of this.
>
> Signed-off-by: Rob Herring <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Kukjin Kim <[email protected]>
> Cc: Tony Lindgren <[email protected]>
> Cc: Simon Horman <[email protected]>
> Cc: Magnus Damm <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: John Stultz <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> This is dependent on my CLKSRC_OF clean-up in arm-soc, my 64-bit sched_clock
> support series, and Arnd's default machine descriptor patch (for default
> clocksource_of_init call). This is only compile tested on arm.
>
> The full series (including sp804 work) is available here:
> git://sources.calxeda.com/kernel/linux.git arm-timers
>
> Rob
>
[...]
> diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
> index d0ad789..6215717 100644
> --- a/arch/arm/mach-vexpress/v2m.c
> +++ b/arch/arm/mach-vexpress/v2m.c
> @@ -1,6 +1,7 @@
> /*
> * Versatile Express V2M Motherboard Support
> */
> +#include <linux/clocksource.h>
> #include <linux/device.h>
> #include <linux/amba/bus.h>
> #include <linux/amba/mmci.h>
> @@ -23,7 +24,6 @@
> #include <linux/regulator/machine.h>
> #include <linux/vexpress.h>
>
> -#include <asm/arch_timer.h>
> #include <asm/mach-types.h>
> #include <asm/sizes.h>
> #include <asm/mach/arch.h>
> @@ -446,10 +446,7 @@ static void __init v2m_dt_timer_init(void)
> irq_of_parse_and_map(node, 0));
> }
>
> - arch_timer_of_register();
> -
> - if (arch_timer_sched_clock_init() != 0)
> - versatile_sched_clock_init(vexpress_get_24mhz_clock_base(),
> + versatile_sched_clock_init(vexpress_get_24mhz_clock_base(),
> 24000000);
> }
>
On TC2 this series leads to using the vexpress 24MHz clock as the sched clock
in preference to the architected timer:
Architected local timer running at 24.00MHz (virt).
Switching to timer-based delay loop
Registered arch_counter_get_cntvct+0x0/0x14 as sched_clock source
sched_clock: 32 bits at 24MHz, resolution 41ns, wraps every 178956ms
Registered versatile_read_sched_clock+0x0/0x28 as sched_clock source
As they both have the same frequency, neither overrides the other, and
whichever gets registered last is used as the sched_clock. As accesses to the
architected timer are going to have a much lower overhead, this isn't very nice
(and it could be better to use it even if it had a lower frequency).
We could move the versatile_sched_clock_init call before the
clocksource_of_init, but that doesn't feel like an ideal solution. We may have
similar problems elsewhere.
[...]
> diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c
> index b0ef18d..a551f88 100644
> --- a/arch/arm64/kernel/time.c
> +++ b/arch/arm64/kernel/time.c
> @@ -32,6 +32,7 @@
> #include <linux/timer.h>
> #include <linux/irq.h>
> #include <linux/delay.h>
> +#include <linux/clocksource.h>
>
> #include <clocksource/arm_arch_timer.h>
>
> @@ -77,10 +78,11 @@ void __init time_init(void)
> {
> u32 arch_timer_rate;
>
> - if (arch_timer_init())
> - panic("Unable to initialise architected timer.\n");
> + clocksource_of_init();
>
> arch_timer_rate = arch_timer_get_rate();
> + if (!arch_timer_rate)
> + panic("Unable to initialise architected timer.\n");
>
> /* Cache the sched_clock multiplier to save a divide in the hot path. */
> sched_clock_mult = NSEC_PER_SEC / arch_timer_rate;
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index e507ab7..d98e7e1 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -62,6 +62,7 @@ config CLKSRC_DBX500_PRCMU_SCHED_CLOCK
>
> config ARM_ARCH_TIMER
> bool
> + select CLKSRC_OF if OF
>
> config CLKSRC_METAG_GENERIC
> def_bool y if METAG
[...]
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index d7ad425..afb70aa 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -337,24 +337,11 @@ out:
> return err;
> }
>
> -static const struct of_device_id arch_timer_of_match[] __initconst = {
> - { .compatible = "arm,armv7-timer", },
> - { .compatible = "arm,armv8-timer", },
> - {},
> -};
> -
> -int __init arch_timer_init(void)
> +static void __init arch_timer_init(struct device_node *np)
> {
> - struct device_node *np;
> u32 freq;
> int i;
>
If we the following here:
if (arch_timer_get_rate()) {
pr_warn("arch_timer: multiple nodes in dt, skipping\n");
return;
}
We may save ourselves a whole world of pain with dts which (erroneously) have
multiple timer nodes (though these are now disappearing). Otherwise we could
have a memory leak and multiple instances of the cpu0 timer registered, which
could lead to all sorts of weirdness. The existing code side-steps this issue
by only grabbing the first node, so this would keep things consistent.
> - np = of_find_matching_node(NULL, arch_timer_of_match);
> - if (!np) {
> - pr_err("arch_timer: can't find DT node\n");
> - return -ENODEV;
> - }
> -
> /* Try to determine the frequency from the device tree or CNTFRQ */
> if (!of_property_read_u32(np, "clock-frequency", &freq))
> arch_timer_rate = freq;
[...]
Thanks,
Mark.
On 21/03/13 11:06, Mark Rutland wrote:
> Hi Rob,
>
> (adding Marc to Cc as he may have comments).
>
> On Wed, Mar 20, 2013 at 10:34:35PM +0000, Rob Herring wrote:
>> From: Rob Herring <[email protected]>
>>
>> This converts arm and arm64 to use CLKSRC_OF DT based initialization for
>> the arch timer. A new function arch_timer_arch_init is added to allow for
>> arch specific setup.
>>
>> This has a side effect of enabling sched_clock on omap5 and exynos5. There
>> should not be any reason not to use the arch timers for sched_clock.
>
> Nice! I was just about to post a (slightly updated) version of Thomas Abraham's
> arch_timer clocksource_of_init patch, but this seems much more comprehensive.
>
> I have some other arch_timer patches which may clash, but they could be rebased
> atop of this.
>
>>
>> Signed-off-by: Rob Herring <[email protected]>
>> Cc: Russell King <[email protected]>
>> Cc: Kukjin Kim <[email protected]>
>> Cc: Tony Lindgren <[email protected]>
>> Cc: Simon Horman <[email protected]>
>> Cc: Magnus Damm <[email protected]>
>> Cc: Catalin Marinas <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Cc: John Stultz <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> ---
>> This is dependent on my CLKSRC_OF clean-up in arm-soc, my 64-bit sched_clock
>> support series, and Arnd's default machine descriptor patch (for default
>> clocksource_of_init call). This is only compile tested on arm.
>>
>> The full series (including sp804 work) is available here:
>> git://sources.calxeda.com/kernel/linux.git arm-timers
>>
>> Rob
>>
>
> [...]
>
>> diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
>> index d0ad789..6215717 100644
>> --- a/arch/arm/mach-vexpress/v2m.c
>> +++ b/arch/arm/mach-vexpress/v2m.c
>> @@ -1,6 +1,7 @@
>> /*
>> * Versatile Express V2M Motherboard Support
>> */
>> +#include <linux/clocksource.h>
>> #include <linux/device.h>
>> #include <linux/amba/bus.h>
>> #include <linux/amba/mmci.h>
>> @@ -23,7 +24,6 @@
>> #include <linux/regulator/machine.h>
>> #include <linux/vexpress.h>
>>
>> -#include <asm/arch_timer.h>
>> #include <asm/mach-types.h>
>> #include <asm/sizes.h>
>> #include <asm/mach/arch.h>
>> @@ -446,10 +446,7 @@ static void __init v2m_dt_timer_init(void)
>> irq_of_parse_and_map(node, 0));
>> }
>>
>> - arch_timer_of_register();
>> -
>> - if (arch_timer_sched_clock_init() != 0)
>> - versatile_sched_clock_init(vexpress_get_24mhz_clock_base(),
>> + versatile_sched_clock_init(vexpress_get_24mhz_clock_base(),
>> 24000000);
>> }
>>
>
> On TC2 this series leads to using the vexpress 24MHz clock as the sched clock
> in preference to the architected timer:
>
> Architected local timer running at 24.00MHz (virt).
> Switching to timer-based delay loop
> Registered arch_counter_get_cntvct+0x0/0x14 as sched_clock source
> sched_clock: 32 bits at 24MHz, resolution 41ns, wraps every 178956ms
> Registered versatile_read_sched_clock+0x0/0x28 as sched_clock source
>
> As they both have the same frequency, neither overrides the other, and
> whichever gets registered last is used as the sched_clock. As accesses to the
> architected timer are going to have a much lower overhead, this isn't very nice
> (and it could be better to use it even if it had a lower frequency).
Indeed. And if I look at it with my KVM hat on, it is even worse (the
guest will exit all the way to platform emulation in userspace on each
sched_clock read - a sure performance killer).
Of course, emulating a VE is not the best option, but that's all we have
so far when using QEMU as platform emulation.
M.
--
Jazz is not dead. It just smells funny...
On 03/21/2013 06:06 AM, Mark Rutland wrote:
> Hi Rob,
>
> (adding Marc to Cc as he may have comments).
>
> On Wed, Mar 20, 2013 at 10:34:35PM +0000, Rob Herring wrote:
>> From: Rob Herring <[email protected]>
>>
>> This converts arm and arm64 to use CLKSRC_OF DT based initialization for
>> the arch timer. A new function arch_timer_arch_init is added to allow for
>> arch specific setup.
>>
>> This has a side effect of enabling sched_clock on omap5 and exynos5. There
>> should not be any reason not to use the arch timers for sched_clock.
>
> Nice! I was just about to post a (slightly updated) version of Thomas Abraham's
> arch_timer clocksource_of_init patch, but this seems much more comprehensive.
>
> I have some other arch_timer patches which may clash, but they could be rebased
> atop of this.
[snip]
>> @@ -446,10 +446,7 @@ static void __init v2m_dt_timer_init(void)
>> irq_of_parse_and_map(node, 0));
>> }
>>
>> - arch_timer_of_register();
>> -
>> - if (arch_timer_sched_clock_init() != 0)
>> - versatile_sched_clock_init(vexpress_get_24mhz_clock_base(),
>> + versatile_sched_clock_init(vexpress_get_24mhz_clock_base(),
>> 24000000);
>> }
>>
>
> On TC2 this series leads to using the vexpress 24MHz clock as the sched clock
> in preference to the architected timer:
>
> Architected local timer running at 24.00MHz (virt).
> Switching to timer-based delay loop
> Registered arch_counter_get_cntvct+0x0/0x14 as sched_clock source
> sched_clock: 32 bits at 24MHz, resolution 41ns, wraps every 178956ms
> Registered versatile_read_sched_clock+0x0/0x28 as sched_clock source
>
> As they both have the same frequency, neither overrides the other, and
> whichever gets registered last is used as the sched_clock. As accesses to the
> architected timer are going to have a much lower overhead, this isn't very nice
> (and it could be better to use it even if it had a lower frequency).
>
> We could move the versatile_sched_clock_init call before the
> clocksource_of_init, but that doesn't feel like an ideal solution. We may have
> similar problems elsewhere.
The intention was that a 64-bit counter is preferred. This should fix
that. It would be nice if we could describe access overhead to make a decision.
For now, I think 32 vs. 64 bit is sufficient.
diff --git a/arch/arm/kernel/sched_clock.c b/arch/arm/kernel/sched_clock.c
index 1708357..aa18e45 100644
--- a/arch/arm/kernel/sched_clock.c
+++ b/arch/arm/kernel/sched_clock.c
@@ -115,7 +115,7 @@ void __init setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate)
u64 res, wrap;
char r_unit;
- if (cd.rate > rate)
+ if (cd.rate > rate || read_sched_clock_64)
return;
BUG_ON(bits > 32);
@@ -168,7 +168,7 @@ void __init setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate)
void __init setup_sched_clock_64(u64 (*read)(void), unsigned long rate)
{
- if (cd.rate > rate)
+ if (read_sched_clock_64 && (cd.rate > rate))
return;
WARN_ON(!irqs_disabled());
>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
>> index d7ad425..afb70aa 100644
>> --- a/drivers/clocksource/arm_arch_timer.c
>> +++ b/drivers/clocksource/arm_arch_timer.c
>> @@ -337,24 +337,11 @@ out:
>> return err;
>> }
>>
>> -static const struct of_device_id arch_timer_of_match[] __initconst = {
>> - { .compatible = "arm,armv7-timer", },
>> - { .compatible = "arm,armv8-timer", },
>> - {},
>> -};
>> -
>> -int __init arch_timer_init(void)
>> +static void __init arch_timer_init(struct device_node *np)
>> {
>> - struct device_node *np;
>> u32 freq;
>> int i;
>>
>
> If we the following here:
>
> if (arch_timer_get_rate()) {
> pr_warn("arch_timer: multiple nodes in dt, skipping\n");
> return;
> }
>
> We may save ourselves a whole world of pain with dts which (erroneously) have
> multiple timer nodes (though these are now disappearing). Otherwise we could
> have a memory leak and multiple instances of the cpu0 timer registered, which
> could lead to all sorts of weirdness. The existing code side-steps this issue
> by only grabbing the first node, so this would keep things consistent.
>
Okay, I'll add.
Rob
On Thu, Mar 21, 2013 at 11:06:47AM +0000, Mark Rutland wrote:
> On TC2 this series leads to using the vexpress 24MHz clock as the sched clock
> in preference to the architected timer:
>
> Architected local timer running at 24.00MHz (virt).
> Switching to timer-based delay loop
> Registered arch_counter_get_cntvct+0x0/0x14 as sched_clock source
> sched_clock: 32 bits at 24MHz, resolution 41ns, wraps every 178956ms
> Registered versatile_read_sched_clock+0x0/0x28 as sched_clock source
>
> As they both have the same frequency, neither overrides the other, and
> whichever gets registered last is used as the sched_clock. As accesses
> to the architected timer are going to have a much lower overhead, this
> isn't very nice (and it could be better to use it even if it had a lower
> frequency).
I'll remind people that sched_clock() is supposed to be functional at
the point in the boot sequence where the call to sched_init() is called.
That is after setup_arch() and *before* time_init() is called.
On 03/25/2013 12:26 PM, Russell King - ARM Linux wrote:
> On Thu, Mar 21, 2013 at 11:06:47AM +0000, Mark Rutland wrote:
>> On TC2 this series leads to using the vexpress 24MHz clock as the sched clock
>> in preference to the architected timer:
>>
>> Architected local timer running at 24.00MHz (virt).
>> Switching to timer-based delay loop
>> Registered arch_counter_get_cntvct+0x0/0x14 as sched_clock source
>> sched_clock: 32 bits at 24MHz, resolution 41ns, wraps every 178956ms
>> Registered versatile_read_sched_clock+0x0/0x28 as sched_clock source
>>
>> As they both have the same frequency, neither overrides the other, and
>> whichever gets registered last is used as the sched_clock. As accesses
>> to the architected timer are going to have a much lower overhead, this
>> isn't very nice (and it could be better to use it even if it had a lower
>> frequency).
>
> I'll remind people that sched_clock() is supposed to be functional at
> the point in the boot sequence where the call to sched_init() is called.
> That is after setup_arch() and *before* time_init() is called.
I count integrator-cp, realview, versatile and non-DT VExpress that do
this (not surprisingly) and 25 platforms or timer implementations plus
arm64 that do sched_clock setup in time_init. What's broken by not
moving these earlier?
We could probably fix arch timers relatively easily, but supporting the
numerous memory-mapped timers will be harder. Most timers are also
dependent on the clocks being initialized. That's really an orthogonal
issue to what this patch series solves. This series is about selecting
the best sched_clock when multiple timers are present. Perhaps one that
is setup in init_early should be preferred over one setup in init_time.
I'm not sure how to detect that.
Rob
On Monday 25 March 2013, Rob Herring wrote:
> I count integrator-cp, realview, versatile and non-DT VExpress that do
> this (not surprisingly) and 25 platforms or timer implementations plus
> arm64 that do sched_clock setup in time_init. What's broken by not
> moving these earlier?
timekeeping_init() will leave the persistent_clock_exist variable as "false",
which is read in rtc_suspend() and timekeeping_inject_sleeptime().
For all I can tell, you will get a little jitter every time you
do a suspend in that case. Or perhaps it means the system clock
will be forwarded by the amount of time spent in suspend twice
after wakeup, but I'm probably misreading the code for that case.
Arnd
On 03/25/2013 03:36 PM, Arnd Bergmann wrote:
> On Monday 25 March 2013, Rob Herring wrote:
>> I count integrator-cp, realview, versatile and non-DT VExpress that do
>> this (not surprisingly) and 25 platforms or timer implementations plus
>> arm64 that do sched_clock setup in time_init. What's broken by not
>> moving these earlier?
> timekeeping_init() will leave the persistent_clock_exist variable as "false",
> which is read in rtc_suspend() and timekeeping_inject_sleeptime().
Are you mixing up the persistent_clock and sched_clock here? From a
generic stand-point they have different requirements.
> For all I can tell, you will get a little jitter every time you
> do a suspend in that case. Or perhaps it means the system clock
> will be forwarded by the amount of time spent in suspend twice
> after wakeup, but I'm probably misreading the code for that case.
No, you shouldn't see timekeeping being incremented twice, we check in
rtc_resume code if the persistent clock is present if so we won't inject
any measured suspend time there. But you're probably right that we're
being a little overly paranoid checking the same value twice.
As far as the benefit to the persistent clock: it is just a little
better to use, since we can access it earlier in resume, prior to
interrupts being enabled. So we should see less time error introduced
each suspend.
thanks
-john
On Mon, Mar 25, 2013 at 09:28:10PM +0000, Rob Herring wrote:
> On 03/25/2013 12:26 PM, Russell King - ARM Linux wrote:
> > On Thu, Mar 21, 2013 at 11:06:47AM +0000, Mark Rutland wrote:
> >> On TC2 this series leads to using the vexpress 24MHz clock as the sched clock
> >> in preference to the architected timer:
> >>
> >> Architected local timer running at 24.00MHz (virt).
> >> Switching to timer-based delay loop
> >> Registered arch_counter_get_cntvct+0x0/0x14 as sched_clock source
> >> sched_clock: 32 bits at 24MHz, resolution 41ns, wraps every 178956ms
> >> Registered versatile_read_sched_clock+0x0/0x28 as sched_clock source
> >>
> >> As they both have the same frequency, neither overrides the other, and
> >> whichever gets registered last is used as the sched_clock. As accesses
> >> to the architected timer are going to have a much lower overhead, this
> >> isn't very nice (and it could be better to use it even if it had a lower
> >> frequency).
> >
> > I'll remind people that sched_clock() is supposed to be functional at
> > the point in the boot sequence where the call to sched_init() is called.
> > That is after setup_arch() and *before* time_init() is called.
>
> I count integrator-cp, realview, versatile and non-DT VExpress that do
> this (not surprisingly) and 25 platforms or timer implementations plus
> arm64 that do sched_clock setup in time_init.
Before time_init(), sched_clock() currently returns 0 with the
architected timers (though I don't particularly like this for arm64).
Marc Rutland has patches to make arch_timer_read_counter() a function
which always returns the virtual counter. It requires the CNTVOFF
register to be set to 0 on AArch32 during boot. But this way
sched_clock() on arm64 would always return meaningful values as we have
the architected timers.
--
Catalin
On 03/25/2013 05:53 PM, John Stultz wrote:
> On 03/25/2013 03:36 PM, Arnd Bergmann wrote:
>> On Monday 25 March 2013, Rob Herring wrote:
>>> I count integrator-cp, realview, versatile and non-DT VExpress that do
>>> this (not surprisingly) and 25 platforms or timer implementations plus
>>> arm64 that do sched_clock setup in time_init. What's broken by not
>>> moving these earlier?
>> timekeeping_init() will leave the persistent_clock_exist variable as
>> "false",
>> which is read in rtc_suspend() and timekeeping_inject_sleeptime().
>
> Are you mixing up the persistent_clock and sched_clock here? From a
> generic stand-point they have different requirements.
Yes. We're talking about sched_clock here. What would be the benefit of
having it setup before sched_init vs. later in time_init?
Rob
On Monday 25 March 2013, John Stultz wrote:
> On 03/25/2013 03:36 PM, Arnd Bergmann wrote:
> > On Monday 25 March 2013, Rob Herring wrote:
> >> I count integrator-cp, realview, versatile and non-DT VExpress that do
> >> this (not surprisingly) and 25 platforms or timer implementations plus
> >> arm64 that do sched_clock setup in time_init. What's broken by not
> >> moving these earlier?
> > timekeeping_init() will leave the persistent_clock_exist variable as "false",
> > which is read in rtc_suspend() and timekeeping_inject_sleeptime().
>
> Are you mixing up the persistent_clock and sched_clock here? From a
> generic stand-point they have different requirements.
Ah, sorry about that. I had stumbled over the persistent_clock
issue earlier and was confusing the two.
> > For all I can tell, you will get a little jitter every time you
> > do a suspend in that case. Or perhaps it means the system clock
> > will be forwarded by the amount of time spent in suspend twice
> > after wakeup, but I'm probably misreading the code for that case.
>
> No, you shouldn't see timekeeping being incremented twice, we check in
> rtc_resume code if the persistent clock is present if so we won't inject
> any measured suspend time there. But you're probably right that we're
> being a little overly paranoid checking the same value twice.
Well, the point is that has_persistent_clock() returns false because
it is not yet active when the flag gets set in timekeeping_init(),
but when we call read_persistent_clock() in timekeeping_suspend(),
it will actually return a non-zero time.
> As far as the benefit to the persistent clock: it is just a little
> better to use, since we can access it earlier in resume, prior to
> interrupts being enabled. So we should see less time error introduced
> each suspend.
Ok.
Arnd
ARM: OMAP: remove unused variable
Commit 0583fe478a7 "ARM: convert arm/arm64 arch timer to use CLKSRC_OF init"
has left the omap5_realtime_timer_init() function with a stale variable and
broken whitespace. This fixes both.
Signed-off-by: Arnd Bergmann <[email protected]>
---
I've applied this patch on top of the timer cleanup series
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 7dd6453..686e498 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -618,12 +618,10 @@ OMAP_SYS_32K_TIMER_INIT(5, 1, OMAP4_32K_SOURCE, "ti,timer-alwon",
2, OMAP4_MPU_SOURCE);
void __init omap5_realtime_timer_init(void)
{
- int err;
-
omap5_sync32k_timer_init();
realtime_counter_init();
- clocksource_of_init();
+ clocksource_of_init();
}
#endif /* CONFIG_SOC_OMAP5 */
* Arnd Bergmann <[email protected]> [130423 14:28]:
> ARM: OMAP: remove unused variable
>
> Commit 0583fe478a7 "ARM: convert arm/arm64 arch timer to use CLKSRC_OF init"
> has left the omap5_realtime_timer_init() function with a stale variable and
> broken whitespace. This fixes both.
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> I've applied this patch on top of the timer cleanup series
OK thanks.
Tony
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index 7dd6453..686e498 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -618,12 +618,10 @@ OMAP_SYS_32K_TIMER_INIT(5, 1, OMAP4_32K_SOURCE, "ti,timer-alwon",
> 2, OMAP4_MPU_SOURCE);
> void __init omap5_realtime_timer_init(void)
> {
> - int err;
> -
> omap5_sync32k_timer_init();
> realtime_counter_init();
>
> - clocksource_of_init();
> + clocksource_of_init();
> }
> #endif /* CONFIG_SOC_OMAP5 */
>