Gigabeat S:

  1) Rework event handling and static registration mechanism. No target-
     specific code in mc13783 driver. GPIO event driver interfaces more
     cleanly.

  2) Somewhat related - enable thread priority for bootloader which is
     desireable here (ffs is used for GPIO event enabling  anyway and that
     goes along with priority).


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

19 files changed, 433 insertions, 225 deletions

diff --git a/firmware/SOURCES b/firmware/SOURCES
index bbdd0bc5a2..88e48bb226 100644
--- a/firmware/SOURCES
+++ b/firmware/SOURCES
@@ -679,12 +679,13 @@ target/arm/imx31/gigabeat-s/backlight-imx31.c
 target/arm/imx31/gigabeat-s/button-imx31.c
 target/arm/imx31/gigabeat-s/clkctl-imx31.c
 target/arm/imx31/gigabeat-s/dma_start.c
-target/arm/imx31/gigabeat-s/gpio-imx31.c
 target/arm/imx31/gigabeat-s/gpio-gigabeat-s.c
+target/arm/imx31/gigabeat-s/gpio-imx31.c
 target/arm/imx31/gigabeat-s/kernel-imx31.c
 target/arm/imx31/gigabeat-s/i2c-imx31.c
 target/arm/imx31/gigabeat-s/i2s-imx31.c
 target/arm/imx31/gigabeat-s/lcd-imx31.c
+target/arm/imx31/gigabeat-s/mc13783-gigabeat-s.c
 target/arm/imx31/gigabeat-s/mc13783-imx31.c
 target/arm/imx31/gigabeat-s/mmu-imx31.c
 target/arm/imx31/gigabeat-s/power-imx31.c
diff --git a/firmware/drivers/rtc/rtc_mc13783.c b/firmware/drivers/rtc/rtc_mc13783.c
index 9d2c50cf11..325a496646 100644
--- a/firmware/drivers/rtc/rtc_mc13783.c
+++ b/firmware/drivers/rtc/rtc_mc13783.c
@@ -67,11 +67,6 @@ enum rtc_registers_indexes
 /* was it an alarm that triggered power on ? */
 static bool alarm_start = false;
 
-void mc13783_alarm_start(void)
-{
-    alarm_start = true;
-}
-
 static const unsigned char rtc_registers[RTC_NUM_REGS] =
 {
     [RTC_REG_TIME]  = MC13783_RTC_TIME,
@@ -122,7 +117,12 @@ static int is_leap_year(int y)
 /** Public APIs **/
 void rtc_init(void)
 {
-    /* Nothing to do */
+    /* only needs to be polled on startup */
+    if (mc13783_read(MC13783_INTERRUPT_STATUS1) & MC13783_TODAI)
+    {
+        alarm_start = true;
+        mc13783_write(MC13783_INTERRUPT_STATUS1, MC13783_TODAI);
+    }
 }
 
 int rtc_read_datetime(unsigned char* buf)
@@ -264,7 +264,9 @@ bool rtc_enable_alarm(bool enable)
 bool rtc_check_alarm_started(bool release_alarm)
 {
     bool rc = alarm_start;
-    alarm_start &= ~release_alarm;
+
+    if (release_alarm)
+        alarm_start = false;
 
     return rc;
 }
diff --git a/firmware/export/config-gigabeat-s.h b/firmware/export/config-gigabeat-s.h
index a0c05dad45..cafb202ebe 100644
--- a/firmware/export/config-gigabeat-s.h
+++ b/firmware/export/config-gigabeat-s.h
@@ -97,6 +97,7 @@
 /* Define the bitmask of modules used */
 #define SPI_MODULE_MASK (USE_CSPI2_MODULE)
 #define I2C_MODULE_MASK (USE_I2C1_MODULE)
+#define GPIO_EVENT_MASK (USE_GPIO1_EVENTS)
 
 /* Define this if target has an additional number of threads specific to it */
 #define TARGET_EXTRA_THREADS 1
diff --git a/firmware/export/config.h b/firmware/export/config.h
index c200355872..c7e204c5df 100644
--- a/firmware/export/config.h
+++ b/firmware/export/config.h
@@ -341,7 +341,14 @@
 #endif
 
 #if (CONFIG_CODEC == SWCODEC)
-#ifndef BOOTLOADER
+#ifdef BOOTLOADER
+
+#if CONFIG_CPU == IMX31L
+/* Priority in bootloader is wanted */
+#define HAVE_PRIORITY_SCHEDULING
+#endif
+
+#else /* !BOOTLOADER */
 
 #define HAVE_EXTENDED_MESSAGING_AND_NAME
 
diff --git a/firmware/export/mc13783.h b/firmware/export/mc13783.h
index 6ed1cb46be..1f6fee8c44 100644
--- a/firmware/export/mc13783.h
+++ b/firmware/export/mc13783.h
@@ -1270,6 +1270,9 @@ enum mc13783_regs_enum
     #define MC13783_TC3PERIODr(x)       (((x) & MC13783_TC3PERIOD) >> 21)
 #define MC13783_TC3TRIODE               (0x1 << 23)
 
+/* For event enum values which are target-defined */
+#include "mc13783-target.h"
+
 void mc13783_init(void);
 void mc13783_close(void);
 uint32_t mc13783_set(unsigned address, uint32_t bits);
@@ -1281,6 +1284,28 @@ int mc13783_write_regset(const unsigned char *regs, const uint32_t *data, int co
 uint32_t mc13783_read(unsigned address);
 int mc13783_read_multiple(unsigned start, uint32_t *buffer, int count);
 int mc13783_read_regset(const unsigned char *regs, uint32_t *buffer, int count);
-void mc13783_alarm_start(void);
+
+/* Statically-registered event enable/disable */
+enum mc13783_event_sets
+{
+    MC13783_EVENT_SET0 = 0,
+    MC13783_EVENT_SET1 = 1,
+};
+
+struct mc13783_event
+{
+    enum mc13783_event_sets set : 8;
+    uint32_t mask               : 24;
+    void (*callback)(void);
+};
+
+struct mc13783_event_list
+{
+    unsigned count;
+    const struct mc13783_event *events;
+};
+
+bool mc13783_enable_event(enum mc13783_event_ids event);
+void mc13783_disable_event(enum mc13783_event_ids event);
 
 #endif /* _MC13783_H_ */
diff --git a/firmware/target/arm/imx31/gigabeat-s/adc-imx31.c b/firmware/target/arm/imx31/gigabeat-s/adc-imx31.c
index d26d708da1..2f4e45c3b1 100644
--- a/firmware/target/arm/imx31/gigabeat-s/adc-imx31.c
+++ b/firmware/target/arm/imx31/gigabeat-s/adc-imx31.c
@@ -77,11 +77,11 @@ unsigned short adc_read(int channel)
 
     mutex_unlock(&adc_mtx);
 
-    /* Channels 0-3/8-11 in ADD1, 0-4/12-15 in ADD2 */
+    /* Channels 0-3/8-11 in ADD1, 4-7/12-15 in ADD2 */
     return (channel & 4) ? MC13783_ADD2r(data) : MC13783_ADD1r(data);
 }
 
-/* Called when conversion is complete */
+/* Called by mc13783 interrupt thread when conversion is complete */
 void adc_done(void)
 {
     wakeup_signal(&adc_wake);
@@ -100,7 +100,8 @@ void adc_init(void)
                   MC13783_RTHEN | MC13783_CHRGICON);
     /* Enable ADC, set multi-channel mode */
     mc13783_write(MC13783_ADC1, MC13783_ADEN);
-    /* Enable the ADCDONE interrupt - notifications are dispatched by
-     * event handler. */
-    mc13783_clear(MC13783_INTERRUPT_MASK0, MC13783_ADCDONEM);
+
+    /* Enable ADCDONE event */
+    mc13783_write(MC13783_INTERRUPT_STATUS0, MC13783_ADCDONEI);
+    mc13783_enable_event(MC13783_ADCDONE_EVENT);
 }
diff --git a/firmware/target/arm/imx31/gigabeat-s/button-imx31.c b/firmware/target/arm/imx31/gigabeat-s/button-imx31.c
index 746883d010..ad1e2b7e49 100644
--- a/firmware/target/arm/imx31/gigabeat-s/button-imx31.c
+++ b/firmware/target/arm/imx31/gigabeat-s/button-imx31.c
@@ -26,6 +26,7 @@
 #include "backlight-target.h"
 #include "avic-imx31.h"
 #include "clkctl-imx31.h"
+#include "mc13783.h"
 
 /* Most code in here is taken from the Linux BSP provided by Freescale
  * Copyright 2004-2006 Freescale Semiconductor, Inc. All Rights Reserved. */
@@ -119,56 +120,6 @@ static __attribute__((interrupt("IRQ"))) void KPP_HANDLER(void)
     int_btn = button;
 }
 
-void button_init_device(void)
-{
-#ifdef BOOTLOADER
-    /* Can be called more than once in the bootloader */
-    if (initialized)
-        return;
-
-    initialized = true;
-#endif
-
-    /* Enable keypad clock */
-    imx31_clkctl_module_clock_gating(CG_KPP, CGM_ON_ALL);
-
-    /* 1. Enable number of rows in keypad (KPCR[4:0])
-     *
-     * Configure the rows/cols in KPP
-     * LSB nybble in KPP is for 5 rows
-     * MSB nybble in KPP is for 3 cols */
-    KPP_KPCR |= 0x1f;
-
-    /* 2. Write 0's to KPDR[10:8] */
-    KPP_KPDR &= ~(0x7 << 8);
-
-    /* 3. Configure the keypad columns as open-drain (KPCR[10:8]). */
-    KPP_KPCR |= (0x7 << 8);
-
-    /* 4. Configure columns as output, rows as input (KDDR[10:8,4:0]) */
-    KPP_KDDR = (KPP_KDDR | (0x7 << 8)) & ~0x1f;
-
-    /* 5. Clear the KPKD Status Flag and Synchronizer chain.
-     * 6. Set the KDIE control bit bit. */
-    KPP_KPSR = KPP_KPSR_KDIE | KPP_KPSR_KRSS | KPP_KPSR_KDSC | KPP_KPSR_KPKD;
-
-    /* KPP IRQ at priority 3 */
-    avic_enable_int(KPP, IRQ, 3, KPP_HANDLER);
-}
-
-#ifdef BUTTON_DRIVER_CLOSE
-void button_close_device(void)
-{
-    int oldlevel = disable_irq_save();
-
-    avic_disable_int(KPP);
-    KPP_KPSR &= ~(KPP_KPSR_KRIE | KPP_KPSR_KDIE);
-    int_btn = BUTTON_NONE;
-
-    restore_irq(oldlevel);
-}
-#endif /* BUTTON_DRIVER_CLOSE */
-
 bool button_hold(void)
 {
     return _button_hold();
@@ -199,8 +150,11 @@ int button_read_device(void)
 }
 
 /* This is called from the mc13783 interrupt thread */
-void button_power_set_state(bool pressed)
+void button_power_event(void)
 {
+    bool pressed =
+        (mc13783_read(MC13783_INTERRUPT_SENSE1) & MC13783_ONOFD1S) == 0;
+
     /* Prevent KPP_HANDLER from changing things */
     int oldlevel = disable_irq_save();
 
@@ -218,16 +172,73 @@ void button_power_set_state(bool pressed)
 
 #ifdef HAVE_HEADPHONE_DETECTION
 /* This is called from the mc13783 interrupt thread */
-void set_headphones_inserted(bool inserted)
+void headphone_detect_event(void)
 {
-    headphones_detect = inserted;
+    /* FIXME: Not really the correct method */
+    headphones_detect =
+        (mc13783_read(MC13783_INTERRUPT_SENSE1) & MC13783_ONOFD2S) == 0;
 }
 
-/* This is called from the mc13783 interrupt thread */
-/* TODO: Just do a post to the button queue directly - implement the
- * appropriate variant in the driver. */
 bool headphones_inserted(void)
 {
     return headphones_detect;
 }
 #endif /* HAVE_HEADPHONE_DETECTION */
+
+void button_init_device(void)
+{
+#ifdef BOOTLOADER
+    /* Can be called more than once in the bootloader */
+    if (initialized)
+        return;
+
+    initialized = true;
+#endif
+
+    /* Enable keypad clock */
+    imx31_clkctl_module_clock_gating(CG_KPP, CGM_ON_ALL);
+
+    /* 1. Enable number of rows in keypad (KPCR[4:0])
+     *
+     * Configure the rows/cols in KPP
+     * LSB nybble in KPP is for 5 rows
+     * MSB nybble in KPP is for 3 cols */
+    KPP_KPCR |= 0x1f;
+
+    /* 2. Write 0's to KPDR[10:8] */
+    KPP_KPDR &= ~(0x7 << 8);
+
+    /* 3. Configure the keypad columns as open-drain (KPCR[10:8]). */
+    KPP_KPCR |= (0x7 << 8);
+
+    /* 4. Configure columns as output, rows as input (KDDR[10:8,4:0]) */
+    KPP_KDDR = (KPP_KDDR | (0x7 << 8)) & ~0x1f;
+
+    /* 5. Clear the KPKD Status Flag and Synchronizer chain.
+     * 6. Set the KDIE control bit bit. */
+    KPP_KPSR = KPP_KPSR_KDIE | KPP_KPSR_KRSS | KPP_KPSR_KDSC | KPP_KPSR_KPKD;
+
+    /* KPP IRQ at priority 3 */
+    avic_enable_int(KPP, IRQ, 3, KPP_HANDLER);
+
+    button_power_event();
+    mc13783_enable_event(MC13783_ONOFD1_EVENT);
+
+#ifdef HAVE_HEADPHONE_DETECTION
+    headphone_detect_event();
+    mc13783_enable_event(MC13783_ONOFD2_EVENT);
+#endif
+}
+
+#ifdef BUTTON_DRIVER_CLOSE
+void button_close_device(void)
+{
+    int oldlevel = disable_irq_save();
+
+    avic_disable_int(KPP);
+    KPP_KPSR &= ~(KPP_KPSR_KRIE | KPP_KPSR_KDIE);
+    int_btn = BUTTON_NONE;
+
+    restore_irq(oldlevel);
+}
+#endif /* BUTTON_DRIVER_CLOSE */
diff --git a/firmware/target/arm/imx31/gigabeat-s/button-target.h b/firmware/target/arm/imx31/gigabeat-s/button-target.h
index 61d33f8e70..28c14f358c 100644
--- a/firmware/target/arm/imx31/gigabeat-s/button-target.h
+++ b/firmware/target/arm/imx31/gigabeat-s/button-target.h
@@ -32,8 +32,8 @@ bool button_hold(void);
 void button_init_device(void);
 void button_close_device(void);
 int button_read_device(void);
-void button_power_set_state(bool pressed);
-void set_headphones_inserted(bool inserted);
+void button_power_event(void);
+void headphone_detect_event(void);
 bool headphones_inserted(void);
 
 /* Toshiba Gigabeat S-specific button codes */
diff --git a/firmware/target/arm/imx31/gigabeat-s/gpio-gigabeat-s.c b/firmware/target/arm/imx31/gigabeat-s/gpio-gigabeat-s.c
index cfbb7fcc4c..64d156407e 100644
--- a/firmware/target/arm/imx31/gigabeat-s/gpio-gigabeat-s.c
+++ b/firmware/target/arm/imx31/gigabeat-s/gpio-gigabeat-s.c
@@ -22,18 +22,23 @@
 #include "system.h"
 #include "gpio-imx31.h"
 
-extern int mc13783_event(void);
+/* Gigabeat S definitions for static GPIO event registration */
 
-static const struct gpio_event gpio1_events =
+/* Describes single events for each GPIO1 pin */
+static const struct gpio_event gpio1_events[] =
 {
-    .line     = MC13783_GPIO_LINE,
-    .sense    = GPIO_SENSE_RISING,
-    .callback = mc13783_event,
+    [MC13783_EVENT_ID-GPIO1_EVENT_FIRST] =
+    {
+        .mask     = 1 << MC13783_GPIO_LINE,
+        .sense    = GPIO_SENSE_RISING,
+        .callback = mc13783_event,
+    }
 };
 
+/* Describes the events attached to GPIO1 port */
 const struct gpio_event_list gpio1_event_list =
 {
-    .priority = 7,
-    .count    = 1,
-    .events   = &gpio1_events,
+    .ints_priority = 7,
+    .count         = ARRAYLEN(gpio1_events),
+    .events        = gpio1_events,
 };
diff --git a/firmware/target/arm/imx31/gigabeat-s/gpio-imx31.c b/firmware/target/arm/imx31/gigabeat-s/gpio-imx31.c
index 0b76b84d36..2a83b5ca73 100644
--- a/firmware/target/arm/imx31/gigabeat-s/gpio-imx31.c
+++ b/firmware/target/arm/imx31/gigabeat-s/gpio-imx31.c
@@ -73,29 +73,38 @@ static struct gpio_module_descriptor
 #endif
 };
 
