Skip to contentSkip to conversation

Remove ARM local timer API

PATCH Superseded Stephen Boyd 4 participants 31 messages Mar 13, 2013 – Apr 5, 2013

Subsystems (7)

v3 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

v3 cover Remove ARM local timer API

In light of Mark Rutland's recent work on divorcing the ARM architected timers from the ARM local timer API and introducing a generic arch hook for broadcast it seems that we should remove the local timer API entirely. Doing so will reduce the architecture dependencies of our timer drivers, reduce code in ARM core, and simplify timer drivers because they no longer go through an architecture layer that is essentially a hotplug notifier.

Previous attempts have been made[1] unsuccessfully. I'm hoping this can be accepted now so that we can clean up the timer drivers that are used in both UP and SMP situations. Right now these drivers have to ignore the timer setup callback on the boot CPU to avoid registering clockevents twice. This is not very symmetric and causes convuluted code that does the same thing in two places.

Patches based on v3.9-rc2. I'm still looking for Acks/Tested-by on EXYNOS and PRIMA2.

[1] http://article.gmane.org/gmane.linux.ports.arm.kernel/145705

Note: A hotplug notifier is used by both x86 for the apb_timer (see apbt_cpuhp_notify) and by metag (see arch_timer_cpu_notify in metag_generic.c) so this is not new.

Changes since v2:
* Bug fixes in smp_twd from Tony Lindgren's testing
* Move smp_twd to use late_time_init hook
* Collected Acks

Changes since v1:
* Picked up Mark's generic dummy timer driver
* Split out omap changes into new patch

Mark Rutland (1):
clocksource: add generic dummy timer driver

Stephen Boyd (9):
ARM: smp: Remove duplicate dummy timer implementation
ARM: smp_twd: Divorce smp_twd from local timer API
ARM: OMAP2+: Divorce from local timer API
ARM: EXYNOS4: Divorce mct from local timer API
ARM: PRIMA2: Divorce timer-marco from local timer API
ARM: msm: Divorce msm_timer from local timer API
clocksource: time-armada-370-xp: Fix sparse warning
clocksource: time-armada-370-xp: Divorce from local timer API
ARM: smp: Remove local timer API

 arch/arm/Kconfig                         |  12 +--
 arch/arm/include/asm/localtimer.h        |  34 ---------
 arch/arm/kernel/smp.c                    |  87 ---------------------
 arch/arm/kernel/smp_twd.c                |  64 ++++++++++------
 arch/arm/mach-exynos/mct.c               |  53 +++++++++----
 arch/arm/mach-msm/timer.c                | 125 +++++++++++++++++--------------
 arch/arm/mach-omap2/Kconfig              |   1 -
 arch/arm/mach-omap2/timer.c              |   7 --
 arch/arm/mach-prima2/timer-marco.c       |  98 ++++++++++++------------
 drivers/clocksource/Makefile             |   1 +
 drivers/clocksource/dummy_timer.c        |  67 +++++++++++++++++
 drivers/clocksource/time-armada-370-xp.c |  88 ++++++++++------------
 include/linux/time-armada-370-xp.h       |   4 +-
 13 files changed, 309 insertions(+), 332 deletions(-)
 delete mode 100644 arch/arm/include/asm/localtimer.h
 create mode 100644 drivers/clocksource/dummy_timer.c
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#1 lore raw

PATCH v3 1/10 clocksource: add generic dummy timer driver

From: Mark Rutland <[email protected]>

Several architectures have a dummy timer driver tightly coupled with their broadcast code to support machines without cpu-local timers (or where there is a lack of driver support).

Since 12ad100046: "clockevents: Add generic timer broadcast function" it's been possible to write broadcast-capable timer drivers decoupled from the broadcast mechanism. We can use this functionality to implement a generic dummy timer driver that can be shared by all architectures with generic tick broadcast (ARCH_HAS_TICK_BROADCAST).

This patch implements a generic dummy timer using this facility.

Cc John Stultz <[email protected]>
Cc Thomas Gleixner <[email protected]>
[sboyd: Make percpu data static and use __this_cpu_ptr()]
---
 drivers/clocksource/Makefile      |  1 +
 drivers/clocksource/dummy_timer.c | 67 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+)
 create mode 100644 drivers/clocksource/dummy_timer.c
drivers/clocksource/Makefile +1 -0
index 4d8283a..655d718 100644
@@ -22,3 +22,4 @@ obj-$(CONFIG_VT8500_TIMER)	+= vt8500_timer.o
 
 obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
 obj-$(CONFIG_CLKSRC_METAG_GENERIC)	+= metag_generic.o
+obj-$(CONFIG_ARCH_HAS_TICK_BROADCAST)	+= dummy_timer.o
drivers/clocksource/dummy_timer.c new file +67 -0

67 lines not on this page · raw message

Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#2 lore raw

Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver

Hi Stephen,

I've just been trying to test the dummy timer, and realised it's broken, as it registers a cpu notifier from a device_initcall (after SMP's been brought up), and doesn't ensure all active CPUs have been set up. Evidently no-one else has attempted to test it thus far, and I'm not able to throughly test it at the moment.

For now I'd recommend going back to the rearranging the existing arm implementation as you had in v1. We seem to have ironed out the kinks with having a simultaneous arch_timer and dummy, so I don't imagine we'll have major issues taking that route.

Thanks,
Mark.

Quoted 112 lines · Stephen Boyd

From: Mark Rutland <[email protected]>

Several architectures have a dummy timer driver tightly coupled with their broadcast code to support machines without cpu-local timers (or where there is a lack of driver support).

Since 12ad100046: "clockevents: Add generic timer broadcast function" it's been possible to write broadcast-capable timer drivers decoupled from the broadcast mechanism. We can use this functionality to implement a generic dummy timer driver that can be shared by all architectures with generic tick broadcast (ARCH_HAS_TICK_BROADCAST).

This patch implements a generic dummy timer using this facility.

Cc John Stultz <[email protected]>
Cc Thomas Gleixner <[email protected]>
[sboyd: Make percpu data static and use __this_cpu_ptr()]
---
 drivers/clocksource/Makefile      |  1 +
 drivers/clocksource/dummy_timer.c | 67 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+)
 create mode 100644 drivers/clocksource/dummy_timer.c
drivers/clocksource/Makefile +1 -0
index 4d8283a..655d718 100644
@@ -22,3 +22,4 @@ obj-$(CONFIG_VT8500_TIMER)	+= vt8500_timer.o
 
 obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
 obj-$(CONFIG_CLKSRC_METAG_GENERIC)	+= metag_generic.o
+obj-$(CONFIG_ARCH_HAS_TICK_BROADCAST)	+= dummy_timer.o
drivers/clocksource/dummy_timer.c new file +67 -0
new file mode 100644
index 0000000..7e1ce45
@@ -0,0 +1,67 @@
+/*
+ *  linux/drivers/clocksource/dummy_timer.c
+ *
+ *  Copyright (C) 2013 ARM Ltd.
+ *  All Rights Reserved
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/clockchips.h>
+#include <linux/cpu.h>
+#include <linux/init.h>
+#include <linux/percpu.h>
+
+static DEFINE_PER_CPU(struct clock_event_device, dummy_timer_evt);
+
+static void dummy_timer_set_mode(enum clock_event_mode mode,
+			   struct clock_event_device *evt)
+{
+	/*
+	 * Core clockevents code will call this when exchanging timer devices.
+	 * We don't need to do anything here.
+	 */
+}
+
+static void __cpuinit dummy_timer_setup(void)
+{
+	int cpu = smp_processor_id();
+	struct clock_event_device *evt = __this_cpu_ptr(&dummy_timer_evt);
+
+	evt->name	= "dummy_timer";
+	evt->features	= CLOCK_EVT_FEAT_PERIODIC |
+			  CLOCK_EVT_FEAT_ONESHOT |
+			  CLOCK_EVT_FEAT_DUMMY;
+	evt->rating	= 100;
+	evt->set_mode	= dummy_timer_set_mode;
+	evt->cpumask	= cpumask_of(cpu);
+
+	clockevents_register_device(evt);
+}
+
+static int __cpuinit dummy_timer_cpu_notify(struct notifier_block *self,
+				      unsigned long action, void *hcpu)
+{
+	if ((action & ~CPU_TASKS_FROZEN) == CPU_STARTING)
+		dummy_timer_setup();
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block dummy_timer_cpu_nb __cpuinitdata = {
+	.notifier_call = dummy_timer_cpu_notify,
+};
+
+static int __init dummy_timer_register(void)
+{
+	int err = register_cpu_notifier(&dummy_timer_cpu_nb);
+	if (err)
+		return err;
+
+	/* We won't get a call on the boot CPU, so register immediately */
+	dummy_timer_setup();
+
+	return 0;
+}
+device_initcall(dummy_timer_register);
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#3 lore raw

Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver

Quoted 7 lines · Mark Rutland

Hi Stephen,

I've just been trying to test the dummy timer, and realised it's broken, as it registers a cpu notifier from a device_initcall (after SMP's been brought up), and doesn't ensure all active CPUs have been set up. Evidently no-one else has attempted to test it thus far, and I'm not able to throughly test it at the moment.

Would it be sufficient to register as a pre-smp initcall?

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#4 lore raw

Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver

Quoted 10 lines · Stephen Boyd
Quoted 7 lines · Mark Rutland

Hi Stephen,

I've just been trying to test the dummy timer, and realised it's broken, as it registers a cpu notifier from a device_initcall (after SMP's been brought up), and doesn't ensure all active CPUs have been set up. Evidently no-one else has attempted to test it thus far, and I'm not able to throughly test it at the moment.

Would it be sufficient to register as a pre-smp initcall?

I've looked a bit further into the problem, and I believe using early_initcall will make it work as well as the current arm-specific dummy timers work with a rating of 100 (this means the recent patch lowering the rating broke tc2 as explained below).

I've spent the last few hours trying to get the dummy_timer driver working on tc2 with the sp804 as the broadcast source (with architected timer support disabled). It turns out that having dummy timer's rating so low means that it won't be selected as the tick device on cpu0 in preference to the sp804, and thus won't push the sp804 out of that spot (allowing it to become the broadcast source). This leads to boot stalling.

Jumping the dummy_timer's rating up would fix this, but that doesn't seem great. Registering the dummy before all other clocks would also fix this (I tried calling dummy_timer_register from time_init), but I can't see a way to do that while keeping the driver self-contained.

Thanks,
Mark.

#5 lore raw

Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver

Quoted 9 lines · Mark Rutland
Quoted 8 lines · Stephen Boyd
Quoted 7 lines · Mark Rutland

Hi Stephen,

I've just been trying to test the dummy timer, and realised it's broken, as it registers a cpu notifier from a device_initcall (after SMP's been brought up), and doesn't ensure all active CPUs have been set up. Evidently no-one else has attempted to test it thus far, and I'm not able to throughly test it at the moment.

Quoted 5 lines
Quoted 1 line

Would it be sufficient to register as a pre-smp initcall?

I've looked a bit further into the problem, and I believe using early_initcall will make it work as well as the current arm-specific dummy timers work with a rating of 100 (this means the recent patch lowering the rating broke tc2 as explained below).

Yes, moving to early_initcall() should make the dummy timer equivalent to the code that is already in the arm directory. It sounds like there is a problem with mainline though?

Quoted 7 lines

I've spent the last few hours trying to get the dummy_timer driver working on tc2 with the sp804 as the broadcast source (with architected timer support disabled). It turns out that having dummy timer's rating so low means that it won't be selected as the tick device on cpu0 in preference to the sp804, and thus won't push the sp804 out of that spot (allowing it to become the broadcast source). This leads to boot stalling.

I'm not following here. Why would we want to remove sp804 from the tick duty?

Quoted 5 lines

Jumping the dummy_timer's rating up would fix this, but that doesn't seem great. Registering the dummy before all other clocks would also fix this (I tried calling dummy_timer_register from time_init), but I can't see a way to do that while keeping the driver self-contained.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#6 lore raw

Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver

Quoted 11 lines · Stephen Boyd
Quoted 10 lines · Mark Rutland
Quoted 9 lines · Stephen Boyd
Quoted 7 lines · Mark Rutland

Hi Stephen,

I've just been trying to test the dummy timer, and realised it's broken, as it registers a cpu notifier from a device_initcall (after SMP's been brought up), and doesn't ensure all active CPUs have been set up. Evidently no-one else has attempted to test it thus far, and I'm not able to throughly test it at the moment.

Would it be sufficient to register as a pre-smp initcall?

Quoted 8 lines
Quoted 4 lines

I've looked a bit further into the problem, and I believe using early_initcall will make it work as well as the current arm-specific dummy timers work with a rating of 100 (this means the recent patch lowering the rating broke tc2 as explained below).

Yes, moving to early_initcall() should make the dummy timer equivalent to the code that is already in the arm directory. It sounds like there is a problem with mainline though?

Sorry, I rushed writing my earlier email in an attempt to get something on the list, I should've spent some time making it clearer.

Using early_initcall will make it equivalent to the current code.

Unfortunately the existing code's current rating of 100 (as of Santosh's patch [1]) breaks running an SMP system without local timers (at least using the sp804 as a broadcast source on tc2 with architected timer support disabled).

It seems my patch making the timer core always reject dummy timers as broadcast sources hasn't gotten to mainline yet, so we can't revert the patch without breaking the platform Santosh noticed the problem on.

Thomas, do you have an idea of if/when tip/timers/urgent will hit mainline?

Quoted 11 lines
Quoted 8 lines

I've spent the last few hours trying to get the dummy_timer driver working on tc2 with the sp804 as the broadcast source (with architected timer support disabled). It turns out that having dummy timer's rating so low means that it won't be selected as the tick device on cpu0 in preference to the sp804, and thus won't push the sp804 out of that spot (allowing it to become the broadcast source). This leads to boot stalling.

I'm not following here. Why would we want to remove sp804 from the tick duty?

To run an SMP system without local timers, we need the sp804 to be the broadcast timer. Unfortunately the tick device and broadcast timer are mutually exclusive positions, so we need to have a dummy timer knock the sp804 out of tick device duty to enable broadcast.

When the dummy timer's rating was 400 (against the sp804's 350), this worked. The sp804 would be registered, and would become cpu0's tick device. Later the dummy would get registered, knocking the sp804 out (whereupon it would get cycled back through tick_check_new_device and become the broadcast timer).

With the dummy timer's rating lower, the sp804 stays as cpu0's tick device, all other cpus get dummy timers, but there's no broadcast source, so the system locks up waiting for secondary cpus.

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-March/154582.html

#7 lore raw

Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver

Quoted 1 line · Mark Rutland

Thomas, do you have an idea of if/when tip/timers/urgent will hit mainline?

Thanks for the reminder. I just sent a pull request Linuswards.

Thanks,

tglx

#8 lore raw

Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver

Quoted 4 lines · Thomas Gleixner
Quoted 1 line · Mark Rutland

Thomas, do you have an idea of if/when tip/timers/urgent will hit mainline?

Thanks for the reminder. I just sent a pull request Linuswards.

Cheers!

Mark.

#9 lore raw

Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver

In reply to Mark Rutland
Quoted 14 lines · Mark Rutland
Quoted 9 lines
Quoted 7 lines

I've spent the last few hours trying to get the dummy_timer driver working on tc2 with the sp804 as the broadcast source (with architected timer support disabled). It turns out that having dummy timer's rating so low means that it won't be selected as the tick device on cpu0 in preference to the sp804, and thus won't push the sp804 out of that spot (allowing it to become the broadcast source). This leads to boot stalling.

I'm not following here. Why would we want to remove sp804 from the tick duty?

To run an SMP system without local timers, we need the sp804 to be the broadcast timer. Unfortunately the tick device and broadcast timer are mutually exclusive positions, so we need to have a dummy timer knock the sp804 out of tick device duty to enable broadcast.

Quoted 8 lines

When the dummy timer's rating was 400 (against the sp804's 350), this worked. The sp804 would be registered, and would become cpu0's tick device. Later the dummy would get registered, knocking the sp804 out (whereupon it would get cycled back through tick_check_new_device and become the broadcast timer).

With the dummy timer's rating lower, the sp804 stays as cpu0's tick device, all other cpus get dummy timers, but there's no broadcast source, so the system locks up waiting for secondary cpus.

Ok. Thanks for clearing up my confusion.

Like you say, increasing the dummy timer rating seems like a hack. But it also sounds like you want to keep the dummy timer driver fully self contained. I'm not opposed to calling dummy_timer_register() at the bottom of tick_init() if we have to, but it sounds like you don't like that.

An alternative would be to push the dummy timer logic into the core clockevents layer under the ifdef for arch has broadcast. This is probably the correct approach because most devices don't want to have a dummy timer sitting around unused. I might be able to take a look at this tomorrow.

One final question, if you remove all other CPUs besides the CPU that is servicing the sp804 interrupt do we end up in a situation where the sp804 is broadcasting to the dummy tick device? I haven't read through all the code yet for that one. I would think tick_switch_to_oneshot() would complain on your device?

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#10 lore raw

Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver

Quoted 24 lines · Stephen Boyd
Quoted 14 lines · Mark Rutland
Quoted 9 lines
Quoted 7 lines

I've spent the last few hours trying to get the dummy_timer driver working on tc2 with the sp804 as the broadcast source (with architected timer support disabled). It turns out that having dummy timer's rating so low means that it won't be selected as the tick device on cpu0 in preference to the sp804, and thus won't push the sp804 out of that spot (allowing it to become the broadcast source). This leads to boot stalling.

I'm not following here. Why would we want to remove sp804 from the tick duty?

To run an SMP system without local timers, we need the sp804 to be the broadcast timer. Unfortunately the tick device and broadcast timer are mutually exclusive positions, so we need to have a dummy timer knock the sp804 out of tick device duty to enable broadcast.

Quoted 8 lines

When the dummy timer's rating was 400 (against the sp804's 350), this worked. The sp804 would be registered, and would become cpu0's tick device. Later the dummy would get registered, knocking the sp804 out (whereupon it would get cycled back through tick_check_new_device and become the broadcast timer).

With the dummy timer's rating lower, the sp804 stays as cpu0's tick device, all other cpus get dummy timers, but there's no broadcast source, so the system locks up waiting for secondary cpus.

Quoted 6 lines

Ok. Thanks for clearing up my confusion.

Like you say, increasing the dummy timer rating seems like a hack. But it also sounds like you want to keep the dummy timer driver fully self contained. I'm not opposed to calling dummy_timer_register() at the bottom of tick_init() if we have to, but it sounds like you don't like that.

I'd like to keep the dummy timer driver self-contained if we can, but if it makes it more robust and/or easier to deal with by having an external call to register the driver, then I'm not opposed to that.

Quoted 6 lines

An alternative would be to push the dummy timer logic into the core clockevents layer under the ifdef for arch has broadcast. This is probably the correct approach because most devices don't want to have a dummy timer sitting around unused. I might be able to take a look at this tomorrow.

I'm also not opposed to this idea.

Quoted 6 lines

One final question, if you remove all other CPUs besides the CPU that is servicing the sp804 interrupt do we end up in a situation where the sp804 is broadcasting to the dummy tick device? I haven't read through all the code yet for that one. I would think tick_switch_to_oneshot() would complain on your device?

The current code in arch/arm/kernel/smp.c only registers the local timer on cpu0 if we have more than one CPU, so the sp804 stays as cpu0's tick device.

With the generic dummy timer (with a rating bumped up to 400, using an early_initcall, and the dummy timers rejected as broadcast sources [1]), even when booting only with one cpu described in the dt the sp804 is relegated to broadcast timer duty, broadcasting to the dummy timer on cpu0. Echoing 'q' to /proc/sysrq-trigger gives me:

Tick Device: mode: 0
Broadcast device
Clock Event Device: v2m-timer0

 max_delta_ns:   4294966591000
 min_delta_ns:   14999
 mult:           2147484
 shift:          31
 mode:           2
 next_event:     9223372036854775807 nsecs
 set_next_event: sp804_set_next_event
 set_mode:       sp804_set_mode
 event_handler:  tick_handle_periodic_broadcast
 retries:        0

tick_broadcast_mask: 00000001
tick_broadcast_oneshot_mask: 00000000

Tick Device: mode: 0
Per CPU device: 0
Clock Event Device: dummy_timer

 max_delta_ns:   0
 min_delta_ns:   0
 mult:           0
 shift:          0
 mode:           1
 next_event:     9223372036854775807 nsecs
 set_next_event: <00000000>
 set_mode:       dummy_timer_set_mode
 event_handler:  tick_handle_periodic
 retries:        0

Though "broadcasting" to the same cpu is special-cased in tick_do_broadcast, which will call the tick device's event_handler directly rather than having the cpu attempt to IPI to itself.

As you suggest, tick_switch_to_oneshot does complain:

Clockevents: could not switch to one-shot mode: dummy_timer is not functional. Could not switch to high resolution mode on CPU 0

To handle this case we could check cpu_possible_mask when initialising the dummy and only register it if more than 1 cpu is possible. That would be roughly in line with how we handle this case in smp.c.

Thanks,
Mark.

[1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=a7dc19b8652c862d5b7c4d2339bd3c428bd29c4a

#11 lore raw

Re: [PATCHv3 01/10] clocksource: add generic dummy timer driver

Quoted 11 lines · Mark Rutland
Quoted 6 lines · Stephen Boyd

Ok. Thanks for clearing up my confusion.

Like you say, increasing the dummy timer rating seems like a hack. But it also sounds like you want to keep the dummy timer driver fully self contained. I'm not opposed to calling dummy_timer_register() at the bottom of tick_init() if we have to, but it sounds like you don't like that.

I'd like to keep the dummy timer driver self-contained if we can, but if it makes it more robust and/or easier to deal with by having an external call to register the driver, then I'm not opposed to that.

Quoted 6 lines
Quoted 5 lines

An alternative would be to push the dummy timer logic into the core clockevents layer under the ifdef for arch has broadcast. This is probably the correct approach because most devices don't want to have a dummy timer sitting around unused. I might be able to take a look at this tomorrow.

I'm also not opposed to this idea.

What if we only check the rating if the two devices are for the same CPU, i.e. always prefer percpu devices over global ones? This would make your dummy timers into tick devices.

kernel/time/tick-common.c +3 -2
index b1600a6..9ea59b9 100644
@@ -251,9 +251,10 @@ static int tick_check_new_device(struct clock_event_device *newdev)
                    !(newdev->features & CLOCK_EVT_FEAT_ONESHOT))
                        goto out_bc;
                /*
-                * Check the rating
+                * Check the rating, but prefer CPU local devices
                 */
-               if (curdev->rating >= newdev->rating)
+               if (curdev->rating >= newdev->rating &&
+                   cpumask_equal(curdev->cpumask, newdev->cpumask))
                        goto out_bc;
        }
 

I think it will work with smp_twd, sp804, and dummy timers registered in any order too.

Quoted 4 lines

Though "broadcasting" to the same cpu is special-cased in tick_do_broadcast, which will call the tick device's event_handler directly rather than having the cpu attempt to IPI to itself.

Quoted 8 lines

As you suggest, tick_switch_to_oneshot does complain:

Clockevents: could not switch to one-shot mode: dummy_timer is not functional. Could not switch to high resolution mode on CPU 0

To handle this case we could check cpu_possible_mask when initialising the dummy and only register it if more than 1 cpu is possible. That would be roughly in line with how we handle this case in smp.c.

Yes, this will work well enough. Squashed it into this patch.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#12 lore raw

PATCH v3 2/10 ARM: smp: Remove duplicate dummy timer implementation

In reply to Stephen Boyd

Drop ARM's version of the dummy timer now that we have a generic implementation in drivers/clocksource.

---
 arch/arm/kernel/smp.c | 22 ++--------------------
 1 file changed, 2 insertions(+), 20 deletions(-)
arch/arm/kernel/smp.c +2 -20
index 31644f1..4de4d14 100644
@@ -469,24 +469,6 @@ void tick_broadcast(const struct cpumask *mask)
 }
 #endif
 
-static void broadcast_timer_set_mode(enum clock_event_mode mode,
-	struct clock_event_device *evt)
-{
-}
-
-static void __cpuinit broadcast_timer_setup(struct clock_event_device *evt)
-{
-	evt->name	= "dummy_timer";
-	evt->features	= CLOCK_EVT_FEAT_ONESHOT |
-			  CLOCK_EVT_FEAT_PERIODIC |
-			  CLOCK_EVT_FEAT_DUMMY;
-	evt->rating	= 400;
-	evt->mult	= 1;
-	evt->set_mode	= broadcast_timer_set_mode;
-
-	clockevents_register_device(evt);
-}
-
 static struct local_timer_ops *lt_ops;
 
 #ifdef CONFIG_LOCAL_TIMERS
@@ -510,8 +492,8 @@ static void __cpuinit percpu_timer_setup(void)
 
 	evt->cpumask = cpumask_of(cpu);
 
-	if (!lt_ops || lt_ops->setup(evt))
-		broadcast_timer_setup(evt);
+	if (lt_ops)
+		lt_ops->setup(evt);
 }
 
 #ifdef CONFIG_HOTPLUG_CPU
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#13 lore raw

PATCH v3 3/10 ARM: smp_twd: Divorce smp_twd from local timer API

In reply to Stephen Boyd

Separate the smp_twd timers from the local timer API. This will allow us to remove ARM local timer support in the near future and gets us closer to moving this driver to drivers/clocksource.

Cc Russell King <[email protected]>
Cc Tony Lindgren <[email protected]>

---

Changes since v2:
* Fix booting on qemu and omap

 arch/arm/Kconfig          |  2 +-
 arch/arm/kernel/smp_twd.c | 64 +++++++++++++++++++++++++++++++----------------
 2 files changed, 44 insertions(+), 22 deletions(-)
arch/arm/Kconfig +1 -1
index 5b71469..5ad2ccf 100644
@@ -1527,6 +1527,7 @@ config SMP
 	depends on HAVE_SMP
 	depends on MMU
 	select HAVE_ARM_SCU if !ARCH_MSM_SCORPIONMP
+	select HAVE_ARM_TWD if (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT)
 	select USE_GENERIC_SMP_HELPERS
 	help
 	  This enables support for systems with more than one CPU. If you have
@@ -1650,7 +1651,6 @@ config LOCAL_TIMERS
 	bool "Use local timer interrupts"
 	depends on SMP
 	default y
-	select HAVE_ARM_TWD if (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT)
 	help
 	  Enable support for local timers on SMP platforms, rather then the
 	  legacy IPI broadcast method.  Local timers allows the system
arch/arm/kernel/smp_twd.c +43 -21

163 lines not on this page · raw message

Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
Tested-by on v3 3/10
#14 lore raw

Re: [PATCHv3 03/10] ARM: smp_twd: Divorce smp_twd from local timer API

This works on my A9x4 coretile, bringing CPUs up and down via /sys/devices/system/cpu/*/online, so:

Otherwise, is there any reason we couldn't now use the twd driver on a UP system? Or would the overhead of handling frequency change make this pointless?

Quoted 21 lines · Stephen Boyd

Separate the smp_twd timers from the local timer API. This will allow us to remove ARM local timer support in the near future and gets us closer to moving this driver to drivers/clocksource.

Cc Russell King <[email protected]>
Cc Tony Lindgren <[email protected]>

---

Changes since v2:
* Fix booting on qemu and omap

 arch/arm/Kconfig          |  2 +-
 arch/arm/kernel/smp_twd.c | 64 +++++++++++++++++++++++++++++++----------------
 2 files changed, 44 insertions(+), 22 deletions(-)
arch/arm/Kconfig +0 -0
index 5b71469..5ad2ccf 100644

arch/arm/Kconfig config SMP · line 1530

Quoted 4 lines
arch/arm/Kconfig +1 -0
@@ -1527,6 +1527,7 @@ config SMP
 	depends on HAVE_SMP
 	depends on MMU
 	select HAVE_ARM_SCU if !ARCH_MSM_SCORPIONMP
+	select HAVE_ARM_TWD if (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT)

Could you not depend on your "Push selects for TWD/SCU into machine entries" for this?

Mark.

#15 lore raw

Re: [PATCHv3 03/10] ARM: smp_twd: Divorce smp_twd from local timer API

Quoted 4 lines · Mark Rutland

This works on my A9x4 coretile, bringing CPUs up and down via /sys/devices/system/cpu/*/online, so:

Thanks. I still need to resolve patch #1 though.

Quoted 3 lines

Otherwise, is there any reason we couldn't now use the twd driver on a UP system? Or would the overhead of handling frequency change make this pointless?

I don't see why not but I don't have any interest in pursuing it.

Quoted 13 lines · Stephen Boyd

arch/arm/Kconfig config SMP · line 1530

Quoted 9 lines
arch/arm/Kconfig +1 -0
index 5b71469..5ad2ccf 100644
@@ -1527,6 +1527,7 @@ config SMP
 	depends on HAVE_SMP
 	depends on MMU
 	select HAVE_ARM_SCU if !ARCH_MSM_SCORPIONMP
+	select HAVE_ARM_TWD if (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT)

Could you not depend on your "Push selects for TWD/SCU into machine entries" for this?

Right now the patches don't depend on the push down patch. Are you saying it would be better to depend on that patch?

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#16 lore raw

Re: [PATCHv3 03/10] ARM: smp_twd: Divorce smp_twd from local timer API

Quoted 6 lines · Stephen Boyd
Quoted 4 lines · Mark Rutland

