[PATCH net-next] net: dsa: mv88e6xxx: support in-band signalling with external PHY for 1000BaseX/2500BaseX

[PATCH net-next] net: dsa: mv88e6xxx: support in-band signalling with external PHY for 1000BaseX/2500BaseX

[Heiner Kallweit]

Propagate the external PHY settings also in 1000BaseX and
2500BaseX mode.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/dsa/mv88e6xxx/serdes.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 6a5de1b72..238f79872 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -509,6 +509,7 @@ int mv88e6390x_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on)
 static void mv88e6390_serdes_irq_link_sgmii(struct mv88e6xxx_chip *chip,
 					    int port, int lane)
 {
+	u8 cmode = chip->ports[port].cmode;
 	struct dsa_switch *ds = chip->ds;
 	int duplex = DUPLEX_UNKNOWN;
 	int speed = SPEED_UNKNOWN;
@@ -525,7 +526,13 @@ static void mv88e6390_serdes_irq_link_sgmii(struct mv88e6xxx_chip *chip,
 	link = status & MV88E6390_SGMII_PHY_STATUS_LINK ?
 	       LINK_FORCED_UP : LINK_FORCED_DOWN;
 
-	if (status & MV88E6390_SGMII_PHY_STATUS_SPD_DPL_VALID) {
+	if (cmode == MV88E6XXX_PORT_STS_CMODE_1000BASE_X) {
+		speed = SPEED_1000;
+		duplex = DUPLEX_FULL;
+	} else if (cmode == MV88E6XXX_PORT_STS_CMODE_2500BASEX) {
+		speed = SPEED_2500;
+		duplex = DUPLEX_FULL;
+	} else if (status & MV88E6390_SGMII_PHY_STATUS_SPD_DPL_VALID) {
 		duplex = status & MV88E6390_SGMII_PHY_STATUS_DUPLEX_FULL ?
 			 DUPLEX_FULL : DUPLEX_HALF;
 
-- 
2.21.0

Re: [PATCH net-next] net: dsa: mv88e6xxx: support in-band signalling with external PHY for 1000BaseX/2500BaseX

[Andrew Lunn]

On Sat, Mar 02, 2019 at 04:57:32PM +0100, Heiner Kallweit wrote:
> Propagate the external PHY settings also in 1000BaseX and
> 2500BaseX mode.

Hi Heiner

I don't think this is needed. 1000BaseX and 2500BaseX does not support
inband signalling. So the mac_config() call for these modes should
include the needed speed configuration. I've used 1000BaseX with an
SFP and there was no need to configure anything.

    Andrew

Re: [PATCH net-next] net: dsa: mv88e6xxx: support in-band signalling with external PHY for 1000BaseX/2500BaseX

[Heiner Kallweit]

On 02.03.2019 21:13, Andrew Lunn wrote:
> On Sat, Mar 02, 2019 at 04:57:32PM +0100, Heiner Kallweit wrote:
>> Propagate the external PHY settings also in 1000BaseX and
>> 2500BaseX mode.
> 
> Hi Heiner
> 
Hi Andrew

> I don't think this is needed. 1000BaseX and 2500BaseX does not support
> inband signalling. So the mac_config() call for these modes should
> include the needed speed configuration. I've used 1000BaseX with an
> SFP and there was no need to configure anything.
> 
1000BaseX and 2500BaseX use 802.3z Clause 37 inband auto-neg. From what
I've read 1000BaseT is an extension of it.
My understanding is that w/o such inband signalling we had no chance
to detect a "link up" situation. All we could detect is: serial connection
is synced. Also my understanding is that the ACK to this inband signalling
triggers the "link up" interrupt.

Here I found something:
http://www.methode.com/Documents/TechnicalLibrary/SFP_Ethernet_Auto_Negotiation.pdf

I briefly looked at the SFP connector and there don't seem to be pins for
out-of-band signalling. So there must be some inband signalling.

>     Andrew
> 
Heiner

Re: [PATCH net-next] net: dsa: mv88e6xxx: support in-band signalling with external PHY for 1000BaseX/2500BaseX

[Heiner Kallweit]

On 02.03.2019 21:34, Heiner Kallweit wrote:
> On 02.03.2019 21:13, Andrew Lunn wrote:
>> On Sat, Mar 02, 2019 at 04:57:32PM +0100, Heiner Kallweit wrote:
>>> Propagate the external PHY settings also in 1000BaseX and
>>> 2500BaseX mode.
>>
>> Hi Heiner
>>
> Hi Andrew
> 
>> I don't think this is needed. 1000BaseX and 2500BaseX does not support
>> inband signalling. So the mac_config() call for these modes should
>> include the needed speed configuration. I've used 1000BaseX with an
>> SFP and there was no need to configure anything.
>>
What I forgot: I can't explain why your SFP use case works. Was it also
with this Marvell switch? 

> 1000BaseX and 2500BaseX use 802.3z Clause 37 inband auto-neg. From what
> I've read 1000BaseT is an extension of it.
> My understanding is that w/o such inband signalling we had no chance
> to detect a "link up" situation. All we could detect is: serial connection
> is synced. Also my understanding is that the ACK to this inband signalling
> triggers the "link up" interrupt.
> 
> Here I found something:
> http://www.methode.com/Documents/TechnicalLibrary/SFP_Ethernet_Auto_Negotiation.pdf
> 
> I briefly looked at the SFP connector and there don't seem to be pins for
> out-of-band signalling. So there must be some inband signalling.
> 
>>     Andrew
>>
> Heiner
> 
Heiner

Re: [PATCH net-next] net: dsa: mv88e6xxx: support in-band signalling with external PHY for 1000BaseX/2500BaseX

[Andrew Lunn]

> I briefly looked at the SFP connector and there don't seem to be pins for
> out-of-band signalling. So there must be some inband signalling.

Hi Heiner

Optical SFPs have an i2c bus with an 'EEPROM' on it. The EEPROM
contains information about the SFP, including its maximum
bit-rate. The Linux SFP driver will look at this bitrate, and pick the
link mode appropriate to it. It then calls mac_config with that link
mode, and the link speed.

The MAC then needs to configure the SERDES to that mode/speed. After a
while, it will get sync and trigger an interrupt. At is then reported
via phylink_mac_change() at which point the carrier is indicated as
up. If one end is using 1000Base-X and the other 2500Base-X, they will
fail to sync. There is no negotiation down to 1000Base-X. 

Optical SFPs are pretty passive devices, they just do electrical to
optical, and not a lot more. All the 'intelligence' is in the SERDES
layers, and they talk to each other end-to-end, unlike copper, where
the MAC SERDES is talking to the PHY SERDES.

Copper SFPs are different. They use SGMII with inband signalling.
There is also a mechanism to encapsulate MDIO over i2c.

       Andrew

Re: [PATCH net-next] net: dsa: mv88e6xxx: support in-band signalling with external PHY for 1000BaseX/2500BaseX

[Heiner Kallweit]

On 02.03.2019 21:45, Andrew Lunn wrote:
>> I briefly looked at the SFP connector and there don't seem to be pins for
>> out-of-band signalling. So there must be some inband signalling.
> 
> Hi Heiner
> 
> Optical SFPs have an i2c bus with an 'EEPROM' on it. The EEPROM
> contains information about the SFP, including its maximum
> bit-rate. The Linux SFP driver will look at this bitrate, and pick the
> link mode appropriate to it. It then calls mac_config with that link
> mode, and the link speed.
> 
I briefly looked at the sfp phy driver and it doesn't seem to use phylink.
So maybe the code in question is needed only if phylink is involved.

As Russell wrote phylink may not yet fully support use case
[Switch port MAC] <--> [SERDES PHY] <--> [External PHY]
He sketched a few ideas.

> The MAC then needs to configure the SERDES to that mode/speed. After a
> while, it will get sync and trigger an interrupt. At is then reported
> via phylink_mac_change() at which point the carrier is indicated as
> up. If one end is using 1000Base-X and the other 2500Base-X, they will
> fail to sync. There is no negotiation down to 1000Base-X. 
> 
> Optical SFPs are pretty passive devices, they just do electrical to
> optical, and not a lot more. All the 'intelligence' is in the SERDES
> layers, and they talk to each other end-to-end, unlike copper, where
> the MAC SERDES is talking to the PHY SERDES.
> 
> Copper SFPs are different. They use SGMII with inband signalling.
> There is also a mechanism to encapsulate MDIO over i2c.
> 
Good to know. Again something learnt.

So there may be cases where the code we talk about isn't needed.
But if let's say a 2.5Gbps copper PHY is connected to port 9 of
a 88E6390 (like the Aquantia PHY), then 2500BaseX is used and I
think the code is needed, or?

>        Andrew
> 
Heiner

Re: [PATCH net-next] net: dsa: mv88e6xxx: support in-band signalling with external PHY for 1000BaseX/2500BaseX

[David Miller]

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sat, 2 Mar 2019 16:57:32 +0100

> Propagate the external PHY settings also in 1000BaseX and
> 2500BaseX mode.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Still some uncertainty about this change, marking as deferred in patchwork.