[PATCH net-next v6 0/4] net: enetc: remove bootloader dependency

[PATCH net-next v6 0/4] net: enetc: remove bootloader dependency

[Michael Walle]

This is a resend [now a new v6] of the series because the conversion to the
phylink interface will likely take longer:
https://lore.kernel.org/netdev/CA+h21hpBodyY8CtNH2ktRdc2FqPi=Fjp94=VVZvzSVbnvnfKVg@mail.gmail.com/
Also the discussion in the v3 resend doesn't look like it will be resolved
soon :/
https://lore.kernel.org/netdev/20200701213433.9217-1-michael@walle.cc/

Unfortunately, we have boards in the wild with a bootloader which doesn't
set the PCS up correctly. Thus I'd really see this patches picked up as an
intermediate step until the phylink conversion is ready. Vladimir Oltean
already offered to convert enetc to phylink when he converts the felix to
phylink. After this series the PCS setup of the enetc looks almost the same
as the current felix setup. Thus conversion should be easy.

These patches were picked from the following series:
https://lore.kernel.org/netdev/1567779344-30965-1-git-send-email-claudiu.manoil@nxp.com/
They have never been resent. I've picked them up, addressed Andrews
comments, fixed some more bugs and asked Claudiu if I can keep their SOB
tags; he agreed. I've tested this on our board which happens to have a
bootloader which doesn't do the enetc setup in all cases. Though, only
SGMII mode was tested.

changes since v5:
 - fixed pcs->autoneg_complete and pcs->link assignment. Thanks Vladimir.

changes since v4:
 - moved (and renamed) the USXGMII constants to include/uapi/linux/mdio.h.
   Suggested by Russell King.

changes since v3:
 - rebased to latest net-next where devm_mdiobus_free() was removed.
   replace it by mdiobus_free(). The internal MDIO bus is optional, if
   there is any error, we try to run with the bootloader default PCS
   settings, thus in the error case, we need to free the mdiobus.

changes since v2:
 - removed SOBs from "net: enetc: Initialize SerDes for SGMII and USXGMII
   protocols" because almost everything has changed.
 - get a phy_device for the internal PCS PHY so we can use the phy_
   functions instead of raw mdiobus writes
 - reuse macros already defined in fsl_mdio.h, move missing bits from
   felix to fsl_mdio.h, because they share the same PCS PHY building
   block
 - added 2500BaseX mode (based on felix init routine)
 - changed xgmii mode to usxgmii mode, because it is actually USXGMII and
   felix does the same.
 - fixed devad, which is 0x1f (MMD_VEND2)

changes since v1:
 - mdiobus id is '"imdio-%s", dev_name(dev)' because the plain dev_name()
   is used by the emdio.
 - use mdiobus_write() instead of imdio->write(imdio, ..), since this is
   already a full featured mdiobus
 - set phy_mask to ~0 to avoid scanning the bus
 - use phy_interface_mode_is_rgmii(phy_mode) to also include the RGMII
   modes with pad delays.
 - move enetc_imdio_init() to enetc_pf.c, there shouldn't be any other
   users, should it?
 - renamed serdes to SerDes
 - printing the error code of mdiobus_register() in the error path
 - call mdiobus_unregister() on _remove()
 - call devm_mdiobus_free() if mdiobus_register() fails, since an
   error is not fatal

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

Michael Walle (3):
  net: phy: add USXGMII link partner ability constants
  net: dsa: felix: (re)use already existing constants
  net: enetc: Initialize SerDes for SGMII and USXGMII protocols

 drivers/net/dsa/ocelot/felix_vsc9959.c        |  45 ++---
 .../net/ethernet/freescale/enetc/enetc_hw.h   |   3 +
 .../net/ethernet/freescale/enetc/enetc_pf.c   | 188 +++++++++++++++---
 .../net/ethernet/freescale/enetc/enetc_pf.h   |   5 +
 include/uapi/linux/mdio.h                     |  26 +++
 5 files changed, 210 insertions(+), 57 deletions(-)

-- 
2.20.1

[PATCH net-next v6 1/4] net: phy: add USXGMII link partner ability constants

[Michael Walle]

The constants are taken from the USXGMII Singleport Copper Interface
specification. The naming are based on the SGMII ones, but with an MDIO_
prefix.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 include/uapi/linux/mdio.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
index 4bcb41c71b8c..784723072578 100644
--- a/include/uapi/linux/mdio.h
+++ b/include/uapi/linux/mdio.h
@@ -324,4 +324,30 @@ static inline __u16 mdio_phy_id_c45(int prtad, int devad)
 	return MDIO_PHY_ID_C45 | (prtad << 5) | devad;
 }
 
+/* UsxgmiiChannelInfo[15:0] for USXGMII in-band auto-negotiation.*/
+#define MDIO_LPA_USXGMII_EEE_CLK_STP	0x0080	/* EEE clock stop supported */
+#define MDIO_LPA_USXGMII_EEE		0x0100	/* EEE supported */
+#define MDIO_LPA_USXGMII_SPD_MASK	0x0e00	/* USXGMII speed mask */
+#define MDIO_LPA_USXGMII_FULL_DUPLEX	0x1000	/* USXGMII full duplex */
+#define MDIO_LPA_USXGMII_DPX_SPD_MASK	0x1e00	/* USXGMII duplex and speed bits */
+#define MDIO_LPA_USXGMII_10		0x0000	/* 10Mbps */
+#define MDIO_LPA_USXGMII_10HALF		0x0000	/* 10Mbps half-duplex */
+#define MDIO_LPA_USXGMII_10FULL		0x1000	/* 10Mbps full-duplex */
+#define MDIO_LPA_USXGMII_100		0x0200	/* 100Mbps */
+#define MDIO_LPA_USXGMII_100HALF	0x0200	/* 100Mbps half-duplex */
+#define MDIO_LPA_USXGMII_100FULL	0x1200	/* 100Mbps full-duplex */
+#define MDIO_LPA_USXGMII_1000		0x0400	/* 1000Mbps */
+#define MDIO_LPA_USXGMII_1000HALF	0x0400	/* 1000Mbps half-duplex */
+#define MDIO_LPA_USXGMII_1000FULL	0x1400	/* 1000Mbps full-duplex */
+#define MDIO_LPA_USXGMII_10G		0x0600	/* 10Gbps */
+#define MDIO_LPA_USXGMII_10GHALF	0x0600	/* 10Gbps half-duplex */
+#define MDIO_LPA_USXGMII_10GFULL	0x1600	/* 10Gbps full-duplex */
+#define MDIO_LPA_USXGMII_2500		0x0800	/* 2500Mbps */
+#define MDIO_LPA_USXGMII_2500HALF	0x0800	/* 2500Mbps half-duplex */
+#define MDIO_LPA_USXGMII_2500FULL	0x1800	/* 2500Mbps full-duplex */
+#define MDIO_LPA_USXGMII_5000		0x0a00	/* 5000Mbps */
+#define MDIO_LPA_USXGMII_5000HALF	0x0a00	/* 5000Mbps half-duplex */
+#define MDIO_LPA_USXGMII_5000FULL	0x1a00	/* 5000Mbps full-duplex */
+#define MDIO_LPA_USXGMII_LINK		0x8000	/* PHY link with copper-side partner */
+
 #endif /* _UAPI__LINUX_MDIO_H__ */
-- 
2.20.1

[PATCH net-next v6 2/4] net: dsa: felix: (re)use already existing constants

[Michael Walle]

Now that there are USXGMII constants available, drop the old definitions
and reuse the generic ones.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/net/dsa/ocelot/felix_vsc9959.c | 45 +++++++-------------------
 1 file changed, 12 insertions(+), 33 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 19614537b1ba..a3ddb1394540 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -10,35 +10,15 @@
 #include <soc/mscc/ocelot.h>
 #include <net/pkt_sched.h>
 #include <linux/iopoll.h>
+#include <linux/mdio.h>
 #include <linux/pci.h>
 #include "felix.h"
 
 #define VSC9959_VCAP_IS2_CNT		1024
 #define VSC9959_VCAP_IS2_ENTRY_WIDTH	376
 #define VSC9959_VCAP_PORT_CNT		6
-
-/* TODO: should find a better place for these */
-#define USXGMII_BMCR_RESET		BIT(15)
-#define USXGMII_BMCR_AN_EN		BIT(12)
-#define USXGMII_BMCR_RST_AN		BIT(9)
-#define USXGMII_BMSR_LNKS(status)	(((status) & GENMASK(2, 2)) >> 2)
-#define USXGMII_BMSR_AN_CMPL(status)	(((status) & GENMASK(5, 5)) >> 5)
-#define USXGMII_ADVERTISE_LNKS(x)	(((x) << 15) & BIT(15))
-#define USXGMII_ADVERTISE_FDX		BIT(12)
-#define USXGMII_ADVERTISE_SPEED(x)	(((x) << 9) & GENMASK(11, 9))
-#define USXGMII_LPA_LNKS(lpa)		((lpa) >> 15)
-#define USXGMII_LPA_DUPLEX(lpa)		(((lpa) & GENMASK(12, 12)) >> 12)
-#define USXGMII_LPA_SPEED(lpa)		(((lpa) & GENMASK(11, 9)) >> 9)
-
 #define VSC9959_TAS_GCL_ENTRY_MAX	63
 
-enum usxgmii_speed {
-	USXGMII_SPEED_10	= 0,
-	USXGMII_SPEED_100	= 1,
-	USXGMII_SPEED_1000	= 2,
-	USXGMII_SPEED_2500	= 4,
-};
-
 static const u32 vsc9959_ana_regmap[] = {
 	REG(ANA_ADVLEARN,			0x0089a0),
 	REG(ANA_VLANMASK,			0x0089a4),
@@ -787,11 +767,10 @@ static void vsc9959_pcs_config_usxgmii(struct phy_device *pcs,
 {
 	/* Configure device ability for the USXGMII Replicator */
 	phy_write_mmd(pcs, MDIO_MMD_VEND2, MII_ADVERTISE,
-		      USXGMII_ADVERTISE_SPEED(USXGMII_SPEED_2500) |
-		      USXGMII_ADVERTISE_LNKS(1) |
+		      MDIO_LPA_USXGMII_2500FULL |
+		      MDIO_LPA_USXGMII_LINK |
 		      ADVERTISE_SGMII |
-		      ADVERTISE_LPACK |
-		      USXGMII_ADVERTISE_FDX);
+		      ADVERTISE_LPACK);
 }
 
 static void vsc9959_pcs_config(struct ocelot *ocelot, int port,
@@ -1005,8 +984,8 @@ static void vsc9959_pcs_link_state_usxgmii(struct phy_device *pcs,
 		return;
 
 	pcs->autoneg = true;
-	pcs->autoneg_complete = USXGMII_BMSR_AN_CMPL(status);
-	pcs->link = USXGMII_BMSR_LNKS(status);
+	pcs->autoneg_complete = !!(status & BMSR_ANEGCOMPLETE);
+	pcs->link = !!(status & BMSR_LSTATUS);
 
 	if (!pcs->link || !pcs->autoneg_complete)
 		return;
@@ -1015,24 +994,24 @@ static void vsc9959_pcs_link_state_usxgmii(struct phy_device *pcs,
 	if (lpa < 0)
 		return;
 
-	switch (USXGMII_LPA_SPEED(lpa)) {
-	case USXGMII_SPEED_10:
+	switch (lpa & MDIO_LPA_USXGMII_SPD_MASK) {
+	case MDIO_LPA_USXGMII_10:
 		pcs->speed = SPEED_10;
 		break;
-	case USXGMII_SPEED_100:
+	case MDIO_LPA_USXGMII_100:
 		pcs->speed = SPEED_100;
 		break;
-	case USXGMII_SPEED_1000:
+	case MDIO_LPA_USXGMII_1000:
 		pcs->speed = SPEED_1000;
 		break;
-	case USXGMII_SPEED_2500:
+	case MDIO_LPA_USXGMII_2500:
 		pcs->speed = SPEED_2500;
 		break;
 	default:
 		break;
 	}
 
-	if (USXGMII_LPA_DUPLEX(lpa))
+	if (lpa & MDIO_LPA_USXGMII_FULL_DUPLEX)
 		pcs->duplex = DUPLEX_FULL;
 	else
 		pcs->duplex = DUPLEX_HALF;
-- 
2.20.1

[PATCH net-next v6 3/4] net: enetc: Initialize SerDes for SGMII and USXGMII protocols

[Michael Walle]

ENETC has ethernet MACs capable of SGMII, 2500BaseX and USXGMII. But in
order to use these protocols some SerDes configurations need to be
performed. The SerDes is configurable via an internal PCS PHY which is
connected to an internal MDIO bus at address 0.

This patch basically removes the dependency on bootloader regarding
SerDes initialization.

Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
 .../net/ethernet/freescale/enetc/enetc_hw.h   |   3 +
 .../net/ethernet/freescale/enetc/enetc_pf.c   | 135 ++++++++++++++++++
 .../net/ethernet/freescale/enetc/enetc_pf.h   |   2 +
 3 files changed, 140 insertions(+)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
index fc357bc56835..135bf46354ea 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
@@ -224,6 +224,9 @@ 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
+
 #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_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 4fac57dbb3c8..662740874841 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
 /* Copyright 2017-2019 NXP */
 
+#include <linux/mdio.h>
 #include <linux/module.h>
 #include <linux/fsl/enetc_mdio.h>
 #include <linux/of_mdio.h>
@@ -833,6 +834,135 @@ static void enetc_of_put_phy(struct enetc_ndev_priv *priv)
 		of_node_put(priv->phy_node);
 }
 
+static int enetc_imdio_init(struct enetc_pf *pf, bool is_c45)
+{
+	struct device *dev = &pf->si->pdev->dev;
+	struct enetc_mdio_priv *mdio_priv;
+	struct phy_device *pcs;
+	struct mii_bus *bus;
+	int err;
+
+	bus = mdiobus_alloc_size(sizeof(*mdio_priv));
+	if (!bus)
+		return -ENOMEM;
+
+	bus->name = "Freescale ENETC internal MDIO Bus";
+	bus->read = enetc_mdio_read;
+	bus->write = enetc_mdio_write;
+	bus->parent = dev;
+	bus->phy_mask = ~0;
+	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-imdio", dev_name(dev));
+
+	err = mdiobus_register(bus);
+	if (err) {
+		dev_err(dev, "cannot register internal MDIO bus (%d)\n", err);
+		goto free_mdio_bus;
+	}
+
+	pcs = get_phy_device(bus, 0, is_c45);
+	if (IS_ERR(pcs)) {
+		err = PTR_ERR(pcs);
+		dev_err(dev, "cannot get internal PCS PHY (%d)\n", err);
+		goto unregister_mdiobus;
+	}
+
+	pf->imdio = bus;
+	pf->pcs = pcs;
+
+	return 0;
+
+unregister_mdiobus:
+	mdiobus_unregister(bus);
+free_mdio_bus:
+	mdiobus_free(bus);
+	return err;
+}
+
+static void enetc_imdio_remove(struct enetc_pf *pf)
+{
+	if (pf->pcs)
+		put_device(&pf->pcs->mdio.dev);
+	if (pf->imdio) {
+		mdiobus_unregister(pf->imdio);
+		mdiobus_free(pf->imdio);
+	}
+}
+
+static void enetc_configure_sgmii(struct phy_device *pcs)
+{
+	/* SGMII spec requires tx_config_Reg[15:0] to be exactly 0x4001
+	 * for the MAC PCS in order to acknowledge the AN.
+	 */
+	phy_write(pcs, MII_ADVERTISE, ADVERTISE_SGMII | ADVERTISE_LPACK);
+
+	phy_write(pcs, ENETC_PCS_IF_MODE,
+		  ENETC_PCS_IF_MODE_SGMII_EN |
+		  ENETC_PCS_IF_MODE_USE_SGMII_AN);
+
+	/* Adjust link timer for SGMII */
+	phy_write(pcs, ENETC_PCS_LINK_TIMER1, ENETC_PCS_LINK_TIMER1_VAL);
+	phy_write(pcs, ENETC_PCS_LINK_TIMER2, ENETC_PCS_LINK_TIMER2_VAL);
+
+	phy_write(pcs, MII_BMCR, BMCR_ANRESTART | BMCR_ANENABLE);
+}
+
+static void enetc_configure_2500basex(struct phy_device *pcs)
+{
+	phy_write(pcs, ENETC_PCS_IF_MODE,
+		  ENETC_PCS_IF_MODE_SGMII_EN |
+		  ENETC_PCS_IF_MODE_SGMII_SPEED(ENETC_PCS_SPEED_2500));
+
+	phy_write(pcs, MII_BMCR, BMCR_SPEED1000 | BMCR_FULLDPLX | BMCR_RESET);
+}
+
+static void enetc_configure_usxgmii(struct phy_device *pcs)
+{
+	/* Configure device ability for the USXGMII Replicator */
+	phy_write_mmd(pcs, MDIO_MMD_VEND2, MII_ADVERTISE,
+		      ADVERTISE_SGMII | ADVERTISE_LPACK |
+		      MDIO_LPA_USXGMII_FULL_DUPLEX);
+
+	/* Restart PCS AN */
+	phy_write_mmd(pcs, MDIO_MMD_VEND2, MII_BMCR,
+		      BMCR_RESET | BMCR_ANENABLE | BMCR_ANRESTART);
+}
+
+static int enetc_configure_serdes(struct enetc_ndev_priv *priv)
+{
+	bool is_c45 = priv->if_mode == PHY_INTERFACE_MODE_USXGMII;
+	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_2500BASEX &&
+	    priv->if_mode != PHY_INTERFACE_MODE_USXGMII)
+		return 0;
+
+	err = enetc_imdio_init(pf, is_c45);
+	if (err)
+		return err;
+
+	switch (priv->if_mode) {
+	case PHY_INTERFACE_MODE_SGMII:
+		enetc_configure_sgmii(pf->pcs);
+		break;
+	case PHY_INTERFACE_MODE_2500BASEX:
+		enetc_configure_2500basex(pf->pcs);
+		break;
+	case PHY_INTERFACE_MODE_USXGMII:
+		enetc_configure_usxgmii(pf->pcs);
+		break;
+	default:
+		dev_err(&pf->si->pdev->dev, "Unsupported link mode %s\n",
+			phy_modes(priv->if_mode));
+	}
+
+	return 0;
+}
+
 static int enetc_pf_probe(struct pci_dev *pdev,
 			  const struct pci_device_id *ent)
 {
@@ -897,6 +1027,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;
@@ -932,6 +1066,7 @@ static void enetc_pf_remove(struct pci_dev *pdev)
 	priv = netdev_priv(si->ndev);
 	unregister_netdev(si->ndev);
 
+	enetc_imdio_remove(pf);
 	enetc_mdio_remove(pf);
 	enetc_of_put_phy(priv);
 
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.h b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
index 59e65a6f6c3e..2cb922b59f46 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
@@ -44,6 +44,8 @@ struct enetc_pf {
 	DECLARE_BITMAP(active_vlans, VLAN_N_VID);
 
 	struct mii_bus *mdio; /* saved for cleanup */
+	struct mii_bus *imdio;
+	struct phy_device *pcs;
 };
 
 int enetc_msg_psi_init(struct enetc_pf *pf);
-- 
2.20.1

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

[Michael Walle]

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>
Signed-off-by: Michael Walle <michael@walle.cc>
---
 .../net/ethernet/freescale/enetc/enetc_pf.c   | 57 ++++++++++---------
 .../net/ethernet/freescale/enetc/enetc_pf.h   |  3 +
 2 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 662740874841..dfc3acc841df 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -482,7 +482,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));
@@ -498,9 +499,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_interface_mode_is_rgmii(phy_mode))
 		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_USXGMII)
 		enetc_port_wr(hw, ENETC_PM0_IF_MODE, ENETC_PM0_IFM_XGMII);
 }
 
@@ -524,7 +527,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);
 
@@ -776,27 +779,27 @@ static void enetc_mdio_remove(struct enetc_pf *pf)
 		mdiobus_unregister(pf->mdio);
 }
 
-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 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");
@@ -804,15 +807,15 @@ 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;
 		}
 	}
 
-	err = of_get_phy_mode(np, &priv->if_mode);
+	err = of_get_phy_mode(np, &pf->if_mode);
 	if (err) {
-		dev_err(priv->dev, "missing phy type\n");
-		of_node_put(priv->phy_node);
+		dev_err(dev, "missing phy type\n");
+		of_node_put(pf->phy_node);
 		if (of_phy_is_fixed_link(np))
 			of_phy_deregister_fixed_link(np);
 		else
@@ -824,14 +827,14 @@ static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
 	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 int enetc_imdio_init(struct enetc_pf *pf, bool is_c45)
@@ -994,6 +997,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);
@@ -1008,6 +1015,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);
 
@@ -1023,10 +1032,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");
@@ -1040,7 +1045,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);
@@ -1048,6 +1052,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);
 
@@ -1068,7 +1073,7 @@ static void enetc_pf_remove(struct pci_dev *pdev)
 
 	enetc_imdio_remove(pf);
 	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 2cb922b59f46..0d0ee91282a5 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
@@ -46,6 +46,9 @@ struct enetc_pf {
 	struct mii_bus *mdio; /* saved for cleanup */
 	struct mii_bus *imdio;
 	struct phy_device *pcs;
+
+	struct device_node *phy_node;
+	phy_interface_t if_mode;
 };
 
 int enetc_msg_psi_init(struct enetc_pf *pf);
-- 
2.20.1

Re: [PATCH net-next v6 0/4] net: enetc: remove bootloader dependency

[Vladimir Oltean]

On Thu, Jul 09, 2020 at 11:35:22PM +0200, Michael Walle wrote:
> This is a resend [now a new v6] of the series because the conversion to the
> phylink interface will likely take longer:
> https://lore.kernel.org/netdev/CA+h21hpBodyY8CtNH2ktRdc2FqPi=Fjp94=VVZvzSVbnvnfKVg@mail.gmail.com/
> Also the discussion in the v3 resend doesn't look like it will be resolved
> soon :/
> https://lore.kernel.org/netdev/20200701213433.9217-1-michael@walle.cc/
> 
> Unfortunately, we have boards in the wild with a bootloader which doesn't
> set the PCS up correctly. Thus I'd really see this patches picked up as an
> intermediate step until the phylink conversion is ready. Vladimir Oltean
> already offered to convert enetc to phylink when he converts the felix to
> phylink. After this series the PCS setup of the enetc looks almost the same
> as the current felix setup. Thus conversion should be easy.
> 
> These patches were picked from the following series:
> https://lore.kernel.org/netdev/1567779344-30965-1-git-send-email-claudiu.manoil@nxp.com/
> They have never been resent. I've picked them up, addressed Andrews
> comments, fixed some more bugs and asked Claudiu if I can keep their SOB
> tags; he agreed. I've tested this on our board which happens to have a
> bootloader which doesn't do the enetc setup in all cases. Though, only
> SGMII mode was tested.
> 
> changes since v5:
>  - fixed pcs->autoneg_complete and pcs->link assignment. Thanks Vladimir.
> 
> changes since v4:
>  - moved (and renamed) the USXGMII constants to include/uapi/linux/mdio.h.
>    Suggested by Russell King.
> 
> changes since v3:
>  - rebased to latest net-next where devm_mdiobus_free() was removed.
>    replace it by mdiobus_free(). The internal MDIO bus is optional, if
>    there is any error, we try to run with the bootloader default PCS
>    settings, thus in the error case, we need to free the mdiobus.
> 
> changes since v2:
>  - removed SOBs from "net: enetc: Initialize SerDes for SGMII and USXGMII
>    protocols" because almost everything has changed.
>  - get a phy_device for the internal PCS PHY so we can use the phy_
>    functions instead of raw mdiobus writes
>  - reuse macros already defined in fsl_mdio.h, move missing bits from
>    felix to fsl_mdio.h, because they share the same PCS PHY building
>    block
>  - added 2500BaseX mode (based on felix init routine)
>  - changed xgmii mode to usxgmii mode, because it is actually USXGMII and
>    felix does the same.
>  - fixed devad, which is 0x1f (MMD_VEND2)
> 
> changes since v1:
>  - mdiobus id is '"imdio-%s", dev_name(dev)' because the plain dev_name()
>    is used by the emdio.
>  - use mdiobus_write() instead of imdio->write(imdio, ..), since this is
>    already a full featured mdiobus
>  - set phy_mask to ~0 to avoid scanning the bus
>  - use phy_interface_mode_is_rgmii(phy_mode) to also include the RGMII
>    modes with pad delays.
>  - move enetc_imdio_init() to enetc_pf.c, there shouldn't be any other
>    users, should it?
>  - renamed serdes to SerDes
>  - printing the error code of mdiobus_register() in the error path
>  - call mdiobus_unregister() on _remove()
>  - call devm_mdiobus_free() if mdiobus_register() fails, since an
>    error is not fatal
> 
> Alex Marginean (1):
>   net: enetc: Use DT protocol information to set up the ports
> 
> Michael Walle (3):
>   net: phy: add USXGMII link partner ability constants
>   net: dsa: felix: (re)use already existing constants
>   net: enetc: Initialize SerDes for SGMII and USXGMII protocols
> 
>  drivers/net/dsa/ocelot/felix_vsc9959.c        |  45 ++---
>  .../net/ethernet/freescale/enetc/enetc_hw.h   |   3 +
>  .../net/ethernet/freescale/enetc/enetc_pf.c   | 188 +++++++++++++++---
>  .../net/ethernet/freescale/enetc/enetc_pf.h   |   5 +
>  include/uapi/linux/mdio.h                     |  26 +++
>  5 files changed, 210 insertions(+), 57 deletions(-)
> 
> -- 
> 2.20.1
> 

I plan to give this series a go on an LS1028A-QDS later today to make
sure there are no regressions.

Thanks,
-Vladimir

Re: [PATCH net-next v6 2/4] net: dsa: felix: (re)use already existing constants

[Vladimir Oltean]

On Thu, Jul 09, 2020 at 11:35:24PM +0200, Michael Walle wrote:
> Now that there are USXGMII constants available, drop the old definitions
> and reuse the generic ones.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---

I did regression-testing of this on an LS1028A-QDS with SerDes protocol
0x13bb.

Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com>

There was a regression, but it wasn't caused by you.

