net: stmmac: improve PCS support

This series is the next of the three part series sorting out the PCS support in stmmac, building on part 2:

net: stmmac: qcom-ethqos: further serdes reorganisation

Similar patches have been posted previously. This series does away with the common SerDes PHY support, instead using a flag to indicate whether 2500Mbps mode is supported (STMMAC_FLAG_SERDES_SUPPORTS_2500M.) At this time, I have no plans to resurect the common SerDes PHY support - the generic PHY layer implementations are just too random to consider that, and I certainly do not want the extra work of fixing that.

Changes from RFC v3:
- only set default_an_inband if using SGMII/2500BASE-X mode.

 drivers/net/ethernet/stmicro/stmmac/common.h       |   4 -
 .../ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c    |   2 +
 drivers/net/ethernet/stmicro/stmmac/dwmac1000.h    |  12 +-
 .../net/ethernet/stmicro/stmmac/dwmac1000_core.c   |  11 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h       |  10 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c  |  10 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |   8 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c   | 156 +++++++++++++++++++--
 drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h   |  29 +++-
 include/linux/stmmac.h                             |   1 +
 10 files changed, 197 insertions(+), 46 deletions(-)

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

We need to describe one more register (offset and field bitmask) to the PCS code. Move the existing PCS offset and interrupt enable bits to a new struct and pass that in to stmmac_integrated_pcs_init().

---
 drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 9 ++++++---
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c    | 8 ++++++--
 drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c     | 8 ++++----
 drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h     | 9 +++++++--
 4 files changed, 23 insertions(+), 11 deletions(-)

index 01f8353eb6ef..c851e3b88ce5 100644
@@ -22,14 +22,17 @@
 #include "stmmac_ptp.h"
 #include "dwmac1000.h"
 
+static const struct stmmac_pcs_info dwmac1000_pcs_info = {
+	.pcs_offset = GMAC_PCS_BASE,
+	.int_mask = GMAC_INT_DISABLE_PCSLINK | GMAC_INT_DISABLE_PCSAN,
+};
+
 static int dwmac1000_pcs_init(struct stmmac_priv *priv)
 {
 	if (!priv->dma_cap.pcs)
 		return 0;
 
-	return stmmac_integrated_pcs_init(priv, GMAC_PCS_BASE,
-					  GMAC_INT_DISABLE_PCSLINK |
-					  GMAC_INT_DISABLE_PCSAN);
+	return stmmac_integrated_pcs_init(priv, &dwmac1000_pcs_info);
 }
 
 static void dwmac1000_core_init(struct mac_device_info *hw,

index 4c6fed3ecbcf..ba5393beb7bb 100644
@@ -22,13 +22,17 @@
 #include "dwmac4.h"
 #include "dwmac5.h"
 
+static const struct stmmac_pcs_info dwmac4_pcs_info = {
+	.pcs_offset = GMAC_PCS_BASE,
+	.int_mask = GMAC_INT_PCS_LINK | GMAC_INT_PCS_ANE,
+};
+
 static int dwmac4_pcs_init(struct stmmac_priv *priv)
 {
 	if (!priv->dma_cap.pcs)
 		return 0;
 
-	return stmmac_integrated_pcs_init(priv, GMAC_PCS_BASE,
-					  GMAC_INT_PCS_LINK | GMAC_INT_PCS_ANE);
+	return stmmac_integrated_pcs_init(priv, &dwmac4_pcs_info);
 }
 
 static void dwmac4_core_init(struct mac_device_info *hw,

index 88fa359ea716..2695e0b9ed03 100644
@@ -90,8 +90,8 @@ int stmmac_integrated_pcs_get_phy_intf_sel(struct phylink_pcs *pcs,
 	return -EINVAL;
 }
 
-int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset,
-			       u32 int_mask)
+int stmmac_integrated_pcs_init(struct stmmac_priv *priv,
+			       const struct stmmac_pcs_info *pcs_info)
 {
 	struct stmmac_pcs *spcs;
 

@@ -100,8 +100,8 @@ int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset,
 		return -ENOMEM;
 
 	spcs->priv = priv;
-	spcs->base = priv->ioaddr + offset;
-	spcs->int_mask = int_mask;
+	spcs->base = priv->ioaddr + pcs_info->pcs_offset;
+	spcs->int_mask = pcs_info->int_mask;
 	spcs->pcs.ops = &dwmac_integrated_pcs_ops;
 
 	__set_bit(PHY_INTERFACE_MODE_SGMII, spcs->pcs.supported_interfaces);

index 23bbd4f10bf8..f1ee473d8e3e 100644
@@ -27,6 +27,11 @@
 
 struct stmmac_priv;
 
+struct stmmac_pcs_info {
+	unsigned int pcs_offset;
+	u32 int_mask;
+};
+
 struct stmmac_pcs {
 	struct stmmac_priv *priv;
 	void __iomem *base;

@@ -44,8 +49,8 @@ void stmmac_integrated_pcs_irq(struct stmmac_priv *priv, u32 status,
 			       struct stmmac_extra_stats *x);
 int stmmac_integrated_pcs_get_phy_intf_sel(struct phylink_pcs *pcs,
 					   phy_interface_t interface);
-int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset,
-			       u32 int_mask);
+int stmmac_integrated_pcs_init(struct stmmac_priv *priv,
+			       const struct stmmac_pcs_info *pcs_info);
 
 /**
  * dwmac_ctrl_ane - To program the AN Control Register.

2.47.3

Report the link, speed and duplex for SGMII links, read from the SGMII, RGMII and SMII status and control register.

---
rfc->v1: fix setting SGMII's link status - depend on both link status. v2:
- fill in rsgmii offset and status fields in dwmac cores
- rename new GMAC_RGSMIII_xxx constants to GMAC_RGSMII_xxx

---
 .../net/ethernet/stmicro/stmmac/dwmac1000.h   | 12 +----
 .../ethernet/stmicro/stmmac/dwmac1000_core.c  |  2 +
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h  | 10 +---
 .../net/ethernet/stmicro/stmmac/dwmac4_core.c |  2 +
 .../net/ethernet/stmicro/stmmac/stmmac_pcs.c  | 49 ++++++++++++++++++-
 .../net/ethernet/stmicro/stmmac/stmmac_pcs.h  |  4 ++
 6 files changed, 58 insertions(+), 21 deletions(-)

index 9fe639fb06bb..1de1f929d61c 100644
@@ -70,18 +70,8 @@ enum power_event {
 #define GMAC_RGSMIIIS		0x000000d8	/* RGMII/SMII status */
 
 /* SGMII/RGMII status register */
-#define GMAC_RGSMIIIS_LNKMODE		BIT(0)
-#define GMAC_RGSMIIIS_SPEED		GENMASK(2, 1)
-#define GMAC_RGSMIIIS_LNKSTS		BIT(3)
-#define GMAC_RGSMIIIS_JABTO		BIT(4)
-#define GMAC_RGSMIIIS_FALSECARDET	BIT(5)
+#define GMAC_RSGMIIIS_MASK		GENMASK(15, 0)
 #define GMAC_RGSMIIIS_SMIDRXS		BIT(16)
-/* LNKMOD */
-#define GMAC_RGSMIIIS_LNKMOD_MASK	0x1
-/* LNKSPEED */
-#define GMAC_RGSMIIIS_SPEED_125		0x2
-#define GMAC_RGSMIIIS_SPEED_25		0x1
-#define GMAC_RGSMIIIS_SPEED_2_5		0x0
 
 /* GMAC Configuration defines */
 #define GMAC_CONTROL_2K 0x08000000	/* IEEE 802.3as 2K packets */

index c851e3b88ce5..caac85fc08f1 100644
@@ -24,6 +24,8 @@
 
 static const struct stmmac_pcs_info dwmac1000_pcs_info = {
 	.pcs_offset = GMAC_PCS_BASE,
+	.rgsmii_offset = GMAC_RGSMIIIS,
+	.rgsmii_status_mask = GMAC_RSGMIIIS_MASK,
 	.int_mask = GMAC_INT_DISABLE_PCSLINK | GMAC_INT_DISABLE_PCSAN,
 };
 

index d797d936aee1..ffcd036d4c02 100644
@@ -470,15 +470,7 @@ static inline u32 mtl_low_credx_base_addr(const struct dwmac4_addrs *addrs,
 #define GMAC_PHYIF_CTRLSTATUS_TC		BIT(0)
 #define GMAC_PHYIF_CTRLSTATUS_LUD		BIT(1)
 #define GMAC_PHYIF_CTRLSTATUS_SMIDRXS		BIT(4)
-#define GMAC_PHYIF_CTRLSTATUS_LNKMOD		BIT(16)
-#define GMAC_PHYIF_CTRLSTATUS_SPEED		GENMASK(18, 17)
-#define GMAC_PHYIF_CTRLSTATUS_LNKSTS		BIT(19)
-#define GMAC_PHYIF_CTRLSTATUS_JABTO		BIT(20)
-#define GMAC_PHYIF_CTRLSTATUS_FALSECARDET	BIT(21)
-/* LNKSPEED */
-#define GMAC_PHYIF_CTRLSTATUS_SPEED_125		0x2
-#define GMAC_PHYIF_CTRLSTATUS_SPEED_25		0x1
-#define GMAC_PHYIF_CTRLSTATUS_SPEED_2_5		0x0
+#define GMAC_PHYIF_CTRLSTATUS_RSGMII_MASK	GENMASK(31, 16)
 
 extern const struct stmmac_dma_ops dwmac4_dma_ops;
 extern const struct stmmac_dma_ops dwmac410_dma_ops;

index ba5393beb7bb..c6fcfae27c3d 100644
@@ -24,6 +24,8 @@
 
 static const struct stmmac_pcs_info dwmac4_pcs_info = {
 	.pcs_offset = GMAC_PCS_BASE,
+	.rgsmii_offset = GMAC_PHYIF_CONTROL_STATUS,
+	.rgsmii_status_mask = GMAC_PHYIF_CTRLSTATUS_RSGMII_MASK,
 	.int_mask = GMAC_INT_PCS_LINK | GMAC_INT_PCS_ANE,
 };
 

index f1ee473d8e3e..09e609f111b1 100644
@@ -29,12 +29,16 @@ struct stmmac_priv;
 
 struct stmmac_pcs_info {
 	unsigned int pcs_offset;
+	unsigned int rgsmii_offset;
+	u32 rgsmii_status_mask;
 	u32 int_mask;
 };
 
 struct stmmac_pcs {
 	struct stmmac_priv *priv;
 	void __iomem *base;
+	void __iomem *rgsmii;
+	u32 rgsmii_status_mask;
 	u32 int_mask;
 	struct phylink_pcs pcs;
 };

2.47.3

The integrated PCS supports 802.3z (BASE-X) modes when the Synopsys IP is coupled with an appropriate SerDes to provide the electrical interface. The PCS presents a TBI interface to the SerDes for this. Thus, the BASE-X related registers are only present when TBI mode is supported.

dwmac-qcom-ethqos added support for using 2.5G with the integrated PCS by calling dwmac_ctrl_ane() directly.

Add support for the following to the integrated PCS:
- 1000BASE-X protocol unconditionally.
- 2500BASE-X if the coupled SerDes supports 2.5G speed.
- The above without autonegotiation.
- If the PCS supports TBI, then optional BASE-X autonegotiation for each of the above.

---
 .../net/ethernet/stmicro/stmmac/stmmac_pcs.c  | 89 ++++++++++++++++++-
 .../net/ethernet/stmicro/stmmac/stmmac_pcs.h  |  1 +
 include/linux/stmmac.h                        |  1 +
 3 files changed, 87 insertions(+), 4 deletions(-)

index df72f7c5a6a7..5dbb8fd41099 100644
@@ -26,6 +26,27 @@
 #define GMAC_RGSMII_SPEED_2_5		0
 #define GMAC_RGSMII_LNKSTS		BIT(3)
 
+static unsigned int dwmac_integrated_pcs_inband_caps(struct phylink_pcs *pcs,
+						     phy_interface_t interface)
+{
+	struct stmmac_pcs *spcs = phylink_pcs_to_stmmac_pcs(pcs);
+	unsigned int ib_caps;
+
+	if (phy_interface_mode_is_8023z(interface)) {
+		ib_caps = LINK_INBAND_DISABLE;
+
+		/* If the PCS supports TBI/RTBI, then BASE-X negotiation is
+		 * supported.
+		 */
+		if (spcs->support_tbi_rtbi)
+			ib_caps |= LINK_INBAND_ENABLE;
+
+		return ib_caps;
+	}
+
+	return 0;
+}
+
 static int dwmac_integrated_pcs_enable(struct phylink_pcs *pcs)
 {
 	struct stmmac_pcs *spcs = phylink_pcs_to_stmmac_pcs(pcs);

@@ -47,12 +68,20 @@ static void dwmac_integrated_pcs_get_state(struct phylink_pcs *pcs,
 					   struct phylink_link_state *state)
 {
 	struct stmmac_pcs *spcs = phylink_pcs_to_stmmac_pcs(pcs);
-	u32 status, rgsmii;
+	u32 status, lpa, rgsmii;
 
 	status = readl(spcs->base + GMAC_AN_STATUS);
 
 	if (phy_interface_mode_is_8023z(state->interface)) {
-		state->link = false;
+		/* For BASE-X modes, the PCS block supports the advertisement
+		 * and link partner advertisement registers using standard
+		 * 802.3 format. The status register also has the link status
+		 * and AN complete bits in the same bit location. This will
+		 * only be used when AN is enabled.
+		 */
+		lpa = readl(spcs->base + GMAC_ANE_LPA);
+
+		phylink_mii_c22_pcs_decode_state(state, neg_mode, status, lpa);
 	} else {
 		rgsmii = field_get(spcs->rgsmii_status_mask,
 				   readl(spcs->rgsmii));

@@ -84,6 +113,21 @@ static void dwmac_integrated_pcs_get_state(struct phylink_pcs *pcs,
 	}
 }
 
+static int dwmac_integrated_pcs_config_aneg(struct stmmac_pcs *spcs,
+					    phy_interface_t interface,
+					    const unsigned long *advertising)
+{
+	bool changed = false;
+	u32 adv;
+
+	adv = phylink_mii_c22_pcs_encode_advertisement(interface, advertising);
+	if (readl(spcs->base + GMAC_ANE_ADV) != adv)
+		changed = true;
+	writel(adv, spcs->base + GMAC_ANE_ADV);
+
+	return changed;
+}
+
 static int dwmac_integrated_pcs_config(struct phylink_pcs *pcs,
 				       unsigned int neg_mode,
 				       phy_interface_t interface,

@@ -91,13 +135,28 @@ static int dwmac_integrated_pcs_config(struct phylink_pcs *pcs,
 				       bool permit_pause_to_mac)
 {
 	struct stmmac_pcs *spcs = phylink_pcs_to_stmmac_pcs(pcs);
+	bool changed = false, ane = true;
+
+	/* Only configure the advertisement and allow AN in BASE-X mode if
+	 * the core supports TBI/RTBI. AN will be filtered out by via phylink
+	 * and the .pcs_inband_caps() method above.
+	 */
+	if (phy_interface_mode_is_8023z(interface) &&
+	    spcs->support_tbi_rtbi) {
+		ane = neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED;
+
+		changed = dwmac_integrated_pcs_config_aneg(spcs, interface,
+							   advertising);
+	}
 
-	dwmac_ctrl_ane(spcs->base, 0, 1, spcs->priv->hw->reverse_sgmii_enable);
+	dwmac_ctrl_ane(spcs->base, 0, ane,
+		       spcs->priv->hw->reverse_sgmii_enable);
 
-	return 0;
+	return changed;
 }
 
 static const struct phylink_pcs_ops dwmac_integrated_pcs_ops = {
+	.pcs_inband_caps = dwmac_integrated_pcs_inband_caps,
 	.pcs_enable = dwmac_integrated_pcs_enable,
 	.pcs_disable = dwmac_integrated_pcs_disable,
 	.pcs_get_state = dwmac_integrated_pcs_get_state,

@@ -129,9 +188,18 @@ void stmmac_integrated_pcs_irq(struct stmmac_priv *priv, u32 status,
 int stmmac_integrated_pcs_get_phy_intf_sel(struct phylink_pcs *pcs,
 					   phy_interface_t interface)
 {
+	struct stmmac_pcs *spcs = phylink_pcs_to_stmmac_pcs(pcs);
+
 	if (interface == PHY_INTERFACE_MODE_SGMII)
 		return PHY_INTF_SEL_SGMII;
 
+	if (phy_interface_mode_is_8023z(interface)) {
+		if (spcs->support_tbi_rtbi)
+			return PHY_INTF_SEL_TBI;
+		else
+			return PHY_INTF_SEL_SGMII;
+	}
+
 	return -EINVAL;
 }
 

@@ -151,7 +219,20 @@ int stmmac_integrated_pcs_init(struct stmmac_priv *priv,
 	spcs->int_mask = pcs_info->int_mask;
 	spcs->pcs.ops = &dwmac_integrated_pcs_ops;
 
+	/* If the PCS supports extended status, then it supports BASE-X AN
+	 * with a TBI interface to the SerDes. Otherwise, we can support
+	 * BASE-X without AN using SGMII, which is required for qcom-ethqos.
+	 */
+	if (readl(spcs->base + GMAC_AN_STATUS) & BMSR_ESTATEN)
+		spcs->support_tbi_rtbi = true;
+
 	__set_bit(PHY_INTERFACE_MODE_SGMII, spcs->pcs.supported_interfaces);
+	__set_bit(PHY_INTERFACE_MODE_1000BASEX, spcs->pcs.supported_interfaces);
+
+	/* Only allow 2500BASE-X if the SerDes has support. */
+	if (priv->plat->flags & STMMAC_FLAG_SERDES_SUPPORTS_2500M)
+		__set_bit(PHY_INTERFACE_MODE_2500BASEX,
+			  spcs->pcs.supported_interfaces);
 
 	priv->integrated_pcs = spcs;
 

index 09e609f111b1..b2b12d34b3dd 100644
@@ -41,6 +41,7 @@ struct stmmac_pcs {
 	u32 rgsmii_status_mask;
 	u32 int_mask;
 	struct phylink_pcs pcs;
+	bool support_tbi_rtbi;
 };
 
 static inline struct stmmac_pcs *

index 937985276e6b..a8e9b5d3289f 100644
@@ -213,6 +213,7 @@ enum dwmac_core_type {
 #define STMMAC_FLAG_EN_TX_LPI_CLK_PHY_CAP	BIT(12)
 #define STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY	BIT(13)
 #define STMMAC_FLAG_KEEP_PREAMBLE_BEFORE_SFD	BIT(14)
+#define STMMAC_FLAG_SERDES_SUPPORTS_2500M	BIT(15)
 
 struct mac_device_info;
 

2.47.3

dwmac-qcom-ethqos supports SGMII and 2500BASE-X using the integrated PCS, so we need to expand the PCS support to include support for BASE-X modes.

Add support to the prereset configuration to detect 2500BASE-X, and arrange for stmmac_mac_select_pcs() to return the integrated PCS if its supported_interfaces bitmap reports support for the interface mode.

This results in priv->hw->pcs now being write-only, so remove it.

---
 drivers/net/ethernet/stmicro/stmmac/common.h      | 4 ----
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 ++------
 2 files changed, 2 insertions(+), 10 deletions(-)

index f1628de8ed18..c9dac55d0c02 100644
@@ -278,9 +278,6 @@ struct stmmac_safety_stats {
 #define FLOW_TX		2
 #define FLOW_AUTO	(FLOW_TX | FLOW_RX)
 
-/* PCS defines */
-#define STMMAC_PCS_SGMII	(1 << 1)
-
 #define SF_DMA_MODE 1		/* DMA STORE-AND-FORWARD Operation Mode */
 
 /* DMA HW feature register fields */

@@ -632,7 +629,6 @@ struct mac_device_info {
 	unsigned int unicast_filter_entries;
 	unsigned int mcast_bits_log2;
 	unsigned int rx_csum;
-	unsigned int pcs;
 	unsigned int num_vlan;
 	u32 vlan_filter[32];
 	bool vlan_fail_q_en;

index 11150bddd872..9e6d7f0e88d5 100644
@@ -946,11 +946,8 @@ static struct phylink_pcs *stmmac_mac_select_pcs(struct phylink_config *config,
 			return pcs;
 	}
 
-	/* The PCS control register is only relevant for SGMII, TBI and RTBI
-	 * modes. We no longer support TBI or RTBI, so only configure this
-	 * register when operating in SGMII mode with the integrated PCS.
-	 */
-	if (priv->hw->pcs & STMMAC_PCS_SGMII && priv->integrated_pcs)
+	if (priv->integrated_pcs &&
+	    test_bit(interface, priv->integrated_pcs->pcs.supported_interfaces))
 		return &priv->integrated_pcs->pcs;
 
 	return NULL;

@@ -1209,7 +1206,6 @@ static void stmmac_check_pcs_mode(struct stmmac_priv *priv)
 
 	if (priv->dma_cap.pcs && interface == PHY_INTERFACE_MODE_SGMII) {
 		netdev_dbg(priv->dev, "PCS SGMII support enabled\n");
-		priv->hw->pcs = STMMAC_PCS_SGMII;
 
 		switch (speed) {
 		case SPEED_10:

2.47.3

Enable 2500BASE-X support in qcom-ethqos to support the 2.5G speed with the integrated PCS.

---
 drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 1 +
 1 file changed, 1 insertion(+)

index 3ccf20fdf52a..f0e7a060ca73 100644
@@ -828,6 +828,7 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
 	if (ethqos->serdes_phy) {
 		plat_dat->serdes_powerup = qcom_ethqos_serdes_powerup;
 		plat_dat->serdes_powerdown  = qcom_ethqos_serdes_powerdown;
+		plat_dat->flags |= STMMAC_FLAG_SERDES_SUPPORTS_2500M;
 	}
 
 	/* Enable TSO on queue0 and enable TBS on rest of the queues */

2.47.3

qcom-ethqos is manually enabling the PCS ANE bit for SGMII modes while phylink remains in outband mode. With support for SGMII inband mode now supported by the stmmac PCS code, switch phylink to use inband mode for SGMII. Phylink will switch back to outband mode for 2500BASE-X since the PCS code reports no BASE-X AN support.

---
 drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 1 +
 1 file changed, 1 insertion(+)

index f0e7a060ca73..a4571c0ee8b6 100644
@@ -765,6 +765,7 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
 	case PHY_INTERFACE_MODE_SGMII:
 		ethqos->configure_func = ethqos_configure_sgmii;
 		plat_dat->mac_finish = ethqos_mac_finish_serdes;
+		plat_dat->mdio_bus_data->default_an_inband = true;
 		break;
 	default:
 		dev_err(dev, "Unsupported phy mode %s\n",

2.47.3

Provide phylink with the integrated PCS autonegotiation capabilities, and configure the PCS's AN settings according to phylink's requested requirements.

This may cause regressions.

---
 .../net/ethernet/stmicro/stmmac/stmmac_pcs.c  | 24 +++++++++++++------
 1 file changed, 17 insertions(+), 7 deletions(-)

index 5dbb8fd41099..c56765b5aea4 100644
@@ -32,6 +32,9 @@ static unsigned int dwmac_integrated_pcs_inband_caps(struct phylink_pcs *pcs,
 	struct stmmac_pcs *spcs = phylink_pcs_to_stmmac_pcs(pcs);
 	unsigned int ib_caps;
 
+	if (interface == PHY_INTERFACE_MODE_SGMII)
+		return LINK_INBAND_ENABLE | LINK_INBAND_DISABLE;
+
 	if (phy_interface_mode_is_8023z(interface)) {
 		ib_caps = LINK_INBAND_DISABLE;
 

@@ -135,22 +138,29 @@ static int dwmac_integrated_pcs_config(struct phylink_pcs *pcs,
 				       bool permit_pause_to_mac)
 {
 	struct stmmac_pcs *spcs = phylink_pcs_to_stmmac_pcs(pcs);
-	bool changed = false, ane = true;
+	void __iomem *an_control = spcs->base + GMAC_AN_CTRL(0);
+	bool ane, changed = false;
+	u32 ctrl;
+
+	ane = neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED;
 
 	/* Only configure the advertisement and allow AN in BASE-X mode if
 	 * the core supports TBI/RTBI. AN will be filtered out by via phylink
 	 * and the .pcs_inband_caps() method above.
 	 */
 	if (phy_interface_mode_is_8023z(interface) &&
-	    spcs->support_tbi_rtbi) {
-		ane = neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED;
-
+	    spcs->support_tbi_rtbi)
 		changed = dwmac_integrated_pcs_config_aneg(spcs, interface,
 							   advertising);
-	}
 
-	dwmac_ctrl_ane(spcs->base, 0, ane,
-		       spcs->priv->hw->reverse_sgmii_enable);
+	ctrl = readl(an_control) & ~(GMAC_AN_CTRL_ANE | GMAC_AN_CTRL_SGMRAL);
+	if (spcs->priv->hw->reverse_sgmii_enable)
+		ctrl |= GMAC_AN_CTRL_SGMRAL | GMAC_AN_CTRL_ANE;
+	else if (ane)
+		ctrl |= GMAC_AN_CTRL_ANE;
+	else
+		ctrl |= GMAC_AN_CTRL_SGMRAL;
+	writel(ctrl, an_control);
 
 	return changed;
 }

2.47.3

Report if/when qcom-ethqos changes the PCS configuration. With phylink now setting the PCS configuration, there should be no need for drivers to change this.

---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

index b2b12d34b3dd..e1160bc8a63d 100644
@@ -71,6 +71,7 @@ static inline void dwmac_ctrl_ane(void __iomem *ioaddr, u32 reg, bool ane,
 				  bool srgmi_ral)
 {
 	u32 value = readl(ioaddr + GMAC_AN_CTRL(reg));
+	u32 old = value, diff;
 
 	/* Enable and restart the Auto-Negotiation */
 	if (ane)

@@ -85,5 +86,19 @@ static inline void dwmac_ctrl_ane(void __iomem *ioaddr, u32 reg, bool ane,
 		value |= GMAC_AN_CTRL_SGMRAL;
 
 	writel(value, ioaddr + GMAC_AN_CTRL(reg));
+
+	diff = old ^ value;
+	if (diff & ~GMAC_AN_CTRL_RAN) {
+		pr_warn("dwmac: PCS configuration changed from phylink by glue, please report: 0x%08x -> 0x%08x\n",
+			old & ~GMAC_AN_CTRL_RAN, value & ~GMAC_AN_CTRL_RAN);
+#define REPORT_BIT(x) \
+		if (diff & GMAC_AN_CTRL_##x) \
+			pr_warn("dwmac: %8s %u -> %u\n", #x, \
+				!!(old & GMAC_AN_CTRL_##x), \
+				!!(value & GMAC_AN_CTRL_##x))
+		REPORT_BIT(ANE);
+		REPORT_BIT(SGMRAL);
+#undef REPORT_BIT
+	}
 }
 #endif /* __STMMAC_PCS_H__ */

2.47.3

Hi Russell,

I encountered two NULL pointer dereferences and had to add a few workarounds to proceed with testing.

1. On QCS9100 Ride R3, the mdio subnode is only defined for ethernet0 (see lemans-ride-ethernet-aqr115c.dtsi). ethernet1 uses ethernet0's MDIO bus, so plat_dat->mdio_bus_data is NULL for ethernet1. Therefore, we get:

	[    9.205979] Hardware name: Qualcomm Technologies, Inc. Lemans Ride Rev3 (DT)
	[    9.221071] pc : qcom_ethqos_probe+0xc4/0x478 [dwmac_qcom_ethqos]
	[    9.221078] lr : qcom_ethqos_probe+0x7c/0x478 [dwmac_qcom_ethqos]
	[    9.221080] sp : ffff800081aa37f0
	[    9.221081] x29: ffff800081aa3860 x28: ffffcf84de69cfe8 x27: 0000000000000001
	[    9.221084] x26: 0000000000000000 x25: ffff0000b4577c80 x24: ffff0000818fd010
	[    9.221087] x23: ffff000ec7057628 x22: 0000000000000000 x21: ffffcf84ca2ce028
	[    9.221089] x20: ffff0000818fd000
	[    9.236975]  x19: ffff0000b3301740 x18: 00000000ffffffff
	[    9.236977] x17: 0000000000000000 x16: ffffcf84dc4a5448 x15: ffff0000b3301ec5
	[    9.236980] x14: 0000000000000000 x13: 0000000000000030 x12: 0101010101010101
	[    9.236982] x11: 7f7f7f7f7f7f7f7f x10: 00000000001305b0 x9 : 0000000000000000
	[    9.236984] x8 : ffff0000b3301780 x7 : 0000000000000000 x6 : 000000000000003f
	[    9.236986] x5 : 0000000000000040 x4 : 0000000000000000 x3 : ffff0000818fd210
	[    9.236988] x2 : 0000000000000017 x1 : 0000000000000000 x0 : 0000000000000001
	[    9.236991] Call trace:
	[    9.236991]  qcom_ethqos_probe+0xc4/0x478 [dwmac_qcom_ethqos] (P)
	[    9.236994]  platform_probe+0x5c/0xac
				...
	[    9.343300] Code: 910a7000 f9400b21 f9010320 52800020 (39005420)
	[    9.343302] ---[ end trace 0000000000000000 ]---

I added the following workaround:
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
@@ -765,7 +767,10 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
case PHY_INTERFACE_MODE_SGMII:
ethqos->configure_func = ethqos_configure_sgmii; plat_dat->mac_finish = ethqos_mac_finish_serdes;
- plat_dat->mdio_bus_data->default_an_inband = true;
+ if (plat_dat->mdio_bus_data)
+ plat_dat->mdio_bus_data->default_an_inband = true;
+ else
+ dev_err(dev, "plat_dat->mdio_bus_data is NULL");
break;
default:
dev_err(dev, "Unsupported phy mode %s\n",

2. After working around issue 1, I hit the following:

	[   27.822907] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
	[   27.831944] Mem abort info:
	[   27.834832]   ESR = 0x0000000086000006
	[   27.838693]   EC = 0x21: IABT (current EL), IL = 32 bits
	[   27.844163]   SET = 0, FnV = 0
	[   27.847320]   EA = 0, S1PTW = 0
	[   27.850563]   FSC = 0x06: level 2 translation fault
	[   27.855587] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000134b1c000
	[   27.862212] [0000000000000000] pgd=080000010185c403, p4d=080000010185c403, pud=0800000134884403, pmd=0000000000000000
	[   27.873107] Internal error: Oops: 0000000086000006 [#1]  SMP
	[   28.001619] Hardware name: Qualcomm Technologies, Inc. Lemans Ride Rev3 (DT)
	[   28.008853] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
	[   28.016001] pc : 0x0
	[   28.018259] lr : phylink_pcs_an_restart+0x50/0x5c
	[   28.023092] sp : ffff8000869fb900
				...
	[   28.185967] ---[ end trace 0000000000000000 ]---

This occurs due to pl->pcs->ops->pcs_an_restart being NULL. I added the following workaround:

	--- a/drivers/net/phy/phylink.c
	+++ b/drivers/net/phy/phylink.c
	@@ -1026,7 +1026,8 @@ static void phylink_pcs_an_restart(struct phylink *pl)
			if (pl->pcs && linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
							 pl->link_config.advertising) &&
			    phy_interface_mode_is_8023z(pl->link_config.interface) &&
	-		    phylink_autoneg_inband(pl->act_link_an_mode))
	+		    phylink_autoneg_inband(pl->act_link_an_mode) &&
	+		    pl->pcs->ops->pcs_an_restart)
					pl->pcs->ops->pcs_an_restart(pl->pcs);
	 }

3. With the recent VLAN filter changes in net-next, ndo_open takes a long time to complete as vlan_restore_hw_rx_fltr() tries to write filters for all 32 indices. This board previously timed out once, but now times out for each index. This is a separate issue unrelated to this series but I added the following workaround to rule out any timing issues.

	--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
	+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
	@@ -158,7 +158,8 @@ static void vlan_restore_hw_rx_fltr(struct net_device *dev,

			/* Extended Rx VLAN Filter Enable */
			for (i = 0; i < hw->num_vlan; i++)
	-               vlan_write_filter(dev, hw, i, hw->vlan_filter[i]);
	+               if (hw->vlan_filter[i])
	+                       vlan_write_filter(dev, hw, i, hw->vlan_filter[i]);
	 }

	 static void vlan_update_hash(struct mac_device_info *hw, u32 hash,

With the above workarounds in place, 2.5G works fine, but 1G (and 100M) do not. With the RFC v1 series, only the 2.5G->1G speed change was broken. With this series, the link does not come up at 1G at all.

Some observations from my failed attempt at debugging so far:
- No change in behaviour after adding the provisioning code in aqr_gen4_config_init().
- Setting GMAC_AN_CTRL_RAN in dwmac_integrated_pcs_config() did not help.
- After shifting to 1G, GMAC_AN_STATUS reads 0x00000008, i.e., BMSR_LSTATUS (bit 2) is clear.

Sharing logs for two 1G scenarios (net-next + this series + the three workarounds above):

# Booting up with a 1G link

	[    8.730854] qcom-ethqos 23040000.ethernet: IRQ eth_wake_irq not found
	[    8.766778] qcom-ethqos 23040000.ethernet: User ID: 0x20, Synopsys ID: 0x52
	[    8.780465] qcom-ethqos 23040000.ethernet:   DWMAC4/5
	[    8.794253] qcom-ethqos 23040000.ethernet: DMA HW capability register supported
	[    8.794258] qcom-ethqos 23040000.ethernet: Active PHY interface: SGMII (2)
	<snip>
	[    8.865690] qcom-ethqos 23040000.ethernet: Using 36/40 bits DMA host/device width
	[    8.865707] qcom-ethqos 23040000.ethernet: clk_csr value out of range (0xffffff00 exceeds mask 0x00000f00), truncating
	[   10.860749] qcom-ethqos 23040000.ethernet eth1: Register MEM_TYPE_PAGE_POOL RxQ-0
	[   10.861119] qcom-ethqos 23040000.ethernet eth1: Register MEM_TYPE_PAGE_POOL RxQ-1
	[   10.861510] qcom-ethqos 23040000.ethernet eth1: Register MEM_TYPE_PAGE_POOL RxQ-2
	[   10.861906] qcom-ethqos 23040000.ethernet eth1: Register MEM_TYPE_PAGE_POOL RxQ-3
	[   10.871763] qcom-ethqos 23040000.ethernet eth1: Enabling Safety Features
	[   10.907513] qcom-ethqos 23040000.ethernet eth1: IEEE 1588-2008 Advanced Timestamp supported
	[   10.921216] qcom-ethqos 23040000.ethernet eth1: registered PTP clock
	[   10.921220] qcom-ethqos 23040000.ethernet eth1: configuring for inband/2500base-x link mode
	[   10.921223] qcom-ethqos 23040000.ethernet eth1: major config, requested inband/2500base-x
	[   10.921226] qcom-ethqos 23040000.ethernet eth1: interface 2500base-x inband modes: pcs=01 phy=00
	[   10.921228] qcom-ethqos 23040000.ethernet eth1: major config, active inband/inband,an-disabled/2500base-x
	[   10.921230] qcom-ethqos 23040000.ethernet eth1: phylink_mac_config: mode=inband/2500base-x/none adv=00000000,00000000,00008000,0000e240 pause=04

No further messages appear after this point, and the link does not come up at 1G.

# Switching from 2.5G to 1G

	[    8.368113] qcom-ethqos 23040000.ethernet: IRQ eth_wake_irq not found
	[    8.379601] qcom-ethqos 23040000.ethernet: User ID: 0x20, Synopsys ID: 0x52
	[    8.392900] qcom-ethqos 23040000.ethernet:   DWMAC4/5
	[    8.432379] qcom-ethqos 23040000.ethernet: DMA HW capability register supported
	<snip>
	[    8.480319] qcom-ethqos 23040000.ethernet: Using 36/40 bits DMA host/device width
	[    8.488609] qcom-ethqos 23040000.ethernet: clk_csr value out of range (0xffffff00 exceeds mask 0x00000f00), truncating
	[   11.192140] qcom-ethqos 23040000.ethernet eth1: Register MEM_TYPE_PAGE_POOL RxQ-0
	[   11.200567] qcom-ethqos 23040000.ethernet eth1: Register MEM_TYPE_PAGE_POOL RxQ-1
	[   11.216466] qcom-ethqos 23040000.ethernet eth1: Register MEM_TYPE_PAGE_POOL RxQ-2
	[   11.232393] qcom-ethqos 23040000.ethernet eth1: Register MEM_TYPE_PAGE_POOL RxQ-3
	[   11.250011] qcom-ethqos 23040000.ethernet eth1: Enabling Safety Features
	[   11.269722] qcom-ethqos 23040000.ethernet eth1: IEEE 1588-2008 Advanced Timestamp supported
	[   11.286758] qcom-ethqos 23040000.ethernet eth1: registered PTP clock
	[   11.300593] qcom-ethqos 23040000.ethernet eth1: configuring for inband/2500base-x link mode
	[   11.300596] qcom-ethqos 23040000.ethernet eth1: major config, requested inband/2500base-x
	[   11.300599] qcom-ethqos 23040000.ethernet eth1: interface 2500base-x inband modes: pcs=01 phy=00
	[   11.316311] qcom-ethqos 23040000.ethernet eth1: major config, active inband/inband,an-disabled/2500base-x
	[   11.316317] qcom-ethqos 23040000.ethernet eth1: phylink_mac_config: mode=inband/2500base-x/none adv=00000000,00000000,00008000,0000e240 pause=04
	[   14.125773] qcom-ethqos 23040000.ethernet: PCS Link Up
	[   14.131081] qcom-ethqos 23040000.ethernet eth1: pcs link up
	[   14.147059] qcom-ethqos 23040000.ethernet eth1: Link is Up - 2.5Gbps/Full - flow control rx/tx
	<link speed changed to 1G>
	[   25.868674] qcom-ethqos 23040000.ethernet: PCS Link Down
	[   25.874184] qcom-ethqos 23040000.ethernet eth1: pcs link down
	[   25.874745] qcom-ethqos 23040000.ethernet eth1: Link is Down

No further messages appear after "Link is Down".

I have been having some issues with the IQ9 EVK (QCA8081 PHY), so no results to report for it yet.

Ayaan

This means we need to move default_an_inband out of mdio_bus_data.

Gah, should've added a pcs_an_restart() implementation.

This problem needs solving, and I suspect we need more than the loopback here. It needs detailed information about your hardware design, and comes down to the missing receive clock. stmmac is unusual that missing clocks affect the accessibility of the host.

This will be because we're not attaching any PHYs when phylink is being told to operate in 2500BASE-X with inband, since it thinks that's the media. That, and, because stmmac has a long history, it needed a special phylink_expects_phy() check added in stmmac_init_phy() which his what prevents the PHY being attached.

One workaround for this would be to specify SGMII in DT rather than 2500base-X. However, that's not a solution for mainline.

I'm going to have to have another think about this... stmmac is beginning to get too painful to solve this problem. As I've said elsewhere, in hind sight, I wish I had blocked dwmac-qcom-ethqos from being merged because of it's direct fiddling with the integrated PCS.

I'm beginning to wonder whether stmmac can ever be sorted out, or whether the driver has gone too far, and the best thing now is to totally decouple it from phylink.

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

Okay, another idea that I hope will work. Please drop the "net: stmmac:
qcom-ethqos: enable inband mode for SGMII" patch so we go back to using outband mode for everything.

Then, disable the call to ethqos_pcs_set_inband() so inband mode doesn't get enabled for SGMII - as phylink will be telling the PHY to disable inband mode.

Does that result in functional 100M, 1G and 2.5G speeds?

Thanks.

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

Hi,

Yes, but with an asterisk.

- 2.5G works fine
- Shifting between 2.5G, 1G, and 100M are usually fine.
- Booting up at 1G results in link flapping (it gets resolved if I
do: ip link set <interface> down and then up on the link partner).
- Similar link flapping also happens when shifting from 2.5G to 1G, though its not always.

Attaching logs of the booting up with a 1G scenario below.

Ayaan
---

[    8.650471] qcom-ethqos 23040000.ethernet: Using 36/40 bits DMA host/device width
[    8.650486] qcom-ethqos 23040000.ethernet: clk_csr value out of range (0xffffff00 exceeds mask 0x00000f00), truncating
[   11.569262] qcom-ethqos 23040000.ethernet eth1: Register MEM_TYPE_PAGE_POOL RxQ-0
[   11.577988] qcom-ethqos 23040000.ethernet eth1: Register MEM_TYPE_PAGE_POOL RxQ-1
[   11.586665] qcom-ethqos 23040000.ethernet eth1: Register MEM_TYPE_PAGE_POOL RxQ-2
[   11.595357] qcom-ethqos 23040000.ethernet eth1: Register MEM_TYPE_PAGE_POOL RxQ-3
[   11.619882] qcom-ethqos 23040000.ethernet eth1: PHY stmmac-0:08 uses interfaces 4,23,27, validating 23
[   11.629468] qcom-ethqos 23040000.ethernet eth1:  interface 23 (2500base-x) rate match pause supports 0-7,9,13-14,47
[   11.640202] qcom-ethqos 23040000.ethernet eth1: PHY [stmmac-0:08] driver [Aquantia AQR115C] (irq=288)
[   11.649691] qcom-ethqos 23040000.ethernet eth1: phy: 2500base-x setting supported 00000000,00000000,00008000,000062ff advertising 00000000,00000000,00008000,000062ff
[   11.680117] qcom-ethqos 23040000.ethernet eth1: Enabling Safety Features
[   11.688517] qcom-ethqos 23040000.ethernet eth1: IEEE 1588-2008 Advanced Timestamp supported
[   11.698080] qcom-ethqos 23040000.ethernet eth1: registered PTP clock
[   11.704633] qcom-ethqos 23040000.ethernet eth1: configuring for phy/2500base-x link mode
[   11.712966] qcom-ethqos 23040000.ethernet eth1: major config, requested phy/2500base-x
[   11.721115] qcom-ethqos 23040000.ethernet eth1: interface 2500base-x inband modes: pcs=01 phy=00
[   11.730157] qcom-ethqos 23040000.ethernet eth1: major config, active phy/outband/2500base-x
[   11.738748] qcom-ethqos 23040000.ethernet eth1: phylink_mac_config: mode=phy/2500base-x/none adv=00000000,00000000,00000000,00000000 pause=00
[   11.763778] qcom-ethqos 23040000.ethernet eth1: phy link down 2500base-x/1Gbps/Full/none/off/nolpi
[   15.215445] qcom-ethqos 23040000.ethernet eth1: phy link up sgmii/1Gbps/Full/none/rx/tx/nolpi
[   15.226572] qcom-ethqos 23040000.ethernet eth1: major config, requested phy/sgmii
[   15.234297] qcom-ethqos 23040000.ethernet eth1: interface sgmii inband modes: pcs=03 phy=03
[   15.242902] qcom-ethqos 23040000.ethernet eth1: major config, active phy/outband/sgmii
[   15.251044] qcom-ethqos 23040000.ethernet eth1: phylink_mac_config: mode=phy/sgmii/none adv=00000000,00000000,00000000,00000000 pause=03
[   15.276661] qcom-ethqos 23040000.ethernet: PCS Link Up
[   15.278115] qcom-ethqos 23040000.ethernet eth1: Link is Up - 1Gbps/Full - flow control rx/tx
[   15.283364] qcom-ethqos 23040000.ethernet eth1: pcs link up
[   18.317922] qcom-ethqos 23040000.ethernet: PCS Link Down
[   18.323424] qcom-ethqos 23040000.ethernet eth1: pcs link down
[   18.323977] qcom-ethqos 23040000.ethernet eth1: Link is Down
[   18.329375] qcom-ethqos 23040000.ethernet: PCS Link Up
[   18.340467] qcom-ethqos 23040000.ethernet eth1: pcs link up
[   18.356522] qcom-ethqos 23040000.ethernet eth1: Link is Up - 1Gbps/Full - flow control rx/tx
[   21.358679] qcom-ethqos 23040000.ethernet: PCS Link Down
[   21.364183] qcom-ethqos 23040000.ethernet eth1: pcs link down
[   21.364745] qcom-ethqos 23040000.ethernet eth1: Link is Down
[   21.370130] qcom-ethqos 23040000.ethernet: PCS Link Up
[   21.370139] qcom-ethqos 23040000.ethernet eth1: pcs link up
[   21.397194] qcom-ethqos 23040000.ethernet eth1: Link is Up - 1Gbps/Full - flow control rx/tx
[   24.399108] qcom-ethqos 23040000.ethernet: PCS Link Down
[   24.404613] qcom-ethqos 23040000.ethernet eth1: pcs link down
[   24.405168] qcom-ethqos 23040000.ethernet eth1: Link is Down
[   24.410563] qcom-ethqos 23040000.ethernet: PCS Link Up
[   24.421657] qcom-ethqos 23040000.ethernet eth1: pcs link up
[   24.437636] qcom-ethqos 23040000.ethernet eth1: Link is Up - 1Gbps/Full - flow control rx/tx
[   27.439468] qcom-ethqos 23040000.ethernet: PCS Link Down
[   27.444971] qcom-ethqos 23040000.ethernet eth1: pcs link down
[   27.445528] qcom-ethqos 23040000.ethernet eth1: Link is Down
[   27.450920] qcom-ethqos 23040000.ethernet: PCS Link Up
[   27.462016] qcom-ethqos 23040000.ethernet eth1: pcs link up
[   27.477991] qcom-ethqos 23040000.ethernet eth1: Link is Up - 1Gbps/Full - flow control rx/tx
[   28.026087] qcom-ethqos 23040000.ethernet eth1: phy link down sgmii/1Gbps/Full/none/rx/tx/nolpi
[   28.035077] qcom-ethqos 23040000.ethernet eth1: Link is Down
[   33.336433] qcom-ethqos 23040000.ethernet eth1: phy link up sgmii/1Gbps/Full/none/rx/tx/nolpi
[   33.342861] qcom-ethqos 23040000.ethernet: PCS Link Down
[   33.350674] qcom-ethqos 23040000.ethernet eth1: pcs link down
[   33.350724] qcom-ethqos 23040000.ethernet: PCS Link Up
[   33.360912] qcom-ethqos 23040000.ethernet eth1: Link is Up - 1Gbps/Full - flow control rx/tx
[   33.366768] qcom-ethqos 23040000.ethernet eth1: pcs link up
[   33.386487] qcom-ethqos 23040000.ethernet eth1: Link is Down

Note: doing a remote link toggle recovers this.

Please look into this first - with the MDIO bus operating at who-knows-what frequency, this could make reading from the PHY unreliable.

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

My guess is clk_get_rate(priv->plat->stmmac_clk) is returning zero, which means we don't know the rate of the CSR clock.

From what I can see in drivers/clk/qcom/gcc-qcs404.c and drivers/clk/qcom/gcc-sdx55.c, this looks like this case - the struct clk_branch makes no mention of any clock rate, nor does it have any parent. From what I can see, neither of these drivers specify any rates for any of their clocks, which likely means that clk_get_rate() will be zero for all of them.

Sadly, when I designed the clk API, I didn't think that people would be stupid enough not to implement the API properly, more fool me.

Under the old code, we would've used STMMAC_CSR_20_35M, which means we're assuming that the CSR clock is between 20 and 35MHz, even though the value is zero. Is that the case? If it's higher than 35MHz, then you've been operating the MDIO bus out of IEEE 802.3 specification, which can make PHY access unrealible.

In any case, please fix your clock drivers.

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

I'm not 100% sure the currently-passed AXI clock is what we want there and the docs aren't super helpful.. is there a synopsys-name for it? What rates would you expect it to run at?

Konrad

There is no easy answer to that - it depends on the bus interfaces and whether the CSR (register) clock is separate.

The likely possible names are hclk_i (for AHB master), aclk_i (for AXI master), or clk_csr_i.

It does state that the CSR clock should have a minimum frequency of 25MHz to allow all statistics to be properly collected.

The rate of the CSR clock needs to be known, as selecting the divider for generating MDC within IEEE 802.3 specifications is rather fundamental. You may find something there which hints at what rate the dwmac's CSR clock runs at.

Hope this helps.

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

If it's either AXI or AHB, in both cases their direct parent is controlled by an entity external to Linux and their rates may change at runtime, based on aggregated needs of the bus. They're defined as levels/corners (abstract term for a hidden volt+freq combo).

It may be that the operating range for the EMAC removes that variability, but with no concrete evidence and just anecdotal experience, that's only the case for the AHB clock

Hopefully Mohd knows more

Konrad

The important thing is that the MDC doesn't exceed the max clock frequency for the PHY and any other device connected to the MDIO bus. IEEE 802.3 specifies a max frequency of 2.5MHz (minimum period for MDC shall be 400 ns). Some PHYs can operate in excess of this, but one would need to confirm that all devices on the MDIO bus supports higher frequencies before using them. In the kernel, we generally err on the side of caution and stick to IEEE 802.3.

There are two ways to achieve the divider value with stmmac.

1. if priv->plat->csr_clk is set to a value other than -1, this configures the hardware divisor (for "normal" cores, it takes STMMAC_CSR_* constants that can be found in include/linux/stmmac.h)

2. otherwise, the rate of priv->plat->stmmac_clk is used as the CSR clock value, which is the reference clock for the divider that generates the MDC clock, and an appropriate divider is selected. Given the available dividers, it works out at between 1.25MHz for a CSR clock of just over 20MHz and 2.47MHz for 800MHz. (I have a patch which documents the ranges for each of the STMMAC_CSR_xxx values.)

Note that the dividier constants are not the actual divider itself, as can be seen in include/linux/stmmac.h

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

Hi,

As noted by Konrad, the AXI and AHB clock rates are indeed unknown to the Linux kernel:

[    7.739389] [DBG] priv->plat->stmmac_clk rate = 0
[    7.739391] [DBG] priv->plat->pclk rate = 0

Additionally, here's what I found (focusing on QCS9100 Ride R3, but most of this should be applicable to all qcom-ethqos consumers):

1. clk_csr_i is connected to the SLV_AHB clock, named "pclk" in the devicetree. This is the source for the MDC. The "stmmaceth" clock, provided by AXI, is used for data transfers. It appears that the devicetree gets it in reverse as per the stmmac clock documentation added by Russell, i.e., the right order would be:

index 147ebf9b1ac6..f1aa2490bf6b 100644
@@ -7111,10 +7111,10 @@ ethernet0: ethernet@23040000 {
                        interrupts = <GIC_SPI 946 IRQ_TYPE_LEVEL_HIGH>,
                                     <GIC_SPI 782 IRQ_TYPE_LEVEL_HIGH>;
                        interrupt-names = "macirq", "sfty";
 
-                       clocks = <&gcc GCC_EMAC0_AXI_CLK>,
-                                <&gcc GCC_EMAC0_SLV_AHB_CLK>,
+                       clocks = <&gcc GCC_EMAC0_SLV_AHB_CLK>,
+                                <&gcc GCC_EMAC0_AXI_CLK>,
                                 <&gcc GCC_EMAC0_PTP_CLK>,
                                 <&gcc GCC_EMAC0_PHY_AUX_CLK>;
                        clock-names = "stmmaceth",
                                      "pclk",

2. However, even with the correct naming, clk_get_rate() would return 0 for both clocks since they are firmware-managed.

3. For GCC_EMAC0_SLV_AHB_CLK, the hardware documentation mentions the range of 50 - 100 MHz. I am trying to check if there's any chance of it turboing to a higher rate. For now, I think we can assume this to be the working range.

In view of this, would setting priv->plat->clk_csr to STMMAC_CSR_60_100M from the glue layer be correct?

Ayaan

The documentation wasn't generated with some special knowledge, but by comparing the stmmac code with the databook and trying to work out what is going on. I would suggest not changing the DT description at the moment.

It seems to me that the original intention was for "stmmac_clk" to be the "application clock", but then the Imagination Technologies Pistachio board came along, and "pclk" was added for possibly the slave interface - and thus pclk would really be more what CSR clock would be.

However, the code that derives the CSR clock divider wasn't changed, and continued to use stmmac_clk.

So now we're stuck with an utter mess, and there is no way now to work out exactly what was the case - I can find no documentation for this Pistachio board let alone the SoC. Hence:

There is confusion around stmmac_clk and pclk which can't be easily resolved today as the Imagination Technologies Pistachio board that pclk was introduced for has no public documentation and is likely now obsolete. So the origins of pclk are lost to the winds of time.

I haven't put much thought into whether this can be solved in some way. One passing throught is to basically deprecate the existing clock names and replace them with "application" and "csr-clock" which are the terms used in the databook.

However, there's a couple of tables in the "Host (System Interface) Clock" section in the databook which gives the clocking for the various different configurations - the application clock is fairly easy to understand, but the CSR clock is quite complex as it depends on the master and slave interface configurations, as well as whether the designer selected the "use different clock for CSR" option.

For the patch which I haven't submitted yet where I worked out the resulting ranges gives the following for this entry:

        { CSR_F_60M,  STMMAC_CSR_60_100M },     // /42  1.43 - 2.38

So, the divider is 42, which gives a range up to 2.38MHz with the 100MHz input. At 50MHz, the MDIO clock will be 1.19MHz which is a little slow but 802.3 doesn't give a minimum rate. It will just take longer to access the PHY.

The important thing is that the MDIO clock isn't too fast, as that will result in corrupted transfers. That said, some PHYs support faster MDIO clocks than 802.3 specifies, but in general it is a good idea that MACs stick to the 802.3 spec especially when they don't know what PHY (or indeed other MDIO devices) will be connected.

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

I tried finding more details about the hardware implementation and it appears that the receive clock is sourced from the SerDes PHY.

As an experiment, the following in __stmmac_open() avoids the issue on this board:

	qcom_ethqos_set_sgmii_loopback(ethqos, true);
	stmmac_vlan_restore(priv);
	qcom_ethqos_set_sgmii_loopback(ethqos, false);

What would be the correct way to handle this? Right now, SGMII loopback remains enabled from qcom_ethqos_probe() up to stmmac_mac_finish().

Ayaan

I also think this needs to be investigated. This message is the result of a recent patch of mine which will error if the clk_csr clock rate is not something we support.

I see all of the in-kernel dts which match qcom,.*ethqos provide a stmmaceth clock, so plat->stmmac_clk should be populated.

stmmac_clk_csr_set() should get the clock rate, and look it up in the stmmac_std_csr_to_mdc table. We'll get this warning if the clock rate is larger than 800MHz or less or equal to 20MHz.

The divisor selection of 15 is not supported by the hardware.

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