[RFC net-next 0/2] split phylink PCS operations and add PCS support for dpaa2

[RFC net-next 0/2] split phylink PCS operations and add PCS support for dpaa2

[Russell King - ARM Linux admin]

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(-)

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

[RFC net-next 1/5] net: phylink: rename 'ops' to 'mac_ops'

[Russell King]

*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 <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index fed0c5907c6a..a34a3be92dba 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -40,7 +40,7 @@ enum {
 struct phylink {
 	/* private: */
 	struct net_device *netdev;
-	const struct phylink_mac_ops *ops;
+	const struct phylink_mac_ops *mac_ops;
 	struct phylink_config *config;
 	struct device *dev;
 	unsigned int old_link_state:1;
@@ -154,7 +154,7 @@ static const char *phylink_an_mode_str(unsigned int mode)
 static int phylink_validate(struct phylink *pl, unsigned long *supported,
 			    struct phylink_link_state *state)
 {
-	pl->ops->validate(pl->config, supported, state);
+	pl->mac_ops->validate(pl->config, supported, state);
 
 	return phylink_is_empty_linkmode(supported) ? -EINVAL : 0;
 }
@@ -415,7 +415,7 @@ static void phylink_mac_config(struct phylink *pl,
 		    __ETHTOOL_LINK_MODE_MASK_NBITS, state->advertising,
 		    state->pause, state->link, state->an_enabled);
 
-	pl->ops->mac_config(pl->config, pl->cur_link_an_mode, state);
+	pl->mac_ops->mac_config(pl->config, pl->cur_link_an_mode, state);
 }
 
 static void phylink_mac_config_up(struct phylink *pl,
@@ -429,7 +429,7 @@ static void phylink_mac_an_restart(struct phylink *pl)
 {
 	if (pl->link_config.an_enabled &&
 	    phy_interface_mode_is_8023z(pl->link_config.interface))
-		pl->ops->mac_an_restart(pl->config);
+		pl->mac_ops->mac_an_restart(pl->config);
 }
 
 static void phylink_mac_pcs_get_state(struct phylink *pl,
@@ -445,7 +445,7 @@ static void phylink_mac_pcs_get_state(struct phylink *pl,
 	state->an_complete = 0;
 	state->link = 1;
 
-	pl->ops->mac_pcs_get_state(pl->config, state);
+	pl->mac_ops->mac_pcs_get_state(pl->config, state);
 }
 
 /* The fixed state is... fixed except for the link state,
@@ -512,11 +512,11 @@ static void phylink_mac_link_up(struct phylink *pl,
 	struct net_device *ndev = pl->netdev;
 
 	pl->cur_interface = link_state.interface;
-	pl->ops->mac_link_up(pl->config, pl->phydev,
-			     pl->cur_link_an_mode, pl->cur_interface,
-			     link_state.speed, link_state.duplex,
-			     !!(link_state.pause & MLO_PAUSE_TX),
-			     !!(link_state.pause & MLO_PAUSE_RX));
+	pl->mac_ops->mac_link_up(pl->config, pl->phydev,
+				 pl->cur_link_an_mode, pl->cur_interface,
+				 link_state.speed, link_state.duplex,
+				 !!(link_state.pause & MLO_PAUSE_TX),
+				 !!(link_state.pause & MLO_PAUSE_RX));
 
 	if (ndev)
 		netif_carrier_on(ndev);
@@ -534,8 +534,8 @@ static void phylink_mac_link_down(struct phylink *pl)
 
 	if (ndev)
 		netif_carrier_off(ndev);
-	pl->ops->mac_link_down(pl->config, pl->cur_link_an_mode,
-			       pl->cur_interface);
+	pl->mac_ops->mac_link_down(pl->config, pl->cur_link_an_mode,
+				   pl->cur_interface);
 	phylink_info(pl, "Link is Down\n");
 }
 
@@ -666,7 +666,7 @@ static int phylink_register_sfp(struct phylink *pl,
  * @fwnode: a pointer to a &struct fwnode_handle describing the network
  *	interface
  * @iface: the desired link mode defined by &typedef phy_interface_t
- * @ops: a pointer to a &struct phylink_mac_ops for the MAC.
+ * @mac_ops: a pointer to a &struct phylink_mac_ops for the MAC.
  *
  * Create a new phylink instance, and parse the link parameters found in @np.
  * This will parse in-band modes, fixed-link or SFP configuration.
@@ -679,7 +679,7 @@ static int phylink_register_sfp(struct phylink *pl,
 struct phylink *phylink_create(struct phylink_config *config,
 			       struct fwnode_handle *fwnode,
 			       phy_interface_t iface,
-			       const struct phylink_mac_ops *ops)
+			       const struct phylink_mac_ops *mac_ops)
 {
 	struct phylink *pl;
 	int ret;
@@ -712,7 +712,7 @@ struct phylink *phylink_create(struct phylink_config *config,
 	pl->link_config.speed = SPEED_UNKNOWN;
 	pl->link_config.duplex = DUPLEX_UNKNOWN;
 	pl->link_config.an_enabled = true;
-	pl->ops = ops;
+	pl->mac_ops = mac_ops;
 	__set_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state);
 	timer_setup(&pl->link_poll, phylink_fixed_poll, 0);
 
-- 
2.20.1

Re: [RFC net-next 1/5] net: phylink: rename 'ops' to 'mac_ops'

[Andrew Lunn]

On Tue, Mar 17, 2020 at 02:52:46PM +0000, Russell King wrote:
> *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 <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[RFC net-next 2/5] net: phylink: add separate pcs operations structure

[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:

- 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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 76 +++++++++++++++++++++++++++------------
 include/linux/phylink.h   | 11 ++++++
 2 files changed, 65 insertions(+), 22 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index a34a3be92dba..abe2cc168f93 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -41,6 +41,7 @@ struct phylink {
 	/* private: */
 	struct net_device *netdev;
 	const struct phylink_mac_ops *mac_ops;
+	const struct phylink_pcs_ops *pcs_ops;
 	struct phylink_config *config;
 	struct device *dev;
 	unsigned int old_link_state:1;
@@ -425,11 +426,31 @@ static void phylink_mac_config_up(struct phylink *pl,
 		phylink_mac_config(pl, state);
 }
 
-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)) {
+		if (pl->pcs_ops)
+			pl->pcs_ops->pcs_an_restart(pl->config);
+		else
+			pl->mac_ops->mac_an_restart(pl->config);
+	}
+}
+
+static void phylink_pcs_config(struct phylink *pl, bool force_restart,
+			       const struct phylink_link_state *state)
+{
+	bool restart = force_restart;
+
+	if (pl->pcs_ops && pl->pcs_ops->pcs_config(pl->config,
+						   pl->cur_link_an_mode,
+						   state))
+		restart = true;
+
+	phylink_mac_config(pl, state);
+
+	if (restart)
+		phylink_mac_pcs_an_restart(pl);
 }
 
 static void phylink_mac_pcs_get_state(struct phylink *pl,
@@ -445,7 +466,10 @@ static void phylink_mac_pcs_get_state(struct phylink *pl,
 	state->an_complete = 0;
 	state->link = 1;
 
-	pl->mac_ops->mac_pcs_get_state(pl->config, state);
+	if (pl->pcs_ops)
+		pl->pcs_ops->pcs_get_state(pl->config, state);
+	else
+		pl->mac_ops->mac_pcs_get_state(pl->config, state);
 }
 
 /* The fixed state is... fixed except for the link state,
@@ -463,7 +487,7 @@ static void phylink_get_fixed_state(struct phylink *pl,
 	phylink_resolve_flow(state);
 }
 
-static void phylink_mac_initial_config(struct phylink *pl)
+static void phylink_mac_initial_config(struct phylink *pl, bool force_restart)
 {
 	struct phylink_link_state link_state;
 
@@ -489,7 +513,7 @@ static void phylink_mac_initial_config(struct phylink *pl)
 	link_state.link = false;
 
 	phylink_apply_manual_flow(pl, &link_state);
-	phylink_mac_config(pl, &link_state);
+	phylink_pcs_config(pl, force_restart, &link_state);
 }
 
 static const char *phylink_pause_to_str(int pause)
@@ -506,12 +530,18 @@ static const char *phylink_pause_to_str(int pause)
 	}
 }
 
-static void phylink_mac_link_up(struct phylink *pl,
-				struct phylink_link_state link_state)
+static void phylink_link_up(struct phylink *pl,
+			    struct phylink_link_state link_state)
 {
 	struct net_device *ndev = pl->netdev;
 
 	pl->cur_interface = link_state.interface;
+
+	if (pl->pcs_ops && pl->pcs_ops->pcs_link_up)
+		pl->pcs_ops->pcs_link_up(pl->config, pl->cur_link_an_mode,
+					 pl->cur_interface,
+					 link_state.speed, link_state.duplex);
+
 	pl->mac_ops->mac_link_up(pl->config, pl->phydev,
 				 pl->cur_link_an_mode, pl->cur_interface,
 				 link_state.speed, link_state.duplex,
@@ -528,7 +558,7 @@ static void phylink_mac_link_up(struct phylink *pl,
 		     phylink_pause_to_str(link_state.pause));
 }
 
-static void phylink_mac_link_down(struct phylink *pl)
+static void phylink_link_down(struct phylink *pl)
 {
 	struct net_device *ndev = pl->netdev;
 
@@ -597,9 +627,9 @@ static void phylink_resolve(struct work_struct *w)
 	if (link_changed) {
 		pl->old_link_state = link_state.link;
 		if (!link_state.link)
-			phylink_mac_link_down(pl);
+			phylink_link_down(pl);
 		else
-			phylink_mac_link_up(pl, link_state);
+			phylink_link_up(pl, link_state);
 	}
 	if (!link_state.link && pl->mac_link_dropped) {
 		pl->mac_link_dropped = false;
@@ -746,6 +776,12 @@ struct phylink *phylink_create(struct phylink_config *config,
 }
 EXPORT_SYMBOL_GPL(phylink_create);
 
+void phylink_add_pcs(struct phylink *pl, const struct phylink_pcs_ops *ops)
+{
+	pl->pcs_ops = ops;
+}
+EXPORT_SYMBOL_GPL(phylink_add_pcs);
+
 /**
  * phylink_destroy() - cleanup and destroy the phylink instance
  * @pl: a pointer to a &struct phylink returned from phylink_create()
@@ -1082,14 +1118,12 @@ void phylink_start(struct phylink *pl)
 	/* Apply the link configuration to the MAC when starting. This allows
 	 * a fixed-link to start with the correct parameters, and also
 	 * ensures that we set the appropriate advertisement for Serdes links.
-	 */
-	phylink_mac_initial_config(pl);
-
-	/* Restart autonegotiation if using 802.3z to ensure that the link
+	 *
+	 * Restart autonegotiation if using 802.3z to ensure that the link
 	 * parameters are properly negotiated.  This is necessary for DSA
 	 * switches using 802.3z negotiation to ensure they see our modes.
 	 */
-	phylink_mac_an_restart(pl);
+	phylink_mac_initial_config(pl, true);
 
 	clear_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state);
 	phylink_run_resolve(pl);
@@ -1386,8 +1420,7 @@ int phylink_ethtool_ksettings_set(struct phylink *pl,
 			 * advertisement; the only thing we have is the pause
 			 * modes which can only come from a PHY.
 			 */
-			phylink_mac_config(pl, &pl->link_config);
-			phylink_mac_an_restart(pl);
+			phylink_pcs_config(pl, true, &pl->link_config);
 		}
 		mutex_unlock(&pl->state_mutex);
 	}
@@ -1415,7 +1448,7 @@ int phylink_ethtool_nway_reset(struct phylink *pl)
 
 	if (pl->phydev)
 		ret = phy_restart_aneg(pl->phydev);
-	phylink_mac_an_restart(pl);
+	phylink_mac_pcs_an_restart(pl);
 
 	return ret;
 }
@@ -1494,8 +1527,7 @@ int phylink_ethtool_set_pauseparam(struct phylink *pl,
 				   pause->tx_pause);
 	} else if (!test_bit(PHYLINK_DISABLE_STOPPED,
 			     &pl->phylink_disable_state)) {
-		phylink_mac_config(pl, &pl->link_config);
-		phylink_mac_an_restart(pl);
+		phylink_pcs_config(pl, true, &pl->link_config);
 	}
 	mutex_unlock(&pl->state_mutex);
 
@@ -1901,7 +1933,7 @@ static int phylink_sfp_config(struct phylink *pl, u8 mode,
 
 	if (changed && !test_bit(PHYLINK_DISABLE_STOPPED,
 				 &pl->phylink_disable_state))
-		phylink_mac_initial_config(pl);
+		phylink_mac_initial_config(pl, false);
 
 	return ret;
 }
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 8fa6df3b881b..dc27dd341ebd 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -97,6 +97,16 @@ struct phylink_mac_ops {
 			    bool tx_pause, bool rx_pause);
 };
 
+struct phylink_pcs_ops {
+	void (*pcs_get_state)(struct phylink_config *config,
+			      struct phylink_link_state *state);
+	int (*pcs_config)(struct phylink_config *config, unsigned int mode,
+			  const struct phylink_link_state *state);
+	void (*pcs_an_restart)(struct phylink_config *config);
+	void (*pcs_link_up)(struct phylink_config *config, unsigned int mode,
+			    phy_interface_t interface, int speed, int duplex);
+};
+
 #if 0 /* For kernel-doc purposes only. */
 /**
  * validate - Validate and update the link configuration
@@ -273,6 +283,7 @@ void mac_link_up(struct phylink_config *config, struct phy_device *phy,
 struct phylink *phylink_create(struct phylink_config *, struct fwnode_handle *,
 			       phy_interface_t iface,
 			       const struct phylink_mac_ops *ops);
+void phylink_add_pcs(struct phylink *, const struct phylink_pcs_ops *ops);
 void phylink_destroy(struct phylink *);
 
 int phylink_connect_phy(struct phylink *, struct phy_device *);
-- 
2.20.1

RE: [RFC net-next 2/5] net: phylink: add separate pcs operations structure

[Jose Abreu]

From: Russell King <rmk+kernel@armlinux.org.uk>
Date: Mar/17/2020, 14:52:51 (UTC+00:00)

> -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.

---
Thanks,
Jose Miguel Abreu

Re: [RFC net-next 2/5] net: phylink: add separate pcs operations structure

[Russell King - ARM Linux admin]

On Tue, Mar 17, 2020 at 03:48:50PM +0000, Jose Abreu wrote:
> From: Russell King <rmk+kernel@armlinux.org.uk>
> Date: Mar/17/2020, 14:52:51 (UTC+00:00)
> 
> > -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.

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.

> 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.

phylink doesn't have a state machine.  At what point do you think that
restarting negotiation (and which negotiation in the case of SGMII or
presumably USXGMII) would be appropriate?

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

RE: [RFC net-next 2/5] net: phylink: add separate pcs operations structure

[Jose Abreu]

From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Date: Mar/17/2020, 15:56:10 (UTC+00:00)

> > 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.

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.

---
Thanks,
Jose Miguel Abreu

Re: [RFC net-next 2/5] net: phylink: add separate pcs operations structure

[Russell King - ARM Linux admin]

On Tue, Mar 17, 2020 at 04:04:28PM +0000, Jose Abreu wrote:
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Date: Mar/17/2020, 15:56:10 (UTC+00:00)
> 
> > > 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.

> > 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.
> 
> 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

> 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.

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

RE: [RFC net-next 2/5] net: phylink: add separate pcs operations structure

[Jose Abreu]

From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Date: Mar/17/2020, 16:52:08 (UTC+00:00)

> On Tue, Mar 17, 2020 at 04:04:28PM +0000, Jose Abreu wrote:
> > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > Date: Mar/17/2020, 15:56:10 (UTC+00:00)
> > 
> > > > 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.
> 
> > > 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.
> > 
> > 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

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.

> 
> > 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.

Yes, I think it makes sense due to the setup I outlined above.

> 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.

Understood. Thanks for such thoughtfully explanation!

---
Thanks,
Jose Miguel Abreu

Re: [RFC net-next 2/5] net: phylink: add separate pcs operations structure

[Russell King - ARM Linux admin]

On Wed, Mar 18, 2020 at 07:45:52AM +0000, Jose Abreu wrote:
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Date: Mar/17/2020, 16:52:08 (UTC+00:00)
> 
> > On Tue, Mar 17, 2020 at 04:04:28PM +0000, Jose Abreu wrote:
> > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > > Date: Mar/17/2020, 15:56:10 (UTC+00:00)
> > > 
> > > > > 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.
> > 
> > > > 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.
> > > 
> > > 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
> 
> 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?

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

RE: [RFC net-next 2/5] net: phylink: add separate pcs operations structure

[Jose Abreu]

From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Date: Mar/19/2020, 11:14:25 (UTC+00:00)

> On Wed, Mar 18, 2020 at 07:45:52AM +0000, Jose Abreu wrote:
> > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > Date: Mar/17/2020, 16:52:08 (UTC+00:00)
> > 
> > > On Tue, Mar 17, 2020 at 04:04:28PM +0000, Jose Abreu wrote:
> > > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > > > Date: Mar/17/2020, 15:56:10 (UTC+00:00)
> > > > 
> > > > > > 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.
> > > 
> > > > > 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.
> > > > 
> > > > 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
> > 
> > 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?

No, I don't think so. I can check with HW team but I think its 10GKR (not 
sure though).

---
Thanks,
Jose Miguel Abreu

Re: [RFC net-next 2/5] net: phylink: add separate pcs operations structure

[Andrew Lunn]

On Tue, Mar 17, 2020 at 02:52:51PM +0000, Russell King wrote:
> *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.

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?

	Andrew

Re: [RFC net-next 2/5] net: phylink: add separate pcs operations structure

[Russell King - ARM Linux admin]

On Tue, Mar 17, 2020 at 05:38:02PM +0100, Andrew Lunn wrote:
> On Tue, Mar 17, 2020 at 02:52:51PM +0000, Russell King wrote:
> > *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.
> 
> 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.

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

Re: [RFC net-next 2/5] net: phylink: add separate pcs operations structure

[Russell King - ARM Linux admin]

On Tue, Mar 17, 2020 at 04:54:22PM +0000, Russell King - ARM Linux admin wrote:
> On Tue, Mar 17, 2020 at 05:38:02PM +0100, Andrew Lunn wrote:
> > On Tue, Mar 17, 2020 at 02:52:51PM +0000, Russell King wrote:
> > > *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.
> > 
> > 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".

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

Re: [RFC net-next 2/5] net: phylink: add separate pcs operations structure

[Andrew Lunn]

> 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.

     Andrew

Re: [RFC net-next 2/5] net: phylink: add separate pcs operations structure

[Russell King - ARM Linux admin]

On Thu, Mar 19, 2020 at 04:06:52PM +0100, Andrew Lunn wrote:
> 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.

I'll dig into the 6390X datasheet to see how that works, thanks for
the info.

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

Re: [RFC net-next 2/5] net: phylink: add separate pcs operations structure

[Russell King - ARM Linux admin]

On Thu, Mar 19, 2020 at 04:06:52PM +0100, Andrew Lunn wrote:
> > 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.

Looking at the 6390X functional specifications, it looks like it will
be rather simple:

In mv88e6390_serdes_pcs_get_state(), we detect if state->interface is
10GBASER (or 10GBASEX2/X4 if we ever support those), and read from the
10G PCS offset.  We only need to be concerned with the link up/down
status there as there is no negotiation present - however, state->speed
and state->duplex still need to be set.

mv88e6390_serdes_pcs_an_restart() won't do anything for 10G speeds -
there is no autonegotiation to restart, and at the moment phylink will
not call that function anyway for 10GBASE* anyway.

mv88e6390_serdes_pcs_link_up() needs a bit of rework to pass the
interface to it, and ignore 10GBASE* modes altogether, rather than
trying to fiddle with the not-currently-in-use 1G PCS.

However, what I can say is that on the ZII rev C, it seems to sort-of
work for the 3310 PHY.  The PHY XS (operating in XAUI mode) reports
that all four lanes are synchronised and the link is established
with the switch.  The switch reports 0x94c in the port status register,
indicating that it's in 10G XAUI mode, link up, but 100M/Half - which
is where things get odd - the 3310 seems not to negotiate correctly,
the copper side is operating at 100M/Half at both ends as reported by
the PHYs at both ends - it looks like the advertisement is not being
sent or received correctly.  The other complexity here is that the
3310 on the ZII rev C is in "XAUI with rate adaption" mode and we
have no support for such a thing.  Consequently, I can't pass traffic
over the link.

So, as far as I can tell, apart from the modifications to the
mv88e6390_serdes_*() functions above, it should be mostly in a
working state.

However, what I said previously applies: the 6390X 1G/10G PCS are
"vendor specific" - similar to the 3310 PHY with its multiple
sub-PHYs at various register offsets, the 6390X PCS are accessed
through the PHYXS MMD rather than the PCS MMD.  The 1G PHY has
a clause 22 compatible register layout, but is in the PHYXS MMD
at offset 0x2000.  The 10G PHY has a clause 45 PCS compatible
register layout, but is in the PHYXS MMD at offset 0x1000.

So, generic code isn't going to be able to access those, just like
we can't use generic phylib code to access the offset PHY blocks
in the 3310.

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

Re: [RFC net-next 2/5] net: phylink: add separate pcs operations structure

[Russell King - ARM Linux admin]

[-- Attachment #1: Type: text/plain, Size: 4020 bytes --]

On Thu, Mar 19, 2020 at 04:06:52PM +0100, Andrew Lunn wrote:
> > 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.

This is what I came up with, but I'm not really able to test it with
my ZII platforms. See the attached patch and the patch below. I
haven't polished them yet - been a little otherwise occupied as I'm
sure you can imagine given the situation.

diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 8cef46ee1d2f..59bd3f447386 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -631,8 +631,8 @@ int mv88e6390_serdes_pcs_config(struct mv88e6xxx_chip *chip, int port,
 				      MV88E6390_SGMII_BMCR, bmcr);
 }
 
-int mv88e6390_serdes_pcs_get_state(struct mv88e6xxx_chip *chip, int port,
-				   u8 lane, struct phylink_link_state *state)
+static int mv88e6390_serdes_pcs_get_state_sgmii(struct mv88e6xxx_chip *chip,
+	int port, u8 lane, struct phylink_link_state *state)
 {
 	u16 lpa, status;
 	int err;
@@ -654,6 +654,45 @@ int mv88e6390_serdes_pcs_get_state(struct mv88e6xxx_chip *chip, int port,
 	return mv88e6xxx_serdes_pcs_get_state(chip, status, lpa, state);
 }
 
+static int mv88e6390_serdes_pcs_get_state_10g(struct mv88e6xxx_chip *chip,
+	int port, u8 lane, struct phylink_link_state *state)
+{
+	u16 status;
+	int err;
+
+	err = mv88e6390_serdes_read(chip, lane, MDIO_MMD_PHYXS,
+				    MV88E6390_10G_STAT1, &status);
+	if (err)
+		return err;
+
+	state->link = !!(status & MDIO_STAT1_LSTATUS);
+	if (state->link) {
+		state->speed = SPEED_10000;
+		state->duplex = DUPLEX_FULL;
+	}
+
+	return 0;
+}
+
+int mv88e6390_serdes_pcs_get_state(struct mv88e6xxx_chip *chip, int port,
+				   u8 lane, struct phylink_link_state *state)
+{
+	switch (state->interface) {
+	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_1000BASEX:
+	case PHY_INTERFACE_MODE_2500BASEX:
+		return mv88e6390_serdes_pcs_get_state_sgmii(chip, port, lane,
+							    state);
+	case PHY_INTERFACE_MODE_XAUI:
+	case PHY_INTERFACE_MODE_RXAUI:
+		return mv88e6390_serdes_pcs_get_state_10g(chip, port, lane,
+							  state);
+
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 int mv88e6390_serdes_pcs_an_restart(struct mv88e6xxx_chip *chip, int port,
 				    u8 lane)
 {
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.h b/drivers/net/dsa/mv88e6xxx/serdes.h
index 12013f22df10..3af236f4b3c8 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.h
+++ b/drivers/net/dsa/mv88e6xxx/serdes.h
@@ -41,6 +41,7 @@
 
 /* 10GBASE-R and 10GBASE-X4/X2 */
 #define MV88E6390_10G_CTRL1		(0x1000 + MDIO_CTRL1)
