[PATCH net-next 0/5] enetc: Link mode init w/o bootloader

[PATCH net-next 0/5] enetc: Link mode init w/o bootloader

[Claudiu Manoil]

The theme of this set is to clear the dependency on bootloader
for PHY link mode protocol init (i.e. SGMII, SXGMII) and MAC
configuration for the ENETC controller.

First patch fixes the DT extracted PHY mode handling.
The second one is a refactoring that prepares the introduction
of the internal MDIO bus.
Internal MDIO bus support is added along with SerDes protocol
configuration routines (3rd patch).
Then after a minor cleanup (patch 4), DT link mode information
is being used to configure the MAC instead of relying on
bootloader configurations.

Alex Marginean (1):
  enetc: Use DT protocol information to set up the ports

Claudiu Manoil (4):
  enetc: Fix if_mode extraction
  enetc: Make mdio accessors more generic
  enetc: Initialize SerDes for SGMII and SXGMII protocols
  enetc: Drop redundant device node check

 .../net/ethernet/freescale/enetc/enetc_hw.h   |  18 +++
 .../net/ethernet/freescale/enetc/enetc_mdio.c |  91 +++++++++----
 .../net/ethernet/freescale/enetc/enetc_mdio.h |   2 +-
 .../ethernet/freescale/enetc/enetc_pci_mdio.c |   2 +
 .../net/ethernet/freescale/enetc/enetc_pf.c   | 127 +++++++++++++-----
 .../net/ethernet/freescale/enetc/enetc_pf.h   |   5 +
 6 files changed, 182 insertions(+), 63 deletions(-)

-- 
2.17.1

[PATCH net-next 1/5] enetc: Fix if_mode extraction

[Claudiu Manoil]

Fix handling of error return code. Before this fix,
the error code was handled as unsigned type.
Also, on this path if if_mode not found then just handle
it as fixed link (i.e mac2mac connection).

Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc_pf.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 7d6513ff8507..3a556646a2fb 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -751,6 +751,7 @@ static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
 	struct enetc_pf *pf = enetc_si_priv(priv->si);
 	struct device_node *np = priv->dev->of_node;
 	struct device_node *mdio_np;
+	int phy_mode;
 	int err;
 
 	if (!np) {
@@ -784,17 +785,11 @@ static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
 		}
 	}
 
-	priv->if_mode = of_get_phy_mode(np);
-	if (priv->if_mode < 0) {
-		dev_err(priv->dev, "missing phy type\n");
-		of_node_put(priv->phy_node);
-		if (of_phy_is_fixed_link(np))
-			of_phy_deregister_fixed_link(np);
-		else
-			enetc_mdio_remove(pf);
-
-		return -EINVAL;
-	}
+	phy_mode = of_get_phy_mode(np);
+	if (phy_mode < 0)
+		priv->if_mode = PHY_INTERFACE_MODE_NA; /* fixed link */
+	else
+		priv->if_mode = phy_mode;
 
 	return 0;
 }
-- 
2.17.1

[PATCH net-next 2/5] enetc: Make mdio accessors more generic

[Claudiu Manoil]

Refactoring needed to support multiple MDIO buses.
'mdio_base' - MDIO registers base address - is being parameterized.
The MDIO accessors are made more generic to be able to work with
different MDIO register bases.
Some includes get cleaned up in the process.

Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
 .../net/ethernet/freescale/enetc/enetc_hw.h   |  1 +
 .../net/ethernet/freescale/enetc/enetc_mdio.c | 60 +++++++++++--------
 .../net/ethernet/freescale/enetc/enetc_mdio.h |  2 +-
 .../ethernet/freescale/enetc/enetc_pci_mdio.c |  2 +
 4 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
index 88276299f447..534de211b243 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
@@ -192,6 +192,7 @@ enum enetc_bdr_type {TX, RX};
 #define ENETC_PFPMR		0x1900
 #define ENETC_PFPMR_PMACE	BIT(1)
 #define ENETC_PFPMR_MWLM	BIT(0)
+#define ENETC_EMDIO_BASE	0x1c00
 #define ENETC_PSIUMHFR0(n, err)	(((err) ? 0x1d08 : 0x1d00) + (n) * 0x10)
 #define ENETC_PSIUMHFR1(n)	(0x1d04 + (n) * 0x10)
 #define ENETC_PSIMMHFR0(n, err)	(((err) ? 0x1d00 : 0x1d08) + (n) * 0x10)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
index 149883c8f0b8..c9a27e7fe5a7 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
@@ -6,19 +6,30 @@
 #include <linux/iopoll.h>
 #include <linux/of.h>
 
+#include "enetc_pf.h"
 #include "enetc_mdio.h"
 
-#define	ENETC_MDIO_REG_OFFSET	0x1c00
 #define	ENETC_MDIO_CFG	0x0	/* MDIO configuration and status */
 #define	ENETC_MDIO_CTL	0x4	/* MDIO control */
 #define	ENETC_MDIO_DATA	0x8	/* MDIO data */
 #define	ENETC_MDIO_ADDR	0xc	/* MDIO address */
 
