Re: [PATCH net v1 2/2] net: phy: smsc: add adaptive polling to recover missed link-up on LAN8700

[Lukas Wunner <lukas@wunner.de>]

On Mon, Jul 07, 2025 at 05:32:32PM +0200, Oleksij Rempel wrote:
> Fixe unreliable link detection on LAN8700 configured for 10 Mbit / half-

s/Fixe/Fix/

> or full-duplex against an autonegotiating partner and similar scenarios.
> 
> The LAN8700 PHY (build in to LAN9512 and similar adapters) can fail to

s/build/built/

> report a link-up event when it is forced to a fixed speed/duplex and the
> link partner still advertises autonegotiation. During link establishment
> the PHY raises several interrupts while the link is not yet up; once the
> link finally comes up no further interrupt is generated, so phylib never
> observes the transition and the kernel keeps the interface down even
> though ethtool shows the link as up.
> 
> Mitigate this by combining interrupts with adaptive polling:
> 
> - When the driver is running in pure polling mode it continues to poll
>   once per second (unchanged).
> - With an IRQ present we now
>   - poll every 30 s while the link is up (low overhead);
>   - switch to a 1 s poll for up to 30 s after the last IRQ and while the
>     link is down, ensuring we catch the final silent link-up.

I think this begs the question, if we *know* that the link may come up
belatedly without an interrupt, why poll?  Would it work to schedule a
one-off link check after a reasonable delay to catch the link change?

I'm also wondering if enabling EDPD changes the behavior?

> +static unsigned int smsc_phy_get_next_update(struct phy_device *phydev)
> +{
> +	struct smsc_phy_priv *priv = phydev->priv;
> +
> +	/* If interrupts are disabled, fall back to default polling */
> +	if (phydev->irq == PHY_POLL)
> +		return SMSC_NOIRQ_POLLING_INTERVAL;
> +
> +	/* The PHY sometimes drops the *final* link-up IRQ when we run
> +	 * with autoneg OFF (10 Mbps HD/FD) against an autonegotiating
> +	 * partner: we see several "link down" IRQs, none for "link up".

Hm, I'm not seeing a check for all those conditions (e.g. autoneg off) here?

Also, you seem to be using UTF-8 characters instead of US-ASCII single or
double quotes around "link down" and "link up".

> +	 *
> +	 * Work-around philosophy:
> +	 *   - If the link is already up, the hazard is past, so we
> +	 *     revert to a relaxed 30 s poll to save power.
> +	 *   - Otherwise we stay in a tighter polling loop for up to one
> +	 *     full interval after the last IRQ in case the crucial
> +	 *     link-up IRQ was lost.  Empirically 5 s is enough but we
> +	 *     keep 30 s to be extra conservative.
> +	 */
> +	if (!priv->last_irq || phydev->link ||
> +	    time_is_before_jiffies(priv->last_irq + SMSC_IRQ_POLLING_INTERVAL))
> +		return SMSC_IRQ_POLLING_INTERVAL;
> +
> +	return SMSC_NOIRQ_POLLING_INTERVAL;
> +}

Thanks,

Lukas