altos: Replace __critical usage with ao_arch_critical as needed

sdcc offers __critical as a machine-independent way to block
interrupts, but as gcc doesn't, we need to use a compiler-independent
construct instead. ao_arch_critical has been around since the AVR
port, but some old __critical usages remained.

This fixes a bunch of random hangs when communicating with MM over USB
or the radio as the various stdio loops were running without
interrupts blocked between the test and the sleep.

Signed-off-by: Keith Packard <keithp@keithp.com>

diff --git a/src/core/ao.h b/src/core/ao.h
index e559e876b0209c62eece17b66785ca6e153606de..2b375cfd68f7115d38ad7b6220adbb9e9f87c44d 100644 (file)
--- a/src/core/ao.h
+++ b/src/core/ao.h
@@ -101,7 +101,7 @@ ao_delay(uint16_t ticks);
 
 /* Set the ADC interval */
 void
-ao_timer_set_adc_interval(uint8_t interval) __critical;
+ao_timer_set_adc_interval(uint8_t interval);
 
 /* Timer interrupt */
 void

diff --git a/src/core/ao_ignite.c b/src/core/ao_ignite.c
index c7829fc3b543334f19c99db52985dae0f7e39b8c..693b7c7aaeb17e3dc384852bc46204ed546fa156 100644 (file)
--- a/src/core/ao_ignite.c
+++ b/src/core/ao_ignite.c
@@ -21,10 +21,12 @@
 __xdata struct ao_ignition ao_ignition[2];
 
 void
-ao_ignite(enum ao_igniter igniter) __critical
+ao_ignite(enum ao_igniter igniter)
 {
+       cli();
        ao_ignition[igniter].request = 1;
        ao_wakeup(&ao_ignition);
+       sei();
 }
 
 #ifndef AO_SENSE_DROGUE
@@ -39,12 +41,12 @@ ao_igniter_status(enum ao_igniter igniter)
        __pdata int16_t value;
        __pdata uint8_t request, firing, fired;
 
-       __critical {
+       ao_arch_critical(
                ao_data_get(&packet);
                request = ao_ignition[igniter].request;
                fired = ao_ignition[igniter].fired;
                firing = ao_ignition[igniter].firing;
-       }
+               );
        if (firing || (request && !fired))
                return ao_igniter_active;
 
@@ -79,7 +81,7 @@ ao_igniter_status(enum ao_igniter igniter)
 #endif
 
 void
