i.MX31 - Dethreading operations continue

Dispense with "pmic" thread and process PMIC events directly within ISR. Add
sense bit reading as part of the handling.


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

7 files changed, 161 insertions, 153 deletions

diff --git a/firmware/target/arm/imx31/gigabeat-s/adc-gigabeat-s.c b/firmware/target/arm/imx31/gigabeat-s/adc-gigabeat-s.c
index eb30919077..b46fc2f63f 100644
--- a/firmware/target/arm/imx31/gigabeat-s/adc-gigabeat-s.c
+++ b/firmware/target/arm/imx31/gigabeat-s/adc-gigabeat-s.c
@@ -110,7 +110,7 @@ bool adc_enable_channel(int channel, bool enable)
                 != MC13783_DATA_ERROR;
 }
 
-/* Called by mc13783 interrupt thread when conversion is complete */
+/* ADC conversion complete event - called from PMIC ISR */
 void adc_done(void)
 {
     semaphore_release(&adc_done_signal);
@@ -132,5 +132,5 @@ void adc_init(void)
 
     /* Enable ADCDONE event */
     mc13783_write(MC13783_INTERRUPT_STATUS0, MC13783_ADCDONEI);
-    mc13783_enable_event(MC13783_ADCDONE_EVENT);
+    mc13783_enable_event(MC13783_ADCDONE_EVENT, true);
 }
diff --git a/firmware/target/arm/imx31/gigabeat-s/button-gigabeat-s.c b/firmware/target/arm/imx31/gigabeat-s/button-gigabeat-s.c
index 90db00dcc3..11d1d5a0b4 100644
--- a/firmware/target/arm/imx31/gigabeat-s/button-gigabeat-s.c
+++ b/firmware/target/arm/imx31/gigabeat-s/button-gigabeat-s.c
@@ -35,7 +35,7 @@
 static bool initialized     = false;
 #endif
 
-static int  ext_btn         = BUTTON_NONE; /* Buttons not on KPP */
+static unsigned long ext_btn = BUTTON_NONE; /* Buttons not on KPP */
 static bool hold_button     = false;
 #ifndef BOOTLOADER
 static bool hold_button_old = false;
@@ -150,24 +150,16 @@ int button_read_device(void)
 #endif
 }
 
-/* This is called from the mc13783 interrupt thread */
-void button_power_event(void)
+/* Helper to update the power button status */
+static void power_button_update(bool pressed)
 {
-    bool pressed =
-        (mc13783_read(MC13783_INTERRUPT_SENSE1) & MC13783_ONOFD1S) == 0;
-
-    int oldlevel = disable_irq_save();
-
-    if (pressed)
-    {
-        ext_btn |= BUTTON_POWER;
-    }
-    else
-    {
-        ext_btn &= ~BUTTON_POWER;
-    }
+    bitmod32(&ext_btn, pressed ? BUTTON_POWER : 0, BUTTON_POWER);
+}
 
-    restore_irq(oldlevel);
+/* Power button event - called from PMIC ISR */
+void button_power_event(void)
+{
+    power_button_update(!mc13783_event_sense(MC13783_ONOFD1_EVENT));
 }
 
 void button_init_device(void)
@@ -203,8 +195,9 @@ void button_init_device(void)
      * 6. Set the KDIE control bit bit. */
     KPP_KPSR = KPP_KPSR_KRSS | KPP_KPSR_KDSC | KPP_KPSR_KPKD;
 
-    button_power_event();
-    mc13783_enable_event(MC13783_ONOFD1_EVENT);
+    power_button_update(!(mc13783_read(MC13783_INTERRUPT_SENSE1)
+                            & MC13783_ONOFD1S));
+    mc13783_enable_event(MC13783_ONOFD1_EVENT, true);
 
 #ifdef HAVE_HEADPHONE_DETECTION
     headphone_init();
