[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.22.394.2405141044470.540832@sj-4150-psse-sw-opae-dev2>
Date: Tue, 14 May 2024 11:30:05 -0700 (PDT)
From: matthew.gerlach@...ux.intel.com
To: Rob Herring <robh@...nel.org>
cc: linux-kernel@...r.kernel.org, conor+dt@...nel.org, lpieralisi@...nel.org,
krzysztof.kozlowski+dt@...aro.org, kw@...ux.com, bhelgaas@...gle.com,
linux-pci@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v5] dt-bindings: PCI: altera: Convert to YAML
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@...000000: 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@...10000000 {
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@...00014080 {
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
>
Powered by blists - more mailing lists