The hand-written bitfield offsets in is_link_up(), is_autoneg_enabled() and is_autoneg_completed() are all one bit short of the actual layout of struct phy_device. Counting from the first bitfield in the unit (is_c45 at bit 0), the C definition places autoneg at bit 14, link at bit 15, and autoneg_complete at bit 16. The accessors instead read bits 13, 14, and 15, i.e.: is_link_up() reads bit 14 = autoneg is_autoneg_enabled() reads bit 13 = is_genphy_driven is_autoneg_completed() reads bit 15 = link The official ax88796b Rust driver uses all three accessors in its link-change handling, so it inherits the bug. On genphy-driven devices is_genphy_driven is always 1, which partially masks the broken is_autoneg_enabled() check and likely explains why this survived the original netdev review rounds of the PHY abstractions, which fixed reference/aliasing issues but never verified the bit numbers against the C layout. Fixes: f20fd5449ada ("rust: core abstractions for network PHY drivers") Link: https://www.spinics.net/lists/netdev/msg951815.html (v8 cover letter, where the bitfield access was flagged as needing a fix) Signed-off-by: Chunfeng Song --- rust/kernel/net/phy.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs index 956cda573ddb..99ed3dd168df 100644 --- a/rust/kernel/net/phy.rs +++ b/rust/kernel/net/phy.rs @@ -130,7 +130,7 @@ pub fn is_link_up(&self) -> bool { // SAFETY: The struct invariant ensures that we may access // this field without additional synchronization. let bit_field = unsafe { &(*self.0.get())._bitfield_1 }; - bit_field.get(14, 1) == LINK_IS_UP + bit_field.get(15, 1) == LINK_IS_UP } /// Gets the current auto-negotiation configuration. @@ -142,7 +142,7 @@ pub fn is_autoneg_enabled(&self) -> bool { // SAFETY: The struct invariant ensures that we may access // this field without additional synchronization. let bit_field = unsafe { &(*self.0.get())._bitfield_1 }; - bit_field.get(13, 1) == u64::from(bindings::AUTONEG_ENABLE) + bit_field.get(14, 1) == u64::from(bindings::AUTONEG_ENABLE) } /// Gets the current auto-negotiation state. @@ -155,7 +155,7 @@ pub fn is_autoneg_completed(&self) -> bool { // SAFETY: The struct invariant ensures that we may access // this field without additional synchronization. let bit_field = unsafe { &(*self.0.get())._bitfield_1 }; - bit_field.get(15, 1) == AUTONEG_COMPLETED + bit_field.get(16, 1) == AUTONEG_COMPLETED } /// Sets the speed of the PHY. -- 2.43.0