Message-ID: <YZaIeiOyhqyVNG8D@shell.armlinux.org.uk>
Date:   Thu, 18 Nov 2021 17:08:10 +0000
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Marek Behún <kabel@...nel.org>,
        netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>,
        David Miller <davem@...emloft.net>,
        Rob Herring <robh+dt@...nel.org>, devicetree@...r.kernel.org
Subject: Re: [PATCH net-next 4/8] net: phylink: update supported_interfaces
 with modes from fwnode

On Thu, Nov 18, 2021 at 05:33:33PM +0100, Andrew Lunn wrote:
> > > +	/* If supported is empty, just copy modes defined in fwnode. */
> > > +	if (phy_interface_empty(supported))
> > > +		return phy_interface_copy(supported, modes);
> > 
> > Doesn't this mean we always end up with the supported_interfaces field
> > filled in, even for drivers that haven't yet been converted? It will
> > have the effect of locking the driver to the interface mode in "modes"
> > where only one interface mode is mentioned in DT.
> > 
> > At the moment, I think the only drivers that would be affected would be
> > some DSA drivers, stmmac and macb as they haven't yet been converted.
> 
> Hi Russell
> 
> What do you think the best way forward is? Got those converted before
> merging this?

The situation is as follows:

- For macb, I have a bunch of patches ready to go against net-next (in
  git log order):
  net: macb: use phylink_generic_validate()
  net: macb: clean up macb_validate()
  net: macb: remove interface checks in macb_validate()
  net: macb: populate supported_interfaces member

  but I think Sean and myself need to finish discussing PCS capabilities
  before I send those.

- For mv88e6xxx DSA, I have some patches - I need to check with Marek
  whether he has any further changes for those as he's been looking over
  them and checking with the Marvell specs before I can send them.

- For mt7530 DSA, I also have some patches, but no way to test them.

All of the above patches are in my net-queue branch:
  http://git.armlinux.org.uk/cgit/linux-arm.git/log/?h=net-queue

- For stmmac, I pinged Jose Abreu about it. stmmac's validate function
  does not check the interface mode at all if there is no XPCS attached.
  Jose says that which interface modes are supported is up to the
  integrator to decide, and it seems there is nothing in the current
  driver to work out which interface modes can be supported.

  If there is an XPCS attached, it defers interface mode checks to
  xpcs_validate(), which uses the interface mode to walk an array to
  find a list of linkmodes that the PCS supports.

  So, I think it's going to take a bit of unpicking to work out what to
  do here, and may depend on what we come up with with Sean for PCS.

That leaves quite a number of DSA drivers untouched.

So, I think we need to realise that even though we have all the users
in the kernel, making changes to phylink's API is always going to be
difficult, and we always need to maintain compatibility for old ways
of doing stuff - at least until every user can be converted. However,
that brings with it its own problem - there is then no motivation for
people to adapt to the new way. This can be seen as we have the
compatibility for calling mac_config() whenever the link comes up
16 months after the PCS stuff was introduced. One of the problem
drivers for this is mtk_eth_soc:

http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=net-queue&id=278da006a936c0743c7fc261c23e7de992ac78e6

René van Dorst said in response to me posting that patch in July 2020:
> I know, you have pointed that out before. But I don't know how to fix
> mtk_gmac0_rgmii_adjust(). This function changes the PLL of the MAC.
> But without documentation I am not sure what all the bits are used
> for.

and my attempts to discuss a way forward didn't get a reply. So,
mtk_eth_soc has become an example of a problematical driver that makes
ongoing phylink changes difficult, and I fear stmmac may become another
example.

I'm quite certain that as we try to develop phylink, such as adding
extra facilities like specifying the interface modes, we're going to
end up running into these kinds of problems that we can't solve, and
we are going to have to keep compatibility for the old ways of doing
stuff going for years to come - which is just going to get more and
more painful.

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

Powered by blists - more mailing lists