[PATCH v4 0/5] net: macb: cover letter

[PATCH v4 0/5] net: macb: cover letter

[Parshuram Thombare]

Hello !

This is 4th version of patch set containing following patches
for Cadence ethernet controller driver.

1. 0001-net-macb-add-phylink-support.patch
   Replace phylib API's with phylink API's.
2. 0002-net-macb-add-support-for-sgmii-MAC-PHY-interface.patch
   This patch add support for SGMII mode.
3. 0004-net-macb-add-support-for-c45-PHY.patch
   This patch is to support C45 PHY.
4. 0005-net-macb-add-support-for-high-speed-interface
   This patch add support for 10G USXGMII PCS in fixed mode.
5. 0006-net-macb-parameter-added-to-cadence-ethernet-controller-DT-binding
   New parameter added to Cadence ethernet controller DT binding
   for USXGMII interface.

Changes in v2:
1. Dropped patch configuring TI PHY DP83867 from
   Cadence PCI wrapper driver.
2. Removed code registering emulated PHY for fixed mode. 
3. Code reformatting as per Andrew's and Florian's suggestions.

Changes in v3:
Based on Russell's suggestions
1. Configure MAC in mac_config only for non in-band modes
2. Handle dynamic phy_mode changes in mac_config
3. Move MAC configurations to mac_config
4. Removed seemingly redundant check for phylink handle
5. Removed code from mac_an_restart and mac_link_state
   now just return -EOPNOTSUPP

Changes in v4:
1. Removed PHY_INTERFACE_MODE_2500BASEX, PHY_INTERFACE_MODE_1000BASEX and
   2.5G PHY_INTERFACE_MODE_SGMII phy modes from supported modes

Regards,
Parshuram Thombare

Parshuram Thombare (5):
  net: macb: add phylink support
  net: macb: add support for sgmii MAC-PHY interface
  net: macb: add support for c45 PHY
  net: macb: add support for high speed interface
  net: macb: parameter added to cadence ethernet controller DT binding

 .../devicetree/bindings/net/macb.txt          |   3 +
 drivers/net/ethernet/cadence/Kconfig          |   2 +-
 drivers/net/ethernet/cadence/macb.h           | 113 +++-
 drivers/net/ethernet/cadence/macb_main.c      | 589 +++++++++++++-----
 4 files changed, 524 insertions(+), 183 deletions(-)

-- 
2.17.1

[PATCH v4 1/5] net: macb: add phylink support

[Parshuram Thombare]

This patch replace phylib API's by phylink API's.

Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
 drivers/net/ethernet/cadence/Kconfig     |   2 +-
 drivers/net/ethernet/cadence/macb.h      |   3 +
 drivers/net/ethernet/cadence/macb_main.c | 304 ++++++++++++-----------
 3 files changed, 166 insertions(+), 143 deletions(-)

diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
index 1766697c9c5a..d71411a71587 100644
--- a/drivers/net/ethernet/cadence/Kconfig
+++ b/drivers/net/ethernet/cadence/Kconfig
@@ -22,7 +22,7 @@ if NET_VENDOR_CADENCE
 config MACB
 	tristate "Cadence MACB/GEM support"
 	depends on HAS_DMA
-	select PHYLIB
+	select PHYLINK
 	---help---
 	  The Cadence MACB ethernet interface is found on many Atmel AT32 and
 	  AT91 parts.  This driver also supports the Cadence GEM (Gigabit
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 6ff123da6a14..8629d345af31 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -11,6 +11,7 @@
 #include <linux/ptp_clock_kernel.h>
 #include <linux/net_tstamp.h>
 #include <linux/interrupt.h>
+#include <linux/phylink.h>
 
 #if defined(CONFIG_ARCH_DMA_ADDR_T_64BIT) || defined(CONFIG_MACB_USE_HWSTAMP)
 #define MACB_EXT_DESC
@@ -1224,6 +1225,8 @@ struct macb {
 	u32	rx_intr_mask;
 
 	struct macb_pm_data pm_data;
+	struct phylink *pl;
+	struct phylink_config pl_config;
 };
 
 #ifdef CONFIG_MACB_USE_HWSTAMP
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index b4fa0111cd7a..1d6527a5313f 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -36,6 +36,7 @@
 #include <linux/tcp.h>
 #include <linux/iopoll.h>
 #include <linux/pm_runtime.h>
+#include <linux/phylink.h>
 #include "macb.h"
 
 /* This structure is only used for MACB on SiFive FU540 devices */
@@ -435,115 +436,145 @@ static void macb_set_tx_clk(struct clk *clk, int speed, struct net_device *dev)
 		netdev_err(dev, "adjusting tx_clk failed.\n");
 }
 
-static void macb_handle_link_change(struct net_device *dev)
+static void gem_phylink_validate(struct phylink_config *pl_config,
+				 unsigned long *supported,
+				 struct phylink_link_state *state)
 {
-	struct macb *bp = netdev_priv(dev);
-	struct phy_device *phydev = dev->phydev;
-	unsigned long flags;
-	int status_change = 0;
+	struct net_device *netdev = to_net_dev(pl_config->dev);
+	struct macb *bp = netdev_priv(netdev);
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
+
+	switch (state->interface) {
+	case PHY_INTERFACE_MODE_GMII:
+	case PHY_INTERFACE_MODE_RGMII:
+		if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE) {
+			phylink_set(mask, 1000baseT_Full);
+			phylink_set(mask, 1000baseX_Full);
+			if (!(bp->caps & MACB_CAPS_NO_GIGABIT_HALF)) {
+				phylink_set(mask, 1000baseT_Half);
+				phylink_set(mask, 1000baseT_Half);
+			}
+		}
+	/* fallthrough */
+	case PHY_INTERFACE_MODE_MII:
+	case PHY_INTERFACE_MODE_RMII:
+		phylink_set(mask, 10baseT_Half);
+		phylink_set(mask, 10baseT_Full);
+		phylink_set(mask, 100baseT_Half);
+		phylink_set(mask, 100baseT_Full);
+		break;
+	default:
+		break;
+	}
 
-	spin_lock_irqsave(&bp->lock, flags);
+	linkmode_and(supported, supported, mask);
+	linkmode_and(state->advertising, state->advertising, mask);
 
-	if (phydev->link) {
-		if ((bp->speed != phydev->speed) ||
-		    (bp->duplex != phydev->duplex)) {
-			u32 reg;
+}
 
-			reg = macb_readl(bp, NCFGR);
-			reg &= ~(MACB_BIT(SPD) | MACB_BIT(FD));
-			if (macb_is_gem(bp))
-				reg &= ~GEM_BIT(GBE);
+static int gem_phylink_mac_link_state(struct phylink_config *pl_config,
+				      struct phylink_link_state *state)
+{
+	return -EOPNOTSUPP;
+}
 
-			if (phydev->duplex)
-				reg |= MACB_BIT(FD);
-			if (phydev->speed == SPEED_100)
-				reg |= MACB_BIT(SPD);
-			if (phydev->speed == SPEED_1000 &&
-			    bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE)
-				reg |= GEM_BIT(GBE);
+static void gem_mac_config(struct phylink_config *pl_config, unsigned int mode,
+			   const struct phylink_link_state *state)
+{
+	struct net_device *netdev = to_net_dev(pl_config->dev);
+	struct macb *bp = netdev_priv(netdev);
+	unsigned long flags;
 
-			macb_or_gem_writel(bp, NCFGR, reg);
+	spin_lock_irqsave(&bp->lock, flags);
 
-			bp->speed = phydev->speed;
-			bp->duplex = phydev->duplex;
-			status_change = 1;
-		}
-	}
+	if (!phylink_autoneg_inband(mode) &&
+	    (bp->speed != state->speed ||
+	     bp->duplex != state->duplex)) {
+		u32 reg;
 
-	if (phydev->link != bp->link) {
-		if (!phydev->link) {
-			bp->speed = 0;
-			bp->duplex = -1;
+		reg = macb_readl(bp, NCFGR);
+		reg &= ~(MACB_BIT(SPD) | MACB_BIT(FD));
+		if (macb_is_gem(bp))
+			reg &= ~GEM_BIT(GBE);
+		if (state->duplex)
+			reg |= MACB_BIT(FD);
+
+		switch (state->speed) {
+		case SPEED_1000:
+			reg |= GEM_BIT(GBE);
+			break;
+		case SPEED_100:
+			reg |= MACB_BIT(SPD);
+			break;
+		default:
+			break;
 		}
-		bp->link = phydev->link;
+		macb_or_gem_writel(bp, NCFGR, reg);
+
+		bp->speed = state->speed;
+		bp->duplex = state->duplex;
 
-		status_change = 1;
+		if (state->link)
+			macb_set_tx_clk(bp->tx_clk, state->speed, netdev);
 	}
 
 	spin_unlock_irqrestore(&bp->lock, flags);
+}
 
-	if (status_change) {
-		if (phydev->link) {
-			/* Update the TX clock rate if and only if the link is
-			 * up and there has been a link change.
-			 */
-			macb_set_tx_clk(bp->tx_clk, phydev->speed, dev);
+static void gem_mac_link_up(struct phylink_config *pl_config, unsigned int mode,
+			    phy_interface_t interface, struct phy_device *phy)
+{
+	struct net_device *netdev = to_net_dev(pl_config->dev);
+	struct macb *bp = netdev_priv(netdev);
 
-			netif_carrier_on(dev);
-			netdev_info(dev, "link up (%d/%s)\n",
-				    phydev->speed,
-				    phydev->duplex == DUPLEX_FULL ?
-				    "Full" : "Half");
-		} else {
-			netif_carrier_off(dev);
-			netdev_info(dev, "link down\n");
-		}
-	}
+	bp->link = 1;
+	/* Enable TX and RX */
+	macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(RE) | MACB_BIT(TE));
+}
+
+static void gem_mac_link_down(struct phylink_config *pl_config,
+			      unsigned int mode, phy_interface_t interface)
+{
+	struct net_device *netdev = to_net_dev(pl_config->dev);
+	struct macb *bp = netdev_priv(netdev);
+
+	bp->link = 0;
+	/* Disable TX and RX */
+	macb_writel(bp, NCR,
+		    macb_readl(bp, NCR) & ~(MACB_BIT(RE) | MACB_BIT(TE)));
 }
 
