Re: [PATCH v5 3/5] arm64: dts: agilex: add dtsi for PCIe Root Port

[matthew.gerlach@linux.intel.com]

On Sun, 2 Feb 2025, Krzysztof Kozlowski wrote:

> On 01/02/2025 20:12, matthew.gerlach@linux.intel.com wrote:
>>>
>>>> they are also referenced in the following:
>>>>      Documentation/devicetree/bindings/soc/intel/intel,hps-copy-engine.yaml
>>>>      arch/arm64/boot/dts/intel/socfpga_agilex_n6000.dts
>>>> I am not exactly sure where the right place is to define them, maybe
>>>> Documentation/devicetree/bindings/arm/intel,socfpga.yaml. On the other
>>>> hand, no code references these names; so it might make sense to just
>>>> remove them.
>>>
>>> In general: nowhere, because simple bus does not have such properties.
>>> It's not about reg-names only - you cannot have reg. You just did not
>>> define here simple-bus.
>>
>> I understand. I will remove reg and reg-names.
>
> If you have there IO address space, then removal does not sound right,
> either. You just need to come with the bindings for this dedicated
> device, whatever this is. There is no description here, not much in
> commit msg, so I don't know what is the device you are adding. PCI has
> several bindings, so is this just host bridge?

The device associated with two address ranges may be best described as a 
simple-bus. It is a bus between the CPU and the directly connected FPGA in 
the same package as the SOC. The design programmed into the FPGA 
determines the device(s) connected to the bus. The hardware implementing 
this bus does have reset lines which allow for safely reprogramming the 
FPGA while the SOC is running, which implies appropriate bindings as you 
suggest. Something like the following might make sense:

 	aglx_hps_bridges: fpga-bus@80000000 {
 		compatible = "altr,agilex-hps-fpga-bridge", "simple-bus";
 		reg = <0x80000000 0x20200000>,
 		      <0xf9000000 0x00100000>;
 		reg-names = "axi_h2f", "axi_h2f_lw";
 		#address-cells = <0x2>;
 		#size-cells = <0x1>;
 		ranges = <0x00000000 0x00000000 0x80000000 0x00040000>,
 			 <0x00000000 0x10000000 0x90100000 0x0ff00000>,
 			 <0x00000000 0x20000000 0xa0000000 0x00200000>,
 			 <0x00000001 0x00010000 0xf9010000 0x00008000>,
 			 <0x00000001 0x00018000 0xf9018000 0x00000080>,
 			 <0x00000001 0x00018080 0xf9018080 0x00000010>;
 		reset = <&rst SOC2FPGA_RESET>, <&rst LWHPS2FPGA_RESET>;
 		reset-names = "soc2fpga", "lwhps2fpga";
 		...
 	};

>
> Best regards,
> Krzysztof
>
Thank you for the feedback,
Matthew Gerlach