Re: [RFC net-next PATCH 10/16] net: macb: Move PCS settings to PCS callbacks

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Sean Anderson <sean.anderson@seco.com>
Cc: netdev@vger.kernel.org, "David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	linux-kernel@vger.kernel.org, Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Claudiu Beznea <claudiu.beznea@microchip.com>,
	Nicolas Ferre <nicolas.ferre@microchip.com>
Subject: Re: [RFC net-next PATCH 10/16] net: macb: Move PCS settings to PCS callbacks
Date: Thu, 7 Oct 2021 11:34:05 +0100	[thread overview]
Message-ID: <YV7NHZRFZ9U3Xj8v@shell.armlinux.org.uk> (raw)
In-Reply-To: <YVzPgTAS0grKl6CN@shell.armlinux.org.uk>

On Tue, Oct 05, 2021 at 11:19:45PM +0100, Russell King (Oracle) wrote:
> On Tue, Oct 05, 2021 at 05:44:11PM -0400, Sean Anderson wrote:
> > At the very least, it should be clearer what things are allowed to fail
> > for what reasons. Several callbacks are void when things can fail under
> > the hood (e.g. link_up or an_restart). And the API seems to have been
> > primarily designed around PCSs which are tightly-coupled to their MACs.

I think what I'd like to see is rather than a validate() callback for
the PCS, a bitmap of phy_interface_t modes that the PCS supports,
which is the direction I was wanting to take phylink and SFP support.

We can then use that information to inform the interface selection in
phylink and avoid interface modes that the PCS doesn't support.

However, things get tricky when we have several PCS that we can switch
between, and the switching is done in mac_prepare(). The current PCS
(if there is even a PCS attached) may not represent the abilities
that are actually available.

It sounds easy - just throw in an extra validation step when calling
phylink_validate(), but it isn't that simple. To avoid breaking
existing setups, phylink would need to know of every PCS that _could_
be attached, and the decisions that the MAC makes to select which PCS
is going to be used for any configuration.

We could possibly introduce a .mac_select_pcs(interface) method
which the MAC could return the PCS it wishes to use for the interface
mode with the guarantee that the PCS it returns is suitable - and if
it returns NULL that means the interface mode is unsupported. That,
along with the bitmask would probably work.

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