The hand-written bitfield offsets in is_link_up(), is_autoneg_enabled() and is_autoneg_completed() were correct when the abstraction was merged: at that time autoneg, link, and autoneg_complete were at bits 13, 14, and 15 of struct phy_device's first bitfield unit. Commit 2796ff1e3dca ("net: phy: add flag is_genphy_driven to struct phy_device") later inserted is_genphy_driven just before autoneg, shifting the three fields up by one, so the accessors now read: 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 masks the broken is_autoneg_enabled() check. Hard-coded offsets will silently break again on the next layout change, so use the bindgen-generated accessors (link(), autoneg(), autoneg_complete()), which are always consistent with the C layout, and drop the hand-written numbers together with the TODO comment that marked them as a stopgap. Found by a static equivalence audit (C2RustDrv, a C-to-Rust driver migration tool) that compares hand-written bitfield offsets against the bindgen layout of struct phy_device. Verified by building the bindings and checking the generated accessors; no runtime testing was possible without PHY hardware. Fixes: 2796ff1e3dca ("net: phy: add flag is_genphy_driven to struct phy_device") Cc: stable@vger.kernel.org Signed-off-by: Chunfeng Song --- v2 -> v3: - Drop the links to previous versions from the commit message (Miguel Ojeda). - Keep the changelog here, after the separator, as is customary. v1 -> v2: - Use the bindgen-generated accessors (link(), autoneg(), autoneg_complete()) instead of the hard-coded bit numbers, as suggested by Andrew Lunn, since the offsets drift whenever the layout changes. - Update the Fixes: tag to 2796ff1e3dca, the commit that inserted is_genphy_driven and shifted the fields (the offsets were correct when the abstraction was merged in v6.8). - Add the net tree prefix to the subject. rust/kernel/net/phy.rs | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs index 956cda573ddb..14dd1e6cb380 100644 --- a/rust/kernel/net/phy.rs +++ b/rust/kernel/net/phy.rs @@ -124,38 +124,30 @@ pub fn state(&self) -> DeviceState { /// /// It returns true if the link is up. pub fn is_link_up(&self) -> bool { - const LINK_IS_UP: u64 = 1; - // TODO: the code to access to the bit field will be replaced with automatically - // generated code by bindgen when it becomes possible. + let phydev = self.0.get(); // 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 + unsafe { (*phydev).link() == 1 } } /// Gets the current auto-negotiation configuration. /// /// It returns true if auto-negotiation is enabled. pub fn is_autoneg_enabled(&self) -> bool { - // TODO: the code to access to the bit field will be replaced with automatically - // generated code by bindgen when it becomes possible. + let phydev = self.0.get(); // 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) + unsafe { (*phydev).autoneg() == bindings::AUTONEG_ENABLE } } /// Gets the current auto-negotiation state. /// /// It returns true if auto-negotiation is completed. pub fn is_autoneg_completed(&self) -> bool { - const AUTONEG_COMPLETED: u64 = 1; - // TODO: the code to access to the bit field will be replaced with automatically - // generated code by bindgen when it becomes possible. + let phydev = self.0.get(); // 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 + unsafe { (*phydev).autoneg_complete() == 1 } } /// Sets the speed of the PHY. -- 2.43.0