[PATCH net-next 0/2] Convert Ocelot and Felix switches to PHYLINK

[PATCH net-next 0/2] Convert Ocelot and Felix switches to PHYLINK

[Vladimir Oltean]

This series is needed on NXP LS1028A to support the CPU port which runs
at 2500Mbps fixed-link, a setting which PHYLIB can't hold in its swphy
design.

In DSA, PHYLINK comes "for free". I added the PHYLINK ops to the Ocelot
driver, integrated them to the VSC7514 ocelot_board module, then tested
them via the Felix front-end. The VSC7514 integration is only
compile-tested.

Vladimir Oltean (2):
  net: mscc: ocelot: treat SPEED_UNKNOWN as SPEED_10
  net: mscc: ocelot: convert to PHYLINK

 drivers/net/dsa/ocelot/felix.c           |  65 +++++++---
 drivers/net/ethernet/mscc/Kconfig        |   2 +-
 drivers/net/ethernet/mscc/ocelot.c       | 153 ++++++++++++-----------
 drivers/net/ethernet/mscc/ocelot.h       |  13 +-
 drivers/net/ethernet/mscc/ocelot_board.c | 151 +++++++++++++++++++---
 include/soc/mscc/ocelot.h                |  21 +++-
 6 files changed, 285 insertions(+), 120 deletions(-)

-- 

Horatiu, I am sorry for abusing your goodwill. Could you please test
this series and confirm it causes no regression on VSC7514?

2.17.1

[PATCH net-next 1/2] net: mscc: ocelot: treat SPEED_UNKNOWN as SPEED_10

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

One can claim there may be a power consumption gain for ports with no
carrier, but the main benefit of this patch is getting rid of this
warning message when unplugging a cable:

Unsupported PHY speed on port 0: -1

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/mscc/ocelot.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 90c46ba763d7..7fd85767aa8e 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -413,6 +413,7 @@ void ocelot_adjust_link(struct ocelot *ocelot, int port,
 	int speed, mode = 0;
 
 	switch (phydev->speed) {
+	case SPEED_UNKNOWN:
 	case SPEED_10:
 		speed = OCELOT_SPEED_10;
 		break;
-- 
2.17.1

[PATCH net-next 2/2] net: mscc: ocelot: convert to PHYLINK

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

This patch reworks ocelot_board.c (aka the MIPS on the VSC7514) to
register a PHYLINK instance for each port. The registration code is
local to the VSC7514, but the PHYLINK callback implementation is common
so that the Felix DSA front-end can use it as well (but DSA does its own
registration).

Now Felix can use native PHYLINK callbacks instead of the PHYLIB
adaptation layer in DSA, which had issues supporting fixed-link slave
ports (no struct phy_device to pass to the adjust_link callback), as
well as fixed-link CPU port at 2.5Gbps.

The old code from ocelot_port_enable and ocelot_port_disable has been
moved into ocelot_phylink_mac_link_up and ocelot_phylink_mac_link_down.

The PHY connect operation has been moved from ocelot_port_open to
mscc_ocelot_probe in ocelot_board.c.

The phy_set_mode_ext() call for the SerDes PHY has also been moved into
mscc_ocelot_probe from ocelot_port_open, and since that was the only
reason why a reference to it was kept in ocelot_port_private, that
reference was removed.

Again, the usage of phy_interface_t phy_mode is now local to
mscc_ocelot_probe only, after moving the PHY connect operation.
So it was also removed from ocelot_port_private.
*Maybe* in the future, it can be added back to the common struct
ocelot_port, with the purpose of validating mismatches between
state->phy_interface and ocelot_port->phy_mode in PHYLINK callbacks.
But at the moment that is not critical, since other DSA drivers are not
doing that either. No SFP+ modules are in use with Felix/Ocelot yet, to
my knowledge.

In-band AN is not yet supported, due to the fact that this is a mostly
mechanical patch for the moment. The mac_an_restart PHYLINK operation
needs to be implemented, as well as mac_link_state. Both are SerDes
specific, and Felix does not have its PCS configured yet (it works just
by virtue of U-Boot initialization at the moment).

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c           |  65 +++++++---
 drivers/net/ethernet/mscc/Kconfig        |   2 +-
 drivers/net/ethernet/mscc/ocelot.c       | 152 ++++++++++++-----------
 drivers/net/ethernet/mscc/ocelot.h       |  13 +-
 drivers/net/ethernet/mscc/ocelot_board.c | 151 +++++++++++++++++++---
 include/soc/mscc/ocelot.h                |  21 +++-
 6 files changed, 284 insertions(+), 120 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 05e3f2898bf6..d73c38c6cbcf 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -25,14 +25,6 @@ static int felix_set_ageing_time(struct dsa_switch *ds,
 	return 0;
 }
 
-static void felix_adjust_link(struct dsa_switch *ds, int port,
-			      struct phy_device *phydev)
-{
-	struct ocelot *ocelot = ds->priv;
-
-	ocelot_adjust_link(ocelot, port, phydev);
-}
-
 static int felix_fdb_dump(struct dsa_switch *ds, int port,
 			  dsa_fdb_dump_cb_t *cb, void *data)
 {
@@ -137,21 +129,57 @@ static int felix_vlan_del(struct dsa_switch *ds, int port,
 	return 0;
 }
 
-static int felix_port_enable(struct dsa_switch *ds, int port,
-			     struct phy_device *phy)
+static void felix_phylink_validate(struct dsa_switch *ds, int port,
+				   unsigned long *supported,
+				   struct phylink_link_state *state)
 {
 	struct ocelot *ocelot = ds->priv;
 
-	ocelot_port_enable(ocelot, port, phy);
+	ocelot_phylink_validate(ocelot, port, supported, state);
+}
 
-	return 0;
+static int felix_phylink_mac_link_state(struct dsa_switch *ds, int port,
+					struct phylink_link_state *state)
+{
+	struct ocelot *ocelot = ds->priv;
+
+	return ocelot_phylink_mac_link_state(ocelot, port, state);
+}
+
+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;
+
+	ocelot_phylink_mac_config(ocelot, port, link_an_mode, state);
+}
+
+static void felix_phylink_mac_an_restart(struct dsa_switch *ds, int port)
+{
+	struct ocelot *ocelot = ds->priv;
+
+	ocelot_phylink_mac_an_restart(ocelot, port);
+}
+
+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;
+
+	ocelot_phylink_mac_link_down(ocelot, port, link_an_mode, interface);
 }
 
-static void felix_port_disable(struct dsa_switch *ds, int 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)
 {
 	struct ocelot *ocelot = ds->priv;
 
-	return ocelot_port_disable(ocelot, port);
+	ocelot_phylink_mac_link_up(ocelot, port, link_an_mode, interface,
+				   phydev);
 }
 
 static void felix_get_strings(struct dsa_switch *ds, int port,
@@ -312,9 +340,12 @@ static const struct dsa_switch_ops felix_switch_ops = {
 	.get_ethtool_stats	= felix_get_ethtool_stats,
 	.get_sset_count		= felix_get_sset_count,
 	.get_ts_info		= felix_get_ts_info,
-	.adjust_link		= felix_adjust_link,
-	.port_enable		= felix_port_enable,
-	.port_disable		= felix_port_disable,
+	.phylink_validate	= felix_phylink_validate,
+	.phylink_mac_link_state	= felix_phylink_mac_link_state,
+	.phylink_mac_config	= felix_phylink_mac_config,
+	.phylink_mac_an_restart	= felix_phylink_mac_an_restart,
+	.phylink_mac_link_down	= felix_phylink_mac_link_down,
+	.phylink_mac_link_up	= felix_phylink_mac_link_up,
 	.port_fdb_dump		= felix_fdb_dump,
 	.port_fdb_add		= felix_fdb_add,
 	.port_fdb_del		= felix_fdb_del,
diff --git a/drivers/net/ethernet/mscc/Kconfig b/drivers/net/ethernet/mscc/Kconfig
index bcec0587cf61..c89dfce529de 100644
--- a/drivers/net/ethernet/mscc/Kconfig
+++ b/drivers/net/ethernet/mscc/Kconfig
@@ -15,7 +15,7 @@ config MSCC_OCELOT_SWITCH
 	tristate "Ocelot switch driver"
 	depends on NET_SWITCHDEV
 	depends on HAS_IOMEM
-	select PHYLIB
+	select PHYLINK
 	select REGMAP_MMIO
 	help
 	  This driver supports the Ocelot network switch device.
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 7fd85767aa8e..a0f918262220 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -13,7 +13,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/netdevice.h>
-#include <linux/phy.h>
+#include <linux/phylink.h>
 #include <linux/ptp_clock_kernel.h>
 #include <linux/skbuff.h>
 #include <linux/iopoll.h>
@@ -406,13 +406,56 @@ static u16 ocelot_wm_enc(u16 value)
 	return value;
 }
 
-void ocelot_adjust_link(struct ocelot *ocelot, int port,
-			struct phy_device *phydev)
+void ocelot_phylink_validate(struct ocelot *ocelot, int port,
+			     unsigned long *supported,
+			     struct phylink_link_state *state)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
+
+	/* No half-duplex. */
+	phylink_set(mask, Autoneg);
+	phylink_set(mask, MII);
+	phylink_set(mask, Pause);
+	phylink_set(mask, Asym_Pause);
+	phylink_set(mask, 10baseT_Full);
+	phylink_set(mask, 100baseT_Full);
+	phylink_set(mask, 1000baseT_Full);
+	phylink_set(mask, 2500baseT_Full);
+
+	bitmap_and(supported, supported, mask,
+		   __ETHTOOL_LINK_MODE_MASK_NBITS);
+	bitmap_and(state->advertising, state->advertising, mask,
+		   __ETHTOOL_LINK_MODE_MASK_NBITS);
+}
+EXPORT_SYMBOL(ocelot_phylink_validate);
+
+int ocelot_phylink_mac_link_state(struct ocelot *ocelot, int port,
+				  struct phylink_link_state *state)
+{
+	return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL(ocelot_phylink_mac_link_state);
+
+void ocelot_phylink_mac_an_restart(struct ocelot *ocelot, int port)
+{
+	/* Not supported */
+}
+EXPORT_SYMBOL(ocelot_phylink_mac_an_restart);
+
+void ocelot_phylink_mac_config(struct ocelot *ocelot, int port,
+			       unsigned int link_an_mode,
+			       const struct phylink_link_state *state)
 {
 	struct ocelot_port *ocelot_port = ocelot->ports[port];
-	int speed, mode = 0;
+	int speed, mac_mode = 0;
+	u32 mac_fc_cfg;
 
-	switch (phydev->speed) {
+	if (link_an_mode == MLO_AN_INBAND) {
+		dev_err(ocelot->dev, "In-band AN not supported!\n");
+		return;
+	}
+
+	switch (state->speed) {
 	case SPEED_UNKNOWN:
 	case SPEED_10:
 		speed = OCELOT_SPEED_10;
@@ -422,26 +465,24 @@ void ocelot_adjust_link(struct ocelot *ocelot, int port,
 		break;
 	case SPEED_1000:
 		speed = OCELOT_SPEED_1000;
-		mode = DEV_MAC_MODE_CFG_GIGA_MODE_ENA;
+		mac_mode = DEV_MAC_MODE_CFG_GIGA_MODE_ENA;
 		break;
 	case SPEED_2500:
 		speed = OCELOT_SPEED_2500;
-		mode = DEV_MAC_MODE_CFG_GIGA_MODE_ENA;
+		mac_mode = DEV_MAC_MODE_CFG_GIGA_MODE_ENA;
 		break;
 	default:
 		dev_err(ocelot->dev, "Unsupported PHY speed on port %d: %d\n",
-			port, phydev->speed);
+			port, state->speed);
 		return;
 	}
 
-	phy_print_status(phydev);
-
-	if (!phydev->link)
+	if (!state->link)
 		return;
 
 	/* Only full duplex supported for now */
 	ocelot_port_writel(ocelot_port, DEV_MAC_MODE_CFG_FDX_ENA |
-			   mode, DEV_MAC_MODE_CFG);
+			   mac_mode, DEV_MAC_MODE_CFG);
 
 	if (ocelot->ops->pcs_init)
 		ocelot->ops->pcs_init(ocelot, port);
@@ -466,27 +507,36 @@ void ocelot_adjust_link(struct ocelot *ocelot, int port,
 			 QSYS_SWITCH_PORT_MODE, port);
 
 	/* Flow control */
-	ocelot_write_rix(ocelot, SYS_MAC_FC_CFG_PAUSE_VAL_CFG(0xffff) |
-			 SYS_MAC_FC_CFG_RX_FC_ENA | SYS_MAC_FC_CFG_TX_FC_ENA |
-			 SYS_MAC_FC_CFG_ZERO_PAUSE_ENA |
-			 SYS_MAC_FC_CFG_FC_LATENCY_CFG(0x7) |
-			 SYS_MAC_FC_CFG_FC_LINK_SPEED(speed),
-			 SYS_MAC_FC_CFG, port);
+	mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(speed);
+	if (state->pause & MLO_PAUSE_RX)
+		mac_fc_cfg |= SYS_MAC_FC_CFG_RX_FC_ENA;
+	if (state->pause & MLO_PAUSE_TX)
+		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) |
+			      SYS_MAC_FC_CFG_ZERO_PAUSE_ENA;
+	ocelot_write_rix(ocelot, mac_fc_cfg, SYS_MAC_FC_CFG, port);
+
 	ocelot_write_rix(ocelot, 0, ANA_POL_FLOWC, port);
 }
-EXPORT_SYMBOL(ocelot_adjust_link);
+EXPORT_SYMBOL(ocelot_phylink_mac_config);
 
-static void ocelot_port_adjust_link(struct net_device *dev)
+void ocelot_phylink_mac_link_down(struct ocelot *ocelot, int port,
+				  unsigned int link_an_mode,
+				  phy_interface_t interface)
 {
-	struct ocelot_port_private *priv = netdev_priv(dev);
-	struct ocelot *ocelot = priv->port.ocelot;
-	int port = priv->chip_port;
+	struct ocelot_port *ocelot_port = ocelot->ports[port];
 
-	ocelot_adjust_link(ocelot, port, dev->phydev);
+	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);
 }
