Driver support for bcm84891 SFP+#192
Open
meehien wants to merge 5 commits into
Open
Conversation
after insertion, returning 0xffff for all reads. get_phy_c45_ids()
detects this (devices-in-package reads as all-ones) and returns
-ENODEV, which the SFP state machine handles with its 25 x 1s retry
loop (R_PHY_RETRY / phy_t_retry set by sfp_fixup_rollball()).
However, some of these modules (observed with an "OEM SFP-10G-T"
containing a Broadcom BCM84891) exit the gated window slightly before
the PHY's identifier registers are stable: devices-in-package reads a
plausible value while MII_PHYSID1/2 in all MMDs still read zero.
get_phy_c45_ids() has no validity check for this case, so a phydev is
created with all-zero c45 device_ids. No PHY driver can match a zero
ID (phy_bus_match() only skips 0xffffffff), so phy->drv is NULL,
phylink_sfp_connect_phy() rejects the PHY, and sfp_add_phy() fails
with -EINVAL:
mtk_soc_eth 15100000.ethernet lsfp: PHY i2c:sfp2:11 (id 0x00000000)
has no driver loaded
sfp sfp2: sfp_add_phy failed: -EINVAL
Unlike -ENODEV, this error path sends the state machine to SFP_S_FAIL,
which is terminal until the module is physically re-seated.
Detect the all-zero identifier case in sfp_sm_probe_phy() and return
-ENODEV so the existing retry logic applies. Also log the c45
identifiers when sfp_add_phy() fails, so a genuinely unsupported PHY
model is diagnosable from dmesg.
meehien
changed the title
zero") and its 0xffff-half tightening accept a phydev as soon as one MMD carries a plausible identifier. That is not sufficient: a RollBall module can leave the gated window with a single MMD answering while the rest of the mailbox still returns 0xffff, including the devices-in-package map that selects which MMDs are enumerated in the first place. Observed on an OEM SFP-10G-T (Broadcom BCM84891): PMA answered cleanly (0x35905081) while PCS (0xffffffff), AN (0x0362ffff) and devices-in- package (0xffff009b) were still gated. The id-only check passed on the clean PMA identifier, the BCM driver matched on it, and the probe went on to attach. phy_init_hw()'s reset poll during attach then read the still -gated mailbox, saw the reset bit permanently set, and returned -ETIMEDOUT from sfp_add_phy(). Worse, a concurrent interface-down racing that failed attach could fault the PHY state machine. Require, in addition to at least one plausible identifier, that neither 16-bit half of devices_in_package reads 0xffff (all MMDs 0-15 or 16-31 "present" is not a real device map, only a timed-out read). If either is corrupt, treat the PHY as not ready and return -ENODEV so the existing R_PHY_RETRY/phy_t_retry logic re-probes once the module is fully awake. Depends on: "net: phy: sfp: retry PHY probe when c45 identifiers read as zero"
-ENODEV, reflecting the board-soldered configuration it was written for. The BCM84891 is however also the PHY inside various RollBall "OEM SFP-10G-T" copper SFP+ modules, where phylink selects 10GBASE-R as the host interface (with SGMII/2500BASE-X rate switching), exactly as for the BCM84881. In that configuration a successful PHY probe still fails at attach time: sfp sfp2: PHY c45 ids: PMA 0x35905081 PCS ... devs_in_pkg ... sfp sfp2: sfp_add_phy failed: -ENODEV with the -ENODEV originating from phy_init_hw() -> bcm8489x_config_init(). Accept the same interface trio as the BCM84881 in addition to USXGMII, and populate possible_interfaces with all four so phylink can rate-switch on copper linkdowns. The LPOWER clearing is kept unconditional; it is harmless on module-hosted PHYs.
LINK_INBAND_DISABLE, but does not implement .config_inband. When a BCM84881-family PHY is used with a phylink MAC in inband mode (e.g. any SFP module PHY, or managed = "in-band-status" in DT), phylink's negotiation resolves to outband operation and sets phy_ib_mode = LINK_INBAND_DISABLE, then calls phy_config_inband() to apply it. With no .config_inband method this returns -EOPNOTSUPP, which phylink_major_config() treats as a fatal configuration error: mtk_soc_eth 15100000.ethernet lsfp: phy_config_inband: -EOPNOTSUPP major_config_failed then forces the link down in phylink_resolve() and holds it down across reconfiguration attempts. The result is a permanently dead link with a perfectly negotiated copper side (observed with a BCM84891-based "OEM SFP-10G-T" module on mt7988: copper AN completes at 10G, carrier never comes up). Implement config_inband: the PHY does not generate inband signalling in any mode, so it is permanently in the "disabled" state and a request to disable inband succeeds as a no-op. Any other request cannot be satisfied and returns -EINVAL (phylink will not request one, given the capabilities this driver reports).
…hile
its PMA/PMD control register is still deaf. The identifier checks added
earlier ("retry PHY probe when c45 identifiers read as zero" and its
0xffff-half / devices-in-package tightening) therefore still let such a
probe through to sfp_add_phy().
phy_attach_direct() -> phy_init_hw() then runs a soft reset: it writes
MDIO_CTRL1 and polls the reset bit to self-clear. On a still-gated read
the register returns 0xffff, the reset bit never appears to clear, and
sfp_add_phy() fails with -ETIMEDOUT. Observed on an OEM SFP-10G-T
(BCM84891) that answered its identifiers (PMA 0x35905081, clean
devices-in-package 0xc000009b) yet still timed out in attach. The failed
attach has also been observed to fault the PHY state machine when the
interface is subsequently brought down.
Read MDIO_CTRL1 in PMAPMD before attaching and treat 0xffff, a read
error, or a set reset bit as "not ready", returning -ENODEV so the
existing R_PHY_RETRY/phy_t_retry loop re-probes. This gates on the exact
register the attach-time reset polls, so the PHY is attached once, after
the module is genuinely awake, instead of faulting a half-awake reset.
This narrows the -ETIMEDOUT window rather than closing it (the mailbox
can still stall between this read and the reset a few ms later); the
underlying attach-failure teardown path remains worth hardening
separately.
Depends on: "net: phy: sfp: retry PHY probe when c45 identifiers read
as zero"
Author
|
Removed some of the timing patches and added a new one to replace those. The module is still slow to start and needs a booster to reduce the downtime during boot-up. The patches removed were causing the kernel to report an unreasonable high load. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please check this series because it was developed with Anthropic Fable 5. I had issues getting the module to work until now because the mainstream driver implementation is not complete. The timings for this SFP+ module are also quite bad.
This series of patches registers the module correctly and there are no crashes. As far as I can tell it works perfectly, but I'm no expert in the SFP+ state machine so someone should check properly.