Skip to contentSkip to conversation

Use MMIO clocksource for 32kHz counter

PATCH New Russell King - ARM Linux 2 participants 6 messages Jun 27, 2011 – Jul 11, 2011

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Use MMIO clocksource for 32kHz counter

Convert OMAPs 32kHz clocksource implementation to use the generic MMIO clocksource support. This achieves several things:

1. It means we get rid of all these helper functions which frankly should never have been necessary.
2. It means omap_readl() inside these helper functions does not appear in ftrace output.

Another plus is that we avoid the overhead of calculating the address to read each time, but a minus is that we use readl() which has a barrier.

--
While we're on the subject of OMAP timers...

I guess we could convert the MMIO clocksource to use readl_relaxed() et.al. because reading the current timer register shouldn't require any barriers. Thomas?

 arch/arm/Kconfig                 |    1 +
 arch/arm/plat-omap/counter_32k.c |  123 +++++++++++--------------------------
 2 files changed, 38 insertions(+), 86 deletions(-)
arch/arm/Kconfig +1 -0
index 9adc278..c1795c9 100644
@@ -852,6 +852,7 @@ config ARCH_OMAP
 	select HAVE_CLK
 	select ARCH_REQUIRE_GPIOLIB
 	select ARCH_HAS_CPUFREQ
+	select CLKSRC_MMIO
 	select GENERIC_CLOCKEVENTS
 	select HAVE_SCHED_CLOCK
 	select ARCH_HAS_HOLES_MEMORYMODEL
arch/arm/plat-omap/counter_32k.c +37 -86

189 lines not on this page · raw message

#1 lore raw

PATCH 1/1 Use MMIO clocksource for 32kHz counter

Quoted 19 lines · Russell King - ARM Linux
arch/arm/plat-omap/counter_32k.c +11 -2
+#ifdef CONFIG_ARCH_OMAP16XX
+		else if (cpu_is_omap16xx())
+			base = OMAP2_L4_IO_ADDRESS(OMAP16XX_TIMER_32K_SYNCHRONIZED);
+#endif
+#ifdef CONFIG_SOC_OMAP2420
 		else if (cpu_is_omap2420())
-			clocksource_32k.read = omap2420_32k_read;
+			base = OMAP2_L4_IO_ADDRESS(OMAP2420_32KSYNCT_BASE + 0x10);
+#endif
+#ifdef CONFIG_SOC_OMAP2430
 		else if (cpu_is_omap2430())
-			clocksource_32k.read = omap2430_32k_read;
+			base = OMAP2_L4_IO_ADDRESS(OMAP2430_32KSYNCT_BASE + 0x10);
+#endif
+#ifdef CONFIG_ARCH_OMAP3
 		else if (cpu_is_omap34xx())

arch/arm/plat-omap/counter_32k.c int __init omap_init_clocksource_32k(void) · line 135

Quoted 10 lines
arch/arm/plat-omap/counter_32k.c +5 -2
-			clocksource_32k.read = omap34xx_32k_read;
+			base = OMAP2_L4_IO_ADDRESS(OMAP3430_32KSYNCT_BASE + 0x10);
+#endif
+#ifdef CONFIG_ARCH_OMAP4
 		else if (cpu_is_omap44xx())
-			clocksource_32k.read = omap44xx_32k_read;
+			base = OMAP2_L4_IO_ADDRESS(OMAP4430_32KSYNCT_BASE + 0x10);
+#endif
 		else
 			return -ENODEV;

You should be able to replace the above with just ioremap as we now have the static mappings.

Tony

#2 lore raw

PATCH 1/1 Use MMIO clocksource for 32kHz counter

Quoted 6 lines · Tony Lindgren

arch/arm/plat-omap/counter_32k.c

Quoted 5 lines · Russell King - ARM Linux
arch/arm/plat-omap/counter_32k.c +3 -0
+#ifdef CONFIG_ARCH_OMAP16XX
+		else if (cpu_is_omap16xx())
+			base = OMAP2_L4_IO_ADDRESS(OMAP16XX_TIMER_32K_SYNCHRONIZED);

BTW, the above should have been OMAP1_IO_ADDRESS instead..

Quoted 2 lines

You should be able to replace the above with just ioremap as we now have the static mappings.

..here's a patch to use ioremap with comment added about the static mapping requirements.

Thanks for getting rid of hopefully the last remaining omap_readl for omap2+ BTW :)

Tony

