Subject: Re: [PATCH RFC net-next 03/12] net: dsa: replace phylink_get_interfaces() with phylink_get_caps()

From: "Russell King (Oracle)" <[email-masked]>
Date: Wed, 24 Nov 2021 20:56:33 +0000

On Wed, Nov 24, 2021 at 08:26:13PM +0000, Vladimir Oltean wrote:
> On Wed, Nov 24, 2021 at 07:10:49PM +0000, Russell King (Oracle) wrote:
> > On Wed, Nov 24, 2021 at 06:26:48PM +0000, Russell King (Oracle) wrote:
> > > On Wed, Nov 24, 2021 at 06:15:08PM +0000, Vladimir Oltean wrote:
> > > > On Wed, Nov 24, 2021 at 05:52:38PM +0000, Russell King (Oracle) wrote:
> > > > > Phylink needs slightly more information than phylink_get_interfaces()
> > > > > allows us to get from the DSA drivers - we need the MAC capabilities.
> > > > > Replace the phylink_get_interfaces() method with phylink_get_caps() to
> > > > > allow DSA drivers to fill in the phylink_config MAC capabilities field
> > > > > as well.
> > > > > 
> > > > > Signed-off-by: Russell King (Oracle) <[email-masked]>
> > > > > ---
> > > > 
> > > > The effects of submitting new API without any user :)
> > > 
> > > It had users at the time, but they were not submitted, and the addition
> > > of MAC capabilities was a future development. Had they been submitted at
> > > the time, then they would have required updating too.
> > 
> > That was a bit rushed... to explain more fully.
> > 
> > Prior to the merge window, the development work was centered around
> > only eliminating the PHY_INTERFACE_MODE_xxx checks and the complexity
> > that the PHY_INTERFACE_MODE_NA technique brought into the many
> > validation functions. Users of this had already been merged, and
> > included mvneta and mvpp2. See these commits, which are all in
> > v5.16-rc1:
> > 
> > b63f1117aefc net: mvpp2: clean up mvpp2_phylink_validate()
> > 76947a635874 net: mvpp2: drop use of phylink_helper_basex_speed()
> > 6c0c4b7ac06f net: mvpp2: remove interface checks in mvpp2_phylink_validate()
> > 8498e17ed4c5 net: mvpp2: populate supported_interfaces member
> > 
> > 099cbfa286ab net: mvneta: drop use of phylink_helper_basex_speed()
> > d9ca72807ecb net: mvneta: remove interface checks in mvneta_validate()
> > fdedb695e6a8 net: mvneta: populate supported_interfaces member
> > 
> > The original commit adding phylink_get_interfaces() extended this
> > into DSA, and the intention was to submit at least mv88e6xxx, but
> > it was too close to the merge window to do so.
> > 
> > Through making that change and eventually eliminating the basex helper
> > from all drivers that were using it, thereby making the validate()
> > behaviour much cleaner, it then became clear that it was possible to
> > push this cleanup further by also introducing a MAC capabilities field
> > to phylink_config.
> > 
> > The addition of the supported_interfaces member and the addition of the
> > mac_capabilities member are two entirely separate developments, but I
> > have now chosen to combine the two after the merge window in order to
> > reduce the number of patches. They were separate patches in my tree up
> > until relatively recently, and still are for the mt7530 and b53 drivers
> > currently.
> > 
> > So no, this is not "The effects of submitting new API without any user".
> > 
> > Thanks.
> 
> Ok, the patch is not the effect of submitting new API without any user.
> It is just the effect of more development done to API without any user,
> without any causal relationship between the two. My bad.

I do wonder whether you intentionally missed where I said "It had users
at the time, but they were not submitted". This is why we don't get on
well, you're always so confrontational.

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