Merge pull request #14362 from maribu/msp430-irq-inline

cpu/msp430_common: Update to inline-able IRQ API

cpu/msp430_common/cpu.c

@@ -21,7 +21,7 @@
 __attribute__((naked)) void thread_yield_higher(void)
 {
     __asm__("push r2"); /* save SR */
-    __disable_irq();
+    irq_disable();
 
     __save_context();
 

cpu/msp430_common/include/cpu.h

@@ -45,28 +45,6 @@ extern "C" {
  */
 #define ISR(a,b)        void __attribute__((naked, interrupt (a))) b(void)
 
-/**
- * @brief Globally disable IRQs
- */
-static inline void __attribute__((always_inline)) __disable_irq(void)
-{
-    __asm__ __volatile__("bic  %0, r2" : : "i"(GIE));
-    /* this NOP is needed to handle a "delay slot" that all MSP430 MCUs
-       impose silently after messing with the GIE bit, DO NOT REMOVE IT! */
-    __asm__ __volatile__("nop");
-}
-
-/**
- * @brief Globally enable IRQs
- */
-static inline void __attribute__((always_inline)) __enable_irq(void)
-{
-    __asm__ __volatile__("bis  %0, r2" : : "i"(GIE));
-    /* this NOP is needed to handle a "delay slot" that all MSP430 MCUs
-       impose silently after messing with the GIE bit, DO NOT REMOVE IT! */
-    __asm__ __volatile__("nop");
-}
-
 /**
  * @brief   The current ISR state (inside or not)
  */

cpu/msp430_common/include/cpu_conf.h

@@ -22,6 +22,11 @@
 extern "C" {
 #endif
 
+/**
+ * @brief   This arch uses the inlined IRQ API.
+ */
+#define IRQ_API_INLINED     (1)
+
 /**
  * @name   Configure the internal flash memory
  * @{

cpu/msp430_common/include/irq_arch.h

@@ -0,0 +1,101 @@
+/*
+ * Copyright (C) 2014 Freie Universität Berlin
+ *               2020 Otto-von-Guericke-Universität Magdeburg
+ *
+ * This file is subject to the terms and conditions of the GNU Lesser
+ * General Public License v2.1. See the file LICENSE in the top level
+ * directory for more details.
+ */
+
+/**
+ * @ingroup     cpu_msp430_common
+* @{
+ *
+ * @file
+ * @brief       ISR related functions
+ *
+ * @author      Kaspar Schleiser <kaspar@schleiser.de>
+ * @author      Oliver Hahm <oliver.hahm@inria.fr>
+ * @author      Marian Buschsieweke <marian.buschsieweke@ovgu.de>
+ *
+ */
+
+#ifndef IRQ_ARCH_H
+#define IRQ_ARCH_H
+
+#include "irq.h"
+#include "cpu.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+extern volatile int __irq_is_in;
+
+__attribute__((always_inline)) static inline unsigned int irq_disable(void)
+{
+    unsigned int state;
+    __asm__ volatile(
+        "mov.w r2, %[state]"                "\n\t"
+        "bic %[gie], r2"                    "\n\t"
+        /*
+         * BEWARE: IRQs remain enabled for one instruction after clearing the
+         * GIE bit in the status register (r2). Thus, the next instruction is
+         * not only used to sanitize the IRQ state, but also delays the actual
+         * critical section by one CPU cycle, so that IRQs are indeed disabled
+         * by then.
+         */
+        "and %[gie], %[state]"              "\n\t"
+        : [state]   "=r"(state)
+        : [gie]     "i"(GIE)
+        : "memory"
+    );
+
+    return state;
+}
+
+__attribute__((always_inline)) static inline unsigned int irq_enable(void)
+{
+    unsigned int state;
+    __asm__ volatile(
+        "mov.w r2, %[state]"                "\n\t"
+        "bis %[gie], r2"                    "\n\t"
+        /*
+         * BEWARE: IRQs remain disabled for one instruction after setting the
+         * GIE bit in the status register (r2). Thus, the next instruction is
+         * not only used to sanitize the IRQ state, but also ensures that the
+         * first instruction after this function is run with IRQs enabled.
+         */
+        "and %[gie], %[state]"              "\n\t"
+        : [state]   "=r"(state)
+        : [gie]     "i"(GIE)
+        : "memory"
+    );
+
+    return state;
+}
+
+__attribute__((always_inline)) static inline void irq_restore(unsigned int state)
+{
+    __asm__ volatile(
+        "bis %[state], r2"                    "\n\t"
+        : /* no outputs */
+        : [state]   "r"(state)
+        : "memory"
+    );
+    /* BEWARE: IRQs remain disabled for up to one CPU cycle after this function
+     * call. But that doesn't seem to be harmful.
+     */
+}
+
+__attribute__((always_inline)) static inline int irq_is_in(void)
+{
+    return __irq_is_in;
+}
+
+#ifdef __cplusplus
+}
+#endif
+
+/** @} */
+#endif /* IRQ_ARCH_H */

cpu/msp430_common/irq.c

@@ -7,11 +7,11 @@
  */
 
 /**
- * @ingroup     cpu
+ * @ingroup     cpu_msp430_common
 * @{
  *
  * @file
- * @brief       ISR related functions
+ * @brief       ISR related variables
  *
  * @author      Kaspar Schleiser <kaspar@schleiser.de>
  * @author      Oliver Hahm <oliver.hahm@inria.fr>
@@ -25,41 +25,3 @@
 volatile int __irq_is_in = 0;
 
 char __isr_stack[ISR_STACKSIZE];
-
-unsigned int irq_disable(void)
-{
-    unsigned int state;
-    __asm__("mov.w r2,%0" : "=r"(state));
-    state &= GIE;
-
-    if (state) {
-        __disable_irq();
-    }
-
-    return state;
-}
-
-unsigned int irq_enable(void)
-{
-    unsigned int state;
-    __asm__("mov.w r2,%0" : "=r"(state));
-    state &= GIE;
-
-    if (!state) {
-        __enable_irq();
-    }
-
-    return state;
-}
-
-void irq_restore(unsigned int state)
-{
-    if (state) {
-        __enable_irq();
-    }
-}
-
-int irq_is_in(void)
-{
-    return __irq_is_in;
-}