Received-SPF: pass (google.com: domain of linux-kernel+bounces-179058-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Tue, 14 May 2024 11:30:05 -0700 (PDT)
From: matthew.gerlach@linux.intel.com
To: Rob Herring <robh@kernel.org>
cc: linux-kernel@vger.kernel.org, conor+dt@kernel.org, lpieralisi@kernel.org, 
    krzysztof.kozlowski+dt@linaro.org, kw@linux.com, bhelgaas@google.com, 
    linux-pci@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v5] dt-bindings: PCI: altera: Convert to YAML
In-Reply-To: <20240514131750.GA1214311-robh@kernel.org>
Message-ID: <alpine.DEB.2.22.394.2405141044470.540832@sj-4150-psse-sw-opae-dev2>
References: <20240513205913.313592-1-matthew.gerlach@linux.intel.com> <171563836233.3319279.14962600621083837198.robh@kernel.org> <20240514131750.GA1214311-robh@kernel.org>
User-Agent: Alpine 2.22 (DEB 394 2020-01-19)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII; format=flowed


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
>