>  drivers/net/dsa/ocelot/felix_vsc9959.c | 45 +++++++-------------------
>  1 file changed, 12 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
> index 19614537b1ba..a3ddb1394540 100644
> --- a/drivers/net/dsa/ocelot/felix_vsc9959.c
> +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
> @@ -10,35 +10,15 @@
>  #include <soc/mscc/ocelot.h>
>  #include <net/pkt_sched.h>
>  #include <linux/iopoll.h>
> +#include <linux/mdio.h>
>  #include <linux/pci.h>
>  #include "felix.h"
>  
>  #define VSC9959_VCAP_IS2_CNT		1024
>  #define VSC9959_VCAP_IS2_ENTRY_WIDTH	376
>  #define VSC9959_VCAP_PORT_CNT		6
> -
> -/* TODO: should find a better place for these */
> -#define USXGMII_BMCR_RESET		BIT(15)
> -#define USXGMII_BMCR_AN_EN		BIT(12)
> -#define USXGMII_BMCR_RST_AN		BIT(9)
> -#define USXGMII_BMSR_LNKS(status)	(((status) & GENMASK(2, 2)) >> 2)
> -#define USXGMII_BMSR_AN_CMPL(status)	(((status) & GENMASK(5, 5)) >> 5)
> -#define USXGMII_ADVERTISE_LNKS(x)	(((x) << 15) & BIT(15))
> -#define USXGMII_ADVERTISE_FDX		BIT(12)
> -#define USXGMII_ADVERTISE_SPEED(x)	(((x) << 9) & GENMASK(11, 9))
> -#define USXGMII_LPA_LNKS(lpa)		((lpa) >> 15)
> -#define USXGMII_LPA_DUPLEX(lpa)		(((lpa) & GENMASK(12, 12)) >> 12)
> -#define USXGMII_LPA_SPEED(lpa)		(((lpa) & GENMASK(11, 9)) >> 9)
> -
>  #define VSC9959_TAS_GCL_ENTRY_MAX	63
>  
> -enum usxgmii_speed {
> -	USXGMII_SPEED_10	= 0,
> -	USXGMII_SPEED_100	= 1,
> -	USXGMII_SPEED_1000	= 2,
> -	USXGMII_SPEED_2500	= 4,
> -};
> -
>  static const u32 vsc9959_ana_regmap[] = {
>  	REG(ANA_ADVLEARN,			0x0089a0),
>  	REG(ANA_VLANMASK,			0x0089a4),
> @@ -787,11 +767,10 @@ static void vsc9959_pcs_config_usxgmii(struct phy_device *pcs,
>  {
>  	/* Configure device ability for the USXGMII Replicator */
>  	phy_write_mmd(pcs, MDIO_MMD_VEND2, MII_ADVERTISE,
> -		      USXGMII_ADVERTISE_SPEED(USXGMII_SPEED_2500) |
> -		      USXGMII_ADVERTISE_LNKS(1) |
> +		      MDIO_LPA_USXGMII_2500FULL |
> +		      MDIO_LPA_USXGMII_LINK |
>  		      ADVERTISE_SGMII |
> -		      ADVERTISE_LPACK |
> -		      USXGMII_ADVERTISE_FDX);
> +		      ADVERTISE_LPACK);
>  }
>  
>  static void vsc9959_pcs_config(struct ocelot *ocelot, int port,
> @@ -1005,8 +984,8 @@ static void vsc9959_pcs_link_state_usxgmii(struct phy_device *pcs,
>  		return;
>  
>  	pcs->autoneg = true;
> -	pcs->autoneg_complete = USXGMII_BMSR_AN_CMPL(status);
> -	pcs->link = USXGMII_BMSR_LNKS(status);
> +	pcs->autoneg_complete = !!(status & BMSR_ANEGCOMPLETE);
> +	pcs->link = !!(status & BMSR_LSTATUS);
>  
>  	if (!pcs->link || !pcs->autoneg_complete)
>  		return;
> @@ -1015,24 +994,24 @@ static void vsc9959_pcs_link_state_usxgmii(struct phy_device *pcs,
>  	if (lpa < 0)
>  		return;
>  
> -	switch (USXGMII_LPA_SPEED(lpa)) {
> -	case USXGMII_SPEED_10:
> +	switch (lpa & MDIO_LPA_USXGMII_SPD_MASK) {
> +	case MDIO_LPA_USXGMII_10:
>  		pcs->speed = SPEED_10;
>  		break;
> -	case USXGMII_SPEED_100:
> +	case MDIO_LPA_USXGMII_100:
>  		pcs->speed = SPEED_100;
>  		break;
> -	case USXGMII_SPEED_1000:
> +	case MDIO_LPA_USXGMII_1000:
>  		pcs->speed = SPEED_1000;
>  		break;
> -	case USXGMII_SPEED_2500:
> +	case MDIO_LPA_USXGMII_2500:
>  		pcs->speed = SPEED_2500;
>  		break;
>  	default:
>  		break;
>  	}
>  
> -	if (USXGMII_LPA_DUPLEX(lpa))
> +	if (lpa & MDIO_LPA_USXGMII_FULL_DUPLEX)
>  		pcs->duplex = DUPLEX_FULL;
>  	else
>  		pcs->duplex = DUPLEX_HALF;
> -- 
> 2.20.1
> 

Re: [PATCH net-next v6 1/4] net: phy: add USXGMII link partner ability constants

[Vladimir Oltean]

On Thu, Jul 09, 2020 at 11:35:23PM +0200, Michael Walle wrote:
> The constants are taken from the USXGMII Singleport Copper Interface
> specification. The naming are based on the SGMII ones, but with an MDIO_
> prefix.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---

Somebody would need to review this patch, as it is introducing UAPI.

>  include/uapi/linux/mdio.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
> index 4bcb41c71b8c..784723072578 100644
> --- a/include/uapi/linux/mdio.h
> +++ b/include/uapi/linux/mdio.h
> @@ -324,4 +324,30 @@ static inline __u16 mdio_phy_id_c45(int prtad, int devad)
>  	return MDIO_PHY_ID_C45 | (prtad << 5) | devad;
>  }
>  
> +/* UsxgmiiChannelInfo[15:0] for USXGMII in-band auto-negotiation.*/
> +#define MDIO_LPA_USXGMII_EEE_CLK_STP	0x0080	/* EEE clock stop supported */
> +#define MDIO_LPA_USXGMII_EEE		0x0100	/* EEE supported */
> +#define MDIO_LPA_USXGMII_SPD_MASK	0x0e00	/* USXGMII speed mask */
> +#define MDIO_LPA_USXGMII_FULL_DUPLEX	0x1000	/* USXGMII full duplex */
> +#define MDIO_LPA_USXGMII_DPX_SPD_MASK	0x1e00	/* USXGMII duplex and speed bits */
> +#define MDIO_LPA_USXGMII_10		0x0000	/* 10Mbps */
> +#define MDIO_LPA_USXGMII_10HALF		0x0000	/* 10Mbps half-duplex */
> +#define MDIO_LPA_USXGMII_10FULL		0x1000	/* 10Mbps full-duplex */
> +#define MDIO_LPA_USXGMII_100		0x0200	/* 100Mbps */
> +#define MDIO_LPA_USXGMII_100HALF	0x0200	/* 100Mbps half-duplex */
> +#define MDIO_LPA_USXGMII_100FULL	0x1200	/* 100Mbps full-duplex */
> +#define MDIO_LPA_USXGMII_1000		0x0400	/* 1000Mbps */
> +#define MDIO_LPA_USXGMII_1000HALF	0x0400	/* 1000Mbps half-duplex */
> +#define MDIO_LPA_USXGMII_1000FULL	0x1400	/* 1000Mbps full-duplex */
> +#define MDIO_LPA_USXGMII_10G		0x0600	/* 10Gbps */
> +#define MDIO_LPA_USXGMII_10GHALF	0x0600	/* 10Gbps half-duplex */
> +#define MDIO_LPA_USXGMII_10GFULL	0x1600	/* 10Gbps full-duplex */
> +#define MDIO_LPA_USXGMII_2500		0x0800	/* 2500Mbps */
> +#define MDIO_LPA_USXGMII_2500HALF	0x0800	/* 2500Mbps half-duplex */
> +#define MDIO_LPA_USXGMII_2500FULL	0x1800	/* 2500Mbps full-duplex */
> +#define MDIO_LPA_USXGMII_5000		0x0a00	/* 5000Mbps */
> +#define MDIO_LPA_USXGMII_5000HALF	0x0a00	/* 5000Mbps half-duplex */
> +#define MDIO_LPA_USXGMII_5000FULL	0x1a00	/* 5000Mbps full-duplex */
> +#define MDIO_LPA_USXGMII_LINK		0x8000	/* PHY link with copper-side partner */
> +
>  #endif /* _UAPI__LINUX_MDIO_H__ */
> -- 
> 2.20.1
> 

Re: [PATCH net-next v6 3/4] net: enetc: Initialize SerDes for SGMII and USXGMII protocols

[Vladimir Oltean]

On Thu, Jul 09, 2020 at 11:35:25PM +0200, Michael Walle wrote:
> ENETC has ethernet MACs capable of SGMII, 2500BaseX and USXGMII. But in
> order to use these protocols some SerDes configurations need to be
> performed. The SerDes is configurable via an internal PCS PHY which is
> connected to an internal MDIO bus at address 0.
> 
> This patch basically removes the dependency on bootloader regarding
> SerDes initialization.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> Reviewed-by: Claudiu Manoil <claudiu.manoil@nxp.com>
> ---

As mentioned earlier, this works for SGMII. For USXGMII, it would also
need this patch to work (something which is also true of Felix):

https://patchwork.ozlabs.org/project/netdev/patch/20200712164815.1763532-1-olteanv@gmail.com/

Considering that patch as external to this series:

Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com>

>  .../net/ethernet/freescale/enetc/enetc_hw.h   |   3 +
>  .../net/ethernet/freescale/enetc/enetc_pf.c   | 135 ++++++++++++++++++
>  .../net/ethernet/freescale/enetc/enetc_pf.h   |   2 +
>  3 files changed, 140 insertions(+)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> index fc357bc56835..135bf46354ea 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> @@ -224,6 +224,9 @@ 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
> +
>  #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_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> index 4fac57dbb3c8..662740874841 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
>  /* Copyright 2017-2019 NXP */
>  
> +#include <linux/mdio.h>
>  #include <linux/module.h>
>  #include <linux/fsl/enetc_mdio.h>
>  #include <linux/of_mdio.h>
> @@ -833,6 +834,135 @@ static void enetc_of_put_phy(struct enetc_ndev_priv *priv)
>  		of_node_put(priv->phy_node);
>  }
>  
> +static int enetc_imdio_init(struct enetc_pf *pf, bool is_c45)
> +{
> +	struct device *dev = &pf->si->pdev->dev;
> +	struct enetc_mdio_priv *mdio_priv;
> +	struct phy_device *pcs;
> +	struct mii_bus *bus;
> +	int err;
> +
> +	bus = mdiobus_alloc_size(sizeof(*mdio_priv));
> +	if (!bus)
> +		return -ENOMEM;
> +
> +	bus->name = "Freescale ENETC internal MDIO Bus";
> +	bus->read = enetc_mdio_read;
> +	bus->write = enetc_mdio_write;
> +	bus->parent = dev;
> +	bus->phy_mask = ~0;
> +	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-imdio", dev_name(dev));
> +
> +	err = mdiobus_register(bus);
> +	if (err) {
> +		dev_err(dev, "cannot register internal MDIO bus (%d)\n", err);
> +		goto free_mdio_bus;
> +	}
> +
> +	pcs = get_phy_device(bus, 0, is_c45);
> +	if (IS_ERR(pcs)) {
> +		err = PTR_ERR(pcs);
> +		dev_err(dev, "cannot get internal PCS PHY (%d)\n", err);
> +		goto unregister_mdiobus;
> +	}
> +
> +	pf->imdio = bus;
> +	pf->pcs = pcs;
> +
> +	return 0;
> +
> +unregister_mdiobus:
> +	mdiobus_unregister(bus);
> +free_mdio_bus:
> +	mdiobus_free(bus);
> +	return err;
> +}
> +
> +static void enetc_imdio_remove(struct enetc_pf *pf)
> +{
> +	if (pf->pcs)
> +		put_device(&pf->pcs->mdio.dev);
> +	if (pf->imdio) {
> +		mdiobus_unregister(pf->imdio);
> +		mdiobus_free(pf->imdio);
> +	}
> +}
> +
> +static void enetc_configure_sgmii(struct phy_device *pcs)
> +{
> +	/* SGMII spec requires tx_config_Reg[15:0] to be exactly 0x4001
> +	 * for the MAC PCS in order to acknowledge the AN.
> +	 */
> +	phy_write(pcs, MII_ADVERTISE, ADVERTISE_SGMII | ADVERTISE_LPACK);
> +
> +	phy_write(pcs, ENETC_PCS_IF_MODE,
> +		  ENETC_PCS_IF_MODE_SGMII_EN |
> +		  ENETC_PCS_IF_MODE_USE_SGMII_AN);
> +
> +	/* Adjust link timer for SGMII */
> +	phy_write(pcs, ENETC_PCS_LINK_TIMER1, ENETC_PCS_LINK_TIMER1_VAL);
> +	phy_write(pcs, ENETC_PCS_LINK_TIMER2, ENETC_PCS_LINK_TIMER2_VAL);
> +
> +	phy_write(pcs, MII_BMCR, BMCR_ANRESTART | BMCR_ANENABLE);
> +}
> +
> +static void enetc_configure_2500basex(struct phy_device *pcs)
> +{
> +	phy_write(pcs, ENETC_PCS_IF_MODE,
> +		  ENETC_PCS_IF_MODE_SGMII_EN |
> +		  ENETC_PCS_IF_MODE_SGMII_SPEED(ENETC_PCS_SPEED_2500));
> +
> +	phy_write(pcs, MII_BMCR, BMCR_SPEED1000 | BMCR_FULLDPLX | BMCR_RESET);
> +}
> +
> +static void enetc_configure_usxgmii(struct phy_device *pcs)
> +{
> +	/* Configure device ability for the USXGMII Replicator */
> +	phy_write_mmd(pcs, MDIO_MMD_VEND2, MII_ADVERTISE,
> +		      ADVERTISE_SGMII | ADVERTISE_LPACK |
> +		      MDIO_LPA_USXGMII_FULL_DUPLEX);
> +
> +	/* Restart PCS AN */
> +	phy_write_mmd(pcs, MDIO_MMD_VEND2, MII_BMCR,
> +		      BMCR_RESET | BMCR_ANENABLE | BMCR_ANRESTART);
> +}
> +
> +static int enetc_configure_serdes(struct enetc_ndev_priv *priv)
> +{
> +	bool is_c45 = priv->if_mode == PHY_INTERFACE_MODE_USXGMII;
> +	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_2500BASEX &&
> +	    priv->if_mode != PHY_INTERFACE_MODE_USXGMII)
> +		return 0;
> +
> +	err = enetc_imdio_init(pf, is_c45);
> +	if (err)
> +		return err;
> +
> +	switch (priv->if_mode) {
> +	case PHY_INTERFACE_MODE_SGMII:
> +		enetc_configure_sgmii(pf->pcs);
> +		break;
> +	case PHY_INTERFACE_MODE_2500BASEX:
> +		enetc_configure_2500basex(pf->pcs);
> +		break;
> +	case PHY_INTERFACE_MODE_USXGMII:
> +		enetc_configure_usxgmii(pf->pcs);
> +		break;
> +	default:
> +		dev_err(&pf->si->pdev->dev, "Unsupported link mode %s\n",
> +			phy_modes(priv->if_mode));
> +	}
> +
> +	return 0;
> +}
> +
>  static int enetc_pf_probe(struct pci_dev *pdev,
>  			  const struct pci_device_id *ent)
>  {
> @@ -897,6 +1027,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;
> @@ -932,6 +1066,7 @@ static void enetc_pf_remove(struct pci_dev *pdev)
>  	priv = netdev_priv(si->ndev);
>  	unregister_netdev(si->ndev);
>  
> +	enetc_imdio_remove(pf);
>  	enetc_mdio_remove(pf);
>  	enetc_of_put_phy(priv);
>  
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.h b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
> index 59e65a6f6c3e..2cb922b59f46 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.h
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
> @@ -44,6 +44,8 @@ struct enetc_pf {
>  	DECLARE_BITMAP(active_vlans, VLAN_N_VID);
>  
>  	struct mii_bus *mdio; /* saved for cleanup */
> +	struct mii_bus *imdio;
> +	struct phy_device *pcs;
>  };
>  
>  int enetc_msg_psi_init(struct enetc_pf *pf);
> -- 
> 2.20.1
> 

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

[Vladimir Oltean]

On Thu, Jul 09, 2020 at 11:35:26PM +0200, Michael Walle wrote:
> 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>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---

Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com>

>  .../net/ethernet/freescale/enetc/enetc_pf.c   | 57 ++++++++++---------
>  .../net/ethernet/freescale/enetc/enetc_pf.h   |  3 +
>  2 files changed, 34 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> index 662740874841..dfc3acc841df 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> @@ -482,7 +482,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));
> @@ -498,9 +499,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_interface_mode_is_rgmii(phy_mode))
>  		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_USXGMII)
>  		enetc_port_wr(hw, ENETC_PM0_IF_MODE, ENETC_PM0_IFM_XGMII);
>  }
>  
> @@ -524,7 +527,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);
>  
> @@ -776,27 +779,27 @@ static void enetc_mdio_remove(struct enetc_pf *pf)
>  		mdiobus_unregister(pf->mdio);
>  }
>  
> -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 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");
> @@ -804,15 +807,15 @@ 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;
>  		}
>  	}
>  
> -	err = of_get_phy_mode(np, &priv->if_mode);
> +	err = of_get_phy_mode(np, &pf->if_mode);
>  	if (err) {
> -		dev_err(priv->dev, "missing phy type\n");
> -		of_node_put(priv->phy_node);
> +		dev_err(dev, "missing phy type\n");
> +		of_node_put(pf->phy_node);
>  		if (of_phy_is_fixed_link(np))
>  			of_phy_deregister_fixed_link(np);
>  		else
> @@ -824,14 +827,14 @@ static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
>  	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 int enetc_imdio_init(struct enetc_pf *pf, bool is_c45)
> @@ -994,6 +997,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);
> @@ -1008,6 +1015,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);
>  
> @@ -1023,10 +1032,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");
> @@ -1040,7 +1045,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);
> @@ -1048,6 +1052,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);
>  
> @@ -1068,7 +1073,7 @@ static void enetc_pf_remove(struct pci_dev *pdev)
>  
>  	enetc_imdio_remove(pf);
>  	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 2cb922b59f46..0d0ee91282a5 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.h
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
> @@ -46,6 +46,9 @@ struct enetc_pf {
>  	struct mii_bus *mdio; /* saved for cleanup */
>  	struct mii_bus *imdio;
>  	struct phy_device *pcs;
> +
> +	struct device_node *phy_node;
> +	phy_interface_t if_mode;
>  };
>  
>  int enetc_msg_psi_init(struct enetc_pf *pf);
> -- 
> 2.20.1
> 

