r3wretrhy opened a new pull request, #20541:
URL: https://github.com/apache/nuttx/pull/20541
## Summary
* Why change is necessary: `ESP32S3_SPIx_CSPIN`, `_MOSIPIN` and `_MISOPIN`
accept `-1` in Kconfig for boards that don't use the pin (#16085), and
`esp32s3_spi_init()` / `esp32s3_spi_select()` skip a pin when it is negative.
But `struct esp32s3_spi_config_s` stores those pins as `uint8_t`, so `-1`
becomes 255, every `pin >= 0` check is always true, and 255 ends up in
`esp_gpiowrite()` / `esp_configgpio()` (which assert `pin <
SOC_GPIO_PIN_COUNT`). The `CONFIG_ESP32S3_SPI_UDCS` fallback for a missing CS
pin is never reached either.
* What functional part of the code is being changed:
`arch/xtensa/src/esp32s3/esp32s3_spi.c`, the type of `cs_pin`, `mosi_pin` and
`miso_pin` in `esp32s3_spi_config_s`.
* How: store those three pins as `int8_t` so `-1` survives and the
existing checks work. `clk_pin` is not optional and stays `uint8_t`.
* Related NuttX Issue: https://github.com/apache/nuttx/issues/20186
* AI assistance: this change was prepared with AI assistance, disclosed
with the `Assisted-by:` trailer in the commit as CONTRIBUTING.md requires. The
`Signed-off-by:` is mine.
## Impact
* Is new feature added? Is existing feature changed? NO
* Impact on user (will user need to adapt to change)? NO
* Impact on build (will build process change)? NO
* Impact on hardware (will arch(s) / board(s) / driver(s) change)? YES,
ESP32-S3 SPI master driver only. Boards that set CS or MISO to `-1` (e.g.
`lckfb-szpi-esp32s3:lcd`, `esp32s3-m5-cardputer:fb`) no longer touch GPIO 255.
Boards with real pins are unchanged.
* Impact on documentation (is update required / provided)? NO
* Impact on security (any sort of implications)? NO
* Impact on compatibility (backward/forward/interoperability)? NO
## Testing
I confirm that changes are verified on local setup and works as intended:
* Build Host(s): Linux x86_64 (Debian 13), xtensa-esp-elf GCC 14.2.0
(esp-14.2.0_20241119)
* Target(s): `lckfb-szpi-esp32s3:lcd` (SPI2 CS=-1, MISO=-1, SWCS+UDCS),
`esp32s3-m5-cardputer:fb` (MISO=-1), `esp32s3-devkit:spi` (all pins set).
Build-tested only, I don't have one of these boards.
Testing logs before change (`lckfb-szpi-esp32s3:lcd`, `make
EXTRAFLAGS=-Wtype-limits`):
```
chip/esp32s3_spi.c:1427:22: warning: comparison is always true due to
limited range of data type [-Wtype-limits]
chip/esp32s3_spi.c:1446:24: warning: comparison is always true due to
limited range of data type [-Wtype-limits]
chip/esp32s3_spi.c:1451:24: warning: comparison is always true due to
limited range of data type [-Wtype-limits]
chip/esp32s3_spi.c:1459:22: warning: comparison is always true due to
limited range of data type [-Wtype-limits]
... (8 in esp32s3_spi.c)
```
and in the generated code `esp32s3_spibus_initialize()` loads `cs_pin`
with `l8ui` and calls `esp_gpiowrite()` / `esp_configgpio()` with no branch.
Testing logs after change:
```
make EXTRAFLAGS=-Wtype-limits # no esp32s3_spi.c warnings
```
the pins are sign-extended and checked (`sext` + `bltz`) before each GPIO
call, and the UDCS `esp32s3_spi2_select()` path is now reached when CS is -1.
All three configurations above build and link with no `esp32s3_spi.c` warnings
(rebuilt on current master). `tools/checkpatch.sh -c -u -m -g HEAD~1..HEAD`:
all checks pass.
## PR verification Self-Check
* [x] This PR introduces only one functional change.
* [x] I have updated all required description fields above.
* [x] My PR adheres to Contributing Guidelines and Documentation (git
commit title and message, coding standard, etc).
* [ ] My PR is still work in progress (not ready for review).
* [x] My PR is ready for review and can be safely merged into a codebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
