| Message ID | 20240513205913.313592-1-matthew.gerlach@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v5] dt-bindings: PCI: altera: Convert to YAML | expand |
On Mon, 13 May 2024 15:59:13 -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> > --- > v5: > - add interrupt-conntroller #interrupt-cells to required field > - don't touch original example dts > > v4: > - reorder reg-names to match original binding > - move reg and reg-names to top level with limits. > > v3: > - Added years to copyright > - Correct order in file of allOf and unevaluatedProperties > - remove items: in compatible field > - fix reg and reg-names constraints > - replace deprecated pci-bus.yaml with pci-host-bridge.yaml > - fix entries in ranges property > - remove device_type from required > > 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 | 93 +++++++++++++++++++ > 2 files changed, 93 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 > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/altr,pcie-root-port.example.dtb: pcie@c00000000: interrupt-map: [[0, 0, 0, 1, 2, 1, 0, 0, 0], [2, 2, 2, 0, 0, 0, 3, 2, 3], [0, 0, 0, 4, 2, 4]] is too short from schema $id: http://devicetree.org/schemas/altr,pcie-root-port.yaml# doc reference errors (make refcheckdocs): Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/pci/altera-pcie.txt MAINTAINERS: Documentation/devicetree/bindings/pci/altera-pcie.txt See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240513205913.313592-1-matthew.gerlach@linux.intel.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus linus/master v6.9 next-20240513]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/matthew-gerlach-linux-intel-com/dt-bindings-PCI-altera-Convert-to-YAML/20240514-050049
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/r/20240513205913.313592-1-matthew.gerlach%40linux.intel.com
patch subject: [PATCH v5] dt-bindings: PCI: altera: Convert to YAML
reproduce: (https://download.01.org/0day-ci/archive/20240514/202405140709.5BmKZtT7-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405140709.5BmKZtT7-lkp@intel.com/
All warnings (new ones prefixed by >>):
Warning: Documentation/devicetree/bindings/regulator/siliconmitus,sm5703-regulator.yaml references a file that doesn't exist: Documentation/devicetree/bindings/mfd/siliconmitus,sm5703.yaml
Warning: Documentation/devicetree/bindings/sound/fsl-asoc-card.txt references a file that doesn't exist: Documentation/devicetree/bindings/sound/fsl,asrc.txt
Warning: Documentation/gpu/amdgpu/display/display-contributing.rst references a file that doesn't exist: Documentation/GPU/amdgpu/display/mpo-overview.rst
Warning: Documentation/userspace-api/netlink/index.rst references a file that doesn't exist: Documentation/networking/netlink_spec/index.rst
Warning: Documentation/userspace-api/netlink/specs.rst references a file that doesn't exist: Documentation/networking/netlink_spec/index.rst
>> Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/pci/altera-pcie.txt
Using alabaster theme
On Mon, May 13, 2024 at 05:12:42PM -0500, Rob Herring (Arm) wrote: > > On Mon, 13 May 2024 15:59:13 -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> > > --- > > v5: > > - add interrupt-conntroller #interrupt-cells to required field > > - don't touch original example dts > > > > v4: > > - reorder reg-names to match original binding > > - move reg and reg-names to top level with limits. > > > > v3: > > - Added years to copyright > > - Correct order in file of allOf and unevaluatedProperties > > - remove items: in compatible field > > - fix reg and reg-names constraints > > - replace deprecated pci-bus.yaml with pci-host-bridge.yaml > > - fix entries in ranges property > > - remove device_type from required > > > > 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 | 93 +++++++++++++++++++ > > 2 files changed, 93 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 > > > > My bot found errors running 'make dt_binding_check' on your patch: > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/altr,pcie-root-port.example.dtb: pcie@c00000000: interrupt-map: [[0, 0, 0, 1, 2, 1, 0, 0, 0], [2, 2, 2, 0, 0, 0, 3, 2, 3], [0, 0, 0, 4, 2, 4]] is too short > from schema $id: http://devicetree.org/schemas/altr,pcie-root-port.yaml# You need 3 address cells after the phandles since the interrupt parent has 3 address cells. What does your actual DT contain and do interrupts work because interrupts never would have worked I think? Making the PCI host the interrupt parent didn't even work in the kernel until somewhat recently (maybe a few years now). That's why a bunch of PCI hosts have an interrupt-controller child node. Rob
On Tue, 14 May 2024, Rob Herring wrote: >>> >> >> My bot found errors running 'make dt_binding_check' on your patch: >> >> yamllint warnings/errors: >> >> dtschema/dtc warnings/errors: >> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/altr,pcie-root-port.example.dtb: pcie@c00000000: interrupt-map: [[0, 0, 0, 1, 2, 1, 0, 0, 0], [2, 2, 2, 0, 0, 0, 3, 2, 3], [0, 0, 0, 4, 2, 4]] is too short >> from schema $id: http://devicetree.org/schemas/altr,pcie-root-port.yaml# > > You need 3 address cells after the phandles since the interrupt parent > has 3 address cells. Thanks for the extra explanation. Adding 3 address cells of 0 made the warning go away. > > What does your actual DT contain and do interrupts work because > interrupts never would have worked I think? Making the PCI host the > interrupt parent didn't even work in the kernel until somewhat recently > (maybe a few years now). That's why a bunch of PCI hosts have an > interrupt-controller child node. The following DT snippet comes from https://www.rocketboards.org/foswiki/Projects/Stratix10PCIeRootPortWithMSI The Linux kernel version is 4.14.130-ltsi. Would the use of the msi-parent node make everything work? pcie_1_pcie_a10_hip_avmm: pcie@0x010000000 { compatible = "altr,pcie-root-port-1.0"; reg = <0xd0000000 0x10000000>, <0xff210000 0x00004000>; reg-names = "Txs", "Cra"; interrupt-parent = <&arria10_hps_0_arm_gic_0>; interrupts = <0 24 4>; interrupt-controller; #interrupt-cells = <1>; device_type = "pci"; msi-parent = <&pcie_0_msi_to_gic_gen_0>; bus-range = <0x00000000 0x000000ff>; #address-cells = <3>; #size-cells = <2>; ranges = <0x82000000 0x00000000 0x00000000 0xd0000000 0x00000000 0x10000000>; interrupt-map-mask = <0 0 0 7>; interrupt-map = <0 0 0 1 &pcie_0_pcie_a10_hip_avmm 1>, <0 0 0 2 &pcie_0_pcie_a10_hip_avmm 2>, <0 0 0 3 &pcie_0_pcie_a10_hip_avmm 3>, <0 0 0 4 &pcie_0_pcie_a10_hip_avmm 4>; }; pcie_0_msi_to_gic_gen_0: msi@0x100014080 { compatible = "altr,msi-1.0", "altr,msi-1.0"; reg = <0x00000001 0x00014080 0x00000010>, <0x00000001 0x00014000 0x00000080>; reg-names = "csr", "vector_slave"; interrupt-parent = <&arria10_hps_0_arm_gic_0>; interrupts = <0 22 4>; clocks = <&pcie_0_clk_100>; msi-controller = <1>; num-vectors = <32>; }; I am doing something similar with newer HIP with the 6.1.68-lts, and it seems to work. I had experimented with an interrupt-controller child node, but I wanted to maintain the existing binding definition and compatibility with the accepted driver code. Matthew Gerlach > > Rob >
On Tue, May 14, 2024 at 11:30:05AM -0700, matthew.gerlach@linux.intel.com wrote: > > > On Tue, 14 May 2024, Rob Herring wrote: > > > > > > > > > > > My bot found errors running 'make dt_binding_check' on your patch: > > > > > > yamllint warnings/errors: > > > > > > dtschema/dtc warnings/errors: > > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/altr,pcie-root-port.example.dtb: pcie@c00000000: interrupt-map: [[0, 0, 0, 1, 2, 1, 0, 0, 0], [2, 2, 2, 0, 0, 0, 3, 2, 3], [0, 0, 0, 4, 2, 4]] is too short > > > from schema $id: http://devicetree.org/schemas/altr,pcie-root-port.yaml# > > > > You need 3 address cells after the phandles since the interrupt parent > > has 3 address cells. > > Thanks for the extra explanation. Adding 3 address cells of 0 made the > warning go away. > > > > > What does your actual DT contain and do interrupts work because > > interrupts never would have worked I think? Making the PCI host the > > interrupt parent didn't even work in the kernel until somewhat recently > > (maybe a few years now). That's why a bunch of PCI hosts have an > > interrupt-controller child node. > > The following DT snippet comes from > https://www.rocketboards.org/foswiki/Projects/Stratix10PCIeRootPortWithMSI > > The Linux kernel version is 4.14.130-ltsi. Would the use of the msi-parent > node make everything work? Possibly? I would think MSIs are preferred and almost anything should support MSIs now. Rob
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..7f02e7fb33e1 --- /dev/null +++ b/Documentation/devicetree/bindings/pci/altr,pcie-root-port.yaml @@ -0,0 +1,93 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +# Copyright (C) 2015, 2019, 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: + enum: + - altr,pcie-root-port-1.0 + - altr,pcie-root-port-2.0 + + reg: + items: + - description: TX slave port region + - description: Control register access region + - description: Hard IP region + minItems: 2 + + reg-names: + items: + - const: Txs + - const: Cra + - const: Hip + minItems: 2 + + interrupts: + maxItems: 1 + + interrupt-controller: true + + 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 + - interrupts + - "#interrupt-cells" + - interrupt-controller + - interrupt-map + - interrupt-map-mask + +allOf: + - $ref: /schemas/pci/pci-host-bridge.yaml# + +unevaluatedProperties: false + +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-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>; + };