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

From: "Marek Behún" <marek.behun@nic.cz>
To: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Maxime Chevallier <maxime.chevallier@bootlin.com>,
	Baruch Siach <baruch@tkos.co.il>, Chris Healy <cphealy@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	netdev@vger.kernel.org
Subject: Re: [PATCH RFC russell-king 3/4] net: phy: marvell10g: change MACTYPE according to phydev->interface
Date: Wed, 12 Aug 2020 16:44:31 +0200	[thread overview]
Message-ID: <20200812164431.34cf569f@dellmb.labs.office.nic.cz> (raw)
In-Reply-To: <20200811152144.GN1551@shell.armlinux.org.uk>

On Tue, 11 Aug 2020 16:21:44 +0100
Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:

> On Tue, Aug 11, 2020 at 12:06:44AM +0200, Marek Behún wrote:
> > RollBall SFPs contain Marvell 88X3310 PHY, but they have
> > configuration pins strapped so that MACTYPE is configured in XFI
> > with Rate Matching mode.
> > 
> > When these SFPs are inserted into a device which only supports lower
> > speeds on host interface, we need to configure the MACTYPE to a mode
> > in which the H unit changes SerDes speed according to speed on the
> > copper interface. I chose to use the
> > 10GBASE-R/5GBASE-R/2500BASE-X/SGMII with AN mode.  
> 
> We actually need to have more inteligence in the driver, since we
> actually assume that it is in the 10GBASE-R/5GBASE-R/2500BASE-X/SGMII
> mode without really checking.
> 
> Note that there are differences in the way the mactype field is
> interpreted depending on exactly what chip we have.  For example,
> 3310 and 3340 are different.  That said, I've not heard of anyone
> using the 3340 yet.
> 

Russell, I am aware that MACTYPE modes are interpreted differently for
3310 vs 3340, but this only affects modes 0-3, which the driver does
not check for even after applying my patch.

There is another problem though: I think the PHY driver, when deciding
whether to set MACTYPE from the XFI with rate matching mode to the
10GBASE-R/5GBASE-R/2500BASE-X/SGMII with AN mode, should check which
modes the underlying MAC support.

If the underlying MAC supports only XFI mode, than the MACTYPE should
be set to XFI with rate matching. But on Omnia for example the MAC
supports SGMII/1000base-s/2500base-x, so on Omnia the MACTYPE should be
changed.

Currently this information is given in your repository by the mvneta
driver to phylink in the call to phylink_create. But there is no way
for the PHY driver to get this information from phylink currently, and
even if phylink exposed a function to return the config member of
struct phylink, the problem is that at the time when mv3310_power_up is
called, the phydev->phylink is not yet set (this is done in
phylink_bringup_phy, and mv3310_power_up is called sometime in the
phylink_attach_phy).

Marek