arch/arm/plat-omap/counter_32k.c +15 -19
@@ -106,34 +106,30 @@ int __init omap_init_clocksource_32k(void)
 			"%s: can't register clocksource!\n";
 
 	if (cpu_is_omap16xx() || cpu_class_is_omap2()) {
+		u32 pbase;
+		unsigned long size = SZ_4K;
 		void __iomem *base;
 		struct clk *sync_32k_ick;
 
-		if (0)
-			base = NULL;
-#ifdef CONFIG_ARCH_OMAP16XX
-		else if (cpu_is_omap16xx())
-			base = OMAP2_L4_IO_ADDRESS(OMAP16XX_TIMER_32K_SYNCHRONIZED);
-#endif
-#ifdef CONFIG_SOC_OMAP2420
-		else if (cpu_is_omap2420())
-			base = OMAP2_L4_IO_ADDRESS(OMAP2420_32KSYNCT_BASE + 0x10);
-#endif
-#ifdef CONFIG_SOC_OMAP2430
+		if (cpu_is_omap16xx()) {
+			pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
+			size = SZ_1K;
+		} else if (cpu_is_omap2420())
+			pbase = OMAP2420_32KSYNCT_BASE + 0x10;
 		else if (cpu_is_omap2430())
-			base = OMAP2_L4_IO_ADDRESS(OMAP2430_32KSYNCT_BASE + 0x10);
-#endif
-#ifdef CONFIG_ARCH_OMAP3
+			pbase = OMAP2430_32KSYNCT_BASE + 0x10;
 		else if (cpu_is_omap34xx())
-			base = OMAP2_L4_IO_ADDRESS(OMAP3430_32KSYNCT_BASE + 0x10);
-#endif
-#ifdef CONFIG_ARCH_OMAP4
+			pbase = OMAP3430_32KSYNCT_BASE + 0x10;
 		else if (cpu_is_omap44xx())
-			base = OMAP2_L4_IO_ADDRESS(OMAP4430_32KSYNCT_BASE + 0x10);
-#endif
+			pbase = OMAP4430_32KSYNCT_BASE + 0x10;
 		else
 			return -ENODEV;
 
+		/* For this to work we must have a static mapping in io.c for this area */
+		base = ioremap(pbase, size);
+		if (!base)
+			return -ENODEV;
+
 		sync_32k_ick = clk_get(NULL, "omap_32ksync_ick");
 		if (!IS_ERR(sync_32k_ick))
 			clk_enable(sync_32k_ick);
#3 lore raw

PATCH 1/1 Use MMIO clocksource for 32kHz counter

Quoted 10 lines · Tony Lindgren
Quoted 6 lines · Tony Lindgren

arch/arm/plat-omap/counter_32k.c

Quoted 5 lines · Russell King - ARM Linux
arch/arm/plat-omap/counter_32k.c +3 -0
+#ifdef CONFIG_ARCH_OMAP16XX
+		else if (cpu_is_omap16xx())
+			base = OMAP2_L4_IO_ADDRESS(OMAP16XX_TIMER_32K_SYNCHRONIZED);

BTW, the above should have been OMAP1_IO_ADDRESS instead..

Quoted 8 lines
Quoted 2 lines

You should be able to replace the above with just ioremap as we now have the static mappings.

..here's a patch to use ioremap with comment added about the static mapping requirements.

Thanks for getting rid of hopefully the last remaining omap_readl for omap2+ BTW :)

What're we doing with this patch and your follow-up patch?

#4 lore raw

PATCH 1/1 Use MMIO clocksource for 32kHz counter

Quoted 14 lines · Russell King - ARM Linux
Quoted 13 lines · Tony Lindgren
Quoted 6 lines · Tony Lindgren

arch/arm/plat-omap/counter_32k.c

Quoted 5 lines · Russell King - ARM Linux
arch/arm/plat-omap/counter_32k.c +3 -0
+#ifdef CONFIG_ARCH_OMAP16XX
+		else if (cpu_is_omap16xx())
+			base = OMAP2_L4_IO_ADDRESS(OMAP16XX_TIMER_32K_SYNCHRONIZED);

BTW, the above should have been OMAP1_IO_ADDRESS instead..

Quoted 2 lines

You should be able to replace the above with just ioremap as we now have the static mappings.

Quoted 7 lines
Quoted 5 lines

..here's a patch to use ioremap with comment added about the static mapping requirements.

Thanks for getting rid of hopefully the last remaining omap_readl for omap2+ BTW :)

What're we doing with this patch and your follow-up patch?

I can queue it, it does not seem to conflict with other patches. Below is the combined version.

Regards,

Tony

From: Russell King - ARM Linux <[email protected]>
Date: Sun, 10 Jul 2011 23:05:34 -0700
Subject: [PATCH] Convert OMAPs 32kHz clocksource implementation to use the generic MMIO
clocksource support. This achieves several things:

1. It means we get rid of all these helper functions which frankly should never have been necessary.
2. It means omap_readl() inside these helper functions does not appear in ftrace output.

Another plus is that we avoid the overhead of calculating the address to read each time, but a minus is that we use readl() which has a barrier.

[tony at atomide.com: updated to use ioremap]
arch/arm/Kconfig +1 -0
index 9adc278..c1795c9 100644
@@ -852,6 +852,7 @@ config ARCH_OMAP
 	select HAVE_CLK
 	select ARCH_REQUIRE_GPIOLIB
 	select ARCH_HAS_CPUFREQ
+	select CLKSRC_MMIO
 	select GENERIC_CLOCKEVENTS
 	select HAVE_SCHED_CLOCK
 	select ARCH_HAS_HOLES_MEMORYMODEL
arch/arm/plat-omap/counter_32k.c +34 -87

187 lines not on this page · raw message

#5 lore raw

PATCH 1/1 Use MMIO clocksource for 32kHz counter

Quoted 2 lines · Tony Lindgren

I can queue it, it does not seem to conflict with other patches. Below is the combined version.

Thanks.

Quoted 7 lines

Regards,

Tony

From: Russell King - ARM Linux <[email protected]>

Please make that rmk+kernel at arm.linux.org.uk.