+static const struct phylink_mac_ops gem_phylink_ops = {
+	.validate = gem_phylink_validate,
+	.mac_link_state = gem_phylink_mac_link_state,
+	.mac_config = gem_mac_config,
+	.mac_link_up = gem_mac_link_up,
+	.mac_link_down = gem_mac_link_down,
+};
+
 /* based on au1000_eth. c*/
 static int macb_mii_probe(struct net_device *dev)
 {
 	struct macb *bp = netdev_priv(dev);
 	struct phy_device *phydev;
 	struct device_node *np;
-	int ret, i;
+	int ret;
 
 	np = bp->pdev->dev.of_node;
 	ret = 0;
 
-	if (np) {
-		if (of_phy_is_fixed_link(np)) {
-			bp->phy_node = of_node_get(np);
-		} else {
-			bp->phy_node = of_parse_phandle(np, "phy-handle", 0);
-			/* fallback to standard phy registration if no
-			 * phy-handle was found nor any phy found during
-			 * dt phy registration
-			 */
-			if (!bp->phy_node && !phy_find_first(bp->mii_bus)) {
-				for (i = 0; i < PHY_MAX_ADDR; i++) {
-					phydev = mdiobus_scan(bp->mii_bus, i);
-					if (IS_ERR(phydev) &&
-					    PTR_ERR(phydev) != -ENODEV) {
-						ret = PTR_ERR(phydev);
-						break;
-					}
-				}
-
-				if (ret)
-					return -ENODEV;
-			}
-		}
+	bp->pl_config.dev = &dev->dev;
+	bp->pl_config.type = PHYLINK_NETDEV;
+	bp->pl = phylink_create(&bp->pl_config, of_fwnode_handle(np),
+				bp->phy_interface, &gem_phylink_ops);
+	if (IS_ERR(bp->pl)) {
+		netdev_err(dev,
+			   "error creating PHYLINK: %ld\n", PTR_ERR(bp->pl));
+		return PTR_ERR(bp->pl);
 	}
 
-	if (bp->phy_node) {
-		phydev = of_phy_connect(dev, bp->phy_node,
-					&macb_handle_link_change, 0,
-					bp->phy_interface);
-		if (!phydev)
-			return -ENODEV;
-	} else {
+	ret = phylink_of_phy_connect(bp->pl, np, 0);
+	if (ret == -ENODEV && bp->mii_bus) {
 		phydev = phy_find_first(bp->mii_bus);
 		if (!phydev) {
 			netdev_err(dev, "no PHY found\n");
@@ -551,29 +582,18 @@ static int macb_mii_probe(struct net_device *dev)
 		}
 
 		/* attach the mac to the phy */
-		ret = phy_connect_direct(dev, phydev, &macb_handle_link_change,
-					 bp->phy_interface);
+		ret = phylink_connect_phy(bp->pl, phydev);
 		if (ret) {
 			netdev_err(dev, "Could not attach to PHY\n");
 			return ret;
 		}
 	}
 
-	/* mask with MAC supported features */
-	if (macb_is_gem(bp) && bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE)
-		phy_set_max_speed(phydev, SPEED_1000);
-	else
-		phy_set_max_speed(phydev, SPEED_100);
-
-	if (bp->caps & MACB_CAPS_NO_GIGABIT_HALF)
-		phy_remove_link_mode(phydev,
-				     ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
-
 	bp->link = 0;
 	bp->speed = 0;
 	bp->duplex = -1;
 
-	return 0;
+	return ret;
 }
 
 static int macb_mii_init(struct macb *bp)
@@ -601,17 +621,7 @@ static int macb_mii_init(struct macb *bp)
 	dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
 
 	np = bp->pdev->dev.of_node;
-	if (np && of_phy_is_fixed_link(np)) {
-		if (of_phy_register_fixed_link(np) < 0) {
-			dev_err(&bp->pdev->dev,
-				"broken fixed-link specification %pOF\n", np);
-			goto err_out_free_mdiobus;
-		}
-
-		err = mdiobus_register(bp->mii_bus);
-	} else {
-		err = of_mdiobus_register(bp->mii_bus, np);
-	}
+	err = of_mdiobus_register(bp->mii_bus, np);
 
 	if (err)
 		goto err_out_free_fixed_link;
@@ -627,7 +637,6 @@ static int macb_mii_init(struct macb *bp)
 err_out_free_fixed_link:
 	if (np && of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
-err_out_free_mdiobus:
 	of_node_put(bp->phy_node);
 	mdiobus_free(bp->mii_bus);
 err_out:
@@ -2418,12 +2427,6 @@ static int macb_open(struct net_device *dev)
 	/* carrier starts down */
 	netif_carrier_off(dev);
 
-	/* if the phy is not yet register, retry later*/
-	if (!dev->phydev) {
-		err = -EAGAIN;
-		goto pm_exit;
-	}
-
 	/* RX buffers initialization */
 	macb_init_rx_buffer_size(bp, bufsz);
 
@@ -2441,7 +2444,7 @@ static int macb_open(struct net_device *dev)
 	macb_init_hw(bp);
 
 	/* schedule a link state check */
-	phy_start(dev->phydev);
+	phylink_start(bp->pl);
 
 	netif_tx_start_all_queues(dev);
 
@@ -2468,8 +2471,7 @@ static int macb_close(struct net_device *dev)
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue)
 		napi_disable(&queue->napi);
 
-	if (dev->phydev)
-		phy_stop(dev->phydev);
+	phylink_stop(bp->pl);
 
 	spin_lock_irqsave(&bp->lock, flags);
 	macb_reset_hw(bp);
@@ -3158,6 +3160,23 @@ static int gem_set_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd)
 	return ret;
 }
 
+static int gem_ethtool_get_link_ksettings(struct net_device *netdev,
+					  struct ethtool_link_ksettings *cmd)
+{
+	struct macb *bp = netdev_priv(netdev);
+
+	return phylink_ethtool_ksettings_get(bp->pl, cmd);
+}
+
+static int
+gem_ethtool_set_link_ksettings(struct net_device *netdev,
+			       const struct ethtool_link_ksettings *cmd)
+{
+	struct macb *bp = netdev_priv(netdev);
+
+	return phylink_ethtool_ksettings_set(bp->pl, cmd);
+}
+
 static const struct ethtool_ops macb_ethtool_ops = {
 	.get_regs_len		= macb_get_regs_len,
 	.get_regs		= macb_get_regs,
@@ -3165,8 +3184,8 @@ static const struct ethtool_ops macb_ethtool_ops = {
 	.get_ts_info		= ethtool_op_get_ts_info,
 	.get_wol		= macb_get_wol,
 	.set_wol		= macb_set_wol,
-	.get_link_ksettings     = phy_ethtool_get_link_ksettings,
-	.set_link_ksettings     = phy_ethtool_set_link_ksettings,
+	.get_link_ksettings     = gem_ethtool_get_link_ksettings,
+	.set_link_ksettings     = gem_ethtool_set_link_ksettings,
 	.get_ringparam		= macb_get_ringparam,
 	.set_ringparam		= macb_set_ringparam,
 };
@@ -3179,8 +3198,8 @@ static const struct ethtool_ops gem_ethtool_ops = {
 	.get_ethtool_stats	= gem_get_ethtool_stats,
 	.get_strings		= gem_get_ethtool_strings,
 	.get_sset_count		= gem_get_sset_count,
-	.get_link_ksettings     = phy_ethtool_get_link_ksettings,
-	.set_link_ksettings     = phy_ethtool_set_link_ksettings,
+	.get_link_ksettings     = gem_ethtool_get_link_ksettings,
+	.set_link_ksettings     = gem_ethtool_set_link_ksettings,
 	.get_ringparam		= macb_get_ringparam,
 	.set_ringparam		= macb_set_ringparam,
 	.get_rxnfc			= gem_get_rxnfc,
@@ -3189,17 +3208,13 @@ static const struct ethtool_ops gem_ethtool_ops = {
 
 static int macb_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 {
-	struct phy_device *phydev = dev->phydev;
 	struct macb *bp = netdev_priv(dev);
 
 	if (!netif_running(dev))
 		return -EINVAL;
 
-	if (!phydev)
-		return -ENODEV;
-
 	if (!bp->ptp_info)
-		return phy_mii_ioctl(phydev, rq, cmd);
+		return phylink_mii_ioctl(bp->pl, rq, cmd);
 
 	switch (cmd) {
 	case SIOCSHWTSTAMP:
@@ -3207,7 +3222,7 @@ static int macb_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	case SIOCGHWTSTAMP:
 		return bp->ptp_info->get_hwtst(dev, rq);
 	default:
-		return phy_mii_ioctl(phydev, rq, cmd);
+		return phylink_mii_ioctl(bp->pl, rq, cmd);
 	}
 }
 
@@ -3707,7 +3722,7 @@ static int at91ether_open(struct net_device *dev)
 			     MACB_BIT(HRESP));
 
 	/* schedule a link state check */
-	phy_start(dev->phydev);
+	phylink_start(lp->pl);
 
 	netif_start_queue(dev);
 
@@ -4180,13 +4195,12 @@ static int macb_probe(struct platform_device *pdev)
 	struct clk *tsu_clk = NULL;
 	unsigned int queue_mask, num_queues;
 	bool native_io;
-	struct phy_device *phydev;
 	struct net_device *dev;
 	struct resource *regs;
 	void __iomem *mem;
 	const char *mac;
 	struct macb *bp;
-	int err, val;
+	int err, val, phy_mode;
 
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	mem = devm_ioremap_resource(&pdev->dev, regs);
@@ -4307,12 +4321,12 @@ static int macb_probe(struct platform_device *pdev)
 		macb_get_hwaddr(bp);
 	}
 
-	err = of_get_phy_mode(np);
-	if (err < 0)
+	phy_mode = of_get_phy_mode(np);
+	if (phy_mode < 0)
 		/* not found in DT, MII by default */
 		bp->phy_interface = PHY_INTERFACE_MODE_MII;
 	else
-		bp->phy_interface = err;
+		bp->phy_interface = phy_mode;
 
 	/* IP specific init */
 	err = init(pdev);
@@ -4323,8 +4337,6 @@ static int macb_probe(struct platform_device *pdev)
 	if (err)
 		goto err_out_free_netdev;
 
-	phydev = dev->phydev;
-
 	netif_carrier_off(dev);
 
 	err = register_netdev(dev);
@@ -4336,8 +4348,6 @@ static int macb_probe(struct platform_device *pdev)
 	tasklet_init(&bp->hresp_err_tasklet, macb_hresp_error_task,
 		     (unsigned long)bp);
 
-	phy_attached_info(phydev);
-
 	netdev_info(dev, "Cadence %s rev 0x%08x at 0x%08lx irq %d (%pM)\n",
 		    macb_is_gem(bp) ? "GEM" : "MACB", macb_readl(bp, MID),
 		    dev->base_addr, dev->irq, dev->dev_addr);
@@ -4348,7 +4358,9 @@ static int macb_probe(struct platform_device *pdev)
 	return 0;
 
 err_out_unregister_mdio:
-	phy_disconnect(dev->phydev);
+	rtnl_lock();
+	phylink_disconnect_phy(bp->pl);
+	rtnl_unlock();
 	mdiobus_unregister(bp->mii_bus);
 	of_node_put(bp->phy_node);
 	if (np && of_phy_is_fixed_link(np))
@@ -4382,13 +4394,18 @@ static int macb_remove(struct platform_device *pdev)
 
 	if (dev) {
 		bp = netdev_priv(dev);
-		if (dev->phydev)
-			phy_disconnect(dev->phydev);
+		if (bp->pl) {
+			rtnl_lock();
+			phylink_disconnect_phy(bp->pl);
+			rtnl_unlock();
+		}
 		mdiobus_unregister(bp->mii_bus);
 		if (np && of_phy_is_fixed_link(np))
 			of_phy_deregister_fixed_link(np);
 		dev->phydev = NULL;
 		mdiobus_free(bp->mii_bus);
+		if (bp->pl)
+			phylink_destroy(bp->pl);
 
 		unregister_netdev(dev);
 		pm_runtime_disable(&pdev->dev);
@@ -4431,8 +4448,9 @@ static int __maybe_unused macb_suspend(struct device *dev)
 		for (q = 0, queue = bp->queues; q < bp->num_queues;
 		     ++q, ++queue)
 			napi_disable(&queue->napi);
-		phy_stop(netdev->phydev);
-		phy_suspend(netdev->phydev);
+		phylink_stop(bp->pl);
+		if (netdev->phydev)
+			phy_suspend(netdev->phydev);
 		spin_lock_irqsave(&bp->lock, flags);
 		macb_reset_hw(bp);
 		spin_unlock_irqrestore(&bp->lock, flags);
@@ -4480,9 +4498,11 @@ static int __maybe_unused macb_resume(struct device *dev)
 		for (q = 0, queue = bp->queues; q < bp->num_queues;
 		     ++q, ++queue)
 			napi_enable(&queue->napi);
-		phy_resume(netdev->phydev);
-		phy_init_hw(netdev->phydev);
-		phy_start(netdev->phydev);
+		if (netdev->phydev) {
+			phy_resume(netdev->phydev);
+			phy_init_hw(netdev->phydev);
+		}
+		phylink_start(bp->pl);
 	}
 
 	bp->macbgem_ops.mog_init_rings(bp);
-- 
2.17.1

Re: [PATCH v4 1/5] net: macb: add phylink support

[Russell King - ARM Linux admin]

On Sun, Jun 23, 2019 at 10:17:37AM +0100, Parshuram Thombare wrote:
> +	switch (state->interface) {
> +	case PHY_INTERFACE_MODE_GMII:
> +	case PHY_INTERFACE_MODE_RGMII:
> +		if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE) {
> +			phylink_set(mask, 1000baseT_Full);
> +			phylink_set(mask, 1000baseX_Full);
> +			if (!(bp->caps & MACB_CAPS_NO_GIGABIT_HALF)) {
> +				phylink_set(mask, 1000baseT_Half);
> +				phylink_set(mask, 1000baseT_Half);

I think this can be cleaned up.

> +			}
> +		}
> +	/* fallthrough */
> +	case PHY_INTERFACE_MODE_MII:
> +	case PHY_INTERFACE_MODE_RMII:
> +		phylink_set(mask, 10baseT_Half);
> +		phylink_set(mask, 10baseT_Full);
> +		phylink_set(mask, 100baseT_Half);
> +		phylink_set(mask, 100baseT_Full);
> +		break;
> +	default:
> +		break;
> +	}
>  
> -	spin_lock_irqsave(&bp->lock, flags);
> +	linkmode_and(supported, supported, mask);
> +	linkmode_and(state->advertising, state->advertising, mask);
>  

You remove this blank line in the next patch, so given that this is a
new function, you might as well clean that up in this patch.

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

RE: [PATCH v4 1/5] net: macb: add phylink support

[Parshuram Raju Thombare]

>From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
>On Sun, Jun 23, 2019 at 10:17:37AM +0100, Parshuram Thombare wrote:
>> +	switch (state->interface) {
>> +	case PHY_INTERFACE_MODE_GMII:
>> +	case PHY_INTERFACE_MODE_RGMII:
>> +		if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE) {
>> +			phylink_set(mask, 1000baseT_Full);
>> +			phylink_set(mask, 1000baseX_Full);
>> +			if (!(bp->caps & MACB_CAPS_NO_GIGABIT_HALF)) {
>> +				phylink_set(mask, 1000baseT_Half);
>> +				phylink_set(mask, 1000baseT_Half);
>I think this can be cleaned up.
Ok, I will remove duplicate 1000baseT_Half

>> -	spin_lock_irqsave(&bp->lock, flags);
>> +	linkmode_and(supported, supported, mask);
>> +	linkmode_and(state->advertising, state->advertising, mask);
>You remove this blank line in the next patch, so given that this is a
>new function, you might as well clean that up in this patch.
Ok
Regards,
Parshuram Thombare

[PATCH v4 2/5] net: macb: add support for sgmii MAC-PHY interface

[Parshuram Thombare]

This patch add support for SGMII interface) and
2.5Gbps MAC in Cadence ethernet controller driver.

Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
 drivers/net/ethernet/cadence/macb.h      | 54 ++++++++++++----
 drivers/net/ethernet/cadence/macb_main.c | 80 +++++++++++++++++++++---
 2 files changed, 112 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 8629d345af31..6d268283c318 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -77,6 +77,7 @@
 #define MACB_RBQPH		0x04D4
 
 /* GEM register offsets. */
+#define GEM_NCR			0x0000 /* Network Control */
 #define GEM_NCFGR		0x0004 /* Network Config */
 #define GEM_USRIO		0x000c /* User IO */
 #define GEM_DMACFG		0x0010 /* DMA Configuration */
@@ -156,6 +157,7 @@
 #define GEM_PEFTN		0x01f4 /* PTP Peer Event Frame Tx Ns */
 #define GEM_PEFRSL		0x01f8 /* PTP Peer Event Frame Rx Sec Low */
 #define GEM_PEFRN		0x01fc /* PTP Peer Event Frame Rx Ns */
+#define GEM_PCS_CTRL		0x0200 /* PCS Control */
 #define GEM_DCFG1		0x0280 /* Design Config 1 */
 #define GEM_DCFG2		0x0284 /* Design Config 2 */
 #define GEM_DCFG3		0x0288 /* Design Config 3 */
@@ -271,6 +273,10 @@
 #define MACB_IRXFCS_OFFSET	19
 #define MACB_IRXFCS_SIZE	1
 
+/* GEM specific NCR bitfields. */
+#define GEM_TWO_PT_FIVE_GIG_OFFSET	29
+#define GEM_TWO_PT_FIVE_GIG_SIZE	1
+
 /* GEM specific NCFGR bitfields. */
 #define GEM_GBE_OFFSET		10 /* Gigabit mode enable */
 #define GEM_GBE_SIZE		1
@@ -323,6 +329,9 @@
 #define MACB_MDIO_SIZE		1
 #define MACB_IDLE_OFFSET	2 /* The PHY management logic is idle */
 #define MACB_IDLE_SIZE		1
+#define MACB_DUPLEX_OFFSET	3
+#define MACB_DUPLEX_SIZE	1
+
 
 /* Bitfields in TSR */
 #define MACB_UBR_OFFSET		0 /* Used bit read */
@@ -456,11 +465,17 @@
 #define MACB_REV_OFFSET				0
 #define MACB_REV_SIZE				16
 
+/* Bitfields in PCS_CONTROL. */
+#define GEM_PCS_CTRL_RST_OFFSET			15
+#define GEM_PCS_CTRL_RST_SIZE			1
+
 /* Bitfields in DCFG1. */
 #define GEM_IRQCOR_OFFSET			23
 #define GEM_IRQCOR_SIZE				1
 #define GEM_DBWDEF_OFFSET			25
 #define GEM_DBWDEF_SIZE				3
+#define GEM_NO_PCS_OFFSET			0
+#define GEM_NO_PCS_SIZE				1
 
 /* Bitfields in DCFG2. */
 #define GEM_RX_PKT_BUFF_OFFSET			20
@@ -633,19 +648,32 @@
 #define MACB_MAN_CODE				2
 
 /* Capability mask bits */
-#define MACB_CAPS_ISR_CLEAR_ON_WRITE		0x00000001
-#define MACB_CAPS_USRIO_HAS_CLKEN		0x00000002
-#define MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII	0x00000004
-#define MACB_CAPS_NO_GIGABIT_HALF		0x00000008
-#define MACB_CAPS_USRIO_DISABLED		0x00000010
-#define MACB_CAPS_JUMBO				0x00000020
-#define MACB_CAPS_GEM_HAS_PTP			0x00000040
-#define MACB_CAPS_BD_RD_PREFETCH		0x00000080
-#define MACB_CAPS_NEEDS_RSTONUBR		0x00000100
-#define MACB_CAPS_FIFO_MODE			0x10000000
-#define MACB_CAPS_GIGABIT_MODE_AVAILABLE	0x20000000
-#define MACB_CAPS_SG_DISABLED			0x40000000
-#define MACB_CAPS_MACB_IS_GEM			0x80000000
+#define MACB_CAPS_ISR_CLEAR_ON_WRITE		BIT(0)
+#define MACB_CAPS_USRIO_HAS_CLKEN		BIT(1)
+#define MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII	BIT(2)
+#define MACB_CAPS_NO_GIGABIT_HALF		BIT(3)
+#define MACB_CAPS_USRIO_DISABLED		BIT(4)
+#define MACB_CAPS_JUMBO				BIT(5)
+#define MACB_CAPS_GEM_HAS_PTP			BIT(6)
+#define MACB_CAPS_BD_RD_PREFETCH		BIT(7)
+#define MACB_CAPS_NEEDS_RSTONUBR		BIT(8)
+#define MACB_CAPS_FIFO_MODE			BIT(28)
+#define MACB_CAPS_GIGABIT_MODE_AVAILABLE	BIT(29)
+#define MACB_CAPS_SG_DISABLED			BIT(30)
+#define MACB_CAPS_MACB_IS_GEM			BIT(31)
+#define MACB_CAPS_PCS				BIT(24)
+#define MACB_CAPS_MACB_IS_GEM_GXL		BIT(25)
+
+#define MACB_GEM7010_IDNUM			0x009
+#define MACB_GEM7014_IDNU			0x107
+#define MACB_GEM7014A_IDNUM			0x207
+#define MACB_GEM7016_IDNUM			0x10a
+#define MACB_GEM7017_IDNUM			0x00a
+#define MACB_GEM7017A_IDNUM			0x20a
+#define MACB_GEM7020_IDNUM			0x003
+#define MACB_GEM7021_IDNUM			0x00c
+#define MACB_GEM7021A_IDNUM			0x20c
+#define MACB_GEM7022_IDNUM			0x00b
 
 /* LSO settings */
 #define MACB_LSO_UFO_ENABLE			0x01
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 1d6527a5313f..10d18b2cef31 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -445,15 +445,15 @@ static void gem_phylink_validate(struct phylink_config *pl_config,
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
 	switch (state->interface) {
+	case PHY_INTERFACE_MODE_NA:
+	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_GMII:
 	case PHY_INTERFACE_MODE_RGMII:
 		if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE) {
 			phylink_set(mask, 1000baseT_Full);
 			phylink_set(mask, 1000baseX_Full);
-			if (!(bp->caps & MACB_CAPS_NO_GIGABIT_HALF)) {
+			if (!(bp->caps & MACB_CAPS_NO_GIGABIT_HALF))
 				phylink_set(mask, 1000baseT_Half);
-				phylink_set(mask, 1000baseT_Half);
-			}
 		}
 	/* fallthrough */
 	case PHY_INTERFACE_MODE_MII:
@@ -469,7 +469,6 @@ static void gem_phylink_validate(struct phylink_config *pl_config,
 
 	linkmode_and(supported, supported, mask);
 	linkmode_and(state->advertising, state->advertising, mask);
-
 }
 
 static int gem_phylink_mac_link_state(struct phylink_config *pl_config,
@@ -483,19 +482,42 @@ static void gem_mac_config(struct phylink_config *pl_config, unsigned int mode,
 {
 	struct net_device *netdev = to_net_dev(pl_config->dev);
 	struct macb *bp = netdev_priv(netdev);
+	bool change_interface = bp->phy_interface != state->interface;
 	unsigned long flags;
 
 	spin_lock_irqsave(&bp->lock, flags);
 
+	if (change_interface) {
+		if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII) {
+			gem_writel(bp, NCFGR, ~GEM_BIT(SGMIIEN) &
+				   ~GEM_BIT(PCSSEL) &
+				   gem_readl(bp, NCFGR));
+			gem_writel(bp, NCR, ~GEM_BIT(TWO_PT_FIVE_GIG) &
+				   gem_readl(bp, NCR));
+			gem_writel(bp, PCS_CTRL, gem_readl(bp, PCS_CTRL) |
+				   GEM_BIT(PCS_CTRL_RST));
+		}
+		bp->phy_interface = state->interface;
+	}
+
 	if (!phylink_autoneg_inband(mode) &&
 	    (bp->speed != state->speed ||
-	     bp->duplex != state->duplex)) {
+	     bp->duplex != state->duplex ||
+	     change_interface)) {
 		u32 reg;
 
 		reg = macb_readl(bp, NCFGR);
 		reg &= ~(MACB_BIT(SPD) | MACB_BIT(FD));
 		if (macb_is_gem(bp))
 			reg &= ~GEM_BIT(GBE);
+		macb_or_gem_writel(bp, NCFGR, reg);
+
+		if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII)
+			gem_writel(bp, NCFGR, GEM_BIT(SGMIIEN) |
+				   GEM_BIT(PCSSEL) |
+				   gem_readl(bp, NCFGR));
+
+		reg = macb_readl(bp, NCFGR);
 		if (state->duplex)
 			reg |= MACB_BIT(FD);
 
@@ -590,8 +612,8 @@ static int macb_mii_probe(struct net_device *dev)
 	}
 
 	bp->link = 0;
-	bp->speed = 0;
-	bp->duplex = -1;
+	bp->speed = SPEED_UNKNOWN;
+	bp->duplex = DUPLEX_UNKNOWN;
 
 	return ret;
 }
@@ -3340,6 +3362,22 @@ static void macb_configure_caps(struct macb *bp,
 		dcfg = gem_readl(bp, DCFG1);
 		if (GEM_BFEXT(IRQCOR, dcfg) == 0)
 			bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
+		if (GEM_BFEXT(NO_PCS, dcfg) == 0)
+			bp->caps |= MACB_CAPS_PCS;
+		switch (MACB_BFEXT(IDNUM, macb_readl(bp, MID))) {
+		case MACB_GEM7016_IDNUM:
+		case MACB_GEM7017_IDNUM:
+		case MACB_GEM7017A_IDNUM:
+		case MACB_GEM7020_IDNUM:
+		case MACB_GEM7021_IDNUM:
+		case MACB_GEM7021A_IDNUM:
+		case MACB_GEM7022_IDNUM:
+			bp->caps |= MACB_CAPS_USRIO_DISABLED;
+			bp->caps |= MACB_CAPS_MACB_IS_GEM_GXL;
+			break;
+		default:
+			break;
+		}
 		dcfg = gem_readl(bp, DCFG2);
 		if ((dcfg & (GEM_BIT(RX_PKT_BUFF) | GEM_BIT(TX_PKT_BUFF))) == 0)
 			bp->caps |= MACB_CAPS_FIFO_MODE;
@@ -4322,11 +4360,35 @@ static int macb_probe(struct platform_device *pdev)
 	}
 
 	phy_mode = of_get_phy_mode(np);
-	if (phy_mode < 0)
+	if (phy_mode < 0) {
 		/* not found in DT, MII by default */
 		bp->phy_interface = PHY_INTERFACE_MODE_MII;
-	else
+	} else if (bp->caps & MACB_CAPS_MACB_IS_GEM_GXL) {
+		u32 interface_supported = 1;
+
+		if (phy_mode == PHY_INTERFACE_MODE_SGMII) {
+			if (!(bp->caps & MACB_CAPS_PCS))
+				interface_supported = 0;
+		} else if (phy_mode == PHY_INTERFACE_MODE_GMII ||
+			   phy_mode == PHY_INTERFACE_MODE_RGMII) {
+			if (!macb_is_gem(bp))
+				interface_supported = 0;
+		} else if (phy_mode != PHY_INTERFACE_MODE_RMII &&
+			   phy_mode != PHY_INTERFACE_MODE_MII) {
+			/* Add new mode before this */
+			interface_supported = 0;
+		}
+
+		if (!interface_supported) {
+			netdev_err(dev, "Phy mode %s not supported",
+				   phy_modes(phy_mode));
+			goto err_out_free_netdev;
+		}
+
 		bp->phy_interface = phy_mode;
+	} else {
+		bp->phy_interface = phy_mode;
+	}
 
 	/* IP specific init */
 	err = init(pdev);
-- 
2.17.1

Re: [PATCH v4 2/5] net: macb: add support for sgmii MAC-PHY interface

[Russell King - ARM Linux admin]

On Sun, Jun 23, 2019 at 10:23:01AM +0100, Parshuram Thombare wrote:
> This patch add support for SGMII interface) and
> 2.5Gbps MAC in Cadence ethernet controller driver.
> 
> Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
> ---
>  drivers/net/ethernet/cadence/macb.h      | 54 ++++++++++++----
>  drivers/net/ethernet/cadence/macb_main.c | 80 +++++++++++++++++++++---
>  2 files changed, 112 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 8629d345af31..6d268283c318 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -77,6 +77,7 @@
>  #define MACB_RBQPH		0x04D4
>  
>  /* GEM register offsets. */
> +#define GEM_NCR			0x0000 /* Network Control */
>  #define GEM_NCFGR		0x0004 /* Network Config */
>  #define GEM_USRIO		0x000c /* User IO */
>  #define GEM_DMACFG		0x0010 /* DMA Configuration */
> @@ -156,6 +157,7 @@
>  #define GEM_PEFTN		0x01f4 /* PTP Peer Event Frame Tx Ns */
>  #define GEM_PEFRSL		0x01f8 /* PTP Peer Event Frame Rx Sec Low */
>  #define GEM_PEFRN		0x01fc /* PTP Peer Event Frame Rx Ns */
> +#define GEM_PCS_CTRL		0x0200 /* PCS Control */
>  #define GEM_DCFG1		0x0280 /* Design Config 1 */
>  #define GEM_DCFG2		0x0284 /* Design Config 2 */
>  #define GEM_DCFG3		0x0288 /* Design Config 3 */
> @@ -271,6 +273,10 @@
>  #define MACB_IRXFCS_OFFSET	19
>  #define MACB_IRXFCS_SIZE	1
>  
> +/* GEM specific NCR bitfields. */
> +#define GEM_TWO_PT_FIVE_GIG_OFFSET	29
> +#define GEM_TWO_PT_FIVE_GIG_SIZE	1
> +
>  /* GEM specific NCFGR bitfields. */
>  #define GEM_GBE_OFFSET		10 /* Gigabit mode enable */
>  #define GEM_GBE_SIZE		1
> @@ -323,6 +329,9 @@
>  #define MACB_MDIO_SIZE		1
>  #define MACB_IDLE_OFFSET	2 /* The PHY management logic is idle */
>  #define MACB_IDLE_SIZE		1
> +#define MACB_DUPLEX_OFFSET	3
> +#define MACB_DUPLEX_SIZE	1
> +
>  
>  /* Bitfields in TSR */
>  #define MACB_UBR_OFFSET		0 /* Used bit read */
> @@ -456,11 +465,17 @@
>  #define MACB_REV_OFFSET				0
>  #define MACB_REV_SIZE				16
>  
> +/* Bitfields in PCS_CONTROL. */
> +#define GEM_PCS_CTRL_RST_OFFSET			15
> +#define GEM_PCS_CTRL_RST_SIZE			1
> +
>  /* Bitfields in DCFG1. */
>  #define GEM_IRQCOR_OFFSET			23
>  #define GEM_IRQCOR_SIZE				1
>  #define GEM_DBWDEF_OFFSET			25
>  #define GEM_DBWDEF_SIZE				3
> +#define GEM_NO_PCS_OFFSET			0
> +#define GEM_NO_PCS_SIZE				1
>  
>  /* Bitfields in DCFG2. */
>  #define GEM_RX_PKT_BUFF_OFFSET			20
> @@ -633,19 +648,32 @@
>  #define MACB_MAN_CODE				2
>  
>  /* Capability mask bits */
> -#define MACB_CAPS_ISR_CLEAR_ON_WRITE		0x00000001
> -#define MACB_CAPS_USRIO_HAS_CLKEN		0x00000002
> -#define MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII	0x00000004
> -#define MACB_CAPS_NO_GIGABIT_HALF		0x00000008
> -#define MACB_CAPS_USRIO_DISABLED		0x00000010
> -#define MACB_CAPS_JUMBO				0x00000020
> -#define MACB_CAPS_GEM_HAS_PTP			0x00000040
> -#define MACB_CAPS_BD_RD_PREFETCH		0x00000080
> -#define MACB_CAPS_NEEDS_RSTONUBR		0x00000100
> -#define MACB_CAPS_FIFO_MODE			0x10000000
> -#define MACB_CAPS_GIGABIT_MODE_AVAILABLE	0x20000000
> -#define MACB_CAPS_SG_DISABLED			0x40000000
> -#define MACB_CAPS_MACB_IS_GEM			0x80000000
> +#define MACB_CAPS_ISR_CLEAR_ON_WRITE		BIT(0)
> +#define MACB_CAPS_USRIO_HAS_CLKEN		BIT(1)
> +#define MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII	BIT(2)
> +#define MACB_CAPS_NO_GIGABIT_HALF		BIT(3)
> +#define MACB_CAPS_USRIO_DISABLED		BIT(4)
> +#define MACB_CAPS_JUMBO				BIT(5)
> +#define MACB_CAPS_GEM_HAS_PTP			BIT(6)
> +#define MACB_CAPS_BD_RD_PREFETCH		BIT(7)
> +#define MACB_CAPS_NEEDS_RSTONUBR		BIT(8)
> +#define MACB_CAPS_FIFO_MODE			BIT(28)
> +#define MACB_CAPS_GIGABIT_MODE_AVAILABLE	BIT(29)
> +#define MACB_CAPS_SG_DISABLED			BIT(30)
> +#define MACB_CAPS_MACB_IS_GEM			BIT(31)
> +#define MACB_CAPS_PCS				BIT(24)
> +#define MACB_CAPS_MACB_IS_GEM_GXL		BIT(25)
> +
> +#define MACB_GEM7010_IDNUM			0x009
> +#define MACB_GEM7014_IDNU			0x107
> +#define MACB_GEM7014A_IDNUM			0x207
> +#define MACB_GEM7016_IDNUM			0x10a
> +#define MACB_GEM7017_IDNUM			0x00a
> +#define MACB_GEM7017A_IDNUM			0x20a
> +#define MACB_GEM7020_IDNUM			0x003
> +#define MACB_GEM7021_IDNUM			0x00c
> +#define MACB_GEM7021A_IDNUM			0x20c
> +#define MACB_GEM7022_IDNUM			0x00b
>  
>  /* LSO settings */
>  #define MACB_LSO_UFO_ENABLE			0x01
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 1d6527a5313f..10d18b2cef31 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -445,15 +445,15 @@ static void gem_phylink_validate(struct phylink_config *pl_config,
>  	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
>  
>  	switch (state->interface) {
> +	case PHY_INTERFACE_MODE_NA:
> +	case PHY_INTERFACE_MODE_SGMII:
>  	case PHY_INTERFACE_MODE_GMII:
>  	case PHY_INTERFACE_MODE_RGMII:
>  		if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE) {
>  			phylink_set(mask, 1000baseT_Full);
>  			phylink_set(mask, 1000baseX_Full);
> -			if (!(bp->caps & MACB_CAPS_NO_GIGABIT_HALF)) {
> +			if (!(bp->caps & MACB_CAPS_NO_GIGABIT_HALF))
>  				phylink_set(mask, 1000baseT_Half);
> -				phylink_set(mask, 1000baseT_Half);
> -			}
>  		}
>  	/* fallthrough */
>  	case PHY_INTERFACE_MODE_MII:
> @@ -469,7 +469,6 @@ static void gem_phylink_validate(struct phylink_config *pl_config,
>  
>  	linkmode_and(supported, supported, mask);
>  	linkmode_and(state->advertising, state->advertising, mask);
> -
>  }
>  
>  static int gem_phylink_mac_link_state(struct phylink_config *pl_config,
> @@ -483,19 +482,42 @@ static void gem_mac_config(struct phylink_config *pl_config, unsigned int mode,
>  {
>  	struct net_device *netdev = to_net_dev(pl_config->dev);
>  	struct macb *bp = netdev_priv(netdev);
> +	bool change_interface = bp->phy_interface != state->interface;
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&bp->lock, flags);
>  
> +	if (change_interface) {
> +		if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII) {
> +			gem_writel(bp, NCFGR, ~GEM_BIT(SGMIIEN) &
> +				   ~GEM_BIT(PCSSEL) &
> +				   gem_readl(bp, NCFGR));
> +			gem_writel(bp, NCR, ~GEM_BIT(TWO_PT_FIVE_GIG) &
> +				   gem_readl(bp, NCR));
> +			gem_writel(bp, PCS_CTRL, gem_readl(bp, PCS_CTRL) |
> +				   GEM_BIT(PCS_CTRL_RST));
> +		}

I still don't think this makes much sense, splitting the interface
configuration between here and below.

> +		bp->phy_interface = state->interface;
> +	}
> +
>  	if (!phylink_autoneg_inband(mode) &&
>  	    (bp->speed != state->speed ||
> -	     bp->duplex != state->duplex)) {
> +	     bp->duplex != state->duplex ||
> +	     change_interface)) {
>  		u32 reg;
>  
>  		reg = macb_readl(bp, NCFGR);
>  		reg &= ~(MACB_BIT(SPD) | MACB_BIT(FD));
>  		if (macb_is_gem(bp))
>  			reg &= ~GEM_BIT(GBE);
> +		macb_or_gem_writel(bp, NCFGR, reg);
> +
> +		if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII)
> +			gem_writel(bp, NCFGR, GEM_BIT(SGMIIEN) |
> +				   GEM_BIT(PCSSEL) |
> +				   gem_readl(bp, NCFGR));

This will only be executed when we are not using inband mode, which
basically means it's not possible to switch to SGMII in-band mode.

> +
> +		reg = macb_readl(bp, NCFGR);
>  		if (state->duplex)
>  			reg |= MACB_BIT(FD);
>  
> @@ -590,8 +612,8 @@ static int macb_mii_probe(struct net_device *dev)
>  	}
>  
>  	bp->link = 0;
> -	bp->speed = 0;
> -	bp->duplex = -1;
> +	bp->speed = SPEED_UNKNOWN;
> +	bp->duplex = DUPLEX_UNKNOWN;
>  
>  	return ret;
>  }
> @@ -3340,6 +3362,22 @@ static void macb_configure_caps(struct macb *bp,
>  		dcfg = gem_readl(bp, DCFG1);
>  		if (GEM_BFEXT(IRQCOR, dcfg) == 0)
>  			bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
> +		if (GEM_BFEXT(NO_PCS, dcfg) == 0)
> +			bp->caps |= MACB_CAPS_PCS;
> +		switch (MACB_BFEXT(IDNUM, macb_readl(bp, MID))) {
> +		case MACB_GEM7016_IDNUM:
> +		case MACB_GEM7017_IDNUM:
> +		case MACB_GEM7017A_IDNUM:
> +		case MACB_GEM7020_IDNUM:
> +		case MACB_GEM7021_IDNUM:
> +		case MACB_GEM7021A_IDNUM:
> +		case MACB_GEM7022_IDNUM:
> +			bp->caps |= MACB_CAPS_USRIO_DISABLED;
> +			bp->caps |= MACB_CAPS_MACB_IS_GEM_GXL;
> +			break;
> +		default:
> +			break;
> +		}
>  		dcfg = gem_readl(bp, DCFG2);
>  		if ((dcfg & (GEM_BIT(RX_PKT_BUFF) | GEM_BIT(TX_PKT_BUFF))) == 0)
>  			bp->caps |= MACB_CAPS_FIFO_MODE;
> @@ -4322,11 +4360,35 @@ static int macb_probe(struct platform_device *pdev)
>  	}
>  
>  	phy_mode = of_get_phy_mode(np);
> -	if (phy_mode < 0)
> +	if (phy_mode < 0) {
>  		/* not found in DT, MII by default */
>  		bp->phy_interface = PHY_INTERFACE_MODE_MII;
> -	else
> +	} else if (bp->caps & MACB_CAPS_MACB_IS_GEM_GXL) {
> +		u32 interface_supported = 1;
> +
> +		if (phy_mode == PHY_INTERFACE_MODE_SGMII) {
> +			if (!(bp->caps & MACB_CAPS_PCS))
> +				interface_supported = 0;
> +		} else if (phy_mode == PHY_INTERFACE_MODE_GMII ||
> +			   phy_mode == PHY_INTERFACE_MODE_RGMII) {
> +			if (!macb_is_gem(bp))
> +				interface_supported = 0;
> +		} else if (phy_mode != PHY_INTERFACE_MODE_RMII &&
> +			   phy_mode != PHY_INTERFACE_MODE_MII) {
> +			/* Add new mode before this */
> +			interface_supported = 0;
> +		}
> +
> +		if (!interface_supported) {
> +			netdev_err(dev, "Phy mode %s not supported",
> +				   phy_modes(phy_mode));
> +			goto err_out_free_netdev;
> +		}
> +
>  		bp->phy_interface = phy_mode;
> +	} else {
> +		bp->phy_interface = phy_mode;
> +	}

