Re: [RFC PATCH net-next 4/5] net: macb: Configure High Speed Mac for given speed.

[Russell King (Oracle)]

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!