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

[Sushrut Shree Trivedi <sushrut.trivedi@oss.qualcomm.com>]

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
>