Re: [PATCH net-next v12 12/12] net: airoha: add phylink support

On Mon, Aug 10, 2026 at 10:53:13PM +0800, win847@gmail.com wrote:

On Sun, 9 Aug 2026, Christian Marangi wrote:

quoted

Subject: [PATCH net-next v12 12/12] net: airoha: add phylink support

Hi Christian,

Thanks for the series. I have a question about the error path in
`airoha_dev_open()` introduced in this patch, and a suggested cleanup.

In the new code:

    err = phylink_of_phy_connect(dev->phylink, netdev->dev.of_node, 0);
    if (err) {
        netdev_err(netdev, "could not attach PHY: %d\n", err);
        return err;
    }

    phylink_start(dev->phylink);

    netif_tx_start_all_queues(netdev);
    err = airoha_set_vip_for_gdm_port(dev, true);
    if (err)
        return err;

If `airoha_set_vip_for_gdm_port(dev, true)` fails after both
`phylink_of_phy_connect()` and `phylink_start()` have already run, we
return `err` directly without doing `phylink_stop()` or
`phylink_disconnect_phy()`. That leaves the phylink in a started /
PHY-connected state. On the next `ndo_open`, `phylink_of_phy_connect()`
will be called again and may fail because the PHY is already connected,
preventing the interface from ever coming back up again.

I understand `airoha_set_vip_for_gdm_port()` currently always returns 0
(based on v12), so today this is a dead path, but it is easy to make the
error handling correct and defensive so a future change to that helper
does not silently break reopen.

Suggested change: add an `err_phy_stop` error label and unwind phylink
before returning:

    netif_tx_start_all_queues(netdev);
    err = airoha_set_vip_for_gdm_port(dev, true);
    if (err)
        goto err_phy_stop;

    return 0;

err_phy_stop:
    netif_tx_stop_all_queues(netdev);
    phylink_stop(dev->phylink);
    phylink_disconnect_phy(dev->phylink);
    return err;

Would you be open to folding this in (or reordering so `phylink_start()`
runs after the VIP setup, leaving fewer error exits that need phylink
teardown)? Happy to write it up as a proper patch if useful.

If you are ok, I can integrate the change in the next revision.

-- 
	Ansuel