If bp->phy_interface is PHY_INTERFACE_MODE_SGMII here, and mac_config()
is called with state->interface = PHY_INTERFACE_MODE_SGMII, then
mac_config() won't configure the MAC for the interface type - is that
intentional?

>  
>  	/* IP specific init */
>  	err = init(pdev);
> -- 
> 2.17.1
> 
> 

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

RE: [PATCH v4 2/5] net: macb: add support for sgmii MAC-PHY interface

[Parshuram Raju Thombare]

>> +	if (change_interface) {
>> +		if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII) {
>> +			gem_writel(bp, NCFGR, ~GEM_BIT(SGMIIEN) &
>> +				   ~GEM_BIT(PCSSEL) &
>> +				   gem_readl(bp, NCFGR));
>> +			gem_writel(bp, NCR, ~GEM_BIT(TWO_PT_FIVE_GIG) &
>> +				   gem_readl(bp, NCR));
>> +			gem_writel(bp, PCS_CTRL, gem_readl(bp, PCS_CTRL) |
>> +				   GEM_BIT(PCS_CTRL_RST));
>> +		}
>I still don't think this makes much sense, splitting the interface
>configuration between here and below.
Do you mean splitting mac_config in two *_configure functions ?
This was done as per Andrew's suggestion to make code mode readable
and easy to manage by splitting MAC configuration for different interfaces.