+EXPORT_SYMBOL(ocelot_phylink_mac_link_down);
 
-void ocelot_port_enable(struct ocelot *ocelot, int port,
-			struct phy_device *phy)
+void ocelot_phylink_mac_link_up(struct ocelot *ocelot, int port,
+				unsigned int link_an_mode,
+				phy_interface_t interface,
+				struct phy_device *phy)
 {
 	/* Enable receiving frames on the port, and activate auto-learning of
 	 * MAC addresses.
@@ -496,62 +546,22 @@ void ocelot_port_enable(struct ocelot *ocelot, int port,
 			 ANA_PORT_PORT_CFG_PORTID_VAL(port),
 			 ANA_PORT_PORT_CFG, port);
 }
-EXPORT_SYMBOL(ocelot_port_enable);
+EXPORT_SYMBOL(ocelot_phylink_mac_link_up);
 
 static int ocelot_port_open(struct net_device *dev)
 {
 	struct ocelot_port_private *priv = netdev_priv(dev);
-	struct ocelot *ocelot = priv->port.ocelot;
-	int port = priv->chip_port;
-	int err;
-
-	if (priv->serdes) {
-		err = phy_set_mode_ext(priv->serdes, PHY_MODE_ETHERNET,
-				       priv->phy_mode);
-		if (err) {
-			netdev_err(dev, "Could not set mode of SerDes\n");
-			return err;
-		}
-	}
 
-	err = phy_connect_direct(dev, priv->phy, &ocelot_port_adjust_link,
-				 priv->phy_mode);
-	if (err) {
-		netdev_err(dev, "Could not attach to PHY\n");
-		return err;
-	}
-
-	dev->phydev = priv->phy;
-
-	phy_attached_info(priv->phy);
-	phy_start(priv->phy);
-
-	ocelot_port_enable(ocelot, port, priv->phy);
+	phylink_start(priv->phylink);
 
 	return 0;
 }
 
-void ocelot_port_disable(struct ocelot *ocelot, int port)
-{
-	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);
-}
-EXPORT_SYMBOL(ocelot_port_disable);
-
 static int ocelot_port_stop(struct net_device *dev)
 {
 	struct ocelot_port_private *priv = netdev_priv(dev);
-	struct ocelot *ocelot = priv->port.ocelot;
-	int port = priv->chip_port;
-
-	phy_disconnect(priv->phy);
-
-	dev->phydev = NULL;
 
-	ocelot_port_disable(ocelot, port);
+	phylink_stop(priv->phylink);
 
 	return 0;
 }
@@ -2183,8 +2193,7 @@ void ocelot_init_port(struct ocelot *ocelot, int port)
 EXPORT_SYMBOL(ocelot_init_port);
 
 int ocelot_probe_port(struct ocelot *ocelot, u8 port,
-		      void __iomem *regs,
-		      struct phy_device *phy)
+		      void __iomem *regs)
 {
 	struct ocelot_port_private *priv;
 	struct ocelot_port *ocelot_port;
@@ -2197,7 +2206,6 @@ int ocelot_probe_port(struct ocelot *ocelot, u8 port,
 	SET_NETDEV_DEV(dev, ocelot->dev);
 	priv = netdev_priv(dev);
 	priv->dev = dev;
-	priv->phy = phy;
 	priv->chip_port = port;
 	ocelot_port = &priv->port;
 	ocelot_port->ocelot = ocelot;
diff --git a/drivers/net/ethernet/mscc/ocelot.h b/drivers/net/ethernet/mscc/ocelot.h
index 32fef4f495aa..14d760f7a1b7 100644
--- a/drivers/net/ethernet/mscc/ocelot.h
+++ b/drivers/net/ethernet/mscc/ocelot.h
@@ -12,8 +12,7 @@
 #include <linux/etherdevice.h>
 #include <linux/if_vlan.h>
 #include <linux/net_tstamp.h>
-#include <linux/phy.h>
-#include <linux/phy/phy.h>
+#include <linux/phylink.h>
 #include <linux/platform_device.h>
 #include <linux/ptp_clock_kernel.h>
 #include <linux/regmap.h>
@@ -63,14 +62,12 @@ struct ocelot_multicast {
 struct ocelot_port_private {
 	struct ocelot_port port;
 	struct net_device *dev;
-	struct phy_device *phy;
+	struct phylink *phylink;
+	struct phylink_config phylink_config;
 	u8 chip_port;
 
 	u8 vlan_aware;
 
-	phy_interface_t phy_mode;
-	struct phy *serdes;
-
 	struct ocelot_port_tc tc;
 };
 
@@ -87,9 +84,7 @@ void ocelot_port_writel(struct ocelot_port *port, u32 val, u32 reg);
 #define ocelot_field_read(ocelot, reg, val) regmap_field_read((ocelot)->regfields[(reg)], (val))
 
 int ocelot_chip_init(struct ocelot *ocelot, const struct ocelot_ops *ops);
-int ocelot_probe_port(struct ocelot *ocelot, u8 port,
-		      void __iomem *regs,
-		      struct phy_device *phy);
+int ocelot_probe_port(struct ocelot *ocelot, u8 port, void __iomem *regs);
 
 void ocelot_set_cpu_port(struct ocelot *ocelot, int cpu,
 			 enum ocelot_tag_prefix injection,
diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c
index 5541ec26f953..aecaf4ef6ef4 100644
--- a/drivers/net/ethernet/mscc/ocelot_board.c
+++ b/drivers/net/ethernet/mscc/ocelot_board.c
@@ -13,6 +13,7 @@
 #include <linux/mfd/syscon.h>
 #include <linux/skbuff.h>
 #include <net/switchdev.h>
+#include <linux/phy/phy.h>
 
 #include "ocelot.h"
 
@@ -305,6 +306,88 @@ static const struct ocelot_ops ocelot_ops = {
 	.reset			= ocelot_reset,
 };
 
+static void ocelot_port_phylink_validate(struct phylink_config *config,
+					 unsigned long *supported,
+					 struct phylink_link_state *state)
+{
+	struct net_device *ndev = to_net_dev(config->dev);
+	struct ocelot_port_private *priv = netdev_priv(ndev);
+	struct ocelot *ocelot = priv->port.ocelot;
+	int port = priv->chip_port;
+
+	ocelot_phylink_validate(ocelot, port, supported, state);
+}
+
+static int ocelot_port_phylink_mac_link_state(struct phylink_config *config,
+					      struct phylink_link_state *state)
+{
+	struct net_device *ndev = to_net_dev(config->dev);
+	struct ocelot_port_private *priv = netdev_priv(ndev);
+	struct ocelot *ocelot = priv->port.ocelot;
+	int port = priv->chip_port;
+
+	return ocelot_phylink_mac_link_state(ocelot, port, state);
+}
+
+static void ocelot_port_phylink_mac_an_restart(struct phylink_config *config)
+{
+	struct net_device *ndev = to_net_dev(config->dev);
+	struct ocelot_port_private *priv = netdev_priv(ndev);
+	struct ocelot *ocelot = priv->port.ocelot;
+	int port = priv->chip_port;
+
+	ocelot_phylink_mac_an_restart(ocelot, port);
+}
+
+static void
+ocelot_port_phylink_mac_config(struct phylink_config *config,
+			       unsigned int link_an_mode,
+			       const struct phylink_link_state *state)
+{
+	struct net_device *ndev = to_net_dev(config->dev);
+	struct ocelot_port_private *priv = netdev_priv(ndev);
+	struct ocelot *ocelot = priv->port.ocelot;
+	int port = priv->chip_port;
+
+	ocelot_phylink_mac_config(ocelot, port, link_an_mode, state);
+}
+
+static void ocelot_port_phylink_mac_link_down(struct phylink_config *config,
+					      unsigned int link_an_mode,
+					      phy_interface_t interface)
+{
+	struct net_device *ndev = to_net_dev(config->dev);
+	struct ocelot_port_private *priv = netdev_priv(ndev);
+	struct ocelot *ocelot = priv->port.ocelot;
+	int port = priv->chip_port;
+
+	return ocelot_phylink_mac_link_down(ocelot, port, link_an_mode,
+					    interface);
+}
+
+static void ocelot_port_phylink_mac_link_up(struct phylink_config *config,
+					    unsigned int link_an_mode,
+					    phy_interface_t interface,
+					    struct phy_device *phy)
+{
+	struct net_device *ndev = to_net_dev(config->dev);
+	struct ocelot_port_private *priv = netdev_priv(ndev);
+	struct ocelot *ocelot = priv->port.ocelot;
+	int port = priv->chip_port;
+
+	return ocelot_phylink_mac_link_up(ocelot, port, link_an_mode,
+					  interface, phy);
+}
+
+static const struct phylink_mac_ops ocelot_phylink_ops = {
+	.validate		= ocelot_port_phylink_validate,
+	.mac_link_state		= ocelot_port_phylink_mac_link_state,
+	.mac_an_restart		= ocelot_port_phylink_mac_an_restart,
+	.mac_config		= ocelot_port_phylink_mac_config,
+	.mac_link_down		= ocelot_port_phylink_mac_link_down,
+	.mac_link_up		= ocelot_port_phylink_mac_link_up,
+};
+
 static int mscc_ocelot_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
@@ -412,9 +495,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
 	for_each_available_child_of_node(ports, portnp) {
 		struct ocelot_port_private *priv;
 		struct ocelot_port *ocelot_port;
-		struct device_node *phy_node;
 		phy_interface_t phy_mode;
-		struct phy_device *phy;
 		struct resource *res;
 		struct phy *serdes;
 		void __iomem *regs;
@@ -432,16 +513,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
 		if (IS_ERR(regs))
 			continue;
 
-		phy_node = of_parse_phandle(portnp, "phy-handle", 0);
-		if (!phy_node)
-			continue;
-
-		phy = of_phy_find_device(phy_node);
-		of_node_put(phy_node);
-		if (!phy)
-			continue;
-
-		err = ocelot_probe_port(ocelot, port, regs, phy);
+		err = ocelot_probe_port(ocelot, port, regs);
 		if (err) {
 			of_node_put(portnp);
 			goto out_put_ports;
@@ -453,9 +525,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
 
 		of_get_phy_mode(portnp, &phy_mode);
 
-		priv->phy_mode = phy_mode;
-
-		switch (priv->phy_mode) {
+		switch (phy_mode) {
 		case PHY_INTERFACE_MODE_NA:
 			continue;
 		case PHY_INTERFACE_MODE_SGMII:
@@ -492,7 +562,41 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
 			goto out_put_ports;
 		}
 
-		priv->serdes = serdes;
+		if (serdes) {
+			err = phy_set_mode_ext(serdes, PHY_MODE_ETHERNET,
+					       phy_mode);
+			if (err) {
+				dev_err(ocelot->dev,
+					"Could not set mode of SerDes\n");
+				of_node_put(portnp);
+				goto out_put_ports;
+			}
+		}
+
+		priv->phylink_config.dev = &priv->dev->dev;
+		priv->phylink_config.type = PHYLINK_NETDEV;
+
+		priv->phylink = phylink_create(&priv->phylink_config,
+					       of_fwnode_handle(portnp),
+					       phy_mode, &ocelot_phylink_ops);
+		if (IS_ERR(priv->phylink)) {
+			dev_err(ocelot->dev,
+				"Could not create a phylink instance (%ld)\n",
+				PTR_ERR(priv->phylink));
+			err = PTR_ERR(priv->phylink);
+			priv->phylink = NULL;
+			of_node_put(portnp);
+			goto out_put_ports;
+		}
+
+		err = phylink_of_phy_connect(priv->phylink, portnp, 0);
+		if (err) {
+			dev_err(ocelot->dev, "Could not connect to PHY: %d\n",
+				err);
+			phylink_destroy(priv->phylink);
+			of_node_put(portnp);
+			goto out_put_ports;
+		}
 	}
 
 	register_netdevice_notifier(&ocelot_netdevice_nb);
@@ -509,12 +613,27 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
 static int mscc_ocelot_remove(struct platform_device *pdev)
 {
 	struct ocelot *ocelot = platform_get_drvdata(pdev);
+	int port;
 
 	ocelot_deinit(ocelot);
 	unregister_switchdev_blocking_notifier(&ocelot_switchdev_blocking_nb);
 	unregister_switchdev_notifier(&ocelot_switchdev_nb);
 	unregister_netdevice_notifier(&ocelot_netdevice_nb);
 
+	for (port = 0; port < ocelot->num_phys_ports; port++) {
+		struct ocelot_port_private *priv;
+
+		priv = container_of(ocelot->ports[port],
+				    struct ocelot_port_private,
+				    port);
+
+		if (priv->phylink) {
+			rtnl_lock();
+			phylink_destroy(priv->phylink);
+			rtnl_unlock();
+		}
+	}
+
 	return 0;
 }
 
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index a836afe8f68e..52c7b53e842f 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -506,17 +506,12 @@ void ocelot_deinit(struct ocelot *ocelot);
 void ocelot_init_port(struct ocelot *ocelot, int port);
 
 /* DSA callbacks */
-void ocelot_port_enable(struct ocelot *ocelot, int port,
-			struct phy_device *phy);
-void ocelot_port_disable(struct ocelot *ocelot, int port);
 void ocelot_get_strings(struct ocelot *ocelot, int port, u32 sset, u8 *data);
 void ocelot_get_ethtool_stats(struct ocelot *ocelot, int port, u64 *data);
 int ocelot_get_sset_count(struct ocelot *ocelot, int port, int sset);
 int ocelot_get_ts_info(struct ocelot *ocelot, int port,
 		       struct ethtool_ts_info *info);
 void ocelot_set_ageing_time(struct ocelot *ocelot, unsigned int msecs);
-void ocelot_adjust_link(struct ocelot *ocelot, int port,
-			struct phy_device *phydev);
 void ocelot_port_vlan_filtering(struct ocelot *ocelot, int port,
 				bool vlan_aware);
 void ocelot_bridge_stp_state_set(struct ocelot *ocelot, int port, u8 state);
@@ -535,5 +530,21 @@ int ocelot_vlan_add(struct ocelot *ocelot, int port, u16 vid, bool pvid,
 int ocelot_vlan_del(struct ocelot *ocelot, int port, u16 vid);
 int ocelot_ptp_gettime64(struct ptp_clock_info *ptp, struct timespec64 *ts);
 void ocelot_get_hwtimestamp(struct ocelot *ocelot, struct timespec64 *ts);
+void ocelot_phylink_validate(struct ocelot *ocelot, int port,
+			     unsigned long *supported,
+			     struct phylink_link_state *state);
+int ocelot_phylink_mac_link_state(struct ocelot *ocelot, int port,
+				  struct phylink_link_state *state);
+void ocelot_phylink_mac_an_restart(struct ocelot *ocelot, int port);
+void ocelot_phylink_mac_config(struct ocelot *ocelot, int port,
+			       unsigned int link_an_mode,
+			       const struct phylink_link_state *state);
+void ocelot_phylink_mac_link_down(struct ocelot *ocelot, int port,
+				  unsigned int link_an_mode,
+				  phy_interface_t interface);
+void ocelot_phylink_mac_link_up(struct ocelot *ocelot, int port,
+				unsigned int link_an_mode,
+				phy_interface_t interface,
+				struct phy_device *phy);
 
 #endif
-- 
2.17.1

Re: [PATCH net-next 2/2] net: mscc: ocelot: convert to PHYLINK

[Russell King - ARM Linux admin]

On Mon, Nov 18, 2019 at 08:10:30PM +0200, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> This patch reworks ocelot_board.c (aka the MIPS on the VSC7514) to
> register a PHYLINK instance for each port. The registration code is
> local to the VSC7514, but the PHYLINK callback implementation is common
> so that the Felix DSA front-end can use it as well (but DSA does its own
> registration).
> 
> Now Felix can use native PHYLINK callbacks instead of the PHYLIB
> adaptation layer in DSA, which had issues supporting fixed-link slave
> ports (no struct phy_device to pass to the adjust_link callback), as
> well as fixed-link CPU port at 2.5Gbps.
> 
> The old code from ocelot_port_enable and ocelot_port_disable has been
> moved into ocelot_phylink_mac_link_up and ocelot_phylink_mac_link_down.
> 
> The PHY connect operation has been moved from ocelot_port_open to
> mscc_ocelot_probe in ocelot_board.c.
> 
> The phy_set_mode_ext() call for the SerDes PHY has also been moved into
> mscc_ocelot_probe from ocelot_port_open, and since that was the only
> reason why a reference to it was kept in ocelot_port_private, that
> reference was removed.
> 
> Again, the usage of phy_interface_t phy_mode is now local to
> mscc_ocelot_probe only, after moving the PHY connect operation.
> So it was also removed from ocelot_port_private.
> *Maybe* in the future, it can be added back to the common struct
> ocelot_port, with the purpose of validating mismatches between
> state->phy_interface and ocelot_port->phy_mode in PHYLINK callbacks.
> But at the moment that is not critical, since other DSA drivers are not
> doing that either. No SFP+ modules are in use with Felix/Ocelot yet, to
> my knowledge.
> 
> In-band AN is not yet supported, due to the fact that this is a mostly
> mechanical patch for the moment. The mac_an_restart PHYLINK operation
> needs to be implemented, as well as mac_link_state. Both are SerDes
> specific, and Felix does not have its PCS configured yet (it works just
> by virtue of U-Boot initialization at the moment).
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

So the crash got me to look at the code to figure out what you're doing
with phylink.

> +int ocelot_phylink_mac_link_state(struct ocelot *ocelot, int port,
> +				  struct phylink_link_state *state)
> +{
> +	return -EOPNOTSUPP;

This does nothing useful.  Set state->link to 0 and just return 0.

> @@ -422,26 +465,24 @@ void ocelot_adjust_link(struct ocelot *ocelot, int port,
>  		break;
>  	case SPEED_1000:
>  		speed = OCELOT_SPEED_1000;
> -		mode = DEV_MAC_MODE_CFG_GIGA_MODE_ENA;
> +		mac_mode = DEV_MAC_MODE_CFG_GIGA_MODE_ENA;
>  		break;
>  	case SPEED_2500:
>  		speed = OCELOT_SPEED_2500;
> -		mode = DEV_MAC_MODE_CFG_GIGA_MODE_ENA;
> +		mac_mode = DEV_MAC_MODE_CFG_GIGA_MODE_ENA;
>  		break;
>  	default:
>  		dev_err(ocelot->dev, "Unsupported PHY speed on port %d: %d\n",
> -			port, phydev->speed);
> +			port, state->speed);
>  		return;
>  	}
>  
> -	phy_print_status(phydev);
> -
> -	if (!phydev->link)
> +	if (!state->link)
>  		return;

Please don't use state->link, it isn't guaranteed to be correct here.
Please read the documentation that I spent time to create for folk
wanting to use phylink, and conform, or ask questions, thanks.

...

> @@ -432,16 +513,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
>  		if (IS_ERR(regs))
>  			continue;
>  
> -		phy_node = of_parse_phandle(portnp, "phy-handle", 0);
> -		if (!phy_node)
> -			continue;
> -
> -		phy = of_phy_find_device(phy_node);
> -		of_node_put(phy_node);
> -		if (!phy)
> -			continue;
> -
> -		err = ocelot_probe_port(ocelot, port, regs, phy);
> +		err = ocelot_probe_port(ocelot, port, regs);

In this function, you  create and register the network device, so by the
time this function returns, the network device is available for use.
Yet, you haven't finished setting it up... so this is racy.

>  		if (err) {
>  			of_node_put(portnp);
>  			goto out_put_ports;
> @@ -453,9 +525,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
>  
>  		of_get_phy_mode(portnp, &phy_mode);
>  
> -		priv->phy_mode = phy_mode;
> -
> -		switch (priv->phy_mode) {
> +		switch (phy_mode) {
>  		case PHY_INTERFACE_MODE_NA:
>  			continue;

What does PHY_INTERFACE_MODE_NA mean here?  That this port is not
connected to anything?

>  		case PHY_INTERFACE_MODE_SGMII:
> @@ -492,7 +562,41 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
>  			goto out_put_ports;
>  		}
>  
> -		priv->serdes = serdes;
> +		if (serdes) {
> +			err = phy_set_mode_ext(serdes, PHY_MODE_ETHERNET,
> +					       phy_mode);
> +			if (err) {
> +				dev_err(ocelot->dev,
> +					"Could not set mode of SerDes\n");
> +				of_node_put(portnp);
> +				goto out_put_ports;
> +			}
> +		}
> +
> +		priv->phylink_config.dev = &priv->dev->dev;
> +		priv->phylink_config.type = PHYLINK_NETDEV;
> +
> +		priv->phylink = phylink_create(&priv->phylink_config,
> +					       of_fwnode_handle(portnp),
> +					       phy_mode, &ocelot_phylink_ops);
> +		if (IS_ERR(priv->phylink)) {
> +			dev_err(ocelot->dev,
> +				"Could not create a phylink instance (%ld)\n",
> +				PTR_ERR(priv->phylink));
> +			err = PTR_ERR(priv->phylink);
> +			priv->phylink = NULL;
> +			of_node_put(portnp);
> +			goto out_put_ports;
> +		}
> +
> +		err = phylink_of_phy_connect(priv->phylink, portnp, 0);
> +		if (err) {
> +			dev_err(ocelot->dev, "Could not connect to PHY: %d\n",
> +				err);
> +			phylink_destroy(priv->phylink);
> +			of_node_put(portnp);
> +			goto out_put_ports;
> +		}
>  	}
>  
>  	register_netdevice_notifier(&ocelot_netdevice_nb);
> @@ -509,12 +613,27 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
>  static int mscc_ocelot_remove(struct platform_device *pdev)
>  {
>  	struct ocelot *ocelot = platform_get_drvdata(pdev);
> +	int port;
>  
>  	ocelot_deinit(ocelot);
>  	unregister_switchdev_blocking_notifier(&ocelot_switchdev_blocking_nb);
>  	unregister_switchdev_notifier(&ocelot_switchdev_nb);
>  	unregister_netdevice_notifier(&ocelot_netdevice_nb);
>  
> +	for (port = 0; port < ocelot->num_phys_ports; port++) {
> +		struct ocelot_port_private *priv;
> +
> +		priv = container_of(ocelot->ports[port],
> +				    struct ocelot_port_private,
> +				    port);
> +
> +		if (priv->phylink) {
> +			rtnl_lock();
> +			phylink_destroy(priv->phylink);

Deadlock waiting to happen.  You must not hold the rtnl lock while
destroying a phylink.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH net-next 0/2] Convert Ocelot and Felix switches to PHYLINK

[Horatiu Vultur]

The 11/18/2019 20:10, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> This series is needed on NXP LS1028A to support the CPU port which runs
> at 2500Mbps fixed-link, a setting which PHYLIB can't hold in its swphy
> design.
> 
> In DSA, PHYLINK comes "for free". I added the PHYLINK ops to the Ocelot
> driver, integrated them to the VSC7514 ocelot_board module, then tested
> them via the Felix front-end. The VSC7514 integration is only
> compile-tested.
> 
> Vladimir Oltean (2):
>   net: mscc: ocelot: treat SPEED_UNKNOWN as SPEED_10
>   net: mscc: ocelot: convert to PHYLINK
> 
>  drivers/net/dsa/ocelot/felix.c           |  65 +++++++---
>  drivers/net/ethernet/mscc/Kconfig        |   2 +-
>  drivers/net/ethernet/mscc/ocelot.c       | 153 ++++++++++++-----------
>  drivers/net/ethernet/mscc/ocelot.h       |  13 +-
>  drivers/net/ethernet/mscc/ocelot_board.c | 151 +++++++++++++++++++---
>  include/soc/mscc/ocelot.h                |  21 +++-
>  6 files changed, 285 insertions(+), 120 deletions(-)
> 
> --
> 
> Horatiu, I am sorry for abusing your goodwill. Could you please test
> this series and confirm it causes no regression on VSC7514?

Hi Vladimir,

Sorry for late reply, I have tried your patches but unfortunetly I get
a segmentation fault when I try to set the link up.
Here is the stack trace:

# ip link set dev eth0 up
[  259.978564] CPU 0 Unable to handle kernel paging request at virtual address 00000008, epc == 805aa7a4, ra == 805aa79c
[  259.989679] Oops[#1]:
[  259.992007] CPU: 0 PID: 98 Comm: ip Not tainted
5.4.0-rc7-01844-g0d53d4ce24f5 #2
[  259.999428] $ 0   : 00000000 00000001 80910000 fffffff8
[  260.004687] $ 4   : 8090838c 0000000e 9e51589c 9e515cbc
[  260.009940] $ 8   : 00000000 807bea44 00000000 00000000
[  260.015193] $12   : 00000000 00000020 00402f1c 00000002
[  260.020445] $16   : 00000000 808e0000 808074f4 9f8a4828
[  260.025699] $20   : 00000000 9e515cbc 9e515ba0 9e54fc10
[  260.030952] $24   : 00000000 9e515dac
[  260.036206] $28   : 9e514000 9e515840 00000000 805aa79c
[  260.041460] Hi    : 00000129
[  260.044351] Lo    : 00001a94
[  260.047311] epc   : 805aa7a4 phylink_start+0x20/0x2e0
[  260.052387] ra    : 805aa79c phylink_start+0x18/0x2e0
[  260.057454] Status: 11008403 KERNEL EXL IE
[  260.061661] Cause : 00800008 (ExcCode 02)
[  260.065683] BadVA : 00000008
[  260.068575] PrId  : 02019654 (MIPS 24KEc)
[  260.072596] Modules linked in:
[  260.075673] Process ip (pid: 98, threadinfo=(ptrval), task=(ptrval), tls=77e564a0)
[  260.083263] Stack : 00000000 00000000 9f8a4800 808e0000 808074f4 9e515cbc 00000000 9f8a4800
[  260.091662]         808e0000 805b7898 00000000 00000000 00000000 808e0000 808074f4 8062cf20
[  260.100058]         00000000 00000000 00000000 00000000 00000000 00000000 00000000 9f8a4800
[  260.108453]         9e515cbc 0295ff75 00000000 9f8a4800 00001003 808e0000 00000000 8062d39c
[  260.116850]         00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[  260.125245]         ...
[  260.127706] Call Trace:
[  260.130176] [<805aa7a4>] phylink_start+0x20/0x2e0
[  260.134955] [<805b7898>] ocelot_port_open+0x10/0x20
[  260.139897] [<8062cf20>] __dev_open+0x10c/0x194
[  260.144457] [<8062d39c>] __dev_change_flags+0x1b0/0x210
[  260.149712] [<8062d420>] dev_change_flags+0x24/0x68
[  260.154625] [<806482d4>] do_setlink+0x340/0xaec
[  260.159182] [<8064978c>] __rtnl_newlink+0x484/0x7d8
[  260.164086] [<80649b30>] rtnl_newlink+0x50/0x84
[  260.168676] [<80643484>] rtnetlink_rcv_msg+0x2e8/0x3b8
[  260.173859] [<8067c9c0>] netlink_rcv_skb+0xa0/0x150
[  260.178766] [<8067abdc>] netlink_unicast+0x1c4/0x26c
[  260.183760] [<8067b478>] netlink_sendmsg+0x2cc/0x3e0
[  260.188758] [<80601df8>] ___sys_sendmsg+0xec/0x280
[  260.193584] [<80603b9c>] __sys_sendmsg+0x60/0xac
[  260.198246] [<80115e98>] syscall_common+0x34/0x58
[  260.202979] Code: afb10020  10400055  3c028091 <8e020008> 8c420004 10400086  24030001  1043006d  00000000
[  260.212788]
[  260.214696] ---[ end trace 42880f8a413b404b ]---
Segmentation fault
#

The reason of this segmentation is that, before it was fine to use the
phy PHY_INTERFACE_MODE_NA. Now if the phy interface is
PHY_INTERFACE_MODE_NA it would not create the phylink. I think this can
be fixed in the device tree.

Apparently there is another issue: mscc mdio bus driver fails to be
probed. So first I need to see this issue and then I will try your
patches.

> 
> 2.17.1
> 

-- 
/Horatiu

Re: [PATCH net-next 0/2] Convert Ocelot and Felix switches to PHYLINK

[Vladimir Oltean]

On Tue, 19 Nov 2019 at 01:13, Horatiu Vultur
<horatiu.vultur@microchip.com> wrote:
>
> The 11/18/2019 20:10, Vladimir Oltean wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > This series is needed on NXP LS1028A to support the CPU port which runs
> > at 2500Mbps fixed-link, a setting which PHYLIB can't hold in its swphy
> > design.
> >
> > In DSA, PHYLINK comes "for free". I added the PHYLINK ops to the Ocelot
> > driver, integrated them to the VSC7514 ocelot_board module, then tested
> > them via the Felix front-end. The VSC7514 integration is only
> > compile-tested.
> >
> > Vladimir Oltean (2):
> >   net: mscc: ocelot: treat SPEED_UNKNOWN as SPEED_10
> >   net: mscc: ocelot: convert to PHYLINK
> >
> >  drivers/net/dsa/ocelot/felix.c           |  65 +++++++---
> >  drivers/net/ethernet/mscc/Kconfig        |   2 +-
> >  drivers/net/ethernet/mscc/ocelot.c       | 153 ++++++++++++-----------
> >  drivers/net/ethernet/mscc/ocelot.h       |  13 +-
> >  drivers/net/ethernet/mscc/ocelot_board.c | 151 +++++++++++++++++++---
> >  include/soc/mscc/ocelot.h                |  21 +++-
> >  6 files changed, 285 insertions(+), 120 deletions(-)
> >
> > --
> >
> > Horatiu, I am sorry for abusing your goodwill. Could you please test
> > this series and confirm it causes no regression on VSC7514?
>
> Hi Vladimir,
>
> Sorry for late reply, I have tried your patches but unfortunetly I get
> a segmentation fault when I try to set the link up.
> Here is the stack trace:
>
> # ip link set dev eth0 up
> [  259.978564] CPU 0 Unable to handle kernel paging request at virtual address 00000008, epc == 805aa7a4, ra == 805aa79c
> [  259.989679] Oops[#1]:
> [  259.992007] CPU: 0 PID: 98 Comm: ip Not tainted
> 5.4.0-rc7-01844-g0d53d4ce24f5 #2
> [  259.999428] $ 0   : 00000000 00000001 80910000 fffffff8
> [  260.004687] $ 4   : 8090838c 0000000e 9e51589c 9e515cbc
> [  260.009940] $ 8   : 00000000 807bea44 00000000 00000000
> [  260.015193] $12   : 00000000 00000020 00402f1c 00000002
> [  260.020445] $16   : 00000000 808e0000 808074f4 9f8a4828
> [  260.025699] $20   : 00000000 9e515cbc 9e515ba0 9e54fc10
> [  260.030952] $24   : 00000000 9e515dac
> [  260.036206] $28   : 9e514000 9e515840 00000000 805aa79c
> [  260.041460] Hi    : 00000129
> [  260.044351] Lo    : 00001a94
> [  260.047311] epc   : 805aa7a4 phylink_start+0x20/0x2e0
> [  260.052387] ra    : 805aa79c phylink_start+0x18/0x2e0
> [  260.057454] Status: 11008403 KERNEL EXL IE
> [  260.061661] Cause : 00800008 (ExcCode 02)
> [  260.065683] BadVA : 00000008
> [  260.068575] PrId  : 02019654 (MIPS 24KEc)
> [  260.072596] Modules linked in:
> [  260.075673] Process ip (pid: 98, threadinfo=(ptrval), task=(ptrval), tls=77e564a0)
> [  260.083263] Stack : 00000000 00000000 9f8a4800 808e0000 808074f4 9e515cbc 00000000 9f8a4800
> [  260.091662]         808e0000 805b7898 00000000 00000000 00000000 808e0000 808074f4 8062cf20
> [  260.100058]         00000000 00000000 00000000 00000000 00000000 00000000 00000000 9f8a4800
> [  260.108453]         9e515cbc 0295ff75 00000000 9f8a4800 00001003 808e0000 00000000 8062d39c
> [  260.116850]         00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [  260.125245]         ...
> [  260.127706] Call Trace:
> [  260.130176] [<805aa7a4>] phylink_start+0x20/0x2e0
> [  260.134955] [<805b7898>] ocelot_port_open+0x10/0x20
> [  260.139897] [<8062cf20>] __dev_open+0x10c/0x194
> [  260.144457] [<8062d39c>] __dev_change_flags+0x1b0/0x210
> [  260.149712] [<8062d420>] dev_change_flags+0x24/0x68
> [  260.154625] [<806482d4>] do_setlink+0x340/0xaec
> [  260.159182] [<8064978c>] __rtnl_newlink+0x484/0x7d8
> [  260.164086] [<80649b30>] rtnl_newlink+0x50/0x84
> [  260.168676] [<80643484>] rtnetlink_rcv_msg+0x2e8/0x3b8
> [  260.173859] [<8067c9c0>] netlink_rcv_skb+0xa0/0x150
> [  260.178766] [<8067abdc>] netlink_unicast+0x1c4/0x26c
> [  260.183760] [<8067b478>] netlink_sendmsg+0x2cc/0x3e0
> [  260.188758] [<80601df8>] ___sys_sendmsg+0xec/0x280
> [  260.193584] [<80603b9c>] __sys_sendmsg+0x60/0xac
> [  260.198246] [<80115e98>] syscall_common+0x34/0x58
> [  260.202979] Code: afb10020  10400055  3c028091 <8e020008> 8c420004 10400086  24030001  1043006d  00000000
> [  260.212788]
> [  260.214696] ---[ end trace 42880f8a413b404b ]---
> Segmentation fault
> #
>
> The reason of this segmentation is that, before it was fine to use the
> phy PHY_INTERFACE_MODE_NA. Now if the phy interface is
> PHY_INTERFACE_MODE_NA it would not create the phylink. I think this can
> be fixed in the device tree.
>

Oops, that does not sound good. But what crashes in phylink_start,
exactly, and why? I see there is a print right at the beginning of the
function, and it isn't visible in your log. Does it crash at the
print?

> Apparently there is another issue: mscc mdio bus driver fails to be
> probed. So first I need to see this issue and then I will try your
> patches.
>
> >
> > 2.17.1
> >
>
> --
> /Horatiu

Thanks,
-Vladimir

Re: [PATCH net-next 0/2] Convert Ocelot and Felix switches to PHYLINK

[Horatiu Vultur]

The 11/19/2019 14:42, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Tue, 19 Nov 2019 at 01:13, Horatiu Vultur
> <horatiu.vultur@microchip.com> wrote:
> >
> > The 11/18/2019 20:10, Vladimir Oltean wrote:
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > >
> > > This series is needed on NXP LS1028A to support the CPU port which runs
> > > at 2500Mbps fixed-link, a setting which PHYLIB can't hold in its swphy
> > > design.
> > >
> > > In DSA, PHYLINK comes "for free". I added the PHYLINK ops to the Ocelot
> > > driver, integrated them to the VSC7514 ocelot_board module, then tested
> > > them via the Felix front-end. The VSC7514 integration is only
> > > compile-tested.
> > >
> > > Vladimir Oltean (2):
> > >   net: mscc: ocelot: treat SPEED_UNKNOWN as SPEED_10
> > >   net: mscc: ocelot: convert to PHYLINK
> > >
> > >  drivers/net/dsa/ocelot/felix.c           |  65 +++++++---
> > >  drivers/net/ethernet/mscc/Kconfig        |   2 +-
> > >  drivers/net/ethernet/mscc/ocelot.c       | 153 ++++++++++++-----------
> > >  drivers/net/ethernet/mscc/ocelot.h       |  13 +-
> > >  drivers/net/ethernet/mscc/ocelot_board.c | 151 +++++++++++++++++++---
> > >  include/soc/mscc/ocelot.h                |  21 +++-
> > >  6 files changed, 285 insertions(+), 120 deletions(-)
> > >
> > > --
> > >
> > > Horatiu, I am sorry for abusing your goodwill. Could you please test
> > > this series and confirm it causes no regression on VSC7514?
> >
> > Hi Vladimir,
> >
> > Sorry for late reply, I have tried your patches but unfortunetly I get
> > a segmentation fault when I try to set the link up.
> > Here is the stack trace:
> >
> > # ip link set dev eth0 up
> > [  259.978564] CPU 0 Unable to handle kernel paging request at virtual address 00000008, epc == 805aa7a4, ra == 805aa79c
> > [  259.989679] Oops[#1]:
> > [  259.992007] CPU: 0 PID: 98 Comm: ip Not tainted
> > 5.4.0-rc7-01844-g0d53d4ce24f5 #2
> > [  259.999428] $ 0   : 00000000 00000001 80910000 fffffff8
> > [  260.004687] $ 4   : 8090838c 0000000e 9e51589c 9e515cbc
> > [  260.009940] $ 8   : 00000000 807bea44 00000000 00000000
> > [  260.015193] $12   : 00000000 00000020 00402f1c 00000002
> > [  260.020445] $16   : 00000000 808e0000 808074f4 9f8a4828
> > [  260.025699] $20   : 00000000 9e515cbc 9e515ba0 9e54fc10
> > [  260.030952] $24   : 00000000 9e515dac
> > [  260.036206] $28   : 9e514000 9e515840 00000000 805aa79c
> > [  260.041460] Hi    : 00000129
> > [  260.044351] Lo    : 00001a94
> > [  260.047311] epc   : 805aa7a4 phylink_start+0x20/0x2e0
> > [  260.052387] ra    : 805aa79c phylink_start+0x18/0x2e0
> > [  260.057454] Status: 11008403 KERNEL EXL IE
> > [  260.061661] Cause : 00800008 (ExcCode 02)
> > [  260.065683] BadVA : 00000008
> > [  260.068575] PrId  : 02019654 (MIPS 24KEc)
> > [  260.072596] Modules linked in:
> > [  260.075673] Process ip (pid: 98, threadinfo=(ptrval), task=(ptrval), tls=77e564a0)
> > [  260.083263] Stack : 00000000 00000000 9f8a4800 808e0000 808074f4 9e515cbc 00000000 9f8a4800
> > [  260.091662]         808e0000 805b7898 00000000 00000000 00000000 808e0000 808074f4 8062cf20
> > [  260.100058]         00000000 00000000 00000000 00000000 00000000 00000000 00000000 9f8a4800
> > [  260.108453]         9e515cbc 0295ff75 00000000 9f8a4800 00001003 808e0000 00000000 8062d39c
> > [  260.116850]         00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > [  260.125245]         ...
> > [  260.127706] Call Trace:
> > [  260.130176] [<805aa7a4>] phylink_start+0x20/0x2e0
> > [  260.134955] [<805b7898>] ocelot_port_open+0x10/0x20
> > [  260.139897] [<8062cf20>] __dev_open+0x10c/0x194
> > [  260.144457] [<8062d39c>] __dev_change_flags+0x1b0/0x210
> > [  260.149712] [<8062d420>] dev_change_flags+0x24/0x68
> > [  260.154625] [<806482d4>] do_setlink+0x340/0xaec
> > [  260.159182] [<8064978c>] __rtnl_newlink+0x484/0x7d8
> > [  260.164086] [<80649b30>] rtnl_newlink+0x50/0x84
> > [  260.168676] [<80643484>] rtnetlink_rcv_msg+0x2e8/0x3b8
> > [  260.173859] [<8067c9c0>] netlink_rcv_skb+0xa0/0x150
> > [  260.178766] [<8067abdc>] netlink_unicast+0x1c4/0x26c
> > [  260.183760] [<8067b478>] netlink_sendmsg+0x2cc/0x3e0
> > [  260.188758] [<80601df8>] ___sys_sendmsg+0xec/0x280
> > [  260.193584] [<80603b9c>] __sys_sendmsg+0x60/0xac
> > [  260.198246] [<80115e98>] syscall_common+0x34/0x58
> > [  260.202979] Code: afb10020  10400055  3c028091 <8e020008> 8c420004 10400086  24030001  1043006d  00000000
> > [  260.212788]
> > [  260.214696] ---[ end trace 42880f8a413b404b ]---
> > Segmentation fault
> > #
> >
> > The reason of this segmentation is that, before it was fine to use the
> > phy PHY_INTERFACE_MODE_NA. Now if the phy interface is
> > PHY_INTERFACE_MODE_NA it would not create the phylink. I think this can
> > be fixed in the device tree.
> >
> 
> Oops, that does not sound good. But what crashes in phylink_start,
> exactly, and why? I see there is a print right at the beginning of the
> function, and it isn't visible in your log. Does it crash at the
> print?

Yes it crashes at the print because in the function 'ocelot_port_open'
the priv->phylink is NULL. In the function mscc_ocelot_probe the phylink
is not created because the function 'of_get_phy_mode' sets phy_mode to
PHY_INTERFACE_MODE_NA because there is no 'phy-mode' attribut in the DT.
And after that it checks the phy_mode and if it is PHY_INTERFACE_MODE_NA
it would just continue to create the next interface so the phylink is
always NULL.

Before this commit it was ok to use PHY_INTERFACE_MODE_NA but now that
is not true anymore. In this case we have 4 ports that have phy and
then 6 sfp ports. So I was looking to describe this in DT but without
any success. If you have any advice that would be great.

> 
> > Apparently there is another issue: mscc mdio bus driver fails to be
> > probed. So first I need to see this issue and then I will try your
> > patches.
> >
> > >
> > > 2.17.1
> > >
> >
> > --
> > /Horatiu
> 
> Thanks,
> -Vladimir

-- 
/Horatiu

Re: [PATCH net-next 0/2] Convert Ocelot and Felix switches to PHYLINK

[Vladimir Oltean]

On Tue, 19 Nov 2019 at 22:49, Horatiu Vultur
<horatiu.vultur@microchip.com> wrote:
>
> The 11/19/2019 14:42, Vladimir Oltean wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > On Tue, 19 Nov 2019 at 01:13, Horatiu Vultur
> > <horatiu.vultur@microchip.com> wrote:
> > >
> > > The 11/18/2019 20:10, Vladimir Oltean wrote:
> > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > > >
> > > > This series is needed on NXP LS1028A to support the CPU port which runs
> > > > at 2500Mbps fixed-link, a setting which PHYLIB can't hold in its swphy
> > > > design.
> > > >
> > > > In DSA, PHYLINK comes "for free". I added the PHYLINK ops to the Ocelot
> > > > driver, integrated them to the VSC7514 ocelot_board module, then tested
> > > > them via the Felix front-end. The VSC7514 integration is only
> > > > compile-tested.
> > > >
> > > > Vladimir Oltean (2):
> > > >   net: mscc: ocelot: treat SPEED_UNKNOWN as SPEED_10
> > > >   net: mscc: ocelot: convert to PHYLINK
> > > >
> > > >  drivers/net/dsa/ocelot/felix.c           |  65 +++++++---
> > > >  drivers/net/ethernet/mscc/Kconfig        |   2 +-
> > > >  drivers/net/ethernet/mscc/ocelot.c       | 153 ++++++++++++-----------
> > > >  drivers/net/ethernet/mscc/ocelot.h       |  13 +-
> > > >  drivers/net/ethernet/mscc/ocelot_board.c | 151 +++++++++++++++++++---
> > > >  include/soc/mscc/ocelot.h                |  21 +++-
> > > >  6 files changed, 285 insertions(+), 120 deletions(-)
> > > >
> > > > --
> > > >
> > > > Horatiu, I am sorry for abusing your goodwill. Could you please test
> > > > this series and confirm it causes no regression on VSC7514?
> > >
> > > Hi Vladimir,
> > >
> > > Sorry for late reply, I have tried your patches but unfortunetly I get
> > > a segmentation fault when I try to set the link up.
> > > Here is the stack trace:
> > >
> > > # ip link set dev eth0 up
> > > [  259.978564] CPU 0 Unable to handle kernel paging request at virtual address 00000008, epc == 805aa7a4, ra == 805aa79c
> > > [  259.989679] Oops[#1]:
> > > [  259.992007] CPU: 0 PID: 98 Comm: ip Not tainted
> > > 5.4.0-rc7-01844-g0d53d4ce24f5 #2
> > > [  259.999428] $ 0   : 00000000 00000001 80910000 fffffff8
> > > [  260.004687] $ 4   : 8090838c 0000000e 9e51589c 9e515cbc
> > > [  260.009940] $ 8   : 00000000 807bea44 00000000 00000000
> > > [  260.015193] $12   : 00000000 00000020 00402f1c 00000002
> > > [  260.020445] $16   : 00000000 808e0000 808074f4 9f8a4828
> > > [  260.025699] $20   : 00000000 9e515cbc 9e515ba0 9e54fc10
> > > [  260.030952] $24   : 00000000 9e515dac
> > > [  260.036206] $28   : 9e514000 9e515840 00000000 805aa79c
> > > [  260.041460] Hi    : 00000129
> > > [  260.044351] Lo    : 00001a94
> > > [  260.047311] epc   : 805aa7a4 phylink_start+0x20/0x2e0
> > > [  260.052387] ra    : 805aa79c phylink_start+0x18/0x2e0
> > > [  260.057454] Status: 11008403 KERNEL EXL IE
> > > [  260.061661] Cause : 00800008 (ExcCode 02)
> > > [  260.065683] BadVA : 00000008
> > > [  260.068575] PrId  : 02019654 (MIPS 24KEc)
> > > [  260.072596] Modules linked in:
> > > [  260.075673] Process ip (pid: 98, threadinfo=(ptrval), task=(ptrval), tls=77e564a0)
> > > [  260.083263] Stack : 00000000 00000000 9f8a4800 808e0000 808074f4 9e515cbc 00000000 9f8a4800
> > > [  260.091662]         808e0000 805b7898 00000000 00000000 00000000 808e0000 808074f4 8062cf20
> > > [  260.100058]         00000000 00000000 00000000 00000000 00000000 00000000 00000000 9f8a4800
> > > [  260.108453]         9e515cbc 0295ff75 00000000 9f8a4800 00001003 808e0000 00000000 8062d39c
> > > [  260.116850]         00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > > [  260.125245]         ...
> > > [  260.127706] Call Trace:
> > > [  260.130176] [<805aa7a4>] phylink_start+0x20/0x2e0
> > > [  260.134955] [<805b7898>] ocelot_port_open+0x10/0x20
> > > [  260.139897] [<8062cf20>] __dev_open+0x10c/0x194
> > > [  260.144457] [<8062d39c>] __dev_change_flags+0x1b0/0x210
> > > [  260.149712] [<8062d420>] dev_change_flags+0x24/0x68
> > > [  260.154625] [<806482d4>] do_setlink+0x340/0xaec
> > > [  260.159182] [<8064978c>] __rtnl_newlink+0x484/0x7d8
> > > [  260.164086] [<80649b30>] rtnl_newlink+0x50/0x84
> > > [  260.168676] [<80643484>] rtnetlink_rcv_msg+0x2e8/0x3b8
> > > [  260.173859] [<8067c9c0>] netlink_rcv_skb+0xa0/0x150
> > > [  260.178766] [<8067abdc>] netlink_unicast+0x1c4/0x26c
> > > [  260.183760] [<8067b478>] netlink_sendmsg+0x2cc/0x3e0
> > > [  260.188758] [<80601df8>] ___sys_sendmsg+0xec/0x280
> > > [  260.193584] [<80603b9c>] __sys_sendmsg+0x60/0xac
> > > [  260.198246] [<80115e98>] syscall_common+0x34/0x58
> > > [  260.202979] Code: afb10020  10400055  3c028091 <8e020008> 8c420004 10400086  24030001  1043006d  00000000
> > > [  260.212788]
> > > [  260.214696] ---[ end trace 42880f8a413b404b ]---
> > > Segmentation fault
> > > #
> > >
> > > The reason of this segmentation is that, before it was fine to use the
> > > phy PHY_INTERFACE_MODE_NA. Now if the phy interface is
> > > PHY_INTERFACE_MODE_NA it would not create the phylink. I think this can
> > > be fixed in the device tree.
> > >
> >
> > Oops, that does not sound good. But what crashes in phylink_start,
> > exactly, and why? I see there is a print right at the beginning of the
> > function, and it isn't visible in your log. Does it crash at the
> > print?
>
> Yes it crashes at the print because in the function 'ocelot_port_open'
> the priv->phylink is NULL. In the function mscc_ocelot_probe the phylink
> is not created because the function 'of_get_phy_mode' sets phy_mode to
> PHY_INTERFACE_MODE_NA because there is no 'phy-mode' attribut in the DT.
> And after that it checks the phy_mode and if it is PHY_INTERFACE_MODE_NA
> it would just continue to create the next interface so the phylink is
> always NULL.
>
> Before this commit it was ok to use PHY_INTERFACE_MODE_NA but now that
> is not true anymore. In this case we have 4 ports that have phy and
> then 6 sfp ports. So I was looking to describe this in DT but without
> any success. If you have any advice that would be great.
>

What driver does your embedded PHY use?
If we use phylink_connect_phy instead of phylink_of_phy_connect, we
might be able to make use of Florian's patch:

commit 4904b6ea1f9dbf47107f50b1c502a22d0160712d
Author: Florian Fainelli <f.fainelli@gmail.com>
Date:   Tue Dec 12 16:00:26 2017 -0800

    net: phy: phylink: Use PHY device interface if N/A

    We may not always be able to resolve a correct phy_interface_t value before
    actually connecting to the PHY device, when that happens, just have
    phylink_connect_phy() utilize what the PHY device/driver provided.

    Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

and have your embedded PHY driver provide a valid phydev->interface
that can be used instead of PHY_INTERFACE_MODE_NA.
That's my best idea at the moment. Comments from others of course welcome.

> >
> > > Apparently there is another issue: mscc mdio bus driver fails to be
> > > probed. So first I need to see this issue and then I will try your
> > > patches.
> > >
> > > >
> > > > 2.17.1
> > > >
> > >
> > > --
> > > /Horatiu
> >
> > Thanks,
> > -Vladimir
>
> --
> /Horatiu

Thanks,
-Vladimir

Re: [PATCH net-next 0/2] Convert Ocelot and Felix switches to PHYLINK

[Andrew Lunn]

> Before this commit it was ok to use PHY_INTERFACE_MODE_NA but now that
> is not true anymore. In this case we have 4 ports that have phy and
> then 6 sfp ports. So I was looking to describe this in DT but without
> any success. If you have any advice that would be great.

Is it the copper ports causing the trouble, or the SFP?  Ideally, you
should describe the SFPs as SFPs. But i don't think the driver has the
needed support for that yet. So you might need to use fixed-link for
the moment.

   Andrew

Re: [PATCH net-next 0/2] Convert Ocelot and Felix switches to PHYLINK

[Horatiu Vultur]

The 11/19/2019 22:42, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> > Before this commit it was ok to use PHY_INTERFACE_MODE_NA but now that
> > is not true anymore. In this case we have 4 ports that have phy and
> > then 6 sfp ports. So I was looking to describe this in DT but without
> > any success. If you have any advice that would be great.
> 
> Is it the copper ports causing the trouble, or the SFP?  Ideally, you
> should describe the SFPs as SFPs. But i don't think the driver has the
> needed support for that yet. So you might need to use fixed-link for
> the moment.

It was both of them. So I have done few small changes to these patches.
- first I added the phy-mode in DT on the interfaces that have a
  phy(internal or external)
- add a check for PHY_INTERFACE_MODE_NA before the port is probed so it
  would not create net device if the phy mode is PHY_INTERFACE_MODE_NA
  because in that case the phylink was not created.

With these changes now only the ports that have phy are probed. This is
the same behaviour as before these patches. I have tried to configure
the sfp ports as fixed-links but unfortunetly it didn't work, I think
because of some missconfiguration on MAC or SerDes, which I need to
figure out. But I think this can be fix in a different patch.

I have done few tests and they seem to work fine.
Here are my changes.

diff --git a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts
index 33991fd209f5..0800a86b7f16 100644
--- a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts
+++ b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts
@@ -60,18 +60,22 @@
 
 &port0 {
 	phy-handle = <&phy0>;
+	phy-mode = "sgmii";
 };
 
 &port1 {
 	phy-handle = <&phy1>;
+	phy-mode = "sgmii";
 };
 
 &port2 {
 	phy-handle = <&phy2>;
+	phy-mode = "sgmii";
 };
 
 &port3 {
 	phy-handle = <&phy3>;
+	phy-mode = "sgmii";
 };
 
 &port4 {
diff --git a/arch/mips/boot/dts/mscc/ocelot_pcb123.dts b/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
index ef852f382da8..6b0b1fb358ad 100644
--- a/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
+++ b/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
@@ -47,17 +47,21 @@
 };
 
 &port0 {
+	phy-mode = "sgmii";
 	phy-handle = <&phy0>;
 };
 
 &port1 {
+	phy-mode = "sgmii";
 	phy-handle = <&phy1>;
 };
 
 &port2 {
+	phy-mode = "sgmii";
 	phy-handle = <&phy2>;
 };
 
 &port3 {
+	phy-mode = "sgmii";
 	phy-handle = <&phy3>;
 };
diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c
index aecaf4ef6ef4..9dad031900b5 100644
--- a/drivers/net/ethernet/mscc/ocelot_board.c
+++ b/drivers/net/ethernet/mscc/ocelot_board.c
@@ -513,6 +513,10 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
 		if (IS_ERR(regs))
 			continue;
 
+		of_get_phy_mode(portnp, &phy_mode);
+		if (phy_mode == PHY_INTERFACE_MODE_NA)
+			continue;
+
 		err = ocelot_probe_port(ocelot, port, regs);
 		if (err) {
 			of_node_put(portnp);
@@ -523,11 +527,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
 		priv = container_of(ocelot_port, struct ocelot_port_private,
 				    port);
 
-		of_get_phy_mode(portnp, &phy_mode);
-
 		switch (phy_mode) {
-		case PHY_INTERFACE_MODE_NA:
-			continue;
 		case PHY_INTERFACE_MODE_SGMII:
 			break;
 		case PHY_INTERFACE_MODE_QSGMII:
@@ -549,20 +549,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
 		}
 
 		serdes = devm_of_phy_get(ocelot->dev, portnp, NULL);
-		if (IS_ERR(serdes)) {
-			err = PTR_ERR(serdes);
-			if (err == -EPROBE_DEFER)
-				dev_dbg(ocelot->dev, "deferring probe\n");
-			else
-				dev_err(ocelot->dev,
-					"missing SerDes phys for port%d\n",
-					port);
-
-			of_node_put(portnp);
-			goto out_put_ports;
-		}
-
-		if (serdes) {
+		if (!IS_ERR(serdes)) {
 			err = phy_set_mode_ext(serdes, PHY_MODE_ETHERNET,
 					       phy_mode);
 			if (err) {
-- 
2.17.1


> 
>    Andrew

-- 
/Horatiu

Re: [PATCH net-next 0/2] Convert Ocelot and Felix switches to PHYLINK

[Vladimir Oltean]

On Wed, 20 Nov 2019 at 14:08, Horatiu Vultur
<horatiu.vultur@microchip.com> wrote:
>
> The 11/19/2019 22:42, Andrew Lunn wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > > Before this commit it was ok to use PHY_INTERFACE_MODE_NA but now that
> > > is not true anymore. In this case we have 4 ports that have phy and
> > > then 6 sfp ports. So I was looking to describe this in DT but without
> > > any success. If you have any advice that would be great.
> >
> > Is it the copper ports causing the trouble, or the SFP?  Ideally, you
> > should describe the SFPs as SFPs. But i don't think the driver has the
> > needed support for that yet. So you might need to use fixed-link for
> > the moment.
>
> It was both of them. So I have done few small changes to these patches.
> - first I added the phy-mode in DT on the interfaces that have a
>   phy(internal or external)
> - add a check for PHY_INTERFACE_MODE_NA before the port is probed so it
>   would not create net device if the phy mode is PHY_INTERFACE_MODE_NA
>   because in that case the phylink was not created.
>
> With these changes now only the ports that have phy are probed. This is
> the same behaviour as before these patches. I have tried to configure
> the sfp ports as fixed-links but unfortunetly it didn't work, I think
> because of some missconfiguration on MAC or SerDes, which I need to
> figure out. But I think this can be fix in a different patch.
>
> I have done few tests and they seem to work fine.
> Here are my changes.
>
> diff --git a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts
> index 33991fd209f5..0800a86b7f16 100644
> --- a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts
> +++ b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts
> @@ -60,18 +60,22 @@
>
>  &port0 {
>         phy-handle = <&phy0>;
> +       phy-mode = "sgmii";
>  };
>
>  &port1 {
>         phy-handle = <&phy1>;
> +       phy-mode = "sgmii";
>  };
>
>  &port2 {
>         phy-handle = <&phy2>;
> +       phy-mode = "sgmii";
>  };
>
>  &port3 {
>         phy-handle = <&phy3>;
> +       phy-mode = "sgmii";
>  };
>
>  &port4 {
> diff --git a/arch/mips/boot/dts/mscc/ocelot_pcb123.dts b/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
> index ef852f382da8..6b0b1fb358ad 100644
> --- a/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
> +++ b/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
> @@ -47,17 +47,21 @@
>  };
>
>  &port0 {
> +       phy-mode = "sgmii";
>         phy-handle = <&phy0>;
>  };
>
>  &port1 {
> +       phy-mode = "sgmii";
>         phy-handle = <&phy1>;
>  };
>
>  &port2 {
> +       phy-mode = "sgmii";
>         phy-handle = <&phy2>;
>  };
>
>  &port3 {
> +       phy-mode = "sgmii";
>         phy-handle = <&phy3>;
>  };
> diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c
> index aecaf4ef6ef4..9dad031900b5 100644
> --- a/drivers/net/ethernet/mscc/ocelot_board.c
> +++ b/drivers/net/ethernet/mscc/ocelot_board.c
> @@ -513,6 +513,10 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
>                 if (IS_ERR(regs))
>                         continue;
>
> +               of_get_phy_mode(portnp, &phy_mode);
> +               if (phy_mode == PHY_INTERFACE_MODE_NA)
> +                       continue;
> +

So this effectively reverts your own patch 4214fa1efffd ("net: mscc:
ocelot: omit error check from of_get_phy_mode")?

>                 err = ocelot_probe_port(ocelot, port, regs);
>                 if (err) {
>                         of_node_put(portnp);
> @@ -523,11 +527,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
>                 priv = container_of(ocelot_port, struct ocelot_port_private,
>                                     port);
>
> -               of_get_phy_mode(portnp, &phy_mode);
> -
>                 switch (phy_mode) {
> -               case PHY_INTERFACE_MODE_NA:
> -                       continue;
>                 case PHY_INTERFACE_MODE_SGMII:
>                         break;
>                 case PHY_INTERFACE_MODE_QSGMII:
> @@ -549,20 +549,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
>                 }
>
>                 serdes = devm_of_phy_get(ocelot->dev, portnp, NULL);
> -               if (IS_ERR(serdes)) {
> -                       err = PTR_ERR(serdes);
> -                       if (err == -EPROBE_DEFER)
> -                               dev_dbg(ocelot->dev, "deferring probe\n");

Why did you remove the probe deferral for the serdes phy?

> -                       else
> -                               dev_err(ocelot->dev,
> -                                       "missing SerDes phys for port%d\n",
> -                                       port);
> -
> -                       of_node_put(portnp);
> -                       goto out_put_ports;
> -               }
> -
> -               if (serdes) {
> +               if (!IS_ERR(serdes)) {
>                         err = phy_set_mode_ext(serdes, PHY_MODE_ETHERNET,
>                                                phy_mode);
>                         if (err) {
> --
> 2.17.1
>
>
> >
> >    Andrew
>
> --
> /Horatiu

Thanks,
-Vladimir

Re: [PATCH net-next 0/2] Convert Ocelot and Felix switches to PHYLINK

[Horatiu Vultur]

> >  };
> >
> >  &port0 {
> > +       phy-mode = "sgmii";
> >         phy-handle = <&phy0>;
> >  };
> >
> >  &port1 {
> > +       phy-mode = "sgmii";
> >         phy-handle = <&phy1>;
> >  };
> >
> >  &port2 {
> > +       phy-mode = "sgmii";
> >         phy-handle = <&phy2>;
> >  };
> >
> >  &port3 {
> > +       phy-mode = "sgmii";
> >         phy-handle = <&phy3>;
> >  };
> > diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c
> > index aecaf4ef6ef4..9dad031900b5 100644
> > --- a/drivers/net/ethernet/mscc/ocelot_board.c
> > +++ b/drivers/net/ethernet/mscc/ocelot_board.c
> > @@ -513,6 +513,10 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
> >                 if (IS_ERR(regs))
> >                         continue;
> >
> > +               of_get_phy_mode(portnp, &phy_mode);
> > +               if (phy_mode == PHY_INTERFACE_MODE_NA)
> > +                       continue;
> > +
> 
> So this effectively reverts your own patch 4214fa1efffd ("net: mscc:
> ocelot: omit error check from of_get_phy_mode")?

Not really, at that point it was OK to have interface
PHY_INTERFACE_MODE_NA. There were few more checks before creating the
network device. Now with your changes you were creating
a network device for each port of the soc even if some ports
were not used on a board. Also with your changes you first create the
port and after that you create the phylink but between these two calls it
was the switch which continue for the interface PHY_INTERFACE_MODE_NA,
which is not correct. So these are the 2 reason why I have added the
property phy-mode to the ports and add back the check to see which ports
are used on each board.

> 
> >                 err = ocelot_probe_port(ocelot, port, regs);
> >                 if (err) {
> >                         of_node_put(portnp);
> > @@ -523,11 +527,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
> >                 priv = container_of(ocelot_port, struct ocelot_port_private,
> >                                     port);
> >
> > -               of_get_phy_mode(portnp, &phy_mode);
> > -
> >                 switch (phy_mode) {
> > -               case PHY_INTERFACE_MODE_NA:
> > -                       continue;
> >                 case PHY_INTERFACE_MODE_SGMII:
> >                         break;
> >                 case PHY_INTERFACE_MODE_QSGMII:
> > @@ -549,20 +549,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
> >                 }
> >
> >                 serdes = devm_of_phy_get(ocelot->dev, portnp, NULL);
> > -               if (IS_ERR(serdes)) {
> > -                       err = PTR_ERR(serdes);
> > -                       if (err == -EPROBE_DEFER)
> > -                               dev_dbg(ocelot->dev, "deferring probe\n");
> 
> Why did you remove the probe deferral for the serdes phy?
Because not all the ports have the "phys" property.
> 
> > -                       else
> > -                               dev_err(ocelot->dev,
> > -                                       "missing SerDes phys for port%d\n",
> > -                                       port);
> > -
> > -                       of_node_put(portnp);
> > -                       goto out_put_ports;
> > -               }
> > -
> > -               if (serdes) {
> > +               if (!IS_ERR(serdes)) {
> >                         err = phy_set_mode_ext(serdes, PHY_MODE_ETHERNET,
> >                                                phy_mode);
> >                         if (err) {
> > --
> > 2.17.1
> >
> >
> > >
> > >    Andrew
> >
> > --
> > /Horatiu
> 
> Thanks,
> -Vladimir

-- 
/Horatiu

Re: [PATCH net-next 0/2] Convert Ocelot and Felix switches to PHYLINK

[Andrew Lunn]

> Not really, at that point it was OK to have interface
> PHY_INTERFACE_MODE_NA. There were few more checks before creating the
> network device. Now with your changes you were creating
> a network device for each port of the soc even if some ports
> were not used on a board.

That does not sound right. If the port is not used, the DSA core will
call port_disable() to allow the driver to power off the port. It will
not create a network device for it.

Or is this just an issue with the switchdev driver, not the DSA
driver?

	Andrew
> > >                 serdes = devm_of_phy_get(ocelot->dev, portnp, NULL);
> > > -               if (IS_ERR(serdes)) {
> > > -                       err = PTR_ERR(serdes);
> > > -                       if (err == -EPROBE_DEFER)
> > > -                               dev_dbg(ocelot->dev, "deferring probe\n");
> > 
> > Why did you remove the probe deferral for the serdes phy?
> Because not all the ports have the "phys" property.

You probably need to differentiate between ENODEV and EPROBE_DEFER.
You definitely do need to return EPROBE_DEFER if you get that.  Shame
you cannot use devm_phy_optional_get().

    Andrew

Re: [PATCH net-next 0/2] Convert Ocelot and Felix switches to PHYLINK

[Horatiu Vultur]

The 11/21/2019 01:18, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> > Not really, at that point it was OK to have interface
> > PHY_INTERFACE_MODE_NA. There were few more checks before creating the
> > network device. Now with your changes you were creating
> > a network device for each port of the soc even if some ports
> > were not used on a board.
> 
> That does not sound right. If the port is not used, the DSA core will
> call port_disable() to allow the driver to power off the port. It will
> not create a network device for it.
> 
> Or is this just an issue with the switchdev driver, not the DSA
> driver?
In my case I just use the switchdev driver. I don't have the DSA driver.

> 
>         Andrew
> > > >                 serdes = devm_of_phy_get(ocelot->dev, portnp, NULL);
> > > > -               if (IS_ERR(serdes)) {
> > > > -                       err = PTR_ERR(serdes);
> > > > -                       if (err == -EPROBE_DEFER)
> > > > -                               dev_dbg(ocelot->dev, "deferring probe\n");
> > >
> > > Why did you remove the probe deferral for the serdes phy?
> > Because not all the ports have the "phys" property.
> 
> You probably need to differentiate between ENODEV and EPROBE_DEFER.
> You definitely do need to return EPROBE_DEFER if you get that.
Thanks, I will keep it in mind.

> Shame you cannot use devm_phy_optional_get().
> 
>     Andrew

-- 
/Horatiu

Re: [PATCH net-next 0/2] Convert Ocelot and Felix switches to PHYLINK

[Vladimir Oltean]

On Thu, 21 Nov 2019 at 01:21, Horatiu Vultur
<horatiu.vultur@microchip.com> wrote:
>
> > >  };
> > >
> > >  &port0 {
> > > +       phy-mode = "sgmii";
> > >         phy-handle = <&phy0>;
> > >  };
> > >
> > >  &port1 {
> > > +       phy-mode = "sgmii";
> > >         phy-handle = <&phy1>;
> > >  };
> > >
> > >  &port2 {
> > > +       phy-mode = "sgmii";
> > >         phy-handle = <&phy2>;
> > >  };
> > >
> > >  &port3 {
> > > +       phy-mode = "sgmii";
> > >         phy-handle = <&phy3>;
> > >  };
> > > diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c
> > > index aecaf4ef6ef4..9dad031900b5 100644
> > > --- a/drivers/net/ethernet/mscc/ocelot_board.c
> > > +++ b/drivers/net/ethernet/mscc/ocelot_board.c
> > > @@ -513,6 +513,10 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
> > >                 if (IS_ERR(regs))
> > >                         continue;
> > >
> > > +               of_get_phy_mode(portnp, &phy_mode);
> > > +               if (phy_mode == PHY_INTERFACE_MODE_NA)
> > > +                       continue;
> > > +
> >
> > So this effectively reverts your own patch 4214fa1efffd ("net: mscc:
> > ocelot: omit error check from of_get_phy_mode")?
>
> Not really, at that point it was OK to have interface
> PHY_INTERFACE_MODE_NA. There were few more checks before creating the
> network device. Now with your changes you were creating
> a network device for each port of the soc even if some ports
> were not used on a board. Also with your changes you first create the
> port and after that you create the phylink but between these two calls it
> was the switch which continue for the interface PHY_INTERFACE_MODE_NA,
> which is not correct. So these are the 2 reason why I have added the
> property phy-mode to the ports and add back the check to see which ports
> are used on each board.
>
> >
> > >                 err = ocelot_probe_port(ocelot, port, regs);
> > >                 if (err) {
> > >                         of_node_put(portnp);
> > > @@ -523,11 +527,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
> > >                 priv = container_of(ocelot_port, struct ocelot_port_private,
> > >                                     port);
> > >
> > > -               of_get_phy_mode(portnp, &phy_mode);
> > > -
> > >                 switch (phy_mode) {
> > > -               case PHY_INTERFACE_MODE_NA:
> > > -                       continue;
> > >                 case PHY_INTERFACE_MODE_SGMII:
> > >                         break;
> > >                 case PHY_INTERFACE_MODE_QSGMII:
> > > @@ -549,20 +549,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
> > >                 }
> > >
> > >                 serdes = devm_of_phy_get(ocelot->dev, portnp, NULL);
> > > -               if (IS_ERR(serdes)) {
> > > -                       err = PTR_ERR(serdes);
> > > -                       if (err == -EPROBE_DEFER)
> > > -                               dev_dbg(ocelot->dev, "deferring probe\n");
> >
> > Why did you remove the probe deferral for the serdes phy?
> Because not all the ports have the "phys" property.
> >
> > > -                       else
> > > -                               dev_err(ocelot->dev,
> > > -                                       "missing SerDes phys for port%d\n",
> > > -                                       port);
> > > -
> > > -                       of_node_put(portnp);
> > > -                       goto out_put_ports;
> > > -               }
> > > -
> > > -               if (serdes) {
> > > +               if (!IS_ERR(serdes)) {
> > >                         err = phy_set_mode_ext(serdes, PHY_MODE_ETHERNET,
> > >                                                phy_mode);
> > >                         if (err) {
> > > --
> > > 2.17.1
> > >
> > >
> > > >
> > > >    Andrew
> > >
> > > --
> > > /Horatiu
> >
> > Thanks,
> > -Vladimir
>
> --
> /Horatiu

Horatiu,

Do the 10/100 speeds work over the SGMII ports on your board? (not
with this patch, but in general)

Thanks,
-Vladimir

Re: [PATCH net-next 0/2] Convert Ocelot and Felix switches to PHYLINK

[Horatiu Vultur]

The 11/21/2019 19:51, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Thu, 21 Nov 2019 at 01:21, Horatiu Vultur
> <horatiu.vultur@microchip.com> wrote:
> >
> > > >  };
> > > >
> > > >  &port0 {
> > > > +       phy-mode = "sgmii";
> > > >         phy-handle = <&phy0>;
> > > >  };
> > > >
> > > >  &port1 {
> > > > +       phy-mode = "sgmii";
> > > >         phy-handle = <&phy1>;
> > > >  };
> > > >
> > > >  &port2 {
> > > > +       phy-mode = "sgmii";
> > > >         phy-handle = <&phy2>;
> > > >  };
> > > >
> > > >  &port3 {
> > > > +       phy-mode = "sgmii";
> > > >         phy-handle = <&phy3>;
> > > >  };
> > > > diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c
> > > > index aecaf4ef6ef4..9dad031900b5 100644
> > > > --- a/drivers/net/ethernet/mscc/ocelot_board.c
> > > > +++ b/drivers/net/ethernet/mscc/ocelot_board.c
> > > > @@ -513,6 +513,10 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
> > > >                 if (IS_ERR(regs))
> > > >                         continue;
> > > >
> > > > +               of_get_phy_mode(portnp, &phy_mode);
> > > > +               if (phy_mode == PHY_INTERFACE_MODE_NA)
> > > > +                       continue;
> > > > +
> > >
> > > So this effectively reverts your own patch 4214fa1efffd ("net: mscc:
> > > ocelot: omit error check from of_get_phy_mode")?
> >
> > Not really, at that point it was OK to have interface
> > PHY_INTERFACE_MODE_NA. There were few more checks before creating the
> > network device. Now with your changes you were creating
> > a network device for each port of the soc even if some ports
> > were not used on a board. Also with your changes you first create the
> > port and after that you create the phylink but between these two calls it
> > was the switch which continue for the interface PHY_INTERFACE_MODE_NA,
> > which is not correct. So these are the 2 reason why I have added the
> > property phy-mode to the ports and add back the check to see which ports
> > are used on each board.
> >
> > >
> > > >                 err = ocelot_probe_port(ocelot, port, regs);
> > > >                 if (err) {
> > > >                         of_node_put(portnp);
> > > > @@ -523,11 +527,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
> > > >                 priv = container_of(ocelot_port, struct ocelot_port_private,
> > > >                                     port);
> > > >
> > > > -               of_get_phy_mode(portnp, &phy_mode);
> > > > -
> > > >                 switch (phy_mode) {
> > > > -               case PHY_INTERFACE_MODE_NA:
> > > > -                       continue;
> > > >                 case PHY_INTERFACE_MODE_SGMII:
> > > >                         break;
> > > >                 case PHY_INTERFACE_MODE_QSGMII:
> > > > @@ -549,20 +549,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
> > > >                 }
> > > >
> > > >                 serdes = devm_of_phy_get(ocelot->dev, portnp, NULL);
> > > > -               if (IS_ERR(serdes)) {
> > > > -                       err = PTR_ERR(serdes);
> > > > -                       if (err == -EPROBE_DEFER)
> > > > -                               dev_dbg(ocelot->dev, "deferring probe\n");
> > >
> > > Why did you remove the probe deferral for the serdes phy?
> > Because not all the ports have the "phys" property.
> > >
> > > > -                       else
> > > > -                               dev_err(ocelot->dev,
> > > > -                                       "missing SerDes phys for port%d\n",
> > > > -                                       port);
> > > > -
> > > > -                       of_node_put(portnp);
> > > > -                       goto out_put_ports;
> > > > -               }
> > > > -
> > > > -               if (serdes) {
> > > > +               if (!IS_ERR(serdes)) {
> > > >                         err = phy_set_mode_ext(serdes, PHY_MODE_ETHERNET,
> > > >                                                phy_mode);
> > > >                         if (err) {
> > > > --
> > > > 2.17.1
> > > >
> > > >
> > > > >
> > > > >    Andrew
> > > >
> > > > --
> > > > /Horatiu
> > >
> > > Thanks,
> > > -Vladimir
> >
> > --
> > /Horatiu
> 
> Horatiu,
> 
> Do the 10/100 speeds work over the SGMII ports on your board? (not
> with this patch, but in general)

I have done a mistake the interface is not SGMII but GMII for the 4
internal phys. Sorry for the confusion.
I can test on Monday to see if it is working with speed 10/100 on SGMII
ports if are still interested.

> 
> Thanks,
> -Vladimir

-- 
/Horatiu

Re: [PATCH net-next 0/2] Convert Ocelot and Felix switches to PHYLINK

[David Miller]

From: Vladimir Oltean <olteanv@gmail.com>
Date: Mon, 18 Nov 2019 20:10:28 +0200

> This series is needed on NXP LS1028A to support the CPU port which runs
> at 2500Mbps fixed-link, a setting which PHYLIB can't hold in its swphy
> design.
> 
> In DSA, PHYLINK comes "for free". I added the PHYLINK ops to the Ocelot
> driver, integrated them to the VSC7514 ocelot_board module, then tested
> them via the Felix front-end. The VSC7514 integration is only
> compile-tested.

Please sort out the crash that was reported and resubmit, thank you.