Crash_Override/OVMS3-idf
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Merge branch 'bugfix/gpio_intr_enable_bug_v3.3' into 'release/v3.3'

Browse source 
bugfix(GPIO):  Fixed GPIO interrupt bug for v3.3

See merge request espressif/esp-idf!5650
This commit is contained in:
Angus Gratton 2019-08-12 13:00:15 +08:00
parent 3be1c70d46 0929dbbc9b
commit f2f5a237c0
2 changed files with 173 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

57
components/driver/gpio.c
View file

@ -22,21 +22,34 @@
#include "soc/soc.h"
#include "esp_log.h"
#include "soc/gpio_periph.h"
#include "esp_ipc.h"
static const char* GPIO_TAG = "gpio";
#define GPIO_CHECK(a, str, ret_val) \
if (!(a)) { \
ESP_LOGE(GPIO_TAG,"%s(%d): %s", __FUNCTION__, __LINE__, str); \
return (ret_val); \
}
#define GPIO_ISR_CORE_ID_UNINIT (3)
typedef struct {
gpio_isr_t fn; /*!< isr function */
void* args; /*!< isr function args */
} gpio_isr_func_t;
// Used by the IPC call to register the interrupt service routine.
typedef struct {
int source; /*!< ISR source */
int intr_alloc_flags; /*!< ISR alloc flag */
void (*fn)(void*); /*!< ISR function */
void *arg; /*!< ISR function args*/
void *handle; /*!< ISR handle */
esp_err_t ret;
} gpio_isr_alloc_t;
static const char* GPIO_TAG = "gpio";
static gpio_isr_func_t* gpio_isr_func = NULL;
static gpio_isr_handle_t gpio_isr_handle;
static uint32_t isr_core_id = GPIO_ISR_CORE_ID_UNINIT;
static portMUX_TYPE gpio_spinlock = portMUX_INITIALIZER_UNLOCKED;
esp_err_t gpio_pullup_en(gpio_num_t gpio_num)
@ -102,7 +115,6 @@ static void gpio_intr_status_clr(gpio_num_t gpio_num)
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_intr_status_clr(gpio_num);
if (core_id == 0) {
GPIO.pin[gpio_num].int_ena = GPIO_PRO_CPU_INTR_ENA; //enable pro cpu intr
@ -114,7 +126,13 @@ static esp_err_t gpio_intr_enable_on_core (gpio_num_t gpio_num, uint32_t core_id
esp_err_t gpio_intr_enable(gpio_num_t gpio_num)
{
return gpio_intr_enable_on_core (gpio_num, xPortGetCoreID());
GPIO_CHECK(GPIO_IS_VALID_GPIO(gpio_num), "GPIO number error", ESP_ERR_INVALID_ARG);
portENTER_CRITICAL(&gpio_spinlock);
if(isr_core_id == GPIO_ISR_CORE_ID_UNINIT) {
isr_core_id = xPortGetCoreID();
}
portEXIT_CRITICAL(&gpio_spinlock);
return gpio_intr_enable_on_core (gpio_num, isr_core_id);
}
esp_err_t gpio_intr_disable(gpio_num_t gpio_num)
@ -332,11 +350,9 @@ void IRAM_ATTR gpio_intr_service(void* arg)
//GPIO intr process
uint32_t gpio_num = 0;
//read status to get interrupt status for GPIO0-31
uint32_t gpio_intr_status;
gpio_intr_status = GPIO.status;
const uint32_t gpio_intr_status = (isr_core_id == 0) ? GPIO.pcpu_int : GPIO.acpu_int;
//read status1 to get interrupt status for GPIO32-39
uint32_t gpio_intr_status_h;
gpio_intr_status_h = GPIO.status1.intr_st;
const uint32_t gpio_intr_status_h = (isr_core_id == 0) ? GPIO.pcpu_int1.intr : GPIO.acpu_int1.intr;
if (gpio_isr_func == NULL) {
return;
@ -395,12 +411,12 @@ esp_err_t gpio_install_isr_service(int intr_alloc_flags)
esp_err_t ret;
portENTER_CRITICAL(&gpio_spinlock);
gpio_isr_func = (gpio_isr_func_t*) calloc(GPIO_NUM_MAX, sizeof(gpio_isr_func_t));
portEXIT_CRITICAL(&gpio_spinlock);
if (gpio_isr_func == NULL) {
ret = ESP_ERR_NO_MEM;
} else {
ret = gpio_isr_register(gpio_intr_service, NULL, intr_alloc_flags, &gpio_isr_handle);
}
portEXIT_CRITICAL(&gpio_spinlock);
return ret;
}
@ -413,14 +429,37 @@ void gpio_uninstall_isr_service()
esp_intr_free(gpio_isr_handle);
free(gpio_isr_func);
gpio_isr_func = NULL;
isr_core_id = GPIO_ISR_CORE_ID_UNINIT;
portEXIT_CRITICAL(&gpio_spinlock);
return;
}
static void gpio_isr_register_on_core_static(void *param)
{
gpio_isr_alloc_t *p = (gpio_isr_alloc_t *)param;
//We need to check the return value.
p->ret = esp_intr_alloc(p->source, p->intr_alloc_flags, p->fn, p->arg, p->handle);
}
esp_err_t gpio_isr_register(void (*fn)(void*), void * arg, int intr_alloc_flags, gpio_isr_handle_t *handle)
{
GPIO_CHECK(fn, "GPIO ISR null", ESP_ERR_INVALID_ARG);
return esp_intr_alloc(ETS_GPIO_INTR_SOURCE, intr_alloc_flags, fn, arg, handle);
gpio_isr_alloc_t p;
p.source = ETS_GPIO_INTR_SOURCE;
p.intr_alloc_flags = intr_alloc_flags;
p.fn = fn;
p.arg = arg;
p.handle = handle;
portENTER_CRITICAL(&gpio_spinlock);
if(isr_core_id == GPIO_ISR_CORE_ID_UNINIT) {
isr_core_id = xPortGetCoreID();
}
portEXIT_CRITICAL(&gpio_spinlock);
esp_err_t ret = esp_ipc_call_blocking(isr_core_id, gpio_isr_register_on_core_static, (void *)&p);
if(ret != ESP_OK || p.ret != ESP_OK) {
return ESP_ERR_NOT_FOUND;
}
return ESP_OK;
}
esp_err_t gpio_wakeup_enable(gpio_num_t gpio_num, gpio_int_type_t intr_type)

125
components/driver/test/test_gpio.c
View file

@ -616,3 +616,128 @@ TEST_CASE("GPIO drive capability test", "[gpio][ignore]")
drive_capability_set_get(GPIO_OUTPUT_IO, GPIO_DRIVE_CAP_3);
prompt_to_continue("If this test finishes");
}
#if !CONFIG_FREERTOS_UNICORE
void gpio_enable_task(void *param)
{
int gpio_num = (int)param;
TEST_ESP_OK(gpio_intr_enable(gpio_num));
vTaskDelete(NULL);
}
/** Test the GPIO Interrupt Enable API with dual core enabled. The GPIO ISR service routine is registered on one core.
* When the GPIO interrupt on another core is enabled, the GPIO interrupt will be lost.
* First on the core 0, Do the following steps:
* 1. Configure the GPIO18 input_output mode, and enable the rising edge interrupt mode.
* 2. Trigger the GPIO18 interrupt and check if the interrupt responds correctly.
* 3. Disable the GPIO18 interrupt
* Then on the core 1, Do the following steps:
* 1. Enable the GPIO18 interrupt again.
* 2. Trigger the GPIO18 interrupt and check if the interrupt responds correctly.
*
*/
TEST_CASE("GPIO Enable/Disable interrupt on multiple cores", "[gpio][ignore]")
{
const int test_io18 = GPIO_NUM_18;
gpio_config_t io_conf;
io_conf.intr_type = GPIO_INTR_NEGEDGE;
io_conf.mode = GPIO_MODE_INPUT_OUTPUT;
io_conf.pin_bit_mask = (1ULL << test_io18);
io_conf.pull_down_en = 0;
io_conf.pull_up_en = 1;
TEST_ESP_OK(gpio_config(&io_conf));
TEST_ESP_OK(gpio_set_level(test_io18, 0));
TEST_ESP_OK(gpio_install_isr_service(0));
TEST_ESP_OK(gpio_isr_handler_add(test_io18, gpio_isr_edge_handler, (void*) test_io18));
vTaskDelay(1000 / portTICK_RATE_MS);
TEST_ESP_OK(gpio_set_level(test_io18, 1));
vTaskDelay(100 / portTICK_RATE_MS);
TEST_ESP_OK(gpio_set_level(test_io18, 0));
vTaskDelay(100 / portTICK_RATE_MS);
TEST_ESP_OK(gpio_intr_disable(test_io18));
TEST_ASSERT(edge_intr_times == 1);
xTaskCreatePinnedToCore(gpio_enable_task, "gpio_enable_task", 1024*4, (void*)test_io18, 8, NULL, (xPortGetCoreID() == 0));
vTaskDelay(1000 / portTICK_RATE_MS);
TEST_ESP_OK(gpio_set_level(test_io18, 1));
vTaskDelay(100 / portTICK_RATE_MS);
TEST_ESP_OK(gpio_set_level(test_io18, 0));
vTaskDelay(100 / portTICK_RATE_MS);
TEST_ESP_OK(gpio_intr_disable(test_io18));
TEST_ESP_OK(gpio_isr_handler_remove(test_io18));
gpio_uninstall_isr_service();
TEST_ASSERT(edge_intr_times == 2);
}
#endif
typedef struct {
int gpio_num;
int isr_cnt;
} gpio_isr_param_t;
static void gpio_isr_handler(void* arg)
{
gpio_isr_param_t *param = (gpio_isr_param_t *)arg;
ets_printf("GPIO[%d] intr, val: %d\n", param->gpio_num, gpio_get_level(param->gpio_num));
param->isr_cnt++;
}
/** The previous GPIO interrupt service routine polls the interrupt raw status register to find the GPIO that triggered the interrupt.
* But this will incorrectly handle the interrupt disabled GPIOs, because the raw interrupt status register can still be set when
* the trigger signal arrives, even if the interrupt is disabled.
* First on the core 0:
* 1. Configure the GPIO18 and GPIO19 input_output mode.
* 2. Enable GPIO18 dual edge triggered interrupt, enable GPIO19 falling edge triggered interrupt.
* 3. Trigger GPIO18 interrupt, then disable the GPIO8 interrupt, and then trigger GPIO18 again(This time will not respond to the interrupt).
* 4. Trigger GPIO19 interrupt.
* If the bug is not fixed, you will see, in the step 4, the interrupt of GPIO18 will also respond.
*/
TEST_CASE("GPIO ISR service test", "[gpio][ignore]")
{
const int test_io18 = GPIO_NUM_18;
const int test_io19 = GPIO_NUM_19;
static gpio_isr_param_t io18_param = {
.gpio_num = GPIO_NUM_18,
.isr_cnt = 0,
};
static gpio_isr_param_t io19_param = {
.gpio_num = GPIO_NUM_19,
.isr_cnt = 0,
};
gpio_config_t io_conf;
io_conf.intr_type = GPIO_INTR_DISABLE;
io_conf.mode = GPIO_MODE_INPUT_OUTPUT;
io_conf.pin_bit_mask = (1ULL << test_io18) | (1ULL << test_io19);
io_conf.pull_down_en = 0;
io_conf.pull_up_en = 1;
TEST_ESP_OK(gpio_config(&io_conf));
TEST_ESP_OK(gpio_set_level(test_io18, 0));
TEST_ESP_OK(gpio_set_level(test_io19, 0));
TEST_ESP_OK(gpio_install_isr_service(0));
TEST_ESP_OK(gpio_set_intr_type(test_io18, GPIO_INTR_ANYEDGE));
TEST_ESP_OK(gpio_set_intr_type(test_io19, GPIO_INTR_NEGEDGE));
TEST_ESP_OK(gpio_isr_handler_add(test_io18, gpio_isr_handler, (void*)&io18_param));
TEST_ESP_OK(gpio_isr_handler_add(test_io19, gpio_isr_handler, (void*)&io19_param));
printf("Triggering the interrupt of GPIO18\n");
vTaskDelay(1000 / portTICK_RATE_MS);
//Rising edge
TEST_ESP_OK(gpio_set_level(test_io18, 1));
printf("Disable the interrupt of GPIO18");
vTaskDelay(100 / portTICK_RATE_MS);
//Disable GPIO18 interrupt, GPIO18 will not respond to the next falling edge interrupt.
TEST_ESP_OK(gpio_intr_disable(test_io18));
vTaskDelay(100 / portTICK_RATE_MS);
//Falling edge
TEST_ESP_OK(gpio_set_level(test_io18, 0));
printf("Triggering the interrupt of GPIO19\n");
vTaskDelay(100 / portTICK_RATE_MS);
TEST_ESP_OK(gpio_set_level(test_io19, 1));
vTaskDelay(100 / portTICK_RATE_MS);
//Falling edge
TEST_ESP_OK(gpio_set_level(test_io19, 0));
vTaskDelay(100 / portTICK_RATE_MS);
TEST_ESP_OK(gpio_isr_handler_remove(test_io18));
TEST_ESP_OK(gpio_isr_handler_remove(test_io19));
gpio_uninstall_isr_service();
TEST_ASSERT((io18_param.isr_cnt == 1) && (io19_param.isr_cnt == 1));
}