[PATCH net-next v3 0/5] net: phy: Fix phy_init_hw() placement and update locking

[PATCH net-next v3 0/5] net: phy: Fix phy_init_hw() placement and update locking

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

This series fixes two related issues in the PHY subsystem: incorrect
placement of phy_init_hw() in the resume path, and drop/update locking
in several PHY drivers.

Patch 1 identifies that when mac_managed_pm is set, mdio_bus_phy_resume()
is skipped entirely, meaning phy_init_hw() which performs soft reset and
config_init is never called on resume for that path. To make both paths
consistent, phy_init_hw() is moved into phy_resume() so it runs
unconditionally. As a consequence, the separate phy_init_hw() +
phy_resume() call sequence in phy_attach_direct() is collapsed
into a single phy_resume() call.

Patch 2 removes the now-redundant explicit phy_init_hw() call in
rtl8169_up(), since phy_resume() already handles it.

Patch 3 removes manual mutex_lock/unlock(&phydev->lock) from four
functions in the MSCC PHY driver. In vsc85xx_edge_rate_cntl_set(),
the lock wraps a single phy_modify_paged() call, which is already a
fully locked atomic operation that acquires the MDIO bus lock
internally, so the additional phydev->lock is unnecessary. The
remaining three functions — vsc85xx_mac_if_set(),
vsc8531_pre_init_seq_set(), and vsc85xx_eee_init_seq_set() — use
phy_read(), phy_write(), phy_select_page(), and phy_restore_page(),
all of which operate under the MDIO bus lock, so taking phydev->lock
around them provides no additional serialisation. Error-path labels
are updated accordingly.

Patch 4 fixes lan937x_dsp_workaround() in the Microchip T1 driver,
which was incorrectly taking phydev->lock. The function performs raw
MDIO bus accesses and must instead hold mdio_lock. The phy_read() and
phy_write() calls are also switched to their unlocked __phy_read() and
__phy_write() variants since mdio_lock is now held explicitly.

Patch 5 refines the placement introduced in patch 1 by moving
phy_init_hw() from phy_resume() down into __phy_resume(), so that it
is called with phydev->lock already held. This is necessary because
many MAC drivers and phylink reach the resume path via phy_start() ->
__phy_resume() directly, bypassing phy_resume().

v2->v3:
 * Moved all the patches into series as the order they get merged
   matters, otherwise a git bisect could land on a deadlock.
 * Updated commit description for patch#2 and #3.
v1->v2:
 * Updated commit description.
 * phy_init_hw() is moved from __phy_resume() -> phy_resume() to make it
   lock-free.
 * Dropped redundant phy_init_hw() call from mdio_bus_phy_resume() and
   phy_attach_direct().

Biju Das (5):
  net: phy: call phy_init_hw() in phy resume path
  r8169: Drop redundant phy_init_hw() call in rtl8169_up()
  net: phy: mscc: Drop unnecessary phydev->lock
  net: phy: microchip_t1: Replace phydev->lock with mdio_lock in
  net: phy: Move phy_init_hw() from phy_resume() to __phy_resume()

 drivers/net/ethernet/realtek/r8169_main.c |  1 -
 drivers/net/phy/microchip_t1.c            |  8 ++---
 drivers/net/phy/mscc/mscc_main.c          | 41 +++++++----------------
 drivers/net/phy/phy_device.c              | 14 ++++----
 4 files changed, 22 insertions(+), 42 deletions(-)

-- 
2.43.0

[PATCH net-next v3 1/5] net: phy: call phy_init_hw() in phy resume path

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

When mac_managed_pm flag is set, mdio_bus_phy_resume() is skipped, so
phy_init_hw(), which performs soft_reset and config_init, is not called
during resume.

This is inconsistent with the non-mac_managed_pm path, where
mdio_bus_phy_resume() calls phy_init_hw() before phy_resume() on every
resume.

To align both paths, move the phy_init_hw() call into phy_resume() itself,
before invoking the driver's resume callback. This ensures PHY soft reset
and re-initialization happen unconditionally, regardless of whether PM is
managed by the MAC or the MDIO bus. As a result, drop the redundant
phy_init_hw() call in mdio_bus_phy_resume().