Re: [PATCH net-next v6 1/4] net: phy: add USXGMII link partner ability constants

[Andrew Lunn]

On Mon, Jul 13, 2020 at 07:34:16PM +0300, Vladimir Oltean wrote:
> On Thu, Jul 09, 2020 at 11:35:23PM +0200, Michael Walle wrote:
> > The constants are taken from the USXGMII Singleport Copper Interface
> > specification. The naming are based on the SGMII ones, but with an MDIO_
> > prefix.
> > 
> > Signed-off-by: Michael Walle <michael@walle.cc>
> > ---
> 
> Somebody would need to review this patch, as it is introducing UAPI.

Anybody have a link to the "USXGMII Singleport Copper Interface"
specification.

Thanks
	Andrew

Re: [PATCH net-next v6 1/4] net: phy: add USXGMII link partner ability constants

[Michael Walle]

Am 2020-07-13 19:01, schrieb Andrew Lunn:
> On Mon, Jul 13, 2020 at 07:34:16PM +0300, Vladimir Oltean wrote:
>> On Thu, Jul 09, 2020 at 11:35:23PM +0200, Michael Walle wrote:
>> > The constants are taken from the USXGMII Singleport Copper Interface
>> > specification. The naming are based on the SGMII ones, but with an MDIO_
>> > prefix.
>> >
>> > Signed-off-by: Michael Walle <michael@walle.cc>
>> > ---
>> 
>> Somebody would need to review this patch, as it is introducing UAPI.
> 
> Anybody have a link to the "USXGMII Singleport Copper Interface"
> specification.