-ao_igniter_fire(enum ao_igniter igniter) __critical
+ao_igniter_fire(enum ao_igniter igniter)
 {
        ao_ignition[igniter].firing = 1;
        switch(ao_config.ignite_mode) {

diff --git a/src/core/ao_mutex.c b/src/core/ao_mutex.c
index c82a7d57e23c37d1e1188fdb8ebd1d69c3c55876..952ff46214a69db9b3e293992ee4f2e602576f46 100644 (file)
--- a/src/core/ao_mutex.c
+++ b/src/core/ao_mutex.c
@@ -22,11 +22,11 @@ ao_mutex_get(__xdata uint8_t *mutex) __reentrant
 {
        if (*mutex == ao_cur_task->task_id)
                ao_panic(AO_PANIC_MUTEX);
-       __critical {
+       ao_arch_critical(
                while (*mutex)
                        ao_sleep(mutex);
                *mutex = ao_cur_task->task_id;
-       }
+               );
 }
 
 void
@@ -34,8 +34,8 @@ ao_mutex_put(__xdata uint8_t *mutex) __reentrant
 {
        if (*mutex != ao_cur_task->task_id)
                ao_panic(AO_PANIC_MUTEX);
-       __critical {
+       ao_arch_critical(
                *mutex = 0;
                ao_wakeup(mutex);
-       }
+               );
 }

diff --git a/src/core/ao_packet.h b/src/core/ao_packet.h
index 9058c347f05876ace2ce7d64cf2bb44f2102bc27..0eafd3b2d5294122847eb7891b85226e2f8b17ef 100644 (file)
--- a/src/core/ao_packet.h
+++ b/src/core/ao_packet.h
@@ -63,7 +63,7 @@ void
 ao_packet_putchar(char c) __reentrant;
 
 char
-ao_packet_pollchar(void) __critical;
+ao_packet_pollchar(void);
 
 #if PACKET_HAS_MASTER
 /* ao_packet_master.c */

diff --git a/src/core/ao_panic.c b/src/core/ao_panic.c
index 3c0b471e922983e99b2356c9b4eafdbe8bb47b6d..c29cd8feb279d605e06ffafa50ff3a53f7864dbd 100644 (file)
--- a/src/core/ao_panic.c
+++ b/src/core/ao_panic.c
@@ -53,7 +53,8 @@ ao_panic(uint8_t reason)
        ao_cur_task = NULL;
        printf ("panic %d\n", reason);
 #endif
-       __critical for (;;) {
+       ao_arch_block_interrupts();
+       for (;;) {
                ao_panic_delay(20);
                for (n = 0; n < 5; n++) {
                        ao_led_on(AO_LED_PANIC);

diff --git a/src/core/ao_stdio.c b/src/core/ao_stdio.c
index 656b23c9e88fadc5f65b560af8487692d2b27c89..8cf66a239ac1db03d8d1d72c5c2bd7d44ab26a0c 100644 (file)
--- a/src/core/ao_stdio.c
+++ b/src/core/ao_stdio.c
@@ -96,21 +96,23 @@ flush(void)
 __xdata uint8_t ao_stdin_ready;
 
 char
-getchar(void) __reentrant __critical
+getchar(void) __reentrant
 {
        char c;
-       int8_t stdio = ao_cur_stdio;
+       ao_arch_critical(
+               int8_t stdio = ao_cur_stdio;
 
-       for (;;) {
-               c = ao_stdios[stdio].pollchar();
-               if (c != AO_READ_AGAIN)
-                       break;
-               if (++stdio == ao_num_stdios)
-                       stdio = 0;
-               if (stdio == ao_cur_stdio)
-                       ao_sleep(&ao_stdin_ready);
-       }
-       ao_cur_stdio = stdio;
+               for (;;) {
+                       c = ao_stdios[stdio].pollchar();
+                       if (c != AO_READ_AGAIN)
+                               break;
+                       if (++stdio == ao_num_stdios)
+                               stdio = 0;
+                       if (stdio == ao_cur_stdio)
+                               ao_sleep(&ao_stdin_ready);
+               }
+               ao_cur_stdio = stdio;
+               );
        return c;
 }
 

diff --git a/src/drivers/ao_btm.c b/src/drivers/ao_btm.c
index f193ac8ee25c0f4ef3731b33af13b6c8ee2caba5..f381604796dd3d390aae7469fce3df611782e49a 100644 (file)
--- a/src/drivers/ao_btm.c
+++ b/src/drivers/ao_btm.c
@@ -312,18 +312,20 @@ __xdata struct ao_task ao_btm_task;
 #endif
 
 void
-ao_btm_check_link() __critical
+ao_btm_check_link()
 {
-       /* Check the pin and configure the interrupt detector to wait for the
-        * pin to flip the other way
-        */
-       if (BT_LINK_PIN) {
-               ao_btm_connected = 0;
-               PICTL |= BT_PICTL_ICON;
-       } else {
-               ao_btm_connected = 1;
-               PICTL &= ~BT_PICTL_ICON;
-       }
+       ao_arch_critical(
+               /* Check the pin and configure the interrupt detector to wait for the
+                * pin to flip the other way
+                */
+               if (BT_LINK_PIN) {
+                       ao_btm_connected = 0;
+                       PICTL |= BT_PICTL_ICON;
+               } else {
+                       ao_btm_connected = 1;
+                       PICTL &= ~BT_PICTL_ICON;
+               }
+               );
 }
 
 void

diff --git a/src/drivers/ao_packet.c b/src/drivers/ao_packet.c
index 2bada949e9c04769d60c0ed8748ca3266f46ec11..3c1e7a18eb643168be3520e558f0ee6781ce963f 100644 (file)
--- a/src/drivers/ao_packet.c
+++ b/src/drivers/ao_packet.c
@@ -155,6 +155,9 @@ ao_packet_flush(void)
 void
 ao_packet_putchar(char c) __reentrant
 {
+       /* No need to block interrupts, all variables here
+        * are only manipulated in task context
+        */
        while (ao_packet_tx_used == AO_PACKET_MAX && ao_packet_enable) {
 #if PACKET_HAS_MASTER
                ao_packet_flush();
@@ -167,8 +170,11 @@ ao_packet_putchar(char c) __reentrant
 }
 
 char
-ao_packet_pollchar(void) __critical
+ao_packet_pollchar(void)
 {
+       /* No need to block interrupts, all variables here
+        * are only manipulated in task context
+        */
        if (!ao_packet_enable)
                return AO_READ_AGAIN;
 

diff --git a/src/drivers/ao_packet_master.c b/src/drivers/ao_packet_master.c
index e97a66488ce11116f9f11694d38b68ca359abc47..481232dff2b821b229c29dc4ff6f8baad63f41db 100644 (file)
--- a/src/drivers/ao_packet_master.c
+++ b/src/drivers/ao_packet_master.c
@@ -18,7 +18,7 @@
 #include "ao.h"
 
 static char
-ao_packet_getchar(void) __critical
+ao_packet_getchar(void)
 {
        char c;
        while ((c = ao_packet_pollchar()) == AO_READ_AGAIN) {

diff --git a/src/stm/ao_arch.h b/src/stm/ao_arch.h
index f2de719c6c3c63346ddebd0037aa1a1fcfcc568a..0c3cfc915f9513813a1153041249746d5e0c1bb9 100644 (file)
--- a/src/stm/ao_arch.h
+++ b/src/stm/ao_arch.h
@@ -43,7 +43,6 @@
 #define __xdata
 #define __code const
 #define __reentrant
-#define __critical
 #define __interrupt(n)
 #define __at(n)
 
@@ -83,8 +82,29 @@ extern const uint32_t ao_radio_cal;
 #define ao_arch_task_members\
        uint32_t *sp;                   /* saved stack pointer */
 
-#define cli()  asm("cpsid i")
-#define sei()  asm("cpsie i")
+#define ao_arch_block_interrupts()     asm("cpsid i")
+#define ao_arch_release_interrupts()   asm("cpsie i")
+
+#define cli()  ao_arch_block_interrupts()
+#define sei()  ao_arch_release_interrupts()
+
+static uint32_t
+ao_arch_irqsave(void) {
+       uint32_t        primask;
+       asm("mrs %0,primask" : "=&r" (primask));
+       ao_arch_block_interrupts();
+       return primask;
+}
+
+static void
+ao_arch_irqrestore(uint32_t primask) {
+       asm("msr primask,%0" : : "r" (primask));
+}
+
+static void
+ao_arch_memory_barrier() {
+       asm volatile("" ::: "memory");
+}
 
 #define ao_arch_init_stack(task, start) do {                           \
                uint32_t        *sp = (uint32_t *) (task->stack + AO_STACK_SIZE); \

diff --git a/src/stm/ao_timer.c b/src/stm/ao_timer.c
index f3011d3f0e159097c71ce8c3e3f49443c98d070b..d82a803e2d819e162151fff60d6b8606454f7219 100644 (file)
--- a/src/stm/ao_timer.c
+++ b/src/stm/ao_timer.c
@@ -56,10 +56,12 @@ void stm_tim6_isr(void)
 
 #if HAS_ADC
 void
-ao_timer_set_adc_interval(uint8_t interval) __critical
+ao_timer_set_adc_interval(uint8_t interval)
 {
-       ao_data_interval = interval;
-       ao_data_count = 0;
+       ao_arch_critical(
+               ao_data_interval = interval;
+               ao_data_count = 0;
+               );
 }
 #endif
 

diff --git a/src/stm/ao_usb_stm.c b/src/stm/ao_usb_stm.c
index 4f37a7d9e7af5256a7ce4b89ecb8a2ec65ca7249..8e7dacc53e2bf98e37d4bb3d0ad48cbc2ef1fa92 100644 (file)
--- a/src/stm/ao_usb_stm.c
+++ b/src/stm/ao_usb_stm.c
@@ -799,25 +799,23 @@ ao_usb_in_send(void)
        ao_usb_tx_count = 0;
 }
 
-/* Wait for a free IN buffer */
+/* Wait for a free IN buffer. Interrupts are blocked */
 static void
-ao_usb_in_wait(void)
+_ao_usb_in_wait(void)
 {
        for (;;) {
                /* Check if the current buffer is writable */
                if (ao_usb_tx_count < AO_USB_IN_SIZE)
                        break;
 
-               cli();
                /* Wait for an IN buffer to be ready */
                while (ao_usb_in_pending)
                        ao_sleep(&ao_usb_in_pending);
-               sei();
        }
 }
 
 void
-ao_usb_flush(void) __critical
+ao_usb_flush(void)
 {
        if (!ao_usb_running)
                return;
@@ -829,24 +827,25 @@ ao_usb_flush(void) __critical
         * packet was full, in which case we now
         * want to send an empty packet
         */
+       ao_arch_block_interrupts();
        if (!ao_usb_in_flushed) {
                ao_usb_in_flushed = 1;
-               cli();
                /* Wait for an IN buffer to be ready */
                while (ao_usb_in_pending)
                        ao_sleep(&ao_usb_in_pending);
-               sei();
                ao_usb_in_send();
        }
+       ao_arch_release_interrupts();
 }
 
 void
-ao_usb_putchar(char c) __critical __reentrant
+ao_usb_putchar(char c)
 {
        if (!ao_usb_running)
                return;
 
-       ao_usb_in_wait();
+       ao_arch_block_interrupts();
+       _ao_usb_in_wait();
 
        ao_usb_in_flushed = 0;
        ao_usb_tx_buffer[ao_usb_tx_count++] = (uint8_t) c;
@@ -854,10 +853,11 @@ ao_usb_putchar(char c) __critical __reentrant
        /* Send the packet when full */
        if (ao_usb_tx_count == AO_USB_IN_SIZE)
                ao_usb_in_send();
+       ao_arch_release_interrupts();
 }
 
 static void
-ao_usb_out_recv(void)
+_ao_usb_out_recv(void)
 {
        ao_usb_out_avail = 0;
 
@@ -888,7 +888,7 @@ _ao_usb_pollchar(void)
                /* Check to see if a packet has arrived */
                if (!ao_usb_out_avail)
                        return AO_READ_AGAIN;
-               ao_usb_out_recv();
+               _ao_usb_out_recv();
        }
 
        /* Pull a character out of the fifo */
@@ -900,27 +900,28 @@ char
 ao_usb_pollchar(void)
 {
        char    c;
-       cli();
+       ao_arch_block_interrupts();
        c = _ao_usb_pollchar();
-       sei();
+       ao_arch_release_interrupts();
        return c;
 }
 
 char
-ao_usb_getchar(void) __critical
+ao_usb_getchar(void)
 {
        char    c;
 
-       cli();
+       ao_arch_block_interrupts();
        while ((c = _ao_usb_pollchar()) == AO_READ_AGAIN)
                ao_sleep(&ao_stdin_ready);
-       sei();
+       ao_arch_release_interrupts();
        return c;
 }
 
 void
 ao_usb_disable(void)
 {
+       ao_arch_block_interrupts();
        stm_usb.cntr = (1 << STM_USB_CNTR_FRES);
        stm_usb.istr = 0;
 
@@ -932,6 +933,7 @@ ao_usb_disable(void)
 
        /* Disable the interface */
        stm_rcc.apb1enr &+ ~(1 << STM_RCC_APB1ENR_USBEN);
+       ao_arch_release_interrupts();
 }
 
 void
@@ -954,6 +956,8 @@ ao_usb_enable(void)
         * pulled low and doesn't work at all
         */
 
+       ao_arch_block_interrupts();
+
        /* Route interrupts */
        stm_nvic_set_priority(STM_ISR_USB_LP_POS, 3);
        stm_nvic_set_enable(STM_ISR_USB_LP_POS);
@@ -985,6 +989,8 @@ ao_usb_enable(void)
                        (0 << STM_USB_CNTR_PDWN) |
                        (0 << STM_USB_CNTR_FRES));
 
+       ao_arch_release_interrupts();
+
        for (t = 0; t < 1000; t++)
                ao_arch_nop();
        /* Enable USB pull-up */