split phylink PCS operations and add PCS support for dpaa2

*NOT FOR MERGING*

Rename the bland 'ops' member of struct phylink to be a more descriptive 'mac_ops' - this is necessary as we're about to introduce another set of operations.

Signed-off-by Russell King <[email protected]>

From: Russell King <[email protected]>
Date: Mar/17/2020, 14:52:51 (UTC+00:00)

drivers/net/phy/phylink.c static void phylink_mac_config_up(struct phylink *pl, · line 432

Quoted 7 lines

drivers/net/phy/phylink.c +2 -3

-static void phylink_mac_an_restart(struct phylink *pl)
+static void phylink_mac_pcs_an_restart(struct phylink *pl)
 {
 	if (pl->link_config.an_enabled &&
-	    phy_interface_mode_is_8023z(pl->link_config.interface))
-		pl->mac_ops->mac_an_restart(pl->config);
+	    phy_interface_mode_is_8023z(pl->link_config.interface)) {

Please consider removing this condition and just rely on an_enabled field. I have USXGMII support for Clause 73 Autoneg so this won't work with that.

Overall, looks nice but I don't see a mechanism to restart AutoNeg in case of failure. i.e. following PHYLIB implementation you should add a call to restart_aneg in the state machine. This was just a quick look so I may have missed this.

Quoted 3 lines

Please consider removing this condition and just rely on an_enabled field. I have USXGMII support for Clause 73 Autoneg so this won't work with that.

That is actually incorrect. SGMII can have an_enabled being true, but SGMII is not an autonegotiation between the MAC and PHY - it is merely a mechanism for the PHY to inform the MAC what the results of _its_ negotiation are.

I suspect USXGMII is the same since it is just an "upgraded" version of SGMII. Please can you check whether there really is any value in trying (and likely failing) to restart the "handshake" with the PHY from the MAC end, rather than requesting the PHY to restart negotiation on its media side.

From: Russell King - ARM Linux admin <[email protected]>
Date: Mar/17/2020, 15:56:10 (UTC+00:00)

Quoted 3 lines

Quoted 3 lines

Please consider removing this condition and just rely on an_enabled field. I have USXGMII support for Clause 73 Autoneg so this won't work with that.

Quoted 5 lines

That is actually incorrect. SGMII can have an_enabled being true, but SGMII is not an autonegotiation between the MAC and PHY - it is merely a mechanism for the PHY to inform the MAC what the results of _its_ negotiation are.

Quoted 5 lines

I suspect USXGMII is the same since it is just an "upgraded" version of SGMII. Please can you check whether there really is any value in trying (and likely failing) to restart the "handshake" with the PHY from the MAC end, rather than requesting the PHY to restart negotiation on its media side.

I think we are speaking of different things here. I'm speaking about end-to-end Autoneg. Not PHY <-> PCS <-> MAC.

I'm so sorry but I'm not an expert in this field, I just deal mostly with IP.

Anyway, I'm speaking about end-to-end Clause 73 Autoneg which involves exchanging info with the peer. If peer for some reason is not available to receive this info then AutoNeg will not succeed. Hence the reason to restart it.

Quoted 6 lines

From: Russell King - ARM Linux admin <[email protected]>
Date: Mar/17/2020, 15:56:10 (UTC+00:00)

Quoted 3 lines

Quoted 3 lines

Please consider removing this condition and just rely on an_enabled field. I have USXGMII support for Clause 73 Autoneg so this won't work with that.

Do you really mean USXGMII or XLGMII that you recently contributed? XLGMII makes more sense for clause 73.

Quoted 5 lines

Quoted 5 lines

That is actually incorrect. SGMII can have an_enabled being true, but SGMII is not an autonegotiation between the MAC and PHY - it is merely a mechanism for the PHY to inform the MAC what the results of _its_ negotiation are.

Quoted 8 lines

Quoted 5 lines

I suspect USXGMII is the same since it is just an "upgraded" version of SGMII. Please can you check whether there really is any value in trying (and likely failing) to restart the "handshake" with the PHY from the MAC end, rather than requesting the PHY to restart negotiation on its media side.

I think we are speaking of different things here. I'm speaking about end-to-end Autoneg. Not PHY <-> PCS <-> MAC.

What do you mean end-to-end autoneg? Let's take a simple example for SGMII, here is the complete setup:

MAC <--> PCS <--SGMII--> PHY <--MEDIA--> PHY <--SGMII--> MAC

Generally, asking the PCS to "renegotiate" will either be ignored, or might cause the PCS to restart the handshake with the PHY depending on the implementation. It will not cause the PHY to renegotiate with the remote PHY.

Asking the PHY to renegotiate will cause the link to drop, which updates the config_reg word sent to the PCS to indicate link down. When the link is re-established, the config_reg word is updated again with the new link parameters and that sent to the PCS.

So, just because something is closer to the MAC does not necessarily mean that it causes more of the blocks to "renegotiate" when asked to do so.

In SGMII, the PHY is the "master" and this is what needs negotiation restarted on "ethtool -r" to have the effect that the user desires.

I believe (but I don't know for certain, because the USXGMII documentation is not available to me) that USXGMII is SGMII extended up to additionally include 10G, 5G and 2.5G speeds, and otherwise is basically the same mechanism.

So, I would suggest that USXGMII and SGMII should be treated the same, and for both of them, a renegotiation should always be performed at the PHY and not the PCS.

There is another reason not to try triggering renegotiation at both the PHY and PCS. When the PHY is renegotiating, the state machines at both the PHY and PCS end are in the process of changing - if we hit the PCS with a request to renegotiate, and the hardware isn't setup to ignore it, that could occur in an unexpected state - the risk of triggering a hardware problem could be higher.

So, based on this, I don't think it's a good idea to restart negotiation at the PCS for SGMII and USXGMII modes.

For the 802.3z based protocols, it makes complete sense to do so, because the PCS are the blocks involved in the actual media negotiation and there is no place else to restart negotiation:

MAC <---> PCS <----fiber 1000base-X----> PCS <---> MAC

Quoted 8 lines

I'm so sorry but I'm not an expert in this field, I just deal mostly with IP.

Anyway, I'm speaking about end-to-end Clause 73 Autoneg which involves exchanging info with the peer. If peer for some reason is not available to receive this info then AutoNeg will not succeed. Hence the reason to restart it.

Clause 73 covers backplane and copper cable, and isn't USXGMII. In the case of copper, I would expect the setup to be very similar to what I've outlined above for the SGMII case, but using USXGMII instead of SGMII, or automatically selecting something like 10GBASE-R, 5GBASE-R, 2500BASE-X, or SGMII depending on the result from copper negotiation. Depending on the Ethernet PHY being used, it may or may not have the in-band control_reg word even for SGMII. In any case, what I've said above applies: to provoke end-to-end renegotiation, the PHY needs to be restarted, not the MAC's PCS.

For backplane, things are a little different, and that may indeed require the MAC's PCS to be restarted, and for that case it may be reasonable to expand the conditional there.

However, note that for the purposes of this patch, no change of behaviour over the current behaviour is intended; a change to this will need to be a separate patch.

Thanks.

From: Russell King - ARM Linux admin <[email protected]>
Date: Mar/17/2020, 16:52:08 (UTC+00:00)

Quoted 70 lines · Jose Abreu

Quoted 6 lines

From: Russell King - ARM Linux admin <[email protected]>
Date: Mar/17/2020, 15:56:10 (UTC+00:00)

Quoted 3 lines

Quoted 3 lines

Please consider removing this condition and just rely on an_enabled field. I have USXGMII support for Clause 73 Autoneg so this won't work with that.

Do you really mean USXGMII or XLGMII that you recently contributed? XLGMII makes more sense for clause 73.

Quoted 5 lines

Quoted 5 lines

That is actually incorrect. SGMII can have an_enabled being true, but SGMII is not an autonegotiation between the MAC and PHY - it is merely a mechanism for the PHY to inform the MAC what the results of _its_ negotiation are.

Quoted 8 lines

Quoted 5 lines

I suspect USXGMII is the same since it is just an "upgraded" version of SGMII. Please can you check whether there really is any value in trying (and likely failing) to restart the "handshake" with the PHY from the MAC end, rather than requesting the PHY to restart negotiation on its media side.

I think we are speaking of different things here. I'm speaking about end-to-end Autoneg. Not PHY <-> PCS <-> MAC.

What do you mean end-to-end autoneg? Let's take a simple example for SGMII, here is the complete setup:

MAC <--> PCS <--SGMII--> PHY <--MEDIA--> PHY <--SGMII--> MAC

Generally, asking the PCS to "renegotiate" will either be ignored, or might cause the PCS to restart the handshake with the PHY depending on the implementation. It will not cause the PHY to renegotiate with the remote PHY.

Asking the PHY to renegotiate will cause the link to drop, which updates the config_reg word sent to the PCS to indicate link down. When the link is re-established, the config_reg word is updated again with the new link parameters and that sent to the PCS.

So, just because something is closer to the MAC does not necessarily mean that it causes more of the blocks to "renegotiate" when asked to do so.

In SGMII, the PHY is the "master" and this is what needs negotiation restarted on "ethtool -r" to have the effect that the user desires.

I believe (but I don't know for certain, because the USXGMII documentation is not available to me) that USXGMII is SGMII extended up to additionally include 10G, 5G and 2.5G speeds, and otherwise is basically the same mechanism.

So, I would suggest that USXGMII and SGMII should be treated the same, and for both of them, a renegotiation should always be performed at the PHY and not the PCS.

There is another reason not to try triggering renegotiation at both the PHY and PCS. When the PHY is renegotiating, the state machines at both the PHY and PCS end are in the process of changing - if we hit the PCS with a request to renegotiate, and the hardware isn't setup to ignore it, that could occur in an unexpected state - the risk of triggering a hardware problem could be higher.

So, based on this, I don't think it's a good idea to restart negotiation at the PCS for SGMII and USXGMII modes.

For the 802.3z based protocols, it makes complete sense to do so, because the PCS are the blocks involved in the actual media negotiation and there is no place else to restart negotiation:

Quoted 1 line

MAC <---> PCS <----fiber 1000base-X----> PCS <---> MAC

That's kind of the setup I have, hence the need for me to restart ... I have this:

MAC <-> PCS <-> SERDES <-> Copper <-> SERDES <-> PCS <-> MAC

So, no PHY to restart Autoneg.

Quoted 75 lines

From: Russell King - ARM Linux admin <[email protected]>
Date: Mar/17/2020, 16:52:08 (UTC+00:00)

Quoted 71 lines · Jose Abreu

Quoted 6 lines

From: Russell King - ARM Linux admin <[email protected]>
Date: Mar/17/2020, 15:56:10 (UTC+00:00)

Quoted 3 lines

Quoted 3 lines

Please consider removing this condition and just rely on an_enabled field. I have USXGMII support for Clause 73 Autoneg so this won't work with that.

Do you really mean USXGMII or XLGMII that you recently contributed? XLGMII makes more sense for clause 73.

Quoted 5 lines

Quoted 5 lines

That is actually incorrect. SGMII can have an_enabled being true, but SGMII is not an autonegotiation between the MAC and PHY - it is merely a mechanism for the PHY to inform the MAC what the results of _its_ negotiation are.

Quoted 8 lines

Quoted 5 lines

I suspect USXGMII is the same since it is just an "upgraded" version of SGMII. Please can you check whether there really is any value in trying (and likely failing) to restart the "handshake" with the PHY from the MAC end, rather than requesting the PHY to restart negotiation on its media side.

I think we are speaking of different things here. I'm speaking about end-to-end Autoneg. Not PHY <-> PCS <-> MAC.

What do you mean end-to-end autoneg? Let's take a simple example for SGMII, here is the complete setup:

MAC <--> PCS <--SGMII--> PHY <--MEDIA--> PHY <--SGMII--> MAC

Generally, asking the PCS to "renegotiate" will either be ignored, or might cause the PCS to restart the handshake with the PHY depending on the implementation. It will not cause the PHY to renegotiate with the remote PHY.

Asking the PHY to renegotiate will cause the link to drop, which updates the config_reg word sent to the PCS to indicate link down. When the link is re-established, the config_reg word is updated again with the new link parameters and that sent to the PCS.

So, just because something is closer to the MAC does not necessarily mean that it causes more of the blocks to "renegotiate" when asked to do so.

In SGMII, the PHY is the "master" and this is what needs negotiation restarted on "ethtool -r" to have the effect that the user desires.

I believe (but I don't know for certain, because the USXGMII documentation is not available to me) that USXGMII is SGMII extended up to additionally include 10G, 5G and 2.5G speeds, and otherwise is basically the same mechanism.

So, I would suggest that USXGMII and SGMII should be treated the same, and for both of them, a renegotiation should always be performed at the PHY and not the PCS.

There is another reason not to try triggering renegotiation at both the PHY and PCS. When the PHY is renegotiating, the state machines at both the PHY and PCS end are in the process of changing - if we hit the PCS with a request to renegotiate, and the hardware isn't setup to ignore it, that could occur in an unexpected state - the risk of triggering a hardware problem could be higher.

So, based on this, I don't think it's a good idea to restart negotiation at the PCS for SGMII and USXGMII modes.

For the 802.3z based protocols, it makes complete sense to do so, because the PCS are the blocks involved in the actual media negotiation and there is no place else to restart negotiation:

MAC <---> PCS <----fiber 1000base-X----> PCS <---> MAC

Quoted 6 lines

That's kind of the setup I have, hence the need for me to restart ... I have this:

MAC <-> PCS <-> SERDES <-> Copper <-> SERDES <-> PCS <-> MAC

So, no PHY to restart Autoneg.

And the protocol over the copper is USXGMII?

*NOT FOR MERGING*

Add a separate set of PCS operations, which MAC drivers can use to couple phylink with their associated MAC PCS layer. The PCS operations include:

- pcs_get_state() - reads the link up/down, resolved speed, duplex and pause from the PCS.
- pcs_config() - configures the PCS for the specified mode, PHY interface type, and setting the advertisement.
- pcs_an_restart() - restarts 802.3 in-band negotiation with the link partner
- pcs_link_up() - informs the PCS that link has come up, and the parameters of the link. Link parameters are used to program the PCS for fixed speed and non-inband modes.

Quoted 6 lines · Russell King

*NOT FOR MERGING*

Add a separate set of PCS operations, which MAC drivers can use to couple phylink with their associated MAC PCS layer. The PCS operations include:

Quoted 9 lines

- pcs_get_state() - reads the link up/down, resolved speed, duplex and pause from the PCS.
- pcs_config() - configures the PCS for the specified mode, PHY interface type, and setting the advertisement.
- pcs_an_restart() - restarts 802.3 in-band negotiation with the link partner
- pcs_link_up() - informs the PCS that link has come up, and the parameters of the link. Link parameters are used to program the PCS for fixed speed and non-inband modes.

Hi Russell

This API makes sense. But it seems quite common to have multiple PCS's. Rather than have MAC drivers implement their own mux, i wonder if there should be core support? Or at least a library to help the implementation?

Quoted 17 lines · Andrew Lunn

Quoted 6 lines · Russell King

*NOT FOR MERGING*

Add a separate set of PCS operations, which MAC drivers can use to couple phylink with their associated MAC PCS layer. The PCS operations include:

Quoted 9 lines

- pcs_get_state() - reads the link up/down, resolved speed, duplex and pause from the PCS.
- pcs_config() - configures the PCS for the specified mode, PHY interface type, and setting the advertisement.
- pcs_an_restart() - restarts 802.3 in-band negotiation with the link partner
- pcs_link_up() - informs the PCS that link has come up, and the parameters of the link. Link parameters are used to program the PCS for fixed speed and non-inband modes.

Quoted 6 lines

Hi Russell

This API makes sense. But it seems quite common to have multiple PCS's. Rather than have MAC drivers implement their own mux, i wonder if there should be core support? Or at least a library to help the implementation?

When each PCS has different characteristics, and may not even be available to be probed (because the hardware holds them in reset, so they don't even respond to MDIO cycles) that becomes very difficult.

That is the situation with LX2160A - when in 1G mode, the 10G C45 PCS does not respond. Already tested that.

So, determining when to switch can't be known by generic code.

Oh, I forgot to mention on the library point - that's what has already been created in:

"net: phylink: pcs: add 802.3 clause 45 helpers"
"net: phylink: pcs: add 802.3 clause 22 helpers"

which add library implementations for the pcs_get_state(), pcs_config() and pcs_an_restart() methods.

What remains is vendor specific - for pcs_link_up(), there is no standard, since it requires fiddling with vendor specific registers to program, e.g. the speed in SGMII mode when in-band is not being used. The selection between different PCS is also vendor specific.

It would have been nice to use these helpers for Marvell DSA switches too, but the complexities of DSA taking a multi-layered approach rather than a library approach, plus the use of paging makes it very difficult.

So, basically on the library point, "already considered and implemented".

Hi Russell

The 6390X family of switches has two PCSs, one for 1000BaseX/SGMII and a second one for 10GBaseR. So at some point there is going to be a mux, but maybe it will be internal to mv88e6xxx and not shareable. Or internal to DSA, and shareable between DSA drivers. We will see.

Quoted 14 lines

Oh, I forgot to mention on the library point - that's what has already been created in:

"net: phylink: pcs: add 802.3 clause 45 helpers"
"net: phylink: pcs: add 802.3 clause 22 helpers"

which add library implementations for the pcs_get_state(), pcs_config() and pcs_an_restart() methods.

What remains is vendor specific - for pcs_link_up(), there is no standard, since it requires fiddling with vendor specific registers to program, e.g. the speed in SGMII mode when in-band is not being used. The selection between different PCS is also vendor specific.

Quoted 7 lines

It would have been nice to use these helpers for Marvell DSA switches too, but the complexities of DSA taking a multi-layered approach rather than a library approach, plus the use of paging makes it very difficult.

So, basically on the library point, "already considered and implemented".

Hi Russell

The 6390X family of switches has two PCSs, one for 1000BaseX/SGMII and a second one for 10GBaseR. So at some point there is going to be a mux, but maybe it will be internal to mv88e6xxx and not shareable. Or internal to DSA, and shareable between DSA drivers. We will see.

Quoted 14 lines

Oh, I forgot to mention on the library point - that's what has already been created in:

"net: phylink: pcs: add 802.3 clause 45 helpers"
"net: phylink: pcs: add 802.3 clause 22 helpers"

which add library implementations for the pcs_get_state(), pcs_config() and pcs_an_restart() methods.

What remains is vendor specific - for pcs_link_up(), there is no standard, since it requires fiddling with vendor specific registers to program, e.g. the speed in SGMII mode when in-band is not being used. The selection between different PCS is also vendor specific.

Quoted 7 lines

It would have been nice to use these helpers for Marvell DSA switches too, but the complexities of DSA taking a multi-layered approach rather than a library approach, plus the use of paging makes it very difficult.

So, basically on the library point, "already considered and implemented".

Hi Russell

The 6390X family of switches has two PCSs, one for 1000BaseX/SGMII and a second one for 10GBaseR. So at some point there is going to be a mux, but maybe it will be internal to mv88e6xxx and not shareable. Or internal to DSA, and shareable between DSA drivers. We will see.

Subject: [RFC net-next 3/5] arm64: dts: lx2160a: add PCS MDIO nodes

*NOT FOR MERGING*

Add PCS MDIO nodes for the LX2160A, which will be used when the MAC is in PHY mode and is using in-band negotiation.

Signed-off-by Russell King <[email protected]>

---
 .../arm64/boot/dts/freescale/fsl-lx2160a.dtsi | 144 ++++++++++++++++++
 1 file changed, 144 insertions(+)

arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi +0 -0

index e5ee5591e52b..732af33eec18 100644

arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi +110 -0

+
+		pcs_mdio2: mdio@8c0b000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c0b000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio3: mdio@8c0f000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c0f000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio4: mdio@8c13000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c13000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio5: mdio@8c17000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c17000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio6: mdio@8c1b000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c1b000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio7: mdio@8c1f000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c1f000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio8: mdio@8c23000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c23000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio9: mdio@8c27000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c27000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio10: mdio@8c2b000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c2b000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio11: mdio@8c2f000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c2f000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio12: mdio@8c33000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c33000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio13: mdio@8c37000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c37000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio14: mdio@8c3b000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c3b000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio15: mdio@8c3f000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c3f000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio16: mdio@8c43000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c43000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio17: mdio@8c47000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c47000 0x0 0x1000>;
+			little-endian;

arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi +19 -1

 		fsl_mc: fsl-mc@80c000000 {
 			compatible = "fsl,qoriq-mc";
 			reg = <0x00000008 0x0c000000 0 0x40>, @@ -988,91
+1114,109 @@
 				dpmac1: dpmac@1 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x1>;
+					pcs-mdio = <&pcs_mdio1>;
 				};
 
 				dpmac2: dpmac@2 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x2>;
+					pcs-mdio = <&pcs_mdio2>;
 				};
 
 				dpmac3: dpmac@3 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x3>;
+					pcs-mdio = <&pcs_mdio3>;
 				};
 
 				dpmac4: dpmac@4 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x4>;
+					pcs-mdio = <&pcs_mdio4>;
 				};
 
 				dpmac5: dpmac@5 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x5>;
+					pcs-mdio = <&pcs_mdio5>;
 				};
 
 				dpmac6: dpmac@6 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x6>;
+					pcs-mdio = <&pcs_mdio6>;
 				};
 
 				dpmac7: dpmac@7 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x7>;
+					pcs-mdio = <&pcs_mdio7>;
 				};
 
 				dpmac8: dpmac@8 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x8>;
+					pcs-mdio = <&pcs_mdio8>;
 				};
 
 				dpmac9: dpmac@9 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x9>;
+					pcs-mdio = <&pcs_mdio9>;
 				};
 
 				dpmac10: dpmac@a {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0xa>;
+					pcs-mdio = <&pcs_mdio10>;
 				};
 
 				dpmac11: dpmac@b {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0xb>;
+					pcs-mdio = <&pcs_mdio11>;
 				};
 
 				dpmac12: dpmac@c {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0xc>;
+					pcs-mdio = <&pcs_mdio12>;
 				};
 
 				dpmac13: dpmac@d {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0xd>;
+					pcs-mdio = <&pcs_mdio13>;
 				};
 
 				dpmac14: dpmac@e {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0xe>;
+					pcs-mdio = <&pcs_mdio14>;
 				};
 
 				dpmac15: dpmac@f {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0xf>;
+					pcs-mdio = <&pcs_mdio15>;
 				};
 
 				dpmac16: dpmac@10 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x10>;
+					pcs-mdio = <&pcs_mdio16>;
 				};
 
 				dpmac17: dpmac@11 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x11>;
+					pcs-mdio = <&pcs_mdio17>;
 				};
 
 				dpmac18: dpmac@12 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x12>;
+					pcs-mdio = <&pcs_mdio18>;
 				};
 			};
 		};
