Hi all,
We encountered a problem that if there are two devices in kernel v5.0 with
marvell phy 88E1510 connect with each other directly, one with autoneg on and
the other off. The one who has disabled auto-negotiation will failed on setting
speed and duplex mode. Their speed and duplex mode will go to 10M/half. And
no matter what speed and duplex we set by ethtool -s ethx speed XX duplex full,
they will go to 10M/half. If we disable auto-negotiation on both sides and set
same speed and duplex, there speed and duplex mode will go to Unknown.
I found that m88e1121_config_aneg() has been modified by commit
d6ab93364734 net: phy: marvell: Avoid unnecessary soft reset
And in that patch, genphy_soft_reset() was moved below genphy_config_aneg(),
which caused value of speed and duplex in MII_BMCR was cleared. And then they
will go to 10M/half.
static int m88e1121_config_aneg(struct phy_device *phydev)
{
+ int changed = 0;
int err = 0;
if (phy_interface_is_rgmii(phydev)) {
@@ -487,15 +455,26 @@ static int m88e1121_config_aneg(struct phy_device
*phydev)
return err;
}
- err = genphy_soft_reset(phydev);
+ err = marvell_set_polarity(phydev, phydev->mdix_ctrl);
if (err < 0)
return err;
- err = marvell_set_polarity(phydev, phydev->mdix_ctrl);
+ changed = err;
+
+ err = genphy_config_aneg(phydev);
if (err < 0)
return err;
- return genphy_config_aneg(phydev);
+ if (phydev->autoneg != autoneg || changed) {
+ /* A software reset is used to ensure a "commit" of the
+ * changes is done.
+ */
+ err = genphy_soft_reset(phydev);
+ if (err < 0)
+ return err;
+ }
+
+ return 0;
}
I moved genphy_soft_reset back and it tested ok. And in my opinion, if we have
to do
soft reset after genphy_config_aneg(phydev), it should be like this:
if (phydev->autoneg != AUTONEG_ENABLE) {
int bmcr;
bmcr = phy_read(phydev, MII_BMCR);
if (bmcr < 0)
return bmcr;
err = phy_write(phydev, MII_BMCR, bmcr | BMCR_RESET);
if (err < 0)
return err;
}
The above code has been mentioned in commit 3438634456c4
net: phy: marvell: Use core genphy_soft_reset(), phy_write() was replaced by
genphy_soft_reset() in that patch.
I think this issue will affect devices with Marvell PHY ID 88E1112/1111/1121/
1318/1240/1510/1540/1545/6390.
If there are any better info or suggestion regarding this problem, it would be
very
helpful, thanks in advance.
reference:
[1] https://lore.kernel.org/patchwork/patch/991682/
[2] https://patchwork.ozlabs.org/patch/795435/