This works on my A9x4 coretile, bringing CPUs up and down via /sys/devices/system/cpu/*/online, so:

Quoted 7 lines

Thanks. I still need to resolve patch #1 though.

Quoted 3 lines

Otherwise, is there any reason we couldn't now use the twd driver on a UP system? Or would the overhead of handling frequency change make this pointless?

I don't see why not but I don't have any interest in pursuing it.

Ok.

Quoted 15 lines
Quoted 14 lines · Stephen Boyd

arch/arm/Kconfig config SMP · line 1530

Quoted 9 lines
arch/arm/Kconfig +1 -0
index 5b71469..5ad2ccf 100644
@@ -1527,6 +1527,7 @@ config SMP
 	depends on HAVE_SMP
 	depends on MMU
 	select HAVE_ARM_SCU if !ARCH_MSM_SCORPIONMP
+	select HAVE_ARM_TWD if (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT)

Could you not depend on your "Push selects for TWD/SCU into machine entries" for this?

Quoted 2 lines

Right now the patches don't depend on the push down patch. Are you saying it would be better to depend on that patch?

It just seemed odd to me that the two series should conflict (though trivially) here.

Mark.

#17 lore raw

PATCH v3 4/10 ARM: OMAP2+: Divorce from local timer API

In reply to Stephen Boyd

Now that the TWD doesn't rely on the local timer API, OMAP can stop selecting it in Kconfig and relying on the config option to decide if it should call smp_twd functions.

---
 arch/arm/mach-omap2/Kconfig | 1 -
 arch/arm/mach-omap2/timer.c | 7 -------
 2 files changed, 8 deletions(-)
arch/arm/mach-omap2/Kconfig +0 -1
index 49ac3df..6e1f871 100644
@@ -88,7 +88,6 @@ config ARCH_OMAP4
 	select CACHE_L2X0
 	select CPU_V7
 	select HAVE_SMP
-	select LOCAL_TIMERS if SMP
 	select OMAP_INTERCONNECT
 	select PL310_ERRATA_588369
 	select PL310_ERRATA_727915
arch/arm/mach-omap2/timer.c +0 -7
index 2bdd4cf..c00a8f8 100644
@@ -587,7 +587,6 @@ OMAP_SYS_GP_TIMER_INIT(3_am33xx, 1, OMAP4_MPU_SOURCE, "ti,timer-alwon",
 #ifdef CONFIG_ARCH_OMAP4
 OMAP_SYS_32K_TIMER_INIT(4, 1, OMAP4_32K_SOURCE, "ti,timer-alwon",
 			2, OMAP4_MPU_SOURCE);
-#ifdef CONFIG_LOCAL_TIMERS
 static DEFINE_TWD_LOCAL_TIMER(twd_local_timer, OMAP44XX_LOCAL_TWD_BASE, 29);
 void __init omap4_local_timer_init(void)
 {
@@ -606,12 +605,6 @@ void __init omap4_local_timer_init(void)
 			pr_err("twd_local_timer_register failed %d\n", err);
 	}
 }
-#else /* CONFIG_LOCAL_TIMERS */
-void __init omap4_local_timer_init(void)
-{
-	omap4_sync32k_timer_init();
-}
-#endif /* CONFIG_LOCAL_TIMERS */
 #endif /* CONFIG_ARCH_OMAP4 */
 
 #ifdef CONFIG_SOC_OMAP5
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#18 lore raw

PATCH v3 5/10 ARM: EXYNOS4: Divorce mct from local timer API

In reply to Stephen Boyd

Separate the mct local timers from the local timer API. This will allow us to remove ARM local timer support in the near future and gets us closer to moving this driver to drivers/clocksource.

---
 arch/arm/mach-exynos/mct.c | 53 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 16 deletions(-)
arch/arm/mach-exynos/mct.c +37 -16
index c9d6650..5a9a73f 100644
@@ -16,13 +16,13 @@
 #include <linux/err.h>
 #include <linux/clk.h>
 #include <linux/clockchips.h>
+#include <linux/cpu.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
 #include <linux/percpu.h>
 #include <linux/of.h>
 
 #include <asm/arch_timer.h>
-#include <asm/localtimer.h>
 
 #include <plat/cpu.h>
 
@@ -42,7 +42,7 @@ static unsigned long clk_rate;
 static unsigned int mct_int_type;
 
 struct mct_clock_event_device {
-	struct clock_event_device *evt;
+	struct clock_event_device evt;
 	void __iomem *base;
 	char name[10];
 };
@@ -264,8 +264,6 @@ static void exynos4_clockevent_init(void)
 		setup_irq(EXYNOS4_IRQ_MCT_G0, &mct_comp_event_irq);
 }
 
-#ifdef CONFIG_LOCAL_TIMERS
-
 static DEFINE_PER_CPU(struct mct_clock_event_device, percpu_mct_tick);
 
 /* Clock event handling */
@@ -338,7 +336,7 @@ static inline void exynos4_tick_set_mode(enum clock_event_mode mode,
 
 static int exynos4_mct_tick_clear(struct mct_clock_event_device *mevt)
 {
-	struct clock_event_device *evt = mevt->evt;
+	struct clock_event_device *evt = &mevt->evt;
 
 	/*
 	 * This is for supporting oneshot mode.
@@ -360,7 +358,7 @@ static int exynos4_mct_tick_clear(struct mct_clock_event_device *mevt)
 static irqreturn_t exynos4_mct_tick_isr(int irq, void *dev_id)
 {
 	struct mct_clock_event_device *mevt = dev_id;
-	struct clock_event_device *evt = mevt->evt;
+	struct clock_event_device *evt = &mevt->evt;
 
 	exynos4_mct_tick_clear(mevt);
 
@@ -388,7 +386,6 @@ static int __cpuinit exynos4_local_timer_setup(struct clock_event_device *evt)
 	int mct_lx_irq;
 
 	mevt = this_cpu_ptr(&percpu_mct_tick);
-	mevt->evt = evt;
 
 	mevt->base = EXYNOS4_MCT_L_BASE(cpu);
 	sprintf(mevt->name, "mct_tick%d", cpu);
@@ -426,7 +423,7 @@ static int __cpuinit exynos4_local_timer_setup(struct clock_event_device *evt)
 	return 0;
 }
 
-static void exynos4_local_timer_stop(struct clock_event_device *evt)
+static void __cpuinit exynos4_local_timer_stop(struct clock_event_device *evt)
 {
 	unsigned int cpu = smp_processor_id();
 	evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
@@ -439,22 +436,38 @@ static void exynos4_local_timer_stop(struct clock_event_device *evt)
 		disable_percpu_irq(EXYNOS_IRQ_MCT_LOCALTIMER);
 }
 
-static struct local_timer_ops exynos4_mct_tick_ops __cpuinitdata = {
-	.setup	= exynos4_local_timer_setup,
-	.stop	= exynos4_local_timer_stop,
+static int __cpuinit exynos4_mct_cpu_notify(struct notifier_block *self,
+					   unsigned long action, void *hcpu)
+{
+	struct mct_clock_event_device *mevt = this_cpu_ptr(&percpu_mct_tick);
+	struct clock_event_device *evt = &mevt->evt;
+
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_STARTING:
+		exynos4_local_timer_setup(evt);
+		break;
+	case CPU_DYING:
+		exynos4_local_timer_stop(evt);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block exynos4_mct_cpu_nb __cpuinitdata = {
+	.notifier_call = exynos4_mct_cpu_notify,
 };
-#endif /* CONFIG_LOCAL_TIMERS */
 
 static void __init exynos4_timer_resources(void)
 {
+	int err;
+	struct mct_clock_event_device *mevt = this_cpu_ptr(&percpu_mct_tick);
 	struct clk *mct_clk;
 	mct_clk = clk_get(NULL, "xtal");
 
 	clk_rate = clk_get_rate(mct_clk);
 
-#ifdef CONFIG_LOCAL_TIMERS
 	if (mct_int_type == MCT_INT_PPI) {
-		int err;
 
 		err = request_percpu_irq(EXYNOS_IRQ_MCT_LOCALTIMER,
 					 exynos4_mct_tick_isr, "MCT",
@@ -463,8 +476,16 @@ static void __init exynos4_timer_resources(void)
 		     EXYNOS_IRQ_MCT_LOCALTIMER, err);
 	}
 
-	local_timer_register(&exynos4_mct_tick_ops);
-#endif /* CONFIG_LOCAL_TIMERS */
+	err = register_cpu_notifier(&exynos4_mct_cpu_nb);
+	if (err)
+		goto out_irq;
+
+	/* Immediately configure the timer on the boot CPU */
+	exynos4_local_timer_setup(&mevt->evt);
+	return;
+
+out_irq:
+	free_percpu_irq(EXYNOS_IRQ_MCT_LOCALTIMER, &percpu_mct_tick);
 }
 
 void __init exynos4_timer_init(void)
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#19 lore raw

PATCH v3 6/10 ARM: PRIMA2: Divorce timer-marco from local timer API

In reply to Stephen Boyd

Separate the marco local timers from the local timer API. This will allow us to remove ARM local timer support in the near future and gets us closer to moving this driver to drivers/clocksource.

---
 arch/arm/mach-prima2/timer-marco.c | 98 ++++++++++++++++++++------------------
 1 file changed, 52 insertions(+), 46 deletions(-)
arch/arm/mach-prima2/timer-marco.c +52 -46
index f4eea2e..d54aac2 100644
@@ -10,6 +10,7 @@
 #include <linux/interrupt.h>
 #include <linux/clockchips.h>
 #include <linux/clocksource.h>
+#include <linux/cpu.h>
 #include <linux/bitops.h>
 #include <linux/irq.h>
 #include <linux/clk.h>
@@ -18,7 +19,6 @@
 #include <linux/of_irq.h>
 #include <linux/of_address.h>
 #include <asm/sched_clock.h>
-#include <asm/localtimer.h>
 #include <asm/mach/time.h>
 
 #include "common.h"
@@ -154,13 +154,7 @@ static void sirfsoc_clocksource_resume(struct clocksource *cs)
 		BIT(1) | BIT(0), sirfsoc_timer_base + SIRFSOC_TIMER_64COUNTER_CTRL);
 }
 
