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

[Hans Zhang]

On 2025/6/30 15:33, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL
>
> On Mon, Jun 30, 2025 at 12:16:00PM +0800, hans.zhang@xxxxxxxxxxx wrote:
> > From: Hans Zhang <hans.zhang@xxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxx>
> > Reviewed-by: Peter Chen <peter.chen@xxxxxxxxxxx>
> > Reviewed-by: Manikandan K Pillai <mpillai@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxx/


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