-static void gpio_call_events(enum gpio_module_number gpio)
+static void gpio_call_events(const struct gpio_module_descriptor * const desc)
 {
-    const struct gpio_module_descriptor * const desc = &gpio_descs[gpio];
     const struct gpio_event_list * const list = desc->list;
     struct gpio_map * const base = desc->base;
-    unsigned i;
+    const struct gpio_event * event, *event_last;
 
     /* Intersect pending and unmasked bits */
-    uint32_t pending = base->isr & base->imr;
+    uint32_t pnd = base->isr & base->imr;
+
+    event = list->events;
+    event_last = event + list->count;
 
     /* Call each event handler in order */
-    for (i = 0; i < list->count; i++)
+    /* .count is surely expected to be > 0 */
+    do
     {
-        const struct gpio_event * const event = &list->events[i];
-        uint32_t bit = 1ul << event->line;
+        uint32_t mask = event->mask;
+
+        if (pnd & mask)
+        {
+            event->callback();
+            pnd &= ~mask;
+        }
 
-        if ((pending & bit) && event->callback())
-            pending &= ~bit;
+        if (pnd == 0)
+            break; /* Teminate early if nothing more to service */
     }
+    while (++event < event_last);
 
-    if (pending != 0)
+    if (pnd != 0)
     {
-        /* Wasn't handled */
+        /* One or more weren't handled */
         UIE_VECTOR();
     }
 }
@@ -103,21 +112,21 @@ static void gpio_call_events(enum gpio_module_number gpio)
 #if (GPIO_EVENT_MASK & USE_GPIO1_EVENTS)
 static __attribute__((interrupt("IRQ"))) void GPIO1_HANDLER(void)
 {
-    gpio_call_events(GPIO1_NUM);
+    gpio_call_events(&gpio_descs[GPIO1_NUM]);
 }
 #endif
 
 #if (GPIO_EVENT_MASK & USE_GPIO2_EVENTS)
 static __attribute__((interrupt("IRQ"))) void GPIO2_HANDLER(void)
 {
-    gpio_call_events(GPIO2_NUM);
+    gpio_call_events(&gpio_descs[GPIO2_NUM]);
 }
 #endif
 
 #if (GPIO_EVENT_MASK & USE_GPIO3_EVENTS)
 static __attribute__((interrupt("IRQ"))) void GPIO3_HANDLER(void)
 {
-    gpio_call_events(GPIO3_NUM);
+    gpio_call_events(&gpio_descs[GPIO3_NUM]);
 }
 #endif
 
@@ -140,19 +149,16 @@ void gpio_init(void)
 #endif
 }
 
-bool gpio_enable_event(enum gpio_module_number gpio, unsigned id)
+bool gpio_enable_event(enum gpio_event_ids id)
 {
-    const struct gpio_module_descriptor * const desc = &gpio_descs[gpio];
-    const struct gpio_event * const event = &desc->list->events[id];
+    const struct gpio_module_descriptor * const desc = &gpio_descs[id >> 5];
+    const struct gpio_event * const event = &desc->list->events[id & 31];
     struct gpio_map * const base = desc->base;
     volatile uint32_t *icr;
-    uint32_t mask;
+    uint32_t mask, line;
     uint32_t imr;
     int shift;
 
-    if (id >= desc->list->count)
-        return false;
-
     int oldlevel = disable_irq_save();
 
     imr =  base->imr;
@@ -160,39 +166,37 @@ bool gpio_enable_event(enum gpio_module_number gpio, unsigned id)
     if (imr == 0)
     {
         /* First enabled interrupt for this GPIO */
-        avic_enable_int(desc->ints, IRQ, desc->list->priority,
+        avic_enable_int(desc->ints, IRQ, desc->list->ints_priority,
                         desc->handler);
     }
 
     /* Set the line sense */
-    icr = &base->icr[event->line >> 4];
-    shift = (event->line & 15) << 1;
+    line = find_first_set_bit(event->mask);
+    icr = &base->icr[line >> 4];
+    shift = (line & 15) << 1;
     mask = GPIO_SENSE_CONFIG_MASK << shift;
 
     *icr = (*icr & ~mask) | ((event->sense << shift) & mask);
 
     /* Unmask the line */
-    base->imr = imr | (1ul << event->line);
+    base->imr = imr | event->mask;
 
     restore_irq(oldlevel);
 
     return true;
 }
 
-void gpio_disable_event(enum gpio_module_number gpio, unsigned id)
+void gpio_disable_event(enum gpio_event_ids id)
 {
-    const struct gpio_module_descriptor * const desc = &gpio_descs[gpio];
-    const struct gpio_event * const event = &desc->list->events[id];
+    const struct gpio_module_descriptor * const desc = &gpio_descs[id >> 5];
+    const struct gpio_event * const event = &desc->list->events[id & 31];
     struct gpio_map * const base = desc->base;
     uint32_t imr;
 
-    if (id >= desc->list->count)
-        return;
-
     int oldlevel = disable_irq_save();
 
     /* Remove bit from mask */
-    imr = base->imr & ~(1ul << event->line);
+    imr = base->imr & ~event->mask;
 
     /* Mask the line */
     base->imr = imr;
diff --git a/firmware/target/arm/imx31/gigabeat-s/gpio-imx31.h b/firmware/target/arm/imx31/gigabeat-s/gpio-imx31.h
index 34d3e72f8e..c3c3e1f63b 100644
--- a/firmware/target/arm/imx31/gigabeat-s/gpio-imx31.h
+++ b/firmware/target/arm/imx31/gigabeat-s/gpio-imx31.h
@@ -19,12 +19,12 @@
 #ifndef GPIO_IMX31_H
 #define GPIO_IMX31_H
 
-#include "gpio-target.h"
-
+/* Static registration mechanism for imx31 GPIO interrupts */
 #define USE_GPIO1_EVENTS (1 << 0)
 #define USE_GPIO2_EVENTS (1 << 1)
 #define USE_GPIO3_EVENTS (1 << 2)
 
+/* Module indexes defined by which GPIO modules are used */
 enum gpio_module_number
 {
     __GPIO_NUM_START = -1,
@@ -40,6 +40,22 @@ enum gpio_module_number
     GPIO_NUM_GPIO,
 };
 
+/* Module corresponding to the event ID is identified by range */
+enum gpio_event_bases
+{
+#if (GPIO_EVENT_MASK & USE_GPIO1_EVENTS)
+    GPIO1_EVENT_FIRST = 32*GPIO1_NUM,
+#endif
+#if (GPIO_EVENT_MASK & USE_GPIO2_EVENTS)
+    GPIO2_EVENT_FIRST = 32*GPIO2_NUM,
+#endif
+#if (GPIO_EVENT_MASK & USE_GPIO3_EVENTS)
+    GPIO3_EVENT_FIRST = 32*GPIO3_NUM,
+#endif
+};
+
+#include "gpio-target.h"
+
 /* Possible values for gpio interrupt line config */
 enum gpio_int_sense_enum
 {
@@ -54,35 +70,43 @@ enum gpio_int_sense_enum
 /* Register map for each module */
 struct gpio_map
 {
-    volatile uint32_t dr;        /* 00h */
-    volatile uint32_t gdir;      /* 04h */
-    volatile uint32_t psr;       /* 08h */
-    volatile uint32_t icr[2];    /* 0Ch */
-    volatile uint32_t imr;       /* 14h */
-    volatile uint32_t isr;       /* 18h */
+    volatile uint32_t dr;           /* 00h */
+    volatile uint32_t gdir;         /* 04h */
+    volatile uint32_t psr;          /* 08h */
+    union
+    {
+        struct
+        {
+            volatile uint32_t icr1; /* 0Ch */
+            volatile uint32_t icr2; /* 10h */
+        };
+        volatile uint32_t icr[2];   /* 0Ch */
+    };
+    volatile uint32_t imr;          /* 14h */
+    volatile uint32_t isr;          /* 18h */
 };
 
-/* Pending events will be called in array order */
+/* Pending events will be called in array order which allows easy
+ * pioritization */
 
 /* Describes a single event for a pin */
 struct gpio_event
 {
-    int line;                       /* Line number (0-31) */
+    uint32_t mask;                  /* mask: 1 << (0...31) */
     enum gpio_int_sense_enum sense; /* Type of sense */
-    int (*callback)(void);          /* Callback function (return nonzero
-                                     * to indicate this event was handled) */
+    void (*callback)(void);         /* Callback function */
 };
 
 /* Describes the events attached to a port */
 struct gpio_event_list
 {
-    int priority;                    /* Interrupt priority for this GPIO */
-    unsigned count;                  /* Count of events */
+    int ints_priority;               /* Interrupt priority for this GPIO */
+    unsigned count;                  /* Count of events for the module */
     const struct gpio_event *events; /* List of events */
 };
 
 void gpio_init(void);
-bool gpio_enable_event(enum gpio_module_number gpio, unsigned id);
-void gpio_disable_event(enum gpio_module_number gpio, unsigned id);
+bool gpio_enable_event(enum gpio_event_ids id);
+void gpio_disable_event(enum gpio_event_ids id);
 
 #endif /* GPIO_IMX31_H */
diff --git a/firmware/target/arm/imx31/gigabeat-s/gpio-target.h b/firmware/target/arm/imx31/gigabeat-s/gpio-target.h
index 46e43af28d..797f9f4552 100644
--- a/firmware/target/arm/imx31/gigabeat-s/gpio-target.h
+++ b/firmware/target/arm/imx31/gigabeat-s/gpio-target.h
@@ -21,11 +21,22 @@
 #ifndef GPIO_TARGET_H
 #define GPIO_TARGET_H
 
-#define GPIO_EVENT_MASK (USE_GPIO1_EVENTS)
-
+/* MC13783 GPIO pin info for this target */
 #define MC13783_GPIO_NUM    GPIO1_NUM   
 #define MC13783_GPIO_ISR    GPIO1_ISR
 #define MC13783_GPIO_LINE   31
-#define MC13783_EVENT_ID    0
+
+/* Declare event indexes in priority order in a packed array */
+enum gpio_event_ids
+{
+    /* GPIO1 event IDs */
+    MC13783_EVENT_ID = GPIO1_EVENT_FIRST,
+    /* GPIO2 event IDs */
+    /* none defined */
+    /* GPIO3 event IDs */
+    /* none defined */
+};
+
+void mc13783_event(void);
 
 #endif /* GPIO_TARGET_H */
diff --git a/firmware/target/arm/imx31/gigabeat-s/mc13783-gigabeat-s.c b/firmware/target/arm/imx31/gigabeat-s/mc13783-gigabeat-s.c
new file mode 100644
index 0000000000..67cfc2d886
--- /dev/null
+++ b/firmware/target/arm/imx31/gigabeat-s/mc13783-gigabeat-s.c
@@ -0,0 +1,72 @@
+/***************************************************************************
+ *             __________               __   ___.
+ *   Open      \______   \ ____   ____ |  | _\_ |__   _______  ___
+ *   Source     |       _//  _ \_/ ___\|  |/ /| __ \ /  _ \  \/  /
+ *   Jukebox    |    |   (  <_> )  \___|    < | \_\ (  <_> > <  <
+ *   Firmware   |____|_  /\____/ \___  >__|_ \|___  /\____/__/\_ \
+ *                     \/            \/     \/    \/            \/
+ * $Id$
+ *
+ * Copyright (c) 2008 by Michael Sevakis
+ *
+ * Gigabeat S MC13783 event descriptions
+ *
+ * All files in this archive are subject to the GNU General Public License.
+ * See the file COPYING in the source tree root for full license agreement.
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+ * KIND, either express or implied.
+ *
+ ****************************************************************************/
+#include "config.h"
+#include "system.h"
+#include "mc13783.h"
+#include "mc13783-target.h"
+#include "adc-target.h"
+#include "button-target.h"
+#include "usb-target.h"
+#include "power-imx31.h"
+
+/* Gigabeat S definitions for static MC13783 event registration */
+
+static const struct mc13783_event mc13783_events[] =
+{
+    [MC13783_ADCDONE_EVENT] = /* ADC conversion complete */
+    {
+        .set  = MC13783_EVENT_SET0,
+        .mask = MC13783_ADCDONEM,
+        .callback = adc_done,
+    },
+    [MC13783_ONOFD1_EVENT] = /* Power button */
+    {
+        .set  = MC13783_EVENT_SET1,
+        .mask = MC13783_ONOFD1M,
+        .callback = button_power_event,
+    },
+#ifdef HAVE_HEADPHONE_DETECTION
+    [MC13783_ONOFD2_EVENT] = /* Headphone jack */
+    {
+        .set  = MC13783_EVENT_SET1,
+        .mask = MC13783_ONOFD2M,
+        .callback = headphone_detect_event,
+    },
+#endif
+    [MC13783_CHGDET_EVENT] = /* Charger detection */
+    {
+        .set  = MC13783_EVENT_SET0,
+        .mask = MC13783_CHGDETM,
+        .callback = charger_detect_event,
+    },
+    [MC13783_USB4V4_EVENT] = /* USB insertion */
+    {
+        .set  = MC13783_EVENT_SET0,
+        .mask = MC13783_USB4V4M,
+        .callback = usb_connect_event,
+    },
+};
+
+const struct mc13783_event_list mc13783_event_list =
+{
+    .count = ARRAYLEN(mc13783_events),
+    .events = mc13783_events
+};
diff --git a/firmware/target/arm/imx31/gigabeat-s/mc13783-imx31.c b/firmware/target/arm/imx31/gigabeat-s/mc13783-imx31.c
index 4f2bd9d931..097e81d7a3 100644
--- a/firmware/target/arm/imx31/gigabeat-s/mc13783-imx31.c
+++ b/firmware/target/arm/imx31/gigabeat-s/mc13783-imx31.c
@@ -49,121 +49,86 @@ static struct spi_node mc13783_spi =
     0,                             /* SPI clock - no wait states */
 };
 
-static int mc13783_thread_stack[DEFAULT_STACK_SIZE/sizeof(int)];
+extern const struct mc13783_event_list mc13783_event_list;
+
+static int mc13783_thread_stack[3*DEFAULT_STACK_SIZE/sizeof(int)];
 static const char *mc13783_thread_name = "pmic";
 static struct wakeup mc13783_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 };
+
 #ifdef PMIC_DRIVER_CLOSE
 static bool pmic_close = false;
 static struct thread_entry *mc13783_thread_p = NULL;
 #endif
 
-/* The next two functions are rather target-specific but they'll just be left
- * here for the moment */
 static void mc13783_interrupt_thread(void)
 {
-    const unsigned char status_regs[2] =
-    {
-        MC13783_INTERRUPT_STATUS0,
-        MC13783_INTERRUPT_STATUS1,
-    };
     uint32_t pending[2];
-    uint32_t value;
-
-    mc13783_read_regset(status_regs, pending, 2);
-    mc13783_write_regset(status_regs, pending, 2);
-
-    gpio_enable_event(MC13783_GPIO_NUM, MC13783_EVENT_ID);
-
-    if (pending[1] & MC13783_TODAI) /* only needs to be polled on startup */
-        mc13783_alarm_start();
-
-    /* Check initial states for events with a sense bit */
-    value = mc13783_read(MC13783_INTERRUPT_SENSE0);
-    usb_set_status(value & MC13783_USB4V4S);
-    set_charger_inserted(value & MC13783_CHGDETS);
-
-    value = mc13783_read(MC13783_INTERRUPT_SENSE1);
-    button_power_set_state((value & MC13783_ONOFD1S) == 0);
-#ifdef HAVE_HEADPHONE_DETECTION
-    set_headphones_inserted((value & MC13783_ONOFD2S) == 0);
-#endif
 
-    pending[0] = pending[1] = 0xffffff;
-    mc13783_write_regset(status_regs, pending, 2);
+    /* Enable mc13783 GPIO event */
+    gpio_enable_event(MC13783_EVENT_ID);
 
-    /* Enable desired PMIC interrupts - some are unmasked in the drivers that
-     * handle a specific task */
-    mc13783_clear(MC13783_INTERRUPT_MASK0, MC13783_CHGDETM);
-    mc13783_clear(MC13783_INTERRUPT_MASK1, MC13783_ONOFD1M |
-                                           MC13783_ONOFD2M);
-    
     while (1)
     {
+        const struct mc13783_event *event, *event_last;
+
         wakeup_wait(&mc13783_wake, TIMEOUT_BLOCK);
 
 #ifdef PMIC_DRIVER_CLOSE
         if (pmic_close)
-        {
-            gpio_disable_event(MC13783_GPIO_NUM, MC13783_EVENT_ID);
-            return;
-        }
+            break;
 #endif
 
-        mc13783_read_regset(status_regs, pending, 2);
-        mc13783_write_regset(status_regs, pending, 2);
-
-        if (pending[0])
-        {
-            /* Handle ...PENDING0 */
-
-            /* Handle interrupts without a sense bit */
-            if (pending[0] & MC13783_ADCDONEI)
-                adc_done();
+        mc13783_read_regset(pmic_ints_regs, pending, 2);
 
-            /* Handle interrupts that have a sense bit that needs to
-             * be checked */
-            if (pending[0] & (MC13783_CHGDETI | MC13783_USB4V4I))
-            {
-                value = mc13783_read(MC13783_INTERRUPT_SENSE0);
+        /* Only clear interrupts being dispatched */
+        pending[0] &= pmic_int_enabled[0];
+        pending[1] &= pmic_int_enabled[1];
 
-                if (pending[0] & MC13783_CHGDETI)
-                    set_charger_inserted(value & MC13783_CHGDETS);
+        mc13783_write_regset(pmic_ints_regs, pending, 2);
 
-                if (pending[0] & MC13783_USB4V4I)
-                    usb_set_status(value & MC13783_USB4V4S);
-            }
-        }
+        event = mc13783_event_list.events;
+        event_last = event + mc13783_event_list.count;
 
-        if (pending[1])
+        /* .count is surely expected to be > 0 */
+        do
         {
-            /* Handle ...PENDING1 */
-
-            /* Handle interrupts without a sense bit */
-            /* ... */
+            enum mc13783_event_sets set = event->set;
+            uint32_t pnd = pending[set];
+            uint32_t mask = event->mask;
 
-            /* Handle interrupts that have a sense bit that needs to
-             * be checked */
-            if (pending[1] & (MC13783_ONOFD1I | MC13783_ONOFD2I))
+            if (pnd & mask)
             {
-                value = mc13783_read(MC13783_INTERRUPT_SENSE1);
-
-                if (pending[1] & MC13783_ONOFD1I)
-                    button_power_set_state((value & MC13783_ONOFD1S) == 0);
-#ifdef HAVE_HEADPHONE_DETECTION
-                if (pending[1] & MC13783_ONOFD2I)
-                    set_headphones_inserted((value & MC13783_ONOFD2S) == 0);
-#endif
+                event->callback();
+                pnd &= ~mask;
+                pending[set] = pnd;
             }
+
+            if ((pending[0] | pending[1]) == 0)
+                break; /* Teminate early if nothing more to service */
         }
+        while (++event < event_last);
     }
+
+#ifdef PMIC_DRIVER_CLOSE
+    gpio_disable_event(MC13783_EVENT_ID);
+#endif
 }
 
 /* GPIO interrupt handler for mc13783 */
-int mc13783_event(void)
+void mc13783_event(void)
 {
     MC13783_GPIO_ISR = (1ul << MC13783_GPIO_LINE);
     wakeup_signal(&mc13783_wake);
-    return 1; /* Yes, it's handled */
 }
 
 void mc13783_init(void)
@@ -174,8 +139,8 @@ void mc13783_init(void)
     /* Enable the PMIC SPI module */
     spi_enable_module(&mc13783_spi);
 
-    /* Mask any PMIC interrupts for now - poll initial status in thread
-     * and enable them there */
+    /* Mask any PMIC interrupts for now - modules will enable them as
+     * required */
     mc13783_write(MC13783_INTERRUPT_MASK0, 0xffffff);
     mc13783_write(MC13783_INTERRUPT_MASK1, 0xffffff);
 
@@ -203,7 +168,39 @@ void mc13783_close(void)
     wakeup_signal(&mc13783_wake);
     thread_wait(thread);
 }
