From 93aa10cc5c74e5f045f7ce812649dccc79d39db8 Mon Sep 17 00:00:00 2001 From: Gerson Fernando Budke Date: Tue, 27 Jun 2023 22:41:18 +0200 Subject: [PATCH] cpu/avr8_common: Rework avr8_state variable The avr8_state variable uses bit operation to set/clear the state. This rework avr8_state to use increment/decrement instead. It introduce the use of General Purpose IO Register 1 (GPIOR1) when available. This is a preparation for future scheduling and irq optimizations. Signed-off-by: Gerson Fernando Budke --- cpu/avr8_common/avr8_cpu.c | 5 +- cpu/avr8_common/include/cpu.h | 57 +---------------------- cpu/avr8_common/include/irq_arch.h | 9 ++-- cpu/avr8_common/include/states_internal.h | 33 +++++++++++++ cpu/avr8_common/thread_arch.c | 7 +-- 5 files changed, 48 insertions(+), 63 deletions(-) diff --git a/cpu/avr8_common/avr8_cpu.c b/cpu/avr8_common/avr8_cpu.c index 5d1c4ccb6e..7bd258bfbe 100644 --- a/cpu/avr8_common/avr8_cpu.c +++ b/cpu/avr8_common/avr8_cpu.c @@ -68,7 +68,9 @@ */ uint8_t mcusr_mirror __attribute__((section(".noinit"))); uint8_t soft_rst __attribute__((section(".noinit"))); -uint8_t avr8_state = 0; +#if (AVR8_STATE_IRQ_USE_SRAM) +uint8_t avr8_state_irq_count_sram = 0; +#endif #if (AVR8_STATE_UART_USE_SRAM) uint8_t avr8_state_uart_sram = 0; #endif @@ -123,6 +125,7 @@ void cpu_init(void) /* Set global resources initial state */ avr8_state_uart = 0; + avr8_state_irq_count = 0; irq_enable(); } diff --git a/cpu/avr8_common/include/cpu.h b/cpu/avr8_common/include/cpu.h index f7df4fa320..5cfeca21d0 100644 --- a/cpu/avr8_common/include/cpu.h +++ b/cpu/avr8_common/include/cpu.h @@ -58,61 +58,6 @@ extern "C" #define PERIPH_I2C_NEED_WRITE_REGS /** @} */ -/** - * @name Flags for the current state of the ATmega MCU - * @{ - */ -#define AVR8_STATE_FLAG_ISR (0x80U) /**< In ISR */ -/** @} */ - -/** - * @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 avr8_get_state should be used. - * - * Contents: - * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - * 7 6 5 4 3 2 1 0 - * +---+---+---+---+---+---+---+---+ - * |IRQ| RESERVED | - * +---+---+---+---+---+---+---+---+ - * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - * - * | Label | Description | - * |:-------|:--------------------------------------------------------------| - * | IRQ | This bit is set when in IRQ context | - */ -extern uint8_t avr8_state; - -/** - * @brief Atomically read the state (@ref avr8_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 avr8_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 avr8_get_state(void) -{ - uint8_t state; - __asm__ volatile( - "lds %[state], avr8_state \n\t" - : [state] "=r" (state) - : - : "memory" - - ); - - return state; -} - /** * @brief Run this code on entering interrupt routines */ @@ -122,7 +67,7 @@ static inline void avr8_enter_isr(void) * 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 */ - avr8_state |= AVR8_STATE_FLAG_ISR; + ++avr8_state_irq_count; } /** diff --git a/cpu/avr8_common/include/irq_arch.h b/cpu/avr8_common/include/irq_arch.h index b7d3ecf948..13cbb8dbed 100644 --- a/cpu/avr8_common/include/irq_arch.h +++ b/cpu/avr8_common/include/irq_arch.h @@ -2,7 +2,7 @@ * Copyright (C) 2014 Freie Universität Berlin, Hinnerk van Bruinehsen * 2018 RWTH Aachen, Josua Arndt * 2020 Otto-von-Guericke-Universität Magdeburg - * 2021 Gerson Fernando Budke + * 2021-2023 Gerson Fernando Budke * * 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 @@ -100,8 +100,11 @@ __attribute__((always_inline)) static inline void irq_restore(unsigned int _stat */ __attribute__((always_inline)) static inline bool irq_is_in(void) { - uint8_t state = avr8_get_state(); - return (state & AVR8_STATE_FLAG_ISR); + bool is_in = avr8_state_irq_count > 0; + + __asm__ volatile ("" : : : "memory"); + + return is_in; } /** diff --git a/cpu/avr8_common/include/states_internal.h b/cpu/avr8_common/include/states_internal.h index 189d9690ec..788c7c2099 100644 --- a/cpu/avr8_common/include/states_internal.h +++ b/cpu/avr8_common/include/states_internal.h @@ -72,6 +72,39 @@ extern uint8_t avr8_state_uart_sram; /**< UART state variable. */ #endif /** @} */ +/** + * @name Internal flag which defines if IRQ state is stored on SRAM + * @{ + */ +#ifdef GPIOR1 +#define AVR8_STATE_IRQ_USE_SRAM 0 /**< IRQ state using GPIOR registers. */ +#else +#define AVR8_STATE_IRQ_USE_SRAM 1 /**< IRQ state using GPIOR registers. */ +#endif +/** @} */ + +/** + * @name 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 should be + * used. + * + * Contents: + * + * | Label | Description | + * |:-------|:--------------------------------------------------------------| + * | IRQ | This variable is incremented when in IRQ context | + */ +#if (AVR8_STATE_IRQ_USE_SRAM) +extern uint8_t avr8_state_irq_count_sram; /**< IRQ state variable. */ +#define avr8_state_irq_count avr8_state_irq_count_sram /**< Definition for SRAM. */ +#else +#define avr8_state_irq_count GPIOR1 /**< Definition for GPIOR1. */ +#endif +/** @} */ + #ifdef __cplusplus } #endif diff --git a/cpu/avr8_common/thread_arch.c b/cpu/avr8_common/thread_arch.c index 768583a93f..ba318767f9 100644 --- a/cpu/avr8_common/thread_arch.c +++ b/cpu/avr8_common/thread_arch.c @@ -2,7 +2,7 @@ * Copyright (C) 2014 Freie Universität Berlin, Hinnerk van Bruinehsen * 2017 Thomas Perrot * 2018 RWTH Aachen, Josua Arndt - * 2021 Gerson Fernando Budke + * 2021-2023 Gerson Fernando Budke * * 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 @@ -273,12 +273,13 @@ void thread_yield_higher(void) void avr8_exit_isr(void) { - avr8_state &= ~AVR8_STATE_FLAG_ISR; + --avr8_state_irq_count; /* Force access to avr8_state to take place */ __asm__ volatile ("" : : : "memory"); - if (sched_context_switch_request) { + /* schedule should switch context when returning from a non nested interrupt */ + if (sched_context_switch_request && avr8_state_irq_count == 0) { avr8_context_save(); sched_run(); avr8_context_restore();