[PATCH net-next 0/3] Add OATC10 Sleep Wake-up support and LAN867x Rev.D0 handling

[PATCH net-next 0/3] Add OATC10 Sleep Wake-up support and LAN867x Rev.D0 handling

[Parthiban Veerasooran]

This patch series adds support for 10BASE‑T1S Open Alliance TC10 (OATC10)
Sleep/Wake‑up to the generic Clause 45 PHY layer and integrates it with
the Microchip LAN867x Rev.D0 T1S PHY driver. It also ensures that PHY
suspend is properly invoked from the smsc95xx USB Ethernet driver so that
low‑power states are entered during system suspend.

Patch Summary:

1. add generic OATC10 suspend/resume helpers
   - Add TC10 Sleep/Wake‑up MDIO register definitions
   - Implement genphy_c45_oatc10_suspend() and genphy_c45_oatc10_resume()
   - Cache PLCA configuration in struct phy_device for restore after wake

2. add LAN867x Rev.D0 TC10 suspend and Wake‑on‑PHY support
   - Enable TC10 low‑power entry via generic helpers
   - Restore configuration correctly on resume
   - Add ethtool WoL support using PHY wake‑up pulse (WUP)
   - Mark driver with PHY_ALWAYS_CALL_SUSPEND

3. suspend attached PHY on system suspend
   - Call phy_suspend() before USB suspend
   - Allows TC10‑capable PHYs to enter low‑power state

Testing:

Tested on EVB-LAN8670-USB Rev.D0 with suspend/resume cycles and verified:
  - PLCA configuration is preserved across low-power transitions.
  - Wake-up Pulse triggers proper resume.
  - ethtool WOL operations function as expected.

Reference:
Open Alliance TC10 10BASE-T1S Sleep/Wake-up Specification:
https://opensig.org/wp-content/uploads/2024/01/TC14_TC10_JWG_10BASE-T1S-Sleep-Wake-up-Specification_1.0_final.pdf


Parthiban Veerasooran (3):
  net: phy: phy-c45: add OATC10 Sleep/Wakeup support in 10BASE-T1S PHYs
  net: phy: microchip_t1s: add suspend and WOL support for LAN867x
    Rev.D0
  net: usb: smsc95xx: suspend PHY during USB suspend

 drivers/net/phy/mdio-open-alliance.h | 13 ++++
 drivers/net/phy/microchip_t1s.c      | 67 ++++++++++++++++++++-
 drivers/net/phy/phy-c45.c            | 82 ++++++++++++++++++++++++++
 drivers/net/usb/smsc95xx.c           |  6 ++
 include/linux/phy.h                  | 88 +++++++++++++++-------------
 5 files changed, 211 insertions(+), 45 deletions(-)


base-commit: cf0d9080c6f795bc6be08babbffa29b62c06e9b0
-- 
2.34.1

[PATCH net-next 1/3] net: phy: phy-c45: add OATC10 Sleep/Wakeup support in 10BASE-T1S PHYs

[Parthiban Veerasooran]

Add generic Clause 45 helpers to support the Open Alliance TC10
10BASE-T1S Sleep/Wake-up specification. This patch introduces
register definitions for the TC10 sleep/wake status and control
registers and adds generic suspend/resume helpers for
OATC10-compliant PHYs.

The new genphy_c45_oatc10_suspend() helper verifies low-power
capability, caches the current PLCA configuration, and requests entry
into the low-power sleep state. Since all PHY configuration is lost
while sleeping, the PLCA configuration is stored in the phy_device
structure for restoration after wake-up.

The corresponding genphy_c45_oatc10_resume() helper reinitializes the
PHY after wake-up and restores the cached PLCA configuration using
driver callbacks.

Additionally, the PLCA configuration structure is moved into
struct phy_device to allow persistent storage across suspend/resume
cycles.

These helpers allow PHY drivers for TC10 10BASE-T1S devices to
implement sleep and wake-up handling in a consistent way without
duplicating common logic.

Open Alliance TC10 10BASE-T1S Sleep/Wake-up Specification ref:
https://opensig.org/wp-content/uploads/2024/01/TC14_TC10_JWG_10BASE-T1S-Sleep-Wake-up-Specification_1.0_final.pdf

