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

Vladimir Oltean posted 16 patches 1 week, 1 day ago
There is a newer version of this series
[PATCH v2 phy 12/16] dt-bindings: phy: lynx-28g: add compatible strings per SerDes and instantiation
Posted by Vladimir Oltean 1 week, 1 day ago
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
Re: [PATCH v2 phy 12/16] dt-bindings: phy: lynx-28g: add compatible strings per SerDes and instantiation
Posted by Rob Herring 1 week ago
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
Posted by Vladimir Oltean 1 week ago
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
Posted by Rob Herring 6 days, 13 hours ago
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
Re: [PATCH v2 phy 12/16] dt-bindings: phy: lynx-28g: add compatible strings per SerDes and instantiation
Posted by Josua Mayer 1 week ago
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
Posted by Rob Herring (Arm) 1 week, 1 day ago
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
Posted by Vladimir Oltean 1 week, 1 day ago
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.