Gigabeat S: PMIC SPI improvement and bugfixes.

Nick some aspects from the as3525 ascodec driver to improve throughput in
the beast's SPI communications by switching tranfer descriptors to the
caller's stack and getting rid of thread synchronization.

Fix a bug that suddenly became obvious that could permanently stall the SPI
driver because all data could be shifted out before the interrupt could get
serviced. In that case, it needs a kick to restart it. Should probably put
the SPI interrupt priority above DVFS.

A tweak to the event registration interface to simplify it.


git-svn-id: svn://svn.rockbox.org/rockbox/trunk@31353 a1c6a512-1295-4272-9138-f99709370657

4 files changed, 176 insertions, 198 deletions

diff --git a/firmware/export/mc13783.h b/firmware/export/mc13783.h
index e513fa1717..99fd004420 100644
--- a/firmware/export/mc13783.h
+++ b/firmware/export/mc13783.h
@@ -1281,17 +1281,59 @@ bool mc13783_write_async(struct spi_transfer_desc *xfer,
 
 #define MC13783_DATA_ERROR UINT32_MAX
 
-/* Statically-registered event enable/disable */
-enum mc13783_event_sets
+/* Statically-registered interrupt enable/disable */
+enum mc13783_int_ids
 {
-    MC13783_EVENT_SET0 = 0, /* *STATUS0/MASK0/SENSE0 */
-    MC13783_EVENT_SET1 = 1, /* *STATUS1/MASK1/SENSE1 */
+    /* *STATUS0/MASK0/SENSE0 */
+    MC13783_INT_ID_ADCDONE    =  0,
+    MC13783_INT_ID_ADCBISDONE =  1,
+    MC13783_INT_ID_TS         =  2,
+    MC13783_INT_ID_WHIGH      =  3,
+    MC13783_INT_ID_WLOW       =  4,
+    MC13783_INT_ID_CHGDET     =  6,
+    MC13783_INT_ID_CHGOV      =  7,
+    MC13783_INT_ID_CHGREV     =  8,
+    MC13783_INT_ID_CHGSHORT   =  9,
+    MC13783_INT_ID_CCCV       = 10,
+    MC13783_INT_ID_CHGCURR    = 11,
+    MC13783_INT_ID_BPONI      = 12,
+    MC13783_INT_ID_LOBATL     = 13,
+    MC13783_INT_ID_LOBATH     = 14,
+    MC13783_INT_ID_UDP        = 15,
+    MC13783_INT_ID_USB        = 16,
+    MC13783_INT_ID_IDFLOAT    = 19,
+    MC13783_INT_ID_SE1        = 21,
+    MC13783_INT_ID_CKDET      = 22,
+    MC13783_INT_ID_UDM        = 23,
+     /* *STATUS1/MASK1/SENSE1 */
+    MC13783_INT_ID_1HZ        =  0 + 0x20,
+    MC13783_INT_ID_TODA       =  1 + 0x20,
+    MC13783_INT_ID_ONOFD1     =  3 + 0x20,  /* ON1B */
+    MC13783_INT_ID_ONOFD2     =  4 + 0x20,  /* ON2B */
+    MC13783_INT_ID_ONOFD3     =  5 + 0x20,  /* ON3B */
+    MC13783_INT_ID_SYSRST     =  6 + 0x20,
+    MC13783_INT_ID_RTCRST     =  7 + 0x20,
+    MC13783_INT_ID_PCI        =  8 + 0x20,
+    MC13783_INT_ID_WARM       =  9 + 0x20,
+    MC13783_INT_ID_MEMHLD     = 10 + 0x20,
+    MC13783_INT_ID_PWRRDY     = 11 + 0x20,
+    MC13783_INT_ID_THWARNL    = 12 + 0x20,
+    MC13783_INT_ID_THWARNH    = 13 + 0x20,
+    MC13783_INT_ID_CLK        = 14 + 0x20,
+    MC13783_INT_ID_SEMAF      = 15 + 0x20,
+    MC13783_INT_ID_MC2B       = 17 + 0x20,
+    MC13783_INT_ID_HSDET      = 18 + 0x20,
+    MC13783_INT_ID_HSL        = 19 + 0x20,
+    MC13783_INT_ID_ALSPTH     = 20 + 0x20,
+    MC13783_INT_ID_AHSSHORT   = 21 + 0x20,
 };
 
+#define MC13783_INT_ID_SET_DIV    (0x20)
+#define MC13783_INT_ID_NUM_MASK   (0x1f)
+
 struct mc13783_event
 {
-    enum mc13783_event_sets set : 8;
-    uint32_t mask               : 24;
+    enum mc13783_int_ids int_id;
     void (*callback)(void);
 };
 
diff --git a/firmware/target/arm/imx31/gigabeat-s/mc13783-gigabeat-s.c b/firmware/target/arm/imx31/gigabeat-s/mc13783-gigabeat-s.c
index e0745a5b8b..6d992388f2 100644
--- a/firmware/target/arm/imx31/gigabeat-s/mc13783-gigabeat-s.c
+++ b/firmware/target/arm/imx31/gigabeat-s/mc13783-gigabeat-s.c
@@ -56,33 +56,28 @@ const struct mc13783_event mc13783_events[MC13783_NUM_EVENTS] =
 {
     [MC13783_ADCDONE_EVENT] = /* ADC conversion complete */
     {
-        .set  = MC13783_EVENT_SET0,
-        .mask = MC13783_ADCDONEM,
+        .int_id = MC13783_INT_ID_ADCDONE,
         .callback = adc_done,
     },
     [MC13783_ONOFD1_EVENT] = /* Power button */
     {
-        .set  = MC13783_EVENT_SET1,
-        .mask = MC13783_ONOFD1M,
+        .int_id = MC13783_INT_ID_ONOFD1,
         .callback = button_power_event,
     },
     [MC13783_SE1_EVENT] = /* Main charger detection */
     {
-        .set  = MC13783_EVENT_SET0,
-        .mask = MC13783_SE1M,
+        .int_id = MC13783_INT_ID_SE1,
         .callback = charger_main_detect_event,
     },
     [MC13783_USB_EVENT] = /* USB insertion/USB charger detection */
     {
-        .set  = MC13783_EVENT_SET0,
-        .mask = MC13783_USBM,
+        .int_id = MC13783_INT_ID_USB,
         .callback = usb_connect_event,
     },
 #ifdef HAVE_HEADPHONE_DETECTION
     [MC13783_ONOFD2_EVENT] = /* Headphone jack */
     {
-        .set  = MC13783_EVENT_SET1,
-        .mask = MC13783_ONOFD2M,
+        .int_id = MC13783_INT_ID_ONOFD2,
         .callback = headphone_detect_event,
     },
 #endif
diff --git a/firmware/target/arm/imx31/mc13783-imx31.c b/firmware/target/arm/imx31/mc13783-imx31.c
index 094fbaa58b..268c33a549 100644
--- a/firmware/target/arm/imx31/mc13783-imx31.c
+++ b/firmware/target/arm/imx31/mc13783-imx31.c
@@ -34,10 +34,6 @@ static int mc13783_thread_stack[DEFAULT_STACK_SIZE/sizeof(int)];
 static const char * const mc13783_thread_name = "pmic";
 static struct semaphore mc13783_svc_wake;
 
-/* Synchronous thread communication objects */
-static struct mutex mc13783_spi_mutex;
-static struct semaphore mc13783_spi_complete;
-
 /* Tracking for which interrupts are enabled */
 static uint32_t pmic_int_enabled[2] =
     { 0x00000000, 0x00000000 };
@@ -50,32 +46,27 @@ static const unsigned char pmic_ints_regs[2] =
 
 static volatile unsigned int mc13783_thread_id = 0;
 
-static void mc13783_xfer_complete_cb(struct spi_transfer_desc *trans);
-
-/* Transfer descriptor for synchronous reads and writes */
-static struct spi_transfer_desc mc13783_transfer =
+/* Extend the basic SPI transfer descriptor with our own fields */
+struct mc13783_transfer_desc
 {
-    .node = &mc13783_spi,
-    .txbuf = NULL,
-    .rxbuf = NULL,
-    .count = 0,
-    .callback = mc13783_xfer_complete_cb,
-    .next = NULL,
+    struct spi_transfer_desc xfer;
+    union
+    {
+        struct semaphore sema;
+        uint32_t data;
+    };
 };
 
 /* Called when a transfer is finished and data is ready/written */
 static void mc13783_xfer_complete_cb(struct spi_transfer_desc *xfer)
 {
-    if (xfer->count != 0)
-        return;
-
-    semaphore_release(&mc13783_spi_complete);
+    semaphore_release(&((struct mc13783_transfer_desc *)xfer)->sema);
 }
 
-static inline bool wait_for_transfer_complete(void)
+static inline bool wait_for_transfer_complete(struct mc13783_transfer_desc *xfer)
 {
-    return semaphore_wait(&mc13783_spi_complete, HZ*2)
-            == OBJ_WAIT_SUCCEEDED && mc13783_transfer.count == 0;
+    return semaphore_wait(&xfer->sema, TIMEOUT_BLOCK)
+            == OBJ_WAIT_SUCCEEDED && xfer->xfer.count == 0;
 }
 
 static void mc13783_interrupt_thread(void)
@@ -114,15 +105,14 @@ static void mc13783_interrupt_thread(void)
         /* .count is surely expected to be > 0 */
         do
         {
-            enum mc13783_event_sets set = event->set;
+            unsigned int set = event->int_id / MC13783_INT_ID_SET_DIV;
             uint32_t pnd = pending[set];
-            uint32_t mask = event->mask;
+            uint32_t mask = 1 << (event->int_id & MC13783_INT_ID_NUM_MASK);
 
             if (pnd & mask)
             {
                 event->callback();
-                pnd &= ~mask;
-                pending[set] = pnd;
+                pending[set] = pnd & ~mask;
             }
 
             if ((pending[0] | pending[1]) == 0)
@@ -147,9 +137,6 @@ void INIT_ATTR mc13783_init(void)
 {
     /* Serial interface must have been initialized first! */
     semaphore_init(&mc13783_svc_wake, 1, 0);
-    mutex_init(&mc13783_spi_mutex);
-
-    semaphore_init(&mc13783_spi_complete, 1, 0);
 
     /* Enable the PMIC SPI module */
     spi_enable_module(&mc13783_spi);
@@ -183,205 +170,169 @@ void mc13783_close(void)
 bool mc13783_enable_event(enum mc13783_event_ids id)
 {
     const struct mc13783_event * const event = &mc13783_events[id];
-    int set = event->set;
-    uint32_t mask = event->mask;
-
-    mutex_lock(&mc13783_spi_mutex);
+    unsigned int set = event->int_id / MC13783_INT_ID_SET_DIV;
+    uint32_t mask = 1 << (event->int_id & MC13783_INT_ID_NUM_MASK);
 
     pmic_int_enabled[set] |= mask;
     mc13783_clear(pmic_intm_regs[set], mask);
 
-    mutex_unlock(&mc13783_spi_mutex);
-
     return true;
 }
 
 void mc13783_disable_event(enum mc13783_event_ids id)
 {
     const struct mc13783_event * const event = &mc13783_events[id];
-    int set = event->set;
-    uint32_t mask = event->mask;
-
-    mutex_lock(&mc13783_spi_mutex);
+    unsigned int set = event->int_id / MC13783_INT_ID_SET_DIV;
+    uint32_t mask = 1 << (event->int_id & MC13783_INT_ID_NUM_MASK);
 
     pmic_int_enabled[set] &= ~mask;
     mc13783_set(pmic_intm_regs[set], mask);
-
-    mutex_unlock(&mc13783_spi_mutex);
 }
 
-uint32_t mc13783_set(unsigned address, uint32_t bits)
+static inline bool mc13783_transfer(struct spi_transfer_desc *xfer,
+                                    uint32_t *txbuf,
+                                    uint32_t *rxbuf,
+                                    int count,
+                                    spi_transfer_cb_fn_type callback)
 {
-    uint32_t data;
-
-    mutex_lock(&mc13783_spi_mutex);
-
-    data = mc13783_read(address);
-
-    if (data != MC13783_DATA_ERROR)
-        mc13783_write(address, data | bits);
+    xfer->node = &mc13783_spi;
+    xfer->txbuf = txbuf;
+    xfer->rxbuf = rxbuf;
+    xfer->count = count;
+    xfer->callback = callback;
+    xfer->next = NULL;
 
-    mutex_unlock(&mc13783_spi_mutex);
+    return spi_transfer(xfer);
+}
 
-    return data;
+uint32_t mc13783_set(unsigned address, uint32_t bits)
+{
+    return mc13783_write_masked(address, bits, bits);
 }
 
 uint32_t mc13783_clear(unsigned address, uint32_t bits)
 {
-    uint32_t data;
-
-    mutex_lock(&mc13783_spi_mutex);
-
-    data = mc13783_read(address);
-
-    if (data != MC13783_DATA_ERROR)
-        mc13783_write(address, data & ~bits);
-
-    mutex_unlock(&mc13783_spi_mutex);
-
-    return data;
+    return mc13783_write_masked(address, 0, bits);
 }
 
-int mc13783_write(unsigned address, uint32_t data)
+/* Called when the first transfer of mc13783_write_masked is complete */
+static void mc13783_write_masked_cb(struct spi_transfer_desc *xfer)
 {
-    uint32_t packet;
-    int i;
+    struct mc13783_transfer_desc *desc = (struct mc13783_transfer_desc *)xfer;
+    uint32_t *packets = desc->xfer.rxbuf; /* Will have been advanced by 1 */
+    packets[0] |= packets[-1] & ~desc->data;
+}
 
+uint32_t mc13783_write_masked(unsigned address, uint32_t data, uint32_t mask)
+{
     if (address >= MC13783_NUM_REGS)
-        return -1;
+        return MC13783_DATA_ERROR;
+
+    mask &= 0xffffff;
 
-    packet = (1 << 31) | (address << 25) | (data & 0xffffff);
+    uint32_t packets[2] =
+    {
+        address << 25,
+        (1 << 31) | (address << 25) | (data & mask)
+    };
 
-    mutex_lock(&mc13783_spi_mutex);
+    struct mc13783_transfer_desc xfers[2];
+    xfers[0].data = mask;
+    semaphore_init(&xfers[1].sema, 1, 0);
 
-    mc13783_transfer.txbuf = &packet;
-    mc13783_transfer.rxbuf = NULL;
-    mc13783_transfer.count = 1;
+    unsigned long cpsr = disable_irq_save();
 
-    i = -1;
+    /* Queue up two transfers in a row */
+    bool ok = mc13783_transfer(&xfers[0].xfer, &packets[0], &packets[0], 1,
+                               mc13783_write_masked_cb) &&
+              mc13783_transfer(&xfers[1].xfer, &packets[1], NULL, 1,
+                               mc13783_xfer_complete_cb);
 
-    if (spi_transfer(&mc13783_transfer) && wait_for_transfer_complete())
-        i = 1 - mc13783_transfer.count;
+    restore_irq(cpsr);
 
-    mutex_unlock(&mc13783_spi_mutex);
+    if (ok && wait_for_transfer_complete(&xfers[1]))
+        return packets[0];
 
-    return i;
+    return MC13783_DATA_ERROR;
 }
 
-uint32_t mc13783_write_masked(unsigned address, uint32_t data, uint32_t mask)
+uint32_t mc13783_read(unsigned address)
 {
-    uint32_t old;
+    if (address >= MC13783_NUM_REGS)
+        return MC13783_DATA_ERROR;
 
-    mutex_lock(&mc13783_spi_mutex);
+    uint32_t packet = address << 25;
 
-    old = mc13783_read(address);
+    struct mc13783_transfer_desc xfer;
+    semaphore_init(&xfer.sema, 1, 0);
 
-    if (old != MC13783_DATA_ERROR)
+    if (mc13783_transfer(&xfer.xfer, &packet, &packet, 1,
+                         mc13783_xfer_complete_cb) &&
+        wait_for_transfer_complete(&xfer))
     {
-        data = (old & ~mask) | (data & mask);
-
-        if (mc13783_write(address, data) != 1)
-            old = MC13783_DATA_ERROR;
+        return packet;
     }
 
-    mutex_unlock(&mc13783_spi_mutex);
-
-    return old;
+    return MC13783_DATA_ERROR;
 }
 
-uint32_t mc13783_read(unsigned address)
+int mc13783_write(unsigned address, uint32_t data)
 {
-    uint32_t packet;
-
     if (address >= MC13783_NUM_REGS)
-        return MC13783_DATA_ERROR;
-
-    packet = address << 25;
-
-    mutex_lock(&mc13783_spi_mutex);
+        return -1;
 
-    mc13783_transfer.txbuf = &packet;
-    mc13783_transfer.rxbuf = &packet;
-    mc13783_transfer.count = 1;
+    uint32_t packet = (1 << 31) | (address << 25) | (data & 0xffffff);
 
-    if (!spi_transfer(&mc13783_transfer) || !wait_for_transfer_complete())
-        packet = MC13783_DATA_ERROR;
+    struct mc13783_transfer_desc xfer;
+    semaphore_init(&xfer.sema, 1, 0);
 
-    mutex_unlock(&mc13783_spi_mutex);
+    if (mc13783_transfer(&xfer.xfer, &packet, NULL, 1,
+                         mc13783_xfer_complete_cb) &&
+        wait_for_transfer_complete(&xfer))
+    {
+        return 1 - xfer.xfer.count;
+    }
 
-    return packet;
+    return -1;
 }
 
 int mc13783_read_regs(const unsigned char *regs, uint32_t *buffer,
                       int count)
 {
-    int i;
+    struct mc13783_transfer_desc xfer;
+    semaphore_init(&xfer.sema, 1, 0);
 
-    for (i = 0; i < count; i++)
+    if (mc13783_read_async(&xfer.xfer, regs, buffer, count,
+                           mc13783_xfer_complete_cb) &&
+        wait_for_transfer_complete(&xfer))
     {
-        unsigned reg = regs[i];
-
-        if (reg >= MC13783_NUM_REGS)
-            return -1;
-
-        buffer[i] = reg << 25;
+        return count - xfer.xfer.count;
     }
 
-    mutex_lock(&mc13783_spi_mutex);
-
-    mc13783_transfer.txbuf = buffer;
-    mc13783_transfer.rxbuf = buffer;
-    mc13783_transfer.count = count;
-
-    i = -1;
-
-    if (spi_transfer(&mc13783_transfer) && wait_for_transfer_complete())
-        i = count - mc13783_transfer.count;
-
-    mutex_unlock(&mc13783_spi_mutex);
-
-    return i;
+    return -1;
 }
 
 int mc13783_write_regs(const unsigned char *regs, uint32_t *buffer,
                        int count)
 {
-    int i;
+    struct mc13783_transfer_desc xfer;
+    semaphore_init(&xfer.sema, 1, 0);
 
-    for (i = 0; i < count; i++)
+    if (mc13783_write_async(&xfer.xfer, regs, buffer, count,
+                            mc13783_xfer_complete_cb) &&
+        wait_for_transfer_complete(&xfer))
     {
-        unsigned reg = regs[i];
-
-        if (reg >= MC13783_NUM_REGS)
-            return -1;
-
-        buffer[i] = (1 << 31) | (reg << 25) | (buffer[i] & 0xffffff);
+        return count - xfer.xfer.count;
     }
 
-    mutex_lock(&mc13783_spi_mutex);
-
-    mc13783_transfer.txbuf = buffer;
-    mc13783_transfer.rxbuf = NULL;
-    mc13783_transfer.count = count;
-
-    i = -1;
-
-    if (spi_transfer(&mc13783_transfer) && wait_for_transfer_complete())
-        i = count - mc13783_transfer.count;
-
-    mutex_unlock(&mc13783_spi_mutex);
-
-    return i;
+    return -1;
 }
 
-#if 0 /* Not needed right now */
 bool mc13783_read_async(struct spi_transfer_desc *xfer,
                         const unsigned char *regs, uint32_t *buffer,
                         int count, spi_transfer_cb_fn_type callback)
 {
-    int i;
-
-    for (i = 0; i < count; i++)
+    for (int i = 0; i < count; i++)
     {
         unsigned reg = regs[i];
 
@@ -391,24 +342,14 @@ bool mc13783_read_async(struct spi_transfer_desc *xfer,
         buffer[i] = reg << 25;
     }
 
-    xfer->node  = &mc13783_spi;
-    xfer->txbuf = buffer;
-    xfer->rxbuf = buffer;
-    xfer->count = count;
-    xfer->callback = callback;
-    xfer->next = NULL;
-
-    return spi_transfer(xfer);
+    return mc13783_transfer(xfer, buffer, buffer, count, callback);
 }
-#endif
 
 bool mc13783_write_async(struct spi_transfer_desc *xfer,
                          const unsigned char *regs, uint32_t *buffer,
                          int count, spi_transfer_cb_fn_type callback)
 {
-    int i;
-
-    for (i = 0; i < count; i++)
+    for (int i = 0; i < count; i++)
     {
         unsigned reg = regs[i];
 
@@ -418,12 +359,5 @@ bool mc13783_write_async(struct spi_transfer_desc *xfer,
         buffer[i] = (1 << 31) | (reg << 25) | (buffer[i] & 0xffffff);
     }
 
-    xfer->node  = &mc13783_spi;
-    xfer->txbuf = buffer;
-    xfer->rxbuf = NULL;
-    xfer->count = count;
-    xfer->callback = callback;
-    xfer->next = NULL;
-
-    return spi_transfer(xfer);
+    return mc13783_transfer(xfer, buffer, NULL, count, callback);
 }
diff --git a/firmware/target/arm/imx31/spi-imx31.c b/firmware/target/arm/imx31/spi-imx31.c
index 7fcf94ce90..ea3d2f8d77 100644
--- a/firmware/target/arm/imx31/spi-imx31.c
+++ b/firmware/target/arm/imx31/spi-imx31.c
@@ -170,7 +170,10 @@ static bool start_transfer(struct spi_module_desc * const desc,
     unsigned long intreg;
 
     if (!spi_set_context(desc, xfer))
+    {
+        xfer->count = -1;
         return false;
+    }
 
     base[CONREG] |= CSPI_CONREG_EN; /* Enable module */
 
@@ -249,8 +252,18 @@ static void spi_interrupt(enum spi_module_number spi)
     if (xfer->count > 0)
     {
         /* Data to transmit - fill TXFIFO or write until exhausted. */
-        if (tx_fill_fifo(desc, base, xfer) != 0)
-            return;
+        int remaining = tx_fill_fifo(desc, base, xfer);
+
+        /* If transfer completed because TXFIFO ran out of data, resume it or
+           else it will not finish. */
+        if (!(base[CONREG] & CSPI_CONREG_XCH))
+        {
+            base[STATREG] = CSPI_STATREG_TC;
+            base[CONREG] |= CSPI_CONREG_XCH;
+        }
+
+        if (remaining > 0)
+            return; /* Still more after this */
 
         /* Out of data - stop TX interrupts, enable TC interrupt. */
         intreg &= ~CSPI_INTREG_THEN;
@@ -263,7 +276,6 @@ static void spi_interrupt(enum spi_module_number spi)
         /* Outbound transfer is complete. */
         intreg &= ~CSPI_INTREG_TCEN;
         base[INTREG] = intreg;
-        base[STATREG] = CSPI_STATREG_TC; /* Ack 'complete' */
     }
 
     if (intreg != 0)
@@ -276,8 +288,6 @@ static void spi_interrupt(enum spi_module_number spi)
         spi_transfer_cb_fn_type callback = xfer->callback;
         xfer->next = NULL;
 
-        base[CONREG] &= ~CSPI_CONREG_EN; /* Disable module */
-
         if (next == xfer)
         {
             /* Last job on queue */
@@ -287,6 +297,8 @@ static void spi_interrupt(enum spi_module_number spi)
                 callback(xfer);
 
             /* Callback may have restarted transfers. */
+            if (desc->head == NULL)
+                base[CONREG] &= ~CSPI_CONREG_EN; /* Disable module */
         }
         else
         {
@@ -299,7 +311,6 @@ static void spi_interrupt(enum spi_module_number spi)
             if (!start_transfer(desc, next))
             {
                 xfer = next;
-                xfer->count = -1;
                 continue; /* Failed: try next */
             }
         }
@@ -416,10 +427,6 @@ bool spi_transfer(struct spi_transfer_desc *xfer)
             desc->tail = xfer;
             xfer->next = xfer; /* First, self-reference terminate */
         }
-        else
-        {
-            xfer->count = -1; /* Signal error */
-        }
     }
     else
     {