Skip to contentSkip to conversation

ARM: smp_twd: Reconfigure clockevents after cpufreq change

PATCH Superseded Linus Walleij 5 participants 5 messages Jun 1, 2011 – Jun 14, 2011 Arm (32-bit)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ARM: smp_twd: Reconfigure clockevents after cpufreq change

From: Colin Cross <[email protected]>

The localtimer's clock changes with the cpu clock. After a cpufreq transition, update the clockevent's frequency and reprogram the next clock event.

Adds a clock called "smp_twd" that is used to determine the twd frequency, which can also be used at init time to avoid calibrating the twd frequency.

Clock changes are based on Rob Herring's work.

Cc Thomas Gleixner <[email protected]>
Cc Russell King <[email protected]>
[ifdef:ed CPUfreq stuff for non-cpufreq configs]

---
I'm sending this as a pull request to Russells in this form, this nasty cpufreq bug which is making the system timeline jump back and forth on ux500 is *certainly* -rc material to me, but I'm leaving it for Russell to decide.

Tested with and without CPUfreq support.

---
 arch/arm/kernel/smp_twd.c |   90 ++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 84 insertions(+), 6 deletions(-)
arch/arm/kernel/smp_twd.c +84 -6
index 60636f4..5c8775d 100644
@@ -10,13 +10,17 @@
  */
 #include <linux/init.h>
 #include <linux/kernel.h>
+#include <linux/clk.h>
+#include <linux/cpufreq.h>
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <linux/err.h>
 #include <linux/smp.h>
 #include <linux/jiffies.h>
 #include <linux/clockchips.h>
 #include <linux/irq.h>
 #include <linux/io.h>
+#include <linux/percpu.h>
 
 #include <asm/smp_twd.h>
 #include <asm/hardware/gic.h>
@@ -24,7 +28,9 @@
 /* set up by the platform code */
 void __iomem *twd_base;
 
+static struct clk *twd_clk;
 static unsigned long twd_timer_rate;
+static DEFINE_PER_CPU(struct clock_event_device *, twd_ce);
 
 static void twd_set_mode(enum clock_event_mode mode,
 			struct clock_event_device *clk)
@@ -80,6 +86,52 @@ int twd_timer_ack(void)
 	return 0;
 }
 
+#ifdef CONFIG_CPU_FREQ
+
+/*
+ * Updates clockevent frequency when the cpu frequency changes.
+ * Called on the cpu that is changing frequency with interrupts disabled.
+ */
+static void twd_update_frequency(void *data)
+{
+	twd_timer_rate = clk_get_rate(twd_clk);
+
+	clockevents_update_freq(__get_cpu_var(twd_ce), twd_timer_rate);
+}
+
+static int twd_cpufreq_transition(struct notifier_block *nb,
+	unsigned long state, void *data)
+{
+	struct cpufreq_freqs *freqs = data;
+
+	/*
+	 * The twd clock events must be reprogrammed to account for the new
+	 * frequency.  The timer is local to a cpu, so cross-call to the
+	 * changing cpu.
+	 */
+	if (state == CPUFREQ_POSTCHANGE || state == CPUFREQ_RESUMECHANGE)
+		smp_call_function_single(freqs->cpu, twd_update_frequency,
+			NULL, 1);
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block twd_cpufreq_nb = {
+	.notifier_call = twd_cpufreq_transition,
+};
+
+static int twd_cpufreq_init(void)
+{
+	if (!IS_ERR_OR_NULL(twd_clk))
+		return cpufreq_register_notifier(&twd_cpufreq_nb,
+			CPUFREQ_TRANSITION_NOTIFIER);
+
+	return 0;
+}
+core_initcall(twd_cpufreq_init);
+
+#endif
+
 static void __cpuinit twd_calibrate_rate(void)
 {
 	unsigned long count;
@@ -119,12 +171,39 @@ static void __cpuinit twd_calibrate_rate(void)
 	}
 }
 
+static struct clk *twd_get_clock(void)
+{
+	struct clk *clk;
+	int err;
+
+	clk = clk_get_sys("smp_twd", NULL);
+	if (IS_ERR(clk)) {
+		pr_err("smp_twd: clock not found: %d\n", (int)PTR_ERR(clk));
+		return clk;
+	}
+
+	err = clk_enable(clk);
+	if (err) {
+		pr_err("smp_twd: clock failed to enable: %d\n", err);
+		clk_put(clk);
+		return ERR_PTR(err);
+	}
+
+	return clk;
+}
+
 /*
  * Setup the local clock events for a CPU.
  */
 void __cpuinit twd_timer_setup(struct clock_event_device *clk)
 {
-	twd_calibrate_rate();
+	if (!twd_clk)
+		twd_clk = twd_get_clock();
+
+	if (!IS_ERR_OR_NULL(twd_clk))
+		twd_timer_rate = clk_get_rate(twd_clk);
+	else
+		twd_calibrate_rate();
 
 	clk->name = "local_timer";
 	clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT |
@@ -132,13 +211,12 @@ void __cpuinit twd_timer_setup(struct clock_event_device *clk)
 	clk->rating = 350;
 	clk->set_mode = twd_set_mode;
 	clk->set_next_event = twd_set_next_event;
-	clk->shift = 20;
-	clk->mult = div_sc(twd_timer_rate, NSEC_PER_SEC, clk->shift);
-	clk->max_delta_ns = clockevent_delta2ns(0xffffffff, clk);
-	clk->min_delta_ns = clockevent_delta2ns(0xf, clk);
 
 	/* Make sure our local interrupt controller has this enabled */
 	gic_enable_ppi(clk->irq);
 
-	clockevents_register_device(clk);
+	__get_cpu_var(twd_ce) = clk;
+
+	clockevents_config_and_register(clk, twd_timer_rate,
+					0xf, 0xffffffff);
 }
