Changing the netif_carrier_*() state behind phylink's back has always been prohibited because it messes up with phylinks state tracking, and means that phylink no longer guarantees to call the mac_link_down() and mac_link_up() methods at the appropriate times. This was later documented in the sfp-phylink network driver conversion guide. stmmac was converted to phylink in 2019, but nothing was done with the "PCS" code. Since then, apart from the updates as part of phylink development, nothing has happened with stmmac to improve its use of phylink, or even to address this point. A couple of years ago, a has_integrated_pcs boolean was added by Bart, which later became the STMMAC_FLAG_HAS_INTEGRATED_PCS flag, to avoid manipulating the netif_carrier_*() state. This flag is mis-named, because whenever the stmmac is synthesized for its native SGMII, TBI or RTBI interfaces, it has an "integrated PCS". This boolean/flag actually means "ignore the status from the integrated PCS". Discussing with Bart, the reasons for this are lost to the winds of time (which is why we should always document the reasons in the commit message.) RGMII also has in-band status, and the dwmac cores and stmmac code supports this but with one bug that saves the day. When dwmac cores are synthesised for RGMII only, they do not contain an integrated PCS, and so priv->dma_cap.pcs is clear, which prevents (incorrectly) the "RGMII PCS" being used, meaning we don't read the in-band status. However, a core synthesised for RGMII and also SGMII, TBI or RTBI will have this capability bit set, thus making these code paths reachable. The Jetson Xavier NX uses RGMII mode to talk to its PHY, and removing the incorrect check for priv->dma_cap.pcs reveals the theortical issue with netif_carrier_*() manipulation is real: dwc-eth-dwmac 2490000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0 dwc-eth-dwmac 2490000.ethernet eth0: PHY [stmmac-0:00] driver [RTL8211F Gigabit Ethernet] (irq=141) dwc-eth-dwmac 2490000.ethernet eth0: No Safety Features support found dwc-eth-dwmac 2490000.ethernet eth0: IEEE 1588-2008 Advanced Timestamp supported dwc-eth-dwmac 2490000.ethernet eth0: registered PTP clock dwc-eth-dwmac 2490000.ethernet eth0: configuring for phy/rgmii-id link mode 8021q: adding VLAN 0 to HW filter on device eth0 dwc-eth-dwmac 2490000.ethernet eth0: Adding VLAN ID 0 is not supported Link is Up - 1000/Full Link is Down Link is Up - 1000/Full This looks good until one realises that the phylink "Link" status messages are missing, even when the RJ45 cable is reconnected. Nothing one can do results in the interface working. The interrupt handler (which prints those "Link is" messages) always wins over phylink's resolve worker, meaning phylink never calls the mac_link_up() nor mac_link_down() methods. eth0 also sees no traffic received, and is unable to obtain a DHCP address: 3: eth0: mtu 1500 qdisc mq state UP group defa ult qlen 1000 link/ether e6:d3:6a:e6:92:de brd ff:ff:ff:ff:ff:ff RX: bytes packets errors dropped overrun mcast 0 0 0 0 0 0 TX: bytes packets errors dropped carrier collsns 27686 149 0 0 0 0 With the STMMAC_FLAG_HAS_INTEGRATED_PCS flag set, which disables the netif_carrier_*() manipulation then stmmac works normally: dwc-eth-dwmac 2490000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0 dwc-eth-dwmac 2490000.ethernet eth0: PHY [stmmac-0:00] driver [RTL8211F Gigabit Ethernet] (irq=141) dwc-eth-dwmac 2490000.ethernet eth0: No Safety Features support found dwc-eth-dwmac 2490000.ethernet eth0: IEEE 1588-2008 Advanced Timestamp supported dwc-eth-dwmac 2490000.ethernet eth0: registered PTP clock dwc-eth-dwmac 2490000.ethernet eth0: configuring for phy/rgmii-id link mode 8021q: adding VLAN 0 to HW filter on device eth0 dwc-eth-dwmac 2490000.ethernet eth0: Adding VLAN ID 0 is not supported Link is Up - 1000/Full dwc-eth-dwmac 2490000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx and packets can be transferred. This clearly shows that when priv->hw->pcs is set, but STMMAC_FLAG_HAS_INTEGRATED_PCS is clear, the driver reliably fails. Discovering whether a platform falls into this is impossible as parsing all the dtsi and dts files to find out which use the stmmac driver, whether any of them use RGMII or SGMII and also depends whether an external interface is being used. The kernel likely doesn't contain all dts files either. The only driver that sets this flag uses the qcom,sa8775p-ethqos compatible, and uses SGMII or 2500BASE-X. but these are saved from this problem by the incorrect check for priv->dma_cap.pcs. So, we have to assume that for every other platform that uses SGMII with stmmac is using an external PCS. Moreover, ethtool output can be incorrect. With the full-duplex link negotiated, ethtool reports: Speed: 1000Mb/s Duplex: Half because with dwmac4, the full-duplex bit is in bit 16 of the status, priv->xstats.pcs_duplex becomes BIT(16) for full duplex, but the ethtool ksettings duplex member is u8 - so becomes zero. Moreover, the supported, advertised and link partner modes are all "not reported". Finally, ksettings_set() won't be able to set the advertisement on a PHY if this PCS code is activated, which is incorrect when SGMII is used with a PHY. Thus, remove: 1. the incorrect netif_carrier_*() manipulation. 2. the broken ethtool ksettings code. Given that all uses of STMMAC_FLAG_HAS_INTEGRATED_PCS are now gone, remove the flag from stmmac.h and dwmac-qcom-ethqos.c. Signed-off-by: Russell King (Oracle) --- .../stmicro/stmmac/dwmac-qcom-ethqos.c | 4 -- .../ethernet/stmicro/stmmac/stmmac_ethtool.c | 55 ------------------- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 9 --- include/linux/stmmac.h | 1 - 4 files changed, 69 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c index d8fd4d8f6ced..f62825220cf7 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c @@ -96,7 +96,6 @@ struct ethqos_emac_driver_data { bool rgmii_config_loopback_en; bool has_emac_ge_3; const char *link_clk_name; - bool has_integrated_pcs; u32 dma_addr_width; struct dwmac4_addrs dwmac4_addrs; bool needs_sgmii_loopback; @@ -282,7 +281,6 @@ static const struct ethqos_emac_driver_data emac_v4_0_0_data = { .rgmii_config_loopback_en = false, .has_emac_ge_3 = true, .link_clk_name = "phyaux", - .has_integrated_pcs = true, .needs_sgmii_loopback = true, .dma_addr_width = 36, .dwmac4_addrs = { @@ -856,8 +854,6 @@ static int qcom_ethqos_probe(struct platform_device *pdev) plat_dat->flags |= STMMAC_FLAG_TSO_EN; if (of_device_is_compatible(np, "qcom,qcs404-ethqos")) plat_dat->flags |= STMMAC_FLAG_RX_CLK_RUNS_IN_LPI; - if (data->has_integrated_pcs) - plat_dat->flags |= STMMAC_FLAG_HAS_INTEGRATED_PCS; if (data->dma_addr_width) plat_dat->host_dma_width = data->dma_addr_width; diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c index 39fa1ec92f82..d89662b48087 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c @@ -322,47 +322,6 @@ static int stmmac_ethtool_get_link_ksettings(struct net_device *dev, { struct stmmac_priv *priv = netdev_priv(dev); - if (!(priv->plat->flags & STMMAC_FLAG_HAS_INTEGRATED_PCS) && - (priv->hw->pcs & STMMAC_PCS_RGMII || - priv->hw->pcs & STMMAC_PCS_SGMII)) { - u32 supported, advertising, lp_advertising; - - if (!priv->xstats.pcs_link) { - cmd->base.speed = SPEED_UNKNOWN; - cmd->base.duplex = DUPLEX_UNKNOWN; - return 0; - } - cmd->base.duplex = priv->xstats.pcs_duplex; - - cmd->base.speed = priv->xstats.pcs_speed; - - /* Encoding of PSE bits is defined in 802.3z, 37.2.1.4 */ - - ethtool_convert_link_mode_to_legacy_u32( - &supported, cmd->link_modes.supported); - ethtool_convert_link_mode_to_legacy_u32( - &advertising, cmd->link_modes.advertising); - ethtool_convert_link_mode_to_legacy_u32( - &lp_advertising, cmd->link_modes.lp_advertising); - - /* Reg49[3] always set because ANE is always supported */ - cmd->base.autoneg = ADVERTISED_Autoneg; - supported |= SUPPORTED_Autoneg; - advertising |= ADVERTISED_Autoneg; - lp_advertising |= ADVERTISED_Autoneg; - - cmd->base.port = PORT_OTHER; - - ethtool_convert_legacy_u32_to_link_mode( - cmd->link_modes.supported, supported); - ethtool_convert_legacy_u32_to_link_mode( - cmd->link_modes.advertising, advertising); - ethtool_convert_legacy_u32_to_link_mode( - cmd->link_modes.lp_advertising, lp_advertising); - - return 0; - } - return phylink_ethtool_ksettings_get(priv->phylink, cmd); } @@ -372,20 +331,6 @@ stmmac_ethtool_set_link_ksettings(struct net_device *dev, { struct stmmac_priv *priv = netdev_priv(dev); - if (!(priv->plat->flags & STMMAC_FLAG_HAS_INTEGRATED_PCS) && - (priv->hw->pcs & STMMAC_PCS_RGMII || - priv->hw->pcs & STMMAC_PCS_SGMII)) { - /* Only support ANE */ - if (cmd->base.autoneg != AUTONEG_ENABLE) - return -EINVAL; - - mutex_lock(&priv->lock); - stmmac_pcs_ctrl_ane(priv, 1, priv->hw->ps, 0); - mutex_unlock(&priv->lock); - - return 0; - } - return phylink_ethtool_ksettings_set(priv->phylink, cmd); } diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index be064f240895..d5e345c6f6eb 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -6001,15 +6001,6 @@ static void stmmac_common_interrupt(struct stmmac_priv *priv) for (queue = 0; queue < queues_count; queue++) stmmac_host_mtl_irq_status(priv, priv->hw, queue); - /* PCS link status */ - if (priv->hw->pcs && - !(priv->plat->flags & STMMAC_FLAG_HAS_INTEGRATED_PCS)) { - if (priv->xstats.pcs_link) - netif_carrier_on(priv->dev); - else - netif_carrier_off(priv->dev); - } - stmmac_timestamp_interrupt(priv, priv); } } diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h index fa1318bac06c..99022620457a 100644 --- a/include/linux/stmmac.h +++ b/include/linux/stmmac.h @@ -171,7 +171,6 @@ struct dwmac4_addrs { u32 mtl_low_cred_offset; }; -#define STMMAC_FLAG_HAS_INTEGRATED_PCS BIT(0) #define STMMAC_FLAG_SPH_DISABLE BIT(1) #define STMMAC_FLAG_USE_PHY_WOL BIT(2) #define STMMAC_FLAG_HAS_SUN8I BIT(3) -- 2.47.3 As a result of the previous commit, the pcs_link, pcs_duplex and pcs_speed members are not used outside of the interrupt handling code, and are only used to print their status using the misleading "Link is" messages that bear no relation to the actual status of the link. Remove the printing of these messages, these members, and the code that decodes them from the hardware. Signed-off-by: Russell King (Oracle) --- drivers/net/ethernet/stmicro/stmmac/common.h | 3 -- .../ethernet/stmicro/stmmac/dwmac1000_core.c | 28 +------------------ .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 28 +------------------ 3 files changed, 2 insertions(+), 57 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h index eaa1f2e1c5a5..8ff3406cdfbf 100644 --- a/drivers/net/ethernet/stmicro/stmmac/common.h +++ b/drivers/net/ethernet/stmicro/stmmac/common.h @@ -192,9 +192,6 @@ struct stmmac_extra_stats { unsigned long irq_pcs_ane_n; unsigned long irq_pcs_link_n; unsigned long irq_rgmii_n; - unsigned long pcs_link; - unsigned long pcs_duplex; - unsigned long pcs_speed; /* debug register */ unsigned long mtl_tx_status_fifo_full; unsigned long mtl_tx_fifo_not_empty; diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c index fe776ddf6889..2c5ee59c3208 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c @@ -266,34 +266,8 @@ static void dwmac1000_pmt(struct mac_device_info *hw, unsigned long mode) /* RGMII or SMII interface */ static void dwmac1000_rgsmii(void __iomem *ioaddr, struct stmmac_extra_stats *x) { - u32 status; - - status = readl(ioaddr + GMAC_RGSMIIIS); + readl(ioaddr + GMAC_RGSMIIIS); x->irq_rgmii_n++; - - /* Check the link status */ - if (status & GMAC_RGSMIIIS_LNKSTS) { - int speed_value; - - x->pcs_link = 1; - - speed_value = ((status & GMAC_RGSMIIIS_SPEED) >> - GMAC_RGSMIIIS_SPEED_SHIFT); - if (speed_value == GMAC_RGSMIIIS_SPEED_125) - x->pcs_speed = SPEED_1000; - else if (speed_value == GMAC_RGSMIIIS_SPEED_25) - x->pcs_speed = SPEED_100; - else - x->pcs_speed = SPEED_10; - - x->pcs_duplex = (status & GMAC_RGSMIIIS_LNKMOD_MASK); - - pr_info("Link is Up - %d/%s\n", (int)x->pcs_speed, - x->pcs_duplex ? "Full" : "Half"); - } else { - x->pcs_link = 0; - pr_info("Link is Down\n"); - } } static int dwmac1000_irq_status(struct mac_device_info *hw, diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c index d85bc0bb5c3c..8a19df7b0577 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c @@ -592,34 +592,8 @@ static void dwmac4_ctrl_ane(struct stmmac_priv *priv, bool ane, bool srgmi_ral, /* RGMII or SMII interface */ static void dwmac4_phystatus(void __iomem *ioaddr, struct stmmac_extra_stats *x) { - u32 status; - - status = readl(ioaddr + GMAC_PHYIF_CONTROL_STATUS); + readl(ioaddr + GMAC_PHYIF_CONTROL_STATUS); x->irq_rgmii_n++; - - /* Check the link status */ - if (status & GMAC_PHYIF_CTRLSTATUS_LNKSTS) { - int speed_value; - - x->pcs_link = 1; - - speed_value = ((status & GMAC_PHYIF_CTRLSTATUS_SPEED) >> - GMAC_PHYIF_CTRLSTATUS_SPEED_SHIFT); - if (speed_value == GMAC_PHYIF_CTRLSTATUS_SPEED_125) - x->pcs_speed = SPEED_1000; - else if (speed_value == GMAC_PHYIF_CTRLSTATUS_SPEED_25) - x->pcs_speed = SPEED_100; - else - x->pcs_speed = SPEED_10; - - x->pcs_duplex = (status & GMAC_PHYIF_CTRLSTATUS_LNKMOD); - - pr_info("Link is Up - %d/%s\n", (int)x->pcs_speed, - x->pcs_duplex ? "Full" : "Half"); - } else { - x->pcs_link = 0; - pr_info("Link is Down\n"); - } } static int dwmac4_irq_mtl_status(struct stmmac_priv *priv, -- 2.47.3 Now that the only use for the interrupt is to clear it and increment a statistic counter (which is not that relevant anymore) remove all this code and ensure that the interrupt remains disabled to avoid a stuck interrupt. Signed-off-by: Russell King (Oracle) --- drivers/net/ethernet/stmicro/stmmac/dwmac1000.h | 6 +++--- drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 10 ---------- drivers/net/ethernet/stmicro/stmmac/dwmac4.h | 3 +-- drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 9 --------- 4 files changed, 4 insertions(+), 24 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h b/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h index 0c011a47d5a3..8f3002d9de78 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h @@ -38,10 +38,10 @@ #define GMAC_INT_DISABLE_PCSAN BIT(2) #define GMAC_INT_DISABLE_PMT BIT(3) #define GMAC_INT_DISABLE_TIMESTAMP BIT(9) -#define GMAC_INT_DISABLE_PCS (GMAC_INT_DISABLE_RGMII | \ - GMAC_INT_DISABLE_PCSLINK | \ +#define GMAC_INT_DISABLE_PCS (GMAC_INT_DISABLE_PCSLINK | \ GMAC_INT_DISABLE_PCSAN) -#define GMAC_INT_DEFAULT_MASK (GMAC_INT_DISABLE_TIMESTAMP | \ +#define GMAC_INT_DEFAULT_MASK (GMAC_INT_DISABLE_RGMII | \ + GMAC_INT_DISABLE_TIMESTAMP | \ GMAC_INT_DISABLE_PCS) /* PMT Control and Status */ diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c index 2c5ee59c3208..654331b411f4 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c @@ -263,13 +263,6 @@ static void dwmac1000_pmt(struct mac_device_info *hw, unsigned long mode) writel(pmt, ioaddr + GMAC_PMT); } -/* RGMII or SMII interface */ -static void dwmac1000_rgsmii(void __iomem *ioaddr, struct stmmac_extra_stats *x) -{ - readl(ioaddr + GMAC_RGSMIIIS); - x->irq_rgmii_n++; -} - static int dwmac1000_irq_status(struct mac_device_info *hw, struct stmmac_extra_stats *x) { @@ -311,9 +304,6 @@ static int dwmac1000_irq_status(struct mac_device_info *hw, dwmac_pcs_isr(ioaddr, GMAC_PCS_BASE, intr_status, x); - if (intr_status & PCS_RGSMIIIS_IRQ) - dwmac1000_rgsmii(ioaddr, x); - return ret; } diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h index 3dec1a264cf6..6dd84b6544cc 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h @@ -106,8 +106,7 @@ #define GMAC_INT_LPI_EN BIT(5) #define GMAC_INT_TSIE BIT(12) -#define GMAC_PCS_IRQ_DEFAULT (GMAC_INT_RGSMIIS | GMAC_INT_PCS_LINK | \ - GMAC_INT_PCS_ANE) +#define GMAC_PCS_IRQ_DEFAULT (GMAC_INT_PCS_LINK | GMAC_INT_PCS_ANE) #define GMAC_INT_DEFAULT_ENABLE (GMAC_INT_PMT_EN | GMAC_INT_LPI_EN | \ GMAC_INT_TSIE) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c index 8a19df7b0577..bff4c371c1d2 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c @@ -589,13 +589,6 @@ static void dwmac4_ctrl_ane(struct stmmac_priv *priv, bool ane, bool srgmi_ral, dwmac_ctrl_ane(priv->ioaddr, GMAC_PCS_BASE, ane, srgmi_ral, loopback); } -/* RGMII or SMII interface */ -static void dwmac4_phystatus(void __iomem *ioaddr, struct stmmac_extra_stats *x) -{ - readl(ioaddr + GMAC_PHYIF_CONTROL_STATUS); - x->irq_rgmii_n++; -} - static int dwmac4_irq_mtl_status(struct stmmac_priv *priv, struct mac_device_info *hw, u32 chan) { @@ -667,8 +660,6 @@ static int dwmac4_irq_status(struct mac_device_info *hw, } dwmac_pcs_isr(ioaddr, GMAC_PCS_BASE, intr_status, x); - if (intr_status & PCS_RGSMIIIS_IRQ) - dwmac4_phystatus(ioaddr, x); return ret; } -- 2.47.3 Remove the "we always autoneg pause" forcing when the stmmac driver decides that a "PCS" is present, which blocks passing the ethtool pause calls to phylink when using SGMII mode. This prevents the pause results being reported when a PHY is attached using SGMII mode, or the pause settings being changed in SGMII mode. There is no reason to prevent this. Signed-off-by: Russell King (Oracle) --- .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c index d89662b48087..c60cd948311e 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c @@ -424,11 +424,7 @@ stmmac_get_pauseparam(struct net_device *netdev, { struct stmmac_priv *priv = netdev_priv(netdev); - if (priv->hw->pcs) { - pause->autoneg = 1; - } else { - phylink_ethtool_get_pauseparam(priv->phylink, pause); - } + phylink_ethtool_get_pauseparam(priv->phylink, pause); } static int @@ -437,12 +433,7 @@ stmmac_set_pauseparam(struct net_device *netdev, { struct stmmac_priv *priv = netdev_priv(netdev); - if (priv->hw->pcs) { - pause->autoneg = 1; - return 0; - } else { - return phylink_ethtool_set_pauseparam(priv->phylink, pause); - } + return phylink_ethtool_set_pauseparam(priv->phylink, pause); } static u64 stmmac_get_rx_normal_irq_n(struct stmmac_priv *priv, int q) -- 2.47.3 Nothing calls stmmac_pcs_ctrl_ane() with the "loopback" argument set to anything except zero, so this serves no useful purpose. Remove the argument to reduce the code complexity. Signed-off-by: Russell King (Oracle) --- drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 2 +- drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 4 ++-- drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 5 ++--- drivers/net/ethernet/stmicro/stmmac/hwif.h | 4 ++-- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h | 6 +----- 6 files changed, 9 insertions(+), 14 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c index f62825220cf7..32244217d952 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c @@ -622,7 +622,7 @@ static void ethqos_set_serdes_speed(struct qcom_ethqos *ethqos, int speed) static void ethqos_pcs_set_inband(struct stmmac_priv *priv, bool enable) { - stmmac_pcs_ctrl_ane(priv, enable, 0, 0); + stmmac_pcs_ctrl_ane(priv, enable, 0); } /* On interface toggle MAC registers gets reset. diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c index 654331b411f4..5c653be3d453 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c @@ -358,9 +358,9 @@ static void dwmac1000_set_eee_timer(struct mac_device_info *hw, int ls, int tw) } static void dwmac1000_ctrl_ane(struct stmmac_priv *priv, bool ane, - bool srgmi_ral, bool loopback) + bool srgmi_ral) { - dwmac_ctrl_ane(priv->ioaddr, GMAC_PCS_BASE, ane, srgmi_ral, loopback); + dwmac_ctrl_ane(priv->ioaddr, GMAC_PCS_BASE, ane, srgmi_ral); } static void dwmac1000_debug(struct stmmac_priv *priv, void __iomem *ioaddr, diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c index bff4c371c1d2..21e4461db937 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c @@ -583,10 +583,9 @@ static void dwmac4_flow_ctrl(struct mac_device_info *hw, unsigned int duplex, } } -static void dwmac4_ctrl_ane(struct stmmac_priv *priv, bool ane, bool srgmi_ral, - bool loopback) +static void dwmac4_ctrl_ane(struct stmmac_priv *priv, bool ane, bool srgmi_ral) { - dwmac_ctrl_ane(priv->ioaddr, GMAC_PCS_BASE, ane, srgmi_ral, loopback); + dwmac_ctrl_ane(priv->ioaddr, GMAC_PCS_BASE, ane, srgmi_ral); } static int dwmac4_irq_mtl_status(struct stmmac_priv *priv, diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h index 14dbe0685997..7796f5f3c96f 100644 --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h @@ -374,8 +374,8 @@ struct stmmac_ops { struct stmmac_extra_stats *x, u32 rx_queues, u32 tx_queues); /* PCS calls */ - void (*pcs_ctrl_ane)(struct stmmac_priv *priv, bool ane, bool srgmi_ral, - bool loopback); + void (*pcs_ctrl_ane)(struct stmmac_priv *priv, bool ane, + bool srgmi_ral); /* Safety Features */ int (*safety_feat_config)(void __iomem *ioaddr, unsigned int asp, struct stmmac_safety_feature_cfg *safety_cfg); diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index d5e345c6f6eb..05eb4b5fbf04 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -3494,7 +3494,7 @@ static int stmmac_hw_setup(struct net_device *dev) } if (priv->hw->pcs) - stmmac_pcs_ctrl_ane(priv, 1, priv->hw->ps, 0); + stmmac_pcs_ctrl_ane(priv, 1, priv->hw->ps); /* set TX and RX rings length */ stmmac_set_rings_length(priv); diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h index 4a684c97dfae..5778f5b2f313 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h @@ -82,13 +82,12 @@ static inline void dwmac_pcs_isr(void __iomem *ioaddr, u32 reg, * @reg: Base address of the AN Control Register. * @ane: to enable the auto-negotiation * @srgmi_ral: to manage MAC-2-MAC SGMII connections. - * @loopback: to cause the PHY to loopback tx data into rx path. * Description: this is the main function to configure the AN control register * and init the ANE, select loopback (usually for debugging purpose) and * configure SGMII RAL. */ static inline void dwmac_ctrl_ane(void __iomem *ioaddr, u32 reg, bool ane, - bool srgmi_ral, bool loopback) + bool srgmi_ral) { u32 value = readl(ioaddr + GMAC_AN_CTRL(reg)); @@ -104,9 +103,6 @@ static inline void dwmac_ctrl_ane(void __iomem *ioaddr, u32 reg, bool ane, if (srgmi_ral) value |= GMAC_AN_CTRL_SGMRAL; - if (loopback) - value |= GMAC_AN_CTRL_ELE; - writel(value, ioaddr + GMAC_AN_CTRL(reg)); } #endif /* __STMMAC_PCS_H__ */ -- 2.47.3 After a lot of digging, it seems that the oddly named hw->ps member is all about configuring the core for reverse SGMII. This member is set to one of 0, SPEED_10, SPEED_100 or SPEED_1000 depending on priv->plat->mac_port_sel_speed. On DT systems, this comes from the "snps,ps-speed" DT property. When set to a non-zero value, it: 1. Configures the MAC at initialisation time to operate at a specific speed. However, this will be overwritten by mac_link_up() when the link comes up (e.g. with the fixed-link parameters.) Note that dwxgmac2 wants to also support SPEED_2500 and SPEED_10000, but both these values are impossible. 2. It _incorrectly_ enables the transmitter (GMAC_CONFIG_TE) which makes no sense, rather than enabling the "transmit configuration" bit (GMAC_CONFIG_TC). Likely a typo. 3. It configures the SGMII rate adapter layer to retrieve its speed setting from the MAC configuration register rather than the PHY. There are two ways forward here: a) fixing (2) so that we set GMAC_CONFIG_TC. However, we have platform that set the "snps,ps-speed" property and that work today. Fixing this will cause the RGMII, SGMII or SMII inband configuration to be transmitted, which will be a functional change which could cause a regression. b) ripping out (1) and (2) as they are ineffective. This also has the possibility of regressions, but the patch author believes this risk is much lower than (a). Therefore, this commit takes the approach in (b). Signed-off-by: Russell King (Oracle) --- .../ethernet/stmicro/stmmac/dwmac1000_core.c | 23 +++-------------- .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 24 +++--------------- .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 25 ++----------------- 3 files changed, 8 insertions(+), 64 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c index 5c653be3d453..d35db8958be1 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c @@ -26,35 +26,18 @@ static void dwmac1000_core_init(struct mac_device_info *hw, struct net_device *dev) { void __iomem *ioaddr = hw->pcsr; - u32 value = readl(ioaddr + GMAC_CONTROL); int mtu = dev->mtu; + u32 value; /* Configure GMAC core */ - value |= GMAC_CORE_INIT; + value = readl(ioaddr + GMAC_CONTROL); if (mtu > 1500) value |= GMAC_CONTROL_2K; if (mtu > 2000) value |= GMAC_CONTROL_JE; - if (hw->ps) { - value |= GMAC_CONTROL_TE; - - value &= ~hw->link.speed_mask; - switch (hw->ps) { - case SPEED_1000: - value |= hw->link.speed1000; - break; - case SPEED_100: - value |= hw->link.speed100; - break; - case SPEED_10: - value |= hw->link.speed10; - break; - } - } - - writel(value, ioaddr + GMAC_CONTROL); + writel(value | GMAC_CORE_INIT, ioaddr + GMAC_CONTROL); /* Mask GMAC interrupts */ value = GMAC_INT_DEFAULT_MASK; diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c index 21e4461db937..d855ab6b9145 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c @@ -27,29 +27,11 @@ static void dwmac4_core_init(struct mac_device_info *hw, { struct stmmac_priv *priv = netdev_priv(dev); void __iomem *ioaddr = hw->pcsr; - u32 value = readl(ioaddr + GMAC_CONFIG); unsigned long clk_rate; + u32 value; - value |= GMAC_CORE_INIT; - - if (hw->ps) { - value |= GMAC_CONFIG_TE; - - value &= hw->link.speed_mask; - switch (hw->ps) { - case SPEED_1000: - value |= hw->link.speed1000; - break; - case SPEED_100: - value |= hw->link.speed100; - break; - case SPEED_10: - value |= hw->link.speed10; - break; - } - } - - writel(value, ioaddr + GMAC_CONFIG); + value = readl(ioaddr + GMAC_CONFIG); + writel(value | GMAC_CORE_INIT, ioaddr + GMAC_CONFIG); /* Configure LPI 1us counter to number of CSR clock ticks in 1us - 1 */ clk_rate = clk_get_rate(priv->plat->stmmac_clk); diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c index 00e929bf280b..0430af27da40 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c @@ -23,29 +23,8 @@ static void dwxgmac2_core_init(struct mac_device_info *hw, tx = readl(ioaddr + XGMAC_TX_CONFIG); rx = readl(ioaddr + XGMAC_RX_CONFIG); - tx |= XGMAC_CORE_INIT_TX; - rx |= XGMAC_CORE_INIT_RX; - - if (hw->ps) { - tx |= XGMAC_CONFIG_TE; - tx &= ~hw->link.speed_mask; - - switch (hw->ps) { - case SPEED_10000: - tx |= hw->link.xgmii.speed10000; - break; - case SPEED_2500: - tx |= hw->link.speed2500; - break; - case SPEED_1000: - default: - tx |= hw->link.speed1000; - break; - } - } - - writel(tx, ioaddr + XGMAC_TX_CONFIG); - writel(rx, ioaddr + XGMAC_RX_CONFIG); + writel(tx | XGMAC_CORE_INIT_TX, ioaddr + XGMAC_TX_CONFIG); + writel(rx | XGMAC_CORE_INIT_RX, ioaddr + XGMAC_RX_CONFIG); writel(XGMAC_INT_DEFAULT_EN, ioaddr + XGMAC_INT_EN); } -- 2.47.3 Remove the RGMII "pcs" code in stmmac_check_pcs_mode() due to: 1) This should never have been conditional on a PCS being present, as when a core is synthesised using only RGMII, the PCS won't be present and priv->dma_cap.pcs will be false. Only multi-interface cores which have a PCS present would have detected RGMII. 2) STMMAC_PCS_RGMII has no effect since the broken netif_carrier and ethtool code was removed. Signed-off-by: Russell King (Oracle) --- drivers/net/ethernet/stmicro/stmmac/common.h | 1 - drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 +++----------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h index 8ff3406cdfbf..6c152be9ff5f 100644 --- a/drivers/net/ethernet/stmicro/stmmac/common.h +++ b/drivers/net/ethernet/stmicro/stmmac/common.h @@ -269,7 +269,6 @@ struct stmmac_safety_stats { #define FLOW_AUTO (FLOW_TX | FLOW_RX) /* PCS defines */ -#define STMMAC_PCS_RGMII (1 << 0) #define STMMAC_PCS_SGMII (1 << 1) #define SF_DMA_MODE 1 /* DMA STORE-AND-FORWARD Operation Mode */ diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 05eb4b5fbf04..f90742ab68ae 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -1087,17 +1087,9 @@ static void stmmac_check_pcs_mode(struct stmmac_priv *priv) { int interface = priv->plat->phy_interface; - if (priv->dma_cap.pcs) { - if ((interface == PHY_INTERFACE_MODE_RGMII) || - (interface == PHY_INTERFACE_MODE_RGMII_ID) || - (interface == PHY_INTERFACE_MODE_RGMII_RXID) || - (interface == PHY_INTERFACE_MODE_RGMII_TXID)) { - netdev_dbg(priv->dev, "PCS RGMII support enabled\n"); - priv->hw->pcs = STMMAC_PCS_RGMII; - } else if (interface == PHY_INTERFACE_MODE_SGMII) { - netdev_dbg(priv->dev, "PCS SGMII support enabled\n"); - priv->hw->pcs = STMMAC_PCS_SGMII; - } + 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; } } -- 2.47.3 The broken reverse-mode, selected by snps,ps-speed, is configured when the platform provides a valid port speed and a PCS is being used. Both these remain constant after the driver has probed, so the software state doesn't need to be re-initialised each time stmmac_hw_setup() is called (which is called at open and resume time.) Move the software setup of reverse-mode to stmmac_check_pcs_mode() which is called from the driver probe function. Signed-off-by: Russell King (Oracle) --- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index f90742ab68ae..414a00ab5012 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -1091,6 +1091,19 @@ static void stmmac_check_pcs_mode(struct stmmac_priv *priv) netdev_dbg(priv->dev, "PCS SGMII support enabled\n"); priv->hw->pcs = STMMAC_PCS_SGMII; } + + /* PS and related bits will be programmed according to the speed */ + if (priv->hw->pcs) { + int speed = priv->plat->mac_port_sel_speed; + + if ((speed == SPEED_10) || (speed == SPEED_100) || + (speed == SPEED_1000)) { + priv->hw->ps = speed; + } else { + dev_warn(priv->device, "invalid port speed\n"); + priv->hw->ps = 0; + } + } } /** @@ -3436,19 +3449,6 @@ static int stmmac_hw_setup(struct net_device *dev) stmmac_set_umac_addr(priv, priv->hw, dev->dev_addr, 0); phylink_rx_clk_stop_unblock(priv->phylink); - /* PS and related bits will be programmed according to the speed */ - if (priv->hw->pcs) { - int speed = priv->plat->mac_port_sel_speed; - - if ((speed == SPEED_10) || (speed == SPEED_100) || - (speed == SPEED_1000)) { - priv->hw->ps = speed; - } else { - dev_warn(priv->device, "invalid port speed\n"); - priv->hw->ps = 0; - } - } - /* Initialize the MAC Core */ stmmac_core_init(priv, priv->hw, dev); -- 2.47.3 Now that we only support one mode, simplify stmmac_check_pcs_mode(). Signed-off-by: Russell King (Oracle) --- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 414a00ab5012..94b615444734 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -1086,22 +1086,23 @@ static const struct phylink_mac_ops stmmac_phylink_mac_ops = { static void stmmac_check_pcs_mode(struct stmmac_priv *priv) { int interface = priv->plat->phy_interface; + int speed = priv->plat->mac_port_sel_speed; 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; - } - - /* PS and related bits will be programmed according to the speed */ - if (priv->hw->pcs) { - int speed = priv->plat->mac_port_sel_speed; - if ((speed == SPEED_10) || (speed == SPEED_100) || - (speed == SPEED_1000)) { + switch (speed) { + case SPEED_10: + case SPEED_100: + case SPEED_1000: priv->hw->ps = speed; - } else { + break; + + default: dev_warn(priv->device, "invalid port speed\n"); priv->hw->ps = 0; + break; } } } -- 2.47.3 After a lot of digging, it seems that the oddly named hw->ps member is all about setting the core into reverse SGMII speed. When set to a non-zero value, it: 1. Configures the MAC at initialisation time to operate at a specific speed. 2. It _incorrectly_ enables the transmitter (GMAC_CONFIG_TE) which makes no sense, rather than enabling the "transmit configuration" bit (GMAC_CONFIG_TC). 3. It configures the SGMII rate adapter layer to retrieve its speed setting from the MAC configuration register rather than the PHY. In the previous commit, we removed (1) and (2) as phylink overwrites the configuration set at that step. Thus, the only functional aspect is (3), which is a boolean operation. This means there is no need to store the actual speed, and just have a boolean flag. Convert the priv->ps member to a boolean, and rename it to priv->reverse_sgmii_enable to make it more understandable. Signed-off-by: Russell King (Oracle) --- drivers/net/ethernet/stmicro/stmmac/common.h | 2 +- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h index 6c152be9ff5f..9758d768fd0a 100644 --- a/drivers/net/ethernet/stmicro/stmmac/common.h +++ b/drivers/net/ethernet/stmicro/stmmac/common.h @@ -598,13 +598,13 @@ struct mac_device_info { unsigned int mcast_bits_log2; unsigned int rx_csum; unsigned int pcs; - unsigned int ps; unsigned int xlgmac; unsigned int num_vlan; u32 vlan_filter[32]; bool vlan_fail_q_en; u8 vlan_fail_q; bool hw_vlan_en; + bool reverse_sgmii_enable; }; struct stmmac_rx_routing { diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 94b615444734..60fcf3feac31 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -1096,12 +1096,12 @@ static void stmmac_check_pcs_mode(struct stmmac_priv *priv) case SPEED_10: case SPEED_100: case SPEED_1000: - priv->hw->ps = speed; + priv->hw->reverse_sgmii_enable = true; break; default: dev_warn(priv->device, "invalid port speed\n"); - priv->hw->ps = 0; + priv->hw->reverse_sgmii_enable = false; break; } } @@ -3487,7 +3487,7 @@ static int stmmac_hw_setup(struct net_device *dev) } if (priv->hw->pcs) - stmmac_pcs_ctrl_ane(priv, 1, priv->hw->ps); + stmmac_pcs_ctrl_ane(priv, 1, priv->hw->reverse_sgmii_enable); /* set TX and RX rings length */ stmmac_set_rings_length(priv); -- 2.47.3 SGMII mode does not require port-speed to be specified; this only switches SGMII to use the MAC configuration register speed settings and the actual value is irrelevant when the link comes up. As it seems the intention was to support "reverse SGMII" with this setting, but the code didn't actually configure that due to a typo, the warning and bad DT binding documentation has led people to specify snps,ps-speed in their DT files inappropriately. If mac_port_sel_speed is zero, then don't complain that the speed is invalid, as this means we're using "normal" SGMII. Signed-off-by: Russell King (Oracle) --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 60fcf3feac31..e2d9c08aace8 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -1101,6 +1101,8 @@ static void stmmac_check_pcs_mode(struct stmmac_priv *priv) default: dev_warn(priv->device, "invalid port speed\n"); + fallthrough; + case 0: priv->hw->reverse_sgmii_enable = false; break; } -- 2.47.3 The internal PCS registers only exist if the core is synthesized with SGMII, TBI or RTBI support. They have no relevance for RGMII. However, priv->hw->pcs contains a STMMAC_PCS_RGMII flag, which is set if a PCS has been synthesized but we are operating in RGMII mode. As the register has no effect for RGMII, there is no point calling stmmac_pcs_ctrl_ane() in this case. Add a comment describing this and make it conditional on STMMAC_PCS_SGMII. Signed-off-by: Russell King (Oracle) --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index e2d9c08aace8..ee48991d336d 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -3488,7 +3488,11 @@ static int stmmac_hw_setup(struct net_device *dev) } } - if (priv->hw->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) stmmac_pcs_ctrl_ane(priv, 1, priv->hw->reverse_sgmii_enable); /* set TX and RX rings length */ -- 2.47.3 Now that stmmac's PCS support is much more simple - just a matter of configuring the control register - the basic conversion to phylink PCS support becomes straight forward. Create the infrastructure to setup a phylink_pcs instance for the integrated PCS: - add a struct stmmac_pcs to encapsulate the phylink_pcs structure, pointer to stmmac_priv, and the core-specific base address of the PCS registers. - modify stmmac_priv and stmmac_mac_select_pcs() to return the embedded phylink_pcs structure when setup and STMMAC_PCS_SGMII is in use, and move the comment from stmmac_hw_setup() to here. - create stmmac_pcs.c, which contains the phylink_pcs_ops structure, a dummy .pcs_get_state() method which always reports link-down, and .pcs_config() method, moving the call to stmmac_pcs_ctrl_ane() here, but without indirecting through the dwmac specific core code. This will ensure that the PCS control register is configured to the same settings as before, but will now happen when the netdev is opened or reusmed rather than only during probe time. However, this will be before the .fix_mac_speed() method is called, which is critical for the behaviour in dwmac-qcom-ethqos's ethqos_configure_sgmii() function to be maintained. Signed-off-by: Russell King (Oracle) --- drivers/net/ethernet/stmicro/stmmac/Makefile | 2 +- .../ethernet/stmicro/stmmac/dwmac1000_core.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 2 +- drivers/net/ethernet/stmicro/stmmac/stmmac.h | 4 ++ .../net/ethernet/stmicro/stmmac/stmmac_main.c | 15 +++--- .../net/ethernet/stmicro/stmmac/stmmac_pcs.c | 47 +++++++++++++++++++ .../net/ethernet/stmicro/stmmac/stmmac_pcs.h | 17 +++++++ 7 files changed, 79 insertions(+), 10 deletions(-) create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile index b591d93f8503..0390d33b4413 100644 --- a/drivers/net/ethernet/stmicro/stmmac/Makefile +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile @@ -7,7 +7,7 @@ stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o ring_mode.o \ dwmac4_dma.o dwmac4_lib.o dwmac4_core.o dwmac5.o hwif.o \ stmmac_tc.o dwxgmac2_core.o dwxgmac2_dma.o dwxgmac2_descs.o \ stmmac_xdp.o stmmac_est.o stmmac_fpe.o stmmac_vlan.o \ - $(stmmac-y) + stmmac_pcs.o $(stmmac-y) stmmac-$(CONFIG_STMMAC_SELFTESTS) += stmmac_selftests.o diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c index d35db8958be1..b01c0fc822f9 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c @@ -484,7 +484,7 @@ int dwmac1000_setup(struct stmmac_priv *priv) mac->mii.clk_csr_shift = 2; mac->mii.clk_csr_mask = GENMASK(5, 2); - return 0; + return stmmac_integrated_pcs_init(priv, GMAC_PCS_BASE); } /* DWMAC 1000 HW Timestaming ops */ diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c index d855ab6b9145..688e45b440dd 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c @@ -1017,5 +1017,5 @@ int dwmac4_setup(struct stmmac_priv *priv) mac->mii.clk_csr_mask = GENMASK(11, 8); mac->num_vlan = stmmac_get_num_vlan(priv->ioaddr); - return 0; + return stmmac_integrated_pcs_init(priv, GMAC_PCS_BASE); } diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h index 7ca5477be390..34f62993a4da 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h @@ -25,6 +25,8 @@ #include #include +struct stmmac_pcs; + struct stmmac_resources { void __iomem *addr; u8 mac[ETH_ALEN]; @@ -273,6 +275,8 @@ struct stmmac_priv { unsigned int pause_time; struct mii_bus *mii; + struct stmmac_pcs *integrated_pcs; + struct phylink_config phylink_config; struct phylink *phylink; diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index ee48991d336d..2cf6e69f3303 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -46,6 +46,7 @@ #include "stmmac_ptp.h" #include "stmmac_fpe.h" #include "stmmac.h" +#include "stmmac_pcs.h" #include "stmmac_xdp.h" #include #include @@ -850,6 +851,13 @@ 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) + return &priv->integrated_pcs->pcs; + return NULL; } @@ -3488,13 +3496,6 @@ static int stmmac_hw_setup(struct net_device *dev) } } - /* 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) - stmmac_pcs_ctrl_ane(priv, 1, priv->hw->reverse_sgmii_enable); - /* set TX and RX rings length */ stmmac_set_rings_length(priv); diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c new file mode 100644 index 000000000000..50ea51d7a1cc --- /dev/null +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c @@ -0,0 +1,47 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include "stmmac.h" +#include "stmmac_pcs.h" + +static void dwmac_integrated_pcs_get_state(struct phylink_pcs *pcs, + unsigned int neg_mode, + struct phylink_link_state *state) +{ + state->link = false; +} + +static int dwmac_integrated_pcs_config(struct phylink_pcs *pcs, + unsigned int neg_mode, + phy_interface_t interface, + const unsigned long *advertising, + bool permit_pause_to_mac) +{ + struct stmmac_pcs *spcs = phylink_pcs_to_stmmac_pcs(pcs); + + dwmac_ctrl_ane(spcs->base, 0, 1, spcs->priv->hw->reverse_sgmii_enable); + + return 0; +} + +static const struct phylink_pcs_ops dwmac_integrated_pcs_ops = { + .pcs_get_state = dwmac_integrated_pcs_get_state, + .pcs_config = dwmac_integrated_pcs_config, +}; + +int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset) +{ + struct stmmac_pcs *spcs; + + spcs = devm_kzalloc(priv->device, sizeof(*spcs), GFP_KERNEL); + if (!spcs) + return -ENOMEM; + + spcs->priv = priv; + spcs->base = priv->ioaddr + offset; + spcs->pcs.ops = &dwmac_integrated_pcs_ops; + + __set_bit(PHY_INTERFACE_MODE_SGMII, spcs->pcs.supported_interfaces); + + priv->integrated_pcs = spcs; + + return 0; +} diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h index 5778f5b2f313..64397ac8ecab 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h @@ -9,6 +9,7 @@ #ifndef __STMMAC_PCS_H__ #define __STMMAC_PCS_H__ +#include #include #include #include "common.h" @@ -46,6 +47,22 @@ #define GMAC_ANE_RFE_SHIFT 12 #define GMAC_ANE_ACK BIT(14) +struct stmmac_priv; + +struct stmmac_pcs { + struct stmmac_priv *priv; + void __iomem *base; + struct phylink_pcs pcs; +}; + +static inline struct stmmac_pcs * +phylink_pcs_to_stmmac_pcs(struct phylink_pcs *pcs) +{ + return container_of(pcs, struct stmmac_pcs, pcs); +} + +int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset); + /** * dwmac_pcs_isr - TBI, RTBI, or SGMII PHY ISR * @ioaddr: IO registers pointer -- 2.47.3 dwmac1000 and dwmac4 both provide a hardware feature bit to indicate when the PCS block is present. There is little point initialising the PCS support when the hardware is not present. Add a new callback which will be made after the hardware features have been read, and heck whether the PCS is present. Signed-off-by: Russell King (Oracle) --- .../net/ethernet/stmicro/stmmac/dwmac1000_core.c | 11 ++++++++++- drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 13 ++++++++++++- drivers/net/ethernet/stmicro/stmmac/hwif.h | 4 ++++ drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++++ 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c index b01c0fc822f9..571e48362444 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c @@ -22,6 +22,14 @@ #include "stmmac_ptp.h" #include "dwmac1000.h" +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); +} + static void dwmac1000_core_init(struct mac_device_info *hw, struct net_device *dev) { @@ -435,6 +443,7 @@ static void dwmac1000_set_mac_loopback(void __iomem *ioaddr, bool enable) } const struct stmmac_ops dwmac1000_ops = { + .pcs_init = dwmac1000_pcs_init, .core_init = dwmac1000_core_init, .set_mac = stmmac_set_mac, .rx_ipc = dwmac1000_rx_ipc_enable, @@ -484,7 +493,7 @@ int dwmac1000_setup(struct stmmac_priv *priv) mac->mii.clk_csr_shift = 2; mac->mii.clk_csr_mask = GENMASK(5, 2); - return stmmac_integrated_pcs_init(priv, GMAC_PCS_BASE); + return 0; } /* DWMAC 1000 HW Timestaming ops */ diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c index 688e45b440dd..0b785389b7ef 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c @@ -22,6 +22,14 @@ #include "dwmac4.h" #include "dwmac5.h" +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); +} + static void dwmac4_core_init(struct mac_device_info *hw, struct net_device *dev) { @@ -875,6 +883,7 @@ static int dwmac4_config_l4_filter(struct mac_device_info *hw, u32 filter_no, } const struct stmmac_ops dwmac4_ops = { + .pcs_init = dwmac4_pcs_init, .core_init = dwmac4_core_init, .update_caps = dwmac4_update_caps, .set_mac = stmmac_set_mac, @@ -909,6 +918,7 @@ const struct stmmac_ops dwmac4_ops = { }; const struct stmmac_ops dwmac410_ops = { + .pcs_init = dwmac4_pcs_init, .core_init = dwmac4_core_init, .update_caps = dwmac4_update_caps, .set_mac = stmmac_dwmac4_set_mac, @@ -945,6 +955,7 @@ const struct stmmac_ops dwmac410_ops = { }; const struct stmmac_ops dwmac510_ops = { + .pcs_init = dwmac4_pcs_init, .core_init = dwmac4_core_init, .update_caps = dwmac4_update_caps, .set_mac = stmmac_dwmac4_set_mac, @@ -1017,5 +1028,5 @@ int dwmac4_setup(struct stmmac_priv *priv) mac->mii.clk_csr_mask = GENMASK(11, 8); mac->num_vlan = stmmac_get_num_vlan(priv->ioaddr); - return stmmac_integrated_pcs_init(priv, GMAC_PCS_BASE); + return 0; } diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h index 7796f5f3c96f..82cfb6bec334 100644 --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h @@ -313,6 +313,8 @@ enum stmmac_lpi_mode { /* Helpers to program the MAC core */ struct stmmac_ops { + /* Initialise any PCS instances */ + int (*pcs_init)(struct stmmac_priv *priv); /* MAC core initialization */ void (*core_init)(struct mac_device_info *hw, struct net_device *dev); /* Update MAC capabilities */ @@ -413,6 +415,8 @@ struct stmmac_ops { u32 pclass); }; +#define stmmac_mac_pcs_init(__priv) \ + stmmac_do_callback(__priv, mac, pcs_init, __priv) #define stmmac_core_init(__priv, __args...) \ stmmac_do_void_callback(__priv, mac, core_init, __args) #define stmmac_mac_update_caps(__priv) \ diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 2cf6e69f3303..0f243f207f99 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -7240,6 +7240,10 @@ static int stmmac_hw_init(struct stmmac_priv *priv) "Enable RX Mitigation via HW Watchdog Timer\n"); } + ret = stmmac_mac_pcs_init(priv); + if (ret != -EINVAL) + return ret; + return 0; } -- 2.47.3 Add a function to allow interrupts to be enabled and disabled in a core independent manner. Signed-off-by: Russell King (Oracle) --- drivers/net/ethernet/stmicro/stmmac/common.h | 5 +++++ .../ethernet/stmicro/stmmac/dwmac1000_core.c | 15 +++++++++++++++ .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 17 +++++++++++++++++ .../net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 16 ++++++++++++++++ drivers/net/ethernet/stmicro/stmmac/hwif.c | 2 ++ drivers/net/ethernet/stmicro/stmmac/hwif.h | 4 ++++ .../net/ethernet/stmicro/stmmac/stmmac_fpe.c | 3 +++ 7 files changed, 62 insertions(+) diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h index 9758d768fd0a..fd6635d36801 100644 --- a/drivers/net/ethernet/stmicro/stmmac/common.h +++ b/drivers/net/ethernet/stmicro/stmmac/common.h @@ -605,6 +605,11 @@ struct mac_device_info { u8 vlan_fail_q; bool hw_vlan_en; bool reverse_sgmii_enable; + + /* This spinlock protects read-modify-write of the interrupt + * mask/enable registers. + */ + spinlock_t irq_ctrl_lock; }; struct stmmac_rx_routing { diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c index 571e48362444..2ca94bfd3f71 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c @@ -61,6 +61,20 @@ static void dwmac1000_core_init(struct mac_device_info *hw, #endif } +static void dwmac1000_irq_modify(struct mac_device_info *hw, u32 disable, + u32 enable) +{ + void __iomem *int_mask = hw->pcsr + GMAC_INT_MASK; + unsigned long flags; + u32 value; + + spin_lock_irqsave(&hw->irq_ctrl_lock, flags); + value = readl(int_mask) | disable; + value &= ~enable; + writel(value, int_mask); + spin_unlock_irqrestore(&hw->irq_ctrl_lock, flags); +} + static int dwmac1000_rx_ipc_enable(struct mac_device_info *hw) { void __iomem *ioaddr = hw->pcsr; @@ -445,6 +459,7 @@ static void dwmac1000_set_mac_loopback(void __iomem *ioaddr, bool enable) const struct stmmac_ops dwmac1000_ops = { .pcs_init = dwmac1000_pcs_init, .core_init = dwmac1000_core_init, + .irq_modify = dwmac1000_irq_modify, .set_mac = stmmac_set_mac, .rx_ipc = dwmac1000_rx_ipc_enable, .dump_regs = dwmac1000_dump_regs, diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c index 0b785389b7ef..6269407d70cd 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c @@ -57,6 +57,20 @@ static void dwmac4_core_init(struct mac_device_info *hw, init_waitqueue_head(&priv->tstamp_busy_wait); } +static void dwmac4_irq_modify(struct mac_device_info *hw, u32 disable, + u32 enable) +{ + void __iomem *int_mask = hw->pcsr + GMAC_INT_EN; + unsigned long flags; + u32 value; + + spin_lock_irqsave(&hw->irq_ctrl_lock, flags); + value = readl(int_mask) & ~disable; + value |= enable; + writel(value, int_mask); + spin_unlock_irqrestore(&hw->irq_ctrl_lock, flags); +} + static void dwmac4_update_caps(struct stmmac_priv *priv) { if (priv->plat->tx_queues_to_use > 1) @@ -885,6 +899,7 @@ static int dwmac4_config_l4_filter(struct mac_device_info *hw, u32 filter_no, const struct stmmac_ops dwmac4_ops = { .pcs_init = dwmac4_pcs_init, .core_init = dwmac4_core_init, + .irq_modify = dwmac4_irq_modify, .update_caps = dwmac4_update_caps, .set_mac = stmmac_set_mac, .rx_ipc = dwmac4_rx_ipc_enable, @@ -920,6 +935,7 @@ const struct stmmac_ops dwmac4_ops = { const struct stmmac_ops dwmac410_ops = { .pcs_init = dwmac4_pcs_init, .core_init = dwmac4_core_init, + .irq_modify = dwmac4_irq_modify, .update_caps = dwmac4_update_caps, .set_mac = stmmac_dwmac4_set_mac, .rx_ipc = dwmac4_rx_ipc_enable, @@ -957,6 +973,7 @@ const struct stmmac_ops dwmac410_ops = { const struct stmmac_ops dwmac510_ops = { .pcs_init = dwmac4_pcs_init, .core_init = dwmac4_core_init, + .irq_modify = dwmac4_irq_modify, .update_caps = dwmac4_update_caps, .set_mac = stmmac_dwmac4_set_mac, .rx_ipc = dwmac4_rx_ipc_enable, diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c index 0430af27da40..b40b3ea50e25 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c @@ -28,6 +28,20 @@ static void dwxgmac2_core_init(struct mac_device_info *hw, writel(XGMAC_INT_DEFAULT_EN, ioaddr + XGMAC_INT_EN); } +static void dwxgmac2_irq_modify(struct mac_device_info *hw, u32 disable, + u32 enable) +{ + void __iomem *int_mask = hw->pcsr + XGMAC_INT_EN; + unsigned long flags; + u32 value; + + spin_lock_irqsave(&hw->irq_ctrl_lock, flags); + value = readl(int_mask) & ~disable; + value |= enable; + writel(value, int_mask); + spin_unlock_irqrestore(&hw->irq_ctrl_lock, flags); +} + static void dwxgmac2_update_caps(struct stmmac_priv *priv) { if (!priv->dma_cap.mbps_10_100) @@ -1411,6 +1425,7 @@ static void dwxgmac2_set_arp_offload(struct mac_device_info *hw, bool en, const struct stmmac_ops dwxgmac210_ops = { .core_init = dwxgmac2_core_init, + .irq_modify = dwxgmac2_irq_modify, .update_caps = dwxgmac2_update_caps, .set_mac = dwxgmac2_set_mac, .rx_ipc = dwxgmac2_rx_ipc, @@ -1466,6 +1481,7 @@ static void dwxlgmac2_rx_queue_enable(struct mac_device_info *hw, u8 mode, const struct stmmac_ops dwxlgmac2_ops = { .core_init = dwxgmac2_core_init, + .irq_modify = dwxgmac2_irq_modify, .set_mac = dwxgmac2_set_mac, .rx_ipc = dwxgmac2_rx_ipc, .rx_queue_enable = dwxlgmac2_rx_queue_enable, diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.c b/drivers/net/ethernet/stmicro/stmmac/hwif.c index 3f7c765dcb79..aa9e06be4a79 100644 --- a/drivers/net/ethernet/stmicro/stmmac/hwif.c +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.c @@ -351,6 +351,8 @@ int stmmac_hwif_init(struct stmmac_priv *priv) if (!mac) return -ENOMEM; + spin_lock_init(&mac->irq_ctrl_lock); + /* Fallback to generic HW */ for (i = ARRAY_SIZE(stmmac_hw) - 1; i >= 0; i--) { entry = &stmmac_hw[i]; diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h index 82cfb6bec334..cb8fc09caf86 100644 --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h @@ -319,6 +319,8 @@ struct stmmac_ops { void (*core_init)(struct mac_device_info *hw, struct net_device *dev); /* Update MAC capabilities */ void (*update_caps)(struct stmmac_priv *priv); + /* Change the interrupt enable setting. Enable takes precedence. */ + void (*irq_modify)(struct mac_device_info *hw, u32 disable, u32 enable); /* Enable the MAC RX/TX */ void (*set_mac)(void __iomem *ioaddr, bool enable); /* Enable and verify that the IPC module is supported */ @@ -421,6 +423,8 @@ struct stmmac_ops { stmmac_do_void_callback(__priv, mac, core_init, __args) #define stmmac_mac_update_caps(__priv) \ stmmac_do_void_callback(__priv, mac, update_caps, __priv) +#define stmmac_mac_irq_modify(__priv, __args...) \ + stmmac_do_void_callback(__priv, mac, irq_modify, (__priv)->hw, __args) #define stmmac_mac_set(__priv, __args...) \ stmmac_do_void_callback(__priv, mac, set_mac, __args) #define stmmac_rx_ipc(__priv, __args...) \ diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c index 75b470ee621a..c54c70224351 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c @@ -70,8 +70,10 @@ static void stmmac_fpe_configure_pmac(struct ethtool_mmsv *mmsv, bool pmac_enabl struct stmmac_priv *priv = container_of(cfg, struct stmmac_priv, fpe_cfg); const struct stmmac_fpe_reg *reg = cfg->reg; void __iomem *ioaddr = priv->ioaddr; + unsigned long flags; u32 value; + spin_lock_irqsave(&priv->hw->irq_ctrl_lock, flags); value = readl(ioaddr + reg->int_en_reg); if (pmac_enable) { @@ -86,6 +88,7 @@ static void stmmac_fpe_configure_pmac(struct ethtool_mmsv *mmsv, bool pmac_enabl } writel(value, ioaddr + reg->int_en_reg); + spin_unlock_irqrestore(&priv->hw->irq_ctrl_lock, flags); } static void stmmac_fpe_send_mpacket(struct ethtool_mmsv *mmsv, -- 2.47.3 Add support to the PCS instance for controlling the PCS interrupts depending on whether the PCS is used. Signed-off-by: Russell King (Oracle) --- .../net/ethernet/stmicro/stmmac/dwmac1000.h | 7 +++--- .../ethernet/stmicro/stmmac/dwmac1000_core.c | 11 ++++------ drivers/net/ethernet/stmicro/stmmac/dwmac4.h | 2 -- .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 10 +++------ .../net/ethernet/stmicro/stmmac/stmmac_pcs.c | 22 ++++++++++++++++++- .../net/ethernet/stmicro/stmmac/stmmac_pcs.h | 4 +++- 6 files changed, 34 insertions(+), 22 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h b/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h index 8f3002d9de78..697bba641e05 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h @@ -38,11 +38,10 @@ #define GMAC_INT_DISABLE_PCSAN BIT(2) #define GMAC_INT_DISABLE_PMT BIT(3) #define GMAC_INT_DISABLE_TIMESTAMP BIT(9) -#define GMAC_INT_DISABLE_PCS (GMAC_INT_DISABLE_PCSLINK | \ - GMAC_INT_DISABLE_PCSAN) #define GMAC_INT_DEFAULT_MASK (GMAC_INT_DISABLE_RGMII | \ - GMAC_INT_DISABLE_TIMESTAMP | \ - GMAC_INT_DISABLE_PCS) + GMAC_INT_DISABLE_PCSLINK | \ + GMAC_INT_DISABLE_PCSAN | \ + GMAC_INT_DISABLE_TIMESTAMP) /* PMT Control and Status */ #define GMAC_PMT 0x0000002c diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c index 2ca94bfd3f71..a2ae136d2c0e 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c @@ -27,7 +27,9 @@ 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); + return stmmac_integrated_pcs_init(priv, GMAC_PCS_BASE, + GMAC_INT_DISABLE_PCSLINK | + GMAC_INT_DISABLE_PCSAN); } static void dwmac1000_core_init(struct mac_device_info *hw, @@ -48,12 +50,7 @@ static void dwmac1000_core_init(struct mac_device_info *hw, writel(value | GMAC_CORE_INIT, ioaddr + GMAC_CONTROL); /* Mask GMAC interrupts */ - value = GMAC_INT_DEFAULT_MASK; - - if (hw->pcs) - value &= ~GMAC_INT_DISABLE_PCS; - - writel(value, ioaddr + GMAC_INT_MASK); + writel(GMAC_INT_DEFAULT_MASK, ioaddr + GMAC_INT_MASK); #ifdef STMMAC_VLAN_TAG_USED /* Tag detection without filtering */ diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h index 6dd84b6544cc..3cb733781e1e 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h @@ -106,8 +106,6 @@ #define GMAC_INT_LPI_EN BIT(5) #define GMAC_INT_TSIE BIT(12) -#define GMAC_PCS_IRQ_DEFAULT (GMAC_INT_PCS_LINK | GMAC_INT_PCS_ANE) - #define GMAC_INT_DEFAULT_ENABLE (GMAC_INT_PMT_EN | GMAC_INT_LPI_EN | \ GMAC_INT_TSIE) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c index 6269407d70cd..a4282fd7c3c7 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c @@ -27,7 +27,8 @@ 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); + return stmmac_integrated_pcs_init(priv, GMAC_PCS_BASE, + GMAC_INT_PCS_LINK | GMAC_INT_PCS_ANE); } static void dwmac4_core_init(struct mac_device_info *hw, @@ -46,12 +47,7 @@ static void dwmac4_core_init(struct mac_device_info *hw, writel((clk_rate / 1000000) - 1, ioaddr + GMAC4_MAC_ONEUS_TIC_COUNTER); /* Enable GMAC interrupts */ - value = GMAC_INT_DEFAULT_ENABLE; - - if (hw->pcs) - value |= GMAC_PCS_IRQ_DEFAULT; - - writel(value, ioaddr + GMAC_INT_EN); + writel(GMAC_INT_DEFAULT_ENABLE, ioaddr + GMAC_INT_EN); if (GMAC_INT_DEFAULT_ENABLE & GMAC_INT_TSIE) init_waitqueue_head(&priv->tstamp_busy_wait); diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c index 50ea51d7a1cc..e2f531c11986 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c @@ -2,6 +2,22 @@ #include "stmmac.h" #include "stmmac_pcs.h" +static int dwmac_integrated_pcs_enable(struct phylink_pcs *pcs) +{ + struct stmmac_pcs *spcs = phylink_pcs_to_stmmac_pcs(pcs); + + stmmac_mac_irq_modify(spcs->priv, 0, spcs->int_mask); + + return 0; +} + +static void dwmac_integrated_pcs_disable(struct phylink_pcs *pcs) +{ + struct stmmac_pcs *spcs = phylink_pcs_to_stmmac_pcs(pcs); + + stmmac_mac_irq_modify(spcs->priv, spcs->int_mask, 0); +} + static void dwmac_integrated_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode, struct phylink_link_state *state) @@ -23,11 +39,14 @@ static int dwmac_integrated_pcs_config(struct phylink_pcs *pcs, } static const struct phylink_pcs_ops dwmac_integrated_pcs_ops = { + .pcs_enable = dwmac_integrated_pcs_enable, + .pcs_disable = dwmac_integrated_pcs_disable, .pcs_get_state = dwmac_integrated_pcs_get_state, .pcs_config = dwmac_integrated_pcs_config, }; -int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset) +int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset, + u32 int_mask) { struct stmmac_pcs *spcs; @@ -37,6 +56,7 @@ int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset) spcs->priv = priv; spcs->base = priv->ioaddr + offset; + spcs->int_mask = int_mask; spcs->pcs.ops = &dwmac_integrated_pcs_ops; __set_bit(PHY_INTERFACE_MODE_SGMII, spcs->pcs.supported_interfaces); diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h index 64397ac8ecab..cda93894168e 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h @@ -52,6 +52,7 @@ struct stmmac_priv; struct stmmac_pcs { struct stmmac_priv *priv; void __iomem *base; + u32 int_mask; struct phylink_pcs pcs; }; @@ -61,7 +62,8 @@ phylink_pcs_to_stmmac_pcs(struct phylink_pcs *pcs) return container_of(pcs, struct stmmac_pcs, pcs); } -int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset); +int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset, + u32 int_mask); /** * dwmac_pcs_isr - TBI, RTBI, or SGMII PHY ISR -- 2.47.3 Provide phylink with the autonegotiation capabilities for this PCS, and configure the PCS's AN settings according to phylink's requested requirements. This may cause regressions. Signed-off-by: Russell King (Oracle) --- .../net/ethernet/stmicro/stmmac/stmmac_pcs.c | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c index e2f531c11986..77d38936d898 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c @@ -2,6 +2,15 @@ #include "stmmac.h" #include "stmmac_pcs.h" +static unsigned int dwmac_integrated_pcs_inband_caps(struct phylink_pcs *pcs, + phy_interface_t interface) +{ + if (interface != PHY_INTERFACE_MODE_SGMII) + return 0; + + return LINK_INBAND_ENABLE | LINK_INBAND_DISABLE; +} + static int dwmac_integrated_pcs_enable(struct phylink_pcs *pcs) { struct stmmac_pcs *spcs = phylink_pcs_to_stmmac_pcs(pcs); @@ -32,13 +41,23 @@ 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); - - dwmac_ctrl_ane(spcs->base, 0, 1, spcs->priv->hw->reverse_sgmii_enable); + void __iomem *an_control = spcs->base + GMAC_AN_CTRL(0); + u32 ctrl; + + 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 (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) + ctrl |= GMAC_AN_CTRL_ANE; + else + ctrl |= GMAC_AN_CTRL_SGMRAL; + writel(ctrl, an_control); return 0; } 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, -- 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. Signed-off-by: Russell King (Oracle) --- drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h index cda93894168e..e42a98162c2b 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h @@ -109,6 +109,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; /* Enable and restart the Auto-Negotiation */ if (ane) @@ -122,6 +123,10 @@ static inline void dwmac_ctrl_ane(void __iomem *ioaddr, u32 reg, bool ane, if (srgmi_ral) value |= GMAC_AN_CTRL_SGMRAL; + if (old != value) + pr_warn("dwmac: PCS configuration changed from phylink by glue, please report: 0x%08x -> 0x%08x\n", + old, value); + writel(value, ioaddr + GMAC_AN_CTRL(reg)); } #endif /* __STMMAC_PCS_H__ */ -- 2.47.3 Signed-off-by: Russell King (Oracle) --- drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 7 ++++++- drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 7 ++++++- drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c | 6 ++++-- drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h | 3 ++- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c index a2ae136d2c0e..0d85902bafd0 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c @@ -24,12 +24,17 @@ static int dwmac1000_pcs_init(struct stmmac_priv *priv) { + phy_interface_t mode; + if (!priv->dma_cap.pcs) return 0; + mode = PHY_INTERFACE_MODE_SGMII; + return stmmac_integrated_pcs_init(priv, GMAC_PCS_BASE, GMAC_INT_DISABLE_PCSLINK | - GMAC_INT_DISABLE_PCSAN); + GMAC_INT_DISABLE_PCSAN, + &mode, 1); } static void dwmac1000_core_init(struct mac_device_info *hw, diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c index a4282fd7c3c7..af9a336a32e6 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c @@ -24,11 +24,16 @@ static int dwmac4_pcs_init(struct stmmac_priv *priv) { + phy_interface_t mode; + if (!priv->dma_cap.pcs) return 0; + mode = PHY_INTERFACE_MODE_SGMII; + return stmmac_integrated_pcs_init(priv, GMAC_PCS_BASE, - GMAC_INT_PCS_LINK | GMAC_INT_PCS_ANE); + GMAC_INT_PCS_LINK | GMAC_INT_PCS_ANE, + &mode, 1); } static void dwmac4_core_init(struct mac_device_info *hw, diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c index 77d38936d898..5293c52cf7af 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c @@ -65,7 +65,8 @@ static const struct phylink_pcs_ops dwmac_integrated_pcs_ops = { }; int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset, - u32 int_mask) + u32 int_mask, const phy_interface_t *modes, + int num) { struct stmmac_pcs *spcs; @@ -78,7 +79,8 @@ int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset, spcs->int_mask = int_mask; spcs->pcs.ops = &dwmac_integrated_pcs_ops; - __set_bit(PHY_INTERFACE_MODE_SGMII, spcs->pcs.supported_interfaces); + while (num--) + __set_bit(*modes++, spcs->pcs.supported_interfaces); priv->integrated_pcs = spcs; diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h index e42a98162c2b..36da4dab4f8f 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h @@ -63,7 +63,8 @@ phylink_pcs_to_stmmac_pcs(struct phylink_pcs *pcs) } int stmmac_integrated_pcs_init(struct stmmac_priv *priv, unsigned int offset, - u32 int_mask); + u32 int_mask, const phy_interface_t *modes, + int num); /** * dwmac_pcs_isr - TBI, RTBI, or SGMII PHY ISR -- 2.47.3