diff options
author | Jack Rosenthal <jrosenth@chromium.org> | 2019-04-02 10:28:23 -0600 |
---|---|---|
committer | chrome-bot <chrome-bot@chromium.org> | 2019-04-11 01:36:32 -0700 |
commit | a98d904327f88372cfa0c920ed558626939bf4e1 (patch) | |
tree | 27fb327b9961486f53fcfec577d5dd182fe6c35c /chip | |
parent | 2b00d45486ea6f827bf3f4a58d802279360d583e (diff) | |
download | chrome-ec-a98d904327f88372cfa0c920ed558626939bf4e1.tar.gz |
ish: solve timer reliability and time jumps
b:129805553 demonstrates that the time jumps by 2^32 us twice during
boot. This appears to have been caused by leftover interrupts that
have not been cleared while the system was booting triggering T0.
In addition, b:124890290 discusses that the LRE bit should not be set
in HPET configuration, and the do-while loop introduced in CL:1457597
is no longer needed if the settling spin-lock is placed before arming
the timer. This resolves that
BRANCH=none
BUG=b:124890290,b:12980553
TEST=timer appears to be working reliably for 24 hours on arcada, no
time jumps
Change-Id: Ib0c6b8c9c7e8fe2111a312b9c48061b296178c72
Signed-off-by: Jack Rosenthal <jrosenth@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1548505
Reviewed-by: Jett Rink <jettrink@chromium.org>
Reviewed-by: Paul Fagerburg <pfagerburg@chromium.org>
Diffstat (limited to 'chip')
-rw-r--r-- | chip/ish/hpet.h | 26 | ||||
-rw-r--r-- | chip/ish/hwtimer.c | 80 |
2 files changed, 61 insertions, 45 deletions
diff --git a/chip/ish/hpet.h b/chip/ish/hpet.h index ee26162518..3823a6e858 100644 --- a/chip/ish/hpet.h +++ b/chip/ish/hpet.h @@ -15,9 +15,8 @@ /* HPET_GENERAL_CONFIG settings */ -#define HPET_GENERAL_CONFIG REG32(ISH_HPET_BASE + 0x10) -#define HPET_ENABLE_CNF (1<<0) -#define HPET_LEGACY_RT_CNF (1<<1) +#define HPET_GENERAL_CONFIG REG32(ISH_HPET_BASE + 0x10) +#define HPET_ENABLE_CNF BIT(0) /* Interrupt status acknowledge register */ #define HPET_INTR_CLEAR REG32(ISH_HPET_BASE + 0x20) @@ -29,11 +28,11 @@ /* HPET Timer 0/1/2 configuration*/ #define HPET_TIMER_CONF_CAP(x) REG32(ISH_HPET_BASE + 0x100 + ((x) * 0x20)) -#define HPET_Tn_INT_TYPE_CNF (1<<1) -#define HPET_Tn_INT_ENB_CNF (1<<2) -#define HPET_Tn_TYPE_CNF (1<<3) -#define HPET_Tn_VAL_SET_CNF (1<<6) -#define HPET_Tn_32MODE_CNF (1<<8) +#define HPET_Tn_INT_TYPE_CNF BIT(1) +#define HPET_Tn_INT_ENB_CNF BIT(2) +#define HPET_Tn_TYPE_CNF BIT(3) +#define HPET_Tn_VAL_SET_CNF BIT(6) +#define HPET_Tn_32MODE_CNF BIT(8) #define HPET_Tn_INT_ROUTE_CNF_SHIFT 0x9 #define HPET_Tn_INT_ROUTE_CNF_MASK (0x1f << 9) @@ -47,12 +46,17 @@ /* ISH 4/5: Special status register * Use this register to see HPET timer are settled after a write. */ -#define HPET_CTRL_STATUS REG32(ISH_HPET_BASE + 0x160) -#define HPET_T1_CMP_SETTLING BIT(8) +#define HPET_CTRL_STATUS REG32(ISH_HPET_BASE + 0x160) +#define HPET_T1_CMP_SETTLING BIT(9) +#define HPET_T0_CMP_SETTLING (BIT(7) | BIT(8)) #define HPET_T1_CAP_SETTLING BIT(5) -#define HPET_MAIN_COUNTER_SETTLING BIT(2) +#define HPET_T0_CAP_SETTLING BIT(4) +#define HPET_MAIN_COUNTER_SETTLING (BIT(2) | BIT(3)) #define HPET_T1_SETTLING (HPET_T1_CAP_SETTLING | \ HPET_T1_CMP_SETTLING) +#define HPET_T0_SETTLING (HPET_T0_CAP_SETTLING | \ + HPET_T0_CMP_SETTLING) +#define HPET_ANY_SETTLING (BIT(12) - 1) #if defined(CHIP_FAMILY_ISH3) #define ISH_HPET_CLK_FREQ 12000000 /* 12 MHz clock */ diff --git a/chip/ish/hwtimer.c b/chip/ish/hwtimer.c index 79a48783d9..393e1af4af 100644 --- a/chip/ish/hwtimer.c +++ b/chip/ish/hwtimer.c @@ -75,6 +75,12 @@ static inline uint32_t scale_ticks2us(uint64_t ticks) return quotient; } +static inline void wait_while_settling(uint32_t mask) +{ + /* Do nothing on ISH3, only ISH4 and ISH5 need settling */ + (void) mask; +} + #elif defined(CHIP_FAMILY_ISH4) || defined(CHIP_FAMILY_ISH5) #define CLOCK_SCALE_BITS 15 BUILD_ASSERT(BIT(CLOCK_SCALE_BITS) == ISH_HPET_CLK_FREQ); @@ -113,7 +119,23 @@ static inline uint32_t scale_ticks2us(uint64_t ticks) return intermediate >> CLOCK_SCALE_BITS; } -#endif /* CHIP_FAMILY_ISH4 || CHIP_FAMILY_ISH5 */ + +/* + * HPET Control & Status register may indicate that a value which has + * been written still needs propogated by hardware. Before updating + * HPET_TIMER_CONF_CAP(N), be sure to wait on the value settling with + * the corresponding mask (see hpet.h). + */ +static inline void wait_while_settling(uint32_t mask) +{ + /* Wait for timer settings to settle ~ 150us */ + while (HPET_CTRL_STATUS & mask) + continue; +} + +#else +#error "Must define CHIP_FAMILY_ISH(3|4|5)" +#endif /* * The 64-bit read on a 32-bit chip can tear during the read. Ensure that the @@ -152,20 +174,10 @@ void __hw_clock_event_set(uint32_t deadline) */ HPET_TIMER_COMP(1) = read_main_timer() + scale_us2ticks(remaining_us); - do { - /* Arm timer */ - HPET_TIMER_CONF_CAP(1) |= HPET_Tn_INT_ENB_CNF; + wait_while_settling(HPET_T1_SETTLING); -#if defined(CHIP_FAMILY_ISH4) || defined(CHIP_FAMILY_ISH5) - /* Wait for timer settings to settle ~ 150us */ - while (HPET_CTRL_STATUS & HPET_T1_SETTLING) - continue; -#endif - /* - * TODO(b/124890290): Remove or update while loop that ensures timer is - * armed once we have a better hardware understanding. - */ - } while (!(HPET_TIMER_CONF_CAP(1) & HPET_Tn_INT_ENB_CNF)); + /* Arm timer */ + HPET_TIMER_CONF_CAP(1) |= HPET_Tn_INT_ENB_CNF; } uint32_t __hw_clock_event_get(void) @@ -175,6 +187,7 @@ uint32_t __hw_clock_event_get(void) void __hw_clock_event_clear(void) { + wait_while_settling(HPET_T1_SETTLING); HPET_TIMER_CONF_CAP(1) &= ~HPET_Tn_INT_ENB_CNF; } @@ -186,18 +199,13 @@ uint32_t __hw_clock_source_read(void) void __hw_clock_source_set(uint32_t ts) { /* Reset both clock and overflow comparators */ - + wait_while_settling(HPET_ANY_SETTLING); HPET_GENERAL_CONFIG &= ~HPET_ENABLE_CNF; HPET_MAIN_COUNTER_64 = scale_us2ticks(ts); HPET_TIMER0_COMP_64 = ROLLOVER_CMP_VAL; -#if defined(CHIP_FAMILY_ISH4) || defined(CHIP_FAMILY_ISH5) - /* Wait for timer to settle. required for ISH 4 */ - while (HPET_CTRL_STATUS & HPET_MAIN_COUNTER_SETTLING) - continue; -#endif - + wait_while_settling(HPET_ANY_SETTLING); HPET_GENERAL_CONFIG |= HPET_ENABLE_CNF; } @@ -227,7 +235,6 @@ DECLARE_IRQ(ISH_HPET_TIMER1_IRQ, __hw_clock_source_irq_1); int __hw_clock_source_init(uint32_t start_t) { - /* * The timer can only fire interrupt when its value reaches zero. * Therefore we need two timers: @@ -239,9 +246,20 @@ int __hw_clock_source_init(uint32_t start_t) uint32_t timer1_config = 0x00000000; /* Disable HPET */ + wait_while_settling(HPET_ANY_SETTLING); HPET_GENERAL_CONFIG &= ~HPET_ENABLE_CNF; + + /* Disable T0 and T1 until we get them set up (below) */ + HPET_TIMER_CONF_CAP(0) &= ~HPET_Tn_INT_ENB_CNF; + HPET_TIMER_CONF_CAP(1) &= ~HPET_Tn_INT_ENB_CNF; + + /* Initialize main counter */ HPET_MAIN_COUNTER_64 = scale_us2ticks(start_t); + /* Clear any interrupts from previously running image */ + HPET_INTR_CLEAR = BIT(0); + HPET_INTR_CLEAR = BIT(1); + /* Set comparator value for Timer 0 and enable periodic mode */ HPET_TIMER0_COMP_64 = ROLLOVER_CMP_VAL; timer0_config |= HPET_Tn_TYPE_CNF; @@ -252,7 +270,7 @@ int __hw_clock_source_init(uint32_t start_t) /* Timer 1 - IRQ routing */ timer1_config &= ~HPET_Tn_INT_ROUTE_CNF_MASK; timer1_config |= (ISH_HPET_TIMER1_IRQ << - HPET_Tn_INT_ROUTE_CNF_SHIFT); + HPET_Tn_INT_ROUTE_CNF_SHIFT); /* Level triggered interrupt */ timer0_config |= HPET_Tn_INT_TYPE_CNF; @@ -261,6 +279,9 @@ int __hw_clock_source_init(uint32_t start_t) /* Enable interrupt */ timer0_config |= HPET_Tn_INT_ENB_CNF; + /* Before enabling, previous values must have settled */ + wait_while_settling(HPET_ANY_SETTLING); + /* Unmask HPET IRQ in IOAPIC */ task_enable_irq(ISH_HPET_TIMER0_IRQ); task_enable_irq(ISH_HPET_TIMER1_IRQ); @@ -269,17 +290,8 @@ int __hw_clock_source_init(uint32_t start_t) HPET_TIMER_CONF_CAP(0) |= timer0_config; HPET_TIMER_CONF_CAP(1) |= timer1_config; -#if defined(CHIP_FAMILY_ISH4) || defined(CHIP_FAMILY_ISH5) - /* Wait for timer to settle. required for ISH 4 */ - while (HPET_CTRL_STATUS & HPET_MAIN_COUNTER_SETTLING) - continue; -#endif - - /* - * LEGACY_RT_CNF for HPET1 interrupt routing - * and enable overall HPET counter/interrupts. - */ - HPET_GENERAL_CONFIG |= (HPET_ENABLE_CNF | HPET_LEGACY_RT_CNF); + /* Enable HPET */ + HPET_GENERAL_CONFIG |= HPET_ENABLE_CNF; /* Return IRQ value for OS event timer */ return ISH_HPET_TIMER1_IRQ; |