[PATCH net-next v1 0/3] dp83tg720: Reduce link recovery

[PATCH net-next v1 0/3] dp83tg720: Reduce link recovery

[Oleksij Rempel]

This patch series improves the link recovery behavior of the TI
DP83TG720 PHY driver.

Previously, we introduced randomized reset delay logic to avoid reset
collisions in multi-PHY setups. While this approach was functional, it
had notable drawbacks: unpredictable behavior, longer and more variable
link recovery times, and overall higher complexity in link handling.

With this new approach, we replace the randomized delay with
deterministic, role-specific delays in the PHY reset logic. This enables
us to:
- Remove the redundant empirical 600 ms delay in read_status()
- Drop the random polling interval logic
- Introduce a clean, adaptive polling strategy with consistent
behavior and improved responsiveness

As a result, the PHY is now able to recover link reliably in under
1000 ms

David Jander (3):
  net: phy: dp83tg720: implement soft reset with asymmetric delay
  net: phy: dp83tg720: remove redundant 600ms post-reset delay
  net: phy: dp83tg720: switch to adaptive polling and remove random
    delays

 drivers/net/phy/dp83tg720.c | 179 ++++++++++++++++++++++++------------
 1 file changed, 120 insertions(+), 59 deletions(-)

--
2.39.5

[PATCH net-next v1 1/3] net: phy: dp83tg720: implement soft reset with asymmetric delay

[Oleksij Rempel]

From: David Jander <david@protonic.nl>

Add a .soft_reset callback for the DP83TG720 PHY that issues a hardware
reset followed by an asymmetric post-reset delay. The delay differs
based on the PHY's master/slave role to avoid synchronized reset
deadlocks, which are known to occur when both link partners use
identical reset intervals.

The delay includes:
- a fixed 1ms wait to satisfy MDC access timing per datasheet, and
- an empirically chosen extra delay (97ms for master, 149ms for slave).

Co-developed-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: David Jander <david@protonic.nl>
---
 drivers/net/phy/dp83tg720.c | 75 ++++++++++++++++++++++++++++++++-----
 1 file changed, 65 insertions(+), 10 deletions(-)

diff --git a/drivers/net/phy/dp83tg720.c b/drivers/net/phy/dp83tg720.c
index 7e76323409c4..2c86d05bf857 100644
--- a/drivers/net/phy/dp83tg720.c
+++ b/drivers/net/phy/dp83tg720.c
@@ -12,6 +12,42 @@
 
 #include "open_alliance_helpers.h"
 
