Skip to contentSkip to conversation

: clockevents for 4.6

GIT PULL In review Daniel Lezcano 2 participants 11 messages Feb 25, 2016 – Feb 26, 2016

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

Hi Thomas,

This pull request contains the following changes:

- Replace the prescaler register usage by the match register in order
to add the periodic mode consistently with the one shot mode and add the delay timer support also on the lpc32xx (Ezequiel Garcia)

- Implement the 'set_state_oneshot_stopped' callback on exynos_mct,
arm_global_timer and arch_arm_timer (Viresh Kumar)

- Add the delay timer support on arm_global_timer (Rabin Vincent)

- Fix missing cleanup when the init function fails on rockchip-timer
(Shawn Lin)

- Enable and verify the memory mapped frames are available and usable
without assuming the firmware will do it for us (Robin Murphy)

Thanks

-- Daniel

The following changes since commit 01414888eaf725a677171605cb051b1c6311e6ab:

clocksource/drivers/acpi_pm: Convert to pr_* macros (2015-12-29
12:52:57 +0100)

are available in the git repository at:

http://git.linaro.org/people/daniel.lezcano/linux.git clockevents/4.6

for you to fetch changes up to fd591419ffa5560e294971b1814ab13b3cbf2fb0:

clockevents/drivers/exynos_mct: Implement
->set_state_oneshot_stopped() (2016-02-17 12:49:17 +0000)

----------------------------------------------------------------
Ezequiel Garcia (3):
clocksource/drivers/lpc32xx: Don't use the prescaler counter for
clockevents
clocksource/drivers/lpc32xx: Support periodic mode clocksource/drivers/lpc32xx: Support timer-based ARM delay

Rabin Vincent (1):
clocksource/drivers/arm_global_timer: Register delay timer

Robin Murphy (1):
clocksource/drivers/arm_arch_timer: Enable and verify MMIO access

Shawn Lin (1):
clocksource/drivers/rockchip: Add err handle for rk_timer_init

Viresh Kumar (3):
clockevents/drivers/arm_arch_timer: Implement
->set_state_oneshot_stopped()
clockevents/drivers/arm_global_timer: Implement
->set_state_oneshot_stopped()
clockevents/drivers/exynos_mct: Implement
->set_state_oneshot_stopped()

 drivers/clocksource/Kconfig            |  1 +
 drivers/clocksource/arm_arch_timer.c   | 40 
++++++++++++++++++++++++++++++++--------
 drivers/clocksource/arm_global_timer.c | 18 ++++++++++++++++++
 drivers/clocksource/exynos_mct.c       |  2 ++
 drivers/clocksource/rockchip_timer.c   | 21 +++++++++++++++------
 drivers/clocksource/time-lpc32xx.c     | 66 
+++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
 6 files changed, 121 insertions(+), 27 deletions(-)
Signature · 5 lines
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
#2 lore raw

PATCH 2/9 clocksource/drivers/rockchip: Add err handle for rk_timer_init

In reply to Daniel Lezcano

From: Shawn Lin <[email protected]>

Currently rockchip_timer doesn't do some basic cleanup work when failing to init the timer. Let's add err handle routine to deal with all the err cases.

---
 drivers/clocksource/rockchip_timer.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)
drivers/clocksource/rockchip_timer.c +15 -6
index 8c77a52..b991b28 100644
@@ -122,23 +122,23 @@ static void __init rk_timer_init(struct device_node *np)
 	pclk = of_clk_get_by_name(np, "pclk");
 	if (IS_ERR(pclk)) {
 		pr_err("Failed to get pclk for '%s'\n", TIMER_NAME);
-		return;
+		goto out_unmap;
 	}
 
 	if (clk_prepare_enable(pclk)) {
 		pr_err("Failed to enable pclk for '%s'\n", TIMER_NAME);
-		return;
+		goto out_unmap;
 	}
 
 	timer_clk = of_clk_get_by_name(np, "timer");
 	if (IS_ERR(timer_clk)) {
 		pr_err("Failed to get timer clock for '%s'\n", TIMER_NAME);
-		return;
+		goto out_timer_clk;
 	}
 
 	if (clk_prepare_enable(timer_clk)) {
 		pr_err("Failed to enable timer clock\n");
-		return;
+		goto out_timer_clk;
 	}
 
 	bc_timer.freq = clk_get_rate(timer_clk);
