* [PATCH net v4 1/4] net: phy: add PHY package locking helpers
2026-08-07 7:07 [PATCH net v4 0/4] net: phy: dp83640: fix shared clock lifetime and probe error cleanup xuanqiang.luo
@ 2026-08-07 7:07 ` xuanqiang.luo
2026-08-07 13:36 ` Andrew Lunn
2026-08-07 7:07 ` [PATCH net v4 2/4] net: phy: dp83640: embed pin configuration in clock xuanqiang.luo
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: xuanqiang.luo @ 2026-08-07 7:07 UTC (permalink / raw)
To: netdev, andrew, kuba, richardcochran, hkallweit1
Cc: linux, davem, edumazet, pabeni, maxime.chevallier, luoxuanqiang
From: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
The PHY package API provides private data shared by all PHYs in a
package. Drivers are responsible for synchronizing access to this data,
but the API does not provide a lock for that purpose.
Add phy_package_lock() and phy_package_unlock() for drivers to serialize
access to package-private data, including its initialization.
Signed-off-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
---
drivers/net/phy/phy_package.c | 23 +++++++++++++++++++++++
drivers/net/phy/phylib.h | 2 ++
2 files changed, 25 insertions(+)
diff --git a/drivers/net/phy/phy_package.c b/drivers/net/phy/phy_package.c
index 16ae8d1c1f89a..735806c5bea88 100644
--- a/drivers/net/phy/phy_package.c
+++ b/drivers/net/phy/phy_package.c
@@ -52,6 +52,29 @@ void *phy_package_get_priv(struct phy_device *phydev)
}
EXPORT_SYMBOL_GPL(phy_package_get_priv);
+/**
+ * phy_package_lock - acquire the PHY package lock
+ * @phydev: PHY device that has joined the package
+ *
+ * Use this to serialize access to package-private data. Release the lock
+ * with phy_package_unlock().
+ */
+void phy_package_lock(struct phy_device *phydev)
+{
+ mutex_lock(&phydev->mdio.bus->shared_lock);
+}
+EXPORT_SYMBOL_GPL(phy_package_lock);
+
+/**
+ * phy_package_unlock - release the PHY package lock
+ * @phydev: PHY device that has joined the package
+ */
+void phy_package_unlock(struct phy_device *phydev)
+{
+ mutex_unlock(&phydev->mdio.bus->shared_lock);
+}
+EXPORT_SYMBOL_GPL(phy_package_unlock);
+
static int phy_package_address(struct phy_device *phydev,
unsigned int addr_offset)
{
diff --git a/drivers/net/phy/phylib.h b/drivers/net/phy/phylib.h
index 0fba245f97458..c6e26ac6b28f0 100644
--- a/drivers/net/phy/phylib.h
+++ b/drivers/net/phy/phylib.h
@@ -12,6 +12,8 @@ struct mii_bus;
struct device_node *phy_package_get_node(struct phy_device *phydev);
void *phy_package_get_priv(struct phy_device *phydev);
+void phy_package_lock(struct phy_device *phydev);
+void phy_package_unlock(struct phy_device *phydev);
int __phy_package_read(struct phy_device *phydev, unsigned int addr_offset,
u32 regnum);
int __phy_package_write(struct phy_device *phydev, unsigned int addr_offset,
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net v4 2/4] net: phy: dp83640: embed pin configuration in clock
2026-08-07 7:07 [PATCH net v4 0/4] net: phy: dp83640: fix shared clock lifetime and probe error cleanup xuanqiang.luo
2026-08-07 7:07 ` [PATCH net v4 1/4] net: phy: add PHY package locking helpers xuanqiang.luo
@ 2026-08-07 7:07 ` xuanqiang.luo
2026-08-07 13:38 ` Andrew Lunn
2026-08-07 7:07 ` [PATCH net v4 3/4] net: phy: dp83640: clear state after PTP registration failure xuanqiang.luo
2026-08-07 7:07 ` [PATCH net v4 4/4] net: phy: dp83640: fix per-bus clock lifetime xuanqiang.luo
3 siblings, 1 reply; 9+ messages in thread
From: xuanqiang.luo @ 2026-08-07 7:07 UTC (permalink / raw)
To: netdev, andrew, kuba, richardcochran, hkallweit1
Cc: linux, davem, edumazet, pabeni, maxime.chevallier, luoxuanqiang
From: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
The DP83640 has a fixed number of PTP pins, and its pin configuration
has the same lifetime as the per-bus clock. Allocating the configuration
separately adds an allocation failure path and requires a separate free.
Embed the pin configuration in struct dp83640_clock and point the PTP
clock information at the embedded array. This changes only the storage;
the pin functions remain configurable at runtime. It also allows all
per-bus clock storage to be managed as one allocation.
Signed-off-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
---
drivers/net/phy/dp83640.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 98472abdd3920..ba39d30b74705 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -146,6 +146,8 @@ struct dp83640_clock {
struct list_head phylist;
/* reference to our PTP hardware clock */
struct ptp_clock *ptp_clock;
+ /* protected by the PTP core pin configuration lock */
+ struct ptp_pin_desc pin_config[DP83640_N_PINS];
};
/* globals */
@@ -960,6 +962,7 @@ static void dp83640_clock_init(struct dp83640_clock *clock, struct mii_bus *bus)
mutex_init(&clock->extreg_lock);
mutex_init(&clock->clock_lock);
INIT_LIST_HEAD(&clock->phylist);
+ clock->caps.pin_config = clock->pin_config;
clock->caps.owner = THIS_MODULE;
sprintf(clock->caps.name, "dp83640 timer");
clock->caps.max_adj = 1953124;
@@ -977,9 +980,7 @@ static void dp83640_clock_init(struct dp83640_clock *clock, struct mii_bus *bus)
clock->caps.settime64 = ptp_dp83640_settime;
clock->caps.enable = ptp_dp83640_enable;
clock->caps.verify = ptp_dp83640_verify;
- /*
- * Convert the module param defaults into a dynamic pin configuration.
- */
+ /* Initialize the runtime pin configuration from gpio_tab. */
dp83640_gpio_defaults(clock->caps.pin_config);
/*
* Get a reference to this bus instance.
@@ -1031,13 +1032,6 @@ static struct dp83640_clock *dp83640_clock_get_bus(struct mii_bus *bus)
if (!clock)
goto out;
- clock->caps.pin_config = kzalloc_objs(struct ptp_pin_desc,
- DP83640_N_PINS);
- if (!clock->caps.pin_config) {
- kfree(clock);
- clock = NULL;
- goto out;
- }
dp83640_clock_init(clock, bus);
list_add_tail(&clock->list, &phyter_clocks);
out:
@@ -1509,7 +1503,6 @@ static void dp83640_remove(struct phy_device *phydev)
mutex_destroy(&clock->extreg_lock);
mutex_destroy(&clock->clock_lock);
put_device(&clock->bus->dev);
- kfree(clock->caps.pin_config);
kfree(clock);
}
}
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net v4 4/4] net: phy: dp83640: fix per-bus clock lifetime
2026-08-07 7:07 [PATCH net v4 0/4] net: phy: dp83640: fix shared clock lifetime and probe error cleanup xuanqiang.luo
` (2 preceding siblings ...)
2026-08-07 7:07 ` [PATCH net v4 3/4] net: phy: dp83640: clear state after PTP registration failure xuanqiang.luo
@ 2026-08-07 7:07 ` xuanqiang.luo
2026-08-07 13:59 ` Andrew Lunn
3 siblings, 1 reply; 9+ messages in thread
From: xuanqiang.luo @ 2026-08-07 7:07 UTC (permalink / raw)
To: netdev, andrew, kuba, richardcochran, hkallweit1
Cc: linux, davem, edumazet, pabeni, maxime.chevallier, luoxuanqiang
From: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
Commit 42e2a9e11a1d ("net: phy: dp83640: improve phydev and driver
removal handling") moved per-bus clock cleanup from module exit to the
remove path. This leaves two lifetime problems.
dp83640_clock_get_bus() publishes a newly allocated clock before the
driver allocates its per-PHY data and registers the PTP clock. If either
operation fails, no PHY is bound and the remove callback cannot release
the clock, leaking the clock and the MII bus device reference.
The remove path can also free a clock after dropping clock_lock. A
concurrent probe may already have found the clock under
phyter_clocks_lock and be waiting for clock_lock, allowing it to acquire
a freed mutex and access the freed clock.
Use the PHY package infrastructure for the per-bus clock. The package
table is scoped to each MII bus and holds the shared object until the
last joined PHY leaves. Serialize the one-time clock initialization with
the package lock because phy_package_probe_once() elects an initializer
but does not wait for initialization to finish.
Manage both the package reference and the per-PHY state with devres.
This is needed because dp83640_probe() may succeed before later PHY core
initialization fails, and the driver remove callback is not called for
that failure. Register the per-PHY cleanup action after the package
reference so probe unwinding first unregisters the PTP clock or removes
the PHY from the clock list, then releases the shared clock. Let devres
run the same action on normal driver detach.
Fixes: 42e2a9e11a1d ("net: phy: dp83640: improve phydev and driver removal handling")
Signed-off-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
---
drivers/net/phy/dp83640.c | 172 ++++++++++++++------------------------
drivers/ptp/Kconfig | 1 +
2 files changed, 65 insertions(+), 108 deletions(-)
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 7aa5cf0a7bb03..543f63eac9caf 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -8,6 +8,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/crc32.h>
+#include <linux/device/devres.h>
#include <linux/ethtool.h>
#include <linux/kernel.h>
#include <linux/list.h>
@@ -21,6 +22,7 @@
#include <linux/ptp_clock_kernel.h>
#include "dp83640_reg.h"
+#include "phylib.h"
#define DP83640_PHY_ID 0x20005ce1
#define PAGESEL 0x13
@@ -128,10 +130,6 @@ struct dp83640_private {
};
struct dp83640_clock {
- /* keeps the instance in the 'phyter_clocks' list */
- struct list_head list;
- /* we create one clock instance per MII bus */
- struct mii_bus *bus;
/* protects extended registers from concurrent access */
struct mutex extreg_lock;
/* remembers which page was last selected */
@@ -208,10 +206,6 @@ static void dp83640_gpio_defaults(struct ptp_pin_desc *pd)
}
}
-/* a list of clocks and a mutex to protect it */
-static LIST_HEAD(phyter_clocks);
-static DEFINE_MUTEX(phyter_clocks_lock);
-
static void rx_timestamp_work(struct work_struct *work);
/* extended register access functions */
@@ -955,10 +949,8 @@ static void decode_status_frame(struct dp83640_private *dp83640,
}
}
-static void dp83640_clock_init(struct dp83640_clock *clock, struct mii_bus *bus)
+static void dp83640_clock_init(struct dp83640_clock *clock)
{
- INIT_LIST_HEAD(&clock->list);
- clock->bus = bus;
mutex_init(&clock->extreg_lock);
mutex_init(&clock->clock_lock);
INIT_LIST_HEAD(&clock->phylist);
@@ -982,10 +974,6 @@ static void dp83640_clock_init(struct dp83640_clock *clock, struct mii_bus *bus)
clock->caps.verify = ptp_dp83640_verify;
/* Initialize the runtime pin configuration from gpio_tab. */
dp83640_gpio_defaults(clock->caps.pin_config);
- /*
- * Get a reference to this bus instance.
- */
- get_device(&bus->dev);
}
static int choose_this_phy(struct dp83640_clock *clock,
@@ -1000,51 +988,6 @@ static int choose_this_phy(struct dp83640_clock *clock,
return 0;
}
-static struct dp83640_clock *dp83640_clock_get(struct dp83640_clock *clock)
-{
- if (clock)
- mutex_lock(&clock->clock_lock);
- return clock;
-}
-
-/*
- * Look up and lock a clock by bus instance.
- * If there is no clock for this bus, then create it first.
- */
-static struct dp83640_clock *dp83640_clock_get_bus(struct mii_bus *bus)
-{
- struct dp83640_clock *clock = NULL, *tmp;
- struct list_head *this;
-
- mutex_lock(&phyter_clocks_lock);
-
- list_for_each(this, &phyter_clocks) {
- tmp = list_entry(this, struct dp83640_clock, list);
- if (tmp->bus == bus) {
- clock = tmp;
- break;
- }
- }
- if (clock)
- goto out;
-
- clock = kzalloc_obj(struct dp83640_clock);
- if (!clock)
- goto out;
-
- dp83640_clock_init(clock, bus);
- list_add_tail(&clock->list, &phyter_clocks);
-out:
- mutex_unlock(&phyter_clocks_lock);
-
- return dp83640_clock_get(clock);
-}
-
-static void dp83640_clock_put(struct dp83640_clock *clock)
-{
- mutex_unlock(&clock->clock_lock);
-}
-
static int dp83640_soft_reset(struct phy_device *phydev)
{
int ret;
@@ -1392,22 +1335,70 @@ static int dp83640_ts_info(struct mii_timestamper *mii_ts,
return 0;
}
-static int dp83640_probe(struct phy_device *phydev)
+static void dp83640_phy_release(void *data)
{
+ struct dp83640_private *dp83640 = data;
+ struct dp83640_private *tmp;
+ struct list_head *this, *next;
struct dp83640_clock *clock;
+ struct phy_device *phydev;
+
+ clock = dp83640->clock;
+ phydev = dp83640->phydev;
+ phydev->mii_ts = NULL;
+ cancel_delayed_work_sync(&dp83640->ts_work);
+ skb_queue_purge(&dp83640->rx_queue);
+ skb_queue_purge(&dp83640->tx_queue);
+
+ mutex_lock(&clock->clock_lock);
+ if (dp83640 == clock->chosen) {
+ ptp_clock_unregister(clock->ptp_clock);
+ clock->ptp_clock = NULL;
+ clock->chosen = NULL;
+ } else {
+ list_for_each_safe(this, next, &clock->phylist) {
+ tmp = list_entry(this, struct dp83640_private, list);
+ if (tmp == dp83640) {
+ list_del_init(&tmp->list);
+ break;
+ }
+ }
+ }
+ mutex_unlock(&clock->clock_lock);
+
+ phydev->default_timestamp = false;
+ phydev->priv = NULL;
+ kfree(dp83640);
+}
+
+static int dp83640_probe(struct phy_device *phydev)
+{
struct dp83640_private *dp83640;
- int err = -ENOMEM, i;
+ struct dp83640_clock *clock;
+ int err, i;
if (phydev->mdio.addr == BROADCAST_ADDR)
return 0;
- clock = dp83640_clock_get_bus(phydev->mdio.bus);
- if (!clock)
+ err = devm_phy_package_join(&phydev->mdio.dev, phydev,
+ BROADCAST_ADDR, sizeof(*clock));
+ if (err)
goto no_clock;
+ clock = phy_package_get_priv(phydev);
+ /* Ensure other PHY probes wait for shared clock initialization. */
+ phy_package_lock(phydev);
+ if (phy_package_probe_once(phydev))
+ dp83640_clock_init(clock);
+ phy_package_unlock(phydev);
+
+ mutex_lock(&clock->clock_lock);
+
dp83640 = kzalloc_obj(struct dp83640_private);
- if (!dp83640)
+ if (!dp83640) {
+ err = -ENOMEM;
goto no_memory;
+ }
dp83640->phydev = phydev;
dp83640->mii_ts.rxtstamp = dp83640_rxtstamp;
@@ -1444,7 +1435,13 @@ static int dp83640_probe(struct phy_device *phydev)
} else
list_add_tail(&dp83640->list, &clock->phylist);
- dp83640_clock_put(clock);
+ mutex_unlock(&clock->clock_lock);
+
+ err = devm_add_action_or_reset(&phydev->mdio.dev,
+ dp83640_phy_release, dp83640);
+ if (err)
+ return err;
+
return 0;
no_register:
@@ -1455,60 +1452,19 @@ static int dp83640_probe(struct phy_device *phydev)
phydev->priv = NULL;
kfree(dp83640);
no_memory:
- dp83640_clock_put(clock);
+ mutex_unlock(&clock->clock_lock);
no_clock:
return err;
}
static void dp83640_remove(struct phy_device *phydev)
{
- struct dp83640_clock *clock;
- struct list_head *this, *next;
- struct dp83640_private *tmp, *dp83640 = phydev->priv;
- bool remove_clock = false;
-
if (phydev->mdio.addr == BROADCAST_ADDR)
return;
phydev->mii_ts = NULL;
enable_status_frames(phydev, false);
- cancel_delayed_work_sync(&dp83640->ts_work);
-
- skb_queue_purge(&dp83640->rx_queue);
- skb_queue_purge(&dp83640->tx_queue);
-
- clock = dp83640_clock_get(dp83640->clock);
-
- if (dp83640 == clock->chosen) {
- ptp_clock_unregister(clock->ptp_clock);
- clock->chosen = NULL;
- } else {
- list_for_each_safe(this, next, &clock->phylist) {
- tmp = list_entry(this, struct dp83640_private, list);
- if (tmp == dp83640) {
- list_del_init(&tmp->list);
- break;
- }
- }
- }
-
- if (!clock->chosen && list_empty(&clock->phylist))
- remove_clock = true;
-
- dp83640_clock_put(clock);
- kfree(dp83640);
-
- if (remove_clock) {
- mutex_lock(&phyter_clocks_lock);
- list_del(&clock->list);
- mutex_unlock(&phyter_clocks_lock);
-
- mutex_destroy(&clock->extreg_lock);
- mutex_destroy(&clock->clock_lock);
- put_device(&clock->bus->dev);
- kfree(clock);
- }
}
static struct phy_driver dp83640_driver[] = {
diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index b93640ca08b72..feb50f8cc406a 100644
--- a/drivers/ptp/Kconfig
+++ b/drivers/ptp/Kconfig
@@ -78,6 +78,7 @@ config DP83640_PHY
depends on PHYLIB
depends on PTP_1588_CLOCK
select CRC32
+ select PHY_PACKAGE
help
Supports the DP83640 PHYTER with IEEE 1588 features.
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread