Add asynchronous SPI target support - #3158
Conversation
2fe33e8 to
023ed83
Compare
be56f0c to
a423a38
Compare
73af96b to
398621d
Compare
398621d to
be780ca
Compare
be780ca to
9955d82
Compare
9955d82 to
707faf8
Compare
561deb0 to
3970550
Compare
e566eb0 to
fe266ce
Compare
|
@CodeRabbit review |
✅ Action performedReview finished.
|
WalkthroughThe PR adds ESP32 SPI target support with asynchronous exchanges, DMA configuration, transfer validation, lifecycle handling, chip-select timing parameters, native resource primitives, IRAM settings, and board-level coverage. ChangesESP32 SPI target support
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/spi.toit`:
- Around line 234-238: Update the finalization lifecycle around Target.finalize_
and exchange-in-flight_ so an in-flight exchange cannot cause permanent resource
retention: either allow target_close to tear down in-flight operations and make
finalize_ always call close, or retain a strong Target reference until
finish-exchange_ completes. Ensure the peripheral, host slot, and GPIO pins are
released without relying on a second finalizer invocation.
In `@src/resources/spi_esp32.cc`:
- Around line 378-385: In the result-size calculation, apply the classic ESP32
DMA word-alignment mask to transferred_bytes immediately after it is computed,
before clamping with resource->receive_size(). Remove the later mask on
result_size so requested sizes such as six bytes are preserved when the DMA
transferred a complete word-aligned amount.
In `@src/resources/spi_esp32.h`:
- Around line 159-166: Update signal_from_isr to call portYIELD_FROM_ISR when
xQueueSendFromISR sets higher_was_woken to pdTRUE, placing the yield before the
function returns while preserving the existing critical-section and queue-send
flow.
In `@toolchains/esp32p4/sdkconfig`:
- Line 1004: Regenerate the ESP-IDF Kconfig settings so
CONFIG_HAL_SPI_SLAVE_FUNC_IN_IRAM is enabled alongside
CONFIG_SPI_SLAVE_ISR_IN_IRAM in toolchains/esp32p4/sdkconfig at lines 1004-1004
and toolchains/esp32s2/sdkconfig at lines 745-745; use ESP-IDF 5.4.2 for the S2
configuration. Also apply the same generated HAL IRAM setting to
toolchains/esp32c3/sdkconfig at lines 983-983 and toolchains/esp32c6/sdkconfig
at lines 1117-1117.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a2f9dd24-83d7-41db-a133-79ed7237b8ce
📒 Files selected for processing (16)
lib/spi.toitsrc/compiler/propagation/type_primitive_spi.ccsrc/primitive.hsrc/resources/spi_esp32.ccsrc/resources/spi_esp32.hsrc/tags.htests/hw/esp32/spi-target-board1.toittests/hw/esp32/spi-target-board2.toittests/hw/esp32/spi-target-shared.toitthird_party/esp-idftoolchains/esp32/sdkconfigtoolchains/esp32c3/sdkconfigtoolchains/esp32c6/sdkconfigtoolchains/esp32p4/sdkconfigtoolchains/esp32s2/sdkconfigtoolchains/esp32s3/sdkconfig
| finalize_ -> none: | ||
| // An armed ESP-IDF transaction cannot be canceled. Leave the native | ||
| // resource registered so process teardown can release it safely. | ||
| if exchange-in-flight_: return | ||
| close |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
The finalizer can leak the peripheral, the host slot, and the GPIO pins.
Toit unregisters a finalizer after it runs. When finalize_ returns early because exchange-in-flight_ is true, no later finalizer call happens. The Target object is then collected and close is never called. The SPI host slot and the reserved pins stay taken until the process exits.
The native destructor already handles this case. SpiTargetResource::~SpiTargetResource in src/resources/spi_esp32.cc frees the slave driver before it releases the buffers, and it calls finish_operation when an operation is in flight. Only the target_close primitive rejects an in-flight operation with INVALID_STATE.
Consider one of the following:
- Let
target_closetear the resource down even when an operation is in flight, and letfinalize_always close. - Keep a strong reference to the
Targetwhile an exchange is in flight, so the object cannot be collected beforefinish-exchange_runs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/spi.toit` around lines 234 - 238, Update the finalization lifecycle
around Target.finalize_ and exchange-in-flight_ so an in-flight exchange cannot
cause permanent resource retention: either allow target_close to tear down
in-flight operations and make finalize_ always call close, or retain a strong
Target reference until finish-exchange_ completes. Ensure the peripheral, host
slot, and GPIO pins are released without relying on a second finalizer
invocation.
| size_t transferred_bytes = (resource->transferred_bits() + 7) / 8; | ||
| size_t result_size = Utils::min(transferred_bytes, resource->receive_size()); | ||
| #if CONFIG_IDF_TARGET_ESP32 | ||
| // Classic ESP32 target DMA only commits complete words to its receive | ||
| // buffer. ESP-IDF documents that a controller's trailing bytes are | ||
| // discarded when its transaction length is not a multiple of four. | ||
| if (resource->dma()) result_size &= ~static_cast<size_t>(3); | ||
| #endif |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The classic-ESP32 word truncation is applied after the clamp and drops valid bytes.
The mask at line 384 applies to result_size, which is already clamped to receive_size. The hardware limitation affects how many bytes the DMA committed, which is transferred_bytes, not how many bytes the caller requested.
Consider a caller that requests 6 bytes while the controller clocks 64 bytes. transferred_bytes is 64 and the DMA committed all 64 bytes. result_size becomes 6, and the mask reduces it to 4. Bytes 4 and 5 are valid but are discarded.
Apply the mask to transferred_bytes before the clamp.
🐛 Proposed fix
size_t transferred_bytes = (resource->transferred_bits() + 7) / 8;
- size_t result_size = Utils::min(transferred_bytes, resource->receive_size());
`#if` CONFIG_IDF_TARGET_ESP32
// Classic ESP32 target DMA only commits complete words to its receive
// buffer. ESP-IDF documents that a controller's trailing bytes are
// discarded when its transaction length is not a multiple of four.
- if (resource->dma()) result_size &= ~static_cast<size_t>(3);
+ if (resource->dma()) transferred_bytes &= ~static_cast<size_t>(3);
`#endif`
+ size_t result_size = Utils::min(transferred_bytes, resource->receive_size());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| size_t transferred_bytes = (resource->transferred_bits() + 7) / 8; | |
| size_t result_size = Utils::min(transferred_bytes, resource->receive_size()); | |
| #if CONFIG_IDF_TARGET_ESP32 | |
| // Classic ESP32 target DMA only commits complete words to its receive | |
| // buffer. ESP-IDF documents that a controller's trailing bytes are | |
| // discarded when its transaction length is not a multiple of four. | |
| if (resource->dma()) result_size &= ~static_cast<size_t>(3); | |
| #endif | |
| size_t transferred_bytes = (resource->transferred_bits() + 7) / 8; | |
| `#if` CONFIG_IDF_TARGET_ESP32 | |
| // Classic ESP32 target DMA only commits complete words to its receive | |
| // buffer. ESP-IDF documents that a controller's trailing bytes are | |
| // discarded when its transaction length is not a multiple of four. | |
| if (resource->dma()) transferred_bytes &= ~static_cast<size_t>(3); | |
| `#endif` | |
| size_t result_size = Utils::min(transferred_bytes, resource->receive_size()); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/resources/spi_esp32.cc` around lines 378 - 385, In the result-size
calculation, apply the classic ESP32 DMA word-alignment mask to
transferred_bytes immediately after it is computed, before clamping with
resource->receive_size(). Remove the later mask on result_size so requested
sizes such as six bytes are preserved when the DMA transferred a complete
word-aligned amount.
| IRAM_ATTR void signal_from_isr(word event) { | ||
| BaseType_t higher_was_woken = pdFALSE; | ||
| portENTER_CRITICAL_ISR(&spinlock_); | ||
| pending_event_ |= event; | ||
| portEXIT_CRITICAL_ISR(&spinlock_); | ||
| word payload = 0; | ||
| xQueueSendFromISR(queue(), &payload, &higher_was_woken); | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare with the yield convention used by other ESP32 ISR senders in this repository.
rg -n -C3 'xQueueSendFromISR' src/ | rg -n -C3 'YIELD|higher|woken'Repository: toitlang/toit
Length of output: 3755
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- spi_esp32.h relevant structure and usages ---'
rg -n -C8 'signal_from_isr|higher_was_woken|queue\(\)|return .*pdTRUE|portYIELD_FROM_ISR|portEND_SWITCHING_ISR' src/resources/spi_esp32.h src/resources/*.cc
printf '%s\n' '--- focused file outline ---'
ast-grep outline src/resources/spi_esp32.h --match 'signal_from_isr' --view expanded || true
printf '%s\n' '--- nearby SPI source ---'
sed -n '1,230p' src/resources/spi_esp32.hRepository: toitlang/toit
Length of output: 30524
🏁 Script executed:
#!/bin/bash
set -eu
rg -n -C8 'signal_from_isr|higher_was_woken|queue\(\)|return .*pdTRUE|portYIELD_FROM_ISR|portEND_SWITCHING_ISR' src/resources/spi_esp32.h src/resources/*.cc
sed -n '1,230p' src/resources/spi_esp32.hRepository: toitlang/toit
Length of output: 30231
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- all SPI ISR callback call sites ---'
rg -n -C12 'ready_from_isr|complete_from_isr|spi_slave|post_trans|pre_trans|isr' src/resources/spi_esp32.cc src/resources/spi_esp32.h
printf '%s\n' '--- event-source dispatch and ISR return handling ---'
rg -n -C10 'portYIELD_FROM_ISR|portEND_SWITCHING_ISR|higher_was_woken|BaseType_t|bool.*callback|callback.*bool' src
printf '%s\n' '--- SPI implementation outline ---'
ast-grep outline src/resources/spi_esp32.ccRepository: toitlang/toit
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -eu
rg -n -C12 'ready_from_isr|complete_from_isr|spi_slave|post_trans|pre_trans|isr' src/resources/spi_esp32.cc src/resources/spi_esp32.h
rg -n -C10 'portYIELD_FROM_ISR|portEND_SWITCHING_ISR|higher_was_woken|BaseType_t|bool.*callback|callback.*bool' src
ast-grep outline src/resources/spi_esp32.ccRepository: toitlang/toit
Length of output: 50369
🌐 Web query:
ESP-IDF spi_slave post_setup_cb post_trans_cb ISR FreeRTOS portYIELD_FROM_ISR callback documentation
💡 Result:
In the ESP-IDF SPI Slave driver, the post_setup_cb and post_trans_cb callbacks are executed within the SPI peripheral's interrupt service routine (ISR) context [1][2]. Key considerations for these callbacks include: 1. Interrupt Context Constraints: Because these callbacks run in an ISR, they must be concise and execute quickly to avoid starving the SPI driver [1][2]. It is highly recommended to place these callback functions in IRAM (using the IRAM_ATTR attribute) to prevent potential crashes caused by flash access during interrupt processing, especially if the driver is initialized with the ESP_INTR_FLAG_IRAM flag [2][3]. 2. FreeRTOS Yielding: The standard SPI Slave driver's post_setup_cb and post_trans_cb do not receive an explicit awoken parameter to manage task yielding [2][3]. If you need to trigger a task-level action from these callbacks, standard inter-task communication (such as queues or task notifications) should be used. 3. SPI Slave Half Duplex (HD) Driver Distinction: Do not confuse the standard SPI Slave driver with the SPI Slave Half Duplex (HD) driver. The HD driver explicitly provides an awoken parameter in its callback signature [4][5]. When using the HD driver, if a callback unblocks a task (e.g., via xSemaphoreGiveFromISR), you must pass the corresponding BaseType_t variable to the awoken parameter and then call portYIELD_FROM_ISR within the ISR context if the value is pdTRUE [4][5]. 4. Thread Safety and ISR Core: The SPI ISR is executed on the same core that initialized the SPI peripheral [2]. To ensure thread safety, it is recommended to register the SPI ISR to the same core where tasks accessing the SPI peripheral are running [1][6]. In summary, for the standard SPI Slave driver, these callbacks are direct ISR hooks [2]. If your goal is to minimize latency or integrate closely with FreeRTOS tasks, follow the guidance on executing minimal code within the ISR and using efficient inter-task communication mechanisms [1][2][7]. If you require explicit, managed task yielding within the callback itself, evaluate if the SPI Slave Half Duplex driver is a more appropriate architectural choice for your application [4][5].
Citations:
- 1: https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-reference/peripherals/spi_slave.html
- 2: https://github.com/espressif/esp-idf/blob/v5.3.5/components/esp_driver_spi/include/driver/spi_slave.h
- 3: https://docs.espressif.com/projects/esp-idf/en/latest/esp32p4/api-reference/peripherals/spi_slave.html
- 4: https://docs.espressif.com/projects/esp-idf/en/stable/esp32p4/api-reference/peripherals/spi_slave_hd.html
- 5: https://docs.espressif.com/projects/esp-idf/en/latest/esp32h2/api-reference/peripherals/spi_slave_hd.html
- 6: https://docs.espressif.com/projects/esp-idf/en/v6.0.1/esp32/api-reference/peripherals/spi_slave.html
- 7: Allow queueing a transaction from an ISR context (IDFGH-7834) espressif/esp-idf#9363
🌐 Web query:
FreeRTOS xQueueSendFromISR pxHigherPriorityTaskWoken portYIELD_FROM_ISR documentation
💡 Result:
In FreeRTOS, xQueueSendFromISR, pxHigherPriorityTaskWoken, and portYIELD_FROM_ISR work together to safely manage task synchronization and context switching within an Interrupt Service Routine (ISR) [1][2][3]. xQueueSendFromISR is the version of the queue send API designed for use inside an ISR [1][3]. Because ISRs cannot block, standard API functions are unsuitable; xQueueSendFromISR ensures that if a queue is full, the function simply returns an error code instead of attempting to wait [4][5][1]. pxHigherPriorityTaskWoken is a pointer to a BaseType_t variable that acts as a flag to notify the ISR that a task of higher priority than the currently running task has been unblocked by the queue operation [1][6][2]. 1. Initialization: You must initialize this variable to pdFALSE before calling any "FromISR" API function within the ISR [2][3]. 2. Function Call: You pass the address of this variable to the API function (e.g., xQueueSendFromISR(..., &xHigherPriorityTaskWoken)) [1][7]. 3. Mechanism: If the operation unblocks a task with a priority higher than the currently executing one, the kernel sets the variable to pdTRUE [1][2]. If it is not NULL, this informs the application that a context switch should occur [6][8]. portYIELD_FROM_ISR is an interrupt-safe macro used to request a context switch [7][3]. After calling your "FromISR" API functions, you pass the pxHigherPriorityTaskWoken variable to portYIELD_FROM_ISR [7][2]. If the variable is pdTRUE, the macro triggers a context switch, ensuring the highest-priority task (the one just unblocked) starts running as soon as possible after the ISR finishes [1][7][3]. If it is pdFALSE, the macro does nothing [7][2]. Note that some FreeRTOS ports provide an identical alternative macro called portEND_SWITCHING_ISR [7][2]. Always check your specific port's documentation, as certain architectures require the yield macro to be called at the very end of the ISR [2][9][10].
Citations:
- 1: https://github.com/FreeRTOS/FreeRTOS-Kernel-Book/blob/a4c1c832/ch07.md
- 2: https://freertos.gitbook.io/mastering-the-freertos-tm-real-time-kernel/mastering.ch07
- 3: https://deepwiki.com/FreeRTOS/FreeRTOS-Kernel-Book/2.7-interrupt-management
- 4: https://www.freertos.org/Documentation/02-Kernel/04-API-references/06-Queues/04-xQueueSendFromISR
- 5: https://mcuxpresso.nxp.com/api_doc/dev/1342/a00887.html
- 6: https://forums.freertos.org/t/pxhigherprioritytaskwoken-usage-clarification/6123
- 7: https://github.com/FreeRTOS/FreeRTOS-Kernel-Book/blob/main/ch07.md
- 8: https://forums.freertos.org/t/more-fromisr-and-portyield-from-isr-confusion/20908
- 9: https://forums.freertos.org/t/isr-xhigherprioritytaskwoken-invoke-context-switch-non-default/20734
- 10: https://forums.freertos.org/t/content-switch-after-isr/7371
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- event queue implementation ---'
fd -i 'ev_queue_esp32' src
rg -n -C12 'class EventQueueEventSource|xQueueReceive|event queue|EventQueueResource|queue_size|queue.*1' src/event_sources src/resources/resource* src
printf '%s\n' '--- callback declarations in available headers ---'
rg -n -C5 'post_setup_cb|post_trans_cb|spi_slave_interface_config_t' . -g '*.h' -g '*.c' -g '*.cc' -g '*.cpp' | head -160Repository: toitlang/toit
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -eu
fd -i 'ev_queue_esp32' src
rg -n -C12 'class EventQueueEventSource|xQueueReceive|EventQueueResource|queue_size|queue.*1' src/event_sources src/resources
rg -n -C5 'post_setup_cb|post_trans_cb|spi_slave_interface_config_t' . -g '*.h' -g '*.c' -g '*.cc' -g '*.cpp' | head -160Repository: toitlang/toit
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- event-source task setup ---'
sed -n '1,135p' src/event_sources/ev_queue_esp32.cc
printf '%s\n' '--- thread priority defaults and yield macros in repository ---'
rg -n -C5 'EventQueueEventSource|xTaskCreatePinnedToCore|tskIDLE_PRIORITY|portYIELD_FROM_ISR|portEND_SWITCHING_ISR' src/event_sources src/os* src
printf '%s\n' '--- read-only structural verifier ---'
python3 - <<'PY'
from pathlib import Path
spi_h = Path("src/resources/spi_esp32.h").read_text()
spi_cc = Path("src/resources/spi_esp32.cc").read_text()
ev_cc = Path("src/event_sources/ev_queue_esp32.cc").read_text()
checks = {
"SPI ISR helper records wake flag": "xQueueSendFromISR(queue(), &payload, &higher_was_woken);" in spi_h,
"SPI ISR helper ignores wake flag": "portYIELD_FROM_ISR" not in spi_h and "portEND_SWITCHING_ISR" not in spi_h,
"ready callback invokes helper": "resource->ready_from_isr();" in spi_cc,
"complete callback invokes helper": "resource->complete_from_isr();" in spi_cc,
"callbacks configured as standard SPI callbacks": ".post_setup_cb = spi_target_ready_callback," in spi_cc and ".post_trans_cb = spi_target_done_callback," in spi_cc,
"event source receives from resource queue": "if (receiver->receive_event(&data))" in ev_cc,
}
for name, value in checks.items():
print(f"{name}: {'PASS' if value else 'FAIL'}")
if not all(checks.values()):
raise SystemExit(1)
PYRepository: toitlang/toit
Length of output: 24688
🏁 Script executed:
#!/bin/bash
set -eu
sed -n '1,135p' src/event_sources/ev_queue_esp32.cc
rg -n -C5 'EventQueueEventSource|xTaskCreatePinnedToCore|tskIDLE_PRIORITY|portYIELD_FROM_ISR|portEND_SWITCHING_ISR' src/event_sources src/os* src
python3 - <<'PY'
from pathlib import Path
spi_h = Path("src/resources/spi_esp32.h").read_text()
spi_cc = Path("src/resources/spi_esp32.cc").read_text()
ev_cc = Path("src/event_sources/ev_queue_esp32.cc").read_text()
checks = {
"SPI ISR helper records wake flag": "xQueueSendFromISR(queue(), &payload, &higher_was_woken);" in spi_h,
"SPI ISR helper ignores wake flag": "portYIELD_FROM_ISR" not in spi_h and "portEND_SWITCHING_ISR" not in spi_h,
"ready callback invokes helper": "resource->ready_from_isr();" in spi_cc,
"complete callback invokes helper": "resource->complete_from_isr();" in spi_cc,
"callbacks configured as standard SPI callbacks": ".post_setup_cb = spi_target_ready_callback," in spi_cc and ".post_trans_cb = spi_target_done_callback," in spi_cc,
"event source receives from resource queue": "if (receiver->receive_event(&data))" in ev_cc,
}
for name, value in checks.items():
print(f"{name}: {'PASS' if value else 'FAIL'}")
if not all(checks.values()):
raise SystemExit(1)
PYRepository: toitlang/toit
Length of output: 24554
Yield from the SPI callbacks
If higher_was_woken == pdTRUE, call portYIELD_FROM_ISR() before signal_from_isr returns. The SPI callbacks wake EventQueueEventSource, which otherwise may not process the event until a tick or another ISR yield, adding latency to both ready and done events.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/resources/spi_esp32.h` around lines 159 - 166, Update signal_from_isr to
call portYIELD_FROM_ISR when xQueueSendFromISR sets higher_was_woken to pdTRUE,
placing the yield before the function returns while preserving the existing
critical-section and queue-send flow.
| CONFIG_SPI_MASTER_ISR_IN_IRAM=y | ||
| # CONFIG_SPI_SLAVE_IN_IRAM is not set | ||
| # CONFIG_SPI_SLAVE_ISR_IN_IRAM is not set | ||
| CONFIG_SPI_SLAVE_ISR_IN_IRAM=y |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target files and nearby configuration ---'
for f in toolchains/esp32p4/sdkconfig toolchains/esp32s2/sdkconfig; do
echo "### $f"
sed -n '985,1015p' "$f" 2>/dev/null || true
sed -n '728,758p' "$f" 2>/dev/null || true
done
printf '%s\n' '--- SPI and HAL IRAM symbols in tracked files ---'
rg -n --hidden -g '!node_modules' \
'SPI_SLAVE_ISR_IN_IRAM|HAL_SPI_SLAVE_FUNC_IN_IRAM|esp_driver_spi|ESP-IDF|IDF_VERSION' \
toolchains components 2>/dev/null || true
printf '%s\n' '--- candidate Kconfig and version files ---'
git ls-files | rg '(^|/)(Kconfig|sdkconfig|idf|esp-idf|version|manifest)' | head -200Repository: toitlang/toit
Length of output: 7354
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- vendored ESP-IDF tree ---'
git ls-files third_party/esp-idf | head -80
find third_party/esp-idf -type f \( -name Kconfig -o -name 'Kconfig.*' \) -print 2>/dev/null | head -120
printf '%s\n' '--- SPI Kconfig definitions and references ---'
rg -n -C 8 \
'SPI_SLAVE_ISR_IN_IRAM|HAL_SPI_SLAVE_FUNC_IN_IRAM|SPI_SLAVE_IN_IRAM' \
third_party/esp-idf toolchains 2>/dev/null || true
printf '%s\n' '--- all generated SPI symbols ---'
for f in toolchains/*/sdkconfig; do
printf '### %s\n' "$f"
rg -n 'CONFIG_(SPI_SLAVE_ISR_IN_IRAM|HAL_SPI_SLAVE_FUNC_IN_IRAM|SPI_SLAVE_IN_IRAM)=' "$f" || true
done
printf '%s\n' '--- target SDK headers ---'
for f in toolchains/*/sdkconfig; do
printf '### %s\n' "$f"
sed -n '1,8p' "$f"
doneRepository: toitlang/toit
Length of output: 18729
🏁 Script executed:
#!/bin/bash
set -eu
base='https://raw.githubusercontent.com/espressif/esp-idf/v5.4.2'
printf '%s\n' '--- ESP-Driver SPI Kconfig ---'
curl -fsSL "$base/components/esp_driver_spi/Kconfig" | \
rg -n -C 12 'SPI_SLAVE_ISR_IN_IRAM|SPI_SLAVE_IN_IRAM|HAL_SPI_SLAVE_FUNC_IN_IRAM' || true
printf '%s\n' '--- HAL Kconfig candidates ---'
for path in components/hal/Kconfig components/hal/Kconfig.projbuild components/esp_hal_spi/Kconfig components/esp_driver_spi/Kconfig.projbuild; do
echo "### $path"
curl -fsSL "$base/$path" 2>/dev/null | \
rg -n -C 12 'HAL_SPI_SLAVE_FUNC_IN_IRAM|SPI_SLAVE_ISR_IN_IRAM|SPI_SLAVE_IN_IRAM' || true
done
printf '%s\n' '--- submodule metadata ---'
git submodule status third_party/esp-idf 2>/dev/null || true
git ls-tree HEAD third_party/esp-idfRepository: toitlang/toit
Length of output: 3827
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- exact submodule revision Kconfig contract ---'
base='https://raw.githubusercontent.com/espressif/esp-idf/4a8004d3ec393c1020bd82373705c5b210eba17d'
curl -fsSL "$base/components/esp_driver_spi/Kconfig" | \
rg -n -C 5 'SPI_SLAVE_ISR_IN_IRAM|SPI_SLAVE_IN_IRAM' || true
curl -fsSL "$base/components/hal/Kconfig" | \
rg -n -C 5 'HAL_SPI_SLAVE_FUNC_IN_IRAM' || true
printf '%s\n' '--- deterministic inventory of generated configurations ---'
python3 - <<'PY'
from pathlib import Path
import re
for path in sorted(Path("toolchains").glob("*/sdkconfig")):
text = path.read_text()
isr = re.search(r"^CONFIG_SPI_SLAVE_ISR_IN_IRAM=(.*)$", text, re.M)
hal = re.search(r"^CONFIG_HAL_SPI_SLAVE_FUNC_IN_IRAM=(.*)$", text, re.M)
print(f"{path}: ISR={isr.group(1) if isr else '<unset>'}; "
f"HAL={hal.group(1) if hal else '<unset>'}")
PY
printf '%s\n' '--- configuration-generation and sdkconfig usage ---'
rg -n -C 4 \
'sdkconfig|idf.py|IDF_PATH|ESP-IDF|SDKCONFIG' \
Makefile CMakeLists.txt tools toolchains .github 2>/dev/null | head -300 || true
printf '%s\n' '--- repository status summary ---'
git status --short
git diff --statRepository: toitlang/toit
Length of output: 18812
Synchronize the SPI slave ISR and HAL IRAM settings.
ESP-IDF 5.4.2 selects CONFIG_HAL_SPI_SLAVE_FUNC_IN_IRAM when CONFIG_SPI_SLAVE_ISR_IN_IRAM=y. Regenerate these configurations with the vendored ESP-IDF Kconfig:
toolchains/esp32p4/sdkconfig#L1004toolchains/esp32s2/sdkconfig#L745toolchains/esp32c3/sdkconfig#L983toolchains/esp32c6/sdkconfig#L1117
The S2 configuration also uses ESP-IDF 5.4.2, not 5.3.1. Missing HAL IRAM placement can cause SPI slave operations to access flash while the cache is disabled.
📍 Affects 2 files
toolchains/esp32p4/sdkconfig#L1004-L1004(this comment)toolchains/esp32s2/sdkconfig#L745-L745
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@toolchains/esp32p4/sdkconfig` at line 1004, Regenerate the ESP-IDF Kconfig
settings so CONFIG_HAL_SPI_SLAVE_FUNC_IN_IRAM is enabled alongside
CONFIG_SPI_SLAVE_ISR_IN_IRAM in toolchains/esp32p4/sdkconfig at lines 1004-1004
and toolchains/esp32s2/sdkconfig at lines 745-745; use ESP-IDF 5.4.2 for the S2
configuration. Also apply the same generated HAL IRAM setting to
toolchains/esp32c3/sdkconfig at lines 983-983 and toolchains/esp32c6/sdkconfig
at lines 1117-1117.
0e63013 to
9e0c4bb
Compare
9e0c4bb to
b43a439
Compare
b43a439 to
3387cb2
Compare
3387cb2 to
b1f78a1
Compare
b1f78a1 to
3b956db
Compare
3b956db to
4cc7c1d
Compare
1 participant
Stacked on #3156.
This introduces an ESP32
spi.TargetAPI for transaction-sized controller/target exchanges. Native primitives only allocate, queue with a zero timeout, copy completed data, or tear down resources; waiting happens throughResourceState_and suspends only the calling Toit task.Highlights:
PendingExchangeso applications can assert a ready GPIO after the peripheral is armedThe required ESP-IDF fixes are in toitware/esp-idf#125, stacked on the I2C ESP-IDF series. The target API intentionally exposes complete transactions only; it does not add half-buffer streaming.