@@ -146,7 +146,7 @@ static void __init rk_timer_init(struct device_node *np)
 	irq = irq_of_parse_and_map(np, 0);
 	if (!irq) {
 		pr_err("Failed to map interrupts for '%s'\n", TIMER_NAME);
-		return;
+		goto out_irq;
 	}
 
 	ce->name = TIMER_NAME;
@@ -164,10 +164,19 @@ static void __init rk_timer_init(struct device_node *np)
 	ret = request_irq(irq, rk_timer_interrupt, IRQF_TIMER, TIMER_NAME, ce);
 	if (ret) {
 		pr_err("Failed to initialize '%s': %d\n", TIMER_NAME, ret);
-		return;
+		goto out_irq;
 	}
 
 	clockevents_config_and_register(ce, bc_timer.freq, 1, UINT_MAX);
+
+	return;
+
+out_irq:
+	clk_disable_unprepare(timer_clk);
+out_timer_clk:
+	clk_disable_unprepare(pclk);
+out_unmap:
+	iounmap(bc_timer.base);
 }
 
 CLOCKSOURCE_OF_DECLARE(rk_timer, "rockchip,rk3288-timer", rk_timer_init);
Signature · 1 line
1.9.1
#3 lore raw

PATCH 3/9 clocksource/drivers/lpc32xx: Don't use the prescaler counter for clockevents

In reply to Daniel Lezcano

From: Ezequiel Garcia <[email protected]>

This commit switches the clockevents one-shot current implementation to avoid using the prescaler counter. The clockevents timer currently uses MR0=1, PR=ticks; and after this commit is uses MR0=ticks, PR=0.

While using the prescaler with PR=1 works fine in one-shot mode, it seems it doesn't work as expected in periodic mode.

By using the only match channel register (MR0) for the timer we make the periodic mode introduction easier, and consistent with one-shot mode.

---
 drivers/clocksource/time-lpc32xx.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)
drivers/clocksource/time-lpc32xx.c +8 -9
index 1316876..50d1a63 100644
@@ -60,14 +60,13 @@ static int lpc32xx_clkevt_next_event(unsigned long delta,
 		container_of(evtdev, struct lpc32xx_clock_event_ddata, evtdev);
 
 	/*
-	 * Place timer in reset and program the delta in the prescale
-	 * register (PR). When the prescale counter matches the value
-	 * in PR the counter register is incremented and the compare
-	 * match will trigger. After setup the timer is released from
-	 * reset and enabled.
+	 * Place timer in reset and program the delta in the match
+	 * channel 0 (MR0). When the timer counter matches the value
+	 * in MR0 register the match will trigger an interrupt.
+	 * After setup the timer is released from reset and enabled.
 	 */
 	writel_relaxed(LPC32XX_TIMER_TCR_CRST, ddata->base + LPC32XX_TIMER_TCR);
-	writel_relaxed(delta, ddata->base + LPC32XX_TIMER_PR);
+	writel_relaxed(delta, ddata->base + LPC32XX_TIMER_MR0);
 	writel_relaxed(LPC32XX_TIMER_TCR_CEN, ddata->base + LPC32XX_TIMER_TCR);
 
 	return 0;
@@ -210,13 +209,13 @@ static int __init lpc32xx_clockevent_init(struct device_node *np)
 
 	/*
 	 * Disable timer and clear any pending interrupt (IR) on match
-	 * channel 0 (MR0). Configure a compare match value of 1 on MR0
-	 * and enable interrupt, reset on match and stop on match (MCR).
+	 * channel 0 (MR0). Clear the prescaler as it's not used.
+	 * Enable interrupt, reset on match and stop on match (MCR).
 	 */
 	writel_relaxed(0, base + LPC32XX_TIMER_TCR);
+	writel_relaxed(0, base + LPC32XX_TIMER_PR);
 	writel_relaxed(0, base + LPC32XX_TIMER_CTCR);
 	writel_relaxed(LPC32XX_TIMER_IR_MR0INT, base + LPC32XX_TIMER_IR);
-	writel_relaxed(1, base + LPC32XX_TIMER_MR0);
 	writel_relaxed(LPC32XX_TIMER_MCR_MR0I | LPC32XX_TIMER_MCR_MR0R |
 		       LPC32XX_TIMER_MCR_MR0S, base + LPC32XX_TIMER_MCR);
 
Signature · 1 line
1.9.1
#4 lore raw

PATCH 4/9 clocksource/drivers/lpc32xx: Support periodic mode

In reply to Daniel Lezcano

From: Ezequiel Garcia <[email protected]>

This commit adds the support for periodic mode. This is done by not setting the MR0S (Stop on TnMR0) bit on MCR, thus allowing interrupts to be periodically generated on MR0 matches.

In order to do this, move the initial configuration that is specific to the one-shot mode to set_state_oneshot().

---
 drivers/clocksource/time-lpc32xx.c | 39 +++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)