+/*
+ * DP83TG720 PHY Limitations and Workarounds
+ *
+ * The DP83TG720 1000BASE-T1 PHY has several limitations that require
+ * software-side mitigations. These workarounds are implemented throughout
+ * this driver. This section documents the known issues and their corresponding
+ * mitigation strategies.
+ *
+ * 1. Unreliable Link Detection and Synchronized Reset Deadlock
+ * ------------------------------------------------------------
+ * After a link loss or during link establishment, the DP83TG720 PHY may fail
+ * to detect or report link status correctly. To work around this, the PHY must
+ * be reset periodically when no link is detected.
+ *
+ * However, in point-to-point setups where both link partners use the same
+ * driver (e.g. Linux on both sides), a synchronized reset pattern may emerge.
+ * This leads to a deadlock, where both PHYs reset at the same time and
+ * continuously miss each other during auto-negotiation.
+ *
+ * To address this, the reset procedure includes two components:
+ *
+ * - A **fixed minimum delay of 1ms** after issuing a hardware reset, as
+ *   required by the "DP83TG720S-Q1 1000BASE-T1 Automotive Ethernet PHY with
+ *   SGMII and RGMII" datasheet. This ensures MDC access timing is respected
+ *   before any further MDIO operations.
+ *
+ * - An **additional asymmetric delay**, empirically chosen based on
+ *   master/slave role. This reduces the risk of synchronized resets on both
+ *   link partners. Values are selected to avoid periodic overlap and ensure
+ *   the link is re-established within a few cycles.
+ *
+ * The functions that implement this logic are:
+ * - dp83tg720_soft_reset()
+ * - dp83tg720_get_next_update_time()
+ */
+
 /*
  * DP83TG720S_POLL_ACTIVE_LINK - Polling interval in milliseconds when the link
  *				 is active.
@@ -19,6 +55,10 @@
  *				 the link is down.
  * DP83TG720S_POLL_NO_LINK_MAX - Maximum polling interval in milliseconds when
  *				 the link is down.
+ * DP83TG720S_RESET_DELAY_MS_MASTER - Delay after a reset before attempting
+ *				 to establish a link again for master phy.
+ * DP83TG720S_RESET_DELAY_MS_SLAVE  - Delay after a reset before attempting
+ *				 to establish a link again for slave phy.
  *
  * These values are not documented or officially recommended by the vendor but
  * were determined through empirical testing. They achieve a good balance in
@@ -28,6 +68,8 @@
 #define DP83TG720S_POLL_ACTIVE_LINK		1000
 #define DP83TG720S_POLL_NO_LINK_MIN		100
 #define DP83TG720S_POLL_NO_LINK_MAX		1000
+#define DP83TG720S_RESET_DELAY_MS_MASTER	97
+#define DP83TG720S_RESET_DELAY_MS_SLAVE		149
 
 #define DP83TG720S_PHY_ID			0x2000a284
 
@@ -201,6 +243,26 @@ static int dp83tg720_update_stats(struct phy_device *phydev)
 	return 0;
 }
 
+static int dp83tg720_soft_reset(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = phy_write(phydev, DP83TG720S_PHY_RESET, DP83TG720S_HW_RESET);
+	if (ret)
+		return ret;
+
+	/* Include mandatory MDC-access delay (1ms) + extra asymmetric delay to
+	 * avoid synchronized reset deadlock. See section 1 in the top-of-file
+	 * comment block.
+	 */
+	if (phydev->master_slave_state == MASTER_SLAVE_STATE_SLAVE)
+		msleep(DP83TG720S_RESET_DELAY_MS_SLAVE);
+	else
+		msleep(DP83TG720S_RESET_DELAY_MS_MASTER);
+
+	return ret;
+}
+
 static void dp83tg720_get_link_stats(struct phy_device *phydev,
 				     struct ethtool_link_ext_stats *link_stats)
 {
@@ -477,19 +539,11 @@ static int dp83tg720_config_init(struct phy_device *phydev)
 {
 	int ret;
 
-	/* Software Restart is not enough to recover from a link failure.
-	 * Using Hardware Reset instead.
-	 */
-	ret = phy_write(phydev, DP83TG720S_PHY_RESET, DP83TG720S_HW_RESET);
+	/* Reset the PHY to recover from a link failure */
+	ret = dp83tg720_soft_reset(phydev);
 	if (ret)
 		return ret;
 
-	/* Wait until MDC can be used again.
-	 * The wait value of one 1ms is documented in "DP83TG720S-Q1 1000BASE-T1
-	 * Automotive Ethernet PHY with SGMII and RGMII" datasheet.
-	 */
-	usleep_range(1000, 2000);
-
 	if (phy_interface_is_rgmii(phydev)) {
 		ret = dp83tg720_config_rgmii_delay(phydev);
 		if (ret)
@@ -582,6 +636,7 @@ static struct phy_driver dp83tg720_driver[] = {
 
 	.flags          = PHY_POLL_CABLE_TEST,
 	.probe		= dp83tg720_probe,
+	.soft_reset	= dp83tg720_soft_reset,
 	.config_aneg	= dp83tg720_config_aneg,
 	.read_status	= dp83tg720_read_status,
 	.get_features	= genphy_c45_pma_read_ext_abilities,
-- 
2.39.5

[PATCH net-next v1 2/3] net: phy: dp83tg720: remove redundant 600ms post-reset delay

[Oleksij Rempel]

From: David Jander <david@protonic.nl>

Now that dp83tg720_soft_reset() introduces role-specific delays to avoid
reset synchronization deadlocks, the fixed 600ms post-reset delay in
dp83tg720_read_status() is no longer needed.

The new logic provides both the required MDC timing and link stabilization,
making the old empirical delay redundant and unnecessarily long.

Co-developed-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: David Jander <david@protonic.nl>
---
 drivers/net/phy/dp83tg720.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/net/phy/dp83tg720.c b/drivers/net/phy/dp83tg720.c
index 2c86d05bf857..00963ce0eb10 100644
--- a/drivers/net/phy/dp83tg720.c
+++ b/drivers/net/phy/dp83tg720.c
@@ -444,21 +444,11 @@ static int dp83tg720_read_status(struct phy_device *phydev)
 		/* According to the "DP83TC81x, DP83TG72x Software
 		 * Implementation Guide", the PHY needs to be reset after a
 		 * link loss or if no link is created after at least 100ms.
-		 *
-		 * Currently we are polling with the PHY_STATE_TIME (1000ms)
-		 * interval, which is still enough for not automotive use cases.
 		 */
 		ret = phy_init_hw(phydev);
 		if (ret)
 			return ret;
 
-		/* Sleep 600ms for PHY stabilization post-reset.
-		 * Empirically chosen value (not documented).
-		 * Helps reduce reset bounces with link partners having similar
-		 * issues.
-		 */
-		msleep(600);
-
 		/* After HW reset we need to restore master/slave configuration.
 		 * genphy_c45_pma_baset1_read_master_slave() call will be done
 		 * by the dp83tg720_config_aneg() function.
-- 
2.39.5

[PATCH net-next v1 3/3] net: phy: dp83tg720: switch to adaptive polling and remove random delays

[Oleksij Rempel]

From: David Jander <david@protonic.nl>

Now that the PHY reset logic includes a role-specific asymmetric delay
to avoid synchronized reset deadlocks, the previously used randomized
polling intervals are no longer necessary.

This patch removes the get_random_u32_below()-based logic and introduces
an adaptive polling strategy:
- Fast polling for a short time after link-down
- Slow polling if the link remains down
- Slower polling when the link is up

This balances CPU usage and responsiveness while avoiding reset
collisions. Additionally, the driver still relies on polling for
all link state changes, as interrupt support is not implemented,
and link-up events are not reliably signaled by the PHY.

The polling parameters are now documented in the updated top-of-file
comment.

Co-developed-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: David Jander <david@protonic.nl>
---
 drivers/net/phy/dp83tg720.c | 94 ++++++++++++++++++++++---------------
 1 file changed, 55 insertions(+), 39 deletions(-)

diff --git a/drivers/net/phy/dp83tg720.c b/drivers/net/phy/dp83tg720.c
index 00963ce0eb10..8c14b5a70cb7 100644
--- a/drivers/net/phy/dp83tg720.c
+++ b/drivers/net/phy/dp83tg720.c
@@ -46,15 +46,37 @@
  * The functions that implement this logic are:
  * - dp83tg720_soft_reset()
  * - dp83tg720_get_next_update_time()
+ *
+ * 2. Polling-Based Link Detection and IRQ Support
+ * -----------------------------------------------
+ * Due to the PHY-specific limitation described in section 1, link-up events
+ * cannot be reliably detected via interrupts on the DP83TG720. Therefore,
+ * polling is required to detect transitions from link-down to link-up.
+ *
+ * While link-down events *can* be detected via IRQs on this PHY, this driver
+ * currently does **not** implement interrupt support. As a result, all link
+ * state changes must be detected using polling.
+ *
+ * Polling behavior:
+ * - When the link is up: slow polling (e.g. 1s).
+ * - When the link just went down: fast polling for a short time.
+ * - When the link stays down: fallback to slow polling.
+ *
+ * This design balances responsiveness and CPU usage. It sacrifices fast link-up
+ * times in cases where the link is expected to remain down for extended periods,
+ * assuming that such systems do not require immediate reactivity.
  */
 
 /*
  * DP83TG720S_POLL_ACTIVE_LINK - Polling interval in milliseconds when the link
  *				 is active.
- * DP83TG720S_POLL_NO_LINK_MIN - Minimum polling interval in milliseconds when
- *				 the link is down.
- * DP83TG720S_POLL_NO_LINK_MAX - Maximum polling interval in milliseconds when
- *				 the link is down.
+ * DP83TG720S_POLL_NO_LINK     - Polling interval in milliseconds when the
+ *				 link is down.
+ * DP83TG720S_FAST_POLL_DURATION_MS - Timeout in milliseconds for no-link
+ *				 polling after which polling interval is
+ *				 increased.
+ * DP83TG720S_POLL_SLOW	       - Slow polling interval when there is no
+ *				 link for a prolongued period.
  * DP83TG720S_RESET_DELAY_MS_MASTER - Delay after a reset before attempting
  *				 to establish a link again for master phy.
  * DP83TG720S_RESET_DELAY_MS_SLAVE  - Delay after a reset before attempting
@@ -65,9 +87,10 @@
  * minimizing the number of reset retries while ensuring reliable link recovery
  * within a reasonable timeframe.
  */
-#define DP83TG720S_POLL_ACTIVE_LINK		1000
-#define DP83TG720S_POLL_NO_LINK_MIN		100
-#define DP83TG720S_POLL_NO_LINK_MAX		1000
+#define DP83TG720S_POLL_ACTIVE_LINK		421
+#define DP83TG720S_POLL_NO_LINK			149
+#define DP83TG720S_FAST_POLL_DURATION_MS	6000
+#define DP83TG720S_POLL_SLOW			1117
 #define DP83TG720S_RESET_DELAY_MS_MASTER	97
 #define DP83TG720S_RESET_DELAY_MS_SLAVE		149
 
@@ -166,6 +189,7 @@ struct dp83tg720_stats {
 
 struct dp83tg720_priv {
 	struct dp83tg720_stats stats;
+	unsigned long last_link_down_jiffies;
 };
 
 /**
@@ -569,50 +593,42 @@ static int dp83tg720_probe(struct phy_device *phydev)
 }
 
 /**
- * dp83tg720_get_next_update_time - Determine the next update time for PHY
- *                                  state
+ * dp83tg720_get_next_update_time - Return next polling interval for PHY state
  * @phydev: Pointer to the phy_device structure
  *
- * This function addresses a limitation of the DP83TG720 PHY, which cannot
- * reliably detect or report a stable link state. To recover from such
- * scenarios, the PHY must be periodically reset when the link is down. However,
- * if the link partner also runs Linux with the same driver, synchronized reset
- * intervals can lead to a deadlock where the link never establishes due to
- * simultaneous resets on both sides.
- *
- * To avoid this, the function implements randomized polling intervals when the
- * link is down. It ensures that reset intervals are desynchronized by
- * introducing a random delay between a configured minimum and maximum range.
- * When the link is up, a fixed polling interval is used to minimize overhead.
- *
- * This mechanism guarantees that the link will reestablish within 10 seconds
- * in the worst-case scenario.
+ * Implements adaptive polling interval logic depending on link state and
+ * downtime duration. See the "2. Polling-Based Link Detection and IRQ Support"
+ * section at the top of this file for details.
  *
- * Return: Time (in jiffies) until the next update event for the PHY state
- * machine.
+ * Return: Time (in jiffies) until the next poll
  */
 static unsigned int dp83tg720_get_next_update_time(struct phy_device *phydev)
 {
+	struct dp83tg720_priv *priv = phydev->priv;
 	unsigned int next_time_jiffies;
 
 	if (phydev->link) {
-		/* When the link is up, use a fixed 1000ms interval
-		 * (in jiffies)
-		 */
+		priv->last_link_down_jiffies = 0;
+
+		/* When the link is up, use a slower interval (in jiffies) */
 		next_time_jiffies =
 			msecs_to_jiffies(DP83TG720S_POLL_ACTIVE_LINK);
 	} else {
-		unsigned int min_jiffies, max_jiffies, rand_jiffies;
-
-		/* When the link is down, randomize interval between min/max
-		 * (in jiffies)
-		 */
-		min_jiffies = msecs_to_jiffies(DP83TG720S_POLL_NO_LINK_MIN);
-		max_jiffies = msecs_to_jiffies(DP83TG720S_POLL_NO_LINK_MAX);
-
-		rand_jiffies = min_jiffies +
-			get_random_u32_below(max_jiffies - min_jiffies + 1);
-		next_time_jiffies = rand_jiffies;
+		unsigned long now = jiffies;
+
+		if (!priv->last_link_down_jiffies)
+			priv->last_link_down_jiffies = now;
+
+		if (time_before(now, priv->last_link_down_jiffies +
+			  msecs_to_jiffies(DP83TG720S_FAST_POLL_DURATION_MS))) {
+			/* Link recently went down: fast polling */
+			next_time_jiffies =
+				msecs_to_jiffies(DP83TG720S_POLL_NO_LINK);
+		} else {
+			/* Link has been down for a while: slow polling */
+			next_time_jiffies =
+				msecs_to_jiffies(DP83TG720S_POLL_SLOW);
+		}
 	}
 
 	/* Ensure the polling time is at least one jiffy */
-- 
2.39.5

Re: [PATCH net-next v1 1/3] net: phy: dp83tg720: implement soft reset with asymmetric delay

[Andrew Lunn]

On Tue, Jun 10, 2025 at 10:10:57AM +0200, Oleksij Rempel wrote:
> From: David Jander <david@protonic.nl>
> 
> Add a .soft_reset callback for the DP83TG720 PHY that issues a hardware
> reset followed by an asymmetric post-reset delay. The delay differs
> based on the PHY's master/slave role to avoid synchronized reset
> deadlocks, which are known to occur when both link partners use
> identical reset intervals.
> 
> The delay includes:
> - a fixed 1ms wait to satisfy MDC access timing per datasheet, and
> - an empirically chosen extra delay (97ms for master, 149ms for slave).
> 
> Co-developed-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Signed-off-by: David Jander <david@protonic.nl>

Hi Oleksij

Since you are submitting it, your Signed-off-by should come last. The
order signifies the developers who passed it along towards merging.

> ---
>  drivers/net/phy/dp83tg720.c | 75 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 65 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/phy/dp83tg720.c b/drivers/net/phy/dp83tg720.c
> index 7e76323409c4..2c86d05bf857 100644
> --- a/drivers/net/phy/dp83tg720.c
> +++ b/drivers/net/phy/dp83tg720.c
> @@ -12,6 +12,42 @@
>  
>  #include "open_alliance_helpers.h"
>  
> +/*
> + * DP83TG720 PHY Limitations and Workarounds
> + *
> + * The DP83TG720 1000BASE-T1 PHY has several limitations that require
> + * software-side mitigations. These workarounds are implemented throughout
> + * this driver. This section documents the known issues and their corresponding
> + * mitigation strategies.

Is there a public errata you can reference?

> + *
> + * 1. Unreliable Link Detection and Synchronized Reset Deadlock
> + * ------------------------------------------------------------
> + * After a link loss or during link establishment, the DP83TG720 PHY may fail
> + * to detect or report link status correctly. To work around this, the PHY must
> + * be reset periodically when no link is detected.
> + *
> + * However, in point-to-point setups where both link partners use the same
> + * driver (e.g. Linux on both sides), a synchronized reset pattern may emerge.
> + * This leads to a deadlock, where both PHYs reset at the same time and
> + * continuously miss each other during auto-negotiation.
> + *
> + * To address this, the reset procedure includes two components:
> + *
> + * - A **fixed minimum delay of 1ms** after issuing a hardware reset, as
> + *   required by the "DP83TG720S-Q1 1000BASE-T1 Automotive Ethernet PHY with
> + *   SGMII and RGMII" datasheet. This ensures MDC access timing is respected
> + *   before any further MDIO operations.
> + *
> + * - An **additional asymmetric delay**, empirically chosen based on
> + *   master/slave role. This reduces the risk of synchronized resets on both
> + *   link partners. Values are selected to avoid periodic overlap and ensure
> + *   the link is re-established within a few cycles.

Maybe there is more about this in the following patches, i've not read
them yet. Does autoneg get as far as determining master/slave role? Or
are you assuming the link partners are somehow set as
prefer_master/prefer_slave?

	Andrew

Re: [PATCH net-next v1 1/3] net: phy: dp83tg720: implement soft reset with asymmetric delay

[Oleksij Rempel]

On Tue, Jun 10, 2025 at 02:16:39PM +0200, Andrew Lunn wrote:
> On Tue, Jun 10, 2025 at 10:10:57AM +0200, Oleksij Rempel wrote:
> > From: David Jander <david@protonic.nl>
> > 
> > Add a .soft_reset callback for the DP83TG720 PHY that issues a hardware
> > reset followed by an asymmetric post-reset delay. The delay differs
> > based on the PHY's master/slave role to avoid synchronized reset
> > deadlocks, which are known to occur when both link partners use
> > identical reset intervals.
> > 
> > The delay includes:
> > - a fixed 1ms wait to satisfy MDC access timing per datasheet, and
> > - an empirically chosen extra delay (97ms for master, 149ms for slave).
> > 
> > Co-developed-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > Signed-off-by: David Jander <david@protonic.nl>
> 
> Hi Oleksij
> 
> Since you are submitting it, your Signed-off-by should come last. The
> order signifies the developers who passed it along towards merging.

Ack. checkpatch blamed it, so i changed the order.

> > ---
> >  drivers/net/phy/dp83tg720.c | 75 ++++++++++++++++++++++++++++++++-----
> >  1 file changed, 65 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/net/phy/dp83tg720.c b/drivers/net/phy/dp83tg720.c
> > index 7e76323409c4..2c86d05bf857 100644
> > --- a/drivers/net/phy/dp83tg720.c
> > +++ b/drivers/net/phy/dp83tg720.c
> > @@ -12,6 +12,42 @@
> >  
> >  #include "open_alliance_helpers.h"
> >  
> > +/*
> > + * DP83TG720 PHY Limitations and Workarounds
> > + *
> > + * The DP83TG720 1000BASE-T1 PHY has several limitations that require
> > + * software-side mitigations. These workarounds are implemented throughout
> > + * this driver. This section documents the known issues and their corresponding
> > + * mitigation strategies.
> 
> Is there a public errata you can reference?

The PHY Reset Sequence on polling is described in the "DP83TC81x,
DP83TG72x Software Implementation Guide", A Appendix:
https://www.ti.com/lit/an/snla404/snla404.pdf

I do not have access to the errata sheet.

> > + *
> > + * 1. Unreliable Link Detection and Synchronized Reset Deadlock
> > + * ------------------------------------------------------------
> > + * After a link loss or during link establishment, the DP83TG720 PHY may fail
> > + * to detect or report link status correctly. To work around this, the PHY must
> > + * be reset periodically when no link is detected.
> > + *
> > + * However, in point-to-point setups where both link partners use the same
> > + * driver (e.g. Linux on both sides), a synchronized reset pattern may emerge.
> > + * This leads to a deadlock, where both PHYs reset at the same time and
> > + * continuously miss each other during auto-negotiation.
> > + *
> > + * To address this, the reset procedure includes two components:
> > + *
> > + * - A **fixed minimum delay of 1ms** after issuing a hardware reset, as
> > + *   required by the "DP83TG720S-Q1 1000BASE-T1 Automotive Ethernet PHY with
> > + *   SGMII and RGMII" datasheet. This ensures MDC access timing is respected
> > + *   before any further MDIO operations.
> > + *
> > + * - An **additional asymmetric delay**, empirically chosen based on
> > + *   master/slave role. This reduces the risk of synchronized resets on both
> > + *   link partners. Values are selected to avoid periodic overlap and ensure
> > + *   the link is re-established within a few cycles.
> 
> Maybe there is more about this in the following patches, i've not read
> them yet. Does autoneg get as far as determining master/slave role? Or
> are you assuming the link partners are somehow set as
> prefer_master/prefer_slave?

This PHY do not support autoneg (as required for automotive PHYs),
master/slave roles should be assigned by strapping or from software to
make the link functional.

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v1 1/3] net: phy: dp83tg720: implement soft reset with asymmetric delay

[Andrew Lunn]

> > > + *
> > > + * 1. Unreliable Link Detection and Synchronized Reset Deadlock
> > > + * ------------------------------------------------------------
> > > + * After a link loss or during link establishment, the DP83TG720 PHY may fail
> > > + * to detect or report link status correctly. To work around this, the PHY must
> > > + * be reset periodically when no link is detected.
> > > + *
> > > + * However, in point-to-point setups where both link partners use the same
> > > + * driver (e.g. Linux on both sides), a synchronized reset pattern may emerge.
> > > + * This leads to a deadlock, where both PHYs reset at the same time and
> > > + * continuously miss each other during auto-negotiation.
> > > + *
> > > + * To address this, the reset procedure includes two components:
> > > + *
> > > + * - A **fixed minimum delay of 1ms** after issuing a hardware reset, as
> > > + *   required by the "DP83TG720S-Q1 1000BASE-T1 Automotive Ethernet PHY with
> > > + *   SGMII and RGMII" datasheet. This ensures MDC access timing is respected
> > > + *   before any further MDIO operations.
> > > + *
> > > + * - An **additional asymmetric delay**, empirically chosen based on
> > > + *   master/slave role. This reduces the risk of synchronized resets on both
> > > + *   link partners. Values are selected to avoid periodic overlap and ensure
> > > + *   the link is re-established within a few cycles.
> > 
> > Maybe there is more about this in the following patches, i've not read
> > them yet. Does autoneg get as far as determining master/slave role? Or
> > are you assuming the link partners are somehow set as
> > prefer_master/prefer_slave?
> 
> This PHY do not support autoneg (as required for automotive PHYs),
> master/slave roles should be assigned by strapping or from software to
> make the link functional.

O.K, please extend the documentation to include this.

If they are incorrectly configured, both have the same role, do they
fail to get a link because of that? So it does not matter they both
have the same delays, it is not going to work whatever...

    Andrew

---
pw-bot: cr