MT-150266: net: fec: add direct MDIO bus access via SIOCGMIIREG/SIOCSMIIREG#43
MT-150266: net: fec: add direct MDIO bus access via SIOCGMIIREG/SIOCSMIIREG#43Overdr0ne wants to merge 2 commits into
Conversation
Add a fec_enet_ioctl() wrapper that services the SIOCGMIIREG and
SIOCSMIIREG ioctls directly against fep->mii_bus, delegating all other
commands to the standard phy_do_ioctl_running() handler.
The generic PHY ioctl path has two limitations for board bring-up and
diagnostics:
- It requires an attached phydev. phy_do_ioctl() returns -ENODEV when
ndev->phydev is NULL, which is the case when the FEC is wired to a
DSA switch (SJA1105 on this platform) over a fixed-link rather than
to a conventional PHY. In that configuration the MDIO bus cannot be
reached from userspace at all.
- For an attached PHY it routes register writes through the PHY state
machine, applying side effects (autoneg disable/restart, phy_init_hw)
for BMCR/ADVERTISE/CTRL1000 writes. That is undesirable for raw
register inspection and poking.
Servicing the two MII register commands directly against the bus gives
side-effect-free access to any device address on the FEC MDIO bus, which
userspace tooling relies on.
This reworks a patch distributed with the Audinate i.MX8MM EVK to fit the
6.6 driver, which no longer has a custom fec_enet_ioctl() (it wired
phy_do_ioctl_running() directly). The original logged failures with
KERN_ALERT and ignored read errors; this version returns the bus error to
the caller and propagates read failures instead of reporting a bogus zero.
This is a diagnostic capability rather than a correctness fix. If it
proves unused (verifiable at runtime by stracing for SIOC[GS]MIIREG) or
problematic, it can be reverted without affecting normal operation.
Overdr0ne
changed the title
There was a problem hiding this comment.
Pull request overview
This PR adds a custom ndo_eth_ioctl handler to the FEC (Freescale Ethernet Controller) driver to provide direct, side-effect-free MDIO register access from userspace via SIOCGMIIREG/SIOCSMIIREG, even when no phydev is attached (e.g., fixed-link to a DSA switch).
Changes:
- Introduces
fec_enet_ioctl()to interceptSIOCGMIIREGandSIOCSMIIREGand execute reads/writes directly againstfep->mii_bus. - Keeps all other ioctl commands delegated to the existing
phy_do_ioctl_running()path. - Hooks the new handler into
fec_netdev_opsvia.ndo_eth_ioctl = fec_enet_ioctl.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The direct SIOCGMIIREG/SIOCSMIIREG path only used the Clause-22 bus accessors and passed mii->phy_id through unmodified. A Clause-45-encoded phy_id (MII_ADDR_C45) would therefore be treated as a C22 address and hit the wrong device/register, shadowing the C45 support that phy_mii_ioctl() provides via FEC_QUIRK_HAS_MDIO_C45. Mirror phy_mii_ioctl(): when mdio_phy_id_is_c45() is set, split the phy_id into prtad/devad and route through mdiobus_c45_read_nested/ mdiobus_c45_write_nested. The C22 path is unchanged.
|
@Overdr0ne fyi I have tested with this added - does not solve APEC or PTP issues as noted to Audinate, not surprising. So will validate with Audinate if this is really needed. |
|
@AmeNote-Michael any word from Audinate? i'll leave it to you if you want to close this or merge it. it gives us parity with their sdk, so we can't say some other bug is there because we don't have some patch from them. OTOH, I prefer not to add some non-standard debug interface that only Audinate uses if we don't really need it... |
|
@Overdr0ne no word from Audinate on this. They asked for more information on a seperate issue, perhaps they are waiting for that info - the information they asked for is more related to another issue than the one they asked in, if that makes sense. Perhaps all rolled together. I plan to respond to their requests later tonight - unfortunately requires an extensive kernel rebuild and I just have not been in a position to do that yet. |
MT-150266
This is the last remaining EVK patch from Audinate that applies to our kernel. Just like the other patches, the patch had no commit message, so I have had to infer what it was for exactly. As far as I can tell, the commit provides low level access to the fec phy for diagnostic purposes. However, I would not put it past Audinate to use an interface designed only for diagnostic purposes for production use. For this reason I would suggest trying it, perhaps for some of the link-local issues we were seeing where PTP/aipec were using the wrong interface ip range for multicast @AmeNote-Michael The commit message below was generated by Claude. I asked it to be particularly verbose for this one.
Add a fec_enet_ioctl() wrapper that services the SIOCGMIIREG and SIOCSMIIREG ioctls directly against fep->mii_bus, delegating all other commands to the standard phy_do_ioctl_running() handler.
The generic PHY ioctl path has two limitations for board bring-up and diagnostics:
It requires an attached phydev. phy_do_ioctl() returns -ENODEV when ndev->phydev is NULL, which is the case when the FEC is wired to a DSA switch (SJA1105 on this platform) over a fixed-link rather than to a conventional PHY. In that configuration the MDIO bus cannot be reached from userspace at all.
For an attached PHY it routes register writes through the PHY state machine, applying side effects (autoneg disable/restart, phy_init_hw) for BMCR/ADVERTISE/CTRL1000 writes. That is undesirable for raw register inspection and poking.
Servicing the two MII register commands directly against the bus gives side-effect-free access to any device address on the FEC MDIO bus, which userspace tooling relies on.
This reworks a patch distributed with the Audinate i.MX8MM EVK to fit the 6.6 driver, which no longer has a custom fec_enet_ioctl() (it wired phy_do_ioctl_running() directly). The original logged failures with KERN_ALERT and ignored read errors; this version returns the bus error to the caller and propagates read failures instead of reporting a bogus zero.
This is a diagnostic capability rather than a correctness fix. If it proves unused (verifiable at runtime by stracing for SIOC[GS]MIIREG) or problematic, it can be reverted without affecting normal operation.
dev QA: @AmeNote-Michael