Additionally, in phy_attach_direct(), replace the separate phy_init_hw()
and phy_resume() calls with a single phy_resume() call, since
phy_init_hw() is now handled inside phy_resume().

Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2->v3:
 * No change,
v1->v2:
 * Updated commit description.
 * phy_init_hw() is moved from __phy_resume() -> phy_resume() to make it
   lock-free.
 * Dropped redundant phy_init_hw() call from mdio_bus_phy_resume() and
   phy_attach_direct().
---
 drivers/net/phy/phy_device.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 0edff47478c2..4a2b19d39373 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -396,10 +396,6 @@ static __maybe_unused int mdio_bus_phy_resume(struct device *dev)
 	WARN_ON(phydev->state != PHY_HALTED && phydev->state != PHY_READY &&
 		phydev->state != PHY_UP);
 
-	ret = phy_init_hw(phydev);
-	if (ret < 0)
-		return ret;
-
 	ret = phy_resume(phydev);
 	if (ret < 0)
 		return ret;
@@ -1857,16 +1853,14 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	if (dev)
 		netif_carrier_off(phydev->attached_dev);
 
-	/* Do initial configuration here, now that
+	/* Do initial configuration inside phy_init_hw(), now that
 	 * we have certain key parameters
 	 * (dev_flags and interface)
 	 */
-	err = phy_init_hw(phydev);
+	err = phy_resume(phydev);
 	if (err)
 		goto error;
 
-	phy_resume(phydev);
-
 	/**
 	 * If the external phy used by current mac interface is managed by
 	 * another mac interface, so we should create a device link between
@@ -2020,6 +2014,10 @@ int phy_resume(struct phy_device *phydev)
 {
 	int ret;
 
+	ret = phy_init_hw(phydev);
+	if (ret)
+		return ret;
+
 	mutex_lock(&phydev->lock);
 	ret = __phy_resume(phydev);
 	mutex_unlock(&phydev->lock);
-- 
2.43.0

[PATCH net-next v3 2/5] r8169: Drop redundant phy_init_hw() call in rtl8169_up()

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

Since phy_resume() now calls phy_init_hw() internally as part of the
resume sequence, the explicit phy_init_hw() call immediately before
phy_resume() in rtl8169_up() is redundant. Remove it.

No functional change intended.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2->v3:
 * Added the patch into series
 * Updated commit description.
v2:
 * New patch
---
 drivers/net/ethernet/realtek/r8169_main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 791277e750ba..cb22105f323f 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -5032,7 +5032,6 @@ static void rtl8169_up(struct rtl8169_private *tp)
 		rtl8168_driver_start(tp);
 
 	pci_set_master(tp->pci_dev);
-	phy_init_hw(tp->phydev);
 	phy_resume(tp->phydev);
 	rtl8169_init_phy(tp);
 	napi_enable(&tp->napi);
-- 
2.43.0

[PATCH net-next v3 3/5] net: phy: mscc: Drop unnecessary phydev->lock

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

Remove manual mutex_lock/unlock(&phydev->lock) calls from several
functions in the MSCC PHY driver.

In vsc85xx_edge_rate_cntl_set(), phydev->lock is taken around a single
phy_modify_paged() call. phy_modify_paged() is already a fully locked
atomic operation that acquires the MDIO bus lock internally, so the
additional phydev->lock is unnecessary.

The remaining three functions — vsc85xx_mac_if_set(),
vsc8531_pre_init_seq_set(), and vsc85xx_eee_init_seq_set() — use
phy_read(), phy_write(), phy_select_page(), and phy_restore_page(),
all of which operate under the MDIO bus lock. Taking phydev->lock
around them provides no additional serialisation.

Along with dropping the locks, error-path labels are renamed from
out_unlock to err or restore_oldpage to better reflect their purpose.
In vsc8531_pre_init_seq_set() and vsc85xx_eee_init_seq_set(), the
redundant intermediate assignment of oldpage before returning is also
eliminated.

No functional change intended.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2->v3:
 * Added the patch into series
 * Updated commit description.
v2:
 * New patch
---
 drivers/net/phy/mscc/mscc_main.c | 41 ++++++++++----------------------
 1 file changed, 12 insertions(+), 29 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index 2b9fb8a675a6..75430f55acfd 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -486,15 +486,9 @@ static int vsc85xx_dt_led_modes_get(struct phy_device *phydev,
 
 static int vsc85xx_edge_rate_cntl_set(struct phy_device *phydev, u8 edge_rate)
 {
-	int rc;
-
-	mutex_lock(&phydev->lock);
-	rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_2,
-			      MSCC_PHY_WOL_MAC_CONTROL, EDGE_RATE_CNTL_MASK,
-			      edge_rate << EDGE_RATE_CNTL_POS);
-	mutex_unlock(&phydev->lock);
-
-	return rc;
+	return phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_2,
+				MSCC_PHY_WOL_MAC_CONTROL, EDGE_RATE_CNTL_MASK,
+				edge_rate << EDGE_RATE_CNTL_POS);
 }
 
 static int vsc85xx_mac_if_set(struct phy_device *phydev,
@@ -503,7 +497,6 @@ static int vsc85xx_mac_if_set(struct phy_device *phydev,
 	int rc;
 	u16 reg_val;
 
-	mutex_lock(&phydev->lock);
 	reg_val = phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_1);
 	reg_val &= ~(MAC_IF_SELECTION_MASK);
 	switch (interface) {
@@ -522,17 +515,15 @@ static int vsc85xx_mac_if_set(struct phy_device *phydev,
 		break;
 	default:
 		rc = -EINVAL;
-		goto out_unlock;
+		goto err;
 	}
 	rc = phy_write(phydev, MSCC_PHY_EXT_PHY_CNTL_1, reg_val);
 	if (rc)
-		goto out_unlock;
+		goto err;
 
 	rc = genphy_soft_reset(phydev);
 
-out_unlock:
-	mutex_unlock(&phydev->lock);
-
+err:
 	return rc;
 }
 
@@ -668,19 +659,15 @@ static int vsc8531_pre_init_seq_set(struct phy_device *phydev)
 	if (rc < 0)
 		return rc;
 
-	mutex_lock(&phydev->lock);
 	oldpage = phy_select_page(phydev, MSCC_PHY_PAGE_TR);
 	if (oldpage < 0)
-		goto out_unlock;
+		goto restore_oldpage;
 
 	for (i = 0; i < ARRAY_SIZE(init_seq); i++)
 		vsc85xx_tr_write(phydev, init_seq[i].reg, init_seq[i].val);
 
-out_unlock:
-	oldpage = phy_restore_page(phydev, oldpage, oldpage);
-	mutex_unlock(&phydev->lock);
-
-	return oldpage;
+restore_oldpage:
+	return phy_restore_page(phydev, oldpage, oldpage);
 }
 
 static int vsc85xx_eee_init_seq_set(struct phy_device *phydev)
@@ -708,19 +695,15 @@ static int vsc85xx_eee_init_seq_set(struct phy_device *phydev)
 	unsigned int i;
 	int oldpage;
 
-	mutex_lock(&phydev->lock);
 	oldpage = phy_select_page(phydev, MSCC_PHY_PAGE_TR);
 	if (oldpage < 0)
-		goto out_unlock;
+		goto restore_oldpage;
 
 	for (i = 0; i < ARRAY_SIZE(init_eee); i++)
 		vsc85xx_tr_write(phydev, init_eee[i].reg, init_eee[i].val);
 
-out_unlock:
-	oldpage = phy_restore_page(phydev, oldpage, oldpage);
-	mutex_unlock(&phydev->lock);
-
-	return oldpage;
+restore_oldpage:
+	return phy_restore_page(phydev, oldpage, oldpage);
 }
 
 /* phydev->bus->mdio_lock should be locked when using this function */
-- 
2.43.0

[PATCH net-next v3 4/5] net: phy: microchip_t1: Replace phydev->lock with mdio_lock in lan937x_dsp_workaround()

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

lan937x_dsp_workaround() performs raw MDIO bus accesses and must
therefore hold mdio_lock rather than phydev->lock. Using phydev->lock
here is incorrect as it does not serialise access to the MDIO bus.