+#define MV88E6390_10G_STAT1		(0x1000 + MDIO_STAT1)
 
 /* 1000BASE-X and SGMII */
 #define MV88E6390_SGMII_BMCR		(0x2000 + MII_BMCR)

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

[-- Attachment #2: 0001-net-dsa-mv88e6xxx-use-generic-clause-45-definitions.patch --]
[-- Type: text/x-diff, Size: 2031 bytes --]

From: Russell King <rmk+kernel@armlinux.org.uk>
Subject: [PATCH] net: dsa: mv88e6xxx: use generic clause 45 definitions

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/mv88e6xxx/serdes.c | 12 ++++++------
 drivers/net/dsa/mv88e6xxx/serdes.h |  6 +-----
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 0a88b416ef9f..8cef46ee1d2f 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -511,21 +511,21 @@ static int mv88e6390_serdes_power_10g(struct mv88e6xxx_chip *chip, u8 lane,
 	int err;
 
 	err = mv88e6390_serdes_read(chip, lane, MDIO_MMD_PHYXS,
-				    MV88E6390_PCS_CONTROL_1, &val);
+				    MV88E6390_10G_CTRL1, &val);
 
 	if (err)
 		return err;
 
 	if (up)
-		new_val = val & ~(MV88E6390_PCS_CONTROL_1_RESET |
-				  MV88E6390_PCS_CONTROL_1_LOOPBACK |
-				  MV88E6390_PCS_CONTROL_1_PDOWN);
+		new_val = val & ~(MDIO_CTRL1_RESET |
+				  MDIO_PCS_CTRL1_LOOPBACK |
+				  MDIO_CTRL1_LPOWER);
 	else
-		new_val = val | MV88E6390_PCS_CONTROL_1_PDOWN;
+		new_val = val | MDIO_CTRL1_LPOWER;
 
 	if (val != new_val)
 		err = mv88e6390_serdes_write(chip, lane, MDIO_MMD_PHYXS,
-					     MV88E6390_PCS_CONTROL_1, new_val);
+					     MV88E6390_10G_CTRL1, new_val);
 
 	return err;
 }
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.h b/drivers/net/dsa/mv88e6xxx/serdes.h
index c92f494a276e..12013f22df10 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.h
+++ b/drivers/net/dsa/mv88e6xxx/serdes.h
@@ -40,11 +40,7 @@
 #define MV88E6390_PORT10_LANE3		0x17
 
 /* 10GBASE-R and 10GBASE-X4/X2 */
