diff options
38 files changed, 987 insertions, 300 deletions
diff --git a/.agents b/.agents new file mode 120000 index 000000000..c8161850a --- /dev/null +++ b/.agents @@ -0,0 +1 @@ +.claude
\ No newline at end of file diff --git a/.claude/agents/hil-operator.md b/.claude/agents/hil-operator.md index d128f6f53..4fd483a73 100644 --- a/.claude/agents/hil-operator.md +++ b/.claude/agents/hil-operator.md @@ -47,9 +47,10 @@ The GitHub Actions runner keeps running during your work. Per-board flock locks schedule them — it round-robins boards across host controllers and budgets simultaneous flashes and usbtest batteries per controller. Those budgets live in one process, so a second `hil_test.py` alongside the first does not share them and the rig sees double the configured - width. (Do not read that as the cause of a dead card: hil_lock.py:128-131 records that every - observed uPD720201 death traced to a marginal DUT port bouncing under concurrent batteries, - and that lowering the widths does not fix a bad port — fix the port or pull the board.) + width. (A dead uPD720201 card is not that: every observed death traced to a marginal DUT + port bouncing under concurrent batteries, and lowering the widths does not fix a bad port — + fix the port or pull the board; the concurrency note above `FLASH_PARALLEL` in `hil_lock.py` + keeps the record.) - On test failure, retry ONCE, with `-v` for diagnosis. Retry from the spec the run just wrote — `<config>.failed`, which already begins with `--accumulate` and restricts each board to its failed tests via `-bt`. If you compose the retry by hand you MUST pass `--accumulate` yourself: @@ -76,6 +77,6 @@ python3 test/hil/helper/hil_report.py <config> -b BOARD [-b BOARD...] # from t `results`, `banner` and `caveat` are copied, never retyped, reworded or re-ordered (`caveat` is the run-level notice — abandoned, aborted, no-boards — and it can say the run failed while every row says pass): report rows are named per variant, a variant name need not start with the board name, and lock contention is a cell -rather than a phrase, so re-deriving any of it by hand is how this contract broke before. +rather than a phrase, so any of it re-derived by hand has come out wrong before. `wedged` is yours — the boards your run left unresponsive, usually none — and the only field you author. diff --git a/.claude/agents/pr-review-validator.md b/.claude/agents/pr-review-validator.md index 4069a856f..a731cd774 100644 --- a/.claude/agents/pr-review-validator.md +++ b/.claude/agents/pr-review-validator.md @@ -10,8 +10,22 @@ You validate the bot review findings on exactly one PR (number given in your pro ## Procedure -- Inline review comments: `gh api repos/{owner}/{repo}/pulls/<N>/comments --paginate` (use `gh repo view --json nameWithOwner -q .nameWithOwner` for owner/repo). Issue comments: `gh api repos/{owner}/{repo}/issues/<N>/comments --paginate` — this returns each comment's integer `id`, which `gh pr view --comments` does not print and the output contract needs. PR reviews (the Copilot/Claude verdict bodies): `gh api repos/{owner}/{repo}/pulls/<N>/reviews --paginate` — compare each review's `commit_id` to the head SHA from `gh pr view <N> --json headRefOid -q .headRefOid` to tell a review of the current push from an older one. -- Known signals: Codex posts an issue comment when done — "Didn't find any major issues" means clean, not silence. It can also signal a clean pass with no comment at all: a 👍 (`+1`) reaction on the PR description (`gh api "repos/{owner}/{repo}/issues/<N>/reactions?content=%2B1&per_page=100" --paginate`, author matching `codex`; without `--paginate` a fresh reaction can fall off the first page and Codex looks pending forever) — settled when the reaction's `created_at` postdates the head push time defined below. Its body carries a `**Reviewed commit:** <short sha>` line: Codex is settled only when that short SHA prefix-matches the head SHA, otherwise the comment is a verdict for an older push and Codex is still pending. Its "Something went wrong" comment has no Reviewed-commit line, so correlate that one by time instead — against the moment the SHA *became* the head, `gh api repos/{owner}/{repo}/commits/<headSha>/check-suites --jq '[.check_suites[].created_at] | min'` (the suites are created when the push lands; fall back to `gh api repos/{owner}/{repo}/commits/<headSha> --jq .commit.committer.date` only if the SHA has no check suites). The committer date alone is when the commit was written, which can precede the push by hours and make a leftover error comment look fresh. An error/quota comment settles Codex only when its `created_at` postdates that push time, or when it arrives as a PR review whose `commit_id` is the head SHA. An older one is a leftover from an earlier push — Codex is still pending. Copilot submits a PR review whose body opens with a verdict header (`### 🟢 Approval recommended` / `### 🟡 Changes recommended`) and leaves `requested_reviewers` once submitted. The Claude bot posts a PR review, or its `claude-review` check run for the head SHA reaches `status: completed` — ask for that check by name, `gh api "repos/{owner}/{repo}/commits/<headSha>/check-runs?check_name=claude-review"`, since the unfiltered listing is paginated and drops it on a PR with more than a page of checks. A bot reporting a usage/quota limit counts as settled once that report postdates the head push time above (the check-suite timestamp, not the committer date) — do not wait on it. Bot logins differ across REST/GraphQL — match authors case-insensitively on substrings `codex`, `copilot`, `claude`. +- Fetch with the REST API (owner/repo via `gh repo view --json nameWithOwner -q .nameWithOwner`; head SHA via `gh pr view <N> --json headRefOid -q .headRefOid`): + - inline review comments: `gh api repos/{owner}/{repo}/pulls/<N>/comments --paginate` + - issue comments: `gh api repos/{owner}/{repo}/issues/<N>/comments --paginate` — this returns each comment's integer `id`, which `gh pr view --comments` does not print and the output contract needs + - PR reviews (the Copilot/Claude verdict bodies): `gh api repos/{owner}/{repo}/pulls/<N>/reviews --paginate` +- A reviewer is **settled** for the current head SHA when its verdict artifact is bound to that SHA (a review's `commit_id`, a check run for the SHA, Codex's `**Reviewed commit:** <short sha>` line prefix-matching it) or, for artifacts that carry no SHA (reactions, error and quota comments), when its `created_at` postdates the head push time. Anything bound to or dated before an older push is a leftover from that push: the reviewer is still pending. A bot reporting a usage/quota limit settles the same way — do not wait on it. +- Head push time = when the SHA *became* the head: `gh api repos/{owner}/{repo}/commits/<headSha>/check-suites --jq '[.check_suites[].created_at] | min'` (suites are created when the push lands). Fall back to `gh api repos/{owner}/{repo}/commits/<headSha> --jq .commit.committer.date` only if the SHA has no check suites: the committer date is when the commit was written, which can precede the push by hours and make a leftover error comment look fresh. +- Where each bot's verdict lives (match authors case-insensitively on the substrings `codex`, `copilot`, `claude` — logins differ across REST/GraphQL): + +| Bot | Verdict artifact | Bound to head by | +| ------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------ | +| Codex | issue comment when done ("Didn't find any major issues" = clean, not silence); body carries `**Reviewed commit:** <short sha>` | short SHA prefix-matches head | +| Codex | 👍 (`+1`) reaction on the PR description = a clean pass with no comment, author matching `codex`: `gh api "repos/{owner}/{repo}/issues/<N>/reactions?content=%2B1&per_page=100" --paginate` (without `--paginate` a fresh reaction can fall off the first page and Codex looks pending forever) | reaction `created_at` postdates head push time | +| Codex | "Something went wrong" comment (no Reviewed-commit line), or a PR review | comment `created_at` postdates head push time; review `commit_id` = head | +| Copilot | PR review whose body opens with `### 🟢 Approval recommended` / `### 🟡 Changes recommended`; it leaves `requested_reviewers` once submitted | review `commit_id` = head | +| Claude | PR review, or the `claude-review` check run reaching `status: completed` — ask for it by name, `gh api "repos/{owner}/{repo}/commits/<headSha>/check-runs?check_name=claude-review"` (the unfiltered listing is paginated and drops it on a PR with more than a page of checks) | review `commit_id` = head; the check run is per SHA | + - For EACH unresolved bot finding: open the file at the cited line in the current checkout and judge the claim adversarially. `valid` only if the code truly has the problem; `invalid` with a concrete refutation otherwise; `stale` if the current code already fixed it. - Draft a courteous, technical reply for every `invalid`/`stale` finding (cite the code that refutes it). Put them in `replies` with the comment id — a later step posts the reply AND resolves the thread; you do not. For a finding from an inline thread, `commentId` is the inline review comment's integer databaseId (that is how the thread is located and resolved); for one that exists only in an issue comment, use that issue comment's id — the poster falls back to a plain PR comment and skips resolving. @@ -25,6 +39,6 @@ Your final message is parsed by a program. Return ONLY this JSON — no prose, n done = true only when no unresolved `valid` findings remain AND every auto-reviewer has settled for the current head SHA: its verdict is posted (Copilot review header, -Codex verdict comment, Claude review or concluded check) or it reported hitting a +Codex verdict comment or 👍 reaction, Claude review or concluded check) or it reported hitting a usage/quota limit. A reviewer that has not reported since the last push is pending — return done = false so the caller re-checks next cycle. diff --git a/.claude/skills/etm-trace/SKILL.md b/.claude/skills/etm-trace/SKILL.md index 43cf6a2a5..5d9b51bcf 100644 --- a/.claude/skills/etm-trace/SKILL.md +++ b/.claude/skills/etm-trace/SKILL.md @@ -196,8 +196,8 @@ both. Timing semantics and clock columns are explained at the top of that file. ## References - Ozone manual (UM08025, automation socket §6.7, project commands §7): - <https://www.segger.com/downloads/jlink/UM08025_Ozone.pdf> — V3.50, same as - the installed Ozone (web is rev 1 vs the local copy's rev 0; the local PDF - under /opt/SEGGER/Ozone_V350/Doc remains the offline fallback). + <https://www.segger.com/downloads/jlink/UM08025_Ozone.pdf>; the installed + Ozone ships its own revision of it under its `Doc/` directory (offline + fallback — section numbers can differ between revisions). - J-Link / J-Trace manual (UM08001, trace ch. 10, timing troubleshooting): <https://kb.segger.com/UM08001_J-Link_/_J-Trace_User_Guide> diff --git a/.claude/skills/hil-pool-check/SKILL.md b/.claude/skills/hil-pool-check/SKILL.md index 65e28b65f..dd128f755 100644 --- a/.claude/skills/hil-pool-check/SKILL.md +++ b/.claude/skills/hil-pool-check/SKILL.md @@ -130,7 +130,9 @@ removes VBUS, so there is nothing further to try. The user-facing answer to a pool check IS the tool's summary table: paste the complete per-board table (and footer counts) verbatim — never truncate rows or reduce it to a prose digest like -"27/27 healthy"; at most one line of commentary below it. +"27/27 healthy". Commentary below it covers only what the table cannot show: which mode ran +and why when it was not the full check, the escalation note below, a board that needs a +physical replug. When an escalation above was needed, add a short note under the table naming: which boards needed it, which root port was cycled (or that a replug was needed instead), which siblings bounced, and diff --git a/.claude/skills/hil/SKILL.md b/.claude/skills/hil/SKILL.md index 03a462ce6..0257c42c1 100644 --- a/.claude/skills/hil/SKILL.md +++ b/.claude/skills/hil/SKILL.md @@ -60,14 +60,15 @@ fi Read `full`, never `args` alone: `args` is empty for BOTH `full: true` (run the whole matrix — a broad or unclassified change) and "nothing selected" (skip). Skip only when `full` is false AND `args` is empty. -Unit suites (no hardware), all five run by the `hil-test`/`ci-select-test` pre-commit -hooks: `test_ci_select.py` covers only selection, `test_ci_metrics.py` only the code-size -plumbing. The containment work --- bounded reads, the kill ladders, the build and pool -guards --- lives in `test_hil_bounded.py`, `test_hil_health.py` and `test_hil_util.py`, so -run all five when changing `test/hil`: -`for f in test/hil/test/test_*.py; do python3 "$f"; done` (~48s, of which -`test_hil_bounded.py` is ~30s of deliberate hang/timeout simulation; the two `test_ci_*` -suites are ~4s together). +Unit suites (no hardware) live in `test/hil/test/test_*.py`; the `hil-test` pre-commit hook +runs every `test_hil*.py`, `ci-select-test` the two `test_ci_*` suites plus +`test_hil_util.BottomLayer`. `test_ci_select.py` covers only selection, `test_ci_metrics.py` +only the code-size plumbing; the containment work --- bounded reads, the kill ladders, the +build and pool guards --- lives in `test_hil_bounded.py`, `test_hil_health.py` and +`test_hil_util.py`; `test_hil_report.py` covers the report document and `test_hil_rtt.py` +the RTT console. Run them all when changing `test/hil`: +`for f in test/hil/test/test_*.py; do python3 "$f"; done` (about a minute, half of it +`test_hil_bounded.py`'s deliberate hang/timeout simulation). ## Pre-flight rig health check @@ -134,23 +135,29 @@ the run before its own guard can write a report. NEVER cancel early. ## Reporting The user-facing answer to a HIL run IS the tool's summary table: paste the complete per-board -table (and footer counts) verbatim — never truncate rows or reduce it to a prose digest; at most -one line of commentary below it. +table (and footer counts) verbatim — never truncate rows or reduce it to a prose digest. +Commentary below it covers only what the table cannot show: a banner verdict from the list +below, a retry, a wedged board. **First check what sits above the table.** Six banners can appear there; match on a PREFIX, since each carries trailing detail and two are blockquotes: -- `**HIL run abandoned: worker pool timed out after …s.**` — no results were collected this - attempt, so any table below is a PREVIOUS attempt's. Report the abandonment, never those - rows, and never `"pass": true`. -- `**HIL run aborted: a worker raised …**` — same rule: a worker crashed before results - were collected; any table below is stale. Report the abort, never the rows. +- `**HIL run abandoned: worker pool timed out after …s.**` and + `**HIL run aborted: a worker raised …**` — the pool guard fired, or a worker crashed. The + banner counts what happened: "N board(s) below finished and are this run's; K never + reported and are NOT in the table: <names>". The N finished boards' rows are this run's: + report them. The K named boards are not this run's whatever the table shows — on a fresh + run they have no row, on an `--accumulate` retry a previous attempt's row survives under + the banner and `hil_report.py` still folds it into `results` as ran — so name them as not + run; the `<config>.failed` re-run spec covers them. Never `"pass": true`. - `**HIL run abandoned: the worker pool would not shut down.**` — DIFFERENT: the table below IS this run's, but the pool could not be shut down afterwards (the job exits non-zero even if every board passed). Report the results AND the abandonment; never `"pass": true`. -- `**HIL run selected no boards.**` — the filters intersected to nothing, so there is no - table at all. Report that (and the filter shown), never `"pass": true`. +- `**HIL run selected no boards.**` — the filters intersected to nothing. A fresh run shows + no table; an `--accumulate` run keeps the previous attempt's rows under the notice, and + they are not this run's. Report the empty selection (and the filter shown), never + `"pass": true`. - `> **Rig note.**` — a process was in D state when the run started. This is NOT a wedge: a healthy in-flight testusb is uninterruptible for most of every case, and the rig supports a dev run alongside CI. On its own it is never `wedged: true` and never turns a diff --git a/.claude/skills/pre-pr/SKILL.md b/.claude/skills/pre-pr/SKILL.md index 8e5c408a6..56db5a5dd 100644 --- a/.claude/skills/pre-pr/SKILL.md +++ b/.claude/skills/pre-pr/SKILL.md @@ -3,7 +3,7 @@ name: pre-pr description: Use before opening or updating a TinyUSB PR — derives affected boards from the branch diff, runs the full-check workflow (software validation + optional HIL on the rig), and summarizes a ship/no-ship verdict. --- -# /pre-pr — pre-PR validation +# Pre-PR validation Run the software + hardware gate for the current branch. The user invoking this skill is the opt-in for launching the workflows below. @@ -52,6 +52,6 @@ Invoke the Workflow tool: ## 5. Summarize - Per-stage table: unit / build:<board> / size / pvs, then HIL per board — pass/fail with the first error for each failure. -- If the hardware result has non-empty `locked` (a CI job held those boards): ask the user with AskUserQuestion — **Force now** (re-invoke `hil-validate` with `force: true` for those boards; user accepts the risk of colliding with a mid-test CI job), **Keep waiting** (re-invoke `hil-validate` for them after a few minutes; ask again if still locked), or **Accept** the partial verdict. Never force without the user's answer. +- If the hardware result has non-empty `locked` (a CI job held those boards): ask the user to choose **Force now** (re-invoke `hil-validate` with `force: true` for those boards; user accepts the risk of colliding with a mid-test CI job), **Keep waiting** (re-invoke `hil-validate` for them after a few minutes; ask again if still locked), or **Accept** the partial verdict. Never force without the user's answer. - Wedged boards: point at `.claude/skills/usb-kernel-recover/SKILL.md`. - End with a clear ship / no-ship verdict and what to fix first. diff --git a/.claude/skills/read-doc/SKILL.md b/.claude/skills/read-doc/SKILL.md index feaa914af..aca30fb8c 100644 --- a/.claude/skills/read-doc/SKILL.md +++ b/.claude/skills/read-doc/SKILL.md @@ -40,7 +40,7 @@ Not for general concepts, repo/code questions, or when no such doc is likely. ## Find -Keywords from `/read-doc <keywords>`, else derived from the question (part +Keywords supplied as skill arguments, else derived from the question (part number, peripheral, spec name). `search.py` ANDs them across every metadata field and prints the best matches first — at most 40, and the header says when more matched: @@ -53,7 +53,7 @@ python3 .claude/skills/read-doc/search.py RT1060 RT1064 --any Exit 0 matched, 1 nothing matched, 2 bad usage or no library — 2 means the search never ran, so fix the invocation instead of broadening. -One match → read it. Several → list and ask via AskUserQuestion. Nothing +One match → read it. Several → list them and ask the user which to read. Nothing (exit 1) → retry with fewer keywords; the part number alone often works where `<part> datasheet` does not, because words like "datasheet" and "manual" are rarely in the metadata. `--any` only changes anything with two or more @@ -66,11 +66,11 @@ Set `CALIBRE_LIBRARY` to search a library elsewhere. `search.py` prints one `FORMAT path` line per stored file: -- **PDF** — Read with `pages`; for >10 pages start `pages: "1-20"` (TOC/overview), - report the page count, then read sections on demand. -- **Any other format** (EPUB, MOBI, CHM, ZIP…) — Read has no decoder for these - and returns mojibake rather than an error. Say the document is not in a - readable format; do not paste what Read returned. +- **PDF** — use the runtime's PDF-reading capability with page ranges; for >10 + pages start with pages 1-20 (TOC/overview), report the page count, then read + sections on demand. +- **Any other format** (EPUB, MOBI, CHM, ZIP…) — if the runtime has no decoder, + say the document is not in a readable format; do not paste mojibake. - **`MISSING`** — the metadata is real but the file is not on disk (library mid-sync, or the file was deleted). Report the file as unavailable, not the document as nonexistent. diff --git a/.claude/skills/rtt/SKILL.md b/.claude/skills/rtt/SKILL.md index 5e14ae84c..58d7312a3 100644 --- a/.claude/skills/rtt/SKILL.md +++ b/.claude/skills/rtt/SKILL.md @@ -169,11 +169,13 @@ then `savebin <file> <pBuffer> <SizeOfBuffer>`.) RTT console output is NOT lossless under load; for high-bandwidth streams size the buffer up (SystemView needs 2048–8192) and watch for overflow. - Non-ARM ports must supply `SEGGER_RTT_LOCK/UNLOCK`: the vendored generic - RISC-V lock uses `mstatus` CSRs that trap (mcause=2) on WCH QingKe. Worked - port on branch `claude/add-systemview-debug`: `hw/bsp/ch583/ - sysview_rtt_lock_wch.h` (brace-scoped save/restore of CSR 0x800), and the - shared `hw/bsp/sysview_rtt_conf_wch.h` that ch32v20x/ch32v30x family.cmake - force-include to win the include-guard race against the vendored conf. + RISC-V lock uses `mstatus` CSRs that trap (mcause=2) on WCH QingKe. A + working WCH port exists but is unmerged (maintainer branch + `claude/add-systemview-debug`, not on master): + `hw/bsp/ch583/sysview_rtt_lock_wch.h` (brace-scoped save/restore of CSR + 0x800), and the shared `hw/bsp/sysview_rtt_conf_wch.h` that + ch32v20x/ch32v30x family.cmake force-include to win the include-guard race + against the vendored conf. ## Common mistakes diff --git a/.claude/skills/usbtest/SKILL.md b/.claude/skills/usbtest/SKILL.md index 606b379b5..c851f85c4 100644 --- a/.claude/skills/usbtest/SKILL.md +++ b/.claude/skills/usbtest/SKILL.md @@ -41,11 +41,11 @@ python3 test/hil/usbtest.py --serial <uid> --keep-binding --tests 29 # one case actions runner. It keeps running; the per-board flock is what arbitrates (see the `hil` skill). Never start a battery by hand next to a running one: `hil_test.py` budgets 2 concurrent batteries per host controller (`HIL_USBTEST_PARALLEL`). The width itself is a profiled throughput/bandwidth - trade, not a safety ceiling (hil_lock.py:122-127) — but a battery outside the budget is a real - hazard, and the hazard is recorded: unbudgeted concurrent batteries have hard-frozen the rig with - a fatal PCIe error on a VFIO-passed xHCI, and a marginal DUT port bouncing under concurrent - batteries has killed a uPD720201 outright, which lowering the widths does not fix - (hil_lock.py:130-132). + trade, not a safety ceiling (the concurrency note above `FLASH_PARALLEL` in `hil_lock.py`) — but + a battery outside the budget is a real hazard: unbudgeted concurrent batteries have hard-frozen + the rig with a fatal PCIe error on a VFIO-passed xHCI, and a marginal DUT port bouncing under + concurrent batteries has killed a uPD720201 outright, which lowering the widths does not fix + (that note records every such death). ## Porting ladder — new MCU/DCD to 30/30 diff --git a/.codex/agents/builder.toml b/.codex/agents/builder.toml new file mode 100644 index 000000000..ed856c2d5 --- /dev/null +++ b/.codex/agents/builder.toml @@ -0,0 +1,7 @@ +name = "builder" +description = "Load the canonical builder agent from .claude/agents/builder.md." +model = "gpt-5.6-luna" +model_reasoning_effort = "low" +developer_instructions = """ +Before acting, locate the repository root with `git rev-parse --show-toplevel`, then read `.claude/agents/builder.md` from that root completely and follow its body as your role instructions. Ignore its Claude-specific YAML frontmatter. +""" diff --git a/.codex/agents/code-verifier.toml b/.codex/agents/code-verifier.toml new file mode 100644 index 000000000..1a3965594 --- /dev/null +++ b/.codex/agents/code-verifier.toml @@ -0,0 +1,7 @@ +name = "code-verifier" +description = "Load the canonical code-verifier agent from .claude/agents/code-verifier.md." +model = "gpt-5.6-sol" +model_reasoning_effort = "xhigh" +developer_instructions = """ +Before acting, locate the repository root with `git rev-parse --show-toplevel`, then read `.claude/agents/code-verifier.md` from that root completely and follow its body as your role instructions. Ignore its Claude-specific YAML frontmatter. +""" diff --git a/.codex/agents/code-writer.toml b/.codex/agents/code-writer.toml new file mode 100644 index 000000000..d63a7e863 --- /dev/null +++ b/.codex/agents/code-writer.toml @@ -0,0 +1,7 @@ +name = "code-writer" +description = "Load the canonical code-writer agent from .claude/agents/code-writer.md." +model = "gpt-5.6-sol" +model_reasoning_effort = "xhigh" +developer_instructions = """ +Before acting, locate the repository root with `git rev-parse --show-toplevel`, then read `.claude/agents/code-writer.md` from that root completely and follow its body as your role instructions. Ignore its Claude-specific YAML frontmatter. +""" diff --git a/.codex/agents/hil-operator.toml b/.codex/agents/hil-operator.toml new file mode 100644 index 000000000..efdf5cc49 --- /dev/null +++ b/.codex/agents/hil-operator.toml @@ -0,0 +1,7 @@ +name = "hil-operator" +description = "Load the canonical hil-operator agent from .claude/agents/hil-operator.md." +model = "gpt-5.6-terra" +model_reasoning_effort = "high" +developer_instructions = """ +Before acting, locate the repository root with `git rev-parse --show-toplevel`, then read `.claude/agents/hil-operator.md` from that root completely and follow its body as your role instructions. Ignore its Claude-specific YAML frontmatter. +""" diff --git a/.codex/agents/pr-ci-watcher.toml b/.codex/agents/pr-ci-watcher.toml new file mode 100644 index 000000000..212dd7ad1 --- /dev/null +++ b/.codex/agents/pr-ci-watcher.toml @@ -0,0 +1,7 @@ +name = "pr-ci-watcher" +description = "Load the canonical pr-ci-watcher agent from .claude/agents/pr-ci-watcher.md." +model = "gpt-5.6-terra" +model_reasoning_effort = "high" +developer_instructions = """ +Before acting, locate the repository root with `git rev-parse --show-toplevel`, then read `.claude/agents/pr-ci-watcher.md` from that root completely and follow its body as your role instructions. Ignore its Claude-specific YAML frontmatter. +""" diff --git a/.codex/agents/pr-review-validator.toml b/.codex/agents/pr-review-validator.toml new file mode 100644 index 000000000..eb0a3be6a --- /dev/null +++ b/.codex/agents/pr-review-validator.toml @@ -0,0 +1,7 @@ +name = "pr-review-validator" +description = "Load the canonical pr-review-validator agent from .claude/agents/pr-review-validator.md." +model = "gpt-5.6-sol" +model_reasoning_effort = "xhigh" +developer_instructions = """ +Before acting, locate the repository root with `git rev-parse --show-toplevel`, then read `.claude/agents/pr-review-validator.md` from that root completely and follow its body as your role instructions. Ignore its Claude-specific YAML frontmatter. +""" diff --git a/.codex/agents/static-analyzer.toml b/.codex/agents/static-analyzer.toml new file mode 100644 index 000000000..ad98a9772 --- /dev/null +++ b/.codex/agents/static-analyzer.toml @@ -0,0 +1,7 @@ +name = "static-analyzer" +description = "Load the canonical static-analyzer agent from .claude/agents/static-analyzer.md." +model = "gpt-5.6-terra" +model_reasoning_effort = "medium" +developer_instructions = """ +Before acting, locate the repository root with `git rev-parse --show-toplevel`, then read `.claude/agents/static-analyzer.md` from that root completely and follow its body as your role instructions. Ignore its Claude-specific YAML frontmatter. +""" diff --git a/.codex/agents/target-debugger.toml b/.codex/agents/target-debugger.toml new file mode 100644 index 000000000..78cd06a3e --- /dev/null +++ b/.codex/agents/target-debugger.toml @@ -0,0 +1,7 @@ +name = "target-debugger" +description = "Load the canonical target-debugger agent from .claude/agents/target-debugger.md." +model = "gpt-5.6-sol" +model_reasoning_effort = "xhigh" +developer_instructions = """ +Before acting, locate the repository root with `git rev-parse --show-toplevel`, then read `.claude/agents/target-debugger.md` from that root completely and follow its body as your role instructions. Ignore its Claude-specific YAML frontmatter. +""" diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index a9b026bfd..9986e100a 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -11,6 +11,10 @@ on: # those. Same-repo branches (yours or write-access contributors) auto-review. types: [opened, synchronize, reopened, ready_for_review] +concurrency: + group: claude-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: claude-review: # Skip drafts, and skip fork PRs (no secrets -> would only fail noisily) @@ -50,9 +54,6 @@ jobs: # --comment makes the code-review command post its findings to the PR. # Without it the command only prints the review to the Actions log. prompt: '/code-review:code-review ${{ github.repository }}/pull/${{ github.event.pull_request.number }} --comment' - # TEMPORARY: expose the full Claude transcript in the Actions log for - # debugging. Revert to remove once done. - show_full_output: true # The headless review was blocked on two tools: compound Bash pipelines # (sed/python/grep) and Write (it dropped a check_headings.py helper to # inspect the diff). Default permission mode gates both; Read/Grep/Glob/ @@ -61,7 +62,7 @@ jobs: # token, so any write is ephemeral and nothing can be pushed. claude_args: | --max-turns 50 - --model claude-opus-4-8 - --effort max + --model claude-opus-5 + --effort high --allowedTools Bash,Write # See https://github.com/anthropics/claude-code-action/blob/main/docs/usage.md diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index 66e36897c..f88387772 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -65,4 +65,4 @@ jobs: # command allowlist can safely contain it. Claude still edits files and # the action commits/opens the PR; the resulting commit is verified by the # repo's CircleCI matrix. --max-turns gives room to investigate + fix. - claude_args: '--max-turns 30' + claude_args: '--max-turns 30 --model claude-opus-5' @@ -8,12 +8,22 @@ Reference these instructions first; fall back to search/bash only when reality d Bias toward caution over speed. For trivial tasks, use judgment. -- **Think first** — state assumptions; ask if unclear; present alternatives instead of picking silently. +- **Think first** — state assumptions; ask if unclear; when a choice matters, name it and recommend one rather than picking silently or surveying every option. - **Simplicity** — no features, abstractions, flexibility, or error handling beyond what was asked. If 200 lines could be 50, rewrite. - **Surgical changes** — touch only what the task requires; match existing style; don't refactor working code; mention unrelated dead code rather than deleting it. Remove only orphans *your* changes created. - **Goal-driven** — turn tasks into verifiable goals ("write failing test, make it pass"). For multi-step work, state a brief `step → verify` plan. - **Worktrees** — default to a git worktree for any branch or multi-step work; never switch the shared primary checkout's branch. Sessions run concurrently: switching the primary checkout mid-flight disrupts other sessions and can silently point a review, build, or commit at the wrong diff. Only trivial one-shot fixes may skip this. Standard location: `.worktrees/<branch-name>` at the repo root (gitignored), e.g. `git worktree add .worktrees/my-branch -b my-branch`. In a new worktree, symlink the dependency dirs (`lib/*`, `hw/mcu/*`, `tools/linkermap` — the keys of `deps_all` in `tools/get_deps.py`) to the primary checkout instead of re-cloning them; only if the branch needs a different dep revision, replace that one symlink with a real dir and run `get_deps.py` for it. +## Claude and Codex Collaboration + +Claude Code is the primary harness. `CLAUDE.md` and `.claude/{agents,skills,workflows}` are canonical; `AGENTS.md -> CLAUDE.md` and `.agents -> .claude` expose the same instructions and skills to standalone Codex. `.codex/agents/*.toml` are thin adapters that pin Codex models and load the canonical Markdown roles; do not copy role bodies or maintain other Codex-specific mirrors. + +- Use `/codex:review` for an independent read-only review and `/codex:adversarial-review` to challenge the implementation or design. +- Use `/codex:rescue` for substantial bounded implementation, diagnosis, or a second pass when Claude is stuck; use its `--background`, `--resume`, and `--fresh` controls when needed. +- When Codex should use a named TinyUSB role, select its `.codex/agents/<role>.toml` adapter; the adapter loads `.claude/agents/<role>.md` as the canonical role. +- Reviews and research may run beside Claude. For write-capable delegation, use a separate worktree if Claude continues editing; otherwise yield the current worktree to Codex until it finishes. Never let both edit overlapping files in one worktree. +- `.claude/workflows/*.js` remain Claude Code-native orchestration. Codex may review or rescue work around a workflow, but no Codex-specific workflow mirror is maintained. + ## Ground Rules - **Language/style:** C99, 2-space indent (no tabs), snake_case helpers, `UPPER_CASE` macros. Public APIs use `tud_`/`tuh_`; macros use `TU_`. Headers self-contained with `#if CFG_TUSB_MCU` guards. diff --git a/docs/superpowers/followup/pr3840-skill-md-no-boards-drift.md b/docs/superpowers/followup/pr3840-skill-md-no-boards-drift.md deleted file mode 100644 index a039a8c12..000000000 --- a/docs/superpowers/followup/pr3840-skill-md-no-boards-drift.md +++ /dev/null @@ -1,38 +0,0 @@ -# `SKILL.md` contradicts the code on no-boards tables - -**Origin:** split out of PR #3840, surfaced by its second review round. Delete this file -when its own PR lands. - -`.claude/skills/hil/SKILL.md:150-151` tells the reading agent: - -> `**HIL run selected no boards.**` — the filters intersected to nothing, so there is **no -> table at all**. Report that (and the filter shown), never `"pass": true`. - -That was true when the no-boards exit wrote a bare notice. It no longer is. An -`--accumulate` no-boards run keeps the accumulated rows — deliberately, because wiping them -destroyed real results — so the artifact now reads: - -``` -**HIL run selected no boards.** filters emptied - -**✅ 1 passed · ❌ 0 failed · ⚪ 0 skipped · blank not run** - -| Board | t | duration | -... -``` - -The behaviour is correct; the documentation is wrong, and wrong in the direction that -matters. An agent is told to expect no table, sees one, and has no rule for whether those -rows are reportable. **They are not this run's** — they are a previous attempt's, carried -forward. - -**What remains:** update that bullet to describe both cases — a fresh run has no table, an -`--accumulate` run shows the previous attempt's rows under the notice and they must not be -reported as this run's. Add a test asserting the fresh case renders no matrix, so the two -halves cannot drift again. - -## Why it was split out - -PR #3840 fixed the findings that changed a verdict. This is a documentation drift: the -behaviour is correct and the doc describing it is not, so it is better reviewed on its own -than appended to a branch already carrying a module consolidation. diff --git a/docs/superpowers/plans/2026-09-04-claude-codex-collaboration.md b/docs/superpowers/plans/2026-09-04-claude-codex-collaboration.md new file mode 100644 index 000000000..98a4b0791 --- /dev/null +++ b/docs/superpowers/plans/2026-09-04-claude-codex-collaboration.md @@ -0,0 +1,178 @@ +# Claude and Codex Collaboration Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make Claude Code the primary TinyUSB harness while sharing its repository instructions and skills with Codex and using the Codex Claude Code plugin for offload and review. + +**Architecture:** Keep `CLAUDE.md` and `.claude/` as the canonical instruction, role, skill, and workflow sources. Expose them to standalone Codex through relative symlinks and thin TOML role adapters, and use Claude Code's existing `codex@openai-codex` bridge for offload and review. + +**Tech Stack:** Markdown, Git symlinks, Claude Code plugins, Codex CLI, existing Claude Code workflow syntax checker + +**Spec:** `docs/superpowers/specs/2026-09-04-claude-codex-collaboration-design.md` + +## Global Constraints + +- `CLAUDE.md` and `.claude/{agents,skills,workflows}` remain the canonical authored content. +- Keep `AGENTS.md -> CLAUDE.md` unchanged. +- Add `.agents -> .claude`; `.codex/agents/*.toml` may contain only adapter metadata and canonical-role loading instructions. +- Do not change Claude Code or Codex permission defaults or enable the stop-time review gate. +- Never allow Claude and Codex to edit overlapping files concurrently in one worktree. + +--- + +### Task 1: Canonical Repository Wiring + +**Files:** +- Create: `.agents` (relative symlink to `.claude`) +- Modify: `CLAUDE.md` + +**Interfaces:** +- Produces: `AGENTS.md -> CLAUDE.md` for Codex instruction discovery. +- Produces: `.agents/skills -> .claude/skills` for Codex skill discovery. +- Produces: a durable collaboration policy used by Claude Code and standalone Codex. + +- [x] **Step 1: Verify the compatibility link is absent** + +Run: + +```bash +test -L .agents && test "$(readlink .agents)" = .claude +``` + +Expected: FAIL because the imported directory is absent in the isolated worktree and no compatibility symlink exists yet. + +- [x] **Step 2: Add the relative compatibility symlink** + +Run: + +```bash +ln -s .claude .agents +``` + +Expected: `readlink .agents` prints `.claude`, and `git status --short` reports `?? .agents` without copied skill files. + +- [x] **Step 3: Document the collaboration contract** + +Add a concise `Claude and Codex Collaboration` section to `CLAUDE.md` after `Behavioral Guidelines`. It must state: + +```markdown +## Claude and Codex Collaboration + +Claude Code is the primary harness. `CLAUDE.md` and `.claude/{agents,skills,workflows}` are canonical; `AGENTS.md -> CLAUDE.md` and `.agents -> .claude` expose the same instructions and skills to standalone Codex. `.codex/agents/*.toml` are thin adapters that pin Codex models and load the canonical Markdown roles; do not copy role bodies or maintain other Codex-specific mirrors. + +- Use `/codex:review` for an independent read-only review and `/codex:adversarial-review` to challenge the implementation or design. +- Use `/codex:rescue` for substantial bounded implementation, diagnosis, or a second pass when Claude is stuck; use its `--background`, `--resume`, and `--fresh` controls when needed. +- When Codex should use a named TinyUSB role, select its `.codex/agents/<role>.toml` adapter; the adapter loads `.claude/agents/<role>.md` as the canonical role. +- Reviews and research may run beside Claude. For write-capable delegation, use a separate worktree if Claude continues editing; otherwise yield the current worktree to Codex until it finishes. Never let both edit overlapping files in one worktree. +- `.claude/workflows/*.js` remain Claude Code-native orchestration. Codex may review or rescue work around a workflow, but no Codex-specific workflow mirror is maintained. +``` + +- [x] **Step 4: Verify links and canonical skill resolution** + +Run: + +```bash +test -L AGENTS.md +test "$(readlink AGENTS.md)" = CLAUDE.md +test -L .agents +test "$(readlink .agents)" = .claude +test "$(realpath .agents/skills)" = "$(realpath .claude/skills)" +test "$(find .codex/agents -maxdepth 1 -name '*.toml' | wc -l)" -eq 8 +git ls-files --error-unmatch AGENTS.md CLAUDE.md +``` + +Expected: all commands succeed; all eight Codex role adapters are present. + +- [x] **Step 5: Verify all canonical dynamic workflows** + +Run: + +```bash +for workflow_file in .claude/workflows/*.js; do + .claude/workflows/check.sh "$workflow_file" +done +``` + +Expected: six `OK:` lines, one for each workflow. + +- [x] **Step 6: Check and commit the repository change** + +Run: + +```bash +git diff --check +git diff -- CLAUDE.md +git status --short +git add .agents CLAUDE.md +git commit -m "docs: share Claude harness with Codex" +``` + +Expected: the commit contains one symlink and the collaboration section; role adapters are added separately. + +--- + +### Task 2: Live Claude-to-Codex Verification + +**Files:** +- Verify only: `AGENTS.md`, `.agents/skills`, `.claude/agents`, `.codex/agents`, `.claude/workflows` + +**Interfaces:** +- Consumes: the symlinks and collaboration policy from Task 1. +- Consumes: the enabled `codex@openai-codex` Claude Code plugin. +- Produces: evidence that standalone Codex and the Claude Code companion both use the shared repository setup. + +- [x] **Step 1: Check the Claude Code Codex companion setup** + +Run from the worktree, resolving the installed plugin root first: + +```bash +plugin_root=$(claude plugin list --json | jq -r '.[] | select(.id == "codex@openai-codex" and .enabled == true) | .installPath' | head -1) +node "$plugin_root/scripts/codex-companion.mjs" setup --json +``` + +Expected: Codex CLI is installed and authenticated; the review gate remains disabled unless it was already enabled before this task. + +- [x] **Step 2: Verify standalone Codex instruction and skill discovery** + +Run: + +```bash +codex --ask-for-approval never exec --sandbox read-only \ + "Read-only setup check. State the repository instruction file you loaded and the project skill root you discovered. Do not modify files." +``` + +Expected: Codex identifies `AGENTS.md` and project skills under `.agents/skills`; the command leaves `git status --short` unchanged. + +- [x] **Step 3: Verify Claude-to-Codex role reuse** + +Run: + +```bash +codex --ask-for-approval never exec --sandbox read-only \ + "Use spawn_agent to run the builder project agent for a read-only integration check. Return only its name and whether its output contract is JSON. Do not build or modify files." +``` + +Expected: Codex selects `.codex/agents/builder.toml`, loads the canonical role, +and reports its JSON output contract without changing files. + +- [x] **Step 4: Run an independent Codex review of the integration diff** + +Run: + +```bash +node "$plugin_root/scripts/codex-companion.mjs" review --wait --base HEAD~1 --scope branch +``` + +Expected: the review completes and reports either no major issue or actionable findings; it makes no edits. + +- [x] **Step 5: Verify a clean bounded result** + +Run: + +```bash +git status --short +git diff --check HEAD~1..HEAD +git show --stat --oneline HEAD +``` + +Expected: only the implementation-plan tracking update, if any, is uncommitted; the collaboration commit is whitespace-clean and contains only `CLAUDE.md` plus `.agents`. diff --git a/docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md b/docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md index 799c83c23..888dfb8dc 100644 --- a/docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md +++ b/docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md @@ -49,7 +49,7 @@ never inflates one axis with another's breadth. | # | Changed path | Build families | Build examples | HIL boards → tests | | --- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------- | ----------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------- | | 1 | `docs/`, `.claude/`, `*.md`, `*.rst`, `LICENSE` | — | — | — | -| 1b | `.gitignore`, `.clang-format`, `.idea/**`, `test/{fuzz,unit-test}/**`, `test/hil/test/**`, non-build `.github/**`, packaging manifests | — | — | — | +| 1b | `.gitignore`, `.clang-format`, `.agents`, `.codex/**`, `.idea/**`, `test/{fuzz,unit-test}/**`, `test/hil/test/**`, non-build `.github/**`, packaging manifests | — | — | — | | 2 | `test/hil/**` (not `test/hil/test/**`) | — | — | all boards → all tests | | 2b | `tools/metrics.py`, `.github/scripts/metrics_*.py` | `ALL` (unchanged — `tinyusb_metrics` runs `metrics.py` as a build target) | `ALL` | — (nothing on the rig runs it) | | 3 | `src/portable/<port>/dcd_*`, `*_device.[ch]` | `FAM` | `DEV`+`DUAL` | `FAM`'s device-role boards → device+dual tests | diff --git a/docs/superpowers/specs/2026-09-04-claude-codex-collaboration-design.md b/docs/superpowers/specs/2026-09-04-claude-codex-collaboration-design.md new file mode 100644 index 000000000..fa8d3b6d1 --- /dev/null +++ b/docs/superpowers/specs/2026-09-04-claude-codex-collaboration-design.md @@ -0,0 +1,95 @@ +# Claude and Codex Collaboration — Design + +Date: 2026-09-04 +Branch: `claude/codex-collaboration` + +## Goal + +Keep Claude Code as the primary TinyUSB harness while making Codex available +for independent review, diagnosis, and bounded implementation work. Preserve +one repository-owned source for instructions, agents, skills, and dynamic +workflows, while keeping standalone Codex useful. + +## Source of Truth + +- `CLAUDE.md` is the only authored repository instruction file. +- `.claude/agents/`, `.claude/skills/`, and `.claude/workflows/` are the only + authored agent, skill, and workflow trees. +- `AGENTS.md` remains a relative symlink to `CLAUDE.md` so standalone Codex + loads the same repository instructions. +- `.agents` becomes a relative symlink to `.claude` so Codex discovers the + canonical skills at `.agents/skills` without a copied mirror. +- The imported `.codex/` agent tree is replaced with thin TOML adapters that + contain only each role's name, model/effort pin, and instruction to load the + canonical `.claude/agents/<role>.md` body. + +Standalone Codex therefore uses the shared project instructions, skills, and +named TinyUSB roles. JavaScript workflows remain Claude Code orchestration +surfaces. + +## Claude-to-Codex Flow + +The enabled `codex@openai-codex` Claude Code plugin is the bridge: + +- `/codex:review` performs an independent, read-only review of local git state. +- `/codex:adversarial-review` challenges the implementation and design. +- `/codex:rescue` delegates diagnosis or an explicitly requested fix. +- `/codex:status`, `/codex:result`, and `/codex:cancel` manage background work. + +Claude should offload work when a second implementation or diagnosis pass is +valuable, when it is stuck, or when a substantial bounded task can be isolated. +Simple tasks stay in the main Claude session. + +When a Codex task should follow a named TinyUSB role, select its +`.codex/agents/<role>.toml` adapter. The role body remains authored once in +`.claude/agents/<role>.md`. + +## Concurrency and Safety + +- Reviews and research may run in parallel with Claude because they are + read-only. +- Claude and Codex must not edit overlapping files concurrently in the same + worktree. +- Write-capable background delegation uses a separate worktree when Claude will + continue editing; otherwise Claude yields ownership of the current worktree + until Codex finishes. +- Codex follows the same destructive-action, HIL, push, and hardware-lock rules + through `AGENTS.md -> CLAUDE.md`. + +## Dynamic Workflows + +`.claude/workflows/*.js` stay Claude Code-native. Claude remains responsible +for workflow control flow and structured joins. Codex can be called before or +after a workflow for a second opinion or rescue task, but the workflow files +are not translated into a Codex-specific format. + +This avoids an unsupported generated workflow mirror and keeps deterministic +orchestration in one place. + +## Repository Changes + +1. Replace the untracked imported `.agents/` directory with the tracked + `.agents -> .claude` symlink. +2. Replace the untracked imported `.codex/` directory with thin agent adapters. +3. Add a concise collaboration section to `CLAUDE.md` covering delegation, + review, role reuse, and the single-writer rule. +4. Keep the existing tracked `AGENTS.md -> CLAUDE.md` symlink unchanged. + +## Verification + +- Confirm both compatibility paths are relative symlinks with the intended + targets. +- Confirm `.agents/skills/*/SKILL.md` resolves to the canonical files and every + `.codex/agents/*.toml` adapter loads its matching canonical role. +- Run every `.claude/workflows/*.js` through `.claude/workflows/check.sh`. +- Run Codex non-interactively from the worktree and verify it identifies the + shared instruction source and project skills. +- Run the Claude Code Codex companion setup check, a small rescue/read task, + and a small review task without changing repository source. + +## Out of Scope + +- Translating Claude Code dynamic workflows to a second runtime. +- Maintaining Codex-native copies of the named Claude agent bodies. +- Changing Claude Code or Codex permission defaults. +- Enabling automatic stop-time reviews without explicit user direction. diff --git a/docs/superpowers/specs/2026-09-04-neutral-skill-wording-design.md b/docs/superpowers/specs/2026-09-04-neutral-skill-wording-design.md new file mode 100644 index 000000000..ee632b1da --- /dev/null +++ b/docs/superpowers/specs/2026-09-04-neutral-skill-wording-design.md @@ -0,0 +1,32 @@ +# Neutral Skill Wording Design + +## Goal + +Make the `read-doc` and `pre-pr` skill instructions understandable in both +Claude Code and Codex without changing their behavior. + +## Changes + +- Replace slash-command wording with references to arguments or context supplied + in the skill request. +- Replace the Claude-specific `AskUserQuestion` name with the neutral instruction + to ask the user. +- Replace Claude-specific `Read` tool wording with capability-based document + reading instructions. +- Remove slash-command syntax from skill headings. + +## Deferred Scope + +Leave the `pre-pr` invocation of the Claude Code `full-check` workflow unchanged. +Standalone Codex execution of that workflow will be designed separately. + +No other skill files need changes: their instructions already use portable +Markdown, repository paths, and shell commands. + +## Verification + +- Search every `.claude/skills/*/SKILL.md` for slash-command headings, + `AskUserQuestion`, and Claude-specific `Read` tool wording. +- Confirm `pre-pr` still invokes `full-check` exactly as before. +- Confirm `.agents` still resolves to `.claude`, so Codex discovers the canonical + skill files without a generated copy. diff --git a/hw/bsp/rp2040/family.c b/hw/bsp/rp2040/family.c index 9de1658eb..3c5022eb6 100644 --- a/hw/bsp/rp2040/family.c +++ b/hw/bsp/rp2040/family.c @@ -215,7 +215,9 @@ void board_init(void) // rp2040 use pico-pio-usb for host tuh_configure() can be used to passed pio configuration to the host stack // Note: tuh_configure() must be called before tuh_init() pio_usb_configuration_t pio_cfg = PIO_USB_DEFAULT_CONFIG; +#ifdef PICO_DEFAULT_PIO_USB_DP_PIN pio_cfg.pin_dp = PICO_DEFAULT_PIO_USB_DP_PIN; +#endif tuh_configure(BOARD_TUH_RHPORT, TUH_CFGID_RPI_PIO_USB_CONFIGURATION, &pio_cfg); #endif diff --git a/src/class/cdc/cdc_device.c b/src/class/cdc/cdc_device.c index 56d4aeed9..ed050ad03 100644 --- a/src/class/cdc/cdc_device.c +++ b/src/class/cdc/cdc_device.c @@ -323,9 +323,7 @@ uint16_t cdcd_open(uint8_t rhport, const tusb_desc_interface_t* itf_desc, uint16 tu_edpt_stream_t *stream_tx = &p_cdc->tx_stream; tu_edpt_stream_open(stream_tx, rhport, desc_ep, CFG_TUD_CDC_TX_EPSIZE); - #if CFG_TUD_CDC_TX_PERSISTENT - tu_edpt_stream_write_xfer(stream_tx); // flush pending data - #else + #if !CFG_TUD_CDC_TX_PERSISTENT tu_edpt_stream_clear(stream_tx); #endif } else { diff --git a/src/class/dfu/dfu_device.c b/src/class/dfu/dfu_device.c index 006a5bcb7..092abed03 100644 --- a/src/class/dfu/dfu_device.c +++ b/src/class/dfu/dfu_device.c @@ -42,9 +42,8 @@ typedef struct { static dfu_state_ctx_t _dfu_ctx; -#if CFG_TUD_DFU_XFER_BUFSIZE > CFG_TUD_ENDPOINT0_BUFSIZE -TU_ATTR_ALIGNED(4) uint8_t _transfer_buf[CFG_TUD_DFU_XFER_BUFSIZE]; -#endif +// Download data must remain valid across the following GETSTATUS control transfer +TU_ATTR_ALIGNED(4) static uint8_t _transfer_buf[CFG_TUD_DFU_XFER_BUFSIZE]; static void reset_state(void) { _dfu_ctx.state = DFU_IDLE; @@ -52,15 +51,6 @@ static void reset_state(void) { _dfu_ctx.flashing_in_progress = false; } -static inline uint8_t* get_xfer_buffer(void) { - // Use EP0 buffer if it is large enough, otherwise use dedicated buffer - #if CFG_TUD_DFU_XFER_BUFSIZE > CFG_TUD_ENDPOINT0_BUFSIZE - return _transfer_buf; - #else - return usbd_get_ctrl_buf(); - #endif -} - static bool reply_getstatus(uint8_t rhport, const tusb_control_request_t* request, dfu_state_t state, dfu_status_t status, uint32_t timeout); static bool process_download_get_status(uint8_t rhport, uint8_t stage, const tusb_control_request_t* request); static bool process_manifest_get_status(uint8_t rhport, uint8_t stage, const tusb_control_request_t* request); @@ -276,10 +266,10 @@ bool dfu_moded_control_xfer_cb(uint8_t rhport, uint8_t stage, const tusb_control TU_VERIFY(_dfu_ctx.attrs & DFU_ATTR_CAN_UPLOAD); TU_VERIFY(request->wLength <= CFG_TUD_DFU_XFER_BUFSIZE); - const uint16_t xfer_len = tud_dfu_upload_cb(_dfu_ctx.alt, request->wValue, get_xfer_buffer(), + const uint16_t xfer_len = tud_dfu_upload_cb(_dfu_ctx.alt, request->wValue, _transfer_buf, request->wLength); - return tud_control_xfer(rhport, request, get_xfer_buffer(), xfer_len); + return tud_control_xfer(rhport, request, _transfer_buf, xfer_len); } break; @@ -299,7 +289,7 @@ bool dfu_moded_control_xfer_cb(uint8_t rhport, uint8_t stage, const tusb_control if (request->wLength > 0) { // Download with payload -> transition to DOWNLOAD SYNC _dfu_ctx.state = DFU_DNLOAD_SYNC; - return tud_control_xfer(rhport, request, get_xfer_buffer(), request->wLength); + return tud_control_xfer(rhport, request, _transfer_buf, request->wLength); } else { // Download is complete -> transition to MANIFEST SYNC _dfu_ctx.state = DFU_MANIFEST_SYNC; @@ -373,7 +363,7 @@ static bool process_download_get_status(uint8_t rhport, uint8_t stage, const tus } else if (stage == CONTROL_STAGE_ACK) { if (_dfu_ctx.flashing_in_progress) { _dfu_ctx.state = DFU_DNBUSY; - tud_dfu_download_cb(_dfu_ctx.alt, _dfu_ctx.block, get_xfer_buffer(), _dfu_ctx.length); + tud_dfu_download_cb(_dfu_ctx.alt, _dfu_ctx.block, _transfer_buf, _dfu_ctx.length); } else { _dfu_ctx.state = DFU_DNLOAD_IDLE; } diff --git a/src/common/tusb_private.h b/src/common/tusb_private.h index 0bbc119fd..b91fc0608 100644 --- a/src/common/tusb_private.h +++ b/src/common/tusb_private.h @@ -63,7 +63,7 @@ TU_ATTR_ALWAYS_INLINE static inline bool tu_edpt_validate(const tusb_desc_endpoi // Bind drivers to all interfaces and endpoints in the provided configuration descriptor bool tu_bind_driver_to_ep_itf(uint8_t driver_id, uint8_t ep2drv[][2], uint8_t itf2drv[], uint8_t itf_max, - const uint8_t *p_desc, uint16_t desc_len); + uint8_t ep_max, const uint8_t *p_desc, uint16_t desc_len); // Claim an endpoint with provided mutex bool tu_edpt_claim(volatile uint8_t* ep_state, osal_mutex_t mutex); diff --git a/src/device/usbd.c b/src/device/usbd.c index e84d72fa4..21d865cf7 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -1296,8 +1296,8 @@ static bool process_set_config(uint8_t rhport, uint8_t cfg_num) { TU_LOG_USBD(" %s opened\r\n", driver->name); // bind found driver to all interfaces and endpoint within drv_len - TU_ASSERT(tu_bind_driver_to_ep_itf(drv_id, _usbd_dev.ep2drv, _usbd_dev.itf2drv, CFG_TUD_INTERFACE_MAX, p_desc, - drv_len)); + TU_ASSERT(tu_bind_driver_to_ep_itf(drv_id, _usbd_dev.ep2drv, _usbd_dev.itf2drv, CFG_TUD_INTERFACE_MAX, + CFG_TUD_ENDPPOINT_MAX, p_desc, drv_len)); p_desc += drv_len; // next Interface break; // exit driver find loop diff --git a/src/host/usbh.c b/src/host/usbh.c index f36ded0ea..fd0c9b7b5 100644 --- a/src/host/usbh.c +++ b/src/host/usbh.c @@ -398,12 +398,6 @@ TU_ATTR_ALWAYS_INLINE static inline void usbh_device_close(uint8_t rhport, uint8 _usbh_data.daddr_gen[daddr]++; (void) osal_mutex_unlock(_usbh_mutex); - // If this device has in-flight control xfer, complete as FAILED - usbh_ctrl_xfer_info_t* ctrl_info = &_usbh_data.ctrl_xfer_info; - if (daddr == ctrl_info->daddr && ctrl_info->stage != CONTROL_STAGE_IDLE) { - control_xfer_complete(daddr, XFER_RESULT_FAILED); - } - // invalidate if enumerating if (daddr == _usbh_data.enumerating_daddr) { _usbh_data.enumerating_daddr = TUSB_INDEX_INVALID_8; @@ -412,6 +406,12 @@ TU_ATTR_ALWAYS_INLINE static inline void usbh_device_close(uint8_t rhport, uint8 _usbh_data.call_after.func = NULL; } } + + // If this device has in-flight control xfer, complete as FAILED + usbh_ctrl_xfer_info_t* ctrl_info = &_usbh_data.ctrl_xfer_info; + if (daddr == ctrl_info->daddr && ctrl_info->stage != CONTROL_STAGE_IDLE) { + control_xfer_complete(daddr, XFER_RESULT_FAILED); + } } //--------------------------------------------------------------------+ @@ -1112,7 +1112,10 @@ static bool usbh_control_xfer_cb (uint8_t daddr, uint8_t ep_addr, xfer_result_t // DATA stage: initial data toggle is always 1 control_xfer_set_stage(CONTROL_STAGE_DATA); const uint8_t ep_data = tu_edpt_addr(0, request->bmRequestType_bit.direction); - TU_ASSERT(hcd_edpt_xfer(rhport, daddr, ep_data, ctrl_info->buffer, request->wLength)); + if (!hcd_edpt_xfer(rhport, daddr, ep_data, ctrl_info->buffer, request->wLength)) { + control_xfer_complete(daddr, XFER_RESULT_FAILED); + return false; + } return true; } TU_ATTR_FALLTHROUGH; @@ -1127,7 +1130,10 @@ static bool usbh_control_xfer_cb (uint8_t daddr, uint8_t ep_addr, xfer_result_t // ACK stage: toggle is always 1 control_xfer_set_stage(CONTROL_STAGE_ACK); const uint8_t ep_status = tu_edpt_addr(0, 1 - request->bmRequestType_bit.direction); - TU_ASSERT(hcd_edpt_xfer(rhport, daddr, ep_status, NULL, 0)); + if (!hcd_edpt_xfer(rhport, daddr, ep_status, NULL, 0)) { + control_xfer_complete(daddr, XFER_RESULT_FAILED); + return false; + } break; } @@ -1729,8 +1735,10 @@ static void enum_delay_async(uintptr_t state) { if (dev0_bus->hub_addr != 0) { // connected via hub TU_VERIFY(dev0_bus->hub_port != 0, ); - TU_ASSERT(hub_port_get_status(dev0_bus->hub_addr, dev0_bus->hub_port, NULL, process_enumeration, - ENUM_HUB_RERSET), ); + if (!hub_port_get_status(dev0_bus->hub_addr, dev0_bus->hub_port, NULL, process_enumeration, + ENUM_HUB_RERSET)) { + enum_full_complete(false); + } } else #endif { @@ -1773,9 +1781,11 @@ static void enum_delay_async(uintptr_t state) { case ENUM_AFTER_RESET_HUB_DELAY: case ENUM_AFTER_RESET_HUB_DELAY_RETRY: // get status after reset complete to check for reset change - TU_ASSERT(hub_port_get_status(dev0_bus->hub_addr, dev0_bus->hub_port, NULL, process_enumeration, - state == ENUM_AFTER_RESET_HUB_DELAY ? ENUM_HUB_CLEAR_RESET - : ENUM_HUB_CLEAR_RESET_RETRY), ); + if (!hub_port_get_status(dev0_bus->hub_addr, dev0_bus->hub_port, NULL, process_enumeration, + state == ENUM_AFTER_RESET_HUB_DELAY ? ENUM_HUB_CLEAR_RESET + : ENUM_HUB_CLEAR_RESET_RETRY)) { + enum_full_complete(false); + } break; #endif @@ -1788,7 +1798,9 @@ static void enum_delay_async(uintptr_t state) { } // Get first 8 bytes of device descriptor for control endpoint size TU_LOG_USBH("Get 8 byte of Device Descriptor\r\n"); - TU_ASSERT(tuh_descriptor_get_device(0, _usbh_epbuf.ctrl, 8, process_enumeration, ENUM_SET_ADDR), ); + if (!tuh_descriptor_get_device(0, _usbh_epbuf.ctrl, 8, process_enumeration, ENUM_SET_ADDR)) { + enum_full_complete(false); + } break; case ENUM_AFTER_SET_ADDRESS_RECOVERY_DELAY: { @@ -1797,13 +1809,14 @@ static void enum_delay_async(uintptr_t state) { TU_ASSERT(new_dev, ); if (!usbh_edpt_control_open(new_addr, new_dev->desc_device.bMaxPacketSize0)) { TU_LOG_USBH("Failed to open new device's control endpoint\r\n"); - clear_device(new_dev); enum_full_complete(false); return; } TU_LOG_USBH("Get Device Descriptor\r\n"); - TU_ASSERT(tuh_descriptor_get_device(new_addr, _usbh_epbuf.ctrl, sizeof(tusb_desc_device_t), process_enumeration, - ENUM_GET_STRING_LANGUAGE_ID_LEN), ); + if (!tuh_descriptor_get_device(new_addr, _usbh_epbuf.ctrl, sizeof(tusb_desc_device_t), process_enumeration, + ENUM_GET_STRING_LANGUAGE_ID_LEN)) { + enum_full_complete(false); + } break; } @@ -1848,8 +1861,8 @@ static void process_enumeration(tuh_xfer_t *xfer) { TU_LOG_USBH("Device unplugged from hub while debouncing\r\n"); is_enum_failed = true; } else { - TU_ASSERT(hub_port_reset(dev0_bus->hub_addr, dev0_bus->hub_port, process_enumeration, - ENUM_HUB_RESET_COMPLETE), ); + is_enum_failed = !hub_port_reset(dev0_bus->hub_addr, dev0_bus->hub_port, process_enumeration, + ENUM_HUB_RESET_COMPLETE); } break; } @@ -1866,8 +1879,8 @@ static void process_enumeration(tuh_xfer_t *xfer) { if (1 == port_status.change.reset) { // Acknowledge Port Reset Change - TU_ASSERT(hub_port_clear_reset_change(dev0_bus->hub_addr, dev0_bus->hub_port, process_enumeration, - ENUM_HUB_CLEAR_RESET_COMPLETE), ); + is_enum_failed = !hub_port_clear_reset_change(dev0_bus->hub_addr, dev0_bus->hub_port, process_enumeration, + ENUM_HUB_CLEAR_RESET_COMPLETE); } else if (state == ENUM_HUB_CLEAR_RESET) { // retry one more time if reset change not set yet usbh_defer_func_ms_async(ENUM_RESET_HUB_DELAY_MS, enum_delay_async, ENUM_AFTER_RESET_HUB_DELAY_RETRY); @@ -1912,10 +1925,9 @@ static void process_enumeration(tuh_xfer_t *xfer) { usbh_device_t* new_dev = get_device(new_addr); new_dev->bus_info = *dev0_bus; - new_dev->connected = 1; new_dev->desc_device.bMaxPacketSize0 = desc_device->bMaxPacketSize0; - TU_ASSERT(tuh_address_set(0, new_addr, process_enumeration, ENUM_GET_DEVICE_DESC), ); + is_enum_failed = !tuh_address_set(0, new_addr, process_enumeration, ENUM_GET_DEVICE_DESC); break; } @@ -1923,6 +1935,7 @@ static void process_enumeration(tuh_xfer_t *xfer) { const uint8_t new_addr = (uint8_t)tu_le16toh(xfer->setup->wValue); usbh_device_t *new_dev = get_device(new_addr); TU_ASSERT(new_dev, ); + new_dev->connected = 1; new_dev->addressed = 1; _usbh_data.enumerating_daddr = new_addr; @@ -1940,15 +1953,15 @@ static void process_enumeration(tuh_xfer_t *xfer) { memcpy(&dev->desc_device, (const uint8_t*) desc_device + offsetof(tusb_desc_device_t, bcdUSB), sizeof(desc_device_noheader_t)); tuh_enum_descriptor_device_cb(daddr, desc_device); // callback - tuh_descriptor_get_string_langid(daddr, _usbh_epbuf.ctrl, 2, - process_enumeration, ENUM_GET_STRING_LANGUAGE_ID); + is_enum_failed = !tuh_descriptor_get_string_langid(daddr, _usbh_epbuf.ctrl, 2, + process_enumeration, ENUM_GET_STRING_LANGUAGE_ID); break; } case ENUM_GET_STRING_LANGUAGE_ID: { const uint8_t str_len = xfer->buffer[0]; - tuh_descriptor_get_string_langid(daddr, _usbh_epbuf.ctrl, str_len, - process_enumeration, ENUM_GET_STRING_MANUFACTURER_LEN); + is_enum_failed = !tuh_descriptor_get_string_langid(daddr, _usbh_epbuf.ctrl, str_len, + process_enumeration, ENUM_GET_STRING_MANUFACTURER_LEN); break; } @@ -1958,8 +1971,8 @@ static void process_enumeration(tuh_xfer_t *xfer) { langid = tu_le16toh(desc_langid->utf16le[0]); // previous request is langid } if (dev->desc_device.iManufacturer != 0) { - tuh_descriptor_get_string(daddr, dev->desc_device.iManufacturer, langid, _usbh_epbuf.ctrl, 2, - process_enumeration, ENUM_GET_STRING_MANUFACTURER); + is_enum_failed = !tuh_descriptor_get_string(daddr, dev->desc_device.iManufacturer, langid, _usbh_epbuf.ctrl, 2, + process_enumeration, ENUM_GET_STRING_MANUFACTURER); break; } TU_ATTR_FALLTHROUGH; @@ -1969,8 +1982,8 @@ static void process_enumeration(tuh_xfer_t *xfer) { if (dev->desc_device.iManufacturer != 0) { langid = tu_le16toh(xfer->setup->wIndex); // langid from length's request const uint8_t str_len = xfer->buffer[0]; - tuh_descriptor_get_string(daddr, dev->desc_device.iManufacturer, langid, _usbh_epbuf.ctrl, str_len, - process_enumeration, ENUM_GET_STRING_PRODUCT_LEN); + is_enum_failed = !tuh_descriptor_get_string(daddr, dev->desc_device.iManufacturer, langid, _usbh_epbuf.ctrl, + str_len, process_enumeration, ENUM_GET_STRING_PRODUCT_LEN); break; } TU_ATTR_FALLTHROUGH; @@ -1981,8 +1994,8 @@ static void process_enumeration(tuh_xfer_t *xfer) { if (state == ENUM_GET_STRING_PRODUCT_LEN) { langid = tu_le16toh(xfer->setup->wIndex); // get langid from previous setup packet if not fall through } - tuh_descriptor_get_string( - daddr, dev->desc_device.iProduct, langid, _usbh_epbuf.ctrl, 2, process_enumeration, ENUM_GET_STRING_PRODUCT); + is_enum_failed = !tuh_descriptor_get_string(daddr, dev->desc_device.iProduct, langid, _usbh_epbuf.ctrl, 2, + process_enumeration, ENUM_GET_STRING_PRODUCT); break; } TU_ATTR_FALLTHROUGH; @@ -1992,8 +2005,8 @@ static void process_enumeration(tuh_xfer_t *xfer) { if (dev->desc_device.iProduct != 0) { langid = tu_le16toh(xfer->setup->wIndex); // langid from length's request const uint8_t str_len = xfer->buffer[0]; - tuh_descriptor_get_string(daddr, dev->desc_device.iProduct, langid, _usbh_epbuf.ctrl, str_len, - process_enumeration, ENUM_GET_STRING_SERIAL_LEN); + is_enum_failed = !tuh_descriptor_get_string(daddr, dev->desc_device.iProduct, langid, _usbh_epbuf.ctrl, str_len, + process_enumeration, ENUM_GET_STRING_SERIAL_LEN); break; } TU_ATTR_FALLTHROUGH; @@ -2004,8 +2017,8 @@ static void process_enumeration(tuh_xfer_t *xfer) { if (state == ENUM_GET_STRING_SERIAL_LEN) { langid = tu_le16toh(xfer->setup->wIndex); // get langid from previous setup packet if not fall through } - tuh_descriptor_get_string( - daddr, dev->desc_device.iSerialNumber, langid, _usbh_epbuf.ctrl, 2, process_enumeration, ENUM_GET_STRING_SERIAL); + is_enum_failed = !tuh_descriptor_get_string(daddr, dev->desc_device.iSerialNumber, langid, _usbh_epbuf.ctrl, 2, + process_enumeration, ENUM_GET_STRING_SERIAL); break; } TU_ATTR_FALLTHROUGH; @@ -2015,8 +2028,8 @@ static void process_enumeration(tuh_xfer_t *xfer) { if (dev->desc_device.iSerialNumber != 0) { langid = tu_le16toh(xfer->setup->wIndex); // langid from length's request const uint8_t str_len = xfer->buffer[0]; - tuh_descriptor_get_string(daddr, dev->desc_device.iSerialNumber, langid, _usbh_epbuf.ctrl, str_len, - process_enumeration, ENUM_GET_9BYTE_CONFIG_DESC); + is_enum_failed = !tuh_descriptor_get_string(daddr, dev->desc_device.iSerialNumber, langid, _usbh_epbuf.ctrl, + str_len, process_enumeration, ENUM_GET_9BYTE_CONFIG_DESC); break; } TU_ATTR_FALLTHROUGH; @@ -2026,8 +2039,8 @@ static void process_enumeration(tuh_xfer_t *xfer) { // Get 9-byte for total length uint8_t const config_idx = 0; TU_LOG_USBH("Get Configuration[%u] Descriptor (9 bytes)\r\n", config_idx); - TU_ASSERT(tuh_descriptor_get_configuration(daddr, config_idx, _usbh_epbuf.ctrl, 9, - process_enumeration, ENUM_GET_FULL_CONFIG_DESC),); + is_enum_failed = !tuh_descriptor_get_configuration(daddr, config_idx, _usbh_epbuf.ctrl, 9, + process_enumeration, ENUM_GET_FULL_CONFIG_DESC); break; } @@ -2043,21 +2056,21 @@ static void process_enumeration(tuh_xfer_t *xfer) { // Get full configuration descriptor uint8_t const config_idx = (uint8_t) tu_le16toh(xfer->setup->wIndex); TU_LOG_USBH("Get Configuration[%u] Descriptor\r\n", config_idx); - TU_ASSERT(tuh_descriptor_get_configuration(daddr, config_idx, _usbh_epbuf.ctrl, total_len, - process_enumeration, ENUM_SET_CONFIG),); + is_enum_failed = !tuh_descriptor_get_configuration(daddr, config_idx, _usbh_epbuf.ctrl, total_len, + process_enumeration, ENUM_SET_CONFIG); break; } case ENUM_SET_CONFIG: { uint8_t config_idx = (uint8_t) tu_le16toh(xfer->setup->wIndex); if (tuh_enum_descriptor_configuration_cb(daddr, config_idx, (const tusb_desc_configuration_t*) _usbh_epbuf.ctrl)) { - TU_ASSERT(tuh_configuration_set(daddr, config_idx+1u, process_enumeration, ENUM_CONFIG_DRIVER),); + is_enum_failed = !tuh_configuration_set(daddr, config_idx+1u, process_enumeration, ENUM_CONFIG_DRIVER); } else { config_idx++; TU_ASSERT(config_idx < dev->desc_device.bNumConfigurations,); TU_LOG_USBH("Get Configuration[%u] Descriptor (9 bytes)\r\n", config_idx); - TU_ASSERT(tuh_descriptor_get_configuration(daddr, config_idx, _usbh_epbuf.ctrl, 9, - process_enumeration, ENUM_GET_FULL_CONFIG_DESC),); + is_enum_failed = !tuh_descriptor_get_configuration(daddr, config_idx, _usbh_epbuf.ctrl, 9, + process_enumeration, ENUM_GET_FULL_CONFIG_DESC); } break; } @@ -2161,7 +2174,8 @@ static bool enum_parse_configuration_desc(uint8_t dev_addr, tusb_desc_configurat TU_LOG_USBH(" %s opened\r\n", driver->name); // bind found driver to all interfaces and endpoint within drv_len - tu_bind_driver_to_ep_itf(drv_id, dev->ep2drv, dev->itf2drv, CFG_TUH_INTERFACE_MAX, p_desc, drv_len); + TU_ASSERT(tu_bind_driver_to_ep_itf(drv_id, dev->ep2drv, dev->itf2drv, CFG_TUH_INTERFACE_MAX, + CFG_TUH_ENDPOINT_MAX, p_desc, drv_len)); p_desc += drv_len; // next Interface break; // exit driver find loop @@ -2211,12 +2225,19 @@ void usbh_driver_set_config_complete(uint8_t dev_addr, uint8_t itf_num) { } static void enum_full_complete(bool success) { - (void)success; TU_LOG_USBH("Enumeration complete: success = %u\r\n", success); + const uint8_t daddr = _usbh_data.enumerating_daddr; _usbh_data.enumerating_daddr = TUSB_INDEX_INVALID_8; // mark enumeration as complete _usbh_data.call_after.func = NULL; + if (!success && daddr <= TOTAL_DEVICES) { + usbh_device_close(_usbh_data.dev0_bus.rhport, daddr); + if (daddr > 0) { + clear_device(get_device(daddr)); + } + } + #if CFG_TUH_HUB // Hub status is already requested in case of successful enumeration if (!success && _usbh_data.dev0_bus.hub_addr != 0) { diff --git a/src/portable/synopsys/dwc2/hcd_dwc2.c b/src/portable/synopsys/dwc2/hcd_dwc2.c index 089b839ae..5a171f80e 100644 --- a/src/portable/synopsys/dwc2/hcd_dwc2.c +++ b/src/portable/synopsys/dwc2/hcd_dwc2.c @@ -26,6 +26,12 @@ #endif #define DWC2_CHANNEL_COUNT_MAX 16u // absolute max channel count + + // Conservative time budget for enabling a slave-mode periodic OUT channel and writing its first packet before the + // current (micro)frame ends. HFNUM.FrRem is measured in PHY clocks; 1024 clocks are 17.1 us at 60 MHz, 21.3 us at + // 48 MHz, or 34.1 us at 30 MHz. Defer to SOF when less time remains. + #define DWC2_PERIODIC_OUT_MIN_FRREM 1024u + TU_VERIFY_STATIC(CFG_TUH_DWC2_ENDPOINT_MAX <= 255, "currently only use 8-bit for index"); enum { @@ -37,7 +43,9 @@ enum { }; enum { - HCD_XFER_PERIOD_SPLIT_NYET_MAX = 3 + HCD_XFER_PERIOD_SPLIT_NYET_MAX = 3, + HCD_FRAME_NUMBER_MASK = 0x3fff, + HCD_FRAME_COUNT = HCD_FRAME_NUMBER_MASK + 1 }; //-------------------------------------------------------------------- @@ -56,18 +64,22 @@ typedef struct { }; struct TU_ATTR_PACKED { - uint32_t uframe_interval : 18; // micro-frame interval + uint32_t uframe_interval : 19; // micro-frame interval uint32_t speed : 2; uint32_t next_pid : 2; // PID for next transfer uint32_t next_do_ping : 1; // Do PING for next transfer if possible (highspeed OUT) uint32_t closing : 1; // endpoint is closing - // uint32_t : 8; + uint32_t aborting : 1; // periodic DMA channel is waiting for its automatic halt + uint32_t periodic_phase : 1; // periodic transfer phase is established + uint32_t xfer_pending : 1; // periodic transfer waiting for its service interval + // uint32_t : 4; }; - uint32_t uframe_countdown; // micro-frame count down to transfer for periodic, only need 18-bit + uint32_t uframe_countdown; // micro-frame count down to transfer for periodic, only need 19-bit uint8_t* buffer; uint16_t buflen; + uint16_t periodic_frame; // frame/microframe number of the last scheduled periodic transaction } hcd_endpoint_t; // Additional info for each channel when it is active @@ -86,6 +98,7 @@ typedef struct { // be composed of multiple channel_xfer_start() (retry with NAK/NYET) uint16_t fifo_bytes; // bytes written/read from/to FIFO (may not be transferred on USB bus). uint8_t retry_disabled; // 1: channel was disabled to throttle a split retry (NAK in / XactErr out); re-arm on its halt + volatile bool aborting; // periodic DMA abort waiting for the channel's automatic halt } hcd_xfer_t; typedef struct { @@ -187,7 +200,7 @@ TU_ATTR_ALWAYS_INLINE static inline bool channel_disable(const dwc2_regs_t* dwc2 // the worst case), the controller generates a channel halted and disables the channel automatically. // - For split enabled channels (both non-periodic and periodic), channel disable must not be programmed randomly. // However, channel disable can be programmed for specific scenarios such as NAK and FrmOvrn. - if (is_period && (channel->hcsplt & HCSPLT_SPLITEN)) { + if (is_period) { return true; } } else { @@ -200,13 +213,86 @@ TU_ATTR_ALWAYS_INLINE static inline bool channel_disable(const dwc2_regs_t* dwc2 return true; } -// attempt to send IN token to receive data -TU_ATTR_ALWAYS_INLINE static inline bool channel_send_in_token(const dwc2_regs_t* dwc2, dwc2_channel_t* channel) { +// Retire all active host channels on root-port disconnect without waiting for +// Channel Halted interrupts. +// stop new channel/FIFO interrupts, flush queued slave requests, request a +// halt for enabled channels, then clear their interrupt and software state. +static void channel_cleanup_on_disconnect(dwc2_regs_t *dwc2) { + const uint32_t xfer_ints = GINTSTS_NPTX_FIFO_EMPTY | GINTSTS_PTX_FIFO_EMPTY | GINTSTS_HCINT; + dwc2->gintmsk &= ~xfer_ints; + dwc2->gintsts = xfer_ints; + dwc2->haintmsk = 0; + + const uint8_t max_channel = dwc2_channel_count(dwc2); + #if CFG_TUH_DWC2_SLAVE_ENABLE + if (!dma_host_enabled(dwc2)) { + // With CHENA clear, CHDIS flushes a posted request without consuming + // request-queue space. Clear EPDIR as required for this flush operation. + for (uint8_t ch_id = 0; ch_id < max_channel; ch_id++) { + if (_hcd_data.xfer[ch_id].allocated) { + dwc2_channel_t *channel = &dwc2->channel[ch_id]; + const uint32_t hcchar = channel->hcchar; + if (hcchar & HCCHAR_CHENA) { + channel->hcchar = (hcchar & ~(HCCHAR_CHENA | HCCHAR_EPDIR)) | HCCHAR_CHDIS; + } + } + } + } + #endif + + for (uint8_t ch_id = 0; ch_id < max_channel; ch_id++) { + if (_hcd_data.xfer[ch_id].allocated) { + dwc2_channel_t *channel = &dwc2->channel[ch_id]; + const uint32_t hcchar = channel->hcchar; + if (hcchar & HCCHAR_CHENA) { + channel->hcchar = hcchar | HCCHAR_CHDIS; + } + channel->hcintmsk = 0; + channel->hcint = 0xFFFFFFFFU; + } + } + + tu_memclr(_hcd_data.xfer, sizeof(_hcd_data.xfer)); + for (uint8_t ep_id = 0; ep_id < CFG_TUH_DWC2_ENDPOINT_MAX; ep_id++) { + hcd_endpoint_t *edpt = &_hcd_data.edpt[ep_id]; + if (edpt->hcchar_bm.enable) { + edpt->closing = 1; + edpt->xfer_pending = 0; + } + } +} + +// Enable a channel, selecting the following frame for a new periodic transfer. +// Return that frame from the same HFNUM sample used for ODDFRM selection. +// Clear CHDIS explicitly: a halted channel may retain it in HCCHAR. +TU_ATTR_ALWAYS_INLINE static inline uint16_t channel_enable(dwc2_regs_t* dwc2, dwc2_channel_t* channel, + bool next_periodic_frame) { + uint32_t hcchar = channel->hcchar & ~HCCHAR_CHDIS; + uint16_t periodic_frame = 0; + if (next_periodic_frame) { + // Prevent the USB interrupt from consuming the selected frame before + // HCCHAR.CHENA is written. Queue-space waits happen before this helper. + const uint32_t gahbcfg = dwc2->gahbcfg; + dwc2->gahbcfg = gahbcfg & ~GAHBCFG_GINT; + const uint32_t hfnum = dwc2->hfnum; + hcchar = (hcchar & ~HCCHAR_ODDFRM) | (((hfnum & 1u) ^ 1u) << HCCHAR_ODDFRM_Pos); + channel->hcchar = hcchar | HCCHAR_CHENA; + periodic_frame = (uint16_t) ((hfnum + 1u) & HCD_FRAME_NUMBER_MASK); + dwc2->gahbcfg = gahbcfg; + } else { + channel->hcchar = hcchar | HCCHAR_CHENA; + } + return periodic_frame; +} + +// Attempt to send an IN token to receive data. For a new periodic transfer, +// select its frame only after request-queue space is available. +TU_ATTR_ALWAYS_INLINE static inline uint16_t channel_send_in_token(dwc2_regs_t* dwc2, dwc2_channel_t* channel, + bool next_periodic_frame) { while (0 == req_queue_avail(dwc2, channel_is_periodic(channel->hcchar))) { // blocking wait for request queue available } - channel->hcchar |= HCCHAR_CHENA; - return true; + return channel_enable(dwc2, channel, next_periodic_frame); } // Find currently enabled channel. Note: EP0 is bidirectional @@ -262,11 +348,13 @@ static void edpt_close(dwc2_regs_t *dwc2, uint8_t ep_id) { // Find an endpoint that is opened previously with hcd_edpt_open() // Note: EP0 is bidirectional -TU_ATTR_ALWAYS_INLINE static inline uint8_t edpt_find_opened(uint8_t dev_addr, uint8_t ep_num, uint8_t ep_dir) { +TU_ATTR_ALWAYS_INLINE static inline uint8_t edpt_find_opened(uint8_t dev_addr, uint8_t ep_num, uint8_t ep_dir, + bool include_closing) { for (uint8_t i = 0; i < (uint8_t)CFG_TUH_DWC2_ENDPOINT_MAX; i++) { const hcd_endpoint_t *edpt = &_hcd_data.edpt[i]; const dwc2_channel_char_t hcchar_bm = edpt->hcchar_bm; - if (hcchar_bm.enable && hcchar_bm.dev_addr == dev_addr && hcchar_bm.ep_num == ep_num && + if (hcchar_bm.enable && (include_closing || !edpt->closing) && hcchar_bm.dev_addr == dev_addr && + hcchar_bm.ep_num == ep_num && (ep_num == 0 || hcchar_bm.ep_dir == ep_dir)) { return i; } @@ -336,13 +424,13 @@ TU_ATTR_ALWAYS_INLINE static inline uint8_t cal_next_pid(uint8_t pid, uint8_t pa static void dfifo_host_init(uint8_t rhport, bool is_hs_phy) { const dwc2_controller_t* dwc2_controller = &_dwc2_controller[rhport]; dwc2_regs_t* dwc2 = DWC2_REG(rhport); - const dwc2_ghwcfg2_t ghwcfg2 = {.value = dwc2->ghwcfg2}; + const uint8_t channel_count = dwc2_channel_count(dwc2); // Scatter/Gather DMA mode is not yet supported. Buffer DMA only need 1 words per channel const bool is_dma = dma_host_enabled(dwc2); uint16_t dfifo_top = dwc2_controller->otg_dfifo_depth; if (is_dma) { - dfifo_top -= ghwcfg2.num_host_ch; + dfifo_top -= channel_count; } // fixed allocation for now, improve later: @@ -358,13 +446,12 @@ static void dfifo_host_init(uint8_t rhport, bool is_hs_phy) { } uint16_t nptxfsiz = 2 * nptx_largest; - uint16_t rxfsiz = 2 * (ptx_largest + 2) + ghwcfg2.num_host_ch; + uint16_t rxfsiz = 2 * (ptx_largest + 2) + channel_count; TU_ASSERT(dfifo_top >= (nptxfsiz + rxfsiz),); uint16_t ptxfsiz = dfifo_top - (nptxfsiz + rxfsiz); dwc2->gdfifocfg = (dfifo_top << GDFIFOCFG_EPINFOBASE_SHIFT) | dfifo_top; - dfifo_top -= rxfsiz; dwc2->grxfsiz = rxfsiz; dfifo_top -= nptxfsiz; @@ -548,7 +635,7 @@ bool hcd_edpt_open(uint8_t rhport, uint8_t dev_addr, const tusb_desc_endpoint_t* edpt->next_pid = HCTSIZ_PID_DATA0; switch (desc_ep->bmAttributes.xfer) { case TUSB_XFER_ISOCHRONOUS: - edpt->uframe_interval = 1 << (desc_ep->bInterval - 1); + edpt->uframe_interval = 1u << (desc_ep->bInterval - 1); if (bus_info.speed == TUSB_SPEED_FULL) { edpt->uframe_interval <<= 3; } @@ -556,7 +643,7 @@ bool hcd_edpt_open(uint8_t rhport, uint8_t dev_addr, const tusb_desc_endpoint_t* case TUSB_XFER_INTERRUPT: if (bus_info.speed == TUSB_SPEED_HIGH) { - edpt->uframe_interval = 1 << (desc_ep->bInterval - 1); + edpt->uframe_interval = 1u << (desc_ep->bInterval - 1); } else { edpt->uframe_interval = desc_ep->bInterval << 3; } @@ -566,6 +653,13 @@ bool hcd_edpt_open(uint8_t rhport, uint8_t dev_addr, const tusb_desc_endpoint_t* break; } + if (channel_is_periodic(edpt->hcchar)) { + // HFNUM cannot distinguish elapsed periods longer than one counter cycle. USB permits the host to provide a + // shorter period, so bound the selected period to the history available from HFNUM. + const uint32_t ucount = (rh_speed == TUSB_SPEED_HIGH) ? 1u : 8u; + edpt->uframe_interval = tu_min32(edpt->uframe_interval, HCD_FRAME_COUNT * ucount); + } + return true; } @@ -573,7 +667,7 @@ bool hcd_edpt_close(uint8_t rhport, uint8_t daddr, uint8_t ep_addr) { dwc2_regs_t *dwc2 = DWC2_REG(rhport); const uint8_t ep_num = tu_edpt_number(ep_addr); const uint8_t ep_dir = tu_edpt_dir(ep_addr); - const uint8_t ep_id = edpt_find_opened(daddr, ep_num, ep_dir); + const uint8_t ep_id = edpt_find_opened(daddr, ep_num, ep_dir, true); TU_ASSERT(ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); edpt_close(dwc2, ep_id); @@ -588,7 +682,10 @@ static void channel_xfer_out_wrapup(dwc2_regs_t* dwc2, uint8_t ch_id) { hcd_endpoint_t* edpt = &_hcd_data.edpt[xfer->ep_id]; const dwc2_channel_tsize_t hctsiz = {.value = channel->hctsiz}; - edpt->next_pid = hctsiz.pid; // save PID + const dwc2_channel_char_t hcchar = {.value = channel->hcchar}; + if (hcchar.ep_type != HCCHAR_EPTYPE_ISOCHRONOUS) { + edpt->next_pid = hctsiz.pid; // save PID + } /* Since hctsiz.xfersize field reflects the number of bytes transferred via the AHB, not the USB) * For IN: we can use hctsiz.xfersize as remaining bytes. @@ -597,7 +694,6 @@ static void channel_xfer_out_wrapup(dwc2_regs_t* dwc2, uint8_t ch_id) { * transfer was halted before its normal completion. */ const uint16_t remain_packets = hctsiz.packet_count; - const dwc2_channel_char_t hcchar = {.value = channel->hcchar}; const uint16_t total_packets = cal_packet_count(edpt->buflen, hcchar.ep_size); const uint16_t actual_bytes = (total_packets - remain_packets) * hcchar.ep_size; @@ -607,20 +703,26 @@ static void channel_xfer_out_wrapup(dwc2_regs_t* dwc2, uint8_t ch_id) { edpt->buflen -= actual_bytes; } -static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id) { +#if CFG_TUH_DWC2_SLAVE_ENABLE +static bool channel_txfifo_write(dwc2_regs_t* dwc2, uint8_t ch_id, bool is_periodic); +#endif +static void periodic_xfer_defer(dwc2_regs_t* dwc2, hcd_endpoint_t* edpt, uint32_t uframe_countdown); + +static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id, bool defer_periodic_out) { hcd_xfer_t* xfer = &_hcd_data.xfer[ch_id]; hcd_endpoint_t* edpt = &_hcd_data.edpt[xfer->ep_id]; dwc2_channel_char_t* hcchar_bm = &edpt->hcchar_bm; dwc2_channel_t* channel = &dwc2->channel[ch_id]; bool const is_period = channel_is_periodic(edpt->hcchar); - +#if CFG_TUH_DWC2_SLAVE_ENABLE + const uint8_t saved_pid = edpt->next_pid; + const uint8_t saved_do_ping = edpt->next_do_ping; +#endif + uint16_t periodic_frame = 0; // clear previous state xfer->fifo_bytes = 0; // hchar: restore but don't enable yet - if (is_period) { - hcchar_bm->odd_frame = 1 - (dwc2->hfnum & 1); // transfer on next frame - } channel->hcchar = (edpt->hcchar & ~HCCHAR_CHENA); // hctsiz: zero length packet still count as 1 @@ -636,15 +738,17 @@ static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id) { channel->hctsiz = hctsiz.value; edpt->next_do_ping = 0; - // pre-calculate next PID based on packet count, adjusted in transfer complete interrupt if short packet + // Single-transaction isochronous endpoints always use DATA0. Pre-calculate the next PID for other endpoints, + // adjusted in the transfer-complete interrupt if a short packet is received. if (hcchar_bm->ep_num == 0) { edpt->next_pid = HCTSIZ_PID_DATA1; // control data and status stage always start with DATA1 - } else { + } else if (hcchar_bm->ep_type != HCCHAR_EPTYPE_ISOCHRONOUS) { edpt->next_pid = cal_next_pid(edpt->next_pid, packet_count); } channel->hcsplt = edpt->hcsplt; channel->hcint = 0xFFFFFFFFU; // clear all channel interrupts + dwc2->gintmsk |= GINTSTS_HCINT; if (dma_host_enabled(dwc2)) { channel->hcintmsk = HCINT_HALTED; @@ -653,13 +757,19 @@ static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id) { channel->hcdma = (uint32_t) edpt->buffer; if (hcchar_bm->ep_dir == TUSB_DIR_IN) { - channel_send_in_token(dwc2, channel); + periodic_frame = channel_send_in_token(dwc2, channel, is_period); } else { hcd_dcache_clean(edpt->buffer, edpt->buflen); - channel->hcchar |= HCCHAR_CHENA; + periodic_frame = channel_enable(dwc2, channel, is_period); + } + } +#if CFG_TUH_DWC2_SLAVE_ENABLE + else { + uint32_t hcintmsk = HCINT_NAK | HCINT_XACT_ERR | HCINT_STALL | + HCINT_XFER_COMPLETE | HCINT_DATATOGGLE_ERR; + if (is_period) { + hcintmsk |= HCINT_FARME_OVERRUN; } - } else { - uint32_t hcintmsk = HCINT_NAK | HCINT_XACT_ERR | HCINT_STALL | HCINT_XFER_COMPLETE | HCINT_DATATOGGLE_ERR; if (hcchar_bm->ep_dir == TUSB_DIR_IN) { hcintmsk |= HCINT_BABBLE_ERR | HCINT_DATATOGGLE_ERR | HCINT_ACK; } else { @@ -677,16 +787,36 @@ static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id) { // IN Token. If we got NAK, we have to re-enable the channel again in the interrupt. Due to the way usbh stack only // call hcd_edpt_xfer() once, we will need to manage de-allocate/re-allocate IN channel dynamically. if (hcchar_bm->ep_dir == TUSB_DIR_IN) { - channel_send_in_token(dwc2, channel); + periodic_frame = channel_send_in_token(dwc2, channel, is_period); } else { - channel->hcchar |= HCCHAR_CHENA; - if (edpt->buflen > 0) { - // To prevent conflict with other channel, we will enable periodic/non-periodic FIFO empty interrupt accordingly - // And write packet in the interrupt handler + // The final FIFO word creates the OUT request. Keep CHENA and that write + // atomic with respect to this controller's ISR. + // This region never waits for FIFO or queue space. + const uint32_t gahbcfg = dwc2->gahbcfg; + dwc2->gahbcfg = gahbcfg & ~GAHBCFG_GINT; + if (defer_periodic_out && is_period) { + const dwc2_hfnum_t hfnum = {.value = dwc2->hfnum}; + if (hfnum.remainning < DWC2_PERIODIC_OUT_MIN_FRREM) { + edpt->next_pid = saved_pid; + edpt->next_do_ping = saved_do_ping; + dwc2->gahbcfg = gahbcfg; + return false; + } + } + periodic_frame = channel_enable(dwc2, channel, is_period); + if (edpt->buflen > 0 && channel_txfifo_write(dwc2, ch_id, is_period)) { + // The FIFO-empty interrupt handles only work that did not fit in the + // initial synchronous write. dwc2->gintmsk |= (is_period ? GINTSTS_PTX_FIFO_EMPTY : GINTSTS_NPTX_FIFO_EMPTY); } + dwc2->gahbcfg = gahbcfg; } } +#endif + + if (is_period && defer_periodic_out) { + edpt->periodic_frame = periodic_frame; + } return true; } @@ -698,8 +828,48 @@ static bool edpt_xfer_kickoff(dwc2_regs_t* dwc2, uint8_t ep_id) { hcd_xfer_t* xfer = &_hcd_data.xfer[ch_id]; xfer->ep_id = ep_id; xfer->result = XFER_RESULT_INVALID; + hcd_endpoint_t* edpt = &_hcd_data.edpt[ep_id]; + const bool result = channel_xfer_start(dwc2, ch_id, true); + if (!result) { + channel_dealloc(dwc2, ch_id); + periodic_xfer_defer(dwc2, edpt, 0); + return true; + } + if (channel_is_periodic(_hcd_data.edpt[ep_id].hcchar)) { + edpt->periodic_phase = 1; + edpt->xfer_pending = 0; + } + return result; +} + +static uint32_t periodic_xfer_countdown(dwc2_regs_t* dwc2, hcd_endpoint_t const* edpt) { + const uint32_t ucount = (hprt_speed_get(dwc2) == TUSB_SPEED_HIGH) ? 1u : 8u; + const uint16_t frame = (uint16_t) (dwc2->hfnum & HCD_FRAME_NUMBER_MASK); + const uint16_t elapsed_frames = (uint16_t) (frame - edpt->periodic_frame) & HCD_FRAME_NUMBER_MASK; + const uint32_t elapsed_uframes = (uint32_t) elapsed_frames * ucount; + + if (elapsed_uframes < edpt->uframe_interval) { + return edpt->uframe_interval - elapsed_uframes - ucount; + } + + // The service opportunity was missed. Keep the established phase and use + // the next interval rather than starting a new interval from this request. + return edpt->uframe_interval - (elapsed_uframes % edpt->uframe_interval) - ucount; +} + +static void periodic_xfer_defer(dwc2_regs_t* dwc2, hcd_endpoint_t* edpt, uint32_t uframe_countdown) { + const uint32_t gahbcfg = dwc2->gahbcfg; + dwc2->gahbcfg = gahbcfg & ~GAHBCFG_GINT; - return channel_xfer_start(dwc2, ch_id); + edpt->uframe_countdown = uframe_countdown; + edpt->xfer_pending = 1; + + if (0 == (dwc2->gintmsk & GINTMSK_SOFM)) { + dwc2->gintsts = GINTSTS_SOF; + dwc2->gintmsk |= GINTMSK_SOFM; + } + + dwc2->gahbcfg = gahbcfg; } bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t * buffer, uint16_t buflen) { @@ -707,10 +877,10 @@ bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t * const uint8_t ep_num = tu_edpt_number(ep_addr); const uint8_t ep_dir = tu_edpt_dir(ep_addr); - uint8_t ep_id = edpt_find_opened(dev_addr, ep_num, ep_dir); - TU_ASSERT(ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); + uint8_t ep_id = edpt_find_opened(dev_addr, ep_num, ep_dir, false); + TU_VERIFY(ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); hcd_endpoint_t *edpt = &_hcd_data.edpt[ep_id]; - TU_VERIFY(edpt->closing == 0); // skip if endpoint is closing + TU_VERIFY(edpt->closing == 0 && edpt->aborting == 0); // skip if endpoint is closing or aborting edpt->buffer = buffer; edpt->buflen = buflen; @@ -720,6 +890,26 @@ bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t * edpt->hcchar_bm.ep_dir = ep_dir; } + if (channel_is_periodic(edpt->hcchar)) { + const uint32_t ucount = (hprt_speed_get(dwc2) == TUSB_SPEED_HIGH) ? 1u : 8u; +#if CFG_TUH_DWC2_SLAVE_ENABLE + // Establish a slower slave-mode OUT schedule from SOF. bInterval=1 must be queued immediately to avoid + // losing every other service opportunity. + if (!dma_host_enabled(dwc2) && ep_dir == TUSB_DIR_OUT && !edpt->periodic_phase && + edpt->uframe_interval > ucount) { + periodic_xfer_defer(dwc2, edpt, 0); + return true; + } +#endif + if (edpt->periodic_phase && edpt->uframe_interval > ucount) { + const uint32_t countdown = periodic_xfer_countdown(dwc2, edpt); + if (countdown > 0) { + periodic_xfer_defer(dwc2, edpt, countdown); + return true; + } + } + } + return edpt_xfer_kickoff(dwc2, ep_id); } @@ -729,11 +919,39 @@ bool hcd_edpt_abort_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr) { dwc2_regs_t* dwc2 = DWC2_REG(rhport); const uint8_t ep_num = tu_edpt_number(ep_addr); const uint8_t ep_dir = tu_edpt_dir(ep_addr); - const uint8_t ep_id = edpt_find_opened(dev_addr, ep_num, ep_dir); + const uint8_t ep_id = edpt_find_opened(dev_addr, ep_num, ep_dir, false); TU_VERIFY(ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); + hcd_endpoint_t* edpt = &_hcd_data.edpt[ep_id]; + + hcd_int_disable(rhport); + + const bool xfer_pending = edpt->xfer_pending; + if (xfer_pending) { + edpt->xfer_pending = 0; + edpt->uframe_countdown = 0; + } + + if (xfer_pending) { + hcd_int_enable(rhport); + return true; + } + + // A periodic DMA channel must halt naturally at the next service boundary. Prevent a replacement transfer until the + // halt ISR retires the channel, and suppress completion for the aborted transfer. + if (dma_host_enabled(dwc2) && channel_is_periodic(edpt->hcchar)) { + const uint8_t ch_id = channel_find_enabled(dwc2, dev_addr, ep_num, ep_dir); + if (ch_id < 16) { + hcd_xfer_t* xfer = &_hcd_data.xfer[ch_id]; + edpt->aborting = 1; + xfer->aborting = true; + hcd_int_enable(rhport); + return true; + } + } - // hcd_int_disable(rhport); + hcd_int_enable(rhport); + // Channel disable may wait for request-queue space in slave mode. // Find enabled channeled and disable it, channel will be de-allocated in the interrupt handler const uint8_t ch_id = channel_find_enabled(dwc2, dev_addr, ep_num, ep_dir); if (ch_id < 16) { @@ -741,15 +959,13 @@ bool hcd_edpt_abort_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr) { channel_disable(dwc2, channel); } - // hcd_int_enable(rhport); - return true; } // Submit a special transfer to send 8-byte Setup Packet, when complete hcd_event_xfer_complete() must be invoked bool hcd_setup_send(uint8_t rhport, uint8_t dev_addr, const uint8_t setup_packet[8]) { - uint8_t ep_id = edpt_find_opened(dev_addr, 0, TUSB_DIR_OUT); - TU_ASSERT(ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); // no opened endpoint + uint8_t ep_id = edpt_find_opened(dev_addr, 0, TUSB_DIR_OUT, false); + TU_VERIFY(ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); // endpoint can close asynchronously on disconnect hcd_endpoint_t* edpt = &_hcd_data.edpt[ep_id]; edpt->next_pid = HCTSIZ_PID_SETUP; @@ -761,7 +977,7 @@ bool hcd_edpt_clear_stall(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr) { (void) rhport; const uint8_t ep_num = tu_edpt_number(ep_addr); const uint8_t ep_dir = tu_edpt_dir(ep_addr); - const uint8_t ep_id = edpt_find_opened(dev_addr, ep_num, ep_dir); + const uint8_t ep_id = edpt_find_opened(dev_addr, ep_num, ep_dir, false); TU_VERIFY(ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); hcd_endpoint_t* edpt = &_hcd_data.edpt[ep_id]; @@ -790,7 +1006,7 @@ static void channel_xfer_in_retry(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci if (xfer->period_split_nyet_count < HCD_XFER_PERIOD_SPLIT_NYET_MAX) { hcchar.odd_frame = 1 - (dwc2->hfnum & 1); // transfer on next frame channel->hcchar = hcchar.value; - channel_send_in_token(dwc2, channel); + channel_send_in_token(dwc2, channel, false); return; } else { // too many NYET, de-allocate channel with below code @@ -803,23 +1019,20 @@ static void channel_xfer_in_retry(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci // retry on next frame if bInterval is 1 hcchar.odd_frame = 1 - (dwc2->hfnum & 1); channel->hcchar = hcchar.value; - channel_send_in_token(dwc2, channel); + channel_send_in_token(dwc2, channel, false); } else { // otherwise, de-allocate channel, enable SOF set frame counter for later transfer const dwc2_channel_tsize_t hctsiz = {.value = channel->hctsiz}; - edpt->next_pid = hctsiz.pid; // save PID - edpt->uframe_countdown = edpt->uframe_interval - ucount; - // enable SOF interrupt if not already enabled - if (0 == (dwc2->gintmsk & GINTMSK_SOFM)) { - dwc2->gintsts = GINTSTS_SOF; - dwc2->gintmsk |= GINTMSK_SOFM; + if (hcchar.ep_type != HCCHAR_EPTYPE_ISOCHRONOUS) { + edpt->next_pid = hctsiz.pid; // save PID } + periodic_xfer_defer(dwc2, edpt, periodic_xfer_countdown(dwc2, edpt)); // already halted, de-allocate channel (called from DMA isr) channel_dealloc(dwc2, ch_id); } } else { // for control/bulk: retry immediately - channel_send_in_token(dwc2, channel); + channel_send_in_token(dwc2, channel, false); } } @@ -854,6 +1067,13 @@ static void handle_rxflvl_irq(uint8_t rhport) { // In packet received, pop this entry --> ACK interrupt const uint16_t byte_count = grxstsp.byte_count; hcd_xfer_t* xfer = &_hcd_data.xfer[ch_id]; + if (!xfer->allocated) { + // Discard data for a channel retired by disconnect. + for (uint16_t count = 0; count < byte_count; count += sizeof(uint32_t)) { + (void) dwc2->fifo[0][0]; + } + break; + } TU_ASSERT(xfer->ep_id < CFG_TUH_DWC2_ENDPOINT_MAX,); hcd_endpoint_t* edpt = &_hcd_data.edpt[xfer->ep_id]; @@ -883,38 +1103,50 @@ static void handle_rxflvl_irq(uint8_t rhport) { } } -// return true if there is still pending data and need more ISR +// Return true if data remains for a later FIFO-empty interrupt. +static bool channel_txfifo_write(dwc2_regs_t* dwc2, uint8_t ch_id, bool is_periodic) { + hcd_xfer_t* xfer = &_hcd_data.xfer[ch_id]; + dwc2_channel_t* channel = &dwc2->channel[ch_id]; + const dwc2_channel_char_t hcchar = {.value = channel->hcchar}; + TU_ASSERT(xfer->ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); + hcd_endpoint_t* edpt = &_hcd_data.edpt[xfer->ep_id]; + const dwc2_channel_tsize_t hctsiz = {.value = channel->hctsiz}; + const uint16_t remain_packets = hctsiz.packet_count; + + for (uint16_t i = 0; i < remain_packets; i++) { + const uint16_t remain_bytes = edpt->buflen - xfer->fifo_bytes; + const uint16_t xact_bytes = tu_min16(remain_bytes, hcchar.ep_size); + + // The packet's last FIFO word creates its request-queue entry. + // HNPTXSTS differs by one request-queue bit, which is outside these fields. + const dwc2_hptxsts_t txsts = {.value = (is_periodic ? dwc2->hptxsts : dwc2->hnptxsts)}; + if ((xact_bytes > (txsts.fifo_available << 2)) || (txsts.req_queue_available == 0)) { + return true; + } + + tu_hwfifo_write(dwc2->fifo[ch_id], edpt->buffer + xfer->fifo_bytes, xact_bytes, NULL); + xfer->fifo_bytes += xact_bytes; + } + + return false; +} + +// Return true if at least one matching channel needs another interrupt. static bool handle_txfifo_empty(dwc2_regs_t* dwc2, bool is_periodic) { const uint8_t max_channel = dwc2_channel_count(dwc2); for (uint8_t ch_id = 0; ch_id < max_channel; ch_id++) { + hcd_xfer_t* xfer = &_hcd_data.xfer[ch_id]; dwc2_channel_t* channel = &dwc2->channel[ch_id]; const dwc2_channel_char_t hcchar = {.value = channel->hcchar}; - // skip writing to FIFO if channel is expecting halted. - if (0 == (channel->hcintmsk & HCINT_HALTED) && (hcchar.ep_dir == TUSB_DIR_OUT)) { - hcd_xfer_t *xfer = &_hcd_data.xfer[ch_id]; - TU_ASSERT(xfer->ep_id < CFG_TUH_DWC2_ENDPOINT_MAX); - hcd_endpoint_t* edpt = &_hcd_data.edpt[xfer->ep_id]; - const dwc2_channel_tsize_t hctsiz = {.value = channel->hctsiz}; - const uint16_t remain_packets = hctsiz.packet_count; - for (uint16_t i = 0; i < remain_packets; i++) { - const uint16_t remain_bytes = edpt->buflen - xfer->fifo_bytes; - const uint16_t xact_bytes = tu_min16(remain_bytes, hcchar.ep_size); - - // skip if there is not enough space in FIFO and RequestQueue. - // Packet's last word written to FIFO will trigger a request queue - // Use period txsts for both p/np to get request queue space available (1-bit difference, it is small enough) - const dwc2_hptxsts_t txsts = {.value = (is_periodic ? dwc2->hptxsts : dwc2->hnptxsts)}; - if ((xact_bytes > (txsts.fifo_available << 2)) || (txsts.req_queue_available == 0)) { - return true; - } - - tu_hwfifo_write(dwc2->fifo[ch_id], edpt->buffer + xfer->fifo_bytes, xact_bytes, NULL); - xfer->fifo_bytes += xact_bytes; + if (xfer->allocated && channel_is_periodic(hcchar.value) == is_periodic && + 0 == (channel->hcintmsk & HCINT_HALTED) && hcchar.ep_dir == TUSB_DIR_OUT) { + if (channel_txfifo_write(dwc2, ch_id, is_periodic)) { + return true; } } } - return false; // no channel has pending data + return false; } static bool handle_channel_in_slave(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hcint) { @@ -932,7 +1164,8 @@ static bool handle_channel_in_slave(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t h // } if (hcint & HCINT_XFER_COMPLETE) { - if (edpt->hcchar_bm.ep_num != 0) { + if (edpt->hcchar_bm.ep_num != 0 && + edpt->hcchar_bm.ep_type != HCCHAR_EPTYPE_ISOCHRONOUS) { edpt->next_pid = hctsiz.pid; // save pid (already toggled) } @@ -945,6 +1178,17 @@ static bool handle_channel_in_slave(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t h xfer->result = XFER_RESULT_SUCCESS; } + if (channel_is_periodic(channel->hcchar) && remain_packets == 0) { + // The core has already halted a completed periodic IN channel. Complete + // it now so the next interval can be submitted without another halt IRQ. + is_done = true; + } else { + channel_disable(dwc2, channel); + } + } else if (hcint & HCINT_FARME_OVERRUN) { + if (edpt->hcchar_bm.ep_type == HCCHAR_EPTYPE_ISOCHRONOUS) { + xfer->result = XFER_RESULT_FAILED; + } channel_disable(dwc2, channel); } else if (hcint & (HCINT_XACT_ERR | HCINT_BABBLE_ERR | HCINT_STALL)) { if (hcint & HCINT_STALL) { @@ -982,7 +1226,7 @@ static bool handle_channel_in_slave(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t h channel->hcintmsk |= HCINT_NYET; hcsplt.split_compl = 1; channel->hcsplt = hcsplt.value; - channel_send_in_token(dwc2, channel); + channel_send_in_token(dwc2, channel, false); } else { // do nothing for complete split with DATA, this will trigger XferComplete and handled there } @@ -993,7 +1237,7 @@ static bool handle_channel_in_slave(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t h // still more packet to receive, also reset to start split hcsplt.split_compl = 0; channel->hcsplt = hcsplt.value; - channel_send_in_token(dwc2, channel); + channel_send_in_token(dwc2, channel, false); } } } else if (hcint & HCINT_HALTED) { @@ -1039,6 +1283,12 @@ static bool handle_channel_out_slave(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t } else if (hcint & HCINT_STALL) { xfer->result = XFER_RESULT_STALLED; channel_disable(dwc2, channel); + } else if (hcint & HCINT_FARME_OVERRUN) { + channel_xfer_out_wrapup(dwc2, ch_id); + if (edpt->hcchar_bm.ep_type == HCCHAR_EPTYPE_ISOCHRONOUS) { + xfer->result = XFER_RESULT_FAILED; + } + channel_disable(dwc2, channel); } else if (hcint & HCINT_NYET) { xfer->err_count = 0; if (hcsplt.split_en == 1u) { @@ -1074,7 +1324,7 @@ static bool handle_channel_out_slave(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t is_done = true; } else { // Got here due to NAK or NYET - TU_ASSERT(channel_xfer_start(dwc2, ch_id)); + TU_ASSERT(channel_xfer_start(dwc2, ch_id, false)); } } else if (hcint & HCINT_ACK) { xfer->err_count = 0; @@ -1126,9 +1376,13 @@ static bool handle_channel_in_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci if (xfer->closing) { is_done = true; } else { - channel_send_in_token(dwc2, channel); + channel_send_in_token(dwc2, channel, false); } } else if (hcint & (HCINT_XFER_COMPLETE | HCINT_STALL | HCINT_BABBLE_ERR)) { + if (edpt->hcchar_bm.ep_num != 0 && (hcint & HCINT_XFER_COMPLETE)) { + edpt->next_pid = hctsiz.pid; // save pid (already toggled) + } + const uint16_t remain_bytes = (uint16_t) hctsiz.xfer_size; const uint16_t remain_packets = hctsiz.packet_count; const uint16_t actual_len = edpt->buflen - remain_bytes; @@ -1187,7 +1441,7 @@ static bool handle_channel_in_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci hcchar.odd_frame = 1 - (dwc2->hfnum & 1); // transfer on next frame channel->hcchar = hcchar.value; } - channel_send_in_token(dwc2, channel); + channel_send_in_token(dwc2, channel, false); } } else if (hcint & (HCINT_NAK | HCINT_DATATOGGLE_ERR)) { xfer->err_count = 0; @@ -1204,8 +1458,12 @@ static bool handle_channel_in_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci channel_xfer_in_retry(dwc2, ch_id, hcint); } } else if (hcint & HCINT_FARME_OVERRUN) { - // retry start-split in next binterval - channel_xfer_in_retry(dwc2, ch_id, hcint); + if (hcchar.ep_type == HCCHAR_EPTYPE_ISOCHRONOUS) { + xfer->result = XFER_RESULT_FAILED; + is_done = true; + } else { + channel_xfer_in_retry(dwc2, ch_id, hcint); + } } if (xfer->closing == 1) { @@ -1234,7 +1492,7 @@ static bool handle_channel_out_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hc if (xfer->closing) { is_done = true; } else { - channel_xfer_start(dwc2, ch_id); + channel_xfer_start(dwc2, ch_id, false); } } else if (hcint & (HCINT_XFER_COMPLETE | HCINT_STALL)) { is_done = true; @@ -1248,30 +1506,38 @@ static bool handle_channel_out_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hc } channel->hcintmsk &= ~HCINT_ACK; } else if (hcint & HCINT_XACT_ERR) { - if (hcint & (HCINT_NAK | HCINT_NYET | HCINT_ACK)) { - xfer->err_count = 0; - // clean up transfer so far and start again - channel_xfer_out_wrapup(dwc2, ch_id); - channel_xfer_start(dwc2, ch_id); - } else { - xfer->err_count++; - if (xfer->err_count >= HCD_XFER_ERROR_MAX) { - xfer->result = XFER_RESULT_FAILED; - is_done = true; - } else { - // Rewind, then retry the start-split. Non-periodic SPLIT throttles via channel_disable + re-arm on - // the halt (immediate re-fire exhausts the retry budget; the disable gives the hub TT a recovery - // gap, like slave). Periodic split is excluded: channel_disable() is a no-op for it, so the halt - // never fires and the channel would wedge. Non-split re-inits immediately (Programming Guide 5.1.2.3). - channel_xfer_out_wrapup(dwc2, ch_id); - if (hcsplt.split_en && !channel_is_periodic(channel->hcchar)) { - xfer->retry_disabled = 1; - channel_disable(dwc2, channel); - } else { - channel_xfer_start(dwc2, ch_id); - } - } - } + if (hcint & (HCINT_NAK | HCINT_NYET | HCINT_ACK)) { + xfer->err_count = 0; + // clean up transfer so far and start again + channel_xfer_out_wrapup(dwc2, ch_id); + channel_xfer_start(dwc2, ch_id, false); + } else { + xfer->err_count++; + if (xfer->err_count >= HCD_XFER_ERROR_MAX) { + xfer->result = XFER_RESULT_FAILED; + is_done = true; + } else { + // Rewind, then retry the start-split. Non-periodic SPLIT throttles via channel_disable + re-arm on + // the halt (immediate re-fire exhausts the retry budget; the disable gives the hub TT a recovery + // gap, like slave). Periodic split is excluded: channel_disable() is a no-op for it, so the halt + // never fires and the channel would wedge. Non-split re-inits immediately (Programming Guide 5.1.2.3). + channel_xfer_out_wrapup(dwc2, ch_id); + if (hcsplt.split_en && !channel_is_periodic(channel->hcchar)) { + xfer->retry_disabled = 1; + channel_disable(dwc2, channel); + } else { + channel_xfer_start(dwc2, ch_id, false); + } + } + } + } else if (hcint & HCINT_FARME_OVERRUN) { + channel_xfer_out_wrapup(dwc2, ch_id); + if (edpt->hcchar_bm.ep_type == HCCHAR_EPTYPE_ISOCHRONOUS) { + xfer->result = XFER_RESULT_FAILED; + is_done = true; + } else { + channel_xfer_start(dwc2, ch_id, false); + } } else if (hcint & HCINT_NYET) { if (hcsplt.split_en && hcsplt.split_compl) { // split not yet mean hub has no data, retry complete split @@ -1292,7 +1558,7 @@ static bool handle_channel_out_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hc // Non-split OUT NAK is core-handled (5.1.2.2), so this is split-only. xfer->err_count = 0; channel_xfer_out_wrapup(dwc2, ch_id); - channel_xfer_start(dwc2, ch_id); + channel_xfer_start(dwc2, ch_id, false); } if (xfer->closing == 1) { @@ -1320,7 +1586,29 @@ static void handle_channel_irq(uint8_t rhport, bool in_isr) { dwc2_channel_char_t hcchar = {.value = channel->hcchar}; const uint32_t hcint = channel->hcint; - channel->hcint = hcint; // clear interrupt + // Slave handlers process one cause per pass. If ChHltd arrived with + // another cause, leave it pending so the next pass retires the halt. + const uint32_t hcint_clear = (!is_dma && (hcint & ~HCINT_HALTED)) ? (hcint & ~HCINT_HALTED) : hcint; + channel->hcint = hcint_clear; + + if (is_dma && xfer->aborting && (hcint & HCINT_HALTED)) { + hcd_endpoint_t* edpt = &_hcd_data.edpt[xfer->ep_id]; + const bool closing = xfer->closing; + // channel_xfer_start() predicts the PID after all requested packets; + // an aborted transfer may have completed fewer. + if (hcchar.ep_type != HCCHAR_EPTYPE_ISOCHRONOUS) { + const dwc2_channel_tsize_t hctsiz = {.value = channel->hctsiz}; + edpt->next_pid = hctsiz.pid; + } + xfer->aborting = false; + channel_dealloc(dwc2, ch_id); + if (closing) { + edpt_dealloc(edpt); + } else { + edpt->aborting = 0; + } + continue; + } bool is_done = false; if (is_dma) { @@ -1373,15 +1661,17 @@ static bool handle_sof_irq(uint8_t rhport, bool in_isr) { for(uint8_t ep_id = 0; ep_id < CFG_TUH_DWC2_ENDPOINT_MAX; ep_id++) { hcd_endpoint_t *edpt = &_hcd_data.edpt[ep_id]; if (edpt->closing == 0) { - if (edpt->hcchar_bm.enable && channel_is_periodic(edpt->hcchar) && edpt->uframe_countdown > 0) { - edpt->uframe_countdown -= tu_min32(ucount, edpt->uframe_countdown); + if (edpt->hcchar_bm.enable && channel_is_periodic(edpt->hcchar) && edpt->xfer_pending) { + if (edpt->uframe_countdown > 0) { + edpt->uframe_countdown -= tu_min32(ucount, edpt->uframe_countdown); + } if (edpt->uframe_countdown == 0) { if (!edpt_xfer_kickoff(dwc2, ep_id)) { edpt->uframe_countdown = ucount; // failed to start, try again next frame } } - more_isr = true; + more_isr = more_isr || edpt->xfer_pending; } } } @@ -1501,25 +1791,23 @@ void hcd_int_handler(uint8_t rhport, bool in_isr) { } } - if (gintsts & GINTSTS_HPRTINT) { - // Host port interrupt: source is cleared in HPRT register - // TU_LOG1_HEX(dwc2->hprt); - handle_hprt_irq(rhport, in_isr); - } - - if (gintsts & GINTSTS_HCINT) { - // Host Channel interrupt: source is cleared in HCINT register - // must be handled after TX FIFO empty - handle_channel_irq(rhport, in_isr); - } - if (gintsts & GINTSTS_DISCINT) { - // Device disconnected dwc2->gintsts = GINTSTS_DISCINT; + channel_cleanup_on_disconnect(dwc2); + hcd_event_device_remove(rhport, in_isr); - if (0 == (dwc2->hprt & HPRT_CONN_STATUS)) { - hcd_event_device_remove(rhport, in_isr); + // A fast replug can be visible without a pending connect-detect interrupt. + const uint32_t hprt = dwc2->hprt; + if (!(hprt & HPRT_CONN_DETECT) && (hprt & HPRT_CONN_STATUS)) { + hcd_event_device_attach(rhport, in_isr); } + return; + } + + if (gintsts & GINTSTS_HPRTINT) { + // Host port interrupt: source is cleared in HPRT register + // TU_LOG1_HEX(dwc2->hprt); + handle_hprt_irq(rhport, in_isr); } #if CFG_TUH_DWC2_SLAVE_ENABLE @@ -1553,6 +1841,13 @@ void hcd_int_handler(uint8_t rhport, bool in_isr) { } } #endif + + // Draining the RxFIFO completion status can assert HCINT.XferCompl. Read + // the live status here so the completion is handled in this ISR invocation. + if ((dwc2->gintsts & dwc2->gintmsk) & GINTSTS_HCINT) { + handle_channel_irq(rhport, in_isr); + } + } #endif diff --git a/src/tusb.c b/src/tusb.c index 78ee7aeda..e1548e8c1 100644 --- a/src/tusb.c +++ b/src/tusb.c @@ -274,7 +274,7 @@ bool tu_edpt_validate(const tusb_desc_endpoint_t *desc_ep, tusb_speed_t speed) { #endif bool tu_bind_driver_to_ep_itf(uint8_t driver_id, uint8_t ep2drv[][2], uint8_t itf2drv[], uint8_t itf_max, - const uint8_t *p_desc, uint16_t desc_len) { + uint8_t ep_max, const uint8_t *p_desc, uint16_t desc_len) { const uint8_t *desc_end = p_desc + desc_len; while (tu_desc_in_bounds(p_desc, desc_end)) { const uint8_t desc_type = tu_desc_type(p_desc); @@ -283,6 +283,7 @@ bool tu_bind_driver_to_ep_itf(uint8_t driver_id, uint8_t ep2drv[][2], uint8_t it const uint8_t ep_addr = ((const tusb_desc_endpoint_t *)p_desc)->bEndpointAddress; const uint8_t ep_num = tu_edpt_number(ep_addr); const uint8_t ep_dir = tu_edpt_dir(ep_addr); + TU_ASSERT(ep_num < ep_max); ep2drv[ep_num][ep_dir] = driver_id; } else if (desc_type == TUSB_DESC_INTERFACE) { const tusb_desc_interface_t *desc_itf = (const tusb_desc_interface_t *)p_desc; diff --git a/src/typec/usbc.c b/src/typec/usbc.c index dc59b35be..5b0e4423a 100644 --- a/src/typec/usbc.c +++ b/src/typec/usbc.c @@ -31,6 +31,7 @@ static bool _usbc_inited = false; // if port is initialized static bool _port_inited[TUP_TYPEC_RHPORTS_NUM]; +static bool _port_attached[TUP_TYPEC_RHPORTS_NUM]; // Max possible PD size is 262 bytes static uint8_t _rx_buf[64] TU_ATTR_ALIGNED(4); @@ -57,6 +58,11 @@ TU_ATTR_WEAK bool tuc_pd_control_received_cb(uint8_t rhport, pd_header_t const* return false; } +TU_ATTR_WEAK void tuc_attach_changed_cb(uint8_t rhport, bool attached) { + (void) rhport; + (void) attached; +} + TU_ATTR_WEAK void tcd_connect(uint8_t rhport) { (void) rhport; } @@ -90,6 +96,7 @@ bool tuc_init(uint8_t rhport, uint32_t port_type) { // Initialize stack if (!_usbc_inited) { tu_memclr(_port_inited, sizeof(_port_inited)); + tu_memclr(_port_attached, sizeof(_port_attached)); _usbc_q = osal_queue_create(&_usbc_qdef); TU_ASSERT(_usbc_q != NULL); @@ -124,8 +131,14 @@ void tuc_task_ext(uint32_t timeout_ms, bool in_isr) { if (!osal_queue_receive(_usbc_q, &event, timeout_ms)) return; switch (event.event_id) { - case TCD_EVENT_CC_CHANGED: + case TCD_EVENT_CC_CHANGED: { + bool const attached = event.cc_changed.cc_state[0] != 0 || event.cc_changed.cc_state[1] != 0; + if (_port_attached[event.rhport] != attached) { + _port_attached[event.rhport] = attached; + tuc_attach_changed_cb(event.rhport, attached); + } break; + } case TCD_EVENT_RX_COMPLETE: // TODO process message here in ISR, move to thread later diff --git a/src/typec/usbc.h b/src/typec/usbc.h index 9fca7da0d..fc4773b07 100644 --- a/src/typec/usbc.h +++ b/src/typec/usbc.h @@ -65,6 +65,7 @@ extern void tcd_int_handler(uint8_t rhport); bool tuc_pd_data_received_cb(uint8_t rhport, pd_header_t const* header, uint8_t const* dobj, uint8_t const* p_end); bool tuc_pd_control_received_cb(uint8_t rhport, pd_header_t const* header); +void tuc_attach_changed_cb(uint8_t rhport, bool attached); //--------------------------------------------------------------------+ // diff --git a/test/hil/test/test_ci_select.py b/test/hil/test/test_ci_select.py index 22fbde17b..61d3f23a4 100644 --- a/test/hil/test/test_ci_select.py +++ b/test/hil/test/test_ci_select.py @@ -203,6 +203,11 @@ class TestFallbackRules(unittest.TestCase): self.assertFalse(s['full']) self.assertEqual(s['boards'], {}) + def test_agent_metadata_is_empty_not_full(self): + s = sel(['.agents', '.codex/agents/builder.toml']) + self.assertFalse(s['full']) + self.assertEqual(s['boards'], {}) + def test_bsp_family_selects_family_boards(self): s = sel(['hw/bsp/rp2040/family.cmake']) self.assertFalse(s['full']) @@ -1624,7 +1629,7 @@ class TestBuildClassifier(unittest.TestCase): '.clang-format', '.idea/misc.xml', 'version.yml', 'library.json', 'examples/CMakePresets.json', 'test/fuzz/fuzz.cc', 'test/unit-test/project.yml', '.github/workflows/pr_comment.yml', - 'tools/gen_doc.py'): + 'tools/gen_doc.py', '.agents', '.codex/agents/builder.toml'): s = self.b([p]) self.assertFalse(s['full'], p) self.assertEqual(s['families'], [], p) diff --git a/tools/ci_select.py b/tools/ci_select.py index 1526f2064..9723ed6fb 100755 --- a/tools/ci_select.py +++ b/tools/ci_select.py @@ -23,7 +23,7 @@ _prune_buildable then intersects each family with what it can actually build. | # | Changed path | Build families | Build examples | HIL boards → tests | | 1 | `docs/`, `.claude/`, `*.md`, `*.rst`, `LICENSE` | — | — | — | -| 1b | `.gitignore`, `.clang-format`, `.idea/**`, `test/{fuzz,unit-test}/**`, `test/hil/test/**`, non-build `.github/**`, packaging manifests | — | — | — | +| 1b | `.gitignore`, `.clang-format`, `.agents`, `.codex/**`, `.idea/**`, `test/{fuzz,unit-test}/**`, `test/hil/test/**`, non-build `.github/**`, packaging manifests | — | — | — | | 2 | `test/hil/**` (not `test/hil/test/**`) | — | — | all boards → all tests | | 2b | `tools/metrics.py`, `.github/scripts/metrics_*.py` | `ALL` (unchanged — `tinyusb_metrics` runs `metrics.py` as a build target) | `ALL` | — (nothing on the rig runs it) | | 3 | `src/portable/<port>/dcd_*`, `*_device.[ch]` | `FAM` | `DEV`+`DUAL` | `FAM`'s device-role boards → device+dual tests | @@ -103,7 +103,7 @@ _NONCODE_RE = re.compile( _META_RE = re.compile( r'^(' r'\.(gitignore|gitattributes|clang-format|codespellrc|readthedocs\.yaml)$|' - r'\.pre-commit-config\.yaml$|\.PVS-Studio/|\.idea/|\.vscode/|' + r'\.pre-commit-config\.yaml$|\.PVS-Studio/|\.agents$|\.codex/|\.idea/|\.vscode/|' r'sonar-project\.properties$|library\.json$|pkg\.yml$|repository\.yml$|' r'version\.yml$|SConscript$|' r'.*CMakePresets\.json$|hw/bsp/BoardPresets\.json$|examples/west\.yml$|' |