diff --git a/components/driver/gpio.c b/components/driver/gpio.c index 3054bdaf96..bfab495634 100644 --- a/components/driver/gpio.c +++ b/components/driver/gpio.c @@ -502,15 +502,21 @@ esp_err_t gpio_isr_handler_remove(gpio_num_t gpio_num) void gpio_uninstall_isr_service(void) { + gpio_isr_func_t *gpio_isr_func_free = NULL; + gpio_isr_handle_t gpio_isr_handle_free = NULL; + portENTER_CRITICAL(&gpio_context.gpio_spinlock); if (gpio_context.gpio_isr_func == NULL) { + portEXIT_CRITICAL(&gpio_context.gpio_spinlock); return; } - portENTER_CRITICAL(&gpio_context.gpio_spinlock); - esp_intr_free(gpio_context.gpio_isr_handle); - free(gpio_context.gpio_isr_func); + gpio_isr_func_free = gpio_context.gpio_isr_func; gpio_context.gpio_isr_func = NULL; + gpio_isr_handle_free = gpio_context.gpio_isr_handle; + gpio_context.gpio_isr_handle = NULL; gpio_context.isr_core_id = GPIO_ISR_CORE_ID_UNINIT; portEXIT_CRITICAL(&gpio_context.gpio_spinlock); + esp_intr_free(gpio_isr_handle_free); + free(gpio_isr_func_free); return; } @@ -543,7 +549,12 @@ esp_err_t gpio_isr_register(void (*fn)(void *), void *arg, int intr_alloc_flags, #else /* CONFIG_FREERTOS_UNICORE */ ret = esp_ipc_call_blocking(gpio_context.isr_core_id, gpio_isr_register_on_core_static, (void *)&p); #endif /* !CONFIG_FREERTOS_UNICORE */ - if(ret != ESP_OK || p.ret != ESP_OK) { + if (ret != ESP_OK) { + ESP_LOGE(GPIO_TAG, "esp_ipc_call_blocking failed (0x%x)", ret); + return ESP_ERR_NOT_FOUND; + } + if (p.ret != ESP_OK) { + ESP_LOGE(GPIO_TAG, "esp_intr_alloc failed (0x%x)", p.ret); return ESP_ERR_NOT_FOUND; } return ESP_OK; diff --git a/components/driver/test/test_gpio.c b/components/driver/test/test_gpio.c index c8b3a69c26..7a78f9255b 100644 --- a/components/driver/test/test_gpio.c +++ b/components/driver/test/test_gpio.c @@ -15,6 +15,7 @@ #include "sdkconfig.h" #include "esp_rom_uart.h" #include "esp_rom_sys.h" +#include "test_utils.h" #define WAKE_UP_IGNORE 1 // gpio_wakeup function development is not completed yet, set it deprecated. @@ -76,16 +77,16 @@ static gpio_config_t init_io(gpio_num_t num) return io_conf; } -#if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32S2, ESP32S3, ESP32C3) -//No runners // edge interrupt event -static void gpio_isr_edge_handler(void* arg) +__attribute__((unused)) static void gpio_isr_edge_handler(void* arg) { uint32_t gpio_num = (uint32_t) arg; - esp_rom_printf("GPIO[%d] intr, val: %d\n", gpio_num, gpio_get_level(gpio_num)); + esp_rom_printf("GPIO[%d] intr on core %d, val: %d\n", gpio_num, cpu_hal_get_core_id(), gpio_get_level(gpio_num)); edge_intr_times++; } +#if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32S2, ESP32S3, ESP32C3) +//No runners // level interrupt event with "gpio_intr_disable" static void gpio_isr_level_handler(void* arg) { @@ -388,6 +389,42 @@ TEST_CASE("GPIO enable and disable interrupt test", "[gpio][test_env=UT_T1_GPIO] } #endif //DISABLED_FOR_TARGETS(ESP32S2, ESP32S3, ESP32C3) +#if !CONFIG_FREERTOS_UNICORE +static void install_isr_service_task(void *arg) +{ + uint32_t gpio_num = (uint32_t) arg; + //rising edge intr + TEST_ESP_OK(gpio_set_intr_type(gpio_num, GPIO_INTR_POSEDGE)); + TEST_ESP_OK(gpio_install_isr_service(0)); + gpio_isr_handler_add(gpio_num, gpio_isr_edge_handler, (void *) gpio_num); + vTaskSuspend(NULL); +} + +TEST_CASE("GPIO interrupt on other CPUs test", "[gpio]") +{ + TaskHandle_t gpio_task_handle; + gpio_config_t input_output_io = init_io(TEST_GPIO_EXT_OUT_IO); + input_output_io.mode = GPIO_MODE_INPUT_OUTPUT; + input_output_io.pull_up_en = 1; + TEST_ESP_OK(gpio_config(&input_output_io)); + + for (int cpu_num = 1; cpu_num < portNUM_PROCESSORS; ++cpu_num) { + // We assume unit-test task is running on core 0, so we install gpio interrupt on other cores + edge_intr_times = 0; + TEST_ESP_OK(gpio_set_level(TEST_GPIO_EXT_OUT_IO, 0)); + xTaskCreatePinnedToCore(install_isr_service_task, "install_isr_service_task", 2048, (void *) TEST_GPIO_EXT_OUT_IO, 1, &gpio_task_handle, cpu_num); + + vTaskDelay(200 / portTICK_RATE_MS); + TEST_ESP_OK(gpio_set_level(TEST_GPIO_EXT_OUT_IO, 1)); + vTaskDelay(100 / portTICK_RATE_MS); + TEST_ASSERT_EQUAL_INT(edge_intr_times, 1); + gpio_isr_handler_remove(TEST_GPIO_EXT_OUT_IO); + gpio_uninstall_isr_service(); + test_utils_task_delete(gpio_task_handle); + } +} +#endif //!CONFIG_FREERTOS_UNICORE + // ESP32 Connect GPIO18 with GPIO19, ESP32-S2 Connect GPIO17 with GPIO21, // ESP32-S3 Connect GPIO19 with GPIO20, ESP32C3 Connect GPIO2 with GPIO3 // use multimeter to test the voltage, so it is ignored in CI diff --git a/components/hal/esp32s3/include/hal/gpio_ll.h b/components/hal/esp32s3/include/hal/gpio_ll.h index 234b477578..3e3c092664 100644 --- a/components/hal/esp32s3/include/hal/gpio_ll.h +++ b/components/hal/esp32s3/include/hal/gpio_ll.h @@ -35,8 +35,9 @@ extern "C" { // Get GPIO hardware instance with giving gpio num #define GPIO_LL_GET_HW(num) (((num) == 0) ? (&GPIO) : NULL) -#define GPIO_LL_PRO_CPU_INTR_ENA (BIT(0)) -#define GPIO_LL_PRO_CPU_NMI_INTR_ENA (BIT(1)) +// On ESP32S3, pro cpu and app cpu shares the same interrupt enable bit +#define GPIO_LL_INTR_ENA (BIT(0)) +#define GPIO_LL_NMI_INTR_ENA (BIT(1)) /** * @brief Enable pull-up on GPIO. @@ -103,6 +104,8 @@ static inline void gpio_ll_set_intr_type(gpio_dev_t *hw, gpio_num_t gpio_num, gp */ static inline void gpio_ll_get_intr_status(gpio_dev_t *hw, uint32_t core_id, uint32_t *status) { + // On ESP32S3, pcpu_int register represents GPIO0-31 interrupt status on both cores + (void)core_id; *status = hw->pcpu_int; } @@ -115,6 +118,8 @@ static inline void gpio_ll_get_intr_status(gpio_dev_t *hw, uint32_t core_id, uin */ static inline void gpio_ll_get_intr_status_high(gpio_dev_t *hw, uint32_t core_id, uint32_t *status) { + // On ESP32S3, pcpu_int1 register represents GPIO32-48 interrupt status on both cores + (void)core_id; *status = hw->pcpu_int1.intr; } @@ -149,9 +154,8 @@ static inline void gpio_ll_clear_intr_status_high(gpio_dev_t *hw, uint32_t mask) */ static inline void gpio_ll_intr_enable_on_core(gpio_dev_t *hw, uint32_t core_id, gpio_num_t gpio_num) { - if (core_id == 0) { - GPIO.pin[gpio_num].int_ena = GPIO_LL_PRO_CPU_INTR_ENA; //enable pro cpu intr - } + (void)core_id; + GPIO.pin[gpio_num].int_ena = GPIO_LL_INTR_ENA; //enable intr } /** diff --git a/components/soc/esp32s3/include/soc/gpio_struct.h b/components/soc/esp32s3/include/soc/gpio_struct.h index ff589aac9b..fdcca9a038 100644 --- a/components/soc/esp32s3/include/soc/gpio_struct.h +++ b/components/soc/esp32s3/include/soc/gpio_struct.h @@ -115,20 +115,20 @@ typedef volatile struct { }; uint32_t val; } status1_w1tc; - uint32_t pcpu_int; /**/ - uint32_t pcpu_nmi_int; /**/ - uint32_t cpusdio_int; /**/ + uint32_t pcpu_int; /*GPIO0~31 PRO & APP CPU interrupt status*/ + uint32_t pcpu_nmi_int; /*GPIO0~31 PRO & APP CPU non-maskable interrupt status*/ + uint32_t cpusdio_int; union { struct { - uint32_t intr: 22; - uint32_t reserved22: 10; + uint32_t intr : 22; /*GPIO32-48 PRO & APP CPU interrupt status*/ + uint32_t reserved22 : 10; }; uint32_t val; } pcpu_int1; union { struct { - uint32_t intr: 22; - uint32_t reserved22: 10; + uint32_t intr : 22; /*GPIO32-48 PRO & APP CPU non-maskable interrupt status*/ + uint32_t reserved22 : 10; }; uint32_t val; } pcpu_nmi_int1;