Skip to content

Commit fea5d56

Browse files
committed
Merge branch 'use-phylib-for-reset-randomization-and-adjustable-polling'
Oleksij Rempel says: ==================== Use PHYlib for reset randomization and adjustable polling This patch set tackles a DP83TG720 reset lock issue and improves PHY polling. Rather than adding a separate polling worker to randomize PHY resets, I chose to extend the PHYlib framework - which already handles most of the needed functionality - with adjustable polling. This approach not only addresses the DP83TG720-specific problem (where synchronized resets can lock the link) but also lays the groundwork for optimizing PHY stats polling across all PHY drivers. With generic PHY stats coming in, we can adjust the polling interval based on hardware characteristics, such as using longer intervals for PHYs with stable HW counters or shorter ones for high-speed links prone to counter overflows. Patch version changes are tracked in separate patches. ==================== Link: https://patch.msgid.link/20250210082358.200751-1-o.rempel@pengutronix.de Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2 parents 1273919 + e252af1 commit fea5d56

File tree

3 files changed

+111
-1
lines changed

3 files changed

+111
-1
lines changed

drivers/net/phy/dp83tg720.c

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,31 @@
44
*/
55
#include <linux/bitfield.h>
66
#include <linux/ethtool_netlink.h>
7+
#include <linux/jiffies.h>
78
#include <linux/kernel.h>
89
#include <linux/module.h>
910
#include <linux/phy.h>
11+
#include <linux/random.h>
1012

1113
#include "open_alliance_helpers.h"
1214

15+
/*
16+
* DP83TG720S_POLL_ACTIVE_LINK - Polling interval in milliseconds when the link
17+
* is active.
18+
* DP83TG720S_POLL_NO_LINK_MIN - Minimum polling interval in milliseconds when
19+
* the link is down.
20+
* DP83TG720S_POLL_NO_LINK_MAX - Maximum polling interval in milliseconds when
21+
* the link is down.
22+
*
23+
* These values are not documented or officially recommended by the vendor but
24+
* were determined through empirical testing. They achieve a good balance in
25+
* minimizing the number of reset retries while ensuring reliable link recovery
26+
* within a reasonable timeframe.
27+
*/
28+
#define DP83TG720S_POLL_ACTIVE_LINK 1000
29+
#define DP83TG720S_POLL_NO_LINK_MIN 100
30+
#define DP83TG720S_POLL_NO_LINK_MAX 1000
31+
1332
#define DP83TG720S_PHY_ID 0x2000a284
1433

1534
/* MDIO_MMD_VEND2 registers */
@@ -371,6 +390,13 @@ static int dp83tg720_read_status(struct phy_device *phydev)
371390
if (ret)
372391
return ret;
373392

393+
/* Sleep 600ms for PHY stabilization post-reset.
394+
* Empirically chosen value (not documented).
395+
* Helps reduce reset bounces with link partners having similar
396+
* issues.
397+
*/
398+
msleep(600);
399+
374400
/* After HW reset we need to restore master/slave configuration.
375401
* genphy_c45_pma_baset1_read_master_slave() call will be done
376402
* by the dp83tg720_config_aneg() function.
@@ -498,6 +524,57 @@ static int dp83tg720_probe(struct phy_device *phydev)
498524
return 0;
499525
}
500526

527+
/**
528+
* dp83tg720_get_next_update_time - Determine the next update time for PHY
529+
* state
530+
* @phydev: Pointer to the phy_device structure
531+
*
532+
* This function addresses a limitation of the DP83TG720 PHY, which cannot
533+
* reliably detect or report a stable link state. To recover from such
534+
* scenarios, the PHY must be periodically reset when the link is down. However,
535+
* if the link partner also runs Linux with the same driver, synchronized reset
536+
* intervals can lead to a deadlock where the link never establishes due to
537+
* simultaneous resets on both sides.
538+
*
539+
* To avoid this, the function implements randomized polling intervals when the
540+
* link is down. It ensures that reset intervals are desynchronized by
541+
* introducing a random delay between a configured minimum and maximum range.
542+
* When the link is up, a fixed polling interval is used to minimize overhead.
543+
*
544+
* This mechanism guarantees that the link will reestablish within 10 seconds
545+
* in the worst-case scenario.
546+
*
547+
* Return: Time (in jiffies) until the next update event for the PHY state
548+
* machine.
549+
*/
550+
static unsigned int dp83tg720_get_next_update_time(struct phy_device *phydev)
551+
{
552+
unsigned int next_time_jiffies;
553+
554+
if (phydev->link) {
555+
/* When the link is up, use a fixed 1000ms interval
556+
* (in jiffies)
557+
*/
558+
next_time_jiffies =
559+
msecs_to_jiffies(DP83TG720S_POLL_ACTIVE_LINK);
560+
} else {
561+
unsigned int min_jiffies, max_jiffies, rand_jiffies;
562+
563+
/* When the link is down, randomize interval between min/max
564+
* (in jiffies)
565+
*/
566+
min_jiffies = msecs_to_jiffies(DP83TG720S_POLL_NO_LINK_MIN);
567+
max_jiffies = msecs_to_jiffies(DP83TG720S_POLL_NO_LINK_MAX);
568+
569+
rand_jiffies = min_jiffies +
570+
get_random_u32_below(max_jiffies - min_jiffies + 1);
571+
next_time_jiffies = rand_jiffies;
572+
}
573+
574+
/* Ensure the polling time is at least one jiffy */
575+
return max(next_time_jiffies, 1U);
576+
}
577+
501578
static struct phy_driver dp83tg720_driver[] = {
502579
{
503580
PHY_ID_MATCH_MODEL(DP83TG720S_PHY_ID),
@@ -516,6 +593,7 @@ static struct phy_driver dp83tg720_driver[] = {
516593
.get_link_stats = dp83tg720_get_link_stats,
517594
.get_phy_stats = dp83tg720_get_phy_stats,
518595
.update_stats = dp83tg720_update_stats,
596+
.get_next_update_time = dp83tg720_get_next_update_time,
519597

520598
.suspend = genphy_suspend,
521599
.resume = genphy_resume,

drivers/net/phy/phy.c

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1501,6 +1501,24 @@ void phy_free_interrupt(struct phy_device *phydev)
15011501
}
15021502
EXPORT_SYMBOL(phy_free_interrupt);
15031503

1504+
/**
1505+
* phy_get_next_update_time - Determine the next PHY update time
1506+
* @phydev: Pointer to the phy_device structure
1507+
*
1508+
* This function queries the PHY driver to get the time for the next polling
1509+
* event. If the driver does not implement the callback, a default value is
1510+
* used.
1511+
*
1512+
* Return: The time for the next polling event in jiffies
1513+
*/
1514+
static unsigned int phy_get_next_update_time(struct phy_device *phydev)
1515+
{
1516+
if (phydev->drv && phydev->drv->get_next_update_time)
1517+
return phydev->drv->get_next_update_time(phydev);
1518+
1519+
return PHY_STATE_TIME;
1520+
}
1521+
15041522
enum phy_state_work {
15051523
PHY_STATE_WORK_NONE,
15061524
PHY_STATE_WORK_ANEG,
@@ -1580,7 +1598,8 @@ static enum phy_state_work _phy_state_machine(struct phy_device *phydev)
15801598
* called from phy_disconnect() synchronously.
15811599
*/
15821600
if (phy_polling_mode(phydev) && phy_is_started(phydev))
1583-
phy_queue_state_machine(phydev, PHY_STATE_TIME);
1601+
phy_queue_state_machine(phydev,
1602+
phy_get_next_update_time(phydev));
15841603

15851604
return state_work;
15861605
}

include/linux/phy.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,6 +1270,19 @@ struct phy_driver {
12701270
*/
12711271
int (*led_polarity_set)(struct phy_device *dev, int index,
12721272
unsigned long modes);
1273+
1274+
/**
1275+
* @get_next_update_time: Get the time until the next update event
1276+
* @dev: PHY device
1277+
*
1278+
* Callback to determine the time (in jiffies) until the next
1279+
* update event for the PHY state machine. Allows PHY drivers to
1280+
* dynamically adjust polling intervals based on link state or other
1281+
* conditions.
1282+
*
1283+
* Returns the time in jiffies until the next update event.
1284+
*/
1285+
unsigned int (*get_next_update_time)(struct phy_device *dev);
12731286
};
12741287
#define to_phy_driver(d) container_of_const(to_mdio_common_driver(d), \
12751288
struct phy_driver, mdiodrv)

0 commit comments

Comments
 (0)