Replace phydev->lock with bus->mdio_lock, and switch the phy_read()/
phy_write() calls to their unlocked __phy_read()/__phy_write()
variants since mdio_lock is now held explicitly.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3:
 * New patch.
---
 drivers/net/phy/microchip_t1.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/microchip_t1.c b/drivers/net/phy/microchip_t1.c
index 62b36a318100..afb4e8908b78 100644
--- a/drivers/net/phy/microchip_t1.c
+++ b/drivers/net/phy/microchip_t1.c
@@ -337,9 +337,9 @@ static int lan937x_dsp_workaround(struct phy_device *phydev, u16 ereg, u8 bank)
 	int rc = 0;
 	u16 val;
 
-	mutex_lock(&phydev->lock);
+	mutex_lock(&phydev->mdio.bus->mdio_lock);
 	/* Read previous selected bank */
-	rc = phy_read(phydev, LAN87XX_EXT_REG_CTL);
+	rc = __phy_read(phydev, LAN87XX_EXT_REG_CTL);
 	if (rc < 0)
 		goto out_unlock;
 
@@ -353,11 +353,11 @@ static int lan937x_dsp_workaround(struct phy_device *phydev, u16 ereg, u8 bank)
 		val |= LAN87XX_EXT_REG_CTL_RD_CTL;
 
 		/* access twice for DSP bank change,dummy access */
-		rc = phy_write(phydev, LAN87XX_EXT_REG_CTL, val);
+		rc = __phy_write(phydev, LAN87XX_EXT_REG_CTL, val);
 	}
 
 out_unlock:
-	mutex_unlock(&phydev->lock);
+	mutex_unlock(&phydev->mdio.bus->mdio_lock);
 
 	return rc;
 }
-- 
2.43.0

[PATCH net-next v3 5/5] net: phy: Move phy_init_hw() from phy_resume() to __phy_resume()

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

Now that redundant locking has been removed from PHY driver callbacks,
phy_init_hw() can be called with phydev->lock held.

Many MAC drivers and the phylink framework resume the PHY via
phy_start(), which invokes __phy_resume() directly without going
through phy_resume(). Keeping phy_init_hw() in phy_resume() means it
is not called in this path.

Move phy_init_hw() into __phy_resume() so that PHY soft reset and
re-initialisation happen unconditionally on every resume, regardless
of which code path triggers it.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3:
 * New patch.
---
 drivers/net/phy/phy_device.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 4a2b19d39373..16fc2fc63c50 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1999,6 +1999,10 @@ int __phy_resume(struct phy_device *phydev)
 
 	lockdep_assert_held(&phydev->lock);
 
+	ret = phy_init_hw(phydev);
+	if (ret)
+		return ret;
+
 	if (!phydrv || !phydrv->resume)
 		return 0;
 
