Skip to content

ports/esp32/ulp: Fix ULP (FSM) support for S2 + S3. #11761

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 14, 2023

Conversation

wnienhaus
Copy link
Contributor

This is a follow-up PR to #11521.

This change updates the IDF version we're building against to v4.4.2, as that version contains important fixes that make the ULP (FSM) actually work on S2/S3 chips. See: espressif/esp-idf@a0e3d48. Without these fixes the ULP never starts when running ulp.run().

This change also ensures the correct header file is included for each ESP32 variant.

Lastly, it enables the ULP (FSM) for all ESP32 variants that have it, rather than requiring it to be enabled for each board specifically.

Notes:

  • The IDF build system will only pick the CONFIG_*_ULP_COPROC_* options relevant to the target being built for, so including the ULP options for multiple targets in sdkconfig.base rather than putting them in each board's config feels ok to me.
  • We could likely switch to building against IDF v4.4.5 (the latest v4.4.x), but for this specific change, the minimum required version is v4.4.2.

@github-actions
Copy link

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% PICO
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS

@codecov
Copy link

codecov bot commented Jun 11, 2023

Codecov Report

Merging #11761 (034502b) into master (ea8f0fd) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #11761   +/-   ##
=======================================
  Coverage   98.40%   98.40%           
=======================================
  Files         155      155           
  Lines       20539    20539           
=======================================
  Hits        20211    20211           
  Misses        328      328           

@wnienhaus
Copy link
Contributor Author

The following code can be used to test the ULP (FSM) on the ESP32-S2/S3.

from esp32 import ULP
from machine import mem32

"""
## SOURCE CODE FOR binary
## from https://github.com/micropython/micropython-esp32-ulp/blob/master/examples/counter.py
data:       .long 0

entry:      move r3, data    # load address of data into r3
            ld r2, r3, 0     # load data contents ([r3+0]) into r2
            add r2, r2, 1    # increment r2
            st r2, r3, 0     # store r2 contents into data ([r3+0])

            halt             # halt ULP co-prozessor (until it gets waked up again)
"""
binary = b'ulp\x00\x0c\x00\x18\x00\x00\x00\x00\x00\x00\x00\x00\x00\x03\x00\x80t\x0e\x00\x00\xd0\x1a\x00\x00t\x0e\x00\x00h\x00\x00\x00\xb0'

load_addr, entry_addr = 0, 4

ULP_MEM_BASE = 0x50000000
ULP_DATA_MASK = 0xffff  # ULP data is only in lower 16 bits

ulp = ULP()
ulp.set_wakeup_period(0, 50000)  # use timer0, wakeup after 50.000 cycles
ulp.load_binary(load_addr, binary)

mem32[ULP_MEM_BASE + load_addr] = 0x1000
ulp.run(entry_addr)

while True:
    print(hex(mem32[ULP_MEM_BASE + load_addr] & ULP_DATA_MASK))

binary was assembled using micropython-esp32-ulp, with the necessary opcodes adjusted for the new binary format of the ULP-FSM of the S2/S3 (this code will not work on the original ESP32).

The code can be used to test 3 scenarios, to show that this PR does what it intends:

  1. Without this PR applied, using the GENERIC_S2 board image, the above code results in an OSError: 260. This happens on line ulp.run(), and the error corresponds to ESP_ERR_INVALID_SIZE from the IDF. The reason is that CONFIG_ESP32S2_ULP_COPROC_RESERVE_MEM is not set, meaning only 0 bytes of memory are available to the ULP but our binary is bigger.

  2. Without this PR applied, but with CONFIG_ESP32S2_ULP_COPROC_RESERVE_MEM=2040 added to GENERIC_S2's board definition, the above code runs without error. However the while loop at the end continuously outputs 0x1000, which is the value that our memory address was initialised to. The ULP simply never starts to run.

  3. With this PR applied in full, the code above works correctly and the while loop outputs 0x1000, 0x1001, 0x1002 and so on, as the value of data is incremented by the ULP. (The difference compared to no 2 above is essentially the bug fixes in v4.4.2 of the IDF)

@ThinkTransit
Copy link
Contributor

Good pick up I only tested with v4.4.4 and didn't realise the it doesn't work on v4.4.

@dpgeorge
Copy link
Member

Thanks for this. I'm happy to merge this before updating to IDF 5 (#11528).

@wnienhaus
Copy link
Contributor Author

Thanks.

As far as I can see, your draft PR for switching to IDF 5.0 should rebase quite easily onto this change. The difference is only really that the ULP build flags have collapsed from per-target to a single set in sdkconfig.base.

This change enables the ULP (FSM) for all ESP32 variants rather than
requiring it to be enabled for each board specifically.

It also ensures the correct header file is included for each variant.

Lastly, it updates the IDF version we're builing against to v4.4.2, as that
version contains important fixes to make the ULP actually work on S2/S3
chips. See: espressif/esp-idf@a0e3d48

Signed-off-by: Wilko Nienhaus <wilko.nienhaus@gmail.com>
@dpgeorge dpgeorge merged commit 034502b into micropython:master Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants