* [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 05/13] net: pcs: lynx: Convert to an MDIO driver 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 05/13] net: pcs: lynx: Convert to an MDIO driver
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, Alexandre Belloni, Alexandre Torgue, Clark Wang,
Claudiu Manoil, Ioana Ciornei, Joyce Ooi, Madalin Bucur,
Maxime Coquelin, UNGLinuxDriver, Vladimir Oltean, Wei Fang, imx,
linux-stm32
This converts the lynx PCS driver to a proper MDIO driver.
This allows using a more conventional driver lifecycle (e.g. with a
probe and remove). It will also make it easier to add interrupt support.
The existing helpers are converted to bind the MDIO driver instead of
creating the PCS directly. As lynx_pcs_create_mdiodev creates the PCS
device, we can just set the modalias. For lynx_pcs_create_fwnode, we try
to get the PCS the usual way, and if that fails we edit the devicetree
to add a compatible and reprobe the device.
Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---
drivers/net/dsa/ocelot/Kconfig | 4 +
drivers/net/dsa/ocelot/felix_vsc9959.c | 11 +-
drivers/net/dsa/ocelot/seville_vsc9953.c | 11 +-
drivers/net/ethernet/altera/Kconfig | 2 +
drivers/net/ethernet/altera/altera_tse_main.c | 7 +-
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/pcs/Kconfig | 11 +-
drivers/net/pcs/pcs-lynx.c | 115 ++++++++++--------
include/linux/pcs-lynx.h | 13 +-
19 files changed, 135 insertions(+), 110 deletions(-)
diff --git a/drivers/net/dsa/ocelot/Kconfig b/drivers/net/dsa/ocelot/Kconfig
index 081e7a88ea02..e879f82537db 100644
--- a/drivers/net/dsa/ocelot/Kconfig
+++ b/drivers/net/dsa/ocelot/Kconfig
@@ -42,7 +42,9 @@ config NET_DSA_MSCC_FELIX
select NET_DSA_TAG_OCELOT_8021Q
select NET_DSA_TAG_OCELOT
select FSL_ENETC_MDIO
+ select PCS
select PCS_LYNX
+ select MDIO_DEVICE
help
This driver supports the VSC9959 (Felix) switch, which is embedded as
a PCIe function of the NXP LS1028A ENETC RCiEP.
@@ -58,7 +60,9 @@ config NET_DSA_MSCC_SEVILLE
select NET_DSA_MSCC_FELIX_DSA_LIB
select NET_DSA_TAG_OCELOT_8021Q
select NET_DSA_TAG_OCELOT
+ select PCS
select PCS_LYNX
+ select MDIO_DEVICE
help
This driver supports the VSC9953 (Seville) switch, which is embedded
as a platform device on the NXP T1040 SoC.
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 2de12611ab57..a67fc2f8528c 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -12,6 +12,7 @@
#include <net/tc_act/tc_gate.h>
#include <soc/mscc/ocelot.h>
#include <linux/dsa/ocelot.h>
+#include <linux/pcs.h>
#include <linux/pcs-lynx.h>
#include <net/pkt_sched.h>
#include <linux/iopoll.h>
@@ -1033,7 +1034,7 @@ static int vsc9959_mdio_bus_alloc(struct ocelot *ocelot)
if (ocelot_port->phy_mode == PHY_INTERFACE_MODE_INTERNAL)
continue;
- phylink_pcs = lynx_pcs_create_mdiodev(felix->imdio, port);
+ phylink_pcs = lynx_pcs_create_mdiodev(dev, felix->imdio, port);
if (IS_ERR(phylink_pcs))
continue;
@@ -1050,12 +1051,8 @@ static void vsc9959_mdio_bus_free(struct ocelot *ocelot)
struct felix *felix = ocelot_to_felix(ocelot);
int port;
- for (port = 0; port < ocelot->num_phys_ports; port++) {
- struct phylink_pcs *phylink_pcs = felix->pcs[port];
-
- if (phylink_pcs)
- lynx_pcs_destroy(phylink_pcs);
- }
+ for (port = 0; port < ocelot->num_phys_ports; port++)
+ pcs_put(ocelot->dev, felix->pcs[port]);
mdiobus_unregister(felix->imdio);
mdiobus_free(felix->imdio);
}
diff --git a/drivers/net/dsa/ocelot/seville_vsc9953.c b/drivers/net/dsa/ocelot/seville_vsc9953.c
index 28bcdef34a6c..0e61bd8bc7cd 100644
--- a/drivers/net/dsa/ocelot/seville_vsc9953.c
+++ b/drivers/net/dsa/ocelot/seville_vsc9953.c
@@ -10,6 +10,7 @@
#include <linux/mdio/mdio-mscc-miim.h>
#include <linux/mod_devicetable.h>
#include <linux/of_mdio.h>
+#include <linux/pcs.h>
#include <linux/pcs-lynx.h>
#include <linux/dsa/ocelot.h>
#include <linux/iopoll.h>
@@ -926,7 +927,7 @@ static int vsc9953_mdio_bus_alloc(struct ocelot *ocelot)
if (ocelot_port->phy_mode == PHY_INTERFACE_MODE_INTERNAL)
continue;
- phylink_pcs = lynx_pcs_create_mdiodev(felix->imdio, addr);
+ phylink_pcs = lynx_pcs_create_mdiodev(dev, felix->imdio, addr);
if (IS_ERR(phylink_pcs))
continue;
@@ -943,12 +944,8 @@ static void vsc9953_mdio_bus_free(struct ocelot *ocelot)
struct felix *felix = ocelot_to_felix(ocelot);
int port;
- for (port = 0; port < ocelot->num_phys_ports; port++) {
- struct phylink_pcs *phylink_pcs = felix->pcs[port];
-
- if (phylink_pcs)
- lynx_pcs_destroy(phylink_pcs);
- }
+ for (port = 0; port < ocelot->num_phys_ports; port++)
+ pcs_put(ocelot->dev, felix->pcs[port]);
/* mdiobus_unregister and mdiobus_free handled by devres */
}
diff --git a/drivers/net/ethernet/altera/Kconfig b/drivers/net/ethernet/altera/Kconfig
index 4ef819a9a1ad..daf508a195a5 100644
--- a/drivers/net/ethernet/altera/Kconfig
+++ b/drivers/net/ethernet/altera/Kconfig
@@ -5,7 +5,9 @@ config ALTERA_TSE
depends on HAS_IOMEM
select PHYLIB
select PHYLINK
+ select PCS
select PCS_LYNX
+ select MDIO_DEVICE
select MDIO_REGMAP
select REGMAP_MMIO
help
diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index 3f6204de9e6b..7b34b6dc521e 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -32,6 +32,7 @@
#include <linux/of.h>
#include <linux/of_mdio.h>
#include <linux/of_net.h>
+#include <linux/pcs.h>
#include <linux/pcs-lynx.h>
#include <linux/phy.h>
#include <linux/platform_device.h>
@@ -1412,7 +1413,7 @@ static int altera_tse_probe(struct platform_device *pdev)
goto err_init_pcs;
}
- priv->pcs = lynx_pcs_create_mdiodev(pcs_bus, 0);
+ priv->pcs = lynx_pcs_create_mdiodev(&pdev->dev, pcs_bus, 0);
if (IS_ERR(priv->pcs)) {
ret = PTR_ERR(priv->pcs);
goto err_init_pcs;
@@ -1444,7 +1445,7 @@ static int altera_tse_probe(struct platform_device *pdev)
return 0;
err_init_phylink:
- lynx_pcs_destroy(priv->pcs);
+ pcs_put(&pdev->dev, priv->pcs);
err_init_pcs:
unregister_netdev(ndev);
err_register_netdev:
@@ -1466,7 +1467,7 @@ static void altera_tse_remove(struct platform_device *pdev)
altera_tse_mdio_destroy(ndev);
unregister_netdev(ndev);
phylink_destroy(priv->phylink);
- lynx_pcs_destroy(priv->pcs);
+ pcs_put(&pdev->dev, priv->pcs);
free_netdev(ndev);
}
diff --git a/drivers/net/ethernet/freescale/dpaa/Kconfig b/drivers/net/ethernet/freescale/dpaa/Kconfig
index 2b560661c82a..3129f029445b 100644
--- a/drivers/net/ethernet/freescale/dpaa/Kconfig
+++ b/drivers/net/ethernet/freescale/dpaa/Kconfig
@@ -3,7 +3,7 @@ menuconfig FSL_DPAA_ETH
tristate "DPAA Ethernet"
depends on FSL_DPAA && FSL_FMAN
select PHYLINK
- select PCS_LYNX
+ select MDIO_DEVICE
help
Data Path Acceleration Architecture Ethernet driver,
supporting the Freescale QorIQ chips.
diff --git a/drivers/net/ethernet/freescale/dpaa2/Kconfig b/drivers/net/ethernet/freescale/dpaa2/Kconfig
index d029b69c3f18..28e63dc4acbf 100644
--- a/drivers/net/ethernet/freescale/dpaa2/Kconfig
+++ b/drivers/net/ethernet/freescale/dpaa2/Kconfig
@@ -2,8 +2,11 @@
config FSL_DPAA2_ETH
tristate "Freescale DPAA2 Ethernet"
depends on FSL_MC_BUS && FSL_MC_DPIO
+ select OF_DYNAMIC
select PHYLINK
+ select PCS
select PCS_LYNX
+ select MDIO_DEVICE
select FSL_XGMAC_MDIO
select NET_DEVLINK
help
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
index 422ce13a7c94..26accbe7e736 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
@@ -2,6 +2,7 @@
/* Copyright 2019 NXP */
#include <linux/acpi.h>
+#include <linux/pcs.h>
#include <linux/pcs-lynx.h>
#include <linux/phy/phy.h>
#include <linux/property.h>
@@ -262,7 +263,7 @@ static int dpaa2_pcs_create(struct dpaa2_mac *mac,
return 0;
}
- pcs = lynx_pcs_create_fwnode(node);
+ pcs = lynx_pcs_create_fwnode(&mac->mc_dev->dev, node);
fwnode_handle_put(node);
if (pcs == ERR_PTR(-EPROBE_DEFER)) {
@@ -288,12 +289,8 @@ static int dpaa2_pcs_create(struct dpaa2_mac *mac,
static void dpaa2_pcs_destroy(struct dpaa2_mac *mac)
{
- struct phylink_pcs *phylink_pcs = mac->pcs;
-
- if (phylink_pcs) {
- lynx_pcs_destroy(phylink_pcs);
- mac->pcs = NULL;
- }
+ pcs_put(&mac->mc_dev->dev, mac->pcs);
+ mac->pcs = NULL;
}
static void dpaa2_mac_set_supported_interfaces(struct dpaa2_mac *mac)
diff --git a/drivers/net/ethernet/freescale/enetc/Kconfig b/drivers/net/ethernet/freescale/enetc/Kconfig
index 6c2779047dcd..6a58254b9a5f 100644
--- a/drivers/net/ethernet/freescale/enetc/Kconfig
+++ b/drivers/net/ethernet/freescale/enetc/Kconfig
@@ -24,7 +24,9 @@ config FSL_ENETC
select FSL_ENETC_MDIO
select NXP_ENETC_PF_COMMON
select PHYLINK
+ select PCS
select PCS_LYNX
+ select MDIO_DEVICE
select DIMLIB
help
This driver supports NXP ENETC gigabit ethernet controller PCIe
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 203862ec1114..737797b541a0 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -34,12 +34,7 @@ static void enetc_pf_set_primary_mac_addr(struct enetc_hw *hw, int si,
static struct phylink_pcs *enetc_pf_create_pcs(struct enetc_pf *pf,
struct mii_bus *bus)
{
- return lynx_pcs_create_mdiodev(bus, 0);
-}
-
-static void enetc_pf_destroy_pcs(struct phylink_pcs *pcs)
-{
- lynx_pcs_destroy(pcs);
+ return lynx_pcs_create_mdiodev(&pf->si->pdev->dev, bus, 0);
}
static void enetc_set_vlan_promisc(struct enetc_hw *hw, char si_map)
@@ -1003,7 +998,6 @@ static const struct enetc_pf_ops enetc_pf_ops = {
.set_si_primary_mac = enetc_pf_set_primary_mac_addr,
.get_si_primary_mac = enetc_pf_get_primary_mac_addr,
.create_pcs = enetc_pf_create_pcs,
- .destroy_pcs = enetc_pf_destroy_pcs,
.enable_psfp = enetc_psfp_enable,
};
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.h b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
index a26a12863855..d5e02d02c7a8 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
@@ -42,7 +42,6 @@ struct enetc_pf_ops {
void (*set_si_primary_mac)(struct enetc_hw *hw, int si, const u8 *addr);
void (*get_si_primary_mac)(struct enetc_hw *hw, int si, u8 *addr);
struct phylink_pcs *(*create_pcs)(struct enetc_pf *pf, struct mii_bus *bus);
- void (*destroy_pcs)(struct phylink_pcs *pcs);
int (*enable_psfp)(struct enetc_ndev_priv *priv);
};
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
index 3fd9b0727875..f5b75d40f7c5 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
@@ -4,6 +4,7 @@
#include <linux/fsl/enetc_mdio.h>
#include <linux/of_mdio.h>
#include <linux/of_net.h>
+#include <linux/pcs.h>
#include "enetc_pf_common.h"
@@ -248,8 +249,7 @@ static int enetc_imdio_create(struct enetc_pf *pf)
static void enetc_imdio_remove(struct enetc_pf *pf)
{
- if (pf->pcs && pf->ops->destroy_pcs)
- pf->ops->destroy_pcs(pf->pcs);
+ pcs_put(&pf->si->pdev->dev, pf->pcs);
if (pf->imdio) {
mdiobus_unregister(pf->imdio);
diff --git a/drivers/net/ethernet/freescale/fman/Kconfig b/drivers/net/ethernet/freescale/fman/Kconfig
index a55542c1ad65..b87b22220972 100644
--- a/drivers/net/ethernet/freescale/fman/Kconfig
+++ b/drivers/net/ethernet/freescale/fman/Kconfig
@@ -3,10 +3,12 @@ config FSL_FMAN
tristate "FMan support"
depends on FSL_SOC || ARCH_LAYERSCAPE || COMPILE_TEST
select GENERIC_ALLOCATOR
+ select OF_DYNAMIC
+ select MDIO_DEVICE
select PHYLINK
+ select PCS
select PCS_LYNX
select CRC32
- default n
help
Freescale Data-Path Acceleration Architecture Frame Manager
(FMan) support
diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c b/drivers/net/ethernet/freescale/fman/fman_memac.c
index 3925441143fa..fc12f3e44824 100644
--- a/drivers/net/ethernet/freescale/fman/fman_memac.c
+++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
@@ -11,6 +11,7 @@
#include <linux/slab.h>
#include <linux/io.h>
+#include <linux/pcs.h>
#include <linux/pcs-lynx.h>
#include <linux/phy.h>
#include <linux/phy_fixed.h>
@@ -972,21 +973,23 @@ static int memac_init(struct fman_mac *memac)
return 0;
}
-static void pcs_put(struct phylink_pcs *pcs)
+static void memac_pcs_put(struct device *dev, struct phylink_pcs *pcs)
{
if (IS_ERR_OR_NULL(pcs))
return;
- lynx_pcs_destroy(pcs);
+ pcs_put(dev, pcs);
}
static int memac_free(struct fman_mac *memac)
{
+ struct device *dev = memac->dev_id->dev;
+
free_init_resources(memac);
- pcs_put(memac->sgmii_pcs);
- pcs_put(memac->qsgmii_pcs);
- pcs_put(memac->xfi_pcs);
+ memac_pcs_put(dev, memac->sgmii_pcs);
+ memac_pcs_put(dev, memac->qsgmii_pcs);
+ memac_pcs_put(dev, memac->xfi_pcs);
kfree(memac->memac_drv_param);
kfree(memac);
@@ -1033,7 +1036,8 @@ static struct fman_mac *memac_config(struct mac_device *mac_dev,
return memac;
}
-static struct phylink_pcs *memac_pcs_create(struct device_node *mac_node,
+static struct phylink_pcs *memac_pcs_create(struct device *dev,
+ struct device_node *mac_node,
int index)
{
struct device_node *node;
@@ -1043,7 +1047,7 @@ static struct phylink_pcs *memac_pcs_create(struct device_node *mac_node,
if (!node)
return ERR_PTR(-ENODEV);
- pcs = lynx_pcs_create_fwnode(of_fwnode_handle(node));
+ pcs = lynx_pcs_create_fwnode(dev, of_fwnode_handle(node));
of_node_put(node);
return pcs;
@@ -1100,7 +1104,7 @@ int memac_initialization(struct mac_device *mac_dev,
err = of_property_match_string(mac_node, "pcs-handle-names", "xfi");
if (err >= 0) {
- memac->xfi_pcs = memac_pcs_create(mac_node, err);
+ memac->xfi_pcs = memac_pcs_create(mac_dev->dev, mac_node, err);
if (IS_ERR(memac->xfi_pcs)) {
err = PTR_ERR(memac->xfi_pcs);
dev_err_probe(mac_dev->dev, err, "missing xfi pcs\n");
@@ -1112,7 +1116,8 @@ int memac_initialization(struct mac_device *mac_dev,
err = of_property_match_string(mac_node, "pcs-handle-names", "qsgmii");
if (err >= 0) {
- memac->qsgmii_pcs = memac_pcs_create(mac_node, err);
+ memac->qsgmii_pcs = memac_pcs_create(mac_dev->dev, mac_node,
+ err);
if (IS_ERR(memac->qsgmii_pcs)) {
err = PTR_ERR(memac->qsgmii_pcs);
dev_err_probe(mac_dev->dev, err,
@@ -1128,11 +1133,11 @@ int memac_initialization(struct mac_device *mac_dev,
*/
err = of_property_match_string(mac_node, "pcs-handle-names", "sgmii");
if (err == -EINVAL || err == -ENODATA)
- pcs = memac_pcs_create(mac_node, 0);
+ pcs = memac_pcs_create(mac_dev->dev, mac_node, 0);
else if (err < 0)
goto _return_fm_mac_free;
else
- pcs = memac_pcs_create(mac_node, err);
+ pcs = memac_pcs_create(mac_dev->dev, mac_node, err);
if (IS_ERR(pcs)) {
err = PTR_ERR(pcs);
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 3c820ef56775..3df23331c726 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -170,9 +170,12 @@ config DWMAC_SOCFPGA
tristate "SOCFPGA dwmac support"
default ARCH_INTEL_SOCFPGA
depends on OF && (ARCH_INTEL_SOCFPGA || COMPILE_TEST)
+ select OF_DYNAMIC
select MFD_SYSCON
+ select MDIO_DEVICE
select MDIO_REGMAP
select REGMAP_MMIO
+ select PCS
select PCS_LYNX
help
Support for ethernet controller on Altera SOCFPGA
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 116855658559..2c2fb820752b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -8,6 +8,7 @@
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_net.h>
+#include <linux/pcs.h>
#include <linux/phy.h>
#include <linux/regmap.h>
#include <linux/mdio/mdio-regmap.h>
@@ -415,7 +416,7 @@ static int socfpga_dwmac_pcs_init(struct stmmac_priv *priv)
if (IS_ERR(pcs_bus))
return PTR_ERR(pcs_bus);
- pcs = lynx_pcs_create_mdiodev(pcs_bus, 0);
+ pcs = lynx_pcs_create_mdiodev(priv->device, pcs_bus, 0);
if (IS_ERR(pcs))
return PTR_ERR(pcs);
@@ -425,8 +426,7 @@ static int socfpga_dwmac_pcs_init(struct stmmac_priv *priv)
static void socfpga_dwmac_pcs_exit(struct stmmac_priv *priv)
{
- if (priv->hw->phylink_pcs)
- lynx_pcs_destroy(priv->hw->phylink_pcs);
+ pcs_put(priv->device, priv->hw->phylink_pcs);
}
static struct phylink_pcs *socfpga_dwmac_select_pcs(struct stmmac_priv *priv,
diff --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig
index b764770063cc..91ff59899aaf 100644
--- a/drivers/net/pcs/Kconfig
+++ b/drivers/net/pcs/Kconfig
@@ -25,10 +25,15 @@ config PCS_XPCS
DesignWare XPCS controllers.
config PCS_LYNX
- tristate
+ tristate "NXP Lynx PCS driver"
+ depends on PCS && MDIO_DEVICE
help
- This module provides helpers to phylink for managing the Lynx PCS
- which is part of the Layerscape and QorIQ Ethernet SERDES.
+ This module provides driver support for the PCSs in Lynx 10g and 28g
+ SerDes devices. These devices are present in NXP QorIQ SoCs,
+ including the Layerscape series.
+
+ If you want to use Ethernet on a QorIQ SoC, say "Y". If compiled as a
+ module, it will be called "pcs-lynx".
config PCS_MTK_LYNXI
tristate
diff --git a/drivers/net/pcs/pcs-lynx.c b/drivers/net/pcs/pcs-lynx.c
index 23b40e9eacbb..4a6bd5248efa 100644
--- a/drivers/net/pcs/pcs-lynx.c
+++ b/drivers/net/pcs/pcs-lynx.c
@@ -1,11 +1,15 @@
-// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
-/* Copyright 2020 NXP
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (C) 2022 Sean Anderson <seanga2@gmail.com>
+ * Copyright 2020 NXP
* Lynx PCS MDIO helpers
*/
#include <linux/mdio.h>
#include <linux/phylink.h>
+#include <linux/of.h>
+#include <linux/pcs.h>
#include <linux/pcs-lynx.h>
+#include <linux/phylink.h>
#include <linux/property.h>
#define SGMII_CLOCK_PERIOD_NS 8 /* PCS is clocked at 125 MHz */
@@ -343,16 +347,16 @@ static const phy_interface_t lynx_interfaces[] = {
PHY_INTERFACE_MODE_USXGMII,
};
-static struct phylink_pcs *lynx_pcs_create(struct mdio_device *mdio)
+static int lynx_pcs_probe(struct mdio_device *mdio)
{
+ struct device *dev = &mdio->dev;
struct lynx_pcs *lynx;
- int i;
+ int i, ret;
- lynx = kzalloc(sizeof(*lynx), GFP_KERNEL);
+ lynx = devm_kzalloc(dev, sizeof(*lynx), GFP_KERNEL);
if (!lynx)
- return ERR_PTR(-ENOMEM);
+ return -ENOMEM;
- mdio_device_get(mdio);
lynx->mdio = mdio;
lynx->pcs.ops = &lynx_pcs_phylink_ops;
lynx->pcs.poll = true;
@@ -360,32 +364,69 @@ static struct phylink_pcs *lynx_pcs_create(struct mdio_device *mdio)
for (i = 0; i < ARRAY_SIZE(lynx_interfaces); i++)
__set_bit(lynx_interfaces[i], lynx->pcs.supported_interfaces);
- return lynx_to_phylink_pcs(lynx);
+ ret = devm_pcs_register(dev, &lynx->pcs);
+ if (ret)
+ return dev_err_probe(dev, ret, "could not register PCS\n");
+ dev_info(dev, "probed\n");
+ return 0;
}
-struct phylink_pcs *lynx_pcs_create_mdiodev(struct mii_bus *bus, int addr)
+static const struct of_device_id lynx_pcs_of_match[] = {
+ { .compatible = "fsl,lynx-pcs" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, lynx_pcs_of_match);
+
+static struct mdio_driver lynx_pcs_driver = {
+ .probe = lynx_pcs_probe,
+ .mdiodrv.driver = {
+ .name = "lynx-pcs",
+ .of_match_table = of_match_ptr(lynx_pcs_of_match),
+ },
+};
+mdio_module_driver(lynx_pcs_driver);
+
+struct phylink_pcs *lynx_pcs_create_mdiodev(struct device *dev,
+ struct mii_bus *bus, int addr)
{
struct mdio_device *mdio;
struct phylink_pcs *pcs;
+ int err;
mdio = mdio_device_create(bus, addr);
if (IS_ERR(mdio))
return ERR_CAST(mdio);
- pcs = lynx_pcs_create(mdio);
-
- /* lynx_create() has taken a refcount on the mdiodev if it was
- * successful. If lynx_create() fails, this will free the mdio
- * device here. In any case, we don't need to hold our reference
- * anymore, and putting it here will allow mdio_device_put() in
- * lynx_destroy() to automatically free the mdio device.
- */
- mdio_device_put(mdio);
+ mdio->bus_match = mdio_device_bus_match;
+ strscpy(mdio->modalias, "lynx-pcs");
+ err = mdio_device_register(mdio);
+ if (err) {
+ mdio_device_free(mdio);
+ return ERR_PTR(err);
+ }
+ pcs = pcs_get_by_dev(dev, &mdio->dev);
+ mdio_device_free(mdio);
return pcs;
}
EXPORT_SYMBOL(lynx_pcs_create_mdiodev);
+static const struct property_entry lynx_properties[] = {
+ PROPERTY_ENTRY_STRING("compatible", "fsl,lynx-pcs"),
+ { },
+};
+
+static int lynx_pcs_fixup(struct of_changeset *ocs,
+ struct device_node *np, void *data)
+{
+#ifdef CONFIG_OF_DYNAMIC
+ return of_changeset_add_prop_string(ocs, np, "compatible",
+ "fsl,lynx-pcs");
+#else
+ return -ENODEV;
+#endif
+}
+
/*
* lynx_pcs_create_fwnode() creates a lynx PCS instance from the fwnode
* device indicated by node.
@@ -396,40 +437,12 @@ EXPORT_SYMBOL(lynx_pcs_create_mdiodev);
* -ENOMEM if we fail to allocate memory
* pointer to a phylink_pcs on success
*/
-struct phylink_pcs *lynx_pcs_create_fwnode(struct fwnode_handle *node)
+struct phylink_pcs *lynx_pcs_create_fwnode(struct device *dev,
+ struct fwnode_handle *node)
{
- struct mdio_device *mdio;
- struct phylink_pcs *pcs;
-
- if (!fwnode_device_is_available(node))
- return ERR_PTR(-ENODEV);
-
- mdio = fwnode_mdio_find_device(node);
- if (!mdio)
- return ERR_PTR(-EPROBE_DEFER);
-
- pcs = lynx_pcs_create(mdio);
-
- /* lynx_create() has taken a refcount on the mdiodev if it was
- * successful. If lynx_create() fails, this will free the mdio
- * device here. In any case, we don't need to hold our reference
- * anymore, and putting it here will allow mdio_device_put() in
- * lynx_destroy() to automatically free the mdio device.
- */
- mdio_device_put(mdio);
-
- return pcs;
+ return pcs_get_by_fwnode_compat(dev, node, lynx_pcs_fixup, NULL);
}
EXPORT_SYMBOL_GPL(lynx_pcs_create_fwnode);
-void lynx_pcs_destroy(struct phylink_pcs *pcs)
-{
- struct lynx_pcs *lynx = phylink_pcs_to_lynx(pcs);
-
- mdio_device_put(lynx->mdio);
- kfree(lynx);
-}
-EXPORT_SYMBOL(lynx_pcs_destroy);
-
-MODULE_DESCRIPTION("NXP Lynx PCS phylink library");
-MODULE_LICENSE("Dual BSD/GPL");
+MODULE_DESCRIPTION("NXP Lynx PCS phylink driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/pcs-lynx.h b/include/linux/pcs-lynx.h
index 7958cccd16f2..a95801337205 100644
--- a/include/linux/pcs-lynx.h
+++ b/include/linux/pcs-lynx.h
@@ -6,12 +6,13 @@
#ifndef __LINUX_PCS_LYNX_H
#define __LINUX_PCS_LYNX_H
-#include <linux/mdio.h>
-#include <linux/phylink.h>
+struct device;
+struct mii_bus;
+struct phylink_pcs;
-struct phylink_pcs *lynx_pcs_create_mdiodev(struct mii_bus *bus, int addr);
-struct phylink_pcs *lynx_pcs_create_fwnode(struct fwnode_handle *node);
-
-void lynx_pcs_destroy(struct phylink_pcs *pcs);
+struct phylink_pcs *lynx_pcs_create_mdiodev(struct device *dev,
+ struct mii_bus *bus, int addr);
+struct phylink_pcs *lynx_pcs_create_fwnode(struct device *dev,
+ struct fwnode_handle *node);
#endif /* __LINUX_PCS_LYNX_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 05/13] net: pcs: lynx: Convert to an MDIO driver 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 05/13] net: pcs: lynx: Convert to an MDIO driver 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