Signed-off-by: Parthiban Veerasooran <parthiban.veerasooran@microchip.com>
---
 drivers/net/phy/mdio-open-alliance.h | 13 ++++
 drivers/net/phy/phy-c45.c            | 82 ++++++++++++++++++++++++++
 include/linux/phy.h                  | 88 +++++++++++++++-------------
 3 files changed, 141 insertions(+), 42 deletions(-)

diff --git a/drivers/net/phy/mdio-open-alliance.h b/drivers/net/phy/mdio-open-alliance.h
index 449d0fb67093..62946be9fb78 100644
--- a/drivers/net/phy/mdio-open-alliance.h
+++ b/drivers/net/phy/mdio-open-alliance.h
@@ -78,6 +78,19 @@
 /* SQI is supported using 3 bits means 8 levels (0-7) */
 #define OATC14_SQI_MAX_LEVEL		7
 
+/* Open Alliance 10BASE-T1S Sleep/Wake-up Registers
+ * Specification:
+ *   "10BASE-T1S Sleep/Wake-up Specification"
+ *   https://opensig.org/wp-content/uploads/2024/01/TC14_TC10_JWG_10BASE-T1S-Sleep-Wake-up-Specification_1.0_final.pdf
+ */
+/* Sleep/Wake-up Status Register */
+#define MDIO_OATC10_WS_STATUS		0xd000
+#define OATC10_WS_STATUS_LPCAP		BIT(15)	/* PM client capability */
+
+/* Sleep/Wake-up Control Register */
+#define MDIO_OATC10_WS_CONTROL		0xd001
+#define OATC10_WS_CONTROL_LPREQ		BIT(15)	/* Request low power */
+
 /* Bus Short/Open Status:
  * 0 0 - no fault; everything is ok. (Default)
  * 0 1 - detected as an open or missing termination(s)
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index d48aa7231b37..627eaae9e60f 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -1832,3 +1832,85 @@ int genphy_c45_oatc14_get_sqi(struct phy_device *phydev)
 	return ret & OATC14_DCQ_SQI_VALUE;
 }
 EXPORT_SYMBOL(genphy_c45_oatc14_get_sqi);
+
+/**
+ * genphy_c45_oatc10_suspend - Suspend OATC10 PHY into low power state
+ * @phydev: PHY device to suspend
+ *
+ * Puts an OATC10 PHY into low power sleep state.
+ *
+ * The function performs the following steps:
+ * 1. Verify low power capability is supported
+ * 2. Cache current PLCA configuration for restoration on wake
+ * 3. Set the low power request bit to enter sleep state
+ *
+ * Return:
+ * * 0 on successful entry to low power state
+ * * -EOPNOTSUPP if PHY doesn't support low power capability
+ * * Negative error code on register read/write failures
+ */
+int genphy_c45_oatc10_suspend(struct phy_device *phydev)
+{
+	int ret;
+
+	/* Check for Low Power capability */
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC10_WS_STATUS);
+	if (ret < 0)
+		return ret;
+
+	if (!(ret & OATC10_WS_STATUS_LPCAP))
+		return -EOPNOTSUPP;
+
+	/* Cache PLCA settings for later use. These values must be restored when
+	 * the PHY wakes up from the low-power sleep state, as all configured
+	 * settings are lost.
+	 */
+	ret = genphy_c45_plca_get_cfg(phydev, &phydev->plca_cfg);
+	if (ret)
+		return ret;
+
+	phydev->plca_cfg.version = -1;
+
+	if (phydev->state == PHY_UP)
+		/* Put the PHY into low power sleep state */
+		return phy_set_bits_mmd(phydev, MDIO_MMD_VEND2,
+					MDIO_OATC10_WS_CONTROL,
+					OATC10_WS_CONTROL_LPREQ);
+
+	return 0;
+}
+EXPORT_SYMBOL(genphy_c45_oatc10_suspend);
+
+/**
+ * genphy_c45_oatc10_resume - Resume OATC10 PHY from low-power sleep state
+ * @phydev: PHY device to resume
+ *
+ * Resume a PHY from suspend state. When the PHY wakes up from the low-power
+ * sleep state, all configured settings are lost. This function reinitializes
+ * the PHY configuration settings.
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+int genphy_c45_oatc10_resume(struct phy_device *phydev)
+{
+	int ret;
+
+	if (!phydev->suspended)
+		return 0;
+
+	/* When the PHY wakes up from the low-power sleep state, it needs to be
+	 * reinitialized as all configured settings are lost.
+	 */
+	if (phydev->drv->config_init) {
+		ret = phydev->drv->config_init(phydev);
+		if (ret)
+			return ret;
+	}
+
+	/* Reconfigure the PHY with cached PLCA settings */
+	if (phydev->drv->set_plca_cfg)
+		return phydev->drv->set_plca_cfg(phydev, &phydev->plca_cfg);
+
+	return 0;
+}
+EXPORT_SYMBOL(genphy_c45_oatc10_resume);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 5de4b172cd0b..9bee520ac421 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -555,6 +555,48 @@ struct phy_oatc14_sqi_capability {
 	u8 sqiplus_bits;
 };
 
