Skip to contentSkip to conversation

PCI: imx6: Add suspend/resume support for i.MX6QDL

PATCH Applied Stefan Eichenberger 5 participants 8 messages Oct 30, 2024 – Nov 4, 2024

Subsystems (4)
Arm i.MX and Layerscape PCI PCI native host bridge and endpoint drivers PCI driver for IMX6
lore
Share & tools
lore

Stefan Eichenberger

author
#0 lore raw

PATCH v4 1/1 PCI: imx6: Add suspend/resume support for i.MX6QDL

From: Stefan Eichenberger <[email protected]>

The suspend/resume functionality is currently broken on the i.MX6QDL platform, as documented in the NXP errata (ERR005723):
https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf

This patch addresses the issue by sharing most of the suspend/resume sequences used by other i.MX devices, while avoiding modifications to critical registers that disrupt the PCIe functionality. It targets the same problem as the following downstream commit:
https://github.com/nxp-imx/linux-imx/commit/4e92355e1f79d225ea842511fcfd42b343b32995

Unlike the downstream commit, this patch also resets the connected PCIe device if possible. Without this reset, certain drivers, such as ath10k or iwlwifi, will crash on resume. The device reset is also done by the driver on other i.MX platforms, making this patch consistent with existing practices.

Without this patch, suspend/resume will fail on i.MX6QDL devices if a
PCIe device is connected. Upon resuming, the kernel will hang and
display an error. Here's an example of the error encountered with the
ath10k driver:
ath10k_pci 0000:01:00.0: Unable to change power state from D3hot to D0, device inaccessible Unhandled fault: imprecise external abort (0x1406) at 0x0106f944

Signed-off-by Stefan Eichenberger <[email protected]>

---
Changes in v4:
- Improve commit message (Bjorn)
- Fix style issue on comments (Bjorn)
- s/msi/MSI (Bjorn)

Changes in v3:
- Added a new flag to the driver data to indicate that the suspend/resume is broken on the i.MX6QDL platform. (Frank)
- Fix comments to be more relevant (Mani)
- Use imx_pcie_assert_core_reset in suspend (Mani)

 drivers/pci/controller/dwc/pci-imx6.c | 57 +++++++++++++++++++++------
 1 file changed, 46 insertions(+), 11 deletions(-)
drivers/pci/controller/dwc/pci-imx6.c +46 -11
index 808d1f1054173..c8d5c90aa4d45 100644
@@ -82,6 +82,11 @@ enum imx_pcie_variants {
 #define IMX_PCIE_FLAG_HAS_SERDES		BIT(6)
 #define IMX_PCIE_FLAG_SUPPORT_64BIT		BIT(7)
 #define IMX_PCIE_FLAG_CPU_ADDR_FIXUP		BIT(8)
+/*
+ * Because of ERR005723 (PCIe does not support L2 power down) we need to
+ * workaround suspend resume on some devices which are affected by this errata.
+ */
+#define IMX_PCIE_FLAG_BROKEN_SUSPEND		BIT(9)
 
 #define imx_check_flag(pci, val)	(pci->drvdata->flags & val)
 

@@ -1237,9 +1242,19 @@ static int imx_pcie_suspend_noirq(struct device *dev)
 		return 0;
 
 	imx_pcie_msi_save_restore(imx_pcie, true);
-	imx_pcie_pm_turnoff(imx_pcie);
-	imx_pcie_stop_link(imx_pcie->pci);
-	imx_pcie_host_exit(pp);
+	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_BROKEN_SUSPEND)) {
+		/*
+		 * The minimum for a workaround would be to set PERST# and to
+		 * set the PCIE_TEST_PD flag. However, we can also disable the
+		 * clock which saves some power.
+		 */
+		imx_pcie_assert_core_reset(imx_pcie);
+		imx_pcie->drvdata->enable_ref_clk(imx_pcie, false);
+	} else {
+		imx_pcie_pm_turnoff(imx_pcie);
+		imx_pcie_stop_link(imx_pcie->pci);
+		imx_pcie_host_exit(pp);
+	}
 
 	return 0;
 }

@@ -1253,14 +1268,32 @@ static int imx_pcie_resume_noirq(struct device *dev)
 	if (!(imx_pcie->drvdata->flags & IMX_PCIE_FLAG_SUPPORTS_SUSPEND))
 		return 0;
 
-	ret = imx_pcie_host_init(pp);
-	if (ret)
-		return ret;
-	imx_pcie_msi_save_restore(imx_pcie, false);
-	dw_pcie_setup_rc(pp);
+	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_BROKEN_SUSPEND)) {
+		ret = imx_pcie->drvdata->enable_ref_clk(imx_pcie, true);
+		if (ret)
+			return ret;
+		ret = imx_pcie_deassert_core_reset(imx_pcie);
+		if (ret)
+			return ret;
+		/*
+		 * Using PCIE_TEST_PD seems to disable MSI and powers down the
+		 * root complex. This is why we have to setup the rc again and
+		 * why we have to restore the MSI register.
+		 */
+		ret = dw_pcie_setup_rc(&imx_pcie->pci->pp);
+		if (ret)
+			return ret;
+		imx_pcie_msi_save_restore(imx_pcie, false);
+	} else {
+		ret = imx_pcie_host_init(pp);
+		if (ret)
+			return ret;
+		imx_pcie_msi_save_restore(imx_pcie, false);
+		dw_pcie_setup_rc(pp);
 
-	if (imx_pcie->link_is_up)
-		imx_pcie_start_link(imx_pcie->pci);
+		if (imx_pcie->link_is_up)
+			imx_pcie_start_link(imx_pcie->pci);
+	}
 
 	return 0;
 }

@@ -1485,7 +1518,9 @@ static const struct imx_pcie_drvdata drvdata[] = {
 	[IMX6Q] = {
 		.variant = IMX6Q,
 		.flags = IMX_PCIE_FLAG_IMX_PHY |
-			 IMX_PCIE_FLAG_IMX_SPEED_CHANGE,
+			 IMX_PCIE_FLAG_IMX_SPEED_CHANGE |
+			 IMX_PCIE_FLAG_BROKEN_SUSPEND |
+			 IMX_PCIE_FLAG_SUPPORTS_SUSPEND,
 		.dbi_length = 0x200,
 		.gpr = "fsl,imx6q-iomuxc-gpr",
 		.clk_names = imx6q_clks,
Signature · 1 line
2.43.0
#1 lore raw
Quoted 18 lines · Stefan Eichenberger

From: Stefan Eichenberger <[email protected]>

The suspend/resume functionality is currently broken on the i.MX6QDL platform, as documented in the NXP errata (ERR005723):
https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf

This patch addresses the issue by sharing most of the suspend/resume sequences used by other i.MX devices, while avoiding modifications to critical registers that disrupt the PCIe functionality. It targets the same problem as the following downstream commit:
https://github.com/nxp-imx/linux-imx/commit/4e92355e1f79d225ea842511fcfd42b343b32995

Unlike the downstream commit, this patch also resets the connected PCIe device if possible. Without this reset, certain drivers, such as ath10k or iwlwifi, will crash on resume. The device reset is also done by the driver on other i.MX platforms, making this patch consistent with existing practices.

Quoted 8 lines

Without this patch, suspend/resume will fail on i.MX6QDL devices if a
PCIe device is connected. Upon resuming, the kernel will hang and
display an error. Here's an example of the error encountered with the
ath10k driver:
ath10k_pci 0000:01:00.0: Unable to change power state from D3hot to D0, device inaccessible Unhandled fault: imprecise external abort (0x1406) at 0x0106f944

Signed-off-by Stefan Eichenberger <[email protected]>

Richard and Lucas, does this look OK to you? Since you're listed as maintainers of pci-imx6.c, I'd like to have your ack before merging this.

Quoted 108 lines

---
Changes in v4:
- Improve commit message (Bjorn)
- Fix style issue on comments (Bjorn)
- s/msi/MSI (Bjorn)

Changes in v3:
- Added a new flag to the driver data to indicate that the suspend/resume is broken on the i.MX6QDL platform. (Frank)
- Fix comments to be more relevant (Mani)
- Use imx_pcie_assert_core_reset in suspend (Mani)

 drivers/pci/controller/dwc/pci-imx6.c | 57 +++++++++++++++++++++------
 1 file changed, 46 insertions(+), 11 deletions(-)
drivers/pci/controller/dwc/pci-imx6.c +46 -11
index 808d1f1054173..c8d5c90aa4d45 100644
@@ -82,6 +82,11 @@ enum imx_pcie_variants {
 #define IMX_PCIE_FLAG_HAS_SERDES		BIT(6)
 #define IMX_PCIE_FLAG_SUPPORT_64BIT		BIT(7)
 #define IMX_PCIE_FLAG_CPU_ADDR_FIXUP		BIT(8)
+/*
+ * Because of ERR005723 (PCIe does not support L2 power down) we need to
+ * workaround suspend resume on some devices which are affected by this errata.
+ */
+#define IMX_PCIE_FLAG_BROKEN_SUSPEND		BIT(9)
 
 #define imx_check_flag(pci, val)	(pci->drvdata->flags & val)
 

@@ -1237,9 +1242,19 @@ static int imx_pcie_suspend_noirq(struct device *dev)
 		return 0;
 
 	imx_pcie_msi_save_restore(imx_pcie, true);
-	imx_pcie_pm_turnoff(imx_pcie);
-	imx_pcie_stop_link(imx_pcie->pci);
-	imx_pcie_host_exit(pp);
+	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_BROKEN_SUSPEND)) {
+		/*
+		 * The minimum for a workaround would be to set PERST# and to
+		 * set the PCIE_TEST_PD flag. However, we can also disable the
+		 * clock which saves some power.
+		 */
+		imx_pcie_assert_core_reset(imx_pcie);
+		imx_pcie->drvdata->enable_ref_clk(imx_pcie, false);
+	} else {
+		imx_pcie_pm_turnoff(imx_pcie);
+		imx_pcie_stop_link(imx_pcie->pci);
+		imx_pcie_host_exit(pp);
+	}
 
 	return 0;
 }

@@ -1253,14 +1268,32 @@ static int imx_pcie_resume_noirq(struct device *dev)
 	if (!(imx_pcie->drvdata->flags & IMX_PCIE_FLAG_SUPPORTS_SUSPEND))
 		return 0;
 
-	ret = imx_pcie_host_init(pp);
-	if (ret)
-		return ret;
-	imx_pcie_msi_save_restore(imx_pcie, false);
-	dw_pcie_setup_rc(pp);
+	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_BROKEN_SUSPEND)) {
+		ret = imx_pcie->drvdata->enable_ref_clk(imx_pcie, true);
+		if (ret)
+			return ret;
+		ret = imx_pcie_deassert_core_reset(imx_pcie);
+		if (ret)
+			return ret;
+		/*
+		 * Using PCIE_TEST_PD seems to disable MSI and powers down the
+		 * root complex. This is why we have to setup the rc again and
+		 * why we have to restore the MSI register.
+		 */
+		ret = dw_pcie_setup_rc(&imx_pcie->pci->pp);
+		if (ret)
+			return ret;
+		imx_pcie_msi_save_restore(imx_pcie, false);
+	} else {
+		ret = imx_pcie_host_init(pp);
+		if (ret)
+			return ret;
+		imx_pcie_msi_save_restore(imx_pcie, false);
+		dw_pcie_setup_rc(pp);
 
-	if (imx_pcie->link_is_up)
-		imx_pcie_start_link(imx_pcie->pci);
+		if (imx_pcie->link_is_up)
+			imx_pcie_start_link(imx_pcie->pci);
+	}
 
 	return 0;
 }

