From 1fa59c442b1af5e3645f54092ce5a4aab5f7e9d2 Mon Sep 17 00:00:00 2001
From: Marius Vikhammer <marius.vikhammer@espressif.com>
Date: Wed, 17 Apr 2024 13:44:46 +0800
Subject: [PATCH] fix(ulp): fixed lp-core not booting during sleep

LP core was unable to boot when system was in deepsleep.
This was due to lp uart init in LP rom using XTAL as clk source,
which is normally powered down during sleep.

This would cause lp uart to get stuck while printing ROM output,
and the app would never boot.

Also fixed wrong wakeup cause used by HP core for ULP wake up
---
 .../port/esp32p4/private_include/pmu_bit_defs.h        |  4 ++--
 components/esp_rom/include/esp32p4/rom/rtc.h           |  7 +++++++
 components/ulp/lp_core/lp_core.c                       | 10 ++++++++++
 .../ulp/lp_core/lp_core/include/ulp_lp_core_print.h    |  2 ++
 components/ulp/test_apps/lp_core/main/test_lp_core.c   |  8 ++++++--
 docs/en/api-reference/system/ulp-lp-core.rst           |  4 ++--
 examples/system/.build-test-rules.yml                  | 10 ----------
 examples/system/ulp/lp_core/gpio/README.md             |  4 ++--
 .../system/ulp/lp_core/lp_uart/lp_uart_echo/README.md  |  4 ++--
 .../system/ulp/lp_core/lp_uart/lp_uart_print/README.md |  4 ++--
 .../rtc_mem_reserve/main/rtc_mem_reserve_test_main.c   |  2 +-
 11 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/components/esp_hw_support/port/esp32p4/private_include/pmu_bit_defs.h b/components/esp_hw_support/port/esp32p4/private_include/pmu_bit_defs.h
index 0dc0f1bd0c..19c94de993 100644
--- a/components/esp_hw_support/port/esp32p4/private_include/pmu_bit_defs.h
+++ b/components/esp_hw_support/port/esp32p4/private_include/pmu_bit_defs.h
@@ -11,7 +11,7 @@ extern "C" {
 #endif
 
 #define PMU_SDIO_WAKEUP_EN              BIT(0)
-#define PMU_SW_WAKEUP_HP_EN             BIT(1)
+#define PMU_LP_CORE_WAKEUP_EN           BIT(1)
 #define PMU_GPIO_WAKEUP_EN              BIT(2)
 #define PMU_USB_WAKEUP_EN               BIT(3)
 #define PMU_UART4_WAKEUP_EN             BIT(4)
@@ -26,7 +26,7 @@ extern "C" {
 #define PMU_LP_TIMER_WAKEUP_EN          BIT(13)
 #define PMU_BOD_WAKEUP_EN               BIT(14)
 #define PMU_VDDBAT_UNDERVOLT_WAKEUP_EN  BIT(15)
-#define PMU_LP_CORE_WAKEUP_EN           BIT(16)
+#define PMU_LP_CORE_TRAP_WAKEUP_EN      BIT(16)
 #define PMU_ETM_WAKEUP_EN               BIT(17)
 #define PMU_LP_TIMER1_WAKEUP_EN         BIT(18)
 #define PMU_LP_I2S_WAKEUP_EN            BIT(19)
diff --git a/components/esp_rom/include/esp32p4/rom/rtc.h b/components/esp_rom/include/esp32p4/rom/rtc.h
index aa45a97d01..6eccaa9ba5 100644
--- a/components/esp_rom/include/esp32p4/rom/rtc.h
+++ b/components/esp_rom/include/esp32p4/rom/rtc.h
@@ -50,6 +50,9 @@ extern "C" {
   *     RTC_CNTL_STORE5_REG     APB bus frequency
   *     RTC_CNTL_STORE6_REG     FAST_RTC_MEMORY_ENTRY
   *     RTC_CNTL_STORE7_REG     FAST_RTC_MEMORY_CRC
+  *     LP_SYS_LP_STORE8_REG    sleep mode and wake stub address
+  *     LP_SYS_LP_STORE9_REG    LP_UART_INIT_CTRL
+  *     LP_SYS_LP_STORE10_REG   LP_ROM_LOG_CTRL
   *************************************************************************************
   */
 
@@ -75,6 +78,10 @@ extern "C" {
 #define RTC_SLEEP_WAKE_STUB_ADDR_REG  LP_SYSTEM_REG_LP_STORE8_REG
 #define RTC_SLEEP_MODE_REG            LP_SYSTEM_REG_LP_STORE8_REG
 
+// lp uart init status, 0 - need init, 1 - no init.
+#define LP_UART_INIT_CTRL_REG   LP_SYSTEM_REG_LP_STORE9_REG
+#define ROM_LOG_CTRL_REG        LP_SYSTEM_REG_LP_STORE10_REG
+
 typedef enum {
     AWAKE = 0,             //<CPU ON
     LIGHT_SLEEP = BIT0,    //CPU waiti, PLL ON.  We don't need explicitly set this mode.
diff --git a/components/ulp/lp_core/lp_core.c b/components/ulp/lp_core/lp_core.c
index 9db3684d5a..71aefe2429 100644
--- a/components/ulp/lp_core/lp_core.c
+++ b/components/ulp/lp_core/lp_core.c
@@ -18,6 +18,10 @@
 #include "ulp_lp_core_lp_timer_shared.h"
 #include "hal/lp_core_ll.h"
 
+#if CONFIG_IDF_TARGET_ESP32P4
+#include "esp32p4/rom/rtc.h"
+#endif
+
 #if CONFIG_IDF_TARGET_ESP32P4 || CONFIG_IDF_TARGET_ESP32C5
 #define LP_CORE_RCC_ATOMIC() PERIPH_RCC_ATOMIC()
 #else
@@ -67,6 +71,12 @@ esp_err_t ulp_lp_core_run(ulp_lp_core_cfg_t* cfg)
         boot_addr = (intptr_t)(&_rtc_ulp_memory_start);
     } else {
         boot_addr = SOC_LP_ROM_LOW;
+        /* Disable UART init in ROM, it defaults to XTAL clk src
+         * which is not powered on during deep sleep
+         * This will cause the ROM code to get stuck during UART output
+         * if used
+         */
+        REG_SET_BIT(LP_UART_INIT_CTRL_REG, 1 << 0);
     }
 
     lp_core_ll_set_boot_address(boot_addr);
diff --git a/components/ulp/lp_core/lp_core/include/ulp_lp_core_print.h b/components/ulp/lp_core/lp_core/include/ulp_lp_core_print.h
index 4c0b214528..56401145d7 100644
--- a/components/ulp/lp_core/lp_core/include/ulp_lp_core_print.h
+++ b/components/ulp/lp_core/lp_core/include/ulp_lp_core_print.h
@@ -29,6 +29,8 @@ void lp_core_printf(const char* format, ...);
  *
  * @note This function must be called before printing anything when the LP core boots from LP ROM but does not install
  *       putc handler. This is possible when the LP ROM is instructed so by setting bit#1 in the LP_SYSTEM_REG_LP_STORE9_REG register.
+ *       Disabling ROM UART init is default behavior in IDF, since the clock configured by the ROM code for UART (XTAL) is normally
+ *       powered down during sleep.
  */
 extern void ets_install_uart_printf(void);
 void (*lp_core_install_uart_printf)(void) = ets_install_uart_printf;
diff --git a/components/ulp/test_apps/lp_core/main/test_lp_core.c b/components/ulp/test_apps/lp_core/main/test_lp_core.c
index 191ca6d045..e27febe2e6 100644
--- a/components/ulp/test_apps/lp_core/main/test_lp_core.c
+++ b/components/ulp/test_apps/lp_core/main/test_lp_core.c
@@ -121,7 +121,7 @@ TEST_CASE("Test LP core delay", "[lp_core]")
 #define LP_TIMER_TEST_DURATION_S        (5)
 #define LP_TIMER_TEST_SLEEP_DURATION_US (20000)
 
-#if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32P4, ESP32C5)
+#if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32C5)
 
 static void do_ulp_wakeup_deepsleep(lp_core_test_commands_t ulp_cmd)
 {
@@ -177,6 +177,10 @@ static void do_ulp_wakeup_with_lp_timer_deepsleep(void)
     ulp_lp_core_cfg_t cfg = {
         .wakeup_source = ULP_LP_CORE_WAKEUP_SOURCE_LP_TIMER,
         .lp_timer_sleep_duration_us = LP_TIMER_TEST_SLEEP_DURATION_US,
+#if ESP_ROM_HAS_LP_ROM
+        /* ROM Boot takes quite a bit longer, which skews the numbers of wake-ups. skip rom boot to keep the calculation simple */
+        .skip_lp_rom_boot = true,
+#endif
     };
 
     load_and_start_lp_core_firmware(&cfg, lp_core_main_counter_bin_start, lp_core_main_counter_bin_end);
@@ -212,7 +216,7 @@ TEST_CASE_MULTIPLE_STAGES("LP Timer can wakeup lp core periodically during deep
                           do_ulp_wakeup_with_lp_timer_deepsleep,
                           check_reset_reason_and_sleep_duration);
 
-#endif //#if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32P4, ESP32C5)
+#endif //#if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32C5)
 
 TEST_CASE("LP Timer can wakeup lp core periodically", "[lp_core]")
 {
diff --git a/docs/en/api-reference/system/ulp-lp-core.rst b/docs/en/api-reference/system/ulp-lp-core.rst
index 65e37b4070..1709cad40e 100644
--- a/docs/en/api-reference/system/ulp-lp-core.rst
+++ b/docs/en/api-reference/system/ulp-lp-core.rst
@@ -165,9 +165,9 @@ To enhance the capabilities of the ULP LP-Core coprocessor, it has access to per
     ULP LP-Core ROM
     ---------------
 
-    The ULP LP-Core ROM is a small pre-built piece of code located in LP-ROM, which is not modifiable by users. Similar to the bootloader ROM code ran by the main CPU, this code is executed when the ULP LP-Core coprocessor is started. The ROM code initializes the ULP LP-Core coprocessor and then jumps to the user program. The ROM code is responsible for initializing the LP UART and printing boot messages.
+    The ULP LP-Core ROM is a small pre-built piece of code located in LP-ROM, which is not modifiable by users. Similar to the bootloader ROM code ran by the main CPU, this code is executed when the ULP LP-Core coprocessor is started. The ROM code initializes the ULP LP-Core coprocessor and then jumps to the user program. The ROM code also prints boot messages if the LP UART has been initialized.
 
-    The ROM code is not executed if :cpp:member:`ulp_lp_core_cfg_t::skip_lp_rom_boot` is set to true. This is useful when you need the ULP to wake-up as quickly as possible and the extra overhead of initializing UART and printing is unwanted.
+    The ROM code is not executed if :cpp:member:`ulp_lp_core_cfg_t::skip_lp_rom_boot` is set to true. This is useful when you need the ULP to wake-up as quickly as possible and the extra overhead of initializing and printing is unwanted.
 
     In addition to the boot-up code mentioned above the ROM code also provides the following functions and interfaces:
 
diff --git a/examples/system/.build-test-rules.yml b/examples/system/.build-test-rules.yml
index 2cd03530c5..5bc2392b62 100644
--- a/examples/system/.build-test-rules.yml
+++ b/examples/system/.build-test-rules.yml
@@ -261,10 +261,6 @@ examples/system/task_watchdog:
 examples/system/ulp/lp_core/gpio:
   enable:
     - if: SOC_LP_CORE_SUPPORTED == 1
-  disable:
-    - if: IDF_TARGET == "esp32p4"
-      temporary: true
-      reason: target esp32p4 is not supported yet, TODO IDF-7536
   depends_components:
     - ulp
 
@@ -281,18 +277,12 @@ examples/system/ulp/lp_core/lp_i2c:
 examples/system/ulp/lp_core/lp_uart/lp_uart_echo:
   disable:
     - if: SOC_ULP_LP_UART_SUPPORTED != 1
-    - if: IDF_TARGET == "esp32p4"
-      temporary: true
-      reason: target esp32p4 is not supported yet TODO IDF-9407
   depends_components:
     - ulp
 
 examples/system/ulp/lp_core/lp_uart/lp_uart_print:
   disable:
     - if: SOC_ULP_LP_UART_SUPPORTED != 1
-    - if: IDF_TARGET == "esp32p4"
-      temporary: true
-      reason: target esp32p4 is not supported yet TODO IDF-9407
   depends_components:
     - ulp
 
diff --git a/examples/system/ulp/lp_core/gpio/README.md b/examples/system/ulp/lp_core/gpio/README.md
index c3940f2825..0f89d7a23c 100644
--- a/examples/system/ulp/lp_core/gpio/README.md
+++ b/examples/system/ulp/lp_core/gpio/README.md
@@ -1,5 +1,5 @@
-| Supported Targets | ESP32-C6 |
-| ----------------- | -------- |
+| Supported Targets | ESP32-C6 | ESP32-P4 |
+| ----------------- | -------- | -------- |
 
 # LP Core simple example with GPIO Polling:
 
diff --git a/examples/system/ulp/lp_core/lp_uart/lp_uart_echo/README.md b/examples/system/ulp/lp_core/lp_uart/lp_uart_echo/README.md
index dbafcc983b..8c5072dbac 100644
--- a/examples/system/ulp/lp_core/lp_uart/lp_uart_echo/README.md
+++ b/examples/system/ulp/lp_core/lp_uart/lp_uart_echo/README.md
@@ -1,5 +1,5 @@
-| Supported Targets | ESP32-C6 |
-| ----------------- | -------- |
+| Supported Targets | ESP32-C6 | ESP32-P4 |
+| ----------------- | -------- | -------- |
 
 # LP UART Echo Example
 
diff --git a/examples/system/ulp/lp_core/lp_uart/lp_uart_print/README.md b/examples/system/ulp/lp_core/lp_uart/lp_uart_print/README.md
index 4124e72b5e..411d1c1645 100644
--- a/examples/system/ulp/lp_core/lp_uart/lp_uart_print/README.md
+++ b/examples/system/ulp/lp_core/lp_uart/lp_uart_print/README.md
@@ -1,5 +1,5 @@
-| Supported Targets | ESP32-C6 |
-| ----------------- | -------- |
+| Supported Targets | ESP32-C6 | ESP32-P4 |
+| ----------------- | -------- | -------- |
 
 # LP UART Print Example
 
diff --git a/tools/test_apps/system/rtc_mem_reserve/main/rtc_mem_reserve_test_main.c b/tools/test_apps/system/rtc_mem_reserve/main/rtc_mem_reserve_test_main.c
index 8cc712e07d..8c1b45ced7 100644
--- a/tools/test_apps/system/rtc_mem_reserve/main/rtc_mem_reserve_test_main.c
+++ b/tools/test_apps/system/rtc_mem_reserve/main/rtc_mem_reserve_test_main.c
@@ -59,7 +59,7 @@ static bool ulp_is_running(uint32_t *counter_variable)
     uint32_t start_cnt = *counter_variable;
 
     /* Wait a few ULP wakeup cycles to ensure ULP has run */
-    vTaskDelay((5 * ULP_SLEEP_DURATION_US / 1000) / portTICK_PERIOD_MS);
+    vTaskDelay(1000 / portTICK_PERIOD_MS);
 
     uint32_t end_cnt = *counter_variable;
     printf("start run count: %" PRIu32 ", end run count %" PRIu32 "\n", start_cnt, end_cnt);