netdev - Re: [PATCH net v2] net: phy: microchip: force IRQ polling mode for lan88xx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fcd60fa6-4bb5-47ec-89ab-cbc94f8a62ce@gmx.de>
Date: Thu, 17 Apr 2025 11:05:35 +0200
From: Fiona Klute <fiona.klute@....de>
To: netdev@...r.kernel.org
Cc: Andrew Lunn <andrew+netdev@...n.ch>,
 Thangaraj Samynathan <Thangaraj.S@...rochip.com>,
 Rengarajan Sundararajan <Rengarajan.S@...rochip.com>,
 UNGLinuxDriver@...rochip.com, "David S . Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, linux-usb@...r.kernel.org,
 linux-kernel@...r.kernel.org, kernel-list@...pberrypi.com
Subject: Re: [PATCH net v2] net: phy: microchip: force IRQ polling mode for
 lan88xx

Am 16.04.25 um 12:24 schrieb Fiona Klute:
> With lan88xx based devices the lan78xx driver can get stuck in an
> interrupt loop while bringing the device up, flooding the kernel log
> with messages like the following:
> 
> lan78xx 2-3:1.0 enp1s0u3: kevent 4 may have been dropped
> 
> Removing interrupt support from the lan88xx PHY driver forces the
> driver to use polling instead, which avoids the problem.
> 
> The issue has been observed with Raspberry Pi devices at least since
> 4.14 (see [1], bug report for their downstream kernel), as well as
> with Nvidia devices [2] in 2020, where disabling polling was the

I noticed I got words mixed up here, needs to be either "disabling 
interrupts" or "forcing polling", not "disabling polling".

Should I re-send, or is that something that can be fixed while applying?

Best regards,
Fiona

> vendor-suggested workaround (together with the claim that phylib
> changes in 4.9 made the interrupt handling in lan78xx incompatible).
> 
> Iperf reports well over 900Mbits/sec per direction with client in
> --dualtest mode, so there does not seem to be a significant impact on
> throughput (lan88xx device connected via switch to the peer).
> 
> [1] https://github.com/raspberrypi/linux/issues/2447
> [2] https://forums.developer.nvidia.com/t/jetson-xavier-and-lan7800-problem/142134/11
> 
> Link: https://lore.kernel.org/0901d90d-3f20-4a10-b680-9c978e04ddda@lunn.ch
> Fixes: 792aec47d59d ("add microchip LAN88xx phy driver")
> Signed-off-by: Fiona Klute <fiona.klute@....de>
> Cc: kernel-list@...pberrypi.com
> Cc: stable@...r.kernel.org
> ---
> v2:
> - add comment why interrupt functions are missing
> - add Fixes reference
> v1: https://lore.kernel.org/netdev/20250414152634.2786447-1-fiona.klute@gmx.de/
> 
>   drivers/net/phy/microchip.c | 46 +++----------------------------------
>   1 file changed, 3 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/net/phy/microchip.c b/drivers/net/phy/microchip.c
> index 0e17cc458efdc..93de88c1c8fd5 100644
> --- a/drivers/net/phy/microchip.c
> +++ b/drivers/net/phy/microchip.c
> @@ -37,47 +37,6 @@ static int lan88xx_write_page(struct phy_device *phydev, int page)
>   	return __phy_write(phydev, LAN88XX_EXT_PAGE_ACCESS, page);
>   }
>   
> -static int lan88xx_phy_config_intr(struct phy_device *phydev)
> -{
> -	int rc;
> -
> -	if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
> -		/* unmask all source and clear them before enable */
> -		rc = phy_write(phydev, LAN88XX_INT_MASK, 0x7FFF);
> -		rc = phy_read(phydev, LAN88XX_INT_STS);
> -		rc = phy_write(phydev, LAN88XX_INT_MASK,
> -			       LAN88XX_INT_MASK_MDINTPIN_EN_ |
> -			       LAN88XX_INT_MASK_LINK_CHANGE_);
> -	} else {
> -		rc = phy_write(phydev, LAN88XX_INT_MASK, 0);
> -		if (rc)
> -			return rc;
> -
> -		/* Ack interrupts after they have been disabled */
> -		rc = phy_read(phydev, LAN88XX_INT_STS);
> -	}
> -
> -	return rc < 0 ? rc : 0;
> -}
> -
> -static irqreturn_t lan88xx_handle_interrupt(struct phy_device *phydev)
> -{
> -	int irq_status;
> -
> -	irq_status = phy_read(phydev, LAN88XX_INT_STS);
> -	if (irq_status < 0) {
> -		phy_error(phydev);
> -		return IRQ_NONE;
> -	}
> -
> -	if (!(irq_status & LAN88XX_INT_STS_LINK_CHANGE_))
> -		return IRQ_NONE;
> -
> -	phy_trigger_machine(phydev);
> -
> -	return IRQ_HANDLED;
> -}
> -
>   static int lan88xx_suspend(struct phy_device *phydev)
>   {
>   	struct lan88xx_priv *priv = phydev->priv;
> @@ -528,8 +487,9 @@ static struct phy_driver microchip_phy_driver[] = {
>   	.config_aneg	= lan88xx_config_aneg,
>   	.link_change_notify = lan88xx_link_change_notify,
>   
> -	.config_intr	= lan88xx_phy_config_intr,
> -	.handle_interrupt = lan88xx_handle_interrupt,
> +	/* Interrupt handling is broken, do not define related
> +	 * functions to force polling.
> +	 */
>   
>   	.suspend	= lan88xx_suspend,
>   	.resume		= genphy_resume,