[PATCH net 1/1] net: phy: dp83867: perform phy reset after modifying auto-neg setting

[PATCH net 1/1] net: phy: dp83867: perform phy reset after modifying auto-neg setting

[Choong Yong Liang]

From: Song Yoong Siang <yoong.siang.song@intel.com>

In the case where TI DP83867 is connected back-to-back with another TI
DP83867, SEEDs match will occurs when advertised link speed is changed from
100 Mbps to 1 Gbps, which causing Master/Slave resolution fails and restart
of the auto-negotiation. As a result, TI DP83867 copper auto-negotiation
completion will takes up to 15 minutes.

To resolve the issue, this patch implemented phy reset (software restart
which perform a full reset, but not including registers) immediately after
modifying auto-negotiation setting. By applying reset to the phy, it would
also reset the lfsr which would increase the probability of SEEDS being
different and help in Master/Slave resolution. Gerome from TI has confirmed
that there is no harm in adding soft restart in auto-negotiation
programming flow, even though the system is not facing SEEDs match issue.

Fixes: 2a10154abcb7 ("net: phy: dp83867: Add TI dp83867 phy")
Cc: <stable@vger.kernel.org> # 5.4.x
Signed-off-by: Song Yoong Siang <yoong.siang.song@intel.com>
Reviewed-by: Cacho Gerome <g-cacho@ti.com>
Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com>
---
 drivers/net/phy/dp83867.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index 6939563d3b7c..6e4b10f35696 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -925,6 +925,17 @@ static void dp83867_link_change_notify(struct phy_device *phydev)
 	}
 }
 
+static int dp83867_config_aneg(struct phy_device *phydev)
+{
+	int err;
+
+	err = genphy_config_aneg(phydev);
+	if (err < 0)
+		return err;
+
+	return dp83867_phy_reset(phydev);
+}
+
 static struct phy_driver dp83867_driver[] = {
 	{
 		.phy_id		= DP83867_PHY_ID,
@@ -951,6 +962,7 @@ static struct phy_driver dp83867_driver[] = {
 		.resume		= genphy_resume,
 
 		.link_change_notify = dp83867_link_change_notify,
+		.config_aneg	= dp83867_config_aneg,
 	},
 };
 module_phy_driver(dp83867_driver);
-- 
2.25.1

Re: [PATCH net 1/1] net: phy: dp83867: perform phy reset after modifying auto-neg setting

[Andrew Lunn]

On Thu, Sep 15, 2022 at 05:02:58AM -0400, Choong Yong Liang wrote:
> From: Song Yoong Siang <yoong.siang.song@intel.com>
> 
> In the case where TI DP83867 is connected back-to-back with another TI
> DP83867, SEEDs match will occurs when advertised link speed is changed from
> 100 Mbps to 1 Gbps, which causing Master/Slave resolution fails and restart
> of the auto-negotiation. As a result, TI DP83867 copper auto-negotiation
> completion will takes up to 15 minutes.

802.3 seems to indicate that if the seeds match, it should immediately
generate a new random seed and try the master/slave selection
again. So you seem to be saying this part is broken.

> To resolve the issue, this patch implemented phy reset (software restart
> which perform a full reset, but not including registers) immediately after
> modifying auto-negotiation setting. By applying reset to the phy, it would
> also reset the lfsr which would increase the probability of SEEDS being
> different and help in Master/Slave resolution.

So this just increases the likelihood of different seed values. The 15
minute wait could still happen. The link peer seed value should be
accessible via the autoneg page registers. Can the local seed value be
determined? Linux can then detect that the same seed is being used and
give the PHY a kick to pick a new seed.

Is there anything useful in register 10 bit, 15?

So long as you don't use interrupts, phylib is going to poll the PHY
once per second. It seems like you can get a better workaround by
using that polling to check if the PHY as got stuck, and give it a
kick.

	Andrew