On Tue, 11 Aug 2020 16:21:44 +0100 Russell King - ARM Linux admin <li...@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
