| Message ID | E1gnAtB-0002Cz-Tr@rmk-PC.armlinux.org.uk |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | Revert "net: phy: marvell: avoid pause mode on SGMII-to-Copper for 88e151x" | expand |
On Fri, Jan 25, 2019 at 11:27:57PM +0000, Russell King wrote: > This reverts commit 6623c0fba10ef45b64ca213ad5dec926f37fa9a0. > > The original diagnosis was incorrect: it appears that the NIC had > PHY polling mode enabled, which meant that it overwrote the PHYs > advertisement register during negotiation. Hi Russell The NIC wrote to PHY registers? The NIC reading the PHY in hardware is bad enough, but changing register as well is not good. What NIC is this? And do you have further patches to really disable PHY polling? > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On Sat, Jan 26, 2019 at 04:58:34PM +0100, Andrew Lunn wrote: > On Fri, Jan 25, 2019 at 11:27:57PM +0000, Russell King wrote: > > This reverts commit 6623c0fba10ef45b64ca213ad5dec926f37fa9a0. > > > > The original diagnosis was incorrect: it appears that the NIC had > > PHY polling mode enabled, which meant that it overwrote the PHYs > > advertisement register during negotiation. > > Hi Russell > > The NIC wrote to PHY registers? The NIC reading the PHY in hardware is > bad enough, but changing register as well is not good. Yep, it seems so. > What NIC is this? And do you have further patches to really disable > PHY polling? The PP2.2 in 8040 when used with the Marvell mvpp2x driver (that I have in my mcbin branch) - mainline's mvpp2 driver doesn't suffer from it as that disables polling mode. What made the diagnosis hard is that disabling phy polling by poking the SMI registers doesn't stop the PHY being polled, it seems to need something extra - and not having the documentation doesn't help. As I said, using a 'scope on the MDIO signal and noticing that the hardware was still polling the PHY despite the poll bit disabled was key to getting to the bottom of what was really going on. Once I arranged to clear it during driver initialisation and rebooted the platform, the PHY then had the expected behaviour.
From: Russell King <rmk+kernel@armlinux.org.uk> Date: Fri, 25 Jan 2019 23:27:57 +0000 > This reverts commit 6623c0fba10ef45b64ca213ad5dec926f37fa9a0. > > The original diagnosis was incorrect: it appears that the NIC had > PHY polling mode enabled, which meant that it overwrote the PHYs > advertisement register during negotiation. > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> The code in the 'net' tree has the linkmode_clear_bit() etc. changes that were done to this driver, so the pause variable is not there and the pause bits are adjusted using that helper. Please respin this against current sources so that it actually applies. Thank you.
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index cbec296107bd..d29c0c3d0c3e 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -847,8 +847,6 @@ static int m88e1510_config_init(struct phy_device *phydev) /* SGMII-to-Copper mode initialization */ if (phydev->interface == PHY_INTERFACE_MODE_SGMII) { - u32 pause; - /* Select page 18 */ err = marvell_set_page(phydev, 18); if (err < 0) @@ -871,16 +869,6 @@ static int m88e1510_config_init(struct phy_device *phydev) err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE); if (err < 0) return err; - - /* There appears to be a bug in the 88e1512 when used in - * SGMII to copper mode, where the AN advertisement register - * clears the pause bits each time a negotiation occurs. - * This means we can never be truely sure what was advertised, - * so disable Pause support. - */ - pause = SUPPORTED_Pause | SUPPORTED_Asym_Pause; - phydev->supported &= ~pause; - phydev->advertising &= ~pause; } return m88e1318_config_init(phydev);
This reverts commit 6623c0fba10ef45b64ca213ad5dec926f37fa9a0. The original diagnosis was incorrect: it appears that the NIC had PHY polling mode enabled, which meant that it overwrote the PHYs advertisement register during negotiation. Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> --- drivers/net/phy/marvell.c | 12 ------------ 1 file changed, 12 deletions(-)