-#define enetc_mdio_rd(hw, off) \
-	enetc_port_rd(hw, ENETC_##off + ENETC_MDIO_REG_OFFSET)
-#define enetc_mdio_wr(hw, off, val) \
-	enetc_port_wr(hw, ENETC_##off + ENETC_MDIO_REG_OFFSET, val)
-#define enetc_mdio_rd_reg(off)	enetc_mdio_rd(hw, off)
+static inline u32 _enetc_mdio_rd(struct enetc_mdio_priv *mdio_priv, int off)
+{
+	return enetc_port_rd(mdio_priv->hw, mdio_priv->mdio_base + off);
+}
+
+static inline void _enetc_mdio_wr(struct enetc_mdio_priv *mdio_priv, int off,
+				  u32 val)
+{
+	enetc_port_wr(mdio_priv->hw, mdio_priv->mdio_base + off, val);
+}
+
+#define enetc_mdio_rd(mdio_priv, off) \
+	_enetc_mdio_rd(mdio_priv, ENETC_##off)
+#define enetc_mdio_wr(mdio_priv, off, val) \
+	_enetc_mdio_wr(mdio_priv, ENETC_##off, val)
+#define enetc_mdio_rd_reg(off)	enetc_mdio_rd(mdio_priv, off)
 
 #define ENETC_MDC_DIV		258
 
@@ -35,7 +46,7 @@
 #define MDIO_DATA(x)		((x) & 0xffff)
 
 #define TIMEOUT	1000
-static int enetc_mdio_wait_complete(struct enetc_hw *hw)
+static int enetc_mdio_wait_complete(struct enetc_mdio_priv *mdio_priv)
 {
 	u32 val;
 
@@ -46,7 +57,6 @@ static int enetc_mdio_wait_complete(struct enetc_hw *hw)
 int enetc_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 value)
 {
 	struct enetc_mdio_priv *mdio_priv = bus->priv;
-	struct enetc_hw *hw = mdio_priv->hw;
 	u32 mdio_ctl, mdio_cfg;
 	u16 dev_addr;
 	int ret;
@@ -61,29 +71,29 @@ int enetc_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 value)
 		mdio_cfg &= ~MDIO_CFG_ENC45;
 	}
 
-	enetc_mdio_wr(hw, MDIO_CFG, mdio_cfg);
+	enetc_mdio_wr(mdio_priv, MDIO_CFG, mdio_cfg);
 
-	ret = enetc_mdio_wait_complete(hw);
+	ret = enetc_mdio_wait_complete(mdio_priv);
 	if (ret)
 		return ret;
 
 	/* set port and dev addr */
 	mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
-	enetc_mdio_wr(hw, MDIO_CTL, mdio_ctl);
+	enetc_mdio_wr(mdio_priv, MDIO_CTL, mdio_ctl);
 
 	/* set the register address */
 	if (regnum & MII_ADDR_C45) {
-		enetc_mdio_wr(hw, MDIO_ADDR, regnum & 0xffff);
+		enetc_mdio_wr(mdio_priv, MDIO_ADDR, regnum & 0xffff);
 
-		ret = enetc_mdio_wait_complete(hw);
+		ret = enetc_mdio_wait_complete(mdio_priv);
 		if (ret)
 			return ret;
 	}
 
 	/* write the value */
-	enetc_mdio_wr(hw, MDIO_DATA, MDIO_DATA(value));
+	enetc_mdio_wr(mdio_priv, MDIO_DATA, MDIO_DATA(value));
 
-	ret = enetc_mdio_wait_complete(hw);
+	ret = enetc_mdio_wait_complete(mdio_priv);
 	if (ret)
 		return ret;
 
@@ -93,7 +103,6 @@ int enetc_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 value)
 int enetc_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
 {
 	struct enetc_mdio_priv *mdio_priv = bus->priv;
-	struct enetc_hw *hw = mdio_priv->hw;
 	u32 mdio_ctl, mdio_cfg;
 	u16 dev_addr, value;
 	int ret;
@@ -107,41 +116,41 @@ int enetc_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
 		mdio_cfg &= ~MDIO_CFG_ENC45;
 	}
 
-	enetc_mdio_wr(hw, MDIO_CFG, mdio_cfg);
+	enetc_mdio_wr(mdio_priv, MDIO_CFG, mdio_cfg);
 
-	ret = enetc_mdio_wait_complete(hw);
+	ret = enetc_mdio_wait_complete(mdio_priv);
 	if (ret)
 		return ret;
 
 	/* set port and device addr */
 	mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
-	enetc_mdio_wr(hw, MDIO_CTL, mdio_ctl);
+	enetc_mdio_wr(mdio_priv, MDIO_CTL, mdio_ctl);
 
 	/* set the register address */
 	if (regnum & MII_ADDR_C45) {
-		enetc_mdio_wr(hw, MDIO_ADDR, regnum & 0xffff);
+		enetc_mdio_wr(mdio_priv, MDIO_ADDR, regnum & 0xffff);
 
-		ret = enetc_mdio_wait_complete(hw);
+		ret = enetc_mdio_wait_complete(mdio_priv);
 		if (ret)
 			return ret;
 	}
 
 	/* initiate the read */
-	enetc_mdio_wr(hw, MDIO_CTL, mdio_ctl | MDIO_CTL_READ);
+	enetc_mdio_wr(mdio_priv, MDIO_CTL, mdio_ctl | MDIO_CTL_READ);
 
-	ret = enetc_mdio_wait_complete(hw);
+	ret = enetc_mdio_wait_complete(mdio_priv);
 	if (ret)
 		return ret;
 
 	/* return all Fs if nothing was there */
-	if (enetc_mdio_rd(hw, MDIO_CFG) & MDIO_CFG_RD_ER) {
+	if (enetc_mdio_rd(mdio_priv, MDIO_CFG) & MDIO_CFG_RD_ER) {
 		dev_dbg(&bus->dev,
 			"Error while reading PHY%d reg at %d.%hhu\n",
 			phy_id, dev_addr, regnum);
 		return 0xffff;
 	}
 
-	value = enetc_mdio_rd(hw, MDIO_DATA) & 0xffff;
+	value = enetc_mdio_rd(mdio_priv, MDIO_DATA) & 0xffff;
 
 	return value;
 }
@@ -164,6 +173,7 @@ int enetc_mdio_probe(struct enetc_pf *pf)
 	bus->parent = dev;
 	mdio_priv = bus->priv;
 	mdio_priv->hw = &pf->si->hw;
+	mdio_priv->mdio_base = ENETC_EMDIO_BASE;
 	snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
 
 	np = of_get_child_by_name(dev->of_node, "mdio");
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_mdio.h b/drivers/net/ethernet/freescale/enetc/enetc_mdio.h
index 60c9a3889824..a8ea3607c7bf 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_mdio.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_mdio.h
@@ -2,10 +2,10 @@
 /* Copyright 2019 NXP */
 
 #include <linux/phy.h>
-#include "enetc_pf.h"
 
 struct enetc_mdio_priv {
 	struct enetc_hw *hw;
+	int mdio_base;
 };
 
 int enetc_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 value);
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pci_mdio.c b/drivers/net/ethernet/freescale/enetc/enetc_pci_mdio.c
index fbd41ce01f06..e12159ac1fa6 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pci_mdio.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pci_mdio.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
 /* Copyright 2019 NXP */
 #include <linux/of_mdio.h>
+#include "enetc_pf.h"
 #include "enetc_mdio.h"
 
 #define ENETC_MDIO_DEV_ID	0xee01
@@ -31,6 +32,7 @@ static int enetc_pci_mdio_probe(struct pci_dev *pdev,
 	bus->parent = dev;
 	mdio_priv = bus->priv;
 	mdio_priv->hw = hw;
+	mdio_priv->mdio_base = ENETC_EMDIO_BASE;
 	snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
 
 	pcie_flr(pdev);
-- 
2.17.1

[PATCH net-next 3/5] enetc: Initialize SerDes for SGMII and SXGMII protocols

[Claudiu Manoil]

ENETC has ethernet MACs capable of SGMII and SXGMII but
in order to use these protocols some serdes configurations
need to be performed.
The serdes is configurable via an internal MDIO bus
connected to an internal PCS device, all reads/writes are
performed at address 0.
This patch basically removes the dependency on bootloader
regarding serdes initialization.

Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
 .../net/ethernet/freescale/enetc/enetc_hw.h   | 17 ++++++
 .../net/ethernet/freescale/enetc/enetc_mdio.c | 31 ++++++++++
 .../net/ethernet/freescale/enetc/enetc_pf.c   | 58 +++++++++++++++++++
 .../net/ethernet/freescale/enetc/enetc_pf.h   |  2 +
 4 files changed, 108 insertions(+)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
index 534de211b243..ced3693dabdb 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
@@ -215,6 +215,23 @@ enum enetc_bdr_type {TX, RX};
 #define ENETC_PM0_MAXFRM	0x8014
 #define ENETC_SET_TX_MTU(val)	((val) << 16)
 #define ENETC_SET_MAXFRM(val)	((val) & 0xffff)
+
+#define ENETC_PM_IMDIO_BASE	0x8030
+/* PCS registers */
+#define ENETC_PCS_CR			0x0
+#define ENETC_PCS_CR_RESET_AN		0x1200
+#define ENETC_PCS_CR_DEF_VAL		0x0140
+#define ENETC_PCS_CR_LANE_RESET		0x8000
+#define ENETC_PCS_DEV_ABILITY		0x04
+#define ENETC_PCS_DEV_ABILITY_SGMII	0x4001
+#define ENETC_PCS_DEV_ABILITY_SXGMII	0x5001
+#define ENETC_PCS_LINK_TIMER1		0x12
+#define ENETC_PCS_LINK_TIMER1_VAL	0x06a0
+#define ENETC_PCS_LINK_TIMER2		0x13
+#define ENETC_PCS_LINK_TIMER2_VAL	0x0003
+#define ENETC_PCS_IF_MODE		0x14
+#define ENETC_PCS_IF_MODE_SGMII_AN	0x0003
+
 #define ENETC_PM0_IF_MODE	0x8300
 #define ENETC_PMO_IFM_RG	BIT(2)
 #define ENETC_PM0_IFM_RLP	(BIT(5) | BIT(11))
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
index c9a27e7fe5a7..0190fcb0c371 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
@@ -200,3 +200,34 @@ void enetc_mdio_remove(struct enetc_pf *pf)
 	if (pf->mdio)
 		mdiobus_unregister(pf->mdio);
 }
+
+int enetc_imdio_init(struct enetc_pf *pf)
+{
+	struct device *dev = &pf->si->pdev->dev;
+	struct enetc_mdio_priv *mdio_priv;
+	struct mii_bus *bus;
+	int err;
+
+	bus = devm_mdiobus_alloc_size(dev, sizeof(*mdio_priv));
+	if (!bus)
+		return -ENOMEM;
+
+	bus->name = "FSL ENETC internal MDIO Bus";
+	bus->read = enetc_mdio_read;
+	bus->write = enetc_mdio_write;
+	bus->parent = dev;
+	mdio_priv = bus->priv;
+	mdio_priv->hw = &pf->si->hw;
+	mdio_priv->mdio_base = ENETC_PM_IMDIO_BASE;
+	snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
+
+	err = mdiobus_register(bus);
+	if (err) {
+		dev_err(dev, "cannot register internal MDIO bus\n");
+		return err;
+	}
+
+	pf->imdio = bus;
+
+	return 0;
+}
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 3a556646a2fb..9e9bb6b97c41 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -804,6 +804,60 @@ static void enetc_of_put_phy(struct enetc_ndev_priv *priv)
 		of_node_put(priv->phy_node);
 }
 
