docs: S28.12 document vendored smsc9220 known limitations

Bdd/Targets/FreeRtosLwip/README.md

@@ -25,10 +25,14 @@ oracle.
   LAN9118 (SMSC9220) low-level driver. RX is driven by the IRQ-13 `EthernetISR`
   through a task notification; TX sends pbufs via `smsc9220_send_by_chunks`.
 - `netif/smsc9220/smsc9220_eth_drv.{c,h}`, `netif/smsc9220/smsc9220_emac_config.h`:
-  the Arm low-level driver, vendored verbatim from FreeRTOS-Plus-TCP's MPS2_AN385
-  network interface (Apache-2.0; copyright and license headers preserved). Kept in
-  its own `smsc9220/` subdirectory with a `DisableFormat` `.clang-format` so
-  `analyze-format` leaves the third-party source untouched.
+  the Arm low-level driver, vendored from FreeRTOS-Plus-TCP's MPS2_AN385 network
+  interface (Apache-2.0; copyright and license headers preserved). Kept in its own
+  `smsc9220/` subdirectory with a `DisableFormat` `.clang-format` so `analyze-format`
+  leaves the third-party source untouched. Our snapshot predates upstream's
+  `-Wconversion` cleanup (PR #1245) and carries three known defects that are benign
+  on QEMU but would bite on real silicon — see
+  [`netif/smsc9220/KNOWN-LIMITATIONS.md`](netif/smsc9220/KNOWN-LIMITATIONS.md) for the
+  provenance snapshot and the defect/fix details.
 
 ## Scope
 

Bdd/Targets/FreeRtosLwip/netif/smsc9220/.clang-format

@@ -1,5 +1,6 @@
-# Vendored third-party code (Arm SMSC9220 LAN9118 driver, Apache-2.0). Keep it
-# byte-for-byte as upstream ships it — DisableFormat makes clang-format a no-op
-# here so the analyze-format lane does not try to reflow it to project style.
+# Vendored third-party code (Arm SMSC9220 LAN9118 driver, Apache-2.0). Keep it as
+# our vendored snapshot ships it — DisableFormat makes clang-format a no-op here so
+# the analyze-format lane does not try to reflow it to project style. See
+# KNOWN-LIMITATIONS.md for the upstream snapshot basis and known defects.
 DisableFormat: true
 SortIncludes: Never

Bdd/Targets/FreeRtosLwip/netif/smsc9220/KNOWN-LIMITATIONS.md

@@ -0,0 +1,124 @@
+# Vendored SMSC9220 / LAN9118 driver — provenance and known limitations
+
+`smsc9220_eth_drv.{c,h}` and `smsc9220_emac_config.h` are the Arm low-level
+LAN9118 (SMSC9220) Ethernet driver, vendored from FreeRTOS-Plus-TCP's
+`source/portable/NetworkInterface/MPS2_AN385/ether_lan9118/` (Apache-2.0;
+copyright and license headers preserved). They are kept in this `smsc9220/`
+subdirectory with a `DisableFormat` `.clang-format` so `analyze-format` leaves
+the third-party source untouched.
+
+## Provenance / snapshot
+
+Our snapshot predates upstream commit
+[`861750b9` (2025-05-22), "smsc9220_eth_drv.c; fix warnings from gcc
+-Wconversion" (#1245)](https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/pull/1245).
+Relative to current upstream `main` we are missing only that commit's narrowing
+casts (the `(char)` casts on the MAC bytes near line 958 and the `(uint32_t) -1`
+sentinel near line 1226). We do not need them because this translation unit is
+compiled with `-Wno-conversion -Wno-sign-conversion` (see the target
+`CMakeLists.txt`). The driver is therefore **not** byte-for-byte identical to
+upstream `main`; it is a known, slightly stale verbatim snapshot.
+
+## Known defects (real silicon only — benign on QEMU)
+
+Three defects exist in this driver, **unchanged in upstream `main`** as of
+2026-05. They are real bugs against actual LAN9118 silicon but **cannot fire on
+QEMU's MPS2 LAN9118 model**, which is deterministic and never enters the
+divergent code path — which is why the S28.09/S28.10/S28.11 oracle lanes are
+green. We run only under QEMU and have no real-hardware test rig, so these are
+documented rather than patched (see "Why not patched" below).
+
+### 1. (Critical) MAC CSR busy-bit poll in `smsc9220_mac_regwrite`
+
+The busy-wait loop condition (≈line 467) is:
+
+```c
+} while( time_out &&
+         ( register_map->mac_csr_cmd &
+           GET_BIT( register_map->mac_csr_cmd, MAC_CSR_CMD_BUSY_INDEX ) ) );
+```
+
+`GET_BIT` already returns `0`/`1`, so this ANDs the whole register with bit 0
+(the LSB of the MAC register address) instead of testing the BUSY bit. The
+write can return before the operation completes and race the next MAC/PHY
+access. The sibling `smsc9220_mac_regread` (≈line 416) shows the correct form:
+condition on `GET_BIT( ... )` alone.
+
+**Correct fix:** condition on `GET_BIT( register_map->mac_csr_cmd,
+MAC_CSR_CMD_BUSY_INDEX )` alone, matching `smsc9220_mac_regread`.
+
+**Benign on QEMU because:** QEMU processes a MAC-CSR command synchronously
+inside the MMIO write handler, so the readback never shows BUSY set. The early
+exit lands on an already-completed operation and reads correct data.
+
+### 2. (Major) TX filler DWORD on word-aligned chunks
+
+`fill_tx_fifo` (≈line 320) computes:
+
+```c
+uint32_t remainder_bytes = ( size_bytes % 4 );
+uint32_t filler_bytes = ( 4 - remainder_bytes );
+```
+
+For a word-aligned chunk (`remainder_bytes == 0`) this is `4` — a full zero
+DWORD is prepended and `smsc9220_send_by_chunks` sets `data_start_offset = 4`
+(≈line 1150, same `4 - (current_size % 4)` shape). The emitted packet is still
+correct (the offset tells the hardware to skip the filler), but a whole FIFO
+DWORD is wasted, and the FIFO free-space check (≈line 1126) does not account for
+it, so a chunk whose footprint is within 4 bytes of TX-FIFO capacity could
+overrun.
+
+**Correct fix:** use `(4 - remainder_bytes) % 4` at **both** `fill_tx_fifo`
+(filler bytes) and `smsc9220_send_by_chunks` (`data_start_offset_bytes`) so the
+two stay consistent — word-aligned chunks then prepend zero filler and set
+offset 0.
+
+**Benign on QEMU because:** QEMU honours `data_start_offset`, so the packet is
+byte-correct; the off-by-4 free-space error is per-packet and size-deterministic
+(never accumulates — the FIFO drains between packets), and syslog records are
+far smaller than the TX FIFO, so the overrun condition is never reached.
+
+### 3. (Major) `smsc9220_check_id` return type
+
+`smsc9220_check_id` (≈line 855) is declared returning `int` and returns `1` on a
+chip-ID mismatch:
+
+```c
+return( ( GET_BIT_FIELD( id, CHIP_ID_MASK, CHIP_ID_POS ) == CHIP_ID ) ? 0 : 1 );
+```
+
+Its only caller stores the result into an `enum smsc9220_error_t` (≈line 996),
+where `1 == SMSC9220_ERROR_TIMEOUT` — so a genuine ID mismatch is misreported as
+a timeout. Initialisation still aborts (non-zero), so this is a misleading
+diagnostic rather than a functional failure.
+
+**Correct fix:** change the signature to `enum smsc9220_error_t` and return
+`SMSC9220_ERROR_NONE` / `SMSC9220_ERROR_INTERNAL`.
+
+**Benign on QEMU because:** the model's chip ID always matches, so the mismatch
+path is never taken.
+
+## Why not patched
+
+We run this driver only under QEMU's deterministic LAN9118 model, which cannot
+exercise any of the three divergences (see each "Benign on QEMU" note). A fix
+would therefore be unverifiable on the only axis that matters — real silicon —
+and an untested change to hardware-facing code is a liability, not an
+improvement, especially as it would *look* correct. This mirrors the project's
+"no untested production code" discipline applied to vendored code.
+
+**Do not "fix" these without a real LAN9118 hardware test rig.** QEMU cannot
+show the difference; a plausible-looking change could regress real silicon with
+no signal here.
+
+## Upstream
+
+All three are present and unchanged in upstream FreeRTOS-Plus-TCP `main`, and a
+search (2026-05) found no public report, fix, fork, CVE, or erratum. They have
+been raised with the upstream community via the FreeRTOS forum (Libraries
+category) as a single good-citizen report; the driver is Arm-origin, so a fix
+may land either in the FreeRTOS-Plus-TCP copy or at the Arm source. If upstream
+lands hardware-validated fixes, prefer re-syncing to the upstream commit over
+carrying local patches.
+
+Tracked locally as story S28.12 (#479), closed as documented known-limitations.

CLAUDE.md

@@ -30,7 +30,7 @@ what matters — it becomes the permanent commit message on `main` on squash mer
 
 **Branch protection rules (configured on GitHub):**
 - Direct pushes to `main` are blocked
-- PRs require all status checks to pass before merging: build-linux-gcc, build-linux-clang, build-windows-msvc, sanitize-linux-gcc, coverage-linux-gcc, analyze-tidy, analyze-cppcheck, analyze-format, analyze-iwyu, integration-linux-openssl, integration-windows-openssl, bdd-linux-syslog-ng, bdd-windows-otel, bdd-freertos-qemu-plustcp, build-freertos-host-tdd-plustcp, build-freertos-target-plustcp, summary
+- PRs require all status checks to pass before merging: build-linux-gcc, build-linux-clang, build-windows-msvc, sanitize-linux-gcc, coverage-linux-gcc, analyze-tidy, analyze-cppcheck, analyze-format, analyze-iwyu, integration-linux-openssl, integration-linux-mbedtls, integration-windows-openssl, bdd-linux-syslog-ng, bdd-windows-otel, bdd-freertos-qemu-plustcp, build-freertos-host-tdd-plustcp, build-freertos-target-plustcp, analyze-tidy-freertos-plustcp, analyze-iwyu-freertos-plustcp, bdd-freertos-qemu-lwip, build-freertos-target-lwip, analyze-tidy-freertos-lwip, analyze-iwyu-freertos-lwip, summary
 - Squash merge only — other merge strategies are disabled
 - Branches are deleted automatically after merge
 

DEVLOG.md

@@ -13167,3 +13167,59 @@ MISRA rule — different category, doesn't set precedent.
   every untriaged style finding becomes a hard gate. Worth E24
   discussion.
 
+
+## 2026-05-30 — S28.12: vendored smsc9220 driver — document, report, no patch
+
+### Decisions
+
+- **Closed S28.12 (#479) as documented known-limitations, not a code
+  patch.** 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 provably inert on QEMU's
+  deterministic MPS2 LAN9118 model — each only diverges on a path the
+  model never enters (BUSY never held, FIFO never near-full with small
+  syslog records, chip ID always matches). 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. Applied the project's "no untested production code"
+  discipline to vendored code: document, don't patch.
+
+- **Confirmed they are not a BDD-flakiness source.** Flakiness needs a
+  non-deterministic source; all three are dead code on the deterministic
+  model and produce bit-identical results every run. Real flake suspects
+  (if any appear) live in the async/timing layer — tcpip thread, RX
+  IRQ→task-notify, ARP timing, syslog-ng readiness, Nagle, harness log
+  handling — not in this driver's logic.
+
+- **Provenance honesty fix.** Established our snapshot predates upstream
+  PR #1245 (2025-05-22 `-Wconversion` cleanup): we're missing only its
+  narrowing casts, which we don't need because the TU is built with
+  `-Wno-conversion -Wno-sign-conversion`. Dropped the "byte-for-byte
+  verbatim" wording in the README and the driver's `.clang-format`;
+  added `netif/smsc9220/KNOWN-LIMITATIONS.md` with the snapshot basis,
+  the three defects (location + correct fix + why benign on QEMU), and a
+  "do not fix without a real-hardware rig" warning.
+
+- **Report upstream as one forum post, not issues or a PR.** Searched
+  (via Claude AI) for prior reports across FreeRTOS-Plus-TCP, TF-M,
+  mbed-os, CMSIS, Zephyr, CVEs — none found. FreeRTOS's CONTRIBUTING
+  points to the forum first; settled on a single good-citizen post under
+  the FreeRTOS forum **Libraries** category (no per-library +TCP
+  sub-category) covering all three, with a provenance note that the
+  driver is Arm-origin (also in TF-M/mbed-os/CMSIS) so a fix may belong
+  at the Arm source too. Not filing issues or a merge-ready PR — we
+  can't validate on real HW, and the aim is to put it on the record
+  without committing to chase follow-ups.
+
+### Deferred
+
+- **Re-sync to current upstream `main`.** Not done — out of scope for a
+  document-and-close story, and re-pinning is itself an untested driver
+  change. If upstream ever lands hardware-validated fixes for the three
+  defects, prefer re-syncing to that commit over carrying local patches.
+
+### Open questions
+
+- None outstanding. Forum post drafted for David to post under the
+  Libraries category at his discretion; in-repo docs land via this PR.