2022-10-19 15:07:44

by Havalige, Thippeswamy

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: PCI: xilinx-pcie: Convert to YAML schemas of Xilinx AXI PCIe Root Port Bridge

Convert to YAML dtschemas of Xilinx AXI PCIe Root Port Bridge
dt binding.

Signed-off-by: Thippeswamy Havalige <[email protected]>
---
.../devicetree/bindings/pci/xilinx-pcie.txt | 88 -------------------
.../devicetree/bindings/pci/xilinx-pcie.yaml | 81 +++++++++++++++++
2 files changed, 81 insertions(+), 88 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/pci/xilinx-pcie.txt
create mode 100644 Documentation/devicetree/bindings/pci/xilinx-pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/xilinx-pcie.txt b/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
deleted file mode 100644
index fd57a81180a4..000000000000
--- a/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
+++ /dev/null
@@ -1,88 +0,0 @@
-* Xilinx AXI PCIe Root Port Bridge DT description
-
-Required properties:
-- #address-cells: Address representation for root ports, set to <3>
-- #size-cells: Size representation for root ports, set to <2>
-- #interrupt-cells: specifies the number of cells needed to encode an
- interrupt source. The value must be 1.
-- compatible: Should contain "xlnx,axi-pcie-host-1.00.a"
-- reg: Should contain AXI PCIe registers location and length
-- device_type: must be "pci"
-- interrupts: Should contain AXI PCIe interrupt
-- interrupt-map-mask,
- interrupt-map: standard PCI properties to define the mapping of the
- PCI interface to interrupt numbers.
-- ranges: ranges for the PCI memory regions (I/O space region is not
- supported by hardware)
- Please refer to the standard PCI bus binding document for a more
- detailed explanation
-
-Optional properties for Zynq/Microblaze:
-- bus-range: PCI bus numbers covered
-
-Interrupt controller child node
-+++++++++++++++++++++++++++++++
-Required properties:
-- interrupt-controller: identifies the node as an interrupt controller
-- #address-cells: specifies the number of cells needed to encode an
- address. The value must be 0.
-- #interrupt-cells: specifies the number of cells needed to encode an
- interrupt source. The value must be 1.
-
-NOTE:
-The core provides a single interrupt for both INTx/MSI messages. So,
-created a interrupt controller node to support 'interrupt-map' DT
-functionality. The driver will create an IRQ domain for this map, decode
-the four INTx interrupts in ISR and route them to this domain.
-
-
-Example:
-++++++++
-Zynq:
- pci_express: axi-pcie@50000000 {
- #address-cells = <3>;
- #size-cells = <2>;
- #interrupt-cells = <1>;
- compatible = "xlnx,axi-pcie-host-1.00.a";
- reg = < 0x50000000 0x1000000 >;
- device_type = "pci";
- interrupts = < 0 52 4 >;
- interrupt-map-mask = <0 0 0 7>;
- interrupt-map = <0 0 0 1 &pcie_intc 1>,
- <0 0 0 2 &pcie_intc 2>,
- <0 0 0 3 &pcie_intc 3>,
- <0 0 0 4 &pcie_intc 4>;
- ranges = < 0x02000000 0 0x60000000 0x60000000 0 0x10000000 >;
-
- pcie_intc: interrupt-controller {
- interrupt-controller;
- #address-cells = <0>;
- #interrupt-cells = <1>;
- };
- };
-
-
-Microblaze:
- pci_express: axi-pcie@10000000 {
- #address-cells = <3>;
- #size-cells = <2>;
- #interrupt-cells = <1>;
- compatible = "xlnx,axi-pcie-host-1.00.a";
- reg = <0x10000000 0x4000000>;
- device_type = "pci";
- interrupt-parent = <&microblaze_0_intc>;
- interrupts = <1 2>;
- interrupt-map-mask = <0 0 0 7>;
- interrupt-map = <0 0 0 1 &pcie_intc 1>,
- <0 0 0 2 &pcie_intc 2>,
- <0 0 0 3 &pcie_intc 3>,
- <0 0 0 4 &pcie_intc 4>;
- ranges = <0x02000000 0x00000000 0x80000000 0x80000000 0x00000000 0x10000000>;
-
- pcie_intc: interrupt-controller {
- interrupt-controller;
- #address-cells = <0>;
- #interrupt-cells = <1>;
- };
-
- };
diff --git a/Documentation/devicetree/bindings/pci/xilinx-pcie.yaml b/Documentation/devicetree/bindings/pci/xilinx-pcie.yaml
new file mode 100644
index 000000000000..6b372ac1763e
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/xilinx-pcie.yaml
@@ -0,0 +1,81 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/xilinx-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx AXI PCIe Root Port Bridge DT description
+
+maintainers:
+ - Thippeswamy Havalige <[email protected]>
+
+allOf:
+ - $ref: /schemas/pci/pci-bus.yaml#
+
+properties:
+ compatible:
+ const: xlnx,axi-pcie-host-1.00.a
+
+ reg:
+ items:
+ - description: should contain AXI PCIe registers location and length
+
+ interrupts:
+ items:
+ - description: should contain AXI PCIe interrupt
+
+ ranges:
+ items:
+ - description: |
+ ranges for the PCI memory regions (I/O space region is not
+ supported by hardware)
+
+ "#interrupt-cells":
+ const: 1
+
+ interrupt-controller:
+ description: identifies the node as an interrupt controller
+ type: object
+ properties:
+ "interrupt-controller": true
+ "#address-cells":
+ const: 0
+ "#interrupt-cells":
+ const: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - ranges
+ - device_type
+ - interrupt-map
+ - "#interrupt-cells"
+ - interrupt-controller
+
+unevaluatedProperties: false
+
+examples:
+ - |
+
+ Zynq:
+ pci_express: pcie@50000000 {
+ #address-cells = <3>;
+ #size-cells = <2>;
+ #interrupt-cells = <1>;
+ compatible = "xlnx,axi-pcie-host-1.00.a";
+ reg = < 0x50000000 0x1000000 >;
+ device_type = "pci";
+ interrupts = < 0 52 4 >;
+ interrupt-map-mask = <0 0 0 7>;
+ interrupt-map = <0 0 0 1 &pcie_intc 1>,
+ <0 0 0 2 &pcie_intc 2>,
+ <0 0 0 3 &pcie_intc 3>,
+ <0 0 0 4 &pcie_intc 4>;
+ ranges = < 0x02000000 0 0x60000000 0x60000000 0 0x10000000 >;
+ pcie_intc: interrupt-controller {
+ interrupt-controller;
+ #address-cells = <0>;
+ #interrupt-cells = <1>;
+ };
+ };
--
2.25.1