+/**
+ * struct phy_plca_cfg - Configuration of the PLCA (Physical Layer Collision
+ * Avoidance) Reconciliation Sublayer.
+ *
+ * @version: read-only PLCA register map version. -1 = not available. Ignored
+ *   when setting the configuration. Format is the same as reported by the PLCA
+ *   IDVER register (31.CA00). -1 = not available.
+ * @enabled: PLCA configured mode (enabled/disabled). -1 = not available / don't
+ *   set. 0 = disabled, anything else = enabled.
+ * @node_id: the PLCA local node identifier. -1 = not available / don't set.
+ *   Allowed values [0 .. 254]. 255 = node disabled.
+ * @node_cnt: the PLCA node count (maximum number of nodes having a TO). Only
+ *   meaningful for the coordinator (node_id = 0). -1 = not available / don't
+ *   set. Allowed values [1 .. 255].
+ * @to_tmr: The value of the PLCA to_timer in bit-times, which determines the
+ *   PLCA transmit opportunity window opening. See IEEE802.3 Clause 148 for
+ *   more details. The to_timer shall be set equal over all nodes.
+ *   -1 = not available / don't set. Allowed values [0 .. 255].
+ * @burst_cnt: controls how many additional frames a node is allowed to send in
+ *   single transmit opportunity (TO). The default value of 0 means that the
+ *   node is allowed exactly one frame per TO. A value of 1 allows two frames
+ *   per TO, and so on. -1 = not available / don't set.
+ *   Allowed values [0 .. 255].
+ * @burst_tmr: controls how many bit times to wait for the MAC to send a new
+ *   frame before interrupting the burst. This value should be set to a value
+ *   greater than the MAC inter-packet gap (which is typically 96 bits).
+ *   -1 = not available / don't set. Allowed values [0 .. 255].
+ *
+ * A structure containing configuration parameters for setting/getting the PLCA
+ * RS configuration. The driver does not need to implement all the parameters,
+ * but should report what is actually used.
+ */
+struct phy_plca_cfg {
+	int version;
+	int enabled;
+	int node_id;
+	int node_cnt;
+	int to_tmr;
+	int burst_cnt;
+	int burst_tmr;
+};
+
 /**
  * struct phy_device - An instance of a PHY
  *
@@ -655,6 +697,7 @@ struct phy_oatc14_sqi_capability {
  * @shared: Pointer to private data shared by phys in one package
  * @priv: Pointer to driver private data
  * @oatc14_sqi_capability: SQI capability information for OATC14 10Base-T1S PHY
+ * @plca_cfg: Cache PLCA configuration for OATC10 compliance 10Base-T1S PHY
  *
  * interrupts currently only supports enabled or disabled,
  * but could be changed in the future to support enabling
@@ -807,6 +850,7 @@ struct phy_device {
 #endif
 
 	struct phy_oatc14_sqi_capability oatc14_sqi_capability;
+	struct phy_plca_cfg plca_cfg;
 };
 
 /* Generic phy_device::dev_flags */
@@ -858,48 +902,6 @@ enum link_inband_signalling {
 	LINK_INBAND_BYPASS		= BIT(2),
 };
 