>> +		bp->phy_interface = state->interface;
>> +	}
>> +
>>  	if (!phylink_autoneg_inband(mode) &&
>>  	    (bp->speed != state->speed ||
>> -	     bp->duplex != state->duplex)) {
>> +	     bp->duplex != state->duplex ||
>> +	     change_interface)) {
>>  		u32 reg;
>>
>>  		reg = macb_readl(bp, NCFGR);
>>  		reg &= ~(MACB_BIT(SPD) | MACB_BIT(FD));
>>  		if (macb_is_gem(bp))
>>  			reg &= ~GEM_BIT(GBE);
>> +		macb_or_gem_writel(bp, NCFGR, reg);
>> +
>> +		if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII)
>> +			gem_writel(bp, NCFGR, GEM_BIT(SGMIIEN) |
>> +				   GEM_BIT(PCSSEL) |
>> +				   gem_readl(bp, NCFGR));
>This will only be executed when we are not using inband mode, which
>basically means it's not possible to switch to SGMII in-band mode.
SGMII is used in default PHY mode. And above code is to program MAC to 
select PCS and SGMII interface.

>> +
>> +		if (!interface_supported) {
>> +			netdev_err(dev, "Phy mode %s not supported",
>> +				   phy_modes(phy_mode));
>> +			goto err_out_free_netdev;
>> +		}
>> +
>>  		bp->phy_interface = phy_mode;
>> +	} else {
>> +		bp->phy_interface = phy_mode;
>> +	}
>If bp->phy_interface is PHY_INTERFACE_MODE_SGMII here, and mac_config()
>is called with state->interface = PHY_INTERFACE_MODE_SGMII, then
>mac_config() won't configure the MAC for the interface type - is that
>intentional?
In mac_config configure MAC for non in-band mode, there is also check for speed, duplex
changes. bp->speed and bp->duplex are initialized to SPEED_UNKNOWN and DUPLEX_UNKNOWN
values so it is expected that for non in band mode state contains valid speed and duplex mode
which are different from *_UNKNOWN values.

