altos: Do not release interrupts from any pollchar function
authorKeith Packard <keithp@keithp.com>
Sun, 24 Mar 2013 22:00:20 +0000 (15:00 -0700)
committerKeith Packard <keithp@keithp.com>
Sun, 31 Mar 2013 19:24:24 +0000 (12:24 -0700)
getchar relies on interrupts being blocked across the pollchar calls
and into the sleep call or it may go to sleep with data pending.

This prefixes all pollchar functions with _ to indicate that they are
to be called with interrupts blocked and eliminates all interrupt
manipulation calls from within the pollchar functions.

Signed-off-by: Keith Packard <keithp@keithp.com>
14 files changed:
src/avr/ao_serial_avr.c
src/avr/ao_usb_avr.c
src/cc1111/ao_serial.c
src/cc1111/ao_usb.c
src/core/ao.h
src/core/ao_packet.h
src/core/ao_serial.h
src/core/ao_stdio.c
src/drivers/ao_btm.c
src/drivers/ao_packet.c
src/drivers/ao_packet_master.c
src/drivers/ao_packet_slave.c
src/stm/ao_serial_stm.c
src/stm/ao_usb_stm.c

index dcee246..e0f813d 100644 (file)
@@ -59,52 +59,51 @@ ISR(USART1_UDRE_vect)
        ao_wakeup(&ao_serial1_tx_fifo);
 }
 
-char
-ao_serial1_getchar(void) __critical
-{
-       char    c;
-       cli();
-       while (ao_fifo_empty(ao_serial1_rx_fifo))
-               ao_sleep(&ao_serial1_rx_fifo);
-       ao_fifo_remove(ao_serial1_rx_fifo, c);
-       sei();
-       return c;
-}
-
 #if USE_SERIAL_1_STDIN
-char
-ao_serial1_pollchar(void) __critical
+int
+_ao_serial1_pollchar(void)
 {
        char    c;
-       cli();
        if (ao_fifo_empty(ao_serial1_rx_fifo)) {
                sei();
                return AO_READ_AGAIN;
        }
        ao_fifo_remove(ao_serial1_rx_fifo,c);
-       sei();
        return c;
 }
 #endif
 
+char
+ao_serial1_getchar(void) __critical
+{
+       char    c;
+
+       ao_arch_block_interrupts();
+       while (ao_fifo_empty(ao_serial1_rx_fifo))
+               ao_sleep(&ao_serial1_rx_fifo);
+       ao_fifo_remove(ao_serial1_rx_fifo, c);
+       ao_arch_release_interrupts();
+       return c;
+}
+
 void
-ao_serial1_putchar(char c) __critical
+ao_serial1_putchar(char c)
 {
-       cli();
+       ao_arch_block_interrupts();
        while (ao_fifo_full(ao_serial1_tx_fifo))
                ao_sleep(&ao_serial1_tx_fifo);
        ao_fifo_insert(ao_serial1_tx_fifo, c);
        ao_serial_tx1_start();
-       sei();
+       ao_arch_release_interrupts();
 }
 
 void
 ao_serial1_drain(void) __critical
 {
-       cli();
+       ao_arch_block_interrupts();
        while (!ao_fifo_empty(ao_serial1_tx_fifo))
                ao_sleep(&ao_serial1_tx_fifo);
-       sei();
+       ao_arch_release_interrupts();
 }
 
 static const struct {
@@ -155,7 +154,7 @@ ao_serial_init(void)
                  (1 << RXCIE1) |       /* Enable receive interrupts */
                  (1 << UDRIE1));       /* Enable transmit empty interrupts */
 #if USE_SERIAL_1_STDIN