Signature · 1 line
1.7.3.2
#1 lore raw

PATCH 1/1 ARM: smp_twd: Reconfigure clockevents after cpufreq change

Colin, Per Fransson noted this:

arch/arm/kernel/smp_twd.c int twd_timer_ack(void) · line 100

Quoted 12 lines

From: Colin Cross <[email protected]>

arch/arm/kernel/smp_twd.c +10 -0
+/*
+ * Updates clockevent frequency when the cpu frequency changes.
+ * Called on the cpu that is changing frequency with interrupts disabled.
+ */
+static void twd_update_frequency(void *data)
+{
+ ? ? ? twd_timer_rate = clk_get_rate(twd_clk);
+
+ ? ? ? clockevents_update_freq(__get_cpu_var(twd_ce), twd_timer_rate);
+}

Will only update the clockevent on the CPU where the cpufreq notifier gets called will it not? It's not called on each CPU AFAICT.

So this function has to traverse all CPUs in twd_ce.

Thanks,
Linus Walleij

#2 lore raw

PATCH 1/1 ARM: smp_twd: Reconfigure clockevents after cpufreq change

Quoted 18 lines · Linus Walleij

Colin, Per Fransson noted this:

arch/arm/kernel/smp_twd.c int twd_timer_ack(void) · line 100

Quoted 12 lines

From: Colin Cross <[email protected]>

arch/arm/kernel/smp_twd.c +10 -0
+/*
+ * Updates clockevent frequency when the cpu frequency changes.
+ * Called on the cpu that is changing frequency with interrupts disabled.
+ */
+static void twd_update_frequency(void *data)
+{
+ ? ? ? twd_timer_rate = clk_get_rate(twd_clk);
+
+ ? ? ? clockevents_update_freq(__get_cpu_var(twd_ce), twd_timer_rate);
+}

Will only update the clockevent on the CPU where the cpufreq notifier gets called will it not? It's not called on each CPU AFAICT.

Quoted 1 line

So this function has to traverse all CPUs in twd_ce.

OMAP and Tegra both iterate through all the affected cpus in the cpufreq driver and call cpufreq_notify_transtion once for each, setting freqs.cpu to the target cpu. The listener in the smp_twd driver then bounces through smp_call_function_single to make sure it is running on the affected cpu. I believe that is the correct way to handle multiple affected cpus.

#3 lore raw

PATCH 1/1 ARM: smp_twd: Reconfigure clockevents after cpufreq change

Quoted 20 lines · Colin Cross
Quoted 18 lines · Linus Walleij

Colin, Per Fransson noted this:

arch/arm/kernel/smp_twd.c int twd_timer_ack(void) · line 100

Quoted 12 lines

From: Colin Cross<[email protected]>

arch/arm/kernel/smp_twd.c +10 -0
+/*
+ * Updates clockevent frequency when the cpu frequency changes.
+ * Called on the cpu that is changing frequency with interrupts disabled.
+ */
+static void twd_update_frequency(void *data)
+{
+       twd_timer_rate = clk_get_rate(twd_clk);
+
+       clockevents_update_freq(__get_cpu_var(twd_ce), twd_timer_rate);
+}

Will only update the clockevent on the CPU where the cpufreq notifier gets called will it not? It's not called on each CPU AFAICT.

Quoted 9 lines
Quoted 1 line

So this function has to traverse all CPUs in twd_ce.

OMAP and Tegra both iterate through all the affected cpus in the cpufreq driver and call cpufreq_notify_transtion once for each, setting freqs.cpu to the target cpu. The listener in the smp_twd driver then bounces through smp_call_function_single to make sure it is running on the affected cpu. I believe that is the correct way to handle multiple affected cpus.

I agree with Collin. That should be taken care by "smp_call_function_single(freqs->cpu, )" with notifier being called with all the available CPU's in the mask.

Linus,
Did you see any issue with this patch on your platform ?

Regards
Santosh

#4 lore raw

PATCH 1/1 ARM: smp_twd: Reconfigure clockevents after cpufreq change

2011/6/14 Santosh Shilimkar <[email protected]>:

Quoted 3 lines

That should be taken care by "smp_call_function_single(freqs->cpu, )" with notifier being called with all the available CPU's in the mask.

Ah! I see now.

Quoted 1 line

Did you see any issue with this patch on your platform ?

No, I don't think so, I think we saw the problem on an earlier version that didn't use smp_call_function_single().

Thanks,
Linus Walleij