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

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

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