From: Manivannan Sadhasivam manivannan.sadhasivam@oss.qualcomm.com
[ Upstream commit 0cc13256b60510936c34098ee7b929098eed823b ]
Though I couldn't confirm ASPM L0s support with the Qcom hardware team, a bug report from Dmitry suggests that L0s is broken on this legacy SoC. Hence, remove L0s support from the Root Port Link Capabilities in this SoC.
Since qcom_pcie_clear_aspm_l0s() is now used by more than one SoC config, call it from qcom_pcie_host_init() instead.
Reported-by: Dmitry Baryshkov dmitry.baryshkov@oss.qualcomm.com Closes: https://lore.kernel.org/linux-pci/4cp5pzmlkkht2ni7us6p3edidnk25l45xrp6w3fxgu... Signed-off-by: Manivannan Sadhasivam manivannan.sadhasivam@oss.qualcomm.com Signed-off-by: Manivannan Sadhasivam mani@kernel.org Signed-off-by: Bjorn Helgaas bhelgaas@google.com Tested-by: Dmitry Baryshkov dmitry.baryshkov@oss.qualcomm.com Reviewed-by: Konrad Dybcio konrad.dybcio@oss.qualcomm.com Link: https://patch.msgid.link/20251126081718.8239-1-mani@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
Looking at this commit, I need to analyze whether it's appropriate for stable backporting.
## Commit Analysis
### What the Commit Does This commit fixes broken ASPM L0s (Active State Power Management L0s) support on the MSM8996 SoC. The key changes are:
1. **Adds `.no_l0s = true`** to the `cfg_2_3_2` configuration struct (MSM8996's config) 2. **Moves `qcom_pcie_clear_aspm_l0s()` call** from `qcom_pcie_post_init_2_7_0()` to `qcom_pcie_host_init()` so it applies to all SoCs that need it (based on their config flags) 3. **Removes the L0s clearing** from the 2_7_0 post_init since it's now centralized
### Bug Being Fixed According to the commit message and linked bug report, ASPM L0s is broken on the MSM8996 SoC. When L0s is enabled on broken hardware, users can experience: - PCIe link instability - Power management failures - Potential system hangs or communication failures
### Classification: Hardware Quirk/Workaround This is clearly a **hardware quirk** - disabling a broken power management feature on specific hardware. This falls under the explicit exception category for stable backporting, similar to USB quirks or PCI quirks for broken devices.
### Code Change Assessment ```c // Addition to MSM8996 config - trivial one-liner: static const struct qcom_pcie_cfg cfg_2_3_2 = { .ops = &ops_2_3_2, + .no_l0s = true, };
// Centralized call in host_init for all SoCs needing it: + qcom_pcie_clear_aspm_l0s(pcie->pci); ```
The actual fix is small and surgical. The refactoring (moving the function call) is minimal and just enables the fix to work for multiple SoCs that need it.
### Stability Indicators - **Tested-by: Dmitry Baryshkov** - The reporter confirmed the fix works - **Reviewed-by: Konrad Dybcio** - Proper review by another Qualcomm developer - **Signed-off by Bjorn Helgaas** - PCI subsystem maintainer approved
### Risk Assessment - **Low risk**: Disabling a broken feature is safer than leaving it enabled - **Targeted scope**: Only affects MSM8996 SoC (legacy Qualcomm platform) - **Well-tested**: Has explicit test confirmation from the bug reporter
### Dependency Concerns This commit assumes: 1. The `no_l0s` field exists in `struct qcom_pcie_cfg` 2. The `qcom_pcie_clear_aspm_l0s()` function exists
Looking at the diff, the commit adds to an existing `no_l0s` field (it wouldn't compile otherwise). The function was already being called in the 2_7_0 init path, so it exists. The infrastructure appears to already be in place.
### User Impact Users with MSM8996 devices (mobile/embedded Qualcomm platform) experiencing PCIe stability issues due to broken L0s will benefit from this fix. This is a real-world hardware problem affecting actual users.
## Conclusion
This commit is a **hardware quirk/workaround** which is explicitly allowed in stable trees. It: - Fixes a real bug (broken ASPM L0s causing hardware issues) - Is small and contained - Has been properly tested and reviewed - Affects only the specific broken hardware (MSM8996) - Falls under the device quirk exception category
The refactoring aspect (moving the function call) is minimal and merely enables the fix to work cleanly. The actual fix is just adding `no_l0s = true` to the MSM8996 configuration.
**YES**
drivers/pci/controller/dwc/pcie-qcom.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index c48a20602d7fa..6e820595ba32a 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -1033,7 +1033,6 @@ 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_clear_aspm_l0s(pcie->pci); qcom_pcie_clear_hpc(pcie->pci);
return 0; @@ -1302,6 +1301,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp) goto err_disable_phy; }
+ qcom_pcie_clear_aspm_l0s(pcie->pci); + qcom_ep_reset_deassert(pcie);
if (pcie->cfg->ops->config_sid) { @@ -1450,6 +1451,7 @@ static const struct qcom_pcie_cfg cfg_2_1_0 = {
static const struct qcom_pcie_cfg cfg_2_3_2 = { .ops = &ops_2_3_2, + .no_l0s = true, };
static const struct qcom_pcie_cfg cfg_2_3_3 = {