-       ao_add_stdio(ao_serial1_pollchar,
+       ao_add_stdio(_ao_serial1_pollchar,
                     ao_serial1_putchar,
                     NULL);
 #endif
index 2ef546c..bd75b17 100644 (file)
@@ -411,7 +411,7 @@ ao_usb_ep0(void)
 
 /* Wait for a free IN buffer */
 static void
-ao_usb_in_wait(void)
+_ao_usb_in_wait(void)
 {
        for (;;) {
                /* Check if the current buffer is writable */
@@ -419,7 +419,6 @@ ao_usb_in_wait(void)
                if (UEINTX & (1 << RWAL))
                        break;
 
-               cli();
                /* Wait for an IN buffer to be ready */
                for (;;) {
                        UENUM = AO_USB_IN_EP;
@@ -430,24 +429,24 @@ ao_usb_in_wait(void)
                }
                /* Ack the interrupt */
                UEINTX &= ~(1 << TXINI);
-               sei();
        }
 }
 
 /* Queue the current IN buffer for transmission */
 static void
-ao_usb_in_send(void)
+_ao_usb_in_send(void)
 {
        UENUM = AO_USB_IN_EP;
        UEINTX &= ~(1 << FIFOCON);
 }
 
 void
-ao_usb_flush(void) __critical
+ao_usb_flush(void)
 {
        if (!ao_usb_running)
                return;
 
+       ao_arch_block_interrupts();
        /* Anytime we've sent a character since
         * the last time we flushed, we'll need
         * to send a packet -- the only other time
@@ -457,18 +456,20 @@ ao_usb_flush(void) __critical
         */
        if (!ao_usb_in_flushed) {
                ao_usb_in_flushed = 1;
-               ao_usb_in_wait();
-               ao_usb_in_send();
+               _ao_usb_in_wait();
+               _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();
 
        /* Queue a byte */
        UENUM = AO_USB_IN_EP;
@@ -476,11 +477,12 @@ ao_usb_putchar(char c) __critical __reentrant
 
        /* Send the packet when full */
        if ((UEINTX & (1 << RWAL)) == 0)
-               ao_usb_in_send();
+               _ao_usb_in_send();
        ao_usb_in_flushed = 0;
+       ao_arch_release_interrupts();
 }
 
-static int
+int
 _ao_usb_pollchar(void)
 {
        uint8_t c;
@@ -517,25 +519,15 @@ _ao_usb_pollchar(void)
        return c;
 }
 
-int
-ao_usb_pollchar(void)
-{
-       int     c;
-       cli();
-       c = _ao_usb_pollchar();
-       sei();
-       return c;
-}
-
 char
-ao_usb_getchar(void) __critical
+ao_usb_getchar(void)
 {
        int     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;
 }
 
@@ -668,5 +660,5 @@ ao_usb_init(void)
 #if USB_DEBUG
        ao_add_task(&ao_usb_echo_task, ao_usb_echo, "usb echo");
 #endif
-       ao_add_stdio(ao_usb_pollchar, ao_usb_putchar, ao_usb_flush);
+       ao_add_stdio(_ao_usb_pollchar, ao_usb_putchar, ao_usb_flush);
 }
index 8913a9b..8172783 100644 (file)
@@ -92,7 +92,7 @@ ao_serial0_getchar(void) __critical
 
 #if USE_SERIAL_0_STDIN
 int
-ao_serial0_pollchar(void) __critical
+_ao_serial0_pollchar(void)
 {
        uint8_t c;
        if (ao_fifo_empty(ao_serial0_rx_fifo))
@@ -180,7 +180,7 @@ ao_serial1_getchar(void) __critical
 
 #if USE_SERIAL_1_STDIN
 int
-ao_serial1_pollchar(void) __critical
+_ao_serial1_pollchar(void)
 {
        uint8_t c;
        if (ao_fifo_empty(ao_serial1_rx_fifo))
@@ -271,7 +271,7 @@ ao_serial_init(void)
        IEN0 |= IEN0_URX0IE;
        IEN2 |= IEN2_UTX0IE;
 #if USE_SERIAL_0_STDIN && !DELAY_SERIAL_0_STDIN
-       ao_add_stdio(ao_serial0_pollchar,
+       ao_add_stdio(_ao_serial0_pollchar,
                     ao_serial0_putchar,
                     NULL);
 #endif
@@ -327,7 +327,7 @@ ao_serial_init(void)
        IEN2 |= IEN2_UTX1IE;
 
 #if USE_SERIAL_1_STDIN && !DELAY_SERIAL_1_STDIN
-       ao_add_stdio(ao_serial1_pollchar,
+       ao_add_stdio(_ao_serial1_pollchar,
                     ao_serial1_putchar,
                     NULL);
 #endif
index f66e807..8bd2efd 100644 (file)
@@ -383,18 +383,18 @@ ao_usb_putchar(char c) __critical __reentrant
 }
 
 int
-ao_usb_pollchar(void) __critical
+_ao_usb_pollchar(void)
 {
        uint8_t c;
        if (ao_usb_out_bytes == 0) {
                USBINDEX = AO_USB_OUT_EP;
                if ((USBCSOL & USBCSOL_OUTPKT_RDY) == 0)
-                       return -1;
+                       return AO_READ_AGAIN;
                ao_usb_out_bytes = (USBCNTH << 8) | USBCNTL;
                if (ao_usb_out_bytes == 0) {
                        USBINDEX = AO_USB_OUT_EP;
                        USBCSOL &= ~USBCSOL_OUTPKT_RDY;
-                       return -1;
+                       return AO_READ_AGAIN;
                }
        }
        --ao_usb_out_bytes;
@@ -407,12 +407,14 @@ ao_usb_pollchar(void) __critical
 }
 
 char
-ao_usb_getchar(void) __critical
+ao_usb_getchar(void)
 {
        int     c;
 
-       while ((c = ao_usb_pollchar()) == AO_READ_AGAIN)
+       ao_arch_block_interrupts();
+       while ((c = _ao_usb_pollchar()) == AO_READ_AGAIN)
                ao_sleep(&ao_stdin_ready);
+       ao_arch_release_interrupts();
        return c;
 }
 
@@ -459,5 +461,5 @@ ao_usb_init(void)
        ao_usb_enable();
 
        ao_add_task(&ao_usb_task, ao_usb_ep0, "usb");
-       ao_add_stdio(ao_usb_pollchar, ao_usb_putchar, ao_usb_flush);
+       ao_add_stdio(_ao_usb_pollchar, ao_usb_putchar, ao_usb_flush);
 }
index e3161b4..6c790f6 100644 (file)
@@ -638,7 +638,7 @@ ao_monitor_init(void) __reentrant;
 #define AO_READ_AGAIN  (-1)
 
 struct ao_stdio {
-       int     (*pollchar)(void);
+       int     (*_pollchar)(void);     /* Called with interrupts blocked */
        void    (*putchar)(char c) __reentrant;
        void    (*flush)(void);
        uint8_t echo;
index 08b184d..6d121bb 100644 (file)
@@ -63,7 +63,7 @@ void
 ao_packet_putchar(char c) __reentrant;
 
 int
-ao_packet_pollchar(void);
+_ao_packet_pollchar(void);
 
 #if PACKET_HAS_MASTER
 /* ao_packet_master.c */
index a799bf2..baf213c 100644 (file)
@@ -32,7 +32,7 @@ char
 ao_serial0_getchar(void);
 
 int
-ao_serial0_pollchar(void);
+_ao_serial0_pollchar(void);
 
 void
 ao_serial0_putchar(char c);
@@ -52,7 +52,7 @@ char
 ao_serial1_getchar(void);
 
 int
-ao_serial1_pollchar(void);
+_ao_serial1_pollchar(void);
 
 void
 ao_serial1_putchar(char c);
@@ -72,7 +72,7 @@ char
 ao_serial2_getchar(void);
 
 int
-ao_serial2_pollchar(void);
+_ao_serial2_pollchar(void);
 
 void
 ao_serial2_putchar(char c);
@@ -92,7 +92,7 @@ char
 ao_serial3_getchar(void);
 
 int
-ao_serial3_pollchar(void);
+_ao_serial3_pollchar(void);
 
 void
 ao_serial3_putchar(char c);
index 1748dfe..977d74b 100644 (file)
@@ -99,20 +99,21 @@ char
 getchar(void) __reentrant
 {
        int c;
-       ao_arch_critical(
-               int8_t stdio = ao_cur_stdio;
+       int8_t 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;
-               );
+       ao_arch_block_interrupts();
+       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;
+       ao_arch_release_interrupts();
        return c;
 }
 
@@ -123,13 +124,13 @@ ao_echo(void)
 }
 
 int8_t
-ao_add_stdio(int (*pollchar)(void),
+ao_add_stdio(int (*_pollchar)(void),
             void (*putchar)(char),
             void (*flush)(void)) __reentrant
 {
        if (ao_num_stdios == AO_NUM_STDIOS)
                ao_panic(AO_PANIC_STDIO);
-       ao_stdios[ao_num_stdios].pollchar = pollchar;
+       ao_stdios[ao_num_stdios]._pollchar = _pollchar;
        ao_stdios[ao_num_stdios].putchar = putchar;
        ao_stdios[ao_num_stdios].flush = flush;
        ao_stdios[ao_num_stdios].echo = 1;
index c862200..de1f31a 100644 (file)
 
 #ifndef ao_serial_btm_getchar
 #define ao_serial_btm_putchar  ao_serial1_putchar
-#define ao_serial_btm_pollchar ao_serial1_pollchar
+#define _ao_serial_btm_pollchar        _ao_serial1_pollchar
 #define ao_serial_btm_set_speed ao_serial1_set_speed
 #define ao_serial_btm_drain    ao_serial1_drain
+#define ao_serial_btm_rx_fifo  ao_serial1_rx_fifo
 #endif
 
 int8_t                 ao_btm_stdio;
@@ -111,6 +112,30 @@ __code struct ao_cmds ao_btm_cmds[] = {
 #define AO_BTM_MAX_REPLY       16
 __xdata char           ao_btm_reply[AO_BTM_MAX_REPLY];
 
+/*
+ * Read one bluetooth character.
+ * Returns AO_READ_AGAIN if no character arrives within 10ms
+ */
+
+static int
+ao_btm_getchar(void)
+{
+       int     c;
+
+       ao_arch_block_interrupts();
+       while ((c = _ao_serial_btm_pollchar()) == AO_READ_AGAIN) {
+               ao_alarm(AO_MS_TO_TICKS(10));
+               c = ao_sleep(&ao_serial_btm_rx_fifo);
+               ao_clear_alarm();
+               if (c) {
+                       c = AO_READ_AGAIN;
+                       break;
+               }
+       }
+       ao_arch_release_interrupts();
+       return c;
+}
+
 /*
  * Read a line of data from the serial port, truncating
  * it after a few characters.
@@ -122,24 +147,13 @@ ao_btm_get_line(void)
        uint8_t ao_btm_reply_len = 0;
        int c;
 
-       for (;;) {
-
-               while ((c = ao_serial_btm_pollchar()) != AO_READ_AGAIN) {
-                       ao_btm_log_in_char(c);
-                       if (ao_btm_reply_len < sizeof (ao_btm_reply))
-                               ao_btm_reply[ao_btm_reply_len++] = c;
-                       if (c == '\r' || c == '\n')
-                               goto done;
-               }
-               for (c = 0; c < 10; c++) {
-                       ao_delay(AO_MS_TO_TICKS(10));
-                       if (!ao_fifo_empty(ao_serial1_rx_fifo))
-                               break;
-               }
-               if (c == 10)
-                       goto done;
+       while ((c = ao_btm_getchar()) != AO_READ_AGAIN) {
+               ao_btm_log_in_char(c);
+               if (ao_btm_reply_len < sizeof (ao_btm_reply))
+                       ao_btm_reply[ao_btm_reply_len++] = c;
+               if (c == '\r' || c == '\n')
+                       break;
        }
-done:
        for (c = ao_btm_reply_len; c < sizeof (ao_btm_reply);)
                ao_btm_reply[c++] = '\0';
        return ao_btm_reply_len;
@@ -279,7 +293,7 @@ ao_btm(void)
        /* Turn off status reporting */
        ao_btm_cmd("ATQ1\r");
 
-       ao_btm_stdio = ao_add_stdio(ao_serial_btm_pollchar,
+       ao_btm_stdio = ao_add_stdio(_ao_serial_btm_pollchar,
                                    ao_serial_btm_putchar,
                                    NULL);
        ao_btm_echo(0);
index 9131992..5a50747 100644 (file)
@@ -169,12 +169,10 @@ ao_packet_putchar(char c) __reentrant
                tx_data[ao_packet_tx_used++] = c;
 }
 
+/* May be called with interrupts blocked */
 int
-ao_packet_pollchar(void)
+_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;
 
index 023c788..4c0dc57 100644 (file)
@@ -21,7 +21,12 @@ static char
 ao_packet_getchar(void)
 {
        int c;
-       while ((c = ao_packet_pollchar()) == AO_READ_AGAIN) {
+
+       /* No need to block interrupts in this function as
+        * all packet variables are only modified from task
+        * context, not an interrupt handler
+        */
+       while ((c = _ao_packet_pollchar()) == AO_READ_AGAIN) {
                if (!ao_packet_enable)
                        break;
                if (ao_packet_master_sleeping)
index e45775c..e75df0d 100644 (file)
@@ -59,7 +59,7 @@ ao_packet_slave_stop(void)
 void
 ao_packet_slave_init(uint8_t enable)
 {
-       ao_add_stdio(ao_packet_pollchar,
+       ao_add_stdio(_ao_packet_pollchar,
                     ao_packet_putchar,
                     NULL);
        if (enable)
index ce33f97..2133c58 100644 (file)
@@ -59,24 +59,11 @@ ao_usart_isr(struct ao_stm_usart *usart, int stdin)
        }
 }
 
-char
-ao_usart_getchar(struct ao_stm_usart *usart)
-{
-       char c;
-       ao_arch_block_interrupts();
-       while (ao_fifo_empty(usart->rx_fifo))
-               ao_sleep(&usart->rx_fifo);
-       ao_fifo_remove(usart->rx_fifo, c);
-       ao_arch_release_interrupts();
-       return c;
-}
-
 int
-ao_usart_pollchar(struct ao_stm_usart *usart)
+_ao_usart_pollchar(struct ao_stm_usart *usart)
 {
        int     c;
        
-       ao_arch_block_interrupts();
        if (ao_fifo_empty(usart->rx_fifo))
                c = AO_READ_AGAIN;
        else {
@@ -84,10 +71,20 @@ ao_usart_pollchar(struct ao_stm_usart *usart)
                ao_fifo_remove(usart->rx_fifo,u);
                c = u;
        }
-       ao_arch_release_interrupts();
        return c;
 }
 
+char
+ao_usart_getchar(struct ao_stm_usart *usart)
+{
+       int c;
+       ao_arch_block_interrupts();
+       while ((c = _ao_usart_pollchar(usart)) == AO_READ_AGAIN)
+               ao_sleep(&usart->rx_fifo);
+       ao_arch_release_interrupts();
+       return (char) c;
+}
+
 void
 ao_usart_putchar(struct ao_stm_usart *usart, char c)
 {
@@ -201,9 +198,9 @@ ao_serial1_putchar(char c)
 }
 
 int
-ao_serial1_pollchar(void)
+_ao_serial1_pollchar(void)
 {
-       return ao_usart_pollchar(&ao_stm_usart1);
+       return _ao_usart_pollchar(&ao_stm_usart1);
 }
 
 void
@@ -232,9 +229,9 @@ ao_serial2_putchar(char c)
 }
 
 int
-ao_serial2_pollchar(void)
+_ao_serial2_pollchar(void)
 {
-       return ao_usart_pollchar(&ao_stm_usart2);
+       return _ao_usart_pollchar(&ao_stm_usart2);
 }
 
 void
@@ -263,9 +260,9 @@ ao_serial3_putchar(char c)
 }
 
 int
-ao_serial3_pollchar(void)
+_ao_serial3_pollchar(void)
 {
-       return ao_usart_pollchar(&ao_stm_usart3);
+       return _ao_usart_pollchar(&ao_stm_usart3);
 }
 
 void
@@ -309,7 +306,7 @@ ao_serial_init(void)
        stm_nvic_set_enable(STM_ISR_USART1_POS);
        stm_nvic_set_priority(STM_ISR_USART1_POS, 4);
 #if USE_SERIAL_1_STDIN
-       ao_add_stdio(ao_serial1_pollchar,
+       ao_add_stdio(_ao_serial1_pollchar,
                     ao_serial1_putchar,
                     NULL);
 #endif
@@ -346,7 +343,7 @@ ao_serial_init(void)
        stm_nvic_set_enable(STM_ISR_USART2_POS);
        stm_nvic_set_priority(STM_ISR_USART2_POS, 4);
 #if USE_SERIAL_2_STDIN
-       ao_add_stdio(ao_serial2_pollchar,
+       ao_add_stdio(_ao_serial2_pollchar,
                     ao_serial2_putchar,
                     NULL);
 #endif
@@ -390,7 +387,7 @@ ao_serial_init(void)
        stm_nvic_set_enable(STM_ISR_USART3_POS);
        stm_nvic_set_priority(STM_ISR_USART3_POS, 4);
 #if USE_SERIAL_3_STDIN
-       ao_add_stdio(ao_serial3_pollchar,
+       ao_add_stdio(_ao_serial3_pollchar,
                     ao_serial3_putchar,
                     NULL);
 #endif
index 9379e5c..dfa58c4 100644 (file)
@@ -229,10 +229,9 @@ ao_usb_set_stat_tx(int ep, uint32_t stat_tx)
 }
 
 static void
-ao_usb_set_stat_rx(int ep, uint32_t stat_rx) {
+_ao_usb_set_stat_rx(int ep, uint32_t stat_rx) {
        uint32_t        epr_write, epr_old;
 
-       ao_arch_block_interrupts();
        epr_write = epr_old = stm_usb.epr[ep];
        epr_write &= STM_USB_EPR_PRESERVE_MASK;
        epr_write |= STM_USB_EPR_INVARIANT;
@@ -240,6 +239,12 @@ ao_usb_set_stat_rx(int ep, uint32_t stat_rx) {
                              STM_USB_EPR_STAT_RX_MASK << STM_USB_EPR_STAT_RX,
                              stat_rx << STM_USB_EPR_STAT_RX);
        stm_usb.epr[ep] = epr_write;
+}
+
+static void
+ao_usb_set_stat_rx(int ep, uint32_t stat_rx) {
+       ao_arch_block_interrupts();
+       _ao_usb_set_stat_rx(ep, stat_rx);
        ao_arch_release_interrupts();
 }
 
@@ -870,10 +875,10 @@ _ao_usb_out_recv(void)
        ao_usb_rx_pos = 0;
 
        /* ACK the packet */
-       ao_usb_set_stat_rx(AO_USB_OUT_EPR, STM_USB_EPR_STAT_RX_VALID);
+       _ao_usb_set_stat_rx(AO_USB_OUT_EPR, STM_USB_EPR_STAT_RX_VALID);
 }
 
-static int
+int
 _ao_usb_pollchar(void)
 {
        uint8_t c;
@@ -896,16 +901,6 @@ _ao_usb_pollchar(void)
        return c;
 }
 
-int
-ao_usb_pollchar(void)
-{
-       int     c;
-       ao_arch_block_interrupts();
-       c = _ao_usb_pollchar();
-       ao_arch_release_interrupts();
-       return c;
-}
-
 char
 ao_usb_getchar(void)
 {