* [RFC net-next PATCH 00/13] Add PCS core support
@ 2025-04-03 18:18 Sean Anderson
2025-04-03 18:18 ` [RFC net-next PATCH 03/13] net: pcs: Add subsystem Sean Anderson
2025-04-07 16:27 ` [RFC net-next PATCH 00/13] Add PCS core support Kory Maincent
0 siblings, 2 replies; 12+ messages in thread
From: Sean Anderson @ 2025-04-03 18:18 UTC (permalink / raw)
To: netdev, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Russell King
Cc: linux-kernel, Christian Marangi, upstream, Heiner Kallweit,
Sean Anderson, Alexandre Belloni, Alexandre Torgue,
Christophe Leroy, Clark Wang, Claudiu Beznea, Claudiu Manoil,
Conor Dooley, Ioana Ciornei, Jonathan Corbet, Joyce Ooi,
Krzysztof Kozlowski, Krzysztof Kozlowski, Li Yang, Madalin Bucur,
Madhavan Srinivasan, Maxime Coquelin, Michael Ellerman,
Michal Simek, Naveen N Rao, Nicholas Piggin, Nicolas Ferre,
Radhey Shyam Pandey, Rob Herring, Rob Herring, Robert Hancock,
Saravana Kannan, Shawn Guo, UNGLinuxDriver, Vladimir Oltean,
Wei Fang, devicetree, imx, linux-arm-kernel, linux-doc,
linux-stm32, linuxppc-dev
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC net-next PATCH 03/13] net: pcs: Add subsystem
2025-04-03 18:18 [RFC net-next PATCH 00/13] Add PCS core support Sean Anderson
@ 2025-04-03 18:18 ` Sean Anderson
2025-04-07 16:27 ` [RFC net-next PATCH 00/13] Add PCS core support Kory Maincent
1 sibling, 0 replies; 12+ messages in thread
From: Sean Anderson @ 2025-04-03 18:18 UTC (permalink / raw)
To: netdev, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Russell King
Cc: linux-kernel, Christian Marangi, upstream, Heiner Kallweit,
Sean Anderson, Jonathan Corbet, linux-doc
This adds support for getting PCS devices from the device tree. PCS
drivers must first register with phylink_register_pcs. After that, MAC
drivers may look up their PCS using phylink_get_pcs.
We wrap registered PCSs in another PCS. This wrapper PCS is refcounted
and can outlive the wrapped PCS (such as if the wrapped PCS's driver is
unbound). The wrapper forwards all PCS callbacks to the wrapped PCS,
first checking to make sure the wrapped PCS still exists. This design
was inspired by Bartosz Golaszewsk's talk at LPC [1].
pcs_get_by_fwnode_compat is a bit hairy, but it's necessary for
compatibility with existing drivers, which often attach to (devicetree)
nodes directly. We use the devicetree changeset system instead of
adding a (secondary) software node because mdio_bus_match calls
of_driver_match_device to match devices, and that function only works on
devicetree nodes.
[1] https://lpc.events/event/17/contributions/1627/
Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---
Documentation/networking/index.rst | 1 +
Documentation/networking/kapi.rst | 4 +
Documentation/networking/pcs.rst | 99 ++++
MAINTAINERS | 2 +
drivers/net/pcs/Kconfig | 12 +
drivers/net/pcs/Makefile | 2 +
drivers/net/pcs/core.c | 701 +++++++++++++++++++++++++++++
include/linux/pcs.h | 122 +++++
8 files changed, 943 insertions(+)
create mode 100644 Documentation/networking/pcs.rst
create mode 100644 drivers/net/pcs/core.c
create mode 100644 include/linux/pcs.h
diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
index 058193ed2eeb..96a2cf71057d 100644
--- a/Documentation/networking/index.rst
+++ b/Documentation/networking/index.rst
@@ -30,6 +30,7 @@ Contents:
page_pool
phy
sfp-phylink
+ pcs
alias
bridge
snmp_counter
diff --git a/Documentation/networking/kapi.rst b/Documentation/networking/kapi.rst
index 98682b9a13ee..7a48178649de 100644
--- a/Documentation/networking/kapi.rst
+++ b/Documentation/networking/kapi.rst
@@ -146,6 +146,10 @@ PHYLINK
.. kernel-doc:: include/linux/phylink.h
:internal:
+ :no-identifiers: phylink_pcs phylink_pcs_ops pcs_validate pcs_inband_caps
+ pcs_enable pcs_disable pcs_pre_config pcs_post_config pcs_get_state
+ pcs_config pcs_an_restart pcs_link_up pcs_disable_eee pcs_enable_eee
+ pcs_pre_init
.. kernel-doc:: drivers/net/phy/phylink.c
diff --git a/Documentation/networking/pcs.rst b/Documentation/networking/pcs.rst
new file mode 100644
index 000000000000..3677527e5e2b
--- /dev/null
+++ b/Documentation/networking/pcs.rst
@@ -0,0 +1,99 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=============
+PCS Subsystem
+=============
+
+The PCS (Physical Coding Sublayer) subsystem handles the registration and lookup
+of PCS devices. These devices contain the upper sublayers of the Ethernet
+physical layer, generally handling framing, scrambling, and encoding tasks. PCS
+devices may also include PMA (Physical Medium Attachment) components. PCS
+devices transfer data between the Link-layer MAC device, and the rest of the
+physical layer, typically via a serdes. The output of the serdes may be
+connected more-or-less directly to the medium when using fiber-optic or
+backplane connections (1000BASE-SX, 1000BASE-KX, etc). It may also communicate
+with a separate PHY (such as over SGMII) which handles the connection to the
+medium (such as 1000BASE-T).
+
+Looking up PCS Devices
+----------------------
+
+There are generally two ways to look up a PCS device. If the PCS device is
+internal to a larger device (such as a MAC or switch), and it does not share an
+implementation with an existing PCS, then it does not need to be registered with
+the PCS subsystem. Instead, you can populate a :c:type:`phylink_pcs`
+in your probe function. Otherwise, you must look up the PCS.
+
+If your device has a :c:type:`fwnode_handle`, you can add a PCS using the
+``pcs-handle`` property::
+
+ ethernet-controller {
+ // ...
+ pcs-handle = <&pcs>;
+ pcs-handle-names = "internal";
+ };
+
+Then, during your probe function, you can get the PCS using :c:func:`pcs_get`::
+
+ mac->pcs = pcs_get(dev, "internal");
+ if (IS_ERR(mac->pcs)) {
+ err = PTR_ERR(mac->pcs);
+ return dev_err_probe(dev, "Could not get PCS\n");
+ }
+
+If your device doesn't have a :c:type:`fwnode_handle`, you can get the PCS
+based on the providing device using :c:func:`pcs_get_by_dev`. Typically, you
+will create the device and bind your PCS driver to it before calling this
+function. This allows reuse of an existing PCS driver.
+
+Once you are done using the PCS, you must call :c:func:`pcs_put`.
+
+Using PCS Devices
+-----------------
+
+To select the PCS from a MAC driver, implement the ``mac_select_pcs`` callback
+of :c:type:`phylink_mac_ops`. In this example, the PCS is selected for SGMII
+and 1000BASE-X, and deselected for other interfaces::
+
+ static struct phylink_pcs *mac_select_pcs(struct phylink_config *config,
+ phy_interface_t iface)
+ {
+ struct mac *mac = config_to_mac(config);
+
+ switch (iface) {
+ case PHY_INTERFACE_MODE_SGMII:
+ case PHY_INTERFACE_MODE_1000BASEX:
+ return mac->pcs;
+ default:
+ return NULL;
+ }
+ }
+
+To do the same from a DSA driver, implement the ``phylink_mac_select_pcs``
+callback of :c:type:`dsa_switch_ops`.
+
+Writing PCS Drivers
+-------------------
+
+To write a PCS driver, first implement :c:type:`phylink_pcs_ops`. Then,
+register your PCS in your probe function using :c:func:`pcs_register`. You must
+call :c:func:`pcs_unregister` from your remove function. You can avoid this step
+by registering with :c:func:`devm_pcs_unregister`.
+
+API Reference
+-------------
+
+.. kernel-doc:: include/linux/phylink.h
+ :identifiers: phylink_pcs phylink_pcs_ops pcs_validate pcs_inband_caps
+ pcs_enable pcs_disable pcs_pre_config pcs_post_config pcs_get_state
+ pcs_config pcs_an_restart pcs_link_up pcs_disable_eee pcs_enable_eee
+ pcs_pre_init
+
+.. kernel-doc:: include/linux/pcs.h
+ :internal:
+
+.. kernel-doc:: drivers/net/pcs/core.c
+ :export:
+
+.. kernel-doc:: drivers/net/pcs/core.c
+ :internal:
diff --git a/MAINTAINERS b/MAINTAINERS
index 1cd25139cc58..9d3b3788a005 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8644,6 +8644,7 @@ F: Documentation/ABI/testing/sysfs-class-net-phydev
F: Documentation/devicetree/bindings/net/ethernet-phy.yaml
F: Documentation/devicetree/bindings/net/mdio*
F: Documentation/devicetree/bindings/net/qca,ar803x.yaml
+F: Documentation/networking/pcs.rst
F: Documentation/networking/phy.rst
F: drivers/net/mdio/
F: drivers/net/mdio/acpi_mdio.c
@@ -8657,6 +8658,7 @@ F: include/linux/linkmode.h
F: include/linux/mdio/*.h
F: include/linux/mii.h
F: include/linux/of_net.h
+F: include/linux/pcs.h
F: include/linux/phy.h
F: include/linux/phy_fixed.h
F: include/linux/phy_link_topology.h
diff --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig
index f6aa437473de..b764770063cc 100644
--- a/drivers/net/pcs/Kconfig
+++ b/drivers/net/pcs/Kconfig
@@ -5,6 +5,18 @@
menu "PCS device drivers"
+config PCS
+ bool "PCS subsystem"
+ help
+ This provides common helper functions for registering and looking up
+ Physical Coding Sublayer (PCS) devices. PCS devices translate between
+ different interface types. In some use cases, they may either
+ translate between different types of Medium-Independent Interfaces
+ (MIIs), such as translating GMII to SGMII. This allows using a fast
+ serial interface to talk to the phy which translates the MII to the
+ Medium-Dependent Interface. Alternatively, they may translate a MII
+ directly to an MDI, such as translating GMII to 1000Base-X.
+
config PCS_XPCS
tristate "Synopsys DesignWare Ethernet XPCS"
select PHYLINK
diff --git a/drivers/net/pcs/Makefile b/drivers/net/pcs/Makefile
index 4f7920618b90..35e3324fc26e 100644
--- a/drivers/net/pcs/Makefile
+++ b/drivers/net/pcs/Makefile
@@ -1,6 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
# Makefile for Linux PCS drivers
+obj-$(CONFIG_PCS) += core.o
+
pcs_xpcs-$(CONFIG_PCS_XPCS) := pcs-xpcs.o pcs-xpcs-plat.o \
pcs-xpcs-nxp.o pcs-xpcs-wx.o
diff --git a/drivers/net/pcs/core.c b/drivers/net/pcs/core.c
new file mode 100644
index 000000000000..de3f9e418cf2
--- /dev/null
+++ b/drivers/net/pcs/core.c
@@ -0,0 +1,701 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022, 2025 Sean Anderson <sean.anderson@seco.com>
+ */
+
+#define pr_fmt(fmt) "pcs-core: " fmt
+
+#include <linux/fwnode.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/pcs.h>
+#include <linux/phylink.h>
+#include <linux/property.h>
+#include <linux/rcupdate.h>
+
+static LIST_HEAD(pcs_wrappers);
+static DEFINE_MUTEX(pcs_mutex);
+
+/**
+ * struct pcs_wrapper - Wrapper for a registered PCS
+ * @pcs: the wrapping PCS
+ * @ssp: SRCU protecting @wrapped
+ * @refcnt: refcount for the wrapper
+ * @list: list head for pcs_wrappers
+ * @dev: the device associated with this PCS
+ * @wrapped: the backing PCS
+ */
+struct pcs_wrapper {
+ struct phylink_pcs pcs;
+ struct srcu_struct ssp;
+ refcount_t refcnt;
+ struct list_head list;
+ struct device *dev;
+ struct phylink_pcs *wrapped;
+};
+
+static const struct phylink_pcs_ops pcs_ops;
+
+static struct pcs_wrapper *pcs_to_wrapper(struct phylink_pcs *pcs)
+{
+ WARN_ON(pcs->ops != &pcs_ops);
+ return container_of(pcs, struct pcs_wrapper, pcs);
+}
+
+static int pcs_validate(struct phylink_pcs *pcs, unsigned long *supported,
+ const struct phylink_link_state *state)
+{
+ struct pcs_wrapper *wrapper = pcs_to_wrapper(pcs);
+ struct phylink_pcs *wrapped;
+ int ret, idx;
+
+ if (!wrapper)
+ return 0;
+
+ idx = srcu_read_lock(&wrapper->ssp);
+
+ wrapped = srcu_dereference(wrapper->wrapped, &wrapper->ssp);
+ if (wrapped) {
+ if (wrapped->ops->pcs_validate)
+ ret = wrapped->ops->pcs_validate(wrapped, supported,
+ state);
+ else
+ ret = 0;
+ } else {
+ ret = -ENODEV;
+ }
+
+ srcu_read_unlock(&wrapper->ssp, idx);
+ return ret;
+}
+
+static unsigned int pcs_inband_caps(struct phylink_pcs *pcs,
+ phy_interface_t interface)
+{
+ struct pcs_wrapper *wrapper = pcs_to_wrapper(pcs);
+ struct phylink_pcs *wrapped;
+ int ret, idx;
+
+ idx = srcu_read_lock(&wrapper->ssp);
+
+ wrapped = srcu_dereference(wrapper->wrapped, &wrapper->ssp);
+ if (wrapped && wrapped->ops->pcs_inband_caps)
+ ret = wrapped->ops->pcs_inband_caps(wrapped, interface);
+ else
+ ret = 0;
+
+ srcu_read_unlock(&wrapper->ssp, idx);
+ return ret;
+}
+
+static int pcs_enable(struct phylink_pcs *pcs)
+{
+ struct pcs_wrapper *wrapper = pcs_to_wrapper(pcs);
+ struct phylink_pcs *wrapped;
+ int ret, idx;
+
+ if (!wrapper)
+ return 0;
+
+ idx = srcu_read_lock(&wrapper->ssp);
+
+ wrapped = srcu_dereference(wrapper->wrapped, &wrapper->ssp);
+ if (wrapped) {
+ if (wrapped->ops->pcs_enable)
+ ret = wrapped->ops->pcs_enable(wrapped);
+ else
+ ret = 0;
+ } else {
+ ret = -ENODEV;
+ }
+
+ srcu_read_unlock(&wrapper->ssp, idx);
+ return ret;
+}
+
+static void pcs_disable(struct phylink_pcs *pcs)
+{
+ struct pcs_wrapper *wrapper = pcs_to_wrapper(pcs);
+ struct phylink_pcs *wrapped;
+ int idx;
+
+ idx = srcu_read_lock(&wrapper->ssp);
+
+ wrapped = srcu_dereference(wrapper->wrapped, &wrapper->ssp);
+ if (wrapped && wrapped->ops->pcs_disable)
+ wrapped->ops->pcs_disable(wrapped);
+
+ srcu_read_unlock(&wrapper->ssp, idx);
+}
+
+static void pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
+ struct phylink_link_state *state)
+{
+ struct pcs_wrapper *wrapper = pcs_to_wrapper(pcs);
+ struct phylink_pcs *wrapped;
+ int idx;
+
+ idx = srcu_read_lock(&wrapper->ssp);
+
+ wrapped = srcu_dereference(wrapper->wrapped, &wrapper->ssp);
+ if (wrapped)
+ wrapped->ops->pcs_get_state(wrapped, neg_mode, state);
+ else
+ state->link = 0;
+
+ srcu_read_unlock(&wrapper->ssp, idx);
+}
+
+static void pcs_pre_config(struct phylink_pcs *pcs,
+ phy_interface_t interface)
+{
+ struct pcs_wrapper *wrapper = pcs_to_wrapper(pcs);
+ struct phylink_pcs *wrapped;
+ int idx;
+
+ idx = srcu_read_lock(&wrapper->ssp);
+
+ wrapped = srcu_dereference(wrapper->wrapped, &wrapper->ssp);
+ if (wrapped && wrapped->ops->pcs_pre_config)
+ wrapped->ops->pcs_pre_config(wrapped, interface);
+
+ srcu_read_unlock(&wrapper->ssp, idx);
+}
+
+static int pcs_post_config(struct phylink_pcs *pcs,
+ phy_interface_t interface)
+{
+ struct pcs_wrapper *wrapper = pcs_to_wrapper(pcs);
+ struct phylink_pcs *wrapped;
+ int ret, idx;
+
+ idx = srcu_read_lock(&wrapper->ssp);
+
+ wrapped = srcu_dereference(wrapper->wrapped, &wrapper->ssp);
+ if (pcs && wrapped->ops->pcs_post_config)
+ ret = wrapped->ops->pcs_post_config(wrapped, interface);
+ else
+ ret = 0;
+
+ srcu_read_unlock(&wrapper->ssp, idx);
+ return ret;
+}
+
+static int pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
+ phy_interface_t interface,
+ const unsigned long *advertising,
+ bool permit_pause_to_mac)
+{
+ struct pcs_wrapper *wrapper = pcs_to_wrapper(pcs);
+ struct phylink_pcs *wrapped;
+ int ret, idx;
+
+ idx = srcu_read_lock(&wrapper->ssp);
+
+ wrapped = srcu_dereference(wrapper->wrapped, &wrapper->ssp);
+ if (wrapped)
+ ret = wrapped->ops->pcs_config(wrapped, neg_mode, interface,
+ advertising, permit_pause_to_mac);
+ else
+ ret = -ENODEV;
+
+ srcu_read_unlock(&wrapper->ssp, idx);
+ return ret;
+}
+
+static void pcs_an_restart(struct phylink_pcs *pcs)
+{
+ struct pcs_wrapper *wrapper = pcs_to_wrapper(pcs);
+ struct phylink_pcs *wrapped;
+ int idx;
+
+ idx = srcu_read_lock(&wrapper->ssp);
+
+ wrapped = srcu_dereference(wrapper->wrapped, &wrapper->ssp);
+ if (wrapped)
+ wrapped->ops->pcs_an_restart(wrapped);
+
+ srcu_read_unlock(&wrapper->ssp, idx);
+}
+
+static void pcs_link_up(struct phylink_pcs *pcs, unsigned int neg_mode,
+ phy_interface_t interface, int speed, int duplex)
+{
+ struct pcs_wrapper *wrapper = pcs_to_wrapper(pcs);
+ struct phylink_pcs *wrapped;
+ int idx;
+
+ idx = srcu_read_lock(&wrapper->ssp);
+
+ wrapped = srcu_dereference(wrapper->wrapped, &wrapper->ssp);
+ if (wrapped && wrapped->ops->pcs_link_up)
+ wrapped->ops->pcs_link_up(wrapped, neg_mode, interface, speed,
+ duplex);
+
+ srcu_read_unlock(&wrapper->ssp, idx);
+}
+
+static void pcs_disable_eee(struct phylink_pcs *pcs)
+{
+ struct pcs_wrapper *wrapper = pcs_to_wrapper(pcs);
+ struct phylink_pcs *wrapped;
+ int idx;
+
+ idx = srcu_read_lock(&wrapper->ssp);
+
+ wrapped = srcu_dereference(wrapper->wrapped, &wrapper->ssp);
+ if (wrapped && wrapped->ops->pcs_disable_eee)
+ wrapped->ops->pcs_disable_eee(wrapped);
+
+ srcu_read_unlock(&wrapper->ssp, idx);
+}
+
+static void pcs_enable_eee(struct phylink_pcs *pcs)
+{
+ struct pcs_wrapper *wrapper = pcs_to_wrapper(pcs);
+ struct phylink_pcs *wrapped;
+ int idx;
+
+ idx = srcu_read_lock(&wrapper->ssp);
+
+ wrapped = srcu_dereference(wrapper->wrapped, &wrapper->ssp);
+ if (wrapped && wrapped->ops->pcs_enable_eee)
+ wrapped->ops->pcs_enable_eee(wrapped);
+
+ srcu_read_unlock(&wrapper->ssp, idx);
+}
+
+static int pcs_pre_init(struct phylink_pcs *pcs)
+{
+ struct pcs_wrapper *wrapper = pcs_to_wrapper(pcs);
+ struct phylink_pcs *wrapped;
+ int ret, idx;
+
+ idx = srcu_read_lock(&wrapper->ssp);
+
+ wrapped = srcu_dereference(wrapper->wrapped, &wrapper->ssp);
+ if (wrapped) {
+ wrapped->rxc_always_on = pcs->rxc_always_on;
+ if (wrapped->ops->pcs_pre_init)
+ ret = wrapped->ops->pcs_pre_init(wrapped);
+ else
+ ret = 0;
+ } else {
+ ret = -ENODEV;
+ }
+
+ srcu_read_unlock(&wrapper->ssp, idx);
+ return ret;
+}
+
+static const struct phylink_pcs_ops pcs_ops = {
+ .pcs_validate = pcs_validate,
+ .pcs_inband_caps = pcs_inband_caps,
+ .pcs_enable = pcs_enable,
+ .pcs_disable = pcs_disable,
+ .pcs_pre_config = pcs_pre_config,
+ .pcs_post_config = pcs_post_config,
+ .pcs_get_state = pcs_get_state,
+ .pcs_config = pcs_config,
+ .pcs_an_restart = pcs_an_restart,
+ .pcs_link_up = pcs_link_up,
+ .pcs_disable_eee = pcs_disable_eee,
+ .pcs_enable_eee = pcs_enable_eee,
+ .pcs_pre_init = pcs_pre_init,
+};
+
+static void pcs_change_callback(void *priv, bool up)
+{
+ struct pcs_wrapper *wrapper = priv;
+
+ phylink_pcs_change(&wrapper->pcs, up);
+}
+
+/**
+ * pcs_register() - register a new PCS
+ * @dev: The device requesting the PCS
+ * @pcs: The PCS to register
+ *
+ * Registers a new PCS which can be attached to a phylink.
+ *
+ * Return: 0 on success, or -errno on error
+ */
+int pcs_register(struct device *dev, struct phylink_pcs *pcs)
+{
+ struct pcs_wrapper *wrapper;
+
+ if (!dev || !pcs->ops)
+ return -EINVAL;
+
+ if (!pcs->ops->pcs_an_restart || !pcs->ops->pcs_config ||
+ !pcs->ops->pcs_get_state)
+ return -EINVAL;
+
+ wrapper = kzalloc(sizeof(*wrapper), GFP_KERNEL);
+ if (!wrapper)
+ return -ENOMEM;
+
+ init_srcu_struct(&wrapper->ssp);
+ refcount_set(&wrapper->refcnt, 1);
+ INIT_LIST_HEAD(&wrapper->list);
+ wrapper->dev = dev;
+ RCU_INIT_POINTER(wrapper->wrapped, pcs);
+
+ wrapper->pcs.ops = &pcs_ops;
+ wrapper->pcs.poll = pcs->poll;
+ bitmap_copy(wrapper->pcs.supported_interfaces, pcs->supported_interfaces,
+ PHY_INTERFACE_MODE_MAX);
+
+ pcs->link_change = pcs_change_callback;
+ pcs->link_change_priv = wrapper;
+
+ mutex_lock(&pcs_mutex);
+ list_add(&wrapper->list, &pcs_wrappers);
+ mutex_unlock(&pcs_mutex);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pcs_register);
+
+static void pcs_destroy(struct pcs_wrapper *wrapper)
+{
+ cleanup_srcu_struct(&wrapper->ssp);
+ kfree(wrapper);
+}
+
+/**
+ * pcs_unregister() - unregister a PCS
+ * @pcs: a PCS previously registered with pcs_register()
+ */
+void pcs_unregister(struct phylink_pcs *pcs)
+{
+ struct pcs_wrapper *wrapper;
+ struct phylink_pcs *wrapped;
+
+ mutex_lock(&pcs_mutex);
+ list_for_each_entry(wrapper, &pcs_wrappers, list) {
+ if (wrapper->wrapped == pcs)
+ goto found;
+ }
+
+ mutex_unlock(&pcs_mutex);
+ WARN(1, "trying to unregister an already-unregistered PCS\n");
+ return;
+
+found:
+ list_del(&wrapper->list);
+ wrapped = rcu_replace_pointer(wrapper->wrapped, NULL, true);
+ mutex_unlock(&pcs_mutex);
+ synchronize_srcu(&wrapper->ssp);
+
+ if (!wrapper->pcs.poll)
+ phylink_pcs_change(&wrapper->pcs, false);
+ if (refcount_dec_and_test(&wrapper->refcnt))
+ pcs_destroy(wrapper);
+}
+EXPORT_SYMBOL_GPL(pcs_unregister);
+
+static void devm_pcs_release(struct device *dev, void *res)
+{
+ pcs_unregister(*(struct phylink_pcs **)res);
+}
+
+/**
+ * devm_pcs_register - resource managed pcs_register()
+ * @dev: device that is registering this PCS
+ * @pcs: the PCS to register
+ *
+ * Managed pcs_register(). For PCSs registered by this function,
+ * pcs_unregister() is automatically called on driver detach. See
+ * pcs_register() for more information.
+ *
+ * Return: 0 on success, or -errno on failure
+ */
+int devm_pcs_register(struct device *dev, struct phylink_pcs *pcs)
+{
+ struct phylink_pcs **pcsp;
+ int ret;
+
+ pcsp = devres_alloc(devm_pcs_release, sizeof(*pcsp),
+ GFP_KERNEL);
+ if (!pcsp)
+ return -ENOMEM;
+
+ ret = pcs_register(dev, pcs);
+ if (ret) {
+ devres_free(pcsp);
+ return ret;
+ }
+
+ *pcsp = pcs;
+ devres_add(dev, pcsp);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(devm_pcs_register);
+
+/**
+ * _pcs_get_tail() - Look up and request a PCS
+ * @dev: The device requesting the PCS
+ * @fwnode: The PCS's fwnode
+ * @pcs_dev: The PCS's device
+ *
+ * Search PCSs registered with pcs_register() for one with a matching
+ * fwnode or device. Either @fwnode or @pcs_dev may be %NULL if matching
+ * against a fwnode or device is not desired (respectively).
+ *
+ * Once a PCS is found, perform common operations necessary when getting a PCS
+ * (increment reference counts, etc).
+ *
+ * You should probably call one of the pcs_get* functions instead of this one.
+ *
+ * Return: A PCS, or an error pointer on failure. If both @fwnode and @pcs_dev are
+ * * %NULL, returns %NULL to allow easier chaining.
+ */
+struct phylink_pcs *_pcs_get_tail(struct device *dev,
+ const struct fwnode_handle *fwnode,
+ const struct device *pcs_dev)
+{
+ struct pcs_wrapper *wrapper;
+
+ if (!fwnode && !pcs_dev)
+ return NULL;
+
+ pr_debug("looking for %pfwf or %s %s...\n", fwnode,
+ pcs_dev ? dev_driver_string(pcs_dev) : "(null)",
+ pcs_dev ? dev_name(pcs_dev) : "(null)");
+
+ /* We need to hold this until we get to device_link_add. Otherwise,
+ * someone could unbind the PCS driver.
+ */
+ mutex_lock(&pcs_mutex);
+ list_for_each_entry(wrapper, &pcs_wrappers, list) {
+ if (pcs_dev && wrapper->dev == pcs_dev)
+ goto found;
+ if (fwnode && wrapper->dev && wrapper->dev->fwnode == fwnode)
+ goto found;
+ }
+ mutex_unlock(&pcs_mutex);
+ pr_debug("...not found\n");
+ return ERR_PTR(-EPROBE_DEFER);
+
+found:
+ pr_debug("...found\n");
+
+ refcount_inc(&wrapper->refcnt);
+ get_device(wrapper->dev);
+
+ mutex_unlock(&pcs_mutex);
+ return &wrapper->pcs;
+}
+EXPORT_SYMBOL_GPL(_pcs_get_tail);
+
+/**
+ * pcs_find_fwnode() - Find a PCS's fwnode
+ * @mac_node: The fwnode referencing the PCS
+ * @id: The name of the PCS to get. May be %NULL to get the first PCS.
+ * @fallback: An optional fallback property to use if pcs-handle is absent
+ * @optional: Whether the PCS is optional
+ *
+ * Find a PCS's fwnode, as referenced by @mac_node. This fwnode can later be
+ * used with _pcs_get_tail() to get the actual PCS. ``pcs-handle-names`` is
+ * used to match @id, then the fwnode is found using ``pcs-handle``.
+ *
+ * This function is internal to the PCS subsystem from a consumer
+ * point-of-view. However, it may be used to implement fallbacks for legacy
+ * behavior in PCS providers.
+ *
+ * Return: %NULL if @optional is set and the PCS cannot be found. Otherwise,
+ * * returns a PCS if found or an error pointer on failure.
+ */
+struct fwnode_handle *pcs_find_fwnode(const struct fwnode_handle *mac_node,
+ const char *id, const char *fallback,
+ bool optional)
+{
+ int index;
+ struct fwnode_handle *pcs_fwnode;
+
+ if (!mac_node)
+ return optional ? NULL : ERR_PTR(-ENODEV);
+
+ if (id)
+ index = fwnode_property_match_string(mac_node,
+ "pcs-handle-names", id);
+ else
+ index = 0;
+
+ if (index < 0) {
+ if (optional && (index == -EINVAL || index == -ENODATA))
+ return NULL;
+ return ERR_PTR(index);
+ }
+
+ /* First try pcs-handle, and if that doesn't work try the fallback */
+ pcs_fwnode = fwnode_find_reference(mac_node, "pcs-handle", index);
+ if (PTR_ERR(pcs_fwnode) == -ENOENT && fallback)
+ pcs_fwnode = fwnode_find_reference(mac_node, fallback, index);
+ if (optional && !id && PTR_ERR(pcs_fwnode) == -ENOENT)
+ return NULL;
+ return pcs_fwnode;
+}
+EXPORT_SYMBOL_GPL(pcs_find_fwnode);
+
+/**
+ * _pcs_get() - Get a PCS from a fwnode property
+ * @dev: The device to get a PCS for
+ * @fwnode: The fwnode to find the PCS with
+ * @id: The name of the PCS to get. May be %NULL to get the first PCS.
+ * @fallback: An optional fallback property to use if pcs-handle is absent
+ * @optional: Whether the PCS is optional
+ *
+ * Find a PCS referenced by @mac_node and return a reference to it. Every call
+ * to _pcs_get_by_fwnode() must be balanced with one to pcs_put().
+ *
+ * Return: a PCS if found, %NULL if not, or an error pointer on failure
+ */
+struct phylink_pcs *_pcs_get(struct device *dev, struct fwnode_handle *fwnode,
+ const char *id, const char *fallback,
+ bool optional)
+{
+ struct fwnode_handle *pcs_fwnode;
+ struct phylink_pcs *pcs;
+
+ pcs_fwnode = pcs_find_fwnode(fwnode, id, fallback, optional);
+ if (IS_ERR(pcs_fwnode))
+ return ERR_CAST(pcs_fwnode);
+
+ pcs = _pcs_get_tail(dev, pcs_fwnode, NULL);
+ fwnode_handle_put(pcs_fwnode);
+ return pcs;
+}
+EXPORT_SYMBOL_GPL(_pcs_get);
+
+static __maybe_unused void of_changeset_cleanup(void *data)
+{
+ struct of_changeset *ocs = data;
+
+ if (WARN(of_changeset_revert(ocs),
+ "could not revert changeset; leaking memory\n"))
+ return;
+
+ of_changeset_destroy(ocs);
+ kfree(ocs);
+}
+
+/**
+ * pcs_get_by_fwnode_compat() - Get a PCS with a compatibility fallback
+ * @dev: The device requesting the PCS
+ * @fwnode: The &struct fwnode_handle of the PCS itself
+ * @fixup: Callback to fix up @fwnode for compatibility
+ * @data: Passed to @fixup
+ *
+ * This function looks up a PCS and retries on failure after fixing up @fwnode.
+ * It is intended to assist in backwards-compatible behavior for drivers that
+ * used to create a PCS directly from a &struct device_node. This function
+ * should NOT be used in new drivers.
+ *
+ * @fixup modifies a devicetree changeset to create any properties necessary to
+ * bind the PCS's &struct device_node. At the very least, it should use
+ * of_changeset_add_prop_string() to add a compatible property.
+ *
+ * Note that unlike pcs_get_by_fwnode, @fwnode is the &struct fwnode_handle of
+ * the PCS itself, and not that of the requesting device. @fwnode could be
+ * looked up with pcs_find_fwnode() or determined by some other means for
+ * compatibility.
+ *
+ * Return: A PCS on success or an error pointer on failure
+ */
+struct phylink_pcs *
+pcs_get_by_fwnode_compat(struct device *dev, struct fwnode_handle *fwnode,
+ int (*fixup)(struct of_changeset *ocs,
+ struct device_node *np, void *data),
+ void *data)
+{
+#ifdef CONFIG_OF_DYNAMIC
+ struct mdio_device *mdiodev;
+ struct of_changeset *ocs;
+ struct phylink_pcs *pcs;
+ struct device_node *np;
+ struct device *pcsdev;
+ int err;
+
+ /* First attempt */
+ pcs = _pcs_get_tail(dev, fwnode, NULL);
+ if (PTR_ERR(pcs) != -EPROBE_DEFER)
+ return pcs;
+
+ /* No luck? Maybe there's no compatible... */
+ np = to_of_node(fwnode);
+ if (!np || of_property_present(np, "compatible"))
+ return pcs;
+
+ /* OK, let's try fixing things up */
+ pr_warn("%pOF is missing a compatible\n", np);
+ ocs = kmalloc(sizeof(*ocs), GFP_KERNEL);
+ if (!ocs)
+ return ERR_PTR(-ENOMEM);
+
+ of_changeset_init(ocs);
+ err = fixup(ocs, np, data);
+ if (err)
+ goto err_ocs;
+
+ err = of_changeset_apply(ocs);
+ if (err)
+ goto err_ocs;
+
+ err = devm_add_action_or_reset(dev, of_changeset_cleanup, ocs);
+ if (err)
+ return ERR_PTR(err);
+
+ mdiodev = fwnode_mdio_find_device(fwnode);
+ if (mdiodev) {
+ /* Clear that pesky PHY flag so we can match PCS drivers */
+ device_lock(&mdiodev->dev);
+ mdiodev->flags &= ~MDIO_DEVICE_FLAG_PHY;
+ device_unlock(&mdiodev->dev);
+ pcsdev = &mdiodev->dev;
+ } else {
+ pcsdev = get_device(fwnode->dev);
+ if (!pcsdev)
+ return ERR_PTR(-EPROBE_DEFER);
+ }
+
+ err = device_reprobe(pcsdev);
+ put_device(pcsdev);
+ if (err)
+ return ERR_PTR(err);
+
+ return _pcs_get_tail(dev, fwnode, NULL);
+
+err_ocs:
+ of_changeset_destroy(ocs);
+ kfree(ocs);
+ return ERR_PTR(err);
+#else
+ return _pcs_get_tail(dev, fwnode, NULL);
+#endif
+}
+EXPORT_SYMBOL_GPL(pcs_get_by_fwnode_compat);
+
+/**
+ * pcs_put() - Release a previously-acquired PCS
+ * @dev: The device used to acquire the PCS
+ * @pcs: The PCS to put
+ *
+ * This frees resources associated with the PCS which were acquired when it was
+ * gotten.
+ */
+void pcs_put(struct device *dev, struct phylink_pcs *pcs)
+{
+ struct pcs_wrapper *wrapper;
+
+ if (!pcs)
+ return;
+
+ wrapper = pcs_to_wrapper(pcs);
+ put_device(wrapper->dev);
+ if (refcount_dec_and_test(&wrapper->refcnt))
+ pcs_destroy(wrapper);
+}
+EXPORT_SYMBOL_GPL(pcs_put);
diff --git a/include/linux/pcs.h b/include/linux/pcs.h
new file mode 100644
index 000000000000..53fbff3af9e8
--- /dev/null
+++ b/include/linux/pcs.h
@@ -0,0 +1,122 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 Sean Anderson <sean.anderson@seco.com>
+ */
+
+#ifndef _PCS_H
+#define _PCS_H
+
+struct device_node;
+struct of_changeset;
+struct phylink_pcs;
+
+int pcs_register(struct device *dev, struct phylink_pcs *pcs);
+void pcs_unregister(struct phylink_pcs *pcs);
+int devm_pcs_register(struct device *dev, struct phylink_pcs *pcs);
+struct phylink_pcs *_pcs_get_tail(struct device *dev,
+ const struct fwnode_handle *fwnode,
+ const struct device *pcs_dev);
+struct phylink_pcs *_pcs_get(struct device *dev, struct fwnode_handle *fwnode,
+ const char *id, const char *fallback,
+ bool optional);
+void pcs_put(struct device *dev, struct phylink_pcs *handle);
+
+/**
+ * pcs_get() - Get a PCS based on a fwnode
+ * @dev: The device requesting the PCS
+ * @id: The name of the PCS
+ *
+ * Find and get a PCS, as referenced by @dev's &struct fwnode_handle. See
+ * pcs_find_fwnode() for details. Each call to this function must be balanced
+ * with one to pcs_put().
+ *
+ * Return: A PCS on success or an error pointer on failure
+ */
+static inline struct phylink_pcs *pcs_get(struct device *dev, const char *id)
+{
+ return _pcs_get(dev, dev_fwnode(dev), id, NULL, false);
+}
+
+/**
+ * pcs_get_optional() - Optionally get a PCS based on a fwnode
+ * @dev: The device requesting the PCS
+ * @id: The name of the PCS
+ *
+ * Optionally find and get a PCS, as referenced by @dev's &struct
+ * fwnode_handle. See pcs_find_fwnode() for details. Each call to this function
+ * must be balanced with one to pcs_put().
+ *
+ * Return: A PCS on success, %NULL if none was found, or an error pointer on
+ * * failure
+ */
+static inline struct phylink_pcs *pcs_get_optional(struct device *dev,
+ const char *id)
+{
+ return _pcs_get(dev, dev_fwnode(dev), id, NULL, true);
+}
+
+/**
+ * pcs_get_by_fwnode() - Get a PCS based on a fwnode
+ * @dev: The device requesting the PCS
+ * @fwnode: The &struct fwnode_handle referencing the PCS
+ * @id: The name of the PCS
+ *
+ * Find and get a PCS, as referenced by @fwnode. See pcs_find_fwnode() for
+ * details. Each call to this function must be balanced with one to pcs_put().
+ *
+ * Return: A PCS on success or an error pointer on failure
+ */
+static inline struct phylink_pcs
+*pcs_get_by_fwnode(struct device *dev, struct fwnode_handle *fwnode,
+ const char *id)
+{
+ return _pcs_get(dev, fwnode, id, NULL, false);
+}
+
+/**
+ * pcs_get_by_fwnode_optional() - Optionally get a PCS based on a fwnode
+ * @dev: The device requesting the PCS
+ * @fwnode: The &struct fwnode_handle referencing the PCS
+ * @id: The name of the PCS
+ *
+ * Optionally find and get a PCS, as referenced by @fwnode. See
+ * pcs_find_fwnode() for details. Each call to this function must be balanced
+ * with one to pcs_put().
+ *
+ * Return: A PCS on success, %NULL if none was found, or an error pointer on
+ * * failure
+ */
+static inline struct phylink_pcs
+*pcs_get_by_fwnode_optional(struct device *dev, struct fwnode_handle *fwnode,
+ const char *id)
+{
+ return _pcs_get(dev, fwnode, id, NULL, true);
+}
+
+/**
+ * pcs_get_by_dev() - Get a PCS from its providing device
+ * @dev: The device requesting the PCS
+ * @pcs_dev: The device providing the PCS
+ *
+ * Get the first PCS registered by @pcs_dev. Each call to this function must be
+ * balanced with one to pcs_put().
+ *
+ * Return: A PCS on success or an error pointer on failure
+ */
+static inline struct phylink_pcs *pcs_get_by_dev(struct device *dev,
+ const struct device *pcs_dev)
+{
+ return _pcs_get_tail(dev, NULL, pcs_dev);
+}
+
+struct fwnode_handle *pcs_find_fwnode(const struct fwnode_handle *mac_node,
+ const char *id, const char *fallback,
+ bool optional);
+
+struct phylink_pcs *
+pcs_get_by_fwnode_compat(struct device *dev, struct fwnode_handle *fwnode,
+ int (*fixup)(struct of_changeset *ocs, struct device_node *np,
+ void *data),
+ void *data);
+
+#endif /* PCS_H */
--
2.35.1.1320.gc452695387.dirty
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC net-next PATCH 00/13] Add PCS core support
2025-04-03 18:18 [RFC net-next PATCH 00/13] Add PCS core support Sean Anderson
2025-04-03 18:18 ` [RFC net-next PATCH 03/13] net: pcs: Add subsystem Sean Anderson
@ 2025-04-07 16:27 ` Kory Maincent
2025-04-07 16:33 ` Sean Anderson
1 sibling, 1 reply; 12+ messages in thread
From: Kory Maincent @ 2025-04-07 16:27 UTC (permalink / raw)
To: Sean Anderson
Cc: netdev, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Russell King, linux-kernel,
Christian Marangi, upstream, Heiner Kallweit, Alexandre Belloni,
Alexandre Torgue, Christophe Leroy, Clark Wang, Claudiu Beznea,
Claudiu Manoil, Conor Dooley, Ioana Ciornei, Jonathan Corbet,
Joyce Ooi, Krzysztof Kozlowski, Krzysztof Kozlowski, Li Yang,
Madalin Bucur, Madhavan Srinivasan, Maxime Coquelin,
Michael Ellerman, Michal Simek, Naveen N Rao, Nicholas Piggin,
Nicolas Ferre, Radhey Shyam Pandey, Rob Herring, Rob Herring,
Robert Hancock, Saravana Kannan, Shawn Guo, UNGLinuxDriver,
Vladimir Oltean, Wei Fang, devicetree, imx, linux-arm-kernel,
linux-doc, linux-stm32, linuxppc-dev
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC net-next PATCH 00/13] Add PCS core support
2025-04-07 16:27 ` [RFC net-next PATCH 00/13] Add PCS core support Kory Maincent
@ 2025-04-07 16:33 ` Sean Anderson
2025-04-07 16:46 ` Christian Marangi (Ansuel)
2025-04-07 16:51 ` Kory Maincent
0 siblings, 2 replies; 12+ messages in thread
From: Sean Anderson @ 2025-04-07 16:33 UTC (permalink / raw)
To: Kory Maincent
Cc: netdev, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Russell King, linux-kernel,
Christian Marangi, upstream, Heiner Kallweit, Alexandre Belloni,
Alexandre Torgue, Christophe Leroy, Clark Wang, Claudiu Beznea,
Claudiu Manoil, Conor Dooley, Ioana Ciornei, Jonathan Corbet,
Joyce Ooi, Krzysztof Kozlowski, Krzysztof Kozlowski, Li Yang,
Madalin Bucur, Madhavan Srinivasan, Maxime Coquelin,
Michael Ellerman, Michal Simek, Naveen N Rao, Nicholas Piggin,
Nicolas Ferre, Radhey Shyam Pandey, Rob Herring, Rob Herring,
Robert Hancock, Saravana Kannan, Shawn Guo, UNGLinuxDriver,
Vladimir Oltean, Wei Fang, devicetree, imx, linux-arm-kernel,
linux-doc, linux-stm32, linuxppc-dev
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC net-next PATCH 00/13] Add PCS core support
2025-04-07 16:33 ` Sean Anderson
@ 2025-04-07 16:46 ` Christian Marangi (Ansuel)
2025-04-07 17:00 ` Sean Anderson
2025-04-07 16:51 ` Kory Maincent
1 sibling, 1 reply; 12+ messages in thread
From: Christian Marangi (Ansuel) @ 2025-04-07 16:46 UTC (permalink / raw)
To: Sean Anderson
Cc: Kory Maincent, netdev, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Russell King,
linux-kernel, upstream, Heiner Kallweit, Alexandre Belloni,
Alexandre Torgue, Christophe Leroy, Clark Wang, Claudiu Beznea,
Claudiu Manoil, Conor Dooley, Ioana Ciornei, Jonathan Corbet,
Joyce Ooi, Krzysztof Kozlowski, Krzysztof Kozlowski, Li Yang,
Madalin Bucur, Madhavan Srinivasan, Maxime Coquelin,
Michael Ellerman, Michal Simek, Naveen N Rao, Nicholas Piggin,
Nicolas Ferre, Radhey Shyam Pandey, Rob Herring, Rob Herring,
Robert Hancock, Saravana Kannan, Shawn Guo, UNGLinuxDriver,
Vladimir Oltean, Wei Fang, devicetree, imx, linux-arm-kernel,
linux-doc, linux-stm32, linuxppc-dev
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC net-next PATCH 00/13] Add PCS core support
2025-04-07 16:33 ` Sean Anderson
2025-04-07 16:46 ` Christian Marangi (Ansuel)
@ 2025-04-07 16:51 ` Kory Maincent
1 sibling, 0 replies; 12+ messages in thread
From: Kory Maincent @ 2025-04-07 16:51 UTC (permalink / raw)
To: Sean Anderson
Cc: netdev, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Russell King, linux-kernel,
Christian Marangi, upstream, Heiner Kallweit, Alexandre Belloni,
Alexandre Torgue, Christophe Leroy, Clark Wang, Claudiu Beznea,
Claudiu Manoil, Conor Dooley, Ioana Ciornei, Jonathan Corbet,
Joyce Ooi, Krzysztof Kozlowski, Krzysztof Kozlowski, Li Yang,
Madalin Bucur, Madhavan Srinivasan, Maxime Coquelin,
Michael Ellerman, Michal Simek, Naveen N Rao, Nicholas Piggin,
Nicolas Ferre, Radhey Shyam Pandey, Rob Herring, Rob Herring,
Robert Hancock, Saravana Kannan, Shawn Guo, UNGLinuxDriver,
Vladimir Oltean, Wei Fang, devicetree, imx, linux-arm-kernel,
linux-doc, linux-stm32, linuxppc-dev
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC net-next PATCH 00/13] Add PCS core support
2025-04-07 16:46 ` Christian Marangi (Ansuel)
@ 2025-04-07 17:00 ` Sean Anderson
2025-04-07 17:21 ` Christian Marangi (Ansuel)
0 siblings, 1 reply; 12+ messages in thread
From: Sean Anderson @ 2025-04-07 17:00 UTC (permalink / raw)
To: Christian Marangi (Ansuel)
Cc: Kory Maincent, netdev, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Russell King,
linux-kernel, upstream, Heiner Kallweit, Alexandre Belloni,
Alexandre Torgue, Christophe Leroy, Clark Wang, Claudiu Beznea,
Claudiu Manoil, Conor Dooley, Ioana Ciornei, Jonathan Corbet,
Joyce Ooi, Krzysztof Kozlowski, Krzysztof Kozlowski, Li Yang,
Madalin Bucur, Madhavan Srinivasan, Maxime Coquelin,
Michael Ellerman, Michal Simek, Naveen N Rao, Nicholas Piggin,
Nicolas Ferre, Radhey Shyam Pandey, Rob Herring, Rob Herring,
Robert Hancock, Saravana Kannan, Shawn Guo, UNGLinuxDriver,
Vladimir Oltean, Wei Fang, devicetree, imx, linux-arm-kernel,
linux-doc, linux-stm32, linuxppc-dev
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC net-next PATCH 00/13] Add PCS core support
2025-04-07 17:00 ` Sean Anderson
@ 2025-04-07 17:21 ` Christian Marangi (Ansuel)
2025-04-07 17:25 ` Daniel Golle
2025-04-07 18:06 ` Sean Anderson
0 siblings, 2 replies; 12+ messages in thread
From: Christian Marangi (Ansuel) @ 2025-04-07 17:21 UTC (permalink / raw)
To: Sean Anderson
Cc: Kory Maincent, netdev, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Russell King,
linux-kernel, upstream, Heiner Kallweit, Alexandre Belloni,
Alexandre Torgue, Christophe Leroy, Clark Wang, Claudiu Beznea,
Claudiu Manoil, Conor Dooley, Ioana Ciornei, Jonathan Corbet,
Joyce Ooi, Krzysztof Kozlowski, Krzysztof Kozlowski, Li Yang,
Madalin Bucur, Madhavan Srinivasan, Maxime Coquelin,
Michael Ellerman, Michal Simek, Naveen N Rao, Nicholas Piggin,
Nicolas Ferre, Radhey Shyam Pandey, Rob Herring, Rob Herring,
Robert Hancock, Saravana Kannan, Shawn Guo, UNGLinuxDriver,
Vladimir Oltean, Wei Fang, devicetree, imx, linux-arm-kernel,
linux-doc, linux-stm32, linuxppc-dev
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC net-next PATCH 00/13] Add PCS core support
2025-04-07 17:21 ` Christian Marangi (Ansuel)
@ 2025-04-07 17:25 ` Daniel Golle
2025-04-07 17:40 ` Sean Anderson
2025-04-08 15:17 ` Sean Anderson
2025-04-07 18:06 ` Sean Anderson
1 sibling, 2 replies; 12+ messages in thread
From: Daniel Golle @ 2025-04-07 17:25 UTC (permalink / raw)
To: Christian Marangi (Ansuel)
Cc: Sean Anderson, Kory Maincent, netdev, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Russell King, linux-kernel, upstream, Heiner Kallweit,
Alexandre Belloni, Alexandre Torgue, Christophe Leroy, Clark Wang,
Claudiu Beznea, Claudiu Manoil, Conor Dooley, Ioana Ciornei,
Jonathan Corbet, Joyce Ooi, Krzysztof Kozlowski,
Krzysztof Kozlowski, Li Yang, Madalin Bucur, Madhavan Srinivasan,
Maxime Coquelin, Michael Ellerman, Michal Simek, Naveen N Rao,
Nicholas Piggin, Nicolas Ferre, Radhey Shyam Pandey, Rob Herring,
Rob Herring, Robert Hancock, Saravana Kannan, Shawn Guo,
UNGLinuxDriver, Vladimir Oltean, Wei Fang, devicetree, imx,
linux-arm-kernel, linux-doc, linux-stm32, linuxppc-dev
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=*
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC net-next PATCH 00/13] Add PCS core support
2025-04-07 17:25 ` Daniel Golle
@ 2025-04-07 17:40 ` Sean Anderson
2025-04-08 15:17 ` Sean Anderson
1 sibling, 0 replies; 12+ messages in thread
From: Sean Anderson @ 2025-04-07 17:40 UTC (permalink / raw)
To: Daniel Golle, Christian Marangi (Ansuel)
Cc: Kory Maincent, netdev, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Russell King,
linux-kernel, upstream, Heiner Kallweit, Alexandre Belloni,
Alexandre Torgue, Christophe Leroy, Clark Wang, Claudiu Beznea,
Claudiu Manoil, Conor Dooley, Ioana Ciornei, Jonathan Corbet,
Joyce Ooi, Krzysztof Kozlowski, Krzysztof Kozlowski, Li Yang,
Madalin Bucur, Madhavan Srinivasan, Maxime Coquelin,
Michael Ellerman, Michal Simek, Naveen N Rao, Nicholas Piggin,
Nicolas Ferre, Radhey Shyam Pandey, Rob Herring, Rob Herring,
Robert Hancock, Saravana Kannan, Shawn Guo, UNGLinuxDriver,
Vladimir Oltean, Wei Fang, devicetree, imx, linux-arm-kernel,
linux-doc, linux-stm32, linuxppc-dev
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC net-next PATCH 00/13] Add PCS core support
2025-04-07 17:21 ` Christian Marangi (Ansuel)
2025-04-07 17:25 ` Daniel Golle
@ 2025-04-07 18:06 ` Sean Anderson
1 sibling, 0 replies; 12+ messages in thread
From: Sean Anderson @ 2025-04-07 18:06 UTC (permalink / raw)
To: Christian Marangi (Ansuel)
Cc: Kory Maincent, netdev, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Russell King,
linux-kernel, upstream, Heiner Kallweit, Alexandre Belloni,
Alexandre Torgue, Christophe Leroy, Clark Wang, Claudiu Beznea,
Claudiu Manoil, Conor Dooley, Ioana Ciornei, Jonathan Corbet,
Joyce Ooi, Krzysztof Kozlowski, Krzysztof Kozlowski, Li Yang,
Madalin Bucur, Madhavan Srinivasan, Maxime Coquelin,
Michael Ellerman, Michal Simek, Naveen N Rao, Nicholas Piggin,
Nicolas Ferre, Radhey Shyam Pandey, Rob Herring, Rob Herring,
Robert Hancock, Saravana Kannan, Shawn Guo, UNGLinuxDriver,
Vladimir Oltean, Wei Fang, devicetree, imx, linux-arm-kernel,
linux-doc, linux-stm32, linuxppc-dev
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC net-next PATCH 00/13] Add PCS core support
2025-04-07 17:25 ` Daniel Golle
2025-04-07 17:40 ` Sean Anderson
@ 2025-04-08 15:17 ` Sean Anderson
1 sibling, 0 replies; 12+ messages in thread
From: Sean Anderson @ 2025-04-08 15:17 UTC (permalink / raw)
To: Daniel Golle, Christian Marangi (Ansuel)
Cc: Kory Maincent, netdev, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Russell King,
linux-kernel, upstream, Heiner Kallweit, Alexandre Belloni,
Alexandre Torgue, Christophe Leroy, Clark Wang, Claudiu Beznea,
Claudiu Manoil, Conor Dooley, Ioana Ciornei, Jonathan Corbet,
Joyce Ooi, Krzysztof Kozlowski, Krzysztof Kozlowski, Li Yang,
Madalin Bucur, Madhavan Srinivasan, Maxime Coquelin,
Michael Ellerman, Michal Simek, Naveen N Rao, Nicholas Piggin,
Nicolas Ferre, Radhey Shyam Pandey, Rob Herring, Rob Herring,
Robert Hancock, Saravana Kannan, Shawn Guo, UNGLinuxDriver,
Vladimir Oltean, Wei Fang, devicetree, imx, linux-arm-kernel,
linux-doc, linux-stm32, linuxppc-dev
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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-04-08 15:18 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-03 18:18 [RFC net-next PATCH 00/13] Add PCS core support Sean Anderson
2025-04-03 18:18 ` [RFC net-next PATCH 03/13] net: pcs: Add subsystem Sean Anderson
2025-04-07 16:27 ` [RFC net-next PATCH 00/13] Add PCS core support Kory Maincent
2025-04-07 16:33 ` Sean Anderson
2025-04-07 16:46 ` Christian Marangi (Ansuel)
2025-04-07 17:00 ` Sean Anderson
2025-04-07 17:21 ` Christian Marangi (Ansuel)
2025-04-07 17:25 ` Daniel Golle
2025-04-07 17:40 ` Sean Anderson
2025-04-08 15:17 ` Sean Anderson
2025-04-07 18:06 ` Sean Anderson
2025-04-07 16:51 ` Kory Maincent
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).