From 475df5b3b6bcc8db524030b2777a77c26d1272a5 Mon Sep 17 00:00:00 2001 From: SoundMatt <47545907+SoundMatt@users.noreply.github.com> Date: Thu, 20 Aug 2026 09:03:54 -0700 Subject: [PATCH 1/2] =?UTF-8?q?feat:=20implement=20Table=2030=20Row-2=20ev?= =?UTF-8?q?t[2:0]=20validation=20=E2=80=94=20MDIO=20(8th=20and=20last=20en?= =?UTF-8?q?dpoint=20type)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TC18 §13.5 Table 33 groups 8 RCP endpoint types (ADC, PWM_IN, I2C, LIN, CAN, UART, ISELED, MDIO) into a shared Row-2 evt[2:0] request-side rule. mdio.rs becomes the eighth and last of these to call the shared evtgroup::evt_row2_kind_of predicate, closing out the full Table 30/33 Row-2 evt[2:0] validation rollout (8/8 endpoint types). New pub items (MINOR bump, v5.10.0 -> v5.11.0): - mdio::MdioRequest / mdio::MdioRequest::from_evt_sub_opcode Signature shape: from_evt_sub_opcode takes an already-decoded MdioTransfer (mirroring can::CanRequest/iseled::IseledRequest), not raw byte_msg_payload bytes (unlike i2c/lin/adc/pwm/uart), because MdioTransfer already has its own dedicated decode() entry point. ConfigWrite judgment call: kept the Ok(Self::ConfigWrite) majority precedent (i2c/lin/adc/pwm/uart) rather than following can.rs's/ iseled.rs's Err(RcpError::ConfigWriteNotImplemented) departure. That departure rests on CanDataFrame/IseledFrame making a real structural claim about their bytes that a genuine TC18 §12.7.1 config-write payload cannot honestly satisfy (decode can fail or silently misinterpret unrelated bytes). MdioTransfer::decode is infallible and interprets no structure at all -- every byte slice is a valid MdioTransfer, matching i2c::I2cByteTransfer's own raw pass-through discipline -- so a caller can honestly construct one from a genuine config-write payload with zero misrepresentation. The 'no caller can honestly construct one' pressure that drove CAN's/ISELED's departure does not apply to MDIO. Did not touch: MdioAddressingMode, MdioFunctionalConfig, or any existing MdioTransfer/MdioTransferResult encoding -- this module's own pre-existing 'Divergence note' documents a previously-invented, non-spec-derived Clause 22/45 addressing scheme (the same issue class as ISELED's #71 and cpp-RCP's MDIO #72). This PR is scoped strictly to evt[2:0] classification and does not reinterpret that encoding. Citation drift found (not fixed here, flagged as follow-up): mdio.rs's pre-existing citations for MDIO's own request-format/mdio_mode table cite 'Table 57'/TC18.txt L5676 and 'Figure 42'/TC18.txt L5664; against current TC18.txt these are actually Table 60/L6088 and Figure 43/L6077. Its functional-config citation ('Table 56'/L5639) is actually Table 59/L6061. Same ~400-line/3-number drift class iseled.rs's own v5.10.0 item found. Not corrected here per task scope -- flagged in mdio.rs's own new 'Editorial note: pre-existing §13.7.13 citation drift' section and in CHANGELOG.md. New citations added by this item were independently re-verified against TC18.txt. Also: .fusa-reqs.json REQ-MDIO-007/008, ARCHITECTURE.md Table 30 centralization row update, docs/PUBLIC_API.txt regenerated via cargo +nightly public-api, tests mirroring CAN/ISELED's shape (Reserved 001b-110b rejection, out-of-3-bit-field, never-panics). Co-Authored-By: Claude Sonnet 5 Signed-off-by: SoundMatt <47545907+SoundMatt@users.noreply.github.com> --- .fusa-reqs.json | 20 +++ ARCHITECTURE.md | 2 +- CHANGELOG.md | 85 ++++++++++++ Cargo.lock | 2 +- Cargo.toml | 2 +- docs/PUBLIC_API.txt | 20 +++ src/mdio.rs | 331 +++++++++++++++++++++++++++++++++++++++++++- 7 files changed, 457 insertions(+), 5 deletions(-) diff --git a/.fusa-reqs.json b/.fusa-reqs.json index 15fc8ff..4990add 100644 --- a/.fusa-reqs.json +++ b/.fusa-reqs.json @@ -5055,6 +5055,26 @@ "asil": "ASIL-B", "verificationMethod": "test" }, + { + "id": "REQ-MDIO-007", + "title": "MdioRequest enumerates the decoded shape of a Row-2 MDIO request after evt[2:0] validation, wrapping an already-decoded MdioTransfer", + "text": "MdioRequest's two variants (Plain(MdioTransfer), ConfigWrite) give an incoming MDIO request's decoded shape after its evt.sub_opcode has been validated against evt_row2_kind_of's Table 33 Row-2 rule; MdioRequest::from_evt_sub_opcode is the request-decode entry point that performs that validation and dispatch. Unlike I2cRequest::from_evt_sub_opcode/LinRequest::from_evt_sub_opcode/AdcRequest::from_evt_sub_opcode/PwmInRequest::from_evt_sub_opcode/UartRequest::from_evt_sub_opcode (each of which take raw byte_msg_payload bytes and decode them internally), MdioRequest::from_evt_sub_opcode takes an already-decoded MdioTransfer, matching CanRequest::from_evt_sub_opcode's/IseledRequest::from_evt_sub_opcode's own shape rather than those five siblings' -- MdioTransfer already has its own dedicated decode entry point (MdioTransfer::decode, pre-existing this item and unchanged by it), so this evt[2:0]-classification entry point does not re-derive that byte-layout logic a second time internally", + "standard": "iso26262", + "level": "HLR", + "asil": "ASIL-B", + "verificationMethod": "test", + "tc18": "§13.5 Table 33's Row-2 row (MDIO is one of its eight endpoint types), TC18.txt L4085-4092, and §13.7.13.3 MDIO request handling, TC18.txt L6065-6066" + }, + { + "id": "REQ-MDIO-008", + "title": "MdioRequest::from_evt_sub_opcode accepts Plain and ConfigWrite, rejects Reserved sub_opcodes with UnsupportedCmd, and keeps the Ok(Self::ConfigWrite) precedent rather than CanRequest/IseledRequest's ConfigWriteNotImplemented departure, never panicking", + "text": "MdioRequest::from_evt_sub_opcode wraps the caller-supplied MdioTransfer unchanged in MdioRequest::Plain for a Plain (sub_opcode 0) request, and returns Ok(MdioRequest::ConfigWrite) without interpreting the supplied MdioTransfer for a ConfigWrite (sub_opcode 7) request -- deliberately keeping the Ok(Self::ConfigWrite) precedent I2cRequest/LinRequest/AdcRequest/PwmInRequest/UartRequest each follow rather than following CanRequest::from_evt_sub_opcode's/IseledRequest::from_evt_sub_opcode's own Err(RcpError::ConfigWriteNotImplemented) departure, because MdioTransfer::decode is infallible and interprets no structure at all (every byte slice is a valid MdioTransfer, matching I2cByteTransfer's own raw pass-through discipline), so unlike CanDataFrame/IseledFrame a caller can honestly construct an MdioTransfer from a genuine TC18 §12.7.1 config-write payload with zero misrepresentation -- the 'no caller can honestly construct one' pressure that drove CAN's/ISELED's own departure does not apply to MDIO. Every Reserved sub_opcode value (evt[2:0] in 001b..=110b, or any value outside the 3-bit field's representable range) returns Err(RcpError::UnsupportedCmd), matching TC18 Table 33's Row-2 error-code requirement and every prior Row-2 endpoint-type module's identical refusal of their own table's reserved code. Never panics for any sub_opcode/transfer combination", + "standard": "iso26262", + "level": "HLR", + "asil": "ASIL-B", + "verificationMethod": "test", + "tc18": "§13.5 Table 33 (\"000b to 110b reserved – request to be rejected with error code = UNSUPPORTED_CMD\" / \"111b: ... used to change the configuration of the endpoint (see 12.7.1)\"), TC18.txt L4085-4092" + }, { "id": "REQ-WAKE-001", "title": "SleepCmdRequest's fixed 0xA5 discriminant round-trips through encode/decode", diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index b76a52e..b796d8c 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -21,7 +21,7 @@ shared with go-RCP, c-RCP, and cpp-RCP. | Canonical choice | Status | |---|---| | Response classification (evt-first) | **not conformant** — no classifier exists at all; tracked | -| Table 30 centralization | **partial** — `evtgroup.rs`'s `EvtRow2Kind`/`evt_row2_kind_of` implement Table 33's unambiguous Row-2 `evt[2:0]` rule (`{ADC, PWM_IN, I2C, LIN, CAN, UART, ISELED, MDIO}`); `i2c.rs` (v5.4.0 pilot), `adc.rs` (v5.5.0), `pwm.rs` (v5.6.0, PWM_IN only — PWM_OUT is not a Row-2 endpoint type), `lin.rs` (v5.7.0), `can.rs` (v5.8.0 — the first of these whose `evt[2:0] == 111b`/`ConfigWrite` arm returns a new dedicated error, `RcpError::ConfigWriteNotImplemented`, rather than `Ok(Self::ConfigWrite)`; see `can.rs`'s own doc comment "Provenance note: evt[2:0] request validation" for why), `uart.rs` (v5.9.0 — reverts to the `Ok(Self::ConfigWrite)` precedent I2C/LIN/ADC/PWM_IN share rather than following `can.rs`'s departure, since `UartRequest`'s own `ConfigWrite` arm constructs no UART-specific value and is under no equivalent pressure; `UartRequest`'s `evt[2:0] == 000b`/`Plain` case additionally splits into `Write`/`Read` variants reflecting UART's own independent TX/RX EP-request-storage split, TC18 §13.7.8.1, confirmed orthogonal to `evt[2:0]` classification — see `uart.rs`'s own doc comment "Provenance note: evt[2:0] request validation" for why), and `iseled.rs` (v5.10.0 — follows `can.rs`'s `Err(RcpError::ConfigWriteNotImplemented)` departure rather than the majority `Ok(Self::ConfigWrite)` precedent, since `IseledRequest::from_evt_sub_opcode` takes an already-decoded `IseledFrame` — matching `can.rs`'s `CanDataFrame`-accepting shape, not `i2c.rs`/`lin.rs`/`adc.rs`/`pwm.rs`/`uart.rs`'s own raw-bytes shape — so the same "no caller can honestly construct a config-write payload as one" pressure `can.rs`'s own doc comment names applies here too; see `iseled.rs`'s own doc comment "Provenance note: evt[2:0] request validation" for the full reasoning) are its callers so far; the remaining one Row-2 endpoint type (`MDIO`) doesn't call it yet, GPIO/SPI's own `sub_opcode` readers remain their own private, unclassified reading, and the broader roadmap-named "Groups A/B/C" classification (`EvtGroup`/`classify_evt_sub_opcode`) is still unresolved. This entry previously read "conformant", which was stale: `classify_evt_sub_opcode` was a stub that always returned `Ok(None)`, by its own doc comment's admission | +| Table 30 centralization | **partial** — `evtgroup.rs`'s `EvtRow2Kind`/`evt_row2_kind_of` implement Table 33's unambiguous Row-2 `evt[2:0]` rule (`{ADC, PWM_IN, I2C, LIN, CAN, UART, ISELED, MDIO}`); `i2c.rs` (v5.4.0 pilot), `adc.rs` (v5.5.0), `pwm.rs` (v5.6.0, PWM_IN only — PWM_OUT is not a Row-2 endpoint type), `lin.rs` (v5.7.0), `can.rs` (v5.8.0 — the first of these whose `evt[2:0] == 111b`/`ConfigWrite` arm returns a new dedicated error, `RcpError::ConfigWriteNotImplemented`, rather than `Ok(Self::ConfigWrite)`; see `can.rs`'s own doc comment "Provenance note: evt[2:0] request validation" for why), `uart.rs` (v5.9.0 — reverts to the `Ok(Self::ConfigWrite)` precedent I2C/LIN/ADC/PWM_IN share rather than following `can.rs`'s departure, since `UartRequest`'s own `ConfigWrite` arm constructs no UART-specific value and is under no equivalent pressure; `UartRequest`'s `evt[2:0] == 000b`/`Plain` case additionally splits into `Write`/`Read` variants reflecting UART's own independent TX/RX EP-request-storage split, TC18 §13.7.8.1, confirmed orthogonal to `evt[2:0]` classification — see `uart.rs`'s own doc comment "Provenance note: evt[2:0] request validation" for why), and `iseled.rs` (v5.10.0 — follows `can.rs`'s `Err(RcpError::ConfigWriteNotImplemented)` departure rather than the majority `Ok(Self::ConfigWrite)` precedent, since `IseledRequest::from_evt_sub_opcode` takes an already-decoded `IseledFrame` — matching `can.rs`'s `CanDataFrame`-accepting shape, not `i2c.rs`/`lin.rs`/`adc.rs`/`pwm.rs`/`uart.rs`'s own raw-bytes shape — so the same "no caller can honestly construct a config-write payload as one" pressure `can.rs`'s own doc comment names applies here too; see `iseled.rs`'s own doc comment "Provenance note: evt[2:0] request validation" for the full reasoning), and `mdio.rs` (v5.11.0 — the eighth and last Row-2 endpoint type; `MdioRequest::from_evt_sub_opcode` takes an already-decoded `MdioTransfer`, matching `can.rs`'s/`iseled.rs`'s own decoded-frame shape rather than `i2c.rs`/`lin.rs`/`adc.rs`/`pwm.rs`/`uart.rs`'s raw-bytes shape, but its `evt[2:0] == 111b`/`ConfigWrite` arm stays `Ok(Self::ConfigWrite)` — the majority precedent — rather than following `can.rs`'s/`iseled.rs`'s `Err(RcpError::ConfigWriteNotImplemented)` departure, since `MdioTransfer::decode` is infallible and interprets no structure at all, so the "no caller can honestly construct one" pressure that drove that departure does not apply to MDIO's own always-valid, opaque-bytes shape; see `mdio.rs`'s own doc comment "Provenance note: evt[2:0] request validation" for the full reasoning) are its callers. All eight Row-2 endpoint types (`{ADC, PWM_IN, I2C, LIN, CAN, UART, ISELED, MDIO}`) now call `evt_row2_kind_of`; GPIO/SPI's own `sub_opcode` readers remain their own private, unclassified reading, and the broader roadmap-named "Groups A/B/C" classification (`EvtGroup`/`classify_evt_sub_opcode`) is still unresolved. This entry previously read "conformant", which was stale: `classify_evt_sub_opcode` was a stub that always returned `Ok(None)`, by its own doc comment's admission | | Conditional-request module unification | **conformant** (this repo is a reference shape, alongside cpp-RCP) | | Per-function requirement tagging | **not conformant** — tags are collected at file level (top of each `.rs`), not per-function | | `.fusa-reqs.json` schema (`tc18`/`tc18_master_id`/`status`) | **partial** — has `verificationMethod` but no citation field (TC18 citations live only in code doc-comments); has a working `status: "not-implemented"` exemption mechanism (`scripts/fusa-gap-check.sh`) that go-RCP/cpp-RCP lack | diff --git a/CHANGELOG.md b/CHANGELOG.md index c221234..08e8c8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,91 @@ OPEN Alliance TC18 core replacement; from `v1.0.0` on, each entry is a real release. See `docs/SEMVER.md` for the versioning scheme, including why a wire-format change is a MAJOR bump even when it is a fix. +## v5.11.0 (Table 30/33 Row-2 evt[2:0] validation — MDIO, 8th and last endpoint type) — closed + +Direct follow-up to v5.10.0: `mdio.rs` becomes the eighth and last of the +TC18 §13.5 Table 33 Row-2 endpoint types (`{ADC, PWM_IN, I2C, LIN, CAN, +UART, ISELED, MDIO}`) to call the shared `evtgroup::evt_row2_kind_of` +predicate. `evtgroup.rs` itself is unchanged — this release only adds +`mdio.rs`'s own caller. This item touches only `evt[2:0]` request +classification; `MdioAddressingMode`'s own (already spec-divergent — see +`mdio.rs`'s pre-existing "Divergence note") `mdio_mode` mapping, +`MdioFunctionalConfig`, and `MdioTransfer`/`MdioTransferResult`'s own +field layout are all unchanged. + +New, purely additive `pub` items (MINOR bump per `docs/SEMVER.md`): + +- `mdio::MdioRequest` / `mdio::MdioRequest::from_evt_sub_opcode` — MDIO's + own request-decode entry point, structurally mirroring `can::CanRequest`/ + `iseled::IseledRequest` in one way and `i2c::I2cRequest`/ + `lin::LinRequest`/`adc::AdcRequest`/`pwm::PwmInRequest`/ + `uart::UartRequest` in another: + - Like `can.rs`/`iseled.rs`, `from_evt_sub_opcode` takes an + already-decoded `MdioTransfer`, not raw `byte_msg_payload` bytes. + `MdioTransfer` already has its own dedicated decode entry point + (`MdioTransfer::decode`, pre-existing this item and unchanged by it), + so this evt[2:0]-classification entry point does not re-derive that + (trivial) byte-layout logic a second time internally. + - Unlike `can.rs`/`iseled.rs`, `evt[2:0] == 111b` still returns + `Ok(MdioRequest::ConfigWrite)` — the majority precedent + `i2c::I2cRequest`/`lin::LinRequest`/`adc::AdcRequest`/ + `pwm::PwmInRequest`/`uart::UartRequest` each already follow — rather + than `Err(RcpError::ConfigWriteNotImplemented)`. This is a deliberate, + independently-reasoned judgment call, not a mechanical copy of either + prior precedent: `can.rs`'s/`iseled.rs`'s own departure rests on their + frame types (`CanDataFrame`, `IseledFrame`) making a real, specific + structural claim about their bytes (a parsed `FrameFormat`/`id`/`data` + or `chain_address`/`command`/`data` shape, either of which can fail to + decode or silently misinterpret unrelated bytes) that a genuine TC18 + §12.7.1 config-write payload cannot honestly satisfy. `MdioTransfer` + makes no such claim: `MdioTransfer::decode` is infallible and totally + uninterpreted — every byte slice, including an empty one, is a valid + `MdioTransfer`, matching `i2c::I2cByteTransfer`'s own raw pass-through + discipline. A caller can therefore decode a genuine config-write + payload through `MdioTransfer::decode` with zero information loss or + misrepresentation, so the "no caller can honestly construct one" + pressure that drove `can.rs`'s/`iseled.rs`'s own departure does not + apply here. + See `mdio.rs`'s own doc comment "Provenance note: evt[2:0] request + validation" for the full citation and reasoning behind both choices. + +Every `Reserved` sub_opcode value is rejected with +`Err(RcpError::UnsupportedCmd)`, unchanged from every prior Row-2 +endpoint-type module — this part of the rule is identical for MDIO. + +Also in this release: while independently re-verifying the TC18.txt +citations this item adds (Table 33 Row-2, §13.7.13.3, Figure 43), a +pre-existing citation drift was found in `mdio.rs`'s own "Divergence note" +and "Provenance note: register-access framing is carried opaque" sections +— the same class of drift `iseled.rs`'s own v5.10.0 item found and flagged +for ISELED. `mdio.rs` cites "Table 57"/"TC18.txt line 5676" and "Figure +42"/"TC18.txt line 5664" for MDIO's own request-format/`mdio_mode` field +table, and "Table 56"/"TC18.txt line 5639" for MDIO's functional-config +register layout; against the current `TC18.txt`, those are actually +**Table 60**/line 6088, **Figure 43**/line 6077, and **Table 59**/line +6061 respectively — both the line numbers and the table/figure numbers +have drifted, by roughly the same ~400-line/3-number offset already found +for ISELED. This item deliberately does **not** correct those pre-existing +citations (that is separate, later work, out of scope for this +evt[2:0]-classification item) — it only flags the drift, documented in +`mdio.rs`'s own new "Editorial note: pre-existing §13.7.13 citation drift" +section. The citations newly added by this item were independently +re-verified against `TC18.txt` rather than copied from `mdio.rs`'s own +pre-existing, now-known-stale citations. + +Not in this release: wiring `MdioRequest::from_evt_sub_opcode` into +`mock::RcServer`'s actual dispatch — `mock::Endpoint`'s trait signature +still does not carry an `evt` value to any implementation at all, the same +gap v5.4.0's pilot found and left as-is (confirmed unchanged here). This +release also does not touch `MdioAddressingMode`, `MdioFunctionalConfig`, +or any of `mdio.rs`'s own pre-existing "Divergence note"/"Provenance +note" content beyond the new "Editorial note" flagging citation drift +described above — all pre-existing, additive standalone plumbing left +untouched. This closes out the full Table 30/33 Row-2 `evt[2:0]` +validation rollout: all eight Row-2 endpoint types +(`{ADC, PWM_IN, I2C, LIN, CAN, UART, ISELED, MDIO}`) now call +`evtgroup::evt_row2_kind_of`. + ## v5.10.0 (Table 30/33 Row-2 evt[2:0] validation — ISELED, 7th endpoint type) — closed Direct follow-up to v5.9.0: `iseled.rs` becomes the seventh of the eight diff --git a/Cargo.lock b/Cargo.lock index f326362..70e982b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -333,7 +333,7 @@ checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" [[package]] name = "rcp" -version = "5.10.0" +version = "5.11.0" dependencies = [ "async-trait", "base64", diff --git a/Cargo.toml b/Cargo.toml index 2a698a5..c90d2a2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rcp" -version = "5.10.0" +version = "5.11.0" edition = "2021" rust-version = "1.75" license = "MPL-2.0" diff --git a/docs/PUBLIC_API.txt b/docs/PUBLIC_API.txt index c9bdf00..99a3a87 100644 --- a/docs/PUBLIC_API.txt +++ b/docs/PUBLIC_API.txt @@ -2163,6 +2163,26 @@ impl core::marker::Unpin for rcp::mdio::MdioAddressingMode impl core::marker::UnsafeUnpin for rcp::mdio::MdioAddressingMode impl core::panic::unwind_safe::RefUnwindSafe for rcp::mdio::MdioAddressingMode impl core::panic::unwind_safe::UnwindSafe for rcp::mdio::MdioAddressingMode +pub enum rcp::mdio::MdioRequest +pub rcp::mdio::MdioRequest::ConfigWrite +pub rcp::mdio::MdioRequest::Plain(rcp::mdio::MdioTransfer) +impl rcp::mdio::MdioRequest +pub fn rcp::mdio::MdioRequest::from_evt_sub_opcode(u8, rcp::mdio::MdioTransfer) -> core::result::Result +impl core::clone::Clone for rcp::mdio::MdioRequest +pub fn rcp::mdio::MdioRequest::clone(&self) -> rcp::mdio::MdioRequest +impl core::cmp::Eq for rcp::mdio::MdioRequest +impl core::cmp::PartialEq for rcp::mdio::MdioRequest +pub fn rcp::mdio::MdioRequest::eq(&self, &rcp::mdio::MdioRequest) -> bool +impl core::fmt::Debug for rcp::mdio::MdioRequest +pub fn rcp::mdio::MdioRequest::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::StructuralPartialEq for rcp::mdio::MdioRequest +impl core::marker::Freeze for rcp::mdio::MdioRequest +impl core::marker::Send for rcp::mdio::MdioRequest +impl core::marker::Sync for rcp::mdio::MdioRequest +impl core::marker::Unpin for rcp::mdio::MdioRequest +impl core::marker::UnsafeUnpin for rcp::mdio::MdioRequest +impl core::panic::unwind_safe::RefUnwindSafe for rcp::mdio::MdioRequest +impl core::panic::unwind_safe::UnwindSafe for rcp::mdio::MdioRequest pub struct rcp::mdio::MdioFunctionalConfig pub rcp::mdio::MdioFunctionalConfig::addressing_mode: rcp::mdio::MdioAddressingMode impl rcp::mdio::MdioFunctionalConfig diff --git a/src/mdio.rs b/src/mdio.rs index 493a65a..b9c7467 100644 --- a/src/mdio.rs +++ b/src/mdio.rs @@ -4,6 +4,8 @@ //fusa:req REQ-MDIO-004 //fusa:req REQ-MDIO-005 //fusa:req REQ-MDIO-006 +//fusa:req REQ-MDIO-007 +//fusa:req REQ-MDIO-008 //! The MDIO endpoint type (`ep_type 0x0D`) — `ROADMAP.md` Milestone 7 //! ("Remaining Endpoint Types"), fourth checklist bullet: IEEE 802.3 @@ -20,7 +22,10 @@ //! has no old-protocol satellite bridge module in this crate (no //! `mdiobr.rs`) to validate against or migrate away from, so every piece //! below is new modeling rather than a read-and-reject exercise against -//! prior code. Two named pieces are in scope, both implemented here: +//! prior code. Two named pieces were originally in scope, both implemented +//! here; a third, [`MdioRequest`]/[`MdioRequest::from_evt_sub_opcode`], was +//! added afterward (see "Provenance note: evt[2:0] request validation" +//! below): //! //! - [`MdioAddressingMode`] — the `mdio_mode` 2-bit Clause-22/Clause-45 //! selector, plus [`MdioFunctionalConfig`] carrying it as this endpoint @@ -31,6 +36,29 @@ //! - [`MdioTransfer`] / [`MdioTransferResult`] — the raw register-access //! byte stream an MDIO request sends and an MDIO response returns. See //! "Provenance note: register-access framing is carried opaque" below. +//! - [`MdioRequest`]/[`MdioRequest::from_evt_sub_opcode`] — MDIO's own +//! request-decode entry point, validating an incoming request's +//! `evt.sub_opcode` against [`crate::evtgroup::evt_row2_kind_of`]'s TC18 +//! §13.5 Table 33 Row-2 rule. See "Provenance note: evt[2:0] request +//! validation" below — this piece was added after this module's own +//! original two-piece scope note above (still accurate for why no +//! `sub_opcode` reading existed here originally) as this crate's eighth +//! and last Row-2 endpoint-type module, following +//! [`crate::i2c::I2cRequest`]/[`crate::lin::LinRequest`]/ +//! [`crate::adc::AdcRequest`]/[`crate::pwm::PwmInRequest`]/ +//! [`crate::uart::UartRequest`]'s own prior applications of the same +//! shared predicate and [`crate::can::CanRequest`]/ +//! [`crate::iseled::IseledRequest`]'s own deliberate departure from their +//! shared `Ok(Self::ConfigWrite)` precedent for `evt[2:0] == 111b`. This +//! module's own judgment call is explained in "Provenance note: evt[2:0] +//! request validation" below — MDIO's [`MdioRequest::from_evt_sub_opcode`] +//! takes an already-decoded [`MdioTransfer`], matching +//! [`crate::can::CanRequest`]'s/[`crate::iseled::IseledRequest`]'s own +//! signature shape, but keeps the majority `Ok(Self::ConfigWrite)` +//! outcome rather than following their `Err` departure, for reasons +//! specific to [`MdioTransfer`]'s own always-valid, uninterpreted-bytes +//! shape. This closes out all eight Row-2 endpoint types +//! (`{ADC, PWM_IN, I2C, LIN, CAN, UART, ISELED, MDIO}`). //! //! Deliberately out of scope, for the same reasons every prior Milestone //! 4/7 entry's own doc comment already gives: @@ -52,7 +80,30 @@ //! module carries [`MdioTransfer::bytes`]/[`MdioTransferResult::bytes`] //! as opaque bytes it does not interpret, matching //! [`crate::i2c::I2cByteTransfer`]'s own raw pass-through discipline. -//! - Wiring any of the below into an actual decoder, dispatch loop, or +//! - The "Groups A/B/C" `evt[2:0]` sub-opcode convention +//! ([`crate::evtgroup::EvtGroup`]) as a general, cross-endpoint-type +//! classification scheme — [`crate::evtgroup`]'s own doc comment already +//! flags that broader scheme as unresolved, independent of the narrower, +//! unambiguous Table 33 Row-2 rule this module's [`MdioRequest`] now +//! implements (see "Provenance note: evt[2:0] request validation" below). +//! - Decoding [`MdioRequest::ConfigWrite`]'s own TC18 §12.7.1 payload shape. +//! [`MdioRequest::from_evt_sub_opcode`] recognizes a config-write request +//! as distinct from a [`Plain`](MdioRequest::Plain) one, but does not +//! itself interpret what the config-write payload contains — that is +//! separate, later work, same as [`crate::i2c::I2cRequest`]/ +//! [`crate::lin::LinRequest`]/[`crate::adc::AdcRequest`]/ +//! [`crate::pwm::PwmInRequest`]/[`crate::uart::UartRequest`]'s own +//! identical `ConfigWrite` arms. +//! - Wiring [`MdioRequest::from_evt_sub_opcode`] into an actual decoder, +//! dispatch loop, or [`crate::mock::Endpoint`] implementation. +//! [`crate::mock::Endpoint`]'s own trait signature still does not carry an +//! `evt` value to any implementation at all — that gap is not specific to +//! MDIO, it applies identically to every other Row-2 endpoint-type +//! module's own `from_evt_sub_opcode` (each confirmed still unwired +//! against [`crate::mock::Endpoint`]'s own doc comment). [`MdioRequest`] +//! is built to that same "additive standalone plumbing only" level. +//! - Wiring any of this module's other, original two pieces into an actual +//! decoder, dispatch loop, or //! [`crate::avtp`]/[`crate::acf`]/[`crate::addressing`] caller — matching //! the discipline every prior Milestone 1-4/7 entry already established. //! @@ -160,6 +211,127 @@ //! device-type specifically can add that parsing later without this module //! having guessed at a framing this crate cannot yet confirm. //! +//! ## Provenance note: evt[2:0] request validation +//! +//! MDIO is the eighth and last of the endpoint types TC18 §13.5 Table 33 +//! groups into one shared "Row 2" `evt[2:0]` rule (TC18.txt lines +//! 4085-4092, `MDIO` itself named at line 4091) — see [`crate::evtgroup`]'s +//! own doc comment "Provenance note: TC18 §13.5 Table 33's Row-2 rule +//! (`evt_row2_kind_of`)" for the full citation, including the literal-text +//! discrepancy that module's doc comment flags and resolves (Table 33's own +//! printed Row-2 cell reads "000b to 110b reserved", including 000b, which +//! this crate does not implement literally). [`MdioRequest::from_evt_sub_opcode`] +//! is this module's own caller of that shared +//! [`crate::evtgroup::evt_row2_kind_of`] predicate — MDIO's own request +//! format (TC18 §13.7.13.3, Figure 43, TC18.txt line 6077) carries the same +//! `evt` field in its Message Info header every other endpoint type's +//! request does, and TC18 names no MDIO-specific override of Table 33's +//! generic rule anywhere in §13.7.13. (This citation is independently +//! verified against `TC18.txt` for this item — see this module's own doc +//! comment "Editorial note: pre-existing §13.7.13 citation drift" below for +//! why the *pre-existing* Table 57/Figure 42 citations elsewhere in this +//! module's "Divergence note" are a separate, not-yet-corrected matter.) +//! +//! **`MdioRequest::from_evt_sub_opcode` takes an already-decoded +//! [`MdioTransfer`], not raw `byte_msg_payload` bytes — matching +//! [`crate::can::CanRequest::from_evt_sub_opcode`]'s/ +//! [`crate::iseled::IseledRequest::from_evt_sub_opcode`]'s own shape, not +//! [`crate::i2c::I2cRequest::from_evt_sub_opcode`]'s/ +//! [`crate::lin::LinRequest::from_evt_sub_opcode`]'s/ +//! [`crate::adc::AdcRequest::from_evt_sub_opcode`]'s/ +//! [`crate::pwm::PwmInRequest::from_evt_sub_opcode`]'s/ +//! [`crate::uart::UartRequest::from_evt_sub_opcode`]'s own raw-bytes +//! shape.** [`MdioTransfer`] already has its own dedicated decode entry +//! point, [`MdioTransfer::decode`] — pre-existing this item and unchanged by +//! it. Rather than [`MdioRequest::from_evt_sub_opcode`] re-deriving that +//! (trivial) byte-layout logic a second time internally, this function +//! instead requires its caller to have already called [`MdioTransfer::decode`] +//! and supply the resulting [`MdioTransfer`] directly — mirroring +//! [`crate::can::CanRequest::from_evt_sub_opcode`]'s/ +//! [`crate::iseled::IseledRequest::from_evt_sub_opcode`]'s own identical +//! choice for [`crate::can::CanDataFrame`]/[`crate::iseled::IseledFrame`]. +//! +//! **Unlike [`crate::can::CanRequest::from_evt_sub_opcode`]/ +//! [`crate::iseled::IseledRequest::from_evt_sub_opcode`], +//! `MdioRequest::from_evt_sub_opcode` returns +//! `Ok(`[`MdioRequest::ConfigWrite`]`)` for `evt[2:0] == 111b`, following +//! [`crate::i2c::I2cRequest::from_evt_sub_opcode`]/ +//! [`crate::lin::LinRequest::from_evt_sub_opcode`]/ +//! [`crate::adc::AdcRequest::from_evt_sub_opcode`]/ +//! [`crate::pwm::PwmInRequest::from_evt_sub_opcode`]/ +//! [`crate::uart::UartRequest::from_evt_sub_opcode`]'s majority precedent +//! rather than [`crate::can::CanRequest`]'s/[`crate::iseled::IseledRequest`]'s +//! own `Err(`[`RcpError::ConfigWriteNotImplemented`]`)` departure — despite +//! sharing their already-decoded-frame signature shape.** This is a +//! deliberate, independent judgment call for MDIO, not a mechanical copy of +//! either prior precedent: `can.rs`'s and `iseled.rs`'s own departure rests +//! on their frame types making a real, specific structural claim about +//! their bytes that a genuine TC18 §12.7.1 config-write payload cannot +//! honestly satisfy — [`crate::can::CanDataFrame::decode`] parses a specific +//! `FrameFormat`/`id`/`data` shape (and can fail or, worse, silently +//! misinterpret bytes that are not really a CAN frame), and +//! [`crate::iseled::IseledFrame::decode`] parses a specific +//! `chain_address`/`command`/`data` shape and can itself fail with +//! [`RcpError::ShortFrame`]. Constructing either type from a real +//! config-write payload would either fail outright or silently mislabel +//! unrelated bytes as if they were a real data frame — the dishonesty their +//! own doc comments decline to paper over. +//! +//! [`MdioTransfer`] makes no such claim. Per this module's own doc comment +//! "Provenance note: register-access framing is carried opaque" above, +//! [`MdioTransfer::decode`] is infallible and totally uninterpreted — *every* +//! byte slice, including an empty one, is a valid [`MdioTransfer`], and the +//! type asserts nothing about what its `bytes` mean beyond "these are the +//! bytes of this request", exactly matching +//! [`crate::i2c::I2cByteTransfer`]'s own raw pass-through discipline (which +//! this module's own doc comment already cites `MdioTransfer` against). A +//! caller can therefore decode a genuine TC18 §12.7.1 config-write payload +//! through [`MdioTransfer::decode`] with zero information loss and zero +//! misrepresentation — unlike a [`CanDataFrame`](crate::can::CanDataFrame) +//! or an [`IseledFrame`](crate::iseled::IseledFrame), an [`MdioTransfer`] +//! never claims to be anything more than an opaque byte carrier, so wrapping +//! one in [`MdioRequest::ConfigWrite`] and discarding it (exactly as +//! [`crate::i2c::I2cRequest::from_evt_sub_opcode`] discards its own raw +//! `payload: &[u8]` for the identical case) is no less honest than any of +//! the five sibling modules that already return `Ok(Self::ConfigWrite)`. +//! The "no caller can honestly construct one" pressure that drove `can.rs`'s +//! and `iseled.rs`'s own departure from this precedent does not apply here. +//! +//! Every `Reserved` sub_opcode value (`evt[2:0]` in `001b..=110b`, or any +//! value outside the 3-bit field's representable range) is rejected with +//! `Err(`[`RcpError::UnsupportedCmd`]`)`, matching Table 33's own stated +//! error code and every prior Row-2 endpoint-type module's identical +//! refusal of their own table's reserved code — this part is unchanged +//! across all eight Row-2 endpoint-type modules, MDIO included. +//! +//! ## Editorial note: pre-existing §13.7.13 citation drift (out of scope for +//! this item) +//! +//! While adding the citations above, this module's own pre-existing +//! "Divergence note: `mdio_mode` does **not** select Clause 22 vs Clause 45" +//! section (above) was spot-checked against the current `TC18.txt` and found +//! to have drifted, the same way this crate's own `iseled.rs` found ISELED's +//! pre-existing citations had drifted (~400 lines) against a stale +//! reference-file version (see that module's own Table 30/33 Row-2 item). +//! Concretely: that section cites "Table 57" at "TC18.txt line 5676" and +//! "Figure 42" at "TC18.txt line 5664" for MDIO's own request format and +//! `mdio_mode` field table; against the current `TC18.txt`, MDIO's request +//! format is actually **Figure 43** (line 6077) and its `mdio_mode` field +//! table is actually **Table 60** ("Usage of ABB message for mdio +//! requests", line 6088) — both the line numbers *and* the table/figure +//! numbers have drifted, by roughly the same ~400-line/3-number offset +//! `iseled.rs` already found for its own section. The "Provenance note: +//! register-access framing is carried opaque" section's own "TC18 Table +//! 56" (line 5639) has the same issue: MDIO's functional-config register +//! layout is actually **Table 59** ("MDIO functional configuration", line +//! 6061). This item deliberately does **not** correct those pre-existing +//! citations — fixing them is a separate, later item, exactly as this +//! module's own instructions require; the citations newly added by this +//! item (Table 33 Row-2, TC18.txt L4085-4092; §13.7.13.3, TC18.txt L6065; +//! Figure 43, TC18.txt L6077) were independently re-verified against +//! `TC18.txt` rather than copied from this module's own pre-existing, +//! now-known-stale citations. +//! //! ## Relationship to [`crate::regmap`] //! //! As with every Milestone 4/7 endpoint-type module, MDIO's real @@ -177,6 +349,7 @@ //! like [`crate::lin::LinFunctionalConfig`], but also no multi-field shape //! like [`crate::iseled::IseledFunctionalConfig`]. +use crate::evtgroup::{evt_row2_kind_of, EvtRow2Kind}; use crate::RcpError; // ── MdioAddressingMode ─────────────────────────────────────────────────────── @@ -352,6 +525,69 @@ impl MdioTransferResult { } } +// ── MdioRequest: evt[2:0] request validation ───────────────────────────────── + +/// The decoded shape of an incoming MDIO request, after validating its +/// `evt[2:0]` sub-opcode against TC18 §13.5 Table 33's Row-2 rule (MDIO is +/// one of that row's eight endpoint types — +/// `{ADC, PWM_IN, I²C, LIN, CAN, UART, ISELED, MDIO}`). +/// +/// See this module's doc comment "Provenance note: evt[2:0] request +/// validation" for the full citation, why +/// [`MdioRequest::from_evt_sub_opcode`] takes an already-decoded +/// [`MdioTransfer`] rather than raw `byte_msg_payload` bytes (matching +/// [`crate::can::CanRequest`]'s/[`crate::iseled::IseledRequest`]'s own +/// shape, not +/// [`crate::i2c::I2cRequest`]'s/[`crate::lin::LinRequest`]'s/ +/// [`crate::adc::AdcRequest`]'s/[`crate::pwm::PwmInRequest`]'s/ +/// [`crate::uart::UartRequest`]'s raw-bytes shape), and why it nonetheless +/// keeps those five siblings' `Ok(Self::ConfigWrite)` outcome rather than +/// following [`crate::can::CanRequest`]'s/[`crate::iseled::IseledRequest`]'s +/// own `Err(`[`RcpError::ConfigWriteNotImplemented`]`)` departure, and +/// [`crate::evtgroup`]'s own doc comment for the literal-text discrepancy +/// this crate resolves `evt[2:0] == 000b` against. +#[derive(Debug, Clone, PartialEq, Eq)] +//fusa:req REQ-MDIO-007 +pub enum MdioRequest { + /// `evt[2:0] == 000b`: an ordinary MDIO register-access request — the + /// caller-decoded [`MdioTransfer`] this endpoint is to send onto, or has + /// received from, the MDIO bus. + Plain(MdioTransfer), + /// `evt[2:0] == 111b`: a functional-config write (TC18 §12.7.1) rather + /// than an ordinary transfer. This crate does not yet decode the + /// config-write payload shape itself — see this module's doc comment + /// "Deliberately out of scope" — so a caller receiving this variant + /// knows only that the request *is* a config-write, not its content. + ConfigWrite, +} + +impl MdioRequest { + /// Decode an incoming MDIO request from its `evt.sub_opcode` + /// ([`crate::acf::Evt::sub_opcode`]) and an already-decoded + /// [`MdioTransfer`] (see this module's doc comment "Provenance note: + /// evt[2:0] request validation" for why this takes a decoded + /// [`MdioTransfer`] rather than raw bytes, and why the `ConfigWrite` + /// arm still returns `Ok`, unlike + /// [`crate::can::CanRequest::from_evt_sub_opcode`]/ + /// [`crate::iseled::IseledRequest::from_evt_sub_opcode`]). + /// + /// Returns `Err(`[`RcpError::UnsupportedCmd`]`)` for every + /// [`EvtRow2Kind::Reserved`] sub_opcode value — TC18 §13.5 Table 33's + /// Row-2 rule requires the request be rejected with error code + /// `UNSUPPORTED_CMD`, matching every prior Row-2 endpoint-type module's + /// identical refusal of their own table's reserved code. Never panics + /// for any `sub_opcode`/`transfer` combination. + //fusa:req REQ-MDIO-007 + //fusa:req REQ-MDIO-008 + pub fn from_evt_sub_opcode(sub_opcode: u8, transfer: MdioTransfer) -> Result { + match evt_row2_kind_of(sub_opcode) { + EvtRow2Kind::Plain => Ok(Self::Plain(transfer)), + EvtRow2Kind::ConfigWrite => Ok(Self::ConfigWrite), + EvtRow2Kind::Reserved => Err(RcpError::UnsupportedCmd), + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -499,4 +735,95 @@ mod tests { let _ = MdioTransferResult::decode(&buf); } } + + // ── MdioRequest::from_evt_sub_opcode ───────────────────────────────────── + + fn sample_transfer() -> MdioTransfer { + MdioTransfer { + bytes: vec![0xDE, 0xAD, 0xBE, 0xEF], + } + } + + #[test] + //fusa:test REQ-MDIO-007 + //fusa:test REQ-MDIO-008 + fn mdio_request_plain_evt_wraps_the_given_transfer_unchanged() { + // Unlike I2cRequest::Plain/LinRequest::Plain/AdcRequest::Plain/ + // PwmInRequest::Plain/UartRequest::Write, MdioRequest::Plain does + // not decode raw bytes itself — it threads the caller's + // already-decoded MdioTransfer through unchanged, matching + // CanRequest::Plain/IseledRequest::Plain. See this module's doc + // comment "Provenance note: evt[2:0] request validation". + let transfer = sample_transfer(); + let request = MdioRequest::from_evt_sub_opcode(0b000, transfer.clone()).unwrap(); + assert_eq!(request, MdioRequest::Plain(transfer)); + } + + #[test] + //fusa:test REQ-MDIO-007 + //fusa:test REQ-MDIO-008 + fn mdio_request_plain_evt_accepts_an_empty_transfer() { + let transfer = MdioTransfer { bytes: vec![] }; + let request = MdioRequest::from_evt_sub_opcode(0b000, transfer.clone()).unwrap(); + assert_eq!(request, MdioRequest::Plain(transfer)); + } + + #[test] + //fusa:test REQ-MDIO-007 + //fusa:test REQ-MDIO-008 + fn mdio_request_config_write_evt_is_recognized_without_interpreting_transfer() { + // Deliberate choice to keep the Ok(Self::ConfigWrite) precedent + // I2cRequest/LinRequest/AdcRequest/PwmInRequest/UartRequest each + // follow, rather than CanRequest's/IseledRequest's own + // Err(RcpError::ConfigWriteNotImplemented) departure — see this + // module's doc comment "Provenance note: evt[2:0] request + // validation" for why MdioTransfer's always-valid, uninterpreted + // bytes shape does not carry the same "no caller can honestly + // construct one" pressure CanDataFrame/IseledFrame do. The given + // transfer is not a real config-write payload — it is passed only + // because the signature requires *some* MdioTransfer — and is not + // echoed back or otherwise used. + let request = + MdioRequest::from_evt_sub_opcode(0b111, sample_transfer()).unwrap(); + assert_eq!(request, MdioRequest::ConfigWrite); + } + + #[test] + //fusa:test REQ-MDIO-008 + fn mdio_request_reserved_evt_values_are_rejected_with_unsupported_cmd() { + for sub_opcode in 0b001..=0b110u8 { + assert_eq!( + MdioRequest::from_evt_sub_opcode(sub_opcode, sample_transfer()), + Err(RcpError::UnsupportedCmd) + ); + } + } + + #[test] + //fusa:test REQ-MDIO-008 + fn mdio_request_values_above_the_3_bit_field_are_also_rejected_with_unsupported_cmd() { + for sub_opcode in (crate::acf::EVT_SUB_OPCODE_MAX + 1)..=u8::MAX { + assert_eq!( + MdioRequest::from_evt_sub_opcode(sub_opcode, sample_transfer()), + Err(RcpError::UnsupportedCmd) + ); + } + } + + #[test] + //fusa:test REQ-MDIO-008 + fn mdio_request_from_evt_sub_opcode_never_panics_for_any_sampled_input() { + let transfers = [ + MdioTransfer { bytes: vec![] }, + sample_transfer(), + MdioTransfer { + bytes: vec![0xAAu8; 64], + }, + ]; + for sub_opcode in 0..=u8::MAX { + for transfer in &transfers { + let _ = MdioRequest::from_evt_sub_opcode(sub_opcode, transfer.clone()); + } + } + } } From 6a609923ecfe892747555f8fef71857d463e45c1 Mon Sep 17 00:00:00 2001 From: Matt <47545907+SoundMatt@users.noreply.github.com> Date: Thu, 20 Aug 2026 09:07:46 -0700 Subject: [PATCH 2/2] fix: cargo fmt line-wrap in MdioRequest ConfigWrite test Co-Authored-By: Claude Sonnet 5 Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com> --- src/mdio.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/mdio.rs b/src/mdio.rs index b9c7467..c96f4aa 100644 --- a/src/mdio.rs +++ b/src/mdio.rs @@ -783,8 +783,7 @@ mod tests { // transfer is not a real config-write payload — it is passed only // because the signature requires *some* MdioTransfer — and is not // echoed back or otherwise used. - let request = - MdioRequest::from_evt_sub_opcode(0b111, sample_transfer()).unwrap(); + let request = MdioRequest::from_evt_sub_opcode(0b111, sample_transfer()).unwrap(); assert_eq!(request, MdioRequest::ConfigWrite); }