On Wed, 12 Aug 2020 16:54:41 +0100 Russell King - ARM Linux admin <li...@armlinux.org.uk> wrote:
> 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. > So I am encountering this now when testing my SFP + marvell10g patches on a board with SFP cage on a DSA port. I get what you find cumbersome, but I don't think there is other reasonable solution here. We already have .phylink_validate, which works with ethtool mode mask. So maybe a .phylink_config_init ? Marek