-#define MV88E6390_PCS_CONTROL_1		0x1000
-#define MV88E6390_PCS_CONTROL_1_RESET		BIT(15)
-#define MV88E6390_PCS_CONTROL_1_LOOPBACK	BIT(14)
-#define MV88E6390_PCS_CONTROL_1_SPEED		BIT(13)
-#define MV88E6390_PCS_CONTROL_1_PDOWN		BIT(11)
+#define MV88E6390_10G_CTRL1		(0x1000 + MDIO_CTRL1)
 
 /* 1000BASE-X and SGMII */
 #define MV88E6390_SGMII_BMCR		(0x2000 + MII_BMCR)
-- 
2.20.1

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

[Russell King]

*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 <rmk+kernel@armlinux.org.uk>
---
 .../arm64/boot/dts/freescale/fsl-lx2160a.dtsi | 144 ++++++++++++++++++
 1 file changed, 144 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
index e5ee5591e52b..732af33eec18 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
@@ -960,6 +960,132 @@
 			status = "disabled";
 		};
 
+		pcs_mdio1: mdio@8c07000 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c07000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
+		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 {
+			compatible = "fsl,fman-memac-mdio";
+			reg = <0x0 0x8c4b000 0x0 0x1000>;
+			little-endian;
+			status = "disabled";
+		};
+
 		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

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

