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

[Vincent Guittot]

Hi,

Sorry for the delayed reply. Some days off kept me away from keyboard

On Thu, 12 Feb 2026 at 22:10, Rob Herring <robh@xxxxxxxxxx> wrote:
>
> 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?

Yes, there are 2 serdes hw ip instances and each instance has 2 lanes
that can be configured to output PCIe and/or SGMII

>
> >
> > >
> > > > +
> > > > +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>;
> };

We only register one phy with PCIE x2 in this case

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

For ethernet, we use pcs-handle and we don't register generic phy as
we don't have anything to do

Also, the mode (if a lane output pcie or sgmii) is a static
configuration that is decided before unresetting the serdes ip, we
don't change the mode at runtime

That being said, we can use phy-type = <PHY_TYPE_PCIE> or
<PHY_TYPE_XPCS> in child node instead of a compatible to describe the
purpose of each lane

>
> 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.

As mentioned previously, we can decide not to use all lanes of a mode.
As an example, using mode 1 for a pcie x1 but no sgmii so we will not
find all lanes description in the DT

Vincent
>
> Rob