Re: [PATCH v5] dt-bindings: PCI: altera: Convert to YAML

matthew.gerlach
2024-05-14T18:30:15Z

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
>