ricardgb opened a new pull request, #19785:
URL: https://github.com/apache/nuttx/pull/19785
## Summary
Two build-level defects around the APA102 RGB-LED driver, found while
bringing an APA102 strip up on an ESP32-S3 board.
### 1. `drivers/lcd/apa102.c` and `drivers/leds/apa102.c` collide on one
object name
`drivers/` is built by a single flat `Makefile`. Each per-directory
`Make.defs` appends to one shared `CSRCS` list and one shared `VPATH`, and
every object lands in `drivers/` named after the *basename* of its source. Two
sources with the same basename in different subdirectories therefore map to the
same object file, and make resolves the prerequisite through `VPATH`, which is
searched in the order `drivers/Makefile` includes the `Make.defs` files. `lcd`
is included before `leds`, so `drivers/lcd` always wins.
Two part names exist in both directories:
| LCD front-end | option | LED driver | option |
|---|---|---|---|
| `drivers/lcd/apa102.c` | `CONFIG_LCD_APA102` | `drivers/leds/apa102.c` |
`CONFIG_LEDS_APA102` |
| `drivers/lcd/max7219.c` | `CONFIG_LCD_MAX7219` | `drivers/leds/max7219.c`
| `CONFIG_LEDS_MAX7219` |
`drivers/lcd/Make.defs` adds `lcd` to the `VPATH` for the whole directory
whenever `CONFIG_LCD=y`, so selecting only the LED driver still compiles the
*LCD* source into `apa102.o` / `max7219.o` and the selected LED driver is never
compiled at all. Because the LCD front-ends take their constants from
`include/nuttx/lcd/apa102.h` / `include/nuttx/lcd/max7219.h`, which are guarded
by `CONFIG_LCD_APA102` / `CONFIG_LCD_MAX7219`, the substituted source does not
even compile. With `CONFIG_LCD=y` + `CONFIG_LEDS_APA102=y` and
`CONFIG_LCD_APA102` unset:
```
lcd/apa102.c:701:20: error: 'APA102_BLACK' undeclared (first use in this
function); did you mean 'APA102_BPP'?
701 | memset(priv->fb, APA102_BLACK, 4 * APA102_FBSIZE);
```
and the same for `CONFIG_LEDS_MAX7219` without `CONFIG_LCD_MAX7219`
(`'MAX7219_BLACK' undeclared`, plus `MAX7219_POWER_OFF`, `MAX7219_SHUTDOWN`,
…). Net effect: **neither LED driver can be built in any configuration that
also enables `CONFIG_LCD`**, and the error message points at a file the user
never selected.
Fix: give the LCD front-ends distinct basenames (`apa102_lcd.c`,
`max7219_lcd.c`, via `git mv` so history follows). The LCD side is the adapted
use of these parts — an LED matrix driven as a display — and `drivers/lcd`
already names such variants for their role (e.g. `ht16k33_14seg.c`), so the
suffix goes there and the LED drivers keep the plain part names.
`drivers/lcd/Make.defs` and `drivers/lcd/CMakeLists.txt` are both updated, plus
the two `.github/CODEOWNERS` lines.
The CMake build was never affected: `target_sources()` resolves relative
paths against the current source directory and CMake namespaces object paths
per directory, so both `apa102.c` files can coexist there. Only the Make build
is broken. The CMake source lists are updated purely to match the new filenames.
**Other duplicate basenames in `drivers/`** — the only remaining one is
`skeleton.c`, which exists four times (`ioexpander/`, `lcd/`, `mtd/`, `net/`).
Three of those are unbuilt driver templates; only `drivers/net/skeleton.c` is
ever added to `CSRCS` (`CONFIG_NET_SKELETON`), and because `lcd` precedes `net`
in the include order it would be shadowed by `drivers/lcd/skeleton.c` in any
`CONFIG_LCD=y` build. That is the same latent defect, but it only affects a
template driver, so it is left out of this PR to keep the change focused —
happy to rename those too if maintainers prefer.
### 2. The APA102 LCD driver ignores its own Kconfig settings
`drivers/lcd/Kconfig` offers `CONFIG_LCD_APA102_XRES`,
`CONFIG_LCD_APA102_YRES` and `CONFIG_LCD_APA102_FREQUENCY` under `if
LCD_APA102`, but the driver tests `CONFIG_APA102_XRES`, `CONFIG_APA102_YRES`
and `CONFIG_APA102_FREQUENCY` — names no Kconfig file in the tree defines. The
`#ifndef` fallbacks therefore always win and the geometry is hard-wired to
16x16 regardless of configuration. The frequency setting is doubly dead: even
the fallback is unused, because `apa102_configspi()` passes
`APA102_SPI_MAXFREQUENCY` from `include/nuttx/leds/apa102.h`, which is 100 kHz
(its `/* Default 4MHz */` comment notwithstanding), so the chain is always
clocked at 100 kHz.
Fix: use the names Kconfig actually defines, and drive the bus at the
configured frequency. The `#ifndef` fallbacks are kept for an out-of-Kconfig
build and aligned with the Kconfig defaults; 16x16 preserves the previous
geometry for anyone who never set the options.
## Impact
- Configurations with `CONFIG_LCD=y` together with `CONFIG_LEDS_APA102=y` or
`CONFIG_LEDS_MAX7219=y` build for the first time, and build the driver that was
actually selected.
- `CONFIG_LCD_APA102` / `CONFIG_LCD_MAX7219` configurations are unchanged
apart from the source filename — same code, same symbols (`apa102_initialize`,
`max7219_initialize`).
- No public header, Kconfig symbol, or API changes; nothing outside
`drivers/lcd` references these `.c` files by name (board glue links against the
headers).
- The `CONFIG_LCD_APA102_*` settings start taking effect. For a
configuration that never set them the behaviour is identical except for the SPI
clock, which moves from the hard-coded 100 kHz to the Kconfig default of 1 MHz.
## Testing
Build-verified on `stm32f4discovery:nsh` (arm-none-eabi-gcc, no new
warnings). `drivers/Make.dep` is quoted because it records which source each
object was actually built from:
```
# Case A: CONFIG_LCD=y, CONFIG_LEDS_APA102=y, CONFIG_LEDS_MAX7219=y,
LCD_APA102/LCD_MAX7219 unset
# before: build fails -> lcd/apa102.c:701: 'APA102_BLACK' undeclared
# after:
$ make -j4 # succeeds
$ grep '^apa102.o:\|^max7219.o:' drivers/Make.dep
apa102.o: leds/apa102.c \
max7219.o: leds/max7219.c \
$ nm drivers/apa102.o | grep ' T ' -> T apa102_register
$ nm drivers/max7219.o | grep ' T ' -> T max7219_leds_register
# Case B: CONFIG_LCD_APA102=y (LEDS_APA102 unset)
$ make -j4 # succeeds
$ nm drivers/apa102_lcd.o | grep ' T ' -> T apa102_initialize
# Case C: all four selected at once - impossible before, links now
$ make -j4 # succeeds
apa102.o: leds/apa102.c apa102_lcd.o -> T apa102_initialize
max7219.o: leds/max7219.c max7219_lcd.o -> T max7219_initialize
# Kconfig plumbing: CONFIG_LCD_APA102_XRES=8 YRES=4 FREQUENCY=4000000
# g_apa102dev shrinks to an 8x4 shadow framebuffer and the SPI_SETFREQUENCY
# argument is 0x003d0900 (4 MHz); before the change both settings were
ignored.
```
`tools/checkpatch.sh -g` on the two commits: all checks pass.
The `drivers/leds/apa102.c` LED-strip path itself was exercised on hardware
today (APA102 on an ESP32-S3 SPI bus, `apps/examples/apa102` writing a colour
cycle to `/dev/leddrv0`) — that is what surfaced the collision. The change in
this PR is **build-verified only**; the APA102/MAX7219 *LCD* front-ends are
pure renames plus the Kconfig-name fix and were not exercised on a panel.
## Disclosure
This analysis, patch, and build validation were performed by an AI agent
(Claude Code, operated and directed by the submitter), and the result was
reviewed by the submitter before posting.
--
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]
