From: Matthew Gerlach <[email protected]>
Convert the device tree bindings for the Altera Root Port PCIe controller
from text to YAML.
Signed-off-by: Matthew Gerlach <[email protected]>
---
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 <[email protected]>
+
+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>;
+ };
--
2.34.1
On 05/04/2024 16:53, [email protected] wrote:
> From: Matthew Gerlach <[email protected]>
>
> Convert the device tree bindings for the Altera Root Port PCIe controller
> from text to YAML.
>
> Signed-off-by: Matthew Gerlach <[email protected]>
> ---
> 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 <[email protected]>
> +
> +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, [email protected] wrote:
>> From: Matthew Gerlach <[email protected]>
>>
>> Convert the device tree bindings for the Altera Root Port PCIe controller
>> from text to YAML.
>>
>> Signed-off-by: Matthew Gerlach <[email protected]>
>> ---
>> 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 <[email protected]>
>> +
>> +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, [email protected] 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, [email protected] 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, [email protected] wrote:
> From: Matthew Gerlach <[email protected]>
>
> Convert the device tree bindings for the Altera Root Port PCIe controller
> from text to YAML.
>
> Signed-off-by: Matthew Gerlach <[email protected]>
> ---
> 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 <[email protected]>
> +
> +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, [email protected] 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, [email protected] wrote:
>> From: Matthew Gerlach <[email protected]>
>>
>> Convert the device tree bindings for the Altera Root Port PCIe controller
>> from text to YAML.
>>
>> Signed-off-by: Matthew Gerlach <[email protected]>
>> ---
>> 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 <[email protected]>
>> +
>> +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, [email protected] 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