Re: [net-next PATCH v2 2/2] net: airoha: add phylink support for GDM1
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
Date: 2025-10-25 20:36:28
Also in:
linux-arm-kernel, linux-mediatek, lkml
On Thu, Oct 23, 2025 at 04:58:49PM +0200, Christian Marangi wrote:
quoted hunk ↗ jump to hunk
In preparation for support of GDM2+ port, fill in phylink OPs for GDM1 that is an INTERNAL port for the Embedded Switch. Add all the phylink start/stop and fill in the MAC capabilities and the internal interface as the supported interface. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- drivers/net/ethernet/airoha/Kconfig | 1 + drivers/net/ethernet/airoha/airoha_eth.c | 77 +++++++++++++++++++++++- drivers/net/ethernet/airoha/airoha_eth.h | 3 + 3 files changed, 80 insertions(+), 1 deletion(-)diff --git a/drivers/net/ethernet/airoha/Kconfig b/drivers/net/ethernet/airoha/Kconfig index ad3ce501e7a5..3c74438bc8a0 100644 --- a/drivers/net/ethernet/airoha/Kconfig +++ b/drivers/net/ethernet/airoha/Kconfig@@ -2,6 +2,7 @@ config NET_VENDOR_AIROHA bool "Airoha devices" depends on ARCH_AIROHA || COMPILE_TEST + select PHYLIB
This looks wrong if you're using phylink.
quoted hunk ↗ jump to hunk
help If you have a Airoha SoC with ethernet, say Y.diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c index ce6d13b10e27..deba909104bb 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)
phylink_start() _can_ bring the carrier up immediately. Is the netdev ready to start operating at the point phylink_start() has been called? This error handling suggests the answer is "no", and the lack of phylink_stop() in the error path is also a red flag.
quoted hunk ↗ jump to hunk
@@ -1665,6 +1667,8 @@ static int airoha_dev_stop(struct net_device *dev) } } + phylink_stop(port->phylink); + return 0; }@@ -2813,6 +2817,18 @@ 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)
I'd write this as: static struct phylink_pcs * airoha_phylink_mac_select_pcs(struct phylink_config *config, phy_interface_t interface) but:
+{
+ return NULL;
+}Not sure what the point of this is, as this will be the effect if this function is not provided.
quoted hunk ↗ jump to hunk
+ +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 +2873,57 @@ 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, +};
All the called methods are entirely empty, meaning that anything that phylink reports may not reflect what is going on with the device. Is there a plan to implement these methods? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!