linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sean Anderson <sean.anderson@seco.com>
To: netdev@vger.kernel.org, "David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	linux-kernel@vger.kernel.org
Cc: Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	Sean Anderson <sean.anderson@seco.com>,
	Claudiu Beznea <claudiu.beznea@microchip.com>,
	Nicolas Ferre <nicolas.ferre@microchip.com>
Subject: [RFC net-next PATCH 10/16] net: macb: Move PCS settings to PCS callbacks
Date: Mon,  4 Oct 2021 15:15:21 -0400	[thread overview]
Message-ID: <20211004191527.1610759-11-sean.anderson@seco.com> (raw)
In-Reply-To: <20211004191527.1610759-1-sean.anderson@seco.com>

This moves all PCS-related settings to the pcs callbacks. This makes it
easy to support external PCSs. In addition, support for 1000BASE-X is
added (since we need it to set the SGMII mux).

pcs_config should set all registers necessary to bring the link up. In
addition, it needs to keep track of whether it modified anything so that
phylink can restart autonegotiation. config is also the right time to
veto configuration parameters, such as using the wrong interface. This
catches someone trying to use a slower speed (which could be supported
otherwise) after using a faster speed. We can't support this because
phylink doesn't support detaching PCSs.

pcs_link_up is necessary IFF the mode is not in-band and the speed and
duplex are different from what was set in config. However, because the
PCS supports only fixed speed (SPEED_1000 or SPEED_10000) and full
duplex, there is nothing to configure. Therefore, link_up has been
removed.

Now that the autonegotiation is done in pcs_config, we no longer need to
do it in macb_mac_config.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

 drivers/net/ethernet/cadence/macb_main.c | 214 +++++++++++++++--------
 1 file changed, 138 insertions(+), 76 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index db7acce42a27..08dcccd94f87 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -543,6 +543,7 @@ static void macb_validate(struct phylink_config *config,
 			goto none;
 		fallthrough;
 	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_1000BASEX:
 		if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE) {
 			phylink_set(mask, 1000baseT_Full);
 			phylink_set(mask, 1000baseX_Full);
@@ -571,25 +572,100 @@ static void macb_validate(struct phylink_config *config,
 		   __ETHTOOL_LINK_MODE_MASK_NBITS);
 }
 