@@ -1485,7 +1518,9 @@ static const struct imx_pcie_drvdata drvdata[] = {
 	[IMX6Q] = {
 		.variant = IMX6Q,
 		.flags = IMX_PCIE_FLAG_IMX_PHY |
-			 IMX_PCIE_FLAG_IMX_SPEED_CHANGE,
+			 IMX_PCIE_FLAG_IMX_SPEED_CHANGE |
+			 IMX_PCIE_FLAG_BROKEN_SUSPEND |
+			 IMX_PCIE_FLAG_SUPPORTS_SUSPEND,
 		.dbi_length = 0x200,
 		.gpr = "fsl,imx6q-iomuxc-gpr",
 		.clk_names = imx6q_clks,
Signature · 1 line
2.43.0
#2 lore raw

Hello,

[...]

Quoted 11 lines
Quoted 6 lines

Without this patch, suspend/resume will fail on i.MX6QDL devices if a
PCIe device is connected. Upon resuming, the kernel will hang and
display an error. Here's an example of the error encountered with the
ath10k driver:
ath10k_pci 0000:01:00.0: Unable to change power state from D3hot to D0, device inaccessible Unhandled fault: imprecise external abort (0x1406) at 0x0106f944

[...]

Richard and Lucas, does this look OK to you? Since you're listed as maintainers of pci-imx6.c, I'd like to have your ack before merging this.

If things look fine here, then I would like to pick it up.

Krzysztof

#3 lore raw
Quoted 9 lines · Krzysztof Wilczyński

Hello,

[...]

Quoted 6 lines
Quoted 6 lines

Without this patch, suspend/resume will fail on i.MX6QDL devices if a
PCIe device is connected. Upon resuming, the kernel will hang and
display an error. Here's an example of the error encountered with the
ath10k driver:
ath10k_pci 0000:01:00.0: Unable to change power state from D3hot to D0, device inaccessible Unhandled fault: imprecise external abort (0x1406) at 0x0106f944

Quoted 8 lines
Quoted 5 lines

[...]

Richard and Lucas, does this look OK to you? Since you're listed as maintainers of pci-imx6.c, I'd like to have your ack before merging this.

If things look fine here, then I would like to pick it up.

LGTM, feel free to pick it up.

- Mani

Signature · 1 line
மணிவண்ணன் சதாசிவம்
#4 lore raw

Hello,

Quoted 9 lines
Quoted 9 lines

[...]

Quoted 8 lines
Quoted 6 lines

Without this patch, suspend/resume will fail on i.MX6QDL devices if a
PCIe device is connected. Upon resuming, the kernel will hang and
display an error. Here's an example of the error encountered with the
ath10k driver:
ath10k_pci 0000:01:00.0: Unable to change power state from D3hot to D0, device inaccessible Unhandled fault: imprecise external abort (0x1406) at 0x0106f944

[...]

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

Richard and Lucas, does this look OK to you? Since you're listed as maintainers of pci-imx6.c, I'd like to have your ack before merging this.

If things look fine here, then I would like to pick it up.

LGTM, feel free to pick it up.

Sounds good! Thank you!

Krzysztof

maintainer Acked-by
#5 lore raw
In reply to Bjorn Helgaas
Quoted 73 lines

-----Original Message-----
From: Bjorn Helgaas <[email protected]>
Sent: 2024年11月2日 3:34
To: Stefan Eichenberger <[email protected]>; Hongxing Zhu <[email protected]>; Lucas Stach <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Francesco Dolcini <[email protected]>; Frank Li <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; Stefan Eichenberger <[email protected]>
Subject: Re: [PATCH v4] PCI: imx6: Add suspend/resume support for i.MX6QDL

Quoted 6 lines · Stefan Eichenberger

From: Stefan Eichenberger <[email protected]>

The suspend/resume functionality is currently broken on the i.MX6QDL platform, as documented in the NXP errata (ERR005723):
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.

nxp.com%2Fdocs%2Fen%2Ferrata%2FIMX6DQCE.pdf&data=05%7C02%7Chong xing.zh

Quoted 1 line

u%40nxp.com%7C9039460e3cf54773efcd08dcfaac2bc7%7C686ea1d3bc2b4c6f a92cd

Quoted 1 line

99c5c301635%7C0%7C0%7C638660864604098652%7CUnknown%7CTWFpbGZs b3d8eyJWI

Quoted 1 line

joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0% 7C%7

Quoted 1 line

C%7C&sdata=WACKwvUPlTZJcxjkLe7YI3ZnITtxEj6wULhMndh8zNE%3D&reserv ed=0

Quoted 7 lines

This patch addresses the issue by sharing most of the suspend/resume sequences used by other i.MX devices, while avoiding modifications to critical registers that disrupt the PCIe functionality. It targets the same problem as the following downstream commit:
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith

ub.com%2Fnxp-imx%2Flinux-imx%2Fcommit%2F4e92355e1f79d225ea842511fc fd42

Quoted 1 line

b343b32995&data=05%7C02%7Chongxing.zhu%40nxp.com%7C9039460e3cf54 773efc

Quoted 1 line

d08dcfaac2bc7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63866 086460

Quoted 1 line

4121904%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2lu MzIiLC

Quoted 1 line

JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=tafoT8ug4EehGXrE NzQOb%

Quoted 16 lines

2F7rD%2Fra8%2BefnAEiV%2FB8Afc%3D&reserved=0

Unlike the downstream commit, this patch also resets the connected PCIe device if possible. Without this reset, certain drivers, such as ath10k or iwlwifi, will crash on resume. The device reset is also done by the driver on other i.MX platforms, making this patch consistent with existing practices.

Without this patch, suspend/resume will fail on i.MX6QDL devices if a PCIe device is connected. Upon resuming, the kernel will hang and display an error. Here's an example of the error encountered with the ath10k driver:
ath10k_pci 0000:01:00.0: Unable to change power state from D3hot to D0, device inaccessible Unhandled fault: imprecise external abort (0x1406) at 0x0106f944

Quoted 5 lines
Quoted 1 line
Signed-off-by Stefan Eichenberger <[email protected]>

Richard and Lucas, does this look OK to you? Since you're listed as maintainers of pci-imx6.c, I'd like to have your ack before merging this.

Sorry to reply late.
I'm fine with that.
Thanks.

Acked-by Richard Zhu <[email protected]>

Best Regards
Richard Zhu

Quoted 114 lines
Quoted 22 lines

---
Changes in v4:
- Improve commit message (Bjorn)
- Fix style issue on comments (Bjorn)
- s/msi/MSI (Bjorn)

Changes in v3:
- Added a new flag to the driver data to indicate that the suspend/resume is broken on the i.MX6QDL platform. (Frank)
- Fix comments to be more relevant (Mani)
- Use imx_pcie_assert_core_reset in suspend (Mani)

 drivers/pci/controller/dwc/pci-imx6.c | 57
+++++++++++++++++++++------
 1 file changed, 46 insertions(+), 11 deletions(-)
drivers/pci/controller/dwc/pci-imx6.c +0 -0
index 808d1f1054173..c8d5c90aa4d45 100644

drivers/pci/controller/dwc/pci-imx6.c enum imx_pcie_variants { · line 88

Quoted 7 lines
drivers/pci/controller/dwc/pci-imx6.c +4 -0
@@ -82,6 +82,11 @@ enum imx_pcie_variants {
 #define IMX_PCIE_FLAG_HAS_SERDES		BIT(6)
 #define IMX_PCIE_FLAG_SUPPORT_64BIT		BIT(7)
 #define IMX_PCIE_FLAG_CPU_ADDR_FIXUP		BIT(8)
+/*
+ * Because of ERR005723 (PCIe does not support L2 power down) we need
+to
+ * workaround suspend resume on some devices which are affected by this

errata.

drivers/pci/controller/dwc/pci-imx6.c static int imx_pcie_suspend_noirq(struct device

Quoted 6 lines
drivers/pci/controller/dwc/pci-imx6.c +2 -0
+ */
+#define IMX_PCIE_FLAG_BROKEN_SUSPEND		BIT(9)
 
 #define imx_check_flag(pci, val)	(pci->drvdata->flags & val)

@@ -1237,9 +1242,19 @@ static int imx_pcie_suspend_noirq(struct device

*dev)

drivers/pci/controller/dwc/pci-imx6.c static int imx_pcie_resume_noirq(struct device

Quoted 23 lines
drivers/pci/controller/dwc/pci-imx6.c +13 -3
 		return 0;
 
 	imx_pcie_msi_save_restore(imx_pcie, true);
-	imx_pcie_pm_turnoff(imx_pcie);
-	imx_pcie_stop_link(imx_pcie->pci);
-	imx_pcie_host_exit(pp);
+	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_BROKEN_SUSPEND)) {
+		/*
+		 * The minimum for a workaround would be to set PERST# and to
+		 * set the PCIE_TEST_PD flag. However, we can also disable the
+		 * clock which saves some power.
+		 */
+		imx_pcie_assert_core_reset(imx_pcie);
+		imx_pcie->drvdata->enable_ref_clk(imx_pcie, false);
+	} else {
+		imx_pcie_pm_turnoff(imx_pcie);
+		imx_pcie_stop_link(imx_pcie->pci);
+		imx_pcie_host_exit(pp);
+	}
 
 	return 0;
 }

@@ -1253,14 +1268,32 @@ static int imx_pcie_resume_noirq(struct device

*dev)

Quoted 53 lines
drivers/pci/controller/dwc/pci-imx6.c +28 -9
 	if (!(imx_pcie->drvdata->flags & IMX_PCIE_FLAG_SUPPORTS_SUSPEND))
 		return 0;
 
-	ret = imx_pcie_host_init(pp);
-	if (ret)
-		return ret;
-	imx_pcie_msi_save_restore(imx_pcie, false);
-	dw_pcie_setup_rc(pp);
+	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_BROKEN_SUSPEND)) {
+		ret = imx_pcie->drvdata->enable_ref_clk(imx_pcie, true);
+		if (ret)
+			return ret;
+		ret = imx_pcie_deassert_core_reset(imx_pcie);
+		if (ret)
+			return ret;
+		/*
+		 * Using PCIE_TEST_PD seems to disable MSI and powers down the
+		 * root complex. This is why we have to setup the rc again and
+		 * why we have to restore the MSI register.
+		 */
+		ret = dw_pcie_setup_rc(&imx_pcie->pci->pp);
+		if (ret)
+			return ret;
+		imx_pcie_msi_save_restore(imx_pcie, false);
+	} else {
+		ret = imx_pcie_host_init(pp);
+		if (ret)
+			return ret;
+		imx_pcie_msi_save_restore(imx_pcie, false);
+		dw_pcie_setup_rc(pp);
 
-	if (imx_pcie->link_is_up)
-		imx_pcie_start_link(imx_pcie->pci);
+		if (imx_pcie->link_is_up)
+			imx_pcie_start_link(imx_pcie->pci);
+	}
 
 	return 0;
 }

