Re: [PATCH net-next v1 2/5] net: lan743x: start phylink last and stop it first

[Nicolai Buchwitz <nb@tipi-net.de>]

On 15.10.2026 17:00, Chris Babroski wrote:
> lan743x_netdev_open() starts phylink before the RX and TX rings exist,
> and lan743x_netdev_close() only stops it after the rings are freed. The
> mac_link_up() callback wakes all TX queues, so it can run while there 
> is
> no TX ring to transmit on.
> 
> For ndo_open and ndo_stop the qdisc is not active at that point, but 
> the
> suspend and resume paths call lan743x_netdev_close() and
> lan743x_netdev_open() directly with the qdisc still attached. A link up
> event in either window lets the stack call ndo_start_xmit() on a 
> channel
> whose ring is not allocated yet or has just been freed.
> 
> The error path of lan743x_netdev_open() is also wrong: a
> lan743x_ptp_open() failure jumps past lan743x_phylink_disconnect(),
> leaving phylink started and the PHY attached after open has failed.
> Every later open then fails because the PHY is already attached.
> 
> Connect and start phylink as the last step of open and stop it as the
> first step of close, as other phylink users do. If phylink fails to
> connect, nothing is attached, so the error path only needs to unwind 
> the
> rings, PTP, MAC and interrupts.
> 
> Fixes: a5f199a8d8a0 ("net: lan743x: Migrate phylib to phylink")
> Assisted-by: LLM
> Signed-off-by: Chris Babroski <cbabroski@nvidia.com>
> ---
>  drivers/net/ethernet/microchip/lan743x_main.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/microchip/lan743x_main.c 
> b/drivers/net/ethernet/microchip/lan743x_main.c
> index 9e1276cf313c..583f631df701 100644
> --- a/drivers/net/ethernet/microchip/lan743x_main.c
> +++ b/drivers/net/ethernet/microchip/lan743x_main.c
> @@ -3334,6 +3334,8 @@ static int lan743x_netdev_close(struct net_device 
> *netdev)
>  	struct lan743x_adapter *adapter = netdev_priv(netdev);
>  	int index;
> 
> +	lan743x_phylink_disconnect(adapter);
> +
>  	for (index = 0; index < adapter->used_tx_channels; index++)
>  		lan743x_tx_close(&adapter->tx[index]);
> 
> @@ -3342,8 +3344,6 @@ static int lan743x_netdev_close(struct net_device 
> *netdev)
> 
>  	lan743x_ptp_close(adapter);
> 
> -	lan743x_phylink_disconnect(adapter);
> -
>  	lan743x_mac_close(adapter);
> 
>  	lan743x_intr_close(adapter);
> @@ -3365,10 +3365,6 @@ static int lan743x_netdev_open(struct net_device 
> *netdev)
>  	if (ret)
>  		goto close_intr;
> 
> -	ret = lan743x_phylink_connect(adapter);
> -	if (ret)
> -		goto close_mac;
> -
>  	ret = lan743x_ptp_open(adapter);
>  	if (ret)
>  		goto close_mac;
> @@ -3387,6 +3383,10 @@ static int lan743x_netdev_open(struct net_device 
> *netdev)
>  			goto close_tx;
>  	}
> 
> +	ret = lan743x_phylink_connect(adapter);
> +	if (ret)
> +		goto close_tx;
> +
>  	if (netdev->phydev)
>  		phy_support_eee(netdev->phydev);
> 
> @@ -3414,8 +3414,6 @@ static int lan743x_netdev_open(struct net_device 
> *netdev)
>  			lan743x_rx_close(&adapter->rx[index]);
>  	}
>  	lan743x_ptp_close(adapter);
> -	if (adapter->phylink)
> -		lan743x_phylink_disconnect(adapter);
> 
>  close_mac:
>  	lan743x_mac_close(adapter);

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai