Re: [PATCH net-next v2 10/10] net: stmmac: Add DW XPCS specified via "pcs-handle" support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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!




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux