Merge branch 'bugfix/gpio_intr_on_core1_s3' into 'master'

gpio: Fix the bug that gpio interrupt cannot be triggered on app cpu on ESP32S3

Closes IDFGH-6211 and IDFGH-3635

See merge request espressif/esp-idf!15945

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);
+    gpio_isr_func_free = gpio_context.gpio_isr_func;
-    esp_intr_free(gpio_context.gpio_isr_handle);
-    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;

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

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))
+// On ESP32S3, pro cpu and app cpu shares the same interrupt enable bit
-#define GPIO_LL_PRO_CPU_NMI_INTR_ENA  (BIT(1))
+#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) {
+    (void)core_id;
-        GPIO.pin[gpio_num].int_ena = GPIO_LL_PRO_CPU_INTR_ENA;     //enable pro cpu intr
+    GPIO.pin[gpio_num].int_ena = GPIO_LL_INTR_ENA;     //enable intr
-    }
 }
 
 /**

components/soc/esp32s3/include/soc/gpio_struct.h

@@ -1,16 +1,9 @@
-// Copyright 2017-2021 Espressif Systems (Shanghai) PTE LTD
+/*
-//
+ * SPDX-FileCopyrightText: 2017-2021 Espressif Systems (Shanghai) CO LTD
-// Licensed under the Apache License, Version 2.0 (the "License");
+ *
-// you may not use this file except in compliance with the License.
+ * SPDX-License-Identifier: Apache-2.0
-// 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.
 #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_int;                                            /*GPIO0~31 PRO & APP CPU interrupt status*/
-    uint32_t pcpu_nmi_int;
+    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;

tools/ci/check_copyright_ignore.txt

@@ -2097,7 +2097,6 @@ components/soc/esp32s3/include/soc/gpio_reg.h
 components/soc/esp32s3/include/soc/gpio_sd_reg.h
 components/soc/esp32s3/include/soc/gpio_sd_struct.h
 components/soc/esp32s3/include/soc/gpio_sig_map.h
-components/soc/esp32s3/include/soc/gpio_struct.h
 components/soc/esp32s3/include/soc/hinf_reg.h
 components/soc/esp32s3/include/soc/hinf_struct.h
 components/soc/esp32s3/include/soc/host_reg.h