Merge branch 'bugfix/fix_gpio_intr_lost' into 'master'

gpio: Fix interrupt lost issue

Closes IDFGH-5069

See merge request espressif/esp-idf!19625

components/driver/gpio/gpio.c

@@ -53,6 +53,7 @@ typedef struct {
     uint32_t isr_core_id;
     gpio_isr_func_t *gpio_isr_func;
     gpio_isr_handle_t gpio_isr_handle;
+    uint64_t isr_clr_on_entry_mask; // for edge-triggered interrupts, interrupt status bits should be cleared before entering per-pin handlers
 } gpio_context_t;
 
 static gpio_hal_context_t _gpio_hal = {
@@ -64,6 +65,7 @@ static gpio_context_t gpio_context = {
     .gpio_spinlock = portMUX_INITIALIZER_UNLOCKED,
     .isr_core_id = GPIO_ISR_CORE_ID_UNINIT,
     .gpio_isr_func = NULL,
+    .isr_clr_on_entry_mask = 0,
 };
 
 esp_err_t gpio_pullup_en(gpio_num_t gpio_num)
@@ -149,13 +151,17 @@ esp_err_t gpio_set_intr_type(gpio_num_t gpio_num, gpio_int_type_t intr_type)
 
     portENTER_CRITICAL(&gpio_context.gpio_spinlock);
     gpio_hal_set_intr_type(gpio_context.gpio_hal, gpio_num, intr_type);
+    if (intr_type == GPIO_INTR_POSEDGE || intr_type == GPIO_INTR_NEGEDGE || intr_type == GPIO_INTR_ANYEDGE) {
+        gpio_context.isr_clr_on_entry_mask |= (1ULL << (gpio_num));
+    } else {
+        gpio_context.isr_clr_on_entry_mask &= ~(1ULL << (gpio_num));
+    }
     portEXIT_CRITICAL(&gpio_context.gpio_spinlock);
     return ESP_OK;
 }
 
 static esp_err_t gpio_intr_enable_on_core(gpio_num_t gpio_num, uint32_t core_id)
 {
-    GPIO_CHECK(GPIO_IS_VALID_GPIO(gpio_num), "GPIO number error", ESP_ERR_INVALID_ARG);
     gpio_hal_intr_enable_on_core(gpio_context.gpio_hal, gpio_num, core_id);
     return ESP_OK;
 }
@@ -412,9 +418,21 @@ static inline void IRAM_ATTR gpio_isr_loop(uint32_t status, const uint32_t gpio_
         status &= ~(1 << nbit);
         int gpio_num = gpio_num_start + nbit;
 
+        bool intr_status_bit_cleared = false;
+        // Edge-triggered type interrupt can clear the interrupt status bit before entering per-pin interrupt handler
+        if ((1ULL << (gpio_num)) & gpio_context.isr_clr_on_entry_mask) {
+            intr_status_bit_cleared = true;
+            gpio_hal_clear_intr_status_bit(gpio_context.gpio_hal, gpio_num);
+        }
+
         if (gpio_context.gpio_isr_func[gpio_num].fn != NULL) {
             gpio_context.gpio_isr_func[gpio_num].fn(gpio_context.gpio_isr_func[gpio_num].args);
         }
+
+        // If the interrupt status bit was not cleared at the entry, then must clear it before exiting
+        if (!intr_status_bit_cleared) {
+            gpio_hal_clear_intr_status_bit(gpio_context.gpio_hal, gpio_num);
+        }
     }
 }
 
@@ -431,7 +449,6 @@ static void IRAM_ATTR gpio_intr_service(void *arg)
 
     if (gpio_intr_status) {
         gpio_isr_loop(gpio_intr_status, 0);
-        gpio_hal_clear_intr_status(gpio_context.gpio_hal, gpio_intr_status);
     }
 
     //read status1 to get interrupt status for GPIO32-39
@@ -440,7 +457,6 @@ static void IRAM_ATTR gpio_intr_service(void *arg)
 
     if (gpio_intr_status_h) {
         gpio_isr_loop(gpio_intr_status_h, 32);
-        gpio_hal_clear_intr_status_high(gpio_context.gpio_hal, gpio_intr_status_h);
     }
 }
 

components/driver/include/driver/gpio.h