You have to login at cisco (registration is free), see here:
   https://archive.nbaset.ethernetalliance.org/technology/specifications/

-michael

Re: [PATCH net-next v6 1/4] net: phy: add USXGMII link partner ability constants

[Russell King - ARM Linux admin]

On Thu, Jul 09, 2020 at 11:35:23PM +0200, Michael Walle wrote:
> The constants are taken from the USXGMII Singleport Copper Interface
> specification. The naming are based on the SGMII ones, but with an MDIO_
> prefix.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  include/uapi/linux/mdio.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
> index 4bcb41c71b8c..784723072578 100644
> --- a/include/uapi/linux/mdio.h
> +++ b/include/uapi/linux/mdio.h
> @@ -324,4 +324,30 @@ static inline __u16 mdio_phy_id_c45(int prtad, int devad)
>  	return MDIO_PHY_ID_C45 | (prtad << 5) | devad;
>  }
>  
> +/* UsxgmiiChannelInfo[15:0] for USXGMII in-band auto-negotiation.*/
> +#define MDIO_LPA_USXGMII_EEE_CLK_STP	0x0080	/* EEE clock stop supported */

Bit 7 is the EEE clock stop capability, set when supported.  Tick.

> +#define MDIO_LPA_USXGMII_EEE		0x0100	/* EEE supported */

Bit 8 is the EEE capability, set when supported.  Tick.

> +#define MDIO_LPA_USXGMII_SPD_MASK	0x0e00	/* USXGMII speed mask */

Bits 9 through 11 are the speed.  Tick.

> +#define MDIO_LPA_USXGMII_FULL_DUPLEX	0x1000	/* USXGMII full duplex */

Bit 12 is the duplex mode, set for full, clear for half.  Tick.

> +#define MDIO_LPA_USXGMII_DPX_SPD_MASK	0x1e00	/* USXGMII duplex and speed bits */
> +#define MDIO_LPA_USXGMII_10		0x0000	/* 10Mbps */
> +#define MDIO_LPA_USXGMII_10HALF		0x0000	/* 10Mbps half-duplex */
> +#define MDIO_LPA_USXGMII_10FULL		0x1000	/* 10Mbps full-duplex */
> +#define MDIO_LPA_USXGMII_100		0x0200	/* 100Mbps */
> +#define MDIO_LPA_USXGMII_100HALF	0x0200	/* 100Mbps half-duplex */
> +#define MDIO_LPA_USXGMII_100FULL	0x1200	/* 100Mbps full-duplex */
> +#define MDIO_LPA_USXGMII_1000		0x0400	/* 1000Mbps */
> +#define MDIO_LPA_USXGMII_1000HALF	0x0400	/* 1000Mbps half-duplex */
> +#define MDIO_LPA_USXGMII_1000FULL	0x1400	/* 1000Mbps full-duplex */
> +#define MDIO_LPA_USXGMII_10G		0x0600	/* 10Gbps */
> +#define MDIO_LPA_USXGMII_10GHALF	0x0600	/* 10Gbps half-duplex */
> +#define MDIO_LPA_USXGMII_10GFULL	0x1600	/* 10Gbps full-duplex */
> +#define MDIO_LPA_USXGMII_2500		0x0800	/* 2500Mbps */
> +#define MDIO_LPA_USXGMII_2500HALF	0x0800	/* 2500Mbps half-duplex */
> +#define MDIO_LPA_USXGMII_2500FULL	0x1800	/* 2500Mbps full-duplex */
> +#define MDIO_LPA_USXGMII_5000		0x0a00	/* 5000Mbps */
> +#define MDIO_LPA_USXGMII_5000HALF	0x0a00	/* 5000Mbps half-duplex */
> +#define MDIO_LPA_USXGMII_5000FULL	0x1a00	/* 5000Mbps full-duplex */
> +#define MDIO_LPA_USXGMII_LINK		0x8000	/* PHY link with copper-side partner */

Bit 15 is the link bit, set for link up.  Tick.

The speed bits correspond (they're a little harder to check, it would
have been easier for them to be 2 << 9 etc), tick.

The speed+duplex bits correspond (same issue with the raw speed bits,
defining them as MDIO_LPA_USXGMII_1000 | MDIO_LPA_USXGMII_FULL_DUPLEX
would've made them more obvious, but at the expense of being more
long winded), tick.

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

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v6 1/4] net: phy: add USXGMII link partner ability constants

[Russell King - ARM Linux admin]

On Thu, Jul 09, 2020 at 11:35:23PM +0200, Michael Walle wrote:
> The constants are taken from the USXGMII Singleport Copper Interface
> specification. The naming are based on the SGMII ones, but with an MDIO_
> prefix.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  include/uapi/linux/mdio.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
> index 4bcb41c71b8c..784723072578 100644
> --- a/include/uapi/linux/mdio.h
> +++ b/include/uapi/linux/mdio.h
> @@ -324,4 +324,30 @@ static inline __u16 mdio_phy_id_c45(int prtad, int devad)
>  	return MDIO_PHY_ID_C45 | (prtad << 5) | devad;
>  }
>  
> +/* UsxgmiiChannelInfo[15:0] for USXGMII in-band auto-negotiation.*/
> +#define MDIO_LPA_USXGMII_EEE_CLK_STP	0x0080	/* EEE clock stop supported */
> +#define MDIO_LPA_USXGMII_EEE		0x0100	/* EEE supported */
> +#define MDIO_LPA_USXGMII_SPD_MASK	0x0e00	/* USXGMII speed mask */
> +#define MDIO_LPA_USXGMII_FULL_DUPLEX	0x1000	/* USXGMII full duplex */
> +#define MDIO_LPA_USXGMII_DPX_SPD_MASK	0x1e00	/* USXGMII duplex and speed bits */
> +#define MDIO_LPA_USXGMII_10		0x0000	/* 10Mbps */
> +#define MDIO_LPA_USXGMII_10HALF		0x0000	/* 10Mbps half-duplex */
> +#define MDIO_LPA_USXGMII_10FULL		0x1000	/* 10Mbps full-duplex */
> +#define MDIO_LPA_USXGMII_100		0x0200	/* 100Mbps */
> +#define MDIO_LPA_USXGMII_100HALF	0x0200	/* 100Mbps half-duplex */
> +#define MDIO_LPA_USXGMII_100FULL	0x1200	/* 100Mbps full-duplex */
> +#define MDIO_LPA_USXGMII_1000		0x0400	/* 1000Mbps */
> +#define MDIO_LPA_USXGMII_1000HALF	0x0400	/* 1000Mbps half-duplex */
> +#define MDIO_LPA_USXGMII_1000FULL	0x1400	/* 1000Mbps full-duplex */
> +#define MDIO_LPA_USXGMII_10G		0x0600	/* 10Gbps */
> +#define MDIO_LPA_USXGMII_10GHALF	0x0600	/* 10Gbps half-duplex */
> +#define MDIO_LPA_USXGMII_10GFULL	0x1600	/* 10Gbps full-duplex */
> +#define MDIO_LPA_USXGMII_2500		0x0800	/* 2500Mbps */
> +#define MDIO_LPA_USXGMII_2500HALF	0x0800	/* 2500Mbps half-duplex */
> +#define MDIO_LPA_USXGMII_2500FULL	0x1800	/* 2500Mbps full-duplex */
> +#define MDIO_LPA_USXGMII_5000		0x0a00	/* 5000Mbps */
> +#define MDIO_LPA_USXGMII_5000HALF	0x0a00	/* 5000Mbps half-duplex */
> +#define MDIO_LPA_USXGMII_5000FULL	0x1a00	/* 5000Mbps full-duplex */
> +#define MDIO_LPA_USXGMII_LINK		0x8000	/* PHY link with copper-side partner */

btw, the only thing which is missing from this is bit 0.

One other point - in the USXGMII specification, this appears to be
somewhat symmetrical - the same definitions are listed as being
used for PHY to MAC as for MAC to PHY (presumably as part of the
acknowledgement that the MAC actually switched to that speed.)
So, it probably makes sense to drop the LPA_ infix.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v6 1/4] net: phy: add USXGMII link partner ability constants

[Michael Walle]