-#endif
+#endif /* PMIC_DRIVER_CLOSE */
+
+bool mc13783_enable_event(enum mc13783_event_ids id)
+{
+    const struct mc13783_event * const event =
+        &mc13783_event_list.events[id];
+    int set = event->set;
+    uint32_t mask = event->mask;
+
+    spi_lock(&mc13783_spi);
+
+    pmic_int_enabled[set] |= mask;
+    mc13783_clear(pmic_intm_regs[set], mask);
+
+    spi_unlock(&mc13783_spi);
+
+    return true;
+}
+
+void mc13783_disable_event(enum mc13783_event_ids id)
+{
+    const struct mc13783_event * const event =
+        &mc13783_event_list.events[id];
+    int set = event->set;
+    uint32_t mask = event->mask;
+
+    spi_lock(&mc13783_spi);
+
+    pmic_int_enabled[set] &= ~mask;
+    mc13783_set(pmic_intm_regs[set], mask);
+
+    spi_unlock(&mc13783_spi);
+}
 
 uint32_t mc13783_set(unsigned address, uint32_t bits)
 {
diff --git a/firmware/target/arm/imx31/gigabeat-s/mc13783-target.h b/firmware/target/arm/imx31/gigabeat-s/mc13783-target.h
new file mode 100644
index 0000000000..a74a229f04
--- /dev/null
+++ b/firmware/target/arm/imx31/gigabeat-s/mc13783-target.h
@@ -0,0 +1,36 @@
+/***************************************************************************
+ *             __________               __   ___.
+ *   Open      \______   \ ____   ____ |  | _\_ |__   _______  ___
+ *   Source     |       _//  _ \_/ ___\|  |/ /| __ \ /  _ \  \/  /
+ *   Jukebox    |    |   (  <_> )  \___|    < | \_\ (  <_> > <  <
+ *   Firmware   |____|_  /\____/ \___  >__|_ \|___  /\____/__/\_ \
+ *                     \/            \/     \/    \/            \/
+ * $Id$
+ *
+ * Copyright (c) 2008 by Michael Sevakis
+ *
+ * Gigabeat S mc13783 event descriptions
+ *
+ * All files in this archive are subject to the GNU General Public License.
+ * See the file COPYING in the source tree root for full license agreement.
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+ * KIND, either express or implied.
+ *
+ ****************************************************************************/
+#ifndef MC13783_TARGET_H
+#define MC13783_TARGET_H
+
+/* Declare event indexes in priority order in a packed array */
+enum mc13783_event_ids
+{
+    MC13783_ADCDONE_EVENT = 0, /* ADC conversion complete */
+    MC13783_ONOFD1_EVENT,      /* Power button */
+#ifdef HAVE_HEADPHONE_DETECTION
+    MC13783_ONOFD2_EVENT,      /* Headphone jack */
+#endif
+    MC13783_CHGDET_EVENT,      /* Charger detection */
+    MC13783_USB4V4_EVENT,      /* USB insertion */
+};
+
+#endif /* MC13783_TARGET_H */
diff --git a/firmware/target/arm/imx31/gigabeat-s/power-imx31.c b/firmware/target/arm/imx31/gigabeat-s/power-imx31.c
index 53cdb7c315..8db59bdf0e 100644
--- a/firmware/target/arm/imx31/gigabeat-s/power-imx31.c
+++ b/firmware/target/arm/imx31/gigabeat-s/power-imx31.c
@@ -28,14 +28,11 @@
 
 static bool charger_detect = false;
 
-void power_init(void)
-{
-}
-
 /* This is called from the mc13783 interrupt thread */
-void set_charger_inserted(bool inserted)
+void charger_detect_event(void)
 {
-    charger_detect = inserted;
+    charger_detect =
+        mc13783_read(MC13783_INTERRUPT_SENSE0) & MC13783_CHGDETS;
 }
 
 bool charger_inserted(void)
@@ -81,6 +78,15 @@ void power_off(void)
     while (1);
 }
 
