| Message ID | 20220309120025.6721-2-bharat.kumar.gogada@xilinx.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | Add support for Xilinx Versal CPM5 Root Port | expand |
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
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
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
> 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.
> 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 --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; + }; + }; };
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(-)