Regards,
Parshuram Thombare

Re: [PATCH v4 2/5] net: macb: add support for sgmii MAC-PHY interface

[Russell King - ARM Linux admin]

On Mon, Jun 24, 2019 at 06:35:44AM +0000, Parshuram Raju Thombare wrote:
> 
> >> +	if (change_interface) {
> >> +		if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII) {
> >> +			gem_writel(bp, NCFGR, ~GEM_BIT(SGMIIEN) &
> >> +				   ~GEM_BIT(PCSSEL) &
> >> +				   gem_readl(bp, NCFGR));
> >> +			gem_writel(bp, NCR, ~GEM_BIT(TWO_PT_FIVE_GIG) &
> >> +				   gem_readl(bp, NCR));
> >> +			gem_writel(bp, PCS_CTRL, gem_readl(bp, PCS_CTRL) |
> >> +				   GEM_BIT(PCS_CTRL_RST));
> >> +		}
> >I still don't think this makes much sense, splitting the interface
> >configuration between here and below.
> Do you mean splitting mac_config in two *_configure functions ?
> This was done as per Andrew's suggestion to make code mode readable
> and easy to manage by splitting MAC configuration for different interfaces.

No, I mean here you disable SGMII if we're switching away from SGMII
mode.... (note, this means there is more to come for this sentence)

> 
> >> +		bp->phy_interface = state->interface;
> >> +	}
> >> +
> >>  	if (!phylink_autoneg_inband(mode) &&
> >>  	    (bp->speed != state->speed ||
> >> -	     bp->duplex != state->duplex)) {
> >> +	     bp->duplex != state->duplex ||
> >> +	     change_interface)) {
> >>  		u32 reg;
> >>
> >>  		reg = macb_readl(bp, NCFGR);
> >>  		reg &= ~(MACB_BIT(SPD) | MACB_BIT(FD));
> >>  		if (macb_is_gem(bp))
> >>  			reg &= ~GEM_BIT(GBE);
> >> +		macb_or_gem_writel(bp, NCFGR, reg);
> >> +
> >> +		if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII)
> >> +			gem_writel(bp, NCFGR, GEM_BIT(SGMIIEN) |
> >> +				   GEM_BIT(PCSSEL) |
> >> +				   gem_readl(bp, NCFGR));
> >This will only be executed when we are not using inband mode, which
> >basically means it's not possible to switch to SGMII in-band mode.
> SGMII is used in default PHY mode. And above code is to program MAC to 
> select PCS and SGMII interface.

... and here you enable it for SGMII mode, but only for non-inband
modes.

For inband modes, you do not have any code that enables SGMII mode.
Since the only inband mode you support is SGMII, this is not very
good behaviour.

Why not:

	if (change_interface) {
		u32 ncfgr;

		bp->phy_interface = state->interface;

		// We don't support 2.5G modes
		gem_writel(bp, NCR, ~GEM_BIT(TWO_PT_FIVE_GIG) &
			   gem_readl(bp, NCR));

		ncfgr = gem_readl(bp, NCFGR);
		if (state->interface == PHY_INTERFACE_MODE_SGMII) {
			// Enable SGMII mode and PCS
			gem_writel(bp, NCFGR, ncfgr | GEM_BIT(SGMIIEN) |
				   GEM_BIT(PCSSEL));
		} else {
			// Disable SGMII mode and PCS
			gem_writel(bp, NCFGR, ncfgr & ~(GEM_BIT(SGMIIEN) |
				   GEM_BIT(PCSSEL)));

			// Reset PCS
			gem_writel(bp, PCS_CTRL, gem_readl(bp, PCS_CTRL) |
				   GEM_BIT(PCS_CTRL_RST));
		}
	}

	if (!phylink_autoneg_inband(mode) &&
	    (bp->speed != state->speed || bp->duplex != state->duplex)) {

?

> 
> >> +
> >> +		if (!interface_supported) {
> >> +			netdev_err(dev, "Phy mode %s not supported",
> >> +				   phy_modes(phy_mode));
> >> +			goto err_out_free_netdev;
> >> +		}
> >> +
> >>  		bp->phy_interface = phy_mode;
> >> +	} else {
> >> +		bp->phy_interface = phy_mode;
> >> +	}
> >If bp->phy_interface is PHY_INTERFACE_MODE_SGMII here, and mac_config()
> >is called with state->interface = PHY_INTERFACE_MODE_SGMII, then
> >mac_config() won't configure the MAC for the interface type - is that
> >intentional?
> 
> In mac_config configure MAC for non in-band mode, there is also check for speed, duplex
> changes. bp->speed and bp->duplex are initialized to SPEED_UNKNOWN and DUPLEX_UNKNOWN
> values so it is expected that for non in band mode state contains valid speed and duplex mode
> which are different from *_UNKNOWN values.

Sorry, this reply doesn't answer my question.  I'm not asking about
bp->speed and bp->duplex.  I'm asking:

1) why you are initialising bp->phy_interface here
2) you to consider the impact that has on the mac_config() implementation
   you are proposing

because I think it's buggy.

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

RE: [PATCH v4 2/5] net: macb: add support for sgmii MAC-PHY interface

[Parshuram Raju Thombare]

>> >I still don't think this makes much sense, splitting the interface
>> > configuration between here and below.
>> Do you mean splitting mac_config in two *_configure functions ?
>> This was done as per Andrew's suggestion to make code mode readable
>> and easy to manage by splitting MAC configuration for different interfaces.
>No, I mean here you disable SGMII if we're switching away from SGMII
>mode.... (note, this means there is more to come for this sentence)
Sorry, I misunderstood your original question. I think disabling old interface
and enabling new one can be done in single place. I will do this change.

>> >This will only be executed when we are not using inband mode, which
>> >basically means it's not possible to switch to SGMII in-band mode.
>> SGMII is used in default PHY mode. And above code is to program MAC to
>> select PCS and SGMII interface.
>... and here you enable it for SGMII mode, but only for non-inband
>modes.
>
>Why not:
>	if (change_interface)  {
>		if (state->interface == PHY_INTERFACE_MODE_SGMII) {
>			// Enable SGMII mode and PCS
>			gem_writel(bp, NCFGR, ncfgr | GEM_BIT(SGMIIEN) |
>				   GEM_BIT(PCSSEL));
>		} else {
>			// Disable SGMII mode and PCS
>			gem_writel(bp, NCFGR, ncfgr & ~(GEM_BIT(SGMIIEN)
>				   GEM_BIT(PCSSEL)));
>			// Reset PCS
>			gem_writel(bp, PCS_CTRL, gem_readl(bp, PCS_CTRL)
>				   GEM_BIT(PCS_CTRL_RST));
>		}
>	}
>	if (!phylink_autoneg_inband(mode) &&
>	    (bp->speed != state->speed || bp->duplex != state->duplex)) {
>?
Ok

>> >> +
>> >> +		if (!interface_supported) {
>> >> +			netdev_err(dev, "Phy mode %s not supported",
>> >> +				   phy_modes(phy_mode));
>> >> +			goto err_out_free_netdev;
>> >> +		}
>> >> +
>> >>  		bp->phy_interface = phy_mode;
>> >> +	} else {
>> >> +		bp->phy_interface = phy_mode;
>> >> +	}
>> >If bp->phy_interface is PHY_INTERFACE_MODE_SGMII here, and
>> > mac_config()
>> >is called with state->interface = PHY_INTERFACE_MODE_SGMII, then
>> >mac_config() won't configure the MAC for the interface type - is that
>> >intentional?
>> In mac_config configure MAC for non in-band mode, there is also check for
>> speed, duplex
>> changes. bp->speed and bp->duplex are initialized to SPEED_UNKNOWN
>> and DUPLEX_UNKNOWN
>> values so it is expected that for non in band mode state contains valid speed
>> and duplex mode
>> which are different from *_UNKNOWN values.

>Sorry, this reply doesn't answer my question.  I'm not asking about
>bp->speed and bp->duplex.  I'm asking:
>1) why you are initialising bp->phy_interface here
>2) you to consider the impact that has on the mac_config() implementation
>  you are proposing
> because I think it's buggy.
bp->phy_interface is to store phy mode value from device tree. This is used later 
to know what phy interface user has selected for PHY-MAC. Same is used
to configure MAC correctly and based on your suggestion code is
added to handle PHY dynamically changing phy interface, in which 
case bp->phy_interface is also updated. Though it may not be what user want, 
if phy interface is totally decided by PHY and is anyway going to be different from what user
has selected in DT, initializing it here doesn't make sense.
But in case of PHY not changing phy_interface dynamically bp->phy_interface need to be
initialized with value from DT.

Regards,
Parshuram Thombare

Re: [PATCH v4 2/5] net: macb: add support for sgmii MAC-PHY interface

[Russell King - ARM Linux admin]