+void power_init(void)
+{
+    /* Poll initial state */
+    charger_detect_event();
+
+    /* Enable detect event */
+    mc13783_enable_event(MC13783_CHGDET_EVENT);
+}
+
 #else /* SIMULATOR */
 
 bool charger_inserted(void)
diff --git a/firmware/target/arm/imx31/gigabeat-s/power-imx31.h b/firmware/target/arm/imx31/gigabeat-s/power-imx31.h
index bb0a7614d8..b949d41d35 100644
--- a/firmware/target/arm/imx31/gigabeat-s/power-imx31.h
+++ b/firmware/target/arm/imx31/gigabeat-s/power-imx31.h
@@ -19,6 +19,6 @@
 #ifndef POWER_IMX31_H
 #define POWER_IMX31_H
 
-void set_charger_inserted(bool inserted);
+void charger_detect_event(void);
 
 #endif /* POWER_IMX31_H */
diff --git a/firmware/target/arm/imx31/gigabeat-s/usb-imx31.c b/firmware/target/arm/imx31/gigabeat-s/usb-imx31.c
index f12fd8f0b1..ad6c77138c 100644
--- a/firmware/target/arm/imx31/gigabeat-s/usb-imx31.c
+++ b/firmware/target/arm/imx31/gigabeat-s/usb-imx31.c
@@ -48,9 +48,10 @@ static void enable_transceiver(bool enable)
     }
 }
 