-/**
- * struct phy_plca_cfg - Configuration of the PLCA (Physical Layer Collision
- * Avoidance) Reconciliation Sublayer.
- *
- * @version: read-only PLCA register map version. -1 = not available. Ignored
- *   when setting the configuration. Format is the same as reported by the PLCA
- *   IDVER register (31.CA00). -1 = not available.
- * @enabled: PLCA configured mode (enabled/disabled). -1 = not available / don't
- *   set. 0 = disabled, anything else = enabled.
- * @node_id: the PLCA local node identifier. -1 = not available / don't set.
- *   Allowed values [0 .. 254]. 255 = node disabled.
- * @node_cnt: the PLCA node count (maximum number of nodes having a TO). Only
- *   meaningful for the coordinator (node_id = 0). -1 = not available / don't
- *   set. Allowed values [1 .. 255].
- * @to_tmr: The value of the PLCA to_timer in bit-times, which determines the
- *   PLCA transmit opportunity window opening. See IEEE802.3 Clause 148 for
- *   more details. The to_timer shall be set equal over all nodes.
- *   -1 = not available / don't set. Allowed values [0 .. 255].
- * @burst_cnt: controls how many additional frames a node is allowed to send in
- *   single transmit opportunity (TO). The default value of 0 means that the
- *   node is allowed exactly one frame per TO. A value of 1 allows two frames
- *   per TO, and so on. -1 = not available / don't set.
- *   Allowed values [0 .. 255].
- * @burst_tmr: controls how many bit times to wait for the MAC to send a new
- *   frame before interrupting the burst. This value should be set to a value
- *   greater than the MAC inter-packet gap (which is typically 96 bits).
- *   -1 = not available / don't set. Allowed values [0 .. 255].
- *
- * A structure containing configuration parameters for setting/getting the PLCA
- * RS configuration. The driver does not need to implement all the parameters,
- * but should report what is actually used.
- */
-struct phy_plca_cfg {
-	int version;
-	int enabled;
-	int node_id;
-	int node_cnt;
-	int to_tmr;
-	int burst_cnt;
-	int burst_tmr;
-};
-
 /**
  * struct phy_plca_status - Status of the PLCA (Physical Layer Collision
  * Avoidance) Reconciliation Sublayer.
@@ -2333,6 +2335,8 @@ int genphy_c45_oatc14_cable_test_get_status(struct phy_device *phydev,
 					    bool *finished);
 int genphy_c45_oatc14_get_sqi_max(struct phy_device *phydev);
 int genphy_c45_oatc14_get_sqi(struct phy_device *phydev);
+int genphy_c45_oatc10_suspend(struct phy_device *phydev);
+int genphy_c45_oatc10_resume(struct phy_device *phydev);
 
 /* The gen10g_* functions are the old Clause 45 stub */
 int gen10g_config_aneg(struct phy_device *phydev);
-- 
2.34.1

Re: [PATCH net-next 1/3] net: phy: phy-c45: add OATC10 Sleep/Wakeup support in 10BASE-T1S PHYs

[Andrew Lunn]

> +	/* Cache PLCA settings for later use. These values must be restored when
> +	 * the PHY wakes up from the low-power sleep state, as all configured
> +	 * settings are lost.
> +	 */

Does the standard define that this configuration is lost in low power
mode? If the standard says this, then fine. But if not, it should be
the PHY driver which saves its state during suspend, and restores it
during resume.

    Andrew

Re: [PATCH net-next 1/3] net: phy: phy-c45: add OATC10 Sleep/Wakeup support in 10BASE-T1S PHYs

[Parthiban.Veerasooran]

Hi Andrew,

On 30/03/26 8:16 pm, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
>> +     /* Cache PLCA settings for later use. These values must be restored when
>> +      * the PHY wakes up from the low-power sleep state, as all configured
>> +      * settings are lost.
>> +      */
> 
> Does the standard define that this configuration is lost in low power
> mode? If the standard says this, then fine. But if not, it should be
> the PHY driver which saves its state during suspend, and restores it
> during resume.

Thank you for the review and pointing this out.

You are correct that the Open Alliance TC10 specification does not 
explicitly state that PHY configuration is lost when entering the 
low‑power state. My earlier assumption was based on the LAN8670/1/2 PHY 
implementation, where the 'monitor and react to wake event' block 
remains active while other power supplies are switched off, resulting in 
configuration loss.

However, since this behavior is not mandated by the specification and 
may vary across PHY implementations, it should not be handled within the 
generic PHY framework.

Given this, I agree that state save and restore is PHY‑specific 
behavior. I will move this logic into the lan867x PHY driver in the next 
revision.

Moreover I will revisit the implementation with internal review and then 
post the next version. Thank you for your support.

Best regards,
Parthiban V
> 
>      Andrew

[PATCH net-next 2/3] net: phy: microchip_t1s: add suspend and WOL support for LAN867x Rev.D0

[Parthiban Veerasooran]

The LAN867x Rev.D0 PHY supports OATC10-compliant sleep and wake
functionality, which can be used for system suspend and wake-up via
MDI Wake-up Pulse (WUP).

Add suspend/resume handling and basic Wake-on-LAN support using
WAKE_PHY, enabling WUP as a wake source via the Sleep Control 0
register. The driver configures MDI wake before entering suspend
and reports/enables the wake source through ethtool WOL callbacks.

Also adjust the Rev.D0 configuration path to avoid checking the reset
completion status when resuming from suspend, as the reset status is
only valid after an explicit PHY reset and not after sleep.

This allows LAN867x Rev.D0 systems to reliably wake from suspend via
MDI activity while preserving correct initialization behavior.

Signed-off-by: Parthiban Veerasooran <parthiban.veerasooran@microchip.com>
---
 drivers/net/phy/microchip_t1s.c | 67 +++++++++++++++++++++++++++++++--
 1 file changed, 64 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c
index e601d56b2507..6b158ae5fb16 100644
--- a/drivers/net/phy/microchip_t1s.c
+++ b/drivers/net/phy/microchip_t1s.c
@@ -38,6 +38,10 @@
 #define LINK_STATUS_CONFIGURATION	GENMASK(12, 11)
 #define LINK_STATUS_SEMAPHORE		BIT(0)
 
+/* Sleep Control 0 Register */
+#define LAN867X_REG_SLEEP_CTRL0		0x0080
+#define SLEEP_CTRL0_MDI_WAKEUP_EN	BIT(14)
+
 /* Link Status Configuration */
 #define LINK_STATUS_CONFIG_PLCA_STATUS	0x1
 #define LINK_STATUS_CONFIG_SEMAPHORE	0x2
@@ -472,9 +476,14 @@ static int lan867x_revd0_config_init(struct phy_device *phydev)
 {
 	int ret;
 
-	ret = lan867x_check_reset_complete(phydev);
-	if (ret)
-		return ret;
+	/* Reset status is only valid after an explicit PHY reset, it is not set
+	 * when the PHY resumes from suspend/sleep state.
+	 */
+	if (!phydev->suspended) {
+		ret = lan867x_check_reset_complete(phydev);
+		if (ret)
+			return ret;
+	}
 
 	for (int i = 0; i < ARRAY_SIZE(lan867x_revd0_fixup_regs); i++) {
 		ret = phy_write_mmd(phydev, MDIO_MMD_VEND2,
@@ -491,6 +500,53 @@ static int lan867x_revd0_config_init(struct phy_device *phydev)
 	return lan867x_revd0_link_active_selection(phydev, false);
 }
 
+static int lan867x_revd0_suspend(struct phy_device *phydev)
+{
+	int ret;
+
+	/* Configure WUP as wake source for system suspend */
+	ret = phy_set_bits_mmd(phydev, MDIO_MMD_VEND2,
+			       LAN867X_REG_SLEEP_CTRL0,
+			       SLEEP_CTRL0_MDI_WAKEUP_EN);
+	if (ret)
+		return ret;
+
+	return genphy_c45_oatc10_suspend(phydev);
+}
+
+static int lan867x_revd0_set_wol(struct phy_device *phydev,
+				 struct ethtool_wolinfo *wol)
+{
+	/* Only support WAKE_PHY via WUP (Wake-up Pulse) */
+	if (wol->wolopts & ~WAKE_PHY)
+		return -EOPNOTSUPP;
+
+	if (wol->wolopts & WAKE_PHY) {
+		return phy_set_bits_mmd(phydev, MDIO_MMD_VEND2,
+					LAN867X_REG_SLEEP_CTRL0,
+					SLEEP_CTRL0_MDI_WAKEUP_EN);
+	} else {
+		/* Disable WUP wake source */
+		return phy_clear_bits_mmd(phydev, MDIO_MMD_VEND2,
+					  LAN867X_REG_SLEEP_CTRL0,
+					  SLEEP_CTRL0_MDI_WAKEUP_EN);
+	}
+}
+
+static void lan867x_revd0_get_wol(struct phy_device *phydev,
+				  struct ethtool_wolinfo *wol)
+{
+	int ret;
+
+	wol->supported = WAKE_PHY;
+	wol->wolopts = 0;
+
+	/* Check if WUP wake source is currently enabled */
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, LAN867X_REG_SLEEP_CTRL0);
+	if (ret >= 0 && (ret & SLEEP_CTRL0_MDI_WAKEUP_EN))
+		wol->wolopts = WAKE_PHY;
+}
+
 static int lan86xx_read_status(struct phy_device *phydev)
 {
 	/* The phy has some limitations, namely:
@@ -569,6 +625,7 @@ static struct phy_driver microchip_t1s_driver[] = {
 		PHY_ID_MATCH_EXACT(PHY_ID_LAN867X_REVD0),
 		.name               = "LAN867X Rev.D0",
 		.features           = PHY_BASIC_T1S_P2MP_FEATURES,
+		.flags              = PHY_ALWAYS_CALL_SUSPEND,
 		.config_init        = lan867x_revd0_config_init,
 		.get_plca_cfg	    = genphy_c45_plca_get_cfg,
 		.set_plca_cfg	    = lan86xx_plca_set_cfg,
@@ -577,6 +634,10 @@ static struct phy_driver microchip_t1s_driver[] = {
 		.cable_test_get_status = genphy_c45_oatc14_cable_test_get_status,
 		.get_sqi            = genphy_c45_oatc14_get_sqi,
 		.get_sqi_max        = genphy_c45_oatc14_get_sqi_max,
+		.suspend            = lan867x_revd0_suspend,
+		.resume             = genphy_c45_oatc10_resume,
+		.get_wol	    = lan867x_revd0_get_wol,
+		.set_wol	    = lan867x_revd0_set_wol,
 	},
 	{
 		PHY_ID_MATCH_EXACT(PHY_ID_LAN865X_REVB),
-- 
2.34.1

[PATCH net-next 3/3] net: usb: smsc95xx: suspend PHY during USB suspend

[Parthiban Veerasooran]

The smsc95xx driver registers a PHY device but does not currently
propagate suspend events to it when the USB interface is suspended.

Call phy_suspend() from the driver's suspend callback so the attached
PHY can properly enter low-power state during system or runtime
suspend. This aligns smsc95xx suspend handling with other network
drivers that manage an external or integrated PHY.

Without this, the PHY may remain active and fail to execute its own
suspend procedure, leading to unnecessary power consumption or
incorrect resume behavior.

This change is also required for the EVB-LAN8670-USB Rev.D0 device to
support OATC10-compliant sleep and wake functionality.

Signed-off-by: Parthiban Veerasooran <parthiban.veerasooran@microchip.com>
---
 drivers/net/usb/smsc95xx.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 42e4048b574b..3a6e03b7410a 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1550,6 +1550,12 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 
 	pdata->pm_task = current;
 
+	if (pdata->phydev) {
+		ret = phy_suspend(pdata->phydev);
+		if (ret)
+			return ret;
+	}
+
 	ret = usbnet_suspend(intf, message);
 	if (ret < 0) {
 		netdev_warn(dev->net, "usbnet_suspend error\n");
-- 
2.34.1

Re: [PATCH net-next 3/3] net: usb: smsc95xx: suspend PHY during USB suspend

[Oliver Neukum]

On 30.03.26 15:42, Parthiban Veerasooran wrote:
> The smsc95xx driver registers a PHY device but does not currently
> propagate suspend events to it when the USB interface is suspended.
> 
> Call phy_suspend() from the driver's suspend callback so the attached
> PHY can properly enter low-power state during system or runtime
> suspend. This aligns smsc95xx suspend handling with other network
> drivers that manage an external or integrated PHY.
> 
> Without this, the PHY may remain active and fail to execute its own
> suspend procedure, leading to unnecessary power consumption or
> incorrect resume behavior.
> 
> This change is also required for the EVB-LAN8670-USB Rev.D0 device to
> support OATC10-compliant sleep and wake functionality.
> 
> Signed-off-by: Parthiban Veerasooran <parthiban.veerasooran@microchip.com>
> ---
>   drivers/net/usb/smsc95xx.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index 42e4048b574b..3a6e03b7410a 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -1550,6 +1550,12 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
>   
>   	pdata->pm_task = current;
>   
> +	if (pdata->phydev) {
> +		ret = phy_suspend(pdata->phydev);
> +		if (ret)
> +			return ret;
> +	}

At this point you have suspended the phy.
Hence the device can no longer transmit

> +
>   	ret = usbnet_suspend(intf, message);

This wants to

1. drain the queue if you do runtime PM
2. can return -EBUSY

>   	if (ret < 0) {
>   		netdev_warn(dev->net, "usbnet_suspend error\n");

And here it will return in the error case. With the phy
already suspended.

And, as a question of principle: Why do you suspend the phy
in suspend(), but take no action in resume()?

	Regards
		Oliver

Re: [PATCH net-next 3/3] net: usb: smsc95xx: suspend PHY during USB suspend

[Parthiban.Veerasooran]

Hi Oliver

Thank you for reviewing the patch.

On 30/03/26 11:09 pm, Oliver Neukum wrote:
>> @@ -1550,6 +1550,12 @@ static int smsc95xx_suspend(struct 
>> usb_interface *intf, pm_message_t message)
>>
>>       pdata->pm_task = current;
>>
>> +     if (pdata->phydev) {
>> +             ret = phy_suspend(pdata->phydev);
>> +             if (ret)
>> +                     return ret;
>> +     }
> 
> At this point you have suspended the phy.
> Hence the device can no longer transmit
> 
>> +
>>       ret = usbnet_suspend(intf, message);
> 
> This wants to
> 
> 1. drain the queue if you do runtime PM
> 2. can return -EBUSY
> 
>>       if (ret < 0) {
>>               netdev_warn(dev->net, "usbnet_suspend error\n");
> 
> And here it will return in the error case. With the phy
> already suspended.

Thank you for pointing it out. I agree with you. I didn’t note it 
earlier since the issue did not occur during my testing. I will move the 
phy_suspend() to the appropriate place.

> 
> And, as a question of principle: Why do you suspend the phy
> in suspend(), but take no action in resume()?

In resume(), I did not call phy_resume() because the resume path already 
invokes phy_init_hw(), which internally calls 
phydev->drv->config_init(). This reinitializes and reconfigures the PHY. 
This is the reason why I didn't call phy_resume(). If there is a 
preference or expectation to use phy_resume() for symmetry or to better 
align with the PHY framework’s suspend/resume semantics, I’m happy to 
update the code accordingly.

Moreover I will revisit the implementation with internal review and then 
post the next version. Thank you for your support.

Best regards,
Parthiban V
> 
>         Regards
>                 Oliver
> 
> 

Re: [PATCH net-next 3/3] net: usb: smsc95xx: suspend PHY during USB suspend

[Oliver Neukum]

Hi,

On 01.04.26 05:18, Parthiban.Veerasooran@microchip.com wrote:

> Thank you for pointing it out. I agree with you. I didn’t note it
> earlier since the issue did not occur during my testing. I will move the
> phy_suspend() to the appropriate place.

Thank you.

>> And, as a question of principle: Why do you suspend the phy
>> in suspend(), but take no action in resume()?
> 
> In resume(), I did not call phy_resume() because the resume path already
> invokes phy_init_hw(), which internally calls
> phydev->drv->config_init(). This reinitializes and reconfigures the PHY.

Thank you for the explanation. May I suggest that you add a comment
to that effect to the driver with your patch? This needs to be pointed
out. Your code as such is fine. It just needs a comment.

	Regards
		Oliver

Re: [PATCH net-next 3/3] net: usb: smsc95xx: suspend PHY during USB suspend

[Parthiban.Veerasooran]

Hi Oliver,

On 01/04/26 10:54 pm, Oliver Neukum wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
> the content is safe
> 
> Hi,
> 
> On 01.04.26 05:18, Parthiban.Veerasooran@microchip.com wrote:
> 
>> Thank you for pointing it out. I agree with you. I didn’t note it
>> earlier since the issue did not occur during my testing. I will move the
>> phy_suspend() to the appropriate place.
> 
> Thank you.
> 
>>> And, as a question of principle: Why do you suspend the phy
>>> in suspend(), but take no action in resume()?
>>
>> In resume(), I did not call phy_resume() because the resume path already
>> invokes phy_init_hw(), which internally calls
>> phydev->drv->config_init(). This reinitializes and reconfigures the PHY.
> 
> Thank you for the explanation. May I suggest that you add a comment
> to that effect to the driver with your patch? This needs to be pointed
> out. Your code as such is fine. It just needs a comment.

Thank you for the suggestion. I agree and I'll add an explanatory 
comment in the driver and update the patch accordingly.

Best regards,
Parthiban V
> 
>         Regards
>                 Oliver
> 
>