On Wed, 12 Aug 2020 16:48:37 +0100 Russell King - ARM Linux admin <li...@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 <li...@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
