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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The patch attempts to fix PCIe surprise hot-plug by clearing the Attention Button Present (ABP) bit, but fails to apply this fix to platforms using the v2.3.3 and v2.9.0 init sequences, leaving hot-plug broken on these platforms.
--

--- Patch [1]: [PATCH] PCI: qcom: Clear Attention Button Present in Slot Capabilities ---
commit 65069d64a9bc5c0ab8831d1f4774fc266f4a082e
Author: Qiang Yu <qiang.yu@oss.qualcomm.com>
Subject: PCI: qcom: Clear Attention Button Present in Slot Capabilities

Qcom PCIe Root Ports advertise the Attention Button Present (ABP) bit in
the Slot Capabilities register, but the slots have no attention button.
This patch clears the ABP bit so that presence detect changes can be
reported, allowing surprise hot-plug detection to work.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index b193c989b2b80..89c2d4cb1fbeb 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c

[ ... ]

> @@ -372,6 +372,11 @@ static void qcom_pcie_set_slot_nccs(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);

[Severity: High]
This is a pre-existing issue, but does this fix need to cover platforms
using the v2.3.3 and v2.9.0 init sequences as well?

While this patch clears the Attention Button Present (ABP) bit in
qcom_pcie_set_slot_cap(), platforms using qcom_pcie_post_init_2_3_3() and
qcom_pcie_post_init_2_9_0() appear to bypass this fix.

Looking at drivers/pci/controller/dwc/pcie-qcom.c, these platforms use a
hardcoded macro that explicitly sets the ABP bit:

#define PCIE_CAP_SLOT_VAL			(PCI_EXP_SLTCAP_ABP | \
...

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

Will this leave PCIe surprise hot-plug broken on these platforms, since
the ABP bit remains set in their slot capabilities?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-clear_abp_0710-v1-1-efd63ebdaa09@oss.qualcomm.com?part=1