[spi] Document nRF52 hardware SPI#6810
Conversation
✅ Deploy Preview for esphome ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
As this is a feature matched with a PR in https://github.com/esphome/esphome, please target your PR to the next branch and rebase.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe SPI component documentation gains two nRF52-specific additions: a note in the "Interface selection" section clarifying that Zephyr exposes only ChangesnRF52 SPI documentation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 295ee03742
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| On nRF52 (Zephyr), a single hardware SPI bus (`spi2`) is available; the `interface:` option does not apply and only one | ||
| `spi:` bus may be configured. Assign `clk_pin`/`mosi_pin`/`miso_pin` to any free GPIOs. |
There was a problem hiding this comment.
Retarget unreleased nRF52 SPI docs
This PR is targeting current, but the parent ESPHome change it documents (esphome/esphome#17078) is still a draft against dev; publishing this on the released docs branch would tell current-release users that nRF52 SPI works even though that implementation is not shipped yet. The repo instructions/CONTRIBUTING say feature docs should target next, so please retarget this docs change or hold it until the feature is released.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This PR already targets next (its base branch is next, not current), which is the correct branch for unreleased feature docs per CONTRIBUTING. No retarget needed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Base branch has been corrected - dismissing previous review.
There was a problem hiding this comment.
As this is a feature matched with a PR in https://github.com/esphome/esphome, please target your PR to the next branch and rebase.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cc582895a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ESPHome binds the nRF52 SPI to Zephyr's spi2 node (DT_NODELABEL(spi2)), which the nRF52840 devicetree caps at 8 MHz (spi3 is the 32 MHz controller). Fix the data-rate table and interface note accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PLATFORM_SPI_CLOCKS[PLATFORM_NRF52] was 32 MHz, but ESPHome binds the nRF52 SPI to Zephyr's spi2 node (DEVICE_DT_GET(DT_NODELABEL(spi2)) = SPIM2), which nrf52840.dtsi caps at DT_FREQ_M(8) = 8 MHz. Only spi3 (SPIM3) supports 32 MHz. The reference clock drives _frequency_validator(): the old value let the validator accept 16/32 MHz rates that SPIM2 cannot deliver (the requested rate flows straight into Zephyr's spi_config.frequency, which the nrfx driver silently clamps), and it computed the "closest available" divisor grid against the wrong clock. 8 MHz both gates correctly and yields the divisor grid (8M/4M/2M/1M/500k/250k/125k) that matches SPIM2's natively supported frequencies. Matches the docs fix in esphome/esphome.io#6810: esphome/esphome.io#6810 (comment) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Documents nRF52 SPI support added in esphome/esphome#17078 (clock source 32 MHz; single fixed
spi2bus).