[RFC net-next PATCH 00/13] Add PCS core support

[RFC net-next PATCH 00/13] Add PCS core support

[Sean Anderson]

This series adds support for creating PCSs as devices on a bus with a
driver (patch 3). As initial users,

- The Lynx PCS (and all of its users) is converted to this system (patch 5)
- The Xilinx PCS is broken out from the AXI Ethernet driver (patches 6-8)
- The Cadence MACB driver is converted to support external PCSs (namely
  the Xilinx PCS) (patches 9-10).

The last few patches add device links for pcs-handle to improve boot times,
and add compatibles for all Lynx PCSs.

Care has been taken to ensure backwards-compatibility. The main source
of this is that many PCS devices lack compatibles and get detected as
PHYs. To address this, pcs_get_by_fwnode_compat allows drivers to edit
the devicetree to add appropriate compatibles.

This is technically a v3 (with [1,2] being v1 and v2, respectively), but
there have been so many architectural changes that I didn't bother
maintaining the series changelog. I think it is best to review this
series independently of its past iterations. I submitted this as an RFC
since net-next is closed, but I believe this series is fully tested and
ready to be applied.

[1] https://lore.kernel.org/netdev/20211004191527.1610759-1-sean.anderson@seco.com/
[2] https://lore.kernel.org/netdev/20221103210650.2325784-1-sean.anderson@seco.com/


Sean Anderson (12):
  dt-bindings: net: Add binding for Xilinx PCS
  net: phylink: Support setting PCS link change callbacks
  net: pcs: Add subsystem
  net: pcs: lynx: Convert to an MDIO driver
  net: phy: Export some functions
  net: pcs: Add Xilinx PCS driver
  net: axienet: Convert to use PCS subsystem
  net: macb: Move most of mac_config to mac_prepare
  net: macb: Support external PCSs
  of: property: Add device link support for PCS
  arm64: dts: Add compatible strings for Lynx PCSs
  powerpc: dts: Add compatible strings for Lynx PCSs

Vladimir Oltean (1):
  net: dsa: ocelot: suppress PHY device scanning on the internal MDIO
    bus

 .../devicetree/bindings/net/xilinx,pcs.yaml   | 129 ++++
 Documentation/networking/index.rst            |   1 +
 Documentation/networking/kapi.rst             |   4 +
 Documentation/networking/pcs.rst              |  99 +++
 MAINTAINERS                                   |   8 +
 .../arm64/boot/dts/freescale/fsl-ls208xa.dtsi |  48 +-
 .../arm64/boot/dts/freescale/fsl-lx2160a.dtsi |  54 +-
 .../dts/freescale/qoriq-fman3-0-10g-0.dtsi    |   3 +-
 .../dts/freescale/qoriq-fman3-0-10g-1.dtsi    |   3 +-
 .../dts/freescale/qoriq-fman3-0-1g-0.dtsi     |   3 +-
 .../dts/freescale/qoriq-fman3-0-1g-1.dtsi     |   3 +-
 .../dts/freescale/qoriq-fman3-0-1g-2.dtsi     |   3 +-
 .../dts/freescale/qoriq-fman3-0-1g-3.dtsi     |   3 +-
 .../dts/freescale/qoriq-fman3-0-1g-4.dtsi     |   3 +-
 .../dts/freescale/qoriq-fman3-0-1g-5.dtsi     |   3 +-
 .../fsl/qoriq-fman3-0-10g-0-best-effort.dtsi  |   3 +-
 .../boot/dts/fsl/qoriq-fman3-0-10g-0.dtsi     |   3 +-
 .../fsl/qoriq-fman3-0-10g-1-best-effort.dtsi  |   3 +-
 .../boot/dts/fsl/qoriq-fman3-0-10g-1.dtsi     |   3 +-
 .../boot/dts/fsl/qoriq-fman3-0-10g-2.dtsi     |   3 +-
 .../boot/dts/fsl/qoriq-fman3-0-10g-3.dtsi     |   3 +-
 .../boot/dts/fsl/qoriq-fman3-0-1g-0.dtsi      |   3 +-
 .../boot/dts/fsl/qoriq-fman3-0-1g-1.dtsi      |   3 +-
 .../boot/dts/fsl/qoriq-fman3-0-1g-2.dtsi      |   3 +-
 .../boot/dts/fsl/qoriq-fman3-0-1g-3.dtsi      |   3 +-
 .../boot/dts/fsl/qoriq-fman3-0-1g-4.dtsi      |   3 +-
 .../boot/dts/fsl/qoriq-fman3-0-1g-5.dtsi      |   3 +-
 .../boot/dts/fsl/qoriq-fman3-1-10g-0.dtsi     |   3 +-
 .../boot/dts/fsl/qoriq-fman3-1-10g-1.dtsi     |   3 +-
 .../boot/dts/fsl/qoriq-fman3-1-1g-0.dtsi      |   3 +-
 .../boot/dts/fsl/qoriq-fman3-1-1g-1.dtsi      |   3 +-
 .../boot/dts/fsl/qoriq-fman3-1-1g-2.dtsi      |   3 +-
 .../boot/dts/fsl/qoriq-fman3-1-1g-3.dtsi      |   3 +-
 .../boot/dts/fsl/qoriq-fman3-1-1g-4.dtsi      |   3 +-
 .../boot/dts/fsl/qoriq-fman3-1-1g-5.dtsi      |   3 +-
 drivers/net/dsa/ocelot/Kconfig                |   4 +
 drivers/net/dsa/ocelot/felix_vsc9959.c        |  15 +-
 drivers/net/dsa/ocelot/seville_vsc9953.c      |  16 +-
 drivers/net/ethernet/altera/Kconfig           |   2 +
 drivers/net/ethernet/altera/altera_tse_main.c |   7 +-
 drivers/net/ethernet/cadence/macb.h           |   1 +
 drivers/net/ethernet/cadence/macb_main.c      | 229 ++++--
 drivers/net/ethernet/freescale/dpaa/Kconfig   |   2 +-
 drivers/net/ethernet/freescale/dpaa2/Kconfig  |   3 +
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  |  11 +-
 drivers/net/ethernet/freescale/enetc/Kconfig  |   2 +
 .../net/ethernet/freescale/enetc/enetc_pf.c   |   8 +-
 .../net/ethernet/freescale/enetc/enetc_pf.h   |   1 -
 .../freescale/enetc/enetc_pf_common.c         |   4 +-
 drivers/net/ethernet/freescale/fman/Kconfig   |   4 +-
 .../net/ethernet/freescale/fman/fman_memac.c  |  27 +-
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |   3 +
 .../ethernet/stmicro/stmmac/dwmac-socfpga.c   |   6 +-
 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                       |  51 +-
 drivers/net/pcs/Makefile                      |   4 +
 drivers/net/pcs/core.c                        | 701 ++++++++++++++++++
 drivers/net/pcs/pcs-lynx.c                    | 115 +--
 drivers/net/pcs/pcs-xilinx.c                  | 481 ++++++++++++
 drivers/net/phy/mdio_device.c                 |   1 +
 drivers/net/phy/phy_device.c                  |   3 +-
 drivers/net/phy/phylink.c                     |  24 +-
 drivers/of/property.c                         |   2 +
 include/linux/pcs-lynx.h                      |  13 +-
 include/linux/pcs-xilinx.h                    |  36 +
 include/linux/pcs.h                           | 122 +++
 include/linux/phy.h                           |   1 +
 include/linux/phylink.h                       |  27 +-
 70 files changed, 2104 insertions(+), 358 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/xilinx,pcs.yaml
 create mode 100644 Documentation/networking/pcs.rst
 create mode 100644 drivers/net/pcs/core.c
 create mode 100644 drivers/net/pcs/pcs-xilinx.c
 create mode 100644 include/linux/pcs-xilinx.h
 create mode 100644 include/linux/pcs.h

-- 
2.35.1.1320.gc452695387.dirty

[RFC net-next PATCH 07/13] net: pcs: Add Xilinx PCS driver

[Sean Anderson]

This adds support for the Xilinx 1G/2.5G Ethernet PCS/PMA or SGMII device.
This is a soft device which converts between GMII and either SGMII,
1000Base-X, or 2500Base-X. If configured correctly, it can also switch
between SGMII and 1000BASE-X at runtime. Thoretically this is also possible
for 2500Base-X, but that requires reconfiguring the serdes. The exact
capabilities depend on synthesis parameters, so they are read from the
devicetree.

This device has a c22-compliant PHY interface, so for the most part we can
just use the phylink helpers. This device supports an interrupt which is
triggered on autonegotiation completion. I'm not sure how useful this is,
since we can never detect a link down (in the PCS).

This device supports sharing some logic between different implementations
of the device. In this case, one device contains the "shared logic" and the
clocks are connected to other devices. To coordinate this, one device
registers a clock that the other devices can request.  The clock is enabled
in the probe function by releasing the device from reset. There are no othe
software controls, so the clock ops are empty.

Later in this series, we will convert the Xilinx AXI Ethernet driver to use
this PCS. To help out, we provide a compatibility function to bind this
driver in the event the MDIO device has no compatible.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 MAINTAINERS                  |   6 +
 drivers/net/pcs/Kconfig      |  28 ++
 drivers/net/pcs/Makefile     |   2 +
 drivers/net/pcs/pcs-xilinx.c | 481 +++++++++++++++++++++++++++++++++++
 include/linux/pcs-xilinx.h   |  36 +++
 5 files changed, 553 insertions(+)
 create mode 100644 drivers/net/pcs/pcs-xilinx.c
 create mode 100644 include/linux/pcs-xilinx.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 9d3b3788a005..452096e6b04f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26160,6 +26160,12 @@ L:	netdev@vger.kernel.org
 S:	Orphan
 F:	drivers/net/ethernet/xilinx/ll_temac*
 
