Merge pull request #9277 from Sola85/improve_espulp

Fix "espulp module broken in CircuitPython 9"

Author: tannewt

locale/circuitpython.pot

@@ -1173,6 +1173,7 @@ msgstr ""
 msgid "Interrupted by output function"
 msgstr ""
 
+#: ports/espressif/common-hal/espulp/ULP.c
 #: ports/mimxrt10xx/common-hal/audiobusio/__init__.c
 #: ports/mimxrt10xx/common-hal/pwmio/PWMOut.c
 #: ports/raspberrypi/bindings/picodvi/Framebuffer.c

ports/espressif/bindings/espulp/ULP.c

@@ -19,7 +19,8 @@
 //|         Raises an exception if another ULP has been instantiated. This
 //|         ensures that is is only used by one piece of code at a time.
 //|
-//|         :param Architecture arch: The ulp arch"""
+//|         :param Architecture arch: The ulp arch.
+//|         """
 //|         ...
 static mp_obj_t espulp_ulp_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) {
     enum { ARG_arch };
@@ -70,21 +71,50 @@ static mp_obj_t espulp_ulp_obj___exit__(size_t n_args, const mp_obj_t *args) {
 }
 static MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(espulp_ulp___exit___obj, 4, 4, espulp_ulp_obj___exit__);
 
+//|     def set_wakeup_period(self, period_index: int, period_us: int) -> None:
+//|         """Sets the wakeup period for the ULP.
+//|
+//|         :param int period_index: = 0..4. Up to 5 different wakeup periods can be stored
+//|             and used by the wakeup timer.
+//|             By default, the value stored in period index 0 is used.
+//|         :param int period_us: The wakeup period given in microseconds."""
+//|         ...
+static mp_obj_t espulp_ulp_set_wakeup_period(mp_obj_t self_in, mp_obj_t period_index, mp_obj_t period_us) {
+    espulp_ulp_obj_t *self = MP_OBJ_TO_PTR(self_in);
+    check_for_deinit(self);
+
+    // period_index should be between 0 and 4 but bounds checking happens in esp-idf, so no need to do that here
+    common_hal_espulp_ulp_set_wakeup_period(self, mp_obj_get_int(period_index), mp_obj_get_int(period_us));
+
+    return mp_const_none;
+}
+static MP_DEFINE_CONST_FUN_OBJ_3(espulp_ulp_set_wakeup_period_obj, espulp_ulp_set_wakeup_period);
+
 //|     def run(
-//|         self, program: ReadableBuffer, *, pins: Sequence[microcontroller.Pin] = ()
+//|         self,
+//|         program: ReadableBuffer,
+//|         *,
+//|         entrypoint: int = 0,
+//|         pins: Sequence[microcontroller.Pin] = ()
 //|     ) -> None:
-//|         """Loads the program into ULP memory and then runs the program. The given pins are
-//|            claimed and not reset until `halt()` is called.
+//|         """Loads the program into ULP memory and then runs the program.
 //|
-//|         The program will continue to run even when the running Python is halted."""
+//|         The program will continue to run even Python is halted or in deep-sleep.
+//|
+//|         :param ReadableBuffer program: the ULP binary.
+//|         :param int entrypoint: Specifies the offset (in bytes) of the first instruction
+//|            from the start of the program (Only used by FSM ULP).
+//|         :param Sequence[microcontroller.Pin] pins: Pins made available to the ULP.
+//|            The pins are claimed and not reset until `halt()` is called."""
 //|         ...
 static mp_obj_t espulp_ulp_run(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
     espulp_ulp_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]);
     check_for_deinit(self);
 
-    enum { ARG_program, ARG_pins };
+    enum { ARG_program, ARG_entrypoint, ARG_pins };
     static const mp_arg_t allowed_args[] = {
         { MP_QSTR_program, MP_ARG_REQUIRED | MP_ARG_OBJ},
+        { MP_QSTR_entrypoint, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 0}},
         { MP_QSTR_pins, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = mp_const_empty_tuple} },
     };
 
@@ -94,6 +124,8 @@ static mp_obj_t espulp_ulp_run(size_t n_args, const mp_obj_t *pos_args, mp_map_t
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[ARG_program].u_obj, &bufinfo, MP_BUFFER_READ);
 
+    mp_uint_t entrypoint = args[ARG_entrypoint].u_int;
+
     mp_obj_t pins_in = args[ARG_pins].u_obj;
     const size_t num_pins = (size_t)MP_OBJ_SMALL_INT_VALUE(mp_obj_len(pins_in));
 