--

2.20.1

Quoted 21 lines

Subject: [RFC net-next 3/5] arm64: dts: lx2160a: add PCS MDIO nodes

*NOT FOR MERGING*

Add PCS MDIO nodes for the LX2160A, which will be used when the MAC is in PHY mode and is using in-band negotiation.

Signed-off-by Russell King <[email protected]>

---
 .../arm64/boot/dts/freescale/fsl-lx2160a.dtsi | 144 ++++++++++++++++++
 1 file changed, 144 insertions(+)

arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi +0 -0

index e5ee5591e52b..732af33eec18 100644
@@ -960,6 +960,132 @@
 			status = "disabled";
 		};
 

arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi line 968

Quoted 6 lines

arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi +6 -0

+		pcs_mdio1: mdio@8c07000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c07000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};

Are the PCS MDIO buses shareable? I am asking this because in case of QSGMII our structure is a little bit quirky. There are 4 MACs but all PCSs sit on the first MACs internal MDIO bus only. The other 3 internal MDIO buses are empty.

Quoted 114 lines

arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi +114 -0

+
+		pcs_mdio2: mdio@8c0b000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c0b000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio3: mdio@8c0f000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c0f000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio4: mdio@8c13000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c13000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio5: mdio@8c17000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c17000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio6: mdio@8c1b000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c1b000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio7: mdio@8c1f000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c1f000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio8: mdio@8c23000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c23000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio9: mdio@8c27000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c27000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio10: mdio@8c2b000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c2b000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio11: mdio@8c2f000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c2f000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio12: mdio@8c33000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c33000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio13: mdio@8c37000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c37000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio14: mdio@8c3b000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c3b000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio15: mdio@8c3f000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c3f000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio16: mdio@8c43000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c43000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio17: mdio@8c47000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c47000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio18: mdio@8c4b000 {

arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi line 1088

Quoted 6 lines

arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi +6 -0

+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c4b000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+

Please sort the nodes alphabetically.

Quoted 114 lines

arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi +19 -1

 		fsl_mc: fsl-mc@80c000000 {
 			compatible = "fsl,qoriq-mc";
 			reg = <0x00000008 0x0c000000 0 0x40>, @@ -988,91
+1114,109 @@
 				dpmac1: dpmac@1 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x1>;
+					pcs-mdio = <&pcs_mdio1>;
 				};
 
 				dpmac2: dpmac@2 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x2>;
+					pcs-mdio = <&pcs_mdio2>;
 				};
 
 				dpmac3: dpmac@3 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x3>;
