On Thu, Oct 10, 2024 at 07:39:16PM +0530, Karumanchi, Vineeth wrote: > Hi Russel, > > On 10/9/2024 2:49 PM, Russell King (Oracle) wrote: > > It also looks like you're messing with MAC registers in the PCS code, > > setting the MAC speed there. Are the PCS and MAC so integrated together > > that abstracting the PCS into its own separate code block leads to > > problems? > > Agreed, Since our current hardware configuration lacks AN and PHY, I've > relocated the ENABLE_HS_MAC configuration into PCS to > allow speed changes using ethtool. When more hardware with a PHY that > supports AN becomes available, > the phylink will invoke macb_mac_config() with the communicated speed > (phylinkstate->speed). Where are you getting that idea from, because that has not been true for a good number of years - and it's been stated in the phylink documentation for a very long time. I've killed all the code references to ->speed in all mac_config() implementations, and I've even gone to the extent of now ensuring that all mac_config() methods will _always_ be called with state->speed set to SPEED_UNKNOWN, so no one can make any useful determinations from that. If people continue to insist on using this, then I'll have no option but to make a disruptive API change, making mac_config() take an explicit set of arguments for the items that it should have access to. > Currently, for fixed-link, will keep the earlier implementation. I want phylink users to be correct and easy to understand - because I maintain phylink, and that means I need to understand the code that makes use of its facilities. So, want to see phylink methods implemented properly. If they aren't going to be implemented properly, then I will ask that the driver ceases to use phylink quite simply because it makes _my_ maintenance more difficult when drivers don't implement phylink methods correctly. The choice is: implement phylink methods properly or don't use phylink. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