@@ -1485,7 +1518,9 @@ static const struct imx_pcie_drvdata drvdata[] = {
 	[IMX6Q] = {
 		.variant = IMX6Q,
 		.flags = IMX_PCIE_FLAG_IMX_PHY |
-			 IMX_PCIE_FLAG_IMX_SPEED_CHANGE,
+			 IMX_PCIE_FLAG_IMX_SPEED_CHANGE |
+			 IMX_PCIE_FLAG_BROKEN_SUSPEND |
+			 IMX_PCIE_FLAG_SUPPORTS_SUSPEND,
 		.dbi_length = 0x200,
 		.gpr = "fsl,imx6q-iomuxc-gpr",
 		.clk_names = imx6q_clks,
--

2.43.0

Reviewed-by on v4 1/1
#6 lore raw
In reply to Stefan Eichenberger
Quoted 18 lines · Stefan Eichenberger

From: Stefan Eichenberger <[email protected]>

The suspend/resume functionality is currently broken on the i.MX6QDL platform, as documented in the NXP errata (ERR005723):
https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf

This patch addresses the issue by sharing most of the suspend/resume sequences used by other i.MX devices, while avoiding modifications to critical registers that disrupt the PCIe functionality. It targets the same problem as the following downstream commit:
https://github.com/nxp-imx/linux-imx/commit/4e92355e1f79d225ea842511fcfd42b343b32995

Unlike the downstream commit, this patch also resets the connected PCIe device if possible. Without this reset, certain drivers, such as ath10k or iwlwifi, will crash on resume. The device reset is also done by the driver on other i.MX platforms, making this patch consistent with existing practices.

Quoted 8 lines

Without this patch, suspend/resume will fail on i.MX6QDL devices if a
PCIe device is connected. Upon resuming, the kernel will hang and
display an error. Here's an example of the error encountered with the
ath10k driver:
ath10k_pci 0000:01:00.0: Unable to change power state from D3hot to D0, device inaccessible Unhandled fault: imprecise external abort (0x1406) at 0x0106f944

Signed-off-by Stefan Eichenberger <[email protected]>
Reviewed-by Manivannan Sadhasivam <[email protected]>

- Mani

Quoted 108 lines

---
Changes in v4:
- Improve commit message (Bjorn)
- Fix style issue on comments (Bjorn)
- s/msi/MSI (Bjorn)

Changes in v3:
- Added a new flag to the driver data to indicate that the suspend/resume is broken on the i.MX6QDL platform. (Frank)
- Fix comments to be more relevant (Mani)
- Use imx_pcie_assert_core_reset in suspend (Mani)

 drivers/pci/controller/dwc/pci-imx6.c | 57 +++++++++++++++++++++------
 1 file changed, 46 insertions(+), 11 deletions(-)
drivers/pci/controller/dwc/pci-imx6.c +46 -11
index 808d1f1054173..c8d5c90aa4d45 100644
@@ -82,6 +82,11 @@ enum imx_pcie_variants {
 #define IMX_PCIE_FLAG_HAS_SERDES		BIT(6)
 #define IMX_PCIE_FLAG_SUPPORT_64BIT		BIT(7)
 #define IMX_PCIE_FLAG_CPU_ADDR_FIXUP		BIT(8)
+/*
+ * Because of ERR005723 (PCIe does not support L2 power down) we need to
+ * workaround suspend resume on some devices which are affected by this errata.
+ */
+#define IMX_PCIE_FLAG_BROKEN_SUSPEND		BIT(9)
 
 #define imx_check_flag(pci, val)	(pci->drvdata->flags & val)
 

@@ -1237,9 +1242,19 @@ static int imx_pcie_suspend_noirq(struct device *dev)
 		return 0;
 
 	imx_pcie_msi_save_restore(imx_pcie, true);
-	imx_pcie_pm_turnoff(imx_pcie);
-	imx_pcie_stop_link(imx_pcie->pci);
-	imx_pcie_host_exit(pp);
+	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_BROKEN_SUSPEND)) {
+		/*
+		 * The minimum for a workaround would be to set PERST# and to
+		 * set the PCIE_TEST_PD flag. However, we can also disable the
+		 * clock which saves some power.
+		 */
+		imx_pcie_assert_core_reset(imx_pcie);
+		imx_pcie->drvdata->enable_ref_clk(imx_pcie, false);
+	} else {
+		imx_pcie_pm_turnoff(imx_pcie);
+		imx_pcie_stop_link(imx_pcie->pci);
+		imx_pcie_host_exit(pp);
+	}
 
 	return 0;
 }

