net: dsa: felix: use resolved link config in mac_link_up()

Phylink now requires that parameters established through
auto-negotiation be written into the MAC at the time of the
mac_link_up() callback. In the case of felix, that means taking the port
out of reset, setting the correct timers for PAUSE frames, and
enabling/disabling TX flow control.

This patch also splits the inband and noinband configuration of the
vsc9959 PCS (currently found in a function called "init") into 2
different functions, which have a nomenclature closer to phylink:
"config", for inband setup, and "link_up", for noinband (forced) setup.

This is necessary as a preparation step for giving up control of the PCS
to phylink, which will be done in further patch series.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

drivers/net/dsa/ocelot/felix.c

@@ -233,6 +233,32 @@ static int felix_phylink_mac_pcs_get_state(struct dsa_switch *ds, int port,
 static void felix_phylink_mac_config(struct dsa_switch *ds, int port,
 				     unsigned int link_an_mode,
 				     const struct phylink_link_state *state)
+{
+	struct ocelot *ocelot = ds->priv;
+	struct felix *felix = ocelot_to_felix(ocelot);
+
+	if (felix->info->pcs_config)
+		felix->info->pcs_config(ocelot, port, link_an_mode, state);
+}
+
+static void felix_phylink_mac_link_down(struct dsa_switch *ds, int port,
+					unsigned int link_an_mode,
+					phy_interface_t interface)
+{
+	struct ocelot *ocelot = ds->priv;
+	struct ocelot_port *ocelot_port = ocelot->ports[port];
+
+	ocelot_port_writel(ocelot_port, 0, DEV_MAC_ENA_CFG);
+	ocelot_rmw_rix(ocelot, 0, QSYS_SWITCH_PORT_MODE_PORT_ENA,
+		       QSYS_SWITCH_PORT_MODE, port);
+}
+
+static void felix_phylink_mac_link_up(struct dsa_switch *ds, int port,
+				      unsigned int link_an_mode,
+				      phy_interface_t interface,
+				      struct phy_device *phydev,
+				      int speed, int duplex,
+				      bool tx_pause, bool rx_pause)
 {
 	struct ocelot *ocelot = ds->priv;
 	struct ocelot_port *ocelot_port = ocelot->ports[port];
@@ -250,7 +276,7 @@ static void felix_phylink_mac_config(struct dsa_switch *ds, int port,
 			   DEV_CLOCK_CFG_LINK_SPEED(OCELOT_SPEED_1000),
 			   DEV_CLOCK_CFG);
 
-	switch (state->speed) {
+	switch (speed) {
 	case SPEED_10:
 		mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(3);
 		break;
@@ -261,12 +287,9 @@ static void felix_phylink_mac_config(struct dsa_switch *ds, int port,
 	case SPEED_2500:
 		mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(1);
 		break;
-	case SPEED_UNKNOWN:
-		mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(0);
-		break;
 	default:
 		dev_err(ocelot->dev, "Unsupported speed on port %d: %d\n",
-			port, state->speed);
+			port, speed);
 		return;
 	}
 
@@ -275,7 +298,7 @@ static void felix_phylink_mac_config(struct dsa_switch *ds, int port,
 	 */
 	mac_fc_cfg |= SYS_MAC_FC_CFG_RX_FC_ENA;
 
-	if (state->pause & MLO_PAUSE_TX)
+	if (tx_pause)
 		mac_fc_cfg |= SYS_MAC_FC_CFG_TX_FC_ENA |
 			      SYS_MAC_FC_CFG_PAUSE_VAL_CFG(0xffff) |
 			      SYS_MAC_FC_CFG_FC_LATENCY_CFG(0x7) |
@@ -288,37 +311,9 @@ static void felix_phylink_mac_config(struct dsa_switch *ds, int port,
 
 	ocelot_write_rix(ocelot, 0, ANA_POL_FLOWC, port);
 
-	if (felix->info->pcs_init)
-		felix->info->pcs_init(ocelot, port, link_an_mode, state);
-
-	if (felix->info->port_sched_speed_set)
-		felix->info->port_sched_speed_set(ocelot, port,
-						  state->speed);
-}
-
-static void felix_phylink_mac_link_down(struct dsa_switch *ds, int port,
-					unsigned int link_an_mode,
-					phy_interface_t interface)
-{
-	struct ocelot *ocelot = ds->priv;
-	struct ocelot_port *ocelot_port = ocelot->ports[port];
-
-	ocelot_port_writel(ocelot_port, 0, DEV_MAC_ENA_CFG);
-	ocelot_rmw_rix(ocelot, 0, QSYS_SWITCH_PORT_MODE_PORT_ENA,
-		       QSYS_SWITCH_PORT_MODE, port);
-}
-
-static void felix_phylink_mac_link_up(struct dsa_switch *ds, int port,
-				      unsigned int link_an_mode,
-				      phy_interface_t interface,
-				      struct phy_device *phydev,
-				      int speed, int duplex,
-				      bool tx_pause, bool rx_pause)
-{
-	struct ocelot *ocelot = ds->priv;
-	struct ocelot_port *ocelot_port = ocelot->ports[port];
-
-	/* Enable MAC module */
+	/* Undo the effects of felix_phylink_mac_link_down:
+	 * enable MAC module
+	 */
 	ocelot_port_writel(ocelot_port, DEV_MAC_ENA_CFG_RX_ENA |
 			   DEV_MAC_ENA_CFG_TX_ENA, DEV_MAC_ENA_CFG);
 
@@ -335,6 +330,13 @@ static void felix_phylink_mac_link_up(struct dsa_switch *ds, int port,
 			 QSYS_SWITCH_PORT_MODE_SCH_NEXT_CFG(1) |
 			 QSYS_SWITCH_PORT_MODE_PORT_ENA,
 			 QSYS_SWITCH_PORT_MODE, port);
+
+	if (felix->info->pcs_link_up)
+		felix->info->pcs_link_up(ocelot, port, link_an_mode, interface,
+					 speed, duplex);
+
+	if (felix->info->port_sched_speed_set)
+		felix->info->port_sched_speed_set(ocelot, port, speed);
 }
 
 static void felix_port_qos_map_init(struct ocelot *ocelot, int port)

drivers/net/dsa/ocelot/felix.h

@@ -28,9 +28,13 @@ struct felix_info {
 	int				imdio_pci_bar;
 	int	(*mdio_bus_alloc)(struct ocelot *ocelot);
 	void	(*mdio_bus_free)(struct ocelot *ocelot);
-	void	(*pcs_init)(struct ocelot *ocelot, int port,
+	void	(*pcs_config)(struct ocelot *ocelot, int port,
 			      unsigned int link_an_mode,
 			      const struct phylink_link_state *state);
+	void	(*pcs_link_up)(struct ocelot *ocelot, int port,
+			       unsigned int link_an_mode,
+			       phy_interface_t interface,
+			       int speed, int duplex);
 	void	(*pcs_link_state)(struct ocelot *ocelot, int port,
 				  struct phylink_link_state *state);
 	int	(*prevalidate_phy_mode)(struct ocelot *ocelot, int port,

drivers/net/dsa/ocelot/felix_vsc9959.c

@@ -737,11 +737,10 @@ static int vsc9959_reset(struct ocelot *ocelot)
  * traffic if SGMII AN is enabled but not completed (acknowledged by us), so
  * setting MLO_AN_INBAND is actually required for those.
  */
-static void vsc9959_pcs_init_sgmii(struct phy_device *pcs,
+static void vsc9959_pcs_config_sgmii(struct phy_device *pcs,
 				     unsigned int link_an_mode,
 				     const struct phylink_link_state *state)
 {
-	if (link_an_mode == MLO_AN_INBAND) {
 	int bmsr, bmcr;
 
 	/* Some PHYs like VSC8234 don't like it when AN restarts on
@@ -780,34 +779,101 @@ static void vsc9959_pcs_init_sgmii(struct phy_device *pcs,
 		  ENETC_PCS_LINK_TIMER2_VAL);
 
 	phy_set_bits(pcs, MII_BMCR, BMCR_ANENABLE);
-	} else {
+}
+
+static void vsc9959_pcs_config_usxgmii(struct phy_device *pcs,
+				       unsigned int link_an_mode,
+				       const struct phylink_link_state *state)
+{
+	/* Configure device ability for the USXGMII Replicator */
+	phy_write_mmd(pcs, MDIO_MMD_VEND2, MII_ADVERTISE,
+		      USXGMII_ADVERTISE_SPEED(USXGMII_SPEED_2500) |
+		      USXGMII_ADVERTISE_LNKS(1) |
+		      ADVERTISE_SGMII |
+		      ADVERTISE_LPACK |
+		      USXGMII_ADVERTISE_FDX);
+}
+
+static void vsc9959_pcs_config(struct ocelot *ocelot, int port,
+			       unsigned int link_an_mode,
+			       const struct phylink_link_state *state)
+{
+	struct felix *felix = ocelot_to_felix(ocelot);
+	struct phy_device *pcs = felix->pcs[port];
+
+	if (!pcs)
+		return;
+
+	/* The PCS does not implement the BMSR register fully, so capability
+	 * detection via genphy_read_abilities does not work. Since we can get
+	 * the PHY config word from the LPA register though, there is still
+	 * value in using the generic phy_resolve_aneg_linkmode function. So
+	 * populate the supported and advertising link modes manually here.
+	 */
+	linkmode_set_bit_array(phy_basic_ports_array,
+			       ARRAY_SIZE(phy_basic_ports_array),
+			       pcs->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, pcs->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, pcs->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, pcs->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, pcs->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, pcs->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, pcs->supported);
+	if (pcs->interface == PHY_INTERFACE_MODE_2500BASEX ||
+	    pcs->interface == PHY_INTERFACE_MODE_USXGMII)
+		linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseX_Full_BIT,
+				 pcs->supported);
+	if (pcs->interface != PHY_INTERFACE_MODE_2500BASEX)
+		linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
+				 pcs->supported);
+	phy_advertise_supported(pcs);
+
+	if (!phylink_autoneg_inband(link_an_mode))
+		return;
+
+	switch (pcs->interface) {
+	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_QSGMII:
+		vsc9959_pcs_config_sgmii(pcs, link_an_mode, state);
+		break;
+	case PHY_INTERFACE_MODE_2500BASEX:
+		phydev_err(pcs, "AN not supported on 3.125GHz SerDes lane\n");
+		break;
+	case PHY_INTERFACE_MODE_USXGMII:
+		vsc9959_pcs_config_usxgmii(pcs, link_an_mode, state);
+		break;
+	default:
+		dev_err(ocelot->dev, "Unsupported link mode %s\n",
+			phy_modes(pcs->interface));
+	}
+}
+
+static void vsc9959_pcs_link_up_sgmii(struct phy_device *pcs,
+				      unsigned int link_an_mode,
+				      int speed, int duplex)
+{
 	u16 if_mode = ENETC_PCS_IF_MODE_SGMII_EN;
-		int speed;
 
-		switch (state->speed) {
+	switch (speed) {
 	case SPEED_1000:
-			speed = ENETC_PCS_SPEED_1000;
+		if_mode |= ENETC_PCS_IF_MODE_SGMII_SPEED(ENETC_PCS_SPEED_1000);
 		break;
 	case SPEED_100:
-			speed = ENETC_PCS_SPEED_100;
+		if_mode |= ENETC_PCS_IF_MODE_SGMII_SPEED(ENETC_PCS_SPEED_100);
 		break;
 	case SPEED_10:
-			speed = ENETC_PCS_SPEED_10;
+		if_mode |= ENETC_PCS_IF_MODE_SGMII_SPEED(ENETC_PCS_SPEED_10);
 		break;
-		case SPEED_UNKNOWN:
-			/* Silently don't do anything */
-			return;
 	default:
-			phydev_err(pcs, "Invalid PCS speed %d\n", state->speed);
+		phydev_err(pcs, "Invalid PCS speed %d\n", speed);
 		return;
 	}
 
-		if_mode |= ENETC_PCS_IF_MODE_SGMII_SPEED(speed);
-		if (state->duplex == DUPLEX_HALF)
+	if (duplex == DUPLEX_HALF)
 		if_mode |= ENETC_PCS_IF_MODE_DUPLEX_HALF;
 
+	phy_write(pcs, ENETC_PCS_IF_MODE, if_mode);
 	phy_clear_bits(pcs, MII_BMCR, BMCR_ANENABLE);
-	}
 }
 
 /* 2500Base-X is SerDes protocol 7 on Felix and 6 on ENETC. It is a SerDes lane
@@ -827,46 +893,24 @@ static void vsc9959_pcs_init_sgmii(struct phy_device *pcs,
  * lower link speed on line side, the system-side interface remains fixed at
  * 2500 Mbps and we do rate adaptation through pause frames.
  */
-static void vsc9959_pcs_init_2500basex(struct phy_device *pcs,
+static void vsc9959_pcs_link_up_2500basex(struct phy_device *pcs,
 					  unsigned int link_an_mode,
-				       const struct phylink_link_state *state)
+					  int speed, int duplex)
 {
 	u16 if_mode = ENETC_PCS_IF_MODE_SGMII_SPEED(ENETC_PCS_SPEED_2500) |
 		      ENETC_PCS_IF_MODE_SGMII_EN;
 
-	if (link_an_mode == MLO_AN_INBAND) {
-		phydev_err(pcs, "AN not supported on 3.125GHz SerDes lane\n");
-		return;
-	}
-
-	if (state->duplex == DUPLEX_HALF)
+	if (duplex == DUPLEX_HALF)
 		if_mode |= ENETC_PCS_IF_MODE_DUPLEX_HALF;
 
 	phy_write(pcs, ENETC_PCS_IF_MODE, if_mode);
 	phy_clear_bits(pcs, MII_BMCR, BMCR_ANENABLE);
 }
 
-static void vsc9959_pcs_init_usxgmii(struct phy_device *pcs,
-				     unsigned int link_an_mode,
-				     const struct phylink_link_state *state)
-{
-	if (link_an_mode != MLO_AN_INBAND) {
-		phydev_err(pcs, "USXGMII only supports in-band AN for now\n");
-		return;
-	}
-
-	/* Configure device ability for the USXGMII Replicator */
-	phy_write_mmd(pcs, MDIO_MMD_VEND2, MII_ADVERTISE,
-		      USXGMII_ADVERTISE_SPEED(USXGMII_SPEED_2500) |
-		      USXGMII_ADVERTISE_LNKS(1) |
-		      ADVERTISE_SGMII |
-		      ADVERTISE_LPACK |
-		      USXGMII_ADVERTISE_FDX);
-}
-
-static void vsc9959_pcs_init(struct ocelot *ocelot, int port,
+static void vsc9959_pcs_link_up(struct ocelot *ocelot, int port,
 				unsigned int link_an_mode,
-			     const struct phylink_link_state *state)
+				phy_interface_t interface,
+				int speed, int duplex)
 {
 	struct felix *felix = ocelot_to_felix(ocelot);
 	struct phy_device *pcs = felix->pcs[port];
@@ -874,40 +918,20 @@ static void vsc9959_pcs_init(struct ocelot *ocelot, int port,
 	if (!pcs)
 		return;
 
-	/* The PCS does not implement the BMSR register fully, so capability
-	 * detection via genphy_read_abilities does not work. Since we can get
-	 * the PHY config word from the LPA register though, there is still
-	 * value in using the generic phy_resolve_aneg_linkmode function. So
-	 * populate the supported and advertising link modes manually here.
-	 */
-	linkmode_set_bit_array(phy_basic_ports_array,
-			       ARRAY_SIZE(phy_basic_ports_array),
-			       pcs->supported);
-	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, pcs->supported);
-	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, pcs->supported);
-	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, pcs->supported);
-	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, pcs->supported);
-	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, pcs->supported);
-	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, pcs->supported);
-	if (pcs->interface == PHY_INTERFACE_MODE_2500BASEX ||
-	    pcs->interface == PHY_INTERFACE_MODE_USXGMII)
-		linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseX_Full_BIT,
-				 pcs->supported);
-	if (pcs->interface != PHY_INTERFACE_MODE_2500BASEX)
-		linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
-				 pcs->supported);
-	phy_advertise_supported(pcs);
+	if (phylink_autoneg_inband(link_an_mode))
+		return;
 
-	switch (pcs->interface) {
+	switch (interface) {
 	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_QSGMII:
-		vsc9959_pcs_init_sgmii(pcs, link_an_mode, state);
+		vsc9959_pcs_link_up_sgmii(pcs, link_an_mode, speed, duplex);
 		break;
 	case PHY_INTERFACE_MODE_2500BASEX:
-		vsc9959_pcs_init_2500basex(pcs, link_an_mode, state);
+		vsc9959_pcs_link_up_2500basex(pcs, link_an_mode, speed,
+					      duplex);
 		break;
 	case PHY_INTERFACE_MODE_USXGMII:
-		vsc9959_pcs_init_usxgmii(pcs, link_an_mode, state);
+		phydev_err(pcs, "USXGMII only supports in-band AN for now\n");
 		break;
 	default:
 		dev_err(ocelot->dev, "Unsupported link mode %s\n",
@@ -1374,7 +1398,8 @@ struct felix_info felix_info_vsc9959 = {
 	.imdio_pci_bar		= 0,
 	.mdio_bus_alloc		= vsc9959_mdio_bus_alloc,
 	.mdio_bus_free		= vsc9959_mdio_bus_free,
-	.pcs_init		= vsc9959_pcs_init,
+	.pcs_config		= vsc9959_pcs_config,
+	.pcs_link_up		= vsc9959_pcs_link_up,
 	.pcs_link_state		= vsc9959_pcs_link_state,
 	.prevalidate_phy_mode	= vsc9959_prevalidate_phy_mode,
 	.port_setup_tc          = vsc9959_port_setup_tc,