[widgetii]

Refs #148.

Problem

HiSiliconCV6xx.send_firmware() waited a fixed DDR_TRAINING_WAIT (1.5 s) after the DDR table and then sent U-Boot regardless. Two quite different outcomes therefore both appeared as Failed to send U-Boot, once the HEAD frame's ACK timed out:

1. The table never trains. The SPL accepts it, prints the table header, and goes silent without printing DDR size. This is what hi3516cv610:20s — bootrom upload fails after DDR training ("Failed to send U-Boot"), stock boot prints "DDR size" but burn never does #148 reports from a CV610 module.
2. The table is refused. The SPL prints The table is incorrect., then trains from the boot table in flash and starts the U-Boot there (flash … DDR size … System startup), so the upload has nowhere to land.

Change

The stage now keeps the SPL's UART text from the training window and acts on it:

• The table is incorrect: fail immediately with a message saying the boot image is for another chip or variant and the SPL fell back to flash.
• Table header but no DDR size: keep waiting up to DDR_TRAINING_TIMEOUT (10 s) past the usual window, stopping as soon as a verdict appears, then fail with "DDR training did not finish … this table does not train this board's memory".
• DDR size, or no SPL output at all: unchanged. U-Boot is sent after the same 1.5 s window. Silence on its own proves nothing, so it isn't treated as a failure.

Tested

On a Hi3516CV608 (DDR2 64 MB), through a vectis UART bridge with power cycling. Every run loaded into RAM only.

Run	Image	Before	After
Matching image	-c hi3516cv608, boot-hi3516cv608-nor.bin	table header, DDR size: 0x00000040, U-Boot console ready	same, unchanged
Wrong chip	-c hi3516cv610:20s, boot-hi3516cv610-20s-nor.bin (DDR3 128 MB table)	SPL printed The table is incorrect. and booted from flash; defib reported Failed to send U-Boot	The SPL rejected the DDR table ("The table is incorrect"): this boot image is for another chip or variant…

The "silent after the header" case from #148 can't be produced on this board: its only accepted table trains. It's covered by a unit test built from the reporter's log.

Unit tests: new TestCV6xxDdrTrainingVerdict covering trained, rejected, never-trains and quiet SPL. Against the old code, the rejected and never-trains scenarios both return Failed to send U-Boot.

Suite: pytest tests/ --ignore=tests/fuzz gives 1091 passed, 3 skipped. ruff check and mypy are clean.

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

PR Summary by Qodo

Report CV6xx DDR training failures before uploading U-Boot

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Distinguish rejected DDR tables from tables that never finish training, replacing misleading
 U-Boot errors.
• Extend the wait only when the SPL reports a table without a training verdict; preserve quiet-SPL
 behavior.
• Add tests for trained, rejected, stalled, and quiet SPL output.

Diagram

graph TD
  DDR["DDR table"] --> UART["SPL UART"] --> Wait["1.5s window"] --> Verdict{"SPL verdict?"}
  Verdict -->|Header only| Extra["Up to 10s"] -->|Timeout| Error["Specific error"]
  Extra -->|New verdict| Verdict
  Verdict -->|Rejected| Error
  Verdict -->|Trained or quiet| Uboot["U-Boot upload"]

Loading

High-Level Assessment

Keep the bounded, SPL-output-based decision in the existing DDR-training stage. It distinguishes the reported failures without changing the transfer protocol or treating an otherwise quiet SPL as a failure; a separate parser or state machine would add complexity for these three output markers.

Files changed (2) +158 / -2

Bug fix (1) +65 / -2

hisilicon_cv6xx.pyClassify SPL DDR-training output before U-Boot transfer +65/-2

Classify SPL DDR-training output before U-Boot transfer

• Retains UART text across reads and detects table rejection, completed training, or a header without completion. A pending header receives up to ten additional seconds; rejection and unfinished training return distinct errors without sending U-Boot, while trained or quiet SPLs retain the existing path.

src/defib/protocol/hisilicon_cv6xx.py

Tests (1) +93 / -0

test_protocol_cv6xx.pyCover four SPL training outcomes +93/-0

Cover four SPL training outcomes

• Adds a mock transport setup and representative SPL logs for trained, rejected, and stalled tables. Tests verify the resulting error or U-Boot transfer, including continued upload when the SPL prints nothing.

tests/test_protocol_cv6xx.py

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

Code Review by Qodo

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

1. Rejected tables still trigger power cycles ✓ Resolved

Description

send_firmware() returns a table-rejection failure without a DDR_INIT stage, which
RecoverySession treats as eligible for an early-failure retry. When automated power control is
enabled, the session power-cycles the board and sends the same image again despite the SPL’s
explicit rejection.

Code

src/defib/protocol/hisilicon_cv6xx.py[R465-467]

+        if SPL_TABLE_REJECTED in spl_text:
+            return RecoveryResult(
+                success=False, stages_completed=stages,

Evidence

The new branch identifies a wrong chip or variant and returns before the DDR-training stage is
appended. The session retries failures lacking DDR_INIT when automated power control permits
another attempt, and its next attempt power-cycles the device.

src/defib/protocol/hisilicon_cv6xx.py[465-475]
src/defib/protocol/hisilicon_cv6xx.py[488-492]
src/defib/recovery/session.py[104-106]
src/defib/recovery/session.py[267-278]
src/defib/recovery/session.py[138-154]

Agent prompt

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

## Issue description
A CV6xx SPL table rejection now produces a definitive firmware failure, but RecoverySession retries it as an early failure and power-cycles the board with the same image.
## Fix Focus Areas
- src/defib/protocol/hisilicon_cv6xx.py[465-475]
- src/defib/recovery/session.py[267-278]
## Recommended Fix
Represent the rejection as a non-retryable failure and have the session honor that classification before its early-failure retry. Add a session-level test with automated power control that confirms a rejected table causes only one attempt.

ⓘ 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 choose which labels appear on a finding, and whether they show icons or text

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[widgetii]

Rejected tables still trigger power cycles: valid, and the hardware run in the PR description showed it (Retrying handshake (attempt 2/2) — previous error: The SPL rejected the DDR table…). Fixed in the latest commit:

• RecoveryResult has a new retryable flag, default True. The CV6xx table rejection sets it to False, and RecoverySession's early-failure retry checks it.
• New session test test_session_does_not_retry_a_failure_marked_final: automated power, max_handshake_attempts=2, a rejected table. It expects one send_firmware call, one power cycle and no retry log. Without the session change it fails with 2 calls.
• Re-run on the CV608 with -c hi3516cv610:20s --power-cycle: one power cycle, no retry, and the run ends with the rejection message.

The "never trains" result deliberately stays retryable: a marginal training could pass on another power-up, and one more attempt is cheap.