mbox series

[net-next,v2,0/3] net: dsa: allow phylink_mac_ops in DSA drivers

Message ID ZhbrbM+d5UfgafGp@shell.armlinux.org.uk (mailing list archive)
Headers show
Series net: dsa: allow phylink_mac_ops in DSA drivers | expand

Message

Russell King (Oracle) April 10, 2024, 7:41 p.m. UTC
Hi,

This series showcases my idea of moving the phylink_mac_ops into DSA
drivers, using mv88e6xxx as an example. Since I'm only changing one
driver, providing the mac_ops has to be optional and the existing shims
need to be kept for unconverted drivers.

The first patch introduces a new helper that converts from the
phylink_config structure that phylink uses to communicate with MAC
drivers to the dsa_port structure. From this, DSA drivers can get
the dsa_switch structure and thus their implementation specific
data structure, and they can also retrieve the port index.

The second patch adds the support to the core DSA layer to allow
DSA drivers to provide phylink_mac_ops.

The third patch converts mv88e6xxx to use this.

I initially made this change after adding yet more phylink to DSA
driver shims for my work with phylink-based EEE support, and decided
that it was getting silly to keep implementing more and more shims.
There are cases where shims don't work well - we had already tripped
over a case a few years ago when the phylink mac_select_pcs operation
was introduced. Phylink tested for the presence of this in the ops
structure, but with DSA shims, this doesn't necessarily mean that
the sub-driver supports this method. The only way to find that out
is to call the method with dummy values and check the return code.

The same thing was partly true when adding EEE support, and I ended
up with this in phylink to determine whether the MAC supported EEE:

+static bool phylink_mac_supports_eee(struct phylink *pl)
+{
+       return pl->mac_ops->mac_disable_tx_lpi &&
+              pl->mac_ops->mac_enable_tx_lpi &&
+              pl->config->lpi_capabilities;
+}

because merely testing for the presence of the operations is
insufficient when shims are involved - and it wasn't possible to call
these functions in the way that mac_select_pcs could be called.

So, I think it's time to get away from this shimming model and instead
have drivers directly interface to the various subsystems.

This converts mv88e6xxx. I have similar patches for other DSA drivers
that will be sent once this has been reviewed.

RFC->v1: fix up patch 2 to call the mac_link_down() method as pointed out
by Vladimir
v2: add checks for phylink_mac_* and adjust_link methods in dsa_switch_ops
    if phylink_mac_ops is populated. Simplify dsa_shared_port_link_down().

 drivers/net/dsa/mv88e6xxx/chip.c | 63 +++++++++++++++++++++++++---------------
 include/net/dsa.h                | 11 +++++++
 net/dsa/dsa.c                    | 11 +++++++
 net/dsa/port.c                   | 38 ++++++++++++++++--------
 4 files changed, 87 insertions(+), 36 deletions(-)

Comments

patchwork-bot+netdevbpf@kernel.org April 12, 2024, 3:10 a.m. UTC | #1
Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 10 Apr 2024 20:41:32 +0100 you wrote:
> Hi,
> 
> This series showcases my idea of moving the phylink_mac_ops into DSA
> drivers, using mv88e6xxx as an example. Since I'm only changing one
> driver, providing the mac_ops has to be optional and the existing shims
> need to be kept for unconverted drivers.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/3] net: dsa: introduce dsa_phylink_to_port()
    https://git.kernel.org/netdev/net-next/c/dd0c9855b413
  - [net-next,v2,2/3] net: dsa: allow DSA switch drivers to provide their own phylink mac ops
    https://git.kernel.org/netdev/net-next/c/cae425cb43fe
  - [net-next,v2,3/3] net: dsa: mv88e6xxx: provide own phylink MAC operations
    https://git.kernel.org/netdev/net-next/c/0cb6da0c487d

You are awesome, thank you!