@@ -103,21 +135,25 @@ static mp_obj_t espulp_ulp_run(size_t n_args, const mp_obj_t *pos_args, mp_map_t
 
     for (mp_uint_t i = 0; i < num_pins; i++) {
         mp_obj_t pin_obj = mp_obj_subscr(pins_in, MP_OBJ_NEW_SMALL_INT(i), MP_OBJ_SENTINEL);
-        validate_obj_is_free_pin(pin_obj, MP_QSTR_pin);
+        // common-hal checks that pin is free (that way a possible "ULP already running" error
+        // is triggered before a possible "Pin in use" error, if ulp.run is called twice with the same pins).
+        validate_obj_is_pin(pin_obj, MP_QSTR_pin);
         const mcu_pin_obj_t *pin = ((const mcu_pin_obj_t *)pin_obj);
         if (pin->number >= 32) {
             raise_ValueError_invalid_pin();
         }
         pin_mask |= 1 << pin->number;
     }
 
-    common_hal_espulp_ulp_run(self, bufinfo.buf, bufinfo.len, pin_mask);
+    common_hal_espulp_ulp_run(self, bufinfo.buf, bufinfo.len, entrypoint, pin_mask);
     return mp_const_none;
 }
 static MP_DEFINE_CONST_FUN_OBJ_KW(espulp_ulp_run_obj, 2, espulp_ulp_run);
 
 //|     def halt(self) -> None:
-//|         """Halts the running program and releases the pins given in `run()`."""
+//|         """Halts the running program and releases the pins given in `run()`.
+//|         Note: for the FSM ULP, a running ULP program is not actually interrupted.
+//|         Instead, only the wakeup timer is stopped."""
 //|         ...
 static mp_obj_t espulp_ulp_halt(mp_obj_t self_in) {
     espulp_ulp_obj_t *self = MP_OBJ_TO_PTR(self_in);
@@ -143,12 +179,13 @@ MP_PROPERTY_GETTER(espulp_ulp_arch_obj,
     (mp_obj_t)&espulp_ulp_get_arch_obj);
 
 static const mp_rom_map_elem_t espulp_ulp_locals_table[] = {
-    { MP_ROM_QSTR(MP_QSTR_deinit),      MP_ROM_PTR(&espulp_ulp_deinit_obj) },
-    { MP_ROM_QSTR(MP_QSTR___enter__),   MP_ROM_PTR(&mp_identity_obj) },
-    { MP_ROM_QSTR(MP_QSTR___exit__),    MP_ROM_PTR(&espulp_ulp___exit___obj) },
-    { MP_ROM_QSTR(MP_QSTR_run),         MP_ROM_PTR(&espulp_ulp_run_obj) },
-    { MP_ROM_QSTR(MP_QSTR_halt),        MP_ROM_PTR(&espulp_ulp_halt_obj) },
-    { MP_ROM_QSTR(MP_QSTR_arch),        MP_ROM_PTR(&espulp_ulp_arch_obj) },
+    { MP_ROM_QSTR(MP_QSTR_deinit),              MP_ROM_PTR(&espulp_ulp_deinit_obj) },
+    { MP_ROM_QSTR(MP_QSTR___enter__),           MP_ROM_PTR(&mp_identity_obj) },
+    { MP_ROM_QSTR(MP_QSTR___exit__),            MP_ROM_PTR(&espulp_ulp___exit___obj) },
+    { MP_ROM_QSTR(MP_QSTR_set_wakeup_period),   MP_ROM_PTR(&espulp_ulp_set_wakeup_period_obj) },
+    { MP_ROM_QSTR(MP_QSTR_run),                 MP_ROM_PTR(&espulp_ulp_run_obj) },
+    { MP_ROM_QSTR(MP_QSTR_halt),                MP_ROM_PTR(&espulp_ulp_halt_obj) },
+    { MP_ROM_QSTR(MP_QSTR_arch),                MP_ROM_PTR(&espulp_ulp_arch_obj) },
 };
 static MP_DEFINE_CONST_DICT(espulp_ulp_locals_dict, espulp_ulp_locals_table);
 

ports/espressif/bindings/espulp/ULP.h

@@ -15,5 +15,6 @@ void common_hal_espulp_ulp_construct(espulp_ulp_obj_t *self, espulp_architecture
 bool common_hal_espulp_ulp_deinited(espulp_ulp_obj_t *self);
 void common_hal_espulp_ulp_deinit(espulp_ulp_obj_t *self);
 
-void common_hal_espulp_ulp_run(espulp_ulp_obj_t *self, uint32_t *program, size_t length, uint32_t pin_mask);
+void common_hal_espulp_ulp_set_wakeup_period(espulp_ulp_obj_t *self, size_t period_index, uint32_t period_us);
+void common_hal_espulp_ulp_run(espulp_ulp_obj_t *self, uint32_t *program, size_t length, uint32_t entry_point, uint32_t pin_mask);
 void common_hal_espulp_ulp_halt(espulp_ulp_obj_t *self);

ports/espressif/common-hal/espulp/ULP.c

@@ -6,10 +6,13 @@
 
 #include "bindings/espulp/__init__.h"
 #include "bindings/espulp/ULP.h"
+#include "bindings/espidf/__init__.h"
 
 #include "py/runtime.h"
 #include "shared-bindings/microcontroller/Pin.h"
 
+#include "esp_sleep.h"
+
 #if defined(CONFIG_IDF_TARGET_ESP32)
 #include "esp32/ulp.h"
 #define ULP_COPROC_RESERVE_MEM (CONFIG_ESP32_ULP_COPROC_RESERVE_MEM)
@@ -34,7 +37,11 @@ void espulp_reset(void) {
     ulp_used = false;
 }
 
-void common_hal_espulp_ulp_run(espulp_ulp_obj_t *self, uint32_t *program, size_t length, uint32_t pin_mask) {
+void common_hal_espulp_ulp_set_wakeup_period(espulp_ulp_obj_t *self, size_t period_index, uint32_t period_us) {
+    CHECK_ESP_RESULT(ulp_set_wakeup_period(period_index, period_us));
+}
+
+void common_hal_espulp_ulp_run(espulp_ulp_obj_t *self, uint32_t *program, size_t length, uint32_t entry_point, uint32_t pin_mask) {
     if (length > CONFIG_ULP_COPROC_RESERVE_MEM) {
         mp_raise_ValueError(MP_ERROR_TEXT("Program too long"));
     }
@@ -67,19 +74,31 @@ void common_hal_espulp_ulp_run(espulp_ulp_obj_t *self, uint32_t *program, size_t
     }
     pins_used = pin_mask;
 
-    ulp_set_wakeup_period(0, 20000);
+    // Main purpose of ULP is to run while main cpu is in deep sleep, so
+    // ensure GPIO Power Domain remains enabled during deep sleep,
+    // if any GPIO were supplied here.
+    if (pins_used > 0) {
+        esp_sleep_pd_config(ESP_PD_DOMAIN_RTC_PERIPH, ESP_PD_OPTION_ON);
+    }
 
+    esp_err_t result;
     switch (self->arch) {
         #ifdef CONFIG_ULP_COPROC_TYPE_FSM
         case FSM:
-            ulp_load_binary(0, (const uint8_t *)program, length);
-            ulp_run(0);
+            result = ulp_load_binary(0, (const uint8_t *)program, length / sizeof(uint32_t));
+            if (result != ESP_OK) {
+                mp_raise_ValueError_varg(MP_ERROR_TEXT("Invalid %q"), MP_QSTR_program);
+            }
+            CHECK_ESP_RESULT(ulp_run(entry_point / sizeof(uint32_t)));
             break;
         #endif
         #ifdef CONFIG_ULP_COPROC_TYPE_RISCV
         case RISCV:
-            ulp_riscv_load_binary((const uint8_t *)program, length);
-            ulp_riscv_run();
+            result = ulp_riscv_load_binary((const uint8_t *)program, length);
+            if (result != ESP_OK) {
+                mp_raise_ValueError_varg(MP_ERROR_TEXT("Invalid %q"), MP_QSTR_program);
+            }
+            CHECK_ESP_RESULT(ulp_riscv_run());
             break;
         #endif
         default:
@@ -90,12 +109,11 @@ void common_hal_espulp_ulp_run(espulp_ulp_obj_t *self, uint32_t *program, size_t
 
 void common_hal_espulp_ulp_halt(espulp_ulp_obj_t *self) {
     switch (self->arch) {
-        /*
         #ifdef CONFIG_ULP_COPROC_TYPE_FSM
         case FSM:
+            ulp_timer_stop();
             break;
         #endif
-        */
         #ifdef CONFIG_ULP_COPROC_TYPE_RISCV
         case RISCV:
             ulp_riscv_timer_stop();

ports/espressif/esp-idf

@@ -83,6 +83,7 @@ CONFIG_SPI_FLASH_SUPPORT_TH_CHIP=y
 # Ultra Low Power (ULP) Co-processor
 #
 CONFIG_ULP_COPROC_ENABLED=y
+CONFIG_ULP_COPROC_TYPE_FSM=y
 CONFIG_ULP_COPROC_RESERVE_MEM=4080
 # end of Ultra Low Power (ULP) Co-processor
 

ports/espressif/esp-idf-config/sdkconfig-esp32.defaults

@@ -52,6 +52,9 @@ CONFIG_NEWLIB_NANO_FORMAT=y
 # Ultra Low Power (ULP) Co-processor
 #
 CONFIG_ULP_COPROC_ENABLED=y
+CONFIG_ULP_COPROC_TYPE_FSM=y
+CONFIG_ULP_COPROC_TYPE_RISCV=y  # Note: enabling both ULPs simultaneously only works due to a modification of adafruit/esp-idf
+                                # (see adafruit/esp-idf/pull/16) until espressif/esp-idf/issues/12999 is fixed.
 CONFIG_ULP_COPROC_RESERVE_MEM=8176
 # end of Ultra Low Power (ULP) Co-processor
 

ports/espressif/esp-idf-config/sdkconfig-esp32s2.defaults

@@ -92,6 +92,9 @@ CONFIG_NEWLIB_NANO_FORMAT=y
 # Ultra Low Power (ULP) Co-processor
 #
 CONFIG_ULP_COPROC_ENABLED=y
+CONFIG_ULP_COPROC_TYPE_FSM=y
+CONFIG_ULP_COPROC_TYPE_RISCV=y  # Note: enabling both ULPs simultaneously only works due to a modification of adafruit/esp-idf
+                                # (see adafruit/esp-idf/pull/16) until espressif/esp-idf/issues/12999 is fixed.
 CONFIG_ULP_COPROC_RESERVE_MEM=8176
 # end of Ultra Low Power (ULP) Co-processor