diff mbox series

[v3,1/2] dt-bindings: PCI: xilinx-cpm: Add Versal CPM5 Root Port

Message ID 20220309120025.6721-2-bharat.kumar.gogada@xilinx.com
State New
Headers show
Series Add support for Xilinx Versal CPM5 Root Port | expand

Commit Message

Bharat Kumar Gogada March 9, 2022, noon UTC
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 <bharat.kumar.gogada@xilinx.com>
---
 .../bindings/pci/xilinx-versal-cpm.yaml       | 47 ++++++++++++++++---
 1 file changed, 40 insertions(+), 7 deletions(-)

Comments

Krzysztof Kozlowski March 10, 2022, 1:24 p.m. UTC | #1
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 <bharat.kumar.gogada@xilinx.com>
> ---
>  .../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
Krzysztof Kozlowski March 10, 2022, 1:33 p.m. UTC | #2
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 <bharat.kumar.gogada@xilinx.com>
> ---
>  .../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
Rob Herring March 10, 2022, 7:13 p.m. UTC | #3
On Wed, Mar 9, 2022 at 6:00 AM Bharat Kumar Gogada
<bharat.kumar.gogada@xilinx.com> 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 <bharat.kumar.gogada@xilinx.com>
> ---
>  .../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
Bharat Kumar Gogada March 14, 2022, 4:32 a.m. UTC | #4
> 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 <bharat.kumar.gogada@xilinx.com>
> > ---
> >  .../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.
Bharat Kumar Gogada March 14, 2022, 4:33 a.m. UTC | #5
> On Wed, Mar 9, 2022 at 6:00 AM Bharat Kumar Gogada
> <bharat.kumar.gogada@xilinx.com> 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 <bharat.kumar.gogada@xilinx.com>
> > ---
> >  .../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
diff mbox series

Patch

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