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

["Marek Behún" <marek.behun@nic.cz>]

On Wed, 12 Aug 2020 16:48:37 +0100
Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:

> On Wed, Aug 12, 2020 at 05:37:16PM +0200, Marek Behún wrote:
> > On Wed, 12 Aug 2020 16:00:54 +0100
> > Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> >   
> > > On Wed, Aug 12, 2020 at 04:44:31PM +0200, Marek Behún wrote:  
> > > > 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.    
> > > 
> > > 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.
> > >   
> > 
> > If by your experimental patches you mean
> >   net: mvneta: fill in phy interface mode bitmap
> >   net: mvpp2: fill in phy interface mode bitmap
> > found here
> >   http://git.arm.linux.org.uk/cgit/linux-arm.git/log/?h=clearfog
> > I am currently working on top of them.
> >   
> > > Having bitmaps means that we can take the union of what the MAC
> > > and PHY supports, and decide which MACTYPE setting would be most
> > > suitable. However, to do that we're into also changing phylib's
> > > interfaces as well.
> > >   
> > > > 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).
> > > >   
> > > 
> > > We _really_ do not want phylib calling back into phylink
> > > functions. That would tie phylink functionality into phylib and
> > > cause problems when phylink is not being used.
> > > 
> > > I would prefer phylib to be passed "the MAC can use these
> > > interface types, and would prefer to use this interface type" and
> > > have the phylib layer (along with the phylib driver) make the
> > > decision about which mode should be used.  That also means that
> > > non-phylink MACs can also use it.
> > >   
> > 
> > I may try to propose something, but in the meantime do you think the
> > current version of the patch
> >   net: phy: marvell10g: change MACTYPE according to
> > phydev->interface is acceptable?  
> 
> Well, I have other questions about it.  Why are you doing it in
> the power_up function?  Do you find that the MACTYPE field is
> lost when clearing the power down bit?  From what I read, it should
> only change on hardware reset, and we don't hardware reset when we
> come out of power down - only software reset.
> 

Changing MACTYPE requires Port Software Reset. I was not sure if this
wouldn't break something. I am going to try to do this differently.

Marek