Add autonomous SPI buffer target

[floitsch]

Stacked on #3158.

This adds an ESP32 spi.BufferTarget for protocols that need the target to remain ready without waiting for a Toit task between transactions. It uses a fixed native response buffer and a bounded native queue of complete MOSI transactions. The completion callback performs no allocation and re-arms the same descriptor from the ISR; all primitives return without waiting.

Highlights:

• continuously armed response buffer with byte indexing plus bounded read and write operations
• complete received transactions retained in a configurable ring, with explicit dropped-transaction accounting
• suspended Toit receive waits through ResourceState_; native calls never wait for controller activity or queue space
• all native buffers, descriptors, queues, and ring storage are allocated before the target is armed, with cleanup on every retryable OOM path
• DMA/cache-line-safe buffers on cache-backed targets and classic ESP32 DMA restrictions at both API boundaries
• active-low CS pull-up during peer setup, early controller termination, response fill, and explicit response-update latency semantics for non-DMA hardware registers
• target ISR remains IRAM-safe on classic ESP32; controller ISR IRAM placement is disabled there to stay within its IRAM budget, since controller clocks can safely pause while flash cache is unavailable
• two-board ESP32 and ESP32-S3 coverage for response updates, short transactions, overflow, blocked receive wakeup, repeated ISR re-arm, independent directions, non-DMA operation, and 4092-byte DMA transfers

The implementation uses ESP-IDF’s private spi_slave_queue_trans_isr entry point. It is present with the same contract in the recent ESP-IDF checkout and is also used by ESP-IDF components. No new ESP-IDF patch is needed for this layer.

This deliberately remains a complete-transaction API. It does not add half-buffer streaming or watermark callbacks.

[floitsch]

@CodeRabbit review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Walkthrough

Adds BufferTarget for autonomous buffered SPI target operation on ESP32. The change includes native resource management, continuous transaction re-arming, receive queuing, primitive wiring, SDK configuration updates, and two-board hardware integration tests.

Changes

Buffered SPI Target

Layer / File(s)	Summary
Public API and primitive wiring lib/spi.toit, src/compiler/propagation/type_primitive_spi.cc, src/primitive.h	Adds BufferTarget with response access, bulk reads and writes, queued receive operations, dropped-receive counts, and cleanup. Registers the corresponding native primitives and resource argument validation.
Resource lifecycle and buffer operations src/resources/spi_esp32.h, src/resources/spi_esp32.cc, src/tags.h	Adds SpiBufferTargetResource, target allocation, validation, GPIO ownership, CS pull-up setup, synchronized buffer operations, cleanup, and resource registration.
Transaction re-arming and interrupt configuration src/resources/spi_esp32.cc, toolchains/esp32/sdkconfig	Processes completed transactions, re-arms the SPI target, queues received data, signals receive events, counts drops, and updates SPI interrupt IRAM configuration.
Hardware integration validation tests/hw/esp32/spi-buffer-target-*.toit	Adds two-board tests for response handling, receive queues, overflow, blocking receive, DMA direction, partial transfers, maximum buffer sizes, and target lifecycle commands.

Estimated code review effort: 4 (Complex) | ~60 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely identifies the main change: adding an autonomous SPI buffer target.
Description check	✅ Passed	The description directly explains the new ESP32 SPI BufferTarget, its design, constraints, and test coverage.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch floitsch/spi.20-buffer-target

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

tests/hw/esp32/spi-buffer-target-shared.toit (1)

215-239: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Guard the command handlers against a null target.

WRITE, SET, READ, RECEIVE, and DROPPED use target without a null check. If a command arrives before CREATE, or after CLOSE sets target to null at Line 242, the test fails with a null dereference instead of a clear protocol error.

Add one check at the top of the loop for the commands that require a target.

♻️ Proposed guard

     else if command == WRITE:
       offset := port.in.little-endian.read-uint32
       size := port.in.little-endian.read-uint32
+      if not target: throw "No target for command: $command"
       target.write offset (port.in.read-bytes size)
       port.out.write #[DONE] --flush

🤖 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 `@tests/hw/esp32/spi-buffer-target-shared.toit` around lines 215 - 239, Add a
guard at the top of the command-processing loop for target-dependent commands
WRITE, SET, READ, RECEIVE, and DROPPED, rejecting them with the test’s
established protocol error when target is null before reaching target.write,
target[], target.read, target.receive, or target.dropped-receive-count. Leave
commands that do not require a target unchanged.

