2022-03-09 12:50:34

by Bharat Kumar Gogada

[permalink] [raw]
Subject: [PATCH v3 1/2] dt-bindings: PCI: xilinx-cpm: Add Versal CPM5 Root Port

Xilinx Versal Premium series has CPM5 block which supports Root Port
functioning at Gen5 speed.

Add support for YAML schemas documentation for Versal CPM5 Root Port driver.

Signed-off-by: Bharat Kumar Gogada <[email protected]>
---
.../bindings/pci/xilinx-versal-cpm.yaml | 47 ++++++++++++++++---
1 file changed, 40 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
index 32f4641085bc..97c7229d7f91 100644
--- a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
+++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
@@ -14,17 +14,21 @@ allOf:

properties:
compatible:
- const: xlnx,versal-cpm-host-1.00
+ contains:
+ enum:
+ - xlnx,versal-cpm-host-1.00
+ - xlnx,versal-cpm5-host-1.00

reg:
- items:
- - description: Configuration space region and bridge registers.
- - description: CPM system level control and status registers.
+ description: |
+ Should contain cpm_slcr, cfg registers location and length.
+ For xlnx,versal-cpm5-host-1.00, it should also contain cpm_csr.
+ minItems: 2
+ maxItems: 3

reg-names:
- items:
- - const: cfg
- - const: cpm_slcr
+ minItems: 2
+ maxItems: 3

interrupts:
maxItems: 1
@@ -95,4 +99,33 @@ examples:
interrupt-controller;
};
};
+
+ cpm5_pcie: pcie@fcdd0000 {
+ compatible = "xlnx,versal-cpm5-host-1.00";
+ device_type = "pci";
+ #address-cells = <3>;
+ #interrupt-cells = <1>;
+ #size-cells = <2>;
+ interrupts = <0 72 4>;
+ interrupt-parent = <&gic>;
+ interrupt-map-mask = <0 0 0 7>;
+ interrupt-map = <0 0 0 1 &pcie_intc_1 0>,
+ <0 0 0 2 &pcie_intc_1 1>,
+ <0 0 0 3 &pcie_intc_1 2>,
+ <0 0 0 4 &pcie_intc_1 3>;
+ bus-range = <0x00 0xff>;
+ ranges = <0x02000000 0x0 0xe0000000 0x0 0xe0000000 0x0 0x10000000>,
+ <0x43000000 0x80 0x00000000 0x80 0x00000000 0x0 0x80000000>;
+ msi-map = <0x0 &its_gic 0x0 0x10000>;
+ reg = <0x00 0xfcdd0000 0x00 0x1000>,
+ <0x06 0x00000000 0x00 0x1000000>,
+ <0x00 0xfce20000 0x00 0x1000000>;
+ reg-names = "cpm_slcr", "cfg", "cpm_csr";
+
+ pcie_intc_1: interrupt-controller {
+ #address-cells = <0>;
+ #interrupt-cells = <1>;
+ interrupt-controller;
+ };
+ };
};
--
2.17.1


2022-03-10 14:26:00

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] dt-bindings: PCI: xilinx-cpm: Add Versal CPM5 Root Port

On 09/03/2022 13:00, Bharat Kumar Gogada wrote:
> Xilinx Versal Premium series has CPM5 block which supports Root Port
> functioning at Gen5 speed.
>
> Add support for YAML schemas documentation for Versal CPM5 Root Port driver.
>
> Signed-off-by: Bharat Kumar Gogada <[email protected]>
> ---
> .../bindings/pci/xilinx-versal-cpm.yaml | 47 ++++++++++++++++---
> 1 file changed, 40 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> index 32f4641085bc..97c7229d7f91 100644
> --- a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> +++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> @@ -14,17 +14,21 @@ allOf:
>
> properties:
> compatible:
> - const: xlnx,versal-cpm-host-1.00
> + contains:
> + enum:
> + - xlnx,versal-cpm-host-1.00
> + - xlnx,versal-cpm5-host-1.00
>
> reg:
> - items:
> - - description: Configuration space region and bridge registers.
> - - description: CPM system level control and status registers.
> + description: |
> + Should contain cpm_slcr, cfg registers location and length.
> + For xlnx,versal-cpm5-host-1.00, it should also contain cpm_csr.
> + minItems: 2
> + maxItems: 3
>
> reg-names:
> - items:
> - - const: cfg
> - - const: cpm_slcr
> + minItems: 2
> + maxItems: 3
>