drivers/clocksource/time-lpc32xx.c +34 -5
index 50d1a63..5694edd 100644
@@ -43,6 +43,7 @@
 struct lpc32xx_clock_event_ddata {
 	struct clock_event_device evtdev;
 	void __iomem *base;
+	u32 ticks_per_jiffy;
 };
 
 /* Needed for the sched clock */
@@ -85,11 +86,39 @@ static int lpc32xx_clkevt_shutdown(struct clock_event_device *evtdev)
 
 static int lpc32xx_clkevt_oneshot(struct clock_event_device *evtdev)
 {
+	struct lpc32xx_clock_event_ddata *ddata =
+		container_of(evtdev, struct lpc32xx_clock_event_ddata, evtdev);
+
 	/*
 	 * When using oneshot, we must also disable the timer
 	 * to wait for the first call to set_next_event().
 	 */
-	return lpc32xx_clkevt_shutdown(evtdev);
+	writel_relaxed(0, ddata->base + LPC32XX_TIMER_TCR);
+
+	/* Enable interrupt, reset on match and stop on match (MCR). */
+	writel_relaxed(LPC32XX_TIMER_MCR_MR0I | LPC32XX_TIMER_MCR_MR0R |
+		       LPC32XX_TIMER_MCR_MR0S, ddata->base + LPC32XX_TIMER_MCR);
+	return 0;
+}
+
+static int lpc32xx_clkevt_periodic(struct clock_event_device *evtdev)
+{
+	struct lpc32xx_clock_event_ddata *ddata =
+		container_of(evtdev, struct lpc32xx_clock_event_ddata, evtdev);
+
+	/* Enable interrupt and reset on match. */
+	writel_relaxed(LPC32XX_TIMER_MCR_MR0I | LPC32XX_TIMER_MCR_MR0R,
+		       ddata->base + LPC32XX_TIMER_MCR);
+
+	/*
+	 * Place timer in reset and program the delta in the match
+	 * channel 0 (MR0).
+	 */
+	writel_relaxed(LPC32XX_TIMER_TCR_CRST, ddata->base + LPC32XX_TIMER_TCR);
+	writel_relaxed(ddata->ticks_per_jiffy, ddata->base + LPC32XX_TIMER_MR0);
+	writel_relaxed(LPC32XX_TIMER_TCR_CEN, ddata->base + LPC32XX_TIMER_TCR);
+
+	return 0;
 }
 
 static irqreturn_t lpc32xx_clock_event_handler(int irq, void *dev_id)
@@ -107,11 +136,13 @@ static irqreturn_t lpc32xx_clock_event_handler(int irq, void *dev_id)
 static struct lpc32xx_clock_event_ddata lpc32xx_clk_event_ddata = {
 	.evtdev = {
 		.name			= "lpc3220 clockevent",
-		.features		= CLOCK_EVT_FEAT_ONESHOT,
+		.features		= CLOCK_EVT_FEAT_ONESHOT |
+					  CLOCK_EVT_FEAT_PERIODIC,
 		.rating			= 300,
 		.set_next_event		= lpc32xx_clkevt_next_event,
 		.set_state_shutdown	= lpc32xx_clkevt_shutdown,
 		.set_state_oneshot	= lpc32xx_clkevt_oneshot,
+		.set_state_periodic	= lpc32xx_clkevt_periodic,
 	},
 };
 
@@ -210,17 +241,15 @@ static int __init lpc32xx_clockevent_init(struct device_node *np)
 	/*
 	 * Disable timer and clear any pending interrupt (IR) on match
 	 * channel 0 (MR0). Clear the prescaler as it's not used.
-	 * Enable interrupt, reset on match and stop on match (MCR).
 	 */
 	writel_relaxed(0, base + LPC32XX_TIMER_TCR);
 	writel_relaxed(0, base + LPC32XX_TIMER_PR);
 	writel_relaxed(0, base + LPC32XX_TIMER_CTCR);
 	writel_relaxed(LPC32XX_TIMER_IR_MR0INT, base + LPC32XX_TIMER_IR);
-	writel_relaxed(LPC32XX_TIMER_MCR_MR0I | LPC32XX_TIMER_MCR_MR0R |
-		       LPC32XX_TIMER_MCR_MR0S, base + LPC32XX_TIMER_MCR);
 
 	rate = clk_get_rate(clk);
 	lpc32xx_clk_event_ddata.base = base;
+	lpc32xx_clk_event_ddata.ticks_per_jiffy = DIV_ROUND_CLOSEST(rate, HZ);
 	clockevents_config_and_register(&lpc32xx_clk_event_ddata.evtdev,
 					rate, 1, -1);
 
Signature · 1 line
1.9.1
#5 lore raw

PATCH 5/9 clocksource/drivers/lpc32xx: Support timer-based ARM delay

In reply to Daniel Lezcano

From: Ezequiel Garcia <[email protected]>

This commit implements the ARM timer-based delay timer for the LPC32xx, LPC18xx, LPC43xx family of SoCs.

Also, add a dependency to restrict compiling this driver for the ARM architecture.

---
 drivers/clocksource/Kconfig        |  1 +
 drivers/clocksource/time-lpc32xx.c | 12 ++++++++++++
 2 files changed, 13 insertions(+)
drivers/clocksource/Kconfig +1 -0
index 56777f0..c9c4db6 100644
@@ -153,6 +153,7 @@ config CLKSRC_EFM32
 config CLKSRC_LPC32XX
 	bool "Clocksource for LPC32XX" if COMPILE_TEST
 	depends on GENERIC_CLOCKEVENTS && HAS_IOMEM
+	depends on ARM
 	select CLKSRC_MMIO
 	select CLKSRC_OF
 	help
drivers/clocksource/time-lpc32xx.c +12 -0
index 5694edd..daae61e 100644
@@ -18,6 +18,7 @@
 #include <linux/clk.h>
 #include <linux/clockchips.h>
 #include <linux/clocksource.h>
+#include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/kernel.h>
@@ -54,6 +55,15 @@ static u64 notrace lpc32xx_read_sched_clock(void)
 	return readl(clocksource_timer_counter);
 }
 
+static unsigned long lpc32xx_delay_timer_read(void)
+{
+	return readl(clocksource_timer_counter);
+}
+
+static struct delay_timer lpc32xx_delay_timer = {
+	.read_current_timer = lpc32xx_delay_timer_read,
+};
+
 static int lpc32xx_clkevt_next_event(unsigned long delta,
 				     struct clock_event_device *evtdev)
 {
@@ -192,6 +202,8 @@ static int __init lpc32xx_clocksource_init(struct device_node *np)
 	}
 
 	clocksource_timer_counter = base + LPC32XX_TIMER_TC;
+	lpc32xx_delay_timer.freq = rate;
+	register_current_timer_delay(&lpc32xx_delay_timer);
 	sched_clock_register(lpc32xx_read_sched_clock, 32, rate);
 
 	return 0;
Signature · 1 line
1.9.1
#6 lore raw

PATCH 6/9 clocksource/drivers/arm_global_timer: Register delay timer

In reply to Daniel Lezcano

From: Rabin Vincent <[email protected]>

Provide a delay timer using the lower 32-bits of the global timer so that we can use that instead of having to calibrating delays.

---
 drivers/clocksource/arm_global_timer.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
drivers/clocksource/arm_global_timer.c +17 -0
index d189d8c..36998fa 100644
@@ -16,6 +16,7 @@
 #include <linux/clockchips.h>
 #include <linux/cpu.h>
 #include <linux/clk.h>
+#include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/of.h>
@@ -221,6 +222,21 @@ static u64 notrace gt_sched_clock_read(void)
 }
 #endif
 