2022-10-19 15:35:12

by Havalige, Thippeswamy

[permalink] [raw]
Subject: [PATCH 2/2] dt-bindings: PCI: xilinx-nwl: Convert to YAML schemas of Xilinx NWL PCIe Root Port Bridge

Convert to YAML schemas for Xilinx NWL PCIe Root Port Bridge
dt binding.

Signed-off-by: Thippeswamy Havalige <[email protected]>
---
.../bindings/pci/xilinx-nwl-pcie.txt | 73 -----------
.../bindings/pci/xilinx-nwl-pcie.yaml | 122 ++++++++++++++++++
2 files changed, 122 insertions(+), 73 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
create mode 100644 Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt b/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
deleted file mode 100644
index f56f8c58c5d9..000000000000
--- a/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
+++ /dev/null
@@ -1,73 +0,0 @@
-* Xilinx NWL PCIe Root Port Bridge DT description
-
-Required properties:
-- compatible: Should contain "xlnx,nwl-pcie-2.11"
-- #address-cells: Address representation for root ports, set to <3>
-- #size-cells: Size representation for root ports, set to <2>
-- #interrupt-cells: specifies the number of cells needed to encode an
- interrupt source. The value must be 1.
-- reg: Should contain Bridge, PCIe Controller registers location,
- configuration space, and length
-- reg-names: Must include the following entries:
- "breg": bridge registers
- "pcireg": PCIe controller registers
- "cfg": configuration space region
-- device_type: must be "pci"
-- interrupts: Should contain NWL PCIe interrupt
-- interrupt-names: Must include the following entries:
- "msi1, msi0": interrupt asserted when an MSI is received
- "intx": interrupt asserted when a legacy interrupt is received
- "misc": interrupt asserted when miscellaneous interrupt is received
-- interrupt-map-mask and interrupt-map: standard PCI properties to define the
- mapping of the PCI interface to interrupt numbers.
-- ranges: ranges for the PCI memory regions (I/O space region is not
- supported by hardware)
- Please refer to the standard PCI bus binding document for a more
- detailed explanation
-- msi-controller: indicates that this is MSI controller node
-- msi-parent: MSI parent of the root complex itself
-- legacy-interrupt-controller: Interrupt controller device node for Legacy
- interrupts
- - interrupt-controller: identifies the node as an interrupt controller
- - #interrupt-cells: should be set to 1
- - #address-cells: specifies the number of cells needed to encode an
- address. The value must be 0.
-
-Optional properties:
-- dma-coherent: present if DMA operations are coherent
-- clocks: Input clock specifier. Refer to common clock bindings
-
-Example:
-++++++++
-
-nwl_pcie: pcie@fd0e0000 {
- #address-cells = <3>;
- #size-cells = <2>;
- compatible = "xlnx,nwl-pcie-2.11";
- #interrupt-cells = <1>;
- msi-controller;
- device_type = "pci";
- interrupt-parent = <&gic>;
- interrupts = <0 114 4>, <0 115 4>, <0 116 4>, <0 117 4>, <0 118 4>;
- interrupt-names = "msi0", "msi1", "intx", "dummy", "misc";
- interrupt-map-mask = <0x0 0x0 0x0 0x7>;
- interrupt-map = <0x0 0x0 0x0 0x1 &pcie_intc 0x1>,
- <0x0 0x0 0x0 0x2 &pcie_intc 0x2>,
- <0x0 0x0 0x0 0x3 &pcie_intc 0x3>,
- <0x0 0x0 0x0 0x4 &pcie_intc 0x4>;
-
- msi-parent = <&nwl_pcie>;
- reg = <0x0 0xfd0e0000 0x0 0x1000>,
- <0x0 0xfd480000 0x0 0x1000>,
- <0x80 0x00000000 0x0 0x1000000>;
- reg-names = "breg", "pcireg", "cfg";
- ranges = <0x02000000 0x00000000 0xe0000000 0x00000000 0xe0000000 0x00000000 0x10000000 /* non-prefetchable memory */
- 0x43000000 0x00000006 0x00000000 0x00000006 0x00000000 0x00000002 0x00000000>;/* prefetchable memory */
-
- pcie_intc: legacy-interrupt-controller {
- interrupt-controller;
- #address-cells = <0>;
- #interrupt-cells = <1>;
- };
-
-};
diff --git a/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.yaml b/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.yaml
new file mode 100644
index 000000000000..97a33e8cc171
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.yaml
@@ -0,0 +1,122 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/xilinx-nwl-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx NWL PCIe Root Port Bridge DT description
+
+maintainers:
+ - Thippeswamy Havalige <[email protected]>
+
+allOf:
+ - $ref: /schemas/pci/pci-bus.yaml#
+
+properties:
+ compatible:
+ const: xlnx,nwl-pcie-2.11
+
+ reg:
+ items:
+ - description: PCIe bridge registers location.
+ - description: PCIe Controller registers location.
+ - description: PCIe Configuration space region.
+
+ reg-names:
+ items:
+ - const: breg
+ - const: pcireg
+ - const: cfg
+
+ interrupts:
+ items:
+ - description: msi0 interrupt asserted when an MSI is received
+ - description: msi1 interrupt asserted when an MSI is received
+ - description: interrupt asserted when a legacy interrupt is received
+ - description: unused interrupt(dummy)
+ - description: interrupt asserted when miscellaneous interrupt is received
+
+ interrupt-names:
+ minItems: 5
+
+ interrupt-map-mask:
+ items:
+ - const: 0
+ - const: 0
+ - const: 0
+ - const: 7
+
+ "#interrupt-cells":
+ const: 1
+
+ msi-controller:
+ description: Identifies the node as an MSI controller.
+
+ msi-parent:
+ description: MSI controller the device is capable of using.
+
+ interrupt-map:
+ maxItems: 4
+
+ legacy-interrupt-controller:
+ description: Interrupt controller node for handling legacy PCI interrupts.
+ type: object
+ properties:
+ "#address-cells":
+ const: 0
+ "#interrupt-cells":
+ const: 1
+ "interrupt-controller": true
+
+required:
+ - reg
+ - reg-names
+ - interrupts
+ - interrupt-names
+ - "#interrupt-cells"
+ - interrupt-map
+ - msi-controller
+ - msi-parent
+ - interrupt-map-mask
+ - legacy-interrupt-controller
+
+unevaluatedProperties: false
+
+examples:
+ - |
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ nwl_pcie: pcie@fd0e0000 {
+ #address-cells = <3>;
+ #size-cells = <2>;
+ compatible = "xlnx,nwl-pcie-2.11";
+ #interrupt-cells = <1>;
+ msi-controller;
+ device_type = "pci";
+ interrupt-parent = <&gic>;
+ interrupts = <0 114 4>, <0 115 4>, <0 116 4>, <0 117 4>, <0 118 4>;
+ interrupt-names = "msi0", "msi1", "intx", "dummy", "misc";
+ interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+ interrupt-map = <0x0 0x0 0x0 0x1 &pcie_intc 0x1>,
+ <0x0 0x0 0x0 0x2 &pcie_intc 0x2>,
+ <0x0 0x0 0x0 0x3 &pcie_intc 0x3>,
+ <0x0 0x0 0x0 0x4 &pcie_intc 0x4>;
+
+ msi-parent = <&nwl_pcie>;
+ reg = <0x0 0xfd0e0000 0x0 0x1000>,
+ <0x0 0xfd480000 0x0 0x1000>,
+ <0x80 0x00000000 0x0 0x1000000>;
+ reg-names = "breg", "pcireg", "cfg";
+ ranges = <0x02000000 0x0 0xe0000000 0x0 0xe0000000 0x0 0x10000000
+ 0x43000000 0x00000006 0x0 0x00000006 0x0 0x00000002 0x0>;
+
+ pcie_intc: legacy-interrupt-controller {
+ interrupt-controller;
+ #address-cells = <0>;
+ #interrupt-cells = <1>;
+ };
+
+ };
+ };
--
2.25.1