-static void macb_usx_pcs_link_up(struct phylink_pcs *pcs, unsigned int mode,
-				 phy_interface_t interface, int speed,
-				 int duplex)
-{
-	struct macb *bp = container_of(pcs, struct macb, phylink_pcs);
-	u32 config;
-
-	config = gem_readl(bp, USX_CONTROL);
-	config = GEM_BFINS(SERDES_RATE, MACB_SERDES_RATE_10G, config);
-	config = GEM_BFINS(USX_CTRL_SPEED, HS_SPEED_10000M, config);
-	config &= ~(GEM_BIT(TX_SCR_BYPASS) | GEM_BIT(RX_SCR_BYPASS));
-	config |= GEM_BIT(TX_EN);
-	gem_writel(bp, USX_CONTROL, config);
+static inline struct macb *pcs_to_macb(struct phylink_pcs *pcs)
+{
+	return container_of(pcs, struct macb, phylink_pcs);
+}
+
+static void macb_pcs_get_state(struct phylink_pcs *pcs,
+			       struct phylink_link_state *state)
+{
+	struct macb *bp = pcs_to_macb(pcs);
+
+	if (gem_readl(bp, NCFGR) & GEM_BIT(SGMIIEN))
+		state->interface = PHY_INTERFACE_MODE_SGMII;
+	else
+		state->interface = PHY_INTERFACE_MODE_1000BASEX;
+
+	phylink_mii_c22_pcs_decode_state(state, gem_readl(bp, PCSSTS),
+					 gem_readl(bp, PCSANLPBASE));
+}
+
+/**
+ * macb_pcs_config_an() - Configure autonegotiation settings for PCSs
+ * @bp - The macb to operate on
+ * @mode - The autonegotiation mode
+ * @interface - The interface to use
+ * @advertising - The advertisement mask
+ *
+ * This provides common configuration for PCS autonegotiation.
+ *
+ * Context: Call with @bp->lock held.
+ * Return: 1 if any registers were changed; 0 otherwise
+ */
+static int macb_pcs_config_an(struct macb *bp, unsigned int mode,
+			      phy_interface_t interface,
+			      const unsigned long *advertising)
+{
+	bool changed = false;
+	u16 old, new;
+
+	old = gem_readl(bp, PCSANADV);
+	new = phylink_mii_c22_pcs_encode_advertisement(interface, advertising,
+						       old);
+	if (old != new) {
+		changed = true;
+		gem_writel(bp, PCSANADV, new);
+	}
+
+	old = new = gem_readl(bp, PCSCNTRL);
+	if (mode == MLO_AN_INBAND)
+		new |= BMCR_ANENABLE;
+	else
+		new &= ~BMCR_ANENABLE;
+	if (old != new) {
+		changed = true;
+		gem_writel(bp, PCSCNTRL, new);
+	}
+	return changed;
+}
+
+static int macb_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
+			   phy_interface_t interface,
+			   const unsigned long *advertising,
+			   bool permit_pause_to_mac)
+{
+	bool changed = false;
+	struct macb *bp = pcs_to_macb(pcs);
+	u16 old, new;
+	unsigned long flags;
+
+	spin_lock_irqsave(&bp->lock, flags);
+	old = new = gem_readl(bp, NCFGR);
+	if (interface == PHY_INTERFACE_MODE_SGMII) {
+		new |= GEM_BIT(SGMIIEN);
+	} else if (interface == PHY_INTERFACE_MODE_1000BASEX) {
+		new &= ~GEM_BIT(SGMIIEN);
+	} else {
+		spin_lock_irqsave(&bp->lock, flags);
+		return -EOPNOTSUPP;
+	}
+	if (old != new) {
+		changed = true;
+		gem_writel(bp, NCFGR, new);
+	}
+
+	if (macb_pcs_config_an(bp, mode, interface, advertising))
+		changed = true;
+
+	spin_unlock_irqrestore(&bp->lock, flags);
+	return changed;
 }
 
 static void macb_usx_pcs_get_state(struct phylink_pcs *pcs,
 				   struct phylink_link_state *state)
 {
-	struct macb *bp = container_of(pcs, struct macb, phylink_pcs);
+	struct macb *bp = pcs_to_macb(pcs);
 	u32 val;
 
 	state->speed = SPEED_10000;
@@ -609,70 +685,60 @@ static int macb_usx_pcs_config(struct phylink_pcs *pcs,
 			       const unsigned long *advertising,
 			       bool permit_pause_to_mac)
 {
-	struct macb *bp = container_of(pcs, struct macb, phylink_pcs);
+	bool changed;
+	struct macb *bp = pcs_to_macb(pcs);
+	u16 old, new;
+	unsigned long flags;
 
-	gem_writel(bp, USX_CONTROL, gem_readl(bp, USX_CONTROL) |
-		   GEM_BIT(SIGNAL_OK));
+	if (interface != PHY_INTERFACE_MODE_10GBASER)
+		return -EOPNOTSUPP;
 
-	return 0;
-}
+	spin_lock_irqsave(&bp->lock, flags);
+	old = new = gem_readl(bp, NCR);
+	new |= GEM_BIT(ENABLE_HS_MAC);
+	if (old != new) {
+		changed = true;
+		gem_writel(bp, NCFGR, new);
+	}
 
-static void macb_pcs_get_state(struct phylink_pcs *pcs,
-			       struct phylink_link_state *state)
-{
-	state->link = 0;
-}
+	if (macb_pcs_config_an(bp, mode, interface, advertising))
+		changed = true;
 
-static void macb_pcs_an_restart(struct phylink_pcs *pcs)
-{
-	/* Not supported */
-}
+	old = new = gem_readl(bp, USX_CONTROL);
+	new |= GEM_BIT(SIGNAL_OK);
+	if (old != new) {
+		changed = true;
+		gem_writel(bp, USX_CONTROL, new);
+	}
 
-static int macb_pcs_config(struct phylink_pcs *pcs,
-			   unsigned int mode,
-			   phy_interface_t interface,
-			   const unsigned long *advertising,
-			   bool permit_pause_to_mac)
-{
-	return 0;
+	old = new = gem_readl(bp, USX_CONTROL);
+	new = GEM_BFINS(SERDES_RATE, MACB_SERDES_RATE_10G, new);
+	new = GEM_BFINS(USX_CTRL_SPEED, HS_SPEED_10000M, new);
+	new &= ~(GEM_BIT(TX_SCR_BYPASS) | GEM_BIT(RX_SCR_BYPASS));
+	new |= GEM_BIT(TX_EN);
+	if (old != new) {
+		changed = true;
+		gem_writel(bp, USX_CONTROL, new);
+	}
+
+	spin_unlock_irqrestore(&bp->lock, flags);
+	return changed;
 }
 
 static const struct phylink_pcs_ops macb_phylink_usx_pcs_ops = {
 	.pcs_get_state = macb_usx_pcs_get_state,
 	.pcs_config = macb_usx_pcs_config,
-	.pcs_link_up = macb_usx_pcs_link_up,
 };
 
 static const struct phylink_pcs_ops macb_phylink_pcs_ops = {
 	.pcs_get_state = macb_pcs_get_state,
-	.pcs_an_restart = macb_pcs_an_restart,
 	.pcs_config = macb_pcs_config,
 };
 
 static void macb_mac_config(struct phylink_config *config, unsigned int mode,
 			    const struct phylink_link_state *state)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&bp->lock, flags);
-
-	/* Disable AN for SGMII fixed link configuration, enable otherwise.
-	 * Must be written after PCSSEL is set in NCFGR,
-	 * otherwise writes will not take effect.
-	 */
-	if (macb_is_gem(bp) && state->interface == PHY_INTERFACE_MODE_SGMII) {
-		u32 pcsctrl, old_pcsctrl;
-
-		old_pcsctrl = gem_readl(bp, PCSCNTRL);
-		if (mode == MLO_AN_FIXED)
-			pcsctrl = old_pcsctrl & ~GEM_BIT(PCSAUTONEG);
-		else
-			pcsctrl = old_pcsctrl | GEM_BIT(PCSAUTONEG);
-		if (old_pcsctrl != pcsctrl)
-			gem_writel(bp, PCSCNTRL, pcsctrl);
-	}
-
-	spin_unlock_irqrestore(&bp->lock, flags);
+	/* Nothing to do */
 }
 
 static void macb_mac_link_down(struct phylink_config *config, unsigned int mode,
@@ -763,20 +829,23 @@ static void macb_mac_link_up(struct phylink_config *config,
 static int macb_mac_prepare(struct phylink_config *config, unsigned int mode,
 			    phy_interface_t interface)
 {
+	int set_pcs = 0;
 	struct net_device *ndev = to_net_dev(config->dev);
 	struct macb *bp = netdev_priv(ndev);
 	unsigned long flags;
 	u32 old_ctrl, ctrl;
 	u32 old_ncr, ncr;
 
-	if (interface == PHY_INTERFACE_MODE_10GBASER)
+	if (interface == PHY_INTERFACE_MODE_10GBASER) {
 		bp->phylink_pcs.ops = &macb_phylink_usx_pcs_ops;
-	else if (interface == PHY_INTERFACE_MODE_SGMII)
+		set_pcs = 1;
+	} else if (interface == PHY_INTERFACE_MODE_SGMII ||
+		   interface == PHY_INTERFACE_MODE_1000BASEX) {
 		bp->phylink_pcs.ops = &macb_phylink_pcs_ops;
-	else
-		bp->phylink_pcs.ops = NULL;
+		set_pcs = 1;
+	}
 
-	if (bp->phylink_pcs.ops)
+	if (set_pcs)
 		phylink_set_pcs(bp->phylink, &bp->phylink_pcs);
 
 	spin_lock_irqsave(&bp->lock, flags);
@@ -787,21 +856,14 @@ static int macb_mac_prepare(struct phylink_config *config, unsigned int mode,
 	if (bp->caps & MACB_CAPS_MACB_IS_EMAC) {
 		if (interface == PHY_INTERFACE_MODE_RMII)
 			ctrl |= MACB_BIT(RM9200_RMII);
-	} else if (macb_is_gem(bp)) {
-		ctrl &= ~(GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL));
-		ncr &= ~GEM_BIT(ENABLE_HS_MAC);
-
-		if (state->interface == PHY_INTERFACE_MODE_SGMII) {
-			ctrl |= GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL);
-		} else if (state->interface == PHY_INTERFACE_MODE_10GBASER) {
-			ctrl |= GEM_BIT(PCSSEL);
-			ncr |= GEM_BIT(ENABLE_HS_MAC);
-		} else if (bp->caps & MACB_CAPS_MIIONRGMII &&
-			   bp->phy_interface == PHY_INTERFACE_MODE_MII) {
-			ncr |= MACB_BIT(MIIONRGMII);
-		}
+	} else if (bp->caps & MACB_CAPS_MIIONRGMII &&
+		   bp->phy_interface == PHY_INTERFACE_MODE_MII) {
+		ncr |= MACB_BIT(MIIONRGMII);
 	}
 
+	if (macb_is_gem(bp) && set_pcs)
+		ctrl |= GEM_BIT(PCSSEL);
+
 	/* Apply the new configuration, if any */
 	if (old_ctrl ^ ctrl)
 		macb_or_gem_writel(bp, NCFGR, ctrl);
-- 
2.25.1


  parent reply	other threads:[~2021-10-04 19:16 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-04 19:15 [RFC net-next PATCH 00/16] Add support for Xilinx PCS Sean Anderson
2021-10-04 19:15 ` [RFC net-next PATCH 01/16] dt-bindings: net: Add pcs property Sean Anderson
2021-10-05  9:39   ` Russell King (Oracle)
2021-10-05 16:18     ` Sean Anderson
2021-10-12 13:16     ` Rob Herring
2021-10-12 16:18       ` Sean Anderson
2021-10-12 16:44         ` Rob Herring
2021-10-12 17:01           ` Sean Anderson
2021-10-04 19:15 ` [RFC net-next PATCH 02/16] dt-bindings: net: Add binding for Xilinx PCS Sean Anderson
2021-10-05 12:26   ` Rob Herring
2021-10-04 19:15 ` [RFC net-next PATCH 03/16] net: sfp: Fix typo in state machine debug string Sean Anderson
2021-10-04 21:31   ` Andrew Lunn
2021-10-04 19:15 ` [RFC net-next PATCH 04/16] net: phylink: Move phylink_set_pcs before phylink_create Sean Anderson
2021-10-05  9:43   ` Russell King (Oracle)
2021-10-04 19:15 ` [RFC net-next PATCH 05/16] net: phylink: Automatically attach PCS devices Sean Anderson
2021-10-05  9:48   ` Russell King (Oracle)
2021-10-05 16:42     ` Sean Anderson
2021-10-07 10:23       ` Russell King (Oracle)
2021-10-08  0:14         ` Sean Anderson
2021-10-04 19:15 ` [RFC net-next PATCH 06/16] net: phylink: Add function for optionally adding a PCS Sean Anderson
2021-10-05  9:51   ` Russell King (Oracle)
2021-10-05 13:43     ` Andrew Lunn
2021-10-05 16:17       ` Sean Anderson
2021-10-04 19:15 ` [RFC net-next PATCH 07/16] net: phylink: Add helpers for c22 registers without MDIO Sean Anderson
2021-10-22 12:33   ` Russell King (Oracle)
2021-10-04 19:15 ` [RFC net-next PATCH 08/16] net: macb: Clean up macb_validate Sean Anderson
2021-10-04 23:04   ` Russell King (Oracle)
2021-10-04 23:09     ` Sean Anderson
2021-10-07 13:22   ` Nicolas Ferre
2021-10-08  0:20     ` Sean Anderson
2021-10-08  8:12       ` Nicolas Ferre
2021-10-04 19:15 ` [RFC net-next PATCH 09/16] net: macb: Move most of mac_prepare to mac_config Sean Anderson
2021-10-04 23:05   ` Russell King (Oracle)
2021-10-04 23:09     ` Sean Anderson
2021-10-04 19:15 ` Sean Anderson [this message]
2021-10-05 10:06   ` [RFC net-next PATCH 10/16] net: macb: Move PCS settings to PCS callbacks Russell King (Oracle)
2021-10-05 16:03     ` Sean Anderson
2021-10-05 18:53       ` Russell King (Oracle)
2021-10-05 21:44         ` Sean Anderson
2021-10-05 22:19           ` Russell King (Oracle)
2021-10-07 10:34             ` Russell King (Oracle)
2021-10-07 11:29               ` Russell King (Oracle)
2021-10-07 16:23                 ` Russell King (Oracle)
2021-10-07 17:04                   ` Sean Anderson
2021-10-04 19:15 ` [RFC net-next PATCH 11/16] net: macb: Support restarting PCS autonegotiation Sean Anderson
2021-10-04 19:15 ` [RFC net-next PATCH 12/16] net: macb: Support external PCSs Sean Anderson
2021-10-04 19:15 ` [RFC net-next PATCH 13/16] net: phy: Export get_phy_c22_id Sean Anderson
2021-10-05 10:12   ` Russell King (Oracle)
2021-10-04 19:15 ` [RFC net-next PATCH 14/16] net: mdio: Add helper functions for accessing MDIO devices Sean Anderson
2021-10-04 19:15 ` [RFC net-next PATCH 15/16] net: pcs: Add Xilinx PCS driver Sean Anderson
2021-10-04 19:15 ` [RFC net-next PATCH 16/16] net: sfp: Add quirk to ignore PHYs Sean Anderson
2021-10-04 22:01   ` Andrew Lunn
2021-10-05 10:33   ` Russell King (Oracle)
2021-10-05 16:45     ` Sean Anderson
2021-10-05 18:10       ` Sean Anderson
2021-10-05 19:12       ` Russell King (Oracle)
2021-10-05 20:38         ` Sean Anderson
2021-10-05 22:17           ` Russell King (Oracle)
2021-10-05 23:16             ` Sean Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211004191527.1610759-11-sean.anderson@seco.com \
    --to=sean.anderson@seco.com \
    --cc=andrew@lunn.ch \
    --cc=claudiu.beznea@microchip.com \
    --cc=davem@davemloft.net \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=nicolas.ferre@microchip.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).