@@ -29,6 +29,24 @@ extern "C" {
 
 typedef intr_handle_t gpio_isr_handle_t;
 
+/**
+ * @brief GPIO interrupt handler
+ *
+ * @param arg User registered data
+ */
+typedef void (*gpio_isr_t)(void *arg);
+
+/**
+ * @brief Configuration parameters of GPIO pad for gpio_config function
+ */
+typedef struct {
+    uint64_t pin_bit_mask;          /*!< GPIO pin: set with bit mask, each bit maps to a GPIO */
+    gpio_mode_t mode;               /*!< GPIO mode: set input/output mode                     */
+    gpio_pullup_t pull_up_en;       /*!< GPIO pull-up                                         */
+    gpio_pulldown_t pull_down_en;   /*!< GPIO pull-down                                       */
+    gpio_int_type_t intr_type;      /*!< GPIO interrupt type                                  */
+} gpio_config_t;
+
 /**
  * @brief GPIO common configuration
  *
@@ -255,7 +273,7 @@ esp_err_t gpio_pulldown_en(gpio_num_t gpio_num);
 esp_err_t gpio_pulldown_dis(gpio_num_t gpio_num);
 
 /**
-  * @brief Install the driver's GPIO ISR handler service, which allows per-pin GPIO interrupt handlers.
+  * @brief Install the GPIO driver's ETS_GPIO_INTR_SOURCE ISR handler service, which allows per-pin GPIO interrupt handlers.
   *
   * This function is incompatible with gpio_isr_register() - if that function is used, a single global ISR is registered for all GPIO interrupts. If this function is used, the ISR service provides a global GPIO ISR and individual pin handlers are registered via the gpio_isr_handler_add() function.
   *

components/hal/include/hal/gpio_hal.h

@@ -95,20 +95,13 @@ typedef struct {
 #define gpio_hal_get_intr_status_high(hal, core_id, status) gpio_ll_get_intr_status_high((hal)->dev, core_id, status)
 
 /**
-  * @brief Clear GPIO interrupt status
-  *
-  * @param hal Context of the HAL layer
-  * @param mask interrupt status clear mask
-  */
-#define gpio_hal_clear_intr_status(hal, mask) gpio_ll_clear_intr_status((hal)->dev, mask)
-
-/**
-  * @brief Clear GPIO interrupt status high
-  *
-  * @param hal Context of the HAL layer
-  * @param mask interrupt status high clear mask
-  */
-#define gpio_hal_clear_intr_status_high(hal, mask) gpio_ll_clear_intr_status_high((hal)->dev, mask)
+ * @brief Clear GPIO interrupt status bit
+ *
+ * @param hal Context of the HAL layer
+ * @param gpio_num GPIO number. If you want to clear the interrupt status bit of e.g. GPIO16, gpio_num should be GPIO_NUM_16 (16);
+ */
+#define gpio_hal_clear_intr_status_bit(hal, gpio_num) (((gpio_num) < 32) ? gpio_ll_clear_intr_status((hal)->dev, 1 << gpio_num) \
+                                                                         : gpio_ll_clear_intr_status_high((hal)->dev, 1 << (gpio_num - 32)))
 
 /**
  * @brief  Enable GPIO module interrupt signal

components/hal/include/hal/gpio_types.h

@@ -423,17 +423,6 @@ typedef enum {
     GPIO_PULLDOWN_ENABLE = 0x1,    /*!< Enable GPIO pull-down resistor  */
 } gpio_pulldown_t;
 
-/**
- * @brief Configuration parameters of GPIO pad for gpio_config function
- */
-typedef struct {
-    uint64_t pin_bit_mask;          /*!< GPIO pin: set with bit mask, each bit maps to a GPIO */
-    gpio_mode_t mode;               /*!< GPIO mode: set input/output mode                     */
-    gpio_pullup_t pull_up_en;       /*!< GPIO pull-up                                         */
-    gpio_pulldown_t pull_down_en;   /*!< GPIO pull-down                                       */
-    gpio_int_type_t intr_type;      /*!< GPIO interrupt type                                  */
-} gpio_config_t;
-
 typedef enum {
     GPIO_PULLUP_ONLY,               /*!< Pad pull up            */
     GPIO_PULLDOWN_ONLY,             /*!< Pad pull down          */
@@ -450,8 +439,6 @@ typedef enum {
     GPIO_DRIVE_CAP_MAX,
 } gpio_drive_cap_t;
 
-typedef void (*gpio_isr_t)(void *);
-
 #ifdef __cplusplus
 }
 #endif

tools/mocks/hal/include/hal/gpio_types.h

@@ -95,17 +95,6 @@ typedef enum {
     GPIO_PULLDOWN_ENABLE = 0x1,    /*!< Enable GPIO pull-down resistor  */
 } gpio_pulldown_t;
 
-/**
- * @brief Configuration parameters of GPIO pad for gpio_config function
- */
-typedef struct {
-    uint64_t pin_bit_mask;          /*!< GPIO pin: set with bit mask, each bit maps to a GPIO */
-    gpio_mode_t mode;               /*!< GPIO mode: set input/output mode                     */
-    gpio_pullup_t pull_up_en;       /*!< GPIO pull-up                                         */
-    gpio_pulldown_t pull_down_en;   /*!< GPIO pull-down                                       */
-    gpio_int_type_t intr_type;      /*!< GPIO interrupt type                                  */
-} gpio_config_t;
-
 typedef enum {
     GPIO_PULLUP_ONLY,               /*!< Pad pull up            */
     GPIO_PULLDOWN_ONLY,             /*!< Pad pull down          */
@@ -122,8 +111,6 @@ typedef enum {
     GPIO_DRIVE_CAP_MAX,
 } gpio_drive_cap_t;
 
-typedef void (*gpio_isr_t)(void *);
-
 #ifdef __cplusplus
 }
 #endif