1
0
Fork 0

clocksource/drivers/fttmr010: Fix invalid interrupt register access

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>
hifive-unleashed-5.1
Tao Ren 2018-10-03 14:53:50 -07:00 committed by Daniel Lezcano
parent 5eb73c8311
commit 86fe57fc47
1 changed files with 42 additions and 31 deletions

View File

@ -21,7 +21,7 @@
#include <linux/delay.h> #include <linux/delay.h>
/* /*
* Register definitions for the timers * Register definitions common for all the timer variants.
*/ */
#define TIMER1_COUNT (0x00) #define TIMER1_COUNT (0x00)
#define TIMER1_LOAD (0x04) #define TIMER1_LOAD (0x04)
@ -36,9 +36,10 @@
#define TIMER3_MATCH1 (0x28) #define TIMER3_MATCH1 (0x28)
#define TIMER3_MATCH2 (0x2c) #define TIMER3_MATCH2 (0x2c)
#define TIMER_CR (0x30) #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_ENABLE BIT(0)
#define TIMER_1_CR_CLOCK BIT(1) #define TIMER_1_CR_CLOCK BIT(1)
#define TIMER_1_CR_INT BIT(2) #define TIMER_1_CR_INT BIT(2)
@ -53,8 +54,9 @@
#define TIMER_3_CR_UPDOWN BIT(11) #define TIMER_3_CR_UPDOWN BIT(11)
/* /*
* The Aspeed AST2400 moves bits around in the control register * Control register (TMC30) bit fields for aspeed ast2400/ast2500 timers.
* and lacks bits for setting the timer to count upwards. * 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_ENABLE BIT(0)
#define TIMER_1_CR_ASPEED_CLOCK BIT(1) #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_CLOCK BIT(9)
#define TIMER_3_CR_ASPEED_INT BIT(10) #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_MATCH1 BIT(0)
#define TIMER_1_INT_MATCH2 BIT(1) #define TIMER_1_INT_MATCH2 BIT(1)
#define TIMER_1_INT_OVERFLOW BIT(2) #define TIMER_1_INT_OVERFLOW BIT(2)
@ -80,7 +94,7 @@
struct fttmr010 { struct fttmr010 {
void __iomem *base; void __iomem *base;
unsigned int tick_rate; unsigned int tick_rate;
bool count_down; bool is_aspeed;
u32 t1_enable_val; u32 t1_enable_val;
struct clock_event_device clkevt; struct clock_event_device clkevt;
#ifdef CONFIG_ARM #ifdef CONFIG_ARM
@ -130,7 +144,7 @@ static int fttmr010_timer_set_next_event(unsigned long cycles,
cr &= ~fttmr010->t1_enable_val; cr &= ~fttmr010->t1_enable_val;
writel(cr, fttmr010->base + TIMER_CR); writel(cr, fttmr010->base + TIMER_CR);
if (fttmr010->count_down) { if (fttmr010->is_aspeed) {
/* /*
* ASPEED Timer Controller will load TIMER1_LOAD register * ASPEED Timer Controller will load TIMER1_LOAD register
* into TIMER1_COUNT register when the timer is re-enabled. * 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 */ /* Setup counter start from 0 or ~0 */
writel(0, fttmr010->base + TIMER1_COUNT); writel(0, fttmr010->base + TIMER1_COUNT);
if (fttmr010->count_down) if (fttmr010->is_aspeed) {
writel(~0, fttmr010->base + TIMER1_LOAD); writel(~0, fttmr010->base + TIMER1_LOAD);
else } else {
writel(0, fttmr010->base + TIMER1_LOAD); writel(0, fttmr010->base + TIMER1_LOAD);
/* Enable interrupt */ /* Enable interrupt */
cr = readl(fttmr010->base + TIMER_INTR_MASK); cr = readl(fttmr010->base + TIMER_INTR_MASK);
cr &= ~(TIMER_1_INT_OVERFLOW | TIMER_1_INT_MATCH2); cr &= ~(TIMER_1_INT_OVERFLOW | TIMER_1_INT_MATCH2);
cr |= TIMER_1_INT_MATCH1; cr |= TIMER_1_INT_MATCH1;
writel(cr, fttmr010->base + TIMER_INTR_MASK); writel(cr, fttmr010->base + TIMER_INTR_MASK);
}
return 0; return 0;
} }
@ -201,9 +216,8 @@ static int fttmr010_timer_set_periodic(struct clock_event_device *evt)
writel(cr, fttmr010->base + TIMER_CR); writel(cr, fttmr010->base + TIMER_CR);
/* Setup timer to fire at 1/HZ intervals. */ /* Setup timer to fire at 1/HZ intervals. */
if (fttmr010->count_down) { if (fttmr010->is_aspeed) {
writel(period, fttmr010->base + TIMER1_LOAD); writel(period, fttmr010->base + TIMER1_LOAD);
writel(0, fttmr010->base + TIMER1_MATCH1);
} else { } else {
cr = 0xffffffff - (period - 1); cr = 0xffffffff - (period - 1);
writel(cr, fttmr010->base + TIMER1_COUNT); 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, * The Aspeed timers move bits around in the control register.
* otherwise it works the same.
*/ */
if (is_aspeed) { if (is_aspeed) {
fttmr010->t1_enable_val = TIMER_1_CR_ASPEED_ENABLE | fttmr010->t1_enable_val = TIMER_1_CR_ASPEED_ENABLE |
TIMER_1_CR_ASPEED_INT; TIMER_1_CR_ASPEED_INT;
/* Downward not available */ fttmr010->is_aspeed = true;
fttmr010->count_down = true;
} else { } else {
fttmr010->t1_enable_val = TIMER_1_CR_ENABLE | TIMER_1_CR_INT; fttmr010->t1_enable_val = TIMER_1_CR_ENABLE | TIMER_1_CR_INT;
}
/* /*
* Reset the interrupt mask and status * Reset the interrupt mask and status
*/ */
writel(TIMER_INT_ALL_MASK, fttmr010->base + TIMER_INTR_MASK); writel(TIMER_INT_ALL_MASK, fttmr010->base + TIMER_INTR_MASK);
writel(0, fttmr010->base + TIMER_INTR_STATE); writel(0, fttmr010->base + TIMER_INTR_STATE);
}
/* /*
* Enable timer 1 count up, timer 2 count up, except on Aspeed, * 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) if (is_aspeed)
val = TIMER_2_CR_ASPEED_ENABLE; val = TIMER_2_CR_ASPEED_ENABLE;
else { else {
val = TIMER_2_CR_ENABLE; val = TIMER_2_CR_ENABLE | TIMER_1_CR_UPDOWN |
if (!fttmr010->count_down) TIMER_2_CR_UPDOWN;
val |= TIMER_1_CR_UPDOWN | TIMER_2_CR_UPDOWN;
} }
writel(val, fttmr010->base + TIMER_CR); 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_MATCH1);
writel(0, fttmr010->base + TIMER2_MATCH2); writel(0, fttmr010->base + TIMER2_MATCH2);
if (fttmr010->count_down) { if (fttmr010->is_aspeed) {
writel(~0, fttmr010->base + TIMER2_LOAD); writel(~0, fttmr010->base + TIMER2_LOAD);
clocksource_mmio_init(fttmr010->base + TIMER2_COUNT, clocksource_mmio_init(fttmr010->base + TIMER2_COUNT,
"FTTMR010-TIMER2", "FTTMR010-TIMER2",
@ -371,7 +382,7 @@ static int __init fttmr010_common_init(struct device_node *np, bool is_aspeed)
#ifdef CONFIG_ARM #ifdef CONFIG_ARM
/* Also use this timer for delays */ /* Also use this timer for delays */
if (fttmr010->count_down) if (fttmr010->is_aspeed)
fttmr010->delay_timer.read_current_timer = fttmr010->delay_timer.read_current_timer =
fttmr010_read_current_timer_down; fttmr010_read_current_timer_down;
else else