[PATCH v2 04/12] PCI: qcom: Add support for disabling ASPM L0s in devicetree

[PATCH v2 04/12] PCI: qcom: Add support for disabling ASPM L0s in devicetree

[Johan Hovold]

Commit 9f4f3dfad8cf ("PCI: qcom: Enable ASPM for platforms supporting
1.9.0 ops") started enabling ASPM unconditionally when the hardware
claims to support it. This triggers Correctable Errors for some PCIe
devices on machines like the Lenovo ThinkPad X13s, which could indicate
an incomplete driver ASPM implementation or that the hardware does in
fact not support L0s.

Add support for disabling ASPM L0s in the devicetree when it is not
supported on a particular machine and controller.

Note that only the 1.9.0 ops enable ASPM currently.

Fixes: 9f4f3dfad8cf ("PCI: qcom: Enable ASPM for platforms supporting 1.9.0 ops")
Cc: stable@vger.kernel.org      # 6.7
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 09d485df34b9..0fb5dc06d2ef 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -273,6 +273,25 @@ static int qcom_pcie_start_link(struct dw_pcie *pci)
 	return 0;
 }
 
+static void qcom_pcie_clear_aspm_l0s(struct dw_pcie *pci)
+{
+	u16 offset;
+	u32 val;
+
+	if (!of_property_read_bool(pci->dev->of_node, "aspm-no-l0s"))
+		return;
+
+	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+
+	dw_pcie_dbi_ro_wr_en(pci);
+
+	val = readl(pci->dbi_base + offset + PCI_EXP_LNKCAP);
+	val &= ~PCI_EXP_LNKCAP_ASPM_L0S;
+	writel(val, pci->dbi_base + offset + PCI_EXP_LNKCAP);
+
+	dw_pcie_dbi_ro_wr_dis(pci);
+}
+
 static void qcom_pcie_clear_hpc(struct dw_pcie *pci)
 {
 	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
@@ -962,6 +981,7 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
 
 static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
 {
+	qcom_pcie_clear_aspm_l0s(pcie->pci);
 	qcom_pcie_clear_hpc(pcie->pci);
 
 	return 0;
-- 
2.43.0

[PATCH v2 05/12] arm64: dts: qcom: sc8280xp: add missing PCIe minimum OPP

[Johan Hovold]

Add the missing PCIe CX performance level votes to avoid relying on
other drivers (e.g. USB or UFS) to maintain the nominal performance
level required for Gen3 speeds.

Fixes: 813e83157001 ("arm64: dts: qcom: sc8280xp/sa8540p: add PCIe2-4 nodes")
Cc: stable@vger.kernel.org      # 6.2
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index 0a40b8dec14e..95c7b746407f 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -1780,6 +1780,7 @@ pcie4: pcie@1c00000 {
 			reset-names = "pci";
 
 			power-domains = <&gcc PCIE_4_GDSC>;
+			required-opps = <&rpmhpd_opp_nom>;
 
 			phys = <&pcie4_phy>;
 			phy-names = "pciephy";
@@ -1878,6 +1879,7 @@ pcie3b: pcie@1c08000 {
 			reset-names = "pci";
 
 			power-domains = <&gcc PCIE_3B_GDSC>;
+			required-opps = <&rpmhpd_opp_nom>;
 
 			phys = <&pcie3b_phy>;
 			phy-names = "pciephy";
@@ -1976,6 +1978,7 @@ pcie3a: pcie@1c10000 {
 			reset-names = "pci";
 
 			power-domains = <&gcc PCIE_3A_GDSC>;
+			required-opps = <&rpmhpd_opp_nom>;
 
 			phys = <&pcie3a_phy>;
 			phy-names = "pciephy";
@@ -2077,6 +2080,7 @@ pcie2b: pcie@1c18000 {
 			reset-names = "pci";
 
 			power-domains = <&gcc PCIE_2B_GDSC>;
+			required-opps = <&rpmhpd_opp_nom>;
 
 			phys = <&pcie2b_phy>;
 			phy-names = "pciephy";
@@ -2175,6 +2179,7 @@ pcie2a: pcie@1c20000 {
 			reset-names = "pci";
 
 			power-domains = <&gcc PCIE_2A_GDSC>;
+			required-opps = <&rpmhpd_opp_nom>;
 
 			phys = <&pcie2a_phy>;
 			phy-names = "pciephy";
-- 
2.43.0

[PATCH v2 07/12] arm64: dts: qcom: sc8280xp-x13s: limit pcie4 link speed

[Johan Hovold]

Limit the WiFi PCIe link speed to Gen2 speed (500 MB/s), which is the
speed that the boot firmware has brought up the link at (and that
Windows uses).

This is specifically needed to avoid a large amount of link errors when
restarting the link during boot (but which are currently not reported).

This also appears to fix intermittent failures to download the ath11k
firmware during boot which can be seen when there is a longer delay
between restarting the link and loading the WiFi driver (e.g. when using
full disk encryption).

Fixes: 123b30a75623 ("arm64: dts: qcom: sc8280xp-x13s: enable WiFi controller")
Cc: stable@vger.kernel.org      # 6.2
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
index 2c17e137563a..a67756ada990 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
@@ -768,6 +768,8 @@ &pcie3a_phy {
 };
 
 &pcie4 {
+	max-link-speed = <2>;
+
 	perst-gpios = <&tlmm 141 GPIO_ACTIVE_LOW>;
 	wake-gpios = <&tlmm 139 GPIO_ACTIVE_LOW>;
 
-- 
2.43.0

[PATCH v2 08/12] arm64: dts: qcom: sc8280xp-crd: disable ASPM L0s for NVMe

[Johan Hovold]

Enabling ASPM L0s on the CRD results in a large amount of Correctable
Errors (Timeout) when accessing the NVMe controller so disable it for
now.

Fixes: 9f4f3dfad8cf ("PCI: qcom: Enable ASPM for platforms supporting 1.9.0 ops")
Cc: stable@vger.kernel.org      # 6.7
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
index 41215567b3ae..7e94a68d5d9f 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
@@ -525,6 +525,8 @@ keyboard@68 {
 };
 
 &pcie2a {
+	aspm-no-l0s;
+
 	perst-gpios = <&tlmm 143 GPIO_ACTIVE_LOW>;
 	wake-gpios = <&tlmm 145 GPIO_ACTIVE_LOW>;
 
-- 
2.43.0

[PATCH v2 09/12] arm64: dts: qcom: sc8280xp-crd: disable ASPM L0s for modem and Wi-Fi

[Johan Hovold]

There are indications that ASPM L0s is not working very well on this
machine so disable it also for the modem and Wi-Fi controllers for now.

This specifically avoids having the modem and Wi-Fi controllers bounce
in an out of L0s when not used (the modem still bounces in and out of
L1) as well as intermittent Correctable errors on the Wi-Fi link when
not used.

Fixes: 9f4f3dfad8cf ("PCI: qcom: Enable ASPM for platforms supporting 1.9.0 ops")
Cc: stable@vger.kernel.org      # 6.7
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
index 7e94a68d5d9f..8fc0380f65a0 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
@@ -546,6 +546,8 @@ &pcie2a_phy {
 };
 
 &pcie3a {
+	aspm-no-l0s;
+
 	perst-gpios = <&tlmm 151 GPIO_ACTIVE_LOW>;
 	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
 
@@ -566,6 +568,7 @@ &pcie3a_phy {
 
 &pcie4 {
 	max-link-speed = <2>;
+	aspm-no-l0s;
 
 	perst-gpios = <&tlmm 141 GPIO_ACTIVE_LOW>;
 	wake-gpios = <&tlmm 139 GPIO_ACTIVE_LOW>;
-- 
2.43.0

[PATCH v2 10/12] arm64: dts: qcom: sc8280xp-x13s: disable ASPM L0s for Wi-Fi

[Johan Hovold]

Enabling ASPM L0s on the Lenovo Thinkpad X13s results in Correctable
Errors (BadTLP, Timeout) when accessing the Wi-Fi controller so disable
it for now.

Fixes: 9f4f3dfad8cf ("PCI: qcom: Enable ASPM for platforms supporting 1.9.0 ops")
Cc: stable@vger.kernel.org      # 6.7
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
index a67756ada990..70824294108e 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
@@ -769,6 +769,7 @@ &pcie3a_phy {
 
 &pcie4 {
 	max-link-speed = <2>;
+	aspm-no-l0s;
 
 	perst-gpios = <&tlmm 141 GPIO_ACTIVE_LOW>;
 	wake-gpios = <&tlmm 139 GPIO_ACTIVE_LOW>;
-- 
2.43.0

[PATCH v2 11/12] arm64: dts: qcom: sc8280xp-x13s: disable ASPM L0s for NVMe and modem

[Johan Hovold]

There are indications that ASPM L0s is not working very well on this
machine so disable it also for the NVMe and modem controllers for now.

Note that this is done as a precaution based on problems with the Wi-Fi
on the X13s as well as the NVMe, modem and Wi-Fi on the sc8280xp-crd
reference design (the NVMe controller on my X13s does not support L0 and
the machine lacks a modem).

Fixes: 9f4f3dfad8cf ("PCI: qcom: Enable ASPM for platforms supporting 1.9.0 ops")
Cc: stable@vger.kernel.org      # 6.7
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
index 70824294108e..06fc88d5d025 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
@@ -730,6 +730,8 @@ keyboard@68 {
 };
 
 &pcie2a {
+	aspm-no-l0s;
+
 	perst-gpios = <&tlmm 143 GPIO_ACTIVE_LOW>;
 	wake-gpios = <&tlmm 145 GPIO_ACTIVE_LOW>;
 
@@ -749,6 +751,8 @@ &pcie2a_phy {
 };
 
 &pcie3a {
+	aspm-no-l0s;
+
 	perst-gpios = <&tlmm 151 GPIO_ACTIVE_LOW>;
 	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
 
-- 
2.43.0

Re: [PATCH v2 05/12] arm64: dts: qcom: sc8280xp: add missing PCIe minimum OPP

[Konrad Dybcio]

On 23.02.2024 16:21, Johan Hovold wrote:
> Add the missing PCIe CX performance level votes to avoid relying on
> other drivers (e.g. USB or UFS) to maintain the nominal performance
> level required for Gen3 speeds.
> 
> Fixes: 813e83157001 ("arm64: dts: qcom: sc8280xp/sa8540p: add PCIe2-4 nodes")
> Cc: stable@vger.kernel.org      # 6.2
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---

Please remember that we should remove these when OPP support
lands :D

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad

Re: [PATCH v2 07/12] arm64: dts: qcom: sc8280xp-x13s: limit pcie4 link speed

[Konrad Dybcio]

On 23.02.2024 16:21, Johan Hovold wrote:
> Limit the WiFi PCIe link speed to Gen2 speed (500 MB/s), which is the
> speed that the boot firmware has brought up the link at (and that
> Windows uses).
> 
> This is specifically needed to avoid a large amount of link errors when
> restarting the link during boot (but which are currently not reported).
> 
> This also appears to fix intermittent failures to download the ath11k
> firmware during boot which can be seen when there is a longer delay
> between restarting the link and loading the WiFi driver (e.g. when using
> full disk encryption).
> 
> Fixes: 123b30a75623 ("arm64: dts: qcom: sc8280xp-x13s: enable WiFi controller")
> Cc: stable@vger.kernel.org      # 6.2
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad

Re: [PATCH v2 08/12] arm64: dts: qcom: sc8280xp-crd: disable ASPM L0s for NVMe

[Konrad Dybcio]

On 23.02.2024 16:21, Johan Hovold wrote:
> Enabling ASPM L0s on the CRD results in a large amount of Correctable
> Errors (Timeout) when accessing the NVMe controller so disable it for
> now.
> 
> Fixes: 9f4f3dfad8cf ("PCI: qcom: Enable ASPM for platforms supporting 1.9.0 ops")
> Cc: stable@vger.kernel.org      # 6.7
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad

Re: [PATCH v2 04/12] PCI: qcom: Add support for disabling ASPM L0s in devicetree

[Bjorn Helgaas]

On Fri, Feb 23, 2024 at 04:21:16PM +0100, Johan Hovold wrote:
> Commit 9f4f3dfad8cf ("PCI: qcom: Enable ASPM for platforms supporting
> 1.9.0 ops") started enabling ASPM unconditionally when the hardware
> claims to support it. This triggers Correctable Errors for some PCIe
> devices on machines like the Lenovo ThinkPad X13s, which could indicate
> an incomplete driver ASPM implementation or that the hardware does in
> fact not support L0s.

Are there any more details about this?  Do the errors occur around
suspend/resume, a power state transition, or some other event?  Might
other DWC-based devices be susceptible?  Is there a specific driver
you suspect might be incomplete?

Do you want the DT approach because the problem is believed to be
platform-specific?  Otherwise, maybe we should consider reverting
9f4f3dfad8cf until the problem is understood?

Could this be done via a quirk like quirk_disable_aspm_l0s()?  That
currently uses pci_disable_link_state(), which I don't think is
completely safe because it leaves the possibility that drivers or
users could re-enable L0s, e.g., via sysfs.

This patch is nice because IIUC it directly changes PCI_EXP_LNKCAP,
which avoids that issue, but quirk_disable_aspm_l0s() could
conceivably be reimplemented to cache PCI_EXP_LNKCAP in struct pci_dev
so quirks could override it, as we do with struct pci_dev.devcap.

> Add support for disabling ASPM L0s in the devicetree when it is not
> supported on a particular machine and controller.
> 
> Note that only the 1.9.0 ops enable ASPM currently.
> 
> Fixes: 9f4f3dfad8cf ("PCI: qcom: Enable ASPM for platforms supporting 1.9.0 ops")
> Cc: stable@vger.kernel.org      # 6.7
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 09d485df34b9..0fb5dc06d2ef 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -273,6 +273,25 @@ static int qcom_pcie_start_link(struct dw_pcie *pci)
>  	return 0;
>  }
>  
> +static void qcom_pcie_clear_aspm_l0s(struct dw_pcie *pci)
> +{
> +	u16 offset;
> +	u32 val;
> +
> +	if (!of_property_read_bool(pci->dev->of_node, "aspm-no-l0s"))
> +		return;
> +
> +	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +
> +	dw_pcie_dbi_ro_wr_en(pci);
> +
> +	val = readl(pci->dbi_base + offset + PCI_EXP_LNKCAP);
> +	val &= ~PCI_EXP_LNKCAP_ASPM_L0S;
> +	writel(val, pci->dbi_base + offset + PCI_EXP_LNKCAP);
> +
> +	dw_pcie_dbi_ro_wr_dis(pci);
> +}
> +
>  static void qcom_pcie_clear_hpc(struct dw_pcie *pci)
>  {
>  	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> @@ -962,6 +981,7 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>  
>  static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
>  {
> +	qcom_pcie_clear_aspm_l0s(pcie->pci);
>  	qcom_pcie_clear_hpc(pcie->pci);
>  
>  	return 0;
> -- 
> 2.43.0
> 

Re: [PATCH v2 04/12] PCI: qcom: Add support for disabling ASPM L0s in devicetree

[Johan Hovold]

On Fri, Feb 23, 2024 at 04:10:00PM -0600, Bjorn Helgaas wrote:
> On Fri, Feb 23, 2024 at 04:21:16PM +0100, Johan Hovold wrote:
> > Commit 9f4f3dfad8cf ("PCI: qcom: Enable ASPM for platforms supporting
> > 1.9.0 ops") started enabling ASPM unconditionally when the hardware
> > claims to support it. This triggers Correctable Errors for some PCIe
> > devices on machines like the Lenovo ThinkPad X13s, which could indicate
> > an incomplete driver ASPM implementation or that the hardware does in
> > fact not support L0s.
> 
> Are there any more details about this?  Do the errors occur around
> suspend/resume, a power state transition, or some other event?  Might
> other DWC-based devices be susceptible?  Is there a specific driver
> you suspect might be incomplete?

I see these errors when the devices in question are active as well as
idle (not during suspend/resume). For example, when running iperf3 or
fio to test the wifi and nvme, but I also see this occasionally for a
wifi device which is (supposedly) not active (e.g. a handful errors over
night).

I skimmed Qualcomm's driver and noted that there are some registers
related to ASPM which that driver updates, while the mainline driver
leaves them at their default settings, but I essentially only mentioned
that the ASPM implementation may be incomplete as a theoretical
possibility. The somewhat erratic ASPM behaviour for one of the modems
also suggests that some further tweak/quirk may be needed, and I was
hoping to catch Mani's interest by reporting it.

But based on what I've since heard from Qualcomm, it seems like these
correctable error may be a known issue with the hardware (e.g. seen
also with Windows), which is also why we decided to disable it for all
controllers on these two platforms where I've seen this in v2.
 
> Do you want the DT approach because the problem is believed to be
> platform-specific?  Otherwise, maybe we should consider reverting
> 9f4f3dfad8cf until the problem is understood?

Enabling ASPM gave a very significant improvement in battery life on the
Lenovo ThinkPad X13s, from 10.5 h to 15 h, so reverting is not really an
option there.

And with L0s disabled, the AER error reports about correctable errors
(that prevent enabling the GIC ITS and possibly degrades performance
somewhat) are gone.

I don't know for sure if there are further Qualcomm platform that are
affected by this so I also don't want to use a too big of a hammer. The
devicetree property allows us to disable L0s only after confirming that
it's needed, and we can always extend this to broader classes of device
when/if we learn more.

> Could this be done via a quirk like quirk_disable_aspm_l0s()?  That
> currently uses pci_disable_link_state(), which I don't think is
> completely safe because it leaves the possibility that drivers or
> users could re-enable L0s, e.g., via sysfs.

That was my first approach, thinking that it was the endpoint devices
which did not really support L0s. But initially it seemed like the wifi
controller on the CRD was not affected by this, while the same
controller on the X13s was. That made me conclude that this is not just
a property of the device but (also) of the controller and/or machine.

I then noticed that we already had some controller drivers implementing
'aspm-no-l0s' and decided to go with that.

> This patch is nice because IIUC it directly changes PCI_EXP_LNKCAP,
> which avoids that issue, but quirk_disable_aspm_l0s() could
> conceivably be reimplemented to cache PCI_EXP_LNKCAP in struct pci_dev
> so quirks could override it, as we do with struct pci_dev.devcap.

Johan

Re: [PATCH v2 04/12] PCI: qcom: Add support for disabling ASPM L0s in devicetree

[Bjorn Helgaas]

On Tue, Feb 27, 2024 at 04:29:15PM +0100, Johan Hovold wrote:
> On Fri, Feb 23, 2024 at 04:10:00PM -0600, Bjorn Helgaas wrote:
> > On Fri, Feb 23, 2024 at 04:21:16PM +0100, Johan Hovold wrote:
> > > Commit 9f4f3dfad8cf ("PCI: qcom: Enable ASPM for platforms supporting
> > > 1.9.0 ops") started enabling ASPM unconditionally when the hardware
> > > claims to support it. This triggers Correctable Errors for some PCIe
> > > devices on machines like the Lenovo ThinkPad X13s, which could indicate
> > > an incomplete driver ASPM implementation or that the hardware does in
> > > fact not support L0s.
> > 
> > Are there any more details about this?  Do the errors occur around
> > suspend/resume, a power state transition, or some other event?  Might
> > other DWC-based devices be susceptible?  Is there a specific driver
> > you suspect might be incomplete?
> 
> I see these errors when the devices in question are active as well as
> idle (not during suspend/resume). For example, when running iperf3 or
> fio to test the wifi and nvme, but I also see this occasionally for a
> wifi device which is (supposedly) not active (e.g. a handful errors over
> night).
> 
> I skimmed Qualcomm's driver and noted that there are some registers
> related to ASPM which that driver updates, while the mainline driver
> leaves them at their default settings, but I essentially only mentioned
> that the ASPM implementation may be incomplete as a theoretical
> possibility. The somewhat erratic ASPM behaviour for one of the modems
> also suggests that some further tweak/quirk may be needed, and I was
> hoping to catch Mani's interest by reporting it.
> 
> But based on what I've since heard from Qualcomm, it seems like these
> correctable error may be a known issue with the hardware (e.g. seen
> also with Windows), which is also why we decided to disable it for all
> controllers on these two platforms where I've seen this in v2.
>  
> > Do you want the DT approach because the problem is believed to be
> > platform-specific?  Otherwise, maybe we should consider reverting
> > 9f4f3dfad8cf until the problem is understood?
> 
> Enabling ASPM gave a very significant improvement in battery life on the
> Lenovo ThinkPad X13s, from 10.5 h to 15 h, so reverting is not really an
> option there.

Ah, I missed that you're only disabling L0s, but leaving L1 enabled,
thanks!

And given that the v1.9.0 ops that enable ASPM are used on a bunch of
platforms, and L0s seems to work fine on most of them, we wouldn't
want to disable L0s for everybody, so this seems like the right
solution.

Bjorn