| Message ID | 20240624132802.14238-8-fancer.lancer@gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: stmmac: convert stmmac "pcs" to phylink | expand |
On Mon, Jun 24, 2024 at 04:26:33PM +0300, Serge Semin wrote: > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index 72c2d3e2c121..743d356f6d12 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -950,13 +950,16 @@ static struct phylink_pcs *stmmac_mac_select_pcs(struct phylink_config *config, > { > struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev)); > > + if (priv->hw->pcs) > + return &priv->hw->mac_pcs; > + > if (priv->hw->xpcs) > return &priv->hw->xpcs->pcs; > > if (priv->hw->phylink_pcs) > return priv->hw->phylink_pcs; > > - return stmmac_mac_phylink_select_pcs(priv, interface); > + return NULL; I really really don't like this due to: 1. I spent a long time working out what the priority here should be, and you've just thrown all that work away by changing it - to something that I believe is incorrect. 2. I want to eventually see this function checking the interface type before just handing out a random PCS, and it was my intention to eventually that into the MACs own select_pcs() methods. Getting rid of those methods means that the MACs themselves now can't make the decision which is where that should be. 3. When operating in RGMII "inband" mode, the .pcs_config etc doesn't make much sense (we're probably accessing registers that don't exist) and I had plans to split this into a RGMII "PCS" which was just a PCS that implemented .pcs_get_state(), a stub .pcs_config(), and a separate fully-featured "SGMII PCS". So, I would like to eventually see here something like: if (priv->hw->xpcs) return &priv->hw->xpcs->pcs; if (priv->hw->phylink_pcs) return priv->hw->phylink_pcs; if (!(priv->plat->flags & STMMAC_FLAG_HAS_INTEGRATED_PCS)) { if (phy_interface_mode_is_rgmii(priv->plat->mac_interface)) return &priv->hw->mac_rgmii_pcs; if (priv->dma_cap.pcs && priv->plat->mac_interface == PHY_INTERFACE_MODE_SGMII) return &priv->hw->mac_sgmii_pcs; } return NULL; > +void dwmac_pcs_init(struct mac_device_info *hw) > +{ > + struct stmmac_priv *priv = hw->priv; > + int interface = priv->plat->mac_interface; > + > + if (priv->plat->flags & STMMAC_FLAG_HAS_INTEGRATED_PCS) > + return; > + else if (phy_interface_mode_is_rgmii(interface)) > + hw->pcs = STMMAC_PCS_RGMII; > + else if (priv->dma_cap.pcs && interface == PHY_INTERFACE_MODE_SGMII) > + hw->pcs = STMMAC_PCS_SGMII; > + > + hw->mac_pcs.neg_mode = true; > +} Please move "hw->mac_pcs.neg_mode = true;" to where the PCS method functions are implemented - it determines whether the PCS method functions take the AN mode or the neg mode, and this is a property of their implementations. It should not be split away from them. Thanks.
On Fri, Jun 28, 2024 at 03:36:20PM +0100, Russell King (Oracle) wrote: > On Mon, Jun 24, 2024 at 04:26:33PM +0300, Serge Semin wrote: > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > index 72c2d3e2c121..743d356f6d12 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > @@ -950,13 +950,16 @@ static struct phylink_pcs *stmmac_mac_select_pcs(struct phylink_config *config, > > { > > struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev)); > > > > + if (priv->hw->pcs) > > + return &priv->hw->mac_pcs; > > + > > if (priv->hw->xpcs) > > return &priv->hw->xpcs->pcs; > > > > if (priv->hw->phylink_pcs) > > return priv->hw->phylink_pcs; > > > > - return stmmac_mac_phylink_select_pcs(priv, interface); > > + return NULL; > > I really really don't like this due to: > > 1. I spent a long time working out what the priority here should be, and > you've just thrown all that work away by changing it - to something that > I believe is incorrect. > Right, the correct precedence would be to use the external PCS if one available. It's easy to fix anyway. > 2. I want to eventually see this function checking the interface type > before just handing out a random PCS, The only problem is that currently it relies on the plat_stmmaenet_data::mac_interface field value instead of parsing the specified interface type.( > and it was my intention to > eventually that into the MACs own select_pcs() methods. Getting rid of > those methods means that the MACs themselves now can't make the > decision which is where that should be. Ok. Why not. We can preserve the MAC-own select_pcs() method. (See my last comment on this email for details.) > > 3. When operating in RGMII "inband" mode, the .pcs_config etc doesn't > make much sense (we're probably accessing registers that don't exist) Absolutely right. Current dwmac_pcs_config() implementation is fully SGMII/TBI/RTBI-specific. > and I had plans to split this into a RGMII "PCS" which was just a PCS > that implemented .pcs_get_state(), a stub .pcs_config(), and a separate > fully-featured "SGMII PCS". Actually it's a good idea. We should have that implemented in v3. > > So, I would like to eventually see here something like: > > if (priv->hw->xpcs) > return &priv->hw->xpcs->pcs; > > if (priv->hw->phylink_pcs) > return priv->hw->phylink_pcs; > > if (!(priv->plat->flags & STMMAC_FLAG_HAS_INTEGRATED_PCS)) { > if (phy_interface_mode_is_rgmii(priv->plat->mac_interface)) > return &priv->hw->mac_rgmii_pcs; > > if (priv->dma_cap.pcs && > priv->plat->mac_interface == PHY_INTERFACE_MODE_SGMII) > return &priv->hw->mac_sgmii_pcs; > } > > return NULL; So the differences of my and your implementations are: 1. priv->hw->pcs field is utilized to determine the RGMII/SGMII PCS availability (it's initialized in dwmac_pcs_init()). 2. The order of the PCS selection: internal PCS has precedence over the external PCS'es. 3. There is a single PHY-link PCS descriptor for both RGMII "inband" and SGMII PCSes. There is nothing hard to settle the 2. and 3. notes. The only problematic part is 1. due to the damn mac_device_info::ps field implying the fixed-speed semantics for the MAC2MAC case. The field is initialized in the stmmac_hw_setup() method based on the mac_device_info::pcs field content. The mac_device_info::ps value is then utilized in the stmmac_ops::core_init() methods and in dwmac_pcs_config() to pre-define the link speed. Since I hadn't come up with a good idea of what to do with that MAC2MAC stuff back then I decided to preserve the mac_device_info::pcs-based semantics everywhere. But now I guess I've got a good idea. We can use the plat_stmmacenet_data::mac_port_sel_speed field directly where it is relevant. Like this: drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: static int stmmac_hw_setup(struct net_device *dev, bool ptp_register) { // Drop everything priv->hw.pcs and priv->hw.ps related from here // due to the changes suggested further. } drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c: drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c: static void dwmac*_core_init(...) { ... // Directly use the plat_stmmacenet_data::mac_port_sel_speed value switch (priv->plat->mac_port_sel_speed) { case SPEED_1000: ps_speed = hw->link.speed1000; break; case SPEED_100: ps_speed = hw->link.speed100; break; case SPEED_10: ps_speed = hw->link.speed10; break; default: dev_warn(priv->device, "Unsupported port speed\n"); break; } if (ps_speed) { value &= hw->link.speed_mask; value |= ps_speed | GMAC_CONFIG_TE; } ... } drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c: static void dwmac*_core_init(...) { // There is no internal PCS in DW XGMACes. So we can freely drop // the hw->ps clause from here. } drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c: static int dwmac_pcs_config(...) { ... // Directly use the plat_stmmacenet_data::mac_port_sel_speed value if (priv->plat->mac_port_sel_speed) val |= PCS_AN_CTRL_SGMRAL; ... } After that we can freely drop the mac_device_info::ps and mac_device_info::pcs fields. Thoughts? > > > +void dwmac_pcs_init(struct mac_device_info *hw) > > +{ > > + struct stmmac_priv *priv = hw->priv; > > + int interface = priv->plat->mac_interface; > > + > > + if (priv->plat->flags & STMMAC_FLAG_HAS_INTEGRATED_PCS) > > + return; > > + else if (phy_interface_mode_is_rgmii(interface)) > > + hw->pcs = STMMAC_PCS_RGMII; > > + else if (priv->dma_cap.pcs && interface == PHY_INTERFACE_MODE_SGMII) > > + hw->pcs = STMMAC_PCS_SGMII; > > + > > + hw->mac_pcs.neg_mode = true; > > +} > > Please move "hw->mac_pcs.neg_mode = true;" to where the PCS method > functions are implemented - it determines whether the PCS method > functions take the AN mode or the neg mode, and this is a property of > their implementations. It should not be split away from them. Ok. --- Seeing the series introducing the plat_stmmacenet_data::select_pcs() method has been recently merged in, let's discuss the entire PCS selection code a bit more. Taking into account what you said above I guess we can implement something like this: drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: static struct phylink_pcs *stmmac_mac_select_pcs(struct phylink_config *config, phy_interface_t interface) { // Platform-specific PCS selection method implying DW XPCS and // Lynx PCS selection (and internal PCS selection if relevant) if (priv->plat->select_pcs) { pcs = priv->plat->select_pcs(priv, interface); if (!IS_ERR(pcs)) return pcs; } // MAC-specific PCS selection method pcs = stmmac_mac_select_int_pcs(priv, priv->hw, priv->plat->mac_interface); if (!IS_ERR(pcs)) return pcs; return NULL; } drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c: static struct phylink_pcs *dwmac1000_select_pcs(struct mac_device_info *hw, phy_interface_t interface) { if (phy_interface_mode_is_rgmii(interface)) return &hw->mac_rgmii_pcs; else if (priv->dma_cap.pcs && interface == PHY_INTERFACE_MODE_SGMII) return &hw->mac_sgmii_pcs; return NULLL } ... const struct stmmac_ops dwmac1000_ops = { ... .select_pcs = dwmac1000_select_pcs, ... }; drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c: // The same changes as in the dwmac1000_core.c file. drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c: // Drop my dwmac_pcs_init() implementation if we get to eliminate the // mac_device_info::ps and mac_device_info::pcs fields as I suggested // earlier in this message So what do you think? -Serge(y) > > Thanks. > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c index 2d77ffd16f0a..2fbb853cc19c 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c @@ -16,7 +16,7 @@ #include <linux/slab.h> #include <linux/ethtool.h> #include <linux/io.h> -#include <linux/phylink.h> + #include "stmmac.h" #include "stmmac_pcs.h" #include "dwmac1000.h" @@ -388,17 +388,6 @@ static u16 dwmac1000_pcs_get_config_reg(struct mac_device_info *hw) return FIELD_GET(GMAC_RGSMIIIS_CONFIG_REG, val); } -static struct phylink_pcs * -dwmac1000_phylink_select_pcs(struct stmmac_priv *priv, - phy_interface_t interface) -{ - if (priv->hw->pcs & STMMAC_PCS_RGMII || - priv->hw->pcs & STMMAC_PCS_SGMII) - return &priv->hw->mac_pcs; - - return NULL; -} - static void dwmac1000_debug(struct stmmac_priv *priv, void __iomem *ioaddr, struct stmmac_extra_stats *x, u32 rx_queues, u32 tx_queues) @@ -489,7 +478,6 @@ static void dwmac1000_set_mac_loopback(void __iomem *ioaddr, bool enable) const struct stmmac_ops dwmac1000_ops = { .core_init = dwmac1000_core_init, - .phylink_select_pcs = dwmac1000_phylink_select_pcs, .set_mac = stmmac_set_mac, .rx_ipc = dwmac1000_rx_ipc_enable, .dump_regs = dwmac1000_dump_regs, @@ -541,7 +529,5 @@ int dwmac1000_setup(struct stmmac_priv *priv) mac->mii.clk_csr_shift = 2; mac->mii.clk_csr_mask = GENMASK(5, 2); - mac->mac_pcs.neg_mode = true; - return 0; } diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c index c58dc20eddeb..f5449f0962ad 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c @@ -14,7 +14,7 @@ #include <linux/slab.h> #include <linux/ethtool.h> #include <linux/io.h> -#include <linux/phylink.h> + #include "stmmac.h" #include "stmmac_pcs.h" #include "dwmac4.h" @@ -780,16 +780,6 @@ static void dwmac4_flow_ctrl(struct mac_device_info *hw, unsigned int duplex, } } -static struct phylink_pcs * -dwmac4_phylink_select_pcs(struct stmmac_priv *priv, phy_interface_t interface) -{ - if (priv->hw->pcs & STMMAC_PCS_RGMII || - priv->hw->pcs & STMMAC_PCS_SGMII) - return &priv->hw->mac_pcs; - - return NULL; -} - static int dwmac4_irq_mtl_status(struct stmmac_priv *priv, struct mac_device_info *hw, u32 chan) { @@ -1178,7 +1168,6 @@ static void dwmac4_set_hw_vlan_mode(struct mac_device_info *hw) const struct stmmac_ops dwmac4_ops = { .core_init = dwmac4_core_init, .update_caps = dwmac4_update_caps, - .phylink_select_pcs = dwmac4_phylink_select_pcs, .set_mac = stmmac_set_mac, .rx_ipc = dwmac4_rx_ipc_enable, .rx_queue_enable = dwmac4_rx_queue_enable, @@ -1224,7 +1213,6 @@ const struct stmmac_ops dwmac4_ops = { const struct stmmac_ops dwmac410_ops = { .core_init = dwmac4_core_init, .update_caps = dwmac4_update_caps, - .phylink_select_pcs = dwmac4_phylink_select_pcs, .set_mac = stmmac_dwmac4_set_mac, .rx_ipc = dwmac4_rx_ipc_enable, .rx_queue_enable = dwmac4_rx_queue_enable, @@ -1274,7 +1262,6 @@ const struct stmmac_ops dwmac410_ops = { const struct stmmac_ops dwmac510_ops = { .core_init = dwmac4_core_init, .update_caps = dwmac4_update_caps, - .phylink_select_pcs = dwmac4_phylink_select_pcs, .set_mac = stmmac_dwmac4_set_mac, .rx_ipc = dwmac4_rx_ipc_enable, .rx_queue_enable = dwmac4_rx_queue_enable, @@ -1389,7 +1376,5 @@ int dwmac4_setup(struct stmmac_priv *priv) mac->mii.clk_csr_mask = GENMASK(11, 8); mac->num_vlan = dwmac4_get_num_vlan(priv->ioaddr); - mac->mac_pcs.neg_mode = true; - return 0; } diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h index 3d39417e906d..4bacfe8a18e0 100644 --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h @@ -18,17 +18,13 @@ } \ __result; \ }) -#define stmmac_do_typed_callback(__type, __fail_ret, __priv, __module, \ - __cname, __arg0, __args...) \ +#define stmmac_do_callback(__priv, __module, __cname, __arg0, __args...) \ ({ \ - __type __result = __fail_ret; \ + int __result = -EINVAL; \ if ((__priv)->hw->__module && (__priv)->hw->__module->__cname) \ __result = (__priv)->hw->__module->__cname((__arg0), ##__args); \ __result; \ }) -#define stmmac_do_callback(__priv, __module, __cname, __arg0, __args...) \ - stmmac_do_typed_callback(int, -EINVAL, __priv, __module, __cname, \ - __arg0, ##__args) struct stmmac_extra_stats; struct stmmac_priv; @@ -315,9 +311,6 @@ 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); - /* Get phylink PCS (for MAC */ - struct phylink_pcs *(*phylink_select_pcs)(struct stmmac_priv *priv, - phy_interface_t interface); /* Enable the MAC RX/TX */ void (*set_mac)(void __iomem *ioaddr, bool enable); /* Enable and verify that the IPC module is supported */ @@ -439,10 +432,6 @@ 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_phylink_select_pcs(__priv, __interface) \ - stmmac_do_typed_callback(struct phylink_pcs *, ERR_PTR(-EOPNOTSUPP), \ - __priv, mac, phylink_select_pcs, __priv,\ - __interface) #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_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 72c2d3e2c121..743d356f6d12 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -950,13 +950,16 @@ static struct phylink_pcs *stmmac_mac_select_pcs(struct phylink_config *config, { struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev)); + if (priv->hw->pcs) + return &priv->hw->mac_pcs; + if (priv->hw->xpcs) return &priv->hw->xpcs->pcs; if (priv->hw->phylink_pcs) return priv->hw->phylink_pcs; - return stmmac_mac_phylink_select_pcs(priv, interface); + return NULL; } static void stmmac_mac_config(struct phylink_config *config, unsigned int mode, @@ -1121,23 +1124,6 @@ static const struct phylink_mac_ops stmmac_phylink_mac_ops = { .mac_link_up = stmmac_mac_link_up, }; -/** - * stmmac_check_pcs_mode - verify if RGMII/SGMII is supported - * @priv: driver private structure - * Description: this is to verify if the HW supports the PCS. - * Physical Coding Sublayer (PCS) interface that can be used when the MAC is - * configured for the TBI, RTBI, or SGMII PHY interface. - */ -static void stmmac_check_pcs_mode(struct stmmac_priv *priv) -{ - int interface = priv->plat->mac_interface; - - if (phy_interface_mode_is_rgmii(interface)) - priv->hw->pcs = STMMAC_PCS_RGMII; - else if (priv->dma_cap.pcs && interface == PHY_INTERFACE_MODE_SGMII) - priv->hw->pcs = STMMAC_PCS_SGMII; -} - /** * stmmac_init_phy - PHY initialization * @dev: net device structure @@ -7704,8 +7690,6 @@ int stmmac_dvr_probe(struct device *device, else stmmac_clk_csr_set(priv); - stmmac_check_pcs_mode(priv); - pm_runtime_get_noresume(device); pm_runtime_set_active(device); if (!pm_runtime_enabled(device)) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c index aa43117134d3..985b4b8c021f 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c @@ -22,6 +22,7 @@ #include "dwxgmac2.h" #include "stmmac.h" +#include "stmmac_pcs.h" #define MII_BUSY 0x00000001 #define MII_WRITE 0x00000002 @@ -505,6 +506,8 @@ int stmmac_pcs_setup(struct net_device *ndev) priv = netdev_priv(ndev); mode = priv->plat->phy_interface; + dwmac_pcs_init(priv->hw); + if (priv->plat->pcs_init) { ret = priv->plat->pcs_init(priv); } else if (priv->plat->mdio_bus_data && diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c index aac49f6472f0..fdfc95299f89 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c @@ -167,3 +167,18 @@ void dwmac_pcs_isr(struct mac_device_info *hw, unsigned int intr_status, if (an_status || sr_status) phylink_pcs_change(&hw->mac_pcs, false); } + +void dwmac_pcs_init(struct mac_device_info *hw) +{ + struct stmmac_priv *priv = hw->priv; + int interface = priv->plat->mac_interface; + + if (priv->plat->flags & STMMAC_FLAG_HAS_INTEGRATED_PCS) + return; + else if (phy_interface_mode_is_rgmii(interface)) + hw->pcs = STMMAC_PCS_RGMII; + else if (priv->dma_cap.pcs && interface == PHY_INTERFACE_MODE_SGMII) + hw->pcs = STMMAC_PCS_SGMII; + + hw->mac_pcs.neg_mode = true; +} diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h index 6e364285a4ef..a17e5b37c411 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h @@ -61,4 +61,6 @@ void dwmac_pcs_isr(struct mac_device_info *hw, unsigned int intr_status, struct stmmac_extra_stats *x); +void dwmac_pcs_init(struct mac_device_info *hw); + #endif /* __STMMAC_PCS_H__ */
Currently the internal PCS initialization procedure is split up into several functions and special flags: 1. stmmac_check_pcs_mode() - determine the internal PCS based on the specified interface and init the mac_device_info::pcs field with the respective flag (STMMAC_PCS_RGMII or STMMAC_PCS_SGMII); 2. stmmac_ops::phylink_select_pcs() - callback specific for the DW GMAC and DW QoS Eth IP-cores which returns the pointer to the phylink_pcs structure if any of the STMMAC_PCS_RGMII or STMMAC_PCS_SGMII flag is found in the mac_device_info::pcs field. The initialization procedure described above can be simplified by converting the stmmac_check_pcs_mode() to the PCS-init method defined in stmmac_pcs.c, which besides would take the STMMAC_FLAG_HAS_INTEGRATED_PCS flag into account. Seeing the RGMII and SGMII MAC-interfaces can be only found on the DW GMAC and DW QoS Eth, the stmmac_ops::phylink_select_pcs() callbacks content can be freely moved right into the generic phylink_mac_ops::mac_select_pcs() function. Signed-off-by: Serge Semin <fancer.lancer@gmail.com> --- The change looks very promising and greatly simplifying the internal PCS initialization procedure by providing a single coherent method activating the internal PCS capability based on the requested interface and the detected DW MAC HW-capability. --- .../ethernet/stmicro/stmmac/dwmac1000_core.c | 16 +------------ .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 17 +------------ drivers/net/ethernet/stmicro/stmmac/hwif.h | 15 ++---------- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 24 ++++--------------- .../net/ethernet/stmicro/stmmac/stmmac_mdio.c | 3 +++ .../net/ethernet/stmicro/stmmac/stmmac_pcs.c | 15 ++++++++++++ .../net/ethernet/stmicro/stmmac/stmmac_pcs.h | 2 ++ 7 files changed, 28 insertions(+), 64 deletions(-)