On Mon, Jun 24, 2019 at 10:14:41AM +0000, Parshuram Raju Thombare wrote:
> >> >I still don't think this makes much sense, splitting the interface
> >> > configuration between here and below.
> >> Do you mean splitting mac_config in two *_configure functions ?
> >> This was done as per Andrew's suggestion to make code mode readable
> >> and easy to manage by splitting MAC configuration for different interfaces.
> >No, I mean here you disable SGMII if we're switching away from SGMII
> >mode.... (note, this means there is more to come for this sentence)
> Sorry, I misunderstood your original question. I think disabling old interface
> and enabling new one can be done in single place. I will do this change.
> 
> >> >This will only be executed when we are not using inband mode, which
> >> >basically means it's not possible to switch to SGMII in-band mode.
> >> SGMII is used in default PHY mode. And above code is to program MAC to
> >> select PCS and SGMII interface.
> >... and here you enable it for SGMII mode, but only for non-inband
> >modes.
> >
> >Why not:
> >	if (change_interface)  {
> >		if (state->interface == PHY_INTERFACE_MODE_SGMII) {
> >			// Enable SGMII mode and PCS
> >			gem_writel(bp, NCFGR, ncfgr | GEM_BIT(SGMIIEN) |
> >				   GEM_BIT(PCSSEL));
> >		} else {
> >			// Disable SGMII mode and PCS
> >			gem_writel(bp, NCFGR, ncfgr & ~(GEM_BIT(SGMIIEN)
> >				   GEM_BIT(PCSSEL)));
> >			// Reset PCS
> >			gem_writel(bp, PCS_CTRL, gem_readl(bp, PCS_CTRL)
> >				   GEM_BIT(PCS_CTRL_RST));
> >		}
> >	}
> >	if (!phylink_autoneg_inband(mode) &&
> >	    (bp->speed != state->speed || bp->duplex != state->duplex)) {
> >?
> Ok
> 
> >> >> +
> >> >> +		if (!interface_supported) {
> >> >> +			netdev_err(dev, "Phy mode %s not supported",
> >> >> +				   phy_modes(phy_mode));
> >> >> +			goto err_out_free_netdev;
> >> >> +		}
> >> >> +
> >> >>  		bp->phy_interface = phy_mode;
> >> >> +	} else {
> >> >> +		bp->phy_interface = phy_mode;
> >> >> +	}
> >> >If bp->phy_interface is PHY_INTERFACE_MODE_SGMII here, and
> >> > mac_config()
> >> >is called with state->interface = PHY_INTERFACE_MODE_SGMII, then
> >> >mac_config() won't configure the MAC for the interface type - is that
> >> >intentional?
> >> In mac_config configure MAC for non in-band mode, there is also check for
> >> speed, duplex
> >> changes. bp->speed and bp->duplex are initialized to SPEED_UNKNOWN
> >> and DUPLEX_UNKNOWN
> >> values so it is expected that for non in band mode state contains valid speed
> >> and duplex mode
> >> which are different from *_UNKNOWN values.
> 
> >Sorry, this reply doesn't answer my question.  I'm not asking about
> >bp->speed and bp->duplex.  I'm asking:
> >1) why you are initialising bp->phy_interface here
> >2) you to consider the impact that has on the mac_config() implementation
> >  you are proposing
> > because I think it's buggy.
> bp->phy_interface is to store phy mode value from device tree. This is used later 
> to know what phy interface user has selected for PHY-MAC. Same is used
> to configure MAC correctly and based on your suggestion code is
> added to handle PHY dynamically changing phy interface, in which 
> case bp->phy_interface is also updated. Though it may not be what user want, 
> if phy interface is totally decided by PHY and is anyway going to be different from what user
> has selected in DT, initializing it here doesn't make sense.
> But in case of PHY not changing phy_interface dynamically bp->phy_interface need to be
> initialized with value from DT.

When phylink_start() is called, you will receive a mac_config() call to
configure the MAC for the initial operating settings, which will include
the current PHY interface mode.  This will initially be whatever
interface mode was passed in to phylink_create().

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

RE: [PATCH v4 2/5] net: macb: add support for sgmii MAC-PHY interface

[Parshuram Raju Thombare]

>> >Sorry, this reply doesn't answer my question.  I'm not asking about
>> >bp->speed and bp->duplex.  I'm asking:
>> >1) why you are initialising bp->phy_interface here
>> >2) you to consider the impact that has on the mac_config() implementation
>> >  you are proposing
>> > because I think it's buggy.
>> bp->phy_interface is to store phy mode value from device tree. This is used
>later
>> to know what phy interface user has selected for PHY-MAC. Same is used
>> to configure MAC correctly and based on your suggestion code is
>> added to handle PHY dynamically changing phy interface, in which
>> case bp->phy_interface is also updated. Though it may not be what user
>> want,
>> if phy interface is totally decided by PHY and is anyway going to be different
>> from what user has selected in DT, initializing it here doesn't make sense.
>> But in case of PHY not changing phy_interface dynamically bp-
>>phy_interface need to be initialized with value from DT.
>When phylink_start() is called, you will receive a mac_config() call to
>configure the MAC for the initial operating settings, which will include
>the current PHY interface mode.  This will initially be whatever
>interface mode was passed in to phylink_create().
Yes, and same bp->phy_interface is passed to phylink_create().
We need this to know what phy interface is selected by user.

Regards,
Parshuram Thombare

[PATCH v4 3/5] net: macb: add support for c45 PHY

[Parshuram Thombare]

This patch modify MDIO read/write functions to support
communication with C45 PHY.

Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
 drivers/net/ethernet/cadence/macb.h      | 15 ++++--
 drivers/net/ethernet/cadence/macb_main.c | 61 +++++++++++++++++++-----
 2 files changed, 61 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 6d268283c318..330da702b946 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -642,10 +642,17 @@
 #define GEM_CLK_DIV96				5
 
 /* Constants for MAN register */
-#define MACB_MAN_SOF				1
-#define MACB_MAN_WRITE				1
-#define MACB_MAN_READ				2
-#define MACB_MAN_CODE				2
+#define MACB_MAN_C22_SOF                        1
+#define MACB_MAN_C22_WRITE                      1
+#define MACB_MAN_C22_READ                       2
+#define MACB_MAN_C22_CODE                       2
+
+#define MACB_MAN_C45_SOF                        0
+#define MACB_MAN_C45_ADDR                       0
+#define MACB_MAN_C45_WRITE                      1
+#define MACB_MAN_C45_POST_READ_INCR             2
+#define MACB_MAN_C45_READ                       3
+#define MACB_MAN_C45_CODE                       2
 
 /* Capability mask bits */
 #define MACB_CAPS_ISR_CLEAR_ON_WRITE		BIT(0)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 10d18b2cef31..94145e460e6e 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -341,11 +341,30 @@ static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
 	if (status < 0)
 		goto mdio_read_exit;
 
-	macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
-			      | MACB_BF(RW, MACB_MAN_READ)
-			      | MACB_BF(PHYA, mii_id)
-			      | MACB_BF(REGA, regnum)
-			      | MACB_BF(CODE, MACB_MAN_CODE)));
+	if (regnum & MII_ADDR_C45) {
+		macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
+			    | MACB_BF(RW, MACB_MAN_C45_ADDR)
+			    | MACB_BF(PHYA, mii_id)
+			    | MACB_BF(REGA, (regnum >> 16) & 0x1F)
+			    | MACB_BF(DATA, regnum & 0xFFFF)
+			    | MACB_BF(CODE, MACB_MAN_C45_CODE)));
+
+		status = macb_mdio_wait_for_idle(bp);
+		if (status < 0)
+			goto mdio_read_exit;
+
+		macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
+			    | MACB_BF(RW, MACB_MAN_C45_READ)
+			    | MACB_BF(PHYA, mii_id)
+			    | MACB_BF(REGA, (regnum >> 16) & 0x1F)
+			    | MACB_BF(CODE, MACB_MAN_C45_CODE)));
+	} else {
+		macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C22_SOF)
+				| MACB_BF(RW, MACB_MAN_C22_READ)
+				| MACB_BF(PHYA, mii_id)
+				| MACB_BF(REGA, regnum)
+				| MACB_BF(CODE, MACB_MAN_C22_CODE)));
+	}
 
 	status = macb_mdio_wait_for_idle(bp);
 	if (status < 0)
@@ -374,12 +393,32 @@ static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
 	if (status < 0)
 		goto mdio_write_exit;
 
-	macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
-			      | MACB_BF(RW, MACB_MAN_WRITE)
-			      | MACB_BF(PHYA, mii_id)
-			      | MACB_BF(REGA, regnum)
-			      | MACB_BF(CODE, MACB_MAN_CODE)
-			      | MACB_BF(DATA, value)));
+	if (regnum & MII_ADDR_C45) {
+		macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
+			    | MACB_BF(RW, MACB_MAN_C45_ADDR)
+			    | MACB_BF(PHYA, mii_id)
+			    | MACB_BF(REGA, (regnum >> 16) & 0x1F)
+			    | MACB_BF(DATA, regnum & 0xFFFF)
+			    | MACB_BF(CODE, MACB_MAN_C45_CODE)));
+
+		status = macb_mdio_wait_for_idle(bp);
+		if (status < 0)
+			goto mdio_write_exit;
+
+		macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
+			    | MACB_BF(RW, MACB_MAN_C45_WRITE)
+			    | MACB_BF(PHYA, mii_id)
+			    | MACB_BF(REGA, (regnum >> 16) & 0x1F)
+			    | MACB_BF(CODE, MACB_MAN_C45_CODE)
+			    | MACB_BF(DATA, value)));
+	} else {
+		macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C22_SOF)
+				| MACB_BF(RW, MACB_MAN_C22_WRITE)
+				| MACB_BF(PHYA, mii_id)
+				| MACB_BF(REGA, regnum)
+				| MACB_BF(CODE, MACB_MAN_C22_CODE)
+				| MACB_BF(DATA, value)));
+	}
 
 	status = macb_mdio_wait_for_idle(bp);
 	if (status < 0)
-- 
2.17.1

Re: [PATCH v4 3/5] net: macb: add support for c45 PHY

[Russell King - ARM Linux admin]

On Sun, Jun 23, 2019 at 10:23:17AM +0100, Parshuram Thombare wrote:
> This patch modify MDIO read/write functions to support
> communication with C45 PHY.

Which Clause 45 PHY are you using?

> 
> Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
> ---
>  drivers/net/ethernet/cadence/macb.h      | 15 ++++--
>  drivers/net/ethernet/cadence/macb_main.c | 61 +++++++++++++++++++-----
>  2 files changed, 61 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 6d268283c318..330da702b946 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -642,10 +642,17 @@
>  #define GEM_CLK_DIV96				5
>  
>  /* Constants for MAN register */
> -#define MACB_MAN_SOF				1
> -#define MACB_MAN_WRITE				1
> -#define MACB_MAN_READ				2
> -#define MACB_MAN_CODE				2
> +#define MACB_MAN_C22_SOF                        1
> +#define MACB_MAN_C22_WRITE                      1
> +#define MACB_MAN_C22_READ                       2
> +#define MACB_MAN_C22_CODE                       2
> +
> +#define MACB_MAN_C45_SOF                        0
> +#define MACB_MAN_C45_ADDR                       0
> +#define MACB_MAN_C45_WRITE                      1
> +#define MACB_MAN_C45_POST_READ_INCR             2
> +#define MACB_MAN_C45_READ                       3
> +#define MACB_MAN_C45_CODE                       2
>  
>  /* Capability mask bits */
>  #define MACB_CAPS_ISR_CLEAR_ON_WRITE		BIT(0)
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 10d18b2cef31..94145e460e6e 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -341,11 +341,30 @@ static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
>  	if (status < 0)
>  		goto mdio_read_exit;
>  
> -	macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
> -			      | MACB_BF(RW, MACB_MAN_READ)
> -			      | MACB_BF(PHYA, mii_id)
> -			      | MACB_BF(REGA, regnum)
> -			      | MACB_BF(CODE, MACB_MAN_CODE)));
> +	if (regnum & MII_ADDR_C45) {
> +		macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
> +			    | MACB_BF(RW, MACB_MAN_C45_ADDR)
> +			    | MACB_BF(PHYA, mii_id)
> +			    | MACB_BF(REGA, (regnum >> 16) & 0x1F)
> +			    | MACB_BF(DATA, regnum & 0xFFFF)
> +			    | MACB_BF(CODE, MACB_MAN_C45_CODE)));
> +
> +		status = macb_mdio_wait_for_idle(bp);
> +		if (status < 0)
> +			goto mdio_read_exit;
> +
> +		macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
> +			    | MACB_BF(RW, MACB_MAN_C45_READ)
> +			    | MACB_BF(PHYA, mii_id)
> +			    | MACB_BF(REGA, (regnum >> 16) & 0x1F)
> +			    | MACB_BF(CODE, MACB_MAN_C45_CODE)));
> +	} else {
> +		macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C22_SOF)
> +				| MACB_BF(RW, MACB_MAN_C22_READ)
> +				| MACB_BF(PHYA, mii_id)
> +				| MACB_BF(REGA, regnum)
> +				| MACB_BF(CODE, MACB_MAN_C22_CODE)));
> +	}
>  
>  	status = macb_mdio_wait_for_idle(bp);
>  	if (status < 0)
> @@ -374,12 +393,32 @@ static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
>  	if (status < 0)
>  		goto mdio_write_exit;
>  
> -	macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
> -			      | MACB_BF(RW, MACB_MAN_WRITE)
> -			      | MACB_BF(PHYA, mii_id)
> -			      | MACB_BF(REGA, regnum)
> -			      | MACB_BF(CODE, MACB_MAN_CODE)
> -			      | MACB_BF(DATA, value)));
> +	if (regnum & MII_ADDR_C45) {
> +		macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
> +			    | MACB_BF(RW, MACB_MAN_C45_ADDR)
> +			    | MACB_BF(PHYA, mii_id)
> +			    | MACB_BF(REGA, (regnum >> 16) & 0x1F)
> +			    | MACB_BF(DATA, regnum & 0xFFFF)
> +			    | MACB_BF(CODE, MACB_MAN_C45_CODE)));
> +
> +		status = macb_mdio_wait_for_idle(bp);
> +		if (status < 0)
> +			goto mdio_write_exit;
> +
> +		macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
> +			    | MACB_BF(RW, MACB_MAN_C45_WRITE)
> +			    | MACB_BF(PHYA, mii_id)
> +			    | MACB_BF(REGA, (regnum >> 16) & 0x1F)
> +			    | MACB_BF(CODE, MACB_MAN_C45_CODE)
> +			    | MACB_BF(DATA, value)));
> +	} else {
> +		macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C22_SOF)
> +				| MACB_BF(RW, MACB_MAN_C22_WRITE)
> +				| MACB_BF(PHYA, mii_id)
> +				| MACB_BF(REGA, regnum)
> +				| MACB_BF(CODE, MACB_MAN_C22_CODE)
> +				| MACB_BF(DATA, value)));
> +	}
>  
>  	status = macb_mdio_wait_for_idle(bp);
>  	if (status < 0)
> -- 
> 2.17.1
> 
> 

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

