From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org, Tao Ren <taoren@fb.com>
Subject: [PATCH 17/25] clocksource/drivers/fttmr010: Fix invalid interrupt register access
Date: Tue, 18 Dec 2018 22:28:35 +0100 [thread overview]
Message-ID: <20181218212844.30445-17-daniel.lezcano@linaro.org> (raw)
In-Reply-To: <20181218212844.30445-1-daniel.lezcano@linaro.org>
From: Tao Ren <taoren@fb.com>
TIMER_INTR_MASK register (Base Address of Timer + 0x38) is not designed
for masking interrupts on ast2500 chips, and it's not even listed in
ast2400 datasheet, so it's not safe to access TIMER_INTR_MASK on aspeed
chips.
Similarly, TIMER_INTR_STATE register (Base Address of Timer + 0x34) is
not interrupt status register on ast2400 and ast2500 chips. Although
there is no side effect to reset the register in fttmr010_common_init(),
it's just misleading to do so.
Besides, "count_down" is renamed to "is_aspeed" in "fttmr010" structure,
and more comments are added so the code is more readble.
Signed-off-by: Tao Ren <taoren@fb.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/clocksource/timer-fttmr010.c | 73 ++++++++++++++++------------
1 file changed, 42 insertions(+), 31 deletions(-)
diff --git a/drivers/clocksource/timer-fttmr010.c b/drivers/clocksource/timer-fttmr010.c
index cf93f6419b51..fadff7915dd9 100644
--- a/drivers/clocksource/timer-fttmr010.c
+++ b/drivers/clocksource/timer-fttmr010.c
@@ -21,7 +21,7 @@
#include <linux/delay.h>
/*
- * Register definitions for the timers
+ * Register definitions common for all the timer variants.
*/
#define TIMER1_COUNT (0x00)
#define TIMER1_LOAD (0x04)
@@ -36,9 +36,10 @@
#define TIMER3_MATCH1 (0x28)
#define TIMER3_MATCH2 (0x2c)
#define TIMER_CR (0x30)
-#define TIMER_INTR_STATE (0x34)
-#define TIMER_INTR_MASK (0x38)
+/*
+ * Control register (TMC30) bit fields for fttmr010/gemini/moxart timers.
+ */
#define TIMER_1_CR_ENABLE BIT(0)
#define TIMER_1_CR_CLOCK BIT(1)
#define TIMER_1_CR_INT BIT(2)
@@ -53,8 +54,9 @@
#define TIMER_3_CR_UPDOWN BIT(11)
/*
- * The Aspeed AST2400 moves bits around in the control register
- * and lacks bits for setting the timer to count upwards.
+ * Control register (TMC30) bit fields for aspeed ast2400/ast2500 timers.
+ * The aspeed timers move bits around in the control register and lacks
+ * bits for setting the timer to count upwards.
*/
#define TIMER_1_CR_ASPEED_ENABLE BIT(0)
#define TIMER_1_CR_ASPEED_CLOCK BIT(1)
@@ -66,6 +68,18 @@
#define TIMER_3_CR_ASPEED_CLOCK BIT(9)
#define TIMER_3_CR_ASPEED_INT BIT(10)
+/*
+ * Interrupt status/mask register definitions for fttmr010/gemini/moxart
+ * timers.
+ * The registers don't exist and they are not needed on aspeed timers
+ * because:
+ * - aspeed timer overflow interrupt is controlled by bits in Control
+ * Register (TMC30).
+ * - aspeed timers always generate interrupt when either one of the
+ * Match registers equals to Status register.
+ */
+#define TIMER_INTR_STATE (0x34)
+#define TIMER_INTR_MASK (0x38)
#define TIMER_1_INT_MATCH1 BIT(0)
#define TIMER_1_INT_MATCH2 BIT(1)
#define TIMER_1_INT_OVERFLOW BIT(2)
@@ -80,7 +94,7 @@
struct fttmr010 {
void __iomem *base;
unsigned int tick_rate;
- bool count_down;
+ bool is_aspeed;
u32 t1_enable_val;
struct clock_event_device clkevt;
#ifdef CONFIG_ARM
@@ -130,7 +144,7 @@ static int fttmr010_timer_set_next_event(unsigned long cycles,
cr &= ~fttmr010->t1_enable_val;
writel(cr, fttmr010->base + TIMER_CR);
- if (fttmr010->count_down) {
+ if (fttmr010->is_aspeed) {
/*
* ASPEED Timer Controller will load TIMER1_LOAD register
* into TIMER1_COUNT register when the timer is re-enabled.
@@ -175,16 +189,17 @@ static int fttmr010_timer_set_oneshot(struct clock_event_device *evt)
/* Setup counter start from 0 or ~0 */
writel(0, fttmr010->base + TIMER1_COUNT);
- if (fttmr010->count_down)
+ if (fttmr010->is_aspeed) {
writel(~0, fttmr010->base + TIMER1_LOAD);
- else
+ } else {
writel(0, fttmr010->base + TIMER1_LOAD);
- /* Enable interrupt */
- cr = readl(fttmr010->base + TIMER_INTR_MASK);
- cr &= ~(TIMER_1_INT_OVERFLOW | TIMER_1_INT_MATCH2);
- cr |= TIMER_1_INT_MATCH1;
- writel(cr, fttmr010->base + TIMER_INTR_MASK);
+ /* Enable interrupt */
+ cr = readl(fttmr010->base + TIMER_INTR_MASK);
+ cr &= ~(TIMER_1_INT_OVERFLOW | TIMER_1_INT_MATCH2);
+ cr |= TIMER_1_INT_MATCH1;
+ writel(cr, fttmr010->base + TIMER_INTR_MASK);
+ }
return 0;
}
@@ -201,9 +216,8 @@ static int fttmr010_timer_set_periodic(struct clock_event_device *evt)
writel(cr, fttmr010->base + TIMER_CR);
/* Setup timer to fire at 1/HZ intervals. */
- if (fttmr010->count_down) {
+ if (fttmr010->is_aspeed) {
writel(period, fttmr010->base + TIMER1_LOAD);
- writel(0, fttmr010->base + TIMER1_MATCH1);
} else {
cr = 0xffffffff - (period - 1);
writel(cr, fttmr010->base + TIMER1_COUNT);
@@ -281,23 +295,21 @@ static int __init fttmr010_common_init(struct device_node *np, bool is_aspeed)
}
/*
- * The Aspeed AST2400 moves bits around in the control register,
- * otherwise it works the same.
+ * The Aspeed timers move bits around in the control register.
*/
if (is_aspeed) {
fttmr010->t1_enable_val = TIMER_1_CR_ASPEED_ENABLE |
TIMER_1_CR_ASPEED_INT;
- /* Downward not available */
- fttmr010->count_down = true;
+ fttmr010->is_aspeed = true;
} else {
fttmr010->t1_enable_val = TIMER_1_CR_ENABLE | TIMER_1_CR_INT;
- }
- /*
- * Reset the interrupt mask and status
- */
- writel(TIMER_INT_ALL_MASK, fttmr010->base + TIMER_INTR_MASK);
- writel(0, fttmr010->base + TIMER_INTR_STATE);
+ /*
+ * Reset the interrupt mask and status
+ */
+ writel(TIMER_INT_ALL_MASK, fttmr010->base + TIMER_INTR_MASK);
+ writel(0, fttmr010->base + TIMER_INTR_STATE);
+ }
/*
* Enable timer 1 count up, timer 2 count up, except on Aspeed,
@@ -306,9 +318,8 @@ static int __init fttmr010_common_init(struct device_node *np, bool is_aspeed)
if (is_aspeed)
val = TIMER_2_CR_ASPEED_ENABLE;
else {
- val = TIMER_2_CR_ENABLE;
- if (!fttmr010->count_down)
- val |= TIMER_1_CR_UPDOWN | TIMER_2_CR_UPDOWN;
+ val = TIMER_2_CR_ENABLE | TIMER_1_CR_UPDOWN |
+ TIMER_2_CR_UPDOWN;
}
writel(val, fttmr010->base + TIMER_CR);
@@ -321,7 +332,7 @@ static int __init fttmr010_common_init(struct device_node *np, bool is_aspeed)
writel(0, fttmr010->base + TIMER2_MATCH1);
writel(0, fttmr010->base + TIMER2_MATCH2);
- if (fttmr010->count_down) {
+ if (fttmr010->is_aspeed) {
writel(~0, fttmr010->base + TIMER2_LOAD);
clocksource_mmio_init(fttmr010->base + TIMER2_COUNT,
"FTTMR010-TIMER2",
@@ -371,7 +382,7 @@ static int __init fttmr010_common_init(struct device_node *np, bool is_aspeed)
#ifdef CONFIG_ARM
/* Also use this timer for delays */
- if (fttmr010->count_down)
+ if (fttmr010->is_aspeed)
fttmr010->delay_timer.read_current_timer =
fttmr010_read_current_timer_down;
else
--
2.17.1
next prev parent reply other threads:[~2018-12-18 21:29 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-18 21:27 [GIT PULL] timers for 4.21 Daniel Lezcano
2018-12-18 21:28 ` [PATCH 01/25] clocksource/drivers/timer-vt8500: Remove duplicate function name Daniel Lezcano
2018-12-18 21:28 ` [PATCH 02/25] clocksource/drivers/dbx500: Demote dbx500 PRCMU clocksource Daniel Lezcano
2018-12-18 21:28 ` [PATCH 03/25] clocksource/drivers/ux500: Drop Ux500 custom SCHED_CLOCK Daniel Lezcano
2018-12-18 21:28 ` [PATCH 04/25] clocksource/drivers/timer-ti-dm: Remove the early platform driver registration Daniel Lezcano
2018-12-18 21:28 ` [PATCH 05/25] clockevents/drivers/tegra20: Remove obsolete inclusion of <asm/smp_twd.h> Daniel Lezcano
2018-12-18 21:28 ` [PATCH 06/25] clocksource/drivers/meson6_timer: Use register names from the datasheet Daniel Lezcano
2018-12-18 21:28 ` [PATCH 07/25] clocksource/drivers/meson6_timer: Implement the ARM delay timer Daniel Lezcano
2018-12-18 21:28 ` [PATCH 08/25] clocksource/drivers/imx-gpt: Add support for ARM64 Daniel Lezcano
2018-12-18 21:28 ` [PATCH 09/25] clocksource/drivers/imx-gpt: Remove unnecessary irq protection Daniel Lezcano
2018-12-18 21:28 ` [PATCH 10/25] dt-bindings: timer: renesas, cmt: Document r8a7796 CMT support Daniel Lezcano
2018-12-18 21:28 ` [PATCH 11/25] dt-bindings: timer: renesas, cmt: Document r8a77470 " Daniel Lezcano
2018-12-18 21:28 ` [PATCH 12/25] clocksource/drivers/arc_timer: Utilize generic sched_clock Daniel Lezcano
[not found] ` <20181219012457.EC02721871@mail.kernel.org>
2018-12-19 9:06 ` Alexey Brodkin
2018-12-18 21:28 ` [PATCH 13/25] clocksource/drivers/timer-imx-tpm: Convert the driver to timer-of Daniel Lezcano
2018-12-18 21:28 ` [PATCH 14/25] dt-bindings: timer: renesas, cmt: Document r8a774a1 CMT support Daniel Lezcano
2018-12-18 21:28 ` [PATCH 15/25] clocksource/drivers/bcm2835: Switch to SPDX identifier Daniel Lezcano
2018-12-18 21:28 ` [PATCH 16/25] clocksource/drivers/integrator-ap: Add missing of_node_put() Daniel Lezcano
2018-12-18 21:28 ` Daniel Lezcano [this message]
2018-12-18 21:28 ` [PATCH 18/25] clocksource/drivers/timer-imx-tpm: Specify clock name for timer-of Daniel Lezcano
2018-12-18 21:28 ` [PATCH 19/25] clocksource/drivers/riscv_timer: Provide the sched_clock Daniel Lezcano
2018-12-18 21:28 ` [PATCH 20/25] clocksource/drivers/riscv: Change name riscv_timer to timer-riscv Daniel Lezcano
2018-12-18 21:28 ` [PATCH 21/25] clocksource/drivers/rockchip: Change name rockchip_timer to timer-rockchip Daniel Lezcano
2018-12-18 21:28 ` [PATCH 22/25] clocksource/drivers/tegra20: Change name tegra20_timer to timer-tegra20 Daniel Lezcano
2018-12-18 21:28 ` [PATCH 23/25] clocksource/drivers/sun4i: Change name sun4i_timer to timer-sun4i Daniel Lezcano
2018-12-18 21:28 ` [PATCH 24/25] clocksource/drivers/meson6: Change name meson6_timer timer-meson6 Daniel Lezcano
2018-12-18 21:28 ` [PATCH 25/25] clocksource/drivers/rda: Add clock driver for RDA8810PL SoC Daniel Lezcano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181218212844.30445-17-daniel.lezcano@linaro.org \
--to=daniel.lezcano@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=taoren@fb.com \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).