RE: [net-next PATCH v2 11/14] net: axienet: Convert to use PCS subsystem

From: Gupta, Suraj
Date: Tue Apr 08 2025 - 08:23:43 EST

Next message: Andy Yan: "Re:Re: [PATCH v3 7/7] drm/rockchip: inno-hdmi: Convert to drm bridge"
Previous message: Jason Gunthorpe: "Re: [PATCH] iommu/arm-smmu-v3: Add missing S2FWB feature detection"
Next in thread: Sean Anderson: "Re: [net-next PATCH v2 11/14] net: axienet: Convert to use PCS subsystem"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Sean Anderson <sean.anderson@xxxxxxxxx>
> Sent: Tuesday, April 8, 2025 4:51 AM
> To: netdev@xxxxxxxxxxxxxxx; Andrew Lunn <andrew+netdev@xxxxxxx>; David S .
> Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet <edumazet@xxxxxxxxxx>; Jakub
> Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni <pabeni@xxxxxxxxxx>; Russell King
> <linux@xxxxxxxxxxxxxxx>
> Cc: Heiner Kallweit <hkallweit1@xxxxxxxxx>; upstream@xxxxxxxxxx; Kory
> Maincent <kory.maincent@xxxxxxxxxxx>; Christian Marangi
> <ansuelsmth@xxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; Simek, Michal
> <michal.simek@xxxxxxx>; Pandey, Radhey Shyam
> <radhey.shyam.pandey@xxxxxxx>; Robert Hancock
> <robert.hancock@xxxxxxxxxx>; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; Sean
> Anderson <sean.anderson@xxxxxxxxx>
> Subject: [net-next PATCH v2 11/14] net: axienet: Convert to use PCS subsystem
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Convert the AXI Ethernet driver to use the PCS subsystem, including the new Xilinx
> PCA/PMA driver. Unfortunately, we must use a helper to work with bare MDIO
> nodes without a compatible.
>

AXI ethernet changes looks fine to me, except one minor nit mentioned below. Using DT changesets for backward compatibility is impressive :)
I'll try reviewing pcs/pma patch also and test it with our setups.
> Signed-off-by: Sean Anderson <sean.anderson@xxxxxxxxx>

Reviewed-by: Suraj Gupta <suraj.gupta2@xxxxxxx>
> ---
>
> (no changes since v1)
>
> drivers/net/ethernet/xilinx/Kconfig | 1 +
> drivers/net/ethernet/xilinx/xilinx_axienet.h | 4 +-
> .../net/ethernet/xilinx/xilinx_axienet_main.c | 104 ++++--------------
> drivers/net/pcs/Kconfig | 1 -
> 4 files changed, 22 insertions(+), 88 deletions(-)
>
> diff --git a/drivers/net/ethernet/xilinx/Kconfig b/drivers/net/ethernet/xilinx/Kconfig
> index 7502214cc7d5..2eab64cf1646 100644
> --- a/drivers/net/ethernet/xilinx/Kconfig
> +++ b/drivers/net/ethernet/xilinx/Kconfig
> @@ -27,6 +27,7 @@ config XILINX_AXI_EMAC
> tristate "Xilinx 10/100/1000 AXI Ethernet support"
> depends on HAS_IOMEM
> depends on XILINX_DMA
> + select OF_DYNAMIC if PCS_XILINX
> select PHYLINK
> select DIMLIB
> help
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h
> b/drivers/net/ethernet/xilinx/xilinx_axienet.h
> index 5ff742103beb..f46e862245eb 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h
> @@ -473,7 +473,6 @@ struct skbuf_dma_descriptor {
> * @dev: Pointer to device structure
> * @phylink: Pointer to phylink instance
> * @phylink_config: phylink configuration settings
> - * @pcs_phy: Reference to PCS/PMA PHY if used
> * @pcs: phylink pcs structure for PCS PHY
> * @switch_x_sgmii: Whether switchable 1000BaseX/SGMII mode is enabled in
> the core
> * @axi_clk: AXI4-Lite bus clock
> @@ -553,8 +552,7 @@ struct axienet_local {
> struct phylink *phylink;
> struct phylink_config phylink_config;
>
> - struct mdio_device *pcs_phy;
> - struct phylink_pcs pcs;
> + struct phylink_pcs *pcs;
>
> bool switch_x_sgmii;
>
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index 054abf283ab3..07487c4b2141 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -35,6 +35,8 @@
> #include <linux/platform_device.h>
> #include <linux/skbuff.h>
> #include <linux/math64.h>
> +#include <linux/pcs.h>
> +#include <linux/pcs-xilinx.h>
> #include <linux/phy.h>
> #include <linux/mii.h>
> #include <linux/ethtool.h>
> @@ -2519,63 +2521,6 @@ static const struct ethtool_ops axienet_ethtool_ops = {
> .get_rmon_stats = axienet_ethtool_get_rmon_stats, };
>
> -static struct axienet_local *pcs_to_axienet_local(struct phylink_pcs *pcs) -{
> - return container_of(pcs, struct axienet_local, pcs);
> -}
> -
> -static void axienet_pcs_get_state(struct phylink_pcs *pcs,
> - unsigned int neg_mode,
> - struct phylink_link_state *state)
> -{
> - struct mdio_device *pcs_phy = pcs_to_axienet_local(pcs)->pcs_phy;
> -
> - phylink_mii_c22_pcs_get_state(pcs_phy, neg_mode, state);
> -}
> -
> -static void axienet_pcs_an_restart(struct phylink_pcs *pcs) -{
> - struct mdio_device *pcs_phy = pcs_to_axienet_local(pcs)->pcs_phy;
> -
> - phylink_mii_c22_pcs_an_restart(pcs_phy);
> -}
> -
> -static int axienet_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
> - phy_interface_t interface,
> - const unsigned long *advertising,
> - bool permit_pause_to_mac)
> -{
> - struct mdio_device *pcs_phy = pcs_to_axienet_local(pcs)->pcs_phy;
> - struct net_device *ndev = pcs_to_axienet_local(pcs)->ndev;
> - struct axienet_local *lp = netdev_priv(ndev);
> - int ret;
> -
> - if (lp->switch_x_sgmii) {
> - ret = mdiodev_write(pcs_phy, XLNX_MII_STD_SELECT_REG,
> - interface == PHY_INTERFACE_MODE_SGMII ?
> - XLNX_MII_STD_SELECT_SGMII : 0);
> - if (ret < 0) {
> - netdev_warn(ndev,
> - "Failed to switch PHY interface: %d\n",
> - ret);
> - return ret;
> - }
> - }
> -
> - ret = phylink_mii_c22_pcs_config(pcs_phy, interface, advertising,
> - neg_mode);
> - if (ret < 0)
> - netdev_warn(ndev, "Failed to configure PCS: %d\n", ret);
> -
> - return ret;
> -}
> -
> -static const struct phylink_pcs_ops axienet_pcs_ops = {
> - .pcs_get_state = axienet_pcs_get_state,
> - .pcs_config = axienet_pcs_config,
> - .pcs_an_restart = axienet_pcs_an_restart,
> -};
> -
> static struct phylink_pcs *axienet_mac_select_pcs(struct phylink_config *config,
> phy_interface_t interface) { @@ -2583,8 +2528,8
> @@ static struct phylink_pcs *axienet_mac_select_pcs(struct phylink_config
> *config,
> struct axienet_local *lp = netdev_priv(ndev);
>
> if (interface == PHY_INTERFACE_MODE_1000BASEX ||
> - interface == PHY_INTERFACE_MODE_SGMII)
> - return &lp->pcs;
> + interface == PHY_INTERFACE_MODE_SGMII)

nit: unchanged check.

> + return lp->pcs;
>
> return NULL;
> }
> @@ -3056,28 +3001,23 @@ static int axienet_probe(struct platform_device *pdev)
>
> if (lp->phy_mode == PHY_INTERFACE_MODE_SGMII ||
> lp->phy_mode == PHY_INTERFACE_MODE_1000BASEX) {
> - np = of_parse_phandle(pdev->dev.of_node, "pcs-handle", 0);
> - if (!np) {
> - /* Deprecated: Always use "pcs-handle" for pcs_phy.
> - * Falling back to "phy-handle" here is only for
> - * backward compatibility with old device trees.
> - */
> - np = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0);
> - }
> - if (!np) {
> - dev_err(&pdev->dev, "pcs-handle (preferred) or phy-handle required
> for 1000BaseX/SGMII\n");
> - ret = -EINVAL;
> - goto cleanup_mdio;
> - }
> - lp->pcs_phy = of_mdio_find_device(np);
> - if (!lp->pcs_phy) {
> - ret = -EPROBE_DEFER;
> - of_node_put(np);
> + DECLARE_PHY_INTERFACE_MASK(interfaces);
> +
> + phy_interface_zero(interfaces);
> + if (lp->switch_x_sgmii ||
> + lp->phy_mode == PHY_INTERFACE_MODE_SGMII)
> + __set_bit(PHY_INTERFACE_MODE_SGMII, interfaces);
> + if (lp->switch_x_sgmii ||
> + lp->phy_mode == PHY_INTERFACE_MODE_1000BASEX)
> + __set_bit(PHY_INTERFACE_MODE_1000BASEX,
> + interfaces);
> +
> + lp->pcs = axienet_xilinx_pcs_get(&pdev->dev, interfaces);
> + if (IS_ERR(lp->pcs)) {
> + ret = PTR_ERR(lp->pcs);
> + dev_err_probe(&pdev->dev, ret,
> + "could not get PCS for
> + 1000BASE-X/SGMII\n");
> goto cleanup_mdio;
> }
> - of_node_put(np);
> - lp->pcs.ops = &axienet_pcs_ops;
> - lp->pcs.poll = true;
> }
>
> lp->phylink_config.dev = &ndev->dev; @@ -3115,8 +3055,6 @@ static int
> axienet_probe(struct platform_device *pdev)
> phylink_destroy(lp->phylink);
>
> cleanup_mdio:
> - if (lp->pcs_phy)
> - put_device(&lp->pcs_phy->dev);
> if (lp->mii_bus)
> axienet_mdio_teardown(lp);
> cleanup_clk:
> @@ -3139,9 +3077,7 @@ static void axienet_remove(struct platform_device
> *pdev)
> if (lp->phylink)
> phylink_destroy(lp->phylink);
>
> - if (lp->pcs_phy)
> - put_device(&lp->pcs_phy->dev);
> -
> + pcs_put(&pdev->dev, lp->pcs);
> axienet_mdio_teardown(lp);
>
> clk_bulk_disable_unprepare(XAE_NUM_MISC_CLOCKS, lp->misc_clks); diff
> --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig index
> 261d2fd29fc7..90ca0002600b 100644
> --- a/drivers/net/pcs/Kconfig
> +++ b/drivers/net/pcs/Kconfig
> @@ -57,7 +57,6 @@ config PCS_XILINX
> depends on PCS
> select MDIO_DEVICE
> select PHYLINK
> - default XILINX_AXI_EMAC
> tristate "Xilinx PCS driver"
> help
> PCS driver for the Xilinx 1G/2.5G Ethernet PCS/PMA or SGMII device.
> --
> 2.35.1.1320.gc452695387.dirty
>

Next message: Andy Yan: "Re:Re: [PATCH v3 7/7] drm/rockchip: inno-hdmi: Convert to drm bridge"
Previous message: Jason Gunthorpe: "Re: [PATCH] iommu/arm-smmu-v3: Add missing S2FWB feature detection"
Next in thread: Sean Anderson: "Re: [net-next PATCH v2 11/14] net: axienet: Convert to use PCS subsystem"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]