Skip to content

docs: S28.12 document vendored smsc9220 known limitations#483

Merged
DavidCozens merged 2 commits into
mainfrom
docs/s28.12-smsc9220-known-limitations
May 30, 2026
Merged

docs: S28.12 document vendored smsc9220 known limitations#483
DavidCozens merged 2 commits into
mainfrom
docs/s28.12-smsc9220-known-limitations

Conversation

@DavidCozens

@DavidCozens DavidCozens commented May 30, 2026

Copy link
Copy Markdown
Owner

Purpose

Closes #479. The S28.09 CodeRabbit review flagged three real defects in the
vendored, Arm-origin SMSC9220 / LAN9118 driver under
Bdd/Targets/FreeRtosLwip/netif/smsc9220/. They are benign on QEMU's
deterministic MPS2 LAN9118 model (which is why every oracle lane is green) but
would bite real silicon. We run only under QEMU and have no real-hardware rig,
so a fix would be unverifiable on the only axis that matters — an untested change
to hardware-facing code is a liability. This PR takes the document-and-report
path rather than patching.

Change Description

  • Adds netif/smsc9220/KNOWN-LIMITATIONS.md — provenance (our snapshot
    predates upstream's -Wconversion cleanup, PR #1245; the casts are unneeded
    because the TU builds with -Wno-conversion -Wno-sign-conversion) plus the
    three defects with exact locations, the correct fix for each, and why each is
    inert on QEMU. Includes an explicit "do not fix without a real-hardware rig"
    warning, since QEMU cannot exercise the difference.
  • Softens the "byte-for-byte verbatim" wording in the target README and the
    driver's .clang-format — our copy is a known, slightly-stale snapshot, not a
    verbatim mirror of upstream main.
  • Records the decision in DEVLOG.md, including the verdict that the three
    defects cannot contribute to BDD flakiness (deterministic model → no
    non-deterministic source) and the choice to report upstream as a single
    good-citizen FreeRTOS-forum post (Libraries category) rather than file issues
    or an untested PR.
  • Driver source is untouched (smsc9220_eth_drv.{c,h},
    smsc9220_emac_config.h unchanged) — this is docs only.

Test Evidence

Documentation only; no production or test code changed. No behavioural change,
so no tests added. The QEMU oracle lanes are unaffected (the driver binary is
byte-identical to before this PR).

Areas Affected

Bdd/Targets/FreeRtosLwip/ (Tier-3 BDD target) docs only — README,
netif/smsc9220/.clang-format comment, new netif/smsc9220/KNOWN-LIMITATIONS.md,
and the root DEVLOG.md. No core/library or platform code touched; no impact on
derived projects.

Summary by CodeRabbit

  • Documentation
    • Updated documentation for a vendored networking driver component to clarify known limitations and provenance.
    • Added detailed limitations reference guide for the SMSC9220 LAN9118 driver.
    • Updated CI workflow documentation to reflect expanded status check requirements.
    • Added development log entry documenting driver assessment and known constraints.

Review Change Stack

DavidCozens and others added 2 commits May 30, 2026 09:17
Adds the checks gated on main that the doc had drifted from:
integration-linux-mbedtls (added earlier) plus the FreeRTOS analyze and
lwIP lanes now required after S28.11 — analyze-tidy/iwyu-freertos-plustcp,
bdd-freertos-qemu-lwip, build-freertos-target-lwip, and
analyze-tidy/iwyu-freertos-lwip. The live set is now 24 checks.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The three CodeRabbit findings in the vendored Arm LAN9118 driver (busy-bit
poll in smsc9220_mac_regwrite, TX filler DWORD on word-aligned chunks,
smsc9220_check_id returning int into an enum) are real against silicon but
inert on QEMU's deterministic MPS2 LAN9118 model. We run only under QEMU and
have no real-hardware rig, so a fix would be unverifiable — document rather
than patch.