@@ -220,7 +213,7 @@ void button_close_device(void)
     /* Assumes HP detection is not available */
     initialized = false;
 
-    mc13783_disable_event(MC13783_ONOFD1_EVENT);
+    mc13783_enable_event(MC13783_ONOFD1_EVENT, true);
     ext_btn = BUTTON_NONE;
 }
 #endif /* BUTTON_DRIVER_CLOSE */
diff --git a/firmware/target/arm/imx31/gigabeat-s/headphone-gigabeat-s.c b/firmware/target/arm/imx31/gigabeat-s/headphone-gigabeat-s.c
index 6e76615308..4e1792d467 100644
--- a/firmware/target/arm/imx31/gigabeat-s/headphone-gigabeat-s.c
+++ b/firmware/target/arm/imx31/gigabeat-s/headphone-gigabeat-s.c
@@ -171,7 +171,7 @@ static void headphone_thread(void)
     }
 }
 
-/* This is called from the mc13783 interrupt thread */
+/* HP plugged/unplugged event - called from PMIC ISR */
 void headphone_detect_event(void)
 {
     /* Trigger the thread immediately. */
@@ -197,5 +197,5 @@ void INIT_ATTR headphone_init(void)
 
     /* Initially poll and then enable PMIC event */
     headphone_detect_event();
-    mc13783_enable_event(MC13783_ONOFD2_EVENT);
+    mc13783_enable_event(MC13783_ONOFD2_EVENT, true);
 }
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 12009fae06..6ae8c23c48 100644
--- a/firmware/target/arm/imx31/gigabeat-s/mc13783-gigabeat-s.c
+++ b/firmware/target/arm/imx31/gigabeat-s/mc13783-gigabeat-s.c
@@ -55,28 +55,33 @@ const struct mc13783_event mc13783_events[MC13783_NUM_EVENTS] =
 {
     [MC13783_ADCDONE_EVENT] = /* ADC conversion complete */
     {
-        .int_id = MC13783_INT_ID_ADCDONE,
+        .int_id   = MC13783_INT_ID_ADCDONE,
+        .sense    = 0,
         .callback = adc_done,
     },
     [MC13783_ONOFD1_EVENT] = /* Power button */
     {
-        .int_id = MC13783_INT_ID_ONOFD1,
+        .int_id   = MC13783_INT_ID_ONOFD1,
+        .sense    = MC13783_ONOFD1S,
         .callback = button_power_event,
     },
     [MC13783_SE1_EVENT] = /* Main charger detection */
     {
-        .int_id = MC13783_INT_ID_SE1,
+        .int_id   = MC13783_INT_ID_SE1,
+        .sense    = MC13783_SE1S,
         .callback = charger_main_detect_event,
     },
     [MC13783_USB_EVENT] = /* USB insertion/USB charger detection */
     {
-        .int_id = MC13783_INT_ID_USB,
+        .int_id   = MC13783_INT_ID_USB,
+        .sense    = MC13783_USB4V4S,
         .callback = usb_connect_event,
     },
 #ifdef HAVE_HEADPHONE_DETECTION
     [MC13783_ONOFD2_EVENT] = /* Headphone jack */
     {
-        .int_id = MC13783_INT_ID_ONOFD2,
+        .int_id   = MC13783_INT_ID_ONOFD2,
+        .sense    = 0,
         .callback = headphone_detect_event,
     },
 #endif
diff --git a/firmware/target/arm/imx31/gigabeat-s/power-gigabeat-s.c b/firmware/target/arm/imx31/gigabeat-s/power-gigabeat-s.c
index 11276a6c3a..5b255a0e27 100644
--- a/firmware/target/arm/imx31/gigabeat-s/power-gigabeat-s.c
+++ b/firmware/target/arm/imx31/gigabeat-s/power-gigabeat-s.c
@@ -33,7 +33,7 @@
 #include "fmradio_i2c.h"
 #endif
 
-static unsigned int power_status = POWER_INPUT_NONE;
+static unsigned long power_status = POWER_INPUT_NONE;
 
 /* Detect which power sources are present. */
 unsigned int power_input_status(void)
@@ -58,24 +58,28 @@ void usb_charging_maxcurrent_change(int maxcurrent)
     /* Nothing to do */
 }
 