[Ioana Ciornei]

> 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 <rmk+kernel@armlinux.org.uk>
> ---
>  .../arm64/boot/dts/freescale/fsl-lx2160a.dtsi | 144 ++++++++++++++++++
>  1 file changed, 144 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> index e5ee5591e52b..732af33eec18 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> @@ -960,6 +960,132 @@
>  			status = "disabled";
>  		};
> 
> +		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.

> +
> +		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 {
> +			compatible = "fsl,fman-memac-mdio";
> +			reg = <0x0 0x8c4b000 0x0 0x1000>;
> +			little-endian;
> +			status = "disabled";
> +		};
> +

Please sort the nodes alphabetically.

>  		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

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

[Russell King - ARM Linux admin]

On Thu, Mar 26, 2020 at 09:14:13PM +0000, Ioana Ciornei wrote:
> > 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 <rmk+kernel@armlinux.org.uk>
> > ---
> >  .../arm64/boot/dts/freescale/fsl-lx2160a.dtsi | 144 ++++++++++++++++++
> >  1 file changed, 144 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > index e5ee5591e52b..732af33eec18 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > @@ -960,6 +960,132 @@
> >  			status = "disabled";
> >  		};
> > 
> > +		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.

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.

> > +
> > +		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 {
> > +			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.

> >  		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
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

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

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

[Ioana Ciornei]

> Subject: Re: [RFC net-next 3/5] arm64: dts: lx2160a: add PCS MDIO nodes
> 
> On Thu, Mar 26, 2020 at 09:14:13PM +0000, Ioana Ciornei wrote:
> > > 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 <rmk+kernel@armlinux.org.uk>
> > > ---
> > >  .../arm64/boot/dts/freescale/fsl-lx2160a.dtsi | 144
> > > ++++++++++++++++++
> > >  1 file changed, 144 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > index e5ee5591e52b..732af33eec18 100644
> > > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > @@ -960,6 +960,132 @@
> > >  			status = "disabled";
> > >  		};
> > >
> > > +		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.
> 
> 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.

