| Message ID | E1sZpoq-000eHy-GR@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: stmmac: convert stmmac "pcs" to phylink | expand |
On Fri, Aug 02, 2024 at 11:47:32AM GMT, Russell King (Oracle) wrote: > The pcs_ctrl_ane() method is no longer required as this will be handled > by the mac_pcs phylink_pcs instance. Remove these methods, their common > implementation, the pcs_link, pcs_duplex and pcs_speed members of > struct stmmac_extra_stats, and stmmac_has_mac_phylink_select_pcs(). > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> ... > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > index 3c8ae3753205..799af80024d2 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > @@ -321,48 +321,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) && This change effectively makes the INTEGRATED_PCS flag useless, I think we should remove it entirely. Thanks, Andrew
On Fri, Aug 02, 2024 at 02:02:25PM -0500, Andrew Halaney wrote: > On Fri, Aug 02, 2024 at 11:47:32AM GMT, Russell King (Oracle) wrote: > > The pcs_ctrl_ane() method is no longer required as this will be handled > > by the mac_pcs phylink_pcs instance. Remove these methods, their common > > implementation, the pcs_link, pcs_duplex and pcs_speed members of > > struct stmmac_extra_stats, and stmmac_has_mac_phylink_select_pcs(). > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > ... > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > > index 3c8ae3753205..799af80024d2 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > > @@ -321,48 +321,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) && > > This change effectively makes the INTEGRATED_PCS flag useless, I think > we should remove it entirely. I'm hoping the ethqos folk are going to test this patch series and tell me whether it works for them - specifically Sneh Shah who added net: stmmac: dwmac-qcom-ethqos: Add support for 2.5G SGMII which directly configures the PCS bypassing phylink. Specifically, if this in stmmac_check_pcs_mode(): priv->dma_cap.pcs && interface == PHY_INTERFACE_MODE_SGMII is true for this device, then we may be in for problems. Since priv->dma_cap.pcs comes from hardware, it's impossible to tell unless one has that hardware.
On Fri, Aug 02, 2024 at 08:22:42PM GMT, Russell King (Oracle) wrote: > On Fri, Aug 02, 2024 at 02:02:25PM -0500, Andrew Halaney wrote: > > On Fri, Aug 02, 2024 at 11:47:32AM GMT, Russell King (Oracle) wrote: > > > The pcs_ctrl_ane() method is no longer required as this will be handled > > > by the mac_pcs phylink_pcs instance. Remove these methods, their common > > > implementation, the pcs_link, pcs_duplex and pcs_speed members of > > > struct stmmac_extra_stats, and stmmac_has_mac_phylink_select_pcs(). > > > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > > > ... > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > > > index 3c8ae3753205..799af80024d2 100644 > > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > > > @@ -321,48 +321,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) && > > > > This change effectively makes the INTEGRATED_PCS flag useless, I think > > we should remove it entirely. > > I'm hoping the ethqos folk are going to test this patch series and tell > me whether it works for them - specifically Sneh Shah who added > > net: stmmac: dwmac-qcom-ethqos: Add support for 2.5G SGMII > > which directly configures the PCS bypassing phylink. Specifically, > if this in stmmac_check_pcs_mode(): > > priv->dma_cap.pcs && interface == PHY_INTERFACE_MODE_SGMII > > is true for this device, then we may be in for problems. Since > priv->dma_cap.pcs comes from hardware, it's impossible to tell > unless one has that hardware. Hopefully we get a response there. For what its worth I have a access to the sa8775p-ride.dts board in a remote lab and dma_cap.pcs is definitely set for this integration of the IP on sa8775p. The only upstream described boards are: 1) sa8775p-ride 2) sa8775p-ride-r3 The difference is that "r3" is the latest spin of the board, with some Aquantia phys attached to the 2 stmmac MACs on the board instead of the Marvell 88EA1512 phys on the former. My understanding is that's to evaluate 2500 Mbps speeds (the 88EA1512 only goes up to 1000 Mbps). The "r3" board's Aquantia aqr115c's are capable of 2500 Mbps, but are "overclock SGMII". The "r3" describes the phy interface as 2500base-x, with no in-band signalling (since the "OCSGMII" is hacked up and doesn't really do the in-band signalling you've described in the past). That's all based on Bart's commit message adding support for that in: 0ebc581f8a4b7 net: phy: aquantia: add support for aqr115c I think Sneh also had access to a board with the sa8775p in a fixed-link configuration doing 2500 Mbps, but that's not described upstream at the moment. I believe that was the board that originally motivated the patch you highlighted from him. At the very least Bartosz and I tested this and things didn't break noticeably for the 2 boards I listed above... so that's good :) Hope that helps, Andrew
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h index 9e8f1659377e..5a49d8db30fe 100644 --- a/drivers/net/ethernet/stmicro/stmmac/common.h +++ b/drivers/net/ethernet/stmicro/stmmac/common.h @@ -191,9 +191,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; @@ -394,13 +391,6 @@ enum request_irq_err { #define CORE_IRQ_MTL_RX_OVERFLOW BIT(8) /* Physical Coding Sublayer */ -struct rgmii_adv { - unsigned int pause; - unsigned int duplex; - unsigned int lp_pause; - unsigned int lp_duplex; -}; - #define STMMAC_PCS_PAUSE 1 #define STMMAC_PCS_ASYM_PAUSE 2 diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h index 06284aee4088..3553e8a767cb 100644 --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h @@ -303,7 +303,6 @@ struct stmmac_dma_ops { struct mac_device_info; struct net_device; -struct rgmii_adv; struct stmmac_tc_entry; struct stmmac_pps_cfg; struct stmmac_rss; @@ -539,9 +538,6 @@ struct stmmac_ops { #define stmmac_fpe_irq_status(__priv, __args...) \ stmmac_do_callback(__priv, mac, fpe_irq_status, __args) -#define stmmac_has_mac_phylink_select_pcs(__priv) \ - ((__priv)->hw->mac->phylink_select_pcs != NULL) - /* PTP and HW Timer helpers */ struct stmmac_hwtimestamp { void (*config_hw_tstamping) (void __iomem *ioaddr, u32 data); diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c index 3c8ae3753205..799af80024d2 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c @@ -321,48 +321,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) && - !stmmac_has_mac_phylink_select_pcs(priv)) { - 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,21 +330,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) && - !stmmac_has_mac_phylink_select_pcs(priv)) { - /* Only support ANE */ - if (cmd->base.autoneg != AUTONEG_ENABLE) - return -EINVAL; - - mutex_lock(&priv->lock); - stmmac_pcs_ctrl_ane(priv, priv->ioaddr, 1, priv->hw->ps, 0); - mutex_unlock(&priv->lock); - - return 0; - } - return phylink_ethtool_ksettings_set(priv->phylink, cmd); } @@ -487,11 +430,7 @@ stmmac_get_pauseparam(struct net_device *netdev, { struct stmmac_priv *priv = netdev_priv(netdev); - if (priv->hw->pcs && !stmmac_has_mac_phylink_select_pcs(priv)) { - pause->autoneg = 1; - } else { - phylink_ethtool_get_pauseparam(priv->phylink, pause); - } + phylink_ethtool_get_pauseparam(priv->phylink, pause); } static int @@ -500,12 +439,7 @@ stmmac_set_pauseparam(struct net_device *netdev, { struct stmmac_priv *priv = netdev_priv(netdev); - if (priv->hw->pcs && !stmmac_has_mac_phylink_select_pcs(priv)) { - 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) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index a08dccad0ff2..3e43f2d6d49f 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -3486,9 +3486,6 @@ static int stmmac_hw_setup(struct net_device *dev, bool ptp_register) } } - if (priv->hw->pcs && !stmmac_has_mac_phylink_select_pcs(priv)) - stmmac_pcs_ctrl_ane(priv, priv->ioaddr, 1, priv->hw->ps, 0); - /* set TX and RX rings length */ stmmac_set_rings_length(priv); @@ -6062,16 +6059,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) && - !stmmac_has_mac_phylink_select_pcs(priv)) { - if (priv->xstats.pcs_link) - netif_carrier_on(priv->dev); - else - netif_carrier_off(priv->dev); - } - stmmac_timestamp_interrupt(priv, priv); } }
The pcs_ctrl_ane() method is no longer required as this will be handled by the mac_pcs phylink_pcs instance. Remove these methods, their common implementation, the pcs_link, pcs_duplex and pcs_speed members of struct stmmac_extra_stats, and stmmac_has_mac_phylink_select_pcs(). Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> --- drivers/net/ethernet/stmicro/stmmac/common.h | 10 --- drivers/net/ethernet/stmicro/stmmac/hwif.h | 4 -- .../ethernet/stmicro/stmmac/stmmac_ethtool.c | 70 +------------------ .../net/ethernet/stmicro/stmmac/stmmac_main.c | 13 ---- 4 files changed, 2 insertions(+), 95 deletions(-)