-/* Detect changes in presence of the AC adaptor. */
+/* Helper to update the charger status */
+static void update_main_charger(bool present)
+{
+    bitmod32(&power_status, present ? POWER_INPUT_MAIN_CHARGER : 0,
+             POWER_INPUT_MAIN_CHARGER);
+}
+
+/* Detect changes in presence of the AC adaptor. Called from PMIC ISR. */
 void charger_main_detect_event(void)
 {
-    if (mc13783_read(MC13783_INTERRUPT_SENSE0) & MC13783_SE1S)
-        power_status |= POWER_INPUT_MAIN_CHARGER;
-    else
-        power_status &= ~POWER_INPUT_MAIN_CHARGER;
+    update_main_charger(mc13783_event_sense(MC13783_INT_ID_SE1)
+                            & MC13783_SE1S);
 }
 
-/* Detect changes in USB bus power. Called from usb connect event handler. */
+/* Detect changes in USB bus power. Called from usb connect event ISR. */
 void charger_usb_detect_event(int status)
 {
     /* USB plugged does not imply charging is possible or even
      * powering the device to maintain the battery. */
-    if (status == USB_INSERTED)
-        power_status |= POWER_INPUT_USB_CHARGER;
-    else
-        power_status &= ~POWER_INPUT_USB_CHARGER;
+    bitmod32(&power_status,
+             status == USB_INSERTED ? POWER_INPUT_USB_CHARGER : 0,
+             POWER_INPUT_USB_CHARGER);
 }
 
 /* charging_state is implemented in powermgmt-imx31.c */
@@ -152,8 +156,9 @@ void power_init(void)
 #endif
 
     /* Poll initial state */
-    charger_main_detect_event();
+    update_main_charger(mc13783_read(MC13783_INTERRUPT_SENSE0)
+                            & MC13783_SE1S);
 
     /* Enable detect event */
-    mc13783_enable_event(MC13783_SE1_EVENT);
+    mc13783_enable_event(MC13783_SE1_EVENT, true);
 }
diff --git a/firmware/target/arm/imx31/gigabeat-s/usb-gigabeat-s.c b/firmware/target/arm/imx31/gigabeat-s/usb-gigabeat-s.c
index b157544016..1584ffb574 100644
--- a/firmware/target/arm/imx31/gigabeat-s/usb-gigabeat-s.c
+++ b/firmware/target/arm/imx31/gigabeat-s/usb-gigabeat-s.c
@@ -59,16 +59,23 @@ bool usb_plugged(void)
     return mc13783_read(MC13783_INTERRUPT_SENSE0) & MC13783_USB4V4S;
 }
 
-void usb_connect_event(void)
+/* Helper to update the USB cable status */
+static void update_usb_status(bool sense)
 {
-    /* Read the immediate state of the cable from the PMIC */
-    int status = usb_plugged() ? USB_INSERTED : USB_EXTRACTED;
+    int status = sense ? USB_INSERTED : USB_EXTRACTED;
     usb_status = status;
     /* Notify power that USB charging is potentially available */
     charger_usb_detect_event(status);
     usb_status_event(status);
 }
 
+/* Detect presence of USB bus - called from PMIC ISR */
+void usb_connect_event(void)
+{
+    /* Read the associated sense value */
+    update_usb_status(mc13783_event_sense(MC13783_USB_EVENT));
+}
+
 int usb_detect(void)
 {
     return usb_status;
@@ -80,10 +87,10 @@ void usb_init_device(void)
     usb_drv_startup();
 
     /* Initially poll */
-    usb_connect_event();
+    update_usb_status(usb_plugged());
 
     /* Enable PMIC event */
-    mc13783_enable_event(MC13783_USB_EVENT);
+    mc13783_enable_event(MC13783_USB_EVENT, true);
 }
 
 void usb_enable(bool on)
diff --git a/firmware/target/arm/imx31/mc13783-imx31.c b/firmware/target/arm/imx31/mc13783-imx31.c
index 01690f0def..31b91b46fc 100644
--- a/firmware/target/arm/imx31/mc13783-imx31.c
+++ b/firmware/target/arm/imx31/mc13783-imx31.c
@@ -26,26 +26,6 @@
 #include "debug.h"
 #include "kernel.h"
 
-extern const struct mc13783_event mc13783_events[MC13783_NUM_EVENTS];
-extern struct spi_node mc13783_spi;
-
-/* PMIC event service data */
-static int mc13783_thread_stack[DEFAULT_STACK_SIZE/sizeof(int)];
-static const char * const mc13783_thread_name = "pmic";
-static struct semaphore mc13783_svc_wake;
-
-/* Tracking for which interrupts are enabled */
-static uint32_t pmic_int_enabled[2] =
-    { 0x00000000, 0x00000000 };
-
-static const unsigned char pmic_intm_regs[2] =
-    { MC13783_INTERRUPT_MASK0, MC13783_INTERRUPT_MASK1 };
-
-static const unsigned char pmic_ints_regs[2] =
-    { MC13783_INTERRUPT_STATUS0, MC13783_INTERRUPT_STATUS1 };
-
-static volatile unsigned int mc13783_thread_id = 0;
-
 /* Extend the basic SPI transfer descriptor with our own fields */
 struct mc13783_transfer_desc
 {
@@ -57,6 +37,32 @@ struct mc13783_transfer_desc
     };
 };
 
+extern const struct mc13783_event mc13783_events[MC13783_NUM_EVENTS];
+extern struct spi_node mc13783_spi;
+
+static uint32_t pmic_int_enb[2]; /* Enabled ints */
+static uint32_t pmic_int_sense_enb[2]; /* Enabled sense reading */
+static uint32_t int_pnd_buf[2];  /* Pending ints */
+static uint32_t int_data_buf[4]; /* ISR data buffer */
+static struct spi_transfer_desc int_xfers[2]; /* ISR transfer descriptor */
+static bool restore_event = true;
+
+static inline bool mc13783_transfer(struct spi_transfer_desc *xfer,
+                                    uint32_t *txbuf,
+                                    uint32_t *rxbuf,
+                                    int count,
+                                    spi_transfer_cb_fn_type callback)
+{
+    xfer->node = &mc13783_spi;
+    xfer->txbuf = txbuf;
+    xfer->rxbuf = rxbuf;
+    xfer->count = count;
+    xfer->callback = callback;
+    xfer->next = NULL;
+
+    return spi_transfer(xfer);
+}
+
 /* Called when a transfer is finished and data is ready/written */
 static void mc13783_xfer_complete_cb(struct spi_transfer_desc *xfer)
 {
@@ -69,74 +75,87 @@ static inline bool wait_for_transfer_complete(struct mc13783_transfer_desc *xfer
             == OBJ_WAIT_SUCCEEDED && xfer->xfer.count == 0;
 }
 
-static void mc13783_interrupt_thread(void)
+/* Efficient interrupt status and acking */
+static void mc13783_int_svc_complete_callback(struct spi_transfer_desc *xfer)
 {
-    uint32_t pending[2];
-
-    /* Enable mc13783 GPIO event */
-    gpio_enable_event(MC13783_EVENT_ID);
+    /* Restore PMIC interrupt events */
+    if (restore_event)
+        bitset32(&MC13783_GPIO_IMR, 1ul << MC13783_GPIO_LINE);
 
-    while (1)
+    /* Call handlers */
+    for (
+        const struct mc13783_event *event = mc13783_events;
+        int_pnd_buf[0] | int_pnd_buf[1];
+        event++
+    )
     {
-        const struct mc13783_event *event, *event_last;
-
-        semaphore_wait(&mc13783_svc_wake, TIMEOUT_BLOCK);
-
-        if (mc13783_thread_id == 0)
-            break;
-
-        mc13783_read_regs(pmic_ints_regs, pending, 2);
-
-        /* Only clear interrupts being dispatched */
-        pending[0] &= pmic_int_enabled[0];
-        pending[1] &= pmic_int_enabled[1];
+        unsigned int set = event->int_id / MC13783_INT_ID_SET_DIV;
+        uint32_t pnd = int_pnd_buf[set];
+        uint32_t mask = 1 << (event->int_id & MC13783_INT_ID_NUM_MASK);
 
-        mc13783_write_regs(pmic_ints_regs, pending, 2);
+        if (pnd & mask)
+        {
+            event->callback();
+            int_pnd_buf[set] = pnd & ~mask;
+        }
+    }
 
-        /* Whatever is going to be serviced in this loop has been
-         * acknowledged. Reenable interrupt and if anything was still
-         * pending or became pending again, another signal will be
-         * generated. */
-        bitset32(&MC13783_GPIO_IMR, 1ul << MC13783_GPIO_LINE);
+    (void)xfer;
+}
 
-        event = mc13783_events;
-        event_last = event + MC13783_NUM_EVENTS;
+static void mc13783_int_svc_callback(struct spi_transfer_desc *xfer)
+{
+    /* Only clear interrupts with handlers */
+    int_pnd_buf[0] &= pmic_int_enb[0];
+    int_pnd_buf[1] &= pmic_int_enb[1];
 
-        /* .count is surely expected to be > 0 */
-        do
-        {
-            unsigned int set = event->int_id / MC13783_INT_ID_SET_DIV;
-            uint32_t pnd = pending[set];
-            uint32_t mask = 1 << (event->int_id & MC13783_INT_ID_NUM_MASK);
-
-            if (pnd & mask)
-            {
-                event->callback();
-                pending[set] = pnd & ~mask;
-            }
-
-            if ((pending[0] | pending[1]) == 0)
-                break; /* Terminate early if nothing more to service */
-        }
-        while (++event < event_last);
+    /* Only read sense if enabled interrupts have them enabled */
+    if ((int_pnd_buf[0] & pmic_int_sense_enb[0]) ||
+        (int_pnd_buf[1] & pmic_int_sense_enb[1]))
+    {
+        int_data_buf[2] = MC13783_INTERRUPT_SENSE0 << 25;
+        int_data_buf[3] = MC13783_INTERRUPT_SENSE1 << 25;
+        int_xfers[1].rxbuf = int_data_buf;
+        int_xfers[1].count = 4;
     }
 
-    gpio_disable_event(MC13783_EVENT_ID);
+    /* Setup the write packets with status(es) to clear */
+    int_data_buf[0] = (1 << 31) | (MC13783_INTERRUPT_STATUS0 << 25)
+                        | int_pnd_buf[0];
+    int_data_buf[1] = (1 << 31) | (MC13783_INTERRUPT_STATUS1 << 25)
+                        | int_pnd_buf[1];
+    (void)xfer;
 }
 
 /* GPIO interrupt handler for mc13783 */
 void mc13783_event(void)
 {
-    /* Mask the interrupt (unmasked when PMIC thread services it). */
+    /* Mask the interrupt (unmasked after final read services it). */
     bitclr32(&MC13783_GPIO_IMR, 1ul << MC13783_GPIO_LINE);
     MC13783_GPIO_ISR = (1ul << MC13783_GPIO_LINE);
-    semaphore_release(&mc13783_svc_wake);
+
+    /* Setup the read packets */
+    int_pnd_buf[0] = MC13783_INTERRUPT_STATUS0 << 25;
+    int_pnd_buf[1] = MC13783_INTERRUPT_STATUS1 << 25;
+
+    unsigned long cpsr = disable_irq_save();
+
+    /* Do these without intervening transfers */
+    if (mc13783_transfer(&int_xfers[0], int_pnd_buf, int_pnd_buf, 2,
+                         mc13783_int_svc_callback))
+    {
+        /* Start this provisionally and fill-in actual values during the
+           first transfer's callback - set whatever could be known */
+        mc13783_transfer(&int_xfers[1], int_data_buf, NULL, 2,
+                         mc13783_int_svc_complete_callback);
+    }
+
+    restore_irq(cpsr);
 }
 
 void INIT_ATTR mc13783_init(void)
 {
     /* Serial interface must have been initialized first! */
-    semaphore_init(&mc13783_svc_wake, 1, 0);
 
     /* Enable the PMIC SPI module */
     spi_enable_node(&mc13783_spi, true);
@@ -147,62 +166,41 @@ void INIT_ATTR mc13783_init(void)
     mc13783_write(MC13783_INTERRUPT_MASK1, 0xffffff);
 
     MC13783_GPIO_ISR = (1ul << MC13783_GPIO_LINE);
-
-    mc13783_thread_id =
-        create_thread(mc13783_interrupt_thread,
-            mc13783_thread_stack, sizeof(mc13783_thread_stack), 0,
-            mc13783_thread_name IF_PRIO(, PRIORITY_REALTIME) IF_COP(, CPU));
+    gpio_enable_event(MC13783_EVENT_ID);
 }
 
 void mc13783_close(void)
 {
-    unsigned int thread_id = mc13783_thread_id;
-
-    if (thread_id == 0)
-        return;
-
-    mc13783_thread_id = 0;
-    semaphore_release(&mc13783_svc_wake);
-    thread_wait(thread_id);
+    gpio_disable_event(MC13783_EVENT_ID);
     spi_enable_node(&mc13783_spi, false);
 }
 
-bool mc13783_enable_event(enum mc13783_event_ids id)
+void mc13783_enable_event(enum mc13783_event_ids id, bool enable)
 {
+    static const unsigned char pmic_intm_regs[2] =
+        { MC13783_INTERRUPT_MASK0, MC13783_INTERRUPT_MASK1 };
+
     const struct mc13783_event * const event = &mc13783_events[id];
     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);
-
-    return true;
+    /* Mask GPIO while changing bits around */
+    restore_event = false;
+    bitclr32(&MC13783_GPIO_IMR, 1ul << MC13783_GPIO_LINE);
+    mc13783_write_masked(pmic_intm_regs[set],
+                         enable ? 0 : mask, mask);
+    bitmod32(&pmic_int_enb[set], enable ? mask : 0, mask);
+    bitmod32(&pmic_int_sense_enb[set], enable ? event->sense : 0,
+             event->sense);
+    restore_event = true;
+    bitset32(&MC13783_GPIO_IMR, 1ul << MC13783_GPIO_LINE);
 }
 
-void mc13783_disable_event(enum mc13783_event_ids id)
+uint32_t mc13783_event_sense(enum mc13783_event_ids id)
 {
     const struct mc13783_event * const event = &mc13783_events[id];
     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);
-}
-
-static inline bool mc13783_transfer(struct spi_transfer_desc *xfer,
-                                    uint32_t *txbuf,
-                                    uint32_t *rxbuf,
-                                    int count,
-                                    spi_transfer_cb_fn_type callback)
-{
-    xfer->node = &mc13783_spi;
-    xfer->txbuf = txbuf;
-    xfer->rxbuf = rxbuf;
-    xfer->count = count;
-    xfer->callback = callback;
-    xfer->next = NULL;
-
-    return spi_transfer(xfer);
+    return int_data_buf[2 + set] & event->sense;
 }
 
 uint32_t mc13783_set(unsigned address, uint32_t bits)