+					pcs-mdio = <&pcs_mdio3>;
 				};
 
 				dpmac4: dpmac@4 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x4>;
+					pcs-mdio = <&pcs_mdio4>;
 				};
 
 				dpmac5: dpmac@5 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x5>;
+					pcs-mdio = <&pcs_mdio5>;
 				};
 
 				dpmac6: dpmac@6 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x6>;
+					pcs-mdio = <&pcs_mdio6>;
 				};
 
 				dpmac7: dpmac@7 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x7>;
+					pcs-mdio = <&pcs_mdio7>;
 				};
 
 				dpmac8: dpmac@8 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x8>;
+					pcs-mdio = <&pcs_mdio8>;
 				};
 
 				dpmac9: dpmac@9 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x9>;
+					pcs-mdio = <&pcs_mdio9>;
 				};
 
 				dpmac10: dpmac@a {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0xa>;
+					pcs-mdio = <&pcs_mdio10>;
 				};
 
 				dpmac11: dpmac@b {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0xb>;
+					pcs-mdio = <&pcs_mdio11>;
 				};
 
 				dpmac12: dpmac@c {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0xc>;
+					pcs-mdio = <&pcs_mdio12>;
 				};
 
 				dpmac13: dpmac@d {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0xd>;
+					pcs-mdio = <&pcs_mdio13>;
 				};
 
 				dpmac14: dpmac@e {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0xe>;
+					pcs-mdio = <&pcs_mdio14>;
 				};
 
 				dpmac15: dpmac@f {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0xf>;
+					pcs-mdio = <&pcs_mdio15>;
 				};
 
 				dpmac16: dpmac@10 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x10>;
