Subject: Re: [RFC v8 net-next 15/16] net: dsa: felix: add phylink_get_caps capability

From: Colin Foster <[email-masked]>
Date: Fri, 9 Sep 2022 12:30:21 -0700

On Fri, Sep 09, 2022 at 08:20:03PM +0100, Russell King (Oracle) wrote:
> On Fri, Sep 09, 2022 at 11:33:59AM -0700, Colin Foster wrote:
> > Seemingly because net/dsa/port.c checks for phylink_validate before it
> > checks for mac_capabilties, it won't make a difference there, but this
> > seems ... wrong? Or maybe it isn't wrong until I implement the QSGMII
> > port that supports 2500FD (as in drivers/net/ethernet/mscc/ocelot_net.c
> > ocelot_port_phylink_create())
> 
> No, the code in dsa_port_phylink_validate() is exactly what I intend.

My apologies... I meant "it seems wrong for me to blindly assign values to
mac_capabilities" not that there are any issues in the logic of
dsa_port_phylink_validate(). My phrasing was misleading.

> 
> If there's a phylink_validate() function, then that gets used to cater
> for Ocelot's rate adaption in the PCS (where the link modes are not
> limited by the interface mode.)
> 
> If there isn't a phylink_validate() function, then we require that
> mac_capabilities() is filled in, and if it is, we use the generic
> validator - essentially I want to see everyone filling in both the
> supported interface masks and the MAC capabilities no matter what.
> 
> The Ocelot rate adaption is something that needs to be tidied up, but
> until that has been done, Ocelot needs to have the phylink_validate()
> hook. Ocelot is currently the sole user of this hook.
> 
> I have some experimental patches to address this, but nothing that I
> felt happy to send out yet.

I think we're on the same page. My version of Ocelot phylink_validate
utilizes phylink_generic_validate while other Felix devices don't, as
intended. Hopefully I'll be able to tidy up this patch set and send it
out for full review soon.

> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!