Add LilyGO T-Deck Max (e-paper) base board support#552
Open
Crazypedia wants to merge 2 commits into
Open
Conversation
Base board bring-up for the LilyGO T-Deck Max (ESP32-S3, 16 MB flash, 8 MB PSRAM), per the plan discussed in TactilityProject#539: display, touch, and keyboard only — power, LoRa radio, GPS, and haptics follow as separate PRs. New reusable drivers: - Drivers/GDEQ031T10: UC8253-based 240x320 1bpp e-paper panel with windowed partial refresh + full-refresh ghost gate, refresh runs on a background task. - Drivers/xl9555-module: XL9555 I2C IO expander as a kernel module with gpio_controller bindings (the board routes touch/keyboard resets and several power rails through it). Board specifics: - CST66xx touch (Hynitron 4-byte register protocol) implemented device-locally; vendor docs mislabel the part as CST328. - TCA8418 keyboard matrix (existing upstream driver) with backlight. - SD card as an espressif,esp32-sdspi devicetree node on the shared SPI2 bus; unused chip-selects (LoRa, SD) are deasserted before the EPD touches the bus. - incubating=true until SD storage is verified on this board. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
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 (10)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughThis PR adds an 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f99c293d-2954-42e0-b773-bd1e5c7308dc
📒 Files selected for processing (26)
Devices/lilygo-tdeck-max/CMakeLists.txtDevices/lilygo-tdeck-max/Source/Configuration.cppDevices/lilygo-tdeck-max/Source/devices/Cst66xxTouch.cppDevices/lilygo-tdeck-max/Source/devices/Cst66xxTouch.hDevices/lilygo-tdeck-max/Source/devices/Display.cppDevices/lilygo-tdeck-max/Source/devices/Display.hDevices/lilygo-tdeck-max/Source/devices/TdeckmaxKeyboard.cppDevices/lilygo-tdeck-max/Source/devices/TdeckmaxKeyboard.hDevices/lilygo-tdeck-max/Source/module.cppDevices/lilygo-tdeck-max/device.propertiesDevices/lilygo-tdeck-max/devicetree.yamlDevices/lilygo-tdeck-max/lilygo,tdeck-max.dtsDrivers/GDEQ031T10/CMakeLists.txtDrivers/GDEQ031T10/README.mdDrivers/GDEQ031T10/Source/Gdeq031t10Display.cppDrivers/GDEQ031T10/Source/Gdeq031t10Display.hDrivers/xl9555-module/CMakeLists.txtDrivers/xl9555-module/LICENSE-Apache-2.0.mdDrivers/xl9555-module/README.mdDrivers/xl9555-module/bindings/xlsemi,xl9555.yamlDrivers/xl9555-module/devicetree.yamlDrivers/xl9555-module/include/bindings/xl9555.hDrivers/xl9555-module/include/drivers/xl9555.hDrivers/xl9555-module/include/xl9555_module.hDrivers/xl9555-module/source/module.cppDrivers/xl9555-module/source/xl9555.cpp
Comment on lines
+36
to
+50
| void Gdeq031t10Display::writeCommand(uint8_t command) { | ||
| gpio_set_level(configuration->pinDc, 0); | ||
| spi_transaction_t transaction = {}; | ||
| transaction.length = 8; | ||
| transaction.tx_buffer = &command; | ||
| spi_device_polling_transmit(spiDevice, &transaction); | ||
| } | ||
|
|
||
| void Gdeq031t10Display::writeData(const uint8_t* data, size_t length) { | ||
| gpio_set_level(configuration->pinDc, 1); | ||
| spi_transaction_t transaction = {}; | ||
| transaction.length = length * 8; | ||
| transaction.tx_buffer = data; | ||
| spi_device_polling_transmit(spiDevice, &transaction); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## File outline\n'
ast-grep outline Drivers/GDEQ031T10/Source/Gdeq031t10Display.cpp --view expanded || true
printf '\n## Relevant source slice\n'
sed -n '1,140p' Drivers/GDEQ031T10/Source/Gdeq031t10Display.cpp
printf '\n## SPI init / device setup search\n'
rg -n "spi_bus_initialize|spi_bus_add_device|spi_device_polling_transmit|spi_device_transmit|SPI_DMA|dma_chan|dma_channel|spi_transaction_t" Drivers . -g '!**/build/**' -g '!**/dist/**' -g '!**/node_modules/**'Repository: TactilityProject/Tactility
Length of output: 7019
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## Gdeq031t10Display.cpp around SPI device setup\n'
sed -n '380,430p' Drivers/GDEQ031T10/Source/Gdeq031t10Display.cpp
printf '\n## Gdeq031t10Display.cpp call sites for writeData/writeCommand\n'
rg -n "writeData\\(|writeCommand\\(" Drivers/GDEQ031T10/Source/Gdeq031t10Display.cpp
printf '\n## ESP32 SPI bus initialization\n'
sed -n '1,180p' Platforms/platform-esp32/source/drivers/esp32_spi.cppRepository: TactilityProject/Tactility
Length of output: 10073
Check SPI transmit errors here
spi_device_polling_transmit() is ignored in both helpers. If a command/data write fails, the driver still advances panelMode/panelPowerOn and the framebuffer state as if the transfer succeeded, which can leave the panel and software state out of sync. Return an error from these helpers and stop the init/refresh path on failure.
| tt::app::stop(); | ||
| break; | ||
| case BEZEL_KEY_SPEECH: // Home: the launcher | ||
| tt::app::start("Launcher"); |
Contributor
There was a problem hiding this comment.
This will create a memory leak, as it adds a new launcher on the stack.
I see 2 options forward:
- Implement it by stopping all apps except the last one. The last one (first started app) is assumed to be the launcher.
- Not binding any functionality to the button
| tt::app::start("Launcher"); | ||
| break; | ||
| case BEZEL_KEY_AIRPLANE: // Recents: the app switcher | ||
| tt::app::start("AppList"); |
Contributor
There was a problem hiding this comment.
This could be problematic with memory usage. Some users will inevitably use this as the main means to navigate to apps and it will keep adding AppList instances and other app instances on the stack and memory will drain.
It might be nice in the future if we can have some kind of overlay, but that's too complex for now (I want to refactor the window manager layer).
| }; | ||
| gpio_config(&config); | ||
| gpio_set_level(LORA_PIN_CS, 1); | ||
| gpio_set_level(SD_PIN_CS, 1); |
Contributor
There was a problem hiding this comment.
esp32_spi_deselect_all_cs() would be better for this. If that doesn't work, the above logic is ok.
| createDisplay() | ||
| }; | ||
|
|
||
| if (i2c != nullptr) { |
Contributor
There was a problem hiding this comment.
You can remove the if () here and do a check(i2c) instead, as we can always expect it to be there.
|
|
||
| i2c0 { | ||
| // i2c-master (not the legacy i2c driver): the CST66xx touch code reads | ||
| // the panel through the new i2c-master bus handle. |
Contributor
There was a problem hiding this comment.
This comment can be removed as it doesn't add much.
| host = <SPI2_HOST>; | ||
| // All chip-selects on the shared bus, by index. The SPI driver drives | ||
| // every CS high on start, so each peripheral is deselected before any | ||
| // other one transacts. The SD card resolves its own CS from index 1. |
Contributor
There was a problem hiding this comment.
This comment can be removed.
| // Replaces the old SpiSdCardDevice. CS comes from cs-gpios[1]. | ||
| // "disabled" => constructed/added but not auto-started; Hal::init's | ||
| // mountAll() starts it before displays init (SPI displays must come | ||
| // up after the SD card is mounted on the shared bus). |
Contributor
There was a problem hiding this comment.
This comment can be removed.
| LOG_E(TAG, "Parent device is not I2C"); | ||
| return ERROR_RESOURCE; | ||
| } | ||
| LOG_I(TAG, "Started XL9555 device %s", device->name); |
Contributor
There was a problem hiding this comment.
This can be removed. I think this is already logged by the parent that calls it.
| if (device_get_type(parent) != &I2C_CONTROLLER_TYPE) { | ||
| LOG_E(TAG, "Parent device is not I2C"); | ||
| return ERROR_RESOURCE; | ||
| } |
Contributor
There was a problem hiding this comment.
This can crash hard with a check(device_get_type(parent) == &I2C_CONTROLLER_TYPE), instead of an if ()
Contributor
|
Thanks, that looks good! I left some comments on a few things, but I didn't find anything major. I am quite busy in the coming 24 hours, so my responses will be a bit slower. |
…ject#552 GDEQ031T10: - waitWhileBusy() now times out (5s) instead of spinning forever on a stuck BUSY pin, which previously could hang the refresh task and make stop() un-recoverable. - writeCommand()/writeData() and all callers now check the SPI transfer result instead of advancing panel/shadow state as if it always succeeds. - start() no longer marks the display powered/initialized before the refresh task is confirmed created; on task-creation failure it now unwinds the SPI device and allocated buffers instead of leaking them. - `powered` and `currentRefreshMode` are now atomic (read on the refresh task, written from arbitrary caller threads). - Corrected a stale doc comment on setRefreshMode() (not one-shot). - README updated to describe the actual windowed partial-refresh pipeline instead of the old full-refresh-every-flush description. TdeckmaxKeyboard: - Modifier state (shift/sym/caps) moved from function-local statics to instance members. - Keyboard queue send is now non-blocking (drops+logs on a full queue) instead of blocking inside the periodic input timer callback. - initBacklight() now returns false when channel config fails, instead of reporting success. xl9555: - set_flags() rejects GPIO_FLAG_ACTIVE_LOW combined with GPIO_FLAG_DIRECTION_OUTPUT: the polarity register only affects input reads, so it silently did nothing for outputs. - set_level()/set_flags() now serialize their read-modify-write register updates with device_lock()/device_unlock(), since the shared i2c_controller_register8_{set,reset}_bits() helpers do a separate read then write. - start() uses check() instead of a soft parent-type check, and drops a log line the parent already emits. Configuration.cpp: i2c0 is unconditionally declared in this board's devicetree, so createDevices() now uses check(i2c) instead of an if, matching the "trust internal guarantees" convention. Cst66xxTouch: unbind the Home/Recents bezel keys. Both called tt::app::start(), which pushes a new app-stack instance on every press; repeated presses would leak Launcher/AppList instances indefinitely. Left unbound until the app stack has a way to resume an existing instance instead of pushing a new one. device.properties / dts: corrected the product name to "T-Deck Max" (was "T-Deck Pro Max") to match the rest of the PR, the vendor repo, and LilyGO's actual naming. Verified with a full idf.py build for lilygo-tdeck-max (IDF 5.5.2). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Crazypedia
added a commit
to Crazypedia/Tactility-lilygo-tdeck-max
that referenced
this pull request
Jul 6, 2026
…-max Follow-up to TactilityProject#552 (base board). Battery metrics come from the BQ27220 fuel gauge (I2C 0x55); charging status, charge control, and power-off (ship mode via SY6970 REG09 BATFET_DIS) use the SY6970 charger (0x6A — newer T-Deck Max revisions use this instead of the older BQ25896 at 0x6B, confirmed by I2C scan; the two are register-compatible for everything used here). Enables the launcher's power-off button. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #539. This is PR 1 of the incremental plan discussed there: base board (display + touch + keyboard) first, with power, LoRa radio, GPS, and haptics to follow as separate PRs once this lands.
What this adds
Board:
Devices/lilygo-tdeck-max— LilyGO T-Deck Max (ESP32-S3, 16 MB flash, 8 MB PSRAM), the e-paper T-Deck variant (vendor repoXinyuan-LilyGO/T-Deck-MAX).incubating=trueper your suggestion in #539, pending fuller SD-card verification (see below).New reusable drivers:
Drivers/GDEQ031T10— generic UC8253-based e-paper panel driver (240x320, 1bpp), with windowed partial refresh and a full-refresh ghost-clear gate (ported from the vendor's GxEPD2 waveforms). Refresh runs on a background task so the ~0.5-3s panel update never blocks LVGL/input.Drivers/xl9555-module— XL9555 I2C IO expander as a kernel module withgpio_controllerbindings. This supersedes Add XL9555 I/O expander kernel module #537, which I'd closed before it was hardware-tested; it's now verified as part of this board (it gates the touch/keyboard reset lines).Board-local:
espressif,esp32-sdspidevicetree node on the shared SPI2 bus (per yourdevelop-branch SD rework). Unused chip-selects (LoRa, SD) are deasserted before the EPD touches the bus so a floating CS can't latch stray command bytes.Radio (SX1262), power (SY6970/BQ27220), GPS, and haptics (DRV2605) are all deliberately left out of this PR — they're built and working on my fork already, just being split into separate PRs per the plan in #539.
Verification
TactilityFreeRtosTests,TactilityKernelTests,TactilityTests,CryptModuleTests) pass.Known caveat (not from this PR)
During hardware testing I hit a case where touch stopped responding to new taps a few minutes into a session (confirmed via the live trace: the touch controller returned the exact same I2C frame on every subsequent poll — i.e. latched/frozen). This matches a quirk from earlier bring-up: a warm reset or reflash does not fully power-cycle the CST66xx, so it can occasionally need a full power-off to clear. Not something this PR's code can fix from the host side; documenting here for visibility. Happy to discuss defensive options (e.g. a periodic touch-health check with auto re-init) as a follow-up if useful.
🤖 Generated with Claude Code
Summary by CodeRabbit