From b722e6f6ecbacd7950cf1eca829572ce829c2a60 Mon Sep 17 00:00:00 2001 From: songruojing Date: Wed, 17 Nov 2021 12:07:00 +0800 Subject: [PATCH 1/3] gpio: Bugfix - Move esp_intr_free() out of the critical section in gpio_uninstall_isr_service() Closes https://github.com/espressif/esp-idf/issues/5571 Fix the bug that if the API was called from one core to free the interrupt source on the other core, it would trigger interrupt watchdog. (cherry picked from commit 0e8286c57b33ca76183151c677f3880c6485db79) --- components/driver/gpio.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/components/driver/gpio.c b/components/driver/gpio.c index db27ab45ab..1b88af4965 100644 --- a/components/driver/gpio.c +++ b/components/driver/gpio.c @@ -491,15 +491,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; } @@ -532,7 +538,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; From 41e452e7d8aacb0c6e2aea5b18fe9d6e2c334b9c Mon Sep 17 00:00:00 2001 From: songruojing Date: Thu, 30 Dec 2021 13:06:00 +0800 Subject: [PATCH 2/3] gpio: Fix the bug that gpio interrupt cannot be triggered on app cpu on ESP32S3 Closes https://github.com/espressif/esp-idf/issues/7885 (cherry picked from commit 91f1159f9c74a120dcacec46fbc7f66f48e169cb) --- components/driver/test/test_gpio.c | 39 ++++++++++++++++++- components/hal/esp32s3/include/hal/gpio_ll.h | 14 ++++--- .../soc/esp32s3/include/soc/gpio_struct.h | 27 +++++-------- 3 files changed, 57 insertions(+), 23 deletions(-) diff --git a/components/driver/test/test_gpio.c b/components/driver/test/test_gpio.c index 9f3d416b04..8a9e8e2d5a 100644 --- a/components/driver/test/test_gpio.c +++ b/components/driver/test/test_gpio.c @@ -20,6 +20,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. @@ -100,7 +101,7 @@ static gpio_config_t init_io(gpio_num_t num) __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++; } @@ -408,6 +409,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 GPIO17 with GPIO21, 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 054333c1d4..14dc4dc3ee 100644 --- a/components/hal/esp32s3/include/hal/gpio_ll.h +++ b/components/hal/esp32s3/include/hal/gpio_ll.h @@ -29,8 +29,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. @@ -97,6 +98,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; } @@ -109,6 +112,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; } @@ -143,9 +148,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 be1c3223dd..8aaebc31ba 100644 --- a/components/soc/esp32s3/include/soc/gpio_struct.h +++ b/components/soc/esp32s3/include/soc/gpio_struct.h @@ -1,16 +1,9 @@ -// Copyright 2017-2021 Espressif Systems (Shanghai) PTE LTD -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +/* + * SPDX-FileCopyrightText: 2017-2021 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ + #ifndef _SOC_GPIO_STRUCT_H_ #define _SOC_GPIO_STRUCT_H_ @@ -116,19 +109,19 @@ typedef volatile struct gpio_dev_s { }; uint32_t val; } status1_w1tc; - uint32_t pcpu_int; - uint32_t pcpu_nmi_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 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 intr : 22; /*GPIO32-48 PRO & APP CPU non-maskable interrupt status*/ uint32_t reserved22 : 10; }; uint32_t val; From 9991b1b021ef2d2f29c94c7fc14f219b5306b841 Mon Sep 17 00:00:00 2001 From: songruojing Date: Wed, 16 Feb 2022 14:52:39 +0800 Subject: [PATCH 3/3] ci: increase target-test.yml UT_T1_1 job to support newly added ut test --- .gitlab/ci/target-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab/ci/target-test.yml b/.gitlab/ci/target-test.yml index ef7b5c7302..0a3e7a8d81 100644 --- a/.gitlab/ci/target-test.yml +++ b/.gitlab/ci/target-test.yml @@ -527,7 +527,7 @@ UT_006: UT_007: extends: .unit_test_esp32_template - parallel: 4 + parallel: 5 tags: - ESP32_IDF - UT_T1_1