From: Lukas Wunner <lukas@wunner.de>
To: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Andrew Lunn <andrew@lunn.ch>,
Heiner Kallweit <hkallweit1@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
kernel@pengutronix.de, linux-kernel@vger.kernel.org,
Russell King <linux@armlinux.org.uk>,
netdev@vger.kernel.org, Andre Edich <andre.edich@microchip.com>
Subject: Re: [PATCH net v1 4/4] net: phy: smsc: Disable IRQ support to prevent link state corruption
Date: Tue, 1 Jul 2025 14:58:19 +0200 [thread overview]
Message-ID: <aGPba6fX1bqgVfYC@wunner.de> (raw)
In-Reply-To: <20250701122146.35579-5-o.rempel@pengutronix.de>
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
next prev parent reply other threads:[~2025-07-01 12:58 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-01 12:21 [PATCH net v1 0/4] net: phy: smsc: robustness fixes for LAN87xx/LAN9500 Oleksij Rempel
2025-07-01 12:21 ` [PATCH net v1 1/4] net: phy: smsc: Fix Auto-MDIX configuration when disabled by strap Oleksij Rempel
2025-07-01 12:21 ` [PATCH net v1 2/4] net: phy: smsc: Force predictable MDI-X state on LAN87xx Oleksij Rempel
2025-07-01 12:21 ` [PATCH net v1 3/4] net: phy: smsc: Fix link failure in forced mode with Auto-MDIX Oleksij Rempel
2025-07-01 12:21 ` [PATCH net v1 4/4] net: phy: smsc: Disable IRQ support to prevent link state corruption Oleksij Rempel
2025-07-01 12:58 ` Lukas Wunner [this message]
2025-07-02 9:46 ` Oleksij Rempel
2025-07-07 17:52 ` Lukas Wunner
2025-07-03 10:17 ` Paolo Abeni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aGPba6fX1bqgVfYC@wunner.de \
--to=lukas@wunner.de \
--cc=andre.edich@microchip.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=hkallweit1@gmail.com \
--cc=kernel@pengutronix.de \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=netdev@vger.kernel.org \
--cc=o.rempel@pengutronix.de \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.