[PATCH v2 phy 00/16] Lynx 28G improvements part 1

[PATCH v2 phy 00/16] 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/16:
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.

Detailed change log in individual patches. Thanks to Conor, Krzysztof,
Josua, 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 (14):
  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: truly power the lanes up or down
  phy: lynx-28g: implement phy_exit() operation

 .../devicetree/bindings/phy/fsl,lynx-28g.yaml |  146 +-
 drivers/phy/freescale/phy-fsl-lynx-28g.c      | 1408 +++++++++++++----
 2 files changed, 1264 insertions(+), 290 deletions(-)

-- 
2.34.1

[PATCH v2 phy 12/16] 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 would be to list in the device tree all
protocols supported by each lane. That was attempted in this unmerged
patch set for the older Lynx 10G family:
https://lore.kernel.org/linux-phy/20230413160607.4128315-3-sean.anderson@seco.com/

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

Since this compatible string change breaks forward compatibility of old
kernels with new device trees (which is OK with the known users), this
is a good time to fulfill another user request, which is that individual
SerDes lanes should have had their own OF nodes, so that we can
customize electrical parameters:
https://lore.kernel.org/lkml/02270f62-9334-400c-b7b9-7e6a44dbbfc9@solid-run.com/

This request requires #phy-cells = <0>, and because "fsl,lynx-28g"
requires #phy-cells = <1>, we obviously cannot have both at the same
time.

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>
---
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 | 146 +++++++++++++++++-
 1 file changed, 140 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml b/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml
index ff9f9ca0f19c..390c9ecd94cc 100644
--- a/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml
+++ b/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml
@@ -9,21 +9,113 @@ 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. The capabilities
+          of managed lanes are limited to 1GbE and 10GbE (depending on the
+          availability of an adequate PLL clock net frequency). Deprecated, use
+          device-specific strings instead.
+      - enum:
+          - fsl,lx2160a-serdes1
+          - fsl,lx2160a-serdes2
+          - fsl,lx2160a-serdes3
+          - fsl,lx2162a-serdes1
+          - fsl,lx2162a-serdes2
 
   reg:
     maxItems: 1
 
+  "#address-cells":
+    const: 1
+    description: "Address cells for child lane nodes"
+
+  "#size-cells":
+    const: 0
+    description: "Size cells for child lane nodes"
+
   "#phy-cells":
+    description: "Number of cells in PHY specifier (legacy binding only)"
     const: 1
 
+patternProperties:
+  "^phy@[0-9a-f]+$":
+    type: object
+    description: Individual SerDes lane acting as PHY provider
+
+    properties:
+      reg:
+        description: Lane number
+        maxItems: 1
+
+      "#phy-cells":
+        description: Number of cells in PHY specifier for this lane
+        const: 0
+
+    required:
+      - reg
+      - "#phy-cells"
+
+    additionalProperties: false
+
 required:
   - compatible
   - reg
-  - "#phy-cells"
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          const: fsl,lynx-28g
+    then:
+      # Legacy case: parent is PHY provider
+      properties:
+        "#phy-cells":
+          const: 1
+        "#address-cells": false
+        "#size-cells": false
+      required:
+        - "#phy-cells"
+      patternProperties:
+        "^phy@[0-9a-f]+$": false
+    else:
+      # Modern case: children are PHY providers
+      properties:
+        "#phy-cells": false
+      required:
+        - "#address-cells"
+        - "#size-cells"
+
+  # LX2162A SerDes 1 has fewer lanes than the others
+  - if:
+      properties:
+        compatible:
+          const: fsl,lx2162a-serdes1
+    then:
+      patternProperties:
+        "^phy@[0-9a-f]+$":
+          properties:
+            reg:
+              description: Lane number (lanes 4-7 only for LX2162A SerDes 1)
+              enum: [4, 5, 6, 7]
+    else:
+      patternProperties:
+        "^phy@[0-9a-f]+$":
+          properties:
+            reg:
+              description: Lane number (lanes 0-7)
+              enum: [0, 1, 2, 3, 4, 5, 6, 7]
 
 additionalProperties: false
 
@@ -32,9 +124,51 @@ examples:
     soc {
       #address-cells = <2>;
       #size-cells = <2>;
-      serdes_1: phy@1ea0000 {
-        compatible = "fsl,lynx-28g";
+
+      serdes_1: serdes@1ea0000 {
+        compatible = "fsl,lx2160a-serdes1";
         reg = <0x0 0x1ea0000 0x0 0x1e30>;
-        #phy-cells = <1>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        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 v2 phy 13/16] 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 have #phy-cells = <0>.

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, otherwise we might access lanes which do not exist (0-3 on
LX2162A SerDes 1) and we would fail to reject lane modes which don't
work (10GbE on SerDes 2 lanes 0-5).

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.

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>
---
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 | 199 ++++++++++++++++++++---
 1 file changed, 174 insertions(+), 25 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
index 5b2a5b1e674f..9b8e24828d0f 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,8 +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 (submode == lane->mode)
@@ -985,13 +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;
 
-	if (!lynx_28g_supports_lane_mode(priv,
-					 phy_interface_to_lane_mode(submode)))
+	lane_mode = phy_interface_to_lane_mode(submode);
+	if (!lynx_28g_supports_lane_mode(lane, lane_mode))
 		return -EOPNOTSUPP;
 
 	return 0;
@@ -1067,7 +1158,7 @@ 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];
 
 		mutex_lock(&lane->phy->mutex);
@@ -1120,24 +1211,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 +1261,55 @@ 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) {
+		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 +1328,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 */
 	{ },
 };
 MODULE_DEVICE_TABLE(of, lynx_28g_of_match_table);
-- 
2.34.1

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

[Rob Herring (Arm)]

On Tue, 23 Sep 2025 22:44:41 +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 would be to list in the device tree all
> protocols supported by each lane. That was attempted in this unmerged
> patch set for the older Lynx 10G family:
> https://lore.kernel.org/linux-phy/20230413160607.4128315-3-sean.anderson@seco.com/
> 
> but IMO that approach is more drawn-out and more prone to errors,
> whereas this one is more succinct and obviously correct.
> 
> Since this compatible string change breaks forward compatibility of old
> kernels with new device trees (which is OK with the known users), this
> is a good time to fulfill another user request, which is that individual
> SerDes lanes should have had their own OF nodes, so that we can
> customize electrical parameters:
> https://lore.kernel.org/lkml/02270f62-9334-400c-b7b9-7e6a44dbbfc9@solid-run.com/
> 
> This request requires #phy-cells = <0>, and because "fsl,lynx-28g"
> requires #phy-cells = <1>, we obviously cannot have both at the same
> time.
> 
> 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>
> ---
> 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 | 146 +++++++++++++++++-
>  1 file changed, 140 insertions(+), 6 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml:42:18: [error] string value is redundantly quoted with any quotes (quoted-strings)
./Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml:46:18: [error] string value is redundantly quoted with any quotes (quoted-strings)
./Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml:49:18: [error] string value is redundantly quoted with any quotes (quoted-strings)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250923194445.454442-13-vladimir.oltean@nxp.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

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

[Vladimir Oltean]

On Tue, Sep 23, 2025 at 03:37:31PM -0500, Rob Herring (Arm) wrote:
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml:42:18: [error] string value is redundantly quoted with any quotes (quoted-strings)
> ./Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml:46:18: [error] string value is redundantly quoted with any quotes (quoted-strings)
> ./Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml:49:18: [error] string value is redundantly quoted with any quotes (quoted-strings)
> 
> dtschema/dtc warnings/errors:
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250923194445.454442-13-vladimir.oltean@nxp.com
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
>

Sorry, I literally didn't notice this warning even though it seems I am
also reproducing it locally.

I hope this doesn't mean the patch won't get reviewed.

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

[Rob Herring]

On Tue, Sep 23, 2025 at 10:44:41PM +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 would be to list in the device tree all
> protocols supported by each lane. That was attempted in this unmerged
> patch set for the older Lynx 10G family:
> https://lore.kernel.org/linux-phy/20230413160607.4128315-3-sean.anderson@seco.com/
> 
> but IMO that approach is more drawn-out and more prone to errors,
> whereas this one is more succinct and obviously correct.
> 
> Since this compatible string change breaks forward compatibility of old
> kernels with new device trees (which is OK with the known users), this
> is a good time to fulfill another user request, which is that individual
> SerDes lanes should have had their own OF nodes, so that we can
> customize electrical parameters:
> https://lore.kernel.org/lkml/02270f62-9334-400c-b7b9-7e6a44dbbfc9@solid-run.com/
> 
> This request requires #phy-cells = <0>, and because "fsl,lynx-28g"
> requires #phy-cells = <1>, we obviously cannot have both at the same
> time.
> 
> 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>
> ---
> 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 | 146 +++++++++++++++++-
>  1 file changed, 140 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml b/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml
> index ff9f9ca0f19c..390c9ecd94cc 100644
> --- a/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml
> +++ b/Documentation/devicetree/bindings/phy/fsl,lynx-28g.yaml
> @@ -9,21 +9,113 @@ title: Freescale Lynx 28G SerDes PHY
>  maintainers:
>    - Ioana Ciornei <ioana.ciornei@nxp.com>
>  
> +description: |

Don't need '|' if no formatting to preserve.

> +  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. The capabilities
> +          of managed lanes are limited to 1GbE and 10GbE (depending on the
> +          availability of an adequate PLL clock net frequency). Deprecated, use
> +          device-specific strings instead.
> +      - enum:
> +          - fsl,lx2160a-serdes1
> +          - fsl,lx2160a-serdes2
> +          - fsl,lx2160a-serdes3
> +          - fsl,lx2162a-serdes1
> +          - fsl,lx2162a-serdes2
>  
>    reg:
>      maxItems: 1
>  
> +  "#address-cells":
> +    const: 1
> +    description: "Address cells for child lane nodes"

You don't need generic descriptions of common properties.

> +
> +  "#size-cells":
> +    const: 0
> +    description: "Size cells for child lane nodes"
> +
>    "#phy-cells":
> +    description: "Number of cells in PHY specifier (legacy binding only)"
>      const: 1
>  
> +patternProperties:
> +  "^phy@[0-9a-f]+$":
> +    type: object
> +    description: Individual SerDes lane acting as PHY provider
> +
> +    properties:
> +      reg:
> +        description: Lane number
> +        maxItems: 1
> +
> +      "#phy-cells":
> +        description: Number of cells in PHY specifier for this lane
> +        const: 0
> +
> +    required:
> +      - reg
> +      - "#phy-cells"
> +
> +    additionalProperties: false
> +
>  required:
>    - compatible
>    - reg
> -  - "#phy-cells"
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          const: fsl,lynx-28g
> +    then:
> +      # Legacy case: parent is PHY provider
> +      properties:
> +        "#phy-cells":
> +          const: 1
> +        "#address-cells": false
> +        "#size-cells": false
> +      required:
> +        - "#phy-cells"
> +      patternProperties:
> +        "^phy@[0-9a-f]+$": false
> +    else:
> +      # Modern case: children are PHY providers
> +      properties:
> +        "#phy-cells": false
> +      required:
> +        - "#address-cells"
> +        - "#size-cells"
> +
> +  # LX2162A SerDes 1 has fewer lanes than the others
> +  - if:
> +      properties:
> +        compatible:
> +          const: fsl,lx2162a-serdes1
> +    then:
> +      patternProperties:
> +        "^phy@[0-9a-f]+$":
> +          properties:
> +            reg:
> +              description: Lane number (lanes 4-7 only for LX2162A SerDes 1)
> +              enum: [4, 5, 6, 7]
> +    else:
> +      patternProperties:
> +        "^phy@[0-9a-f]+$":
> +          properties:
> +            reg:
> +              description: Lane number (lanes 0-7)
> +              enum: [0, 1, 2, 3, 4, 5, 6, 7]
>  
>  additionalProperties: false
>  
> @@ -32,9 +124,51 @@ examples:
>      soc {
>        #address-cells = <2>;
>        #size-cells = <2>;
> -      serdes_1: phy@1ea0000 {
> -        compatible = "fsl,lynx-28g";
> +
> +      serdes_1: serdes@1ea0000 {
> +        compatible = "fsl,lx2160a-serdes1";
>          reg = <0x0 0x1ea0000 0x0 0x1e30>;
> -        #phy-cells = <1>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        phy@0 {
> +          reg = <0>;
> +          #phy-cells = <0>;
> +        };

There's really no difference between having child nodes 0-7 and 8 phy 
providers vs. putting 0-7 into a phy cell arg and 1 phy provider. 

The only difference I see is it is more straight-forward to determine 
what lanes are present in the phy driver if the driver needs to know 
that. But you can also just read all 'phys' properties in the DT with a 
&serdes_1 phandle and determine that. Is that efficient? No, but you 
have to do that exactly once and probably has no measurable impact.

With that, then can't you simply just add a more specific compatible:

compatible = "fsl,lx2160a-serdes1", "fsl,lynx-28g";

Then you maintain some compatibility.

Rob

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

[Vladimir Oltean]

Hi Rob,

On Wed, Sep 24, 2025 at 08:54:29AM -0500, Rob Herring wrote:
> > +description: |
> 
> Don't need '|' if no formatting to preserve.

Thanks, will drop.

> > +  "#address-cells":
> > +    const: 1
> > +    description: "Address cells for child lane nodes"
> 
> You don't need generic descriptions of common properties.

Ok, I'll also drop the description from #size-cells but keep it in
#phy-cells (less obvious).

> > +
> > +  "#size-cells":
> > +    const: 0
> > +    description: "Size cells for child lane nodes"
> > +
> >    "#phy-cells":
> > +    description: "Number of cells in PHY specifier (legacy binding only)"
> >      const: 1
> >  
> > @@ -32,9 +124,51 @@ examples:
> >      soc {
> >        #address-cells = <2>;
> >        #size-cells = <2>;
> > -      serdes_1: phy@1ea0000 {
> > -        compatible = "fsl,lynx-28g";
> > +
> > +      serdes_1: serdes@1ea0000 {
> > +        compatible = "fsl,lx2160a-serdes1";
> >          reg = <0x0 0x1ea0000 0x0 0x1e30>;
> > -        #phy-cells = <1>;
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        phy@0 {
> > +          reg = <0>;
> > +          #phy-cells = <0>;
> > +        };
> 
> There's really no difference between having child nodes 0-7 and 8 phy 
> providers vs. putting 0-7 into a phy cell arg and 1 phy provider. 
> 
> The only difference I see is it is more straight-forward to determine 
> what lanes are present in the phy driver if the driver needs to know 
> that. But you can also just read all 'phys' properties in the DT with a 
> &serdes_1 phandle and determine that. Is that efficient? No, but you 
> have to do that exactly once and probably has no measurable impact.
> 
> With that, then can't you simply just add a more specific compatible:
> 
> compatible = "fsl,lx2160a-serdes1", "fsl,lynx-28g";
> 
> Then you maintain some compatibility.
> 
> Rob

With the patches that have been presented to you thus far -- yes, this
is the correct conclusion, there is not much of a difference. But this
is not all.

If I want in the future to apply the properties from
Documentation/devicetree/bindings/phy/transmit-amplitude.yaml to just
one of the lanes, how would I do that with just 1 phy provider? It's not
so clear. Compared to 8 phy providers, each with its OF node => much
easier to structure and to understand.

This is essentially what the discussion with Josua from v1 boils down to.

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

[Josua Mayer]

Am 24.09.25 um 17:45 schrieb Vladimir Oltean:
> Hi Rob,
>
> On Wed, Sep 24, 2025 at 08:54:29AM -0500, Rob Herring wrote:
>>> +description: |
>> Don't need '|' if no formatting to preserve.
> Thanks, will drop.
>
>>> +  "#address-cells":
>>> +    const: 1
>>> +    description: "Address cells for child lane nodes"
>> You don't need generic descriptions of common properties.
> Ok, I'll also drop the description from #size-cells but keep it in
> #phy-cells (less obvious).
>
>>> +
>>> +  "#size-cells":
>>> +    const: 0
>>> +    description: "Size cells for child lane nodes"
>>> +
>>>    "#phy-cells":
>>> +    description: "Number of cells in PHY specifier (legacy binding only)"
>>>      const: 1
>>>  
>>> @@ -32,9 +124,51 @@ examples:
>>>      soc {
>>>        #address-cells = <2>;
>>>        #size-cells = <2>;
>>> -      serdes_1: phy@1ea0000 {
>>> -        compatible = "fsl,lynx-28g";
>>> +
>>> +      serdes_1: serdes@1ea0000 {
>>> +        compatible = "fsl,lx2160a-serdes1";
>>>          reg = <0x0 0x1ea0000 0x0 0x1e30>;
>>> -        #phy-cells = <1>;
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +
>>> +        phy@0 {
>>> +          reg = <0>;
>>> +          #phy-cells = <0>;
>>> +        };
>> There's really no difference between having child nodes 0-7 and 8 phy 
>> providers vs. putting 0-7 into a phy cell arg and 1 phy provider. 
>>
>> The only difference I see is it is more straight-forward to determine 
>> what lanes are present in the phy driver if the driver needs to know 
>> that. But you can also just read all 'phys' properties in the DT with a 
>> &serdes_1 phandle and determine that. Is that efficient? No, but you 
>> have to do that exactly once and probably has no measurable impact.
>>
>> With that, then can't you simply just add a more specific compatible:
>>
>> compatible = "fsl,lx2160a-serdes1", "fsl,lynx-28g";
>>
>> Then you maintain some compatibility.
>>
>> Rob
> With the patches that have been presented to you thus far -- yes, this
> is the correct conclusion, there is not much of a difference. But this
> is not all.
>
> If I want in the future to apply the properties from
> Documentation/devicetree/bindings/phy/transmit-amplitude.yaml to just
> one of the lanes, how would I do that with just 1 phy provider?
I believe it is possible for a driver to create multiple phy objects
during probe, and for the xlate function to return the correct one.

Then, whether you follow a phandle to the parent with 1 argument,
or a  phandle to the phy child with 0 arguments provides same results.

The driver already creates a phy object for each lane with:

phy = devm_phy_create(&pdev->dev, NULL, &lynx_28g_ops);

Once the second argument is changed to a valid lane node,
it's properties will be accessible.

I prototyped this a while ago:
https://github.com/SolidRun/lx2160a_build/blob/develop-ls-5.15.71-2.2.0/patches/linux/0030-phy-lynx-28g-add-support-for-device-tree-per-lane-ph.patch


> It's not
> so clear. Compared to 8 phy providers, each with its OF node => much
> easier to structure and to understand.
>
> This is essentially what the discussion with Josua from v1 boils down to.

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

[Vladimir Oltean]

On Wed, Sep 24, 2025 at 03:56:23PM +0000, Josua Mayer wrote:
> >> There's really no difference between having child nodes 0-7 and 8 phy 
> >> providers vs. putting 0-7 into a phy cell arg and 1 phy provider. 
> >>
> >> The only difference I see is it is more straight-forward to determine 
> >> what lanes are present in the phy driver if the driver needs to know 
> >> that. But you can also just read all 'phys' properties in the DT with a 
> >> &serdes_1 phandle and determine that. Is that efficient? No, but you 
> >> have to do that exactly once and probably has no measurable impact.
> >>
> >> With that, then can't you simply just add a more specific compatible:
> >>
> >> compatible = "fsl,lx2160a-serdes1", "fsl,lynx-28g";
> >>
> >> Then you maintain some compatibility.
> >>
> >> Rob
> > With the patches that have been presented to you thus far -- yes, this
> > is the correct conclusion, there is not much of a difference. But this
> > is not all.
> >
> > If I want in the future to apply the properties from
> > Documentation/devicetree/bindings/phy/transmit-amplitude.yaml to just
> > one of the lanes, how would I do that with just 1 phy provider?
> I believe it is possible for a driver to create multiple phy objects
> during probe, and for the xlate function to return the correct one.
> 
> Then, whether you follow a phandle to the parent with 1 argument,
> or a  phandle to the phy child with 0 arguments provides same results.
> 
> The driver already creates a phy object for each lane with:
> 
> phy = devm_phy_create(&pdev->dev, NULL, &lynx_28g_ops);
> 
> Once the second argument is changed to a valid lane node,
> it's properties will be accessible.
> 
> I prototyped this a while ago:
> https://github.com/SolidRun/lx2160a_build/blob/develop-ls-5.15.71-2.2.0/patches/linux/0030-phy-lynx-28g-add-support-for-device-tree-per-lane-ph.patch

Ok, so because I did not actually try to prototype this, it seems things
got mixed up in my head and I did not realize it would be possible to
keep forward compatibility of old kernels with new device trees as well.

Essentially, because #phy-cells = <1> goes into the top-level "serdes"
node, and #phy-cells = <0> goes into the child "phy" per-lane nodes, it
becomes possible to superimpose the legacy and the modern bindings onto
the same structure, and have compatible = "fsl,lx2160a-serdes1", "fsl,lynx-28g"
so that each kernel revision picks its own format in a way that doesn't
bother the other.

Because I do care about use cases such as bisections with the same (latest)
device tree blob, I can take this as an action item for v3 and keep bug
compatibility with "fsl,lynx-28g". What I said here:
https://lore.kernel.org/lkml/20250908093709.owcha6ypm5lqqdwz@skbuf/
about "fsl,lynx-28g" being unable to reject unsupported protocols on
SerDes #2 remains valid, but on the premise that it hasn't been a
practical problem for the current mainline users, it seems to not matter
regarding this decision.

For the next revision I will allow "fsl,lynx-28g" as a fallback
compatible for SerDes #1 and #2, but not #3 (i.e. the current mainline
users, but not more), and #phy-cells = <1> will be allowed to be present
in the top-level SerDes node only if "compatible" contains "fsl,lynx-28g".
Otherwise, we need to have #phy-cells = <0> in child "phy" nodes.

Is that ok with everyone?

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

[Rob Herring]

On Wed, Sep 24, 2025 at 06:45:34PM +0300, Vladimir Oltean wrote:
> Hi Rob,
> 
> On Wed, Sep 24, 2025 at 08:54:29AM -0500, Rob Herring wrote:
> > > +description: |
> > 
> > Don't need '|' if no formatting to preserve.
> 
> Thanks, will drop.
> 
> > > +  "#address-cells":
> > > +    const: 1
> > > +    description: "Address cells for child lane nodes"
> > 
> > You don't need generic descriptions of common properties.
> 
> Ok, I'll also drop the description from #size-cells but keep it in
> #phy-cells (less obvious).
> 
> > > +
> > > +  "#size-cells":
> > > +    const: 0
> > > +    description: "Size cells for child lane nodes"
> > > +
> > >    "#phy-cells":
> > > +    description: "Number of cells in PHY specifier (legacy binding only)"
> > >      const: 1
> > >  
> > > @@ -32,9 +124,51 @@ examples:
> > >      soc {
> > >        #address-cells = <2>;
> > >        #size-cells = <2>;
> > > -      serdes_1: phy@1ea0000 {
> > > -        compatible = "fsl,lynx-28g";
> > > +
> > > +      serdes_1: serdes@1ea0000 {
> > > +        compatible = "fsl,lx2160a-serdes1";
> > >          reg = <0x0 0x1ea0000 0x0 0x1e30>;
> > > -        #phy-cells = <1>;
> > > +        #address-cells = <1>;
> > > +        #size-cells = <0>;
> > > +
> > > +        phy@0 {
> > > +          reg = <0>;
> > > +          #phy-cells = <0>;
> > > +        };
> > 
> > There's really no difference between having child nodes 0-7 and 8 phy 
> > providers vs. putting 0-7 into a phy cell arg and 1 phy provider. 
> > 
> > The only difference I see is it is more straight-forward to determine 
> > what lanes are present in the phy driver if the driver needs to know 
> > that. But you can also just read all 'phys' properties in the DT with a 
> > &serdes_1 phandle and determine that. Is that efficient? No, but you 
> > have to do that exactly once and probably has no measurable impact.
> > 
> > With that, then can't you simply just add a more specific compatible:
> > 
> > compatible = "fsl,lx2160a-serdes1", "fsl,lynx-28g";
> > 
> > Then you maintain some compatibility.
> > 
> > Rob
> 
> With the patches that have been presented to you thus far -- yes, this
> is the correct conclusion, there is not much of a difference. But this
> is not all.

That's all I can base my conclusion on if you don't tell me more...

> If I want in the future to apply the properties from
> Documentation/devicetree/bindings/phy/transmit-amplitude.yaml to just
> one of the lanes, how would I do that with just 1 phy provider? It's not
> so clear. Compared to 8 phy providers, each with its OF node => much
> easier to structure and to understand.

That's unfortunate that binding wasn't designed to support more than 
1 instance. You could do:

lane@0 {
  reg = <0>;
  tx-p2p-microvolt = <123>;
};

lane@1 {
  reg = <1>;
  tx-p2p-microvolt = <123>;
};

Yeah, that's about what you had, but it avoids changing the cell size. 
That should be a bit simpler to implement in the driver and to add to 
existing DTs as a fixup (because you don't have to change 'phys' entries 
everywhere).

Another option is go to cell size of 2 and stick the voltage in a cell. 
That approach doesn't work well if you have a 3rd, 4th, etc. cell to add 
later for more properties.

Your overlaying the old and new bindings approach works too. That 
approach is fine with me.

Rob