alex/esp-idf
alex/esp-idf
1
0
Fork 0
mirror of https://github.com/espressif/esp-idf.git synced 2024-10-05 20:47:46 -04:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

gpio: Fix interrupt lost issue

Browse Source 
In previous gpio default isr, interrupt status bits get cleared at the exit of the isr.
However, for edge-triggered interrupt type, the interrupt status bit should be cleared before entering the per-pin handlers to avoid any potential interrupt lost.

Closes https://github.com/espressif/esp-idf/pull/6853
This commit is contained in:
Song Ruo Jing 2022-08-24 17:06:20 +08:00
parent 7568139778
commit 3017b65d4d
6 changed files with 52 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
components/driver/gpio/gpio.c
View File

@ -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);
}
}

20
components/driver/include/driver/gpio.h
View File

@ -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.
*

21
components/hal/include/hal/gpio_hal.h
View File

@ -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

13
components/hal/include/hal/gpio_types.h
View File

@ -386,17 +386,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 */
@ -413,8 +402,6 @@ typedef enum {
GPIO_DRIVE_CAP_MAX,
} gpio_drive_cap_t;
typedef void (*gpio_isr_t)(void *);
#ifdef __cplusplus
}
#endif

8
docs/en/migration-guides/release-5.x/peripherals.rst
View File

@ -31,7 +31,13 @@ ADC
GPIO
----
The previous Kconfig option `RTCIO_SUPPORT_RTC_GPIO_DESC` has been removed, thus the ``rtc_gpio_desc`` array is unavailable. Please use ``rtc_io_desc`` array instead.
- The previous Kconfig option `RTCIO_SUPPORT_RTC_GPIO_DESC` has been removed, thus the ``rtc_gpio_desc`` array is unavailable. Please use ``rtc_io_desc`` array instead.
- GPIO interrupt users callbacks should no longer read the GPIO interrupt status register to get the triggered GPIO's pin number. Users should use the callback argument to determine the GPIO's pin number instead.
- Previously, when a GPIO interrupt occurs, the GPIO's interrupt status register is cleared after calling the user callbacks. Thus, it was possible for users to read the GPIO's interrupt status register inside the callback to determine which GPIO was triggered.
- However, clearing the interrupt status after the user callbacks can potentially cause edge-triggered interrupts to be lost. For example, if an edge-triggered interrupt (re)triggers while the user callbacks are being called, that interrupt will be cleared without its registered user callback being handled.
- Now, the GPIO's interrupt status register is cleared **before** invoking the user callbacks. Thus, users can no longer read the GPIO interrupt status register to determine which pin has triggered the interrupt. Instead, users should use the callback argument to pass the pin number.
.. only:: SOC_SDM_SUPPORTED

13
tools/mocks/hal/include/hal/gpio_types.h
View File

@ -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