Am 2020-07-13 20:23, schrieb Russell King - ARM Linux admin:
> On Thu, Jul 09, 2020 at 11:35:23PM +0200, Michael Walle wrote:
>> The constants are taken from the USXGMII Singleport Copper Interface
>> specification. The naming are based on the SGMII ones, but with an 
>> MDIO_
>> prefix.
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>>  include/uapi/linux/mdio.h | 26 ++++++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>> 
>> diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
>> index 4bcb41c71b8c..784723072578 100644
>> --- a/include/uapi/linux/mdio.h
>> +++ b/include/uapi/linux/mdio.h
>> @@ -324,4 +324,30 @@ static inline __u16 mdio_phy_id_c45(int prtad, 
>> int devad)
>>  	return MDIO_PHY_ID_C45 | (prtad << 5) | devad;
>>  }
>> 
>> +/* UsxgmiiChannelInfo[15:0] for USXGMII in-band auto-negotiation.*/
>> +#define MDIO_LPA_USXGMII_EEE_CLK_STP	0x0080	/* EEE clock stop 
>> supported */
>> +#define MDIO_LPA_USXGMII_EEE		0x0100	/* EEE supported */
>> +#define MDIO_LPA_USXGMII_SPD_MASK	0x0e00	/* USXGMII speed mask */
>> +#define MDIO_LPA_USXGMII_FULL_DUPLEX	0x1000	/* USXGMII full duplex */
>> +#define MDIO_LPA_USXGMII_DPX_SPD_MASK	0x1e00	/* USXGMII duplex and 
>> speed bits */
>> +#define MDIO_LPA_USXGMII_10		0x0000	/* 10Mbps */
>> +#define MDIO_LPA_USXGMII_10HALF		0x0000	/* 10Mbps half-duplex */
>> +#define MDIO_LPA_USXGMII_10FULL		0x1000	/* 10Mbps full-duplex */
>> +#define MDIO_LPA_USXGMII_100		0x0200	/* 100Mbps */
>> +#define MDIO_LPA_USXGMII_100HALF	0x0200	/* 100Mbps half-duplex */
>> +#define MDIO_LPA_USXGMII_100FULL	0x1200	/* 100Mbps full-duplex */
>> +#define MDIO_LPA_USXGMII_1000		0x0400	/* 1000Mbps */
>> +#define MDIO_LPA_USXGMII_1000HALF	0x0400	/* 1000Mbps half-duplex */
>> +#define MDIO_LPA_USXGMII_1000FULL	0x1400	/* 1000Mbps full-duplex */
>> +#define MDIO_LPA_USXGMII_10G		0x0600	/* 10Gbps */
>> +#define MDIO_LPA_USXGMII_10GHALF	0x0600	/* 10Gbps half-duplex */
>> +#define MDIO_LPA_USXGMII_10GFULL	0x1600	/* 10Gbps full-duplex */
>> +#define MDIO_LPA_USXGMII_2500		0x0800	/* 2500Mbps */
>> +#define MDIO_LPA_USXGMII_2500HALF	0x0800	/* 2500Mbps half-duplex */
>> +#define MDIO_LPA_USXGMII_2500FULL	0x1800	/* 2500Mbps full-duplex */
>> +#define MDIO_LPA_USXGMII_5000		0x0a00	/* 5000Mbps */
>> +#define MDIO_LPA_USXGMII_5000HALF	0x0a00	/* 5000Mbps half-duplex */
>> +#define MDIO_LPA_USXGMII_5000FULL	0x1a00	/* 5000Mbps full-duplex */
>> +#define MDIO_LPA_USXGMII_LINK		0x8000	/* PHY link with copper-side 
>> partner */
> 
> btw, the only thing which is missing from this is bit 0.

TBH, I didn't know how to name it. Any suggestions?

> One other point - in the USXGMII specification, this appears to be
> somewhat symmetrical - the same definitions are listed as being
> used for PHY to MAC as for MAC to PHY (presumably as part of the
> acknowledgement that the MAC actually switched to that speed.)
> So, it probably makes sense to drop the LPA_ infix.

Ok. I'll send a new version, once we have a name for bit 0.

-michael

Re: [PATCH net-next v6 1/4] net: phy: add USXGMII link partner ability constants

[Michael Walle]

Am 2020-07-13 20:37, schrieb Michael Walle:
> Am 2020-07-13 20:23, schrieb Russell King - ARM Linux admin:
>> On Thu, Jul 09, 2020 at 11:35:23PM +0200, Michael Walle wrote:
>>> The constants are taken from the USXGMII Singleport Copper Interface
>>> specification. The naming are based on the SGMII ones, but with an 
>>> MDIO_
>>> prefix.
>>> 
>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>> ---
>>>  include/uapi/linux/mdio.h | 26 ++++++++++++++++++++++++++
>>>  1 file changed, 26 insertions(+)
>>> 
>>> diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
>>> index 4bcb41c71b8c..784723072578 100644
>>> --- a/include/uapi/linux/mdio.h
>>> +++ b/include/uapi/linux/mdio.h
>>> @@ -324,4 +324,30 @@ static inline __u16 mdio_phy_id_c45(int prtad, 
>>> int devad)
>>>  	return MDIO_PHY_ID_C45 | (prtad << 5) | devad;
>>>  }
>>> 
>>> +/* UsxgmiiChannelInfo[15:0] for USXGMII in-band auto-negotiation.*/
>>> +#define MDIO_LPA_USXGMII_EEE_CLK_STP	0x0080	/* EEE clock stop 
>>> supported */
>>> +#define MDIO_LPA_USXGMII_EEE		0x0100	/* EEE supported */
>>> +#define MDIO_LPA_USXGMII_SPD_MASK	0x0e00	/* USXGMII speed mask */
>>> +#define MDIO_LPA_USXGMII_FULL_DUPLEX	0x1000	/* USXGMII full duplex 
>>> */
>>> +#define MDIO_LPA_USXGMII_DPX_SPD_MASK	0x1e00	/* USXGMII duplex and 
>>> speed bits */
>>> +#define MDIO_LPA_USXGMII_10		0x0000	/* 10Mbps */
>>> +#define MDIO_LPA_USXGMII_10HALF		0x0000	/* 10Mbps half-duplex */
>>> +#define MDIO_LPA_USXGMII_10FULL		0x1000	/* 10Mbps full-duplex */
>>> +#define MDIO_LPA_USXGMII_100		0x0200	/* 100Mbps */
>>> +#define MDIO_LPA_USXGMII_100HALF	0x0200	/* 100Mbps half-duplex */
>>> +#define MDIO_LPA_USXGMII_100FULL	0x1200	/* 100Mbps full-duplex */
>>> +#define MDIO_LPA_USXGMII_1000		0x0400	/* 1000Mbps */
>>> +#define MDIO_LPA_USXGMII_1000HALF	0x0400	/* 1000Mbps half-duplex */
>>> +#define MDIO_LPA_USXGMII_1000FULL	0x1400	/* 1000Mbps full-duplex */
>>> +#define MDIO_LPA_USXGMII_10G		0x0600	/* 10Gbps */
>>> +#define MDIO_LPA_USXGMII_10GHALF	0x0600	/* 10Gbps half-duplex */
>>> +#define MDIO_LPA_USXGMII_10GFULL	0x1600	/* 10Gbps full-duplex */
>>> +#define MDIO_LPA_USXGMII_2500		0x0800	/* 2500Mbps */
>>> +#define MDIO_LPA_USXGMII_2500HALF	0x0800	/* 2500Mbps half-duplex */
>>> +#define MDIO_LPA_USXGMII_2500FULL	0x1800	/* 2500Mbps full-duplex */
>>> +#define MDIO_LPA_USXGMII_5000		0x0a00	/* 5000Mbps */
>>> +#define MDIO_LPA_USXGMII_5000HALF	0x0a00	/* 5000Mbps half-duplex */
>>> +#define MDIO_LPA_USXGMII_5000FULL	0x1a00	/* 5000Mbps full-duplex */
>>> +#define MDIO_LPA_USXGMII_LINK		0x8000	/* PHY link with copper-side 
>>> partner */
>> 
>> btw, the only thing which is missing from this is bit 0.
> 
> TBH, I didn't know how to name it. Any suggestions?

NXP calls it ABIL0, in xilinx docs its called USXGMII [1]. In the 
USXGMII
spec, its "set to 1 (0 is SGMII)" which I don't understand because its
also 1 for SGMII, right? At least as described in the tx_configReg[15:0] 
in
the SGMII spec.

#define MDIO_USXGMII_USXGMII 0x0001 ?

-michael

[1] 
https://www.xilinx.com/support/documentation/ip_documentation/usxgmii/v1_0/pg251-usxgmii.pdf

Re: [PATCH net-next v6 1/4] net: phy: add USXGMII link partner ability constants

[Vladimir Oltean]

