| Message ID | 20240405145322.3805828-1-matthew.gerlach@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | [v2] dt-bindings: PCI: altera: Convert to YAML | expand |
On 05/04/2024 16:53, matthew.gerlach@linux.intel.com wrote: > From: Matthew Gerlach <matthew.gerlach@linux.intel.com> > > Convert the device tree bindings for the Altera Root Port PCIe controller > from text to YAML. > > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com> > --- > v2: > - Move allOf: to bottom of file, just like example-schema is showing No, just open it and you will see it is placed differently... > - add constraint for reg and reg-names Not complete... > - remove unneeded device_type > - drop #address-cells and #size-cells > - change minItems to maxItems for interrupts: > - change msi-parent to just "msi-parent: true" > - cleaned up required: > - make subject consistent with other commits coverting to YAML > - s/overt/onvert/g > --- > .../devicetree/bindings/pci/altera-pcie.txt | 50 --------- > .../bindings/pci/altr,pcie-root-port.yaml | 106 ++++++++++++++++++ > 2 files changed, 106 insertions(+), 50 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/pci/altera-pcie.txt > create mode 100644 Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml > > diff --git a/Documentation/devicetree/bindings/pci/altera-pcie.txt b/Documentation/devicetree/bindings/pci/altera-pcie.txt > deleted file mode 100644 > index 816b244a221e..000000000000 > --- a/Documentation/devicetree/bindings/pci/altera-pcie.txt > +++ /dev/null > @@ -1,50 +0,0 @@ > -* Altera PCIe controller > - > -Required properties: > -- compatible : should contain "altr,pcie-root-port-1.0" or "altr,pcie-root-port-2.0" > -- reg: a list of physical base address and length for TXS and CRA. > - For "altr,pcie-root-port-2.0", additional HIP base address and length. > -- reg-names: must include the following entries: > - "Txs": TX slave port region > - "Cra": Control register access region > - "Hip": Hard IP region (if "altr,pcie-root-port-2.0") > -- interrupts: specifies the interrupt source of the parent interrupt > - controller. The format of the interrupt specifier depends > - on the parent interrupt controller. > -- device_type: must be "pci" > -- #address-cells: set to <3> > -- #size-cells: set to <2> > -- #interrupt-cells: set to <1> > -- ranges: describes the translation of addresses for root ports and > - standard PCI regions. > -- interrupt-map-mask and interrupt-map: standard PCI properties to define the > - mapping of the PCIe interface to interrupt numbers. > - > -Optional properties: > -- msi-parent: Link to the hardware entity that serves as the MSI controller > - for this PCIe controller. > -- bus-range: PCI bus numbers covered > - > -Example > - pcie_0: pcie@c00000000 { > - compatible = "altr,pcie-root-port-1.0"; > - reg = <0xc0000000 0x20000000>, > - <0xff220000 0x00004000>; > - reg-names = "Txs", "Cra"; > - interrupt-parent = <&hps_0_arm_gic_0>; > - interrupts = <0 40 4>; > - interrupt-controller; > - #interrupt-cells = <1>; > - bus-range = <0x0 0xFF>; > - device_type = "pci"; > - msi-parent = <&msi_to_gic_gen_0>; > - #address-cells = <3>; > - #size-cells = <2>; > - interrupt-map-mask = <0 0 0 7>; > - interrupt-map = <0 0 0 1 &pcie_0 1>, > - <0 0 0 2 &pcie_0 2>, > - <0 0 0 3 &pcie_0 3>, > - <0 0 0 4 &pcie_0 4>; > - ranges = <0x82000000 0x00000000 0x00000000 0xc0000000 0x00000000 0x10000000 > - 0x82000000 0x00000000 0x10000000 0xd0000000 0x00000000 0x10000000>; > - }; > diff --git a/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml b/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml > new file mode 100644 > index 000000000000..999dcda05f55 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml > @@ -0,0 +1,106 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +# Copyright (C) 2024, Intel Corporation This is derivative of previous work, which is easily visible by doing the same mistakes in DTS as they were before. You now added fresh copyrights ignoring all previous work, even though you copied it. I don't agree. If you want to ignore previous copyrights, then at least don't copy existing code... although even that would not be sufficient. > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/altr,pcie-root-port.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Altera PCIe Root Port > + > +maintainers: > + - Matthew Gerlach <matthew.gerlach@linux.intel.com> > + > +properties: > + compatible: > + items: Drop items. > + - enum: > + - altr,pcie-root-port-1.0 > + - altr,pcie-root-port-2.0 > + Missing reg with constraints. > + interrupts: > + maxItems: 1 > + > + interrupt-map-mask: > + items: > + - const: 0 > + - const: 0 > + - const: 0 > + - const: 7 > + > + interrupt-map: > + maxItems: 4 > + > + "#interrupt-cells": > + const: 1 > + > + msi-parent: true > + > +required: > + - compatible > + - reg > + - reg-names > + - device_type > + - interrupts > + - interrupt-map > + - interrupt-map-mask > + > +unevaluatedProperties: false > + > +allOf: > + - $ref: /schemas/pci/pci-bus.yaml# That's deprecated, as explained in its description. You should use pci-host-bridge.yaml. > + - if: > + properties: > + compatible: > + enum: > + - altr,pcie-root-port-1.0 > + then: > + properties: > + reg: > + items: > + - description: TX slave port region > + - description: Control register access region > + > + reg-names: > + items: > + - const: Txs > + - const: Cra > + > + else: > + properties: > + reg: > + items: > + - description: Hard IP region > + - description: TX slave port region > + - description: Control register access region > + > + reg-names: > + items: > + - const: Hip > + - const: Txs > + - const: Cra > + unevaluated goes here, just like example-schema. > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + pcie_0: pcie@c00000000 { > + compatible = "altr,pcie-root-port-1.0"; > + reg = <0xc0000000 0x20000000>, > + <0xff220000 0x00004000>; > + reg-names = "Txs", "Cra"; > + interrupt-parent = <&hps_0_arm_gic_0>; > + interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>; > + #interrupt-cells = <1>; > + bus-range = <0x0 0xff>; > + device_type = "pci"; > + msi-parent = <&msi_to_gic_gen_0>; > + #address-cells = <3>; > + #size-cells = <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 = <0x82000000 0x00000000 0x00000000 0xc0000000 0x00000000 0x10000000 > + 0x82000000 0x00000000 0x10000000 0xd0000000 0x00000000 0x10000000>; That's two entries. Best regards, Krzysztof
On Sun, 7 Apr 2024, Krzysztof Kozlowski wrote: > On 05/04/2024 16:53, matthew.gerlach@linux.intel.com wrote: >> From: Matthew Gerlach <matthew.gerlach@linux.intel.com> >> >> Convert the device tree bindings for the Altera Root Port PCIe controller >> from text to YAML. >> >> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com> >> --- >> v2: >> - Move allOf: to bottom of file, just like example-schema is showing > > No, just open it and you will see it is placed differently... I see what you mean. I will match the ordering of example-schema. > >> - add constraint for reg and reg-names > > Not complete... I will complete. > >> - remove unneeded device_type >> - drop #address-cells and #size-cells >> - change minItems to maxItems for interrupts: >> - change msi-parent to just "msi-parent: true" >> - cleaned up required: >> - make subject consistent with other commits coverting to YAML >> - s/overt/onvert/g >> --- >> .../devicetree/bindings/pci/altera-pcie.txt | 50 --------- >> .../bindings/pci/altr,pcie-root-port.yaml | 106 ++++++++++++++++++ >> 2 files changed, 106 insertions(+), 50 deletions(-) >> delete mode 100644 Documentation/devicetree/bindings/pci/altera-pcie.txt >> create mode 100644 Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml >> >> diff --git a/Documentation/devicetree/bindings/pci/altera-pcie.txt b/Documentation/devicetree/bindings/pci/altera-pcie.txt >> deleted file mode 100644 >> index 816b244a221e..000000000000 >> --- a/Documentation/devicetree/bindings/pci/altera-pcie.txt >> +++ /dev/null >> @@ -1,50 +0,0 @@ >> -* Altera PCIe controller >> - >> -Required properties: >> -- compatible : should contain "altr,pcie-root-port-1.0" or "altr,pcie-root-port-2.0" >> -- reg: a list of physical base address and length for TXS and CRA. >> - For "altr,pcie-root-port-2.0", additional HIP base address and length. >> -- reg-names: must include the following entries: >> - "Txs": TX slave port region >> - "Cra": Control register access region >> - "Hip": Hard IP region (if "altr,pcie-root-port-2.0") >> -- interrupts: specifies the interrupt source of the parent interrupt >> - controller. The format of the interrupt specifier depends >> - on the parent interrupt controller. >> -- device_type: must be "pci" >> -- #address-cells: set to <3> >> -- #size-cells: set to <2> >> -- #interrupt-cells: set to <1> >> -- ranges: describes the translation of addresses for root ports and >> - standard PCI regions. >> -- interrupt-map-mask and interrupt-map: standard PCI properties to define the >> - mapping of the PCIe interface to interrupt numbers. >> - >> -Optional properties: >> -- msi-parent: Link to the hardware entity that serves as the MSI controller >> - for this PCIe controller. >> -- bus-range: PCI bus numbers covered >> - >> -Example >> - pcie_0: pcie@c00000000 { >> - compatible = "altr,pcie-root-port-1.0"; >> - reg = <0xc0000000 0x20000000>, >> - <0xff220000 0x00004000>; >> - reg-names = "Txs", "Cra"; >> - interrupt-parent = <&hps_0_arm_gic_0>; >> - interrupts = <0 40 4>; >> - interrupt-controller; >> - #interrupt-cells = <1>; >> - bus-range = <0x0 0xFF>; >> - device_type = "pci"; >> - msi-parent = <&msi_to_gic_gen_0>; >> - #address-cells = <3>; >> - #size-cells = <2>; >> - interrupt-map-mask = <0 0 0 7>; >> - interrupt-map = <0 0 0 1 &pcie_0 1>, >> - <0 0 0 2 &pcie_0 2>, >> - <0 0 0 3 &pcie_0 3>, >> - <0 0 0 4 &pcie_0 4>; >> - ranges = <0x82000000 0x00000000 0x00000000 0xc0000000 0x00000000 0x10000000 >> - 0x82000000 0x00000000 0x10000000 0xd0000000 0x00000000 0x10000000>; >> - }; >> diff --git a/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml b/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml >> new file mode 100644 >> index 000000000000..999dcda05f55 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml >> @@ -0,0 +1,106 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +# Copyright (C) 2024, Intel Corporation > > This is derivative of previous work, which is easily visible by doing > the same mistakes in DTS as they were before. This is definitely derivative of previous work, and I want to fix the DTS mistakes too. > > You now added fresh copyrights ignoring all previous work, even though > you copied it. I don't agree. > > If you want to ignore previous copyrights, then at least don't copy > existing code... although even that would not be sufficient. Ignoring previous copyrights was not my intent. There is no copyright statement in the original text version of the device tree bindings. Should that lack of copyright statement carry forward? > >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/altr,pcie-root-port.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Altera PCIe Root Port >> + >> +maintainers: >> + - Matthew Gerlach <matthew.gerlach@linux.intel.com> >> + >> +properties: >> + compatible: >> + items: > > Drop items. I will drop the items. > >> + - enum: >> + - altr,pcie-root-port-1.0 >> + - altr,pcie-root-port-2.0 >> + > > Missing reg with constraints. I will add the following here: reg: minItems: 2 maxItems: 3 reg-names: minItems: 2 maxItems: 3 > >> + interrupts: >> + maxItems: 1 >> + >> + interrupt-map-mask: >> + items: >> + - const: 0 >> + - const: 0 >> + - const: 0 >> + - const: 7 >> + >> + interrupt-map: >> + maxItems: 4 >> + >> + "#interrupt-cells": >> + const: 1 >> + >> + msi-parent: true >> + >> +required: >> + - compatible >> + - reg >> + - reg-names >> + - device_type >> + - interrupts >> + - interrupt-map >> + - interrupt-map-mask >> + >> +unevaluatedProperties: false >> + >> +allOf: >> + - $ref: /schemas/pci/pci-bus.yaml# > > That's deprecated, as explained in its description. You should use > pci-host-bridge.yaml. I will switch to pci-host-bridge.yaml. > > > >> + - if: >> + properties: >> + compatible: >> + enum: >> + - altr,pcie-root-port-1.0 >> + then: >> + properties: >> + reg: >> + items: >> + - description: TX slave port region >> + - description: Control register access region >> + >> + reg-names: >> + items: >> + - const: Txs >> + - const: Cra >> + >> + else: >> + properties: >> + reg: >> + items: >> + - description: Hard IP region >> + - description: TX slave port region >> + - description: Control register access region >> + >> + reg-names: >> + items: >> + - const: Hip >> + - const: Txs >> + - const: Cra >> + > > unevaluated goes here, just like example-schema. Yes, just like the example-schema > >> +examples: >> + - | >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + #include <dt-bindings/interrupt-controller/irq.h> >> + pcie_0: pcie@c00000000 { >> + compatible = "altr,pcie-root-port-1.0"; >> + reg = <0xc0000000 0x20000000>, >> + <0xff220000 0x00004000>; >> + reg-names = "Txs", "Cra"; >> + interrupt-parent = <&hps_0_arm_gic_0>; >> + interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>; >> + #interrupt-cells = <1>; >> + bus-range = <0x0 0xff>; >> + device_type = "pci"; >> + msi-parent = <&msi_to_gic_gen_0>; >> + #address-cells = <3>; >> + #size-cells = <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 = <0x82000000 0x00000000 0x00000000 0xc0000000 0x00000000 0x10000000 >> + 0x82000000 0x00000000 0x10000000 0xd0000000 0x00000000 0x10000000>; > > That's two entries. I will fix the broken DTS as follows: ranges = <0x82000000 0x00000000 0x00000000 0xc0000000 0x00000000 0x10000000>, <0x82000000 0x00000000 0x10000000 0xd0000000 0x00000000 0x10000000>; > > Best regards, > Krzysztof > > Thank you for the review, Matthew Gerlach
On 08/04/2024 22:34, matthew.gerlach@linux.intel.com wrote: >>> diff --git a/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml b/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml >>> new file mode 100644 >>> index 000000000000..999dcda05f55 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml >>> @@ -0,0 +1,106 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +# Copyright (C) 2024, Intel Corporation >> >> This is derivative of previous work, which is easily visible by doing >> the same mistakes in DTS as they were before. > > This is definitely derivative of previous work, and I want to fix the > DTS mistakes too. > >> >> You now added fresh copyrights ignoring all previous work, even though >> you copied it. I don't agree. >> >> If you want to ignore previous copyrights, then at least don't copy >> existing code... although even that would not be sufficient. > > Ignoring previous copyrights was not my intent. There is no copyright > statement in the original text version of the device tree bindings. Should > that lack of copyright statement carry forward? All the authors are copyright holders automatically, at least in some or maybe most jurisdictions. You do not need to add copyright label for material to be copyrighted. That's why you are not allowed to relicense the work for example, without other authors' agreement. The problem is that GPL requires to keep original copyright notices, but such notices were not present. Best regards, Krzysztof
On Tue, Apr 09, 2024 at 08:29:07AM +0200, Krzysztof Kozlowski wrote: > On 08/04/2024 22:34, matthew.gerlach@linux.intel.com wrote: > >>> diff --git a/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml b/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml > >>> new file mode 100644 > >>> index 000000000000..999dcda05f55 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml > >>> @@ -0,0 +1,106 @@ > >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > >>> +# Copyright (C) 2024, Intel Corporation > >> > >> This is derivative of previous work, which is easily visible by doing > >> the same mistakes in DTS as they were before. > > > > This is definitely derivative of previous work, and I want to fix the > > DTS mistakes too. > > > >> > >> You now added fresh copyrights ignoring all previous work, even though > >> you copied it. I don't agree. > >> > >> If you want to ignore previous copyrights, then at least don't copy > >> existing code... although even that would not be sufficient. > > > > Ignoring previous copyrights was not my intent. There is no copyright > > statement in the original text version of the device tree bindings. Should > > that lack of copyright statement carry forward? > > All the authors are copyright holders automatically, at least in some or > maybe most jurisdictions. You do not need to add copyright label for > material to be copyrighted. That's why you are not allowed to relicense > the work for example, without other authors' agreement. The only thing I see as missing is some years. All the authors were Altera which is now Intel, so Intel is the sole copyright holder. Whether is says 2015 contributions were Altera or Intel is probably immaterial. There were also contributions by Google (Bjorn), but those were purely editorial. Rob
On Fri, Apr 05, 2024 at 09:53:22AM -0500, matthew.gerlach@linux.intel.com wrote: > From: Matthew Gerlach <matthew.gerlach@linux.intel.com> > > Convert the device tree bindings for the Altera Root Port PCIe controller > from text to YAML. > > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com> > --- > v2: > - Move allOf: to bottom of file, just like example-schema is showing > - add constraint for reg and reg-names > - remove unneeded device_type > - drop #address-cells and #size-cells > - change minItems to maxItems for interrupts: > - change msi-parent to just "msi-parent: true" > - cleaned up required: > - make subject consistent with other commits coverting to YAML > - s/overt/onvert/g > --- > .../devicetree/bindings/pci/altera-pcie.txt | 50 --------- > .../bindings/pci/altr,pcie-root-port.yaml | 106 ++++++++++++++++++ > 2 files changed, 106 insertions(+), 50 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/pci/altera-pcie.txt > create mode 100644 Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml > > diff --git a/Documentation/devicetree/bindings/pci/altera-pcie.txt b/Documentation/devicetree/bindings/pci/altera-pcie.txt > deleted file mode 100644 > index 816b244a221e..000000000000 > --- a/Documentation/devicetree/bindings/pci/altera-pcie.txt > +++ /dev/null > @@ -1,50 +0,0 @@ > -* Altera PCIe controller > - > -Required properties: > -- compatible : should contain "altr,pcie-root-port-1.0" or "altr,pcie-root-port-2.0" > -- reg: a list of physical base address and length for TXS and CRA. > - For "altr,pcie-root-port-2.0", additional HIP base address and length. > -- reg-names: must include the following entries: > - "Txs": TX slave port region > - "Cra": Control register access region > - "Hip": Hard IP region (if "altr,pcie-root-port-2.0") > -- interrupts: specifies the interrupt source of the parent interrupt > - controller. The format of the interrupt specifier depends > - on the parent interrupt controller. > -- device_type: must be "pci" > -- #address-cells: set to <3> > -- #size-cells: set to <2> > -- #interrupt-cells: set to <1> > -- ranges: describes the translation of addresses for root ports and > - standard PCI regions. > -- interrupt-map-mask and interrupt-map: standard PCI properties to define the > - mapping of the PCIe interface to interrupt numbers. > - > -Optional properties: > -- msi-parent: Link to the hardware entity that serves as the MSI controller > - for this PCIe controller. > -- bus-range: PCI bus numbers covered > - > -Example > - pcie_0: pcie@c00000000 { > - compatible = "altr,pcie-root-port-1.0"; > - reg = <0xc0000000 0x20000000>, > - <0xff220000 0x00004000>; > - reg-names = "Txs", "Cra"; > - interrupt-parent = <&hps_0_arm_gic_0>; > - interrupts = <0 40 4>; > - interrupt-controller; > - #interrupt-cells = <1>; > - bus-range = <0x0 0xFF>; > - device_type = "pci"; > - msi-parent = <&msi_to_gic_gen_0>; > - #address-cells = <3>; > - #size-cells = <2>; > - interrupt-map-mask = <0 0 0 7>; > - interrupt-map = <0 0 0 1 &pcie_0 1>, > - <0 0 0 2 &pcie_0 2>, > - <0 0 0 3 &pcie_0 3>, > - <0 0 0 4 &pcie_0 4>; > - ranges = <0x82000000 0x00000000 0x00000000 0xc0000000 0x00000000 0x10000000 > - 0x82000000 0x00000000 0x10000000 0xd0000000 0x00000000 0x10000000>; > - }; > diff --git a/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml b/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml > new file mode 100644 > index 000000000000..999dcda05f55 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml > @@ -0,0 +1,106 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +# Copyright (C) 2024, Intel Corporation > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/altr,pcie-root-port.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Altera PCIe Root Port > + > +maintainers: > + - Matthew Gerlach <matthew.gerlach@linux.intel.com> > + > +properties: > + compatible: > + items: > + - enum: > + - altr,pcie-root-port-1.0 > + - altr,pcie-root-port-2.0 > + > + interrupts: > + maxItems: 1 > + > + interrupt-map-mask: > + items: > + - const: 0 > + - const: 0 > + - const: 0 > + - const: 7 > + > + interrupt-map: > + maxItems: 4 > + > + "#interrupt-cells": > + const: 1 Already defined in the common schema, drop. > + > + msi-parent: true > + > +required: > + - compatible > + - reg > + - reg-names > + - device_type Drop. > + - interrupts > + - interrupt-map > + - interrupt-map-mask > + > +unevaluatedProperties: false > + > +allOf: > + - $ref: /schemas/pci/pci-bus.yaml# > + - if: > + properties: > + compatible: > + enum: > + - altr,pcie-root-port-1.0 > + then: > + properties: > + reg: > + items: > + - description: TX slave port region > + - description: Control register access region > + > + reg-names: > + items: > + - const: Txs > + - const: Cra > + > + else: > + properties: > + reg: > + items: > + - description: Hard IP region > + - description: TX slave port region > + - description: Control register access region > + > + reg-names: > + items: > + - const: Hip > + - const: Txs > + - const: Cra > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + pcie_0: pcie@c00000000 { > + compatible = "altr,pcie-root-port-1.0"; > + reg = <0xc0000000 0x20000000>, > + <0xff220000 0x00004000>; > + reg-names = "Txs", "Cra"; > + interrupt-parent = <&hps_0_arm_gic_0>; > + interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>; > + #interrupt-cells = <1>; > + bus-range = <0x0 0xff>; > + device_type = "pci"; > + msi-parent = <&msi_to_gic_gen_0>; > + #address-cells = <3>; > + #size-cells = <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 = <0x82000000 0x00000000 0x00000000 0xc0000000 0x00000000 0x10000000 > + 0x82000000 0x00000000 0x10000000 0xd0000000 0x00000000 0x10000000>; > + }; > -- > 2.34.1 >
On Wed, 10 Apr 2024, Rob Herring wrote: > On Tue, Apr 09, 2024 at 08:29:07AM +0200, Krzysztof Kozlowski wrote: >> On 08/04/2024 22:34, matthew.gerlach@linux.intel.com wrote: >>>>> diff --git a/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml b/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml >>>>> new file mode 100644 >>>>> index 000000000000..999dcda05f55 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml >>>>> @@ -0,0 +1,106 @@ >>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>>>> +# Copyright (C) 2024, Intel Corporation >>>> >>>> This is derivative of previous work, which is easily visible by doing >>>> the same mistakes in DTS as they were before. >>> >>> This is definitely derivative of previous work, and I want to fix the >>> DTS mistakes too. >>> >>>> >>>> You now added fresh copyrights ignoring all previous work, even though >>>> you copied it. I don't agree. >>>> >>>> If you want to ignore previous copyrights, then at least don't copy >>>> existing code... although even that would not be sufficient. >>> >>> Ignoring previous copyrights was not my intent. There is no copyright >>> statement in the original text version of the device tree bindings. Should >>> that lack of copyright statement carry forward? >> >> All the authors are copyright holders automatically, at least in some or >> maybe most jurisdictions. You do not need to add copyright label for >> material to be copyrighted. That's why you are not allowed to relicense >> the work for example, without other authors' agreement. > > The only thing I see as missing is some years. All the authors were > Altera which is now Intel, so Intel is the sole copyright holder. > Whether is says 2015 contributions were Altera or Intel is probably > immaterial. There were also contributions by Google (Bjorn), but those > were purely editorial. Yes, Altera is now Intel who now owns the copyrights. At some point the future this might change as Intel spins off Altera. So given the state now, should the copyright line be as follows? # Copyright (C) 2015, 2019, 2024, Intel Corporation Matthew > > Rob >
On Wed, 10 Apr 2024, Rob Herring wrote: > On Fri, Apr 05, 2024 at 09:53:22AM -0500, matthew.gerlach@linux.intel.com wrote: >> From: Matthew Gerlach <matthew.gerlach@linux.intel.com> >> >> Convert the device tree bindings for the Altera Root Port PCIe controller >> from text to YAML. >> >> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com> >> --- >> v2: >> - Move allOf: to bottom of file, just like example-schema is showing >> - add constraint for reg and reg-names >> - remove unneeded device_type >> - drop #address-cells and #size-cells >> - change minItems to maxItems for interrupts: >> - change msi-parent to just "msi-parent: true" >> - cleaned up required: >> - make subject consistent with other commits coverting to YAML >> - s/overt/onvert/g >> --- >> .../devicetree/bindings/pci/altera-pcie.txt | 50 --------- >> .../bindings/pci/altr,pcie-root-port.yaml | 106 ++++++++++++++++++ >> 2 files changed, 106 insertions(+), 50 deletions(-) >> delete mode 100644 Documentation/devicetree/bindings/pci/altera-pcie.txt >> create mode 100644 Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml >> >> diff --git a/Documentation/devicetree/bindings/pci/altera-pcie.txt b/Documentation/devicetree/bindings/pci/altera-pcie.txt >> deleted file mode 100644 >> index 816b244a221e..000000000000 >> --- a/Documentation/devicetree/bindings/pci/altera-pcie.txt >> +++ /dev/null >> @@ -1,50 +0,0 @@ >> -* Altera PCIe controller >> - >> -Required properties: >> -- compatible : should contain "altr,pcie-root-port-1.0" or "altr,pcie-root-port-2.0" >> -- reg: a list of physical base address and length for TXS and CRA. >> - For "altr,pcie-root-port-2.0", additional HIP base address and length. >> -- reg-names: must include the following entries: >> - "Txs": TX slave port region >> - "Cra": Control register access region >> - "Hip": Hard IP region (if "altr,pcie-root-port-2.0") >> -- interrupts: specifies the interrupt source of the parent interrupt >> - controller. The format of the interrupt specifier depends >> - on the parent interrupt controller. >> -- device_type: must be "pci" >> -- #address-cells: set to <3> >> -- #size-cells: set to <2> >> -- #interrupt-cells: set to <1> >> -- ranges: describes the translation of addresses for root ports and >> - standard PCI regions. >> -- interrupt-map-mask and interrupt-map: standard PCI properties to define the >> - mapping of the PCIe interface to interrupt numbers. >> - >> -Optional properties: >> -- msi-parent: Link to the hardware entity that serves as the MSI controller >> - for this PCIe controller. >> -- bus-range: PCI bus numbers covered >> - >> -Example >> - pcie_0: pcie@c00000000 { >> - compatible = "altr,pcie-root-port-1.0"; >> - reg = <0xc0000000 0x20000000>, >> - <0xff220000 0x00004000>; >> - reg-names = "Txs", "Cra"; >> - interrupt-parent = <&hps_0_arm_gic_0>; >> - interrupts = <0 40 4>; >> - interrupt-controller; >> - #interrupt-cells = <1>; >> - bus-range = <0x0 0xFF>; >> - device_type = "pci"; >> - msi-parent = <&msi_to_gic_gen_0>; >> - #address-cells = <3>; >> - #size-cells = <2>; >> - interrupt-map-mask = <0 0 0 7>; >> - interrupt-map = <0 0 0 1 &pcie_0 1>, >> - <0 0 0 2 &pcie_0 2>, >> - <0 0 0 3 &pcie_0 3>, >> - <0 0 0 4 &pcie_0 4>; >> - ranges = <0x82000000 0x00000000 0x00000000 0xc0000000 0x00000000 0x10000000 >> - 0x82000000 0x00000000 0x10000000 0xd0000000 0x00000000 0x10000000>; >> - }; >> diff --git a/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml b/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml >> new file mode 100644 >> index 000000000000..999dcda05f55 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml >> @@ -0,0 +1,106 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +# Copyright (C) 2024, Intel Corporation >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/altr,pcie-root-port.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Altera PCIe Root Port >> + >> +maintainers: >> + - Matthew Gerlach <matthew.gerlach@linux.intel.com> >> + >> +properties: >> + compatible: >> + items: >> + - enum: >> + - altr,pcie-root-port-1.0 >> + - altr,pcie-root-port-2.0 >> + >> + interrupts: >> + maxItems: 1 >> + >> + interrupt-map-mask: >> + items: >> + - const: 0 >> + - const: 0 >> + - const: 0 >> + - const: 7 >> + >> + interrupt-map: >> + maxItems: 4 >> + >> + "#interrupt-cells": >> + const: 1 > > Already defined in the common schema, drop. When I remove the lines above, "make dt_binding_check" gives me the warning, properties: '#interrupt-cells' is a dependency of 'interrupt-map'. > >> + >> + msi-parent: true >> + >> +required: >> + - compatible >> + - reg >> + - reg-names >> + - device_type > > Drop. I will drop this in v3. > >> + - interrupts >> + - interrupt-map >> + - interrupt-map-mask >> + >> +unevaluatedProperties: false >> + >> +allOf: >> + - $ref: /schemas/pci/pci-bus.yaml# >> + - if: >> + properties: >> + compatible: >> + enum: >> + - altr,pcie-root-port-1.0 >> + then: >> + properties: >> + reg: >> + items: >> + - description: TX slave port region >> + - description: Control register access region >> + >> + reg-names: >> + items: >> + - const: Txs >> + - const: Cra >> + >> + else: >> + properties: >> + reg: >> + items: >> + - description: Hard IP region >> + - description: TX slave port region >> + - description: Control register access region >> + >> + reg-names: >> + items: >> + - const: Hip >> + - const: Txs >> + - const: Cra >> + >> +examples: >> + - | >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + #include <dt-bindings/interrupt-controller/irq.h> >> + pcie_0: pcie@c00000000 { >> + compatible = "altr,pcie-root-port-1.0"; >> + reg = <0xc0000000 0x20000000>, >> + <0xff220000 0x00004000>; >> + reg-names = "Txs", "Cra"; >> + interrupt-parent = <&hps_0_arm_gic_0>; >> + interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>; >> + #interrupt-cells = <1>; >> + bus-range = <0x0 0xff>; >> + device_type = "pci"; >> + msi-parent = <&msi_to_gic_gen_0>; >> + #address-cells = <3>; >> + #size-cells = <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 = <0x82000000 0x00000000 0x00000000 0xc0000000 0x00000000 0x10000000 >> + 0x82000000 0x00000000 0x10000000 0xd0000000 0x00000000 0x10000000>; >> + }; >> -- >> 2.34.1 >> >
On 12/04/2024 02:04, matthew.gerlach@linux.intel.com wrote: >>> + interrupt-map: >>> + maxItems: 4 >>> + >>> + "#interrupt-cells": >>> + const: 1 >> >> Already defined in the common schema, drop. > > When I remove the lines above, "make dt_binding_check" gives me the > warning, properties: '#interrupt-cells' is a dependency of 'interrupt-map'. Could be, reminds me why we must have "reg:" in cdns,cdns-pcie-ep.yaml (and host), not in shared cdns-pcie.yaml schema. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/pci/altera-pcie.txt b/Documentation/devicetree/bindings/pci/altera-pcie.txt deleted file mode 100644 index 816b244a221e..000000000000 --- a/Documentation/devicetree/bindings/pci/altera-pcie.txt +++ /dev/null @@ -1,50 +0,0 @@ -* Altera PCIe controller - -Required properties: -- compatible : should contain "altr,pcie-root-port-1.0" or "altr,pcie-root-port-2.0" -- reg: a list of physical base address and length for TXS and CRA. - For "altr,pcie-root-port-2.0", additional HIP base address and length. -- reg-names: must include the following entries: - "Txs": TX slave port region - "Cra": Control register access region - "Hip": Hard IP region (if "altr,pcie-root-port-2.0") -- interrupts: specifies the interrupt source of the parent interrupt - controller. The format of the interrupt specifier depends - on the parent interrupt controller. -- device_type: must be "pci" -- #address-cells: set to <3> -- #size-cells: set to <2> -- #interrupt-cells: set to <1> -- ranges: describes the translation of addresses for root ports and - standard PCI regions. -- interrupt-map-mask and interrupt-map: standard PCI properties to define the - mapping of the PCIe interface to interrupt numbers. - -Optional properties: -- msi-parent: Link to the hardware entity that serves as the MSI controller - for this PCIe controller. -- bus-range: PCI bus numbers covered - -Example - pcie_0: pcie@c00000000 { - compatible = "altr,pcie-root-port-1.0"; - reg = <0xc0000000 0x20000000>, - <0xff220000 0x00004000>; - reg-names = "Txs", "Cra"; - interrupt-parent = <&hps_0_arm_gic_0>; - interrupts = <0 40 4>; - interrupt-controller; - #interrupt-cells = <1>; - bus-range = <0x0 0xFF>; - device_type = "pci"; - msi-parent = <&msi_to_gic_gen_0>; - #address-cells = <3>; - #size-cells = <2>; - interrupt-map-mask = <0 0 0 7>; - interrupt-map = <0 0 0 1 &pcie_0 1>, - <0 0 0 2 &pcie_0 2>, - <0 0 0 3 &pcie_0 3>, - <0 0 0 4 &pcie_0 4>; - ranges = <0x82000000 0x00000000 0x00000000 0xc0000000 0x00000000 0x10000000 - 0x82000000 0x00000000 0x10000000 0xd0000000 0x00000000 0x10000000>; - }; diff --git a/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml b/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml new file mode 100644 index 000000000000..999dcda05f55 --- /dev/null +++ b/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml @@ -0,0 +1,106 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +# Copyright (C) 2024, Intel Corporation +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/altr,pcie-root-port.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Altera PCIe Root Port + +maintainers: + - Matthew Gerlach <matthew.gerlach@linux.intel.com> + +properties: + compatible: + items: + - enum: + - altr,pcie-root-port-1.0 + - altr,pcie-root-port-2.0 + + interrupts: + maxItems: 1 + + interrupt-map-mask: + items: + - const: 0 + - const: 0 + - const: 0 + - const: 7 + + interrupt-map: + maxItems: 4 + + "#interrupt-cells": + const: 1 + + msi-parent: true + +required: + - compatible + - reg + - reg-names + - device_type + - interrupts + - interrupt-map + - interrupt-map-mask + +unevaluatedProperties: false + +allOf: + - $ref: /schemas/pci/pci-bus.yaml# + - if: + properties: + compatible: + enum: + - altr,pcie-root-port-1.0 + then: + properties: + reg: + items: + - description: TX slave port region + - description: Control register access region + + reg-names: + items: + - const: Txs + - const: Cra + + else: + properties: + reg: + items: + - description: Hard IP region + - description: TX slave port region + - description: Control register access region + + reg-names: + items: + - const: Hip + - const: Txs + - const: Cra + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/interrupt-controller/irq.h> + pcie_0: pcie@c00000000 { + compatible = "altr,pcie-root-port-1.0"; + reg = <0xc0000000 0x20000000>, + <0xff220000 0x00004000>; + reg-names = "Txs", "Cra"; + interrupt-parent = <&hps_0_arm_gic_0>; + interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>; + #interrupt-cells = <1>; + bus-range = <0x0 0xff>; + device_type = "pci"; + msi-parent = <&msi_to_gic_gen_0>; + #address-cells = <3>; + #size-cells = <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 = <0x82000000 0x00000000 0x00000000 0xc0000000 0x00000000 0x10000000 + 0x82000000 0x00000000 0x10000000 0xd0000000 0x00000000 0x10000000>; + };