One more thought - it seems three items are required on cpm5, so you
miss later proper allOf-if: which enforces this.

Best regards,
Krzysztof

2022-03-10 23:51:51

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] dt-bindings: PCI: xilinx-cpm: Add Versal CPM5 Root Port

On 09/03/2022 13:00, Bharat Kumar Gogada wrote:
> Xilinx Versal Premium series has CPM5 block which supports Root Port
> functioning at Gen5 speed.
>
> Add support for YAML schemas documentation for Versal CPM5 Root Port driver.
>
> Signed-off-by: Bharat Kumar Gogada <[email protected]>
> ---
> .../bindings/pci/xilinx-versal-cpm.yaml | 47 ++++++++++++++++---
> 1 file changed, 40 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> index 32f4641085bc..97c7229d7f91 100644
> --- a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> +++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> @@ -14,17 +14,21 @@ allOf:
>
> properties:
> compatible:
> - const: xlnx,versal-cpm-host-1.00
> + contains:
> + enum:
> + - xlnx,versal-cpm-host-1.00
> + - xlnx,versal-cpm5-host-1.00
>
> reg:
> - items:
> - - description: Configuration space region and bridge registers.
> - - description: CPM system level control and status registers.
> + description: |
> + Should contain cpm_slcr, cfg registers location and length.
> + For xlnx,versal-cpm5-host-1.00, it should also contain cpm_csr.
> + minItems: 2
> + maxItems: 3

You removed here list of items, which should stay. See also
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/example-schema.yaml#L91
how to do it.

>
> reg-names:
> - items:
> - - const: cfg
> - - const: cpm_slcr
> + minItems: 2
> + maxItems: 3

The same.


Best regards,
Krzysztof

2022-03-11 05:52:04

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] dt-bindings: PCI: xilinx-cpm: Add Versal CPM5 Root Port

On Wed, Mar 9, 2022 at 6:00 AM Bharat Kumar Gogada
<[email protected]> wrote:
>
> Xilinx Versal Premium series has CPM5 block which supports Root Port
> functioning at Gen5 speed.
>
> Add support for YAML schemas documentation for Versal CPM5 Root Port driver.
>
> Signed-off-by: Bharat Kumar Gogada <[email protected]>
> ---
> .../bindings/pci/xilinx-versal-cpm.yaml | 47 ++++++++++++++++---
> 1 file changed, 40 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> index 32f4641085bc..97c7229d7f91 100644
> --- a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> +++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> @@ -14,17 +14,21 @@ allOf:
>
> properties:
> compatible:
> - const: xlnx,versal-cpm-host-1.00
> + contains:

Nope. That means 'compatible = "foo", "xlnx,versal-cpm-host-1.00",
"bar";' would be valid.

> + enum:
> + - xlnx,versal-cpm-host-1.00
> + - xlnx,versal-cpm5-host-1.00

Where does 1.00 come from? My guess is you or whoever did the original
binding just made it up. Version numbers are only used when they
correspond to something documented for the h/w. In the case of Xilinx,
that should be soft IP (which I assume has versioned releases) and
nothing else. If 'versal' is not specific enough to identify a
specific SoC, then add to it.


> reg:
> - items:
> - - description: Configuration space region and bridge registers.
> - - description: CPM system level control and status registers.
> + description: |
> + Should contain cpm_slcr, cfg registers location and length.
> + For xlnx,versal-cpm5-host-1.00, it should also contain cpm_csr.

Not an improvement in defining what each entry is.

> + minItems: 2
> + maxItems: 3

2022-03-14 07:16:16

by Bharat Kumar Gogada

[permalink] [raw]
Subject: RE: [PATCH v3 1/2] dt-bindings: PCI: xilinx-cpm: Add Versal CPM5 Root Port

> On 09/03/2022 13:00, Bharat Kumar Gogada wrote:
> > Xilinx Versal Premium series has CPM5 block which supports Root Port
> > functioning at Gen5 speed.
> >
> > Add support for YAML schemas documentation for Versal CPM5 Root Port
> driver.
> >
> > Signed-off-by: Bharat Kumar Gogada <[email protected]>
> > ---
> > .../bindings/pci/xilinx-versal-cpm.yaml | 47 ++++++++++++++++---
> > 1 file changed, 40 insertions(+), 7 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> > b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> > index 32f4641085bc..97c7229d7f91 100644
> > --- a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> > +++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> > @@ -14,17 +14,21 @@ allOf:
> >
> > properties:
> > compatible:
> > - const: xlnx,versal-cpm-host-1.00
> > + contains:
> > + enum:
> > + - xlnx,versal-cpm-host-1.00
> > + - xlnx,versal-cpm5-host-1.00
> >
> > reg:
> > - items:
> > - - description: Configuration space region and bridge registers.
> > - - description: CPM system level control and status registers.
> > + description: |
> > + Should contain cpm_slcr, cfg registers location and length.
> > + For xlnx,versal-cpm5-host-1.00, it should also contain cpm_csr.
> > + minItems: 2
> > + maxItems: 3
>
> You removed here list of items, which should stay. See also
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bi
> ndings/example-schema.yaml#L91
> how to do it.
>
> >
> > reg-names:
> > - items:
> > - - const: cfg
> > - - const: cpm_slcr
> > + minItems: 2
> > + maxItems: 3
>
> The same.
>
>
Thanks Krzysztof, will fix this in next patch.

2022-03-14 07:57:14

by Bharat Kumar Gogada

[permalink] [raw]
Subject: RE: [PATCH v3 1/2] dt-bindings: PCI: xilinx-cpm: Add Versal CPM5 Root Port

> On Wed, Mar 9, 2022 at 6:00 AM Bharat Kumar Gogada
> <[email protected]> wrote:
> >
> > Xilinx Versal Premium series has CPM5 block which supports Root Port
> > functioning at Gen5 speed.
> >
> > Add support for YAML schemas documentation for Versal CPM5 Root Port
> driver.
> >
> > Signed-off-by: Bharat Kumar Gogada <[email protected]>
> > ---
> > .../bindings/pci/xilinx-versal-cpm.yaml | 47 ++++++++++++++++---
> > 1 file changed, 40 insertions(+), 7 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> > b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> > index 32f4641085bc..97c7229d7f91 100644
> > --- a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> > +++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> > @@ -14,17 +14,21 @@ allOf:
> >
> > properties:
> > compatible:
> > - const: xlnx,versal-cpm-host-1.00
> > + contains:
>
> Nope. That means 'compatible = "foo", "xlnx,versal-cpm-host-1.00", "bar";'
> would be valid.
>
> > + enum:
> > + - xlnx,versal-cpm-host-1.00
> > + - xlnx,versal-cpm5-host-1.00
>
> Where does 1.00 come from? My guess is you or whoever did the original
> binding just made it up. Version numbers are only used when they
> correspond to something documented for the h/w. In the case of Xilinx, that
> should be soft IP (which I assume has versioned releases) and nothing else. If
> 'versal' is not specific enough to identify a specific SoC, then add to it.
>
Agreed, will remove version numbers.
>
> > reg:
> > - items:
> > - - description: Configuration space region and bridge registers.
> > - - description: CPM system level control and status registers.
> > + description: |
> > + Should contain cpm_slcr, cfg registers location and length.
> > + For xlnx,versal-cpm5-host-1.00, it should also contain cpm_csr.
>
> Not an improvement in defining what each entry is.
Agreed, will fix this in next patch.

regards,
Bharat