From 2b1bee750a04dff969e0df4ced1e31d9f8294dd4 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Sat, 23 Nov 2019 11:53:00 +0100 Subject: [PATCH 1/3] cpu/atmega_common: Stop using reserved names Names with two leading underscores are reserved in any context of the c standard, and thus must not be used. This ATmega platform used it however for defining internal stuff. This commit fixes this. --- cpu/atmega_common/cpu.c | 4 ++-- cpu/atmega_common/include/cpu.h | 16 +++++++-------- cpu/atmega_common/irq_arch.c | 18 ++++++++-------- cpu/atmega_common/periph/gpio.c | 8 ++++---- cpu/atmega_common/periph/timer.c | 4 ++-- cpu/atmega_common/periph/uart.c | 4 ++-- cpu/atmega_common/thread_arch.c | 35 +++++++++++++++----------------- 7 files changed, 43 insertions(+), 46 deletions(-) diff --git a/cpu/atmega_common/cpu.c b/cpu/atmega_common/cpu.c index 359dccdcb5..490298886f 100644 --- a/cpu/atmega_common/cpu.c +++ b/cpu/atmega_common/cpu.c @@ -173,8 +173,8 @@ ISR(BADISR_vect) #if defined(CPU_ATMEGA128RFA1) || defined (CPU_ATMEGA256RFR2) ISR(BAT_LOW_vect, ISR_BLOCK) { - __enter_isr(); + atmega_enter_isr(); DEBUG("BAT_LOW\n"); - __exit_isr(); + atmega_exit_isr(); } #endif diff --git a/cpu/atmega_common/include/cpu.h b/cpu/atmega_common/include/cpu.h index d20d103090..d0db41e42c 100644 --- a/cpu/atmega_common/include/cpu.h +++ b/cpu/atmega_common/include/cpu.h @@ -61,14 +61,14 @@ extern "C" /** * @brief global in-ISR state variable */ -extern volatile uint8_t __in_isr; +extern volatile uint8_t atmega_in_isr; /** * @brief Run this code on entering interrupt routines */ -static inline void __enter_isr(void) +static inline void atmega_enter_isr(void) { - __in_isr = 1; + atmega_in_isr = 1; } /** @@ -76,19 +76,19 @@ static inline void __enter_isr(void) * end of ISRs in place of thread_yield_higher. If thread_yield is needed, use * thread_yield followed by thread_yield_isr instead of thread_yield alone. */ -void thread_yield_isr(void); +void atmega_thread_yield_isr(void) /** * @brief Run this code on exiting interrupt routines */ -static inline void __exit_isr(void) +static inline void atmega_exit_isr(void) { if (sched_context_switch_request) { thread_yield(); - __in_isr = 0; - thread_yield_isr(); + atmega_in_isr = 0; + atmega_thread_yield_isr() } - __in_isr = 0; + atmega_in_isr = 0; } /** diff --git a/cpu/atmega_common/irq_arch.c b/cpu/atmega_common/irq_arch.c index 0362111407..a2f0dc035a 100644 --- a/cpu/atmega_common/irq_arch.c +++ b/cpu/atmega_common/irq_arch.c @@ -29,12 +29,12 @@ /** * @brief Macro returns state of the global interrupt register */ -static uint8_t __get_interrupt_state(void); -static void __set_interrupt_state(uint8_t state); +static uint8_t atmega_get_interrupt_state(void); +static void atmega_set_interrupt_state(uint8_t state); -volatile uint8_t __in_isr = 0; +volatile uint8_t atmega_in_isr = 0; -__attribute__((always_inline)) static inline uint8_t __get_interrupt_state(void) +__attribute__((always_inline)) static inline uint8_t atmega_get_interrupt_state(void) { uint8_t sreg; __asm__ volatile( "in __tmp_reg__, __SREG__ \n\t" @@ -43,7 +43,7 @@ __attribute__((always_inline)) static inline uint8_t __get_interrupt_state(void return sreg & (1 << 7); } -__attribute__((always_inline)) inline void __set_interrupt_state(uint8_t state) +__attribute__((always_inline)) inline void atmega_set_interrupt_state(uint8_t state) { __asm__ volatile( "mov r15,%0 \n\t" "in r16, __SREG__ \n\t" @@ -60,7 +60,7 @@ __attribute__((always_inline)) inline void __set_interrupt_state(uint8_t state) */ unsigned int irq_disable(void) { - uint8_t mask = __get_interrupt_state(); + uint8_t mask = atmega_get_interrupt_state(); cli(); /* <-- acts as memory barrier, see doc of avr-libc */ return (unsigned int) mask; } @@ -70,7 +70,7 @@ unsigned int irq_disable(void) */ unsigned int irq_enable(void) { - uint8_t mask = __get_interrupt_state(); + uint8_t mask = atmega_get_interrupt_state(); sei(); /* <-- acts as memory barrier, see doc of avr-libc */ return mask; } @@ -80,7 +80,7 @@ unsigned int irq_enable(void) */ void irq_restore(unsigned int state) { - __set_interrupt_state(state); + atmega_set_interrupt_state(state); } /** @@ -88,7 +88,7 @@ void irq_restore(unsigned int state) */ int irq_is_in(void) { - int result = __in_isr; + int result = atmega_in_isr; __asm__ volatile("" ::: "memory"); return result; } diff --git a/cpu/atmega_common/periph/gpio.c b/cpu/atmega_common/periph/gpio.c index b5046005a2..9c0c5017c4 100644 --- a/cpu/atmega_common/periph/gpio.c +++ b/cpu/atmega_common/periph/gpio.c @@ -429,16 +429,16 @@ void gpio_irq_disable(gpio_t pin) static inline void irq_handler(uint8_t int_num) { - __enter_isr(); + atmega_enter_isr(); config[int_num].cb(config[int_num].arg); - __exit_isr(); + atmega_exit_isr(); } #ifdef PCINT_NUM_BANKS /* inline function that is used by the PCINT ISR */ static inline void pcint_handler(uint8_t bank, uint8_t enabled_pcints) { - __enter_isr(); + atmega_enter_isr(); /* Find right item */ uint8_t idx = 0; @@ -468,7 +468,7 @@ static inline void pcint_handler(uint8_t bank, uint8_t enabled_pcints) idx++; } - __exit_isr(); + atmega_exit_isr(); } #if defined(PCINT0_IDX) ISR(PCINT0_vect, ISR_BLOCK) diff --git a/cpu/atmega_common/periph/timer.c b/cpu/atmega_common/periph/timer.c index b5ef5ea454..e86ef45613 100644 --- a/cpu/atmega_common/periph/timer.c +++ b/cpu/atmega_common/periph/timer.c @@ -176,7 +176,7 @@ static inline void _isr(tim_t tim, int chan) DEBUG_TIMER_PORT |= (1 << DEBUG_TIMER_PIN); #endif - __enter_isr(); + atmega_enter_isr(); *ctx[tim].mask &= ~(1 << (chan + OCIE1A)); ctx[tim].cb(ctx[tim].arg, chan); @@ -185,7 +185,7 @@ static inline void _isr(tim_t tim, int chan) DEBUG_TIMER_PORT &= ~(1 << DEBUG_TIMER_PIN); #endif - __exit_isr(); + atmega_exit_isr(); } #endif diff --git a/cpu/atmega_common/periph/uart.c b/cpu/atmega_common/periph/uart.c index 1d62bc1640..6958500c57 100644 --- a/cpu/atmega_common/periph/uart.c +++ b/cpu/atmega_common/periph/uart.c @@ -186,11 +186,11 @@ void uart_poweroff(uart_t uart) static inline void isr_handler(int num) { - __enter_isr(); + atmega_enter_isr(); isr_ctx[num].rx_cb(isr_ctx[num].arg, dev[num]->DR); - __exit_isr(); + atmega_exit_isr(); } #ifdef UART_0_ISR diff --git a/cpu/atmega_common/thread_arch.c b/cpu/atmega_common/thread_arch.c index db9607d11f..263791f273 100644 --- a/cpu/atmega_common/thread_arch.c +++ b/cpu/atmega_common/thread_arch.c @@ -30,16 +30,13 @@ #include "cpu.h" #include "board.h" -/* - * local function declarations (prefixed with __) - */ -static void __context_save(void); -static void __context_restore(void); -static void __enter_thread_mode(void); +static void atmega_context_save(void); +static void atmega_context_restore(void); +static void atmega_enter_thread_mode(void); /** * @brief Since AVR doesn't support direct manipulation of the program counter we - * model a stack like it would be left by __context_save(). + * model a stack like it would be left by atmega_context_save(). * The resulting layout in memory is the following: * ---------------thread_t (not created by thread_stack_init) ---------- * local variables (a temporary value and the stackpointer) @@ -51,7 +48,7 @@ static void __enter_thread_mode(void); * ----------------------------------------------------------------------- * a 16 Bit pointer to task_func * this is placed exactly at the place where the program counter would be - * stored normally and thus can be returned to when __context_restore() + * stored normally and thus can be returned to when atmega_context_restore() * has been run * (Optional 17 bit (bit is set to zero) for devices with > 128kb FLASH) * ----------------------------------------------------------------------- @@ -64,7 +61,7 @@ static void __enter_thread_mode(void); * r26 - r31 * ----------------------------------------------------------------------- * - * After the invocation of __context_restore() the pointer to task_func is + * After the invocation of atmega_context_restore() the pointer to task_func is * on top of the stack and can be returned to. This way we can actually place * it inside of the programm counter of the MCU. * if task_func returns sched_task_exit gets popped into the PC @@ -201,7 +198,7 @@ void thread_stack_print(void) void cpu_switch_context_exit(void) { sched_run(); - __enter_thread_mode(); + atmega_enter_thread_mode(); } #define STACK_POINTER ((char *)AVR_STACK_POINTER_REG) @@ -213,7 +210,7 @@ extern char *__brkval; /** * @brief Set the MCU into Thread-Mode and load the initial task from the stack and run it */ -void NORETURN __enter_thread_mode(void) +void NORETURN atmega_enter_thread_mode(void) { irq_enable(); @@ -229,7 +226,7 @@ void NORETURN __enter_thread_mode(void) __brkval = __malloc_heap_start; } - __context_restore(); + atmega_context_restore(); __asm__ volatile ("ret"); UNREACHABLE(); @@ -238,9 +235,9 @@ void NORETURN __enter_thread_mode(void) void thread_yield_higher(void) { if (irq_is_in() == 0) { - __context_save(); + atmega_context_save(); sched_run(); - __context_restore(); + atmega_context_restore(); __asm__ volatile ("ret"); } else { @@ -248,16 +245,16 @@ void thread_yield_higher(void) } } -void thread_yield_isr(void) +void atmega_thread_yield_isr(void) { - __context_save(); + atmega_context_save(); sched_run(); - __context_restore(); + atmega_context_restore(); __asm__ volatile ("reti"); } -__attribute__((always_inline)) static inline void __context_save(void) +__attribute__((always_inline)) static inline void atmega_context_save(void) { __asm__ volatile ( "push __tmp_reg__ \n\t" @@ -312,7 +309,7 @@ __attribute__((always_inline)) static inline void __context_save(void) "st x+, __tmp_reg__ \n\t"); } -__attribute__((always_inline)) static inline void __context_restore(void) +__attribute__((always_inline)) static inline void atmega_context_restore(void) { __asm__ volatile ( "lds r26, sched_active_thread \n\t" From 606d72f64bb7a2b77e428032dde93d44c2d42a26 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Sat, 23 Nov 2019 11:57:11 +0100 Subject: [PATCH 2/3] cpu/atmega_common: Clean up & fix IRQ handling At the end of an ISR, the ATmega code was doing an `thread_yield()` instead of a `thread_yield_higher()`. This resulted in tests/isr_yield_higher failing. Fixing this saves a few lines of code, some ROM, and solves the issue. --- cpu/atmega_common/include/cpu.h | 17 +---------------- cpu/atmega_common/thread_arch.c | 3 ++- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/cpu/atmega_common/include/cpu.h b/cpu/atmega_common/include/cpu.h index d0db41e42c..c7321287ce 100644 --- a/cpu/atmega_common/include/cpu.h +++ b/cpu/atmega_common/include/cpu.h @@ -71,25 +71,10 @@ static inline void atmega_enter_isr(void) atmega_in_isr = 1; } -/** - * @brief Exit ISR mode and yield with a return from interrupt. Use at the - * end of ISRs in place of thread_yield_higher. If thread_yield is needed, use - * thread_yield followed by thread_yield_isr instead of thread_yield alone. - */ -void atmega_thread_yield_isr(void) - /** * @brief Run this code on exiting interrupt routines */ -static inline void atmega_exit_isr(void) -{ - if (sched_context_switch_request) { - thread_yield(); - atmega_in_isr = 0; - atmega_thread_yield_isr() - } - atmega_in_isr = 0; -} +void atmega_exit_isr(void); /** * @brief Initialization of the CPU diff --git a/cpu/atmega_common/thread_arch.c b/cpu/atmega_common/thread_arch.c index 263791f273..6edbf4c942 100644 --- a/cpu/atmega_common/thread_arch.c +++ b/cpu/atmega_common/thread_arch.c @@ -245,8 +245,9 @@ void thread_yield_higher(void) } } -void atmega_thread_yield_isr(void) +void atmega_exit_isr(void) { + atmega_in_isr = 0; atmega_context_save(); sched_run(); atmega_context_restore(); From 4347f720f5e113fca43421f0ef83fe4b166e99e6 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Sat, 23 Nov 2019 12:25:30 +0100 Subject: [PATCH 3/3] drivers/at86rf2xx: Update to new ATmega API Replace __enter_isr() with atmega_enter_isr() and __exit_isr() with atmega_exit_isr() --- drivers/at86rf2xx/at86rf2xx_netdev.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/at86rf2xx/at86rf2xx_netdev.c b/drivers/at86rf2xx/at86rf2xx_netdev.c index 3786e3b1cd..51004081ce 100644 --- a/drivers/at86rf2xx/at86rf2xx_netdev.c +++ b/drivers/at86rf2xx/at86rf2xx_netdev.c @@ -732,7 +732,7 @@ static void _isr(netdev_t *netdev) */ ISR(TRX24_RX_END_vect, ISR_BLOCK) { - __enter_isr(); + atmega_enter_isr(); uint8_t status = *AT86RF2XX_REG__TRX_STATE & AT86RF2XX_TRX_STATUS_MASK__TRX_STATUS; DEBUG("TRX24_RX_END 0x%x\n", status); @@ -741,7 +741,7 @@ ISR(TRX24_RX_END_vect, ISR_BLOCK) /* Call upper layer to process received data */ at86rfmega_dev->event_callback(at86rfmega_dev, NETDEV_EVENT_ISR); - __exit_isr(); + atmega_exit_isr(); } /** @@ -754,12 +754,12 @@ ISR(TRX24_RX_END_vect, ISR_BLOCK) */ ISR(TRX24_XAH_AMI_vect, ISR_BLOCK) { - __enter_isr(); + atmega_enter_isr(); DEBUG("TRX24_XAH_AMI\n"); ((at86rf2xx_t *)at86rfmega_dev)->irq_status |= AT86RF2XX_IRQ_STATUS_MASK__AMI; - __exit_isr(); + atmega_exit_isr(); } /** @@ -771,7 +771,7 @@ ISR(TRX24_XAH_AMI_vect, ISR_BLOCK) */ ISR(TRX24_TX_END_vect, ISR_BLOCK) { - __enter_isr(); + atmega_enter_isr(); at86rf2xx_t *dev = (at86rf2xx_t *) at86rfmega_dev; uint8_t status = *AT86RF2XX_REG__TRX_STATE & AT86RF2XX_TRX_STATUS_MASK__TRX_STATUS; @@ -786,7 +786,7 @@ ISR(TRX24_TX_END_vect, ISR_BLOCK) at86rfmega_dev->event_callback(at86rfmega_dev, NETDEV_EVENT_ISR); } - __exit_isr(); + atmega_exit_isr(); } #endif /* MODULE_AT86RFA1 || MODULE_AT86RFR2 */