diff mbox series

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

Message ID 20220915090258.2154767-1-yong.liang.choong@linux.intel.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net,1/1] net: phy: dp83867: perform phy reset after modifying auto-neg setting | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 9 of 9 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 24 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Choong Yong Liang Sept. 15, 2022, 9:02 a.m. UTC 
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(+)

Comments

Andrew Lunn Sept. 15, 2022, 12:30 p.m. UTC | #1 
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
diff mbox series

Patch

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);