+static void enetc_configure_sgmii(struct mii_bus *imdio)
+{
+	/* Set to SGMII mode, use AN */
+	imdio->write(imdio, 0, ENETC_PCS_IF_MODE,
+		     ENETC_PCS_IF_MODE_SGMII_AN);
+
+	/* Dev ability - SGMII */
+	imdio->write(imdio, 0, ENETC_PCS_DEV_ABILITY,
+		     ENETC_PCS_DEV_ABILITY_SGMII);
+
+	/* Adjust link timer for SGMII */
+	imdio->write(imdio, 0, ENETC_PCS_LINK_TIMER1,
+		     ENETC_PCS_LINK_TIMER1_VAL);
+	imdio->write(imdio, 0, ENETC_PCS_LINK_TIMER2,
+		     ENETC_PCS_LINK_TIMER2_VAL);
+
+	/* restart PCS AN */
+	imdio->write(imdio, 0, ENETC_PCS_CR,
+		     ENETC_PCS_CR_RESET_AN | ENETC_PCS_CR_DEF_VAL);
+}
+
+static void enetc_configure_sxgmii(struct mii_bus *imdio)
+{
+	/* Dev ability - SXGMII */
+	imdio->write(imdio, 0, ENETC_PCS_DEV_ABILITY | MII_ADDR_C45,
+		     ENETC_PCS_DEV_ABILITY_SXGMII);
+
+	/* Restart PCS AN */
+	imdio->write(imdio, 0, ENETC_PCS_CR | MII_ADDR_C45,
+		     ENETC_PCS_CR_LANE_RESET | ENETC_PCS_CR_RESET_AN);
+}
+
+static int enetc_configure_serdes(struct enetc_ndev_priv *priv)
+{
+	struct enetc_pf *pf = enetc_si_priv(priv->si);
+	int err;
+
+	if (priv->if_mode != PHY_INTERFACE_MODE_SGMII &&
+	    priv->if_mode != PHY_INTERFACE_MODE_XGMII)
+		return 0;
+
+	err = enetc_imdio_init(pf);
+	if (err)
+		return err;
+
+	if (priv->if_mode == PHY_INTERFACE_MODE_SGMII)
+		enetc_configure_sgmii(pf->imdio);
+
+	if (priv->if_mode == PHY_INTERFACE_MODE_XGMII)
+		enetc_configure_sxgmii(pf->imdio);
+
+	return 0;
+}
+
 static int enetc_pf_probe(struct pci_dev *pdev,
 			  const struct pci_device_id *ent)
 {
@@ -868,6 +922,10 @@ static int enetc_pf_probe(struct pci_dev *pdev,
 	if (err)
 		dev_warn(&pdev->dev, "Fallback to PHY-less operation\n");
 
+	err = enetc_configure_serdes(priv);
+	if (err)
+		dev_warn(&pdev->dev, "Attempted serdes config but failed\n");
+
 	err = register_netdev(ndev);
 	if (err)
 		goto err_reg_netdev;
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.h b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
index 10dd1b53bb08..1357cdb71d64 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
@@ -44,6 +44,7 @@ struct enetc_pf {
 	DECLARE_BITMAP(active_vlans, VLAN_N_VID);
 
 	struct mii_bus *mdio; /* saved for cleanup */
+	struct mii_bus *imdio;
 };
 
 int enetc_msg_psi_init(struct enetc_pf *pf);
@@ -53,3 +54,4 @@ void enetc_msg_handle_rxmsg(struct enetc_pf *pf, int mbox_id, u16 *status);
 /* MDIO */
 int enetc_mdio_probe(struct enetc_pf *pf);
 void enetc_mdio_remove(struct enetc_pf *pf);
+int enetc_imdio_init(struct enetc_pf *pf);
-- 
2.17.1

[PATCH net-next 4/5] enetc: Drop redundant device node check

[Claudiu Manoil]

The existence of the DT port node is the first thing checked
at probe time, and probing won't continue if the node is missing.

Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc_pf.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 9e9bb6b97c41..ebf2996ebe69 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -754,11 +754,6 @@ static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
 	int phy_mode;
 	int err;
 
-	if (!np) {
-		dev_err(priv->dev, "missing ENETC port node\n");
-		return -ENODEV;
-	}
-
 	priv->phy_node = of_parse_phandle(np, "phy-handle", 0);
 	if (!priv->phy_node) {
 		if (!of_phy_is_fixed_link(np)) {
-- 
2.17.1

[PATCH net-next 5/5] enetc: Use DT protocol information to set up the ports

[Claudiu Manoil]

From: Alex Marginean <alexandru.marginean@nxp.com>

Use DT information rather than in-band information from bootloader to
set up MAC for XGMII. For RGMII use the DT indication in addition to
RGMII defaults in hardware.
However, this implies that PHY connection information needs to be
extracted before netdevice creation, when the ENETC Port MAC is
being configured.

Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
 .../net/ethernet/freescale/enetc/enetc_pf.c   | 55 ++++++++++---------
 .../net/ethernet/freescale/enetc/enetc_pf.h   |  3 +
 2 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index ebf2996ebe69..dbe9515a89b1 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -507,7 +507,8 @@ static void enetc_port_si_configure(struct enetc_si *si)
 	enetc_port_wr(hw, ENETC_PSIVLANFMR, ENETC_PSIVLANFMR_VS);
 }
 
-static void enetc_configure_port_mac(struct enetc_hw *hw)
+static void enetc_configure_port_mac(struct enetc_hw *hw,
+				     phy_interface_t phy_mode)
 {
 	enetc_port_wr(hw, ENETC_PM0_MAXFRM,
 		      ENETC_SET_MAXFRM(ENETC_RX_MAXFRM_SIZE));
@@ -523,9 +524,11 @@ static void enetc_configure_port_mac(struct enetc_hw *hw)
 		      ENETC_PM0_CMD_TXP	| ENETC_PM0_PROMISC |
 		      ENETC_PM0_TX_EN | ENETC_PM0_RX_EN);
 	/* set auto-speed for RGMII */
-	if (enetc_port_rd(hw, ENETC_PM0_IF_MODE) & ENETC_PMO_IFM_RG)
+	if (enetc_port_rd(hw, ENETC_PM0_IF_MODE) & ENETC_PMO_IFM_RG ||
+	    phy_mode == PHY_INTERFACE_MODE_RGMII)
 		enetc_port_wr(hw, ENETC_PM0_IF_MODE, ENETC_PM0_IFM_RGAUTO);
-	if (enetc_global_rd(hw, ENETC_G_EPFBLPR(1)) == ENETC_G_EPFBLPR1_XGMII)
+
+	if (phy_mode == PHY_INTERFACE_MODE_XGMII)
 		enetc_port_wr(hw, ENETC_PM0_IF_MODE, ENETC_PM0_IFM_XGMII);
 }
 
@@ -549,7 +552,7 @@ static void enetc_configure_port(struct enetc_pf *pf)
 
 	enetc_configure_port_pmac(hw);
 
-	enetc_configure_port_mac(hw);
+	enetc_configure_port_mac(hw, pf->if_mode);
 
 	enetc_port_si_configure(pf->si);
 
@@ -746,28 +749,28 @@ static void enetc_pf_netdev_setup(struct enetc_si *si, struct net_device *ndev,
 	enetc_get_primary_mac_addr(&si->hw, ndev->dev_addr);
 }
 
-static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
+static int enetc_of_get_phy(struct enetc_pf *pf)
 {
-	struct enetc_pf *pf = enetc_si_priv(priv->si);
-	struct device_node *np = priv->dev->of_node;
+	struct device *dev = &pf->si->pdev->dev;
+	struct device_node *np = dev->of_node;
 	struct device_node *mdio_np;
 	int phy_mode;
 	int err;
 
-	priv->phy_node = of_parse_phandle(np, "phy-handle", 0);
-	if (!priv->phy_node) {
+	pf->phy_node = of_parse_phandle(np, "phy-handle", 0);
+	if (!pf->phy_node) {
 		if (!of_phy_is_fixed_link(np)) {
-			dev_err(priv->dev, "PHY not specified\n");
+			dev_err(dev, "PHY not specified\n");
 			return -ENODEV;
 		}
 
 		err = of_phy_register_fixed_link(np);
 		if (err < 0) {
-			dev_err(priv->dev, "fixed link registration failed\n");
+			dev_err(dev, "fixed link registration failed\n");
 			return err;
 		}
 
-		priv->phy_node = of_node_get(np);
+		pf->phy_node = of_node_get(np);
 	}
 
 	mdio_np = of_get_child_by_name(np, "mdio");
@@ -775,28 +778,28 @@ static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
 		of_node_put(mdio_np);
 		err = enetc_mdio_probe(pf);
 		if (err) {
-			of_node_put(priv->phy_node);
+			of_node_put(pf->phy_node);
 			return err;
 		}
 	}
 
 	phy_mode = of_get_phy_mode(np);
 	if (phy_mode < 0)
-		priv->if_mode = PHY_INTERFACE_MODE_NA; /* fixed link */
+		pf->if_mode = PHY_INTERFACE_MODE_NA; /* fixed link */
 	else
-		priv->if_mode = phy_mode;
+		pf->if_mode = phy_mode;
 
 	return 0;
 }
 
-static void enetc_of_put_phy(struct enetc_ndev_priv *priv)
+static void enetc_of_put_phy(struct enetc_pf *pf)
 {
-	struct device_node *np = priv->dev->of_node;
+	struct device_node *np = pf->si->pdev->dev.of_node;
 
 	if (np && of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
-	if (priv->phy_node)
-		of_node_put(priv->phy_node);
+	if (pf->phy_node)
+		of_node_put(pf->phy_node);
 }
 
 static void enetc_configure_sgmii(struct mii_bus *imdio)
@@ -884,6 +887,10 @@ static int enetc_pf_probe(struct pci_dev *pdev,
 	pf->si = si;
 	pf->total_vfs = pci_sriov_get_totalvfs(pdev);
 
+	err = enetc_of_get_phy(pf);
+	if (err)
+		dev_warn(&pdev->dev, "Fallback to PHY-less operation\n");
+
 	enetc_configure_port(pf);
 
 	enetc_get_si_caps(si);
@@ -898,6 +905,8 @@ static int enetc_pf_probe(struct pci_dev *pdev,
 	enetc_pf_netdev_setup(si, ndev, &enetc_ndev_ops);
 
 	priv = netdev_priv(ndev);
+	priv->phy_node = pf->phy_node;
+	priv->if_mode = pf->if_mode;
 
 	enetc_init_si_rings_params(priv);
 
@@ -913,10 +922,6 @@ static int enetc_pf_probe(struct pci_dev *pdev,
 		goto err_alloc_msix;
 	}
 
-	err = enetc_of_get_phy(priv);
-	if (err)
-		dev_warn(&pdev->dev, "Fallback to PHY-less operation\n");
-
 	err = enetc_configure_serdes(priv);
 	if (err)
 		dev_warn(&pdev->dev, "Attempted serdes config but failed\n");
@@ -933,7 +938,6 @@ static int enetc_pf_probe(struct pci_dev *pdev,
 	return 0;
 
 err_reg_netdev:
-	enetc_of_put_phy(priv);
 	enetc_free_msix(priv);
 err_alloc_msix:
 	enetc_free_si_resources(priv);
@@ -941,6 +945,7 @@ static int enetc_pf_probe(struct pci_dev *pdev,
 	si->ndev = NULL;
 	free_netdev(ndev);
 err_alloc_netdev:
+	enetc_of_put_phy(pf);
 err_map_pf_space:
 	enetc_pci_remove(pdev);
 
@@ -963,7 +968,7 @@ static void enetc_pf_remove(struct pci_dev *pdev)
 	unregister_netdev(si->ndev);
 
 	enetc_mdio_remove(pf);
-	enetc_of_put_phy(priv);
+	enetc_of_put_phy(pf);
 
 	enetc_free_msix(priv);
 
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.h b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
index 1357cdb71d64..e0346dcf9ed7 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
@@ -45,6 +45,9 @@ struct enetc_pf {
 
 	struct mii_bus *mdio; /* saved for cleanup */
 	struct mii_bus *imdio;
+
+	struct device_node *phy_node;
+	phy_interface_t if_mode;
 };
 
 int enetc_msg_psi_init(struct enetc_pf *pf);
-- 
2.17.1

Re: [PATCH net-next 2/5] enetc: Make mdio accessors more generic

[Andrew Lunn]

On Fri, Sep 06, 2019 at 05:15:41PM +0300, Claudiu Manoil wrote:
> +#define enetc_mdio_rd(mdio_priv, off) \
> +	_enetc_mdio_rd(mdio_priv, ENETC_##off)
> +#define enetc_mdio_wr(mdio_priv, off, val) \
> +	_enetc_mdio_wr(mdio_priv, ENETC_##off, val)

Hi Claudiu

The MDIO code appears to be the only part of this driver which does
these ENETC_##off games. Could you please clean this up and use the
full name in the enetc_mdio_rd() and enetc_mdio_wr() calls.

Otherwise this looks good.

	  Andrew

Re: [PATCH net-next 1/5] enetc: Fix if_mode extraction

[Andrew Lunn]

On Fri, Sep 06, 2019 at 05:15:40PM +0300, Claudiu Manoil wrote:
> Fix handling of error return code. Before this fix,
> the error code was handled as unsigned type.
> Also, on this path if if_mode not found then just handle
> it as fixed link (i.e mac2mac connection).
> 
> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
> ---
>  drivers/net/ethernet/freescale/enetc/enetc_pf.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> index 7d6513ff8507..3a556646a2fb 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> @@ -751,6 +751,7 @@ static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
>  	struct enetc_pf *pf = enetc_si_priv(priv->si);
>  	struct device_node *np = priv->dev->of_node;
>  	struct device_node *mdio_np;
> +	int phy_mode;
>  	int err;
>  
>  	if (!np) {
> @@ -784,17 +785,11 @@ static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
>  		}
>  	}
>  
> -	priv->if_mode = of_get_phy_mode(np);
> -	if (priv->if_mode < 0) {
> -		dev_err(priv->dev, "missing phy type\n");
> -		of_node_put(priv->phy_node);
> -		if (of_phy_is_fixed_link(np))
> -			of_phy_deregister_fixed_link(np);
> -		else
> -			enetc_mdio_remove(pf);
> -
> -		return -EINVAL;
> -	}

Hi Claudiu

It is not clear to me why it is no longer necessary to deregister the
fixed link, or remove the mdio bus?

> +	phy_mode = of_get_phy_mode(np);
> +	if (phy_mode < 0)
> +		priv->if_mode = PHY_INTERFACE_MODE_NA; /* fixed link */
> +	else
> +		priv->if_mode = phy_mode;

Thanks
	Andrew

Re: [PATCH net-next 3/5] enetc: Initialize SerDes for SGMII and SXGMII protocols

[Andrew Lunn]

On Fri, Sep 06, 2019 at 05:15:42PM +0300, Claudiu Manoil wrote:
> +int enetc_imdio_init(struct enetc_pf *pf)
> +{
> +	struct device *dev = &pf->si->pdev->dev;
> +	struct enetc_mdio_priv *mdio_priv;
> +	struct mii_bus *bus;
> +	int err;
> +
> +	bus = devm_mdiobus_alloc_size(dev, sizeof(*mdio_priv));
> +	if (!bus)
> +		return -ENOMEM;
> +
> +	bus->name = "FSL ENETC internal MDIO Bus";
> +	bus->read = enetc_mdio_read;
> +	bus->write = enetc_mdio_write;
> +	bus->parent = dev;

Hi Claudiu

Since you don't expect any PHYs to be on this bus, maybe you should
set bus->phy_mask;

    Andrew

Re: [PATCH net-next 5/5] enetc: Use DT protocol information to set up the ports

[Andrew Lunn]

> +static void enetc_configure_port_mac(struct enetc_hw *hw,
> +				     phy_interface_t phy_mode)
>  {
>  	enetc_port_wr(hw, ENETC_PM0_MAXFRM,
>  		      ENETC_SET_MAXFRM(ENETC_RX_MAXFRM_SIZE));
> @@ -523,9 +524,11 @@ static void enetc_configure_port_mac(struct enetc_hw *hw)
>  		      ENETC_PM0_CMD_TXP	| ENETC_PM0_PROMISC |
>  		      ENETC_PM0_TX_EN | ENETC_PM0_RX_EN);
>  	/* set auto-speed for RGMII */
> -	if (enetc_port_rd(hw, ENETC_PM0_IF_MODE) & ENETC_PMO_IFM_RG)
> +	if (enetc_port_rd(hw, ENETC_PM0_IF_MODE) & ENETC_PMO_IFM_RG ||
> +	    phy_mode == PHY_INTERFACE_MODE_RGMII)
>  		enetc_port_wr(hw, ENETC_PM0_IF_MODE, ENETC_PM0_IFM_RGAUTO);

What about PHY_INTERFACE_MODE_RGMII_ID, PHY_INTERFACE_MODE_RGMII_RXID
and PHY_INTERFACE_MODE_RGMII_TXID.

    Andrew

RE: [PATCH net-next 1/5] enetc: Fix if_mode extraction

[Claudiu Manoil]

>-----Original Message-----
>From: Andrew Lunn <andrew@lunn.ch>
>Sent: Friday, September 6, 2019 10:58 PM
>To: Claudiu Manoil <claudiu.manoil@nxp.com>
>Cc: David S . Miller <davem@davemloft.net>; Alexandru Marginean
><alexandru.marginean@nxp.com>; netdev@vger.kernel.org
>Subject: Re: [PATCH net-next 1/5] enetc: Fix if_mode extraction
>
>On Fri, Sep 06, 2019 at 05:15:40PM +0300, Claudiu Manoil wrote:
>> Fix handling of error return code. Before this fix,
>> the error code was handled as unsigned type.
>> Also, on this path if if_mode not found then just handle
>> it as fixed link (i.e mac2mac connection).
>>
>> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
>> ---
>>  drivers/net/ethernet/freescale/enetc/enetc_pf.c | 17 ++++++-----------
>>  1 file changed, 6 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
>b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
>> index 7d6513ff8507..3a556646a2fb 100644
>> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
>> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
>> @@ -751,6 +751,7 @@ static int enetc_of_get_phy(struct enetc_ndev_priv
>*priv)
>>  	struct enetc_pf *pf = enetc_si_priv(priv->si);
>>  	struct device_node *np = priv->dev->of_node;
>>  	struct device_node *mdio_np;
>> +	int phy_mode;
>>  	int err;
>>
>>  	if (!np) {
>> @@ -784,17 +785,11 @@ static int enetc_of_get_phy(struct enetc_ndev_priv
>*priv)
>>  		}
>>  	}
>>
>> -	priv->if_mode = of_get_phy_mode(np);
>> -	if (priv->if_mode < 0) {
>> -		dev_err(priv->dev, "missing phy type\n");
>> -		of_node_put(priv->phy_node);
>> -		if (of_phy_is_fixed_link(np))
>> -			of_phy_deregister_fixed_link(np);
>> -		else
>> -			enetc_mdio_remove(pf);
>> -
>> -		return -EINVAL;
>> -	}
>
>Hi Claudiu
>
>It is not clear to me why it is no longer necessary to deregister the
>fixed link, or remove the mdio bus?
>
>> +	phy_mode = of_get_phy_mode(np);
>> +	if (phy_mode < 0)
>> +		priv->if_mode = PHY_INTERFACE_MODE_NA; /* fixed link */
>> +	else
>> +		priv->if_mode = phy_mode;
>

Hi Andrew,

The MAC2MAC connections are defined as fixed-link too, but without
phy-mode/phy-connection-type properties.  We don't want to de-register
these links.  Initial code was bogus in this regard.  So this MODE_NA interface
mode is reserved for the current representation of enetc ethernet ports
that are MAC2MAC connected with switch ports.  Also true that the current
upstream driver does not have the MAC2MAC connections enabled.

Your question however leads to the following discussion 😊.  The LS1028 SoC
has internal MAC2MAC connections between enetc eth ports and switch ports.
However the switch driver for LS1028a is not available upstream yet, it needs
to be re-worked as a DSA driver as you know (if you remember the discussions
we had on switchdev vs DSA for the Felix driver).  Now that we're at it, how
would you like to represent these MAC2MAC connections?

Current proposal is:
			ethernet@0,2 { /* SoC internal, connected to switch port 4 */
				compatible = "fsl,enetc";
				reg = <0x000200 0 0 0 0>;
				fixed-link {
					speed = <1000>;
					full-duplex;
				};
			};
			switch@0,5 {
				compatible = "mscc,felix-switch";
				[...]
				ports {
					#address-cells = <1>;
					#size-cells = <0>;

					/* external ports */
					[...]
					/* internal SoC ports */
					port@4 { /* connected to ENETC port2 */
						reg = <4>;
						fixed-link {
							speed = <1000>;
							full-duplex;
						};
					};
					port@5 { /* CPU port, connected to ENETC port3 */
						reg = <5>;
						ethernet = <&enetc_port3>;
						fixed-link {
							speed = <1000>;
							full-duplex;
						};
					};
				};
			};
			enetc_port3: ethernet@0,6 { /* SoC internal connected to switch port 5 */
				compatible = "fsl,enetc";
				reg = <0x000600 0 0 0 0>;
				fixed-link {
					speed = <1000>;
					full-duplex;
				};
			};
		};

Thanks.

Claudiu

Re: [PATCH net-next 1/5] enetc: Fix if_mode extraction

[Andrew Lunn]

On Mon, Sep 09, 2019 at 04:24:01PM +0000, Claudiu Manoil wrote:
> >-----Original Message-----
> >From: Andrew Lunn <andrew@lunn.ch>
> >Sent: Friday, September 6, 2019 10:58 PM
> >To: Claudiu Manoil <claudiu.manoil@nxp.com>
> >Cc: David S . Miller <davem@davemloft.net>; Alexandru Marginean
> ><alexandru.marginean@nxp.com>; netdev@vger.kernel.org
> >Subject: Re: [PATCH net-next 1/5] enetc: Fix if_mode extraction
> >
> >On Fri, Sep 06, 2019 at 05:15:40PM +0300, Claudiu Manoil wrote:
> >> Fix handling of error return code. Before this fix,
> >> the error code was handled as unsigned type.
> >> Also, on this path if if_mode not found then just handle
> >> it as fixed link (i.e mac2mac connection).
> >>
> >> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
> >> ---
> >>  drivers/net/ethernet/freescale/enetc/enetc_pf.c | 17 ++++++-----------
> >>  1 file changed, 6 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> >b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> >> index 7d6513ff8507..3a556646a2fb 100644
> >> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> >> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> >> @@ -751,6 +751,7 @@ static int enetc_of_get_phy(struct enetc_ndev_priv
> >*priv)
> >>  	struct enetc_pf *pf = enetc_si_priv(priv->si);
> >>  	struct device_node *np = priv->dev->of_node;
> >>  	struct device_node *mdio_np;
> >> +	int phy_mode;
> >>  	int err;
> >>
> >>  	if (!np) {
> >> @@ -784,17 +785,11 @@ static int enetc_of_get_phy(struct enetc_ndev_priv
> >*priv)
> >>  		}
> >>  	}
> >>
> >> -	priv->if_mode = of_get_phy_mode(np);
> >> -	if (priv->if_mode < 0) {
> >> -		dev_err(priv->dev, "missing phy type\n");
> >> -		of_node_put(priv->phy_node);
> >> -		if (of_phy_is_fixed_link(np))
> >> -			of_phy_deregister_fixed_link(np);
> >> -		else
> >> -			enetc_mdio_remove(pf);
> >> -
> >> -		return -EINVAL;
> >> -	}
> >
> >Hi Claudiu
> >
> >It is not clear to me why it is no longer necessary to deregister the
> >fixed link, or remove the mdio bus?
> >
> >> +	phy_mode = of_get_phy_mode(np);
> >> +	if (phy_mode < 0)
> >> +		priv->if_mode = PHY_INTERFACE_MODE_NA; /* fixed link */
> >> +	else
> >> +		priv->if_mode = phy_mode;
> >
> 
> Hi Andrew,
> 
> The MAC2MAC connections are defined as fixed-link too, but without
> phy-mode/phy-connection-type properties.  We don't want to de-register
> these links.  Initial code was bogus in this regard.

Hi Claudiu

This is what is not clear in the change log. That this code is removed
because it is wrong. Please could you expand the explanation to make
this clearer.

> Current proposal is:
> 			ethernet@0,2 { /* SoC internal, connected to switch port 4 */
> 				compatible = "fsl,enetc";
> 				reg = <0x000200 0 0 0 0>;
> 				fixed-link {
> 					speed = <1000>;
> 					full-duplex;
> 				};
> 			};
> 			switch@0,5 {
> 				compatible = "mscc,felix-switch";
> 				[...]
> 				ports {
> 					#address-cells = <1>;
> 					#size-cells = <0>;
> 
> 					/* external ports */
> 					[...]
> 					/* internal SoC ports */
> 					port@4 { /* connected to ENETC port2 */
> 						reg = <4>;
> 						fixed-link {
> 							speed = <1000>;
> 							full-duplex;
> 						};
> 					};

So this connection between the SoC and the switch does not use tags?
Can it use tags? Does the hardware allow you to have two CPU ports,
and load balance over them?

This second half is just standard DSA. This looks good.

     Andrew



> 					port@5 { /* CPU port, connected to ENETC port3 */
> 						reg = <5>;
> 						ethernet = <&enetc_port3>;
> 						fixed-link {
> 							speed = <1000>;
> 							full-duplex;
> 						};
> 					};
> 				};
> 			};
> 			enetc_port3: ethernet@0,6 { /* SoC internal connected to switch port 5 */
> 				compatible = "fsl,enetc";
> 				reg = <0x000600 0 0 0 0>;
> 				fixed-link {
> 					speed = <1000>;
> 					full-duplex;
> 				};
> 			};
> 		};
> 
> Thanks.
> 
> Claudiu

RE: [PATCH net-next 1/5] enetc: Fix if_mode extraction

[Claudiu Manoil]

>-----Original Message-----
>From: Andrew Lunn <andrew@lunn.ch>
>Sent: Tuesday, September 10, 2019 10:44 AM
>To: Claudiu Manoil <claudiu.manoil@nxp.com>
>Cc: David S . Miller <davem@davemloft.net>; Alexandru Marginean
><alexandru.marginean@nxp.com>; netdev@vger.kernel.org
>Subject: Re: [PATCH net-next 1/5] enetc: Fix if_mode extraction
>
>On Mon, Sep 09, 2019 at 04:24:01PM +0000, Claudiu Manoil wrote:
[...]
>>
>> Hi Andrew,
>>
>> The MAC2MAC connections are defined as fixed-link too, but without
>> phy-mode/phy-connection-type properties.  We don't want to de-register
>> these links.  Initial code was bogus in this regard.
>
>Hi Claudiu
>
>This is what is not clear in the change log. That this code is removed
>because it is wrong. Please could you expand the explanation to make
>this clearer.
>

I agree, but I also need to modify the patch to handle both the error case
of invalid phy-mode for mdio and normal fixed link phy connections, and
the mac2mac connection case.  The mac2mac connection case can be also
deferred to a later patch, when the switch driver - Felix - will be available
(there's no use for it in the current enetc upstream driver).

>> Current proposal is:
>> 			ethernet@0,2 { /* SoC internal, connected to switch port 4 */
>> 				compatible = "fsl,enetc";
>> 				reg = <0x000200 0 0 0 0>;
>> 				fixed-link {
>> 					speed = <1000>;
>> 					full-duplex;
>> 				};
>> 			};
>> 			switch@0,5 {
>> 				compatible = "mscc,felix-switch";
>> 				[...]
>> 				ports {
>> 					#address-cells = <1>;
>> 					#size-cells = <0>;
>>
>> 					/* external ports */
>> 					[...]
>> 					/* internal SoC ports */
>> 					port@4 { /* connected to ENETC port2 */
>> 						reg = <4>;
>> 						fixed-link {
>> 							speed = <1000>;
>> 							full-duplex;
>> 						};
>> 					};
>
>So this connection between the SoC and the switch does not use tags?
>Can it use tags? Does the hardware allow you to have two CPU ports,
>and load balance over them?
>

Unfortunately the switch can handle only one port with tags.  There's only
one CPU port, switch port 4 is just like another front panel port.  On top of
that, the CPU port is not capable of flow control (pause frames don't work with
tagged traffic on the switch side).  So we may be forced to use port 4 to mitigate
this.  Note that the switch is inside the SoC.

>This second half is just standard DSA. This looks good.
>

Thanks for the confirmation and the rest of the review, all valid findings.

Regards,
Claudiu