Sorry for misplacing the comment.

I am going to take a look tomorrow and see how workable this approach is going to be in the long term since I have a board with QSGMII handy.


> 
> > > +
> > > +		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 {
> > > +			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.

Uhh, I remember now. For some reason I thought this was a board file.

Ioana

[snip]

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

[Russell King]

*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 <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  | 206 +++++++++++++++++-
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.h  |   1 +
 2 files changed, 204 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
index 3ee236c5fc37..e7b2dc366338 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
@@ -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);
+}
+
+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;
 	}
@@ -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:
 	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:
 		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,
+			    struct device_node *dpmac_node, int id)
+{
+	struct mdio_device *mdiodev;
+	struct device_node *node;
+	struct mii_bus *bus;
+	int err;
+
+	node = of_parse_phandle(dpmac_node, "pcs-mdio", 0);
+	if (!node) {
+		/* allow old DT files to work */
+		netdev_warn(mac->netdev, "pcs-mdio node not found\n");
+		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)
 	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);
 }
 
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
index 2130d9c7d40e..5cfae5f8f55e 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
@@ -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

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

[Ioana Ciornei]

> 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 <rmk+kernel@armlinux.org.uk>
> ---
>  .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  | 206 +++++++++++++++++-
>  .../net/ethernet/freescale/dpaa2/dpaa2-mac.h  |   1 +
>  2 files changed, 204 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> index 3ee236c5fc37..e7b2dc366338 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> @@ -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); }
> +
> +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;
>  	}
> @@ -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:
>  	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:
>  		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,
> +			    struct device_node *dpmac_node, int id) {
> +	struct mdio_device *mdiodev;
> +	struct device_node *node;
> +	struct mii_bus *bus;
> +	int err;
> +
> +	node = of_parse_phandle(dpmac_node, "pcs-mdio", 0);
> +	if (!node) {
> +		/* allow old DT files to work */
> +		netdev_warn(mac->netdev, "pcs-mdio node not found\n");

s/mac->netdev/mac->net_dev/

> +		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)
>  	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);  }
> 
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> index 2130d9c7d40e..5cfae5f8f55e 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> @@ -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

[RFC net-next 5/5] dpaa2-mac: add 10GBASE-R PCS support

[Russell King]

*NOT FOR MERGING*

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
index e7b2dc366338..38f8d31bf426 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
@@ -27,6 +27,10 @@ static void dpaa2_mac_pcs_get_state(struct phylink_config *config,
 		phylink_mii_c22_pcs_get_state(pcs, state);
 		break;
 
+	case PHY_INTERFACE_MODE_10GBASER:
+		phylink_mii_c45_pcs_get_state(pcs, state);
+		break;
+
 	default:
 		break;
 	}
@@ -131,6 +135,10 @@ static int phy_mode(enum dpmac_eth_if eth_if, phy_interface_t *if_mode)
 		*if_mode = PHY_INTERFACE_MODE_SGMII;
 		break;
 
+	case DPMAC_ETH_IF_XFI:
+		*if_mode = PHY_INTERFACE_MODE_10GBASER;
+		break;
+
 	default:
 		return -EINVAL;
 	}
@@ -191,6 +199,7 @@ static bool dpaa2_mac_phy_mode_mismatch(struct dpaa2_mac *mac,
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
+	case PHY_INTERFACE_MODE_10GBASER:
 		return (interface != mac->if_mode);
 	default:
 		return true;
@@ -216,6 +225,17 @@ static void dpaa2_mac_validate(struct phylink_config *config,
 
 	switch (state->interface) {
 	case PHY_INTERFACE_MODE_NA:
+	case PHY_INTERFACE_MODE_10GBASER:
+		phylink_set(mask, 10000baseT_Full);
+		phylink_set(mask, 10000baseKR_Full);
+		phylink_set(mask, 10000baseCR_Full);
+		phylink_set(mask, 10000baseSR_Full);
+		phylink_set(mask, 10000baseLR_Full);
+		phylink_set(mask, 10000baseLRM_Full);
+		phylink_set(mask, 10000baseER_Full);
+		if (state->interface != PHY_INTERFACE_MODE_NA)
+			break;
+		/* fallthrough */
 	case PHY_INTERFACE_MODE_1000BASEX:
 	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_RGMII:
-- 
2.20.1

Re: [RFC net-next 0/2] split phylink PCS operations and add PCS support for dpaa2

[Russell King - ARM Linux admin]

Hi,

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

The discussion around patch 2 seems to have tailed off, and no one
seems to have replied to patches 3 to 5.

Thanks.

On Tue, Mar 17, 2020 at 02:49:44PM +0000, Russell King - ARM Linux admin wrote:
> 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(-)
> 
> -- 
> 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
> 

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

Re: [RFC net-next 0/2] split phylink PCS operations and add PCS support for dpaa2

[Andrew Lunn]

On Thu, Mar 26, 2020 at 02:57:50PM +0000, Russell King - ARM Linux admin wrote:
> Hi,
> 
> Was there any conclusion on this 5 patch series, and whether I should
> submit it for net-next?

Hi Russell

The basic idea seems sound. So i suggest re-submitting without the RFC
tag, and let people comment on it again.

     Andrew

[PATCH net-next 0/2] split phylink PCS operations and add PCS support for dpaa2

[Russell King - ARM Linux admin]

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.

 drivers/net/phy/phylink.c | 102 ++++++++++++++++++++++++++++++----------------
 include/linux/phylink.h   |  11 +++++
 2 files changed, 78 insertions(+), 35 deletions(-)

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

Re: [PATCH net-next 0/2] split phylink PCS operations and add PCS support for dpaa2

[David Miller]

From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Date: Thu, 26 Mar 2020 15:14:04 +0000

> 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.

Re: [PATCH net-next 0/2] split phylink PCS operations and add PCS support for dpaa2

[Russell King - ARM Linux admin]

On Sun, Mar 29, 2020 at 09:57:03PM -0700, David Miller wrote:
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Date: Thu, 26 Mar 2020 15:14:04 +0000
> 
> > 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.

FYI, it's been through two more respins already, this series has
already been superseded.  The most recent series is:

"[PATCH net-next v2 0/3] split phylink PCS operations and add PCS
 support for dpaa2"

which I still need to fix the cover message subject line.

Thanks.

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