On Wed, Jul 01, 2020 at 10:23:12AM +0300, Baruch Siach wrote: > Hi Russell, > > On Mon, Jun 29 2020, Russell King - ARM Linux admin wrote: > > Then there's the whole question of what phydev->speed etc should be set > > to - the media speed or the host side link speed with the PHY, and then > > how the host side should configure itself. At least the 88E6390x > > switch will force itself to the media side speed using that while in > > XAUI mode, resulting in a non-functioning speed. So should the host > > side force itself to 10G whenever in something like XAUI mode? > > How does the switch discover the media side speed? Is there some sort of > in-band information exchange?
The media-side results are passed via phydev->speed and phydev->duplex, and therefore will be passed through phylink. mvpp2 will ignore them for 10GBASE-R as it has separate MACs - XLG and GMAC, but 88E6390x, it's just one. Consequently, it's possible that the port mode is in XAUI, but you can force the speed to (e.g.) 100M. What I know from what I can do with this media-side broken 88X3310, is that it will pass data if the 88E6390x is forced to 10G, but not if it's forced to 100M. We're moving from a situation where MAC drivers can expect (with either phylib or phylink): interface = <some 10G interface> speed is always 10G duplex is always full to: interface = <some 10G interface> speed is 10, 100, 1G or 10G duplex is half or full So, adding rate-matching brings with it a non-obvious change in the API of phylib and phylink: * what do the phydev->{speed,duplex,pause,asym_pause} represent - the media side parameters or the PHY to MAC parameters? * what do the "speed, duplex, pause" passed into mac_link_up() refer to, the media side, or the link side? Both of those need to be properly documented and explained. The next two points, I haven't re-read the 3310 datasheet. We also need to consider a situation which is less obvious: if the PHY is operating in rate matching mode, doesn't generate pause frames itself as its rate matching buffers fill, but the media side negotiated for pause frames. Should we be advertising no support for pause frames in this case? Will the PHY pass pause frames through as a priority? Consider that in a 16k outbound buffer, there could be up to 10 full sized frames queued, so if the link partner is asking us to stop sending, it could take up to 10 frames before we actually stop. What are the requirements for half duplex in rate matching mode - is that handled internally by the PHY, or do we need to disable all half duplex advertisements in the PHY. When rate matching, the PHY can no longer signal the MAC when a collision occurs, as it would normally do without rate matching. I think I've covered everything, but may have missed something. I do think we need documentation updated before we should accept this patch so that we have the phylib and phylink behaviour in this case clearly defined from the outset. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!