MAX7456: bound the clear-display wait

[xhlsa]

Problem

max7456RefreshAll() sets DMM_CLEAR_DISPLAY and then polls DMM bit 2 in an unbounded while (1) until the chip clears it. On a NewBeeDrone LionBee (AT32F435, AT7456E-compatible OSD) the bit never clears: DMM reads back 0x06. The OSD task spins forever and starves the serial task. USB VCP still enumerates but accepts no data, so the board looks hung at boot.

Cause

The busy loop has no timeout. Betaflight's MAX7456 driver never polls this bit.

Fix

Bound the wait to 2 ms (MAX7456_CLEAR_TIMEOUT_US). The datasheet clear time is about 20 µs, so a working chip still leaves the loop on the first or second read. If the chip never clears the bit, the refresh carries on instead of hanging.

On timeout the DMM shadow is left as written rather than taken from the readback. The stuck value 0x06 has bit 1 set, and keeping it would turn every later clear into a deferred VSYNC clear that can erase freshly drawn characters (thanks to the Qodo review for catching this).

How it was found / tested

• No SWD was available, so a debug build wrote boot and main-loop progress markers into the last flash sector and reset into DFU on a stall. The trace pinned the hang to spiBusReadRegister called from this loop.
• With the first version of this fix (which adopted the readback on timeout) the board booted, the OSD drew, and the quad flew. The shadow change above has been build-tested only.
• Other boards: with a chip that clears the bit normally, nothing changes.

[qodo-code-review]

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

[qodo-free-for-open-source-projects]

PR Summary by Qodo

MAX7456: Bound display-clear polling to prevent scheduler stalls

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Limits display-clear polling to two milliseconds, preventing scheduler starvation on incompatible
 OSD chips.
• Preserves normal clear detection and resumes redraw after timeout with a sanitized DMM cache.

Diagram

graph TD
    A["Refresh request"] --> B["Issue clear"] --> C{"Bit cleared?"}
    C -- Yes --> D["Update DMM cache"] --> E["Redraw display"]
    C -- No --> F{"2 ms elapsed?"}
    F -- No --> C
    F -- Yes --> D

Loading

High-Level Assessment

The bounded poll is the best fit because it preserves completion detection for compliant chips while preventing incompatible AT7456E variants from hanging the scheduler. Removing polling entirely was considered, but the timeout is more conservative and retains existing behavior on supported hardware.

Files changed (1) +10 / -1

Bug fix (1) +10 / -1

max7456.cBound MAX7456 display-clear polling to two milliseconds +10/-1

Bound MAX7456 display-clear polling to two milliseconds

• Adds a two-millisecond deadline to max7456RefreshAll so a permanently asserted clear bit cannot stall the scheduler. On timeout, the cached DMM value is sanitized and display redraw proceeds normally.

src/main/drivers/max7456.c

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Later refreshes can erase new output 🐞 Bug ≡ Correctness

Description

max7456RefreshAll() caches dmm & ~DMM_CLEAR_DISPLAY on timeout, so the documented stuck value
0x06 changes the persistent shadow from 0x00 to 0x02 instead of preserving the requested mode.
On the next resync, bit 2 is added to that shadow to produce the defined vertical-clear command
0x06, while the function stops waiting after 2 ms and immediately redraws, allowing the deferred
clear to remove freshly written content.

Code

src/main/drivers/max7456.c[664]

+                 state.registers.dmm = dmm & ~DMM_CLEAR_DISPLAY;

Evidence

The DMM definitions establish that 0x06 is the vertical-clear command, while initialization
establishes a zero-valued shadow. The timeout converts the affected chip's documented 0x06
response to persistent 0x02; later refreshes OR in bit 2, and display resynchronization invokes
this path before returning to normal rendering.

src/main/drivers/max7456.c[117-124]
src/main/drivers/max7456.c[403-407]
src/main/drivers/max7456.c[648-678]
src/main/io/displayport_max7456.c[114-119]
src/main/io/osd.c[4868-4870]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The timeout path derives the persistent DMM shadow from the anomalous hardware readback. A stuck `0x06` response therefore preserves the vertical-clear selector and changes all later refresh commands.
## Fix Focus Areas
- src/main/drivers/max7456.c[648-665]
## Recommended Fix
On timeout, retain the pre-command `state.registers.dmm` value rather than copying the untrusted DMM readback; the clear bit was only ORed into the bus write and is not present in the shadow. Add coverage that repeatedly returns `0x06`, times out, and verifies that the next clear command remains the original shadow plus only `DMM_CLEAR_DISPLAY`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[qodo-free-for-open-source-projects]

1. Later refreshes can erase new output 🐞 Bug ≡ Correctness

max7456RefreshAll() caches dmm & ~DMM_CLEAR_DISPLAY on timeout, so the documented stuck value
0x06 changes the persistent shadow from 0x00 to 0x02 instead of preserving the requested mode.
On the next resync, bit 2 is added to that shadow to produce the defined vertical-clear command
0x06, while the function stops waiting after 2 ms and immediately redraws, allowing the deferred
clear to remove freshly written content.

Agent Prompt

## Issue description
The timeout path derives the persistent DMM shadow from the anomalous hardware readback. A stuck `0x06` response therefore preserves the vertical-clear selector and changes all later refresh commands.

## Fix Focus Areas
- src/main/drivers/max7456.c[648-665]

## Recommended Fix
On timeout, retain the pre-command `state.registers.dmm` value rather than copying the untrusted DMM readback; the clear bit was only ORed into the bus write and is not present in the shadow. Add coverage that repeatedly returns `0x06`, times out, and verifies that the next clear command remains the original shadow plus only `DMM_CLEAR_DISPLAY`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[xhlsa]

Fixed in f115375: on timeout the DMM shadow is now left as written instead of being taken from the readback, so a stuck 0x06 can no longer turn later clears into deferred VSYNC clears.