On Wed, Jul 15, 2020 at 10:33:23PM +0200, Michael Walle wrote:
> Am 2020-07-13 20:37, schrieb Michael Walle:
> > Am 2020-07-13 20:23, schrieb Russell King - ARM Linux admin:
> > > On Thu, Jul 09, 2020 at 11:35:23PM +0200, Michael Walle wrote:
> > > > The constants are taken from the USXGMII Singleport Copper Interface
> > > > specification. The naming are based on the SGMII ones, but with
> > > > an MDIO_
> > > > prefix.
> > > > 
> > > > Signed-off-by: Michael Walle <michael@walle.cc>
> > > > ---
> > > >  include/uapi/linux/mdio.h | 26 ++++++++++++++++++++++++++
> > > >  1 file changed, 26 insertions(+)
> > > > 
> > > > diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
> > > > index 4bcb41c71b8c..784723072578 100644
> > > > --- a/include/uapi/linux/mdio.h
> > > > +++ b/include/uapi/linux/mdio.h
> > > > @@ -324,4 +324,30 @@ static inline __u16 mdio_phy_id_c45(int
> > > > prtad, int devad)
> > > >  	return MDIO_PHY_ID_C45 | (prtad << 5) | devad;
> > > >  }
> > > > 
> > > > +/* UsxgmiiChannelInfo[15:0] for USXGMII in-band auto-negotiation.*/
> > > > +#define MDIO_LPA_USXGMII_EEE_CLK_STP	0x0080	/* EEE clock stop
> > > > supported */
> > > > +#define MDIO_LPA_USXGMII_EEE		0x0100	/* EEE supported */
> > > > +#define MDIO_LPA_USXGMII_SPD_MASK	0x0e00	/* USXGMII speed mask */
> > > > +#define MDIO_LPA_USXGMII_FULL_DUPLEX	0x1000	/* USXGMII full
> > > > duplex */
> > > > +#define MDIO_LPA_USXGMII_DPX_SPD_MASK	0x1e00	/* USXGMII duplex
> > > > and speed bits */
> > > > +#define MDIO_LPA_USXGMII_10		0x0000	/* 10Mbps */
> > > > +#define MDIO_LPA_USXGMII_10HALF		0x0000	/* 10Mbps half-duplex */
> > > > +#define MDIO_LPA_USXGMII_10FULL		0x1000	/* 10Mbps full-duplex */
> > > > +#define MDIO_LPA_USXGMII_100		0x0200	/* 100Mbps */
> > > > +#define MDIO_LPA_USXGMII_100HALF	0x0200	/* 100Mbps half-duplex */
> > > > +#define MDIO_LPA_USXGMII_100FULL	0x1200	/* 100Mbps full-duplex */
> > > > +#define MDIO_LPA_USXGMII_1000		0x0400	/* 1000Mbps */
> > > > +#define MDIO_LPA_USXGMII_1000HALF	0x0400	/* 1000Mbps half-duplex */
> > > > +#define MDIO_LPA_USXGMII_1000FULL	0x1400	/* 1000Mbps full-duplex */
> > > > +#define MDIO_LPA_USXGMII_10G		0x0600	/* 10Gbps */
> > > > +#define MDIO_LPA_USXGMII_10GHALF	0x0600	/* 10Gbps half-duplex */
> > > > +#define MDIO_LPA_USXGMII_10GFULL	0x1600	/* 10Gbps full-duplex */
> > > > +#define MDIO_LPA_USXGMII_2500		0x0800	/* 2500Mbps */
> > > > +#define MDIO_LPA_USXGMII_2500HALF	0x0800	/* 2500Mbps half-duplex */
> > > > +#define MDIO_LPA_USXGMII_2500FULL	0x1800	/* 2500Mbps full-duplex */
> > > > +#define MDIO_LPA_USXGMII_5000		0x0a00	/* 5000Mbps */
> > > > +#define MDIO_LPA_USXGMII_5000HALF	0x0a00	/* 5000Mbps half-duplex */
> > > > +#define MDIO_LPA_USXGMII_5000FULL	0x1a00	/* 5000Mbps full-duplex */
> > > > +#define MDIO_LPA_USXGMII_LINK		0x8000	/* PHY link with
> > > > copper-side partner */
> > > 
> > > btw, the only thing which is missing from this is bit 0.
> > 
> > TBH, I didn't know how to name it. Any suggestions?
> 
> NXP calls it ABIL0, in xilinx docs its called USXGMII [1]. In the USXGMII
> spec, its "set to 1 (0 is SGMII)" which I don't understand because its
> also 1 for SGMII, right? At least as described in the tx_configReg[15:0] in
> the SGMII spec.
> 
> #define MDIO_USXGMII_USXGMII 0x0001 ?
> 
> -michael
> 
> [1] https://www.xilinx.com/support/documentation/ip_documentation/usxgmii/v1_0/pg251-usxgmii.pdf

The explanation in the spec is quite cryptic, I've taken that to mean
"corresponds to bit 0 in SGMII". Hence the reason why, in the code I've
introduced in Felix, this is simply used as ADVERTISE_SGMII. I have no
problem in creating an alias to ADVERTISE_SGMII named
MDIO_LPA_USXGMII_SGMII. That being said, I don't see, right now, a
practical situation where you might want to parse bit 0 from LPA, it's
just like in Forrest Gump: "life is like a box of chocolates, you never
know what you're gonna get". Adding it now to the UAPI might very well
be a non-issue.

-Vladimir

Re: [PATCH net-next v6 1/4] net: phy: add USXGMII link partner ability constants

[Michael Walle]

Am 2020-07-16 00:44, schrieb Vladimir Oltean:
> On Wed, Jul 15, 2020 at 10:33:23PM +0200, Michael Walle wrote:
>> Am 2020-07-13 20:37, schrieb Michael Walle:
>> > Am 2020-07-13 20:23, schrieb Russell King - ARM Linux admin:
>> > > On Thu, Jul 09, 2020 at 11:35:23PM +0200, Michael Walle wrote:
>> > > > The constants are taken from the USXGMII Singleport Copper Interface
>> > > > specification. The naming are based on the SGMII ones, but with
>> > > > an MDIO_
>> > > > prefix.
>> > > >
>> > > > Signed-off-by: Michael Walle <michael@walle.cc>
>> > > > ---
>> > > >  include/uapi/linux/mdio.h | 26 ++++++++++++++++++++++++++
>> > > >  1 file changed, 26 insertions(+)
>> > > >
>> > > > diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
>> > > > index 4bcb41c71b8c..784723072578 100644
>> > > > --- a/include/uapi/linux/mdio.h
>> > > > +++ b/include/uapi/linux/mdio.h
>> > > > @@ -324,4 +324,30 @@ static inline __u16 mdio_phy_id_c45(int
>> > > > prtad, int devad)
>> > > >  	return MDIO_PHY_ID_C45 | (prtad << 5) | devad;
>> > > >  }
>> > > >
>> > > > +/* UsxgmiiChannelInfo[15:0] for USXGMII in-band auto-negotiation.*/
>> > > > +#define MDIO_LPA_USXGMII_EEE_CLK_STP	0x0080	/* EEE clock stop
>> > > > supported */
>> > > > +#define MDIO_LPA_USXGMII_EEE		0x0100	/* EEE supported */
>> > > > +#define MDIO_LPA_USXGMII_SPD_MASK	0x0e00	/* USXGMII speed mask */
>> > > > +#define MDIO_LPA_USXGMII_FULL_DUPLEX	0x1000	/* USXGMII full
>> > > > duplex */
>> > > > +#define MDIO_LPA_USXGMII_DPX_SPD_MASK	0x1e00	/* USXGMII duplex
>> > > > and speed bits */
>> > > > +#define MDIO_LPA_USXGMII_10		0x0000	/* 10Mbps */
>> > > > +#define MDIO_LPA_USXGMII_10HALF		0x0000	/* 10Mbps half-duplex */
>> > > > +#define MDIO_LPA_USXGMII_10FULL		0x1000	/* 10Mbps full-duplex */
>> > > > +#define MDIO_LPA_USXGMII_100		0x0200	/* 100Mbps */
>> > > > +#define MDIO_LPA_USXGMII_100HALF	0x0200	/* 100Mbps half-duplex */
>> > > > +#define MDIO_LPA_USXGMII_100FULL	0x1200	/* 100Mbps full-duplex */
>> > > > +#define MDIO_LPA_USXGMII_1000		0x0400	/* 1000Mbps */
>> > > > +#define MDIO_LPA_USXGMII_1000HALF	0x0400	/* 1000Mbps half-duplex */
>> > > > +#define MDIO_LPA_USXGMII_1000FULL	0x1400	/* 1000Mbps full-duplex */
>> > > > +#define MDIO_LPA_USXGMII_10G		0x0600	/* 10Gbps */
>> > > > +#define MDIO_LPA_USXGMII_10GHALF	0x0600	/* 10Gbps half-duplex */
>> > > > +#define MDIO_LPA_USXGMII_10GFULL	0x1600	/* 10Gbps full-duplex */
>> > > > +#define MDIO_LPA_USXGMII_2500		0x0800	/* 2500Mbps */
>> > > > +#define MDIO_LPA_USXGMII_2500HALF	0x0800	/* 2500Mbps half-duplex */
>> > > > +#define MDIO_LPA_USXGMII_2500FULL	0x1800	/* 2500Mbps full-duplex */
>> > > > +#define MDIO_LPA_USXGMII_5000		0x0a00	/* 5000Mbps */
>> > > > +#define MDIO_LPA_USXGMII_5000HALF	0x0a00	/* 5000Mbps half-duplex */
>> > > > +#define MDIO_LPA_USXGMII_5000FULL	0x1a00	/* 5000Mbps full-duplex */
>> > > > +#define MDIO_LPA_USXGMII_LINK		0x8000	/* PHY link with
>> > > > copper-side partner */
>> > >
>> > > btw, the only thing which is missing from this is bit 0.
>> >
>> > TBH, I didn't know how to name it. Any suggestions?
>> 
>> NXP calls it ABIL0, in xilinx docs its called USXGMII [1]. In the 
>> USXGMII
>> spec, its "set to 1 (0 is SGMII)" which I don't understand because its
>> also 1 for SGMII, right? At least as described in the 
>> tx_configReg[15:0] in
>> the SGMII spec.
>> 
>> #define MDIO_USXGMII_USXGMII 0x0001 ?
>> 
>> -michael
>> 
>> [1] 
>> https://www.xilinx.com/support/documentation/ip_documentation/usxgmii/v1_0/pg251-usxgmii.pdf
> 
> The explanation in the spec is quite cryptic, I've taken that to mean
> "corresponds to bit 0 in SGMII". Hence the reason why, in the code I've
> introduced in Felix, this is simply used as ADVERTISE_SGMII. I have no
> problem in creating an alias to ADVERTISE_SGMII named
> MDIO_LPA_USXGMII_SGMII. That being said, I don't see, right now, a
> practical situation where you might want to parse bit 0 from LPA, it's
> just like in Forrest Gump: "life is like a box of chocolates, you never
> know what you're gonna get". Adding it now to the UAPI might very well
> be a non-issue.

what about
   #define MDIO_USXGMII_ADVERTISE 0x0001 /* must always be set */

Russell, do you agree? Then I'd send a new version with your 
"Reviewed-by:".

-michael