[PATCH v2 0/2] PCI: qcom: Clear Attention Button Present in Slot Capabilities

[PATCH v2 0/2] PCI: qcom: Clear Attention Button Present in Slot Capabilities

[Qiang Yu]

Qcom PCIe Root Ports advertise the Attention Button Present (ABP) bit in
the Slot Capabilities register, but the slots have no attention button.
This causes pciehp's pcie_enable_notification() to enable the Attention
Button Pressed interrupt instead of the Presence Detect Changed
interrupt, so presence detect changes are never reported and surprise
hot-plug detection does not work.

Patch 1 renames qcom_pcie_set_slot_nccs() to qcom_pcie_set_slot_cap() as
a preparatory, no-functional-change step, split out per Bjorn's review
comment. Patch 2 clears the ABP bit.

---
Changes in v2:
- Split into two patches (rename as prep patch, ABP clear as the fix)
- Added a blank line before the new comment block
- Kept Konrad's Reviewed-by/Tested-by only on the functional (ABP) patch
- Link to v1: https://lore.kernel.org/all/20260710-clear_abp_0710-v1-1-efd63ebdaa09@oss.qualcomm.com/

To: Manivannan Sadhasivam <mani@kernel.org>
To: Lorenzo Pieralisi <lpieralisi@kernel.org>
To: Krzysztof Wilczyński <kwilczynski@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>

---
Qiang Yu (2):
      PCI: qcom: Rename qcom_pcie_set_slot_nccs() to qcom_pcie_set_slot_cap()
      PCI: qcom: Clear Attention Button Present in Slot Capabilities

 drivers/pci/controller/dwc/pcie-qcom.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)
---
base-commit: 3edb3a038d423480efeb204dbc6ddc9a292f8ecb
change-id: 20260712-b4-clear_abp_0713-d424da4dad3c

Best regards,
--  
Qiang Yu <qiang.yu@oss.qualcomm.com>

[PATCH v2 1/2] PCI: qcom: Rename qcom_pcie_set_slot_nccs() to qcom_pcie_set_slot_cap()

[Qiang Yu]

qcom_pcie_set_slot_nccs() currently only sets the NCCS field in the Slot
Capabilities register. A following patch adds programming of another
field in the same register, so rename the function to
qcom_pcie_set_slot_cap() to reflect that it configures Slot Capabilities
as a whole, not just NCCS.

No functional change intended.

Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index b193c989b2b8..465c940109a7 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_aspm_l0s(struct dw_pcie *pci)
 	dw_pcie_dbi_ro_wr_dis(pci);
 }
 
-static void qcom_pcie_set_slot_nccs(struct dw_pcie *pci)
+static void qcom_pcie_set_slot_cap(struct dw_pcie *pci)
 {
 	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
 	u32 val;
@@ -580,7 +580,7 @@ static int qcom_pcie_post_init_2_1_0(struct qcom_pcie *pcie)
 	writel(CFG_BRIDGE_SB_INIT,
 	       pci->dbi_base + AXI_MSTR_RESP_COMP_CTRL1);
 
-	qcom_pcie_set_slot_nccs(pcie->pci);
+	qcom_pcie_set_slot_cap(pcie->pci);
 
 	return 0;
 }
@@ -660,7 +660,7 @@ static int qcom_pcie_post_init_1_0_0(struct qcom_pcie *pcie)
 		writel(val, pcie->parf + PARF_AXI_MSTR_WR_ADDR_HALT);
 	}
 
-	qcom_pcie_set_slot_nccs(pcie->pci);
+	qcom_pcie_set_slot_cap(pcie->pci);
 
 	return 0;
 }
@@ -759,7 +759,7 @@ static int qcom_pcie_post_init_2_3_2(struct qcom_pcie *pcie)
 	val |= EN;
 	writel(val, pcie->parf + PARF_AXI_MSTR_WR_ADDR_HALT_V2);
 
-	qcom_pcie_set_slot_nccs(pcie->pci);
+	qcom_pcie_set_slot_cap(pcie->pci);
 
 	return 0;
 }
@@ -1078,7 +1078,7 @@ static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
 		writel(WR_NO_SNOOP_OVERRIDE_EN | RD_NO_SNOOP_OVERRIDE_EN,
 				pcie->parf + PARF_NO_SNOOP_OVERRIDE);
 
-	qcom_pcie_set_slot_nccs(pcie->pci);
+	qcom_pcie_set_slot_cap(pcie->pci);
 
 	return 0;
 }

-- 
2.34.1

Re: [PATCH v2 1/2] PCI: qcom: Rename qcom_pcie_set_slot_nccs() to qcom_pcie_set_slot_cap()

[sashiko-bot]

> qcom_pcie_set_slot_nccs() currently only sets the NCCS field in the Slot
> Capabilities register. A following patch adds programming of another
> field in the same register, so rename the function to
> qcom_pcie_set_slot_cap() to reflect that it configures Slot Capabilities
> as a whole, not just NCCS.
> 
> No functional change intended.
> 
> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713-b4-clear_abp_0713-v2-0-4f0333a98d4e@oss.qualcomm.com?part=1

[PATCH v2 2/2] PCI: qcom: Clear Attention Button Present in Slot Capabilities

[Qiang Yu]

Qcom PCIe Root Ports advertise the Attention Button Present (ABP) bit in
the Slot Capabilities register, but the slots have no attention button.

When ABP is set, pcie_enable_notification() in the pciehp driver enables
the Attention Button Pressed interrupt (ABPE) and leaves the Presence
Detect Changed interrupt (PDCE) disabled. As a result, presence detect
changes are never reported and surprise hot-plug detection that relies on
Presence Detect Changed events does not work.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Tested-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> # X1E80100 CRD
Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 465c940109a7..0a826ba63277 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -372,6 +372,12 @@ static void qcom_pcie_set_slot_cap(struct dw_pcie *pci)
 	 */
 	val = readl(pci->dbi_base + offset + PCI_EXP_SLTCAP);
 	val |= PCI_EXP_SLTCAP_NCCS;
+
+	/*
+	 * Qcom PCIe Root Ports do not support Attention Button, so clear
+	 * Attention Button Present in Slot Capabilities.
+	 */
+	val &= ~PCI_EXP_SLTCAP_ABP;
 	writel(val, pci->dbi_base + offset + PCI_EXP_SLTCAP);
 
 	dw_pcie_dbi_ro_wr_dis(pci);

-- 
2.34.1

Re: [PATCH v2 2/2] PCI: qcom: Clear Attention Button Present in Slot Capabilities

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix for broken hot-plug detection on certain Qcom PCIe IP revisions.
--

--- Patch [2]: [PATCH v2 2/2] PCI: qcom: Clear Attention Button Present in Slot Capabilities ---
commit 2d72cd4fa9f71ba9a26ad4415df596d93e7bf902
Author: Qiang Yu <qiang.yu@oss.qualcomm.com>

PCI: qcom: Clear Attention Button Present in Slot Capabilities

This patch clears the Attention Button Present (ABP) bit in the Slot
Capabilities register for Qcom PCIe Root Ports, as the slots do not actually
have an attention button. This allows surprise hot-plug detection to
function properly by ensuring the Presence Detect Changed interrupt is not
disabled by pciehp.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 465c940109a7d..0a826ba632772 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -372,6 +372,12 @@ static void qcom_pcie_set_slot_cap(struct dw_pcie *pci)
>  	 */
>  	val = readl(pci->dbi_base + offset + PCI_EXP_SLTCAP);
>  	val |= PCI_EXP_SLTCAP_NCCS;
> +
> +	/*
> +	 * Qcom PCIe Root Ports do not support Attention Button, so clear
> +	 * Attention Button Present in Slot Capabilities.
> +	 */
> +	val &= ~PCI_EXP_SLTCAP_ABP;

[Severity: High]
This is a pre-existing issue, but does this change fully resolve the problem
for all Qcom PCIe IP revisions?

Looking at PCIE_CAP_SLOT_VAL in drivers/pci/controller/dwc/pcie-qcom.c, it
explicitly includes the PCI_EXP_SLTCAP_ABP bit:

#define PCIE_CAP_SLOT_VAL   (PCI_EXP_SLTCAP_ABP | \
                             ...)

Functions like qcom_pcie_post_init_2_3_3() and qcom_pcie_post_init_2_9_0()
do not appear to call qcom_pcie_set_slot_cap(). Instead, they directly write
PCIE_CAP_SLOT_VAL to the capability register:

qcom_pcie_post_init_2_3_3() {
    ...
    writel(PCIE_CAP_SLOT_VAL, pci->dbi_base + offset + PCI_EXP_SLTCAP);
    ...
}

Since PCIE_CAP_SLOT_VAL explicitly includes PCI_EXP_SLTCAP_ABP, won't the
Attention Button Present bit remain set for platforms using revisions 2.3.3
and 2.9.0, leaving surprise hot-plug detection broken on those platforms?

>  	writel(val, pci->dbi_base + offset + PCI_EXP_SLTCAP);
>  
>  	dw_pcie_dbi_ro_wr_dis(pci);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713-b4-clear_abp_0713-v2-0-4f0333a98d4e@oss.qualcomm.com?part=2