@@ -2014,10 +2018,6 @@ int phy_resume(struct phy_device *phydev)
 {
 	int ret;
 
-	ret = phy_init_hw(phydev);
-	if (ret)
-		return ret;
-
 	mutex_lock(&phydev->lock);
 	ret = __phy_resume(phydev);
 	mutex_unlock(&phydev->lock);
-- 
2.43.0

Re: [PATCH net-next v3 0/5] net: phy: Fix phy_init_hw() placement and update locking

[Jakub Kicinski]

On Sun, 12 Apr 2026 15:00:22 +0100 Biju wrote:
> This series fixes two related issues in the PHY subsystem: incorrect
> placement of phy_init_hw() in the resume path, and drop/update locking
> in several PHY drivers.

Hi Andrew, IIUC this should be applied for 7.1 but we're waiting 
for Russell (who is AFK/busy) to review. Did I get that right?

Re: [PATCH net-next v3 5/5] net: phy: Move phy_init_hw() from phy_resume() to __phy_resume()

[Andrew Lunn]

On Sun, Apr 12, 2026 at 03:00:27PM +0100, Biju wrote:
> From: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Now that redundant locking has been removed from PHY driver callbacks,
> phy_init_hw() can be called with phydev->lock held.
> 
> Many MAC drivers and the phylink framework resume the PHY via
> phy_start(), which invokes __phy_resume() directly without going
> through phy_resume(). Keeping phy_init_hw() in phy_resume() means it
> is not called in this path.
> 
> Move phy_init_hw() into __phy_resume() so that PHY soft reset and
> re-initialisation happen unconditionally on every resume, regardless
> of which code path triggers it.

I would change the order of these patches. First remove the redundant
locks. You can then put phy_init_hw() into __phy_resume(), rather than
first moving it into phy_resume() and then __phy_resume().

      Andrew

Re: [PATCH net-next v3 3/5] net: phy: mscc: Drop unnecessary phydev->lock

[Andrew Lunn]

On Sun, Apr 12, 2026 at 03:00:25PM +0100, Biju wrote:
> From: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Remove manual mutex_lock/unlock(&phydev->lock) calls from several
> functions in the MSCC PHY driver.
> 
> In vsc85xx_edge_rate_cntl_set(), phydev->lock is taken around a single
> phy_modify_paged() call. phy_modify_paged() is already a fully locked
> atomic operation that acquires the MDIO bus lock internally, so the
> additional phydev->lock is unnecessary.
> 
> The remaining three functions — vsc85xx_mac_if_set(),
> vsc8531_pre_init_seq_set(), and vsc85xx_eee_init_seq_set() — use
> phy_read(), phy_write(), phy_select_page(), and phy_restore_page(),
> all of which operate under the MDIO bus lock. Taking phydev->lock
> around them provides no additional serialisation.
> 
> Along with dropping the locks, error-path labels are renamed from
> out_unlock to err or restore_oldpage to better reflect their purpose.
> In vsc8531_pre_init_seq_set() and vsc85xx_eee_init_seq_set(), the
> redundant intermediate assignment of oldpage before returning is also
> eliminated.
> 
> No functional change intended.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v3 4/5] net: phy: microchip_t1: Replace phydev->lock with mdio_lock in lan937x_dsp_workaround()

[Andrew Lunn]

> -	mutex_lock(&phydev->lock);
> +	mutex_lock(&phydev->mdio.bus->mdio_lock);

phy_lock_mdio_bus(), and the phy_unlock_mdio_bus().

    Andrew

---
pw-bot: cr

Re: [PATCH net-next v3 3/5] net: phy: mscc: Drop unnecessary phydev->lock

[Russell King (Oracle)]

On Sun, Apr 12, 2026 at 03:00:25PM +0100, Biju wrote:
> @@ -486,15 +486,9 @@ static int vsc85xx_dt_led_modes_get(struct phy_device *phydev,
>  
>  static int vsc85xx_edge_rate_cntl_set(struct phy_device *phydev, u8 edge_rate)
>  {
> -	int rc;
> -
> -	mutex_lock(&phydev->lock);
> -	rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_2,
> -			      MSCC_PHY_WOL_MAC_CONTROL, EDGE_RATE_CNTL_MASK,
> -			      edge_rate << EDGE_RATE_CNTL_POS);
> -	mutex_unlock(&phydev->lock);
> -
> -	return rc;
> +	return phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_2,
> +				MSCC_PHY_WOL_MAC_CONTROL, EDGE_RATE_CNTL_MASK,
> +				edge_rate << EDGE_RATE_CNTL_POS);

This one is fine.

