Re: [net-next PATCH 2/2] net: airoha: add phylink support for GDM1

Maxime Chevallier
2025-10-22T07:42:25Z

Hi Christian,

On 21/10/2025 21:33, Christian Marangi wrote:
> In preparation for support of GDM2+ port, fill in phylink OPs for GDM1
> that is an INTERNAL port for the Embedded Switch.
>
> Rework the GDM init logic by first preparing the struct with all the
> required info and creating the phylink interface and only after the
> parsing register the related netdev.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
> drivers/net/ethernet/airoha/airoha_eth.c | 108 ++++++++++++++++++++---
> drivers/net/ethernet/airoha/airoha_eth.h | 3 +
> 2 files changed, 99 insertions(+), 12 deletions(-)

You also need to select PHYLINK in Kconfig

>
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index ce6d13b10e27..fc237775a998 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> @@ -1613,6 +1613,8 @@ static int airoha_dev_open(struct net_device *dev)
> struct airoha_gdm_port *port = netdev_priv(dev);
> struct airoha_qdma *qdma = port->qdma;
>
> + phylink_start(port->phylink);
> +
> netif_tx_start_all_queues(dev);
> err = airoha_set_vip_for_gdm_port(port, true);
> if (err)
> @@ -1665,6 +1667,8 @@ static int airoha_dev_stop(struct net_device *dev)
> }
> }
>
> + phylink_stop(port->phylink);
> +
> return 0;
> }
>
> @@ -2813,6 +2817,17 @@ static const struct ethtool_ops airoha_ethtool_ops = {
> .get_link = ethtool_op_get_link,
> };
>
> +static struct phylink_pcs *airoha_phylink_mac_select_pcs(struct phylink_config *config,
> + phy_interface_t interface)
> +{
> + return NULL;
> +}
> +
> +static void airoha_mac_config(struct phylink_config *config, unsigned int mode,
> + const struct phylink_link_state *state)
> +{
> +}
> +
> static int airoha_metadata_dst_alloc(struct airoha_gdm_port *port)
> {
> int i;
> @@ -2857,6 +2872,55 @@ bool airoha_is_valid_gdm_port(struct airoha_eth *eth,
> return false;
> }
>
> +static void airoha_mac_link_up(struct phylink_config *config, struct phy_device *phy,
> + unsigned int mode, phy_interface_t interface,
> + int speed, int duplex, bool tx_pause, bool rx_pause)
> +{
> +}
> +
> +static void airoha_mac_link_down(struct phylink_config *config, unsigned int mode,
> + phy_interface_t interface)
> +{
> +}
> +
> +static const struct phylink_mac_ops airoha_phylink_ops = {
> + .mac_select_pcs = airoha_phylink_mac_select_pcs,
> + .mac_config = airoha_mac_config,
> + .mac_link_up = airoha_mac_link_up,
> + .mac_link_down = airoha_mac_link_down,
> +};
> +
> +static int airoha_setup_phylink(struct net_device *netdev)
> +{
> + struct airoha_gdm_port *port = netdev_priv(netdev);
> + struct device *dev = &netdev->dev;
> + phy_interface_t phy_mode;
> + struct phylink *phylink;
> +
> + phy_mode = device_get_phy_mode(dev);
> + if (phy_mode < 0) {
> + dev_err(dev, "incorrect phy-mode\n");
> + return phy_mode;
> + }
> +
> + port->phylink_config.dev = dev;
> + port->phylink_config.type = PHYLINK_NETDEV;
> + port->phylink_config.mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
> + MAC_10000FD;
> +
> + __set_bit(PHY_INTERFACE_MODE_INTERNAL,
> + port->phylink_config.supported_interfaces);
> +
> + phylink = phylink_create(&port->phylink_config, dev_fwnode(dev),
> + phy_mode, &airoha_phylink_ops);
> + if (IS_ERR(phylink))
> + return PTR_ERR(phylink);
> +
> + port->phylink = phylink;
> +
> + return 0;
> +}
> +
> static int airoha_alloc_gdm_port(struct airoha_eth *eth,
> struct device_node *np, int index)
> {
> @@ -2931,19 +2995,30 @@ static int airoha_alloc_gdm_port(struct airoha_eth *eth,
> port->id = id;
> eth->ports[p] = port;
>
> - err = airoha_metadata_dst_alloc(port);
> - if (err)
> - return err;
> + return airoha_metadata_dst_alloc(port);
> +}
>
> - err = register_netdev(dev);
> - if (err)
> - goto free_metadata_dst;
> +static int airoha_register_gdm_ports(struct airoha_eth *eth)
> +{
> + int i;
>
> - return 0;
> + for (i = 0; i < ARRAY_SIZE(eth->ports); i++) {
> + struct airoha_gdm_port *port = eth->ports[i];
> + int err;
>
> -free_metadata_dst:
> - airoha_metadata_dst_free(port);
> - return err;
> + if (!port)
> + continue;
> +
> + err = airoha_setup_phylink(port->dev);
> + if (err)
> + return err;
> +
> + err = register_netdev(port->dev);
> + if (err)
> + return err;

The cleanup for that path seems to only be done if

port->dev->reg_state == NETREG_REGISTERED

So if netdev registration fails here, you'll never destroy
the phylink instance :(

Maxime