| From fe2df0bfc0505cc146d76991f9e2370bd903238a Mon Sep 17 00:00:00 2001 |
| From: Stephen Boyd <sboyd@codeaurora.org> |
| Date: Thu, 18 Jul 2013 16:59:28 -0700 |
| Subject: clocksource: arch_timer: Make register accessors less error-prone |
| |
| Using an enum for the register we wish to access allows newer |
| compilers to determine if we've forgotten a case in our switch |
| statement. This allows us to remove the BUILD_BUG() instances in |
| the arm64 port, avoiding problems where optimizations may not |
| happen. |
| |
| To try and force better code generation we're currently marking |
| the accessor functions as inline, but newer compilers can ignore |
| the inline keyword unless it's marked __always_inline. Luckily on |
| arm and arm64 inline is __always_inline, but let's make |
| everything __always_inline to be explicit. |
| |
| Suggested-by: Thomas Gleixner <tglx@linutronix.de> |
| Cc: Thomas Gleixner <tglx@linutronix.de> |
| Cc: Mark Rutland <mark.rutland@arm.com> |
| Cc: Marc Zyngier <Marc.Zyngier@arm.com> |
| Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> |
| Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> |
| Acked-by: Mark Rutland <mark.rutland@arm.com> |
| (cherry picked from commit e09f3cc0184d6b5c3816f921b7ffb67623e5e834) |
| Signed-off-by: Simon Horman <horms+renesas@verge.net.au> |
| --- |
| arch/arm/include/asm/arch_timer.h | 14 ++++++-------- |
| arch/arm64/include/asm/arch_timer.h | 23 +++++++++-------------- |
| drivers/clocksource/arm_arch_timer.c | 6 +++--- |
| include/clocksource/arm_arch_timer.h | 6 ++++-- |
| 4 files changed, 22 insertions(+), 27 deletions(-) |
| |
| diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h |
| index accefe09..aeb93f38 100644 |
| --- a/arch/arm/include/asm/arch_timer.h |
| +++ b/arch/arm/include/asm/arch_timer.h |
| @@ -17,7 +17,8 @@ int arch_timer_arch_init(void); |
| * nicely work out which register we want, and chuck away the rest of |
| * the code. At least it does so with a recent GCC (4.6.3). |
| */ |
| -static inline void arch_timer_reg_write(const int access, const int reg, u32 val) |
| +static __always_inline |
| +void arch_timer_reg_write(int access, enum arch_timer_reg reg, u32 val) |
| { |
| if (access == ARCH_TIMER_PHYS_ACCESS) { |
| switch (reg) { |
| @@ -28,9 +29,7 @@ static inline void arch_timer_reg_write(const int access, const int reg, u32 val |
| asm volatile("mcr p15, 0, %0, c14, c2, 0" : : "r" (val)); |
| break; |
| } |
| - } |
| - |
| - if (access == ARCH_TIMER_VIRT_ACCESS) { |
| + } else if (access == ARCH_TIMER_VIRT_ACCESS) { |
| switch (reg) { |
| case ARCH_TIMER_REG_CTRL: |
| asm volatile("mcr p15, 0, %0, c14, c3, 1" : : "r" (val)); |
| @@ -44,7 +43,8 @@ static inline void arch_timer_reg_write(const int access, const int reg, u32 val |
| isb(); |
| } |
| |
| -static inline u32 arch_timer_reg_read(const int access, const int reg) |
| +static __always_inline |
| +u32 arch_timer_reg_read(int access, enum arch_timer_reg reg) |
| { |
| u32 val = 0; |
| |
| @@ -57,9 +57,7 @@ static inline u32 arch_timer_reg_read(const int access, const int reg) |
| asm volatile("mrc p15, 0, %0, c14, c2, 0" : "=r" (val)); |
| break; |
| } |
| - } |
| - |
| - if (access == ARCH_TIMER_VIRT_ACCESS) { |
| + } else if (access == ARCH_TIMER_VIRT_ACCESS) { |
| switch (reg) { |
| case ARCH_TIMER_REG_CTRL: |
| asm volatile("mrc p15, 0, %0, c14, c3, 1" : "=r" (val)); |
| diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h |
| index d56ed11b..dbca7716 100644 |
| --- a/arch/arm64/include/asm/arch_timer.h |
| +++ b/arch/arm64/include/asm/arch_timer.h |
| @@ -26,7 +26,13 @@ |
| |
| #include <clocksource/arm_arch_timer.h> |
| |
| -static inline void arch_timer_reg_write(int access, int reg, u32 val) |
| +/* |
| + * These register accessors are marked inline so the compiler can |
| + * nicely work out which register we want, and chuck away the rest of |
| + * the code. |
| + */ |
| +static __always_inline |
| +void arch_timer_reg_write(int access, enum arch_timer_reg reg, u32 val) |
| { |
| if (access == ARCH_TIMER_PHYS_ACCESS) { |
| switch (reg) { |
| @@ -36,8 +42,6 @@ static inline void arch_timer_reg_write(int access, int reg, u32 val) |
| case ARCH_TIMER_REG_TVAL: |
| asm volatile("msr cntp_tval_el0, %0" : : "r" (val)); |
| break; |
| - default: |
| - BUILD_BUG(); |
| } |
| } else if (access == ARCH_TIMER_VIRT_ACCESS) { |
| switch (reg) { |
| @@ -47,17 +51,14 @@ static inline void arch_timer_reg_write(int access, int reg, u32 val) |
| case ARCH_TIMER_REG_TVAL: |
| asm volatile("msr cntv_tval_el0, %0" : : "r" (val)); |
| break; |
| - default: |
| - BUILD_BUG(); |
| } |
| - } else { |
| - BUILD_BUG(); |
| } |
| |
| isb(); |
| } |
| |
| -static inline u32 arch_timer_reg_read(int access, int reg) |
| +static __always_inline |
| +u32 arch_timer_reg_read(int access, enum arch_timer_reg reg) |
| { |
| u32 val; |
| |
| @@ -69,8 +70,6 @@ static inline u32 arch_timer_reg_read(int access, int reg) |
| case ARCH_TIMER_REG_TVAL: |
| asm volatile("mrs %0, cntp_tval_el0" : "=r" (val)); |
| break; |
| - default: |
| - BUILD_BUG(); |
| } |
| } else if (access == ARCH_TIMER_VIRT_ACCESS) { |
| switch (reg) { |
| @@ -80,11 +79,7 @@ static inline u32 arch_timer_reg_read(int access, int reg) |
| case ARCH_TIMER_REG_TVAL: |
| asm volatile("mrs %0, cntv_tval_el0" : "=r" (val)); |
| break; |
| - default: |
| - BUILD_BUG(); |
| } |
| - } else { |
| - BUILD_BUG(); |
| } |
| |
| return val; |
| diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c |
| index 053d846a..aa070384 100644 |
| --- a/drivers/clocksource/arm_arch_timer.c |
| +++ b/drivers/clocksource/arm_arch_timer.c |
| @@ -43,7 +43,7 @@ static bool arch_timer_use_virtual = true; |
| * Architected system timer support. |
| */ |
| |
| -static inline irqreturn_t timer_handler(const int access, |
| +static __always_inline irqreturn_t timer_handler(const int access, |
| struct clock_event_device *evt) |
| { |
| unsigned long ctrl; |
| @@ -72,7 +72,7 @@ static irqreturn_t arch_timer_handler_phys(int irq, void *dev_id) |
| return timer_handler(ARCH_TIMER_PHYS_ACCESS, evt); |
| } |
| |
| -static inline void timer_set_mode(const int access, int mode) |
| +static __always_inline void timer_set_mode(const int access, int mode) |
| { |
| unsigned long ctrl; |
| switch (mode) { |
| @@ -99,7 +99,7 @@ static void arch_timer_set_mode_phys(enum clock_event_mode mode, |
| timer_set_mode(ARCH_TIMER_PHYS_ACCESS, mode); |
| } |
| |
| -static inline void set_next_event(const int access, unsigned long evt) |
| +static __always_inline void set_next_event(const int access, unsigned long evt) |
| { |
| unsigned long ctrl; |
| ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL); |
| diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h |
| index c463ce99..f3da817b 100644 |
| --- a/include/clocksource/arm_arch_timer.h |
| +++ b/include/clocksource/arm_arch_timer.h |
| @@ -23,8 +23,10 @@ |
| #define ARCH_TIMER_CTRL_IT_MASK (1 << 1) |
| #define ARCH_TIMER_CTRL_IT_STAT (1 << 2) |
| |
| -#define ARCH_TIMER_REG_CTRL 0 |
| -#define ARCH_TIMER_REG_TVAL 1 |
| +enum arch_timer_reg { |
| + ARCH_TIMER_REG_CTRL, |
| + ARCH_TIMER_REG_TVAL, |
| +}; |
| |
| #define ARCH_TIMER_PHYS_ACCESS 0 |
| #define ARCH_TIMER_VIRT_ACCESS 1 |
| -- |
| 1.8.4.3.gca3854a |
| |