> @@ -503,7 +497,6 @@ static int vsc85xx_mac_if_set(struct phy_device *phydev,
>  	int rc;
>  	u16 reg_val;
>  
> -	mutex_lock(&phydev->lock);
>  	reg_val = phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_1);
>  	reg_val &= ~(MAC_IF_SELECTION_MASK);
>  	switch (interface) {
> @@ -522,17 +515,15 @@ static int vsc85xx_mac_if_set(struct phy_device *phydev,
>  		break;
>  	default:
>  		rc = -EINVAL;
> -		goto out_unlock;
> +		goto err;
>  	}
>  	rc = phy_write(phydev, MSCC_PHY_EXT_PHY_CNTL_1, reg_val);

I would much rather this was converted to use phy_modify() as well so
that we ensure that the update is atomic.

	rc = phy_modify(phydev, MSCC_PHY_EXT_PHY_CNTL_1,
			MAC_IF_SELECTION_MASK, reg_val);

where reg_val is assigned the field value in the switch above.

> @@ -668,19 +659,15 @@ static int vsc8531_pre_init_seq_set(struct phy_device *phydev)
>  	if (rc < 0)
>  		return rc;
>  
> -	mutex_lock(&phydev->lock);
>  	oldpage = phy_select_page(phydev, MSCC_PHY_PAGE_TR);
>  	if (oldpage < 0)
> -		goto out_unlock;
> +		goto restore_oldpage;
>  
>  	for (i = 0; i < ARRAY_SIZE(init_seq); i++)
>  		vsc85xx_tr_write(phydev, init_seq[i].reg, init_seq[i].val);
>  
> -out_unlock:
> -	oldpage = phy_restore_page(phydev, oldpage, oldpage);
> -	mutex_unlock(&phydev->lock);
> -
> -	return oldpage;
> +restore_oldpage:
> +	return phy_restore_page(phydev, oldpage, oldpage);

This is fine.

> @@ -708,19 +695,15 @@ static int vsc85xx_eee_init_seq_set(struct phy_device *phydev)
>  	unsigned int i;
>  	int oldpage;
>  
> -	mutex_lock(&phydev->lock);
>  	oldpage = phy_select_page(phydev, MSCC_PHY_PAGE_TR);
>  	if (oldpage < 0)
> -		goto out_unlock;
> +		goto restore_oldpage;
>  
>  	for (i = 0; i < ARRAY_SIZE(init_eee); i++)
>  		vsc85xx_tr_write(phydev, init_eee[i].reg, init_eee[i].val);
>  
> -out_unlock:
> -	oldpage = phy_restore_page(phydev, oldpage, oldpage);
> -	mutex_unlock(&phydev->lock);
> -
> -	return oldpage;
> +restore_oldpage:
> +	return phy_restore_page(phydev, oldpage, oldpage);

Also fine.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

RE: [PATCH net-next v3 5/5] net: phy: Move phy_init_hw() from phy_resume() to __phy_resume()

[Biju Das]

Hi Andrew,

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: 14 April 2026 17:03
> Subject: Re: [PATCH net-next v3 5/5] net: phy: Move phy_init_hw() from phy_resume() to __phy_resume()
> 
> On Sun, Apr 12, 2026 at 03:00:27PM +0100, Biju wrote:
> > From: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > Now that redundant locking has been removed from PHY driver callbacks,
> > phy_init_hw() can be called with phydev->lock held.
> >
> > Many MAC drivers and the phylink framework resume the PHY via
> > phy_start(), which invokes __phy_resume() directly without going
> > through phy_resume(). Keeping phy_init_hw() in phy_resume() means it
> > is not called in this path.
> >
> > Move phy_init_hw() into __phy_resume() so that PHY soft reset and
> > re-initialisation happen unconditionally on every resume, regardless
> > of which code path triggers it.
> 
> I would change the order of these patches. First remove the redundant locks. You can then put
> phy_init_hw() into __phy_resume(), rather than first moving it into phy_resume() and then
> __phy_resume().

Agreed.

Cheers,
Biju

RE: [PATCH net-next v3 4/5] net: phy: microchip_t1: Replace phydev->lock with mdio_lock in lan937x_dsp_workaround()

[Biju Das]

Hi Andrew,

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: 14 April 2026 17:09
> Subject: Re: [PATCH net-next v3 4/5] net: phy: microchip_t1: Replace phydev->lock with mdio_lock in
> lan937x_dsp_workaround()
> 
> > -	mutex_lock(&phydev->lock);
> > +	mutex_lock(&phydev->mdio.bus->mdio_lock);
> 
> phy_lock_mdio_bus(), and the phy_unlock_mdio_bus().

OK, will fix this.

Cheers,
Biju

RE: [PATCH net-next v3 3/5] net: phy: mscc: Drop unnecessary phydev->lock

[Biju Das]

Hi Russell King,

> -----Original Message-----
> From: Russell King <linux@armlinux.org.uk>
> Sent: 14 April 2026 18:48
> Subject: Re: [PATCH net-next v3 3/5] net: phy: mscc: Drop unnecessary phydev->lock
> 
> On Sun, Apr 12, 2026 at 03:00:25PM +0100, Biju wrote:
> > @@ -486,15 +486,9 @@ static int vsc85xx_dt_led_modes_get(struct
> > phy_device *phydev,
> >
> >  static int vsc85xx_edge_rate_cntl_set(struct phy_device *phydev, u8
> > edge_rate)  {
> > -	int rc;
> > -
> > -	mutex_lock(&phydev->lock);
> > -	rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_2,
> > -			      MSCC_PHY_WOL_MAC_CONTROL, EDGE_RATE_CNTL_MASK,
> > -			      edge_rate << EDGE_RATE_CNTL_POS);
> > -	mutex_unlock(&phydev->lock);
> > -
> > -	return rc;
> > +	return phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_2,
> > +				MSCC_PHY_WOL_MAC_CONTROL, EDGE_RATE_CNTL_MASK,
> > +				edge_rate << EDGE_RATE_CNTL_POS);
> 
> This one is fine.
> 
> > @@ -503,7 +497,6 @@ static int vsc85xx_mac_if_set(struct phy_device *phydev,
> >  	int rc;
> >  	u16 reg_val;
> >
> > -	mutex_lock(&phydev->lock);
> >  	reg_val = phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_1);
> >  	reg_val &= ~(MAC_IF_SELECTION_MASK);
> >  	switch (interface) {
> > @@ -522,17 +515,15 @@ static int vsc85xx_mac_if_set(struct phy_device *phydev,
> >  		break;
> >  	default:
> >  		rc = -EINVAL;
> > -		goto out_unlock;
> > +		goto err;
> >  	}
> >  	rc = phy_write(phydev, MSCC_PHY_EXT_PHY_CNTL_1, reg_val);
> 
> I would much rather this was converted to use phy_modify() as well so that we ensure that the update is
> atomic.
> 
> 	rc = phy_modify(phydev, MSCC_PHY_EXT_PHY_CNTL_1,
> 			MAC_IF_SELECTION_MASK, reg_val);

Agreed, will use phy_modify()

Cheers,
Biju

RE: [PATCH net-next v3 5/5] net: phy: Move phy_init_hw() from phy_resume() to __phy_resume()

[Biju Das]

Hi Andrew/Russell,

> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: 14 April 2026 19:43
> Subject: RE: [PATCH net-next v3 5/5] net: phy: Move phy_init_hw() from phy_resume() to __phy_resume()
> 
> Hi Andrew,
> 
> > -----Original Message-----
> > From: Andrew Lunn <andrew@lunn.ch>
> > Sent: 14 April 2026 17:03
> > Subject: Re: [PATCH net-next v3 5/5] net: phy: Move phy_init_hw() from
> > phy_resume() to __phy_resume()
> >
> > On Sun, Apr 12, 2026 at 03:00:27PM +0100, Biju wrote:
> > > From: Biju Das <biju.das.jz@bp.renesas.com>
> > >
> > > Now that redundant locking has been removed from PHY driver
> > > callbacks,
> > > phy_init_hw() can be called with phydev->lock held.
> > >
> > > Many MAC drivers and the phylink framework resume the PHY via
> > > phy_start(), which invokes __phy_resume() directly without going
> > > through phy_resume(). Keeping phy_init_hw() in phy_resume() means it
> > > is not called in this path.
> > >
> > > Move phy_init_hw() into __phy_resume() so that PHY soft reset and
> > > re-initialisation happen unconditionally on every resume, regardless
> > > of which code path triggers it.
> >
> > I would change the order of these patches. First remove the redundant
> > locks. You can then put
> > phy_init_hw() into __phy_resume(), rather than first moving it into
> > phy_resume() and then __phy_resume().
> 
> Agreed.

One of my colleague pointed out that this patch may break[1], but we don't have the hardware to
test this IP. According to him

 "It does a phy_init_hw(), resetting the PHY, it applies some ethtool settings and then it calls
phy_start(). Since we are calling phy_init_hw() again during phy_start(),we might be undoing the
ethtool settings"

This kind of cleanup/fixing will be in 7.2 cycle, right?

[1] https://elixir.bootlin.com/linux/v7.0/source/drivers/net/ethernet/marvell/mv643xx_eth.c#L2327

Cheers,
Biju