cpu/atmega_common: Cleanup state flags

- Use one byte of RAM to track both IRQ and UART TX state
- Fix incorrect use of volatile

cpu/atmega_common/include/cpu.h

@@ -59,16 +59,88 @@ extern "C"
 /** @} */
 
 /**
- * @brief global in-ISR state variable
+ * @name    Flags for the current state of the ATmega MCU
+ * @{
  */
-extern volatile uint8_t atmega_in_isr;
+#define ATMEGA_STATE_FLAG_ISR           (0x80U) /**< In ISR */
+#define ATMEGA_STATE_FLAG_UART0_TX      (0x01U) /**< TX pending for UART 0 */
+#define ATMEGA_STATE_FLAG_UART1_TX      (0x02U) /**< TX pending for UART 1 */
+#define ATMEGA_STATE_FLAG_UART_TX(x)    (0x01U << x) /**< TX pending for UART x */
+/** @} */
 
 /**
- * @brief Run this code on entering interrupt routines
+ * @brief   Global variable containing the current state of the MCU
+ *
+ * @note    This variable is updated from IRQ context; access to it should
+ *          be wrapped into @ref irq_disable and @ref irq_restore or
+ *          @ref atmega_get_state should be used.
+ *
+ * Contents:
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *   7   6   5   4   3   2   1   0
+ * +---+---+---+---+---+---+---+---+
+ * |IRQ| unused            |TX1|TX0|
+ * +---+---+---+---+---+---+---+---+
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * | Label  | Description                                                   |
+ * |:-------|:--------------------------------------------------------------|
+ * | IRQ    | This bit is set when in IRQ context                           |
+ * | unused | This bits are currently not used                              |
+ * | TX1    | This bit is set when on UART1 TX is pending                   |
+ * | TX0    | This bit is set when on UART0 TX is pending                   |
+ */
+extern uint8_t atmega_state;
+
+/**
+ * @brief   Atomically read the state (@ref atmega_state)
+ *
+ * This function guarantees that the read is not optimized out, not reordered
+ * and done atomically. This does not mean that by the time return value is
+ * processed that it still reflects the value currently stored in
+ * @ref atmega_state.
+ *
+ * Using ASM rather than C11 atomics has less overhead, as not every access to
+ * the state has to be performed atomically: Those done from ISR will not be
+ * interrupted (no support for nested interrupts) and barriers at the begin and
+ * end of the ISRs make sure the access takes place before IRQ context is left.
+ */
+static inline uint8_t atmega_get_state(void)
+{
+    uint8_t state;
+    __asm__ volatile(
+        "lds   %[state], atmega_state       \n\t"
+        : [state]   "=r" (state)
+        :
+        : "memory"
+
+    );
+
+    return state;
+}
+
+/**
+ * @brief   Run this code on entering interrupt routines
  */
 static inline void atmega_enter_isr(void)
 {
-    atmega_in_isr = 1;
+    /* This flag is only called from IRQ context, and nested IRQs are not
+     * supported as of now. The flag will be unset before the IRQ context is
+     * left, so no need to use memory barriers or atomics here
+     */
+    atmega_state |= ATMEGA_STATE_FLAG_ISR;
+}
+
+/**
+ * @brief   Check if TX on any present UART device is still pending
+ *
+ * @retval  !=0     At least on UART device is still sending data out
+ * @retval  0       No UART is currently sending data
+ */
+static inline int atmega_is_uart_tx_pending(void)
+{
+    uint8_t state = atmega_get_state();
+    return (state & (ATMEGA_STATE_FLAG_UART0_TX | ATMEGA_STATE_FLAG_UART1_TX));
 }
 
 /**

cpu/atmega_common/irq_arch.c

@@ -23,6 +23,7 @@
 
 #include <stdint.h>
 #include <stdio.h>
+#include <stdint.h>
 #include "irq.h"
 #include "cpu.h"
 
@@ -32,7 +33,7 @@
 static uint8_t atmega_get_interrupt_state(void);
 static void atmega_set_interrupt_state(uint8_t state);
 
-volatile uint8_t atmega_in_isr = 0;
+uint8_t atmega_state = 0;
 
 __attribute__((always_inline)) static inline uint8_t atmega_get_interrupt_state(void)
 {
@@ -88,7 +89,6 @@ void irq_restore(unsigned int state)
  */
 int irq_is_in(void)
 {
-    int result = atmega_in_isr;
+    uint8_t state = atmega_get_state();
-    __asm__ volatile("" ::: "memory");
+    return (state & ATMEGA_STATE_FLAG_ISR);
-    return result;
 }

cpu/atmega_common/periph/uart.c

@@ -80,12 +80,6 @@ static mega_uart_t *dev[] = {
  */
 static uart_isr_ctx_t isr_ctx[UART_NUMOF];
 
-/**
- * @brief   Allocate variable to hold transmission status, set when there
- *          is data in UDRn or in the Transmit Shift Register
- */
-static volatile uint8_t _tx_pending = 0;
-
 static void _update_brr(uart_t uart, uint16_t brr, bool double_speed)
 {
     dev[uart]->BRR = brr;
@@ -126,8 +120,8 @@ int uart_init(uart_t uart, uint32_t baudrate, uart_rx_cb_t rx_cb, void *arg)
         return UART_NODEV;
     }
 
-    uint16_t count = 0xffff;
+    uint16_t count = UINT16_MAX;
-    while (_tx_pending && count--) {}
+    while (atmega_is_uart_tx_pending() && count--) {}
 
     /* register interrupt context */
     isr_ctx[uart].rx_cb = rx_cb;
@@ -178,8 +172,9 @@ void uart_write(uart_t uart, const uint8_t *data, size_t len)
 #endif
         /* start of TX won't finish until no data in UDRn and transmit shift
            register is empty */
-        _tx_pending |= (1 << uart);
+        unsigned long state = irq_disable();
-
+        atmega_state |= ATMEGA_STATE_FLAG_UART_TX(uart);
+        irq_restore(state);
         dev[uart]->DR = data[i];
     }
 }
@@ -211,7 +206,7 @@ static inline void _tx_isr_handler(int num)
 
     /* entire frame in the Transmit Shift Register has been shifted out and
        there are no new data currently present in the transmit buffer */
-    _tx_pending &= ~(1 << num);
+    atmega_state &= ~ATMEGA_STATE_FLAG_UART_TX(num);
 
     atmega_exit_isr();
 }

cpu/atmega_common/thread_arch.c

@@ -247,7 +247,9 @@ void thread_yield_higher(void)
 
 void atmega_exit_isr(void)
 {
-    atmega_in_isr = 0;
+    atmega_state &= ~ATMEGA_STATE_FLAG_ISR;
+    /* Force access to atmega_state to take place */
+    __asm__ volatile ("" : : : "memory");
     if (sched_context_switch_request) {
         atmega_context_save();
         sched_run();