2022-10-19 15:41:35

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] dt-bindings: PCI: xilinx-nwl: Convert to YAML schemas of Xilinx NWL PCIe Root Port Bridge

On 19/10/2022 10:46, Thippeswamy Havalige wrote:
> Convert to YAML schemas for Xilinx NWL PCIe Root Port Bridge
> dt binding.
>
> Signed-off-by: Thippeswamy Havalige <[email protected]>

(...)

> diff --git a/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.yaml b/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.yaml
> new file mode 100644
> index 000000000000..97a33e8cc171
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.yaml
> @@ -0,0 +1,122 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/xilinx-nwl-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Xilinx NWL PCIe Root Port Bridge DT description

Same comments apply.

> +
> +maintainers:
> + - Thippeswamy Havalige <[email protected]>

Use current email address.

> +
> +allOf:
> + - $ref: /schemas/pci/pci-bus.yaml#
> +
> +properties:
> + compatible:
> + const: xlnx,nwl-pcie-2.11
> +
> + reg:
> + items:
> + - description: PCIe bridge registers location.
> + - description: PCIe Controller registers location.
> + - description: PCIe Configuration space region.
> +
> + reg-names:
> + items:
> + - const: breg
> + - const: pcireg
> + - const: cfg
> +
> + interrupts:
> + items:
> + - description: msi0 interrupt asserted when an MSI is received
> + - description: msi1 interrupt asserted when an MSI is received
> + - description: interrupt asserted when a legacy interrupt is received
> + - description: unused interrupt(dummy)
> + - description: interrupt asserted when miscellaneous interrupt is received
> +
> + interrupt-names:
> + minItems: 5

maxItems instead

> +
> + interrupt-map-mask:
> + items:
> + - const: 0
> + - const: 0
> + - const: 0
> + - const: 7
> +
> + "#interrupt-cells":
> + const: 1
> +
> + msi-controller:
> + description: Identifies the node as an MSI controller.

If it is a MSI controller, shouldn't you reference
/schemas/interrupt-controller/msi-controller.yaml ?

> +
> + msi-parent:
> + description: MSI controller the device is capable of using.

msi-parent: true

> +
> + interrupt-map:
> + maxItems: 4
> +
> + legacy-interrupt-controller:
> + description: Interrupt controller node for handling legacy PCI interrupts.
> + type: object

Same comments apply.

> + properties:
> + "#address-cells":
> + const: 0
> + "#interrupt-cells":
> + const: 1
> + "interrupt-controller": true
> +

what happened to clocks? You did not describe any changes in commit msg.

> +required:

compatible

> + - reg
> + - reg-names
> + - interrupts
> + - interrupt-names
> + - "#interrupt-cells"
> + - interrupt-map
> + - msi-controller
> + - msi-parent
> + - interrupt-map-mask
> + - legacy-interrupt-controller

Drop properties required by referenced schema.

> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + nwl_pcie: pcie@fd0e0000 {
> + #address-cells = <3>;

Mess up indentation

Use 4 spaces for example indentation.

> + #size-cells = <2>;
> + compatible = "xlnx,nwl-pcie-2.11";

Same comments apply

> + #interrupt-cells = <1>;
> + msi-controller;
> + device_type = "pci";
> + interrupt-parent = <&gic>;
> + interrupts = <0 114 4>, <0 115 4>, <0 116 4>, <0 117 4>, <0 118 4>;
> + interrupt-names = "msi0", "msi1", "intx", "dummy", "misc";
> + interrupt-map-mask = <0x0 0x0 0x0 0x7>;
> + interrupt-map = <0x0 0x0 0x0 0x1 &pcie_intc 0x1>,
> + <0x0 0x0 0x0 0x2 &pcie_intc 0x2>,
> + <0x0 0x0 0x0 0x3 &pcie_intc 0x3>,
> + <0x0 0x0 0x0 0x4 &pcie_intc 0x4>;
> +
> + msi-parent = <&nwl_pcie>;
> + reg = <0x0 0xfd0e0000 0x0 0x1000>,
> + <0x0 0xfd480000 0x0 0x1000>,
> + <0x80 0x00000000 0x0 0x1000000>;

This is a second property in the list (followed by reg-names, ranges)

> + reg-names = "breg", "pcireg", "cfg";
> + ranges = <0x02000000 0x0 0xe0000000 0x0 0xe0000000 0x0 0x10000000
> + 0x43000000 0x00000006 0x0 0x00000006 0x0 0x00000002 0x0>;
> +
> + pcie_intc: legacy-interrupt-controller {
> + interrupt-controller;

That's even worse...

> + #address-cells = <0>;
> + #interrupt-cells = <1>;
> + };
> +
> + };
> + };

