| Message ID | 20240701-b4-dp83869-sfp-v1-1-a71d6d0ad5f8@bootlin.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy: dp83869: Add support for downstream SFP cages | expand |
On Mon, Jul 01, 2024 at 10:51:03AM +0200, Romain Gantois wrote: > Currently, the DP83869 driver only disables autonegotiation in fiber > configurations for 100Base-FX mode. However, the DP83869 PHY does not > support autonegotiation in any of its fiber modes. > > Disable autonegotiation for all fiber modes. I'm assuming to does work in copper mode? Andrew
On lundi 1 juillet 2024 18:40:24 UTC+2 Andrew Lunn wrote: > On Mon, Jul 01, 2024 at 10:51:03AM +0200, Romain Gantois wrote: > > Currently, the DP83869 driver only disables autonegotiation in fiber > > configurations for 100Base-FX mode. However, the DP83869 PHY does not > > support autonegotiation in any of its fiber modes. > > > > Disable autonegotiation for all fiber modes. > > I'm assuming to does work in copper mode? I'm unable to test any of the copper modes for the DP83869HM but according to the datasheet, autonegotiation should work in those. To be clear, "fiber mode" in the DP83869 linguo also includes 1000Base-X which can be used with a direct-attach copper cable. From what I've seen, autonegotiation is not supported in this configuration. Thanks,
On Tue, Jul 02, 2024 at 10:44:12AM +0200, Romain Gantois wrote: > To be clear, "fiber mode" in the DP83869 linguo also includes > 1000Base-X which can be used with a direct-attach copper cable. From > what I've seen, autonegotiation is not supported in this configuration. Why? Direct-attached cables are 1000base-CX, which is defined by 802.3. It uses the 1000base-X PCS which is shared with 1000base-SX, 1000base-LX etc. If one looks at 37.1.3, Relationship to architectural layering, or 36.1.5, Inter-sublayer interfaces, one can see in that diagram that the PCS *including* auto-negotiation is included for SX, LX *and* CX. Moreover, 39.3 states that TP1 and TP4 will be commin in many implementations of LX, SX and CX. Moreover, 39.1, 1000base-CX introduction, states that it incorporates clause 36 and clause 38. Clause 36.2.2 states that the 1000base-X PCS incorporates clause 37 auto-negotiation. So, I think AN is supposed to be supported on CX in the same way that it's supported for SX and LX.
Hello Russell, On mardi 2 juillet 2024 11:24:18 UTC+2 Russell King (Oracle) wrote: > On Tue, Jul 02, 2024 at 10:44:12AM +0200, Romain Gantois wrote: > > To be clear, "fiber mode" in the DP83869 linguo also includes > > 1000Base-X which can be used with a direct-attach copper cable. From > > what I've seen, autonegotiation is not supported in this configuration. > > Why? Direct-attached cables are 1000base-CX, which is defined by 802.3. > It uses the 1000base-X PCS which is shared with 1000base-SX, 1000base-LX > etc. If one looks at 37.1.3, Relationship to architectural layering, > or 36.1.5, Inter-sublayer interfaces, one can see in that diagram that > the PCS *including* auto-negotiation is included for SX, LX *and* CX. > > Moreover, 39.3 states that TP1 and TP4 will be commin in many > implementations of LX, SX and CX. > > Moreover, 39.1, 1000base-CX introduction, states that it incorporates > clause 36 and clause 38. Clause 36.2.2 states that the 1000base-X PCS > incorporates clause 37 auto-negotiation. > > So, I think AN is supposed to be supported on CX in the same way that > it's supported for SX and LX. This seems to be a limitation of this particular PHY. From the DP83869 datasheet: "7.4.2.1 1000BASE-X The DP83869HM supports the 1000Base-X Fiber Ethernet protocol as defined in IEEE 802.3 standard. In 1000M Fiber mode, the PHY will use two differential channels for communication. In fiber mode, the speed is not decided through auto-negotiation. Both sides of the link must be configured to the same operating speed. The PHY can be configured to operate in 1000BASE-X through the register settings (Section 7.4.8) or strap settings (Section 7.5.1.2)." Thanks,
On Tue, Jul 02, 2024 at 11:42:04AM +0200, Romain Gantois wrote: > Hello Russell, > > This seems to be a limitation of this particular PHY. From the DP83869 > datasheet: > > "7.4.2.1 1000BASE-X > The DP83869HM supports the 1000Base-X Fiber Ethernet protocol as > defined in IEEE 802.3 standard. In 1000M Fiber mode, the PHY will use > two differential channels for communication. In fiber mode, the speed is not > decided through auto-negotiation. Both sides of the link must be > configured to the same operating speed. The PHY can be configured to > operate in 1000BASE-X through the register settings (Section 7.4.8) or > strap settings (Section 7.5.1.2)." I think you grossly misunderstand 1000base-X there. You seem to be equating auto-negotiation with negotiation of speed. That isn't necessarily the case. Clause 37 auto-negotiation doesn't negotiate speed. It negotiates other aspects of the link. See 37.2.1: LSB MSB D0 D1 D2 D3 D4 D5 D6 D7 D8 D9 D10 D11 D12 D13 D14 D15 rsvd rsvd rsvd rsvd rsvd FD HD PS1 PS2 rsvd rsvd rsvd RF1 RF2 Ack NP FD/HD - full duplex/half duplex capability PS1/PS2 - pause capabilties RF1/RF2 - remote fault bits Ack - Ack bit NP - Next Page bit So, just because the PHY documentation states that speed is not negotiated, that doesn't mean that negotiation is not supported. IEEE 802.3 *requires* AN be implemented. Moreover, the clue is in the name - 1000base-X. The 1000 part. That means it's a protocol operating at 1G speed, just the same as 1000base-T which only operates at 1G speed. BTW, with twisted pair, negotiation does include speed, and the result of that is used to select between 1000base-T for 1G speeds, 100base-Tx for 100M, and 10base-T for 10M - these each are separate protocols. There is no 1000base-T operating at 100M or 10M speeds - that just doesn't exist. Hope this clears up the issue.
On mardi 2 juillet 2024 12:15:24 UTC+2 Russell King (Oracle) wrote: > On Tue, Jul 02, 2024 at 11:42:04AM +0200, Romain Gantois wrote: ... > So, just because the PHY documentation states that speed is not > negotiated, that doesn't mean that negotiation is not supported. > IEEE 802.3 *requires* AN be implemented. Got it. Thanks for clarifying, I'll remove this patch from v2 and see if I can get this autoneg to work. Best Regards,
diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c index d7aaefb5226b6..f6b05e3a3173e 100644 --- a/drivers/net/phy/dp83869.c +++ b/drivers/net/phy/dp83869.c @@ -647,6 +647,21 @@ static int dp83869_configure_fiber(struct phy_device *phydev, linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, phydev->supported); linkmode_set_bit(ADVERTISED_FIBRE, phydev->advertising); + /* Auto neg is not supported in 100/1000base FX modes */ + bmcr = phy_read(phydev, MII_BMCR); + if (bmcr < 0) + return bmcr; + + phydev->autoneg = AUTONEG_DISABLE; + linkmode_clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->supported); + linkmode_clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->advertising); + + if (bmcr & BMCR_ANENABLE) { + ret = phy_modify(phydev, MII_BMCR, BMCR_ANENABLE, 0); + if (ret < 0) + return ret; + } + if (dp83869->mode == DP83869_RGMII_1000_BASE) { linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, phydev->supported); @@ -655,21 +670,6 @@ static int dp83869_configure_fiber(struct phy_device *phydev, phydev->supported); linkmode_set_bit(ETHTOOL_LINK_MODE_100baseFX_Half_BIT, phydev->supported); - - /* Auto neg is not supported in 100base FX mode */ - bmcr = phy_read(phydev, MII_BMCR); - if (bmcr < 0) - return bmcr; - - phydev->autoneg = AUTONEG_DISABLE; - linkmode_clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->supported); - linkmode_clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->advertising); - - if (bmcr & BMCR_ANENABLE) { - ret = phy_modify(phydev, MII_BMCR, BMCR_ANENABLE, 0); - if (ret < 0) - return ret; - } } /* Update advertising from supported */
Currently, the DP83869 driver only disables autonegotiation in fiber configurations for 100Base-FX mode. However, the DP83869 PHY does not support autonegotiation in any of its fiber modes. Disable autonegotiation for all fiber modes. Signed-off-by: Romain Gantois <romain.gantois@bootlin.com> --- drivers/net/phy/dp83869.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)