Re: [PATCH 1/4 v2] dt-bindings: serdes: s32g: Add NXP serdes subsystem

[Rob Herring]

On Thu, Feb 12, 2026 at 1:17 AM Vincent Guittot
<vincent.guittot@xxxxxxxxxx> wrote:
>
> On Tue, 10 Feb 2026 at 01:40, Rob Herring <robh@xxxxxxxxxx> wrote:
> >
> > On Tue, Feb 03, 2026 at 05:19:14PM +0100, Vincent Guittot wrote:
> > > Describe the serdes subsystem available on the S32G platforms.
> > >
> > > Signed-off-by: Vincent Guittot <vincent.guittot@xxxxxxxxxx>
> > > ---
> > >  .../bindings/phy/nxp,s32g-serdes.yaml         | 154 ++++++++++++++++++
> > >  1 file changed, 154 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/phy/nxp,s32g-serdes.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/phy/nxp,s32g-serdes.yaml b/Documentation/devicetree/bindings/phy/nxp,s32g-serdes.yaml
> > > new file mode 100644
> > > index 000000000000..fad34bee2a4f
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/phy/nxp,s32g-serdes.yaml
> > > @@ -0,0 +1,154 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/phy/nxp,s32g-serdes.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: NXP S32G2xxx/S32G3xxx SerDes PHY subsystem
> > > +
> > > +maintainers:
> > > +  - Ghennadi Procopciuc <ghennadi.procopciuc@xxxxxxx>
> > > +
> > > +description: |
> > > +  The SerDes subsystem on S32G SoC Family includes two types of PHYs:
> > > +    - One PCIe PHY: Supports various PCIe operation modes
> > > +    - Two Ethernet Physical Coding Sublayer (XPCS) controllers
> > > +
> > > +  SerDes operation mode selects the enabled PHYs and speeds. Clock frequency
> > > +  must be adapted accordingly. Below table describes all possible operation
> > > +  modes.
> > > +
> > > +  Mode  PCIe XPCS0           XPCS1           PHY clock       Description
> > > +                SGMII                SGMII             (MHz)
> > > +  -------------------------------------------------------------------------
> > > +  0  Gen3    N/A             N/A             100             Single PCIe
> > > +  1  Gen2    1.25Gbps        N/A             100             PCIe/SGMII
> > > +  2  Gen2    N/A             1.25Gbps        100             PCIe/SGMII
> > > +  3  N/A     1.25Gbps        1.25Gbps        100,125         SGMII
> > > +  4  N/A     3.125/1.25Gbps  3.125/1.25Gbps  125             SGMII
> > > +  5  Gen2    N/A             3.125Gbps       100             PCIe/SGMII
> >
> > Mixed tabs and spaces. Drop the tabs.
>
> okay
>
> >
> > What's not clear to me is do you have 2 or 4 lanes?
>
> 2 lanes per serdes
> as an example mode 0 is one PCIe x2 lane
> and mode 1 is one PCIe x1 and one xpcs0/SGMII on lane 1
> or mode 3 is one  xpcs0/SGMII on lane 0 and one xpcs1/SGMII on lane 1

Still confused. So 2 total lanes?

>
> >
> > > +
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - enum:
> > > +          - nxp,s32g2-serdes
> > > +      - items:
> > > +          - const: nxp,s32g3-serdes
> > > +          - const: nxp,s32g2-serdes
> > > +
> > > +  reg:
> > > +    maxItems: 4
> > > +
> > > +  reg-names:
> > > +    items:
> > > +      - const: ss_pcie
> > > +      - const: pcie_phy
> > > +      - const: xpcs0
> > > +      - const: xpcs1
> > > +
> > > +  clocks:
> > > +    minItems: 4
> > > +    maxItems: 5
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: axi
> > > +      - const: aux
> > > +      - const: apb
> > > +      - const: ref
> > > +      - const: ext
> > > +    minItems: 4
> > > +
> > > +  resets:
> > > +    maxItems: 2
> > > +
> > > +  reset-names:
> > > +    items:
> > > +      - const: serdes
> > > +      - const: pcie
> > > +
> > > +  nxp,sys-mode:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> >
> >        maximum: 5
> >
> > Though isn't this redundant with the child nodes? You could use the
> > standard 'phy-mode' property in each child.
>
> not really because we can have mode 1 but only a node to describe
> lane0 for PCIe x1 if the lane 1 is not used
>
> >
> > > +    description: |
> > > +      SerDes operational mode. See above table for possible values.
> > > +
> > > +  '#address-cells':
> > > +    const: 1
> > > +
> > > +  '#size-cells':
> > > +    const: 0
> > > +
> > > +patternProperties:
> > > +  '^serdes[0,1]_lane@[0,1]$':
> >
> > Do you need to support serdes0_lane@0 and serdes1_lane@0 (or similar
> > with "@1")? That's illegal as you have 2 nodes with the same address.
>
> okay, we can find other naming
>
> >
> > > +    description:
> > > +      Describe a serdes lane.
> > > +    type: object
> > > +
> > > +    properties:
> > > +      compatible:
> > > +        enum:
> > > +          - nxp,s32g2-serdes-pcie-phy
> > > +          - nxp,s32g2-serdes-xpcs
> >
> > Seems like phy-mode would be sufficient. Are these separate blocks from
> > the parent?
>
> Isn't phy-mode only for ethernet phy ?

Sorry, it is "phy-type" that I was thinking about. That takes the
types defined in dt-bindings/phy/phy.h. The type can be defined either
in "phy-type" or in the phy cells if the type is per identifier.

Really, Given each lane doesn't have any of its own resources, I'd
probably get rid of the child nodes and put the type into the phy
cells. Then you'd have something like this:

// PCIE on lanes 0 and 1 (mode 0)
pcie {
  phys = <&phy 0 PHY_TYPE_PCIE>, <&phy 1 PHY_TYPE_PCIE>;
};

// PCIE on lane 0 (mode 1)
pcie {
  phys = <&phy 0 PHY_TYPE_PCIE>;
};
// Ethernet on lane 1
ethernet {
  phys = <&phy 1 PHY_TYPE_SGMII>;
};

I perhaps don't have the cells right if it is more than just lane 0
and lane 1, but you can put anything there you want. The cell
definition is provider specific.

If you need to get the overall system wide configuration, that can be
done. It's not terribly efficient, but you can iterate all 'phys'
nodes in the DT, find the ones for your provider (&phy) and examine
the cell values.

Rob