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>
---
.../devicetree/bindings/pci/nxp,s32-pcie.yaml | 169 ++++++++++++++++++
1 file changed, 169 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml
diff --git a/Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml b/Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml
new file mode 100644
index 000000000000..287596d7162d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml
@@ -0,0 +1,169 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/nxp,s32-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP S32G2xx/S32G3xx PCIe controller
+
+maintainers:
+ - Bogdan Hamciuc <bogdan.hamciuc@nxp.com>
+ - Ionut Vicovan <ionut.vicovan@nxp.com>
+
+description:
+ This PCIe controller is based on the Synopsys DesignWare PCIe IP.
+ The S32G SoC family has two PCIe controllers, which can be configured as
+ either Root Complex or End Point.
+
+properties:
+ compatible:
+ oneOf:
+ - enum:
+ - nxp,s32g2-pcie # S32G2 SoCs RC mode
+ - items:
+ - const: nxp,s32g3-pcie
+ - const: nxp,s32g2-pcie
+
+ reg:
+ minItems: 7
+ maxItems: 7
+
+ reg-names:
+ items:
+ - const: dbi
+ - const: dbi2
+ - const: atu
+ - const: dma
+ - const: ctrl
+ - const: config
+ - const: addr_space
+
+ interrupts:
+ minItems: 8
+ maxItems: 8
+
+ interrupt-names:
+ items:
+ - const: link_req_stat
+ - const: dma
+ - const: msi
+ - const: phy_link_down
+ - const: phy_link_up
+ - const: misc
+ - const: pcs
+ - const: tlp_req_no_comp
+
+ msi-parent:
+ description:
+ Use this option to reference the GIC controller node which will
+ handle the MSIs. This property can be used only in Root Complex mode.
+ The msi-parent node must be declared as "msi-controller" and the list of
+ available SPIs that can be used must be declared using "mbi-ranges".
+ If "msi-parent" is not present in the PCIe node, MSIs will be handled
+ by iMSI-RX -Integrated MSI Receiver [AXI Bridge]-, an integrated
+ MSI reception module in the PCIe controller's AXI Bridge which
+ detects and terminates inbound MSI requests (received on the RX wire).
+ These MSIs no longer appear on the AXI bus, instead a hard-wired
+ interrupt is raised, documented as "DSP AXI MSI Interrupt" in the SoC
+ Reference Manual.
+
+ nxp,phy-mode:
+ $ref: /schemas/types.yaml#/definitions/string
+ description: Select PHY mode for PCIe controller
+ enum:
+ - crns # Common Reference Clock, No Spread Spectrum
+ - crss # Common Reference Clock, Spread Spectrum
+ - srns # Separate reference Clock, No Spread Spectrum
+ - sris # Separate Reference Clock, Independent Spread Spectrum
+
+ max-link-speed:
+ description:
+ The max link speed is normaly Gen3, but can be enforced to a lower value
+ in case of special limitations.
+ maximum: 3
+
+ num-lanes:
+ description:
+ Max bus width (1 or 2); it is the number of physical lanes
+ minimum: 1
+ maximum: 2
+
+allOf:
+ - $ref: /schemas/pci/snps,dw-pcie-common.yaml#
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - interrupts
+ - interrupt-names
+ - ranges
+ - nxp,phy-mode
+ - num-lanes
+ - phys
+
+additionalProperties: true
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/phy/phy.h>
+
+ bus {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ pcie0: pcie@40400000 {
+ compatible = "nxp,s32g3-pcie",
+ "nxp,s32g2-pcie";
+ dma-coherent;
+ 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";
+ #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>;
+
+ nxp,phy-mode = "crns";
+ 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 0 128 4>,
+ <0 0 0 2 &gic 0 0 0 129 4>,
+ <0 0 0 3 &gic 0 0 0 130 4>,
+ <0 0 0 4 &gic 0 0 0 131 4>;
+ msi-parent = <&gic>;
+
+ num-lanes = <2>;
+ phys = <&serdes0 PHY_TYPE_PCIE 0 0>;
+ };
+ };
--
2.43.0Suggest following convention for subject lines (run "git log --oneline
Documentation/devicetree/bindings/pci/"), e.g.,
dt-bindings: PCI: s32g: Add NXP PCIe controller
On Fri, Sep 12, 2025 at 04:14:33PM +0200, Vincent Guittot wrote:
> Describe the PCIe controller available on the S32G platforms.
> + pcie0: pcie@40400000 {
> + compatible = "nxp,s32g3-pcie",
> + "nxp,s32g2-pcie";
> + dma-coherent;
> + 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";
Looks like an indentation error. Shouldn't "reg-names" and subsequent
properties be aligned under "reg"?
> + #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>;
> +
> + nxp,phy-mode = "crns";
If "nxp,phy-mode" goes with "phys", should it be adjacent to it?
> + 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 0 128 4>,
> + <0 0 0 2 &gic 0 0 0 129 4>,
> + <0 0 0 3 &gic 0 0 0 130 4>,
> + <0 0 0 4 &gic 0 0 0 131 4>;
> + msi-parent = <&gic>;
> +
> + num-lanes = <2>;
> + phys = <&serdes0 PHY_TYPE_PCIE 0 0>;
> + };
> + };
On Wed, 17 Sept 2025 at 23:18, Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> Suggest following convention for subject lines (run "git log --oneline
> Documentation/devicetree/bindings/pci/"), e.g.,
>
> dt-bindings: PCI: s32g: Add NXP PCIe controller
okay
>
> On Fri, Sep 12, 2025 at 04:14:33PM +0200, Vincent Guittot wrote:
> > Describe the PCIe controller available on the S32G platforms.
>
> > + pcie0: pcie@40400000 {
> > + compatible = "nxp,s32g3-pcie",
> > + "nxp,s32g2-pcie";
> > + dma-coherent;
> > + 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";
>
> Looks like an indentation error. Shouldn't "reg-names" and subsequent
> properties be aligned under "reg"?
yeah, that's a mistake.
>
> > + #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>;
> > +
> > + nxp,phy-mode = "crns";
>
> If "nxp,phy-mode" goes with "phys", should it be adjacent to it?
yes, this , phys and num-lane should be together
>
> > + 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 0 128 4>,
> > + <0 0 0 2 &gic 0 0 0 129 4>,
> > + <0 0 0 3 &gic 0 0 0 130 4>,
> > + <0 0 0 4 &gic 0 0 0 131 4>;
> > + msi-parent = <&gic>;
> > +
> > + num-lanes = <2>;
> > + phys = <&serdes0 PHY_TYPE_PCIE 0 0>;
> > + };
> > + };
Hello Vincent, Nice to see you sending some PCIe patches :) Quite different from the scheduler and power management patches that you usually work on :) (snip) > + nxp,phy-mode: > + $ref: /schemas/types.yaml#/definitions/string > + description: Select PHY mode for PCIe controller > + enum: > + - crns # Common Reference Clock, No Spread Spectrum > + - crss # Common Reference Clock, Spread Spectrum > + - srns # Separate reference Clock, No Spread Spectrum > + - sris # Separate Reference Clock, Independent Spread Spectrum This does not seem to be anything NXP specific, so I really think that this should be some kind of generic property. Note that I tried to add a similar property, but for the PCIe endpoint side rather that the PCIe root complex side: https://lore.kernel.org/linux-pci/20250425092012.95418-2-cassel@kernel.org/ However, due to shifting priorities, I haven't been able to follow up with a new version/proposal. My problem is not exactly the same, but even for a root complex, the PCI specification allows the endpoint side to provide the common clock, which means that the root complex would source the refclk from the PCIe slot, so I would say that our problems are quite similar. Rob Herring suggested to use the clock binding rather than an enum. I can see his point of view, but personally I'm not convinced that his suggestion of not having a clock specified means "source the refclock from the slot" is better than a simple enum. To me, it seems way clearer to explicitly specify the mode in device tree, rather than the mode implictly being set if a "clk" phandle is there or not. That approach seems way easier to misunderstand, as the user would need to know that the clocking mode is inferred from a "clk" phandle being there or not. I also note that Rob Herring was not really a fan of having separate spread spectrum options. Instead, it seems like he wanted a separate way to define if SSC was used or not. I have seen the following patch merged: https://github.com/devicetree-org/dt-schema/pull/154 https://github.com/devicetree-org/dt-schema/commit/d7c9156d46bd287f21a5ed3303bea8a4d66d452a So I'm not sure if that is the intended way they want SSC to be defined or not. I apologize for bringing up my own problem in this discussion, but at least it is clear to me that we cannot continue with each PCIe driver adding their own vendor specific properties (with completely different names) for this. Some kind of generic solution is needed, at least for new drivers. Kind regards, Niklas
Hi Niklas, On Wed, 17 Sept 2025 at 10:42, Niklas Cassel <cassel@kernel.org> wrote: > > Hello Vincent, > > Nice to see you sending some PCIe patches :) > > Quite different from the scheduler and power management patches that you > usually work on :) Yeah, It's always interesting to explore different areas > > (snip) > > > + nxp,phy-mode: > > + $ref: /schemas/types.yaml#/definitions/string > > + description: Select PHY mode for PCIe controller > > + enum: > > + - crns # Common Reference Clock, No Spread Spectrum > > + - crss # Common Reference Clock, Spread Spectrum > > + - srns # Separate reference Clock, No Spread Spectrum > > + - sris # Separate Reference Clock, Independent Spread Spectrum > > This does not seem to be anything NXP specific, so I really think that this > should be some kind of generic property. I agree. Thanks for having shared the email threads on the subject. > > > Note that I tried to add a similar property, but for the PCIe endpoint side > rather that the PCIe root complex side: > https://lore.kernel.org/linux-pci/20250425092012.95418-2-cassel@kernel.org/ > > However, due to shifting priorities, I haven't been able to follow up with > a new version/proposal. > > My problem is not exactly the same, but even for a root complex, the PCI > specification allows the endpoint side to provide the common clock, which > means that the root complex would source the refclk from the PCIe slot, > so I would say that our problems are quite similar. yes, they are all the same - common or separate clock - Spread spectrum or not and finally which clock to use as the reference behind internal or external In my case, I only need to know the first 2 items > > Rob Herring suggested to use the clock binding rather than an enum. > I can see his point of view, but personally I'm not convinced that his > suggestion of not having a clock specified means "source the refclock from > the slot" is better than a simple enum. Having a clock binding to define where the clock(s) comes from could be a good way to describe the various ways to provide the ref clock and an empty "ref" clock can suggest using an internal clock for those which have one. But I don't see an easy way to describe common vs separate and with or without spread spectrum. > > To me, it seems way clearer to explicitly specify the mode in device tree, > rather than the mode implictly being set if a "clk" phandle is there or not. I tend to agree that getting the common/separate and w/ or w/o spread spectrum is not straightforward. > That approach seems way easier to misunderstand, as the user would need to > know that the clocking mode is inferred from a "clk" phandle being there or > not. > > > I also note that Rob Herring was not really a fan of having separate spread > spectrum options. Instead, it seems like he wanted a separate way to define > if SSC was used or not. > > I have seen the following patch merged: > https://github.com/devicetree-org/dt-schema/pull/154 > https://github.com/devicetree-org/dt-schema/commit/d7c9156d46bd287f21a5ed3303bea8a4d66d452a > > So I'm not sure if that is the intended way they want SSC to be defined or > not. The above provides much more than what we need as it is mainly a boolean for pcie than characterizing the spread spectrum itself > > > I apologize for bringing up my own problem in this discussion, but at least > it is clear to me that we cannot continue with each PCIe driver adding their > own vendor specific properties (with completely different names) for this. > Some kind of generic solution is needed, at least for new drivers. I agree. Regards, Vincent > > > Kind regards, > Niklas
On Fri, Sep 12, 2025 at 04:14:33PM +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>
> ---
> .../devicetree/bindings/pci/nxp,s32-pcie.yaml | 169 ++++++++++++++++++
> 1 file changed, 169 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml
>
> diff --git a/Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml b/Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml
> new file mode 100644
> index 000000000000..287596d7162d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml
> @@ -0,0 +1,169 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/nxp,s32-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP S32G2xx/S32G3xx PCIe controller
> +
> +maintainers:
> + - Bogdan Hamciuc <bogdan.hamciuc@nxp.com>
> + - Ionut Vicovan <ionut.vicovan@nxp.com>
> +
> +description:
> + This PCIe controller is based on the Synopsys DesignWare PCIe IP.
> + The S32G SoC family has two PCIe controllers, which can be configured as
> + either Root Complex or End Point.
s/End Point/Endpoint/ to match spec usage.
> +properties:
> + compatible:
> + oneOf:
> + - enum:
> + - nxp,s32g2-pcie # S32G2 SoCs RC mode
> + - items:
> + - const: nxp,s32g3-pcie
> + - const: nxp,s32g2-pcie
> +
> + reg:
> + minItems: 7
> + maxItems: 7
> +
> + reg-names:
> + items:
> + - const: dbi
> + - const: dbi2
> + - const: atu
> + - const: dma
> + - const: ctrl
> + - const: config
> + - const: addr_space
> +
> + interrupts:
> + minItems: 8
> + maxItems: 8
> +
> + interrupt-names:
> + items:
> + - const: link_req_stat
> + - const: dma
> + - const: msi
> + - const: phy_link_down
> + - const: phy_link_up
> + - const: misc
> + - const: pcs
> + - const: tlp_req_no_comp
> +
> + msi-parent:
> + description:
> + Use this option to reference the GIC controller node which will
> + handle the MSIs. This property can be used only in Root Complex mode.
> + The msi-parent node must be declared as "msi-controller" and the list of
> + available SPIs that can be used must be declared using "mbi-ranges".
> + If "msi-parent" is not present in the PCIe node, MSIs will be handled
> + by iMSI-RX -Integrated MSI Receiver [AXI Bridge]-, an integrated
> + MSI reception module in the PCIe controller's AXI Bridge which
> + detects and terminates inbound MSI requests (received on the RX wire).
> + These MSIs no longer appear on the AXI bus, instead a hard-wired
> + interrupt is raised, documented as "DSP AXI MSI Interrupt" in the SoC
> + Reference Manual.
> +
> + nxp,phy-mode:
> + $ref: /schemas/types.yaml#/definitions/string
> + description: Select PHY mode for PCIe controller
> + enum:
> + - crns # Common Reference Clock, No Spread Spectrum
> + - crss # Common Reference Clock, Spread Spectrum
> + - srns # Separate reference Clock, No Spread Spectrum
> + - sris # Separate Reference Clock, Independent Spread Spectrum
> +
> + max-link-speed:
> + description:
> + The max link speed is normaly Gen3, but can be enforced to a lower value
> + in case of special limitations.
s/normaly/normally/
But I doubt you need this here at all.
> + maximum: 3
> +
> + num-lanes:
> + description:
> + Max bus width (1 or 2); it is the number of physical lanes
> + minimum: 1
> + maximum: 2
> +
> +allOf:
> + - $ref: /schemas/pci/snps,dw-pcie-common.yaml#
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - interrupts
> + - interrupt-names
> + - ranges
> + - nxp,phy-mode
> + - num-lanes
> + - phys
> +
> +additionalProperties: true
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/phy/phy.h>
> +
> + bus {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + pcie0: pcie@40400000 {
> + compatible = "nxp,s32g3-pcie",
> + "nxp,s32g2-pcie";
> + dma-coherent;
> + 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";
> + #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>;
> +
> + nxp,phy-mode = "crns";
> + 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 0 128 4>,
> + <0 0 0 2 &gic 0 0 0 129 4>,
> + <0 0 0 3 &gic 0 0 0 130 4>,
> + <0 0 0 4 &gic 0 0 0 131 4>;
> + msi-parent = <&gic>;
> +
> + num-lanes = <2>;
> + phys = <&serdes0 PHY_TYPE_PCIE 0 0>;
num-lanes and phys are properties of a Root Port, not the host bridge.
Please put them in a separate stanza. See this for details and
examples:
https://lore.kernel.org/linux-pci/20250625221653.GA1590146@bhelgaas/
> + };
> + };
> --
> 2.43.0
>
On Sat, 13 Sept 2025 at 00:50, Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Fri, Sep 12, 2025 at 04:14:33PM +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>
> > ---
> > .../devicetree/bindings/pci/nxp,s32-pcie.yaml | 169 ++++++++++++++++++
> > 1 file changed, 169 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml b/Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml
> > new file mode 100644
> > index 000000000000..287596d7162d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml
> > @@ -0,0 +1,169 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pci/nxp,s32-pcie.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP S32G2xx/S32G3xx PCIe controller
> > +
> > +maintainers:
> > + - Bogdan Hamciuc <bogdan.hamciuc@nxp.com>
> > + - Ionut Vicovan <ionut.vicovan@nxp.com>
> > +
> > +description:
> > + This PCIe controller is based on the Synopsys DesignWare PCIe IP.
> > + The S32G SoC family has two PCIe controllers, which can be configured as
> > + either Root Complex or End Point.
>
> s/End Point/Endpoint/ to match spec usage.
Ok
>
> > +properties:
> > + compatible:
> > + oneOf:
> > + - enum:
> > + - nxp,s32g2-pcie # S32G2 SoCs RC mode
> > + - items:
> > + - const: nxp,s32g3-pcie
> > + - const: nxp,s32g2-pcie
> > +
> > + reg:
> > + minItems: 7
> > + maxItems: 7
> > +
> > + reg-names:
> > + items:
> > + - const: dbi
> > + - const: dbi2
> > + - const: atu
> > + - const: dma
> > + - const: ctrl
> > + - const: config
> > + - const: addr_space
> > +
> > + interrupts:
> > + minItems: 8
> > + maxItems: 8
> > +
> > + interrupt-names:
> > + items:
> > + - const: link_req_stat
> > + - const: dma
> > + - const: msi
> > + - const: phy_link_down
> > + - const: phy_link_up
> > + - const: misc
> > + - const: pcs
> > + - const: tlp_req_no_comp
> > +
> > + msi-parent:
> > + description:
> > + Use this option to reference the GIC controller node which will
> > + handle the MSIs. This property can be used only in Root Complex mode.
> > + The msi-parent node must be declared as "msi-controller" and the list of
> > + available SPIs that can be used must be declared using "mbi-ranges".
> > + If "msi-parent" is not present in the PCIe node, MSIs will be handled
> > + by iMSI-RX -Integrated MSI Receiver [AXI Bridge]-, an integrated
> > + MSI reception module in the PCIe controller's AXI Bridge which
> > + detects and terminates inbound MSI requests (received on the RX wire).
> > + These MSIs no longer appear on the AXI bus, instead a hard-wired
> > + interrupt is raised, documented as "DSP AXI MSI Interrupt" in the SoC
> > + Reference Manual.
> > +
> > + nxp,phy-mode:
> > + $ref: /schemas/types.yaml#/definitions/string
> > + description: Select PHY mode for PCIe controller
> > + enum:
> > + - crns # Common Reference Clock, No Spread Spectrum
> > + - crss # Common Reference Clock, Spread Spectrum
> > + - srns # Separate reference Clock, No Spread Spectrum
> > + - sris # Separate Reference Clock, Independent Spread Spectrum
> > +
> > + max-link-speed:
> > + description:
> > + The max link speed is normaly Gen3, but can be enforced to a lower value
> > + in case of special limitations.
>
> s/normaly/normally/
>
> But I doubt you need this here at all.
I'm going to remove the description
>
> > + maximum: 3
> > +
> > + num-lanes:
> > + description:
> > + Max bus width (1 or 2); it is the number of physical lanes
> > + minimum: 1
> > + maximum: 2
> > +
> > +allOf:
> > + - $ref: /schemas/pci/snps,dw-pcie-common.yaml#
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - reg-names
> > + - interrupts
> > + - interrupt-names
> > + - ranges
> > + - nxp,phy-mode
> > + - num-lanes
> > + - phys
> > +
> > +additionalProperties: true
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/phy/phy.h>
> > +
> > + bus {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + pcie0: pcie@40400000 {
> > + compatible = "nxp,s32g3-pcie",
> > + "nxp,s32g2-pcie";
> > + dma-coherent;
> > + 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";
> > + #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>;
> > +
> > + nxp,phy-mode = "crns";
> > + 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 0 128 4>,
> > + <0 0 0 2 &gic 0 0 0 129 4>,
> > + <0 0 0 3 &gic 0 0 0 130 4>,
> > + <0 0 0 4 &gic 0 0 0 131 4>;
> > + msi-parent = <&gic>;
> > +
> > + num-lanes = <2>;
> > + phys = <&serdes0 PHY_TYPE_PCIE 0 0>;
>
> num-lanes and phys are properties of a Root Port, not the host bridge.
> Please put them in a separate stanza. See this for details and
> examples:
>
> https://lore.kernel.org/linux-pci/20250625221653.GA1590146@bhelgaas/
Ok, I'm going to have a look
>
> > + };
> > + };
> > --
> > 2.43.0
> >
On Sun, 14 Sept 2025 at 14:35, Vincent Guittot
<vincent.guittot@linaro.org> wrote:
>
> On Sat, 13 Sept 2025 at 00:50, Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > On Fri, Sep 12, 2025 at 04:14:33PM +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>
> > > ---
> > > .../devicetree/bindings/pci/nxp,s32-pcie.yaml | 169 ++++++++++++++++++
> > > 1 file changed, 169 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml b/Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml
> > > new file mode 100644
> > > index 000000000000..287596d7162d
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml
> > > @@ -0,0 +1,169 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/pci/nxp,s32-pcie.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: NXP S32G2xx/S32G3xx PCIe controller
> > > +
> > > +maintainers:
> > > + - Bogdan Hamciuc <bogdan.hamciuc@nxp.com>
> > > + - Ionut Vicovan <ionut.vicovan@nxp.com>
> > > +
> > > +description:
> > > + This PCIe controller is based on the Synopsys DesignWare PCIe IP.
> > > + The S32G SoC family has two PCIe controllers, which can be configured as
> > > + either Root Complex or End Point.
> >
> > s/End Point/Endpoint/ to match spec usage.
>
> Ok
>
> >
> > > +properties:
> > > + compatible:
> > > + oneOf:
> > > + - enum:
> > > + - nxp,s32g2-pcie # S32G2 SoCs RC mode
> > > + - items:
> > > + - const: nxp,s32g3-pcie
> > > + - const: nxp,s32g2-pcie
> > > +
> > > + reg:
> > > + minItems: 7
> > > + maxItems: 7
> > > +
> > > + reg-names:
> > > + items:
> > > + - const: dbi
> > > + - const: dbi2
> > > + - const: atu
> > > + - const: dma
> > > + - const: ctrl
> > > + - const: config
> > > + - const: addr_space
> > > +
> > > + interrupts:
> > > + minItems: 8
> > > + maxItems: 8
> > > +
> > > + interrupt-names:
> > > + items:
> > > + - const: link_req_stat
> > > + - const: dma
> > > + - const: msi
> > > + - const: phy_link_down
> > > + - const: phy_link_up
> > > + - const: misc
> > > + - const: pcs
> > > + - const: tlp_req_no_comp
> > > +
> > > + msi-parent:
> > > + description:
> > > + Use this option to reference the GIC controller node which will
> > > + handle the MSIs. This property can be used only in Root Complex mode.
> > > + The msi-parent node must be declared as "msi-controller" and the list of
> > > + available SPIs that can be used must be declared using "mbi-ranges".
> > > + If "msi-parent" is not present in the PCIe node, MSIs will be handled
> > > + by iMSI-RX -Integrated MSI Receiver [AXI Bridge]-, an integrated
> > > + MSI reception module in the PCIe controller's AXI Bridge which
> > > + detects and terminates inbound MSI requests (received on the RX wire).
> > > + These MSIs no longer appear on the AXI bus, instead a hard-wired
> > > + interrupt is raised, documented as "DSP AXI MSI Interrupt" in the SoC
> > > + Reference Manual.
> > > +
> > > + nxp,phy-mode:
> > > + $ref: /schemas/types.yaml#/definitions/string
> > > + description: Select PHY mode for PCIe controller
> > > + enum:
> > > + - crns # Common Reference Clock, No Spread Spectrum
> > > + - crss # Common Reference Clock, Spread Spectrum
> > > + - srns # Separate reference Clock, No Spread Spectrum
> > > + - sris # Separate Reference Clock, Independent Spread Spectrum
> > > +
> > > + max-link-speed:
> > > + description:
> > > + The max link speed is normaly Gen3, but can be enforced to a lower value
> > > + in case of special limitations.
> >
> > s/normaly/normally/
> >
> > But I doubt you need this here at all.
>
> I'm going to remove the description
>
> >
> > > + maximum: 3
> > > +
> > > + num-lanes:
> > > + description:
> > > + Max bus width (1 or 2); it is the number of physical lanes
> > > + minimum: 1
> > > + maximum: 2
> > > +
> > > +allOf:
> > > + - $ref: /schemas/pci/snps,dw-pcie-common.yaml#
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > + - reg-names
> > > + - interrupts
> > > + - interrupt-names
> > > + - ranges
> > > + - nxp,phy-mode
> > > + - num-lanes
> > > + - phys
> > > +
> > > +additionalProperties: true
> > > +
> > > +examples:
> > > + - |
> > > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > + #include <dt-bindings/phy/phy.h>
> > > +
> > > + bus {
> > > + #address-cells = <2>;
> > > + #size-cells = <2>;
> > > +
> > > + pcie0: pcie@40400000 {
> > > + compatible = "nxp,s32g3-pcie",
> > > + "nxp,s32g2-pcie";
> > > + dma-coherent;
> > > + 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";
> > > + #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>;
> > > +
> > > + nxp,phy-mode = "crns";
> > > + 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 0 128 4>,
> > > + <0 0 0 2 &gic 0 0 0 129 4>,
> > > + <0 0 0 3 &gic 0 0 0 130 4>,
> > > + <0 0 0 4 &gic 0 0 0 131 4>;
> > > + msi-parent = <&gic>;
> > > +
> > > + num-lanes = <2>;
> > > + phys = <&serdes0 PHY_TYPE_PCIE 0 0>;
> >
> > num-lanes and phys are properties of a Root Port, not the host bridge.
> > Please put them in a separate stanza. See this for details and
> > examples:
> >
> > https://lore.kernel.org/linux-pci/20250625221653.GA1590146@bhelgaas/
>
> Ok, I'm going to have a look
This driver relies on dw_pcie_host_init() to get common resources like
num-lane which doesn't look at childs to get num-lane.
I have to keep num-lane in the pcie node. Having this in mind should I
keep phys as well as they are both linked ?
>
> >
> > > + };
> > > + };
> > > --
> > > 2.43.0
> > >
On Tue, Sep 16, 2025 at 10:10:31AM +0200, Vincent Guittot wrote: > On Sun, 14 Sept 2025 at 14:35, Vincent Guittot > <vincent.guittot@linaro.org> wrote: > > On Sat, 13 Sept 2025 at 00:50, Bjorn Helgaas <helgaas@kernel.org> wrote: > > > On Fri, Sep 12, 2025 at 04:14:33PM +0200, Vincent Guittot wrote: > > > > Describe the PCIe controller available on the S32G platforms. > > > > + num-lanes = <2>; > > > > + phys = <&serdes0 PHY_TYPE_PCIE 0 0>; > > > > > > num-lanes and phys are properties of a Root Port, not the host bridge. > > > Please put them in a separate stanza. See this for details and > > > examples: > > > > > > https://lore.kernel.org/linux-pci/20250625221653.GA1590146@bhelgaas/ > > > > Ok, I'm going to have a look > > This driver relies on dw_pcie_host_init() to get common resources like > num-lane which doesn't look at childs to get num-lane. > > I have to keep num-lane in the pcie node. Having this in mind should I > keep phys as well as they are both linked ? Huh, that sounds like an issue in the DWC core. Jingoo, Mani? dw_pcie_host_init() includes several things that assume a single Root Port: num_lanes, of_pci_get_equalization_presets(), dw_pcie_start_link() are all per-Root Port things. Bjorn
On Tue, Sep 16, 2025 at 09:23:13AM GMT, Bjorn Helgaas wrote: > On Tue, Sep 16, 2025 at 10:10:31AM +0200, Vincent Guittot wrote: > > On Sun, 14 Sept 2025 at 14:35, Vincent Guittot > > <vincent.guittot@linaro.org> wrote: > > > On Sat, 13 Sept 2025 at 00:50, Bjorn Helgaas <helgaas@kernel.org> wrote: > > > > On Fri, Sep 12, 2025 at 04:14:33PM +0200, Vincent Guittot wrote: > > > > > Describe the PCIe controller available on the S32G platforms. > > > > > > + num-lanes = <2>; > > > > > + phys = <&serdes0 PHY_TYPE_PCIE 0 0>; > > > > > > > > num-lanes and phys are properties of a Root Port, not the host bridge. > > > > Please put them in a separate stanza. See this for details and > > > > examples: > > > > > > > > https://lore.kernel.org/linux-pci/20250625221653.GA1590146@bhelgaas/ > > > > > > Ok, I'm going to have a look > > > > This driver relies on dw_pcie_host_init() to get common resources like > > num-lane which doesn't look at childs to get num-lane. > > > > I have to keep num-lane in the pcie node. Having this in mind should I > > keep phys as well as they are both linked ? > > Huh, that sounds like an issue in the DWC core. Jingoo, Mani? > > dw_pcie_host_init() includes several things that assume a single Root > Port: num_lanes, of_pci_get_equalization_presets(), > dw_pcie_start_link() are all per-Root Port things. > Yeah, it is a gap right now. We only recently started moving the DWC platforms to per Root Port binding (like Qcom). Unfortunately, I don't have cycles for it atleast this week. - Mani -- மணிவண்ணன் சதாசிவம்
On Wed, Sep 17, 2025 at 10:41:08PM +0530, Manivannan Sadhasivam wrote: > On Tue, Sep 16, 2025 at 09:23:13AM GMT, Bjorn Helgaas wrote: > > On Tue, Sep 16, 2025 at 10:10:31AM +0200, Vincent Guittot wrote: > > > On Sun, 14 Sept 2025 at 14:35, Vincent Guittot > > > <vincent.guittot@linaro.org> wrote: > > > > On Sat, 13 Sept 2025 at 00:50, Bjorn Helgaas <helgaas@kernel.org> wrote: > > > > > On Fri, Sep 12, 2025 at 04:14:33PM +0200, Vincent Guittot wrote: > > > > > > Describe the PCIe controller available on the S32G platforms. > > > > > > > > + num-lanes = <2>; > > > > > > + phys = <&serdes0 PHY_TYPE_PCIE 0 0>; > > > > > > > > > > num-lanes and phys are properties of a Root Port, not the host bridge. > > > > > Please put them in a separate stanza. See this for details and > > > > > examples: > > > > > > > > > > https://lore.kernel.org/linux-pci/20250625221653.GA1590146@bhelgaas/ > > > > > > > > Ok, I'm going to have a look > > > > > > This driver relies on dw_pcie_host_init() to get common resources like > > > num-lane which doesn't look at childs to get num-lane. > > > > > > I have to keep num-lane in the pcie node. Having this in mind should I > > > keep phys as well as they are both linked ? > > Huh, that sounds like an issue in the DWC core. Jingoo, Mani? > > > > dw_pcie_host_init() includes several things that assume a single Root > > Port: num_lanes, of_pci_get_equalization_presets(), > > dw_pcie_start_link() are all per-Root Port things. > > Yeah, it is a gap right now. We only recently started moving the DWC > platforms to per Root Port binding (like Qcom). Do you need num-lanes in the devicetree? dw_pcie_link_get_max_link_width() will read it from PCI_EXP_LNKCAP, so if that works maybe you can omit it from the binding? If you do need num-lanes in the binding, maybe you could make a Root Port parser similar to mvebu_pcie_parse_port() or qcom_pcie_parse_port() that would get num-lanes, the PHY, and nxp,phy-mode from a Root Port node? Then all this would be in one place, and if you set ->num_lanes there it looks like the DWC core wouldn't do anything with it.
On Wed, Sep 17, 2025 at 04:28:33PM -0500, Bjorn Helgaas wrote: > On Wed, Sep 17, 2025 at 10:41:08PM +0530, Manivannan Sadhasivam wrote: > > On Tue, Sep 16, 2025 at 09:23:13AM GMT, Bjorn Helgaas wrote: > > > On Tue, Sep 16, 2025 at 10:10:31AM +0200, Vincent Guittot wrote: > > > > On Sun, 14 Sept 2025 at 14:35, Vincent Guittot > > > > <vincent.guittot@linaro.org> wrote: > > > > > On Sat, 13 Sept 2025 at 00:50, Bjorn Helgaas <helgaas@kernel.org> wrote: > > > > > > On Fri, Sep 12, 2025 at 04:14:33PM +0200, Vincent Guittot wrote: > > > > > > > Describe the PCIe controller available on the S32G platforms. > > > > > > > > > > + num-lanes = <2>; > > > > > > > + phys = <&serdes0 PHY_TYPE_PCIE 0 0>; > > > > > > > > > > > > num-lanes and phys are properties of a Root Port, not the host bridge. > > > > > > Please put them in a separate stanza. See this for details and > > > > > > examples: > > > > > > > > > > > > https://lore.kernel.org/linux-pci/20250625221653.GA1590146@bhelgaas/ > > > > > > > > > > Ok, I'm going to have a look > > > > > > > > This driver relies on dw_pcie_host_init() to get common resources like > > > > num-lane which doesn't look at childs to get num-lane. > > > > > > > > I have to keep num-lane in the pcie node. Having this in mind should I > > > > keep phys as well as they are both linked ? > > > > Huh, that sounds like an issue in the DWC core. Jingoo, Mani? > > > > > > dw_pcie_host_init() includes several things that assume a single Root > > > Port: num_lanes, of_pci_get_equalization_presets(), > > > dw_pcie_start_link() are all per-Root Port things. > > > > Yeah, it is a gap right now. We only recently started moving the DWC > > platforms to per Root Port binding (like Qcom). > > Do you need num-lanes in the devicetree? > dw_pcie_link_get_max_link_width() will read it from PCI_EXP_LNKCAP, so > if that works maybe you can omit it from the binding? > 'num-lanes' is an optional property. But we do need it to be specified in devicetree so that the drivers can set proper MLW, Link Width and Link Control settings if the default values are wrong. > If you do need num-lanes in the binding, maybe you could make a Root > Port parser similar to mvebu_pcie_parse_port() or > qcom_pcie_parse_port() that would get num-lanes, the PHY, and > nxp,phy-mode from a Root Port node? > Yeah, we need to have a similar parser. - Mani -- மணிவண்ணன் சதாசிவம்
On Wed, 17 Sept 2025 at 23:28, Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Wed, Sep 17, 2025 at 10:41:08PM +0530, Manivannan Sadhasivam wrote: > > On Tue, Sep 16, 2025 at 09:23:13AM GMT, Bjorn Helgaas wrote: > > > On Tue, Sep 16, 2025 at 10:10:31AM +0200, Vincent Guittot wrote: > > > > On Sun, 14 Sept 2025 at 14:35, Vincent Guittot > > > > <vincent.guittot@linaro.org> wrote: > > > > > On Sat, 13 Sept 2025 at 00:50, Bjorn Helgaas <helgaas@kernel.org> wrote: > > > > > > On Fri, Sep 12, 2025 at 04:14:33PM +0200, Vincent Guittot wrote: > > > > > > > Describe the PCIe controller available on the S32G platforms. > > > > > > > > > > + num-lanes = <2>; > > > > > > > + phys = <&serdes0 PHY_TYPE_PCIE 0 0>; > > > > > > > > > > > > num-lanes and phys are properties of a Root Port, not the host bridge. > > > > > > Please put them in a separate stanza. See this for details and > > > > > > examples: > > > > > > > > > > > > https://lore.kernel.org/linux-pci/20250625221653.GA1590146@bhelgaas/ > > > > > > > > > > Ok, I'm going to have a look > > > > > > > > This driver relies on dw_pcie_host_init() to get common resources like > > > > num-lane which doesn't look at childs to get num-lane. > > > > > > > > I have to keep num-lane in the pcie node. Having this in mind should I > > > > keep phys as well as they are both linked ? > > > > Huh, that sounds like an issue in the DWC core. Jingoo, Mani? > > > > > > dw_pcie_host_init() includes several things that assume a single Root > > > Port: num_lanes, of_pci_get_equalization_presets(), > > > dw_pcie_start_link() are all per-Root Port things. > > > > Yeah, it is a gap right now. We only recently started moving the DWC > > platforms to per Root Port binding (like Qcom). > > Do you need num-lanes in the devicetree? > dw_pcie_link_get_max_link_width() will read it from PCI_EXP_LNKCAP, so > if that works maybe you can omit it from the binding? num-lane is not mandatory but we can have 1 or 2 lanes so should be able to restrict to only 1 lane for some platform the "num-lanes = <2>;" in the example is wrong as we don't need it in case of 2 lanes but only if we want to restrict to 1 lane > > If you do need num-lanes in the binding, maybe you could make a Root > Port parser similar to mvebu_pcie_parse_port() or > qcom_pcie_parse_port() that would get num-lanes, the PHY, and > nxp,phy-mode from a Root Port node? > > Then all this would be in one place, and if you set ->num_lanes there > it looks like the DWC core wouldn't do anything with it.
On Fri, Sep 12, 2025 at 04:14:33PM +0200, Vincent Guittot wrote:
> Describe the PCIe controller available on the S32G platforms.
can you cc imx@lists.linux.dev next time? suppose most part is similar with
imx and layerscape chips.
>
> 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>
> ---
> .../devicetree/bindings/pci/nxp,s32-pcie.yaml | 169 ++++++++++++++++++
> 1 file changed, 169 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml
>
> diff --git a/Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml b/Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml
> new file mode 100644
> index 000000000000..287596d7162d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml
> @@ -0,0 +1,169 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/nxp,s32-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP S32G2xx/S32G3xx PCIe controller
> +
> +maintainers:
> + - Bogdan Hamciuc <bogdan.hamciuc@nxp.com>
> + - Ionut Vicovan <ionut.vicovan@nxp.com>
> +
> +description:
> + This PCIe controller is based on the Synopsys DesignWare PCIe IP.
> + The S32G SoC family has two PCIe controllers, which can be configured as
> + either Root Complex or End Point.
> +
> +properties:
> + compatible:
> + oneOf:
> + - enum:
> + - nxp,s32g2-pcie # S32G2 SoCs RC mode
> + - items:
> + - const: nxp,s32g3-pcie
> + - const: nxp,s32g2-pcie
> +
> + reg:
> + minItems: 7
If minItems is the same maxItems, needn't minItems.
> + maxItems: 7
> +
> + reg-names:
> + items:
> + - const: dbi
> + - const: dbi2
> + - const: atu
> + - const: dma
> + - const: ctrl
> + - const: config
> + - const: addr_space
> +
> + interrupts:
> + minItems: 8
> + maxItems: 8
> +
> + interrupt-names:
> + items:
> + - const: link_req_stat
> + - const: dma
> + - const: msi
> + - const: phy_link_down
> + - const: phy_link_up
> + - const: misc
> + - const: pcs
> + - const: tlp_req_no_comp
use - for names
> +
> + msi-parent:
> + description:
> + Use this option to reference the GIC controller node which will
> + handle the MSIs. This property can be used only in Root Complex mode.
> + The msi-parent node must be declared as "msi-controller" and the list of
> + available SPIs that can be used must be declared using "mbi-ranges".
> + If "msi-parent" is not present in the PCIe node, MSIs will be handled
> + by iMSI-RX -Integrated MSI Receiver [AXI Bridge]-, an integrated
> + MSI reception module in the PCIe controller's AXI Bridge which
> + detects and terminates inbound MSI requests (received on the RX wire).
> + These MSIs no longer appear on the AXI bus, instead a hard-wired
> + interrupt is raised, documented as "DSP AXI MSI Interrupt" in the SoC
> + Reference Manual.
Don't need description for this common property.
msi-parent for pcie devices is most likely wrong. It should use msi-map.
> +
> + nxp,phy-mode:
> + $ref: /schemas/types.yaml#/definitions/string
> + description: Select PHY mode for PCIe controller
> + enum:
> + - crns # Common Reference Clock, No Spread Spectrum
> + - crss # Common Reference Clock, Spread Spectrum
> + - srns # Separate reference Clock, No Spread Spectrum
> + - sris # Separate Reference Clock, Independent Spread Spectrum
> +
> + max-link-speed:
> + description:
> + The max link speed is normaly Gen3, but can be enforced to a lower value
> + in case of special limitations.
needn't description here.
> + maximum: 3
> +
> + num-lanes:
> + description:
> + Max bus width (1 or 2); it is the number of physical lanes
needn't description here.
> + minimum: 1
> + maximum: 2
> +
> +allOf:
> + - $ref: /schemas/pci/snps,dw-pcie-common.yaml#
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - interrupts
> + - interrupt-names
> + - ranges
> + - nxp,phy-mode
> + - num-lanes
> + - phys
> +
> +additionalProperties: true
unevaluatedProperties: false
because you refs to /schemas/pci/snps,dw-pcie-common.yaml
You can send to me do internal review before you post to upstream.
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/phy/phy.h>
> +
> + bus {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + pcie0: pcie@40400000 {
Needn't label "pcie0"
> + compatible = "nxp,s32g3-pcie",
> + "nxp,s32g2-pcie";
> + dma-coherent;
> + 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";
> + #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>;
> +
> + nxp,phy-mode = "crns";
> + 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 0 128 4>,
> + <0 0 0 2 &gic 0 0 0 129 4>,
> + <0 0 0 3 &gic 0 0 0 130 4>,
> + <0 0 0 4 &gic 0 0 0 131 4>;
use pre define macro
<0 0 0 1 &gic GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>,
> + msi-parent = <&gic>;
Suppose it is wrong for pcie
you should use msi-map
Frank
> +
> + num-lanes = <2>;
> + phys = <&serdes0 PHY_TYPE_PCIE 0 0>;
> + };
> + };
> --
> 2.43.0
>
On Fri, 12 Sept 2025 at 22:51, Frank Li <Frank.li@nxp.com> wrote:
>
> On Fri, Sep 12, 2025 at 04:14:33PM +0200, Vincent Guittot wrote:
> > Describe the PCIe controller available on the S32G platforms.
>
> can you cc imx@lists.linux.dev next time? suppose most part is similar with
> imx and layerscape chips.
Ok will do
>
> >
> > 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>
> > ---
> > .../devicetree/bindings/pci/nxp,s32-pcie.yaml | 169 ++++++++++++++++++
> > 1 file changed, 169 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml b/Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml
> > new file mode 100644
> > index 000000000000..287596d7162d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/nxp,s32-pcie.yaml
> > @@ -0,0 +1,169 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pci/nxp,s32-pcie.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP S32G2xx/S32G3xx PCIe controller
> > +
> > +maintainers:
> > + - Bogdan Hamciuc <bogdan.hamciuc@nxp.com>
> > + - Ionut Vicovan <ionut.vicovan@nxp.com>
> > +
> > +description:
> > + This PCIe controller is based on the Synopsys DesignWare PCIe IP.
> > + The S32G SoC family has two PCIe controllers, which can be configured as
> > + either Root Complex or End Point.
> > +
> > +properties:
> > + compatible:
> > + oneOf:
> > + - enum:
> > + - nxp,s32g2-pcie # S32G2 SoCs RC mode
> > + - items:
> > + - const: nxp,s32g3-pcie
> > + - const: nxp,s32g2-pcie
> > +
> > + reg:
> > + minItems: 7
>
> If minItems is the same maxItems, needn't minItems.
Ok, I didn't know that
>
> > + maxItems: 7
> > +
> > + reg-names:
> > + items:
> > + - const: dbi
> > + - const: dbi2
> > + - const: atu
> > + - const: dma
> > + - const: ctrl
> > + - const: config
> > + - const: addr_space
> > +
> > + interrupts:
> > + minItems: 8
> > + maxItems: 8
> > +
> > + interrupt-names:
> > + items:
> > + - const: link_req_stat
> > + - const: dma
> > + - const: msi
> > + - const: phy_link_down
> > + - const: phy_link_up
> > + - const: misc
> > + - const: pcs
> > + - const: tlp_req_no_comp
>
> use - for names
Yes, I forgot to change this
>
> > +
> > + msi-parent:
> > + description:
> > + Use this option to reference the GIC controller node which will
> > + handle the MSIs. This property can be used only in Root Complex mode.
> > + The msi-parent node must be declared as "msi-controller" and the list of
> > + available SPIs that can be used must be declared using "mbi-ranges".
> > + If "msi-parent" is not present in the PCIe node, MSIs will be handled
> > + by iMSI-RX -Integrated MSI Receiver [AXI Bridge]-, an integrated
> > + MSI reception module in the PCIe controller's AXI Bridge which
> > + detects and terminates inbound MSI requests (received on the RX wire).
> > + These MSIs no longer appear on the AXI bus, instead a hard-wired
> > + interrupt is raised, documented as "DSP AXI MSI Interrupt" in the SoC
> > + Reference Manual.
>
> Don't need description for this common property.
>
> msi-parent for pcie devices is most likely wrong. It should use msi-map.
Ok, I'm going to have a look.
>
> > +
> > + nxp,phy-mode:
> > + $ref: /schemas/types.yaml#/definitions/string
> > + description: Select PHY mode for PCIe controller
> > + enum:
> > + - crns # Common Reference Clock, No Spread Spectrum
> > + - crss # Common Reference Clock, Spread Spectrum
> > + - srns # Separate reference Clock, No Spread Spectrum
> > + - sris # Separate Reference Clock, Independent Spread Spectrum
> > +
> > + max-link-speed:
> > + description:
> > + The max link speed is normaly Gen3, but can be enforced to a lower value
> > + in case of special limitations.
>
> needn't description here.
ok
>
> > + maximum: 3
> > +
> > + num-lanes:
> > + description:
> > + Max bus width (1 or 2); it is the number of physical lanes
>
> needn't description here.
ok
>
> > + minimum: 1
> > + maximum: 2
> > +
> > +allOf:
> > + - $ref: /schemas/pci/snps,dw-pcie-common.yaml#
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - reg-names
> > + - interrupts
> > + - interrupt-names
> > + - ranges
> > + - nxp,phy-mode
> > + - num-lanes
> > + - phys
> > +
> > +additionalProperties: true
>
> unevaluatedProperties: false
>
> because you refs to /schemas/pci/snps,dw-pcie-common.yaml
>
> You can send to me do internal review before you post to upstream.
ok, thanks
>
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/phy/phy.h>
> > +
> > + bus {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + pcie0: pcie@40400000 {
>
> Needn't label "pcie0"
ok
>
> > + compatible = "nxp,s32g3-pcie",
> > + "nxp,s32g2-pcie";
> > + dma-coherent;
> > + 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";
> > + #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>;
> > +
> > + nxp,phy-mode = "crns";
> > + 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 0 128 4>,
> > + <0 0 0 2 &gic 0 0 0 129 4>,
> > + <0 0 0 3 &gic 0 0 0 130 4>,
> > + <0 0 0 4 &gic 0 0 0 131 4>;
>
> use pre define macro
>
> <0 0 0 1 &gic GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>,
yes
>
> > + msi-parent = <&gic>;
>
> Suppose it is wrong for pcie
>
> you should use msi-map
>
> Frank
> > +
> > + num-lanes = <2>;
> > + phys = <&serdes0 PHY_TYPE_PCIE 0 0>;
> > + };
> > + };
> > --
> > 2.43.0
> >
© 2016 - 2025 Red Hat, Inc.