🤖 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 `@src/resources/spi_esp32.cc`:
- Around line 224-241: Update complete_from_isr to reserve an available
receive-ring slot under spinlock_, copy received bytes outside the critical
section, then reacquire spinlock_ to publish receive_lengths_ and increment
receive_count_. Add and maintain a separate receive_reserved_ counter so
concurrent completions cannot claim the same capacity, while preserving
dropped_receive_count_ handling when no slot can be reserved.

---

Nitpick comments:
In `@tests/hw/esp32/spi-buffer-target-shared.toit`:
- Around line 215-239: Add a guard at the top of the command-processing loop for
target-dependent commands WRITE, SET, READ, RECEIVE, and DROPPED, rejecting them
with the test’s established protocol error when target is null before reaching
target.write, target[], target.read, target.receive, or
target.dropped-receive-count. Leave commands that do not require a target
unchanged.

🪄 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: 0f922544-02a8-44c7-b499-97365e34dd3c

📥 Commits

Reviewing files that changed from the base of the PR and between fe266ce and 2bbefb0.

📒 Files selected for processing (10)

• lib/spi.toit
• src/compiler/propagation/type_primitive_spi.cc
• src/primitive.h
• src/resources/spi_esp32.cc
• src/resources/spi_esp32.h
• src/tags.h
• tests/hw/esp32/spi-buffer-target-board1.toit
• tests/hw/esp32/spi-buffer-target-board2.toit
• tests/hw/esp32/spi-buffer-target-shared.toit
• toolchains/esp32/sdkconfig

[coderabbitai]

🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift

Reduce the interrupt-disabled copy in complete_from_isr.

The byte copy at Line 233 runs inside portENTER_CRITICAL_ISR. It can copy up to buffer_size_ bytes, which is 4092 in the DMA configuration. Interrupts stay disabled for the whole copy on that core.

take_receive deliberately avoids this pattern and documents the reason at Lines 192-194. Apply the same approach here: reserve the slot under the spinlock, copy outside it, then publish the length and increment receive_count_ under the spinlock.

♻️ Sketch of the reservation approach

   portENTER_CRITICAL_ISR(&spinlock_);
-  if (receive_buffer_ != null && received != 0) {
-    if (receive_count_ < receive_queue_depth_) {
-      uint32_t index = receive_head_ + receive_count_;
-      if (index >= receive_queue_depth_) index -= receive_queue_depth_;
-      uint8_t* destination = receive_ring_ + index * buffer_size_;
-      for (size_t i = 0; i < received; i++) destination[i] = receive_buffer_[i];
-      receive_lengths_[index] = received;
-      receive_count_++;
-      enqueued = true;
-    } else if (dropped_receive_count_ != Smi::MAX_SMI_VALUE) {
+  uint32_t reserved = 0;
+  bool has_slot = false;
+  if (receive_buffer_ != null && received != 0) {
+    if (receive_count_ < receive_queue_depth_) {
+      reserved = receive_head_ + receive_count_;
+      if (reserved >= receive_queue_depth_) reserved -= receive_queue_depth_;
+      has_slot = true;
+    } else if (dropped_receive_count_ != Smi::MAX_SMI_VALUE) {
       dropped_receive_count_++;
     }
   }
   portEXIT_CRITICAL_ISR(&spinlock_);
+  if (has_slot) {
+    uint8_t* destination = receive_ring_ + reserved * buffer_size_;
+    for (size_t i = 0; i < received; i++) destination[i] = receive_buffer_[i];
+    portENTER_CRITICAL_ISR(&spinlock_);
+    receive_lengths_[reserved] = received;
+    receive_count_++;
+    portEXIT_CRITICAL_ISR(&spinlock_);
+    enqueued = true;
+  }

Note that the slot must stay reserved against a second completion. Track a separate receive_reserved_ counter, or keep the current design and document the latency cost.

🤖 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 224 - 241, Update complete_from_isr
to reserve an available receive-ring slot under spinlock_, copy received bytes
outside the critical section, then reacquire spinlock_ to publish
receive_lengths_ and increment receive_count_. Add and maintain a separate
receive_reserved_ counter so concurrent completions cannot claim the same
capacity, while preserving dropped_receive_count_ handling when no slot can be
reserved.