+static unsigned long gt_read_long(void)
+{
+	return readl_relaxed(gt_base + GT_COUNTER0);
+}
+
+static struct delay_timer gt_delay_timer = {
+	.read_current_timer = gt_read_long,
+};
+
+static void __init gt_delay_timer_init(void)
+{
+	gt_delay_timer.freq = gt_clk_rate;
+	register_current_timer_delay(&gt_delay_timer);
+}
+
 static void __init gt_clocksource_init(void)
 {
 	writel(0, gt_base + GT_CONTROL);
@@ -317,6 +333,7 @@ static void __init global_timer_of_register(struct device_node *np)
 	/* Immediately configure the timer on the boot CPU */
 	gt_clocksource_init();
 	gt_clockevents_init(this_cpu_ptr(gt_evt));
+	gt_delay_timer_init();
 
 	return;
 
Signature · 1 line
1.9.1
#8 lore raw

PATCH 8/9 clockevents/drivers/arm_global_timer: Implement ->set_state_oneshot_stopped()

In reply to Daniel Lezcano

From: Viresh Kumar <[email protected]>

set_state_oneshot_stopped() is called by the clkevt core, when the next event is required at an expiry time of 'KTIME_MAX'. This normally happens with NO_HZ_{IDLE|FULL} in both LOWRES/HIGHRES modes.

This patch makes the clockevent device to stop on such an event, to avoid spurious interrupts, as explained by: commit 8fff52fd5093 ("clockevents: Introduce CLOCK_EVT_STATE_ONESHOT_STOPPED state").

---
 drivers/clocksource/arm_global_timer.c | 1 +
 1 file changed, 1 insertion(+)
drivers/clocksource/arm_global_timer.c +1 -0
index 36998fa..9df0d16 100644
@@ -175,6 +175,7 @@ static int gt_clockevents_init(struct clock_event_device *clk)
 	clk->set_state_shutdown = gt_clockevent_shutdown;
 	clk->set_state_periodic = gt_clockevent_set_periodic;
 	clk->set_state_oneshot = gt_clockevent_shutdown;
+	clk->set_state_oneshot_stopped = gt_clockevent_shutdown;
 	clk->set_next_event = gt_clockevent_set_next_event;
 	clk->cpumask = cpumask_of(cpu);
 	clk->rating = 300;
Signature · 1 line
1.9.1
#10 lore raw

PATCH 9/9 clockevents/drivers/exynos_mct: Implement ->set_state_oneshot_stopped()

In reply to Daniel Lezcano

From: Viresh Kumar <[email protected]>

set_state_oneshot_stopped() is called by the clkevt core, when the next event is required at an expiry time of 'KTIME_MAX'. This normally happens with NO_HZ_{IDLE|FULL} in both LOWRES/HIGHRES modes.

This patch makes the clockevent device to stop on such an event, to avoid spurious interrupts, as explained by: commit 8fff52fd5093 ("clockevents: Introduce CLOCK_EVT_STATE_ONESHOT_STOPPED state").

---
 drivers/clocksource/exynos_mct.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/clocksource/exynos_mct.c +2 -0
index ff44082..be09bc0 100644
@@ -313,6 +313,7 @@ static struct clock_event_device mct_comp_device = {
 	.set_state_periodic	= mct_set_state_periodic,
 	.set_state_shutdown	= mct_set_state_shutdown,
 	.set_state_oneshot	= mct_set_state_shutdown,
+	.set_state_oneshot_stopped = mct_set_state_shutdown,
 	.tick_resume		= mct_set_state_shutdown,
 };
 
@@ -452,6 +453,7 @@ static int exynos4_local_timer_setup(struct mct_clock_event_device *mevt)
 	evt->set_state_periodic = set_state_periodic;
 	evt->set_state_shutdown = set_state_shutdown;
 	evt->set_state_oneshot = set_state_shutdown;
+	evt->set_state_oneshot_stopped = set_state_shutdown;
 	evt->tick_resume = set_state_shutdown;
 	evt->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
 	evt->rating = 450;
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/9 clocksource/drivers/arm_arch_timer: Enable and verify MMIO access

In reply to Daniel Lezcano

From: Robin Murphy <[email protected]>

So far, we have been blindly assuming that having access to a memory-mapped timer frame implies that the individual elements of that frame frame are already enabled. Whilst it's the firmware's job to give us non-secure access to frames in the first place, we should not rely on implementations always being generous enough to also configure CNTACR for those non-secure frames (e.g. [1]).

Explicitly enable feature-level access per-frame, and verify that the access we want is really implemented before trying to make use of it.

[1]:https://github.com/ARM-software/tf-issues/issues/170

---
 drivers/clocksource/arm_arch_timer.c | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)
drivers/clocksource/arm_arch_timer.c +28 -8
index c64d543..7c567f0 100644
@@ -32,6 +32,14 @@
 #define CNTTIDR		0x08
 #define CNTTIDR_VIRT(n)	(BIT(1) << ((n) * 4))
 
+#define CNTACR(n)	(0x40 + ((n) * 4))
+#define CNTACR_RPCT	BIT(0)
+#define CNTACR_RVCT	BIT(1)
+#define CNTACR_RFRQ	BIT(2)
+#define CNTACR_RVOFF	BIT(3)
+#define CNTACR_RWVT	BIT(4)
+#define CNTACR_RWPT	BIT(5)
+
 #define CNTVCT_LO	0x08
 #define CNTVCT_HI	0x0c
 #define CNTFRQ		0x10
@@ -757,7 +765,6 @@ static void __init arch_timer_mem_init(struct device_node *np)
 	}
 
 	cnttidr = readl_relaxed(cntctlbase + CNTTIDR);
-	iounmap(cntctlbase);
 
 	/*
 	 * Try to find a virtual capable frame. Otherwise fall back to a
@@ -765,20 +772,31 @@ static void __init arch_timer_mem_init(struct device_node *np)
 	 */
 	for_each_available_child_of_node(np, frame) {
 		int n;
+		u32 cntacr;
 
 		if (of_property_read_u32(frame, "frame-number", &n)) {
 			pr_err("arch_timer: Missing frame-number\n");
-			of_node_put(best_frame);
 			of_node_put(frame);
-			return;
+			goto out;
 		}
 
-		if (cnttidr & CNTTIDR_VIRT(n)) {
+		/* Try enabling everything, and see what sticks */
+		cntacr = CNTACR_RFRQ | CNTACR_RWPT | CNTACR_RPCT |
+			 CNTACR_RWVT | CNTACR_RVOFF | CNTACR_RVCT;
+		writel_relaxed(cntacr, cntctlbase + CNTACR(n));
+		cntacr = readl_relaxed(cntctlbase + CNTACR(n));
+
+		if ((cnttidr & CNTTIDR_VIRT(n)) &&
+		    !(~cntacr & (CNTACR_RWVT | CNTACR_RVCT))) {
 			of_node_put(best_frame);
 			best_frame = frame;
 			arch_timer_mem_use_virtual = true;
 			break;
 		}
+
+		if (~cntacr & (CNTACR_RWPT | CNTACR_RPCT))
+			continue;
+
 		of_node_put(best_frame);
 		best_frame = of_node_get(frame);
 	}
@@ -786,24 +804,26 @@ static void __init arch_timer_mem_init(struct device_node *np)
 	base = arch_counter_base = of_iomap(best_frame, 0);
 	if (!base) {
 		pr_err("arch_timer: Can't map frame's registers\n");
-		of_node_put(best_frame);
-		return;
+		goto out;
 	}
 
 	if (arch_timer_mem_use_virtual)
 		irq = irq_of_parse_and_map(best_frame, 1);
 	else
 		irq = irq_of_parse_and_map(best_frame, 0);
-	of_node_put(best_frame);
+
 	if (!irq) {
 		pr_err("arch_timer: Frame missing %s irq",
 		       arch_timer_mem_use_virtual ? "virt" : "phys");
-		return;
+		goto out;
 	}
 
 	arch_timer_detect_rate(base, np);
 	arch_timer_mem_register(base, irq);
 	arch_timer_common_init();
+out:
+	iounmap(cntctlbase);
+	of_node_put(best_frame);
 }
 CLOCKSOURCE_OF_DECLARE(armv7_arch_timer_mem, "arm,armv7-timer-mem",
 		       arch_timer_mem_init);
Signature · 1 line
1.9.1
#7 lore raw

PATCH 7/9 clockevents/drivers/arm_arch_timer: Implement ->set_state_oneshot_stopped()

From: Viresh Kumar <[email protected]>

set_state_oneshot_stopped() is called by the clkevt core, when the next event is required at an expiry time of 'KTIME_MAX'. This normally happens with NO_HZ_{IDLE|FULL} in both LOWRES/HIGHRES modes.

This patch makes the clockevent device to stop on such an event, to avoid spurious interrupts, as explained by: commit 8fff52fd5093 ("clockevents: Introduce CLOCK_EVT_STATE_ONESHOT_STOPPED state").

---
 drivers/clocksource/arm_arch_timer.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/clocksource/arm_arch_timer.c +4 -0
index 7c567f0..f0dd9d4 100644
@@ -274,10 +274,12 @@ static void __arch_timer_setup(unsigned type,
 		if (arch_timer_use_virtual) {
 			clk->irq = arch_timer_ppi[VIRT_PPI];
 			clk->set_state_shutdown = arch_timer_shutdown_virt;
+			clk->set_state_oneshot_stopped = arch_timer_shutdown_virt;
 			clk->set_next_event = arch_timer_set_next_event_virt;
 		} else {
 			clk->irq = arch_timer_ppi[PHYS_SECURE_PPI];
 			clk->set_state_shutdown = arch_timer_shutdown_phys;
+			clk->set_state_oneshot_stopped = arch_timer_shutdown_phys;
 			clk->set_next_event = arch_timer_set_next_event_phys;
 		}
 	} else {
@@ -287,10 +289,12 @@ static void __arch_timer_setup(unsigned type,
 		clk->cpumask = cpu_all_mask;
 		if (arch_timer_mem_use_virtual) {
 			clk->set_state_shutdown = arch_timer_shutdown_virt_mem;
+			clk->set_state_oneshot_stopped = arch_timer_shutdown_virt_mem;
 			clk->set_next_event =
 				arch_timer_set_next_event_virt_mem;
 		} else {
 			clk->set_state_shutdown = arch_timer_shutdown_phys_mem;
+			clk->set_state_oneshot_stopped = arch_timer_shutdown_phys_mem;
 			clk->set_next_event =
 				arch_timer_set_next_event_phys_mem;
 		}
Signature · 1 line
1.9.1
#9 lore raw

Re: [PATCH 8/9] clockevents/drivers/arm_global_timer: Implement ->set_state_oneshot_stopped()

In reply to Daniel Lezcano

Dear Daniel, Viresh,

Quoted 9 lines · On Thu, 25 Feb 2016 14:37:16 +0100 Daniel Lezcano wrote:

From: Viresh Kumar <[email protected]>

set_state_oneshot_stopped() is called by the clkevt core, when the next event is required at an expiry time of 'KTIME_MAX'. This normally happens with NO_HZ_{IDLE|FULL} in both LOWRES/HIGHRES modes.

This patch makes the clockevent device to stop on such an event, to avoid spurious interrupts, as explained by: commit 8fff52fd5093 ("clockevents: Introduce CLOCK_EVT_STATE_ONESHOT_STOPPED state").

Inspired by this commit, I think dw_apb_timer also needs similar patch because it only supports PERIODIC mode and the driver emulate ONESHOT over that" as described in commit 8fff52fd5093. What do you think?

I'm cooking one patch for this purpose.

Thanks for the inspiration,
Jisheng

Quoted 19 lines
---
 drivers/clocksource/arm_global_timer.c | 1 +
 1 file changed, 1 insertion(+)
drivers/clocksource/arm_global_timer.c +1 -0
index 36998fa..9df0d16 100644
@@ -175,6 +175,7 @@ static int gt_clockevents_init(struct clock_event_device *clk)
 	clk->set_state_shutdown = gt_clockevent_shutdown;
 	clk->set_state_periodic = gt_clockevent_set_periodic;
 	clk->set_state_oneshot = gt_clockevent_shutdown;
+	clk->set_state_oneshot_stopped = gt_clockevent_shutdown;
 	clk->set_next_event = gt_clockevent_set_next_event;
 	clk->cpumask = cpumask_of(cpu);
 	clk->rating = 300;