[WIP] Code review picked up some bugs/issues.

7 files changed, 131 insertions, 74 deletions

diff --git a/libmaple/bkp.c b/libmaple/bkp.c
index 5b0ad4a..e89abd0 100644
--- a/libmaple/bkp.c
+++ b/libmaple/bkp.c
@@ -73,7 +73,7 @@ uint16 bkp_read(uint8 reg) {
     if (addr != 0) {
         return *addr;
     }
-    ASSERT(0);
+    ASSERT(0);                  /* nonexistent register */
     return 0;
 }
 
@@ -82,4 +82,5 @@ void bkp_write(uint8 reg, uint16 val) {
     if (addr != 0) {
         *addr = val;
     }
+    ASSERT(0);                  /* nonexistent register */
 }
diff --git a/libmaple/exti.c b/libmaple/exti.c
index 150dd05..e806df9 100644
--- a/libmaple/exti.c
+++ b/libmaple/exti.c
@@ -35,7 +35,7 @@ typedef struct ExtIChannel {
     uint32 irq_line;
 } ExtIChannel;
 
-static ExtIChannel exti_channels[] = {
+volatile static ExtIChannel exti_channels[] = {
     { .handler = NULL, .irq_line = NVIC_EXTI0     },  // EXTI0
     { .handler = NULL, .irq_line = NVIC_EXTI1     },  // EXTI1
     { .handler = NULL, .irq_line = NVIC_EXTI2     },  // EXTI2
diff --git a/libmaple/nvic.c b/libmaple/nvic.c
index ad816ba..155da27 100644
--- a/libmaple/nvic.c
+++ b/libmaple/nvic.c
@@ -39,9 +39,8 @@ void nvic_set_vector_table(uint32 addr, uint32 offset) {
  * @param n interrupt number
  */
 void nvic_irq_enable(uint32 n) {
-    /* TODO: bit-banding would be faster */
-    uint32 *iser = &((uint32*)NVIC_ISER0)[(n/32)];
-    __write(iser, BIT(n % 32));
+    /* TODO: test */
+    __write(BITBAND_PERI(NVIC_ISER0, n), 1);
 }
 
 /**
@@ -49,20 +48,20 @@ void nvic_irq_enable(uint32 n) {
  * @param n interrupt number
  */
 void nvic_irq_disable(uint32 n) {
-    /* TODO: bit-banding would be faster */
-    uint32 *icer = &((uint32*)NVIC_ICER0)[(n/32)];
-    __write(icer, BIT(n % 32));
+    /* TODO: test */
+    __write(BITBAND_PERI(NVIC_ICER0, n), 1);
 }
 
 void nvic_irq_disable_all(void) {
-    /* TODO why not:
-       __write(NVIC_ICER0, 0);
-       __write(NVIC_ICER1, 0);
-       */
-    short n;
-    for(n=0; n<65; n++) {
-        nvic_irq_disable(n);
-    }
+    /* Each ICER register contains 1 bit per interrupt.  Writing a 1
+       to that bit disables the corresponding interrupt.  So each of
+       the following lines disables up to 32 interrupts at a time.
+       Since low, medium, and high-density devices all have less than
+       64 interrupts, this suffices. */
+    /* TODO: fix for connectivity line: __write(NVIC_ICER2,1),
+       requires connectivity line support in libmaple.h */
+    __write(NVIC_ICER0, 1);
+    __write(NVIC_ICER1, 1);
 }
 
 /**
diff --git a/libmaple/nvic.h b/libmaple/nvic.h
index c037a38..2a54b27 100644
--- a/libmaple/nvic.h
+++ b/libmaple/nvic.h
@@ -50,25 +50,21 @@ extern "C"{
 /* System control registers  */
 #define SCB_VTOR            0xE000ED08  // Vector table offset register
 
-/* PENDING: aren't these obsolete? they're not used anywhere. */
-#define NVIC_VectTab_RAM             ((u32)0x20000000)
-#define NVIC_VectTab_FLASH           ((u32)0x08000000)
-
 enum {
     NVIC_TIMER1       = 27,
     NVIC_TIMER2       = 28,
     NVIC_TIMER3       = 29,
     NVIC_TIMER4       = 30,
-    NVIC_TIMER5       = 50,   // high density only (Maple Native)
-    NVIC_TIMER6       = 54,   // high density only (Maple Native)
-    NVIC_TIMER7       = 55,   // high density only (Maple Native)
-    NVIC_TIMER8       = 46,   // high density only (Maple Native)
+    NVIC_TIMER5       = 50,   // high density only (Maple Native, Maple Audio)
+    NVIC_TIMER6       = 54,   // high density only
+    NVIC_TIMER7       = 55,   // high density only
+    NVIC_TIMER8       = 46,   // high density only
 
     NVIC_USART1       = 37,
     NVIC_USART2       = 38,
     NVIC_USART3       = 39,
-    NVIC_USART4       = 52,   // high density only (Maple Native)
-    NVIC_USART5       = 53,   // high density only (Maple Native)
+    NVIC_USART4       = 52,   // high density only
+    NVIC_USART5       = 53,   // high density only
 
     NVIC_EXTI0        = 6,
     NVIC_EXTI1        = 7,
diff --git a/libmaple/ring_buffer.h b/libmaple/ring_buffer.h
index 43e0b28..a44088e 100644
--- a/libmaple/ring_buffer.h
+++ b/libmaple/ring_buffer.h
@@ -1,5 +1,9 @@
 /**
- *  @brief simple circular buffer
+ * @file ring_buffer.h
+ * @brief Simple circular buffer
+ *
+ * This implementation is not thread-safe.  In particular, none of
+ * these functions are guaranteed re-entrant.
  */
 
 #ifndef _RING_BUFFER_H_
@@ -10,9 +14,11 @@ extern "C"{
 #endif
 
 /* The buffer is empty when head == tail.
- * The buffer is full when the head is one byte in front of the tail
- * The total buffer size must be a power of two
- * One byte is left free to distinguish empty from full */
+ *
+ * The buffer is full when the head is one byte in front of the tail,
+ * modulo buffer length.
+ *
+ * One byte is left free to distinguish empty from full. */
 typedef struct ring_buffer {
     /** Buffer items are stored into */
     volatile uint8 *buf;
@@ -24,40 +30,84 @@ typedef struct ring_buffer {
     uint16 size;
 } ring_buffer;
 
-/** Initialise a ring buffer.
+/**
+ * Initialise a ring buffer.
+ *
+ *  @param rb   Instance to initialise
  *
- *  @param rb    instance to initialise
- *  @param size  number of items in the buffer
- *  @param buf   buffer to store items into
-*/
-static inline void rb_init(ring_buffer *rb, uint16 size, uint8 *buf) {
+ *  @param size Number of items in buf.  The ring buffer will always
+ *              leave one element unoccupied, so the maximum number of
+ *              elements it can store will be size - 1.  Thus, size
+ *              must be at least 2.
+ *
+ *  @param buf  Buffer to store items into
+ */
+__attribute__((unused))
+static void rb_init(ring_buffer *rb, uint16 size, uint8 *buf) {
     rb->head = 0;
     rb->tail = 0;
     rb->size = size - 1;
     rb->buf = buf;
 }
 
-/** Append an item onto the end of the ring buffer */
+/** Return the number of elements stored in the ring buffer. */
+static inline uint16 rb_full_count(ring_buffer *rb) {
+    volatile ring_buffer *arb = rb;
+    int32 size = arb->tail - arb->head;
+    if (arb->tail < arb->head) {
+        size += arb->size + 1;
+    }
+    return (uint16)size;
+}
+
+/** Return true if and only if the ring buffer is full. */
+static inline int rb_is_full(ring_buffer *rb) {
+    return (rb->tail + 1 == rb->head) ||
+        (rb->tail == rb->size && rb->head == 0);
+}
+
+/** Append element onto the end of the ring buffer. */
 static inline void rb_insert(ring_buffer *rb, uint8 element) {
     rb->buf[rb->tail] = element;
     rb->tail = (rb->tail == rb->size) ? 0 : rb->tail + 1;
 }
 
-/** Remove and return the first item from the ring buffer */
+/** Remove and return the first item from the ring buffer. */
 static inline uint8 rb_remove(ring_buffer *rb) {
     uint8 ch = rb->buf[rb->head];
     rb->head = (rb->head == rb->size) ? 0 : rb->head + 1;
     return ch;
 }
 
-static inline uint32 rb_full_count(ring_buffer *rb) {
-    /* PENDING: Broken */
-    volatile ring_buffer *arb = rb;
-    return arb->tail - arb->head;
+/**
+ * If rb is not full, appends element and returns true; otherwise,
+ * does nothing and returns false. */
+static inline int rb_safe_insert(ring_buffer *rb, uint8 element) {
+    if (rb_is_full(rb)) {
+        return 0;
+    }
+    rb_insert(rb, element);
+    return 1;
+}
+
+/**
+ * Append an item onto the end of a non-full ring buffer.  If the
+ * buffer is full, removes its first item, then inserts the new
+ * element at the end.
+ *
+ * On success, returns -1.  If an element was popped, returns the
+ * popped value. */
+static inline int rb_push_insert(ring_buffer *rb, uint8 element) {
+    int ret = -1;
+    if (rb_is_full(rb)) {
+        ret = rb_remove(rb);
+    }
+    rb_insert(rb, element);
+    return ret;
 }
 
 /** Discard all items from the buffer */
-static inline void  rb_reset(ring_buffer *rb) {
+static inline void rb_reset(ring_buffer *rb) {
     rb->tail = rb->head;
 }
 
diff --git a/libmaple/usart.c b/libmaple/usart.c
index 34095f8..dca7ea5 100644
--- a/libmaple/usart.c
+++ b/libmaple/usart.c
@@ -36,8 +36,8 @@
 #define USART1_BASE    0x40013800
 #define USART2_BASE    0x40004400
 #define USART3_BASE    0x40004800
-#define UART4_BASE     0x40004C00  // High-density devices only (Maple Native)
-#define UART5_BASE     0x40005000  // High-density devices only (Maple Native)
+#define USART4_BASE    0x40004C00  // High-density devices only (Maple Native)
+#define USART5_BASE    0x40005000  // High-density devices only (Maple Native)
 
 #define USART_UE       BIT(13)
 #define USART_M        BIT(12)
@@ -63,45 +63,56 @@ struct usart_dev usart_dev_table[] = {
         .rcc_dev_num = RCC_USART3,
         .nvic_dev_num = NVIC_USART3
     },
-    /*
-      #if NR_USART >= 5
-      [UART4] = {
-      .base = (usart_port*)UART4_BASE,
-      .rcc_dev_num = RCC_UART4,
-      .nvic_dev_num = NVIC_UART4
-      },
-      [UART5] = {
-      .base = (usart_port*)UART5_BASE,
-      .rcc_dev_num = RCC_UART5,
-      .nvic_dev_num = NVIC_UART5
-      },
-      #endif
-    */
+#if NR_USART >= 5
+    /* TODO test */
+    [USART4] = {
+        .base = (usart_port*)USART4_BASE,
+        .rcc_dev_num = RCC_USART4,
+        .nvic_dev_num = NVIC_USART4
+    },
+    [USART5] = {
+        .base = (usart_port*)USART5_BASE,
+        .rcc_dev_num = RCC_USART5,
+        .nvic_dev_num = NVIC_USART5
+    },
+#endif
 };
 
-/* usart interrupt handlers  */
+/* usart interrupt handlers.
+
+   Use of rb_safe_insert() implies that a full buffer ignores new
+   bytes. */
+__attribute__((always_inline)) static inline void usart_irq(int usart_num) {
+    /* TODO: use attributes to let GCC know it's always called with
+       constants, just to force the inliner to do its thing */
+#ifdef USART_SAFE_INSERT
+    rb_safe_insert(&(usart_dev_table[usart_num].rb),
+                   (uint8)((usart_dev_table[usart_num].base)->DR));
+#else
+    rb_push_insert(&(usart_dev_table[usart_num].rb),
+                   (uint8)((usart_dev_table[usart_num].base)->DR));
+#endif
+}
+
 void USART1_IRQHandler(void) {
-    rb_insert(&(usart_dev_table[USART1].rb),
-              (uint8)(((usart_port*)(USART1_BASE))->DR));
+    usart_irq(USART1);
 }
 
 void USART2_IRQHandler(void) {
-    rb_insert(&(usart_dev_table[USART2].rb),
-              (uint8)(((usart_port*)(USART2_BASE))->DR));
+    usart_irq(USART2);
 }
 
 void USART3_IRQHandler(void) {
-    rb_insert(&usart_dev_table[USART3].rb,
-              (uint8)(((usart_port*)(USART3_BASE))->DR));
+    usart_irq(USART3);
 }
+
 #if NR_USART >= 5
-void UART4_IRQHandler(void) {
-    rb_insert(&usart_dev_table[UART4].rb,
-              (uint8)(((usart_port*)(UART4_BASE))->DR));
+void USART4_IRQHandler(void) {
+    usart_irq(USART4);
 }
-void UART5_IRQHandler(void) {
-    rb_insert(&usart_dev_table[UART5].rb,
-              (uint8)(((usart_port*)(UART5_BASE))->DR));
+
+void USART5_IRQHandler(void) {
+    usart_irq(USART5);
 }
 #endif
 
diff --git a/libmaple/usart.h b/libmaple/usart.h
index cbc7bde..2735ac6 100644
--- a/libmaple/usart.h
+++ b/libmaple/usart.h
@@ -42,8 +42,8 @@ enum {
     USART1,
     USART2,
     USART3,
-    UART4,
-    UART5,
+    USART4,
+    USART5,
 };
 
 /* peripheral register struct  */