RE: [PATCH v4 3/5] net: macb: add support for c45 PHY

[Parshuram Raju Thombare]

>Which Clause 45 PHY are you using?

I am using emulated PHY in our CSP environment.
This is using 10G generic PHY driver, with PHY having compatible = "ethernet-phy-ieee802.3-c45"

Hi Andrew,
Can I add your "Reviewed-by" tag for this patch. You added it to this patch in last series.

Regards,
Parshuram Thombare

Re: [PATCH v4 3/5] net: macb: add support for c45 PHY

[Russell King - ARM Linux admin]

On Mon, Jun 24, 2019 at 06:47:48AM +0000, Parshuram Raju Thombare wrote:
> >Which Clause 45 PHY are you using?
> 
> I am using emulated PHY in our CSP environment.

Concentrated Solar Power?  Chartered Society of Physiotherapy?  Center
for Space Physics?

Sorry, I don't know what a "CSP environment" is in this context, neither
it seems does google.  TLAs in general tend to be bad when it comes to
communication.

However, it seems from that comment that you're not talking about real
hardware.  Is there no real hardware out there supporting 10G mode with
these proposed driver changes yet?

> This is using 10G generic PHY driver, with PHY having compatible = "ethernet-phy-ieee802.3-c45"

The generic 10G PHY driver is really dumb and basic - it only supports
a very basic 10G mode.

> 
> Hi Andrew,
> Can I add your "Reviewed-by" tag for this patch. You added it to this patch in last series.
> 
> Regards,
> Parshuram Thombare
> 

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

RE: [PATCH v4 3/5] net: macb: add support for c45 PHY

[Parshuram Raju Thombare]

>However, it seems from that comment that you're not talking about real
>hardware.  Is there no real hardware out there supporting 10G mode with
>these proposed driver changes yet?
I think there are some 10GBaseT PHY out there, but I don't have any test
setup with those. This patch is tested on emulation test setup. 

Regards,
Parshuram Thombare

[PATCH v4 4/5] net: macb: add support for high speed interface

[Parshuram Thombare]

This patch add support for high speed USXGMII PCS and 10G
speed in Cadence ethernet controller driver.

Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
 drivers/net/ethernet/cadence/macb.h      |  41 +++++
 drivers/net/ethernet/cadence/macb_main.c | 194 +++++++++++++++++++----
 2 files changed, 207 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 330da702b946..809acff19be9 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -82,6 +82,7 @@
 #define GEM_USRIO		0x000c /* User IO */
 #define GEM_DMACFG		0x0010 /* DMA Configuration */
 #define GEM_JML			0x0048 /* Jumbo Max Length */
+#define GEM_HS_MAC_CONFIG	0x0050 /* GEM high speed config */
 #define GEM_HRB			0x0080 /* Hash Bottom */
 #define GEM_HRT			0x0084 /* Hash Top */
 #define GEM_SA1B		0x0088 /* Specific1 Bottom */
@@ -167,6 +168,9 @@
 #define GEM_DCFG7		0x0298 /* Design Config 7 */
 #define GEM_DCFG8		0x029C /* Design Config 8 */
 #define GEM_DCFG10		0x02A4 /* Design Config 10 */
+#define GEM_DCFG12		0x02AC /* Design Config 12 */
+#define GEM_USX_CONTROL		0x0A80 /* USXGMII control register */
+#define GEM_USX_STATUS		0x0A88 /* USXGMII status register */
 
 #define GEM_TXBDCTRL	0x04cc /* TX Buffer Descriptor control register */
 #define GEM_RXBDCTRL	0x04d0 /* RX Buffer Descriptor control register */
@@ -274,6 +278,8 @@
 #define MACB_IRXFCS_SIZE	1
 
 /* GEM specific NCR bitfields. */
+#define GEM_ENABLE_HS_MAC_OFFSET	31
+#define GEM_ENABLE_HS_MAC_SIZE		1
 #define GEM_TWO_PT_FIVE_GIG_OFFSET	29
 #define GEM_TWO_PT_FIVE_GIG_SIZE	1
 
@@ -465,6 +471,10 @@
 #define MACB_REV_OFFSET				0
 #define MACB_REV_SIZE				16
 
+/* Bitfield in HS_MAC_CONFIG */
+#define GEM_HS_MAC_SPEED_OFFSET			0
+#define GEM_HS_MAC_SPEED_SIZE			3
+
 /* Bitfields in PCS_CONTROL. */
 #define GEM_PCS_CTRL_RST_OFFSET			15
 #define GEM_PCS_CTRL_RST_SIZE			1
@@ -510,6 +520,34 @@
 #define GEM_RXBD_RDBUFF_OFFSET			8
 #define GEM_RXBD_RDBUFF_SIZE			4
 
+/* Bitfields in DCFG12. */
+#define GEM_HIGH_SPEED_OFFSET			26
+#define GEM_HIGH_SPEED_SIZE			1
+
+/* Bitfields in USX_CONTROL. */
+#define GEM_USX_CTRL_SPEED_OFFSET		14
+#define GEM_USX_CTRL_SPEED_SIZE			3
+#define GEM_SERDES_RATE_OFFSET			12
+#define GEM_SERDES_RATE_SIZE			2
+#define GEM_RX_SCR_BYPASS_OFFSET		9
+#define GEM_RX_SCR_BYPASS_SIZE			1
+#define GEM_TX_SCR_BYPASS_OFFSET		8
+#define GEM_TX_SCR_BYPASS_SIZE			1
+#define GEM_RX_SYNC_RESET_OFFSET		2
+#define GEM_RX_SYNC_RESET_SIZE			1
+#define GEM_TX_EN_OFFSET			1
+#define GEM_TX_EN_SIZE				1
+#define GEM_SIGNAL_OK_OFFSET			0
+#define GEM_SIGNAL_OK_SIZE			1
+
+/* Bitfields in USX_STATUS. */
+#define GEM_USX_TX_FAULT_OFFSET			28
+#define GEM_USX_TX_FAULT_SIZE			1
+#define GEM_USX_RX_FAULT_OFFSET			27
+#define GEM_USX_RX_FAULT_SIZE			1
+#define GEM_USX_BLOCK_LOCK_OFFSET		0
+#define GEM_USX_BLOCK_LOCK_SIZE			1
+
 /* Bitfields in TISUBN */
 #define GEM_SUBNSINCR_OFFSET			0
 #define GEM_SUBNSINCR_SIZE			16
@@ -670,6 +708,7 @@
 #define MACB_CAPS_MACB_IS_GEM			BIT(31)
 #define MACB_CAPS_PCS				BIT(24)
 #define MACB_CAPS_MACB_IS_GEM_GXL		BIT(25)
+#define MACB_CAPS_HIGH_SPEED			BIT(26)
 
 #define MACB_GEM7010_IDNUM			0x009
 #define MACB_GEM7014_IDNU			0x107
@@ -749,6 +788,7 @@
 	})
 
 #define MACB_READ_NSR(bp)	macb_readl(bp, NSR)
+#define GEM_READ_USX_STATUS(bp)	gem_readl(bp, USX_STATUS)
 
 /* struct macb_dma_desc - Hardware DMA descriptor
  * @addr: DMA address of data buffer
@@ -1262,6 +1302,7 @@ struct macb {
 	struct macb_pm_data pm_data;
 	struct phylink *pl;
 	struct phylink_config pl_config;
+	u32 serdes_rate;
 };
 
 #ifdef CONFIG_MACB_USE_HWSTAMP
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 94145e460e6e..65eda399aef8 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -84,6 +84,20 @@ static struct sifive_fu540_macb_mgmt *mgmt;
 #define MACB_WOL_HAS_MAGIC_PACKET	(0x1 << 0)
 #define MACB_WOL_ENABLED		(0x1 << 1)
 
+enum {
+	HS_MAC_SPEED_100M,
+	HS_MAC_SPEED_1000M,
+	HS_MAC_SPEED_2500M,
+	HS_MAC_SPEED_5000M,
+	HS_MAC_SPEED_10000M,
+	HS_MAC_SPEED_25000M,
+};
+
+enum {
+	MACB_SERDES_RATE_5_PT_15625Gbps = 5,
+	MACB_SERDES_RATE_10_PT_3125Gbps = 10,
+};
+
 /* Graceful stop timeouts in us. We should allow up to
  * 1 frame time (10 Mbits/s, full-duplex, ignoring collisions)
  */
@@ -93,6 +107,8 @@ static struct sifive_fu540_macb_mgmt *mgmt;
 
 #define MACB_MDIO_TIMEOUT	1000000 /* in usecs */
 
+#define MACB_USX_BLOCK_LOCK_TIMEOUT	1000000 /* in usecs */
+
 /* DMA buffer descriptor might be different size
  * depends on hardware configuration:
  *
@@ -439,23 +455,37 @@ static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
  */
 static void macb_set_tx_clk(struct clk *clk, int speed, struct net_device *dev)
 {
+	struct macb *bp = netdev_priv(dev);
 	long ferr, rate, rate_rounded;
 
 	if (!clk)
 		return;
 
-	switch (speed) {
-	case SPEED_10:
-		rate = 2500000;
-		break;
-	case SPEED_100:
-		rate = 25000000;
-		break;
-	case SPEED_1000:
-		rate = 125000000;
-		break;
-	default:
-		return;
+	if (bp->phy_interface == PHY_INTERFACE_MODE_USXGMII) {
+		switch (bp->serdes_rate) {
+		case MACB_SERDES_RATE_5_PT_15625Gbps:
+			rate = 78125000;
+			break;
+		case MACB_SERDES_RATE_10_PT_3125Gbps:
+			rate = 156250000;
+			break;
+		default:
+			return;
+		}
+	} else {
+		switch (speed) {
+		case SPEED_10:
+			rate = 2500000;
+			break;
+		case SPEED_100:
+			rate = 25000000;
+			break;
+		case SPEED_1000:
+			rate = 125000000;
+			break;
+		default:
+			return;
+		}
 	}
 
 	rate_rounded = clk_round_rate(clk, rate);
@@ -485,6 +515,21 @@ static void gem_phylink_validate(struct phylink_config *pl_config,
 
 	switch (state->interface) {
 	case PHY_INTERFACE_MODE_NA:
+	case PHY_INTERFACE_MODE_USXGMII:
+	case PHY_INTERFACE_MODE_10GKR:
+		if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE) {
+			phylink_set(mask, 10000baseCR_Full);
+			phylink_set(mask, 10000baseER_Full);
+			phylink_set(mask, 10000baseKR_Full);
+			phylink_set(mask, 10000baseLR_Full);
+			phylink_set(mask, 10000baseLRM_Full);
+			phylink_set(mask, 10000baseSR_Full);
+			phylink_set(mask, 10000baseT_Full);
+			phylink_set(mask, 5000baseT_Full);
+			phylink_set(mask, 2500baseX_Full);
+			phylink_set(mask, 1000baseX_Full);
+		}
+	/* fallthrough */
 	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_GMII:
 	case PHY_INTERFACE_MODE_RGMII:
@@ -516,6 +561,91 @@ static int gem_phylink_mac_link_state(struct phylink_config *pl_config,
 	return -EOPNOTSUPP;
 }
 
+static int macb_wait_for_usx_block_lock(struct macb *bp)
+{
+	u32 val;
+
+	return readx_poll_timeout(GEM_READ_USX_STATUS, bp, val,
+				  val & GEM_BIT(USX_BLOCK_LOCK),
+				  1, MACB_USX_BLOCK_LOCK_TIMEOUT);
+}
+
+static inline int gem_mac_usx_configure(struct macb *bp, int spd)
+{
+	u32 speed, config;
+
+	gem_writel(bp, NCFGR, GEM_BIT(PCSSEL) |
+		   (~GEM_BIT(SGMIIEN) & gem_readl(bp, NCFGR)));
+	gem_writel(bp, NCR, gem_readl(bp, NCR) |
+		   GEM_BIT(ENABLE_HS_MAC));
+	gem_writel(bp, NCFGR, gem_readl(bp, NCFGR) |
+		   MACB_BIT(FD));
+	config = gem_readl(bp, USX_CONTROL);
+	config = GEM_BFINS(SERDES_RATE, bp->serdes_rate, config);
+	config &= ~GEM_BIT(TX_SCR_BYPASS);
+	config &= ~GEM_BIT(RX_SCR_BYPASS);
+	gem_writel(bp, USX_CONTROL, config |
+		   GEM_BIT(TX_EN));
+	config = gem_readl(bp, USX_CONTROL);
+	gem_writel(bp, USX_CONTROL, config | GEM_BIT(SIGNAL_OK));
+	if (macb_wait_for_usx_block_lock(bp) < 0) {
+		netdev_warn(bp->dev, "USXGMII block lock failed");
+		return -ETIMEDOUT;
+	}
+
+	switch (spd) {
+	case SPEED_10000:
+		if (bp->serdes_rate >= MACB_SERDES_RATE_10_PT_3125Gbps) {
+			speed = HS_MAC_SPEED_10000M;
+		} else {
+			netdev_warn(bp->dev, "10G speed isn't supported by HW");
+			netdev_warn(bp->dev, "Setting speed to 1G");
+			speed = HS_MAC_SPEED_1000M;
+		}
+		break;
+	case SPEED_5000:
+		speed = HS_MAC_SPEED_5000M;
+		break;
+	case SPEED_2500:
+		speed = HS_MAC_SPEED_2500M;
+		break;
+	case SPEED_1000:
+		speed = HS_MAC_SPEED_1000M;
+		break;
+	default:
+	case SPEED_100:
+		speed = HS_MAC_SPEED_100M;
+		break;
+	}
+
+	gem_writel(bp, HS_MAC_CONFIG, GEM_BFINS(HS_MAC_SPEED, speed,
+						gem_readl(bp, HS_MAC_CONFIG)));
+	gem_writel(bp, USX_CONTROL, GEM_BFINS(USX_CTRL_SPEED, speed,
+					      gem_readl(bp, USX_CONTROL)));
+	return 0;
+}
+
+static inline void gem_mac_configure(struct macb *bp, int speed)
+{
+	if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII)
+		gem_writel(bp, NCFGR, GEM_BIT(SGMIIEN) |
+			   GEM_BIT(PCSSEL) |
+			   gem_readl(bp, NCFGR));
+
+	switch (speed) {
+	case SPEED_1000:
+		gem_writel(bp, NCFGR, GEM_BIT(GBE) |
+			   gem_readl(bp, NCFGR));
+		break;
+	case SPEED_100:
+		macb_writel(bp, NCFGR, MACB_BIT(SPD) |
+			    macb_readl(bp, NCFGR));
+		break;
+	default:
+		break;
+	}
+}
+
 static void gem_mac_config(struct phylink_config *pl_config, unsigned int mode,
 			   const struct phylink_link_state *state)
 {
@@ -551,26 +681,20 @@ static void gem_mac_config(struct phylink_config *pl_config, unsigned int mode,
 			reg &= ~GEM_BIT(GBE);
 		macb_or_gem_writel(bp, NCFGR, reg);
 
-		if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII)
-			gem_writel(bp, NCFGR, GEM_BIT(SGMIIEN) |
-				   GEM_BIT(PCSSEL) |
-				   gem_readl(bp, NCFGR));
-
 		reg = macb_readl(bp, NCFGR);
 		if (state->duplex)
 			reg |= MACB_BIT(FD);
+		macb_or_gem_writel(bp, NCFGR, reg);
 
-		switch (state->speed) {
-		case SPEED_1000:
-			reg |= GEM_BIT(GBE);
-			break;
-		case SPEED_100:
-			reg |= MACB_BIT(SPD);
-			break;
-		default:
-			break;
+		if (bp->phy_interface == PHY_INTERFACE_MODE_USXGMII) {
+			if (gem_mac_usx_configure(bp, state->speed) < 0) {
+				spin_unlock_irqrestore(&bp->lock, flags);
+				phylink_mac_change(bp->pl, false);
+				return;
+			}
+		} else {
+			gem_mac_configure(bp, state->speed);
 		}
-		macb_or_gem_writel(bp, NCFGR, reg);
 
 		bp->speed = state->speed;
 		bp->duplex = state->duplex;
@@ -3417,6 +3541,9 @@ static void macb_configure_caps(struct macb *bp,
 		default:
 			break;
 		}
+		dcfg = gem_readl(bp, DCFG12);
+		if (GEM_BFEXT(HIGH_SPEED, dcfg) == 1)
+			bp->caps |= MACB_CAPS_HIGH_SPEED;
 		dcfg = gem_readl(bp, DCFG2);
 		if ((dcfg & (GEM_BIT(RX_PKT_BUFF) | GEM_BIT(TX_PKT_BUFF))) == 0)
 			bp->caps |= MACB_CAPS_FIFO_MODE;
@@ -4405,7 +4532,18 @@ static int macb_probe(struct platform_device *pdev)
 	} else if (bp->caps & MACB_CAPS_MACB_IS_GEM_GXL) {
 		u32 interface_supported = 1;
 
-		if (phy_mode == PHY_INTERFACE_MODE_SGMII) {
+		if (phy_mode == PHY_INTERFACE_MODE_USXGMII) {
+			if (!(bp->caps & MACB_CAPS_HIGH_SPEED &&
+			      bp->caps & MACB_CAPS_PCS))
+				interface_supported = 0;
+
+			if (of_property_read_u32(np, "serdes-rate-gbps",
+						 &bp->serdes_rate)) {
+				netdev_err(dev,
+					   "GEM serdes_rate not specified");
+				interface_supported = 0;
+			}
+		} else if (phy_mode == PHY_INTERFACE_MODE_SGMII) {
 			if (!(bp->caps & MACB_CAPS_PCS))
 				interface_supported = 0;
 		} else if (phy_mode == PHY_INTERFACE_MODE_GMII ||
-- 
2.17.1

Re: [PATCH v4 4/5] net: macb: add support for high speed interface

[Andrew Lunn]

> +enum {
> +	HS_MAC_SPEED_100M,
> +	HS_MAC_SPEED_1000M,
> +	HS_MAC_SPEED_2500M,
> +	HS_MAC_SPEED_5000M,
> +	HS_MAC_SPEED_10000M,
> +	HS_MAC_SPEED_25000M,
> +};
> +
> +enum {
> +	MACB_SERDES_RATE_5_PT_15625Gbps = 5,
> +	MACB_SERDES_RATE_10_PT_3125Gbps = 10,
> +};

What do the units mean here? Why would you clock the SERDES at 15Tbps,
or 3Tbps? 3.125Mbps would give you 2.5Gbps when using 8b/10b encoding.

> +	if (bp->phy_interface == PHY_INTERFACE_MODE_USXGMII) {
> +		switch (bp->serdes_rate) {
> +		case MACB_SERDES_RATE_5_PT_15625Gbps:
> +			rate = 78125000;
> +			break;
> +		case MACB_SERDES_RATE_10_PT_3125Gbps:
> +			rate = 156250000;
> +			break;
> +		default:
> +			return;
> +		}

Xilinx documentation:
https://www.xilinx.com/support/documentation/ip_documentation/usxgmii/v1_1/pg251-usxgmii.pdf
seems to suggest USXGMII uses a fixed rate of 10.3125Gb/s. So why do
you need to change the rate?

    Andrew

RE: [PATCH v4 4/5] net: macb: add support for high speed interface

[Parshuram Raju Thombare]

Hi Andrew,

>> +enum {
>> +	MACB_SERDES_RATE_5_PT_15625Gbps = 5,
>> +	MACB_SERDES_RATE_10_PT_3125Gbps = 10,
>> +};
>What do the units mean here? Why would you clock the SERDES at 15Tbps,
>or 3Tbps? 3.125Mbps would give you 2.5Gbps when using 8b/10b encoding.
>
MACB_SERDES_RATE_5_PT_15625Gbps is for 5.15625Gbps, I think this should be just
MACB_SERDES_RATE_5_Gbps and MACB_SERDES_RATE_10_Gbps. I will do it in next patch set.

>Xilinx documentation:
>https://urldefense.proofpoint.com/v2/url?u=https-
>3A__www.xilinx.com_support_documentation_ip-5Fdocumentation_usxgmii_v1-
>5F1_pg251-
>2Dusxgmii.pdf&d=DwIBAg&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-
>_haXqY&r=GTefrem3hiBCnsjCOqAuapQHRN8-rKC1FRbk0it-
>LDs&m=6V8fNIg49czRjfvVtDJ5BbR28p9UPlLLyB7fah7ypcw&s=LsDphgLBe1VDpM
>_K9pkuyal873WeKqHDv64NDRUWy1Q&e=
>seems to suggest USXGMII uses a fixed rate of 10.3125Gb/s. So why do
>you need to change the rate?
For USXGMII, Cadence MAC need to be correctly programmed for external serdes rate.

Regards,
Parshuram Thombare

Re: [PATCH v4 4/5] net: macb: add support for high speed interface

[Andrew Lunn]

On Mon, Jun 24, 2019 at 06:52:51AM +0000, Parshuram Raju Thombare wrote:
> Hi Andrew,
> 
> >> +enum {
> >> +	MACB_SERDES_RATE_5_PT_15625Gbps = 5,
> >> +	MACB_SERDES_RATE_10_PT_3125Gbps = 10,
> >> +};
> >What do the units mean here? Why would you clock the SERDES at 15Tbps,
> >or 3Tbps? 3.125Mbps would give you 2.5Gbps when using 8b/10b encoding.
> >
> MACB_SERDES_RATE_5_PT_15625Gbps is for 5.15625Gbps, I think this should be just
> MACB_SERDES_RATE_5_Gbps and MACB_SERDES_RATE_10_Gbps. I will do it in next patch set.
 
OK.

> >Xilinx documentation:
> >https://urldefense.proofpoint.com/v2/url?u=https-
> >3A__www.xilinx.com_support_documentation_ip-5Fdocumentation_usxgmii_v1-
> >5F1_pg251-
> >2Dusxgmii.pdf&d=DwIBAg&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-
> >_haXqY&r=GTefrem3hiBCnsjCOqAuapQHRN8-rKC1FRbk0it-
> >LDs&m=6V8fNIg49czRjfvVtDJ5BbR28p9UPlLLyB7fah7ypcw&s=LsDphgLBe1VDpM
> >_K9pkuyal873WeKqHDv64NDRUWy1Q&e=
> >seems to suggest USXGMII uses a fixed rate of 10.3125Gb/s. So why do
> >you need to change the rate?
> For USXGMII, Cadence MAC need to be correctly programmed for external serdes rate.

What i'm saying is that the USXGMII rate is fixed. So why do you need
a device tree property for the SERDES rate?

     Andrew

RE: [PATCH v4 4/5] net: macb: add support for high speed interface

[Parshuram Raju Thombare]

Hi Andrew,

>What i'm saying is that the USXGMII rate is fixed. So why do you need a device
>tree property for the SERDES rate?
This is based on Cisco USXGMII specification, it specify USXGMII 5G and USXGMII 10G.
Sorry I can't share that document here.

Regards,
Parshuram Thombare

Re: [PATCH v4 4/5] net: macb: add support for high speed interface

[Russell King - ARM Linux admin]

On Tue, Jun 25, 2019 at 08:26:29AM +0000, Parshuram Raju Thombare wrote:
> Hi Andrew,
> 
> >What i'm saying is that the USXGMII rate is fixed. So why do you need a device
> >tree property for the SERDES rate?
> This is based on Cisco USXGMII specification, it specify USXGMII 5G and USXGMII 10G.
> Sorry I can't share that document here.

The closed nature of the USXGMII spec makes it very hard for us to know
whether your implementation is correct or not.

I have some documentation which suggests that USVGMII is a USXGMII link
running at "5GR" rate as opposed to USXGMII running at "10GR" rate.

So, I think 5G mode should be left out until it becomes clear that (a)
we should specify it as USXGMII with a 5G rate, or as USVGMII.

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

RE: [PATCH v4 4/5] net: macb: add support for high speed interface

[Parshuram Raju Thombare]

>The closed nature of the USXGMII spec makes it very hard for us to know
>whether your implementation is correct or not.
>
>I have some documentation which suggests that USVGMII is a USXGMII link
>running at "5GR" rate as opposed to USXGMII running at "10GR" rate.
>
>So, I think 5G mode should be left out until it becomes clear that (a)
>we should specify it as USXGMII with a 5G rate, or as USVGMII.
>
Ok, I will remove 5G rate for USXGMII.

Regards,
Parshuram Thombare

[PATCH v4 5/5] net: macb: parameter added to cadence ethernet controller DT binding

[Parshuram Thombare]

New parameters added to Cadence ethernet controller DT binding
for USXGMII interface.

Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
 Documentation/devicetree/bindings/net/macb.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 63c73fafe26d..dabdf9d3b574 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -28,6 +28,9 @@ Required properties:
 	Optional elements: 'rx_clk' applies to cdns,zynqmp-gem
 	Optional elements: 'tsu_clk'
 - clocks: Phandles to input clocks.
+- serdes-rate-gbps External serdes rate.Mandatory for USXGMII mode.
+	5 - 5G
+	10 - 10G
 
 The MAC address will be determined using the optional properties
 defined in ethernet.txt.
-- 
2.17.1