[PATCH net v1 0/4] net: phy: smsc: robustness fixes for LAN87xx/LAN9500

[PATCH net v1 0/4] net: phy: smsc: robustness fixes for LAN87xx/LAN9500

[Oleksij Rempel]

Hi all,

The SMSC 10/100 PHYs (LAN87xx family) found in smsc95xx (lan95xx)
USB-Ethernet adapters show several quirks around the Auto-MDIX feature:

- A hardware strap (AUTOMDIX_EN) may boot the PHY in fixed-MDI mode, and
  the current driver cannot always override it.

- When Auto-MDIX is left enabled while autonegotiation is forced off,
  the PHY endlessly swaps the TX/RX pairs and never links up.

- The driver sets the enable bit for Auto-MDIX but forgets the override
  bit, so userspace requests are silently ignored.

- Rapid configuration changes can wedge the link if PHY IRQs are
  enabled.

The four patches below make the MDIX state fully predictable and prevent
link failures in every tested strap / autoneg / MDI-X permutation.

Tested on LAN9512 Eval board.

Best Regards,
Oleksij

Oleksij Rempel (4):
  net: phy: smsc: Fix Auto-MDIX configuration when disabled by strap
  net: phy: smsc: Force predictable MDI-X state on LAN87xx
  net: phy: smsc: Fix link failure in forced mode with Auto-MDIX
  net: phy: smsc: Disable IRQ support to prevent link state corruption

 drivers/net/phy/smsc.c | 61 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 52 insertions(+), 9 deletions(-)

--
2.39.5

[PATCH net v1 1/4] net: phy: smsc: Fix Auto-MDIX configuration when disabled by strap

[Oleksij Rempel]

Correct the Auto-MDIX configuration to ensure userspace settings are
respected when the feature is disabled by the AUTOMDIX_EN hardware strap.

The LAN9500 PHY allows its default MDI-X mode to be configured via a
hardware strap. If this strap sets the default to "MDI-X off", the
driver was previously unable to enable Auto-MDIX from userspace.

When handling the ETH_TP_MDI_AUTO case, the driver would set the
SPECIAL_CTRL_STS_AMDIX_ENABLE_ bit but neglected to set the required
SPECIAL_CTRL_STS_OVRRD_AMDIX_ bit. Without the override flag, the PHY
falls back to its hardware strap default, ignoring the software request.

This patch corrects the behavior by also setting the override bit when
enabling Auto-MDIX. This ensures that the userspace configuration takes
precedence over the hardware strap, allowing Auto-MDIX to be enabled
correctly in all scenarios.

Fixes: 05b35e7eb9a1 ("smsc95xx: add phylib support")
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Andre Edich <andre.edich@microchip.com>
---
 drivers/net/phy/smsc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 31463b9e5697..adf12d7108b5 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -167,7 +167,8 @@ static int lan87xx_config_aneg(struct phy_device *phydev)
 			SPECIAL_CTRL_STS_AMDIX_STATE_;
 		break;
 	case ETH_TP_MDI_AUTO:
-		val = SPECIAL_CTRL_STS_AMDIX_ENABLE_;
+		val = SPECIAL_CTRL_STS_OVRRD_AMDIX_ |
+			SPECIAL_CTRL_STS_AMDIX_ENABLE_;
 		break;
 	default:
 		return genphy_config_aneg(phydev);
-- 
2.39.5

[PATCH net v1 2/4] net: phy: smsc: Force predictable MDI-X state on LAN87xx

[Oleksij Rempel]

Override the hardware strap configuration for MDI-X mode to ensure a
predictable initial state for the driver. The initial mode of the LAN87xx
PHY is determined by the AUTOMDIX_EN strap pin, but the driver has no
documented way to read its latched status.

This unpredictability means the driver cannot know if the PHY has
initialized with Auto-MDIX enabled or disabled, preventing it from
providing a reliable interface to the user.

This patch introduces a `config_init` hook that forces the PHY into a
known state by explicitly enabling Auto-MDIX.

Fixes: 05b35e7eb9a1 ("smsc95xx: add phylib support")
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Andre Edich <andre.edich@microchip.com>
---
 drivers/net/phy/smsc.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index adf12d7108b5..ad9a3d91bb8a 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -262,6 +262,33 @@ int lan87xx_read_status(struct phy_device *phydev)
 }
 EXPORT_SYMBOL_GPL(lan87xx_read_status);
 
+static int lan87xx_phy_config_init(struct phy_device *phydev)
+{
+	int rc;
+
+	/* The LAN87xx PHY's initial MDI-X mode is determined by the AUTOMDIX_EN
+	 * hardware strap, but the driver cannot read the strap's status. This
+	 * creates an unpredictable initial state.
+	 *
+	 * To ensure consistent and reliable behavior across all boards,
+	 * override the strap configuration on initialization and force the PHY
+	 * into a known state with Auto-MDIX enabled, which is the expected
+	 * default for modern hardware.
+	 */
+	rc = phy_modify(phydev, SPECIAL_CTRL_STS,
+			SPECIAL_CTRL_STS_OVRRD_AMDIX_ |
+			SPECIAL_CTRL_STS_AMDIX_ENABLE_ |
+			SPECIAL_CTRL_STS_AMDIX_STATE_,
+			SPECIAL_CTRL_STS_OVRRD_AMDIX_ |
+			SPECIAL_CTRL_STS_AMDIX_ENABLE_);
+	if (rc < 0)
+		return rc;
+
+	phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
+
+	return smsc_phy_config_init(phydev);
+}
+
 static int lan874x_phy_config_init(struct phy_device *phydev)
 {
 	u16 val;
@@ -696,7 +723,7 @@ static struct phy_driver smsc_phy_driver[] = {
 
 	/* basic functions */
 	.read_status	= lan87xx_read_status,
-	.config_init	= smsc_phy_config_init,
+	.config_init	= lan87xx_phy_config_init,
 	.soft_reset	= smsc_phy_reset,
 	.config_aneg	= lan87xx_config_aneg,
 
-- 
2.39.5

[PATCH net v1 3/4] net: phy: smsc: Fix link failure in forced mode with Auto-MDIX

[Oleksij Rempel]

Force a fixed MDI-X mode when auto-negotiation is disabled to prevent
link instability.

When forcing the link speed and duplex on a LAN9500 PHY (e.g., with
`ethtool -s eth0 autoneg off ...`) while leaving MDI-X control in auto
mode, the PHY fails to establish a stable link. This occurs because the
PHY's Auto-MDIX algorithm is not designed to operate when
auto-negotiation is disabled. In this state, the PHY continuously
toggles the TX/RX signal pairs, which prevents the link partner from
synchronizing.

This patch resolves the issue by detecting when auto-negotiation is
disabled. If the MDI-X control mode is set to 'auto', the driver now
forces a specific, stable mode (ETH_TP_MDI) to prevent the pair
toggling. This choice of a fixed MDI mode mirrors the behavior the
hardware would exhibit if the AUTOMDIX_EN strap were configured for a
fixed MDI connection.

Fixes: 05b35e7eb9a1 ("smsc95xx: add phylib support")
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Andre Edich <andre.edich@microchip.com>
---
 drivers/net/phy/smsc.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index ad9a3d91bb8a..b6489da5cfcd 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -155,10 +155,29 @@ static int smsc_phy_reset(struct phy_device *phydev)
 
 static int lan87xx_config_aneg(struct phy_device *phydev)
 {
-	int rc;
+	u8 mdix_ctrl;
 	int val;
+	int rc;
+
+	/* When auto-negotiation is disabled (forced mode), the PHY's
+	 * Auto-MDIX will continue toggling the TX/RX pairs.
+	 *
+	 * To establish a stable link, we must select a fixed MDI mode.
+	 * If the user has not specified a fixed MDI mode (i.e., mdix_ctrl is
+	 * 'auto'), we default to ETH_TP_MDI. This choice of a ETH_TP_MDI mode
+	 * mirrors the behavior the hardware would exhibit if the AUTOMDIX_EN
+	 * strap were configured for a fixed MDI connection.
+	 */
+	if (phydev->autoneg == AUTONEG_DISABLE) {
+		if (phydev->mdix_ctrl == ETH_TP_MDI_AUTO)
+			mdix_ctrl = ETH_TP_MDI;
+		else
+			mdix_ctrl = phydev->mdix_ctrl;
+	} else {
+		mdix_ctrl = phydev->mdix_ctrl;
+	}
 
-	switch (phydev->mdix_ctrl) {
+	switch (mdix_ctrl) {
 	case ETH_TP_MDI:
 		val = SPECIAL_CTRL_STS_OVRRD_AMDIX_;
 		break;
@@ -184,7 +203,7 @@ static int lan87xx_config_aneg(struct phy_device *phydev)
 	rc |= val;
 	phy_write(phydev, SPECIAL_CTRL_STS, rc);
 
-	phydev->mdix = phydev->mdix_ctrl;
+	phydev->mdix = mdix_ctrl;
 	return genphy_config_aneg(phydev);
 }
 
-- 
2.39.5

[PATCH net v1 4/4] net: phy: smsc: Disable IRQ support to prevent link state corruption

[Oleksij Rempel]

Disable interrupt handling for the LAN87xx PHY to prevent the network
interface from entering a corrupted state after rapid configuration
changes.

When the link configuration is changed quickly, the PHY can get stuck in
a non-functional state. In this state, 'ethtool' reports that a link is
present, but 'ip link' shows NO-CARRIER, and the interface is unable to
transfer data.

Fixes: 1ce8b37241ed ("usbnet: smsc95xx: Forward PHY interrupts to PHY driver to avoid polling")
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Lukas Wunner <lukas@wunner.de>
---
 drivers/net/phy/smsc.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index b6489da5cfcd..dac6bf156d15 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -746,10 +746,6 @@ static struct phy_driver smsc_phy_driver[] = {
 	.soft_reset	= smsc_phy_reset,
 	.config_aneg	= lan87xx_config_aneg,
 
-	/* IRQ related */
-	.config_intr	= smsc_phy_config_intr,
-	.handle_interrupt = smsc_phy_handle_interrupt,
-
 	/* Statistics */
 	.get_sset_count = smsc_get_sset_count,
 	.get_strings	= smsc_get_strings,
-- 
2.39.5

Re: [PATCH net v1 4/4] net: phy: smsc: Disable IRQ support to prevent link state corruption

[Lukas Wunner]

On Tue, Jul 01, 2025 at 02:21:46PM +0200, Oleksij Rempel wrote:
> Disable interrupt handling for the LAN87xx PHY to prevent the network
> interface from entering a corrupted state after rapid configuration
> changes.
> 
> When the link configuration is changed quickly, the PHY can get stuck in
> a non-functional state. In this state, 'ethtool' reports that a link is
> present, but 'ip link' shows NO-CARRIER, and the interface is unable to
> transfer data.
[...]
> --- a/drivers/net/phy/smsc.c
> +++ b/drivers/net/phy/smsc.c
> @@ -746,10 +746,6 @@ static struct phy_driver smsc_phy_driver[] = {
>  	.soft_reset	= smsc_phy_reset,
>  	.config_aneg	= lan87xx_config_aneg,
>  
> -	/* IRQ related */
> -	.config_intr	= smsc_phy_config_intr,
> -	.handle_interrupt = smsc_phy_handle_interrupt,
> -

Well, that's not good.  I guess this means that the interrupt is
polled again, so we basically go back to the suboptimal behavior
prior to 1ce8b37241ed?

Without support for interrupt handling, we can't take advantage
of the GPIOs on the chip for interrupt generation.  Nor can we
properly support runtime PM if no cable is attached.

What's the actual root cause?  Is it the issue described in this
paragraph of 1ce8b37241ed's commit message?

    Normally the PHY interrupt should be masked until the PHY driver has
    cleared it.  However masking requires a (sleeping) USB transaction and
    interrupts are received in (non-sleepable) softirq context.  I decided
    not to mask the interrupt at all (by using the dummy_irq_chip's noop
    ->irq_mask() callback):  The USB interrupt endpoint is polled in 1 msec
    intervals and normally that's sufficient to wake the PHY driver's IRQ
    thread and have it clear the interrupt.  If it does take longer, worst
    thing that can happen is the IRQ thread is woken again.  No big deal.

There must be better options than going back to polling.
E.g. inserting delays to avoid the PHY getting wedged.

TBH I did test this thoroughly back in the day and never
witnessed the issue.

Thanks,

Lukas

Re: [PATCH net v1 4/4] net: phy: smsc: Disable IRQ support to prevent link state corruption

[Oleksij Rempel]

Hi Lukas,

On Tue, Jul 01, 2025 at 02:58:19PM +0200, Lukas Wunner wrote:
> On Tue, Jul 01, 2025 at 02:21:46PM +0200, Oleksij Rempel wrote:
> > Disable interrupt handling for the LAN87xx PHY to prevent the network
> > interface from entering a corrupted state after rapid configuration
> > changes.
> > 
> > When the link configuration is changed quickly, the PHY can get stuck in
> > a non-functional state. In this state, 'ethtool' reports that a link is
> > present, but 'ip link' shows NO-CARRIER, and the interface is unable to
> > transfer data.
> [...]
> > --- a/drivers/net/phy/smsc.c
> > +++ b/drivers/net/phy/smsc.c
> > @@ -746,10 +746,6 @@ static struct phy_driver smsc_phy_driver[] = {
> >  	.soft_reset	= smsc_phy_reset,
> >  	.config_aneg	= lan87xx_config_aneg,
> >  
> > -	/* IRQ related */
> > -	.config_intr	= smsc_phy_config_intr,
> > -	.handle_interrupt = smsc_phy_handle_interrupt,
> > -
> 
> Well, that's not good.  I guess this means that the interrupt is
> polled again, so we basically go back to the suboptimal behavior
> prior to 1ce8b37241ed?

Not fully. It will disable interrupt support only for the embedded PHY,
other types of interrupts should work as expected.

> Without support for interrupt handling, we can't take advantage
> of the GPIOs on the chip for interrupt generation.  Nor can we
> properly support runtime PM if no cable is attached.

Hm... the PHY smsc driver is not using EDPD mode by default if PHY
interrupts are enabled. Or do you mean other kind of PM?

> What's the actual root cause?  Is it the issue described in this
> paragraph of 1ce8b37241ed's commit message?
> 
>     Normally the PHY interrupt should be masked until the PHY driver has
>     cleared it.  However masking requires a (sleeping) USB transaction and
>     interrupts are received in (non-sleepable) softirq context.  I decided
>     not to mask the interrupt at all (by using the dummy_irq_chip's noop
>     ->irq_mask() callback):  The USB interrupt endpoint is polled in 1 msec
>     intervals and normally that's sufficient to wake the PHY driver's IRQ
>     thread and have it clear the interrupt.  If it does take longer, worst
>     thing that can happen is the IRQ thread is woken again.  No big deal.

I'm not sure. It seems to be not the problem.

> There must be better options than going back to polling.
> E.g. inserting delays to avoid the PHY getting wedged.
> 
> TBH I did test this thoroughly back in the day and never
> witnessed the issue.

I did some testing back in time too. It worked and still works normally
in the autoneg mode.

What is not working as expected is the fixed mode, especially 10 mbit
fixed mode.

Here are my current testing results:

# configure 10 mbit forced mode:
ethtool -s eth0 autoneg off speed 10 duplex half
# attach cable (can be done wothout reataching cable)
[10174.585150] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[10174.586760] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[10174.594636] lan87xx_read_status: link: no, speed: 10, duplex: half, autoneg: off
[10174.602777] lan87xx_read_status: link: no, speed: 10, duplex: half, autoneg: off
[10174.841458] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[10174.843017] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[10174.850619] lan87xx_read_status: link: no, speed: 10, duplex: half, autoneg: off
[10174.857026] lan87xx_read_status: link: no, speed: 10, duplex: half, autoneg: off
[10175.425513] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[10175.427046] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[10175.434871] lan87xx_read_status: link: no, speed: 10, duplex: half, autoneg: off
[10175.441332] lan87xx_read_status: link: no, speed: 10, duplex: half, autoneg: off

At this point no more interrupts will come and link up state will not be
detected. Replugging cable will have same result.

The worst part - unplugging the cable may trigger an endless interrupt storm
(which is some times reproducible in the 10Mbit forced mode):
[ 1584.132799] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 1584.134220] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 1584.389134] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 1584.390591] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 1584.644757] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 1584.646177] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 1584.900781] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 1584.902305] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 1585.158416] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098

With latest kernel we can use adaptive polling, wich I added now for
testing. Here are the results:

[ 2200.702427] lan87xx_read_status: link: no, speed: 10, duplex: half, autoneg: off
[ 2200.948552] smsc95xx 1-1.1:1.0 enu1u1: intdata: 0x00008000
[ 2200.949640] smsc95xx 1-1.1:1.0 enu1u1: intdata: 0x00008000
[ 2200.950182] smsc95xx 1-1.1:1.0 enu1u1: intdata: 0x00008000
[ 2200.951374] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 2200.953186] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 2200.959234] lan87xx_read_status: link: no, speed: 10, duplex: half, autoneg: off
[ 2201.204270] smsc95xx 1-1.1:1.0 enu1u1: intdata: 0x00008000
[ 2201.205284] smsc95xx 1-1.1:1.0 enu1u1: intdata: 0x00008000
[ 2201.207139] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 2201.208825] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 2201.216406] lan87xx_read_status: link: no, speed: 10, duplex: half, autoneg: off
[ 2201.460548] smsc95xx 1-1.1:1.0 enu1u1: intdata: 0x00008000
[ 2201.461618] smsc95xx 1-1.1:1.0 enu1u1: intdata: 0x00008000
[ 2201.462181] smsc95xx 1-1.1:1.0 enu1u1: intdata: 0x00008000
[ 2201.463273] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 2201.464764] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 2201.471066] lan87xx_read_status: link: no, speed: 10, duplex: half, autoneg: off
[ 2201.716547] smsc95xx 1-1.1:1.0 enu1u1: intdata: 0x00008000
[ 2201.717607] smsc95xx 1-1.1:1.0 enu1u1: intdata: 0x00008000
[ 2201.718235] smsc95xx 1-1.1:1.0 enu1u1: intdata: 0x00008000
[ 2201.719267] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 2201.721035] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 2201.727488] lan87xx_read_status: link: no, speed: 10, duplex: half, autoneg: off
[ 2201.972542] smsc95xx 1-1.1:1.0 enu1u1: intdata: 0x00008000
[ 2201.973614] smsc95xx 1-1.1:1.0 enu1u1: intdata: 0x00008000
[ 2201.974176] smsc95xx 1-1.1:1.0 enu1u1: intdata: 0x00008000
[ 2201.975321] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 2201.977078] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 2201.983500] lan87xx_read_status: link: no, speed: 10, duplex: half, autoneg: off
[ 2202.228538] smsc95xx 1-1.1:1.0 enu1u1: intdata: 0x00008000
[ 2202.229615] smsc95xx 1-1.1:1.0 enu1u1: intdata: 0x00008000
[ 2202.230174] smsc95xx 1-1.1:1.0 enu1u1: intdata: 0x00008000
[ 2202.231292] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 2202.233038] smsc_phy_handle_interrupt: MII_LAN83C185_ISF = 0x0098
[ 2202.238972] lan87xx_read_status: link: no, speed: 10, duplex: half, autoneg: off
[ 2202.239018] smsc_phy_get_next_update: next update in 250 jiffies
[ 2203.258566] lan87xx_read_status: link: yes, speed: 10, duplex: half, autoneg: off
[ 2203.258756] smsc95xx 1-1.1:1.0 enu1u1: Link is Up - 10Mbps/Half - flow control off

With adaptive polling we can use both. Since IRQ down interrupt works as
expected, we can use low frequency polling (one per 30 seconds) in link up
state. On link down state, after last interrupt poll one time per second
for 30 seconds, then switch to low frequency polling (one per 30
seconds).

I need to figure out haw to handle an interrupt storm.

-- 
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 v1 4/4] net: phy: smsc: Disable IRQ support to prevent link state corruption

[Paolo Abeni]

On 7/1/25 2:58 PM, Lukas Wunner wrote:
> On Tue, Jul 01, 2025 at 02:21:46PM +0200, Oleksij Rempel wrote:
>> Disable interrupt handling for the LAN87xx PHY to prevent the network
>> interface from entering a corrupted state after rapid configuration
>> changes.
>>
>> When the link configuration is changed quickly, the PHY can get stuck in
>> a non-functional state. In this state, 'ethtool' reports that a link is
>> present, but 'ip link' shows NO-CARRIER, and the interface is unable to
>> transfer data.
> [...]
>> --- a/drivers/net/phy/smsc.c
>> +++ b/drivers/net/phy/smsc.c
>> @@ -746,10 +746,6 @@ static struct phy_driver smsc_phy_driver[] = {
>>  	.soft_reset	= smsc_phy_reset,
>>  	.config_aneg	= lan87xx_config_aneg,
>>  
>> -	/* IRQ related */
>> -	.config_intr	= smsc_phy_config_intr,
>> -	.handle_interrupt = smsc_phy_handle_interrupt,
>> -
> 
> Well, that's not good.  I guess this means that the interrupt is
> polled again, so we basically go back to the suboptimal behavior
> prior to 1ce8b37241ed?
> 
> Without support for interrupt handling, we can't take advantage
> of the GPIOs on the chip for interrupt generation.  Nor can we
> properly support runtime PM if no cable is attached.
> 
> What's the actual root cause?  Is it the issue described in this
> paragraph of 1ce8b37241ed's commit message?
> 
>     Normally the PHY interrupt should be masked until the PHY driver has
>     cleared it.  However masking requires a (sleeping) USB transaction and
>     interrupts are received in (non-sleepable) softirq context.  I decided
>     not to mask the interrupt at all (by using the dummy_irq_chip's noop
>     ->irq_mask() callback):  The USB interrupt endpoint is polled in 1 msec
>     intervals and normally that's sufficient to wake the PHY driver's IRQ
>     thread and have it clear the interrupt.  If it does take longer, worst
>     thing that can happen is the IRQ thread is woken again.  No big deal.
> 
> There must be better options than going back to polling.
> E.g. inserting delays to avoid the PHY getting wedged.

I agree the solution proposed by this patch looks too rough. I think
more effort should be invested to at least understand why the phy got stuck.

@Oleksij: possibly you could re-submit patch 1-3 only while you keep
investigating the issue addressed here.

/P

Re: [PATCH net v1 4/4] net: phy: smsc: Disable IRQ support to prevent link state corruption

[Lukas Wunner]

On Wed, Jul 02, 2025 at 11:46:05AM +0200, Oleksij Rempel wrote:
> On Tue, Jul 01, 2025 at 02:58:19PM +0200, Lukas Wunner wrote:
> > Without support for interrupt handling, we can't take advantage
> > of the GPIOs on the chip for interrupt generation.  Nor can we
> > properly support runtime PM if no cable is attached.
> 
> Hm... the PHY smsc driver is not using EDPD mode by default if PHY
> interrupts are enabled. Or do you mean other kind of PM?

See commit 2642cc6c3bbe ("net: phy: smsc: Disable Energy Detect
Power-Down in interrupt mode"):

The LAN9514 used on older RasPi boards only supports SUSPEND0,
SUSPEND1, SUSPEND2.  Other incarnations of the LAN95xx family
such as LAN9500A additionally support SUSPEND3.  The smsc95xx.c
driver enables suspend only on SUSPEND3-capable chips.

I was planning to add suspend support for LAN9514 but never got
around to finish it.  Enabling SUSPEND1 if no cable is attached
saves quite a bit of power, so RasPis without a network connection
consume less.

Back in the day I did some tests and noticed that while EDPD
wasn't working at all outside of SUSPEND modes, it did work
at least sometimes to wake up from SUSPEND1.  The driver fiddles
with EDPD settings upon entering suspend.  I concluded that more
testing is necessary to enable EDPD before SUSPEND1 and that's
when I had to put this project aside. :(

Thanks,

Lukas