[PATCH net-next 0/4] net: dsa: Plug in PHYLINK support

[PATCH net-next 0/4] net: dsa: Plug in PHYLINK support

[Florian Fainelli]

Hi all,

This patch series adds PHYLINK support to DSA which is necessary to support more
complex PHY and pluggable modules setups.

Patch series can be found here:

https://github.com/ffainelli/linux/commits/dsa-phylink

This was tested on:

- dsa-loop
- bcm_sf2
- mv88e6xxx
- b53

With a variety of test cases:
- internal & external MDIO PHYs
- MoCA with link notification through interrupt/MMIO register
- built-in PHYs
- ifconfig up/down for several cycles works
- bind/unbind of the drivers

And everything should still work as expected. Please be aware of the following:

- switch drivers (like bcm_sf2) which may have user-facing network ports using
  fixed links would need to implement phylink_mac_ops to remain functional.
  PHYLINK does not create a phy_device for fixed links, therefore our
  call to adjust_link() from phylink_mac_link_{up,down} would not be calling
  into the driver. This *should not* affect CPU/DSA ports which are configured
  through adjust_link() but have no network devices

- support for SFP/SFF is now possible, but switch drivers will still need some
  modifications to properly support those, including, but not limited to using
  the correct binding information. This will be submitted on top of this series

Russell, we could theoretically eliminate patch 3 and resolve this within DSA
entirely by keeping a per-port phy_interface_t (we did that before), this is
not a big change if we have to, let me know if you feel like this is cleaner. I
was initially considering passing a phylink_link_state reference to
mac_link_{up,down} but only a couple of fields are valid during link_down and
ended up with passing the phy_interface_t value we need instead. This is
necessary for switch drivers which have different types of port interfaces (see
bcm_sf2 documentation in tree).

Thank you!

Florian Fainelli (4):
  net: dsa: Eliminate dsa_slave_get_link()
  net: phy: phylink: Provide PHY interface to mac_link_{up,down}
  net: dsa: Plug in PHYLINK support
  net: dsa: bcm_sf2: Implement phylink_mac_ops

 drivers/net/dsa/bcm_sf2.c             | 190 +++++++++++++--------
 drivers/net/ethernet/marvell/mvneta.c |   4 +-
 drivers/net/phy/phylink.c             |   6 +-
 include/linux/phylink.h               |  10 +-
 include/net/dsa.h                     |  27 ++-
 net/dsa/Kconfig                       |   2 +-
 net/dsa/dsa_priv.h                    |   9 -
 net/dsa/slave.c                       | 304 ++++++++++++++++++++--------------
 8 files changed, 340 insertions(+), 212 deletions(-)

-- 
2.14.1

[PATCH net-next 1/4] net: dsa: Eliminate dsa_slave_get_link()

[Florian Fainelli]

Since we use PHYLIB to manage the per-port link indication, this will
also be reflected correctly in the network device's carrier state, so we
can use ethtool_op_get_link() instead.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/slave.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 18561af7a8f1..9714e8b002d3 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -498,16 +498,6 @@ dsa_slave_get_regs(struct net_device *dev, struct ethtool_regs *regs, void *_p)
 		ds->ops->get_regs(ds, dp->index, regs, _p);
 }
 
-static u32 dsa_slave_get_link(struct net_device *dev)
-{
-	if (!dev->phydev)
-		return -ENODEV;
-
-	genphy_update_link(dev->phydev);
-
-	return dev->phydev->link;
-}
-
 static int dsa_slave_get_eeprom_len(struct net_device *dev)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
@@ -981,7 +971,7 @@ static const struct ethtool_ops dsa_slave_ethtool_ops = {
 	.get_regs_len		= dsa_slave_get_regs_len,
 	.get_regs		= dsa_slave_get_regs,
 	.nway_reset		= phy_ethtool_nway_reset,
-	.get_link		= dsa_slave_get_link,
+	.get_link		= ethtool_op_get_link,
 	.get_eeprom_len		= dsa_slave_get_eeprom_len,
 	.get_eeprom		= dsa_slave_get_eeprom,
 	.set_eeprom		= dsa_slave_set_eeprom,
-- 
2.14.1

[PATCH net-next 2/4] net: phy: phylink: Provide PHY interface to mac_link_{up,down}

[Florian Fainelli]

In preparation for having DSA transition entirely to PHYLINK, we need to pass a
PHY interface type to the mac_link_{up,down} callbacks because we may have to
make decisions on that (e.g: turn on/off RGMII interfaces etc.). We do not pass
an entire phylink_link_state because not all parameters (pause, duplex etc.) are
defined when the link is down, only link and interface are.

Update mvneta accordingly since it currently implements phylink_mac_ops.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/marvell/mvneta.c |  4 +++-
 drivers/net/phy/phylink.c             |  6 +++++-
 include/linux/phylink.h               | 10 ++++++++--
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 25e9a551cc8c..60de9b8d62c2 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -3396,7 +3396,8 @@ static void mvneta_set_eee(struct mvneta_port *pp, bool enable)
 	mvreg_write(pp, MVNETA_LPI_CTRL_1, lpi_ctl1);
 }
 
-static void mvneta_mac_link_down(struct net_device *ndev, unsigned int mode)
+static void mvneta_mac_link_down(struct net_device *ndev, unsigned int mode,
+				 phy_interface_t interface)
 {
 	struct mvneta_port *pp = netdev_priv(ndev);
 	u32 val;
@@ -3415,6 +3416,7 @@ static void mvneta_mac_link_down(struct net_device *ndev, unsigned int mode)
 }
 
 static void mvneta_mac_link_up(struct net_device *ndev, unsigned int mode,
+			       phy_interface_t interface,
 			       struct phy_device *phy)
 {
 	struct mvneta_port *pp = netdev_priv(ndev);
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 51a011a349fe..cef3c1356a8c 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -423,8 +423,10 @@ static void phylink_resolve(struct work_struct *w)
 	if (pl->phylink_disable_state) {
 		pl->mac_link_dropped = false;
 		link_state.link = false;
+		link_state.interface = pl->phy_state.interface;
 	} else if (pl->mac_link_dropped) {
 		link_state.link = false;
+		link_state.interface = pl->phy_state.interface;
 	} else {
 		switch (pl->link_an_mode) {
 		case MLO_AN_PHY:
@@ -470,10 +472,12 @@ static void phylink_resolve(struct work_struct *w)
 	if (link_state.link != netif_carrier_ok(ndev)) {
 		if (!link_state.link) {
 			netif_carrier_off(ndev);
-			pl->ops->mac_link_down(ndev, pl->link_an_mode);
+			pl->ops->mac_link_down(ndev, pl->link_an_mode,
+					       pl->phy_state.interface);
 			netdev_info(ndev, "Link is Down\n");
 		} else {
 			pl->ops->mac_link_up(ndev, pl->link_an_mode,
+					     pl->phy_state.interface,
 					     pl->phydev);
 
 			netif_carrier_on(ndev);
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index bd137c273d38..f29a40947de9 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -73,8 +73,10 @@ struct phylink_mac_ops {
 	void (*mac_config)(struct net_device *ndev, unsigned int mode,
 			   const struct phylink_link_state *state);
 	void (*mac_an_restart)(struct net_device *ndev);
-	void (*mac_link_down)(struct net_device *ndev, unsigned int mode);
+	void (*mac_link_down)(struct net_device *ndev, unsigned int mode,
+			      phy_interface_t interface);
 	void (*mac_link_up)(struct net_device *ndev, unsigned int mode,
+			    phy_interface_t interface,
 			    struct phy_device *phy);
 };
 
@@ -161,17 +163,20 @@ void mac_an_restart(struct net_device *ndev);
  * mac_link_down() - take the link down
  * @ndev: a pointer to a &struct net_device for the MAC.
  * @mode: link autonegotiation mode
+ * @interface: link &typedef phy_interface_t mode
  *
  * If @mode is not an in-band negotiation mode (as defined by
  * phylink_autoneg_inband()), force the link down and disable any
  * Energy Efficient Ethernet MAC configuration.
  */
-void mac_link_down(struct net_device *ndev, unsigned int mode);
+void mac_link_down(struct net_device *ndev, unsigned int mode,
+		   phy_interface_t interface);
 
 /**
  * mac_link_up() - allow the link to come up
  * @ndev: a pointer to a &struct net_device for the MAC.
  * @mode: link autonegotiation mode
+ * @interface: link &typedef phy_interface_t mode
  * @phy: any attached phy
  *
  * If @mode is not an in-band negotiation mode (as defined by
@@ -180,6 +185,7 @@ void mac_link_down(struct net_device *ndev, unsigned int mode);
  * phy_init_eee() and perform appropriate MAC configuration for EEE.
  */
 void mac_link_up(struct net_device *ndev, unsigned int mode,
+		 phy_interface_t interface,
 		 struct phy_device *phy);
 #endif
 
-- 
2.14.1

[PATCH net-next 3/4] net: dsa: Plug in PHYLINK support

[Florian Fainelli]

Add support for PHYLINK within the DSA subsystem in order to support more
complex devices such as pluggable (SFP) and non-pluggable (SFF) modules, 10G
PHYs, and traditional PHYs. Using PHYLINK allows us to drop some amount of
complexity we had while probing fixed and non-fixed PHYs using Device Tree.

Because PHYLINK separates the Ethernet MAC/port configuration into different
stages, we let switch drivers implement those, and for now, we maintain
functionality by calling dsa_slave_adjust_link() during
phylink_mac_link_{up,down} which provides semantically equivalent steps.

Drivers willing to take advantage of PHYLINK should implement the phylink_mac_*
operations that DSA wraps.

We cannot quite remove the adjust_link() callback just yet, because a number of
drivers rely on that for configuring their "CPU" and "DSA" ports, this is done
dsa_port_setup_phy_of() and dsa_port_fixed_link_register_of() still.

Drivers that utilize fixed links for user-facing ports (e.g: bcm_sf2) will need
to implement phylink_mac_ops from now on to preserve functionality, since PHYLINK
*does not* create a phy_device instance for fixed links.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/bcm_sf2.c |  12 +-
 include/net/dsa.h         |  27 ++++-
 net/dsa/Kconfig           |   2 +-
 net/dsa/dsa_priv.h        |   9 --
 net/dsa/slave.c           | 300 ++++++++++++++++++++++++++++------------------
 5 files changed, 213 insertions(+), 137 deletions(-)

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 0378eded31f2..726d75a61795 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -15,7 +15,7 @@
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/phy.h>
-#include <linux/phy_fixed.h>
+#include <linux/phylink.h>
 #include <linux/mii.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
@@ -563,7 +563,7 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
 }
 
 static void bcm_sf2_sw_fixed_link_update(struct dsa_switch *ds, int port,
-					 struct fixed_phy_status *status)
+					 struct phylink_link_state *status)
 {
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
 	u32 duplex, pause, offset;
@@ -611,13 +611,11 @@ static void bcm_sf2_sw_fixed_link_update(struct dsa_switch *ds, int port,
 	core_writel(priv, reg, offset);
 
 	if ((pause & (1 << port)) &&
-	    (pause & (1 << (port + PAUSESTS_TX_PAUSE_SHIFT)))) {
-		status->asym_pause = 1;
-		status->pause = 1;
-	}
+	    (pause & (1 << (port + PAUSESTS_TX_PAUSE_SHIFT))))
+		status->pause |= MLO_PAUSE_TX;
 
 	if (pause & (1 << port))
-		status->pause = 1;
+		status->pause |= MLO_PAUSE_TXRX_MASK;
 }
 
 static void bcm_sf2_enable_acb(struct dsa_switch *ds)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 60fb4ec8ba61..5399bed88df8 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -20,12 +20,13 @@
 #include <linux/of.h>
 #include <linux/ethtool.h>
 #include <linux/net_tstamp.h>
+#include <linux/phy.h>
 #include <net/devlink.h>
 #include <net/switchdev.h>
 
 struct tc_action;
 struct phy_device;
-struct fixed_phy_status;
+struct phylink_link_state;
 
 enum dsa_tag_protocol {
 	DSA_TAG_PROTO_NONE = 0,
@@ -199,6 +200,7 @@ struct dsa_port {
 	u8			stp_state;
 	struct net_device	*bridge_dev;
 	struct devlink_port	devlink_port;
+	struct phylink		*pl;
 	/*
 	 * Original copy of the master netdev ethtool_ops
 	 */
@@ -350,8 +352,27 @@ struct dsa_switch_ops {
 	 */
 	void	(*adjust_link)(struct dsa_switch *ds, int port,
 				struct phy_device *phydev);
+	/*
+	 * PHYLINK integration
+	 */
+	void	(*phylink_validate)(struct dsa_switch *ds, int port,
+				    unsigned long *supported,
+				    struct phylink_link_state *state);
+	int	(*phylink_mac_link_state)(struct dsa_switch *ds, int port,
+					  struct phylink_link_state *state);
+	void	(*phylink_mac_config)(struct dsa_switch *ds, int port,
+				      unsigned int mode,
+				      const struct phylink_link_state *state);
+	void	(*phylink_mac_an_restart)(struct dsa_switch *ds, int port);
+	void	(*phylink_mac_link_down)(struct dsa_switch *ds, int port,
+					 unsigned int mode,
+					 phy_interface_t interface);
+	void	(*phylink_mac_link_up)(struct dsa_switch *ds, int port,
+				       unsigned int mode,
+				       phy_interface_t interface,
+				       struct phy_device *phydev);
 	void	(*fixed_link_update)(struct dsa_switch *ds, int port,
-				struct fixed_phy_status *st);
+				     struct phylink_link_state *state);
 
 	/*
 	 * ethtool hardware statistics.
@@ -588,4 +609,6 @@ static inline int call_dsa_notifiers(unsigned long val, struct net_device *dev,
 #define BRCM_TAG_GET_PORT(v)		((v) >> 8)
 #define BRCM_TAG_GET_QUEUE(v)		((v) & 0xff)
 
+void dsa_port_phylink_mac_change(struct dsa_switch *ds, int port, bool up);
+
 #endif
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index bbf2c82cf7b2..4183e4ba27a5 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -9,7 +9,7 @@ config NET_DSA
 	depends on HAVE_NET_DSA && MAY_USE_DEVLINK
 	depends on BRIDGE || BRIDGE=n
 	select NET_SWITCHDEV
-	select PHYLIB
+	select PHYLINK
 	---help---
 	  Say Y if you want to enable support for the hardware switches supported
 	  by the Distributed Switch Architecture.
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 70de7895e5b8..aebaa5970d91 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -75,15 +75,6 @@ struct dsa_slave_priv {
 	/* DSA port data, such as switch, port index, etc. */
 	struct dsa_port		*dp;
 
-	/*
-	 * The phylib phy_device pointer for the PHY connected
-	 * to this port.
-	 */
-	phy_interface_t		phy_interface;
-	int			old_link;
-	int			old_pause;
-	int			old_duplex;
-
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	struct netpoll		*netpoll;
 #endif
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 9714e8b002d3..6d9914964c20 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -13,6 +13,7 @@
 #include <linux/netdevice.h>
 #include <linux/phy.h>
 #include <linux/phy_fixed.h>
+#include <linux/phylink.h>
 #include <linux/of_net.h>
 #include <linux/of_mdio.h>
 #include <linux/mdio.h>
@@ -97,8 +98,7 @@ static int dsa_slave_open(struct net_device *dev)
 	if (err)
 		goto clear_promisc;
 
-	if (dev->phydev)
-		phy_start(dev->phydev);
+	phylink_start(dp->pl);
 
 	return 0;
 
@@ -120,8 +120,7 @@ static int dsa_slave_close(struct net_device *dev)
 	struct net_device *master = dsa_slave_to_master(dev);
 	struct dsa_port *dp = dsa_slave_to_port(dev);
 
-	if (dev->phydev)
-		phy_stop(dev->phydev);
+	phylink_stop(dp->pl);
 
 	dsa_port_disable(dp, dev->phydev);
 
@@ -272,10 +271,7 @@ static int dsa_slave_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 		break;
 	}
 
-	if (!dev->phydev)
-		return -ENODEV;
-
-	return phy_mii_ioctl(dev->phydev, ifr, cmd);
+	return phylink_mii_ioctl(p->dp->pl, ifr, cmd);
 }
 
 static int dsa_slave_port_attr_set(struct net_device *dev,
@@ -498,6 +494,13 @@ dsa_slave_get_regs(struct net_device *dev, struct ethtool_regs *regs, void *_p)
 		ds->ops->get_regs(ds, dp->index, regs, _p);
 }
 
+static int dsa_slave_nway_reset(struct net_device *dev)
+{
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+
+	return phylink_ethtool_nway_reset(dp->pl);
+}
+
 static int dsa_slave_get_eeprom_len(struct net_device *dev)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
@@ -536,6 +539,22 @@ static int dsa_slave_set_eeprom(struct net_device *dev,
 	return -EOPNOTSUPP;
 }
 
+static int dsa_slave_get_module_info(struct net_device *dev,
+				     struct ethtool_modinfo *modinfo)
+{
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+
+	return phylink_ethtool_get_module_info(dp->pl, modinfo);
+}
+
+static int dsa_slave_get_module_eeprom(struct net_device *dev,
+				       struct ethtool_eeprom *ee, u8 *buf)
+{
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+
+	return phylink_ethtool_get_module_eeprom(dp->pl, ee, buf);
+}
+
 static void dsa_slave_get_strings(struct net_device *dev,
 				  uint32_t stringset, uint8_t *data)
 {
@@ -608,6 +627,8 @@ static void dsa_slave_get_wol(struct net_device *dev, struct ethtool_wolinfo *w)
 	struct dsa_port *dp = dsa_slave_to_port(dev);
 	struct dsa_switch *ds = dp->ds;
 
+	phylink_ethtool_get_wol(dp->pl, w);
+
 	if (ds->ops->get_wol)
 		ds->ops->get_wol(ds, dp->index, w);
 }
@@ -618,6 +639,8 @@ static int dsa_slave_set_wol(struct net_device *dev, struct ethtool_wolinfo *w)
 	struct dsa_switch *ds = dp->ds;
 	int ret = -EOPNOTSUPP;
 
+	phylink_ethtool_set_wol(dp->pl, w);
+
 	if (ds->ops->set_wol)
 		ret = ds->ops->set_wol(ds, dp->index, w);
 
@@ -641,13 +664,7 @@ static int dsa_slave_set_eee(struct net_device *dev, struct ethtool_eee *e)
 	if (ret)
 		return ret;
 
-	if (e->eee_enabled) {
-		ret = phy_init_eee(dev->phydev, 0);
-		if (ret)
-			return ret;
-	}
-
-	return phy_ethtool_set_eee(dev->phydev, e);
+	return phylink_ethtool_set_eee(dp->pl, e);
 }
 
 static int dsa_slave_get_eee(struct net_device *dev, struct ethtool_eee *e)
@@ -667,7 +684,23 @@ static int dsa_slave_get_eee(struct net_device *dev, struct ethtool_eee *e)
 	if (ret)
 		return ret;
 
-	return phy_ethtool_get_eee(dev->phydev, e);
+	return phylink_ethtool_get_eee(dp->pl, e);
+}
+
+static int dsa_slave_get_link_ksettings(struct net_device *dev,
+					struct ethtool_link_ksettings *cmd)
+{
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+
+	return phylink_ethtool_ksettings_get(dp->pl, cmd);
+}
+
+static int dsa_slave_set_link_ksettings(struct net_device *dev,
+					const struct ethtool_link_ksettings *cmd)
+{
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+
+	return phylink_ethtool_ksettings_set(dp->pl, cmd);
 }
 
 #ifdef CONFIG_NET_POLL_CONTROLLER
@@ -970,11 +1003,13 @@ static const struct ethtool_ops dsa_slave_ethtool_ops = {
 	.get_drvinfo		= dsa_slave_get_drvinfo,
 	.get_regs_len		= dsa_slave_get_regs_len,
 	.get_regs		= dsa_slave_get_regs,
-	.nway_reset		= phy_ethtool_nway_reset,
+	.nway_reset		= dsa_slave_nway_reset,
 	.get_link		= ethtool_op_get_link,
 	.get_eeprom_len		= dsa_slave_get_eeprom_len,
 	.get_eeprom		= dsa_slave_get_eeprom,
 	.set_eeprom		= dsa_slave_set_eeprom,
+	.get_module_info	= dsa_slave_get_module_info,
+	.get_module_eeprom	= dsa_slave_get_module_eeprom,
 	.get_strings		= dsa_slave_get_strings,
 	.get_ethtool_stats	= dsa_slave_get_ethtool_stats,
 	.get_sset_count		= dsa_slave_get_sset_count,
@@ -982,8 +1017,8 @@ static const struct ethtool_ops dsa_slave_ethtool_ops = {
 	.get_wol		= dsa_slave_get_wol,
 	.set_eee		= dsa_slave_set_eee,
 	.get_eee		= dsa_slave_get_eee,
-	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
-	.set_link_ksettings	= phy_ethtool_set_link_ksettings,
+	.get_link_ksettings	= dsa_slave_get_link_ksettings,
+	.set_link_ksettings	= dsa_slave_set_link_ksettings,
 	.get_rxnfc		= dsa_slave_get_rxnfc,
 	.set_rxnfc		= dsa_slave_set_rxnfc,
 	.get_ts_info		= dsa_slave_get_ts_info,
@@ -1042,56 +1077,120 @@ static struct device_type dsa_type = {
 	.name	= "dsa",
 };
 
-static void dsa_slave_adjust_link(struct net_device *dev)
+static void dsa_slave_phylink_validate(struct net_device *dev,
+				       unsigned long *supported,
+				       struct phylink_link_state *state)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
-	struct dsa_slave_priv *p = netdev_priv(dev);
 	struct dsa_switch *ds = dp->ds;
-	unsigned int status_changed = 0;
 
-	if (p->old_link != dev->phydev->link) {
-		status_changed = 1;
-		p->old_link = dev->phydev->link;
-	}
+	if (!ds->ops->phylink_validate)
+		return;
 
-	if (p->old_duplex != dev->phydev->duplex) {
-		status_changed = 1;
-		p->old_duplex = dev->phydev->duplex;
-	}
+	ds->ops->phylink_validate(ds, dp->index, supported, state);
+}
 
-	if (p->old_pause != dev->phydev->pause) {
-		status_changed = 1;
-		p->old_pause = dev->phydev->pause;
-	}
+static int dsa_slave_phylink_mac_link_state(struct net_device *dev,
+					    struct phylink_link_state *state)
+{
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
 
-	if (ds->ops->adjust_link && status_changed)
-		ds->ops->adjust_link(ds, dp->index, dev->phydev);
+	/* Only called for SGMII and 802.3z */
+	if (!ds->ops->phylink_mac_link_state)
+		return -EOPNOTSUPP;
 
-	if (status_changed)
-		phy_print_status(dev->phydev);
+	return ds->ops->phylink_mac_link_state(ds, dp->index, state);
 }
 
-static int dsa_slave_fixed_link_update(struct net_device *dev,
-				       struct fixed_phy_status *status)
+static void dsa_slave_phylink_mac_config(struct net_device *dev,
+					 unsigned int mode,
+					 const struct phylink_link_state *state)
 {
-	struct dsa_switch *ds;
-	struct dsa_port *dp;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
 
-	if (dev) {
-		dp = dsa_slave_to_port(dev);
-		ds = dp->ds;
-		if (ds->ops->fixed_link_update)
-			ds->ops->fixed_link_update(ds, dp->index, status);
+	if (!ds->ops->phylink_mac_config)
+		return;
+
+	ds->ops->phylink_mac_config(ds, dp->index, mode, state);
+}
+
+static void dsa_slave_phylink_mac_an_restart(struct net_device *dev)
+{
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
+
+	if (!ds->ops->phylink_mac_an_restart)
+		return;
+
+	ds->ops->phylink_mac_an_restart(ds, dp->index);
+}
+
+static void dsa_slave_phylink_mac_link_down(struct net_device *dev,
+					    unsigned int mode,
+					    phy_interface_t interface)
+{
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
+
+	if (!ds->ops->phylink_mac_link_down) {
+		if (ds->ops->adjust_link && dev->phydev)
+			ds->ops->adjust_link(ds, dp->index, dev->phydev);
+		return;
 	}
 
-	return 0;
+	ds->ops->phylink_mac_link_down(ds, dp->index, mode, interface);
+}
+
+static void dsa_slave_phylink_mac_link_up(struct net_device *dev,
+					  unsigned int mode,
+					  phy_interface_t interface,
+					  struct phy_device *phydev)
+{
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
+
+	if (!ds->ops->phylink_mac_link_up) {
+		if (ds->ops->adjust_link && dev->phydev)
+			ds->ops->adjust_link(ds, dp->index, dev->phydev);
+		return;
+	}
+
+	ds->ops->phylink_mac_link_up(ds, dp->index, mode, interface, phydev);
+}
+
+static const struct phylink_mac_ops dsa_slave_phylink_mac_ops = {
+	.validate = dsa_slave_phylink_validate,
+	.mac_link_state = dsa_slave_phylink_mac_link_state,
+	.mac_config = dsa_slave_phylink_mac_config,
+	.mac_an_restart = dsa_slave_phylink_mac_an_restart,
+	.mac_link_down = dsa_slave_phylink_mac_link_down,
+	.mac_link_up = dsa_slave_phylink_mac_link_up,
+};
+
+void dsa_port_phylink_mac_change(struct dsa_switch *ds, int port, bool up)
+{
+	const struct dsa_port *dp = dsa_to_port(ds, port);
+
+	phylink_mac_change(dp->pl, up);
+}
+EXPORT_SYMBOL_GPL(dsa_port_phylink_mac_change);
+
+static void dsa_slave_phylink_fixed_state(struct net_device *dev,
+					  struct phylink_link_state *state)
+{
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
+
+	if (ds->ops->fixed_link_update)
+		ds->ops->fixed_link_update(ds, dp->index, state);
 }
 
 /* slave device setup *******************************************************/
 static int dsa_slave_phy_connect(struct net_device *slave_dev, int addr)
 {
 	struct dsa_port *dp = dsa_slave_to_port(slave_dev);
-	struct dsa_slave_priv *p = netdev_priv(slave_dev);
 	struct dsa_switch *ds = dp->ds;
 
 	slave_dev->phydev = mdiobus_get_phy(ds->slave_mii_bus, addr);
@@ -1100,75 +1199,49 @@ static int dsa_slave_phy_connect(struct net_device *slave_dev, int addr)
 		return -ENODEV;
 	}
 
-	/* Use already configured phy mode */
-	if (p->phy_interface == PHY_INTERFACE_MODE_NA)
-		p->phy_interface = slave_dev->phydev->interface;
-
-	return phy_connect_direct(slave_dev, slave_dev->phydev,
-				  dsa_slave_adjust_link, p->phy_interface);
+	return phylink_connect_phy(dp->pl, slave_dev->phydev);
 }
 
 static int dsa_slave_phy_setup(struct net_device *slave_dev)
 {
 	struct dsa_port *dp = dsa_slave_to_port(slave_dev);
-	struct dsa_slave_priv *p = netdev_priv(slave_dev);
 	struct device_node *port_dn = dp->dn;
 	struct dsa_switch *ds = dp->ds;
-	struct device_node *phy_dn;
-	bool phy_is_fixed = false;
 	u32 phy_flags = 0;
 	int mode, ret;
 
 	mode = of_get_phy_mode(port_dn);
 	if (mode < 0)
 		mode = PHY_INTERFACE_MODE_NA;
-	p->phy_interface = mode;
 
-	phy_dn = of_parse_phandle(port_dn, "phy-handle", 0);
-	if (!phy_dn && of_phy_is_fixed_link(port_dn)) {
-		/* In the case of a fixed PHY, the DT node associated
-		 * to the fixed PHY is the Port DT node
-		 */
-		ret = of_phy_register_fixed_link(port_dn);
-		if (ret) {
-			netdev_err(slave_dev, "failed to register fixed PHY: %d\n", ret);
-			return ret;
-		}
-		phy_is_fixed = true;
-		phy_dn = of_node_get(port_dn);
+	dp->pl = phylink_create(slave_dev, of_fwnode_handle(port_dn), mode,
+				&dsa_slave_phylink_mac_ops);
+	if (IS_ERR(dp->pl)) {
+		netdev_err(slave_dev,
+			   "error creating PHYLINK: %ld\n", PTR_ERR(dp->pl));
+		return PTR_ERR(dp->pl);
 	}
 
+	phylink_fixed_state_cb(dp->pl, dsa_slave_phylink_fixed_state);
+
 	if (ds->ops->get_phy_flags)
 		phy_flags = ds->ops->get_phy_flags(ds, dp->index);
 
-	if (phy_dn) {
-		slave_dev->phydev = of_phy_connect(slave_dev, phy_dn,
-						   dsa_slave_adjust_link,
-						   phy_flags,
-						   p->phy_interface);
-		of_node_put(phy_dn);
-	}
-
-	if (slave_dev->phydev && phy_is_fixed)
-		fixed_phy_set_link_update(slave_dev->phydev,
-					  dsa_slave_fixed_link_update);
-
-	/* We could not connect to a designated PHY, so use the switch internal
-	 * MDIO bus instead
-	 */
-	if (!slave_dev->phydev) {
+	ret = phylink_of_phy_connect(dp->pl, port_dn, phy_flags);
+	if (ret == -ENODEV) {
+		/* We could not connect to a designated PHY or SFP, so use the
+		 * switch internal MDIO bus instead
+		 */
 		ret = dsa_slave_phy_connect(slave_dev, dp->index);
 		if (ret) {
-			netdev_err(slave_dev, "failed to connect to port %d: %d\n",
+			netdev_err(slave_dev,
+				   "failed to connect to port %d: %d\n",
 				   dp->index, ret);
-			if (phy_is_fixed)
-				of_phy_deregister_fixed_link(port_dn);
+			phylink_destroy(dp->pl);
 			return ret;
 		}
 	}
 
-	phy_attached_info(slave_dev->phydev);
-
 	return 0;
 }
 
@@ -1183,29 +1256,26 @@ static void dsa_slave_set_lockdep_class_one(struct net_device *dev,
 
 int dsa_slave_suspend(struct net_device *slave_dev)
 {
-	struct dsa_slave_priv *p = netdev_priv(slave_dev);
+	struct dsa_port *dp = dsa_slave_to_port(slave_dev);
 
 	netif_device_detach(slave_dev);
 
-	if (slave_dev->phydev) {
-		phy_stop(slave_dev->phydev);
-		p->old_pause = -1;
-		p->old_link = -1;
-		p->old_duplex = -1;
-		phy_suspend(slave_dev->phydev);
-	}
+	rtnl_lock();
+	phylink_stop(dp->pl);
+	rtnl_unlock();
 
 	return 0;
 }
 
 int dsa_slave_resume(struct net_device *slave_dev)
 {
+	struct dsa_port *dp = dsa_slave_to_port(slave_dev);
+
 	netif_device_attach(slave_dev);
 
-	if (slave_dev->phydev) {
-		phy_resume(slave_dev->phydev);
-		phy_start(slave_dev->phydev);
-	}
+	rtnl_lock();
+	phylink_start(dp->pl);
+	rtnl_unlock();
 
 	return 0;
 }
@@ -1270,11 +1340,6 @@ int dsa_slave_create(struct dsa_port *port)
 	p->dp = port;
 	INIT_LIST_HEAD(&p->mall_tc_list);
 	p->xmit = cpu_dp->tag_ops->xmit;
-
-	p->old_pause = -1;
-	p->old_link = -1;
-	p->old_duplex = -1;
-
 	port->slave = slave_dev;
 
 	netif_carrier_off(slave_dev);
@@ -1297,9 +1362,10 @@ int dsa_slave_create(struct dsa_port *port)
 	return 0;
 
 out_phy:
-	phy_disconnect(slave_dev->phydev);
-	if (of_phy_is_fixed_link(port->dn))
-		of_phy_deregister_fixed_link(port->dn);
+	rtnl_lock();
+	phylink_disconnect_phy(p->dp->pl);
+	rtnl_unlock();
+	phylink_destroy(p->dp->pl);
 out_free:
 	free_percpu(p->stats64);
 	free_netdev(slave_dev);
@@ -1311,15 +1377,13 @@ void dsa_slave_destroy(struct net_device *slave_dev)
 {
 	struct dsa_port *dp = dsa_slave_to_port(slave_dev);
 	struct dsa_slave_priv *p = netdev_priv(slave_dev);
-	struct device_node *port_dn = dp->dn;
 
 	netif_carrier_off(slave_dev);
-	if (slave_dev->phydev) {
-		phy_disconnect(slave_dev->phydev);
+	rtnl_lock();
+	phylink_disconnect_phy(dp->pl);
+	rtnl_unlock();
+	phylink_destroy(dp->pl);
 
-		if (of_phy_is_fixed_link(port_dn))
-			of_phy_deregister_fixed_link(port_dn);
-	}
 	dsa_slave_notify(slave_dev, DSA_PORT_UNREGISTER);
 	unregister_netdev(slave_dev);
 	free_percpu(p->stats64);
-- 
2.14.1

[PATCH net-next 4/4] net: dsa: bcm_sf2: Implement phylink_mac_ops

[Florian Fainelli]

Make the bcm_sf2 driver implement phylink_mac_ops since it needs to
support a wide variety of network interfaces: internal & external MDIO
PHYs, fixed PHYs, MoCA with MMIO link status.

A large amount of what needs to be done already exists under
bcm_sf2_sw_adjust_link() so we are essentially breaking this down into
the necessary operation for PHYLINK to work: mac_config, mac_link_up,
mac_link_down and validate. We can now entirely get rid of most of what
fixed_link_update() provided because only the link information is actually
necessary. We still have to force DUPLEX_FULL for legacy Device Tree bindings
that did not specify that before.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/bcm_sf2.c | 186 +++++++++++++++++++++++++++++-----------------
 1 file changed, 118 insertions(+), 68 deletions(-)

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 726d75a61795..b4d5fdcf4183 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -306,7 +306,8 @@ static int bcm_sf2_sw_mdio_write(struct mii_bus *bus, int addr, int regnum,
 
 static irqreturn_t bcm_sf2_switch_0_isr(int irq, void *dev_id)
 {
-	struct bcm_sf2_priv *priv = dev_id;
+	struct dsa_switch *ds = dev_id;
+	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
 
 	priv->irq0_stat = intrl2_0_readl(priv, INTRL2_CPU_STATUS) &
 				~priv->irq0_mask;
@@ -317,16 +318,21 @@ static irqreturn_t bcm_sf2_switch_0_isr(int irq, void *dev_id)
 
 static irqreturn_t bcm_sf2_switch_1_isr(int irq, void *dev_id)
 {
-	struct bcm_sf2_priv *priv = dev_id;
+	struct dsa_switch *ds = dev_id;
+	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
 
 	priv->irq1_stat = intrl2_1_readl(priv, INTRL2_CPU_STATUS) &
 				~priv->irq1_mask;
 	intrl2_1_writel(priv, priv->irq1_stat, INTRL2_CPU_CLEAR);
 
-	if (priv->irq1_stat & P_LINK_UP_IRQ(P7_IRQ_OFF))
-		priv->port_sts[7].link = 1;
-	if (priv->irq1_stat & P_LINK_DOWN_IRQ(P7_IRQ_OFF))
-		priv->port_sts[7].link = 0;
+	if (priv->irq1_stat & P_LINK_UP_IRQ(P7_IRQ_OFF)) {
+		priv->port_sts[7].link = true;
+		dsa_port_phylink_mac_change(ds, 7, true);
+	}
+	if (priv->irq1_stat & P_LINK_DOWN_IRQ(P7_IRQ_OFF)) {
+		priv->port_sts[7].link = false;
+		dsa_port_phylink_mac_change(ds, 7, false);
+	}
 
 	return IRQ_HANDLED;
 }
@@ -473,13 +479,56 @@ static u32 bcm_sf2_sw_get_phy_flags(struct dsa_switch *ds, int port)
 	return priv->hw_params.gphy_rev;
 }
 
-static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
-				   struct phy_device *phydev)
+static void bcm_sf2_sw_validate(struct dsa_switch *ds, int port,
+				unsigned long *supported,
+				struct phylink_link_state *state)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
+
+	if (!phy_interface_mode_is_rgmii(state->interface) &&
+	    state->interface != PHY_INTERFACE_MODE_MII &&
+	    state->interface != PHY_INTERFACE_MODE_REVMII &&
+	    state->interface != PHY_INTERFACE_MODE_GMII &&
+	    state->interface != PHY_INTERFACE_MODE_INTERNAL &&
+	    state->interface != PHY_INTERFACE_MODE_MOCA) {
+		bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
+		dev_err(ds->dev,
+			"Unsupported interface: %d\n", state->interface);
+		return;
+	}
+
+	/* Allow all the expected bits */
+	phylink_set(mask, Autoneg);
+	phylink_set_port_modes(mask);
+	phylink_set(mask, Pause);
+	phylink_set(mask, Asym_Pause);
+
+	/* With the exclusion of MII and Reverse MII, we support Gigabit,
+	 * including Half duplex
+	 */
+	if (state->interface != PHY_INTERFACE_MODE_MII &&
+	    state->interface != PHY_INTERFACE_MODE_REVMII) {
+		phylink_set(mask, 1000baseT_Full);
+		phylink_set(mask, 1000baseT_Half);
+	}
+
+	phylink_set(mask, 10baseT_Half);
+	phylink_set(mask, 10baseT_Full);
+	phylink_set(mask, 100baseT_Half);
+	phylink_set(mask, 100baseT_Full);
+
+	bitmap_and(supported, supported, mask,
+		   __ETHTOOL_LINK_MODE_MASK_NBITS);
+	bitmap_and(state->advertising, state->advertising, mask,
+		   __ETHTOOL_LINK_MODE_MASK_NBITS);
+}
+
+static void bcm_sf2_sw_mac_config(struct dsa_switch *ds, int port,
+				  unsigned int mode,
+				  const struct phylink_link_state *state)
 {
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
-	struct ethtool_eee *p = &priv->dev->ports[port].eee;
 	u32 id_mode_dis = 0, port_mode;
-	const char *str = NULL;
 	u32 reg, offset;
 
 	if (priv->type == BCM7445_DEVICE_ID)
@@ -487,62 +536,48 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
 	else
 		offset = CORE_STS_OVERRIDE_GMIIP2_PORT(port);
 
-	switch (phydev->interface) {
+	switch (state->interface) {
 	case PHY_INTERFACE_MODE_RGMII:
-		str = "RGMII (no delay)";
 		id_mode_dis = 1;
+		/* fallthrough */
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		if (!str)
-			str = "RGMII (TX delay)";
 		port_mode = EXT_GPHY;
 		break;
 	case PHY_INTERFACE_MODE_MII:
-		str = "MII";
 		port_mode = EXT_EPHY;
 		break;
 	case PHY_INTERFACE_MODE_REVMII:
-		str = "Reverse MII";
 		port_mode = EXT_REVMII;
 		break;
 	default:
-		/* All other PHYs: internal and MoCA */
-		goto force_link;
-	}
-
-	/* If the link is down, just disable the interface to conserve power */
-	if (!phydev->link) {
-		reg = reg_readl(priv, REG_RGMII_CNTRL_P(port));
-		reg &= ~RGMII_MODE_EN;
-		reg_writel(priv, reg, REG_RGMII_CNTRL_P(port));
+		/* all other PHYs: internal and MoCA */
 		goto force_link;
 	}
 
-	/* Clear id_mode_dis bit, and the existing port mode, but
-	 * make sure we enable the RGMII block for data to pass
+	/* Clear id_mode_dis bit, and the existing port mode, let
+	 * RGMII_MODE_EN bet set by mac_link_{up,down}
 	 */
 	reg = reg_readl(priv, REG_RGMII_CNTRL_P(port));
 	reg &= ~ID_MODE_DIS;
 	reg &= ~(PORT_MODE_MASK << PORT_MODE_SHIFT);
 	reg &= ~(RX_PAUSE_EN | TX_PAUSE_EN);
 
-	reg |= port_mode | RGMII_MODE_EN;
+	reg |= port_mode;
 	if (id_mode_dis)
 		reg |= ID_MODE_DIS;
 
-	if (phydev->pause) {
-		if (phydev->asym_pause)
+	if (state->pause & MLO_PAUSE_TXRX_MASK) {
+		if (state->pause & MLO_PAUSE_TX)
 			reg |= TX_PAUSE_EN;
 		reg |= RX_PAUSE_EN;
 	}
 
 	reg_writel(priv, reg, REG_RGMII_CNTRL_P(port));
 
-	pr_info("Port %d configured for %s\n", port, str);
-
 force_link:
 	/* Force link settings detected from the PHY */
 	reg = SW_OVERRIDE;
-	switch (phydev->speed) {
+	switch (state->speed) {
 	case SPEED_1000:
 		reg |= SPDSTS_1000 << SPEED_SHIFT;
 		break;
@@ -551,33 +586,61 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
 		break;
 	}
 
-	if (phydev->link)
+	if (state->link)
 		reg |= LINK_STS;
-	if (phydev->duplex == DUPLEX_FULL)
+	if (state->duplex == DUPLEX_FULL)
 		reg |= DUPLX_MODE;
 
 	core_writel(priv, reg, offset);
-
-	if (!phydev->is_pseudo_fixed_link)
-		p->eee_enabled = b53_eee_init(ds, port, phydev);
 }
 
-static void bcm_sf2_sw_fixed_link_update(struct dsa_switch *ds, int port,
-					 struct phylink_link_state *status)
+static void bcm_sf2_sw_mac_link_set(struct dsa_switch *ds, int port,
+				    phy_interface_t interface, bool link)
 {
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
-	u32 duplex, pause, offset;
 	u32 reg;
 
-	if (priv->type == BCM7445_DEVICE_ID)
-		offset = CORE_STS_OVERRIDE_GMIIP_PORT(port);
+	if (!phy_interface_mode_is_rgmii(interface) &&
+	    interface != PHY_INTERFACE_MODE_MII &&
+	    interface != PHY_INTERFACE_MODE_REVMII)
+		return;
+
+	/* If the link is down, just disable the interface to conserve power */
+	reg = reg_readl(priv, REG_RGMII_CNTRL_P(port));
+	if (link)
+		reg |= RGMII_MODE_EN;
 	else
-		offset = CORE_STS_OVERRIDE_GMIIP2_PORT(port);
+		reg &= ~RGMII_MODE_EN;
+	reg_writel(priv, reg, REG_RGMII_CNTRL_P(port));
+}
+
+static void bcm_sf2_sw_mac_link_down(struct dsa_switch *ds, int port,
+				     unsigned int mode,
+				     phy_interface_t interface)
+{
+	bcm_sf2_sw_mac_link_set(ds, port, interface, false);
+}
+
+static void bcm_sf2_sw_mac_link_up(struct dsa_switch *ds, int port,
+				   unsigned int mode,
+				   phy_interface_t interface,
+				   struct phy_device *phydev)
+{
+	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
+	struct ethtool_eee *p = &priv->dev->ports[port].eee;
 
-	duplex = core_readl(priv, CORE_DUPSTS);
-	pause = core_readl(priv, CORE_PAUSESTS);
+	bcm_sf2_sw_mac_link_set(ds, port, interface, true);
 
-	status->link = 0;
+	if (mode == MLO_AN_PHY && phydev)
+		p->eee_enabled = b53_eee_init(ds, port, phydev);
+}
+
+static void bcm_sf2_sw_fixed_link_update(struct dsa_switch *ds, int port,
+					 struct phylink_link_state *status)
+{
+	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
+
+	status->link = false;
 
 	/* MoCA port is special as we do not get link status from CORE_LNKSTS,
 	 * which means that we need to force the link at the port override
@@ -596,26 +659,10 @@ static void bcm_sf2_sw_fixed_link_update(struct dsa_switch *ds, int port,
 		 */
 		if (!status->link)
 			netif_carrier_off(ds->ports[port].slave);
-		status->duplex = 1;
+		status->duplex = DUPLEX_FULL;
 	} else {
-		status->link = 1;
-		status->duplex = !!(duplex & (1 << port));
+		status->link = true;
 	}
-
-	reg = core_readl(priv, offset);
-	reg |= SW_OVERRIDE;
-	if (status->link)
-		reg |= LINK_STS;
-	else
-		reg &= ~LINK_STS;
-	core_writel(priv, reg, offset);
-
-	if ((pause & (1 << port)) &&
-	    (pause & (1 << (port + PAUSESTS_TX_PAUSE_SHIFT))))
-		status->pause |= MLO_PAUSE_TX;
-
-	if (pause & (1 << port))
-		status->pause |= MLO_PAUSE_TXRX_MASK;
 }
 
 static void bcm_sf2_enable_acb(struct dsa_switch *ds)
@@ -858,7 +905,10 @@ static const struct dsa_switch_ops bcm_sf2_ops = {
 	.get_ethtool_stats	= b53_get_ethtool_stats,
 	.get_sset_count		= b53_get_sset_count,
 	.get_phy_flags		= bcm_sf2_sw_get_phy_flags,
-	.adjust_link		= bcm_sf2_sw_adjust_link,
+	.phylink_validate	= bcm_sf2_sw_validate,
+	.phylink_mac_config	= bcm_sf2_sw_mac_config,
+	.phylink_mac_link_down	= bcm_sf2_sw_mac_link_down,
+	.phylink_mac_link_up	= bcm_sf2_sw_mac_link_up,
 	.fixed_link_update	= bcm_sf2_sw_fixed_link_update,
 	.suspend		= bcm_sf2_sw_suspend,
 	.resume			= bcm_sf2_sw_resume,
@@ -1062,14 +1112,14 @@ static int bcm_sf2_sw_probe(struct platform_device *pdev)
 	bcm_sf2_intr_disable(priv);
 
 	ret = devm_request_irq(&pdev->dev, priv->irq0, bcm_sf2_switch_0_isr, 0,
-			       "switch_0", priv);
+			       "switch_0", ds);
 	if (ret < 0) {
 		pr_err("failed to request switch_0 IRQ\n");
 		goto out_mdio;
 	}
 
 	ret = devm_request_irq(&pdev->dev, priv->irq1, bcm_sf2_switch_1_isr, 0,
-			       "switch_1", priv);
+			       "switch_1", ds);
 	if (ret < 0) {
 		pr_err("failed to request switch_1 IRQ\n");
 		goto out_mdio;
-- 
2.14.1

Re: [PATCH net-next 1/4] net: dsa: Eliminate dsa_slave_get_link()

[Andrew Lunn]

On Sun, Mar 18, 2018 at 11:52:43AM -0700, Florian Fainelli wrote:
> Since we use PHYLIB to manage the per-port link indication, this will
> also be reflected correctly in the network device's carrier state, so we
> can use ethtool_op_get_link() instead.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

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

    Andrew

Re: [PATCH net-next 3/4] net: dsa: Plug in PHYLINK support

[Andrew Lunn]

> +static int dsa_slave_nway_reset(struct net_device *dev)
> +{
> +	struct dsa_port *dp = dsa_slave_to_port(dev);
> +
> +	return phylink_ethtool_nway_reset(dp->pl);
> +}

Hi Florian

I've seen in one of Russells trees a patch to put a phylink into
net_device. That would make a generic slave_nway_reset() possible, and
a few others as well. Maybe it makes sense to pull in that patch?

  Andrew

Re: [PATCH net-next 0/4] net: dsa: Plug in PHYLINK support

[Florian Fainelli]

On 03/18/2018 11:52 AM, Florian Fainelli wrote:
> Hi all,
> 
> This patch series adds PHYLINK support to DSA which is necessary to support more
> complex PHY and pluggable modules setups.
> 
> Patch series can be found here:
> 
> https://github.com/ffainelli/linux/commits/dsa-phylink
> 
> This was tested on:
> 
> - dsa-loop
> - bcm_sf2
> - mv88e6xxx
> - b53
> 
> With a variety of test cases:
> - internal & external MDIO PHYs
> - MoCA with link notification through interrupt/MMIO register
> - built-in PHYs
> - ifconfig up/down for several cycles works
> - bind/unbind of the drivers
> 
> And everything should still work as expected. Please be aware of the following:
> 
> - switch drivers (like bcm_sf2) which may have user-facing network ports using
>   fixed links would need to implement phylink_mac_ops to remain functional.
>   PHYLINK does not create a phy_device for fixed links, therefore our
>   call to adjust_link() from phylink_mac_link_{up,down} would not be calling
>   into the driver. This *should not* affect CPU/DSA ports which are configured
>   through adjust_link() but have no network devices
> 
> - support for SFP/SFF is now possible, but switch drivers will still need some
>   modifications to properly support those, including, but not limited to using
>   the correct binding information. This will be submitted on top of this series
> 
> Russell, we could theoretically eliminate patch 3 and resolve this within DSA
> entirely by keeping a per-port phy_interface_t (we did that before), this is
> not a big change if we have to, let me know if you feel like this is cleaner. I
> was initially considering passing a phylink_link_state reference to
> mac_link_{up,down} but only a couple of fields are valid during link_down and
> ended up with passing the phy_interface_t value we need instead. This is
> necessary for switch drivers which have different types of port interfaces (see
> bcm_sf2 documentation in tree).

I think I will proceed differently for v2:

- introduce DSA phylink_mac_ops in dsa_switch_ops, such that drivers can
define those as preliminary commits, those won't be used by
net/dsa/slave.c just yet though

- have all relevant drivers implement phylink_mac_ops such that the
pluming is there and functional

- switch net/dsa/slave.c to using PHYLINK

That way, we should avoid any breakage in between and have an "atomic"
switch between PHYLIB and PHYLINK.

> 
> Thank you!
> 
> Florian Fainelli (4):
>   net: dsa: Eliminate dsa_slave_get_link()
>   net: phy: phylink: Provide PHY interface to mac_link_{up,down}
>   net: dsa: Plug in PHYLINK support
>   net: dsa: bcm_sf2: Implement phylink_mac_ops
> 
>  drivers/net/dsa/bcm_sf2.c             | 190 +++++++++++++--------
>  drivers/net/ethernet/marvell/mvneta.c |   4 +-
>  drivers/net/phy/phylink.c             |   6 +-
>  include/linux/phylink.h               |  10 +-
>  include/net/dsa.h                     |  27 ++-
>  net/dsa/Kconfig                       |   2 +-
>  net/dsa/dsa_priv.h                    |   9 -
>  net/dsa/slave.c                       | 304 ++++++++++++++++++++--------------
>  8 files changed, 340 insertions(+), 212 deletions(-)
> 

-- 
Florian

Re: [PATCH net-next 0/4] net: dsa: Plug in PHYLINK support

[Andrew Lunn]

> I think I will proceed differently for v2:
> 
> - introduce DSA phylink_mac_ops in dsa_switch_ops, such that drivers can
> define those as preliminary commits, those won't be used by
> net/dsa/slave.c just yet though
> 
> - have all relevant drivers implement phylink_mac_ops such that the
> pluming is there and functional
> 
> - switch net/dsa/slave.c to using PHYLINK
> 
> That way, we should avoid any breakage in between and have an "atomic"
> switch between PHYLIB and PHYLINK.

Hi Florian

That sounds good. Maybe we can also convert ZII devel B to using SFP
at the same time?

I can take a look at implementing the Marvell phylink_mac_ops.

  Andrew

Re: [PATCH net-next 0/4] net: dsa: Plug in PHYLINK support

[Florian Fainelli]

On 03/19/2018 06:44 AM, Andrew Lunn wrote:
>> I think I will proceed differently for v2:
>>
>> - introduce DSA phylink_mac_ops in dsa_switch_ops, such that drivers can
>> define those as preliminary commits, those won't be used by
>> net/dsa/slave.c just yet though
>>
>> - have all relevant drivers implement phylink_mac_ops such that the
>> pluming is there and functional
>>
>> - switch net/dsa/slave.c to using PHYLINK
>>
>> That way, we should avoid any breakage in between and have an "atomic"
>> switch between PHYLIB and PHYLINK.
> 
> Hi Florian
> 
> That sounds good. Maybe we can also convert ZII devel B to using SFP
> at the same time?

That should be done, yes.

> 
> I can take a look at implementing the Marvell phylink_mac_ops.

If you could do that, I would be grateful, without the datasheet things
will take time to be reverse engineered for me. AFAIR, Russell had sent
us patches separately that he used.

The way the patch series is structured right now, only the user-facing
ports will need to have phylink_mac_ops, which should help reduce the
amount of testing.

Thanks!
-- 
Florian

Re: [PATCH net-next 3/4] net: dsa: Plug in PHYLINK support

[Florian Fainelli]

Hi Andrew,

On 03/18/2018 12:19 PM, Andrew Lunn wrote:
>> +static int dsa_slave_nway_reset(struct net_device *dev)
>> +{
>> +	struct dsa_port *dp = dsa_slave_to_port(dev);
>> +
>> +	return phylink_ethtool_nway_reset(dp->pl);
>> +}
> 
> Hi Florian
> 
> I've seen in one of Russells trees a patch to put a phylink into
> net_device. That would make a generic slave_nway_reset() possible, and
> a few others as well. Maybe it makes sense to pull in that patch?

To make this generic, we would have to have net_device carry a reference
to a phylink instance, which I would rather not do. Were you possibly
referring to this patch set:

http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=4eda3b76573473d811bc80a6f0e5a2e06dd76bf6

in which case I think it was discussed and rejected (that was my
recollection).
-- 
Florian

Re: [PATCH net-next 3/4] net: dsa: Plug in PHYLINK support

[Russell King - ARM Linux]

On Mon, Mar 19, 2018 at 10:59:55AM -0700, Florian Fainelli wrote:
> Hi Andrew,
> 
> On 03/18/2018 12:19 PM, Andrew Lunn wrote:
> >> +static int dsa_slave_nway_reset(struct net_device *dev)
> >> +{
> >> +	struct dsa_port *dp = dsa_slave_to_port(dev);
> >> +
> >> +	return phylink_ethtool_nway_reset(dp->pl);
> >> +}
> > 
> > Hi Florian
> > 
> > I've seen in one of Russells trees a patch to put a phylink into
> > net_device. That would make a generic slave_nway_reset() possible, and
> > a few others as well. Maybe it makes sense to pull in that patch?
> 
> To make this generic, we would have to have net_device carry a reference
> to a phylink instance, which I would rather not do. Were you possibly
> referring to this patch set:
> 
> http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=4eda3b76573473d811bc80a6f0e5a2e06dd76bf6
> 
> in which case I think it was discussed and rejected (that was my
> recollection).

Unfortunately, that rejection kind'a prevents SFP support on a PHY,
which is why I'm not pushing the 3310 patches (I don't have any other
solution to this problem at the moment as PHYLIB gets in the way of
knowing what state the network interface is in.)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH net-next 3/4] net: dsa: Plug in PHYLINK support

[Florian Fainelli]

On 03/19/2018 11:09 AM, Russell King - ARM Linux wrote:
> On Mon, Mar 19, 2018 at 10:59:55AM -0700, Florian Fainelli wrote:
>> Hi Andrew,
>>
>> On 03/18/2018 12:19 PM, Andrew Lunn wrote:
>>>> +static int dsa_slave_nway_reset(struct net_device *dev)
>>>> +{
>>>> +	struct dsa_port *dp = dsa_slave_to_port(dev);
>>>> +
>>>> +	return phylink_ethtool_nway_reset(dp->pl);
>>>> +}
>>>
>>> Hi Florian
>>>
>>> I've seen in one of Russells trees a patch to put a phylink into
>>> net_device. That would make a generic slave_nway_reset() possible, and
>>> a few others as well. Maybe it makes sense to pull in that patch?
>>
>> To make this generic, we would have to have net_device carry a reference
>> to a phylink instance, which I would rather not do. Were you possibly
>> referring to this patch set:
>>
>> http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=4eda3b76573473d811bc80a6f0e5a2e06dd76bf6
>>
>> in which case I think it was discussed and rejected (that was my
>> recollection).
> 
> Unfortunately, that rejection kind'a prevents SFP support on a PHY,
> which is why I'm not pushing the 3310 patches (I don't have any other
> solution to this problem at the moment as PHYLIB gets in the way of
> knowing what state the network interface is in.)

I don't remember the basis on which this was rejected, and since then, I
had many sleepless nights which don't help with long term memory :) Can
you refresh the context?
-- 
Florian

Re: [PATCH net-next 0/4] net: dsa: Plug in PHYLINK support

[David Miller]

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Sun, 18 Mar 2018 11:52:42 -0700

> This patch series adds PHYLINK support to DSA which is necessary to
> support more complex PHY and pluggable modules setups.

This series is kinda stuck in the mud because of the discussion about
putting a phylink reference into netdevice.

I think the only way to move forward is for Russell to submit his
patches again, that way we can see the requirements and discuss
them properly.

Thanks.

Re: [PATCH net-next 0/4] net: dsa: Plug in PHYLINK support

[Andrew Lunn]

On Tue, Mar 20, 2018 at 11:28:42AM -0400, David Miller wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Sun, 18 Mar 2018 11:52:42 -0700
> 
> > This patch series adds PHYLINK support to DSA which is necessary to
> > support more complex PHY and pluggable modules setups.
> 
> This series is kinda stuck in the mud because of the discussion about
> putting a phylink reference into netdevice.

It is only partially stuck. I've run some tests with the current
patches and found some regressions. I'm working on the Marvell switch
support to fix this.

However, i agree, it would be good to have a repost of Russells
patches.

	Andrew

Re: [PATCH net-next 2/4] net: phy: phylink: Provide PHY interface to mac_link_{up,down}

[Russell King - ARM Linux]

On Sun, Mar 18, 2018 at 11:52:44AM -0700, Florian Fainelli wrote:
> In preparation for having DSA transition entirely to PHYLINK, we need to pass a
> PHY interface type to the mac_link_{up,down} callbacks because we may have to
> make decisions on that (e.g: turn on/off RGMII interfaces etc.). We do not pass
> an entire phylink_link_state because not all parameters (pause, duplex etc.) are
> defined when the link is down, only link and interface are.

If we're going to make this change, we ought to decide whether David
should pick this up for the coming merge window or not independently
of the remaining patches - there are other users of phylink in the
pipeline (bootlin are working on mvpp2 support, so this will be a
minor source of build error pain for folk.)

To that end,

Acked-by: Russell King <rmk+kernel@armlinux.org.uk>

However, the documentation probably ought to make it clear that the
configuration of the interface mode of the MAC should always happen
in the mac_config() callback, not in the mac_link_*() functions.

Thanks.

> Update mvneta accordingly since it currently implements phylink_mac_ops.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/net/ethernet/marvell/mvneta.c |  4 +++-
>  drivers/net/phy/phylink.c             |  6 +++++-
>  include/linux/phylink.h               | 10 ++++++++--
>  3 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 25e9a551cc8c..60de9b8d62c2 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -3396,7 +3396,8 @@ static void mvneta_set_eee(struct mvneta_port *pp, bool enable)
>  	mvreg_write(pp, MVNETA_LPI_CTRL_1, lpi_ctl1);
>  }
>  
> -static void mvneta_mac_link_down(struct net_device *ndev, unsigned int mode)
> +static void mvneta_mac_link_down(struct net_device *ndev, unsigned int mode,
> +				 phy_interface_t interface)
>  {
>  	struct mvneta_port *pp = netdev_priv(ndev);
>  	u32 val;
> @@ -3415,6 +3416,7 @@ static void mvneta_mac_link_down(struct net_device *ndev, unsigned int mode)
>  }
>  
>  static void mvneta_mac_link_up(struct net_device *ndev, unsigned int mode,
> +			       phy_interface_t interface,
>  			       struct phy_device *phy)
>  {
>  	struct mvneta_port *pp = netdev_priv(ndev);
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 51a011a349fe..cef3c1356a8c 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -423,8 +423,10 @@ static void phylink_resolve(struct work_struct *w)
>  	if (pl->phylink_disable_state) {
>  		pl->mac_link_dropped = false;
>  		link_state.link = false;
> +		link_state.interface = pl->phy_state.interface;
>  	} else if (pl->mac_link_dropped) {
>  		link_state.link = false;
> +		link_state.interface = pl->phy_state.interface;
>  	} else {
>  		switch (pl->link_an_mode) {
>  		case MLO_AN_PHY:
> @@ -470,10 +472,12 @@ static void phylink_resolve(struct work_struct *w)
>  	if (link_state.link != netif_carrier_ok(ndev)) {
>  		if (!link_state.link) {
>  			netif_carrier_off(ndev);
> -			pl->ops->mac_link_down(ndev, pl->link_an_mode);
> +			pl->ops->mac_link_down(ndev, pl->link_an_mode,
> +					       pl->phy_state.interface);
>  			netdev_info(ndev, "Link is Down\n");
>  		} else {
>  			pl->ops->mac_link_up(ndev, pl->link_an_mode,
> +					     pl->phy_state.interface,
>  					     pl->phydev);
>  
>  			netif_carrier_on(ndev);
> diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> index bd137c273d38..f29a40947de9 100644
> --- a/include/linux/phylink.h
> +++ b/include/linux/phylink.h
> @@ -73,8 +73,10 @@ struct phylink_mac_ops {
>  	void (*mac_config)(struct net_device *ndev, unsigned int mode,
>  			   const struct phylink_link_state *state);
>  	void (*mac_an_restart)(struct net_device *ndev);
> -	void (*mac_link_down)(struct net_device *ndev, unsigned int mode);
> +	void (*mac_link_down)(struct net_device *ndev, unsigned int mode,
> +			      phy_interface_t interface);
>  	void (*mac_link_up)(struct net_device *ndev, unsigned int mode,
> +			    phy_interface_t interface,
>  			    struct phy_device *phy);
>  };
>  
> @@ -161,17 +163,20 @@ void mac_an_restart(struct net_device *ndev);
>   * mac_link_down() - take the link down
>   * @ndev: a pointer to a &struct net_device for the MAC.
>   * @mode: link autonegotiation mode
> + * @interface: link &typedef phy_interface_t mode
>   *
>   * If @mode is not an in-band negotiation mode (as defined by
>   * phylink_autoneg_inband()), force the link down and disable any
>   * Energy Efficient Ethernet MAC configuration.
>   */
> -void mac_link_down(struct net_device *ndev, unsigned int mode);
> +void mac_link_down(struct net_device *ndev, unsigned int mode,
> +		   phy_interface_t interface);
>  
>  /**
>   * mac_link_up() - allow the link to come up
>   * @ndev: a pointer to a &struct net_device for the MAC.
>   * @mode: link autonegotiation mode
> + * @interface: link &typedef phy_interface_t mode
>   * @phy: any attached phy
>   *
>   * If @mode is not an in-band negotiation mode (as defined by
> @@ -180,6 +185,7 @@ void mac_link_down(struct net_device *ndev, unsigned int mode);
>   * phy_init_eee() and perform appropriate MAC configuration for EEE.
>   */
>  void mac_link_up(struct net_device *ndev, unsigned int mode,
> +		 phy_interface_t interface,
>  		 struct phy_device *phy);
>  #endif
>  
> -- 
> 2.14.1
> 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up