-void usb_set_status(bool plugged)
+void usb_connect_event(void)
 {
-    usb_status = plugged ? USB_INSERTED : USB_EXTRACTED;
+    uint32_t status = mc13783_read(MC13783_INTERRUPT_SENSE0);    
+    usb_status = (status & MC13783_USB4V4S) ? USB_INSERTED : USB_EXTRACTED;
 }
 
 int usb_detect(void)
@@ -73,7 +74,11 @@ void usb_init_device(void)
     /* Module will be turned off later after firmware init */
     usb_drv_startup();
 
-    mc13783_clear(MC13783_INTERRUPT_MASK0, MC13783_USB4V4M);
+    /* Initially poll */
+    usb_connect_event();
+
+    /* Enable PMIC event */
+    mc13783_enable_event(MC13783_USB4V4_EVENT);
 }
 
 void usb_enable(bool on)
diff --git a/firmware/target/arm/imx31/gigabeat-s/usb-target.h b/firmware/target/arm/imx31/gigabeat-s/usb-target.h
index c6e2850b9d..0af740ac7c 100644
--- a/firmware/target/arm/imx31/gigabeat-s/usb-target.h
+++ b/firmware/target/arm/imx31/gigabeat-s/usb-target.h
@@ -23,7 +23,7 @@
 #define USB_DRIVER_CLOSE
 #endif
 
-void usb_set_status(bool plugged);
+void usb_connect_event(void);
 bool usb_init_device(void);
 int usb_detect(void);
 /* Read the immediate state of the cable from the PMIC */