fix(async): complete block writes from OUTPUT_LAST status - #84
Open
gly11 wants to merge 9 commits into
Open
Conversation
gly11
force-pushed
the
sbp2/fix-orb-pointer-completion
branch
3 times, most recently
from
July 26, 2026 10:43
febe8f5 to
22db267
Compare
This was referenced Jul 26, 2026
gly11
marked this pull request as ready for review
July 27, 2026 13:12
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0236dca2b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The pin added earlier in this branch only bound CI. build.sh still ran whatever xcodegen was on PATH, and it regenerates ASFW.xcodeproj on every non-test-only build. Homebrew currently ships 2.46.0 while the pin was 2.45.4, and their output is not byte-identical -- 2.46.0 reorders the pbxproj `targets` array. So a single ./build.sh on a brew-current machine produces a project the drift check rejects, and that check's error message tells you to regenerate, which reproduces the same diff. Move the version and its SHA-256 into .xcodegen-version and have both the workflow and build.sh read it, so the two can no longer disagree. build.sh now stops with an actionable error when the installed version differs, rather than warning and skipping regeneration: a build silently missing a newly added source file is much harder to diagnose than a version mismatch. --no-xcodegen builds the committed project as-is for machines that can't install the pin. Bump the pin to 2.46.0 to match what Homebrew ships today, and regenerate ASFW.xcodeproj with it. The only change is the `targets` array ordering, which has no semantic effect. Document how to install the pinned release in README -- the same download, pin, and checksum CI uses -- since "install the pinned version" isn't actionable without it.
gly11
force-pushed
the
sbp2/fix-orb-pointer-completion
branch
from
July 28, 2026 11:50
bd3a8bb to
6594e52
Compare
InspectChainTail() returns Pending for a recognized OUTPUT_MORE_IMMEDIATE +
OUTPUT_LAST chain whose tail status is still zero, and ScanCompletion() then
returns without consulting the orphan path. That is correct while the context
is running, but a quiesced context will never write a status, so the ring head
stayed pinned to the chain forever.
Gate the Pending result on the context still being live. run==0 && active==0
falls through to HandlePendingDescriptor(), restoring the drain that existed
before this branch was introduced.
The check is deliberately narrower than IsOrphanedDescriptor(): its second
clause (commandPtr != headIOVA) also matches a live chain whose command pointer
has advanced to a pending OUTPUT_LAST, which is the false positive the Pending
result was added to fix. Only the run/active bits are consulted here, so both
cases keep working.
Both references drain unconditionally once the context is stopped, rather than
inferring liveness during the scan:
- Linux gates handle_at_packet()'s early return on !ctx->flushing and calls
at_context_flush() after context_stop() (ohci.c:1364, 2000-2010), citing
OHCI 1.2 draft clause 7.2.3.3 — hardware may leave unsent packets in the
AT queues for software to drain.
- AppleFWOHCI 5.5.9 calls resetDMA() after stopDMA() in handleBusResetInt()
(symbol offsets 0x5fae-0x5fd2); resetDMA() frees every pending ATxElement
regardless of status.
Apple can also afford to keep scanning past a zero-status element because its
pending set is a linked list; a single head cursor cannot. Correct the
ScanCompletion() doc comment accordingly. An explicit flush entry point is the
real fix and is left to a follow-up; this keeps the ring releasable meanwhile.
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
1 participant
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
OUTPUT_LASTdescriptorOUTPUT_MORE→OUTPUT_LASTchainsRoot cause
ASFW scans AT completions from the ring head. For a block write, the ring head is an immediate
OUTPUT_MOREheader and the payload is anOUTPUT_LASTdescriptor. The tested OHCI controller records the completion status onOUTPUT_LASTwhile leaving the precursor status at zero. The scanner therefore treated a completed packet as pending forever, so the SBP-2 ORB_POINTER write kept its tLabel outstanding and the command path stalled after the first successful INQUIRY.This behavior matches the Linux OHCI implementation, which assigns and reads packet completion from the chain's last descriptor. The same compatibility handling existed in the earlier SBP-2 bring-up work from #14 but was lost during the later async-stack refactor.
Impact
The fix is transport-level and applies to any async block write that uses the standard immediate-header plus payload descriptor chain. It does not add SBP-2-specific knowledge to the async layer.
Validation
Host and build
ATContextCompletionTests: 5/5 passeddevheadHardware
Validated on an Epson V750 Pro / GT-X970 SBP-2 scanner with the rebuilt dext installed:
0.2.0/2activated and enabledsystem_profilerdroppedRecords = 0The original hardware blocker is resolved. This PR is ready for review.
Dependency and merge order
Depends on #88. This PR now targets
devand is stacked directly on the current #88 head.Until #88 merges, GitHub's Files changed view for this PR will also include #88's CI/project-generation changes. The #84-specific review scope is the top two async commits and these three files:
ASFWDriver/Async/Contexts/ATContextBase.hpptests/async/ATContextCompletionTests.cpptests/async/CMakeLists.txtIntended merge order:
dev;devtip if GitHub does not collapse the stacked diff automatically, retaining only the two async commits;Please merge #88 before #84.