+XILINX PCS DRIVER
+M:	Sean Anderson <sean.anderson@linux.dev>
+S:	Maintained
+F:	Documentation/devicetree/bindings/net/xilinx,pcs.yaml
+F:	drivers/net/pcs/pcs-xilinx.c
+
 XILINX PWM DRIVER
 M:	Sean Anderson <sean.anderson@seco.com>
 S:	Maintained
diff --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig
index 91ff59899aaf..c28b4630492a 100644
--- a/drivers/net/pcs/Kconfig
+++ b/drivers/net/pcs/Kconfig
@@ -50,4 +50,32 @@ config PCS_RZN1_MIIC
 	  on RZ/N1 SoCs. This PCS converts MII to RMII/RGMII or can be set in
 	  pass-through mode for MII.
 
+config PCS_ALTERA_TSE
+	tristate
+	help
+	  This module provides helper functions for the Altera Triple Speed
+	  Ethernet SGMII PCS, that can be found on the Intel Socfpga family.
+
+config PCS_XILINX
+	depends on OF
+	depends on GPIOLIB
+	depends on COMMON_CLK
+	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.
+	  This device can either act as a PCS+PMA for 1000BASE-X or 2500BASE-X,
+	  or as a GMII-to-SGMII bridge. It can also switch between 1000BASE-X
+	  and SGMII dynamically if configured correctly when synthesized.
+	  Typical applications use this device on an FPGA connected to a GEM or
+	  TEMAC on the GMII side. The other side is typically connected to
+	  on-device gigabit transceivers, off-device SERDES devices using TBI,
+	  or LVDS IO resources directly.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pcs-xilinx.
+
 endmenu
diff --git a/drivers/net/pcs/Makefile b/drivers/net/pcs/Makefile
index 35e3324fc26e..347afd91f034 100644
--- a/drivers/net/pcs/Makefile
+++ b/drivers/net/pcs/Makefile
@@ -10,3 +10,5 @@ obj-$(CONFIG_PCS_XPCS)		+= pcs_xpcs.o
 obj-$(CONFIG_PCS_LYNX)		+= pcs-lynx.o
 obj-$(CONFIG_PCS_MTK_LYNXI)	+= pcs-mtk-lynxi.o
 obj-$(CONFIG_PCS_RZN1_MIIC)	+= pcs-rzn1-miic.o
+obj-$(CONFIG_PCS_ALTERA_TSE)	+= pcs-altera-tse.o
+obj-$(CONFIG_PCS_XILINX)	+= pcs-xilinx.o
diff --git a/drivers/net/pcs/pcs-xilinx.c b/drivers/net/pcs/pcs-xilinx.c
new file mode 100644
index 000000000000..a278f86513c2
--- /dev/null
+++ b/drivers/net/pcs/pcs-xilinx.c
@@ -0,0 +1,481 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2021-25 Sean Anderson <sean.anderson@seco.com>
+ *
+ * This is the driver for the Xilinx 1G/2.5G Ethernet PCS/PMA or SGMII LogiCORE
+ * IP. A typical setup will look something like
+ *
+ * MAC <--GMII--> PCS/PMA <--1000BASE-X--> SFP module (PMD)
+ *
+ * The IEEE model mostly describes this device, but the PCS layer has a
+ * separate sublayer for 8b/10b en/decoding:
+ *
+ * - When using a device-specific transceiver (serdes), the serdes handles 8b/10b
+ *   en/decoding and PMA functions. The IP implements other PCS functions.
+ * - When using LVDS IO resources, the IP implements PCS and PMA functions,
+ *   including 8b/10b en/decoding and (de)serialization.
+ * - When using an external serdes (accessed via TBI), the IP implements all
+ *   PCS functions, including 8b/10b en/decoding.
+ *
+ * The link to the PMD is not modeled by this driver, except for refclk. It is
+ * assumed that the serdes (if present) needs no configuration, though it
+ * should be fairly easy to add support. It is also possible to go from SGMII
+ * to GMII (PHY mode), but this is not supported.
+ *
+ * This driver was written with reference to PG047:
+ * https://docs.amd.com/r/en-US/pg047-gig-eth-pcs-pma
+ */
+
+#include <linux/bitmap.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/gpio/consumer.h>
+#include <linux/iopoll.h>
+#include <linux/mdio.h>
+#include <linux/of.h>
+#include <linux/pcs.h>
+#include <linux/pcs-xilinx.h>
+#include <linux/phylink.h>
+#include <linux/property.h>
+
+/* Vendor-specific MDIO registers */
+#define XILINX_PCS_ANICR 16 /* Auto-Negotiation Interrupt Control Register */
+#define XILINX_PCS_SSR   17 /* Standard Selection Register */
+
+#define XILINX_PCS_ANICR_IE BIT(0) /* Interrupt Enable */
+#define XILINX_PCS_ANICR_IS BIT(1) /* Interrupt Status */
+
+#define XILINX_PCS_SSR_SGMII BIT(0) /* Select SGMII standard */
+
+/**
+ * struct xilinx_pcs - Private data for Xilinx PCS devices
+ * @pcs: The phylink PCS
+ * @mdiodev: The mdiodevice used to access the PCS
+ * @refclk: The reference clock for the PMD
+ * @refclk_out: Optional reference clock for other PCSs using this PCS's shared
+ *              logic
+ * @reset: The reset line for the PCS
+ * @done: Optional GPIO for reset_done
+ * @irq: IRQ, or -EINVAL if polling
+ * @enabled: Set if @pcs.link_change is valid and we can call phylink_pcs_change()
+ */
+struct xilinx_pcs {
+	struct phylink_pcs pcs;
+	struct clk_hw refclk_out;
+	struct clk *refclk;
+	struct gpio_desc *reset, *done;
+	struct mdio_device *mdiodev;
+	int irq;
+	bool enabled;
+};
+
+static inline struct xilinx_pcs *pcs_to_xilinx(struct phylink_pcs *pcs)
+{
+	return container_of(pcs, struct xilinx_pcs, pcs);
+}
+
+static irqreturn_t xilinx_pcs_an_irq(int irq, void *dev_id)
+{
+	struct xilinx_pcs *xp = dev_id;
+
+	if (mdiodev_modify_changed(xp->mdiodev, XILINX_PCS_ANICR,
+				   XILINX_PCS_ANICR_IS, 0) <= 0)
+		return IRQ_NONE;
+
+	/* paired with xilinx_pcs_enable/disable; protects xp->pcs->link_change */
+	if (smp_load_acquire(&xp->enabled))
+		phylink_pcs_change(&xp->pcs, true);
+	return IRQ_HANDLED;
+}
+
+static int xilinx_pcs_enable(struct phylink_pcs *pcs)
+{
+	struct xilinx_pcs *xp = pcs_to_xilinx(pcs);
+	struct device *dev = &xp->mdiodev->dev;
+	int ret;
+
+	if (xp->irq < 0)
+		return 0;
+
+	ret = mdiodev_modify(xp->mdiodev, XILINX_PCS_ANICR, 0,
+			     XILINX_PCS_ANICR_IE);
+	if (ret)
+		dev_err(dev, "could not clear IRQ enable: %d\n", ret);
+	else
+		/* paired with xilinx_pcs_an_irq */
+		smp_store_release(&xp->enabled, true);
+	return ret;
+}
+
+static void xilinx_pcs_disable(struct phylink_pcs *pcs)
+{
+	struct xilinx_pcs *xp = pcs_to_xilinx(pcs);
+	struct device *dev = &xp->mdiodev->dev;
+	int err;
+
+	if (xp->irq < 0)
+		return;
+
+	WRITE_ONCE(xp->enabled, false);
+	/* paired with xilinx_pcs_an_irq */
+	smp_wmb();
+
+	err = mdiodev_modify(xp->mdiodev, XILINX_PCS_ANICR,
+			     XILINX_PCS_ANICR_IE, 0);
+	if (err)
+		dev_err(dev, "could not clear IRQ enable: %d\n", err);
+}
+
+static int xilinx_pcs_validate(struct phylink_pcs *pcs,
+			       unsigned long *supported,
+			       const struct phylink_link_state *state)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(xilinx_supported) = { 0 };
+
+	phylink_set_port_modes(xilinx_supported);
+	phylink_set(xilinx_supported, Autoneg);
+	phylink_set(xilinx_supported, Pause);
+	phylink_set(xilinx_supported, Asym_Pause);
+	switch (state->interface) {
+	case PHY_INTERFACE_MODE_SGMII:
+		/* Half duplex not supported */
+		phylink_set(xilinx_supported, 10baseT_Full);
+		phylink_set(xilinx_supported, 100baseT_Full);
+		phylink_set(xilinx_supported, 1000baseT_Full);
+		break;
+	case PHY_INTERFACE_MODE_1000BASEX:
+		phylink_set(xilinx_supported, 1000baseX_Full);
+		break;
+	case PHY_INTERFACE_MODE_2500BASEX:
+		phylink_set(xilinx_supported, 2500baseX_Full);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	linkmode_and(supported, supported, xilinx_supported);
+	return 0;
+}
+
+static void xilinx_pcs_get_state(struct phylink_pcs *pcs,
+				 unsigned int neg_mode,
+				 struct phylink_link_state *state)
+{
+	struct xilinx_pcs *xp = pcs_to_xilinx(pcs);
+
+	phylink_mii_c22_pcs_get_state(xp->mdiodev, neg_mode, state);
+}
+
+static int xilinx_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
+			     phy_interface_t interface,
+			     const unsigned long *advertising,
+			     bool permit_pause_to_mac)
+{
+	int ret, changed = 0;
+	struct xilinx_pcs *xp = pcs_to_xilinx(pcs);
+
+	if (test_bit(PHY_INTERFACE_MODE_SGMII, pcs->supported_interfaces) &&
+	    test_bit(PHY_INTERFACE_MODE_1000BASEX, pcs->supported_interfaces)) {
+		u16 ssr;
+
+		if (interface == PHY_INTERFACE_MODE_SGMII)
+			ssr = XILINX_PCS_SSR_SGMII;
+		else
+			ssr = 0;
+
+		changed = mdiodev_modify_changed(xp->mdiodev, XILINX_PCS_SSR,
+						 XILINX_PCS_SSR_SGMII, ssr);
+		if (changed < 0)
+			return changed;
+	}
+
+	ret = phylink_mii_c22_pcs_config(xp->mdiodev, interface, advertising,
+					 neg_mode);
+	return ret ?: changed;
+}
+
+static void xilinx_pcs_an_restart(struct phylink_pcs *pcs)
+{
+	struct xilinx_pcs *xp = pcs_to_xilinx(pcs);
+
+	phylink_mii_c22_pcs_an_restart(xp->mdiodev);
+}
+
+static void xilinx_pcs_link_up(struct phylink_pcs *pcs, unsigned int mode,
+			       phy_interface_t interface, int speed, int duplex)
+{
+	int bmcr;
+	struct xilinx_pcs *xp = pcs_to_xilinx(pcs);
+
+	if (phylink_autoneg_inband(mode))
+		return;
+
+	bmcr = mdiodev_read(xp->mdiodev, MII_BMCR);
+	if (bmcr < 0) {
+		dev_err(&xp->mdiodev->dev, "could not read BMCR (err=%d)\n",
+			bmcr);
+		return;
+	}
+
+	bmcr &= ~(BMCR_SPEED1000 | BMCR_SPEED100);
+	switch (speed) {
+	case SPEED_2500:
+	case SPEED_1000:
+		bmcr |= BMCR_SPEED1000;
+		break;
+	case SPEED_100:
+		bmcr |= BMCR_SPEED100;
+		break;
+	case SPEED_10:
+		bmcr |= BMCR_SPEED10;
+		break;
+	default:
+		dev_err(&xp->mdiodev->dev, "invalid speed %d\n", speed);
+	}
+
+	bmcr = mdiodev_write(xp->mdiodev, MII_BMCR, bmcr);
+	if (bmcr < 0)
+		dev_err(&xp->mdiodev->dev, "could not write BMCR (err=%d)\n",
+			bmcr);
+}
+
+static const struct phylink_pcs_ops xilinx_pcs_ops = {
+	.pcs_validate = xilinx_pcs_validate,
+	.pcs_enable = xilinx_pcs_enable,
+	.pcs_disable = xilinx_pcs_disable,
+	.pcs_get_state = xilinx_pcs_get_state,
+	.pcs_config = xilinx_pcs_config,
+	.pcs_an_restart = xilinx_pcs_an_restart,
+	.pcs_link_up = xilinx_pcs_link_up,
+};
+
+static const struct clk_ops xilinx_pcs_clk_ops = { };
+
+static const phy_interface_t xilinx_pcs_interfaces[] = {
+	PHY_INTERFACE_MODE_SGMII,
+	PHY_INTERFACE_MODE_1000BASEX,
+	PHY_INTERFACE_MODE_2500BASEX,
+};
+
+static int xilinx_pcs_probe(struct mdio_device *mdiodev)
+{
+	struct device *dev = &mdiodev->dev;
+	struct fwnode_handle *fwnode = dev->fwnode;
+	int ret, i, j, mode_count;
+	struct xilinx_pcs *xp;
+	const char **modes;
+	u32 phy_id;
+
+	xp = devm_kzalloc(dev, sizeof(*xp), GFP_KERNEL);
+	if (!xp)
+		return -ENOMEM;
+	xp->mdiodev = mdiodev;
+	dev_set_drvdata(dev, xp);
+
+	xp->irq = fwnode_irq_get_byname(fwnode, "an");
+	/* There's no _optional variant, so this is the best we've got */
+	if (xp->irq < 0 && xp->irq != -EINVAL)
+		return dev_err_probe(dev, xp->irq, "could not get IRQ\n");
+
+	mode_count = fwnode_property_string_array_count(fwnode, "pcs-modes");
+	if (!mode_count)
+		mode_count = -ENODATA;
+	if (mode_count < 0) {
+		dev_err(dev, "could not read pcs-modes: %d", mode_count);
+		return mode_count;
+	}
+
+	modes = kcalloc(mode_count, sizeof(*modes), GFP_KERNEL);
+	if (!modes)
+		return -ENOMEM;
+
+	ret = fwnode_property_read_string_array(fwnode, "pcs-modes",
+						modes, mode_count);
+	if (ret < 0) {
+		dev_err(dev, "could not read pcs-modes: %d\n", ret);
+		kfree(modes);
+		return ret;
+	}
+
+	for (i = 0; i < mode_count; i++) {
+		for (j = 0; j < ARRAY_SIZE(xilinx_pcs_interfaces); j++) {
+			if (!strcmp(phy_modes(xilinx_pcs_interfaces[j]), modes[i])) {
+				__set_bit(xilinx_pcs_interfaces[j],
+					  xp->pcs.supported_interfaces);
+				goto next;
+			}
+		}
+
+		dev_err(dev, "invalid pcs-mode \"%s\"\n", modes[i]);
+		kfree(modes);
+		return -EINVAL;
+next:
+	}
+
+	kfree(modes);
+	if ((test_bit(PHY_INTERFACE_MODE_SGMII, xp->pcs.supported_interfaces) ||
+	     test_bit(PHY_INTERFACE_MODE_1000BASEX, xp->pcs.supported_interfaces)) &&
+	    test_bit(PHY_INTERFACE_MODE_2500BASEX, xp->pcs.supported_interfaces)) {
+		dev_err(dev,
+			"Switching from SGMII or 1000Base-X to 2500Base-X not supported\n");
+		return -EINVAL;
+	}
+
+	xp->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(xp->reset))
+		return dev_err_probe(dev, PTR_ERR(xp->reset),
+				     "could not get reset gpio\n");
+
+	xp->done = devm_gpiod_get_optional(dev, "done", GPIOD_IN);
+	if (IS_ERR(xp->done))
+		return dev_err_probe(dev, PTR_ERR(xp->done),
+				     "could not get done gpio\n");
+
+	xp->refclk = devm_clk_get_optional_enabled(dev, "refclk");
+	if (IS_ERR(xp->refclk))
+		return dev_err_probe(dev, PTR_ERR(xp->refclk),
+				     "could not get/enable reference clock\n");
+
+	gpiod_set_value_cansleep(xp->reset, 0);
+	if (xp->done) {
+		if (read_poll_timeout(gpiod_get_value_cansleep, ret, ret, 1000,
+				      100000, true, xp->done))
+			return dev_err_probe(dev, -ETIMEDOUT,
+					     "timed out waiting for reset\n");
+	} else {
+		/* Just wait for a while and hope we're done */
+		usleep_range(50000, 100000);
+	}
+
+	if (fwnode_property_present(fwnode, "#clock-cells")) {
+		const char *parent = "refclk";
+		struct clk_init_data init = {
+			.name = fwnode_get_name(fwnode),
+			.ops = &xilinx_pcs_clk_ops,
+			.parent_names = &parent,
+			.num_parents = 1,
+			.flags = 0,
+		};
+
+		xp->refclk_out.init = &init;
+		ret = devm_clk_hw_register(dev, &xp->refclk_out);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "could not register refclk\n");
+
+		ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
+						  &xp->refclk_out);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "could not register refclk\n");
+	}
+
+	/* Sanity check */
+	ret = get_phy_c22_id(mdiodev->bus, mdiodev->addr, &phy_id);
+	if (ret) {
+		dev_err_probe(dev, ret, "could not read id\n");
+		return ret;
+	}
+	if ((phy_id & 0xfffffff0) != 0x01740c00)
+		dev_warn(dev, "unknown phy id %x\n", phy_id);
+
+	if (xp->irq < 0) {
+		xp->pcs.poll = true;
+	} else {
+		/* The IRQ is enabled by default; turn it off */
+		ret = mdiodev_write(xp->mdiodev, XILINX_PCS_ANICR, 0);
+		if (ret) {
+			dev_err(dev, "could not disable IRQ: %d\n", ret);
+			return ret;
+		}
+
+		/* Some PCSs have a bad habit of re-enabling their IRQ!
+		 * Request the IRQ in probe so we don't end up triggering the
+		 * spurious IRQ logic.
+		 */
+		ret = devm_request_threaded_irq(dev, xp->irq, NULL, xilinx_pcs_an_irq,
+						IRQF_SHARED | IRQF_ONESHOT,
+						dev_name(dev), xp);
+		if (ret) {
+			dev_err(dev, "could not request IRQ: %d\n", ret);
+			return ret;
+		}
+	}
+
+	xp->pcs.ops = &xilinx_pcs_ops;
+	ret = devm_pcs_register(dev, &xp->pcs);
+	if (ret)
+		return dev_err_probe(dev, ret, "could not register PCS\n");
+
+	if (xp->irq < 0)
+		dev_info(dev, "probed with irq=poll\n");
+	else
+		dev_info(dev, "probed with irq=%d\n", xp->irq);
+	return 0;
+}
+
+static const struct of_device_id xilinx_pcs_of_match[] = {
+	{ .compatible = "xlnx,pcs-16.2", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, xilinx_timer_of_match);
+
+static struct mdio_driver xilinx_pcs_driver = {
+	.probe = xilinx_pcs_probe,
+	.mdiodrv.driver = {
+		.name = "xilinx-pcs",
+		.of_match_table = of_match_ptr(xilinx_pcs_of_match),
+		.suppress_bind_attrs = true,
+	},
+};
+mdio_module_driver(xilinx_pcs_driver);
+
+static int axienet_xilinx_pcs_fixup(struct of_changeset *ocs,
+				    struct device_node *np, void *data)
+{
+#ifdef CONFIG_OF_DYNAMIC
+	unsigned int interface, mode_count, mode = 0;
+	const unsigned long *interfaces = data;
+	const char **modes;
+	int ret;
+
+	mode_count = bitmap_weight(interfaces, PHY_INTERFACE_MODE_MAX);
+	WARN_ON_ONCE(!mode_count);
+	modes = kcalloc(mode_count, sizeof(*modes), GFP_KERNEL);
+	if (!modes)
+		return -ENOMEM;
+
+	for_each_set_bit(interface, interfaces, PHY_INTERFACE_MODE_MAX)
+		modes[mode++] = phy_modes(interface);
+	ret = of_changeset_add_prop_string_array(ocs, np, "pcs-modes", modes,
+						 mode_count);
+	kfree(modes);
+	if (ret)
+		return ret;
+
+	return of_changeset_add_prop_string(ocs, np, "compatible",
+					    "xlnx,pcs-16.2");
+#else
+	return -ENODEV;
+#endif
+}
+
+struct phylink_pcs *axienet_xilinx_pcs_get(struct device *dev,
+					   const unsigned long *interfaces)
+{
+	struct fwnode_handle *fwnode;
+	struct phylink_pcs *pcs;
+
+	fwnode = pcs_find_fwnode(dev_fwnode(dev), NULL, "phy-handle", false);
+	if (IS_ERR(fwnode))
+		return ERR_CAST(fwnode);
+
+	pcs = pcs_get_by_fwnode_compat(dev, fwnode, axienet_xilinx_pcs_fixup,
+				       (void *)interfaces);
+	fwnode_handle_put(fwnode);
+	return pcs;
+}
+
+MODULE_ALIAS("platform:xilinx-pcs");
+MODULE_DESCRIPTION("Xilinx PCS driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/pcs-xilinx.h b/include/linux/pcs-xilinx.h
new file mode 100644
index 000000000000..409057fbdf34
--- /dev/null
+++ b/include/linux/pcs-xilinx.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2024 Sean Anderson <sean.anderson@seco.com>
+ */
+
+#ifndef PCS_XILINX_H
+#define PCS_XILINX_H
+
+#include <linux/err.h>
+
+struct device;
+struct phylink_pcs;
+
+#ifdef CONFIG_PCS_XILINX
+/**
+ * axienet_xilinx_pcs_get() - Compatibility function for the AXI Ethernet driver
+ * @dev: The MAC device
+ * @interfaces: The interfaces to use as a fallback
+ *
+ * This is a helper function for the AXI Ethernet driver to ensure backwards
+ * compatibility with device trees which do not include compatible strings for
+ * the PCS. It should not be used by new code.
+ *
+ * Return: a PCS, or an error pointer
+ */
+struct phylink_pcs *axienet_xilinx_pcs_get(struct device *dev,
+					   const unsigned long *interfaces);
+#else
+static inline struct phylink_pcs *
+axienet_xilinx_pcs_get(struct device *dev, const unsigned long *interfaces)
+{
+	return ERR_PTR(-ENODEV);
+}
+#endif
+
+#endif /* PCS_XILINX_H */
-- 
2.35.1.1320.gc452695387.dirty

[RFC net-next PATCH 08/13] net: axienet: Convert to use PCS subsystem

[Sean Anderson]

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.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 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)
+		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 c28b4630492a..d9369a525498 100644
--- a/drivers/net/pcs/Kconfig
+++ b/drivers/net/pcs/Kconfig
@@ -63,7 +63,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

[RFC net-next PATCH 12/13] arm64: dts: Add compatible strings for Lynx PCSs

[Sean Anderson]

This adds appropriate compatible strings for Lynx PCSs on arm64 QorIQ
platforms. This also changes the node name to avoid warnings from
ethernet-phy.yaml.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 .../arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 48 +++++++++++------
 .../arm64/boot/dts/freescale/fsl-lx2160a.dtsi | 54 ++++++++++++-------
 .../dts/freescale/qoriq-fman3-0-10g-0.dtsi    |  3 +-
 .../dts/freescale/qoriq-fman3-0-10g-1.dtsi    |  3 +-
 .../dts/freescale/qoriq-fman3-0-1g-0.dtsi     |  3 +-
 .../dts/freescale/qoriq-fman3-0-1g-1.dtsi     |  3 +-
 .../dts/freescale/qoriq-fman3-0-1g-2.dtsi     |  3 +-
 .../dts/freescale/qoriq-fman3-0-1g-3.dtsi     |  3 +-
 .../dts/freescale/qoriq-fman3-0-1g-4.dtsi     |  3 +-
 .../dts/freescale/qoriq-fman3-0-1g-5.dtsi     |  3 +-
 10 files changed, 84 insertions(+), 42 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
index 9421fdd7e30e..90c1631c958e 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
@@ -554,7 +554,8 @@ pcs_mdio1: mdio@8c07000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs1: ethernet-phy@0 {
+			pcs1: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -567,7 +568,8 @@ pcs_mdio2: mdio@8c0b000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs2: ethernet-phy@0 {
+			pcs2: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -580,7 +582,8 @@ pcs_mdio3: mdio@8c0f000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs3: ethernet-phy@0 {
+			pcs3: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -593,7 +596,8 @@ pcs_mdio4: mdio@8c13000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs4: ethernet-phy@0 {
+			pcs4: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -606,7 +610,8 @@ pcs_mdio5: mdio@8c17000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs5: ethernet-phy@0 {
+			pcs5: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -619,7 +624,8 @@ pcs_mdio6: mdio@8c1b000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs6: ethernet-phy@0 {
+			pcs6: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -632,7 +638,8 @@ pcs_mdio7: mdio@8c1f000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs7: ethernet-phy@0 {
+			pcs7: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -645,7 +652,8 @@ pcs_mdio8: mdio@8c23000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs8: ethernet-phy@0 {
+			pcs8: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -658,7 +666,8 @@ pcs_mdio9: mdio@8c27000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs9: ethernet-phy@0 {
+			pcs9: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -671,7 +680,8 @@ pcs_mdio10: mdio@8c2b000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs10: ethernet-phy@0 {
+			pcs10: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -684,7 +694,8 @@ pcs_mdio11: mdio@8c2f000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs11: ethernet-phy@0 {
+			pcs11: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -697,7 +708,8 @@ pcs_mdio12: mdio@8c33000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs12: ethernet-phy@0 {
+			pcs12: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -710,7 +722,8 @@ pcs_mdio13: mdio@8c37000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs13: ethernet-phy@0 {
+			pcs13: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -723,7 +736,8 @@ pcs_mdio14: mdio@8c3b000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs14: ethernet-phy@0 {
+			pcs14: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -736,7 +750,8 @@ pcs_mdio15: mdio@8c3f000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs15: ethernet-phy@0 {
+			pcs15: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -749,7 +764,8 @@ pcs_mdio16: mdio@8c43000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs16: ethernet-phy@0 {
+			pcs16: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
index c9541403bcd8..f35da67b6e61 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
@@ -1474,7 +1474,8 @@ pcs_mdio1: mdio@8c07000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs1: ethernet-phy@0 {
+			pcs1: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -1487,7 +1488,8 @@ pcs_mdio2: mdio@8c0b000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs2: ethernet-phy@0 {
+			pcs2: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -1500,7 +1502,8 @@ pcs_mdio3: mdio@8c0f000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs3: ethernet-phy@0 {
+			pcs3: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -1513,7 +1516,8 @@ pcs_mdio4: mdio@8c13000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs4: ethernet-phy@0 {
+			pcs4: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -1526,7 +1530,8 @@ pcs_mdio5: mdio@8c17000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs5: ethernet-phy@0 {
+			pcs5: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -1539,7 +1544,8 @@ pcs_mdio6: mdio@8c1b000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs6: ethernet-phy@0 {
+			pcs6: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -1552,7 +1558,8 @@ pcs_mdio7: mdio@8c1f000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs7: ethernet-phy@0 {
+			pcs7: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -1565,7 +1572,8 @@ pcs_mdio8: mdio@8c23000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs8: ethernet-phy@0 {
+			pcs8: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -1578,7 +1586,8 @@ pcs_mdio9: mdio@8c27000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs9: ethernet-phy@0 {
+			pcs9: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -1591,7 +1600,8 @@ pcs_mdio10: mdio@8c2b000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs10: ethernet-phy@0 {
+			pcs10: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -1604,7 +1614,8 @@ pcs_mdio11: mdio@8c2f000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs11: ethernet-phy@0 {
+			pcs11: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -1617,7 +1628,8 @@ pcs_mdio12: mdio@8c33000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs12: ethernet-phy@0 {
+			pcs12: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -1630,7 +1642,8 @@ pcs_mdio13: mdio@8c37000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs13: ethernet-phy@0 {
+			pcs13: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -1643,7 +1656,8 @@ pcs_mdio14: mdio@8c3b000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs14: ethernet-phy@0 {
+			pcs14: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -1656,7 +1670,8 @@ pcs_mdio15: mdio@8c3f000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs15: ethernet-phy@0 {
+			pcs15: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -1669,7 +1684,8 @@ pcs_mdio16: mdio@8c43000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs16: ethernet-phy@0 {
+			pcs16: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -1682,7 +1698,8 @@ pcs_mdio17: mdio@8c47000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs17: ethernet-phy@0 {
+			pcs17: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
@@ -1695,7 +1712,8 @@ pcs_mdio18: mdio@8c4b000 {
 			#size-cells = <0>;
 			status = "disabled";
 
-			pcs18: ethernet-phy@0 {
+			pcs18: ethernet-pcs@0 {
+				compatible = "fsl,lynx-pcs";
 				reg = <0>;
 			};
 		};
diff --git a/arch/arm64/boot/dts/freescale/qoriq-fman3-0-10g-0.dtsi b/arch/arm64/boot/dts/freescale/qoriq-fman3-0-10g-0.dtsi
index 1b2b20c6126d..e11c6ddab457 100644
--- a/arch/arm64/boot/dts/freescale/qoriq-fman3-0-10g-0.dtsi
+++ b/arch/arm64/boot/dts/freescale/qoriq-fman3-0-10g-0.dtsi
@@ -36,7 +36,8 @@ mdio@f1000 {
 		compatible = "fsl,fman-memac-mdio";
 		reg = <0xf1000 0x1000>;
 
-		pcsphy6: ethernet-phy@0 {
+		pcsphy6: ethernet-pcs@0 {
+			compatible = "fsl,lynx-pcs";
 			reg = <0x0>;
 		};
 	};
diff --git a/arch/arm64/boot/dts/freescale/qoriq-fman3-0-10g-1.dtsi b/arch/arm64/boot/dts/freescale/qoriq-fman3-0-10g-1.dtsi
index 55d78f6f7c6c..c8b7f2c61a8f 100644
--- a/arch/arm64/boot/dts/freescale/qoriq-fman3-0-10g-1.dtsi
+++ b/arch/arm64/boot/dts/freescale/qoriq-fman3-0-10g-1.dtsi
@@ -36,7 +36,8 @@ mdio@f3000 {
 		compatible = "fsl,fman-memac-mdio";
 		reg = <0xf3000 0x1000>;
 
-		pcsphy7: ethernet-phy@0 {
+		pcsphy7: ethernet-pcs@0 {
+			compatible = "fsl,lynx-pcs";
 			reg = <0x0>;
 		};
 	};
diff --git a/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-0.dtsi b/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-0.dtsi
index 18916a860c2e..1a4bcb38646e 100644
--- a/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-0.dtsi
+++ b/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-0.dtsi
@@ -35,7 +35,8 @@ mdio@e1000 {
 		compatible = "fsl,fman-memac-mdio";
 		reg = <0xe1000 0x1000>;
 
-		pcsphy0: ethernet-phy@0 {
+		pcsphy0: ethernet-pcs@0 {
+			compatible = "fsl,lynx-pcs";
 			reg = <0x0>;
 		};
 	};
diff --git a/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-1.dtsi b/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-1.dtsi
index e90af445a293..6a4d55f9d045 100644
--- a/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-1.dtsi
+++ b/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-1.dtsi
@@ -35,7 +35,8 @@ mdio@e3000 {
 		compatible = "fsl,fman-memac-mdio";
 		reg = <0xe3000 0x1000>;
 
-		pcsphy1: ethernet-phy@0 {
+		pcsphy1: ethernet-pcs@0 {
+			compatible = "fsl,lynx-pcs";
 			reg = <0x0>;
 		};
 	};
diff --git a/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-2.dtsi b/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-2.dtsi
index fec93905bc81..0de30065aa3b 100644
--- a/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-2.dtsi
+++ b/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-2.dtsi
@@ -35,7 +35,8 @@ mdio@e5000 {
 		compatible = "fsl,fman-memac-mdio";
 		reg = <0xe5000 0x1000>;
 
-		pcsphy2: ethernet-phy@0 {
+		pcsphy2: ethernet-pcs@0 {
+			compatible = "fsl,lynx-pcs";
 			reg = <0x0>;
 		};
 	};
diff --git a/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-3.dtsi b/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-3.dtsi
index 2aa953faa62b..2f8064b1039f 100644
--- a/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-3.dtsi
+++ b/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-3.dtsi
@@ -35,7 +35,8 @@ mdio@e7000 {
 		compatible = "fsl,fman-memac-mdio";
 		reg = <0xe7000 0x1000>;
 
-		pcsphy3: ethernet-phy@0 {
+		pcsphy3: ethernet-pcs@0 {
+			compatible = "fsl,lynx-pcs";
 			reg = <0x0>;
 		};
 	};
diff --git a/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-4.dtsi b/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-4.dtsi
index 948e39411415..6246f1fdac2d 100644
--- a/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-4.dtsi
+++ b/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-4.dtsi
@@ -35,7 +35,8 @@ mdio@e9000 {
 		compatible = "fsl,fman-memac-mdio";
 		reg = <0xe9000 0x1000>;
 
-		pcsphy4: ethernet-phy@0 {
+		pcsphy4: ethernet-pcs@0 {
+			compatible = "fsl,lynx-pcs";
 			reg = <0x0>;
 		};
 	};
diff --git a/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-5.dtsi b/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-5.dtsi
index 01b78c0463a7..c205e1e8bfc8 100644
--- a/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-5.dtsi
+++ b/arch/arm64/boot/dts/freescale/qoriq-fman3-0-1g-5.dtsi
@@ -34,7 +34,8 @@ mdio@eb000 {
 		compatible = "fsl,fman-memac-mdio";
 		reg = <0xeb000 0x1000>;
 
-		pcsphy5: ethernet-phy@0 {
+		pcsphy5: ethernet-pcs@0 {
+			compatible = "fsl,lynx-pcs";
 			reg = <0x0>;
 		};
 	};
-- 
2.35.1.1320.gc452695387.dirty

Re: [RFC net-next PATCH 07/13] net: pcs: Add Xilinx PCS driver

[Russell King (Oracle)]

On Thu, Apr 03, 2025 at 02:19:01PM -0400, Sean Anderson wrote:
> +static int xilinx_pcs_validate(struct phylink_pcs *pcs,
> +			       unsigned long *supported,
> +			       const struct phylink_link_state *state)
> +{
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(xilinx_supported) = { 0 };
> +
> +	phylink_set_port_modes(xilinx_supported);
> +	phylink_set(xilinx_supported, Autoneg);
> +	phylink_set(xilinx_supported, Pause);
> +	phylink_set(xilinx_supported, Asym_Pause);
> +	switch (state->interface) {
> +	case PHY_INTERFACE_MODE_SGMII:
> +		/* Half duplex not supported */
> +		phylink_set(xilinx_supported, 10baseT_Full);
> +		phylink_set(xilinx_supported, 100baseT_Full);
> +		phylink_set(xilinx_supported, 1000baseT_Full);
> +		break;
> +	case PHY_INTERFACE_MODE_1000BASEX:
> +		phylink_set(xilinx_supported, 1000baseX_Full);
> +		break;
> +	case PHY_INTERFACE_MODE_2500BASEX:
> +		phylink_set(xilinx_supported, 2500baseX_Full);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	linkmode_and(supported, supported, xilinx_supported);
> +	return 0;

You can not assume that an interface mode implies any particular media.
For example, you can not assume that just because you have SGMII, that
the only supported media is BaseT. This has been a fundamental principle
in phylink's validation since day one.

Phylink documentation for the pcs_validate() callback states:

 * Validate the interface mode, and advertising's autoneg bit, removing any
 * media ethtool link modes that would not be supportable from the supported
 * mask. Phylink will propagate the changes to the advertising mask. See the
 * &struct phylink_mac_ops validate() method.

and if we look at the MAC ops validate (before it was removed):

- * Clear bits in the @supported and @state->advertising masks that
- * are not supportable by the MAC.
- *
- * Note that the PHY may be able to transform from one connection
- * technology to another, so, eg, don't clear 1000BaseX just
- * because the MAC is unable to BaseX mode. This is more about
- * clearing unsupported speeds and duplex settings. The port modes
- * should not be cleared; phylink_set_port_modes() will help with this.

PHYs can and do take SGMII and provide both BaseT and BaseX or BaseR
connections. A PCS that is not directly media facing can not dictate
the link modes.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [RFC net-next PATCH 07/13] net: pcs: Add Xilinx PCS driver

[Sean Anderson]

On 4/3/25 16:27, Russell King (Oracle) wrote:
> On Thu, Apr 03, 2025 at 02:19:01PM -0400, Sean Anderson wrote:
>> +static int xilinx_pcs_validate(struct phylink_pcs *pcs,
>> +			       unsigned long *supported,
>> +			       const struct phylink_link_state *state)
>> +{
>> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(xilinx_supported) = { 0 };
>> +
>> +	phylink_set_port_modes(xilinx_supported);
>> +	phylink_set(xilinx_supported, Autoneg);
>> +	phylink_set(xilinx_supported, Pause);
>> +	phylink_set(xilinx_supported, Asym_Pause);
>> +	switch (state->interface) {
>> +	case PHY_INTERFACE_MODE_SGMII:
>> +		/* Half duplex not supported */
>> +		phylink_set(xilinx_supported, 10baseT_Full);
>> +		phylink_set(xilinx_supported, 100baseT_Full);
>> +		phylink_set(xilinx_supported, 1000baseT_Full);
>> +		break;
>> +	case PHY_INTERFACE_MODE_1000BASEX:
>> +		phylink_set(xilinx_supported, 1000baseX_Full);
>> +		break;
>> +	case PHY_INTERFACE_MODE_2500BASEX:
>> +		phylink_set(xilinx_supported, 2500baseX_Full);
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	linkmode_and(supported, supported, xilinx_supported);
>> +	return 0;
> 
> You can not assume that an interface mode implies any particular media.
> For example, you can not assume that just because you have SGMII, that
> the only supported media is BaseT. This has been a fundamental principle
> in phylink's validation since day one.
> 
> Phylink documentation for the pcs_validate() callback states:
> 
>  * Validate the interface mode, and advertising's autoneg bit, removing any
>  * media ethtool link modes that would not be supportable from the supported
>  * mask. Phylink will propagate the changes to the advertising mask. See the
>  * &struct phylink_mac_ops validate() method.
> 
> and if we look at the MAC ops validate (before it was removed):
> 
> - * Clear bits in the @supported and @state->advertising masks that
> - * are not supportable by the MAC.
> - *
> - * Note that the PHY may be able to transform from one connection
> - * technology to another, so, eg, don't clear 1000BaseX just
> - * because the MAC is unable to BaseX mode. This is more about
> - * clearing unsupported speeds and duplex settings. The port modes
> - * should not be cleared; phylink_set_port_modes() will help with this.
> 
> PHYs can and do take SGMII and provide both BaseT and BaseX or BaseR
> connections. A PCS that is not directly media facing can not dictate
> the link modes.
> 

OK, how about this:

static int xilinx_pcs_validate(struct phylink_pcs *pcs,
			       unsigned long *supported,
			       const struct phylink_link_state *state)
{
	__ETHTOOL_DECLARE_LINK_MODE_MASK(xilinx_supported) = { 0 };
	unsigned long caps = phy_caps_from_interface(state->interface);

	phylink_set_port_modes(xilinx_supported);
	phylink_set(xilinx_supported, Autoneg);
	phylink_set(xilinx_supported, Pause);
	phylink_set(xilinx_supported, Asym_Pause);
	/* Half duplex not supported */
	caps &= ~(LINK_CAPA_10HD | LINK_CAPA_100HD | LINK_CAPA_1000HD);
	phy_caps_linkmodes(caps, xilinx_supported);
	linkmode_and(supported, supported, xilinx_supported);
	return 0;
}

--Sean

Re: [RFC net-next PATCH 00/13] Add PCS core support

[Kory Maincent]

On Thu,  3 Apr 2025 14:18:54 -0400
Sean Anderson <sean.anderson@linux.dev> wrote:

> This series adds support for creating PCSs as devices on a bus with a
> driver (patch 3). As initial users,
> 
> - The Lynx PCS (and all of its users) is converted to this system (patch 5)
> - The Xilinx PCS is broken out from the AXI Ethernet driver (patches 6-8)
> - The Cadence MACB driver is converted to support external PCSs (namely
>   the Xilinx PCS) (patches 9-10).
> 
> The last few patches add device links for pcs-handle to improve boot times,
> and add compatibles for all Lynx PCSs.
> 
> Care has been taken to ensure backwards-compatibility. The main source
> of this is that many PCS devices lack compatibles and get detected as
> PHYs. To address this, pcs_get_by_fwnode_compat allows drivers to edit
> the devicetree to add appropriate compatibles.

I don't dive into your patch series and I don't know if you have heard about it
but Christian Marangi is currently working on fwnode for PCS:
https://lore.kernel.org/netdev/20250406221423.9723-1-ansuelsmth@gmail.com

Maybe you should sync with him!

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [RFC net-next PATCH 00/13] Add PCS core support

[Sean Anderson]

On 4/7/25 12:27, Kory Maincent wrote:
> On Thu,  3 Apr 2025 14:18:54 -0400
> Sean Anderson <sean.anderson@linux.dev> wrote:
> 
>> This series adds support for creating PCSs as devices on a bus with a
>> driver (patch 3). As initial users,
>> 
>> - The Lynx PCS (and all of its users) is converted to this system (patch 5)
>> - The Xilinx PCS is broken out from the AXI Ethernet driver (patches 6-8)
>> - The Cadence MACB driver is converted to support external PCSs (namely
>>   the Xilinx PCS) (patches 9-10).
>> 
>> The last few patches add device links for pcs-handle to improve boot times,
>> and add compatibles for all Lynx PCSs.
>> 
>> Care has been taken to ensure backwards-compatibility. The main source
>> of this is that many PCS devices lack compatibles and get detected as
>> PHYs. To address this, pcs_get_by_fwnode_compat allows drivers to edit
>> the devicetree to add appropriate compatibles.
> 
> I don't dive into your patch series and I don't know if you have heard about it
> but Christian Marangi is currently working on fwnode for PCS:
> https://lore.kernel.org/netdev/20250406221423.9723-1-ansuelsmth@gmail.com
> 
> Maybe you should sync with him!

I saw that series and made some comments. He is CC'd on this one.

I think this approach has two advantages:

- It completely solves the problem of the PCS being unregistered while the netdev
  (or whatever) is up
- I have designed the interface to make it easy to convert existing
  drivers that may not be able to use the "standard" probing process
  (because they have to support other devicetree structures for
  backwards-compatibility).

--Sean

Re: [RFC net-next PATCH 00/13] Add PCS core support

[Christian Marangi (Ansuel)]

Il giorno lun 7 apr 2025 alle ore 18:33 Sean Anderson
<sean.anderson@linux.dev> ha scritto:
>
> On 4/7/25 12:27, Kory Maincent wrote:
> > On Thu,  3 Apr 2025 14:18:54 -0400
> > Sean Anderson <sean.anderson@linux.dev> wrote:
> >
> >> This series adds support for creating PCSs as devices on a bus with a
> >> driver (patch 3). As initial users,
> >>
> >> - The Lynx PCS (and all of its users) is converted to this system (patch 5)
> >> - The Xilinx PCS is broken out from the AXI Ethernet driver (patches 6-8)
> >> - The Cadence MACB driver is converted to support external PCSs (namely
> >>   the Xilinx PCS) (patches 9-10).
> >>
> >> The last few patches add device links for pcs-handle to improve boot times,
> >> and add compatibles for all Lynx PCSs.
> >>
> >> Care has been taken to ensure backwards-compatibility. The main source
> >> of this is that many PCS devices lack compatibles and get detected as
> >> PHYs. To address this, pcs_get_by_fwnode_compat allows drivers to edit
> >> the devicetree to add appropriate compatibles.
> >
> > I don't dive into your patch series and I don't know if you have heard about it
> > but Christian Marangi is currently working on fwnode for PCS:
> > https://lore.kernel.org/netdev/20250406221423.9723-1-ansuelsmth@gmail.com
> >
> > Maybe you should sync with him!
>
> I saw that series and made some comments. He is CC'd on this one.
>
> I think this approach has two advantages:
>
> - It completely solves the problem of the PCS being unregistered while the netdev
>   (or whatever) is up
> - I have designed the interface to make it easy to convert existing
>   drivers that may not be able to use the "standard" probing process
>   (because they have to support other devicetree structures for
>   backwards-compatibility).
>

I notice this and it's my fault for taking too long to post v2 of the PCS patch.
There was also this idea of entering the wrapper hell but I scrapped that early
as I really feel it's a workaround to the current problem present for
PCS handling.

And the real problem IMHO is that currently PCS handling is fragile and with too
many assumptions. With Daniel we also discussed backwards-compatibility.
(mainly needed for mt7621 and mt7986 (for mediatek side those are the 2
that slipped in before it was correctly complained that things were
taking a bad path)

We feel v2 permits correct support of old implementations.
The ""legacy"" implementation pose the assumption that PCS is never removed
(unless the MAC driver is removed)
That fits v2 where a MAC has to initially provide a list of PCS to
phylink instance.
With this implementation, a MAC can manually parse whatever PCS node structure
is in place and fill the PCS.

As really the "late" removal/addition of a PCS can only be supported with fwnode
implementation as dedicated PCS driver will make use of that.

I honestly hope we can skip having to enter the wrapper hell.
Anyway I also see you made REALLY GOOD documentation. Would be ideal to
collaborate for that. Anyway it's up to net maintainers on what path to follow.

Just my 2 cent on the PCS topic.

Re: [RFC net-next PATCH 00/13] Add PCS core support

[Kory Maincent]

On Mon, 7 Apr 2025 12:33:28 -0400
Sean Anderson <sean.anderson@linux.dev> wrote:

> On 4/7/25 12:27, Kory Maincent wrote:
> > On Thu,  3 Apr 2025 14:18:54 -0400
> > Sean Anderson <sean.anderson@linux.dev> wrote:
> >   
> >> This series adds support for creating PCSs as devices on a bus with a
> >> driver (patch 3). As initial users,
> >> 
> >> - The Lynx PCS (and all of its users) is converted to this system (patch 5)
> >> - The Xilinx PCS is broken out from the AXI Ethernet driver (patches 6-8)
> >> - The Cadence MACB driver is converted to support external PCSs (namely
> >>   the Xilinx PCS) (patches 9-10).
> >> 
> >> The last few patches add device links for pcs-handle to improve boot times,
> >> and add compatibles for all Lynx PCSs.
> >> 
> >> Care has been taken to ensure backwards-compatibility. The main source
> >> of this is that many PCS devices lack compatibles and get detected as
> >> PHYs. To address this, pcs_get_by_fwnode_compat allows drivers to edit
> >> the devicetree to add appropriate compatibles.  
> > 
> > I don't dive into your patch series and I don't know if you have heard
> > about it but Christian Marangi is currently working on fwnode for PCS:
> > https://lore.kernel.org/netdev/20250406221423.9723-1-ansuelsmth@gmail.com
> > 
> > Maybe you should sync with him!  
> 
> I saw that series and made some comments. He is CC'd on this one.

Oh indeed, you have replied on his v1, sorry I missed it.
It seems he forgot to add you in CC in the v2.

> I think this approach has two advantages:
> 
> - It completely solves the problem of the PCS being unregistered while the
> netdev (or whatever) is up
> - I have designed the interface to make it easy to convert existing
>   drivers that may not be able to use the "standard" probing process
>   (because they have to support other devicetree structures for
>   backwards-compatibility).

Ok, thanks for the clarification!
I was working on the axienet driver to add support for the 10G version that's
why I discovered your series.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [RFC net-next PATCH 00/13] Add PCS core support

[Sean Anderson]

On 4/7/25 12:46, Christian Marangi (Ansuel) wrote:
> Il giorno lun 7 apr 2025 alle ore 18:33 Sean Anderson
> <sean.anderson@linux.dev> ha scritto:
>>
>> On 4/7/25 12:27, Kory Maincent wrote:
>> > On Thu,  3 Apr 2025 14:18:54 -0400
>> > Sean Anderson <sean.anderson@linux.dev> wrote:
>> >
>> >> This series adds support for creating PCSs as devices on a bus with a
>> >> driver (patch 3). As initial users,
>> >>
>> >> - The Lynx PCS (and all of its users) is converted to this system (patch 5)
>> >> - The Xilinx PCS is broken out from the AXI Ethernet driver (patches 6-8)
>> >> - The Cadence MACB driver is converted to support external PCSs (namely
>> >>   the Xilinx PCS) (patches 9-10).
>> >>
>> >> The last few patches add device links for pcs-handle to improve boot times,
>> >> and add compatibles for all Lynx PCSs.
>> >>
>> >> Care has been taken to ensure backwards-compatibility. The main source
>> >> of this is that many PCS devices lack compatibles and get detected as
>> >> PHYs. To address this, pcs_get_by_fwnode_compat allows drivers to edit
>> >> the devicetree to add appropriate compatibles.
>> >
>> > I don't dive into your patch series and I don't know if you have heard about it
>> > but Christian Marangi is currently working on fwnode for PCS:
>> > https://lore.kernel.org/netdev/20250406221423.9723-1-ansuelsmth@gmail.com
>> >
>> > Maybe you should sync with him!
>>
>> I saw that series and made some comments. He is CC'd on this one.
>>
>> I think this approach has two advantages:
>>
>> - It completely solves the problem of the PCS being unregistered while the netdev
>>   (or whatever) is up
>> - I have designed the interface to make it easy to convert existing
>>   drivers that may not be able to use the "standard" probing process
>>   (because they have to support other devicetree structures for
>>   backwards-compatibility).
>>
> 
> I notice this and it's my fault for taking too long to post v2 of the PCS patch.
> There was also this idea of entering the wrapper hell but I scrapped that early
> as I really feel it's a workaround to the current problem present for
> PCS handling.

It's no workaround. The fundamental problem is that drivers can become
unbound at any time, and we cannot make consumers drop their references.
Every subsystem must deal with this reality, or suffer from
user-after-free bugs. See [1-3] for discussion of this problem in
relation to PCSs and PHYs, and [4] for more discussion of my approach.

[1] https://lore.kernel.org/netdev/YV7Kp2k8VvN7J0fY@shell.armlinux.org.uk/
[2] https://lore.kernel.org/netdev/20220816163701.1578850-1-sean.anderson@seco.com/
[3] https://lore.kernel.org/netdev/9747f8ef-66b3-0870-cbc0-c1783896b30d@seco.com/
[3] https://lpc.events/event/17/contributions/1627/

> And the real problem IMHO is that currently PCS handling is fragile and with too
> many assumptions. With Daniel we also discussed backwards-compatibility.
> (mainly needed for mt7621 and mt7986 (for mediatek side those are the 2
> that slipped in before it was correctly complained that things were
> taking a bad path)
> 
> We feel v2 permits correct support of old implementations.
> The ""legacy"" implementation pose the assumption that PCS is never removed
> (unless the MAC driver is removed)
> That fits v2 where a MAC has to initially provide a list of PCS to
> phylink instance.

And what happens when the driver is unbound from the device and suddenly
a PCS on that list is free'd memory but is in active use by a netdev?

> With this implementation, a MAC can manually parse whatever PCS node structure
> is in place and fill the PCS.
> 
> As really the "late" removal/addition of a PCS can only be supported with fwnode
> implementation as dedicated PCS driver will make use of that.

I agree that a "cells" approach would require this, but

- There are no in-tree examples of where this is necessary
- I think this would be easy to add when necessary

> I honestly hope we can skip having to enter the wrapper hell.

Unfortunately, this is required by the kernel driver model :l

> Anyway I also see you made REALLY GOOD documentation.

Thanks. One of my peeves is subsystems that have zero docs...

> Would be ideal to
> collaborate for that. Anyway it's up to net maintainers on what path to follow.
> 
> Just my 2 cent on the PCS topic.

--Sean

Re: [RFC net-next PATCH 00/13] Add PCS core support

[Christian Marangi (Ansuel)]

Il giorno lun 7 apr 2025 alle ore 19:00 Sean Anderson
<sean.anderson@linux.dev> ha scritto:
>
> On 4/7/25 12:46, Christian Marangi (Ansuel) wrote:
> > Il giorno lun 7 apr 2025 alle ore 18:33 Sean Anderson
> > <sean.anderson@linux.dev> ha scritto:
> >>
> >> On 4/7/25 12:27, Kory Maincent wrote:
> >> > On Thu,  3 Apr 2025 14:18:54 -0400
> >> > Sean Anderson <sean.anderson@linux.dev> wrote:
> >> >
> >> >> This series adds support for creating PCSs as devices on a bus with a
> >> >> driver (patch 3). As initial users,
> >> >>
> >> >> - The Lynx PCS (and all of its users) is converted to this system (patch 5)
> >> >> - The Xilinx PCS is broken out from the AXI Ethernet driver (patches 6-8)
> >> >> - The Cadence MACB driver is converted to support external PCSs (namely
> >> >>   the Xilinx PCS) (patches 9-10).
> >> >>
> >> >> The last few patches add device links for pcs-handle to improve boot times,
> >> >> and add compatibles for all Lynx PCSs.
> >> >>
> >> >> Care has been taken to ensure backwards-compatibility. The main source
> >> >> of this is that many PCS devices lack compatibles and get detected as
> >> >> PHYs. To address this, pcs_get_by_fwnode_compat allows drivers to edit
> >> >> the devicetree to add appropriate compatibles.
> >> >
> >> > I don't dive into your patch series and I don't know if you have heard about it
> >> > but Christian Marangi is currently working on fwnode for PCS:
> >> > https://lore.kernel.org/netdev/20250406221423.9723-1-ansuelsmth@gmail.com
> >> >
> >> > Maybe you should sync with him!
> >>
> >> I saw that series and made some comments. He is CC'd on this one.
> >>
> >> I think this approach has two advantages:
> >>
> >> - It completely solves the problem of the PCS being unregistered while the netdev
> >>   (or whatever) is up
> >> - I have designed the interface to make it easy to convert existing
> >>   drivers that may not be able to use the "standard" probing process
> >>   (because they have to support other devicetree structures for
> >>   backwards-compatibility).
> >>
> >
> > I notice this and it's my fault for taking too long to post v2 of the PCS patch.
> > There was also this idea of entering the wrapper hell but I scrapped that early
> > as I really feel it's a workaround to the current problem present for
> > PCS handling.
>
> It's no workaround. The fundamental problem is that drivers can become
> unbound at any time, and we cannot make consumers drop their references.
> Every subsystem must deal with this reality, or suffer from
> user-after-free bugs. See [1-3] for discussion of this problem in
> relation to PCSs and PHYs, and [4] for more discussion of my approach.
>
> [1] https://lore.kernel.org/netdev/YV7Kp2k8VvN7J0fY@shell.armlinux.org.uk/
> [2] https://lore.kernel.org/netdev/20220816163701.1578850-1-sean.anderson@seco.com/
> [3] https://lore.kernel.org/netdev/9747f8ef-66b3-0870-cbc0-c1783896b30d@seco.com/
> [3] https://lpc.events/event/17/contributions/1627/
>
> > And the real problem IMHO is that currently PCS handling is fragile and with too
> > many assumptions. With Daniel we also discussed backwards-compatibility.
> > (mainly needed for mt7621 and mt7986 (for mediatek side those are the 2
> > that slipped in before it was correctly complained that things were
> > taking a bad path)
> >
> > We feel v2 permits correct support of old implementations.
> > The ""legacy"" implementation pose the assumption that PCS is never removed
> > (unless the MAC driver is removed)
> > That fits v2 where a MAC has to initially provide a list of PCS to
> > phylink instance.
>
> And what happens when the driver is unbound from the device and suddenly
> a PCS on that list is free'd memory but is in active use by a netdev?
>

driver bug for not correctly implementing the removal task.

The approach is remove as provider and call phylink removal phase
under rtnl lock.
We tested this with unbind, that is actually the main problem we are
trying to address
and works correctly.

> > With this implementation, a MAC can manually parse whatever PCS node structure
> > is in place and fill the PCS.
> >
> > As really the "late" removal/addition of a PCS can only be supported with fwnode
> > implementation as dedicated PCS driver will make use of that.
>
> I agree that a "cells" approach would require this, but
>
> - There are no in-tree examples of where this is necessary
> - I think this would be easy to add when necessary
>

There are no in-tree cause only now we are starting to support
complex configuration with multiple PCS placed outside the MAC.

I feel it's better to define a standard API for them now before
we permit even more MAC driver to implement custom property
and have to address tons of workaround for compatibility.

> > I honestly hope we can skip having to enter the wrapper hell.
>
> Unfortunately, this is required by the kernel driver model :l
>
> > Anyway I also see you made REALLY GOOD documentation.
>
> Thanks. One of my peeves is subsystems that have zero docs...
>
> > Would be ideal to
> > collaborate for that. Anyway it's up to net maintainers on what path to follow.
> >
> > Just my 2 cent on the PCS topic.
>
> --Sean

Re: [RFC net-next PATCH 00/13] Add PCS core support

[Daniel Golle]

On Mon, Apr 07, 2025 at 07:21:38PM +0200, Christian Marangi (Ansuel) wrote:
> Il giorno lun 7 apr 2025 alle ore 19:00 Sean Anderson
> > I agree that a "cells" approach would require this, but
> >
> > - There are no in-tree examples of where this is necessary
> > - I think this would be easy to add when necessary
> >
> 
> There are no in-tree cause only now we are starting to support
> complex configuration with multiple PCS placed outside the MAC.
> 
> I feel it's better to define a standard API for them now before
> we permit even more MAC driver to implement custom property
> and have to address tons of workaround for compatibility.

Qualcomm's PCS driver will require offering multiple phylink_pcs by a
single device/of_node. So while it's true that there is currently no
in-tree user for that, that very user is already knocking on our doors.

See
https://patchwork.kernel.org/project/netdevbpf/list/?series=931658&state=*

Re: [RFC net-next PATCH 00/13] Add PCS core support

[Sean Anderson]

On 4/7/25 13:25, Daniel Golle wrote:
> On Mon, Apr 07, 2025 at 07:21:38PM +0200, Christian Marangi (Ansuel) wrote:
>> Il giorno lun 7 apr 2025 alle ore 19:00 Sean Anderson
>> > I agree that a "cells" approach would require this, but
>> >
>> > - There are no in-tree examples of where this is necessary
>> > - I think this would be easy to add when necessary
>> >
>> 
>> There are no in-tree cause only now we are starting to support
>> complex configuration with multiple PCS placed outside the MAC.
>> 
>> I feel it's better to define a standard API for them now before
>> we permit even more MAC driver to implement custom property
>> and have to address tons of workaround for compatibility.
> 
> Qualcomm's PCS driver will require offering multiple phylink_pcs by a
> single device/of_node. So while it's true that there is currently no
> in-tree user for that, that very user is already knocking on our doors.
> 
> See
> https://patchwork.kernel.org/project/netdevbpf/list/?series=931658&state=*

OK, but I still think this is quite easy to add.

--Sean

Re: [RFC net-next PATCH 00/13] Add PCS core support

[Sean Anderson]

On 4/7/25 13:21, Christian Marangi (Ansuel) wrote:
> Il giorno lun 7 apr 2025 alle ore 19:00 Sean Anderson
> <sean.anderson@linux.dev> ha scritto:
>>
>> On 4/7/25 12:46, Christian Marangi (Ansuel) wrote:
>> > Il giorno lun 7 apr 2025 alle ore 18:33 Sean Anderson
>> > <sean.anderson@linux.dev> ha scritto:
>> >>
>> >> On 4/7/25 12:27, Kory Maincent wrote:
>> >> > On Thu,  3 Apr 2025 14:18:54 -0400
>> >> > Sean Anderson <sean.anderson@linux.dev> wrote:
>> >> >
>> >> >> This series adds support for creating PCSs as devices on a bus with a
>> >> >> driver (patch 3). As initial users,
>> >> >>
>> >> >> - The Lynx PCS (and all of its users) is converted to this system (patch 5)
>> >> >> - The Xilinx PCS is broken out from the AXI Ethernet driver (patches 6-8)
>> >> >> - The Cadence MACB driver is converted to support external PCSs (namely
>> >> >>   the Xilinx PCS) (patches 9-10).
>> >> >>
>> >> >> The last few patches add device links for pcs-handle to improve boot times,
>> >> >> and add compatibles for all Lynx PCSs.
>> >> >>
>> >> >> Care has been taken to ensure backwards-compatibility. The main source
>> >> >> of this is that many PCS devices lack compatibles and get detected as
>> >> >> PHYs. To address this, pcs_get_by_fwnode_compat allows drivers to edit
>> >> >> the devicetree to add appropriate compatibles.
>> >> >
>> >> > I don't dive into your patch series and I don't know if you have heard about it
>> >> > but Christian Marangi is currently working on fwnode for PCS:
>> >> > https://lore.kernel.org/netdev/20250406221423.9723-1-ansuelsmth@gmail.com
>> >> >
>> >> > Maybe you should sync with him!
>> >>
>> >> I saw that series and made some comments. He is CC'd on this one.
>> >>
>> >> I think this approach has two advantages:
>> >>
>> >> - It completely solves the problem of the PCS being unregistered while the netdev
>> >>   (or whatever) is up
>> >> - I have designed the interface to make it easy to convert existing
>> >>   drivers that may not be able to use the "standard" probing process
>> >>   (because they have to support other devicetree structures for
>> >>   backwards-compatibility).
>> >>
>> >
>> > I notice this and it's my fault for taking too long to post v2 of the PCS patch.
>> > There was also this idea of entering the wrapper hell but I scrapped that early
>> > as I really feel it's a workaround to the current problem present for
>> > PCS handling.
>>
>> It's no workaround. The fundamental problem is that drivers can become
>> unbound at any time, and we cannot make consumers drop their references.
>> Every subsystem must deal with this reality, or suffer from
>> user-after-free bugs. See [1-3] for discussion of this problem in
>> relation to PCSs and PHYs, and [4] for more discussion of my approach.
>>
>> [1] https://lore.kernel.org/netdev/YV7Kp2k8VvN7J0fY@shell.armlinux.org.uk/
>> [2] https://lore.kernel.org/netdev/20220816163701.1578850-1-sean.anderson@seco.com/
>> [3] https://lore.kernel.org/netdev/9747f8ef-66b3-0870-cbc0-c1783896b30d@seco.com/
>> [3] https://lpc.events/event/17/contributions/1627/
>>
>> > And the real problem IMHO is that currently PCS handling is fragile and with too
>> > many assumptions. With Daniel we also discussed backwards-compatibility.
>> > (mainly needed for mt7621 and mt7986 (for mediatek side those are the 2
>> > that slipped in before it was correctly complained that things were
>> > taking a bad path)
>> >
>> > We feel v2 permits correct support of old implementations.
>> > The ""legacy"" implementation pose the assumption that PCS is never removed
>> > (unless the MAC driver is removed)
>> > That fits v2 where a MAC has to initially provide a list of PCS to
>> > phylink instance.
>>
>> And what happens when the driver is unbound from the device and suddenly
>> a PCS on that list is free'd memory but is in active use by a netdev?
>>
> 
> driver bug for not correctly implementing the removal task.
> 
> The approach is remove as provider and call phylink removal phase
> under rtnl lock.
> We tested this with unbind, that is actually the main problem we are
> trying to address
> and works correctly.

OK, so this is a different approach since your last submission. Please
CC me on your series.

- Fundamentally this is going to make backwards compatibility very
  difficult, since your approach cannot work with mac_select_pcs. How
  are you going to handle the case of MAC-internal PCSs? Are you going
  to make them create a swnode and bind to it just to create a PCS for
  e.g. MMIO registers? And how is the MAC supposed to know how to select
  the PCS? From what I can tell you don't even notify the MAC about
  which PCS it's using.

  I considered an approach like this, where the phylink would be in the
  driver's seat (so to speak), but I decided not to persue it due to
  the problems listed above. A lot of PCSs are tightly-integrated with
  their MACs, so it does not make sense to introduce this little
  coupling. I think it is better to let the MAC select the PCS e.g.
  based on the phy interface. This tends to be a few lines of code for
  the MAC and saves so much complexity in phylink.

  I think you should try doing the macb and lynx conversions for your
  approach. It will make the above problems obvious.

- Your approach is very intrusive. There are lots of changes all over
  phylink across several patches and it is hard to verify all the
  assumptions. Whereas a wrapper keeps everything contained to one file,
  and most of the functions can be evaluated independently.

--Sean

Re: [RFC net-next PATCH 00/13] Add PCS core support

[Sean Anderson]

On 4/7/25 13:25, Daniel Golle wrote:
> On Mon, Apr 07, 2025 at 07:21:38PM +0200, Christian Marangi (Ansuel) wrote:
>> Il giorno lun 7 apr 2025 alle ore 19:00 Sean Anderson
>> > I agree that a "cells" approach would require this, but
>> >
>> > - There are no in-tree examples of where this is necessary
>> > - I think this would be easy to add when necessary
>> >
>> 
>> There are no in-tree cause only now we are starting to support
>> complex configuration with multiple PCS placed outside the MAC.
>> 
>> I feel it's better to define a standard API for them now before
>> we permit even more MAC driver to implement custom property
>> and have to address tons of workaround for compatibility.
> 
> Qualcomm's PCS driver will require offering multiple phylink_pcs by a
> single device/of_node. So while it's true that there is currently no
> in-tree user for that, that very user is already knocking on our doors.
> 
> See
> https://patchwork.kernel.org/project/netdevbpf/list/?series=931658&state=*

OK, but you have separate nodes for each PCS? So maybe the best thing is to
allow customizing the fwnode? E.g. something like

pcs_register_fwnode(struct device *dev, struct phylink_pcs *pcs, struct fwnode_handle *fwnode)

--Sean