Re: [PATCH v5 13/14] arm64: dts: cix: Add PCIe Root Complex on sky1

[Hans Zhang <hans.zhang@cixtech.com>]

On 2025/6/30 15:33, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL
> 
> On Mon, Jun 30, 2025 at 12:16:00PM +0800, hans.zhang@cixtech.com wrote:
>> From: Hans Zhang <hans.zhang@cixtech.com>
>>
>> Add pcie_x*_rc node to support Sky1 PCIe driver based on the
>> Cadence PCIe core.
>>
>> Supports Gen1/Gen2/Gen3/Gen4, 1/2/4/8 lane, MSI/MSI-x interrupts
>> using the ARM GICv3.
>>
>> Signed-off-by: Hans Zhang <hans.zhang@cixtech.com>
>> Reviewed-by: Peter Chen <peter.chen@cixtech.com>
>> Reviewed-by: Manikandan K Pillai <mpillai@cadence.com>
> 
> Where?
Dear Krzysztof,

Thank you very much for your reply. Will delete.

> 
>> ---
>>   arch/arm64/boot/dts/cix/sky1.dtsi | 150 ++++++++++++++++++++++++++++++
>>   1 file changed, 150 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/cix/sky1.dtsi b/arch/arm64/boot/dts/cix/sky1.dtsi
>> index 9c723917d8ca..1dac0e8d5fc1 100644
>> --- a/arch/arm64/boot/dts/cix/sky1.dtsi
>> +++ b/arch/arm64/boot/dts/cix/sky1.dtsi
>> @@ -289,6 +289,156 @@ mbox_ap2sfh: mailbox@80a0000 {
>>                        cix,mbox-dir = "tx";
>>                };
>>
>> +             pcie_x8_rc: pcie@a010000 { /* X8 */
>> +                     compatible = "cix,sky1-pcie-host";
>> +                     reg = <0x00 0x0a010000 0x00 0x10000>,
>> +                           <0x00 0x0a000000 0x00 0x10000>,
>> +                           <0x00 0x2c000000 0x00 0x4000000>,
>> +                           <0x00 0x60000000 0x00 0x00100000>;
>> +                     reg-names = "reg", "rcsu", "cfg", "msg";
>> +                     #interrupt-cells = <1>;
>> +                     interrupt-map-mask = <0 0 0 0x7>;
>> +                     interrupt-map = <0 0 0 1 &gic 0 0 GIC_SPI 407 IRQ_TYPE_LEVEL_HIGH 0>,
>> +                                     <0 0 0 2 &gic 0 0 GIC_SPI 408 IRQ_TYPE_LEVEL_HIGH 0>,
>> +                                     <0 0 0 3 &gic 0 0 GIC_SPI 409 IRQ_TYPE_LEVEL_HIGH 0>,
>> +                                     <0 0 0 4 &gic 0 0 GIC_SPI 410 IRQ_TYPE_LEVEL_HIGH 0>;
>> +                     max-link-speed = <4>;
>> +                     num-lanes = <8>;
>> +                     #address-cells = <3>;
>> +                     #size-cells = <2>;
>> +                     bus-range = <0xc0 0xff>;
>> +                     device_type = "pci";
>> +                     ranges = <0x01000000 0x0 0x60100000 0x0 0x60100000 0x0 0x00100000>,
>> +                              <0x02000000 0x0 0x60200000 0x0 0x60200000 0x0 0x1fe00000>,
>> +                              <0x43000000 0x18 0x00000000 0x18 0x00000000 0x04 0x00000000>;
> 
> And none of the two reviewers asked you to follow DTS coding style? If
> reviewer knows not much about DTS, don't review. Add an ack or
> something, dunno, or actually perform proper review.
> 

Understood.

For the arrangement of attributes this time, I referred to the following 
submission:

linux master branch：
arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
pcie2x1l2: pcie@fe190000

Submissions under review:
https://patchwork.kernel.org/project/linux-pci/patch/20250610090714.3321129-8-christian.bruel@foss.st.com/


Then should I follow the following documents exactly?

Documentation/devicetree/bindings/dts-coding-style.rst
The following order of properties in device nodes is preferred:

1. "compatible"
2. "reg"
3. "ranges"
4. Standard/common properties (defined by common bindings, e.g. without
    vendor-prefixes)
5. Vendor-specific properties
6. "status" (if applicable)
7. Child nodes, where each node is preceded with a blank line



Best regards,
Hans

> Best regards,
> Krzysztof
>