Best regards,
Krzysztof

2022-10-19 16:01:55

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: PCI: xilinx-pcie: Convert to YAML schemas of Xilinx AXI PCIe Root Port Bridge

On 19/10/2022 10:46, Thippeswamy Havalige wrote:
> Convert to YAML dtschemas of Xilinx AXI PCIe Root Port Bridge
> dt binding.


> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - ranges
> + - device_type

Drop all properties which are already required by referenced schemas.

> + - interrupt-map
> + - "#interrupt-cells"
> + - interrupt-controller
> +


Best regards,
Krzysztof

2022-10-19 16:04:19

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: PCI: xilinx-pcie: Convert to YAML schemas of Xilinx AXI PCIe Root Port Bridge

On 19/10/2022 10:46, Thippeswamy Havalige wrote:
> Convert to YAML dtschemas of Xilinx AXI PCIe Root Port Bridge
> dt binding.
>
> Signed-off-by: Thippeswamy Havalige <[email protected]>
> ---
> .../devicetree/bindings/pci/xilinx-pcie.txt | 88 -------------------
> .../devicetree/bindings/pci/xilinx-pcie.yaml | 81 +++++++++++++++++
> 2 files changed, 81 insertions(+), 88 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/pci/xilinx-pcie.txt
> create mode 100644 Documentation/devicetree/bindings/pci/xilinx-pcie.yaml
>
> diff --git a/Documentation/devicetree/bindings/pci/xilinx-pcie.txt b/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
> deleted file mode 100644
> index fd57a81180a4..000000000000
> --- a/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
> +++ /dev/null
> @@ -1,88 +0,0 @@
> -* Xilinx AXI PCIe Root Port Bridge DT description
> -
> -Required properties:
> -- #address-cells: Address representation for root ports, set to <3>
> -- #size-cells: Size representation for root ports, set to <2>
> -- #interrupt-cells: specifies the number of cells needed to encode an
> - interrupt source. The value must be 1.
> -- compatible: Should contain "xlnx,axi-pcie-host-1.00.a"
> -- reg: Should contain AXI PCIe registers location and length
> -- device_type: must be "pci"
> -- interrupts: Should contain AXI PCIe interrupt
> -- interrupt-map-mask,
> - interrupt-map: standard PCI properties to define the mapping of the
> - PCI interface to interrupt numbers.
> -- ranges: ranges for the PCI memory regions (I/O space region is not
> - supported by hardware)
> - Please refer to the standard PCI bus binding document for a more
> - detailed explanation
> -
> -Optional properties for Zynq/Microblaze:
> -- bus-range: PCI bus numbers covered
> -
> -Interrupt controller child node
> -+++++++++++++++++++++++++++++++
> -Required properties:
> -- interrupt-controller: identifies the node as an interrupt controller
> -- #address-cells: specifies the number of cells needed to encode an
> - address. The value must be 0.
> -- #interrupt-cells: specifies the number of cells needed to encode an
> - interrupt source. The value must be 1.
> -
> -NOTE:
> -The core provides a single interrupt for both INTx/MSI messages. So,
> -created a interrupt controller node to support 'interrupt-map' DT
> -functionality. The driver will create an IRQ domain for this map, decode
> -the four INTx interrupts in ISR and route them to this domain.
> -
> -
> -Example:
> -++++++++
> -Zynq:
> - pci_express: axi-pcie@50000000 {
> - #address-cells = <3>;
> - #size-cells = <2>;
> - #interrupt-cells = <1>;
> - compatible = "xlnx,axi-pcie-host-1.00.a";
> - reg = < 0x50000000 0x1000000 >;
> - device_type = "pci";
> - interrupts = < 0 52 4 >;
> - interrupt-map-mask = <0 0 0 7>;
> - interrupt-map = <0 0 0 1 &pcie_intc 1>,
> - <0 0 0 2 &pcie_intc 2>,
> - <0 0 0 3 &pcie_intc 3>,
> - <0 0 0 4 &pcie_intc 4>;
> - ranges = < 0x02000000 0 0x60000000 0x60000000 0 0x10000000 >;
> -
> - pcie_intc: interrupt-controller {
> - interrupt-controller;
> - #address-cells = <0>;
> - #interrupt-cells = <1>;
> - };
> - };
> -
> -
> -Microblaze:
> - pci_express: axi-pcie@10000000 {
> - #address-cells = <3>;
> - #size-cells = <2>;
> - #interrupt-cells = <1>;
> - compatible = "xlnx,axi-pcie-host-1.00.a";
> - reg = <0x10000000 0x4000000>;
> - device_type = "pci";
> - interrupt-parent = <&microblaze_0_intc>;
> - interrupts = <1 2>;
> - interrupt-map-mask = <0 0 0 7>;
> - interrupt-map = <0 0 0 1 &pcie_intc 1>,
> - <0 0 0 2 &pcie_intc 2>,
> - <0 0 0 3 &pcie_intc 3>,
> - <0 0 0 4 &pcie_intc 4>;
> - ranges = <0x02000000 0x00000000 0x80000000 0x80000000 0x00000000 0x10000000>;
> -
> - pcie_intc: interrupt-controller {
> - interrupt-controller;
> - #address-cells = <0>;
> - #interrupt-cells = <1>;
> - };
> -
> - };
> diff --git a/Documentation/devicetree/bindings/pci/xilinx-pcie.yaml b/Documentation/devicetree/bindings/pci/xilinx-pcie.yaml
> new file mode 100644
> index 000000000000..6b372ac1763e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/xilinx-pcie.yaml
> @@ -0,0 +1,81 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/xilinx-pcie.yaml#

Filename based on compatible, so:
xlnx,axi-pcie-host.yaml

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Xilinx AXI PCIe Root Port Bridge DT description

Drop "DT description"

> +
> +maintainers:
> + - Thippeswamy Havalige <[email protected]>
> +
> +allOf:
> + - $ref: /schemas/pci/pci-bus.yaml#
> +
> +properties:
> + compatible:
> + const: xlnx,axi-pcie-host-1.00.a
> +
> + reg:
> + items:
> + - description: should contain AXI PCIe registers location and length

Drop description, just maxItems: 1

> +
> + interrupts:
> + items:
> + - description: should contain AXI PCIe interrupt

Ditto


> +
> + ranges:
> + items:
> + - description: |
> + ranges for the PCI memory regions (I/O space region is not
> + supported by hardware)
> +
> + "#interrupt-cells":
> + const: 1
> +
> + interrupt-controller:
> + description: identifies the node as an interrupt controller
> + type: object

additionalProperties: false

> + properties:
> + "interrupt-controller": true
> + "#address-cells":
> + const: 0
> + "#interrupt-cells":
> + const: 1

Add also required properties for this node.

> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - ranges
> + - device_type
> + - interrupt-map
> + - "#interrupt-cells"
> + - interrupt-controller
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> +
> + Zynq:
> + pci_express: pcie@50000000 {
> + #address-cells = <3>;

Use 4 spaces for example indentation.

> + #size-cells = <2>;
> + #interrupt-cells = <1>;
> + compatible = "xlnx,axi-pcie-host-1.00.a";
> + reg = < 0x50000000 0x1000000 >;

Fix style.

compatible goes first, then reg, then the rest.

> + device_type = "pci";
> + interrupts = < 0 52 4 >;

Fix the style, use defines.

> + interrupt-map-mask = <0 0 0 7>;
> + interrupt-map = <0 0 0 1 &pcie_intc 1>,
> + <0 0 0 2 &pcie_intc 2>,
> + <0 0 0 3 &pcie_intc 3>,
> + <0 0 0 4 &pcie_intc 4>;
> + ranges = < 0x02000000 0 0x60000000 0x60000000 0 0x10000000 >;

Original example looked correct, so how did it become with these spaces?

> + pcie_intc: interrupt-controller {
> + interrupt-controller;
> + #address-cells = <0>;
> + #interrupt-cells = <1>;
> + };
> + };

Best regards,
Krzysztof

2022-10-19 23:33:38

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 2/2] dt-bindings: PCI: xilinx-nwl: Convert to YAML schemas of Xilinx NWL PCIe Root Port Bridge

On Wed, 19 Oct 2022 20:16:40 +0530, Thippeswamy Havalige wrote:
> Convert to YAML schemas for Xilinx NWL PCIe Root Port Bridge
> dt binding.
>
> Signed-off-by: Thippeswamy Havalige <[email protected]>
> ---
> .../bindings/pci/xilinx-nwl-pcie.txt | 73 -----------
> .../bindings/pci/xilinx-nwl-pcie.yaml | 122 ++++++++++++++++++
> 2 files changed, 122 insertions(+), 73 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
> create mode 100644 Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.yaml
>

Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.

Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/


