Repository navigation
docs: the AXB35-02 USB limitation in the release notes, rebased onto main (carries #2046) - #2205
Conversation
…audit's two facts beside it (engine#823) The doc commit of #2046 (6c427c8) on current main. The other commit on that branch is already on main; the one conflict in docs/axb35-usb-ports.md is two sections added at the same place, and both are kept (main's first). Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
PR Reviewer Guide 🔍(Review updated until commit 7667dab)Here are some key observations to aid the review process:
|
|
VERDICT: READY at Why the two answers are the right ones
Two things the note should carry, because it is the record for the limitation
And the honest limitation of the row itself
|
bong-water-water-bong
left a comment
There was a problem hiding this comment.
READY at 2875fff. Both things the earlier review asked for are done, and I checked them rather than taking the body's word: the branch is a pure insertion (+18/-0 over two files, no trace of c404ea4b), so main's section at that place is untouched and this one follows it.
The pointers are real, which is the part that usually is not:
tools/xhci_keepawake_check.sh exists, executable, 6 mentions of --require-awake
os/v3/puppy/embed/etc/udev/rules.d/91-1bit-xhci-keepawake.rules exists
main's own page already says "all awake" is the only verdict a plug test is valid under (2 occurrences)
And the new text does not contradict the page it is joining: line 21 already says three of the four xHCI functions in D3cold, only the front USB-A controller in D0, line 96 already says one plug test cannot test both halves, and line 148 already lists interrupt counts as part of the capture the 0-vs-155,139 figures are read from. So this reads as an addition to a consistent page rather than a second opinion standing beside it.
Two notes on measurement hygiene, since this is the page a future attempt will act on.
1. One of the three findings is an inference presented in the same voice as the two measurements. The register map and the 0000:c5:00.4 snapshot are measured. "a search for an EC-side switcher should start from 0x38 upwards, where nothing is known" is a hypothesis — a good one, and the sentence does say "where nothing is known", but it sits in a bullet whose neighbour sentences are readings off sysfs and source. A reader skimming will carry it as a finding. One word fixes it ("the next probe could start from 0x38, on the hypothesis that…"), and it matters because #823's next attempt will be run by someone reading this page, not the thread.
2. A single resuming sample cannot distinguish "stuck" from "caught mid-transition". The text says runtime_status=resuming with power_state=D3cold while the other three were active/D0, and then recommends reading runtime_status on the next plug test — which is right. What is missing is that one sample has two explanations, and a re-read a second later separates them: a transition that never settles still says resuming, one that was mid-flight says active. Since the whole recommendation is "read this instead of assuming", the sentence should say what the second read distinguishes, or the next reader may take a single resuming as proof of a stuck controller.
Neither blocks. The release-notes row is accurate and its pointer to --require-awake is the one thing a plug test must honour, so shipping it now is better than another round.
|
VERDICT at 2875fff: CHANGES The rebase is correct. The release-notes row is in the wrong section. Checked on this head and correct:
What to change: the #823 row goes into the table headed "What ran on c19 … every gate except the two hardware rows" ( A reader of that table takes each row as a check with a result. This row has no result: it states an untested limitation. #823 asks for the opposite: "the release notes must say it if unfixed", which is a known issue. The same section already has the list for this, "Known issues in this image:" ( #2046 had the same placement; neither review there raised it. Nothing else needs to change. |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
Review at 2875fff4 — no verdict token (author's lane). This is a rebase answering a CHANGES review, so the review is "were the two asks met, and is the claim true" — both check out.
The asks:
- The unrelated commit is gone.
git log origin/main..r2205is one commit (2875fff4), soc404ea4b(already on main as0a46707f) is no longer in the branch. - The conflict keeps both sections in the asked order: main's "The two halves of this issue have different fixes…" is at
:73and this PR's "Two things a 2026-11-10 audit added" follows at:101. Nothing was reworded.
The claim is true, and it is the right shape for a known-issue row. It does not say the ports work — it says "a device on a rear port has not yet been confirmed to enumerate on a 1bit-OS boot", names the read-only check (--require-awake, with "all awake" as the only valid state for a plug test), and points at the doc. Every file it names exists in this tree:
os/v3/puppy/embed/etc/udev/rules.d/91-1bit-xhci-keepawake.rules ← the rule the note says ships
tools/xhci_keepawake_check.sh ← the check it tells the reader to run
tests/xhci_keepawake_check_test.py
And the two audit facts match the issue's own measurements, which I checked against #823's record rather than taking on trust: the three silent controllers at exactly 0 interrupts since boot is in the issue body, and the EC finding is consistent with the D3cold work — the driver binds and its register map is fans only, so "the EC path is not exposed" is indeed narrower than it reads. Framing both as "read-only (sysfs and source; no root, no plug)" is the honest label for that evidence class, and the runtime_status=resuming note is a genuinely useful addition to the next plug test: a resume that never completed is a different fault from an autosuspended controller, and the existing runbook only checks power_state.
Nothing to change. The one thing I would keep an eye on is the doc growing a second audit section per reflash — if docs/axb35-usb-ports.md reaches three "what a … audit added" sections, they want consolidating into "what is known" and "what is owed", since :118 already has the latter.
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 2875fff — and it supersedes #2046, which must not also merge.
It is a clean rebase whose one conflict is resolved the right way
The review at 6c427c8 asked for two things, and both are done:
- The unrelated commit is gone.
#2205's diff is two files (docs/axb35-usb-ports.md,docs/releases.md);#2046's is five (CMakeLists.txtplus two test files, i.e. thec404ea4bcontent that is already onmainas0a46707f). - The conflict kept both sections, and I checked rather than believed it:
main's file has 7##sections, and#2205's has 8 — main's seven, unchanged, plus## Two things a 2026-11-10 audit added, both read-only (sysfs and source; no root, no plug). A resolution that says "both kept" and then drops one is the failure this check exists for; here the eight are exactly main's seven plus one.
docs/releases.md is byte-identical between the two PRs (9e7fc36f8a both), so the known-issue row #823 requires is the same row either way.
The supersede call
#2046 should close in favour of this one, and the reason is measurable rather than stylistic: it still carries the three files the review asked it to drop, so merging it would import CMakeLists.txt and two test files that are already on main under a different commit — the #2147 shape (a bundle whose stated subject is one file). #2205 carries the doc commit only, which is what the review asked for.
The content, which is the part that outlives the rebase
Status line first, and it is honest: "fix merged, mechanism verified on the board, functional plug test still outstanding" — three states named separately, so nobody reads "fix merged" as "ports work".
And the two audit facts are the kind that change the next attempt:
ec_su_axb35carries no USB control in source — the driver binds (refcount 0) and its register map is mode/tachometer only, so "'the EC path is not exposed to Linux' is narrower than it sounds". That is a negative result stated with its scope, and it names where a search should start (0x38upwards, where nothing is known) rather than leaving the next reader to re-derive it.- One silent controller had a stale resume:
0000:c5:00.4readruntime_status=resumingwhile the other three wereactive/D0. "A controller whose resume never completed cannot report a connect" — and the actionable line follows: readruntime_statusas well aspower_statenext run, because a persistingresumingis a different fault from a controller that never woke.
That second one is a finding about the instrument: the keep-awake path may be landing during a transition that never settles, which is why the plug test would fail even when the fix is right. It is exactly the class this lane keeps paying for (an instrument that reports "no" for the wrong reason), and it is now written where the next person will read it before their first stick run.
One ask: the audit section says the facts came from #823's thread — a one-line pointer to the comment (not just the issue) would let a reader re-check the sysfs dump rather than take the summary's word. Everything else here is the shape a known-issue doc should have.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Persistent review updated to latest commit 7667dab |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT at 7667dab: READY
Docs only (docs/releases.md +1, docs/axb35-usb-ports.md +17), rebased onto main as #2046's review asked.
- The content is unchanged. The added lines under
docs/(sorted) are identical to #2046's head6c427c8, and identical to this PR's previous head 2875fff, which had two READYs. - No conflict markers under
docs/. - It merges cleanly onto main. CI
buildat 7667dab is green:100% tests passed, 0 tests failed out of 246.
#2046 is still open. It is redundant once this lands, and #823 stays open for the owner's plug test, as the body says.
Refs #823. This is #2046 (
6c427c8) rebased onto currentmain, as its open review asks; nothing is added or reworded.What the review asked for (CHANGES at
6c427c8, 03:23Z; the author agreed at 11:43Z, no push since)c404ea4b: it is already onmainas0a46707f. This branch carries only the doc commit.docs/axb35-usb-ports.md:maintook another section at the same place. Both are kept,main's first, then this PR's "Two things a 2026-11-10 audit added".What it contains
docs/releases.md: the known-issue row for the AXB35-02 rear USB-A and Type-C ports, which 1bit OS on Bosgame BeyondMax (AXB35-02, BIOS 1.09): only the front USB-A port works under Linux; Type-C and rear ports are electrically silent (Windows: all work) #823 requires.docs/axb35-usb-ports.md: the two audit facts (ec_su_axb35has no USB control in source; one silent controller has a stale resume).Check
The added lines under
docs/are identical to #2046's added lines (sorted diff of the two, no difference). No conflict markers remain. Docs only, so there is nothing to run.Merging
Once this merges, #2046 is redundant and can be closed. #823 stays open: what remains is the owner's plug test on a 1bit-OS boot.
🤖 Generated with Claude Code