> -----Original Message----- > From: Russell King - ARM Linux [mailto:li...@armlinux.org.uk] > Sent: Thursday, January 12, 2017 4:42 PM > To: Kwok, WingMan > Cc: Andrew Lunn; Karicheri, Muralidharan; netdev@vger.kernel.org > Subject: Re: Marvell Phy (1510) issue since v4.7 kernel > > On Thu, Jan 12, 2017 at 09:31:27PM +0000, Kwok, WingMan wrote: > > > > > > I double checked that ours is actually a 88E1514. According > > > > to Marvell's brief description, it seems that it does support > > > > fiber. > > > > > > > > Reading page 18 reg 30 returns 6. > > > > > > O.K, that is consistent. Looking at the Marvell SDK: > > > > > > phy/Include/madApiDefs.h:#define MAD_SUB_88E1510 4 /* > 88E1510 */ > > > phy/Include/madApiDefs.h:#define MAD_SUB_88E1518 5 /* > 88E1518 */ > > > phy/Include/madApiDefs.h:#define MAD_SUB_88E1512 6 /* > > > 88E1512/88E1514 */ > > > > > > I took another look at the patch adding Fibre support. The > > > suspend/resume functions are wrong. > > > > > > /* Suspend the fiber mode first */ > > > if (!(phydev->supported & SUPPORTED_FIBRE)) { > > > > > > The ! should not be there. Does removing them both fix your > problem? > > > > > > > Hi Andrew, > > > > Thanks for taking the time to look into those patches. Yes we notice > > the error in the suspend/resume functions also. > > > > But our problem is caused by the read_status function: > > > > if ((phydev->supported & SUPPORTED_FIBRE)) { > > err = phy_write(phydev, MII_MARVELL_PHY_PAGE, > MII_M1111_FIBER); > > if (err < 0) > > goto error; > > > > err = marvell_read_status_page(phydev, MII_M1111_FIBER); > > if (err < 0) > > goto error; > > > > /* If the fiber link is up, it is the selected and used > link. > > * In this case, we need to stay in the fiber page. > > * Please to be careful about that, avoid to restore Copper > page > > * in other functions which could break the behaviour > > * for some fiber phy like 88E1512. > > * */ > > if (phydev->link) > > return 0; > > > > which keeps the fiber page if phydev->link is true (for some > > reason this is the case even though we are not using fiber) > > See below. > > > However, this causes a problem in kernel reboot because neither > > the suspend/resume is called to restore the copper page and > > u-boot marvell phy driver does not support 1510 fiber, which > > will then result in writing to the wrong phy regs and causes > > a sgmii auto-nego time out. > > So what we need to do is to have a .shutdown function installed - but > it's not that simple... > > .mdiodrv = { > .driver = { > .shutdown = mv88e1512_shutdown, > }, > }, > > in the appropriate phy_driver array element, and then have: > > static void mv88e1512_shutdown(struct device *dev) > { > struct phy_device *phydev = to_phy_device(dev); > > phy_write(phydev, MII_MARVELL_PHY_PAGE, MII_M1111_COPPER); > } > > which will restore the copper page on non-emergency reboots. For > emergency reboots, we're out of luck... the real fix would be for > uboot to ensure that when it detects this PHY, it ensures that the > right page is selected. > > Now, that all said, there's a bug in this code, which is fixed by > a patch I recently submitted. If you have an 88E151x connected via > SGMII, then the read_status function incorrectly believes it should > be reading from the fiber page - when in fact the fiber page is > reporting the status of the host-side link. So, before trying the > above modification, please try this patch - it's already been merged > into the net tree, so should hit -rc soon. > > My guess from what you've said above is that your 88E151x is connected > via SGMII, so you need the patch below rather than trying to install > a shutdown function. > > 8<==== > From: Russell King <rmk+ker...@armlinux.org.uk> > Subject: [PATCH] net: phy: marvell: fix Marvell 88E1512 used in SGMII > mode >
Hi Russell, Thanks for the explanations. Yes, our 88e1514 is connected to the host via sgmii and your patch does provide a solution to our problem. Regards, WingMan
