Re: [PATCH 1/3 v2] dt-bindings: PCI: s32g: Add NXP PCIe controller

[Frank Li <Frank.li@nxp.com>]

On Tue, Sep 23, 2025 at 04:49:13PM +0200, Vincent Guittot wrote:
> On Fri, 19 Sept 2025 at 18:39, Frank Li <Frank.li@nxp.com> wrote:
> >
> > On Fri, Sep 19, 2025 at 05:58:19PM +0200, Vincent Guittot wrote:
> > > Describe the PCIe controller available on the S32G platforms.
> > >
> > > Co-developed-by: Ionut Vicovan <Ionut.Vicovan@nxp.com>
> > > Signed-off-by: Ionut Vicovan <Ionut.Vicovan@nxp.com>
> > > Co-developed-by: Bogdan-Gabriel Roman <bogdan-gabriel.roman@nxp.com>
> > > Signed-off-by: Bogdan-Gabriel Roman <bogdan-gabriel.roman@nxp.com>
> > > Co-developed-by: Larisa Grigore <larisa.grigore@nxp.com>
> > > Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>
> > > Co-developed-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com>
> > > Signed-off-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com>
> > > Co-developed-by: Ciprian Marian Costea <ciprianmarian.costea@nxp.com>
> > > Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@nxp.com>
> > > Co-developed-by: Bogdan Hamciuc <bogdan.hamciuc@nxp.com>
> > > Signed-off-by: Bogdan Hamciuc <bogdan.hamciuc@nxp.com>
> > > Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> > > ---
> > ...
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - reg-names
> > > +  - interrupts
> > > +  - interrupt-names
> > > +  - ranges
> > > +  - phys
> > > +
> > > +allOf:
> > > +  - $ref: /schemas/pci/snps,dw-pcie-common.yaml#
> > > +  - $ref: /schemas/pci/pci-bus.yaml#
> >
> > why not snps,dw-pcie.yaml?
>
> dt binding check reports a number errors and warnings with snps,dw-pcie.yaml.
> In addition to the reg and irq names which I can't all map on the
> snps,dw-pcie.yaml, it reports unevaluated properties which I don't
> have with schemas/pci/pci-bus.yaml
>

Then you need update snps,dw-pcie.yaml, match as much as possible what
defined in snps,dw-pcie.yaml, which try to avoid every vendor define their
difference names.

Frank

>
>
> >
> > Frank
> > > +
> > > +unevaluatedProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > +    #include <dt-bindings/phy/phy.h>
> > > +
> > > +    bus {
> > > +        #address-cells = <2>;
> > > +        #size-cells = <2>;
> > > +
> > > +        pcie@40400000 {
> > > +            compatible = "nxp,s32g3-pcie",
> > > +                         "nxp,s32g2-pcie";
> > > +            reg = <0x00 0x40400000 0x0 0x00001000>,   /* dbi registers */
> > > +                  <0x00 0x40420000 0x0 0x00001000>,   /* dbi2 registers */
> > > +                  <0x00 0x40460000 0x0 0x00001000>,   /* atu registers */
> > > +                  <0x00 0x40470000 0x0 0x00001000>,   /* dma registers */
> > > +                  <0x00 0x40481000 0x0 0x000000f8>,   /* ctrl registers */
> > > +                  /*
> > > +                   * RC configuration space, 4KB each for cfg0 and cfg1
> > > +                   * at the end of the outbound memory map
> > > +                   */
> > > +                  <0x5f 0xffffe000 0x0 0x00002000>,
> > > +                  <0x58 0x00000000 0x0 0x40000000>; /* 1GB EP addr space */
> > > +            reg-names = "dbi", "dbi2", "atu", "dma", "ctrl",
> > > +                        "config", "addr_space";
> > > +            dma-coherent;
> > > +            #address-cells = <3>;
> > > +            #size-cells = <2>;
> > > +            device_type = "pci";
> > > +            ranges =
> > > +                  /*
> > > +                   * downstream I/O, 64KB and aligned naturally just
> > > +                   * before the config space to minimize fragmentation
> > > +                   */
> > > +                  <0x81000000 0x0 0x00000000 0x5f 0xfffe0000 0x0 0x00010000>,
> > > +                  /*
> > > +                   * non-prefetchable memory, with best case size and
> > > +                   * alignment
> > > +                   */
> > > +                  <0x82000000 0x0 0x00000000 0x58 0x00000000 0x7 0xfffe0000>;
> > > +
> > > +            bus-range = <0x0 0xff>;
> > > +            interrupts =  <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
> > > +                          <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
> > > +                          <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
> > > +                          <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>,
> > > +                          <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>,
> > > +                          <GIC_SPI 132 IRQ_TYPE_LEVEL_HIGH>,
> > > +                          <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>,
> > > +                          <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>;
> > > +            interrupt-names = "link-req-stat", "dma", "msi",
> > > +                              "phy-link-down", "phy-link-up", "misc",
> > > +                              "pcs", "tlp-req-no-comp";
> > > +            #interrupt-cells = <1>;
> > > +            interrupt-map-mask = <0 0 0 0x7>;
> > > +            interrupt-map = <0 0 0 1 &gic 0 0 GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>,
> > > +                            <0 0 0 2 &gic 0 0 GIC_SPI 129 IRQ_TYPE_LEVEL_HIGH>,
> > > +                            <0 0 0 3 &gic 0 0 GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>,
> > > +                            <0 0 0 4 &gic 0 0 GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
> > > +
> > > +            phys = <&serdes0 PHY_TYPE_PCIE 0 0>;
> > > +        };
> > > +    };
> > > --
> > > 2.43.0
> > >