+					pcs-mdio = <&pcs_mdio16>;
 				};
 
 				dpmac17: dpmac@11 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x11>;
+					pcs-mdio = <&pcs_mdio17>;
 				};
 
 				dpmac18: dpmac@12 {
 					compatible = "fsl,qoriq-mc-dpmac";
 					reg = <0x12>;
+					pcs-mdio = <&pcs_mdio18>;
 				};
 			};
 		};
--

Quoted 1 line

2.20.1

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Subject: Re: [RFC net-next 3/5] arm64: dts: lx2160a: add PCS MDIO nodes

Quoted 22 lines · Ioana Ciornei

Quoted 22 lines

Subject: [RFC net-next 3/5] arm64: dts: lx2160a: add PCS MDIO nodes

*NOT FOR MERGING*

Add PCS MDIO nodes for the LX2160A, which will be used when the MAC is in PHY mode and is using in-band negotiation.

Signed-off-by Russell King <[email protected]>

---
 .../arm64/boot/dts/freescale/fsl-lx2160a.dtsi | 144
++++++++++++++++++
 1 file changed, 144 insertions(+)

arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi +0 -0

index e5ee5591e52b..732af33eec18 100644
@@ -960,6 +960,132 @@
 			status = "disabled";
 		};
 

Quoted 8 lines

arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi line 968

Quoted 6 lines

arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi +6 -0

+		pcs_mdio1: mdio@8c07000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c07000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};

Are the PCS MDIO buses shareable? I am asking this because in case of QSGMII

our structure is a little bit quirky.

Quoted 1 line

There are 4 MACs but all PCSs sit on the first MACs internal MDIO bus only. The

other 3 internal MDIO buses are empty.

I haven't looked at QSGMII yet, I've only considered single-lane setups and only implemented that. For _this_ part, it doesn't matter as this is just declaring where the hardware is. I think that matters more for the dpmac nodes.

Quoted 115 lines

Quoted 115 lines

arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi +114 -0

+
+		pcs_mdio2: mdio@8c0b000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c0b000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio3: mdio@8c0f000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c0f000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio4: mdio@8c13000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c13000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio5: mdio@8c17000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c17000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio6: mdio@8c1b000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c1b000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio7: mdio@8c1f000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c1f000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio8: mdio@8c23000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c23000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio9: mdio@8c27000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c27000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio10: mdio@8c2b000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c2b000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio11: mdio@8c2f000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c2f000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio12: mdio@8c33000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c33000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio13: mdio@8c37000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c37000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio14: mdio@8c3b000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c3b000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio15: mdio@8c3f000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c3f000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio16: mdio@8c43000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c43000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio17: mdio@8c47000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c47000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		pcs_mdio18: mdio@8c4b000 {

Quoted 8 lines

arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi line 1088

Quoted 6 lines

arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi +6 -0

+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c4b000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+

Please sort the nodes alphabetically.

Huh? The nodes in this file are already sorted according to address, and this patch preserves that sorting. The hex address field also happens to be alphabetical.

Or do you mean the label for these modes - I've never heard of sorting by label for a SoC file.

Subject: [RFC net-next 4/5] dpaa2-mac: add 1000BASE-X/SGMII PCS support

*NOT FOR MERGING*

Add support for the PCS block, so we can dynamically configure it for 1000base- X or SGMII depending on the SFP module inserted. This gives us more flexibility than using the MC firmware with a "fixed" link type, which can only be setup to support a single interface mode.

Signed-off-by Russell King <[email protected]>

---
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  | 206 +++++++++++++++++-
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.h  |   1 +
 2 files changed, 204 insertions(+), 3 deletions(-)

drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c +99 -0

index 3ee236c5fc37..e7b2dc366338 100644
@@ -7,6 +7,117 @@
 #define phylink_to_dpaa2_mac(config) \
 	container_of((config), struct dpaa2_mac, phylink_config)
 
+#define MII_IFMODE		0x14
+#define IF_MODE_SGMII_ENA	BIT(0)
+#define IF_MODE_USE_SGMII_AN	BIT(1)
+#define IF_MODE_SGMII_SPEED_10	(0 << 2)
+#define IF_MODE_SGMII_SPEED_100	(1 << 2)
+#define IF_MODE_SGMII_SPEED_1G	(2 << 2)
+#define IF_MODE_SGMII_SPEED_MSK	(3 << 2)
+#define IF_MODE_SGMII_DUPLEX	BIT(4)		// set = half duplex
+
+static void dpaa2_mac_pcs_get_state(struct phylink_config *config,
+				    struct phylink_link_state *state) {
+	struct mdio_device *pcs = phylink_to_dpaa2_mac(config)->pcs;
+
+	switch (state->interface) {
+	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_1000BASEX:
+		phylink_mii_c22_pcs_get_state(pcs, state);
+		break;
+
+	default:
+		break;
+	}
+}
+
+static void dpaa2_mac_pcs_an_restart(struct phylink_config *config) {
+	struct mdio_device *pcs = phylink_to_dpaa2_mac(config)->pcs;
+
+	phylink_mii_c22_pcs_an_restart(pcs);
+}
+
+static int dpaa2_mac_pcs_config(struct phylink_config *config,
+				unsigned int mode,
+				const struct phylink_link_state *state) {
+	struct mdio_device *pcs = phylink_to_dpaa2_mac(config)->pcs;
+	u16 if_mode;
+	int bmcr, ret;
+
+	if (mode == MLO_AN_INBAND)
+		bmcr = BMCR_ANENABLE;
+	else
+		bmcr = 0;
+
+	switch (state->interface) {
+	case PHY_INTERFACE_MODE_SGMII:
+		if_mode = IF_MODE_SGMII_ENA;
+		if (mode == MLO_AN_INBAND)
+			if_mode |= IF_MODE_USE_SGMII_AN;
+		mdiobus_modify(pcs->bus, 0, MII_IFMODE,
+			       IF_MODE_SGMII_ENA | IF_MODE_USE_SGMII_AN,
+			       if_mode);
+		mdiobus_modify(pcs->bus, 0, MII_BMCR, BMCR_ANENABLE,
 bmcr);
+		ret = phylink_mii_c22_pcs_set_advertisement(pcs, state);
+		break;
+
+	case PHY_INTERFACE_MODE_1000BASEX:
+		mdiobus_write(pcs->bus, 0, MII_IFMODE, 0);
+		mdiobus_modify(pcs->bus, 0, MII_BMCR, BMCR_ANENABLE,
 bmcr);
+		ret = phylink_mii_c22_pcs_set_advertisement(pcs, state);
+		break;
+
+	default:
+		ret = 0;
+		break;
+	}
+
+	return ret;
+}
+
+static void dpaa2_mac_pcs_link_up(struct phylink_config *config,
+				  unsigned int mode, phy_interface_t interface,
+				  int speed, int duplex)
+{
+	struct mdio_device *pcs = phylink_to_dpaa2_mac(config)->pcs;
+	u16 if_mode;
+
+	/* The PCS PHY needs to be configured manually for the speed and
+	 * duplex when operating in SGMII mode without in-band negotiation.
+	 */
+	if (mode == MLO_AN_INBAND || interface !=
 PHY_INTERFACE_MODE_SGMII)
+		return;
+
+	switch (speed) {
+	case SPEED_10:
+		if_mode = IF_MODE_SGMII_SPEED_10;
+		break;
+
+	case SPEED_100:
+		if_mode = IF_MODE_SGMII_SPEED_100;
+		break;
+
+	default:
+		if_mode = IF_MODE_SGMII_SPEED_1G;
+		break;
+	}
+	if (duplex == DUPLEX_HALF)
+		if_mode |= IF_MODE_SGMII_DUPLEX;
+
+	mdiobus_modify(pcs->bus, pcs->addr, MII_IFMODE,
+		       IF_MODE_SGMII_DUPLEX | IF_MODE_SGMII_SPEED_MSK,

if_mode); }

drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c +13 -0

+
+static const struct phylink_pcs_ops dpaa2_pcs_phylink_ops = {
+	.pcs_get_state = dpaa2_mac_pcs_get_state,
+	.pcs_config = dpaa2_mac_pcs_config,
+	.pcs_an_restart = dpaa2_mac_pcs_an_restart,
+	.pcs_link_up = dpaa2_mac_pcs_link_up,
+};
+
 static int phy_mode(enum dpmac_eth_if eth_if, phy_interface_t *if_mode)  {
 	*if_mode = PHY_INTERFACE_MODE_NA;

@@ -15,6 +126,11 @@ static int phy_mode(enum dpmac_eth_if eth_if,
 phy_interface_t *if_mode)
 	case DPMAC_ETH_IF_RGMII:
 		*if_mode = PHY_INTERFACE_MODE_RGMII;
 		break;
+
+	case DPMAC_ETH_IF_SGMII:
+		*if_mode = PHY_INTERFACE_MODE_SGMII;
+		break;
+
 	default:
 		return -EINVAL;

drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c +4 -0

 	}

@@ -67,6 +183,10 @@ static bool dpaa2_mac_phy_mode_mismatch(struct
 dpaa2_mac *mac,
 					phy_interface_t interface)
 {
 	switch (interface) {
+	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_1000BASEX:
+		return interface != mac->if_mode && !mac->pcs;
+
 	case PHY_INTERFACE_MODE_RGMII:
 	case PHY_INTERFACE_MODE_RGMII_ID:

drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c +8 -2

 	case PHY_INTERFACE_MODE_RGMII_RXID:

@@ -95,13 +215,19 @@ static void dpaa2_mac_validate(struct phylink_config
 *config,
 	phylink_set(mask, Asym_Pause);
 
 	switch (state->interface) {
+	case PHY_INTERFACE_MODE_NA:
+	case PHY_INTERFACE_MODE_1000BASEX:
+	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_RGMII:
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		phylink_set(mask, 10baseT_Full);
-		phylink_set(mask, 100baseT_Full);
+		phylink_set(mask, 1000baseX_Full);
 		phylink_set(mask, 1000baseT_Full);
+		if (state->interface == PHY_INTERFACE_MODE_1000BASEX)
+			break;
+		phylink_set(mask, 100baseT_Full);
+		phylink_set(mask, 10baseT_Full);
 		break;
 	default:

drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c +1 -0

 		goto empty_set;

@@ -227,6 +353,65 @@ bool dpaa2_mac_is_type_fixed(struct fsl_mc_device
 *dpmac_dev,
 	return fixed;
 }
 
+static int dpaa2_pcs_create(struct dpaa2_mac *mac,

drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c +46 -0

+		return 0;
+	}
+
+	if (!of_device_is_available(node)) {
+		netdev_err(mac->net_dev, "pcs-mdio node not available\n");
+		return -ENODEV;
+	}
+
+	bus = of_mdio_find_bus(node);
+	of_node_put(node);
+	if (!bus)
+		return -EPROBE_DEFER;
+
+	mdiodev = mdio_device_create(bus, 0);
+	if (IS_ERR(mdiodev)) {
+		err = PTR_ERR(mdiodev);
+		netdev_err(mac->net_dev, "failed to create mdio device:
 %d\n",
+			   err);
+		goto err;
+	}
+
+	err = mdio_device_register(mdiodev);
+	if (err) {
+		netdev_err(mac->net_dev, "failed to register mdio device:
 %d\n",
+			   err);
+		goto dev_free;
+	}
+
+	mac->pcs = mdiodev;
+	mac->phylink_config.pcs_poll = true;
+
+	return 0;
+
+dev_free:
+	mdio_device_free(mdiodev);
+err:
+	return err;
+}
+
+static void dpaa2_pcs_destroy(struct dpaa2_mac *mac) {
+	if (mac->pcs) {
+		mdio_device_remove(mac->pcs);
+		mdio_device_free(mac->pcs);
+	}
+}
+
 int dpaa2_mac_connect(struct dpaa2_mac *mac)  {
 	struct fsl_mc_device *dpmac_dev = mac->mc_dev; @@ -236,6 +421,8

@@ int dpaa2_mac_connect(struct dpaa2_mac *mac)

drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c +16 -1

 	struct dpmac_attr attr;
 	int err;
 
+	memset(&mac->phylink_config, 0, sizeof(mac->phylink_config));
+
 	err = dpmac_open(mac->mc_io, 0, dpmac_dev->obj_desc.id,
 			 &dpmac_dev->mc_handle);
 	if (err || !dpmac_dev->mc_handle) {

@@ -278,6 +465,13 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
 		goto err_put_node;
 	}
 
+	if (attr.link_type == DPMAC_LINK_TYPE_PHY &&
+	    attr.eth_if != DPMAC_ETH_IF_RGMII) {
+		err = dpaa2_pcs_create(mac, dpmac_node, attr.id);
+		if (err)
+			goto err_put_node;
+	}
+
 	mac->phylink_config.dev = &net_dev->dev;
 	mac->phylink_config.type = PHYLINK_NETDEV;
 

@@ -286,10 +480,13 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
 				 &dpaa2_mac_phylink_ops);
 	if (IS_ERR(phylink)) {
 		err = PTR_ERR(phylink);
-		goto err_put_node;
+		goto err_pcs_destroy;
 	}
 	mac->phylink = phylink;
 
+	if (mac->pcs)
+		phylink_add_pcs(mac->phylink, &dpaa2_pcs_phylink_ops);
+
 	err = phylink_of_phy_connect(mac->phylink, dpmac_node, 0);
 	if (err) {
 		netdev_err(net_dev, "phylink_of_phy_connect() = %d\n", err);

@@ -302,6 +499,8 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
 
 err_phylink_destroy:
 	phylink_destroy(mac->phylink);
+err_pcs_destroy:
+	dpaa2_pcs_destroy(mac);
 err_put_node:
 	of_node_put(dpmac_node);
 err_close_dpmac:

@@ -316,6 +515,7 @@ void dpaa2_mac_disconnect(struct dpaa2_mac *mac)
 
 	phylink_disconnect_phy(mac->phylink);
 	phylink_destroy(mac->phylink);
+	dpaa2_pcs_destroy(mac);
 	dpmac_close(mac->mc_io, 0, mac->mc_dev->mc_handle);  }
 

drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h +1 -1

index 2130d9c7d40e..5cfae5f8f55e 100644
@@ -19,6 +19,7 @@ struct dpaa2_mac {
 
 	struct phylink_config phylink_config;
 	struct phylink *phylink;
+	struct mdio_device *pcs;
 	phy_interface_t if_mode;
 	enum dpmac_link_type if_link_type;
 };
--

2.20.1

This series splits the phylink_mac_ops structure so that PCS can be supported separately with their own PCS operations, and illustrates the use of the helpers in the previous patch series (net: add phylink support for PCS) in the DPAA2 driver.

This is prototype code, not intended to be merged yet, and is merely being sent for illustrative purposes only.

 arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi   | 144 +++++++++++++++
 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c | 226 ++++++++++++++++++++++-
 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h |   1 +
 drivers/net/phy/phylink.c                        | 102 ++++++----
 include/linux/phylink.h                          |  11 ++
 5 files changed, 446 insertions(+), 38 deletions(-)

Signature · 2 lines

RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up

Hi,

Was there any conclusion on this 5 patch series, and whether I should submit it for net-next?

This series splits the phylink_mac_ops structure so that PCS can be supported separately with their own PCS operations, separating them from the MAC layer. This may need adaption later as more users come along.

From: Russell King - ARM Linux admin <[email protected]>
Date: Thu, 26 Mar 2020 15:14:04 +0000

Quoted 4 lines

This series splits the phylink_mac_ops structure so that PCS can be supported separately with their own PCS operations, separating them from the MAC layer. This may need adaption later as more users come along.

It looks like there will be a respin of this based upon Andrew's feedback.