On Tue, Jun 04, 2024 at 10:29:40AM +0100, Russell King (Oracle) wrote: > On Tue, Jun 04, 2024 at 12:04:57PM +0300, Serge Semin wrote: > > On Mon, Jun 03, 2024 at 10:03:54AM +0100, Russell King (Oracle) wrote: > > > I can't think of a reasonable solution to this at the moment. One > > > solution could be pushing this down into the platform code to deal > > > with as an interim solution, via the new .pcs_init() method. > > > > > > We could also do that with the current XPCS code, since we know that > > > only Intel mGBE uses xpcs. This would probably allow us to get rid > > > of the has_xpcs flag. > > > > Basically you suggest to move the entire stmmac_pcs_setup() to the > > platforms, don't you? The patch 9 of this series indeed could have > > been converted to just moving the entire PCS-detection loop from > > stmmac_pcs_setup() to the Intel-specific pcs_init. > > Yes, it's not like XPCS is used by more than one platform, it's only > Intel mGBE. So I don't see why it should have a privileged position > over any other PCS implementation that stmmac supports (there's now > three different PCS.) > Alas DW XPCS has already got a more privileged position. The STMMAC driver calls the XPCS driver methods here and there (supported ifaces, EEE or PHY setup). Unless these calls are converted to some standard/new phylink_pcs calls IMO it would be better to preserve the default DW XPCS init at least for the "pcs-handle" property to motivate the platform drivers developers to follow some pre-defined device description pattern (e.g. defining DW XPCS devices in device tree), but leave the .pcs_init() for some platform-specific PCS inits (including which have already been implemented). As I already mentioned DW XPCS is of Synopsys vendor. The IP-core has been invented to provide a bridge between the Synopsys MAC IP-cores and PMA (mainly Synopsys PMAs) for the 1G/10G links like 1000Base-X, and 10GBase-X/-R/-KX4/-KR. The reason we see just a single use-case of the XPCS in the driver is that even though the STMMAC driver has DW XGMAC support the driver is mainly utilized for the 1G MACs (I don't see any platform currently having DW XGMAC defined). Since DW GMAC/QoS Eth can be configured to have the standard PHY interfaces available there is no need in XPCS in these cases (except a weird Intel mGBE). But when it comes to DW XGMAC it can be synthesized with GMII and XGMII interfaces only. These're exactly interfaces which DW XPCS supports on upstream. Thus basically the DW XPCS IP-core has been mainly produced for been utilized in a couple with DW XGMAC providing a ready-to-use solution for the XFP/SFP(+) ports or backplane-based applications. So should we have more DW XGMACs supported in the kernel we would have met more DW XPCS defined in there too. > If you don't want the code in the Intel driver, then what could be > done is provide a core implementation that gets hooked into the > .pcs_init() method. I don't mind converting patch 9 to moving the XPCS registration in the Intel-specific .pcs_init() (especially seeing it's just a single xpcs_create_mdiodev() call), but having the "pcs-handle" property handled generically in the STMMAC core would be a useful thing to have (see my reasoning above). -Serge(y) > > The same is probably true of other PCSes if they end up being shared > across several different platforms. > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!