Ambiq Apollo3: Make SFW_ARTEMIS use arduino uno form factor, fix GPIO IRQ API

Author: multiplemonomials

hal/include/hal/gpio_irq_api.h

@@ -49,6 +49,7 @@ typedef void (*gpio_irq_handler)(uintptr_t context, gpio_irq_event event);
  * # Defined behavior
  * * ::gpio_irq_init initializes the GPIO IRQ pin
  * * ::gpio_irq_init attaches the interrupt handler
+ * * ::gpio_irq_init enables the IRQ
  * * ::gpio_irq_free releases the GPIO IRQ pin
  * * ::gpio_irq_set enables/disables pin IRQ event
  * * ::gpio_irq_enable enables GPIO IRQ

targets/TARGET_Ambiq_Micro/TARGET_Apollo3/TARGET_SFE_ARTEMIS/PinNames.h

@@ -39,68 +39,82 @@ extern "C"
 typedef enum
 {
     // Digital naming
-    D0 = 25,
-    D1 = 24,
-    D2 = 35,
-    D3 = 4,
-    D4 = 22,
-    D5 = 23,
-    D6 = 27,
-    D7 = 28,
-    D8 = 32,
-    D9 = 12,
-    D10 = 13,
-    D11 = 7,
-    D12 = 6,
-    D13 = 5,
-    D14 = 40,
-    D15 = 39,
-    D16 = 29,
-    D17 = 11,
-    D18 = 34,
-    D19 = 33,
-    D20 = 16,
-    D21 = 31,
-
-    // Analog naming
-    A0 = D16,
-    A1 = D17,
-    A2 = D18,
-    A3 = D19,
-    A4 = D20,
-    A5 = D21,
-    A6 = D2,
-    // A7 = ??
-    A8 = D8,
-    A9 = D9,
-    A10 = D10,
+    p0 = 25,
+    p1 = 24,
+    p2 = 35,
+    p3 = 4,
+    p4 = 22,
+    p5 = 23,
+    p6 = 27,
+    p7 = 28,
+    p8 = 32,
+    p9 = 12,
+    p10 = 13,
+    p11 = 7,
+    p12 = 6,
+    p13 = 5,
+    p14 = 40,
+    p15 = 39,
+    p16 = 29,
+    p17 = 11,
+    p18 = 34,
+    p19 = 33,
+    p20 = 16,
+    p21 = 31,
+
+#ifdef TARGET_FF_ARDUINO_UNO
+    // Arduino form factor pins
+    ARDUINO_UNO_D0 = p0,
+    ARDUINO_UNO_D1 = p1,
+    ARDUINO_UNO_D2 = p2,
+    ARDUINO_UNO_D3 = p3,
+    ARDUINO_UNO_D4 = p4,
+    ARDUINO_UNO_D5 = p5,
+    ARDUINO_UNO_D6 = p6,
+    ARDUINO_UNO_D7 = p7,
+    ARDUINO_UNO_D8 = p8,
+    ARDUINO_UNO_D9 = p9,
+    ARDUINO_UNO_D10 = p10,
+    ARDUINO_UNO_D11 = p11,
+    ARDUINO_UNO_D12 = p12,
+    ARDUINO_UNO_D13 = p13,
+    ARDUINO_UNO_D14 = p14,
+    ARDUINO_UNO_D15 = p15,
+
+    ARDUINO_UNO_A0 = p16,
+    ARDUINO_UNO_A1 = p17,
+    ARDUINO_UNO_A2 = p18,
+    ARDUINO_UNO_A3 = p19,
+    ARDUINO_UNO_A4 = p20,
+    ARDUINO_UNO_A5 = p21,
+    ARDUINO_UNO_A6 = p2,
+#endif
 
     // UART
     SERIAL_TX = AM_BSP_PRIM_UART_TX_PIN,
     SERIAL_RX = AM_BSP_PRIM_UART_RX_PIN,
     CONSOLE_TX = SERIAL_TX,
     CONSOLE_RX = SERIAL_RX,
 
-    SERIAL1_TX = D1,
-    SERIAL1_RX = D0,
+    SERIAL1_TX = p1,
+    SERIAL1_RX = p0,
 
     // Not connected
     NC = NC_VAL
 } PinName;
 
 // LEDs
-#define LED1 D13 // Blue LED
+#define LED1 p13 // Blue LED
 
 // I2C bus
-#define I2C_SCL D15
-#define I2C_SDA D14
+// note: I2C_SCL and I2C_SDA defines are provided by the FF_ARDUINO_UNO header
 #define QWIIC_SCL I2C_SCL
 #define QWIIC_SDA I2C_SDA
 
 // SPI bus
-#define SPI_CLK D13
-#define SPI_SDO D11
-#define SPI_SDI D12
+#define SPI_CLK p13
+#define SPI_SDO p11
+#define SPI_SDI p12
 
 #if defined(MBED_CONF_TARGET_STDIO_UART_TX)
 #define STDIO_UART_TX MBED_CONF_TARGET_STDIO_UART_TX

targets/TARGET_Ambiq_Micro/TARGET_Apollo3/device/gpio_irq_api.c

@@ -31,6 +31,7 @@ extern "C"
 {
 #endif
 
+static void ap3_gpio_update_int_en(gpio_irq_t *obj);
 uint32_t ap3_gpio_enable_interrupts(uint32_t ui32Pin, am_hal_gpio_intdir_e eIntDir);
 /** GPIO IRQ HAL structure. gpio_irq_s is declared in the target's HAL
 */
@@ -70,20 +71,19 @@ int gpio_irq_init(gpio_irq_t *obj, PinName pin, gpio_irq_handler handler, uintpt
     //grab the correct irq control object
     ap3_gpio_irq_control_t *control = &gpio_irq_control[pin];
 
-    //Register locally
+    // Register locally
     control->pad = pin;
     control->handler = handler;
     control->id = context;
     control->events = IRQ_NONE;
 
-    //Attach to object
+    // Attach to object
     obj->control = control;
 
-    //Make sure the interrupt is set to none to reflect the new events value
-    ap3_gpio_enable_interrupts(control->pad, AM_HAL_GPIO_PIN_INTDIR_NONE);
+    // Start with the IRQ "enabled", but don't actually enable it till something is attached
+    obj->irq_requested_enabled = true;
 
-    //Enable GPIO IRQ's in the NVIC
-    gpio_irq_enable(obj);
+    // Make sure the GPIO IRQ is enabled in NVIC
     NVIC_SetVector((IRQn_Type)GPIO_IRQn, (uint32_t)am_gpio_isr);
     NVIC_EnableIRQ((IRQn_Type)GPIO_IRQn);
     return 0;
@@ -99,7 +99,18 @@ void am_gpio_isr(void)
         if (gpio_int_mask & 0x0000000000000001) {
             am_hal_gpio_interrupt_clear(AM_HAL_GPIO_BIT(pinNum));
             ap3_gpio_irq_control_t irq_ctrl = gpio_irq_control[pinNum];
-            ((gpio_irq_handler)irq_ctrl.handler)(irq_ctrl.id, irq_ctrl.events);
+
+            uint8_t event = irq_ctrl.events;
+            if(event == (IRQ_RISE | IRQ_FALL))
+            {
+                // This pin is configured for both rise and fall events. However, this MCU does not have separate
+                // status registers for tracking rising/falling interrupts.
+                // So, read the pin to figure out which interrupt happened. It's not totally foolproof but
+                // it should work in most cases.
+                event = am_hal_gpio_input_read(irq_ctrl.pad) ? IRQ_RISE : IRQ_FALL;
+            }
+
+            ((gpio_irq_handler)irq_ctrl.handler)(irq_ctrl.id, event);
         }
         gpio_int_mask >>= 1;
         pinNum++;
@@ -112,6 +123,8 @@ void am_gpio_isr(void)
 */
 void gpio_irq_free(gpio_irq_t *obj)
 {
+    // Make sure interrupt can't trigger
+    gpio_irq_disable(obj);
 }
 
 /** Enable/disable pin IRQ event
@@ -121,20 +134,21 @@ void gpio_irq_free(gpio_irq_t *obj)
 * @param enable The enable flag
 */
 void gpio_irq_set(gpio_irq_t *obj, gpio_irq_event event, uint32_t enable)
-{
-    //Clear state
-    obj->control->events &= (~event);
+{    
     if (enable) {
-        //Reset if enabled
         obj->control->events |= event;
     }
+    else {
+        obj->control->events &= ~event;
+    }
 
-    // Map enabled events to a value the reflects the ambiq hal/register values
+    // Map enabled events to a value the reflects the ambiq hal/register values.
+    // Note that we don't want to actually set INTDIR_NONE, because this disables reading the pin (!!)
+    // So instead, if asked to disable the IRQ, we leave LO2HIGH interrupt enabled in the PINCFG register but
+    // don't enable the interrupt for this pin in the register
     am_hal_gpio_intdir_e ap3_int_dir = 0x00;
     switch (obj->control->events) {
         case IRQ_NONE:
-            ap3_int_dir = AM_HAL_GPIO_PIN_INTDIR_NONE;
-            break;
         case IRQ_RISE:
             ap3_int_dir = AM_HAL_GPIO_PIN_INTDIR_LO2HI;
             break;
@@ -146,7 +160,17 @@ void gpio_irq_set(gpio_irq_t *obj, gpio_irq_event event, uint32_t enable)
             break;
     }
 
+    // If switching to NONE, disable the IRQ first
+    if(obj->control->events == IRQ_NONE) {
+        ap3_gpio_update_int_en(obj);
+    }
+
     ap3_gpio_enable_interrupts(obj->control->pad, ap3_int_dir);
+
+    // Otherwise enable IRQ now
+    if(obj->control->events != IRQ_NONE) {
+        ap3_gpio_update_int_en(obj);
+    }
 }
 
 /** Enable GPIO IRQ
@@ -156,8 +180,8 @@ void gpio_irq_set(gpio_irq_t *obj, gpio_irq_event event, uint32_t enable)
 */
 void gpio_irq_enable(gpio_irq_t *obj)
 {
-    am_hal_gpio_interrupt_clear(AM_HAL_GPIO_BIT(obj->control->pad));
-    am_hal_gpio_interrupt_enable(AM_HAL_GPIO_BIT(obj->control->pad));
+    obj->irq_requested_enabled = true;
+    ap3_gpio_update_int_en(obj);
 }
 
 /** Disable GPIO IRQ
@@ -167,11 +191,27 @@ void gpio_irq_enable(gpio_irq_t *obj)
 */
 void gpio_irq_disable(gpio_irq_t *obj)
 {
-    am_hal_gpio_interrupt_clear(AM_HAL_GPIO_BIT(obj->control->pad));
-    am_hal_gpio_interrupt_disable(AM_HAL_GPIO_BIT(obj->control->pad));
+    obj->irq_requested_enabled = false;
+    ap3_gpio_update_int_en(obj);
 }
 
 /**@}*/
+
+// Based on the enabled events and irq_requested_enabled, enable or disable the IRQ
+static void ap3_gpio_update_int_en(gpio_irq_t *obj)
+{
+    if(obj->irq_requested_enabled && obj->control->events != IRQ_NONE) {
+        // Enable!
+        am_hal_gpio_interrupt_clear(AM_HAL_GPIO_BIT(obj->control->pad));
+        am_hal_gpio_interrupt_enable(AM_HAL_GPIO_BIT(obj->control->pad));
+    }
+    else {
+        // Disable
+        am_hal_gpio_interrupt_disable(AM_HAL_GPIO_BIT(obj->control->pad));
+    }
+}
+
+
 uint32_t ap3_gpio_enable_interrupts(uint32_t ui32Pin, am_hal_gpio_intdir_e eIntDir)
 {
     uint32_t ui32Padreg, ui32AltPadCfg, ui32GPCfg;
@@ -208,7 +248,8 @@ uint32_t ap3_gpio_enable_interrupts(uint32_t ui32Pin, am_hal_gpio_intdir_e eIntD
 
     ui32GPCfgShft = ((ui32Pin & 0x7) << 2);
 
-    ui32GPCfgAddr = AM_REGADDR(GPIO, CFGA) + ((ui32Pin >> 1) & ~0x3);
+    // 8 pins per register, and each register is 32 bits wide
+    ui32GPCfgAddr = AM_REGADDR(GPIO, CFGA) + (ui32Pin / 8) * sizeof(uint32_t);
 
     ui32GPCfgClearMask = ~((uint32_t)0xF << ui32GPCfgShft);
 

targets/TARGET_Ambiq_Micro/TARGET_Apollo3/device/objects_gpio.h

@@ -27,6 +27,8 @@
 #include "am_hal_gpio.h"
 #include "PinNames.h"
 
+#include <stdbool.h>
+
 #ifdef __cplusplus
 extern "C"
 {
@@ -124,6 +126,7 @@ typedef struct ap3_gpio_irq_control_t {
 
 typedef struct gpio_irq_s {
     ap3_gpio_irq_control_t *control;
+    bool irq_requested_enabled;
 } gpio_irq_s;
 
 #define AP3_PINCFG_FUNCSEL_GPIO 3

targets/targets.json5

@@ -9665,7 +9665,10 @@ mode is recommended for target MCUs with small amounts of flash and RAM.",
     },
     "SFE_ARTEMIS": { // AKA SparkFun RedBoard Artemis
         "inherits": ["AMA3B1KK"],
-        "image_url": "https://cdn.sparkfun.com/r/455-455/assets/parts/1/4/0/1/9/15444-SparkFun_RedBoard_Artemis-01.jpg"
+        "image_url": "https://cdn.sparkfun.com/r/455-455/assets/parts/1/4/0/1/9/15444-SparkFun_RedBoard_Artemis-01.jpg",
+        "supported_form_factors": [
+            "ARDUINO_UNO"
+        ]
     },
     "SFE_ARTEMIS_ATP": { // AKA SparkFun RedBoard Artemis ATP
         "inherits": ["AMA3B1KK"],