-static struct clock_event_device sirfsoc_clockevent = {
-	.name = "sirfsoc_clockevent",
-	.rating = 200,
-	.features = CLOCK_EVT_FEAT_ONESHOT,
-	.set_mode = sirfsoc_timer_set_mode,
-	.set_next_event = sirfsoc_timer_set_next_event,
-};
+static struct clock_event_device __percpu *sirfsoc_clockevent;
 
 static struct clocksource sirfsoc_clocksource = {
 	.name = "sirfsoc_clocksource",
@@ -176,11 +170,8 @@ static struct irqaction sirfsoc_timer_irq = {
 	.name = "sirfsoc_timer0",
 	.flags = IRQF_TIMER | IRQF_NOBALANCING,
 	.handler = sirfsoc_timer_interrupt,
-	.dev_id = &sirfsoc_clockevent,
 };
 
-#ifdef CONFIG_LOCAL_TIMERS
-
 static struct irqaction sirfsoc_timer1_irq = {
 	.name = "sirfsoc_timer1",
 	.flags = IRQF_TIMER | IRQF_NOBALANCING,
@@ -189,56 +180,75 @@ static struct irqaction sirfsoc_timer1_irq = {
 
 static int __cpuinit sirfsoc_local_timer_setup(struct clock_event_device *ce)
 {
-	/* Use existing clock_event for cpu 0 */
-	if (!smp_processor_id())
-		return 0;
+	int cpu = smp_processor_id();
+	struct irqaction *action;
+
+	if (cpu == 0)
+		action = &sirfsoc_timer_irq;
+	else
+		action = &sirfsoc_timer1_irq;
 
-	ce->irq = sirfsoc_timer1_irq.irq;
+	ce->irq = action->irq;
 	ce->name = "local_timer";
-	ce->features = sirfsoc_clockevent.features;
-	ce->rating = sirfsoc_clockevent.rating;
+	ce->features = CLOCK_EVT_FEAT_ONESHOT;
+	ce->rating = 200;
 	ce->set_mode = sirfsoc_timer_set_mode;
 	ce->set_next_event = sirfsoc_timer_set_next_event;
-	ce->shift = sirfsoc_clockevent.shift;
-	ce->mult = sirfsoc_clockevent.mult;
-	ce->max_delta_ns = sirfsoc_clockevent.max_delta_ns;
-	ce->min_delta_ns = sirfsoc_clockevent.min_delta_ns;
+	clockevents_calc_mult_shift(ce, CLOCK_TICK_RATE, 60);
+	ce->max_delta_ns = clockevent_delta2ns(-2, ce);
+	ce->min_delta_ns = clockevent_delta2ns(2, ce);
+	ce->cpumask = cpumask_of(cpu);
 
-	sirfsoc_timer1_irq.dev_id = ce;
-	BUG_ON(setup_irq(ce->irq, &sirfsoc_timer1_irq));
-	irq_set_affinity(sirfsoc_timer1_irq.irq, cpumask_of(1));
+	action->dev_id = ce;
+	BUG_ON(setup_irq(ce->irq, action));
+	irq_set_affinity(action->irq, cpumask_of(cpu));
 
 	clockevents_register_device(ce);
 	return 0;
 }
 
-static void sirfsoc_local_timer_stop(struct clock_event_device *ce)
+static void __cpuinit sirfsoc_local_timer_stop(struct clock_event_device *ce)
 {
+	int cpu = smp_processor_id();
+
 	sirfsoc_timer_count_disable(1);
 
-	remove_irq(sirfsoc_timer1_irq.irq, &sirfsoc_timer1_irq);
+	if (cpu == 0)
+		remove_irq(sirfsoc_timer_irq.irq, &sirfsoc_timer_irq);
+	else
+		remove_irq(sirfsoc_timer1_irq.irq, &sirfsoc_timer1_irq);
+}
+
+static int __cpuinit sirfsoc_cpu_notify(struct notifier_block *self,
+					   unsigned long action, void *hcpu)
+{
+	struct clock_event_device *evt = this_cpu_ptr(sirfsoc_clockevent);
+
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_STARTING:
+		sirfsoc_local_timer_setup(evt);
+		break;
+	case CPU_DYING:
+		sirfsoc_local_timer_stop(evt);
+		break;
+	}
+
+	return NOTIFY_OK;
 }
 
-static struct local_timer_ops sirfsoc_local_timer_ops __cpuinitdata = {
-	.setup	= sirfsoc_local_timer_setup,
-	.stop	= sirfsoc_local_timer_stop,
+static struct notifier_block sirfsoc_cpu_nb __cpuinitdata = {
+	.notifier_call = sirfsoc_cpu_notify,
 };
-#endif /* CONFIG_LOCAL_TIMERS */
 
 static void __init sirfsoc_clockevent_init(void)
 {
-	clockevents_calc_mult_shift(&sirfsoc_clockevent, CLOCK_TICK_RATE, 60);
-
-	sirfsoc_clockevent.max_delta_ns =
-		clockevent_delta2ns(-2, &sirfsoc_clockevent);
-	sirfsoc_clockevent.min_delta_ns =
-		clockevent_delta2ns(2, &sirfsoc_clockevent);
-
-	sirfsoc_clockevent.cpumask = cpumask_of(0);
-	clockevents_register_device(&sirfsoc_clockevent);
-#ifdef CONFIG_LOCAL_TIMERS
-	local_timer_register(&sirfsoc_local_timer_ops);
-#endif
+	sirfsoc_clockevent = alloc_percpu(struct clock_event_device);
+	BUG_ON(!sirfsoc_clockevent);
+
+	BUG_ON(register_cpu_notifier(&sirfsoc_cpu_nb));
+
+	/* Immediately configure the timer on the boot CPU */
+	sirfsoc_local_timer_setup(this_cpu_ptr(sirfsoc_clockevent));
 }
 
 /* initialize the kernel jiffy timer source */
@@ -281,8 +291,6 @@ void __init sirfsoc_marco_timer_init(void)
 
 	BUG_ON(clocksource_register_hz(&sirfsoc_clocksource, CLOCK_TICK_RATE));
 
-	BUG_ON(setup_irq(sirfsoc_timer_irq.irq, &sirfsoc_timer_irq));
-
 	sirfsoc_clockevent_init();
 }
 
@@ -306,11 +314,9 @@ static void __init sirfsoc_of_timer_map(void)
 	if (!sirfsoc_timer_irq.irq)
 		panic("No irq passed for timer0 via DT\n");
 
-#ifdef CONFIG_LOCAL_TIMERS
 	sirfsoc_timer1_irq.irq = irq_of_parse_and_map(np, 1);
 	if (!sirfsoc_timer1_irq.irq)
 		panic("No irq passed for timer1 via DT\n");
-#endif
 
 	of_node_put(np);
 }
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#20 lore raw

PATCH v3 7/10 ARM: msm: Divorce msm_timer from local timer API

In reply to Stephen Boyd

Separate the msm_timer from the local timer API. This will allow us to remove ARM local timer support in the near future and gets us closer to moving this driver to drivers/clocksource.

---
 arch/arm/mach-msm/timer.c | 125 +++++++++++++++++++++++++---------------------
 1 file changed, 67 insertions(+), 58 deletions(-)
arch/arm/mach-msm/timer.c +67 -58
index 2969027..4675c5e 100644
@@ -16,6 +16,7 @@
 
 #include <linux/clocksource.h>
 #include <linux/clockchips.h>
+#include <linux/cpu.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
@@ -25,7 +26,6 @@
 #include <linux/of_irq.h>
 
 #include <asm/mach/time.h>
-#include <asm/localtimer.h>
 #include <asm/sched_clock.h>
 
 #include "common.h"
@@ -46,7 +46,7 @@ static void __iomem *event_base;
 
 static irqreturn_t msm_timer_interrupt(int irq, void *dev_id)
 {
-	struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
+	struct clock_event_device *evt = dev_id;
 	/* Stop the timer tick */
 	if (evt->mode == CLOCK_EVT_MODE_ONESHOT) {
 		u32 ctrl = readl_relaxed(event_base + TIMER_ENABLE);
@@ -90,18 +90,7 @@ static void msm_timer_set_mode(enum clock_event_mode mode,
 	writel_relaxed(ctrl, event_base + TIMER_ENABLE);
 }
 
-static struct clock_event_device msm_clockevent = {
-	.name		= "gp_timer",
-	.features	= CLOCK_EVT_FEAT_ONESHOT,
-	.rating		= 200,
-	.set_next_event	= msm_timer_set_next_event,
-	.set_mode	= msm_timer_set_mode,
-};
-
-static union {
-	struct clock_event_device *evt;
-	struct clock_event_device * __percpu *percpu_evt;
-} msm_evt;
+static struct clock_event_device __percpu *msm_evt;
 
 static void __iomem *source_base;
 
@@ -127,40 +116,66 @@ static struct clocksource msm_clocksource = {
 	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
-#ifdef CONFIG_LOCAL_TIMERS
+static int msm_timer_irq;
+static int msm_timer_has_ppi;
+
 static int __cpuinit msm_local_timer_setup(struct clock_event_device *evt)
 {
-	/* Use existing clock_event for cpu 0 */
-	if (!smp_processor_id())
-		return 0;
+	int cpu = smp_processor_id();
+	int err;
 
 	writel_relaxed(0, event_base + TIMER_ENABLE);
 	writel_relaxed(0, event_base + TIMER_CLEAR);
 	writel_relaxed(~0, event_base + TIMER_MATCH_VAL);
-	evt->irq = msm_clockevent.irq;
+	evt->irq = msm_timer_irq;
 	evt->name = "local_timer";
-	evt->features = msm_clockevent.features;
-	evt->rating = msm_clockevent.rating;
+	evt->features = CLOCK_EVT_FEAT_ONESHOT;
+	evt->rating = 200;
 	evt->set_mode = msm_timer_set_mode;
 	evt->set_next_event = msm_timer_set_next_event;
+	evt->cpumask = cpumask_of(cpu);
+
+	clockevents_config_and_register(evt, GPT_HZ, 4, 0xffffffff);
+
+	if (msm_timer_has_ppi) {
+		enable_percpu_irq(evt->irq, IRQ_TYPE_EDGE_RISING);
+	} else {
+		err = request_irq(evt->irq, msm_timer_interrupt,
+				IRQF_TIMER | IRQF_NOBALANCING |
+				IRQF_TRIGGER_RISING, "gp_timer", evt);
+		if (err)
+			pr_err("request_irq failed\n");
+	}
 
-	*__this_cpu_ptr(msm_evt.percpu_evt) = evt;
-	clockevents_config_and_register(evt, GPT_HZ, 4, 0xf0000000);
-	enable_percpu_irq(evt->irq, IRQ_TYPE_EDGE_RISING);
 	return 0;
 }
 
-static void msm_local_timer_stop(struct clock_event_device *evt)
+static void __cpuinit msm_local_timer_stop(struct clock_event_device *evt)
 {
 	evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
 	disable_percpu_irq(evt->irq);
 }
 
-static struct local_timer_ops msm_local_timer_ops __cpuinitdata = {
-	.setup	= msm_local_timer_setup,
-	.stop	= msm_local_timer_stop,
+static int __cpuinit msm_timer_cpu_notify(struct notifier_block *self,
+					   unsigned long action, void *hcpu)
+{
+	struct clock_event_device *evt = this_cpu_ptr(msm_evt);
+
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_STARTING:
+		msm_local_timer_setup(evt);
+		break;
+	case CPU_DYING:
+		msm_local_timer_stop(evt);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block msm_timer_cpu_nb __cpuinitdata = {
+	.notifier_call = msm_timer_cpu_notify,
 };
-#endif /* CONFIG_LOCAL_TIMERS */
 
 static notrace u32 msm_sched_clock_read(void)
 {
@@ -170,41 +185,35 @@ static notrace u32 msm_sched_clock_read(void)
 static void __init msm_timer_init(u32 dgt_hz, int sched_bits, int irq,
 				  bool percpu)
 {
-	struct clock_event_device *ce = &msm_clockevent;
 	struct clocksource *cs = &msm_clocksource;
-	int res;
+	int res = 0;
 
-	writel_relaxed(0, event_base + TIMER_ENABLE);
-	writel_relaxed(0, event_base + TIMER_CLEAR);
-	writel_relaxed(~0, event_base + TIMER_MATCH_VAL);
-	ce->cpumask = cpumask_of(0);
-	ce->irq = irq;
-
-	clockevents_config_and_register(ce, GPT_HZ, 4, 0xffffffff);
-	if (percpu) {
-		msm_evt.percpu_evt = alloc_percpu(struct clock_event_device *);
-		if (!msm_evt.percpu_evt) {
-			pr_err("memory allocation failed for %s\n", ce->name);
+	msm_timer_irq = irq;
+	msm_timer_has_ppi = percpu;
+
+	msm_evt = alloc_percpu(struct clock_event_device);
+	if (!msm_evt) {
+		pr_err("memory allocation failed for clockevents\n");
+		goto err;
+	}
+
+	if (percpu)
+		res = request_percpu_irq(irq, msm_timer_interrupt,
+					 "gp_timer", msm_evt);
+
+	if (res) {
+		pr_err("request_percpu_irq failed\n");
+	} else {
+		res = register_cpu_notifier(&msm_timer_cpu_nb);
+		if (res) {
+			free_percpu_irq(irq, msm_evt);
 			goto err;
 		}
-		*__this_cpu_ptr(msm_evt.percpu_evt) = ce;
-		res = request_percpu_irq(ce->irq, msm_timer_interrupt,
-					 ce->name, msm_evt.percpu_evt);
-		if (!res) {
-			enable_percpu_irq(ce->irq, IRQ_TYPE_EDGE_RISING);
-#ifdef CONFIG_LOCAL_TIMERS
-			local_timer_register(&msm_local_timer_ops);
-#endif
-		}
-	} else {
-		msm_evt.evt = ce;
-		res = request_irq(ce->irq, msm_timer_interrupt,
-				  IRQF_TIMER | IRQF_NOBALANCING |
-				  IRQF_TRIGGER_RISING, ce->name, &msm_evt.evt);
+
+		/* Immediately configure the timer on the boot CPU */
+		msm_local_timer_setup(this_cpu_ptr(msm_evt));
 	}
 
-	if (res)
-		pr_err("request_irq failed for %s\n", ce->name);
 err:
 	writel_relaxed(TIMER_ENABLE_EN, source_base + TIMER_ENABLE);
 	res = clocksource_register_hz(cs, dgt_hz);
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#21 lore raw

PATCH v3 8/10 clocksource: time-armada-370-xp: Fix sparse warning

In reply to Stephen Boyd

drivers/clocksource/time-armada-370-xp.c:217:13: warning: symbol 'armada_370_xp_timer_init' was not declared. Should it be static?

Also remove the __init marking in the prototype as it's unnecessary and drop the init.h file.

Cc Gregory CLEMENT <[email protected]>
---
 drivers/clocksource/time-armada-370-xp.c | 3 ++-
 include/linux/time-armada-370-xp.h       | 4 +---
 2 files changed, 3 insertions(+), 4 deletions(-)
drivers/clocksource/time-armada-370-xp.c +2 -1
index 47a6730..efe4aef 100644
@@ -27,10 +27,11 @@
 #include <linux/of_address.h>
 #include <linux/irq.h>
 #include <linux/module.h>
+#include <linux/percpu.h>
+#include <linux/time-armada-370-xp.h>
 
 #include <asm/sched_clock.h>
 #include <asm/localtimer.h>
-#include <linux/percpu.h>
 /*
  * Timer block registers.
  */
include/linux/time-armada-370-xp.h +1 -3
index dfdfdc0..6fb0856 100644
@@ -11,8 +11,6 @@
 #ifndef __TIME_ARMADA_370_XPPRCMU_H
 #define __TIME_ARMADA_370_XPPRCMU_H
 
-#include <linux/init.h>
-
-void __init armada_370_xp_timer_init(void);
+void armada_370_xp_timer_init(void);
 
 #endif
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
Acked-by on v3 8/10
#22 lore raw

Re: [PATCHv3 08/10] clocksource: time-armada-370-xp: Fix sparse warning

Quoted 36 lines · Stephen Boyd

drivers/clocksource/time-armada-370-xp.c:217:13: warning: symbol 'armada_370_xp_timer_init' was not declared. Should it be static?

Also remove the __init marking in the prototype as it's unnecessary and drop the init.h file.

Cc Gregory CLEMENT <[email protected]>
---
 drivers/clocksource/time-armada-370-xp.c | 3 ++-
 include/linux/time-armada-370-xp.h       | 4 +---
 2 files changed, 3 insertions(+), 4 deletions(-)
drivers/clocksource/time-armada-370-xp.c +2 -1
index 47a6730..efe4aef 100644
@@ -27,10 +27,11 @@
 #include <linux/of_address.h>
 #include <linux/irq.h>
 #include <linux/module.h>
+#include <linux/percpu.h>
+#include <linux/time-armada-370-xp.h>
 
 #include <asm/sched_clock.h>
 #include <asm/localtimer.h>
-#include <linux/percpu.h>
 /*
  * Timer block registers.
  */
include/linux/time-armada-370-xp.h +0 -0
index dfdfdc0..6fb0856 100644

include/linux/time-armada-370-xp.h line 16

Quoted 9 lines
include/linux/time-armada-370-xp.h +1 -3
@@ -11,8 +11,6 @@
 #ifndef __TIME_ARMADA_370_XPPRCMU_H
 #define __TIME_ARMADA_370_XPPRCMU_H
 
-#include <linux/init.h>
-
-void __init armada_370_xp_timer_init(void);
+void armada_370_xp_timer_init(void);
 
 #endif
Signature · 4 lines
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#23 lore raw

PATCH v3 9/10 clocksource: time-armada-370-xp: Divorce from local timer API

In reply to Stephen Boyd

Separate the armada 370xp local timers from the local timer API. This will allow us to remove ARM local timer support in the near future and makes this driver multi-architecture friendly.

Cc Gregory CLEMENT <[email protected]>
---
 drivers/clocksource/time-armada-370-xp.c | 85 ++++++++++++++------------------
 1 file changed, 38 insertions(+), 47 deletions(-)
drivers/clocksource/time-armada-370-xp.c +38 -47
index efe4aef..ee2e50c5 100644
@@ -19,6 +19,7 @@
 #include <linux/platform_device.h>
 #include <linux/kernel.h>
 #include <linux/clk.h>
+#include <linux/cpu.h>
 #include <linux/timer.h>
 #include <linux/clockchips.h>
 #include <linux/interrupt.h>
@@ -31,7 +32,6 @@
 #include <linux/time-armada-370-xp.h>
 
 #include <asm/sched_clock.h>
-#include <asm/localtimer.h>
 /*
  * Timer block registers.
  */
@@ -70,7 +70,7 @@ static bool timer25Mhz = true;
  */
 static u32 ticks_per_jiffy;
 
-static struct clock_event_device __percpu **percpu_armada_370_xp_evt;
+static struct clock_event_device __percpu *armada_370_xp_evt;
 
 static u32 notrace armada_370_xp_read_sched_clock(void)
 {
@@ -143,14 +143,7 @@ armada_370_xp_clkevt_mode(enum clock_event_mode mode,
 	}
 }
 
-static struct clock_event_device armada_370_xp_clkevt = {
-	.name		= "armada_370_xp_per_cpu_tick",
-	.features	= CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
-	.shift		= 32,
-	.rating		= 300,
-	.set_next_event	= armada_370_xp_clkevt_next_event,
-	.set_mode	= armada_370_xp_clkevt_mode,
-};
+static int armada_370_xp_clkevt_irq;
 
 static irqreturn_t armada_370_xp_timer_interrupt(int irq, void *dev_id)
 {
@@ -173,42 +166,53 @@ static int __cpuinit armada_370_xp_timer_setup(struct clock_event_device *evt)
 	u32 u;
 	int cpu = smp_processor_id();
 
-	/* Use existing clock_event for cpu 0 */
-	if (!smp_processor_id())
-		return 0;
-
 	u = readl(local_base + TIMER_CTRL_OFF);
 	if (timer25Mhz)
 		writel(u | TIMER0_25MHZ, local_base + TIMER_CTRL_OFF);
 	else
 		writel(u & ~TIMER0_25MHZ, local_base + TIMER_CTRL_OFF);
 
-	evt->name		= armada_370_xp_clkevt.name;
-	evt->irq		= armada_370_xp_clkevt.irq;
-	evt->features		= armada_370_xp_clkevt.features;
-	evt->shift		= armada_370_xp_clkevt.shift;
-	evt->rating		= armada_370_xp_clkevt.rating,
+	evt->name		= "armada_370_xp_per_cpu_tick",
+	evt->features		= CLOCK_EVT_FEAT_ONESHOT |
+				  CLOCK_EVT_FEAT_PERIODIC;
+	evt->shift		= 32,
+	evt->rating		= 300,
 	evt->set_next_event	= armada_370_xp_clkevt_next_event,
 	evt->set_mode		= armada_370_xp_clkevt_mode,
+	evt->irq		= armada_370_xp_clkevt_irq;
 	evt->cpumask		= cpumask_of(cpu);
 
-	*__this_cpu_ptr(percpu_armada_370_xp_evt) = evt;
-
 	clockevents_config_and_register(evt, timer_clk, 1, 0xfffffffe);
 	enable_percpu_irq(evt->irq, 0);
 
 	return 0;
 }
 
-static void  armada_370_xp_timer_stop(struct clock_event_device *evt)
+static void __cpuinit armada_370_xp_timer_stop(struct clock_event_device *evt)
 {
 	evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
 	disable_percpu_irq(evt->irq);
 }
 
-static struct local_timer_ops armada_370_xp_local_timer_ops __cpuinitdata = {
-	.setup	= armada_370_xp_timer_setup,
-	.stop	=  armada_370_xp_timer_stop,
+static int __cpuinit armada_370_xp_timer_cpu_notify(struct notifier_block *self,
+					   unsigned long action, void *hcpu)
+{
+	struct clock_event_device *evt = this_cpu_ptr(armada_370_xp_evt);
+
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_STARTING:
+		armada_370_xp_timer_setup(evt);
+		break;
+	case CPU_DYING:
+		armada_370_xp_timer_stop(evt);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block armada_370_xp_timer_cpu_nb __cpuinitdata = {
+	.notifier_call = armada_370_xp_timer_cpu_notify,
 };
 
 void __init armada_370_xp_timer_init(void)
@@ -224,9 +228,6 @@ void __init armada_370_xp_timer_init(void)
 
 	if (of_find_property(np, "marvell,timer-25Mhz", NULL)) {
 		/* The fixed 25MHz timer is available so let's use it */
-		u = readl(local_base + TIMER_CTRL_OFF);
-		writel(u | TIMER0_25MHZ,
-		       local_base + TIMER_CTRL_OFF);
 		u = readl(timer_base + TIMER_CTRL_OFF);
 		writel(u | TIMER0_25MHZ,
 		       timer_base + TIMER_CTRL_OFF);
@@ -236,9 +237,6 @@ void __init armada_370_xp_timer_init(void)
 		struct clk *clk = of_clk_get(np, 0);
 		WARN_ON(IS_ERR(clk));
 		rate =  clk_get_rate(clk);
-		u = readl(local_base + TIMER_CTRL_OFF);
-		writel(u & ~(TIMER0_25MHZ),
-		       local_base + TIMER_CTRL_OFF);
 
 		u = readl(timer_base + TIMER_CTRL_OFF);
 		writel(u & ~(TIMER0_25MHZ),
@@ -252,7 +250,7 @@ void __init armada_370_xp_timer_init(void)
 	 * We use timer 0 as clocksource, and private(local) timer 0
 	 * for clockevents
 	 */
-	armada_370_xp_clkevt.irq = irq_of_parse_and_map(np, 4);
+	armada_370_xp_clkevt_irq = irq_of_parse_and_map(np, 4);
 
 	ticks_per_jiffy = (timer_clk + HZ / 2) / HZ;
 
@@ -277,26 +275,19 @@ void __init armada_370_xp_timer_init(void)
 			      "armada_370_xp_clocksource",
 			      timer_clk, 300, 32, clocksource_mmio_readl_down);
 
-	/* Register the clockevent on the private timer of CPU 0 */
-	armada_370_xp_clkevt.cpumask = cpumask_of(0);
-	clockevents_config_and_register(&armada_370_xp_clkevt,
-					timer_clk, 1, 0xfffffffe);
+	register_cpu_notifier(&armada_370_xp_timer_cpu_nb);
 
-	percpu_armada_370_xp_evt = alloc_percpu(struct clock_event_device *);
+	armada_370_xp_evt = alloc_percpu(struct clock_event_device);
 
 
 	/*
 	 * Setup clockevent timer (interrupt-driven).
 	 */
-	*__this_cpu_ptr(percpu_armada_370_xp_evt) = &armada_370_xp_clkevt;
-	res = request_percpu_irq(armada_370_xp_clkevt.irq,
+	res = request_percpu_irq(armada_370_xp_clkevt_irq,
 				armada_370_xp_timer_interrupt,
-				armada_370_xp_clkevt.name,
-				percpu_armada_370_xp_evt);
-	if (!res) {
-		enable_percpu_irq(armada_370_xp_clkevt.irq, 0);
-#ifdef CONFIG_LOCAL_TIMERS
-		local_timer_register(&armada_370_xp_local_timer_ops);
-#endif
-	}
+				"armada_370_xp_per_cpu_tick",
+				armada_370_xp_evt);
+	/* Immediately configure the timer on the boot CPU */
+	if (!res)
+		armada_370_xp_timer_setup(this_cpu_ptr(armada_370_xp_evt));
 }
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#24 lore raw

Re: [PATCHv3 09/10] clocksource: time-armada-370-xp: Divorce from local timer API

Quoted 3 lines · Stephen Boyd

Separate the armada 370xp local timers from the local timer API. This will allow us to remove ARM local timer support in the near future and makes this driver multi-architecture friendly.

At first view the code looks good, but when I applied your patch set on linux-next, build it and run it on a Armada XP based board (AX3 with 2 cores), it crashed:

Booting Linux on physical CPU 0x0
Linux version 3.9.0-rc3-next-20130319-00010-g728b448 (gclement at FE-greg-laptop) (gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5) ) #153 SMP Wed Mar 20 17:58:57 CET 2013
CPU: ARMv7 Processor [562f5842] revision 2 (ARMv7), cr=10c53c7d
CPU: PIPT / VIPT nonaliasing data cache, PIPT instruction cache
Machine: Marvell Armada 370/XP (Device Tree), model: PlatHome OpenBlocks AX3-4 board
bootconsole [earlycon0] enabled
Memory policy: ECC disabled, Data cache writealloc
PERCPU: Embedded 7 pages/cpu @c22b0000 s7104 r8192 d13376 u32768
Built 1 zonelists in Zone order, mobility grouping on. Total pages: 784912
Kernel command line: console=ttyS0,115200 earlyprintk
PID hash table entries: 4096 (order: 2, 16384 bytes)
Dentry cache hash table entries: 131072 (order: 7, 524288 bytes)
Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
__ex_table already sorted, skipping sort
Memory: 3072MB = 3072MB total
Memory: 3113520k/3113520k available, 32208k reserved, 2367488K highmem
Virtual kernel memory layout:

    vector  : 0xffff0000 - 0xffff1000   (   4 kB)
    fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
    vmalloc : 0xf0000000 - 0xff000000   ( 240 MB)
    lowmem  : 0xc0000000 - 0xef800000   ( 760 MB)
    pkmap   : 0xbfe00000 - 0xc0000000   (   2 MB)
    modules : 0xbf000000 - 0xbfe00000   (  14 MB)
      .text : 0xc0008000 - 0xc041647c   (4154 kB)
      .init : 0xc0417000 - 0xc0633bc0   (2163 kB)
      .data : 0xc0634000 - 0xc06605c0   ( 178 kB)
       .bss : 0xc06605c0 - 0xc0686a2c   ( 154 kB)

Hierarchical RCU implementation.
RCU restricting CPUs from NR_CPUS=4 to nr_cpu_ids=2.
NR_IRQS:16 nr_irqs:16 16
Aurora cache controller enabled
l2x0: 16 ways, CACHE_ID 0x00000100, AUX_CTRL 0x1a696b12, Cache size: 1048576 B
sched_clock: 32 bits at 25MHz, resolution 40ns, wraps every 171798ms
Console: colour dummy device 80x30
Calibrating delay loop...

Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
Modules linked in:
CPU: 0    Not tainted  (3.9.0-rc3-next-20130319-00010-g728b448 #153)
PC is at 0xe92d45f0
LR is at armada_370_xp_timer_interrupt+0x3c/0x4c
pc : [<e92d45f0>]    lr : [<c023c2bc>]    psr: 600001d3

sp : c0635eb8 ip : 00000000 fp : c063c3f0
r10: 000003ff r9 : 00000000 r8 : 00000010
r7 : c22b3c40 r6 : ef007c00 r5 : c0640fcc r4 : c0053e30 r3 : e92d45f0 r2 : fffffffe r1 : c22b3c40 r0 : c0053e30
Flags: nZCv IRQs off FIQs off Mode SVC_32 ISA ARM Segment kernel
Control: 10c53c7d Table: 0000406a DAC: 00000015
Process swapper/0 (pid: 0, stack limit = 0xc0634238)

Stack: (0xc0635eb8 to 0xc0636000)

5ea0: ef004c80 c0063224
5ec0: 00000010 00000010 00000000 c0660ac0 c0635f18 c005fcb8 c0632b90 c000ed94
5ee0: c0313c60 60000153 00000001 c00085a8 c0313c54 c0313c60 60000153 ffffffff
5f00: c0635f4c 00000000 562f5842 c06360c0 00000000 c000db60 0000001a ffff8ad0
5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
5f60: 00000021 00000000 00000003 00000004 0000006e c065fcc0 c067924c c063ceb8
5f80: c063cc84 c006d8c0 00000005 c065fcc0 c067924c c0421764 c22ad780 c063c42c
5fa0: 562f5842 c063cca8 c06605c0 c04379c0 c22ad780 00000000 562f5842 00000000
5fc0: 00000000 c0417754 ffffffff ffffffff c04172dc 00000000 00000000 c04379c0
5fe0: 10c53c7d c063c414 c04379bc c063febc 0000406a 00008074 00000000 00000000
[<c023c2bc>] (armada_370_xp_timer_interrupt+0x3c/0x4c) from [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) from [<c005fcb8>] (generic_handle_irq+0x20/0x30) [<c005fcb8>] (generic_handle_irq+0x20/0x30) from [<c000ed94>] (handle_IRQ+0x38/0x90) [<c000ed94>] (handle_IRQ+0x38/0x90) from [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) from [<c000db60>] (__irq_svc+0x40/0x50)

Exception stack(0xc0635f18 to 0xc0635f60)

5f00: 0000001a ffff8ad0
5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842 5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff [<c000db60>] (__irq_svc+0x40/0x50) from [<c0313c60>] (calibrate_delay+0x378/0x528) [<c0313c60>] (calibrate_delay+0x378/0x528) from [<c0417754>] (start_kernel+0x250/0x2dc) [<c0417754>] (start_kernel+0x250/0x2dc) from [<00008074>] (0x8074)

Code: 1fe7deb7 cd5772dd fff5692e ed55f79e (7ed5a5f7)
---[ end trace 1b75b31a2719ed1c ]---
Kernel panic - not syncing: Fatal exception in interrupt
Internal error: Oops - undefined instruction: 0 [#2] SMP ARM
Modules linked in:
CPU: 0    Tainted: G      D       (3.9.0-rc3-next-20130319-00010-g728b448 #153)
PC is at 0xe92d45f0
LR is at armada_370_xp_timer_interrupt+0x3c/0x4c
pc : [<e92d45f0>]    lr : [<c023c2bc>]    psr: 600001d3

sp : c0635cc0 ip : 00000000 fp : c063c3f0
r10: 000003ff r9 : 00000000 r8 : 00000010
r7 : c22b3c40 r6 : ef007c00 r5 : c0640fcc r4 : c0053e30 r3 : e92d45f0 r2 : fffffffe r1 : c22b3c40 r0 : c0053e30
Flags: nZCv IRQs off FIQs off Mode SVC_32 ISA ARM Segment kernel
Control: 10c53c7d Table: 0000406a DAC: 00000015
Process swapper/0 (pid: 0, stack limit = 0xc0634238)

Stack: (0xc0635cc0 to 0xc0636000)

5cc0: ef004c80 c0063224 00000010 00000010 c0635f18 c0660ac0 c0635d20 c005fcb8
5ce0: c0632b90 c000ed94 c0316d54 60000153 00000001 c00085a8 c0316ca4 c0316d54
5d00: 60000153 ffffffff c0635d54 c03a53b0 00000000 c0660b70 600001d3 c000db60
5d20: c0660fc0 00000000 c0642f80 00000000 00000000 00000000 0000000b c06400e0
5d40: c03a53b0 00000000 c0660b70 600001d3 00000000 c0635d68 c0316ca4 c0316d54
5d60: 60000153 ffffffff 600001d3 c0635d94 c06608c0 c0634000 0000000b c06400e0
5d80: c03a53b0 00000000 c063f180 c0011624 c03a5310 200001d3 c0642f80 00010000
5da0: c0634238 0000000b 00100100 c0635e70 e92d45f0 7ed5a5f7 00000001 00000500
5dc0: c000dbcc c0634000 c063c3f0 c00082a0 00000006 c22b0768 00000004 00000000
5de0: 00030001 e92d45f0 ffffffff 00000001 c22b076c 00000000 c065fa1c c065f9c0
5e00: 00100100 00000002 c065f9c0 00000000 c063cca0 00000000 00000002 c065f9c0
5e20: c0661ca4 c06618e6 00000041 00000002 0000000a 00000002 ffffffff 00000000
5e40: 00000000 00000000 00000000 00000000 c06602cc 00000000 c00146d4 e92d45f4
5e60: 600001d3 c0634050 00000001 c000dbcc c0053e30 c22b3c40 fffffffe e92d45f0
5e80: c0053e30 c0640fcc ef007c00 c22b3c40 00000010 00000000 000003ff c063c3f0
5ea0: 00000000 c0635eb8 c023c2bc e92d45f0 600001d3 ffffffff ef004c80 c0063224
5ec0: 00000010 00000010 00000000 c0660ac0 c0635f18 c005fcb8 c0632b90 c000ed94
5ee0: c0313c60 60000153 00000001 c00085a8 c0313c54 c0313c60 60000153 ffffffff
5f00: c0635f4c 00000000 562f5842 c06360c0 00000000 c000db60 0000001a ffff8ad0
5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
5f60: 00000021 00000000 00000003 00000004 0000006e c065fcc0 c067924c c063ceb8
5f80: c063cc84 c006d8c0 00000005 c065fcc0 c067924c c0421764 c22ad780 c063c42c
5fa0: 562f5842 c063cca8 c06605c0 c04379c0 c22ad780 00000000 562f5842 00000000
5fc0: 00000000 c0417754 ffffffff ffffffff c04172dc 00000000 00000000 c04379c0
5fe0: 10c53c7d c063c414 c04379bc c063febc 0000406a 00008074 00000000 00000000
[<c023c2bc>] (armada_370_xp_timer_interrupt+0x3c/0x4c) from [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) from [<c005fcb8>] (generic_handle_irq+0x20/0x30) [<c005fcb8>] (generic_handle_irq+0x20/0x30) from [<c000ed94>] (handle_IRQ+0x38/0x90) [<c000ed94>] (handle_IRQ+0x38/0x90) from [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) from [<c000db60>] (__irq_svc+0x40/0x50)

Exception stack(0xc0635d20 to 0xc0635d68)

5d20: c0660fc0 00000000 c0642f80 00000000 00000000 00000000 0000000b c06400e0 5d40: c03a53b0 00000000 c0660b70 600001d3 00000000 c0635d68 c0316ca4 c0316d54 5d60: 60000153 ffffffff
[<c000db60>] (__irq_svc+0x40/0x50) from [<c0316d54>] (panic+0x144/0x1b0) [<c0316d54>] (panic+0x144/0x1b0) from [<c0011624>] (die+0x278/0x2a8) [<c0011624>] (die+0x278/0x2a8) from [<c00082a0>] (do_undefinstr+0x9c/0x1c4) [<c00082a0>] (do_undefinstr+0x9c/0x1c4) from [<c000dbcc>] (__und_svc_finish+0x0/0x14)

Exception stack(0xc0635e70 to 0xc0635eb8)

5e60: c0053e30 c22b3c40 fffffffe e92d45f0
5e80: c0053e30 c0640fcc ef007c00 c22b3c40 00000010 00000000 000003ff c063c3f0 5ea0: 00000000 c0635eb8 c023c2bc e92d45f0 600001d3 ffffffff [<c000dbcc>] (__und_svc_finish+0x0/0x14) from [<e92d45f0>] (0xe92d45f0)

Code: 1fe7deb7 cd5772dd fff5692e ed55f79e (7ed5a5f7)
---[ end trace 1b75b31a2719ed1d ]---

I am trying to figure out what happened.

Quoted 186 lines
Cc Gregory CLEMENT <[email protected]>
---
 drivers/clocksource/time-armada-370-xp.c | 85 ++++++++++++++------------------
 1 file changed, 38 insertions(+), 47 deletions(-)
drivers/clocksource/time-armada-370-xp.c +38 -47
index efe4aef..ee2e50c5 100644
@@ -19,6 +19,7 @@
 #include <linux/platform_device.h>
 #include <linux/kernel.h>
 #include <linux/clk.h>
+#include <linux/cpu.h>
 #include <linux/timer.h>
 #include <linux/clockchips.h>
 #include <linux/interrupt.h>
@@ -31,7 +32,6 @@
 #include <linux/time-armada-370-xp.h>
 
 #include <asm/sched_clock.h>
-#include <asm/localtimer.h>
 /*
  * Timer block registers.
  */
@@ -70,7 +70,7 @@ static bool timer25Mhz = true;
  */
 static u32 ticks_per_jiffy;
 
-static struct clock_event_device __percpu **percpu_armada_370_xp_evt;
+static struct clock_event_device __percpu *armada_370_xp_evt;
 
 static u32 notrace armada_370_xp_read_sched_clock(void)
 {
@@ -143,14 +143,7 @@ armada_370_xp_clkevt_mode(enum clock_event_mode mode,
 	}
 }
 
-static struct clock_event_device armada_370_xp_clkevt = {
-	.name		= "armada_370_xp_per_cpu_tick",
-	.features	= CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
-	.shift		= 32,
-	.rating		= 300,
-	.set_next_event	= armada_370_xp_clkevt_next_event,
-	.set_mode	= armada_370_xp_clkevt_mode,
-};
+static int armada_370_xp_clkevt_irq;
 
 static irqreturn_t armada_370_xp_timer_interrupt(int irq, void *dev_id)
 {
@@ -173,42 +166,53 @@ static int __cpuinit armada_370_xp_timer_setup(struct clock_event_device *evt)
 	u32 u;
 	int cpu = smp_processor_id();
 
-	/* Use existing clock_event for cpu 0 */
-	if (!smp_processor_id())
-		return 0;
-
 	u = readl(local_base + TIMER_CTRL_OFF);
 	if (timer25Mhz)
 		writel(u | TIMER0_25MHZ, local_base + TIMER_CTRL_OFF);
 	else
 		writel(u & ~TIMER0_25MHZ, local_base + TIMER_CTRL_OFF);
 
-	evt->name		= armada_370_xp_clkevt.name;
-	evt->irq		= armada_370_xp_clkevt.irq;
-	evt->features		= armada_370_xp_clkevt.features;
-	evt->shift		= armada_370_xp_clkevt.shift;
-	evt->rating		= armada_370_xp_clkevt.rating,
+	evt->name		= "armada_370_xp_per_cpu_tick",
+	evt->features		= CLOCK_EVT_FEAT_ONESHOT |
+				  CLOCK_EVT_FEAT_PERIODIC;
+	evt->shift		= 32,
+	evt->rating		= 300,
 	evt->set_next_event	= armada_370_xp_clkevt_next_event,
 	evt->set_mode		= armada_370_xp_clkevt_mode,
+	evt->irq		= armada_370_xp_clkevt_irq;
 	evt->cpumask		= cpumask_of(cpu);
 
-	*__this_cpu_ptr(percpu_armada_370_xp_evt) = evt;
-
 	clockevents_config_and_register(evt, timer_clk, 1, 0xfffffffe);
 	enable_percpu_irq(evt->irq, 0);
 
 	return 0;
 }
 
-static void  armada_370_xp_timer_stop(struct clock_event_device *evt)
+static void __cpuinit armada_370_xp_timer_stop(struct clock_event_device *evt)
 {
 	evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
 	disable_percpu_irq(evt->irq);
 }
 
-static struct local_timer_ops armada_370_xp_local_timer_ops __cpuinitdata = {
-	.setup	= armada_370_xp_timer_setup,
-	.stop	=  armada_370_xp_timer_stop,
+static int __cpuinit armada_370_xp_timer_cpu_notify(struct notifier_block *self,
+					   unsigned long action, void *hcpu)
+{
+	struct clock_event_device *evt = this_cpu_ptr(armada_370_xp_evt);
+
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_STARTING:
+		armada_370_xp_timer_setup(evt);
+		break;
+	case CPU_DYING:
+		armada_370_xp_timer_stop(evt);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block armada_370_xp_timer_cpu_nb __cpuinitdata = {
+	.notifier_call = armada_370_xp_timer_cpu_notify,
 };
 
 void __init armada_370_xp_timer_init(void)
@@ -224,9 +228,6 @@ void __init armada_370_xp_timer_init(void)
 
 	if (of_find_property(np, "marvell,timer-25Mhz", NULL)) {
 		/* The fixed 25MHz timer is available so let's use it */
-		u = readl(local_base + TIMER_CTRL_OFF);
-		writel(u | TIMER0_25MHZ,
-		       local_base + TIMER_CTRL_OFF);
 		u = readl(timer_base + TIMER_CTRL_OFF);
 		writel(u | TIMER0_25MHZ,
 		       timer_base + TIMER_CTRL_OFF);
@@ -236,9 +237,6 @@ void __init armada_370_xp_timer_init(void)
 		struct clk *clk = of_clk_get(np, 0);
 		WARN_ON(IS_ERR(clk));
 		rate =  clk_get_rate(clk);
-		u = readl(local_base + TIMER_CTRL_OFF);
-		writel(u & ~(TIMER0_25MHZ),
-		       local_base + TIMER_CTRL_OFF);
 
 		u = readl(timer_base + TIMER_CTRL_OFF);
 		writel(u & ~(TIMER0_25MHZ),
@@ -252,7 +250,7 @@ void __init armada_370_xp_timer_init(void)
 	 * We use timer 0 as clocksource, and private(local) timer 0
 	 * for clockevents
 	 */
-	armada_370_xp_clkevt.irq = irq_of_parse_and_map(np, 4);
+	armada_370_xp_clkevt_irq = irq_of_parse_and_map(np, 4);
 
 	ticks_per_jiffy = (timer_clk + HZ / 2) / HZ;
 
@@ -277,26 +275,19 @@ void __init armada_370_xp_timer_init(void)
 			      "armada_370_xp_clocksource",
 			      timer_clk, 300, 32, clocksource_mmio_readl_down);
 
-	/* Register the clockevent on the private timer of CPU 0 */
-	armada_370_xp_clkevt.cpumask = cpumask_of(0);
-	clockevents_config_and_register(&armada_370_xp_clkevt,
-					timer_clk, 1, 0xfffffffe);
+	register_cpu_notifier(&armada_370_xp_timer_cpu_nb);
 
-	percpu_armada_370_xp_evt = alloc_percpu(struct clock_event_device *);
+	armada_370_xp_evt = alloc_percpu(struct clock_event_device);
 
 
 	/*
 	 * Setup clockevent timer (interrupt-driven).
 	 */
-	*__this_cpu_ptr(percpu_armada_370_xp_evt) = &armada_370_xp_clkevt;
-	res = request_percpu_irq(armada_370_xp_clkevt.irq,
+	res = request_percpu_irq(armada_370_xp_clkevt_irq,
 				armada_370_xp_timer_interrupt,
-				armada_370_xp_clkevt.name,
-				percpu_armada_370_xp_evt);
-	if (!res) {
-		enable_percpu_irq(armada_370_xp_clkevt.irq, 0);
-#ifdef CONFIG_LOCAL_TIMERS
-		local_timer_register(&armada_370_xp_local_timer_ops);
-#endif
-	}
+				"armada_370_xp_per_cpu_tick",
+				armada_370_xp_evt);
+	/* Immediately configure the timer on the boot CPU */
+	if (!res)
+		armada_370_xp_timer_setup(this_cpu_ptr(armada_370_xp_evt));
 }
Signature · 4 lines
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#25 lore raw

Re: [PATCHv3 09/10] clocksource: time-armada-370-xp: Divorce from local timer API

Quoted 7 lines · Gregory CLEMENT
Quoted 3 lines · Stephen Boyd

Separate the armada 370xp local timers from the local timer API. This will allow us to remove ARM local timer support in the near future and makes this driver multi-architecture friendly.

At first view the code looks good, but when I applied your patch set on linux-next, build it and run it on a Armada XP based board (AX3 with 2 cores), it crashed:

[...]

Quoted 37 lines
Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
Modules linked in:
CPU: 0    Not tainted  (3.9.0-rc3-next-20130319-00010-g728b448 #153)
PC is at 0xe92d45f0
LR is at armada_370_xp_timer_interrupt+0x3c/0x4c
pc : [<e92d45f0>]    lr : [<c023c2bc>]    psr: 600001d3

sp : c0635eb8 ip : 00000000 fp : c063c3f0
r10: 000003ff r9 : 00000000 r8 : 00000010
r7 : c22b3c40 r6 : ef007c00 r5 : c0640fcc r4 : c0053e30 r3 : e92d45f0 r2 : fffffffe r1 : c22b3c40 r0 : c0053e30
Flags: nZCv IRQs off FIQs off Mode SVC_32 ISA ARM Segment kernel
Control: 10c53c7d Table: 0000406a DAC: 00000015
Process swapper/0 (pid: 0, stack limit = 0xc0634238)

Stack: (0xc0635eb8 to 0xc0636000)

5ea0: ef004c80 c0063224
5ec0: 00000010 00000010 00000000 c0660ac0 c0635f18 c005fcb8 c0632b90 c000ed94
5ee0: c0313c60 60000153 00000001 c00085a8 c0313c54 c0313c60 60000153 ffffffff
5f00: c0635f4c 00000000 562f5842 c06360c0 00000000 c000db60 0000001a ffff8ad0
5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
5f60: 00000021 00000000 00000003 00000004 0000006e c065fcc0 c067924c c063ceb8
5f80: c063cc84 c006d8c0 00000005 c065fcc0 c067924c c0421764 c22ad780 c063c42c
5fa0: 562f5842 c063cca8 c06605c0 c04379c0 c22ad780 00000000 562f5842 00000000
5fc0: 00000000 c0417754 ffffffff ffffffff c04172dc 00000000 00000000 c04379c0
5fe0: 10c53c7d c063c414 c04379bc c063febc 0000406a 00008074 00000000 00000000
[<c023c2bc>] (armada_370_xp_timer_interrupt+0x3c/0x4c) from [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) from [<c005fcb8>] (generic_handle_irq+0x20/0x30) [<c005fcb8>] (generic_handle_irq+0x20/0x30) from [<c000ed94>] (handle_IRQ+0x38/0x90) [<c000ed94>] (handle_IRQ+0x38/0x90) from [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) from [<c000db60>] (__irq_svc+0x40/0x50)

Exception stack(0xc0635f18 to 0xc0635f60)

5f00: 0000001a ffff8ad0
5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842 5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff [<c000db60>] (__irq_svc+0x40/0x50) from [<c0313c60>] (calibrate_delay+0x378/0x528) [<c0313c60>] (calibrate_delay+0x378/0x528) from [<c0417754>] (start_kernel+0x250/0x2dc) [<c0417754>] (start_kernel+0x250/0x2dc) from [<00008074>] (0x8074)

Quoted 5 lines
Code: 1fe7deb7 cd5772dd fff5692e ed55f79e (7ed5a5f7)

I am trying to figure out what happened.

Argh. Stupid casting again. Can you try this?

---8<----

drivers/clocksource/time-armada-370-xp.c +1 -1
index ee2e50c5..a9bf37a 100644
@@ -150,7 +150,7 @@ static irqreturn_t armada_370_xp_timer_interrupt(int irq, void *dev_id)
        /*
         * ACK timer interrupt and call event handler.
         */
-       struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
+       struct clock_event_device *evt = dev_id;
 
        writel(TIMER0_CLR_MASK, local_base + LCL_TIMER_EVENTS_STATUS);
        evt->event_handler(evt);
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#26 lore raw

Re: [PATCHv3 09/10] clocksource: time-armada-370-xp: Divorce from local timer API

Quoted 50 lines · Stephen Boyd
Quoted 7 lines · Gregory CLEMENT
Quoted 3 lines · Stephen Boyd

Separate the armada 370xp local timers from the local timer API. This will allow us to remove ARM local timer support in the near future and makes this driver multi-architecture friendly.

At first view the code looks good, but when I applied your patch set on linux-next, build it and run it on a Armada XP based board (AX3 with 2 cores), it crashed:

[...]

Quoted 41 lines
Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
Modules linked in:
CPU: 0    Not tainted  (3.9.0-rc3-next-20130319-00010-g728b448 #153)
PC is at 0xe92d45f0
LR is at armada_370_xp_timer_interrupt+0x3c/0x4c
pc : [<e92d45f0>]    lr : [<c023c2bc>]    psr: 600001d3

sp : c0635eb8 ip : 00000000 fp : c063c3f0
r10: 000003ff r9 : 00000000 r8 : 00000010
r7 : c22b3c40 r6 : ef007c00 r5 : c0640fcc r4 : c0053e30 r3 : e92d45f0 r2 : fffffffe r1 : c22b3c40 r0 : c0053e30
Flags: nZCv IRQs off FIQs off Mode SVC_32 ISA ARM Segment kernel
Control: 10c53c7d Table: 0000406a DAC: 00000015
Process swapper/0 (pid: 0, stack limit = 0xc0634238)

Stack: (0xc0635eb8 to 0xc0636000)

5ea0: ef004c80 c0063224
5ec0: 00000010 00000010 00000000 c0660ac0 c0635f18 c005fcb8 c0632b90 c000ed94
5ee0: c0313c60 60000153 00000001 c00085a8 c0313c54 c0313c60 60000153 ffffffff
5f00: c0635f4c 00000000 562f5842 c06360c0 00000000 c000db60 0000001a ffff8ad0
5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
5f60: 00000021 00000000 00000003 00000004 0000006e c065fcc0 c067924c c063ceb8
5f80: c063cc84 c006d8c0 00000005 c065fcc0 c067924c c0421764 c22ad780 c063c42c
5fa0: 562f5842 c063cca8 c06605c0 c04379c0 c22ad780 00000000 562f5842 00000000
5fc0: 00000000 c0417754 ffffffff ffffffff c04172dc 00000000 00000000 c04379c0
5fe0: 10c53c7d c063c414 c04379bc c063febc 0000406a 00008074 00000000 00000000
[<c023c2bc>] (armada_370_xp_timer_interrupt+0x3c/0x4c) from [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) from [<c005fcb8>] (generic_handle_irq+0x20/0x30) [<c005fcb8>] (generic_handle_irq+0x20/0x30) from [<c000ed94>] (handle_IRQ+0x38/0x90) [<c000ed94>] (handle_IRQ+0x38/0x90) from [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) from [<c000db60>] (__irq_svc+0x40/0x50)

Exception stack(0xc0635f18 to 0xc0635f60)

5f00: 0000001a ffff8ad0
5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842 5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff [<c000db60>] (__irq_svc+0x40/0x50) from [<c0313c60>] (calibrate_delay+0x378/0x528) [<c0313c60>] (calibrate_delay+0x378/0x528) from [<c0417754>] (start_kernel+0x250/0x2dc) [<c0417754>] (start_kernel+0x250/0x2dc) from [<00008074>] (0x8074)

Code: 1fe7deb7 cd5772dd fff5692e ed55f79e (7ed5a5f7)
Quoted 3 lines
Quoted 1 line

I am trying to figure out what happened.

Argh. Stupid casting again. Can you try this?

Our emails must have crossed, your fix is also fine :)

Quoted 17 lines

---8<----

drivers/clocksource/time-armada-370-xp.c +1 -1
index ee2e50c5..a9bf37a 100644
@@ -150,7 +150,7 @@ static irqreturn_t armada_370_xp_timer_interrupt(int irq, void *dev_id)
        /*
         * ACK timer interrupt and call event handler.
         */
-       struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
+       struct clock_event_device *evt = dev_id;
 
        writel(TIMER0_CLR_MASK, local_base + LCL_TIMER_EVENTS_STATUS);
        evt->event_handler(evt);
Signature · 4 lines
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
Acked-by Tested-by
#27 lore raw

Re: [PATCHv3 09/10] clocksource: time-armada-370-xp: Divorce from local timer API

Quoted 51 lines · Gregory CLEMENT
Quoted 50 lines · Stephen Boyd
Quoted 7 lines · Gregory CLEMENT
Quoted 3 lines · Stephen Boyd

Separate the armada 370xp local timers from the local timer API. This will allow us to remove ARM local timer support in the near future and makes this driver multi-architecture friendly.

At first view the code looks good, but when I applied your patch set on linux-next, build it and run it on a Armada XP based board (AX3 with 2 cores), it crashed:

[...]

Quoted 41 lines
Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
Modules linked in:
CPU: 0    Not tainted  (3.9.0-rc3-next-20130319-00010-g728b448 #153)
PC is at 0xe92d45f0
LR is at armada_370_xp_timer_interrupt+0x3c/0x4c
pc : [<e92d45f0>]    lr : [<c023c2bc>]    psr: 600001d3

sp : c0635eb8 ip : 00000000 fp : c063c3f0
r10: 000003ff r9 : 00000000 r8 : 00000010
r7 : c22b3c40 r6 : ef007c00 r5 : c0640fcc r4 : c0053e30 r3 : e92d45f0 r2 : fffffffe r1 : c22b3c40 r0 : c0053e30
Flags: nZCv IRQs off FIQs off Mode SVC_32 ISA ARM Segment kernel
Control: 10c53c7d Table: 0000406a DAC: 00000015
Process swapper/0 (pid: 0, stack limit = 0xc0634238)

Stack: (0xc0635eb8 to 0xc0636000)

5ea0: ef004c80 c0063224
5ec0: 00000010 00000010 00000000 c0660ac0 c0635f18 c005fcb8 c0632b90 c000ed94
5ee0: c0313c60 60000153 00000001 c00085a8 c0313c54 c0313c60 60000153 ffffffff
5f00: c0635f4c 00000000 562f5842 c06360c0 00000000 c000db60 0000001a ffff8ad0
5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
5f60: 00000021 00000000 00000003 00000004 0000006e c065fcc0 c067924c c063ceb8
5f80: c063cc84 c006d8c0 00000005 c065fcc0 c067924c c0421764 c22ad780 c063c42c
5fa0: 562f5842 c063cca8 c06605c0 c04379c0 c22ad780 00000000 562f5842 00000000
5fc0: 00000000 c0417754 ffffffff ffffffff c04172dc 00000000 00000000 c04379c0
5fe0: 10c53c7d c063c414 c04379bc c063febc 0000406a 00008074 00000000 00000000
[<c023c2bc>] (armada_370_xp_timer_interrupt+0x3c/0x4c) from [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) from [<c005fcb8>] (generic_handle_irq+0x20/0x30) [<c005fcb8>] (generic_handle_irq+0x20/0x30) from [<c000ed94>] (handle_IRQ+0x38/0x90) [<c000ed94>] (handle_IRQ+0x38/0x90) from [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) from [<c000db60>] (__irq_svc+0x40/0x50)

Exception stack(0xc0635f18 to 0xc0635f60)

5f00: 0000001a ffff8ad0
5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842 5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff [<c000db60>] (__irq_svc+0x40/0x50) from [<c0313c60>] (calibrate_delay+0x378/0x528) [<c0313c60>] (calibrate_delay+0x378/0x528) from [<c0417754>] (start_kernel+0x250/0x2dc) [<c0417754>] (start_kernel+0x250/0x2dc) from [<00008074>] (0x8074)

Code: 1fe7deb7 cd5772dd fff5692e ed55f79e (7ed5a5f7)
Quoted 6 lines
Quoted 3 lines
Quoted 1 line

I am trying to figure out what happened.

Argh. Stupid casting again. Can you try this?

Our emails must have crossed, your fix is also fine :)

I have also tested the patch set on a Armada 370 based board (Armada 370 DB), and it works well.
So with this last chunk you can add my:

and also my

Quoted 20 lines
Quoted 18 lines

---8<----

drivers/clocksource/time-armada-370-xp.c +1 -1
index ee2e50c5..a9bf37a 100644
@@ -150,7 +150,7 @@ static irqreturn_t armada_370_xp_timer_interrupt(int irq, void *dev_id)
        /*
         * ACK timer interrupt and call event handler.
         */
-       struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
+       struct clock_event_device *evt = dev_id;
 
        writel(TIMER0_CLR_MASK, local_base + LCL_TIMER_EVENTS_STATUS);
        evt->event_handler(evt);
Signature · 4 lines
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#28 lore raw

Re: [PATCHv3 09/10] clocksource: time-armada-370-xp: Divorce from local timer API

Quoted 54 lines · Gregory CLEMENT
Quoted 53 lines · Gregory CLEMENT
Quoted 52 lines · Stephen Boyd
Quoted 7 lines · Gregory CLEMENT
Quoted 3 lines · Stephen Boyd

Separate the armada 370xp local timers from the local timer API. This will allow us to remove ARM local timer support in the near future and makes this driver multi-architecture friendly.

At first view the code looks good, but when I applied your patch set on linux-next, build it and run it on a Armada XP based board (AX3 with 2 cores), it crashed:

[...]

Quoted 37 lines
Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
Modules linked in:
CPU: 0    Not tainted  (3.9.0-rc3-next-20130319-00010-g728b448 #153)
PC is at 0xe92d45f0
LR is at armada_370_xp_timer_interrupt+0x3c/0x4c
pc : [<e92d45f0>]    lr : [<c023c2bc>]    psr: 600001d3

sp : c0635eb8 ip : 00000000 fp : c063c3f0
r10: 000003ff r9 : 00000000 r8 : 00000010
r7 : c22b3c40 r6 : ef007c00 r5 : c0640fcc r4 : c0053e30 r3 : e92d45f0 r2 : fffffffe r1 : c22b3c40 r0 : c0053e30
Flags: nZCv IRQs off FIQs off Mode SVC_32 ISA ARM Segment kernel
Control: 10c53c7d Table: 0000406a DAC: 00000015
Process swapper/0 (pid: 0, stack limit = 0xc0634238)

Stack: (0xc0635eb8 to 0xc0636000)

5ea0: ef004c80 c0063224
5ec0: 00000010 00000010 00000000 c0660ac0 c0635f18 c005fcb8 c0632b90 c000ed94
5ee0: c0313c60 60000153 00000001 c00085a8 c0313c54 c0313c60 60000153 ffffffff
5f00: c0635f4c 00000000 562f5842 c06360c0 00000000 c000db60 0000001a ffff8ad0
5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
5f60: 00000021 00000000 00000003 00000004 0000006e c065fcc0 c067924c c063ceb8
5f80: c063cc84 c006d8c0 00000005 c065fcc0 c067924c c0421764 c22ad780 c063c42c
5fa0: 562f5842 c063cca8 c06605c0 c04379c0 c22ad780 00000000 562f5842 00000000
5fc0: 00000000 c0417754 ffffffff ffffffff c04172dc 00000000 00000000 c04379c0
5fe0: 10c53c7d c063c414 c04379bc c063febc 0000406a 00008074 00000000 00000000
[<c023c2bc>] (armada_370_xp_timer_interrupt+0x3c/0x4c) from [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) from [<c005fcb8>] (generic_handle_irq+0x20/0x30) [<c005fcb8>] (generic_handle_irq+0x20/0x30) from [<c000ed94>] (handle_IRQ+0x38/0x90) [<c000ed94>] (handle_IRQ+0x38/0x90) from [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) from [<c000db60>] (__irq_svc+0x40/0x50)

Exception stack(0xc0635f18 to 0xc0635f60)

5f00: 0000001a ffff8ad0
5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842 5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff [<c000db60>] (__irq_svc+0x40/0x50) from [<c0313c60>] (calibrate_delay+0x378/0x528) [<c0313c60>] (calibrate_delay+0x378/0x528) from [<c0417754>] (start_kernel+0x250/0x2dc) [<c0417754>] (start_kernel+0x250/0x2dc) from [<00008074>] (0x8074)

Quoted 5 lines
Code: 1fe7deb7 cd5772dd fff5692e ed55f79e (7ed5a5f7)

I am trying to figure out what happened.

Argh. Stupid casting again. Can you try this?

Quoted 9 lines
Quoted 2 lines

Our emails must have crossed, your fix is also fine :)

I have also tested the patch set on a Armada 370 based board (Armada 370 DB), and it works well.
So with this last chunk you can add my:

and also my

Great. Thanks for testing.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#29 lore raw

Re: [PATCHv3 09/10] clocksource: time-armada-370-xp: Divorce from local timer API

In reply to Gregory CLEMENT
Quoted 142 lines · Gregory CLEMENT
Quoted 3 lines · Stephen Boyd

Separate the armada 370xp local timers from the local timer API. This will allow us to remove ARM local timer support in the near future and makes this driver multi-architecture friendly.

At first view the code looks good, but when I applied your patch set on linux-next, build it and run it on a Armada XP based board (AX3 with 2 cores), it crashed:

Booting Linux on physical CPU 0x0
Linux version 3.9.0-rc3-next-20130319-00010-g728b448 (gclement at FE-greg-laptop) (gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5) ) #153 SMP Wed Mar 20 17:58:57 CET 2013
CPU: ARMv7 Processor [562f5842] revision 2 (ARMv7), cr=10c53c7d
CPU: PIPT / VIPT nonaliasing data cache, PIPT instruction cache
Machine: Marvell Armada 370/XP (Device Tree), model: PlatHome OpenBlocks AX3-4 board
bootconsole [earlycon0] enabled
Memory policy: ECC disabled, Data cache writealloc
PERCPU: Embedded 7 pages/cpu @c22b0000 s7104 r8192 d13376 u32768
Built 1 zonelists in Zone order, mobility grouping on. Total pages: 784912
Kernel command line: console=ttyS0,115200 earlyprintk
PID hash table entries: 4096 (order: 2, 16384 bytes)
Dentry cache hash table entries: 131072 (order: 7, 524288 bytes)
Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
__ex_table already sorted, skipping sort
Memory: 3072MB = 3072MB total
Memory: 3113520k/3113520k available, 32208k reserved, 2367488K highmem
Virtual kernel memory layout:

    vector  : 0xffff0000 - 0xffff1000   (   4 kB)
    fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
    vmalloc : 0xf0000000 - 0xff000000   ( 240 MB)
    lowmem  : 0xc0000000 - 0xef800000   ( 760 MB)
    pkmap   : 0xbfe00000 - 0xc0000000   (   2 MB)
    modules : 0xbf000000 - 0xbfe00000   (  14 MB)
      .text : 0xc0008000 - 0xc041647c   (4154 kB)
      .init : 0xc0417000 - 0xc0633bc0   (2163 kB)
      .data : 0xc0634000 - 0xc06605c0   ( 178 kB)
       .bss : 0xc06605c0 - 0xc0686a2c   ( 154 kB)

Hierarchical RCU implementation.
RCU restricting CPUs from NR_CPUS=4 to nr_cpu_ids=2.
NR_IRQS:16 nr_irqs:16 16
Aurora cache controller enabled
l2x0: 16 ways, CACHE_ID 0x00000100, AUX_CTRL 0x1a696b12, Cache size: 1048576 B
sched_clock: 32 bits at 25MHz, resolution 40ns, wraps every 171798ms
Console: colour dummy device 80x30
Calibrating delay loop...

Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
Modules linked in:
CPU: 0    Not tainted  (3.9.0-rc3-next-20130319-00010-g728b448 #153)
PC is at 0xe92d45f0
LR is at armada_370_xp_timer_interrupt+0x3c/0x4c
pc : [<e92d45f0>]    lr : [<c023c2bc>]    psr: 600001d3

sp : c0635eb8 ip : 00000000 fp : c063c3f0
r10: 000003ff r9 : 00000000 r8 : 00000010
r7 : c22b3c40 r6 : ef007c00 r5 : c0640fcc r4 : c0053e30 r3 : e92d45f0 r2 : fffffffe r1 : c22b3c40 r0 : c0053e30
Flags: nZCv IRQs off FIQs off Mode SVC_32 ISA ARM Segment kernel
Control: 10c53c7d Table: 0000406a DAC: 00000015
Process swapper/0 (pid: 0, stack limit = 0xc0634238)

Stack: (0xc0635eb8 to 0xc0636000)

5ea0: ef004c80 c0063224
5ec0: 00000010 00000010 00000000 c0660ac0 c0635f18 c005fcb8 c0632b90 c000ed94
5ee0: c0313c60 60000153 00000001 c00085a8 c0313c54 c0313c60 60000153 ffffffff
5f00: c0635f4c 00000000 562f5842 c06360c0 00000000 c000db60 0000001a ffff8ad0
5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
5f60: 00000021 00000000 00000003 00000004 0000006e c065fcc0 c067924c c063ceb8
5f80: c063cc84 c006d8c0 00000005 c065fcc0 c067924c c0421764 c22ad780 c063c42c
5fa0: 562f5842 c063cca8 c06605c0 c04379c0 c22ad780 00000000 562f5842 00000000
5fc0: 00000000 c0417754 ffffffff ffffffff c04172dc 00000000 00000000 c04379c0
5fe0: 10c53c7d c063c414 c04379bc c063febc 0000406a 00008074 00000000 00000000
[<c023c2bc>] (armada_370_xp_timer_interrupt+0x3c/0x4c) from [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) from [<c005fcb8>] (generic_handle_irq+0x20/0x30) [<c005fcb8>] (generic_handle_irq+0x20/0x30) from [<c000ed94>] (handle_IRQ+0x38/0x90) [<c000ed94>] (handle_IRQ+0x38/0x90) from [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) from [<c000db60>] (__irq_svc+0x40/0x50)

Exception stack(0xc0635f18 to 0xc0635f60)

5f00: 0000001a ffff8ad0
5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842 5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff [<c000db60>] (__irq_svc+0x40/0x50) from [<c0313c60>] (calibrate_delay+0x378/0x528) [<c0313c60>] (calibrate_delay+0x378/0x528) from [<c0417754>] (start_kernel+0x250/0x2dc) [<c0417754>] (start_kernel+0x250/0x2dc) from [<00008074>] (0x8074)

Code: 1fe7deb7 cd5772dd fff5692e ed55f79e (7ed5a5f7)
---[ end trace 1b75b31a2719ed1c ]---
Kernel panic - not syncing: Fatal exception in interrupt
Internal error: Oops - undefined instruction: 0 [#2] SMP ARM
Modules linked in:
CPU: 0    Tainted: G      D       (3.9.0-rc3-next-20130319-00010-g728b448 #153)
PC is at 0xe92d45f0
LR is at armada_370_xp_timer_interrupt+0x3c/0x4c
pc : [<e92d45f0>]    lr : [<c023c2bc>]    psr: 600001d3

sp : c0635cc0 ip : 00000000 fp : c063c3f0
r10: 000003ff r9 : 00000000 r8 : 00000010
r7 : c22b3c40 r6 : ef007c00 r5 : c0640fcc r4 : c0053e30 r3 : e92d45f0 r2 : fffffffe r1 : c22b3c40 r0 : c0053e30
Flags: nZCv IRQs off FIQs off Mode SVC_32 ISA ARM Segment kernel
Control: 10c53c7d Table: 0000406a DAC: 00000015
Process swapper/0 (pid: 0, stack limit = 0xc0634238)

Stack: (0xc0635cc0 to 0xc0636000)

5cc0: ef004c80 c0063224 00000010 00000010 c0635f18 c0660ac0 c0635d20 c005fcb8
5ce0: c0632b90 c000ed94 c0316d54 60000153 00000001 c00085a8 c0316ca4 c0316d54
5d00: 60000153 ffffffff c0635d54 c03a53b0 00000000 c0660b70 600001d3 c000db60
5d20: c0660fc0 00000000 c0642f80 00000000 00000000 00000000 0000000b c06400e0
5d40: c03a53b0 00000000 c0660b70 600001d3 00000000 c0635d68 c0316ca4 c0316d54
5d60: 60000153 ffffffff 600001d3 c0635d94 c06608c0 c0634000 0000000b c06400e0
5d80: c03a53b0 00000000 c063f180 c0011624 c03a5310 200001d3 c0642f80 00010000
5da0: c0634238 0000000b 00100100 c0635e70 e92d45f0 7ed5a5f7 00000001 00000500
5dc0: c000dbcc c0634000 c063c3f0 c00082a0 00000006 c22b0768 00000004 00000000
5de0: 00030001 e92d45f0 ffffffff 00000001 c22b076c 00000000 c065fa1c c065f9c0
5e00: 00100100 00000002 c065f9c0 00000000 c063cca0 00000000 00000002 c065f9c0
5e20: c0661ca4 c06618e6 00000041 00000002 0000000a 00000002 ffffffff 00000000
5e40: 00000000 00000000 00000000 00000000 c06602cc 00000000 c00146d4 e92d45f4
5e60: 600001d3 c0634050 00000001 c000dbcc c0053e30 c22b3c40 fffffffe e92d45f0
5e80: c0053e30 c0640fcc ef007c00 c22b3c40 00000010 00000000 000003ff c063c3f0
5ea0: 00000000 c0635eb8 c023c2bc e92d45f0 600001d3 ffffffff ef004c80 c0063224
5ec0: 00000010 00000010 00000000 c0660ac0 c0635f18 c005fcb8 c0632b90 c000ed94
5ee0: c0313c60 60000153 00000001 c00085a8 c0313c54 c0313c60 60000153 ffffffff
5f00: c0635f4c 00000000 562f5842 c06360c0 00000000 c000db60 0000001a ffff8ad0
5f20: ffff8ad0 c06360c0 00000000 00000000 c04379c0 c22ad780 00000000 562f5842
5f40: c06360c0 00000000 60000153 c0635f60 c0313c54 c0313c60 60000153 ffffffff
5f60: 00000021 00000000 00000003 00000004 0000006e c065fcc0 c067924c c063ceb8
5f80: c063cc84 c006d8c0 00000005 c065fcc0 c067924c c0421764 c22ad780 c063c42c
5fa0: 562f5842 c063cca8 c06605c0 c04379c0 c22ad780 00000000 562f5842 00000000
5fc0: 00000000 c0417754 ffffffff ffffffff c04172dc 00000000 00000000 c04379c0
5fe0: 10c53c7d c063c414 c04379bc c063febc 0000406a 00008074 00000000 00000000
[<c023c2bc>] (armada_370_xp_timer_interrupt+0x3c/0x4c) from [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) [<c0063224>] (handle_percpu_devid_irq+0x64/0x80) from [<c005fcb8>] (generic_handle_irq+0x20/0x30) [<c005fcb8>] (generic_handle_irq+0x20/0x30) from [<c000ed94>] (handle_IRQ+0x38/0x90) [<c000ed94>] (handle_IRQ+0x38/0x90) from [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) [<c00085a8>] (armada_370_xp_handle_irq+0xa4/0xb0) from [<c000db60>] (__irq_svc+0x40/0x50)

Exception stack(0xc0635d20 to 0xc0635d68)

5d20: c0660fc0 00000000 c0642f80 00000000 00000000 00000000 0000000b c06400e0 5d40: c03a53b0 00000000 c0660b70 600001d3 00000000 c0635d68 c0316ca4 c0316d54 5d60: 60000153 ffffffff
[<c000db60>] (__irq_svc+0x40/0x50) from [<c0316d54>] (panic+0x144/0x1b0) [<c0316d54>] (panic+0x144/0x1b0) from [<c0011624>] (die+0x278/0x2a8) [<c0011624>] (die+0x278/0x2a8) from [<c00082a0>] (do_undefinstr+0x9c/0x1c4) [<c00082a0>] (do_undefinstr+0x9c/0x1c4) from [<c000dbcc>] (__und_svc_finish+0x0/0x14)

Exception stack(0xc0635e70 to 0xc0635eb8)

5e60: c0053e30 c22b3c40 fffffffe e92d45f0
5e80: c0053e30 c0640fcc ef007c00 c22b3c40 00000010 00000000 000003ff c063c3f0 5ea0: 00000000 c0635eb8 c023c2bc e92d45f0 600001d3 ffffffff [<c000dbcc>] (__und_svc_finish+0x0/0x14) from [<e92d45f0>] (0xe92d45f0)

Quoted 5 lines
Code: 1fe7deb7 cd5772dd fff5692e ed55f79e (7ed5a5f7)
---[ end trace 1b75b31a2719ed1d ]---

I am trying to figure out what happened.

Ok I found it, you forgot to also change the indirection in interrupt handler, if you add the following chunk to your patch, it fixes the issue:

drivers/clocksource/time-armada-370-xp.c +1 -1
@@ -150,7 +150,7 @@ static irqreturn_t armada_370_xp_timer_interrupt(int irq, void *dev_id)
        /*
         * ACK timer interrupt and call event handler.
         */
-       struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
+       struct clock_event_device *evt = (struct clock_event_device *)dev_id;
 
        writel(TIMER0_CLR_MASK, local_base + LCL_TIMER_EVENTS_STATUS);
        evt->event_handler(evt);
Quoted 190 lines
Quoted 188 lines
Cc Gregory CLEMENT <[email protected]>
---
 drivers/clocksource/time-armada-370-xp.c | 85 ++++++++++++++------------------
 1 file changed, 38 insertions(+), 47 deletions(-)
drivers/clocksource/time-armada-370-xp.c +38 -47
index efe4aef..ee2e50c5 100644
@@ -19,6 +19,7 @@
 #include <linux/platform_device.h>
 #include <linux/kernel.h>
 #include <linux/clk.h>
+#include <linux/cpu.h>
 #include <linux/timer.h>
 #include <linux/clockchips.h>
 #include <linux/interrupt.h>
@@ -31,7 +32,6 @@
 #include <linux/time-armada-370-xp.h>
 
 #include <asm/sched_clock.h>
-#include <asm/localtimer.h>
 /*
  * Timer block registers.
  */
@@ -70,7 +70,7 @@ static bool timer25Mhz = true;
  */
 static u32 ticks_per_jiffy;
 
-static struct clock_event_device __percpu **percpu_armada_370_xp_evt;
+static struct clock_event_device __percpu *armada_370_xp_evt;
 
 static u32 notrace armada_370_xp_read_sched_clock(void)
 {
@@ -143,14 +143,7 @@ armada_370_xp_clkevt_mode(enum clock_event_mode mode,
 	}
 }
 
-static struct clock_event_device armada_370_xp_clkevt = {
-	.name		= "armada_370_xp_per_cpu_tick",
-	.features	= CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
-	.shift		= 32,
-	.rating		= 300,
-	.set_next_event	= armada_370_xp_clkevt_next_event,
-	.set_mode	= armada_370_xp_clkevt_mode,
-};
+static int armada_370_xp_clkevt_irq;
 
 static irqreturn_t armada_370_xp_timer_interrupt(int irq, void *dev_id)
 {
@@ -173,42 +166,53 @@ static int __cpuinit armada_370_xp_timer_setup(struct clock_event_device *evt)
 	u32 u;
 	int cpu = smp_processor_id();
 
-	/* Use existing clock_event for cpu 0 */
-	if (!smp_processor_id())
-		return 0;
-
 	u = readl(local_base + TIMER_CTRL_OFF);
 	if (timer25Mhz)
 		writel(u | TIMER0_25MHZ, local_base + TIMER_CTRL_OFF);
 	else
 		writel(u & ~TIMER0_25MHZ, local_base + TIMER_CTRL_OFF);
 
-	evt->name		= armada_370_xp_clkevt.name;
-	evt->irq		= armada_370_xp_clkevt.irq;
-	evt->features		= armada_370_xp_clkevt.features;
-	evt->shift		= armada_370_xp_clkevt.shift;
-	evt->rating		= armada_370_xp_clkevt.rating,
+	evt->name		= "armada_370_xp_per_cpu_tick",
+	evt->features		= CLOCK_EVT_FEAT_ONESHOT |
+				  CLOCK_EVT_FEAT_PERIODIC;
+	evt->shift		= 32,
+	evt->rating		= 300,
 	evt->set_next_event	= armada_370_xp_clkevt_next_event,
 	evt->set_mode		= armada_370_xp_clkevt_mode,
+	evt->irq		= armada_370_xp_clkevt_irq;
 	evt->cpumask		= cpumask_of(cpu);
 
-	*__this_cpu_ptr(percpu_armada_370_xp_evt) = evt;
-
 	clockevents_config_and_register(evt, timer_clk, 1, 0xfffffffe);
 	enable_percpu_irq(evt->irq, 0);
 
 	return 0;
 }
 
-static void  armada_370_xp_timer_stop(struct clock_event_device *evt)
+static void __cpuinit armada_370_xp_timer_stop(struct clock_event_device *evt)
 {
 	evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
 	disable_percpu_irq(evt->irq);
 }
 
-static struct local_timer_ops armada_370_xp_local_timer_ops __cpuinitdata = {
-	.setup	= armada_370_xp_timer_setup,
-	.stop	=  armada_370_xp_timer_stop,
+static int __cpuinit armada_370_xp_timer_cpu_notify(struct notifier_block *self,
+					   unsigned long action, void *hcpu)
+{
+	struct clock_event_device *evt = this_cpu_ptr(armada_370_xp_evt);
+
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_STARTING:
+		armada_370_xp_timer_setup(evt);
+		break;
+	case CPU_DYING:
+		armada_370_xp_timer_stop(evt);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block armada_370_xp_timer_cpu_nb __cpuinitdata = {
+	.notifier_call = armada_370_xp_timer_cpu_notify,
 };
 
 void __init armada_370_xp_timer_init(void)
@@ -224,9 +228,6 @@ void __init armada_370_xp_timer_init(void)
 
 	if (of_find_property(np, "marvell,timer-25Mhz", NULL)) {
 		/* The fixed 25MHz timer is available so let's use it */
-		u = readl(local_base + TIMER_CTRL_OFF);
-		writel(u | TIMER0_25MHZ,
-		       local_base + TIMER_CTRL_OFF);
 		u = readl(timer_base + TIMER_CTRL_OFF);
 		writel(u | TIMER0_25MHZ,
 		       timer_base + TIMER_CTRL_OFF);
@@ -236,9 +237,6 @@ void __init armada_370_xp_timer_init(void)
 		struct clk *clk = of_clk_get(np, 0);
 		WARN_ON(IS_ERR(clk));
 		rate =  clk_get_rate(clk);
-		u = readl(local_base + TIMER_CTRL_OFF);
-		writel(u & ~(TIMER0_25MHZ),
-		       local_base + TIMER_CTRL_OFF);
 
 		u = readl(timer_base + TIMER_CTRL_OFF);
 		writel(u & ~(TIMER0_25MHZ),
@@ -252,7 +250,7 @@ void __init armada_370_xp_timer_init(void)
 	 * We use timer 0 as clocksource, and private(local) timer 0
 	 * for clockevents
 	 */
-	armada_370_xp_clkevt.irq = irq_of_parse_and_map(np, 4);
+	armada_370_xp_clkevt_irq = irq_of_parse_and_map(np, 4);
 
 	ticks_per_jiffy = (timer_clk + HZ / 2) / HZ;
 
@@ -277,26 +275,19 @@ void __init armada_370_xp_timer_init(void)
 			      "armada_370_xp_clocksource",
 			      timer_clk, 300, 32, clocksource_mmio_readl_down);
 
-	/* Register the clockevent on the private timer of CPU 0 */
-	armada_370_xp_clkevt.cpumask = cpumask_of(0);
-	clockevents_config_and_register(&armada_370_xp_clkevt,
-					timer_clk, 1, 0xfffffffe);
+	register_cpu_notifier(&armada_370_xp_timer_cpu_nb);
 
-	percpu_armada_370_xp_evt = alloc_percpu(struct clock_event_device *);
+	armada_370_xp_evt = alloc_percpu(struct clock_event_device);
 
 
 	/*
 	 * Setup clockevent timer (interrupt-driven).
 	 */
-	*__this_cpu_ptr(percpu_armada_370_xp_evt) = &armada_370_xp_clkevt;
-	res = request_percpu_irq(armada_370_xp_clkevt.irq,
+	res = request_percpu_irq(armada_370_xp_clkevt_irq,
 				armada_370_xp_timer_interrupt,
-				armada_370_xp_clkevt.name,
-				percpu_armada_370_xp_evt);
-	if (!res) {
-		enable_percpu_irq(armada_370_xp_clkevt.irq, 0);
-#ifdef CONFIG_LOCAL_TIMERS
-		local_timer_register(&armada_370_xp_local_timer_ops);
-#endif
-	}
+				"armada_370_xp_per_cpu_tick",
+				armada_370_xp_evt);
+	/* Immediately configure the timer on the boot CPU */
+	if (!res)
+		armada_370_xp_timer_setup(this_cpu_ptr(armada_370_xp_evt));
 }
Signature · 4 lines
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#30 lore raw

PATCH v3 10/10 ARM: smp: Remove local timer API

In reply to Stephen Boyd

There are no more users of this API, remove it.

---
 arch/arm/Kconfig                  | 10 ------
 arch/arm/include/asm/localtimer.h | 34 -------------------
 arch/arm/kernel/smp.c             | 69 ---------------------------------------
 3 files changed, 113 deletions(-)
 delete mode 100644 arch/arm/include/asm/localtimer.h
arch/arm/Kconfig +0 -10
index 5ad2ccf..f1a4c57 100644
@@ -1647,16 +1647,6 @@ config ARM_PSCI
 	  0022A ("Power State Coordination Interface System Software on
 	  ARM processors").
 
-config LOCAL_TIMERS
-	bool "Use local timer interrupts"
-	depends on SMP
-	default y
-	help
-	  Enable support for local timers on SMP platforms, rather then the
-	  legacy IPI broadcast method.  Local timers allows the system
-	  accounting to be spread across the timer interval, preventing a
-	  "thundering herd" at every timer tick.
-
 config ARCH_NR_GPIO
 	int
 	default 1024 if ARCH_SHMOBILE || ARCH_TEGRA
arch/arm/include/asm/localtimer.h deleted +0 -34
deleted file mode 100644
index f77ffc1..0000000
@@ -1,34 +0,0 @@
-/*
- *  arch/arm/include/asm/localtimer.h
- *
- *  Copyright (C) 2004-2005 ARM Ltd.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-#ifndef __ASM_ARM_LOCALTIMER_H
-#define __ASM_ARM_LOCALTIMER_H
-
-#include <linux/errno.h>
-
-struct clock_event_device;
-
-struct local_timer_ops {
-	int  (*setup)(struct clock_event_device *);
-	void (*stop)(struct clock_event_device *);
-};
-
-#ifdef CONFIG_LOCAL_TIMERS
-/*
- * Register a local timer driver
- */
-int local_timer_register(struct local_timer_ops *);
-#else
-static inline int local_timer_register(struct local_timer_ops *ops)
-{
-	return -ENXIO;
-}
-#endif
-
-#endif
arch/arm/kernel/smp.c +0 -69

117 lines not on this page · raw message

Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation