[PATCH v3 phy 00/17] Lynx 28G improvements part 1

[PATCH v3 phy 00/17] Lynx 28G improvements part 1

[Vladimir Oltean]

This is the first part in upstreaming a set of around 100 patches that
were developed in NXP's vendor Linux Factory kernel over the course of
several years.

This part is mainly concerned with correcting some historical mistakes
which make extending the driver more difficult:
- the register naming scheme forces us to modify a single register field
  per lynx_28g_lane_rmw() call - leads to inefficient code
- lynx_28g_lane_set_sgmii(), lynx_28g_lane_set_10gbaser() are unfit for
  their required roles when the current SerDes protocol is 25GBase-R.
  They are replaced with a better structured approach.
- USXGMII and 10GBase-R have different protocol converters, and should
  be treated separately by the SerDes driver.
- Lane power management does not really power down the lanes.
- Consumer drivers using phy_exit() would cause the kernel to hang.
- The 3 instances of this SerDes block, as seen on NXP LX2160A, need to
  be differentiated somehow, because otherwise, the driver cannot reject
  a configuration which is unsupported by the hardware. The proposal is
  to do that based on compatible string.

In addition to the above, a new feature is also added in patch 14/17:
25GBase-R. Code allowing this mode to be used is also necessary in the
Ethernet MAC and PCS drivers - not present here.

The set grew in size (sorry!) from v1 here:
https://lore.kernel.org/lkml/20250904154402.300032-1-vladimir.oltean@nxp.com/
due to Josua's request for a device tree binding where individual lanes
have their own OF nodes. This seems to be the right moment to make that
change.

It also grew in size from v2 due to Josua's request to avoid unbounded
loops waiting for lane resets/halts/stops to complete:
https://lore.kernel.org/lkml/d0c8bbf8-a0c5-469f-a148-de2235948c0f@solid-run.com/

Detailed change log in individual patches. Thanks to Josua, Rob, Conor,
Krzysztof, Ioana who provided feedback on the previous version, and I
hope it has all been addressed.

Cc: Rob Herring <robh@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: devicetree@vger.kernel.org

Ioana Ciornei (2):
  phy: lynx-28g: configure more equalization params for 1GbE and 10GbE
  phy: lynx-28g: add support for 25GBASER

Vladimir Oltean (15):
  phy: lynx-28g: remove LYNX_28G_ prefix from register names
  phy: lynx-28g: don't concatenate lynx_28g_lane_rmw() argument "reg"
    with "val" and "mask"
  phy: lynx-28g: use FIELD_GET() and FIELD_PREP()
  phy: lynx-28g: convert iowrite32() calls with magic values to macros
  phy: lynx-28g: restructure protocol configuration register accesses
  phy: lynx-28g: make lynx_28g_set_lane_mode() more systematic
  phy: lynx-28g: refactor lane->interface to lane->mode
  phy: lynx-28g: distinguish between 10GBASE-R and USXGMII
  phy: lynx-28g: use "dev" argument more in lynx_28g_probe()
  phy: lynx-28g: improve lynx_28g_probe() sequence
  dt-bindings: phy: lynx-28g: add compatible strings per SerDes and
    instantiation
  phy: lynx-28g: probe on per-SoC and per-instance compatible strings
  phy: lynx-28g: use timeouts when waiting for lane halt and reset
  phy: lynx-28g: truly power the lanes up or down
  phy: lynx-28g: implement phy_exit() operation

 .../devicetree/bindings/phy/fsl,lynx-28g.yaml |  159 +-
 drivers/phy/freescale/phy-fsl-lynx-28g.c      | 1505 +++++++++++++----
 2 files changed, 1366 insertions(+), 298 deletions(-)

-- 
2.34.1

[PATCH v3 phy 12/17] dt-bindings: phy: lynx-28g: add compatible strings per SerDes and instantiation

[Vladimir Oltean]

Going by the generic "fsl,lynx-28g" compatible string and expecting all
SerDes instantiations on all SoCs to use it was a mistake.

They all share the same register map, sure, but the number of protocol
converters and lanes which are instantiated differs in a way that isn't
detectable by the programming interface.

Using a separate compatible string per SerDes instantiation is
sufficient for any device driver to distinguish these features and/or
any instance-specific quirk. It also reflects how the SoC reference
manual provides different tables with protocol combinations for each
SerDes. NXP clearly documents these as not identical, and refers to them
as such (SerDes 1, 2, etc).

The other sufficient approach for Lynx 28G would be to list in the
device tree all protocols supported by each lane. That would be
insufficient for the very similar Lynx 10G SerDes however, for which
there exists a higher degree of variability in the PCCR register values
that need to be written per protocol. This attempt can be seen in this
unmerged patch set for Lynx 10G:
https://lore.kernel.org/linux-phy/20230413160607.4128315-3-sean.anderson@seco.com/

but that approach is more drawn-out and more prone to errors, whereas
this one is more succinct and obviously correct.

One aspect which is different with the per-SoC compatible strings is
that they have one PHY provider for each lane (and #phy-cells = <0> in
lane sub-nodes), rather than "fsl,lynx-28g" which has a single PHY
provider for all lanes (and #phy-cells = <1> in the top-level node).

This is done to fulfill Josua Mayer's request:
https://lore.kernel.org/lkml/02270f62-9334-400c-b7b9-7e6a44dbbfc9@solid-run.com/
to have OF nodes for each lane, so that we can further apply schemas
such as Documentation/devicetree/bindings/phy/transmit-amplitude.yaml
individually.

This is the easiest and most intuitive way to describe that. The above
is not the only electrical tuning that needs to be done, but rather the
only one currently standardized in a schema. TX equalization parameters
are TBD, but we need to not limit ourselves to just what currently exists.

Luckily, we can overlap the modern binding format over the legacy one
and they can coexist without interfering. Old kernels use compatible =
"fsl,lynx-28g" and the top-level PHY provider, whereas new kernels probe
on e.g. compatible = "fsl,lx2160a-serdes1" and use the per-lane PHY
providers.

Overlaying modern on top of legacy is only necessary for SerDes 1 and 2.
LX2160A SerDes #3 (a non-networking SerDes) is not yet present in any
device trees in circulation, and will only have the device-specific
compatible (even though it shares the Lynx 28G programming model,
specifying the "fsl,lynx-28g" compatible string for it provides no
benefit that I can see).

Change the expected name of the top-level node to "serdes", and update
the example too.

Cc: Rob Herring <robh@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v2->v3:
- re-add "fsl,lynx-28g" as fallback compatible, and #phy-cells = <1> in
  top-level "serdes" node
- drop useless description texts
- fix text formatting
- schema is more lax to allow overlaying old and new required properties
v1->v2:
- drop the usage of "fsl,lynx-28g" as a fallback compatible
- mark "fsl,lynx-28g" as deprecated
- implement Josua's request for per-lane OF nodes for the new compatible
  strings

 .../devicetree/bindings/phy/fsl,lynx-28g.yaml | 159 +++++++++++++++++-
 1 file changed, 152 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml b/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml
index ff9f9ca0f19c..e8b3a48b9515 100644
--- a/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml
+++ b/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml
@@ -9,21 +9,123 @@ title: Freescale Lynx 28G SerDes PHY
 maintainers:
   - Ioana Ciornei <ioana.ciornei@nxp.com>
 
+description:
+  The Lynx 28G is a multi-lane, multi-protocol SerDes (PCIe, SATA, Ethernet)
+  present in multiple instances on NXP LX2160A and LX2162A SoCs. All instances
+  share a common register map and programming model, however they differ in
+  supported protocols per lane in a way that is not detectable by said
+  programming model without prior knowledge. The distinction is made through
+  the compatible string.
+
 properties:
   compatible:
-    enum:
-      - fsl,lynx-28g
+    oneOf:
+      - const: fsl,lynx-28g
+        deprecated: true
+        description:
+          Legacy compatibility string for Lynx 28G SerDes. Any assumption
+          regarding whether a certain lane supports a certain protocol may
+          be incorrect. Deprecated except when used as a fallback. Use
+          device-specific strings instead.
+      - items:
+          - const: fsl,lx2160a-serdes1
+          - const: fsl,lynx-28g
+      - items:
+          - const: fsl,lx2160a-serdes2
+          - const: fsl,lynx-28g
+      - items:
+          - const: fsl,lx2162a-serdes1
+          - const: fsl,lynx-28g
+      - items:
+          - const: fsl,lx2162a-serdes2
+          - const: fsl,lynx-28g
+      - const: fsl,lx2160a-serdes3
 
   reg:
     maxItems: 1
 
-  "#phy-cells":
-    const: 1
+  "#address-cells": true
+
+  "#size-cells": true
+
+  "#phy-cells": true
+
+patternProperties:
+  "^phy@[0-9a-f]+$":
+    type: object
+    description: Individual SerDes lane acting as PHY provider
+
+    properties:
+      reg:
+        description: Lane index as seen in register map
+        maxItems: 1
+
+      "#phy-cells":
+        const: 0
+
+    required:
+      - reg
+      - "#phy-cells"
+
+    additionalProperties: false
 
 required:
   - compatible
   - reg
-  - "#phy-cells"
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: fsl,lynx-28g
+    then:
+      # Legacy case: parent is the PHY provider, cell encodes lane index
+      properties:
+        "#phy-cells":
+          const: 1
+      required:
+        - "#phy-cells"
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - fsl,lx2160a-serdes1
+              - fsl,lx2160a-serdes2
+              - fsl,lx2160a-serdes3
+              - fsl,lx2162a-serdes1
+              - fsl,lx2162a-serdes2
+    then:
+      # Modern binding: lanes must have their own nodes
+      properties:
+        "#address-cells":
+          const: 1
+        "#size-cells":
+          const: 0
+      required:
+        - "#address-cells"
+        - "#size-cells"
+
+  # LX2162A SerDes 1 has fewer lanes than the others
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: fsl,lx2162a-serdes1
+    then:
+      patternProperties:
+        "^phy@[0-9a-f]+$":
+          properties:
+            reg:
+              enum: [4, 5, 6, 7]
+    else:
+      patternProperties:
+        "^phy@[0-9a-f]+$":
+          properties:
+            reg:
+              enum: [0, 1, 2, 3, 4, 5, 6, 7]
 
 additionalProperties: false
 
@@ -32,9 +134,52 @@ examples:
     soc {
       #address-cells = <2>;
       #size-cells = <2>;
-      serdes_1: phy@1ea0000 {
-        compatible = "fsl,lynx-28g";
+
+      serdes_1: serdes@1ea0000 {
+        compatible = "fsl,lx2160a-serdes1", "fsl,lynx-28g";
         reg = <0x0 0x1ea0000 0x0 0x1e30>;
+        #address-cells = <1>;
+        #size-cells = <0>;
         #phy-cells = <1>;
+
+        phy@0 {
+          reg = <0>;
+          #phy-cells = <0>;
+        };
+
+        phy@1 {
+          reg = <1>;
+          #phy-cells = <0>;
+        };
+
+        phy@2 {
+          reg = <2>;
+          #phy-cells = <0>;
+        };
+
+        phy@3 {
+          reg = <3>;
+          #phy-cells = <0>;
+        };
+
+        phy@4 {
+          reg = <4>;
+          #phy-cells = <0>;
+        };
+
+        phy@5 {
+          reg = <5>;
+          #phy-cells = <0>;
+        };
+
+        phy@6 {
+          reg = <6>;
+          #phy-cells = <0>;
+        };
+
+        phy@7 {
+          reg = <7>;
+          #phy-cells = <0>;
+        };
       };
     };
-- 
2.34.1

[PATCH v3 phy 13/17] phy: lynx-28g: probe on per-SoC and per-instance compatible strings

[Vladimir Oltean]

Add driver support for probing on the new, per-instance and per-SoC
bindings, which provide the following benefits:
- they allow rejecting unsupported protocols per lane (10GbE on SerDes 2
  lanes 0-5)
- individual lanes have their own OF nodes as PHY providers, which
  allows board device tree authors to tune electrical parameters such as
  TX amplitude, equalization etc.

Probing on "fsl,lynx-28g" is still supported, but the feature set is
frozen in time to just 1GbE and 10GbE (essentially the feature set as of
this change). However, we encourage the user at probe time to update the
device tree.

Refactor the per-lane logic from lynx_28g_probe() into
lynx_28g_lane_probe(), and call it from two distinct paths depending on
whether the modern or the legacy compatible string is used, with an OF
node for the lane or without.

Notable implication of the above: when lanes have disabled OF nodes, we
skip creating PHYs for them, and must also skip the CDR lock workaround.

lynx_28g_supports_lane_mode() was a SerDes-global function and now
becomes per lane, to reflect the specific capabilities each instance may
have.

Cc: Rob Herring <robh@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v2->v3:
- reword commit message
- add some comments regarding the "fsl,lynx-28g" fallback mechanism
- skip CDR lock workaround for lanes with no PHY (disabled in the device
  tree in the new binding)
v1->v2:
- remove priv->info->get_pccr() and priv->info->get_pcvt_offset().
  These were always called directly as lynx_28g_get_pccr() and
  lynx_28g_get_pcvt_offset().
- Add forgotten priv->info->lane_supports_mode() test to
  lynx_28g_supports_lane_mode().
- Rename the "fsl,lynx-28g" drvdata as lynx_info_compat rather than
  lynx_info_lx2160a_serdes1, to reflect its treatment as less featured.
- Implement a separate lane probing path for the #phy-cells = <0> case.

 drivers/phy/freescale/phy-fsl-lynx-28g.c | 202 ++++++++++++++++++++---
 1 file changed, 179 insertions(+), 23 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
index 453e76e0a6b7..1ddca8b4de17 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
@@ -433,9 +433,15 @@ struct lynx_28g_lane {
 	enum lynx_lane_mode mode;
 };
 
+struct lynx_info {
+	bool (*lane_supports_mode)(int lane, enum lynx_lane_mode mode);
+	int first_lane;
+};
+
 struct lynx_28g_priv {
 	void __iomem *base;
 	struct device *dev;
+	const struct lynx_info *info;
 	/* Serialize concurrent access to registers shared between lanes,
 	 * like PCCn
 	 */
@@ -500,11 +506,18 @@ static enum lynx_lane_mode phy_interface_to_lane_mode(phy_interface_t intf)
 	}
 }
 
-static bool lynx_28g_supports_lane_mode(struct lynx_28g_priv *priv,
+/* A lane mode is supported if we have a PLL that can provide its required
+ * clock net, and if there is a protocol converter for that mode on that lane.
+ */
+static bool lynx_28g_supports_lane_mode(struct lynx_28g_lane *lane,
 					enum lynx_lane_mode mode)
 {
+	struct lynx_28g_priv *priv = lane->priv;
 	int i;
 
+	if (!priv->info->lane_supports_mode(lane->id, mode))
+		return false;
+
 	for (i = 0; i < LYNX_28G_NUM_PLL; i++) {
 		if (PLLnRSTCTL_DIS(priv->pll[i].rstctl))
 			continue;
@@ -687,6 +700,86 @@ static int lynx_28g_get_pcvt_offset(int lane, enum lynx_lane_mode lane_mode)
 	}
 }
 
+static bool lx2160a_serdes1_lane_supports_mode(int lane,
+					       enum lynx_lane_mode mode)
+{
+	return true;
+}
+
+static bool lx2160a_serdes2_lane_supports_mode(int lane,
+					       enum lynx_lane_mode mode)
+{
+	switch (mode) {
+	case LANE_MODE_1000BASEX_SGMII:
+		return true;
+	case LANE_MODE_USXGMII:
+	case LANE_MODE_10GBASER:
+		return lane == 6 || lane == 7;
+	default:
+		return false;
+	}
+}
+
+static bool lx2160a_serdes3_lane_supports_mode(int lane,
+					       enum lynx_lane_mode mode)
+{
+	/*
+	 * Non-networking SerDes, and this driver supports only
+	 * networking protocols
+	 */
+	return false;
+}
+
+static bool lx2162a_serdes1_lane_supports_mode(int lane,
+					       enum lynx_lane_mode mode)
+{
+	return true;
+}
+
+static bool lx2162a_serdes2_lane_supports_mode(int lane,
+					       enum lynx_lane_mode mode)
+{
+	return lx2160a_serdes2_lane_supports_mode(lane, mode);
+}
+
+static bool lynx_28g_compat_lane_supports_mode(int lane,
+					       enum lynx_lane_mode mode)
+{
+	switch (mode) {
+	case LANE_MODE_1000BASEX_SGMII:
+	case LANE_MODE_USXGMII:
+	case LANE_MODE_10GBASER:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct lynx_info lynx_info_compat = {
+	.lane_supports_mode = lynx_28g_compat_lane_supports_mode,
+};
+
+static const struct lynx_info lynx_info_lx2160a_serdes1 = {
+	.lane_supports_mode = lx2160a_serdes1_lane_supports_mode,
+};
+
+static const struct lynx_info lynx_info_lx2160a_serdes2 = {
+	.lane_supports_mode = lx2160a_serdes2_lane_supports_mode,
+};
+
+static const struct lynx_info lynx_info_lx2160a_serdes3 = {
+	.lane_supports_mode = lx2160a_serdes3_lane_supports_mode,
+};
+
+static const struct lynx_info lynx_info_lx2162a_serdes1 = {
+	.lane_supports_mode = lx2162a_serdes1_lane_supports_mode,
+	.first_lane = 4,
+};
+
+static const struct lynx_info lynx_info_lx2162a_serdes2 = {
+	.lane_supports_mode = lx2162a_serdes2_lane_supports_mode,
+};
+
 static int lynx_pccr_read(struct lynx_28g_lane *lane, enum lynx_lane_mode mode,
 			  u32 *val)
 {
@@ -939,7 +1032,6 @@ static int lynx_28g_lane_enable_pcvt(struct lynx_28g_lane *lane,
 static int lynx_28g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 {
 	struct lynx_28g_lane *lane = phy_get_drvdata(phy);
-	struct lynx_28g_priv *priv = lane->priv;
 	int powered_up = lane->powered_up;
 	enum lynx_lane_mode lane_mode;
 	int err = 0;
@@ -951,7 +1043,7 @@ static int lynx_28g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 		return -EOPNOTSUPP;
 
 	lane_mode = phy_interface_to_lane_mode(submode);
-	if (!lynx_28g_supports_lane_mode(priv, lane_mode))
+	if (!lynx_28g_supports_lane_mode(lane, lane_mode))
 		return -EOPNOTSUPP;
 
 	if (lane_mode == lane->mode)
@@ -984,14 +1076,13 @@ static int lynx_28g_validate(struct phy *phy, enum phy_mode mode, int submode,
 			     union phy_configure_opts *opts __always_unused)
 {
 	struct lynx_28g_lane *lane = phy_get_drvdata(phy);
-	struct lynx_28g_priv *priv = lane->priv;
 	enum lynx_lane_mode lane_mode;
 
 	if (mode != PHY_MODE_ETHERNET)
 		return -EOPNOTSUPP;
 
 	lane_mode = phy_interface_to_lane_mode(submode);
-	if (!lynx_28g_supports_lane_mode(priv, lane_mode))
+	if (!lynx_28g_supports_lane_mode(lane, lane_mode))
 		return -EOPNOTSUPP;
 
 	return 0;
@@ -1067,8 +1158,10 @@ static void lynx_28g_cdr_lock_check(struct work_struct *work)
 	u32 rrstctl;
 	int i;
 
-	for (i = 0; i < LYNX_28G_NUM_LANE; i++) {
+	for (i = priv->info->first_lane; i < LYNX_28G_NUM_LANE; i++) {
 		lane = &priv->lane[i];
+		if (!lane->phy)
+			continue;
 
 		mutex_lock(&lane->phy->mutex);
 
@@ -1120,24 +1213,48 @@ static struct phy *lynx_28g_xlate(struct device *dev,
 	struct lynx_28g_priv *priv = dev_get_drvdata(dev);
 	int idx = args->args[0];
 
-	if (WARN_ON(idx >= LYNX_28G_NUM_LANE))
+	if (WARN_ON(idx >= LYNX_28G_NUM_LANE ||
+		    idx < priv->info->first_lane))
 		return ERR_PTR(-EINVAL);
 
 	return priv->lane[idx].phy;
 }
 
+static int lynx_28g_probe_lane(struct lynx_28g_priv *priv, int id,
+			       struct device_node *dn)
+{
+	struct lynx_28g_lane *lane = &priv->lane[id];
+	struct phy *phy;
+
+	memset(lane, 0, sizeof(*lane));
+
+	phy = devm_phy_create(priv->dev, dn, &lynx_28g_ops);
+	if (IS_ERR(phy))
+		return PTR_ERR(phy);
+
+	lane->priv = priv;
+	lane->phy = phy;
+	lane->id = id;
+	phy_set_drvdata(phy, lane);
+	lynx_28g_lane_read_configuration(lane);
+
+	return 0;
+}
+
 static int lynx_28g_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	bool lane_phy_providers = true;
 	struct phy_provider *provider;
 	struct lynx_28g_priv *priv;
-	int i;
+	int err;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
 	priv->dev = dev;
+	priv->info = of_device_get_match_data(dev);
 	dev_set_drvdata(dev, priv);
 	spin_lock_init(&priv->pcc_lock);
 	INIT_DELAYED_WORK(&priv->cdr_check, lynx_28g_cdr_lock_check);
@@ -1146,26 +1263,60 @@ static int lynx_28g_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
 
-	lynx_28g_pll_read_configuration(priv);
+	if (priv->info == &lynx_info_compat) {
+		/*
+		 * If we get here it means we probed on a device tree where
+		 * "fsl,lynx-28g" wasn't the fallback, but the sole compatible
+		 * string.
+		 */
+		dev_warn(dev, "Please update device tree to use per-device compatible strings\n");
+		lane_phy_providers = false;
+	}
 
-	for (i = 0; i < LYNX_28G_NUM_LANE; i++) {
-		struct lynx_28g_lane *lane = &priv->lane[i];
-		struct phy *phy;
+	lynx_28g_pll_read_configuration(priv);
 
-		memset(lane, 0, sizeof(*lane));
+	if (lane_phy_providers) {
+		struct device_node *dn = dev_of_node(dev), *child;
+
+		for_each_available_child_of_node(dn, child) {
+			u32 reg;
+
+			/* PHY subnode name must be 'phy'. */
+			if (!(of_node_name_eq(child, "phy")))
+				continue;
+
+			if (of_property_read_u32(child, "reg", &reg)) {
+				dev_err(dev, "No \"reg\" property for %pOF\n", child);
+				of_node_put(child);
+				return -EINVAL;
+			}
+
+			if (reg < priv->info->first_lane || reg >= LYNX_28G_NUM_LANE) {
+				dev_err(dev, "\"reg\" property out of range for %pOF\n", child);
+				of_node_put(child);
+				return -EINVAL;
+			}
+
+			err = lynx_28g_probe_lane(priv, reg, child);
+			if (err) {
+				of_node_put(child);
+				return err;
+			}
+		}
 
-		phy = devm_phy_create(dev, NULL, &lynx_28g_ops);
-		if (IS_ERR(phy))
-			return PTR_ERR(phy);
+		provider = devm_of_phy_provider_register(&pdev->dev,
+							 of_phy_simple_xlate);
+	} else {
+		for (int i = priv->info->first_lane; i < LYNX_28G_NUM_LANE; i++) {
+			err = lynx_28g_probe_lane(priv, i, NULL);
+			if (err)
+				return err;
+		}
 
-		lane->priv = priv;
-		lane->phy = phy;
-		lane->id = i;
-		phy_set_drvdata(phy, lane);
-		lynx_28g_lane_read_configuration(lane);
+		provider = devm_of_phy_provider_register(&pdev->dev,
+							 lynx_28g_xlate);
 	}
 
-	provider = devm_of_phy_provider_register(dev, lynx_28g_xlate);
 	if (IS_ERR(provider))
 		return PTR_ERR(provider);
 
@@ -1184,7 +1335,12 @@ static void lynx_28g_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id lynx_28g_of_match_table[] = {
-	{ .compatible = "fsl,lynx-28g" },
+	{ .compatible = "fsl,lx2160a-serdes1", .data = &lynx_info_lx2160a_serdes1 },
+	{ .compatible = "fsl,lx2160a-serdes2", .data = &lynx_info_lx2160a_serdes2 },
+	{ .compatible = "fsl,lx2160a-serdes3", .data = &lynx_info_lx2160a_serdes3 },
+	{ .compatible = "fsl,lx2162a-serdes1", .data = &lynx_info_lx2162a_serdes1 },
+	{ .compatible = "fsl,lx2162a-serdes2", .data = &lynx_info_lx2162a_serdes2 },
+	{ .compatible = "fsl,lynx-28g", .data = &lynx_info_compat }, /* fallback, keep last */
 	{ },
 };
 MODULE_DEVICE_TABLE(of, lynx_28g_of_match_table);
-- 
2.34.1

Re: [PATCH v3 phy 12/17] dt-bindings: phy: lynx-28g: add compatible strings per SerDes and instantiation

[Vladimir Oltean]

On Fri, Sep 26, 2025 at 09:05:00PM +0300, Vladimir Oltean wrote:
> Going by the generic "fsl,lynx-28g" compatible string and expecting all
> SerDes instantiations on all SoCs to use it was a mistake.
> 
> They all share the same register map, sure, but the number of protocol
> converters and lanes which are instantiated differs in a way that isn't
> detectable by the programming interface.
> 
> Using a separate compatible string per SerDes instantiation is
> sufficient for any device driver to distinguish these features and/or
> any instance-specific quirk. It also reflects how the SoC reference
> manual provides different tables with protocol combinations for each
> SerDes. NXP clearly documents these as not identical, and refers to them
> as such (SerDes 1, 2, etc).
> 
> The other sufficient approach for Lynx 28G would be to list in the
> device tree all protocols supported by each lane. That would be
> insufficient for the very similar Lynx 10G SerDes however, for which
> there exists a higher degree of variability in the PCCR register values
> that need to be written per protocol. This attempt can be seen in this
> unmerged patch set for Lynx 10G:
> https://lore.kernel.org/linux-phy/20230413160607.4128315-3-sean.anderson@seco.com/
> 
> but that approach is more drawn-out and more prone to errors, whereas
> this one is more succinct and obviously correct.
> 
> One aspect which is different with the per-SoC compatible strings is
> that they have one PHY provider for each lane (and #phy-cells = <0> in
> lane sub-nodes), rather than "fsl,lynx-28g" which has a single PHY
> provider for all lanes (and #phy-cells = <1> in the top-level node).
> 
> This is done to fulfill Josua Mayer's request:
> https://lore.kernel.org/lkml/02270f62-9334-400c-b7b9-7e6a44dbbfc9@solid-run.com/
> to have OF nodes for each lane, so that we can further apply schemas
> such as Documentation/devicetree/bindings/phy/transmit-amplitude.yaml
> individually.
> 
> This is the easiest and most intuitive way to describe that. The above
> is not the only electrical tuning that needs to be done, but rather the
> only one currently standardized in a schema. TX equalization parameters
> are TBD, but we need to not limit ourselves to just what currently exists.
> 
> Luckily, we can overlap the modern binding format over the legacy one
> and they can coexist without interfering. Old kernels use compatible =
> "fsl,lynx-28g" and the top-level PHY provider, whereas new kernels probe
> on e.g. compatible = "fsl,lx2160a-serdes1" and use the per-lane PHY
> providers.
> 
> Overlaying modern on top of legacy is only necessary for SerDes 1 and 2.
> LX2160A SerDes #3 (a non-networking SerDes) is not yet present in any
> device trees in circulation, and will only have the device-specific
> compatible (even though it shares the Lynx 28G programming model,
> specifying the "fsl,lynx-28g" compatible string for it provides no
> benefit that I can see).
> 
> Change the expected name of the top-level node to "serdes", and update
> the example too.
> 
> Cc: Rob Herring <robh@kernel.org>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> v2->v3:
> - re-add "fsl,lynx-28g" as fallback compatible, and #phy-cells = <1> in
>   top-level "serdes" node
> - drop useless description texts
> - fix text formatting
> - schema is more lax to allow overlaying old and new required properties
> v1->v2:
> - drop the usage of "fsl,lynx-28g" as a fallback compatible
> - mark "fsl,lynx-28g" as deprecated
> - implement Josua's request for per-lane OF nodes for the new compatible
>   strings
> 
>  .../devicetree/bindings/phy/fsl,lynx-28g.yaml | 159 +++++++++++++++++-
>  1 file changed, 152 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml b/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml
> index ff9f9ca0f19c..e8b3a48b9515 100644
> --- a/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml
> +++ b/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml
> @@ -9,21 +9,123 @@ title: Freescale Lynx 28G SerDes PHY
>  maintainers:
>    - Ioana Ciornei <ioana.ciornei@nxp.com>
>  
> +description:
> +  The Lynx 28G is a multi-lane, multi-protocol SerDes (PCIe, SATA, Ethernet)
> +  present in multiple instances on NXP LX2160A and LX2162A SoCs. All instances
> +  share a common register map and programming model, however they differ in
> +  supported protocols per lane in a way that is not detectable by said
> +  programming model without prior knowledge. The distinction is made through
> +  the compatible string.
> +
>  properties:
>    compatible:
> -    enum:
> -      - fsl,lynx-28g
> +    oneOf:
> +      - const: fsl,lynx-28g
> +        deprecated: true
> +        description:
> +          Legacy compatibility string for Lynx 28G SerDes. Any assumption
> +          regarding whether a certain lane supports a certain protocol may
> +          be incorrect. Deprecated except when used as a fallback. Use
> +          device-specific strings instead.
> +      - items:
> +          - const: fsl,lx2160a-serdes1
> +          - const: fsl,lynx-28g
> +      - items:
> +          - const: fsl,lx2160a-serdes2
> +          - const: fsl,lynx-28g
> +      - items:
> +          - const: fsl,lx2162a-serdes1
> +          - const: fsl,lynx-28g
> +      - items:
> +          - const: fsl,lx2162a-serdes2
> +          - const: fsl,lynx-28g
> +      - const: fsl,lx2160a-serdes3
>  
>    reg:
>      maxItems: 1
>  
> -  "#phy-cells":
> -    const: 1
> +  "#address-cells": true
> +
> +  "#size-cells": true
> +
> +  "#phy-cells": true
> +
> +patternProperties:
> +  "^phy@[0-9a-f]+$":
> +    type: object
> +    description: Individual SerDes lane acting as PHY provider
> +
> +    properties:
> +      reg:
> +        description: Lane index as seen in register map
> +        maxItems: 1
> +
> +      "#phy-cells":
> +        const: 0
> +
> +    required:
> +      - reg
> +      - "#phy-cells"
> +
> +    additionalProperties: false
>  
>  required:
>    - compatible
>    - reg
> -  - "#phy-cells"
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: fsl,lynx-28g
> +    then:
> +      # Legacy case: parent is the PHY provider, cell encodes lane index
> +      properties:
> +        "#phy-cells":
> +          const: 1
> +      required:
> +        - "#phy-cells"
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - fsl,lx2160a-serdes1
> +              - fsl,lx2160a-serdes2
> +              - fsl,lx2160a-serdes3
> +              - fsl,lx2162a-serdes1
> +              - fsl,lx2162a-serdes2
> +    then:
> +      # Modern binding: lanes must have their own nodes
> +      properties:
> +        "#address-cells":
> +          const: 1
> +        "#size-cells":
> +          const: 0
> +      required:
> +        - "#address-cells"
> +        - "#size-cells"
> +
> +  # LX2162A SerDes 1 has fewer lanes than the others
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: fsl,lx2162a-serdes1
> +    then:
> +      patternProperties:
> +        "^phy@[0-9a-f]+$":
> +          properties:
> +            reg:
> +              enum: [4, 5, 6, 7]
> +    else:
> +      patternProperties:
> +        "^phy@[0-9a-f]+$":
> +          properties:
> +            reg:
> +              enum: [0, 1, 2, 3, 4, 5, 6, 7]
>  
>  additionalProperties: false
>  
> @@ -32,9 +134,52 @@ examples:
>      soc {
>        #address-cells = <2>;
>        #size-cells = <2>;
> -      serdes_1: phy@1ea0000 {
> -        compatible = "fsl,lynx-28g";
> +
> +      serdes_1: serdes@1ea0000 {
> +        compatible = "fsl,lx2160a-serdes1", "fsl,lynx-28g";
>          reg = <0x0 0x1ea0000 0x0 0x1e30>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
>          #phy-cells = <1>;
> +
> +        phy@0 {
> +          reg = <0>;
> +          #phy-cells = <0>;
> +        };
> +
> +        phy@1 {
> +          reg = <1>;
> +          #phy-cells = <0>;
> +        };
> +
> +        phy@2 {
> +          reg = <2>;
> +          #phy-cells = <0>;
> +        };
> +
> +        phy@3 {
> +          reg = <3>;
> +          #phy-cells = <0>;
> +        };
> +
> +        phy@4 {
> +          reg = <4>;
> +          #phy-cells = <0>;
> +        };
> +
> +        phy@5 {
> +          reg = <5>;
> +          #phy-cells = <0>;
> +        };
> +
> +        phy@6 {
> +          reg = <6>;
> +          #phy-cells = <0>;
> +        };
> +
> +        phy@7 {
> +          reg = <7>;
> +          #phy-cells = <0>;
> +        };
>        };
>      };
> -- 
> 2.34.1
>

I should have realized sooner when Rob/Josua requested the changes for
backwards schema compatibility in v2:
https://lore.kernel.org/lkml/20250925080317.2ocgybitliwddhcf@skbuf/
that despite our attempts to preserve compatibility with old kernels,
we actually fail to do that. Actually I should have documented my
earlier thought process better, where I already came to that conclusion,
but which I had forgotten when told that this could work...

The SerDes schema itself is technically backwards-compatible, but the
problem is with consumers, which aren't. In old device trees, they have:

	phys = <&serdes_1 0>;

and in new ones, they have:

	phys = <&serdes_1_lane_a>;

Because the consumer has a single "phys" phandle to the PHY provider, it
either has to point to one, or to the other. But on old kernels, we do
not register PHY providers per lane, so "phys = <&serdes_1_lane_a>"
results in a broken reference.

There are 2 directions to go from here:
1. Have optional per-lane "phy" OF node children, which exist solely for
   tuning electrical parameters. We need to keep the top-level SerDes as
   the only PHY provider, with #phy-cells = <1> denoting the lane.

2. Accept that keeping "fsl,lynx-28g" and overlaid properties has
   absolutely no practical benefit, and drop them (effectively returning
   to Conor's suggestion, as implemented in v2)

3. Extend the schema and the driver support for it as a backportable bug
   fix, to allow registering PHY providers for lanes with OF nodes in
   stable kernels. This avoids regressing when the device trees are
   updated, assuming the stable kernel is also updated.

It's not that I particularly like #2, but going with #1 would imply that
lane OF nodes exist, but the "phys" phandles do not point to them.

Combine that with the fact that anything we do with the 28G Lynx
bindings will have to be replicated, for uniformity's sake, with the
upcoming 10G Lynx SerDes binding (very similar hardware IP), and #1 is
suddenly not looking so pretty at all. I.e. introducing the 10G Lynx
bindings like the 28G Lynx ones would mean deviating from the widely
established norm, and introducing them like the widely established norm
would mean deviating from the 28G Lynx. I can easily see how someone
might look at them one day and think "hmm, can't we make them more
uniform?"

OTOH, the fact that device tree updates require kernel updates (as
implied by #2) is acceptably by everyone in this thread who expressed an
opinion on this topic.

As for option #3, while IMO it would be a justified "new feature as
bug fix", it sounds a bit counterintuitive and I'm afraid I won't manage
to convince all maintainers along the way that this is the way forward.

I'll wait for the merge window to close before reposting anything, but
I'd like an explicit ack from Rob and Josua in the meantime, whose
change request I'd be effectively reverting, to make sure that this
topic is closed.

Re: [PATCH v3 phy 12/17] dt-bindings: phy: lynx-28g: add compatible strings per SerDes and instantiation

[Rob Herring]

On Tue, Sep 30, 2025 at 05:07:35PM +0300, Vladimir Oltean wrote:
> On Fri, Sep 26, 2025 at 09:05:00PM +0300, Vladimir Oltean wrote:
> > Going by the generic "fsl,lynx-28g" compatible string and expecting all
> > SerDes instantiations on all SoCs to use it was a mistake.
> > 
> > They all share the same register map, sure, but the number of protocol
> > converters and lanes which are instantiated differs in a way that isn't
> > detectable by the programming interface.
> > 
> > Using a separate compatible string per SerDes instantiation is
> > sufficient for any device driver to distinguish these features and/or
> > any instance-specific quirk. It also reflects how the SoC reference
> > manual provides different tables with protocol combinations for each
> > SerDes. NXP clearly documents these as not identical, and refers to them
> > as such (SerDes 1, 2, etc).
> > 
> > The other sufficient approach for Lynx 28G would be to list in the
> > device tree all protocols supported by each lane. That would be
> > insufficient for the very similar Lynx 10G SerDes however, for which
> > there exists a higher degree of variability in the PCCR register values
> > that need to be written per protocol. This attempt can be seen in this
> > unmerged patch set for Lynx 10G:
> > https://lore.kernel.org/linux-phy/20230413160607.4128315-3-sean.anderson@seco.com/
> > 
> > but that approach is more drawn-out and more prone to errors, whereas
> > this one is more succinct and obviously correct.
> > 
> > One aspect which is different with the per-SoC compatible strings is
> > that they have one PHY provider for each lane (and #phy-cells = <0> in
> > lane sub-nodes), rather than "fsl,lynx-28g" which has a single PHY
> > provider for all lanes (and #phy-cells = <1> in the top-level node).
> > 
> > This is done to fulfill Josua Mayer's request:
> > https://lore.kernel.org/lkml/02270f62-9334-400c-b7b9-7e6a44dbbfc9@solid-run.com/
> > to have OF nodes for each lane, so that we can further apply schemas
> > such as Documentation/devicetree/bindings/phy/transmit-amplitude.yaml
> > individually.
> > 
> > This is the easiest and most intuitive way to describe that. The above
> > is not the only electrical tuning that needs to be done, but rather the
> > only one currently standardized in a schema. TX equalization parameters
> > are TBD, but we need to not limit ourselves to just what currently exists.
> > 
> > Luckily, we can overlap the modern binding format over the legacy one
> > and they can coexist without interfering. Old kernels use compatible =
> > "fsl,lynx-28g" and the top-level PHY provider, whereas new kernels probe
> > on e.g. compatible = "fsl,lx2160a-serdes1" and use the per-lane PHY
> > providers.
> > 
> > Overlaying modern on top of legacy is only necessary for SerDes 1 and 2.
> > LX2160A SerDes #3 (a non-networking SerDes) is not yet present in any
> > device trees in circulation, and will only have the device-specific
> > compatible (even though it shares the Lynx 28G programming model,
> > specifying the "fsl,lynx-28g" compatible string for it provides no
> > benefit that I can see).
> > 
> > Change the expected name of the top-level node to "serdes", and update
> > the example too.
> > 
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> > Cc: Conor Dooley <conor+dt@kernel.org>
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
> > v2->v3:
> > - re-add "fsl,lynx-28g" as fallback compatible, and #phy-cells = <1> in
> >   top-level "serdes" node
> > - drop useless description texts
> > - fix text formatting
> > - schema is more lax to allow overlaying old and new required properties
> > v1->v2:
> > - drop the usage of "fsl,lynx-28g" as a fallback compatible
> > - mark "fsl,lynx-28g" as deprecated
> > - implement Josua's request for per-lane OF nodes for the new compatible
> >   strings
> > 
> >  .../devicetree/bindings/phy/fsl,lynx-28g.yaml | 159 +++++++++++++++++-
> >  1 file changed, 152 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml b/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml
> > index ff9f9ca0f19c..e8b3a48b9515 100644
> > --- a/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml
> > +++ b/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml
> > @@ -9,21 +9,123 @@ title: Freescale Lynx 28G SerDes PHY
> >  maintainers:
> >    - Ioana Ciornei <ioana.ciornei@nxp.com>
> >  
> > +description:
> > +  The Lynx 28G is a multi-lane, multi-protocol SerDes (PCIe, SATA, Ethernet)
> > +  present in multiple instances on NXP LX2160A and LX2162A SoCs. All instances
> > +  share a common register map and programming model, however they differ in
> > +  supported protocols per lane in a way that is not detectable by said
> > +  programming model without prior knowledge. The distinction is made through
> > +  the compatible string.
> > +
> >  properties:
> >    compatible:
> > -    enum:
> > -      - fsl,lynx-28g
> > +    oneOf:
> > +      - const: fsl,lynx-28g
> > +        deprecated: true
> > +        description:
> > +          Legacy compatibility string for Lynx 28G SerDes. Any assumption
> > +          regarding whether a certain lane supports a certain protocol may
> > +          be incorrect. Deprecated except when used as a fallback. Use
> > +          device-specific strings instead.
> > +      - items:
> > +          - const: fsl,lx2160a-serdes1
> > +          - const: fsl,lynx-28g
> > +      - items:
> > +          - const: fsl,lx2160a-serdes2
> > +          - const: fsl,lynx-28g
> > +      - items:
> > +          - const: fsl,lx2162a-serdes1
> > +          - const: fsl,lynx-28g
> > +      - items:
> > +          - const: fsl,lx2162a-serdes2
> > +          - const: fsl,lynx-28g
> > +      - const: fsl,lx2160a-serdes3
> >  
> >    reg:
> >      maxItems: 1
> >  
> > -  "#phy-cells":
> > -    const: 1
> > +  "#address-cells": true
> > +
> > +  "#size-cells": true
> > +
> > +  "#phy-cells": true
> > +
> > +patternProperties:
> > +  "^phy@[0-9a-f]+$":
> > +    type: object
> > +    description: Individual SerDes lane acting as PHY provider
> > +
> > +    properties:
> > +      reg:
> > +        description: Lane index as seen in register map
> > +        maxItems: 1
> > +
> > +      "#phy-cells":
> > +        const: 0
> > +
> > +    required:
> > +      - reg
> > +      - "#phy-cells"
> > +
> > +    additionalProperties: false
> >  
> >  required:
> >    - compatible
> >    - reg
> > -  - "#phy-cells"
> > +
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: fsl,lynx-28g
> > +    then:
> > +      # Legacy case: parent is the PHY provider, cell encodes lane index
> > +      properties:
> > +        "#phy-cells":
> > +          const: 1
> > +      required:
> > +        - "#phy-cells"
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - fsl,lx2160a-serdes1
> > +              - fsl,lx2160a-serdes2
> > +              - fsl,lx2160a-serdes3
> > +              - fsl,lx2162a-serdes1
> > +              - fsl,lx2162a-serdes2
> > +    then:
> > +      # Modern binding: lanes must have their own nodes
> > +      properties:
> > +        "#address-cells":
> > +          const: 1
> > +        "#size-cells":
> > +          const: 0
> > +      required:
> > +        - "#address-cells"
> > +        - "#size-cells"
> > +
> > +  # LX2162A SerDes 1 has fewer lanes than the others
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: fsl,lx2162a-serdes1
> > +    then:
> > +      patternProperties:
> > +        "^phy@[0-9a-f]+$":
> > +          properties:
> > +            reg:
> > +              enum: [4, 5, 6, 7]
> > +    else:
> > +      patternProperties:
> > +        "^phy@[0-9a-f]+$":
> > +          properties:
> > +            reg:
> > +              enum: [0, 1, 2, 3, 4, 5, 6, 7]
> >  
> >  additionalProperties: false
> >  
> > @@ -32,9 +134,52 @@ examples:
> >      soc {
> >        #address-cells = <2>;
> >        #size-cells = <2>;
> > -      serdes_1: phy@1ea0000 {
> > -        compatible = "fsl,lynx-28g";
> > +
> > +      serdes_1: serdes@1ea0000 {
> > +        compatible = "fsl,lx2160a-serdes1", "fsl,lynx-28g";
> >          reg = <0x0 0x1ea0000 0x0 0x1e30>;
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> >          #phy-cells = <1>;
> > +
> > +        phy@0 {
> > +          reg = <0>;
> > +          #phy-cells = <0>;
> > +        };
> > +
> > +        phy@1 {
> > +          reg = <1>;
> > +          #phy-cells = <0>;
> > +        };
> > +
> > +        phy@2 {
> > +          reg = <2>;
> > +          #phy-cells = <0>;
> > +        };
> > +
> > +        phy@3 {
> > +          reg = <3>;
> > +          #phy-cells = <0>;
> > +        };
> > +
> > +        phy@4 {
> > +          reg = <4>;
> > +          #phy-cells = <0>;
> > +        };
> > +
> > +        phy@5 {
> > +          reg = <5>;
> > +          #phy-cells = <0>;
> > +        };
> > +
> > +        phy@6 {
> > +          reg = <6>;
> > +          #phy-cells = <0>;
> > +        };
> > +
> > +        phy@7 {
> > +          reg = <7>;
> > +          #phy-cells = <0>;
> > +        };
> >        };
> >      };
> > -- 
> > 2.34.1
> >
> 
> I should have realized sooner when Rob/Josua requested the changes for
> backwards schema compatibility in v2:
> https://lore.kernel.org/lkml/20250925080317.2ocgybitliwddhcf@skbuf/
> that despite our attempts to preserve compatibility with old kernels,
> we actually fail to do that. Actually I should have documented my
> earlier thought process better, where I already came to that conclusion,
> but which I had forgotten when told that this could work...
> 
> The SerDes schema itself is technically backwards-compatible, but the
> problem is with consumers, which aren't. In old device trees, they have:
> 
> 	phys = <&serdes_1 0>;
> 
> and in new ones, they have:
> 
> 	phys = <&serdes_1_lane_a>;
> 
> Because the consumer has a single "phys" phandle to the PHY provider, it
> either has to point to one, or to the other. But on old kernels, we do
> not register PHY providers per lane, so "phys = <&serdes_1_lane_a>"
> results in a broken reference.
> 
> There are 2 directions to go from here:
> 1. Have optional per-lane "phy" OF node children, which exist solely for
>    tuning electrical parameters. We need to keep the top-level SerDes as
>    the only PHY provider, with #phy-cells = <1> denoting the lane.
> 
> 2. Accept that keeping "fsl,lynx-28g" and overlaid properties has
>    absolutely no practical benefit, and drop them (effectively returning
>    to Conor's suggestion, as implemented in v2)
> 
> 3. Extend the schema and the driver support for it as a backportable bug
>    fix, to allow registering PHY providers for lanes with OF nodes in
>    stable kernels. This avoids regressing when the device trees are
>    updated, assuming the stable kernel is also updated.
> 
> It's not that I particularly like #2, but going with #1 would imply that
> lane OF nodes exist, but the "phys" phandles do not point to them.
> 
> Combine that with the fact that anything we do with the 28G Lynx
> bindings will have to be replicated, for uniformity's sake, with the
> upcoming 10G Lynx SerDes binding (very similar hardware IP), and #1 is
> suddenly not looking so pretty at all. I.e. introducing the 10G Lynx
> bindings like the 28G Lynx ones would mean deviating from the widely
> established norm, and introducing them like the widely established norm
> would mean deviating from the 28G Lynx. I can easily see how someone
> might look at them one day and think "hmm, can't we make them more
> uniform?"
> 
> OTOH, the fact that device tree updates require kernel updates (as
> implied by #2) is acceptably by everyone in this thread who expressed an
> opinion on this topic.
> 
> As for option #3, while IMO it would be a justified "new feature as
> bug fix", it sounds a bit counterintuitive and I'm afraid I won't manage
> to convince all maintainers along the way that this is the way forward.
> 
> I'll wait for the merge window to close before reposting anything, but
> I'd like an explicit ack from Rob and Josua in the meantime, whose
> change request I'd be effectively reverting, to make sure that this
> topic is closed.

If #2 is not going to upset anyone (that their device stopped working), 
then that route is fine.

Rob

Re: [PATCH v3 phy 12/17] dt-bindings: phy: lynx-28g: add compatible strings per SerDes and instantiation

[Josua Mayer]

Am 02.10.25 um 05:03 schrieb Rob Herring:

> On Tue, Sep 30, 2025 at 05:07:35PM +0300, Vladimir Oltean wrote:
>> On Fri, Sep 26, 2025 at 09:05:00PM +0300, Vladimir Oltean wrote:
>>> Going by the generic "fsl,lynx-28g" compatible string and expecting all
>>> SerDes instantiations on all SoCs to use it was a mistake.
>>>
>>> They all share the same register map, sure, but the number of protocol
>>> converters and lanes which are instantiated differs in a way that isn't
>>> detectable by the programming interface.
>>>
>>> Using a separate compatible string per SerDes instantiation is
>>> sufficient for any device driver to distinguish these features and/or
>>> any instance-specific quirk. It also reflects how the SoC reference
>>> manual provides different tables with protocol combinations for each
>>> SerDes. NXP clearly documents these as not identical, and refers to them
>>> as such (SerDes 1, 2, etc).
>>>
>>> The other sufficient approach for Lynx 28G would be to list in the
>>> device tree all protocols supported by each lane. That would be
>>> insufficient for the very similar Lynx 10G SerDes however, for which
>>> there exists a higher degree of variability in the PCCR register values
>>> that need to be written per protocol. This attempt can be seen in this
>>> unmerged patch set for Lynx 10G:
>>> https://lore.kernel.org/linux-phy/20230413160607.4128315-3-sean.anderson@seco.com/
>>>
>>> but that approach is more drawn-out and more prone to errors, whereas
>>> this one is more succinct and obviously correct.
>>>
>>> One aspect which is different with the per-SoC compatible strings is
>>> that they have one PHY provider for each lane (and #phy-cells = <0> in
>>> lane sub-nodes), rather than "fsl,lynx-28g" which has a single PHY
>>> provider for all lanes (and #phy-cells = <1> in the top-level node).
>>>
>>> This is done to fulfill Josua Mayer's request:
>>> https://lore.kernel.org/lkml/02270f62-9334-400c-b7b9-7e6a44dbbfc9@solid-run.com/
>>> to have OF nodes for each lane, so that we can further apply schemas
>>> such as Documentation/devicetree/bindings/phy/transmit-amplitude.yaml
>>> individually.
>>>
>>> This is the easiest and most intuitive way to describe that. The above
>>> is not the only electrical tuning that needs to be done, but rather the
>>> only one currently standardized in a schema. TX equalization parameters
>>> are TBD, but we need to not limit ourselves to just what currently exists.
>>>
>>> Luckily, we can overlap the modern binding format over the legacy one
>>> and they can coexist without interfering. Old kernels use compatible =
>>> "fsl,lynx-28g" and the top-level PHY provider, whereas new kernels probe
>>> on e.g. compatible = "fsl,lx2160a-serdes1" and use the per-lane PHY
>>> providers.
>>>
>>> Overlaying modern on top of legacy is only necessary for SerDes 1 and 2.
>>> LX2160A SerDes #3 (a non-networking SerDes) is not yet present in any
>>> device trees in circulation, and will only have the device-specific
>>> compatible (even though it shares the Lynx 28G programming model,
>>> specifying the "fsl,lynx-28g" compatible string for it provides no
>>> benefit that I can see).
>>>
>>> Change the expected name of the top-level node to "serdes", and update
>>> the example too.
>>>
>>> Cc: Rob Herring <robh@kernel.org>
>>> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
>>> Cc: Conor Dooley <conor+dt@kernel.org>
>>> Cc: devicetree@vger.kernel.org
>>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>>> ---
>>> v2->v3:
>>> - re-add "fsl,lynx-28g" as fallback compatible, and #phy-cells = <1> in
>>>   top-level "serdes" node
>>> - drop useless description texts
>>> - fix text formatting
>>> - schema is more lax to allow overlaying old and new required properties
>>> v1->v2:
>>> - drop the usage of "fsl,lynx-28g" as a fallback compatible
>>> - mark "fsl,lynx-28g" as deprecated
>>> - implement Josua's request for per-lane OF nodes for the new compatible
>>>   strings
>>>
>>>  .../devicetree/bindings/phy/fsl,lynx-28g.yaml | 159 +++++++++++++++++-
>>>  1 file changed, 152 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml b/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml
>>> index ff9f9ca0f19c..e8b3a48b9515 100644
>>> --- a/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml
>>> +++ b/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml
>>> @@ -9,21 +9,123 @@ title: Freescale Lynx 28G SerDes PHY
>>>  maintainers:
>>>    - Ioana Ciornei <ioana.ciornei@nxp.com>
>>>  
>>> +description:
>>> +  The Lynx 28G is a multi-lane, multi-protocol SerDes (PCIe, SATA, Ethernet)
>>> +  present in multiple instances on NXP LX2160A and LX2162A SoCs. All instances
>>> +  share a common register map and programming model, however they differ in
>>> +  supported protocols per lane in a way that is not detectable by said
>>> +  programming model without prior knowledge. The distinction is made through
>>> +  the compatible string.
>>> +
>>>  properties:
>>>    compatible:
>>> -    enum:
>>> -      - fsl,lynx-28g
>>> +    oneOf:
>>> +      - const: fsl,lynx-28g
>>> +        deprecated: true
>>> +        description:
>>> +          Legacy compatibility string for Lynx 28G SerDes. Any assumption
>>> +          regarding whether a certain lane supports a certain protocol may
>>> +          be incorrect. Deprecated except when used as a fallback. Use
>>> +          device-specific strings instead.
>>> +      - items:
>>> +          - const: fsl,lx2160a-serdes1
>>> +          - const: fsl,lynx-28g
>>> +      - items:
>>> +          - const: fsl,lx2160a-serdes2
>>> +          - const: fsl,lynx-28g
>>> +      - items:
>>> +          - const: fsl,lx2162a-serdes1
>>> +          - const: fsl,lynx-28g
>>> +      - items:
>>> +          - const: fsl,lx2162a-serdes2
>>> +          - const: fsl,lynx-28g
>>> +      - const: fsl,lx2160a-serdes3
>>>  
>>>    reg:
>>>      maxItems: 1
>>>  
>>> -  "#phy-cells":
>>> -    const: 1
>>> +  "#address-cells": true
>>> +
>>> +  "#size-cells": true
>>> +
>>> +  "#phy-cells": true
>>> +
>>> +patternProperties:
>>> +  "^phy@[0-9a-f]+$":
>>> +    type: object
>>> +    description: Individual SerDes lane acting as PHY provider
>>> +
>>> +    properties:
>>> +      reg:
>>> +        description: Lane index as seen in register map
>>> +        maxItems: 1
>>> +
>>> +      "#phy-cells":
>>> +        const: 0
>>> +
>>> +    required:
>>> +      - reg
>>> +      - "#phy-cells"
>>> +
>>> +    additionalProperties: false
>>>  
>>>  required:
>>>    - compatible
>>>    - reg
>>> -  - "#phy-cells"
>>> +
>>> +allOf:
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: fsl,lynx-28g
>>> +    then:
>>> +      # Legacy case: parent is the PHY provider, cell encodes lane index
>>> +      properties:
>>> +        "#phy-cells":
>>> +          const: 1
>>> +      required:
>>> +        - "#phy-cells"
>>> +
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - fsl,lx2160a-serdes1
>>> +              - fsl,lx2160a-serdes2
>>> +              - fsl,lx2160a-serdes3
>>> +              - fsl,lx2162a-serdes1
>>> +              - fsl,lx2162a-serdes2
>>> +    then:
>>> +      # Modern binding: lanes must have their own nodes
>>> +      properties:
>>> +        "#address-cells":
>>> +          const: 1
>>> +        "#size-cells":
>>> +          const: 0
>>> +      required:
>>> +        - "#address-cells"
>>> +        - "#size-cells"
>>> +
>>> +  # LX2162A SerDes 1 has fewer lanes than the others
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: fsl,lx2162a-serdes1
>>> +    then:
>>> +      patternProperties:
>>> +        "^phy@[0-9a-f]+$":
>>> +          properties:
>>> +            reg:
>>> +              enum: [4, 5, 6, 7]
>>> +    else:
>>> +      patternProperties:
>>> +        "^phy@[0-9a-f]+$":
>>> +          properties:
>>> +            reg:
>>> +              enum: [0, 1, 2, 3, 4, 5, 6, 7]
>>>  
>>>  additionalProperties: false
>>>  
>>> @@ -32,9 +134,52 @@ examples:
>>>      soc {
>>>        #address-cells = <2>;
>>>        #size-cells = <2>;
>>> -      serdes_1: phy@1ea0000 {
>>> -        compatible = "fsl,lynx-28g";
>>> +
>>> +      serdes_1: serdes@1ea0000 {
>>> +        compatible = "fsl,lx2160a-serdes1", "fsl,lynx-28g";
>>>          reg = <0x0 0x1ea0000 0x0 0x1e30>;
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>>          #phy-cells = <1>;
>>> +
>>> +        phy@0 {
>>> +          reg = <0>;
>>> +          #phy-cells = <0>;
>>> +        };
>>> +
>>> +        phy@1 {
>>> +          reg = <1>;
>>> +          #phy-cells = <0>;
>>> +        };
>>> +
>>> +        phy@2 {
>>> +          reg = <2>;
>>> +          #phy-cells = <0>;
>>> +        };
>>> +
>>> +        phy@3 {
>>> +          reg = <3>;
>>> +          #phy-cells = <0>;
>>> +        };
>>> +
>>> +        phy@4 {
>>> +          reg = <4>;
>>> +          #phy-cells = <0>;
>>> +        };
>>> +
>>> +        phy@5 {
>>> +          reg = <5>;
>>> +          #phy-cells = <0>;
>>> +        };
>>> +
>>> +        phy@6 {
>>> +          reg = <6>;
>>> +          #phy-cells = <0>;
>>> +        };
>>> +
>>> +        phy@7 {
>>> +          reg = <7>;
>>> +          #phy-cells = <0>;
>>> +        };
>>>        };
>>>      };
>>> -- 
>>> 2.34.1
>>>
>> I should have realized sooner when Rob/Josua requested the changes for
>> backwards schema compatibility in v2:
>> https://lore.kernel.org/lkml/20250925080317.2ocgybitliwddhcf@skbuf/
>> that despite our attempts to preserve compatibility with old kernels,
>> we actually fail to do that. Actually I should have documented my
>> earlier thought process better, where I already came to that conclusion,
>> but which I had forgotten when told that this could work...
>>
>> The SerDes schema itself is technically backwards-compatible, but the
>> problem is with consumers, which aren't. In old device trees, they have:
>>
>> 	phys = <&serdes_1 0>;
>>
>> and in new ones, they have:
>>
>> 	phys = <&serdes_1_lane_a>;
>>
>> Because the consumer has a single "phys" phandle to the PHY provider, it
>> either has to point to one, or to the other. But on old kernels, we do
>> not register PHY providers per lane, so "phys = <&serdes_1_lane_a>"
>> results in a broken reference.
I would not expect being able to use a new schema in an older kernel.
Maybe DT maintainers can confirm whether anyone expects that.
>>
>> There are 2 directions to go from here:
>> 1. Have optional per-lane "phy" OF node children, which exist solely for
>>    tuning electrical parameters. We need to keep the top-level SerDes as
>>    the only PHY provider, with #phy-cells = <1> denoting the lane.
>>
>> 2. Accept that keeping "fsl,lynx-28g" and overlaid properties has
>>    absolutely no practical benefit, and drop them (effectively returning
>>    to Conor's suggestion, as implemented in v2)
I don't quite understand how keeping or dropping fsl,lynx-28g compatible
impacts the ability to set phandles to the base and child nodes.

An updated driver can be made to handle both references,
and an older driver will only handle the one to the base node.

>>
>> 3. Extend the schema and the driver support for it as a backportable bug
>>    fix, to allow registering PHY providers for lanes with OF nodes in
>>    stable kernels. This avoids regressing when the device trees are
>>    updated, assuming the stable kernel is also updated.
I think whatever will be done with phy sub-nodes on the serdes block node
should not count as a fix to be backported in stable.
Any attempt to backport reference to &serdes_1_lane_a will produce a
compile-time error.
>>
>> It's not that I particularly like #2, but going with #1 would imply that
>> lane OF nodes exist, but the "phys" phandles do not point to them.
>>
>> Combine that with the fact that anything we do with the 28G Lynx
>> bindings will have to be replicated, for uniformity's sake, with the
>> upcoming 10G Lynx SerDes binding (very similar hardware IP), and #1 is
>> suddenly not looking so pretty at all. I.e. introducing the 10G Lynx
>> bindings like the 28G Lynx ones would mean deviating from the widely
>> established norm, and introducing them like the widely established norm
>> would mean deviating from the 28G Lynx. I can easily see how someone
>> might look at them one day and think "hmm, can't we make them more
>> uniform?"
>>
>> OTOH, the fact that device tree updates require kernel updates (as
>> implied by #2) is acceptably by everyone in this thread who expressed an
>> opinion on this topic.
>>
>> As for option #3, while IMO it would be a justified "new feature as
>> bug fix", it sounds a bit counterintuitive and I'm afraid I won't manage
>> to convince all maintainers along the way that this is the way forward.
>>
>> I'll wait for the merge window to close before reposting anything, but
>> I'd like an explicit ack from Rob and Josua in the meantime, whose
>> change request I'd be effectively reverting, to make sure that this
>> topic is closed.
> If #2 is not going to upset anyone (that their device stopped working), 
> then that route is fine.
>
> Rob

Re: [PATCH v3 phy 13/17] phy: lynx-28g: probe on per-SoC and per-instance compatible strings

[Josua Mayer]

Am 26.09.25 um 20:05 schrieb Vladimir Oltean:

> Add driver support for probing on the new, per-instance and per-SoC
> bindings, which provide the following benefits:
> - they allow rejecting unsupported protocols per lane (10GbE on SerDes 2
>   lanes 0-5)
> - individual lanes have their own OF nodes as PHY providers, which
>   allows board device tree authors to tune electrical parameters such as
>   TX amplitude, equalization etc.
>
> Probing on "fsl,lynx-28g" is still supported, but the feature set is
> frozen in time to just 1GbE and 10GbE (essentially the feature set as of
> this change). However, we encourage the user at probe time to update the
> device tree.
>
> Refactor the per-lane logic from lynx_28g_probe() into
> lynx_28g_lane_probe(), and call it from two distinct paths depending on
> whether the modern or the legacy compatible string is used, with an OF
> node for the lane or without.
>
> Notable implication of the above: when lanes have disabled OF nodes, we
> skip creating PHYs for them, and must also skip the CDR lock workaround.
>
> lynx_28g_supports_lane_mode() was a SerDes-global function and now
> becomes per lane, to reflect the specific capabilities each instance may
> have.
>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> v2->v3:
> - reword commit message
> - add some comments regarding the "fsl,lynx-28g" fallback mechanism
> - skip CDR lock workaround for lanes with no PHY (disabled in the device
>   tree in the new binding)
> v1->v2:
> - remove priv->info->get_pccr() and priv->info->get_pcvt_offset().
>   These were always called directly as lynx_28g_get_pccr() and
>   lynx_28g_get_pcvt_offset().
> - Add forgotten priv->info->lane_supports_mode() test to
>   lynx_28g_supports_lane_mode().
> - Rename the "fsl,lynx-28g" drvdata as lynx_info_compat rather than
>   lynx_info_lx2160a_serdes1, to reflect its treatment as less featured.
> - Implement a separate lane probing path for the #phy-cells = <0> case.
>
>  drivers/phy/freescale/phy-fsl-lynx-28g.c | 202 ++++++++++++++++++++---
>  1 file changed, 179 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
> index 453e76e0a6b7..1ddca8b4de17 100644
> --- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
> +++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
> @@ -433,9 +433,15 @@ struct lynx_28g_lane {
>  	enum lynx_lane_mode mode;
>  };
>  
> +struct lynx_info {
> +	bool (*lane_supports_mode)(int lane, enum lynx_lane_mode mode);
> +	int first_lane;
> +};
> +
>  struct lynx_28g_priv {
>  	void __iomem *base;
>  	struct device *dev;
> +	const struct lynx_info *info;
>  	/* Serialize concurrent access to registers shared between lanes,
>  	 * like PCCn
>  	 */
> @@ -500,11 +506,18 @@ static enum lynx_lane_mode phy_interface_to_lane_mode(phy_interface_t intf)
>  	}
>  }
>  
> -static bool lynx_28g_supports_lane_mode(struct lynx_28g_priv *priv,
> +/* A lane mode is supported if we have a PLL that can provide its required
> + * clock net, and if there is a protocol converter for that mode on that lane.
> + */
> +static bool lynx_28g_supports_lane_mode(struct lynx_28g_lane *lane,
>  					enum lynx_lane_mode mode)
>  {
> +	struct lynx_28g_priv *priv = lane->priv;
>  	int i;
>  
> +	if (!priv->info->lane_supports_mode(lane->id, mode))
> +		return false;
> +
>  	for (i = 0; i < LYNX_28G_NUM_PLL; i++) {
>  		if (PLLnRSTCTL_DIS(priv->pll[i].rstctl))
>  			continue;
> @@ -687,6 +700,86 @@ static int lynx_28g_get_pcvt_offset(int lane, enum lynx_lane_mode lane_mode)
>  	}
>  }
>  
> +static bool lx2160a_serdes1_lane_supports_mode(int lane,
> +					       enum lynx_lane_mode mode)
> +{
> +	return true;
> +}
> +
> +static bool lx2160a_serdes2_lane_supports_mode(int lane,
> +					       enum lynx_lane_mode mode)
> +{
> +	switch (mode) {
> +	case LANE_MODE_1000BASEX_SGMII:
> +		return true;
> +	case LANE_MODE_USXGMII:
> +	case LANE_MODE_10GBASER:
> +		return lane == 6 || lane == 7;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool lx2160a_serdes3_lane_supports_mode(int lane,
> +					       enum lynx_lane_mode mode)
> +{
> +	/*
> +	 * Non-networking SerDes, and this driver supports only
> +	 * networking protocols
> +	 */
> +	return false;
> +}
> +
> +static bool lx2162a_serdes1_lane_supports_mode(int lane,
> +					       enum lynx_lane_mode mode)
> +{
> +	return true;
> +}
> +
> +static bool lx2162a_serdes2_lane_supports_mode(int lane,
> +					       enum lynx_lane_mode mode)
> +{
> +	return lx2160a_serdes2_lane_supports_mode(lane, mode);
> +}
> +
> +static bool lynx_28g_compat_lane_supports_mode(int lane,
> +					       enum lynx_lane_mode mode)
> +{
> +	switch (mode) {
> +	case LANE_MODE_1000BASEX_SGMII:
> +	case LANE_MODE_USXGMII:
> +	case LANE_MODE_10GBASER:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static const struct lynx_info lynx_info_compat = {
> +	.lane_supports_mode = lynx_28g_compat_lane_supports_mode,
> +};
> +
> +static const struct lynx_info lynx_info_lx2160a_serdes1 = {
> +	.lane_supports_mode = lx2160a_serdes1_lane_supports_mode,
> +};
> +
> +static const struct lynx_info lynx_info_lx2160a_serdes2 = {
> +	.lane_supports_mode = lx2160a_serdes2_lane_supports_mode,
> +};
> +
> +static const struct lynx_info lynx_info_lx2160a_serdes3 = {
> +	.lane_supports_mode = lx2160a_serdes3_lane_supports_mode,
> +};
> +
> +static const struct lynx_info lynx_info_lx2162a_serdes1 = {
> +	.lane_supports_mode = lx2162a_serdes1_lane_supports_mode,
> +	.first_lane = 4,
> +};
> +
> +static const struct lynx_info lynx_info_lx2162a_serdes2 = {
> +	.lane_supports_mode = lx2162a_serdes2_lane_supports_mode,
> +};
> +
>  static int lynx_pccr_read(struct lynx_28g_lane *lane, enum lynx_lane_mode mode,
>  			  u32 *val)
>  {
> @@ -939,7 +1032,6 @@ static int lynx_28g_lane_enable_pcvt(struct lynx_28g_lane *lane,
>  static int lynx_28g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
>  {
>  	struct lynx_28g_lane *lane = phy_get_drvdata(phy);
> -	struct lynx_28g_priv *priv = lane->priv;
>  	int powered_up = lane->powered_up;
>  	enum lynx_lane_mode lane_mode;
>  	int err = 0;
> @@ -951,7 +1043,7 @@ static int lynx_28g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
>  		return -EOPNOTSUPP;
>  
>  	lane_mode = phy_interface_to_lane_mode(submode);
> -	if (!lynx_28g_supports_lane_mode(priv, lane_mode))
> +	if (!lynx_28g_supports_lane_mode(lane, lane_mode))
>  		return -EOPNOTSUPP;
>  
>  	if (lane_mode == lane->mode)
> @@ -984,14 +1076,13 @@ static int lynx_28g_validate(struct phy *phy, enum phy_mode mode, int submode,
>  			     union phy_configure_opts *opts __always_unused)
>  {
>  	struct lynx_28g_lane *lane = phy_get_drvdata(phy);
> -	struct lynx_28g_priv *priv = lane->priv;
>  	enum lynx_lane_mode lane_mode;
>  
>  	if (mode != PHY_MODE_ETHERNET)
>  		return -EOPNOTSUPP;
>  
>  	lane_mode = phy_interface_to_lane_mode(submode);
> -	if (!lynx_28g_supports_lane_mode(priv, lane_mode))
> +	if (!lynx_28g_supports_lane_mode(lane, lane_mode))
>  		return -EOPNOTSUPP;
>  
>  	return 0;
> @@ -1067,8 +1158,10 @@ static void lynx_28g_cdr_lock_check(struct work_struct *work)
>  	u32 rrstctl;
>  	int i;
>  
> -	for (i = 0; i < LYNX_28G_NUM_LANE; i++) {
> +	for (i = priv->info->first_lane; i < LYNX_28G_NUM_LANE; i++) {
>  		lane = &priv->lane[i];
> +		if (!lane->phy)
> +			continue;
>  
>  		mutex_lock(&lane->phy->mutex);
>  
> @@ -1120,24 +1213,48 @@ static struct phy *lynx_28g_xlate(struct device *dev,
>  	struct lynx_28g_priv *priv = dev_get_drvdata(dev);
>  	int idx = args->args[0];
>  
> -	if (WARN_ON(idx >= LYNX_28G_NUM_LANE))
> +	if (WARN_ON(idx >= LYNX_28G_NUM_LANE ||
> +		    idx < priv->info->first_lane))
>  		return ERR_PTR(-EINVAL);
>  
>  	return priv->lane[idx].phy;
>  }
>  
> +static int lynx_28g_probe_lane(struct lynx_28g_priv *priv, int id,
> +			       struct device_node *dn)
> +{
> +	struct lynx_28g_lane *lane = &priv->lane[id];
> +	struct phy *phy;
> +
> +	memset(lane, 0, sizeof(*lane));
> +
> +	phy = devm_phy_create(priv->dev, dn, &lynx_28g_ops);
> +	if (IS_ERR(phy))
> +		return PTR_ERR(phy);
> +
> +	lane->priv = priv;
> +	lane->phy = phy;
> +	lane->id = id;
> +	phy_set_drvdata(phy, lane);
> +	lynx_28g_lane_read_configuration(lane);
> +
> +	return 0;
> +}
> +
>  static int lynx_28g_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> +	bool lane_phy_providers = true;
>  	struct phy_provider *provider;
>  	struct lynx_28g_priv *priv;
> -	int i;
> +	int err;
>  
>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>  	if (!priv)
>  		return -ENOMEM;
>  
>  	priv->dev = dev;
> +	priv->info = of_device_get_match_data(dev);
>  	dev_set_drvdata(dev, priv);
>  	spin_lock_init(&priv->pcc_lock);
>  	INIT_DELAYED_WORK(&priv->cdr_check, lynx_28g_cdr_lock_check);
> @@ -1146,26 +1263,60 @@ static int lynx_28g_probe(struct platform_device *pdev)
>  	if (IS_ERR(priv->base))
>  		return PTR_ERR(priv->base);
>  
> -	lynx_28g_pll_read_configuration(priv);
> +	if (priv->info == &lynx_info_compat) {
> +		/*
> +		 * If we get here it means we probed on a device tree where
> +		 * "fsl,lynx-28g" wasn't the fallback, but the sole compatible
> +		 * string.
> +		 */
> +		dev_warn(dev, "Please update device tree to use per-device compatible strings\n");
> +		lane_phy_providers = false;
> +	}
>  
> -	for (i = 0; i < LYNX_28G_NUM_LANE; i++) {
> -		struct lynx_28g_lane *lane = &priv->lane[i];
> -		struct phy *phy;
> +	lynx_28g_pll_read_configuration(priv);
>  
> -		memset(lane, 0, sizeof(*lane));
> +	if (lane_phy_providers) {
> +		struct device_node *dn = dev_of_node(dev), *child;
> +
> +		for_each_available_child_of_node(dn, child) {
> +			u32 reg;
> +
> +			/* PHY subnode name must be 'phy'. */
> +			if (!(of_node_name_eq(child, "phy")))
> +				continue;
> +
> +			if (of_property_read_u32(child, "reg", &reg)) {
> +				dev_err(dev, "No \"reg\" property for %pOF\n", child);
> +				of_node_put(child);
> +				return -EINVAL;
> +			}
> +
> +			if (reg < priv->info->first_lane || reg >= LYNX_28G_NUM_LANE) {
> +				dev_err(dev, "\"reg\" property out of range for %pOF\n", child);
> +				of_node_put(child);
> +				return -EINVAL;
> +			}
> +
> +			err = lynx_28g_probe_lane(priv, reg, child);
> +			if (err) {
> +				of_node_put(child);
> +				return err;
> +			}
> +		}
>  
> -		phy = devm_phy_create(dev, NULL, &lynx_28g_ops);
> -		if (IS_ERR(phy))
> -			return PTR_ERR(phy);
> +		provider = devm_of_phy_provider_register(&pdev->dev,
> +							 of_phy_simple_xlate);
Is this really necessary? I believe phy_provider is not needed to resolve phandle
to a single phy, and is used only when one driver provides multiple phys.
> +	} else {
> +		for (int i = priv->info->first_lane; i < LYNX_28G_NUM_LANE; i++) {
> +			err = lynx_28g_probe_lane(priv, i, NULL);
> +			if (err)
> +				return err;
> +		}
>  
> -		lane->priv = priv;
> -		lane->phy = phy;
> -		lane->id = i;
> -		phy_set_drvdata(phy, lane);
> -		lynx_28g_lane_read_configuration(lane);
> +		provider = devm_of_phy_provider_register(&pdev->dev,
> +							 lynx_28g_xlate);
>  	}
>  
> -	provider = devm_of_phy_provider_register(dev, lynx_28g_xlate);
>  	if (IS_ERR(provider))
>  		return PTR_ERR(provider);
Keep this to have compatibility with phandles to the parent.
>  
> @@ -1184,7 +1335,12 @@ static void lynx_28g_remove(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id lynx_28g_of_match_table[] = {
> -	{ .compatible = "fsl,lynx-28g" },
> +	{ .compatible = "fsl,lx2160a-serdes1", .data = &lynx_info_lx2160a_serdes1 },
> +	{ .compatible = "fsl,lx2160a-serdes2", .data = &lynx_info_lx2160a_serdes2 },
> +	{ .compatible = "fsl,lx2160a-serdes3", .data = &lynx_info_lx2160a_serdes3 },
> +	{ .compatible = "fsl,lx2162a-serdes1", .data = &lynx_info_lx2162a_serdes1 },
> +	{ .compatible = "fsl,lx2162a-serdes2", .data = &lynx_info_lx2162a_serdes2 },
> +	{ .compatible = "fsl,lynx-28g", .data = &lynx_info_compat }, /* fallback, keep last */
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, lynx_28g_of_match_table);

Re: [PATCH v3 phy 13/17] phy: lynx-28g: probe on per-SoC and per-instance compatible strings

[Josua Mayer]

Am 02.10.25 um 12:40 schrieb Josua Mayer:
> Am 26.09.25 um 20:05 schrieb Vladimir Oltean:
>
>> Add driver support for probing on the new, per-instance and per-SoC
>> bindings, which provide the following benefits:
>> - they allow rejecting unsupported protocols per lane (10GbE on SerDes 2
>>   lanes 0-5)
>> - individual lanes have their own OF nodes as PHY providers, which
>>   allows board device tree authors to tune electrical parameters such as
>>   TX amplitude, equalization etc.
>>
>> Probing on "fsl,lynx-28g" is still supported, but the feature set is
>> frozen in time to just 1GbE and 10GbE (essentially the feature set as of
>> this change). However, we encourage the user at probe time to update the
>> device tree.
>>
>> Refactor the per-lane logic from lynx_28g_probe() into
>> lynx_28g_lane_probe(), and call it from two distinct paths depending on
>> whether the modern or the legacy compatible string is used, with an OF
>> node for the lane or without.
>>
>> Notable implication of the above: when lanes have disabled OF nodes, we
>> skip creating PHYs for them, and must also skip the CDR lock workaround.
>>
>> lynx_28g_supports_lane_mode() was a SerDes-global function and now
>> becomes per lane, to reflect the specific capabilities each instance may
>> have.
>>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
>> Cc: Conor Dooley <conor+dt@kernel.org>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>> ---
>> v2->v3:
>> - reword commit message
>> - add some comments regarding the "fsl,lynx-28g" fallback mechanism
>> - skip CDR lock workaround for lanes with no PHY (disabled in the device
>>   tree in the new binding)
>> v1->v2:
>> - remove priv->info->get_pccr() and priv->info->get_pcvt_offset().
>>   These were always called directly as lynx_28g_get_pccr() and
>>   lynx_28g_get_pcvt_offset().
>> - Add forgotten priv->info->lane_supports_mode() test to
>>   lynx_28g_supports_lane_mode().
>> - Rename the "fsl,lynx-28g" drvdata as lynx_info_compat rather than
>>   lynx_info_lx2160a_serdes1, to reflect its treatment as less featured.
>> - Implement a separate lane probing path for the #phy-cells = <0> case.
>>
>>  drivers/phy/freescale/phy-fsl-lynx-28g.c | 202 ++++++++++++++++++++---
>>  1 file changed, 179 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
>> index 453e76e0a6b7..1ddca8b4de17 100644
>> --- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
>> +++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
>> @@ -433,9 +433,15 @@ struct lynx_28g_lane {
>>  	enum lynx_lane_mode mode;
>>  };
>>  
>> +struct lynx_info {
>> +	bool (*lane_supports_mode)(int lane, enum lynx_lane_mode mode);
>> +	int first_lane;
>> +};
>> +
>>  struct lynx_28g_priv {
>>  	void __iomem *base;
>>  	struct device *dev;
>> +	const struct lynx_info *info;
>>  	/* Serialize concurrent access to registers shared between lanes,
>>  	 * like PCCn
>>  	 */
>> @@ -500,11 +506,18 @@ static enum lynx_lane_mode phy_interface_to_lane_mode(phy_interface_t intf)
>>  	}
>>  }
>>  
>> -static bool lynx_28g_supports_lane_mode(struct lynx_28g_priv *priv,
>> +/* A lane mode is supported if we have a PLL that can provide its required
>> + * clock net, and if there is a protocol converter for that mode on that lane.
>> + */
>> +static bool lynx_28g_supports_lane_mode(struct lynx_28g_lane *lane,
>>  					enum lynx_lane_mode mode)
>>  {
>> +	struct lynx_28g_priv *priv = lane->priv;
>>  	int i;
>>  
>> +	if (!priv->info->lane_supports_mode(lane->id, mode))
>> +		return false;
>> +
>>  	for (i = 0; i < LYNX_28G_NUM_PLL; i++) {
>>  		if (PLLnRSTCTL_DIS(priv->pll[i].rstctl))
>>  			continue;
>> @@ -687,6 +700,86 @@ static int lynx_28g_get_pcvt_offset(int lane, enum lynx_lane_mode lane_mode)
>>  	}
>>  }
>>  
>> +static bool lx2160a_serdes1_lane_supports_mode(int lane,
>> +					       enum lynx_lane_mode mode)
>> +{
>> +	return true;
>> +}
>> +
>> +static bool lx2160a_serdes2_lane_supports_mode(int lane,
>> +					       enum lynx_lane_mode mode)
>> +{
>> +	switch (mode) {
>> +	case LANE_MODE_1000BASEX_SGMII:
>> +		return true;
>> +	case LANE_MODE_USXGMII:
>> +	case LANE_MODE_10GBASER:
>> +		return lane == 6 || lane == 7;
>> +	default:
>> +		return false;
>> +	}
>> +}
>> +
>> +static bool lx2160a_serdes3_lane_supports_mode(int lane,
>> +					       enum lynx_lane_mode mode)
>> +{
>> +	/*
>> +	 * Non-networking SerDes, and this driver supports only
>> +	 * networking protocols
>> +	 */
>> +	return false;
>> +}
>> +
>> +static bool lx2162a_serdes1_lane_supports_mode(int lane,
>> +					       enum lynx_lane_mode mode)
>> +{
>> +	return true;
>> +}
>> +
>> +static bool lx2162a_serdes2_lane_supports_mode(int lane,
>> +					       enum lynx_lane_mode mode)
>> +{
>> +	return lx2160a_serdes2_lane_supports_mode(lane, mode);
>> +}
>> +
>> +static bool lynx_28g_compat_lane_supports_mode(int lane,
>> +					       enum lynx_lane_mode mode)
>> +{
>> +	switch (mode) {
>> +	case LANE_MODE_1000BASEX_SGMII:
>> +	case LANE_MODE_USXGMII:
>> +	case LANE_MODE_10GBASER:
>> +		return true;
>> +	default:
>> +		return false;
>> +	}
>> +}
>> +
>> +static const struct lynx_info lynx_info_compat = {
>> +	.lane_supports_mode = lynx_28g_compat_lane_supports_mode,
>> +};
>> +
>> +static const struct lynx_info lynx_info_lx2160a_serdes1 = {
>> +	.lane_supports_mode = lx2160a_serdes1_lane_supports_mode,
>> +};
>> +
>> +static const struct lynx_info lynx_info_lx2160a_serdes2 = {
>> +	.lane_supports_mode = lx2160a_serdes2_lane_supports_mode,
>> +};
>> +
>> +static const struct lynx_info lynx_info_lx2160a_serdes3 = {
>> +	.lane_supports_mode = lx2160a_serdes3_lane_supports_mode,
>> +};
>> +
>> +static const struct lynx_info lynx_info_lx2162a_serdes1 = {
>> +	.lane_supports_mode = lx2162a_serdes1_lane_supports_mode,
>> +	.first_lane = 4,
>> +};
>> +
>> +static const struct lynx_info lynx_info_lx2162a_serdes2 = {
>> +	.lane_supports_mode = lx2162a_serdes2_lane_supports_mode,
>> +};
>> +
>>  static int lynx_pccr_read(struct lynx_28g_lane *lane, enum lynx_lane_mode mode,
>>  			  u32 *val)
>>  {
>> @@ -939,7 +1032,6 @@ static int lynx_28g_lane_enable_pcvt(struct lynx_28g_lane *lane,
>>  static int lynx_28g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
>>  {
>>  	struct lynx_28g_lane *lane = phy_get_drvdata(phy);
>> -	struct lynx_28g_priv *priv = lane->priv;
>>  	int powered_up = lane->powered_up;
>>  	enum lynx_lane_mode lane_mode;
>>  	int err = 0;
>> @@ -951,7 +1043,7 @@ static int lynx_28g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
>>  		return -EOPNOTSUPP;
>>  
>>  	lane_mode = phy_interface_to_lane_mode(submode);
>> -	if (!lynx_28g_supports_lane_mode(priv, lane_mode))
>> +	if (!lynx_28g_supports_lane_mode(lane, lane_mode))
>>  		return -EOPNOTSUPP;
>>  
>>  	if (lane_mode == lane->mode)
>> @@ -984,14 +1076,13 @@ static int lynx_28g_validate(struct phy *phy, enum phy_mode mode, int submode,
>>  			     union phy_configure_opts *opts __always_unused)
>>  {
>>  	struct lynx_28g_lane *lane = phy_get_drvdata(phy);
>> -	struct lynx_28g_priv *priv = lane->priv;
>>  	enum lynx_lane_mode lane_mode;
>>  
>>  	if (mode != PHY_MODE_ETHERNET)
>>  		return -EOPNOTSUPP;
>>  
>>  	lane_mode = phy_interface_to_lane_mode(submode);
>> -	if (!lynx_28g_supports_lane_mode(priv, lane_mode))
>> +	if (!lynx_28g_supports_lane_mode(lane, lane_mode))
>>  		return -EOPNOTSUPP;
>>  
>>  	return 0;
>> @@ -1067,8 +1158,10 @@ static void lynx_28g_cdr_lock_check(struct work_struct *work)
>>  	u32 rrstctl;
>>  	int i;
>>  
>> -	for (i = 0; i < LYNX_28G_NUM_LANE; i++) {
>> +	for (i = priv->info->first_lane; i < LYNX_28G_NUM_LANE; i++) {
>>  		lane = &priv->lane[i];
>> +		if (!lane->phy)
>> +			continue;
>>  
>>  		mutex_lock(&lane->phy->mutex);
>>  
>> @@ -1120,24 +1213,48 @@ static struct phy *lynx_28g_xlate(struct device *dev,
>>  	struct lynx_28g_priv *priv = dev_get_drvdata(dev);
>>  	int idx = args->args[0];
>>  
>> -	if (WARN_ON(idx >= LYNX_28G_NUM_LANE))
>> +	if (WARN_ON(idx >= LYNX_28G_NUM_LANE ||
>> +		    idx < priv->info->first_lane))
>>  		return ERR_PTR(-EINVAL);
>>  
>>  	return priv->lane[idx].phy;
>>  }
FYI as an example please see below how I handled this previously.
The xlate function below can translate both phandles with 0 and 1 arguments:

static struct phy *lynx_28g_xlate(struct device *dev,
                  struct of_phandle_args *args)
{
    struct lynx_28g_priv *priv;
    struct phy *phy;
    int idx;

    if (args->args_count == 0) {
        /* direct look-up */
        phy = of_phy_simple_xlate(dev, args);
    } else if (args->args_count == 1) {
        /* look-up from parent by index */
        idx = args->args[0];
        if (WARN_ON(idx >= LYNX_28G_NUM_LANE))
            return ERR_PTR(-EINVAL);

        priv = dev_get_drvdata(dev);
        phy = priv->lane[idx].phy;
        if (!phy)
            phy = ERR_PTR(-ENODEV);
    } else {
        phy = ERR_PTR(-EINVAL);
    }

    return phy;
}

While in probe only one phy_provider is registered.

>>  
>> +static int lynx_28g_probe_lane(struct lynx_28g_priv *priv, int id,
>> +			       struct device_node *dn)
>> +{
>> +	struct lynx_28g_lane *lane = &priv->lane[id];
>> +	struct phy *phy;
>> +
>> +	memset(lane, 0, sizeof(*lane));
>> +
>> +	phy = devm_phy_create(priv->dev, dn, &lynx_28g_ops);
>> +	if (IS_ERR(phy))
>> +		return PTR_ERR(phy);
>> +
>> +	lane->priv = priv;
>> +	lane->phy = phy;
>> +	lane->id = id;
>> +	phy_set_drvdata(phy, lane);
>> +	lynx_28g_lane_read_configuration(lane);
>> +
>> +	return 0;
>> +}
>> +
>>  static int lynx_28g_probe(struct platform_device *pdev)
>>  {
>>  	struct device *dev = &pdev->dev;
>> +	bool lane_phy_providers = true;
>>  	struct phy_provider *provider;
>>  	struct lynx_28g_priv *priv;
>> -	int i;
>> +	int err;
>>  
>>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>  	if (!priv)
>>  		return -ENOMEM;
>>  
>>  	priv->dev = dev;
>> +	priv->info = of_device_get_match_data(dev);
>>  	dev_set_drvdata(dev, priv);
>>  	spin_lock_init(&priv->pcc_lock);
>>  	INIT_DELAYED_WORK(&priv->cdr_check, lynx_28g_cdr_lock_check);
>> @@ -1146,26 +1263,60 @@ static int lynx_28g_probe(struct platform_device *pdev)
>>  	if (IS_ERR(priv->base))
>>  		return PTR_ERR(priv->base);
>>  
>> -	lynx_28g_pll_read_configuration(priv);
>> +	if (priv->info == &lynx_info_compat) {
>> +		/*
>> +		 * If we get here it means we probed on a device tree where
>> +		 * "fsl,lynx-28g" wasn't the fallback, but the sole compatible
>> +		 * string.
>> +		 */
>> +		dev_warn(dev, "Please update device tree to use per-device compatible strings\n");
>> +		lane_phy_providers = false;
>> +	}
>>  
>> -	for (i = 0; i < LYNX_28G_NUM_LANE; i++) {
>> -		struct lynx_28g_lane *lane = &priv->lane[i];
>> -		struct phy *phy;
>> +	lynx_28g_pll_read_configuration(priv);
>>  
>> -		memset(lane, 0, sizeof(*lane));
>> +	if (lane_phy_providers) {
>> +		struct device_node *dn = dev_of_node(dev), *child;
>> +
>> +		for_each_available_child_of_node(dn, child) {
>> +			u32 reg;
>> +
>> +			/* PHY subnode name must be 'phy'. */
>> +			if (!(of_node_name_eq(child, "phy")))
>> +				continue;
>> +
>> +			if (of_property_read_u32(child, "reg", &reg)) {
>> +				dev_err(dev, "No \"reg\" property for %pOF\n", child);
>> +				of_node_put(child);
>> +				return -EINVAL;
>> +			}
>> +
>> +			if (reg < priv->info->first_lane || reg >= LYNX_28G_NUM_LANE) {
>> +				dev_err(dev, "\"reg\" property out of range for %pOF\n", child);
>> +				of_node_put(child);
>> +				return -EINVAL;
>> +			}
>> +
>> +			err = lynx_28g_probe_lane(priv, reg, child);
>> +			if (err) {
>> +				of_node_put(child);
>> +				return err;
>> +			}
>> +		}
>>  
>> -		phy = devm_phy_create(dev, NULL, &lynx_28g_ops);
>> -		if (IS_ERR(phy))
>> -			return PTR_ERR(phy);
>> +		provider = devm_of_phy_provider_register(&pdev->dev,
>> +							 of_phy_simple_xlate);
> Is this really necessary? I believe phy_provider is not needed to resolve phandle
> to a single phy, and is used only when one driver provides multiple phys.
>> +	} else {
>> +		for (int i = priv->info->first_lane; i < LYNX_28G_NUM_LANE; i++) {
>> +			err = lynx_28g_probe_lane(priv, i, NULL);
>> +			if (err)
>> +				return err;
>> +		}
>>  
>> -		lane->priv = priv;
>> -		lane->phy = phy;
>> -		lane->id = i;
>> -		phy_set_drvdata(phy, lane);
>> -		lynx_28g_lane_read_configuration(lane);
>> +		provider = devm_of_phy_provider_register(&pdev->dev,
>> +							 lynx_28g_xlate);
>>  	}
>>  
>> -	provider = devm_of_phy_provider_register(dev, lynx_28g_xlate);
>>  	if (IS_ERR(provider))
>>  		return PTR_ERR(provider);
> Keep this to have compatibility with phandles to the parent.
>>  
>> @@ -1184,7 +1335,12 @@ static void lynx_28g_remove(struct platform_device *pdev)
>>  }
>>  
>>  static const struct of_device_id lynx_28g_of_match_table[] = {
>> -	{ .compatible = "fsl,lynx-28g" },
>> +	{ .compatible = "fsl,lx2160a-serdes1", .data = &lynx_info_lx2160a_serdes1 },
>> +	{ .compatible = "fsl,lx2160a-serdes2", .data = &lynx_info_lx2160a_serdes2 },
>> +	{ .compatible = "fsl,lx2160a-serdes3", .data = &lynx_info_lx2160a_serdes3 },
>> +	{ .compatible = "fsl,lx2162a-serdes1", .data = &lynx_info_lx2162a_serdes1 },
>> +	{ .compatible = "fsl,lx2162a-serdes2", .data = &lynx_info_lx2162a_serdes2 },
>> +	{ .compatible = "fsl,lynx-28g", .data = &lynx_info_compat }, /* fallback, keep last */
>>  	{ },
>>  };
>>  MODULE_DEVICE_TABLE(of, lynx_28g_of_match_table);

Re: [PATCH v3 phy 13/17] phy: lynx-28g: probe on per-SoC and per-instance compatible strings

[Vladimir Oltean]

On Thu, Oct 02, 2025 at 10:50:46AM +0000, Josua Mayer wrote:
> FYI as an example please see below how I handled this previously.
> The xlate function below can translate both phandles with 0 and 1 arguments:
> 
> static struct phy *lynx_28g_xlate(struct device *dev,
>                   struct of_phandle_args *args)
> {
>     struct lynx_28g_priv *priv;
>     struct phy *phy;
>     int idx;
> 
>     if (args->args_count == 0) {
>         /* direct look-up */
>         phy = of_phy_simple_xlate(dev, args);
>     } else if (args->args_count == 1) {
>         /* look-up from parent by index */
>         idx = args->args[0];
>         if (WARN_ON(idx >= LYNX_28G_NUM_LANE))
>             return ERR_PTR(-EINVAL);
> 
>         priv = dev_get_drvdata(dev);
>         phy = priv->lane[idx].phy;
>         if (!phy)
>             phy = ERR_PTR(-ENODEV);
>     } else {
>         phy = ERR_PTR(-EINVAL);
>     }
> 
>     return phy;
> }
> 
> While in probe only one phy_provider is registered.

So what is the practical difference, in the #phy-cells = <0> case,
between registering a single phy provider and a custom xlate function
that redirects to of_phy_simple_xlate(), vs registering a phy provider
per lane and passing the of_phy_simple_xlate() function directly?

Re: [PATCH v3 phy 13/17] phy: lynx-28g: probe on per-SoC and per-instance compatible strings

[Vladimir Oltean]

On Thu, Oct 02, 2025 at 10:40:51AM +0000, Josua Mayer wrote:
> > -	provider = devm_of_phy_provider_register(dev, lynx_28g_xlate);
> >  	if (IS_ERR(provider))
> >  		return PTR_ERR(provider);
> Keep this to have compatibility with phandles to the parent.

I am not removing it. Both the "if (lane_phy_providers)" and "else"
branches call devm_of_phy_provider_register(), with a different second
xlate argument (of_phy_simple_xlate vs lynx_28g_xlate).

Re: [PATCH v3 phy 12/17] dt-bindings: phy: lynx-28g: add compatible strings per SerDes and instantiation

[Vladimir Oltean]

On Thu, Oct 02, 2025 at 10:08:31AM +0000, Josua Mayer wrote:
> I would not expect being able to use a new schema in an older kernel.
> Maybe DT maintainers can confirm whether anyone expects that.

I can confirm from my own experience - some people expect that.
https://lore.kernel.org/lkml/87czlzjxmz.wl-maz@kernel.org/

> >> There are 2 directions to go from here:
> >> 1. Have optional per-lane "phy" OF node children, which exist solely for
> >>    tuning electrical parameters. We need to keep the top-level SerDes as
> >>    the only PHY provider, with #phy-cells = <1> denoting the lane.
> >>
> >> 2. Accept that keeping "fsl,lynx-28g" and overlaid properties has
> >>    absolutely no practical benefit, and drop them (effectively returning
> >>    to Conor's suggestion, as implemented in v2)
> I don't quite understand how keeping or dropping fsl,lynx-28g compatible
> impacts the ability to set phandles to the base and child nodes.

It doesn't "impact" it, but it doesn't help it either. It is an unnecessary
complication once you accept the fact that the consumer can have a single
phandle, either to the SerDes or to the lane, and that old kernels only
understand phandles to the SerDes, not to the lane.

> >> 3. Extend the schema and the driver support for it as a backportable bug
> >>    fix, to allow registering PHY providers for lanes with OF nodes in
> >>    stable kernels. This avoids regressing when the device trees are
> >>    updated, assuming the stable kernel is also updated.
> I think whatever will be done with phy sub-nodes on the serdes block node
> should not count as a fix to be backported in stable.
> Any attempt to backport reference to &serdes_1_lane_a will produce a
> compile-time error.

Changing the phandle from <&serdes_1> to <&serdes_1_lane_a> is a
breaking change for unpatched kernels, so your options are:
- never do it. If you still want to add OF nodes per lane, this is
  strange for the reason quoted below (summary: lane OF node exists, but
  the phandle does not point to it).
- fix stable kernels so it's not a breaking change.
- live with the breakage.

> >>
> >> It's not that I particularly like #2, but going with #1 would imply that
> >> lane OF nodes exist, but the "phys" phandles do not point to them.
> >>
> >> Combine that with the fact that anything we do with the 28G Lynx
> >> bindings will have to be replicated, for uniformity's sake, with the
> >> upcoming 10G Lynx SerDes binding (very similar hardware IP), and #1 is
> >> suddenly not looking so pretty at all. I.e. introducing the 10G Lynx
> >> bindings like the 28G Lynx ones would mean deviating from the widely
> >> established norm, and introducing them like the widely established norm
> >> would mean deviating from the 28G Lynx. I can easily see how someone
> >> might look at them one day and think "hmm, can't we make them more
> >> uniform?"
> >>
> >> OTOH, the fact that device tree updates require kernel updates (as
> >> implied by #2) is acceptably by everyone in this thread who expressed an
> >> opinion on this topic.
> >>
> >> As for option #3, while IMO it would be a justified "new feature as
> >> bug fix", it sounds a bit counterintuitive and I'm afraid I won't manage
> >> to convince all maintainers along the way that this is the way forward.
> >>
> >> I'll wait for the merge window to close before reposting anything, but
> >> I'd like an explicit ack from Rob and Josua in the meantime, whose
> >> change request I'd be effectively reverting, to make sure that this
> >> topic is closed.
> > If #2 is not going to upset anyone (that their device stopped working), 
> > then that route is fine.
> >
> > Rob