Re: [PATCH RFC russell-king 3/4] net: phy: marvell10g: change MACTYPE according to phydev->interface

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

On Wed, Aug 12, 2020 at 05:44:36PM +0200, Andrew Lunn wrote:
> > I'm aware of that problem.  I have some experimental patches which add
> > PHY interface mode bitmaps to the MAC, PHY, and SFP module parsing
> > functions.  I have stumbled on some problems though - it's going to be
> > another API change (and people are already whinging about the phylink
> > API changing "too quickly", were too quickly seems to be defined as
> > once in three years), and in some cases, DSA, it's extremely hard to
> > work out how to properly set such a bitmap due to DSA's layered
> > approach.
> 
> Hi Russell
> 
> If DSAs layering is causing real problems, we could rip it out, and
> let the driver directly interact with phylink. I'm not opposed to
> that.

The reason I mentioned it is because I have this unpublished patch
(beware, whitespace damaged due to copy-n-pasted):

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index c1967e08b017..ba32492f3ec0 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1629,6 +1629,12 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev)

        dp->pl_config.dev = &slave_dev->dev;
        dp->pl_config.type = PHYLINK_NETDEV;
+       __set_bit(PHY_INTERFACE_MODE_SGMII,
+                 dp->pl_config.supported_interfaces);
+       __set_bit(PHY_INTERFACE_MODE_2500BASEX,
+                 dp->pl_config.supported_interfaces);
+       __set_bit(PHY_INTERFACE_MODE_1000BASEX,
+                 dp->pl_config.supported_interfaces);

        /* The get_fixed_state callback takes precedence over polling the
         * link GPIO in PHYLINK (see phylink_get_fixed_state).  Only set

Which clearly is a gross hack - this code certainly has no idea about
what interfaces the port itself supports.  How do we get around that
with DSA layering?

We could add yet-another-driver-call down into the DSA driver for it
to fill in that information and keep the current structure.  However,
is that really the best solution - to have lots of fine grained driver
calls?

DSA feels to be very cumbersome and awkward to modify at least to me.
Almost everything seems to be "add another driver call" at the DSA
layer, followed by "add another sub-driver call" at the mv88e6xxx
layer.

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