Re: [PATCH 2/9] dt-bindings: PCI: qcom: Document the Shikra PCIe Controller

[Sushrut Shree Trivedi]

On 8/12/2026 6:33 PM, Manivannan Sadhasivam wrote:
> On Wed, Aug 12, 2026 at 11:25:12AM +0530, Sushrut Shree Trivedi wrote:
>
> [...]
>
> > > > +    #include <dt-bindings/clock/qcom,rpmcc.h>
> > > > +
> > > > +    soc {
> > > > +        #address-cells = <2>;
> > > > +        #size-cells = <2>;
> > > > +
> > > Get rid of 'soc' node.
> > If we remove soc node, dt-schema uses address-cells = <1>,
> > size-cells  = <1> for dt-binding check and throws errors.
> >
> > Do you suggest ignoring those errors ?
> No, you need to make the 'reg' entries 1 cell.

As discussed separately, since we have a 64 bit BAR region
specified inside ranges, we need the soc node and the
associated address-cells, size-cells update to avoid
dt-binding errors.

> > > > +        pcie@45e8000 {
> > > > +          device_type = "pci";
> > > > +          compatible = "qcom,shikra-pcie";
> > > > +          reg = <0x0 0x045e8000 0x0 0x3000>,
> > > > +                <0x0 0x60000000 0x0 0xf1d>,
> > > > +                <0x0 0x60000f20 0x0 0xa8>,
> > > > +                <0x0 0x60001000 0x0 0x1000>,
> > > > +                <0x0 0x60100000 0x0 0x100000>,
> > > > +                <0x0 0x045eb000 0x0 0x1000>;
> > > > +          reg-names = "parf",
> > > > +                      "dbi",
> > > > +                      "elbi",
> > > > +                      "atu",
> > > > +                      "config",
> > > > +                      "mhi";
> > > > +          #address-cells = <3>;
> > > > +          #size-cells = <2>;
> > > > +          ranges = <0x01000000 0x0 0x00000000 0x0 0x60200000 0x0 0x100000>,
> > > > +                   <0x02000000 0x0 0x60300000 0x0 0x60300000 0x0 0x3d00000>,
> > > > +                   <0x03000000 0x4 0x00000000 0x4 0x00000000 0x3 0x0000000>;
> > > > +          bus-range = <0x00 0xff>;
> > > > +
> > > > +          linux,pci-domain = <0>;
> > > > +          num-lanes = <1>;
> > > > +
> > > > +          interrupts = <GIC_SPI 491 IRQ_TYPE_LEVEL_HIGH 0>,
> > > > +                       <GIC_SPI 492 IRQ_TYPE_LEVEL_HIGH 0>,
> > > > +                       <GIC_SPI 493 IRQ_TYPE_LEVEL_HIGH 0>,
> > > > +                       <GIC_SPI 494 IRQ_TYPE_LEVEL_HIGH 0>,
> > > > +                       <GIC_SPI 495 IRQ_TYPE_LEVEL_HIGH 0>,
> > > > +                       <GIC_SPI 496 IRQ_TYPE_LEVEL_HIGH 0>,
> > > > +                       <GIC_SPI 497 IRQ_TYPE_LEVEL_HIGH 0>,
> > > > +                       <GIC_SPI 498 IRQ_TYPE_LEVEL_HIGH 0>,
> > > > +                       <GIC_SPI 489 IRQ_TYPE_LEVEL_HIGH 0>;
> > > > +          interrupt-names = "msi0",
> > > > +                            "msi1",
> > > > +                            "msi2",
> > > > +                            "msi3",
> > > > +                            "msi4",
> > > > +                            "msi5",
> > > > +                            "msi6",
> > > > +                            "msi7",
> > > > +                            "global";
> > > > +
> > > > +          interrupt-map = <0 0 0 1 &intc 0 0 0 499 IRQ_TYPE_LEVEL_HIGH>,
> > > > +                          <0 0 0 2 &intc 0 0 0 500 IRQ_TYPE_LEVEL_HIGH>,
> > > > +                          <0 0 0 3 &intc 0 0 0 501 IRQ_TYPE_LEVEL_HIGH>,
> > > > +                          <0 0 0 4 &intc 0 0 0 502 IRQ_TYPE_LEVEL_HIGH>;
> > > > +          interrupt-map-mask = <0 0 0 0x7>;
> > > > +          #interrupt-cells = <1>;
> > > > +
> > > > +          clocks = <&gcc GCC_PCIE_AUX_CLK>,
> > > > +                   <&gcc GCC_PCIE_CFG_AHB_CLK>,
> > > > +                   <&gcc GCC_PCIE_MSTR_AXI_CLK>,
> > > > +                   <&gcc GCC_PCIE_SLV_AXI_CLK>,
> > > > +                   <&gcc GCC_PCIE_SLV_Q2A_AXI_CLK>,
> > > > +                   <&gcc GCC_DDRSS_MEMNOC_PCIE_SF_CLK>,
> > > > +                   <&gcc GCC_PCIE_TILE_AXI_SYS_NOC_CLK>,
> > > > +                   <&gcc GCC_QMIP_PCIE_CFG_AHB_CLK>;
> > > > +          clock-names = "aux",
> > > > +                        "cfg",
> > > > +                        "bus_master",
> > > > +                        "bus_slave",
> > > > +                        "slave_q2a",
> > > > +                        "ddrss_memnoc_pcie",
> > > > +                        "tile",
> > > > +                        "qmip_pcie_ahb";
> > > > +
> > > > +          assigned-clocks = <&gcc GCC_PCIE_AUX_CLK>;
> > > > +          assigned-clock-rates = <19200000>;
> > > > +
> > > > +          interconnects = <&system_noc MASTER_PCIE2_0 RPM_ALWAYS_TAG
> > > > +                          &mc_virt SLAVE_EBI_CH0 RPM_ALWAYS_TAG>,
> > > > +                          <&mem_noc MASTER_AMPSS_M0 RPM_ACTIVE_TAG
> > > > +                          &config_noc SLAVE_PCIE2_0 RPM_ACTIVE_TAG>;
> > > > +
> > > > +          interconnect-names = "pcie-mem",
> > > > +                               "cpu-pcie";
> > > > +
> > > > +          iommu-map = <0x0 &apps_smmu 0x800 0x1>,
> > > > +                      <0x100 &apps_smmu 0x801 0x1>;
> > > > +
> > > > +          resets = <&gcc GCC_PCIE_BCR>;
> > > > +          reset-names = "pci";
> > > > +
> > > > +          power-domains = <&gcc GCC_PCIE_GDSC>;
> > > > +
> > > > +          max-link-speed = <2>;
> > > You don't need 'max-link-speed' unless you want to limit the link speed.
> > This is needed since Shikra controller supports Gen3 but the PHY
> > is only capable of Gen2. Without limiting the speed, link-up fails.
> This is a critical limitation that should've been mentioned in the commit
> message.
>
> - Mani
ACK
>