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: Mark Brown <broonie@linaro.org>

diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index 7c1bfc0aea0c200a268ec82c62fd4cb624c17ecf..fa2b4f07de5a7ec8005cf37a200753f16d10e0ea 100644 (file)
--- 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 bf6ab242f04725e56e8cf93d3ab6e930668651f0..0d74a130386c9f472601b84c3a7635c0d64581ac 100644 (file)
--- 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 a2b25418978244d1ae129cf74f031020951f35d4..f72dd518070b9c79a0b9a1654e95a5318c7e2941 100644 (file)
--- 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 e6c9c4cc9b23415fad39dfd142a6ce3bd44f4b7c..bfa1a0141b6e1e057c786d95120f1c61f84bb14f 100644 (file)
--- 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