Skip to content

fix(async): complete block writes from OUTPUT_LAST status#84

Open
gly11 wants to merge 3 commits into
mrmidi:mainfrom
gly11:sbp2/fix-orb-pointer-completion
Open

fix(async): complete block writes from OUTPUT_LAST status#84
gly11 wants to merge 3 commits into
mrmidi:mainfrom
gly11:sbp2/fix-orb-pointer-completion

Conversation

@gly11

@gly11 gly11 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • complete two-descriptor async block writes from the terminal OUTPUT_LAST descriptor
  • keep pending chains untouched until the terminal descriptor has a non-zero transfer status
  • add focused host tests for completed and still-pending OUTPUT_MOREOUTPUT_LAST chains

Root cause

ASFW scans AT completions from the ring head. For a block write, the ring head is an immediate OUTPUT_MORE header and the payload is an OUTPUT_LAST descriptor. The tested OHCI controller records the completion status on OUTPUT_LAST while 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: 2/2 passed
  • async host-test slice: 61 passed, 1 conditional skip
  • full C++ host suite: 1,390 passed, 6 conditional skips
  • Debug Xcode build with the experimental SCSI HBA enabled: succeeded with signing disabled
  • GitHub Build and Test check: passed

Hardware

Validated on an Epson V750 Pro / GT-X970 SBP-2 scanner with the rebuilt dext installed:

  • ASFWDriver 0.2.0/2 activated and enabled
  • scanner enumerated as an SBP-2 unit
  • SBP-2 login completed successfully
  • status blocks completed without ORB timeout, ORB-pointer write failure, or unmatched-response errors
  • target 0 / LUN 0 appeared in IORegistry and system_profiler
  • diagnostic log ring reported droppedRecords = 0

The original hardware blocker is resolved. This PR is ready for review.

CI organization and merge order

Shared CI workflow cleanup is tracked separately in #88. This PR is marked ready now so code review can proceed while #88 awaits owner action.

The intended merge order remains:

  1. merge fix(build): pin XcodeGen for CI and local builds, stabilize test failure reporting #88;
  2. rebase this branch onto the updated main and drop the duplicated CI commits;
  3. merge fix(async): complete block writes from OUTPUT_LAST status #84 after its rebased checks pass.

Please do not merge #84 before that rebase.

@gly11
gly11 force-pushed the sbp2/fix-orb-pointer-completion branch 2 times, most recently from d60690b to febe8f5 Compare July 26, 2026 10:31
@gly11
gly11 force-pushed the sbp2/fix-orb-pointer-completion branch from febe8f5 to 22db267 Compare July 26, 2026 10:43

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +859 to +861
if (tailCommand != HW::OHCIDescriptor::kCmdOutputLast ||
HW::AT_xferStatus(*tailState.desc) == 0) {
return false;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve recognized chains while OUTPUT_LAST is pending

When the watchdog scans after hardware has advanced CommandPtr from an OUTPUT_MORE header to its still-pending OUTPUT_LAST, the zero terminal status makes this helper return false; ScanCompletion() then invokes HandlePendingDescriptor(), whose pointer-mismatch check clears the header and advances the ring head. Under sustained submissions those prematurely freed blocks can be reused before the terminal descriptor completes, overwriting the immediate header used to recover the tLabel and causing a lost or misattributed completion. Distinguish a recognized-but-pending chain from an unrecognized descriptor and return without orphan cleanup until the terminal status becomes nonzero. CLAUDE.mdL230-L232

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant