casaroli opened a new pull request, #21366: URL: https://github.com/apache/nuttx/pull/21366
## Summary This PR fixes four defects in the WS2812 drivers, one commit each. **drivers/leds/ws2812: Declare ws2812_open() and ws2812_close() as int.** With `CONFIG_WS2812_NON_SPI_DRIVER`, `ws2812.c` declared `ws2812_open()` and `ws2812_close()` as `ssize_t`, defined `ws2812_close()` as `int`, and put both in `struct file_operations`, which takes `int`. This compiles only where `ssize_t` and `int` are the same type. On 64-bit architectures (for example qemu-armv8a) the upper half does not build: "conflicting types for 'ws2812_close'" and incompatible pointer types in `g_ws2812fops`. Both functions are now `static int`. **drivers/leds/ws2812: Bound the byte order transformation to the LED chain.** With `CONFIG_WS2812_COMPONENT_TRANSFORM`, a device with a byte order transformation sends the pixel data through `byte_order_buffer`, which holds `WS2812_RW_PIXEL_SIZE * nleds` bytes. `ws2812_write()` checked only `len % 4` and transformed the full request into this buffer, so a write past the end of the LED chain overflowed the heap buffer. The lower half limits the length only after this copy. `ws2812_read()` transformed `len >> 2` pixels also when the lower half returned fewer bytes or an error, so it copied memory after the buffer to the caller. The length is now limited to the LEDs from the file position to the end of the chain before the transformation, as the lower halves do (0 at the end of the chain). The read path transforms only the bytes that the lower half returned. The transformation came with #21338. **arch/avr/src/avrdx: Leave the critical section when no swint vector is free.** `avrdx_swint_allocate()` returned 0 without `leave_critical_section()` when all software interrupt vectors were in use, so the caller continued with interrupts disabled. **arch/avr/src/avrdx: Fix the error path of avrdx_ws2812_register().** When `avrdx_swint_allocate()` found no free vector, `avrdx_ws2812_register()` freed the lane and returned `OK`, so the board code took the lane as registered. It now returns `-ENOMEM`. The error path also left the per-lane global buffer pointers set after it freed the buffers; it now clears them. ## Impact Commit 1: no change where `ssize_t` and `int` are the same type (all in-tree users of the non-SPI driver: rp2040, rp23xx, Espressif RMT, AVR DA/DB). 64-bit builds with `CONFIG_WS2812_NON_SPI_DRIVER` now compile. Commit 2: only builds with `CONFIG_WS2812_NON_SPI_DRIVER` and `CONFIG_WS2812_COMPONENT_TRANSFORM`, and a lower half that sets `byte_order_transformation`. No in-tree lower half sets it today. Without the option, or with `WS2812_COMPONENT_TRANSFORM_NONE`, the code path does not change. A write or read past the end of the chain now returns the clamped byte count, the same as the lower halves already return. Commits 3 and 4: AVR DA/DB only, and only when no software interrupt vector is free. No API or ABI change. No documentation change (the header already gives the buffer size; the AVR page lists `ENOMEM` for allocation failures). ## Testing Base: master 4a98cd7b44f. Builds with `EXTRAFLAGS="-Wno-cpp -Werror"`: - qemu-armv8a:nsh + `WS2812` + `WS2812_NON_SPI_DRIVER`: master fails (5 errors in `leds/ws2812.c`), this PR builds without warnings; also with `WS2812_COMPONENT_TRANSFORM` + `_RGB`. - xiao-rp2350:combo (non-SPI rp23xx lower half), with and without `WS2812_COMPONENT_TRANSFORM` + `_RGB`: OK. - stm32f401rc-rs485:ws2812 (SPI variant): OK. - breadxavr:ws2812 (avr-gcc 15.1.0), and breadxavr:ws2812 with reads on, `SHORT_LANE` off and `WS2812_COMPONENT_TRANSFORM` + `_RGB`: OK, 0 warnings (`avrdx_swint.c`, `avrdx_ws2812.c` and `leds/ws2812.c` compiled). Runtime (QEMU, qemu-armv8a:knsh kernel build and qemu-armv8a:nsh flat build): no in-tree lower half sets `byte_order_transformation`, so a test-only fake lower half in the board code registered `/dev/leds0` (8 LEDs, `WS2812_COMPONENT_TRANSFORM_RGB`) and `/dev/leds1` (8 LEDs, no transformation). Its buffers and `byte_order_buffer` have a guard area after them, and a write or read that finds the guard changed fails with `EIO`. A test program writes 12 pixels from position 0, writes 3 pixels from pixel 7, reads 12 pixels, then reads and writes at the end of the chain. Master needed the commit 1 change to build on arm64. | Run | master | this PR | |---|---|---| | knsh, transform on | `/dev/leds0`: write of 12 pixels overwrites 16 guard bytes (`EIO`); read of 12 pixels returns 32 bytes plus 16 guard bytes after them; read at the end returns 0 but copies 16 stale bytes; `/dev/leds1` 5/5 | 10/10 pass: writes and reads return 32, 4 and 0 bytes, no guard change, nothing copied after the returned data | | nsh (flat), transform on | same as knsh | 10/10 pass | | nsh (flat), transform off | 10/10 pass | 10/10 pass (unchanged) | | ostest | 0 in all runs | 0 in all runs | Commits 3 and 4 were build-tested only (no AVR DA/DB board, and QEMU has no AVR DA model). -- 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]
