Fix unbounded BUSY-pin spin in Panel_IT8951::_write_args() - #246
Merged
lovyan03 merged 3 commits intoAug 13, 2026
Merged
Conversation
_write_args() polled the IT8951's BUSY GPIO between argument words with a raw while(!gpio_in(pin)) loop, unlike every other BUSY wait in this file. If BUSY gets stuck asserted (seen after a panel power brownout, e.g. switching an M5Paper's e-paper supply between USB and battery), this spins forever with no recovery short of a power cycle, hanging the whole calling stack. _write_args() can't just call the existing _wait_busy() helper because it toggles chip-select, while _write_args() must hold CS asserted continuously across a multi-word burst. Extract the bounded, timeout-based poll out of _wait_busy() into a new CS-free helper, _wait_busy_pin(timeout = 4096), matching the timeout already used elsewhere in this file. _wait_busy() now wraps it with CS handling exactly as before; _write_args() calls it directly per word and bails out instead of spinning forever. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR prevents Panel_IT8951::_write_args() from potentially hanging forever by replacing an unbounded BUSY-pin spin with a bounded, timeout-based poll that preserves the IT8951 multi-word CS-burst requirements.
Changes:
- Extracted a CS-free BUSY polling helper (
_wait_busy_pin(timeout)) with the same default timeout used elsewhere. - Refactored
_wait_busy()to wrap_wait_busy_pin()while preserving its CS toggling behavior for existing callers. - Updated
_write_args()to use_wait_busy_pin()between argument words and fail out on timeout instead of spinning indefinitely.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/lgfx/v1/panel/Panel_IT8951.hpp | Declares the new _wait_busy_pin() helper used to implement bounded BUSY polling without CS toggling. |
| src/lgfx/v1/panel/Panel_IT8951.cpp | Implements _wait_busy_pin(), refactors _wait_busy() to call it, and replaces the unbounded BUSY spin in _write_args() with a timeout-based check. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Collaborator
|
Hello, @thecybershotguy |
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
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.
Summary
Panel_IT8951::_write_args()polls the BUSY GPIO between argument words with a rawwhile (!lgfx::gpio_in(_cfg.pin_busy));— unlike every other BUSY wait in this file, it has no timeout._write_args()can't just call the existing_wait_busy()helper here, because_wait_busy()toggles chip-select (cs_control(true)/cs_control(false)) around its wait, while_write_args()deliberately holds CS asserted continuously across the whole multi-word burst (toggling CS per word would break the IT8951 multi-word write protocol)._wait_busy()into a new CS-free helper,_wait_busy_pin(timeout = 4096)(same default timeout already used elsewhere in this file)._wait_busy()now wraps it with CS handling exactly as before;_write_args()calls it directly per word, with CS left untouched, and bails out (return false) if BUSY doesn't clear within the timeout instead of spinning forever.Why
Found while diagnosing a reproducible freeze on an M5Paper-based device: unplugging USB power (running on battery) reliably hung the device within 1–2 minutes, with no crash or reboot — just a dead main loop (no display updates, no input handling, no further activity of any kind). That pointed at a brief brownout on the panel's own boost converter during the power-source transition, which can leave the IT8951's BUSY line stuck asserted. Once that happens, the old unbounded loop in
_write_args()never returns.This mirrors the existing timeout/recovery behavior
_wait_busy()already has for every other BUSY wait in this file —_write_args()was the one place that could still hang forever.Testing
Panel_IT8951.cpp/.hppto confirm timeout value (4096 ms) and CS-handling behavior are unchanged for existing callers._write_args()still writes each word only after BUSY deasserts, CS is still held continuously across the burst.masterand the exact loop body from this branch, run against a fake BUSY pin that's held stuck. Result — same behavior as this PR claims:masterloop does not return at all (confirmed hung past a 3s watchdog); this branch's loop returnsfalseat ~4096ms via the samewait_busy: timeoutlog line_wait_busy()already emits elsewhere in the file.