pcie@fd0e0000: Unevaluated properties are not allowed ('clocks', 'iommus', 'power-domains' were unexpected)
arch/arm64/boot/dts/xilinx/avnet-ultra96-rev1.dtb
arch/arm64/boot/dts/xilinx/zynqmp-sm-k26-revA.dtb
arch/arm64/boot/dts/xilinx/zynqmp-smk-k26-revA.dtb
arch/arm64/boot/dts/xilinx/zynqmp-zc1232-revA.dtb
arch/arm64/boot/dts/xilinx/zynqmp-zc1254-revA.dtb
arch/arm64/boot/dts/xilinx/zynqmp-zc1275-revA.dtb
arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm015-dc1.dtb
arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm016-dc2.dtb
arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm017-dc3.dtb
arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm018-dc4.dtb
arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm019-dc5.dtb
arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dtb
arch/arm64/boot/dts/xilinx/zynqmp-zcu102-rev1.0.dtb
arch/arm64/boot/dts/xilinx/zynqmp-zcu102-rev1.1.dtb
arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dtb
arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revB.dtb
arch/arm64/boot/dts/xilinx/zynqmp-zcu104-revA.dtb
arch/arm64/boot/dts/xilinx/zynqmp-zcu104-revC.dtb
arch/arm64/boot/dts/xilinx/zynqmp-zcu106-revA.dtb
arch/arm64/boot/dts/xilinx/zynqmp-zcu111-revA.dtb

2022-10-25 14:30:08

by Havalige, Thippeswamy

[permalink] [raw]
Subject: RE: [PATCH 1/2] dt-bindings: PCI: xilinx-pcie: Convert to YAML schemas of Xilinx AXI PCIe Root Port Bridge