@@ -1253,14 +1268,32 @@ static int imx_pcie_resume_noirq(struct device *dev)
 	if (!(imx_pcie->drvdata->flags & IMX_PCIE_FLAG_SUPPORTS_SUSPEND))
 		return 0;
 
-	ret = imx_pcie_host_init(pp);
-	if (ret)
-		return ret;
-	imx_pcie_msi_save_restore(imx_pcie, false);
-	dw_pcie_setup_rc(pp);
+	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_BROKEN_SUSPEND)) {
+		ret = imx_pcie->drvdata->enable_ref_clk(imx_pcie, true);
+		if (ret)
+			return ret;
+		ret = imx_pcie_deassert_core_reset(imx_pcie);
+		if (ret)
+			return ret;
+		/*
+		 * Using PCIE_TEST_PD seems to disable MSI and powers down the
+		 * root complex. This is why we have to setup the rc again and
+		 * why we have to restore the MSI register.
+		 */
+		ret = dw_pcie_setup_rc(&imx_pcie->pci->pp);
+		if (ret)
+			return ret;
+		imx_pcie_msi_save_restore(imx_pcie, false);
+	} else {
+		ret = imx_pcie_host_init(pp);
+		if (ret)
+			return ret;
+		imx_pcie_msi_save_restore(imx_pcie, false);
+		dw_pcie_setup_rc(pp);
 
-	if (imx_pcie->link_is_up)
-		imx_pcie_start_link(imx_pcie->pci);
+		if (imx_pcie->link_is_up)
+			imx_pcie_start_link(imx_pcie->pci);
+	}
 
 	return 0;
 }

@@ -1485,7 +1518,9 @@ static const struct imx_pcie_drvdata drvdata[] = {
 	[IMX6Q] = {
 		.variant = IMX6Q,
 		.flags = IMX_PCIE_FLAG_IMX_PHY |
-			 IMX_PCIE_FLAG_IMX_SPEED_CHANGE,
+			 IMX_PCIE_FLAG_IMX_SPEED_CHANGE |
+			 IMX_PCIE_FLAG_BROKEN_SUSPEND |
+			 IMX_PCIE_FLAG_SUPPORTS_SUSPEND,
 		.dbi_length = 0x200,
 		.gpr = "fsl,imx6q-iomuxc-gpr",
 		.clk_names = imx6q_clks,
Signature · 1 line
2.43.0
Signature · 1 line
மணிவண்ணன் சதாசிவம்
Applied
#7 lore raw
In reply to Stefan Eichenberger

Hello,

Quoted 16 lines

The suspend/resume functionality is currently broken on the i.MX6QDL platform, as documented in the NXP errata (ERR005723):
https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf

This patch addresses the issue by sharing most of the suspend/resume sequences used by other i.MX devices, while avoiding modifications to critical registers that disrupt the PCIe functionality. It targets the same problem as the following downstream commit:
https://github.com/nxp-imx/linux-imx/commit/4e92355e1f79d225ea842511fcfd42b343b32995

Unlike the downstream commit, this patch also resets the connected PCIe device if possible. Without this reset, certain drivers, such as ath10k or iwlwifi, will crash on resume. The device reset is also done by the driver on other i.MX platforms, making this patch consistent with existing practices.

Quoted 6 lines

Without this patch, suspend/resume will fail on i.MX6QDL devices if a
PCIe device is connected. Upon resuming, the kernel will hang and
display an error. Here's an example of the error encountered with the
ath10k driver:
ath10k_pci 0000:01:00.0: Unable to change power state from D3hot to D0, device inaccessible Unhandled fault: imprecise external abort (0x1406) at 0x0106f944

Applied to controller/imx6, thank you!

[01/01] PCI: imx6: Fix suspend/resume support on i.MX6QDL

        https://git.kernel.org/pci/pci/c/1a2a9024f84d

	Krzysztof