[PATCH net-next 1/6] net: phy: dp83869: Disable autonegotiation in RGMII/1000Base-X mode

Romain Gantois posted 6 patches 1 year, 5 months ago
[PATCH net-next 1/6] net: phy: dp83869: Disable autonegotiation in RGMII/1000Base-X mode
Posted by Romain Gantois 1 year, 5 months ago
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(-)

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 */

-- 
2.45.2
Re: [PATCH net-next 1/6] net: phy: dp83869: Disable autonegotiation in RGMII/1000Base-X mode
Posted by Andrew Lunn 1 year, 5 months ago
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
Re: [PATCH net-next 1/6] net: phy: dp83869: Disable autonegotiation in RGMII/1000Base-X mode
Posted by Romain Gantois 1 year, 5 months ago
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,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH net-next 1/6] net: phy: dp83869: Disable autonegotiation in RGMII/1000Base-X mode
Posted by Russell King (Oracle) 1 year, 5 months ago
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.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next 1/6] net: phy: dp83869: Disable autonegotiation in RGMII/1000Base-X mode
Posted by Romain Gantois 1 year, 5 months ago
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,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH net-next 1/6] net: phy: dp83869: Disable autonegotiation in RGMII/1000Base-X mode
Posted by Russell King (Oracle) 1 year, 5 months ago
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.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next 1/6] net: phy: dp83869: Disable autonegotiation in RGMII/1000Base-X mode
Posted by Romain Gantois 1 year, 5 months ago
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,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com