[PATCH v2 4/6] Revert "PCI: qcom: Enable MSI interrupts together with Link up if 'Global IRQ' is supported"
This reverts commit ba4a2e2317b9faeca9193ed6d3193ddc3cf2aba3.
While this fake hotplugging was a nice idea, it has shown that this feature does not handle PCIe switches correctly: pci_bus 0004:43: busn_res: can not insert [bus 43-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:43: busn_res: [bus 43-41] end is updated to 43 pci_bus 0004:43: busn_res: can not insert [bus 43] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:00.0: devices behind bridge are unusable because [bus 43] cannot be assigned for them pci_bus 0004:44: busn_res: can not insert [bus 44-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:44: busn_res: [bus 44-41] end is updated to 44 pci_bus 0004:44: busn_res: can not insert [bus 44] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:02.0: devices behind bridge are unusable because [bus 44] cannot be assigned for them pci_bus 0004:45: busn_res: can not insert [bus 45-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:45: busn_res: [bus 45-41] end is updated to 45 pci_bus 0004:45: busn_res: can not insert [bus 45] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:06.0: devices behind bridge are unusable because [bus 45] cannot be assigned for them pci_bus 0004:46: busn_res: can not insert [bus 46-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:46: busn_res: [bus 46-41] end is updated to 46 pci_bus 0004:46: busn_res: can not insert [bus 46] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:0e.0: devices behind bridge are unusable because [bus 46] cannot be assigned for them pci_bus 0004:42: busn_res: [bus 42-41] end is updated to 46 pci_bus 0004:42: busn_res: can not insert [bus 42-46] under [bus 41] (conflicts with (null) [bus 41]) pci 0004:41:00.0: devices behind bridge are unusable because [bus 42-46] cannot be assigned for them pcieport 0004:40:00.0: bridge has subordinate 41 but max busn 46
During the initial scan, PCI core doesn't see the switch and since the Root Port is not hot plug capable, the secondary bus number gets assigned as the subordinate bus number. This means, the PCI core assumes that only one bus will appear behind the Root Port since the Root Port is not hot plug capable.
This works perfectly fine for PCIe endpoints connected to the Root Port, since they don't extend the bus. However, if a PCIe switch is connected, then there is a problem when the downstream busses starts showing up and the PCI core doesn't extend the subordinate bus number after initial scan during boot.
The long term plan is to migrate this driver to the pwrctrl framework, once it adds proper support for powering up and enumerating PCIe switches.
Cc: stable@vger.kernel.org Suggested-by: Manivannan Sadhasivam mani@kernel.org Acked-by: Shawn Lin shawn.lin@rock-chips.com Tested-by: Shawn Lin shawn.lin@rock-chips.com Signed-off-by: Niklas Cassel cassel@kernel.org --- drivers/pci/controller/dwc/pcie-qcom.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index e87ec6779d44..c5fcb87972e9 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -136,7 +136,6 @@
/* PARF_INT_ALL_{STATUS/CLEAR/MASK} register fields */ #define PARF_INT_ALL_LINK_UP BIT(13) -#define PARF_INT_MSI_DEV_0_7 GENMASK(30, 23)
/* PARF_NO_SNOOP_OVERRIDE register fields */ #define WR_NO_SNOOP_OVERRIDE_EN BIT(1) @@ -1982,8 +1981,7 @@ static int qcom_pcie_probe(struct platform_device *pdev) goto err_host_deinit; }
- writel_relaxed(PARF_INT_ALL_LINK_UP | PARF_INT_MSI_DEV_0_7, - pcie->parf + PARF_INT_ALL_MASK); + writel_relaxed(PARF_INT_ALL_LINK_UP, pcie->parf + PARF_INT_ALL_MASK); }
qcom_pcie_icc_opp_update(pcie);
On 12/22/2025 12:12 PM, Niklas Cassel wrote:
This reverts commit ba4a2e2317b9faeca9193ed6d3193ddc3cf2aba3.
While this fake hotplugging was a nice idea, it has shown that this feature does not handle PCIe switches correctly: pci_bus 0004:43: busn_res: can not insert [bus 43-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:43: busn_res: [bus 43-41] end is updated to 43 pci_bus 0004:43: busn_res: can not insert [bus 43] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:00.0: devices behind bridge are unusable because [bus 43] cannot be assigned for them pci_bus 0004:44: busn_res: can not insert [bus 44-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:44: busn_res: [bus 44-41] end is updated to 44 pci_bus 0004:44: busn_res: can not insert [bus 44] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:02.0: devices behind bridge are unusable because [bus 44] cannot be assigned for them pci_bus 0004:45: busn_res: can not insert [bus 45-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:45: busn_res: [bus 45-41] end is updated to 45 pci_bus 0004:45: busn_res: can not insert [bus 45] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:06.0: devices behind bridge are unusable because [bus 45] cannot be assigned for them pci_bus 0004:46: busn_res: can not insert [bus 46-41] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci_bus 0004:46: busn_res: [bus 46-41] end is updated to 46 pci_bus 0004:46: busn_res: can not insert [bus 46] under [bus 42-41] (conflicts with (null) [bus 42-41]) pci 0004:42:0e.0: devices behind bridge are unusable because [bus 46] cannot be assigned for them pci_bus 0004:42: busn_res: [bus 42-41] end is updated to 46 pci_bus 0004:42: busn_res: can not insert [bus 42-46] under [bus 41] (conflicts with (null) [bus 41]) pci 0004:41:00.0: devices behind bridge are unusable because [bus 42-46] cannot be assigned for them pcieport 0004:40:00.0: bridge has subordinate 41 but max busn 46
During the initial scan, PCI core doesn't see the switch and since the Root Port is not hot plug capable, the secondary bus number gets assigned as the subordinate bus number. This means, the PCI core assumes that only one bus will appear behind the Root Port since the Root Port is not hot plug capable.
This works perfectly fine for PCIe endpoints connected to the Root Port, since they don't extend the bus. However, if a PCIe switch is connected, then there is a problem when the downstream busses starts showing up and the PCI core doesn't extend the subordinate bus number after initial scan during boot.
The long term plan is to migrate this driver to the pwrctrl framework, once it adds proper support for powering up and enumerating PCIe switches.
Cc: stable@vger.kernel.org Suggested-by: Manivannan Sadhasivam mani@kernel.org Acked-by: Shawn Lin shawn.lin@rock-chips.com Tested-by: Shawn Lin shawn.lin@rock-chips.com Signed-off-by: Niklas Cassel cassel@kernel.org
drivers/pci/controller/dwc/pcie-qcom.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index e87ec6779d44..c5fcb87972e9 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -136,7 +136,6 @@ /* PARF_INT_ALL_{STATUS/CLEAR/MASK} register fields */ #define PARF_INT_ALL_LINK_UP BIT(13) -#define PARF_INT_MSI_DEV_0_7 GENMASK(30, 23) /* PARF_NO_SNOOP_OVERRIDE register fields */ #define WR_NO_SNOOP_OVERRIDE_EN BIT(1) @@ -1982,8 +1981,7 @@ static int qcom_pcie_probe(struct platform_device *pdev) goto err_host_deinit; }
writel_relaxed(PARF_INT_ALL_LINK_UP | PARF_INT_MSI_DEV_0_7,pcie->parf + PARF_INT_ALL_MASK);
MSI's needs to be enabled irrespective of this series as part of global IRQ otherwise MSI's will not be triggered in few platforms.
Mani, exclude this patch while applying.
- Krishna Chaitanya
writel_relaxed(PARF_INT_ALL_LINK_UP, pcie->parf + PARF_INT_ALL_MASK);qcom_pcie_icc_opp_update(pcie);
On Mon, Dec 22, 2025 at 12:18:32PM +0530, Krishna Chaitanya Chundru wrote:
@@ -1982,8 +1981,7 @@ static int qcom_pcie_probe(struct platform_device *pdev) goto err_host_deinit; }
writel_relaxed(PARF_INT_ALL_LINK_UP | PARF_INT_MSI_DEV_0_7,pcie->parf + PARF_INT_ALL_MASK);MSI's needs to be enabled irrespective of this series as part of global IRQ otherwise MSI's will not be triggered in few platforms.
Mani, exclude this patch while applying.
From the commit message of the reverted patch:
The MSI bits in PARF_INT_ALL_MASK register are enabled by default in the hardware, but commit 4581403f6792 ("PCI: qcom: Enumerate endpoints based on Link up event in 'global_irq' interrupt") disabled them and enabled only the Link up interrupt.
Thus, applying this patch should be safe, or at least bring back the original behavior as it was before the Link Up IRQ patches got applied.
But if you prefer to enable the MSI bits explicitly, in order to not rely on HW defaults, that should also be fine.
Kind regards, Niklas
+ Qiang
On Mon, Dec 22, 2025 at 08:00:57AM +0100, Niklas Cassel wrote:
On Mon, Dec 22, 2025 at 12:18:32PM +0530, Krishna Chaitanya Chundru wrote:
@@ -1982,8 +1981,7 @@ static int qcom_pcie_probe(struct platform_device *pdev) goto err_host_deinit; }
writel_relaxed(PARF_INT_ALL_LINK_UP | PARF_INT_MSI_DEV_0_7,pcie->parf + PARF_INT_ALL_MASK);MSI's needs to be enabled irrespective of this series as part of global IRQ otherwise MSI's will not be triggered in few platforms.
Mani, exclude this patch while applying.
From the commit message of the reverted patch:
The MSI bits in PARF_INT_ALL_MASK register are enabled by default in the hardware, but commit 4581403f6792 ("PCI: qcom: Enumerate endpoints based on Link up event in 'global_irq' interrupt") disabled them and enabled only the Link up interrupt.
Thus, applying this patch should be safe, or at least bring back the original behavior as it was before the Link Up IRQ patches got applied.
Yes!
But if you prefer to enable the MSI bits explicitly, in order to not rely on HW defaults, that should also be fine.
I believe Qiang reported internally that we need to explicitly enable PARF_INT_MSI_DEV_0_7 for all platforms as the hardware default is not always correct. So that should come as a separate patch.
- Mani
linux-stable-mirror@lists.linaro.org
-
Krishna Chaitanya Chundru -
Manivannan Sadhasivam -
Niklas Cassel