Adds netif/smsc9220/KNOWN-LIMITATIONS.md (provenance snapshot + the three
defects with locations, correct fixes, and why each is benign on QEMU),
softens the 'byte-for-byte verbatim' wording in the README and the driver's
.clang-format (our snapshot predates upstream -Wconversion PR #1245), and
records the decision in DEVLOG.

Closes #479

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8820858e-aad1-442d-a3a2-cedaeaf4024c

📥 Commits

Reviewing files that changed from the base of the PR and between 3a0ffdd and 508a0dc.

📒 Files selected for processing (5)
  • Bdd/Targets/FreeRtosLwip/README.md
  • Bdd/Targets/FreeRtosLwip/netif/smsc9220/.clang-format
  • Bdd/Targets/FreeRtosLwip/netif/smsc9220/KNOWN-LIMITATIONS.md
  • CLAUDE.md
  • DEVLOG.md
👮 Files not reviewed due to content moderation or server errors (1)
  • DEVLOG.md

📝 Walkthrough

Walkthrough

This PR documents three known hardware defects in the vendored smsc9220/LAN9118 driver that are benign on QEMU but problematic on real silicon. Instead of patching, a KNOWN-LIMITATIONS.md file establishes provenance, describes each defect, and justifies deferred patching pending real hardware testing. Cross-references are added to README and .clang-format comment. DEVLOG records S28.12 closure and upstream reporting plans. CI workflow status checks are expanded.

Changes

Vendored smsc9220 Driver Documentation and CI Updates

Layer / File(s) Summary
smsc9220 Known Limitations Documentation
Bdd/Targets/FreeRtosLwip/netif/smsc9220/KNOWN-LIMITATIONS.md
KNOWN-LIMITATIONS.md establishes driver snapshot provenance (based on FreeRTOS-Plus-TCP prior to upstream -Wconversion cleanup), documents three known defects unchanged from upstream: (1) MAC CSR busy-bit polling bug in smsc9220_mac_regwrite causing potential race conditions, (2) TX FIFO padding waste on word-aligned chunks in smsc9220_send_by_chunks, (3) smsc9220_check_id return-type mismatch misreporting ID mismatch as timeout. Justifies non-patching due to QEMU benignity and lack of real LAN9118 hardware testing; notes upstream search found no public patches.
Documentation Cross-References and Closure Record
Bdd/Targets/FreeRtosLwip/README.md, Bdd/Targets/FreeRtosLwip/netif/smsc9220/.clang-format, DEVLOG.md
README.md section on vendored smsc9220 driver updated to clarify snapshot relationship to upstream conversion cleanup and cross-reference KNOWN-LIMITATIONS.md. .clang-format comment block clarified with provenance wording and reference to new limitations doc. DEVLOG entry S28.12 (2026-05-30) records closure decision: documented known limitations rather than code patch; plans single upstream forum post and defers re-sync to current upstream main.
CI Workflow Status Checks Expansion
CLAUDE.md
Required PR status checks list expanded to include additional analyze-*, integration-*, and bdd-* CI job names.

Possibly Related PRs

  • DavidCozens/solid-syslog#476: PR #476 originally flagged these three smsc9220 defects in CodeRabbit review; this PR documents them as known limitations with provenance and closure decision.
  • DavidCozens/solid-syslog#287: Both PRs update CLAUDE.md to expand the required PR status checks list with additional CI job names.
  • DavidCozens/solid-syslog#216: Both PRs modify CLAUDE.md CI workflow documentation, though with different scope and context.

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~10 minutes


🐰 Three bugs found in the driver old,
QEMU benign, but stories to be told!
Documented well, not patched with care,
Real hardware waits—we'll meet it there. 🔧

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: documenting known limitations in the vendored smsc9220 driver as part of story S28.12.
Description check ✅ Passed The description includes all required template sections: Purpose (links to #479), Change Description (detailed list of additions and modifications), Test Evidence (acknowledges documentation-only nature), and Areas Affected (specifies scope).
Linked Issues check ✅ Passed The PR fully addresses #479 requirements: documents three defects with exact locations and fixes, explains why each is benign on QEMU, includes provenance information, records the decision not to patch (choosing to report upstream instead), and keeps the driver source untouched as intended.
Out of Scope Changes check ✅ Passed All changes are directly aligned with #479 objectives: KNOWN-LIMITATIONS.md documents the defects, README and .clang-format softening clarify snapshot status, and DEVLOG.md records the decision. The CLAUDE.md update (required checks alignment) is a separate, necessary maintenance change unrelated to the core PR scope but valid as a prerequisite.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/s28.12-smsc9220-known-limitations

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown
Contributor

☀️   Quality Summary

   🚦   build-linux-gcc: 100% successful (✔️ 1344 passed)
   🚦   build-freertos-host-tdd-plustcp: 100% successful (✔️ 1652 passed)
   🚦   build-linux-clang: 100% successful (✔️ 1296 passed)
   🚦   sanitize-linux-gcc: 100% successful (✔️ 1296 passed)
   🚦   integration-linux-openssl: 100% successful (✔️ 9 passed)
   🚦   integration-linux-mbedtls: 100% successful (✔️ 7 passed)
   🚦   integration-windows-openssl: 100% successful (✔️ 9 passed)
   🚦   bdd-linux-syslog-ng: 94% successful (✔️ 46 passed, 🙈 3 skipped)
   🚦   bdd-windows-otel: 90% successful (✔️ 44 passed, 🙈 5 skipped)
   🚦   bdd-freertos-qemu-plustcp: 86% successful (✔️ 42 passed, 🙈 7 skipped)
   🚦   bdd-freertos-qemu-lwip: 65% successful (✔️ 32 passed, 🙈 17 skipped)
   🚦   build-windows-msvc: 100% successful (✔️ 1149 passed)
   🚦   build-linux-tunable-override: 100% successful (✔️ 1296 passed)
   ⚠️   Clang-Tidy: No warnings
   ⚠️   CPPCheck: No warnings


Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result.

@DavidCozens DavidCozens merged commit 868017e into main May 30, 2026
27 checks passed
@DavidCozens DavidCozens deleted the docs/s28.12-smsc9220-known-limitations branch May 30, 2026 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

S28.12: Harden vendored smsc9220 LAN9118 driver for real hardware

1 participant