Re: [PATCH] PCI: qcom: Advertise hotplug with no command completion support

[Manivannan Sadhasivam]

On Sat, Mar 21, 2026 at 05:31:08PM +0530, Manivannan Sadhasivam wrote:
> On Fri, Mar 20, 2026 at 07:10:19AM +0530, Krishna Chaitanya Chundru wrote:
> > 
> > 
> > On 3/19/2026 4:29 PM, Konrad Dybcio wrote:
> > > On 3/16/26 1:22 PM, Krishna Chaitanya Chundru wrote:
> > > > 
> > > > On 3/15/2026 3:39 PM, Manivannan Sadhasivam wrote:
> > > > > On Sat, Mar 14, 2026 at 07:26:34AM +0530, Krishna Chaitanya Chundru wrote:
> > > > > > QCOM PCIe controller advertise hotplug capability in hardware but do not
> > > > > > support hotplug command completion. As a result, the PCI core registers
> > > > > > the pciehp service and issues hotplug commands that never gets completions,
> > > > > > leading to repeated timeout warnings and multi-second delays during boot
> > > > > > and suspend/resume.
> > > > > > 
> > > > > > Commit a54db86ddc153 ("PCI: qcom: Do not advertise hotplug capability for
> > > > > > IPs v2.7.0 and v1.9.0") avoided these timeouts by clearing the Hot-Plug
> > > > > > Capability bit entirely, which also disabled all hotplug functionality.
> > > > > > 
> > > > > Just some background: I added commit a54db86ddc153 to disable hotplug for Qcom
> > > > > PCIe Root Ports since we were seeing completion timeouts for hotplug commands
> > > > > and also the PRSNT# signal was not exposed on any of our SoCs. After checking
> > > > > with some internal folks I learned that hotplug functionality was not exercised
> > > > > in Linux. So these facts made me believe that hotplug was not suppored at all.
> > > > > 
> > > > > But it turned out that the Qcom Root Ports support "Data Link Layer State
> > > > > Changed Events" such as DL_Up/Down events.
> > > > > 
> > > > > > Instead of disabling hotplug, mark these controllers as not supporting
> > > > > > command completion by setting the No Command Completed Support (NCCS) bit
> > > > > > in the Slot Capabilities register. This prevents the PCI hotplug driver
> > > > > > from waiting for commands completion while still allowing hotplug-related
> > > > > > functionality such as Data Link Layer state change events.
> > > > > > 
> > > > > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@xxxxxxxxxxxxxxxx>
> > > > > > ---
> > > > > >    drivers/pci/controller/dwc/pcie-qcom.c | 2 +-
> > > > > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > > index 67a16af69ddc75fca1b123e70715e692a91a9135..a2924610f3625f2456a491473c135840e31bafb9 100644
> > > > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > > @@ -358,7 +358,7 @@ static void qcom_pcie_clear_hpc(struct dw_pcie *pci)
> > > > > >        dw_pcie_dbi_ro_wr_en(pci);
> > > > > >          val = readl(pci->dbi_base + offset + PCI_EXP_SLTCAP);
> > > > > > -    val &= ~PCI_EXP_SLTCAP_HPC;
> > > > > > +    val |= PCI_EXP_SLTCAP_NCCS;
> > > > > Are you sure that this is the only non-supported capability? What about
> > > > > Attention, Presence, Power Fault, MRL etc...?
> > > > Even though there no signals required for attention, presence etc in the hardware
> > > > there is a way to generate these MSI's with these bits set through parf, so technically
> > > > so other co-processor in the system can trigger interrupts.
> > > Are you saying that the RC itself will not generate them based on what
> > > happens on the bus, but they can be triggered artificially?
> > Yes there are parf registers through which msi's can be triggered
> > artificially.
> > 
> 
> As Krishna said, it seems there are ways to *inject* these events through
> platform specific means (through PARF register space), even if the associated
> signals are not exposed to the slot. And we might end up using those events at
> some point.
> 
> So it is OK from my opinion to just disable NCCS. But this also warrants a
> comment in the code.
> 

I added the comments and also changed the helper function name while applying.

- Mani

-- 
மணிவண்ணன் சதாசிவம்