Hi,
> -----Original Message-----
> From: Krzysztof Kozlowski <[email protected]>
> Sent: Wednesday, October 19, 2022 8:34 PM
> To: Havalige, Thippeswamy <[email protected]>; linux-
> [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> Gogada, Bharat Kumar <[email protected]>
> Subject: Re: [PATCH 1/2] dt-bindings: PCI: xilinx-pcie: Convert to YAML
> schemas of Xilinx AXI PCIe Root Port Bridge
>
> On 19/10/2022 10:46, Thippeswamy Havalige wrote:
> > Convert to YAML dtschemas of Xilinx AXI PCIe Root Port Bridge dt
> > binding.
> >
> > Signed-off-by: Thippeswamy Havalige <[email protected]>
> > ---
> > .../devicetree/bindings/pci/xilinx-pcie.txt | 88 -------------------
> > .../devicetree/bindings/pci/xilinx-pcie.yaml | 81 +++++++++++++++++
> > 2 files changed, 81 insertions(+), 88 deletions(-) delete mode
> > 100644 Documentation/devicetree/bindings/pci/xilinx-pcie.txt
> > create mode 100644
> > Documentation/devicetree/bindings/pci/xilinx-pcie.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
> > b/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
> > deleted file mode 100644
> > index fd57a81180a4..000000000000
> > --- a/Documentation/devicetree/bindings/pci/xilinx-pcie.txt
> > +++ /dev/null
> > @@ -1,88 +0,0 @@
> > -* Xilinx AXI PCIe Root Port Bridge DT description
> > -
> > -Required properties:
> > -- #address-cells: Address representation for root ports, set to <3>
> > -- #size-cells: Size representation for root ports, set to <2>
> > -- #interrupt-cells: specifies the number of cells needed to encode an
> > - interrupt source. The value must be 1.
> > -- compatible: Should contain "xlnx,axi-pcie-host-1.00.a"
> > -- reg: Should contain AXI PCIe registers location and length
> > -- device_type: must be "pci"
> > -- interrupts: Should contain AXI PCIe interrupt
> > -- interrupt-map-mask,
> > - interrupt-map: standard PCI properties to define the mapping of the
> > - PCI interface to interrupt numbers.
> > -- ranges: ranges for the PCI memory regions (I/O space region is not
> > - supported by hardware)
> > - Please refer to the standard PCI bus binding document for a more
> > - detailed explanation
> > -
> > -Optional properties for Zynq/Microblaze:
> > -- bus-range: PCI bus numbers covered
> > -
> > -Interrupt controller child node
> > -+++++++++++++++++++++++++++++++
> > -Required properties:
> > -- interrupt-controller: identifies the node as an interrupt
> > controller
> > -- #address-cells: specifies the number of cells needed to encode an
> > - address. The value must be 0.
> > -- #interrupt-cells: specifies the number of cells needed to encode an
> > - interrupt source. The value must be 1.
> > -
> > -NOTE:
> > -The core provides a single interrupt for both INTx/MSI messages. So,
> > -created a interrupt controller node to support 'interrupt-map' DT
> > -functionality. The driver will create an IRQ domain for this map,
> > decode -the four INTx interrupts in ISR and route them to this domain.
> > -
> > -
> > -Example:
> > -++++++++
> > -Zynq:
> > - pci_express: axi-pcie@50000000 {
> > - #address-cells = <3>;
> > - #size-cells = <2>;
> > - #interrupt-cells = <1>;
> > - compatible = "xlnx,axi-pcie-host-1.00.a";
> > - reg = < 0x50000000 0x1000000 >;
> > - device_type = "pci";
> > - interrupts = < 0 52 4 >;
> > - interrupt-map-mask = <0 0 0 7>;
> > - interrupt-map = <0 0 0 1 &pcie_intc 1>,
> > - <0 0 0 2 &pcie_intc 2>,
> > - <0 0 0 3 &pcie_intc 3>,
> > - <0 0 0 4 &pcie_intc 4>;
> > - ranges = < 0x02000000 0 0x60000000 0x60000000 0
> 0x10000000 >;
> > -
> > - pcie_intc: interrupt-controller {
> > - interrupt-controller;
> > - #address-cells = <0>;
> > - #interrupt-cells = <1>;
> > - };
> > - };
> > -
> > -
> > -Microblaze:
> > - pci_express: axi-pcie@10000000 {
> > - #address-cells = <3>;
> > - #size-cells = <2>;
> > - #interrupt-cells = <1>;
> > - compatible = "xlnx,axi-pcie-host-1.00.a";
> > - reg = <0x10000000 0x4000000>;
> > - device_type = "pci";
> > - interrupt-parent = <&microblaze_0_intc>;
> > - interrupts = <1 2>;
> > - interrupt-map-mask = <0 0 0 7>;
> > - interrupt-map = <0 0 0 1 &pcie_intc 1>,
> > - <0 0 0 2 &pcie_intc 2>,
> > - <0 0 0 3 &pcie_intc 3>,
> > - <0 0 0 4 &pcie_intc 4>;
> > - ranges = <0x02000000 0x00000000 0x80000000 0x80000000
> 0x00000000 0x10000000>;
> > -
> > - pcie_intc: interrupt-controller {
> > - interrupt-controller;
> > - #address-cells = <0>;
> > - #interrupt-cells = <1>;
> > - };
> > -
> > - };
> > diff --git a/Documentation/devicetree/bindings/pci/xilinx-pcie.yaml
> > b/Documentation/devicetree/bindings/pci/xilinx-pcie.yaml
> > new file mode 100644
> > index 000000000000..6b372ac1763e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/xilinx-pcie.yaml
> > @@ -0,0 +1,81 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pci/xilinx-pcie.yaml#
>
> Filename based on compatible, so:
> xlnx,axi-pcie-host.yaml
>
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Xilinx AXI PCIe Root Port Bridge DT description
>
> Drop "DT description"
>
> > +
> > +maintainers:
> > + - Thippeswamy Havalige <[email protected]>
> > +
> > +allOf:
> > + - $ref: /schemas/pci/pci-bus.yaml#
> > +
> > +properties:
> > + compatible:
> > + const: xlnx,axi-pcie-host-1.00.a
> > +
> > + reg:
> > + items:
> > + - description: should contain AXI PCIe registers location and
> > + length
>
> Drop description, just maxItems: 1
>
> > +
> > + interrupts:
> > + items:
> > + - description: should contain AXI PCIe interrupt
>
> Ditto
>
>
> > +
> > + ranges:
> > + items:
> > + - description: |
> > + ranges for the PCI memory regions (I/O space region is not
> > + supported by hardware)
> > +
> > + "#interrupt-cells":
> > + const: 1
> > +
> > + interrupt-controller:
> > + description: identifies the node as an interrupt controller
> > + type: object
>
> additionalProperties: false
>
> > + properties:
> > + "interrupt-controller": true
> > + "#address-cells":
> > + const: 0
> > + "#interrupt-cells":
> > + const: 1
>
> Add also required properties for this node.
>
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - ranges
> > + - device_type
> > + - interrupt-map
> > + - "#interrupt-cells"
> > + - interrupt-controller
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > +
> > + Zynq:
> > + pci_express: pcie@50000000 {
> > + #address-cells = <3>;
>
> Use 4 spaces for example indentation.
>
> > + #size-cells = <2>;
> > + #interrupt-cells = <1>;
> > + compatible = "xlnx,axi-pcie-host-1.00.a";
> > + reg = < 0x50000000 0x1000000 >;
>
> Fix style.
>
> compatible goes first, then reg, then the rest.
>
> > + device_type = "pci";
> > + interrupts = < 0 52 4 >;
>
> Fix the style, use defines.
>
> > + interrupt-map-mask = <0 0 0 7>;
> > + interrupt-map = <0 0 0 1 &pcie_intc 1>,
> > + <0 0 0 2 &pcie_intc 2>,
> > + <0 0 0 3 &pcie_intc 3>,
> > + <0 0 0 4 &pcie_intc 4>;
> > + ranges = < 0x02000000 0 0x60000000 0x60000000 0
> > + 0x10000000 >;
>
> Original example looked correct, so how did it become with these spaces?
>
> > + pcie_intc: interrupt-controller {
> > + interrupt-controller;
> > + #address-cells = <0>;
> > + #interrupt-cells = <1>;
> > + };
> > + };
>
> Best regards,
> Krzysztof

Thanks for reviewing, I agree with all the recommended changes. I'll send updated patches.

Regards,
Thippeswamy H