ARM: mx6: Fix suspend/resume with PCI

PATCH New Fabio Estevam 4 participants 11 messages Aug 7, 2014 – Oct 22, 2014 1 message not in the archive

Subsystems (3)

Arm (32-bit) Arm sub-architectures Arm i.MX and Layerscape

lore

Share & tools

lore

Fabio Estevam

author reviewer

2014-08-07 00:39 UTC #0 lore raw

PATCH 1/1 ARM: mx6: Fix suspend/resume with PCI

From: Fabio Estevam <[email protected]>

When PCI is used and a suspend/resume sequence is done we see the following kernel hang:

root at freescale /$ echo mem > /sys/power/state

[   16.099018] PM: Syncing filesystems ... done.
[   16.141010] Freezing user space processes ... (elapsed 0.002 seconds) done.
[   16.150840] Freezing remaining freezable tasks ... (elapsed 0.003 seconds) done.
[   16.199438] random: nonblocking pool is initialized
[   16.229639] PM: suspend of devices complete after 64.793 msecs
[   16.235488] PM: suspend devices took 0.070 seconds
[   16.245301] PM: late suspend of devices complete after 4.968 msecs
[   16.257063] PM: noirq suspend of devices complete after 5.538 msecs
[   16.263425] Disabling non-boot CPUs ...
[   16.274666] CPU1: shutdown
[   16.286351] CPU2: shutdown
[   16.294169] CPU3: shutdown
[   16.299551] Enabling non-boot CPUs ...
[   16.304155] CPU1: Booted secondary processor
[   16.305717] CPU1 is up
[   16.313078] CPU2: Booted secondary processor
[   16.313456] CPU2 is up
[   16.320778] CPU3: Booted secondary processor
[   16.321174] CPU3 is up
	(hangs here)

Implement a workaround for the erratum ERR005723: "PCIe does not support L2 Power Down", which consists in toggling bit 18 (TEST_POWERDOWN) of GPR1 register.

Tested on a mx6qsabresd TO1.2 revC2.

Reported-by Shawn Guo <[email protected]>

Signed-off-by Fabio Estevam <[email protected]>

---
Richard,

Is there any better method to fix this issue? Or should we do the same as in the FSL kernel?

I also tried to add pm ops into the imx6 pci driver and did the toggle there, but I still got the kernel hang.

 arch/arm/mach-imx/pm-imx6.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

arch/arm/mach-imx/pm-imx6.c +27 -0

index 5c3af8f..92511cf 100644
@@ -341,6 +341,23 @@ static int imx6q_suspend_finish(unsigned long val)
 
 static int imx6q_pm_enter(suspend_state_t state)
 {
+	struct regmap *gpr;
+	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+	if (IS_ERR(gpr)) {
+		pr_err("failed to find fsl,imx6q-iomux-gpr regmap\n");
+		return PTR_ERR(gpr);
+	}
+
+	/*
+	 * L2 can exit by 'reset' or Inband beacon (from remote EP)
+	 * toggling phy_powerdown has same effect as 'inband beacon'
+	 * So, toggle bit18 of GPR1 as a workaround of erratum
+	 * ERR005723 - "PCIe does not support L2 Power Down"
+	 */
+	if (IS_ENABLED(CONFIG_PCI_IMX6) && !cpu_is_imx6sx())
+		regmap_update_bits(gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_TEST_PD,
+				   IMX6Q_GPR1_PCIE_TEST_PD);
+
 	switch (state) {
 	case PM_SUSPEND_STANDBY:
 		imx6q_set_lpm(STOP_POWER_ON);

@@ -383,6 +400,16 @@ static int imx6q_pm_enter(suspend_state_t state)
 		return -EINVAL;
 	}
 
+	/*
+	 * L2 can exit by 'reset' or Inband beacon (from remote EP)
+	 * toggling phy_powerdown has same effect as 'inband beacon'
+	 * So, toggle bit18 of GPR1 as a workaround of erratum
+	 * ERR005723 - "PCIe does not support L2 Power Down"
+	 */
+	if (IS_ENABLED(CONFIG_PCI_IMX6) && !cpu_is_imx6sx())
+		regmap_update_bits(gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_TEST_PD,
+				   !IMX6Q_GPR1_PCIE_TEST_PD);
+
 	return 0;
 }

Signature · 1 line

1.9.1

Shawn Guo

2014-08-07 02:18 UTC #1 lore raw

PATCH 1/1 ARM: mx6: Fix suspend/resume with PCI

Quoted 27 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>

When PCI is used and a suspend/resume sequence is done we see the following kernel hang:

root at freescale /$ echo mem > /sys/power/state

[   16.099018] PM: Syncing filesystems ... done.
[   16.141010] Freezing user space processes ... (elapsed 0.002 seconds) done.
[   16.150840] Freezing remaining freezable tasks ... (elapsed 0.003 seconds) done.
[   16.199438] random: nonblocking pool is initialized
[   16.229639] PM: suspend of devices complete after 64.793 msecs
[   16.235488] PM: suspend devices took 0.070 seconds
[   16.245301] PM: late suspend of devices complete after 4.968 msecs
[   16.257063] PM: noirq suspend of devices complete after 5.538 msecs
[   16.263425] Disabling non-boot CPUs ...
[   16.274666] CPU1: shutdown
[   16.286351] CPU2: shutdown
[   16.294169] CPU3: shutdown
[   16.299551] Enabling non-boot CPUs ...
[   16.304155] CPU1: Booted secondary processor
[   16.305717] CPU1 is up
[   16.313078] CPU2: Booted secondary processor
[   16.313456] CPU2 is up
[   16.320778] CPU3: Booted secondary processor
[   16.321174] CPU3 is up
	(hangs here)

Quoted 4 lines

Implement a workaround for the erratum ERR005723: "PCIe does not support L2 Power Down", which consists in toggling bit 18 (TEST_POWERDOWN) of GPR1 register.
Tested on a mx6qsabresd TO1.2 revC2.

I intend to agree with Lucas that we should align our testing and understanding on the issue before trying to fix it.

Again, I do not see this issue on my TO1.2 chip, but only TO1.5, aka Rev 1.3, one.

Shawn

Quoted 3 lines

Reported-by Shawn Guo <[email protected]>
Signed-off-by Fabio Estevam <[email protected]>

Fabio Estevam

author reviewer

2014-08-07 02:52 UTC #2 lore raw

PATCH 1/1 ARM: mx6: Fix suspend/resume with PCI

Quoted 5 lines · Shawn Guo

I intend to agree with Lucas that we should align our testing and understanding on the issue before trying to fix it.
Again, I do not see this issue on my TO1.2 chip, but only TO1.5, aka Rev 1.3, one.

Which version is your sabresd with TO1.2? Mine is rev C2 and the suspend/resume issue happens 100%.

Shawn Guo

2014-08-07 03:08 UTC #3 lore raw

PATCH 1/1 ARM: mx6: Fix suspend/resume with PCI

Quoted 10 lines · Fabio Estevam

Quoted 5 lines · Shawn Guo
I intend to agree with Lucas that we should align our testing and understanding on the issue before trying to fix it.
Again, I do not see this issue on my TO1.2 chip, but only TO1.5, aka Rev 1.3, one.
Which version is your sabresd with TO1.2? Mine is rev C2 and the suspend/resume issue happens 100%.

The label on the board says it's Rev B.

Shawn

Fabio Estevam

author reviewer

2014-08-07 03:24 UTC #4 lore raw

PATCH 1/1 ARM: mx6: Fix suspend/resume with PCI

Quoted 6 lines · Shawn Guo

Quoted 5 lines · Fabio Estevam
Quoted 3 lines · Shawn Guo
I intend to agree with Lucas that we should align our testing and understanding on the issue before trying to fix it.

Quoted 7 lines

Quoted 5 lines
Quoted 2 lines
Again, I do not see this issue on my TO1.2 chip, but only TO1.5, aka Rev 1.3, one.
Which version is your sabresd with TO1.2? Mine is rev C2 and the suspend/resume issue happens 100%.
The label on the board says it's Rev B.

If I remember correctly you are testing suspend/resume with no PCI card connected to the TO1.2 board, right?

I have just tried suspend/resume without any PCI card connect and it does work fine.

Only when I have the PCI card connected I get the hang.

Shawn Guo

2014-08-07 03:30 UTC #5 lore raw

PATCH 1/1 ARM: mx6: Fix suspend/resume with PCI

Quoted 10 lines · Fabio Estevam

Quoted 9 lines · Shawn Guo
Quoted 8 lines · Fabio Estevam
Quoted 5 lines · Shawn Guo
I intend to agree with Lucas that we should align our testing and understanding on the issue before trying to fix it.
Again, I do not see this issue on my TO1.2 chip, but only TO1.5, aka Rev 1.3, one.

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
Which version is your sabresd with TO1.2? Mine is rev C2 and the suspend/resume issue happens 100%.
The label on the board says it's Rev B.
If I remember correctly you are testing suspend/resume with no PCI card connected to the TO1.2 board, right?

Right. I do not have any PCIe device on hands, so all my testing are without PCIe device connected, and TO1.2 works while TO1.5 doesn't.

Shawn

Quoted 5 lines

I have just tried suspend/resume without any PCI card connect and it does work fine.
Only when I have the PCI card connected I get the hang.

Lucas Stach

NAK

2014-08-07 10:04 UTC #6 lore raw

PATCH 1/1 ARM: mx6: Fix suspend/resume with PCI

In reply to Fabio Estevam

Am Mittwoch, den 06.08.2014, 21:39 -0300 schrieb Fabio Estevam:

Quoted 27 lines

From: Fabio Estevam <[email protected]>

When PCI is used and a suspend/resume sequence is done we see the following kernel hang:

root at freescale /$ echo mem > /sys/power/state

[   16.099018] PM: Syncing filesystems ... done.
[   16.141010] Freezing user space processes ... (elapsed 0.002 seconds) done.
[   16.150840] Freezing remaining freezable tasks ... (elapsed 0.003 seconds) done.
[   16.199438] random: nonblocking pool is initialized
[   16.229639] PM: suspend of devices complete after 64.793 msecs
[   16.235488] PM: suspend devices took 0.070 seconds
[   16.245301] PM: late suspend of devices complete after 4.968 msecs
[   16.257063] PM: noirq suspend of devices complete after 5.538 msecs
[   16.263425] Disabling non-boot CPUs ...
[   16.274666] CPU1: shutdown
[   16.286351] CPU2: shutdown
[   16.294169] CPU3: shutdown
[   16.299551] Enabling non-boot CPUs ...
[   16.304155] CPU1: Booted secondary processor
[   16.305717] CPU1 is up
[   16.313078] CPU2: Booted secondary processor
[   16.313456] CPU2 is up
[   16.320778] CPU3: Booted secondary processor
[   16.321174] CPU3 is up
	(hangs here)

Quoted 7 lines

Implement a workaround for the erratum ERR005723: "PCIe does not support L2 Power Down", which consists in toggling bit 18 (TEST_POWERDOWN) of GPR1 register.
Tested on a mx6qsabresd TO1.2 revC2.
Reported-by Shawn Guo <[email protected]>
Signed-off-by Fabio Estevam <[email protected]>

NACK

To my understanding the DW PCIe core needs a specific sequence to enter/exit L2 at suspend that we currently ignore. Contrary to the errata description I think it can be done without a dedicated core reset signal. Hitting the PHY PD is a sledgehammer without proper understanding of the problem.

Also hiding such things in the platform code seems really bad. Please keep in mind that this is the mainline kernel, not a vendor downstream tree where you are free to smash things all over the place.

Regards,
Lucas

Signature · 2 lines

Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Fabio Estevam

author reviewer

2014-08-07 11:55 UTC #7 lore raw

PATCH 1/1 ARM: mx6: Fix suspend/resume with PCI

Quoted 3 lines · Lucas Stach

Also hiding such things in the platform code seems really bad. Please keep in mind that this is the mainline kernel, not a vendor downstream tree where you are free to smash things all over the place.

Sure, I should have marked it as RFC. I also asked for a better way to handle this issue below the --- line.

Bjørn Erik Nilsen

2014-09-30 10:12 UTC #8 lore raw

PATCH 1/1 ARM: mx6: Fix suspend/resume with PCI

In reply to Fabio Estevam

Hi Fabio,

Quoted 27 lines · an unknown sender

From: Fabio Estevam <[email protected]>

When PCI is used and a suspend/resume sequence is done we see the following kernel hang:

root at freescale /$ echo mem > /sys/power/state

[   16.099018] PM: Syncing filesystems ... done.
[   16.141010] Freezing user space processes ... (elapsed 0.002 seconds) done.
[   16.150840] Freezing remaining freezable tasks ... (elapsed 0.003 seconds) done.
[   16.199438] random: nonblocking pool is initialized
[   16.229639] PM: suspend of devices complete after 64.793 msecs
[   16.235488] PM: suspend devices took 0.070 seconds
[   16.245301] PM: late suspend of devices complete after 4.968 msecs
[   16.257063] PM: noirq suspend of devices complete after 5.538 msecs
[   16.263425] Disabling non-boot CPUs ...
[   16.274666] CPU1: shutdown
[   16.286351] CPU2: shutdown
[   16.294169] CPU3: shutdown
[   16.299551] Enabling non-boot CPUs ...
[   16.304155] CPU1: Booted secondary processor
[   16.305717] CPU1 is up
[   16.313078] CPU2: Booted secondary processor
[   16.313456] CPU2 is up
[   16.320778] CPU3: Booted secondary processor
[   16.321174] CPU3 is up
	(hangs here)

Quoted 7 lines

Implement a workaround for the erratum ERR005723: "PCIe does not support L2 Power Down", which consists in toggling bit 18 (TEST_POWERDOWN) of GPR1 register.
Tested on a mx6qsabresd TO1.2 revC2.
Reported-by Shawn Guo <[email protected]>
Signed-off-by Fabio Estevam <[email protected]>

Tested-by Bj?rn Erik Nilsen <[email protected]>

I encountered exactly the same problem and can confirm your patch works.

Thank you!

I hope this patch (or one that is considered a proper solution) will make it to the mainline kernel any time soon.

Best regards,
Bj?rn Erik Nilsen

Quoted 57 lines

---
Richard,

Is there any better method to fix this issue? Or should we do the same as in the FSL kernel?

I also tried to add pm ops into the imx6 pci driver and did the toggle there, but I still got the kernel hang.

 arch/arm/mach-imx/pm-imx6.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

arch/arm/mach-imx/pm-imx6.c +27 -0

index 5c3af8f..92511cf 100644
@@ -341,6 +341,23 @@ static int imx6q_suspend_finish(unsigned long val)
 
 static int imx6q_pm_enter(suspend_state_t state)
 {
+	struct regmap *gpr;
+	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+	if (IS_ERR(gpr)) {
+		pr_err("failed to find fsl,imx6q-iomux-gpr regmap\n");
+		return PTR_ERR(gpr);
+	}
+
+	/*
+	 * L2 can exit by 'reset' or Inband beacon (from remote EP)
+	 * toggling phy_powerdown has same effect as 'inband beacon'
+	 * So, toggle bit18 of GPR1 as a workaround of erratum
+	 * ERR005723 - "PCIe does not support L2 Power Down"
+	 */
+	if (IS_ENABLED(CONFIG_PCI_IMX6) && !cpu_is_imx6sx())
+		regmap_update_bits(gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_TEST_PD,
+				   IMX6Q_GPR1_PCIE_TEST_PD);
+
 	switch (state) {
 	case PM_SUSPEND_STANDBY:
 		imx6q_set_lpm(STOP_POWER_ON);

@@ -383,6 +400,16 @@ static int imx6q_pm_enter(suspend_state_t state)
 		return -EINVAL;
 	}
 
+	/*
+	 * L2 can exit by 'reset' or Inband beacon (from remote EP)
+	 * toggling phy_powerdown has same effect as 'inband beacon'
+	 * So, toggle bit18 of GPR1 as a workaround of erratum
+	 * ERR005723 - "PCIe does not support L2 Power Down"
+	 */
+	if (IS_ENABLED(CONFIG_PCI_IMX6) && !cpu_is_imx6sx())
+		regmap_update_bits(gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_TEST_PD,
+				   !IMX6Q_GPR1_PCIE_TEST_PD);
+
 	return 0;
 }

A message that is not in the archive (sent off-list or not yet received): [email protected]

Fabio Estevam

author reviewer

2014-09-30 11:42 UTC #10 lore raw

PATCH 1/1 ARM: mx6: Fix suspend/resume with PCI

Hi Bj?rn,

Quoted 38 lines · Bj?rn Erik Nilsen

Hi Fabio,
Quoted 27 lines · an unknown sender
From: Fabio Estevam <[email protected]>
When PCI is used and a suspend/resume sequence is done we see the following kernel hang:
root at freescale /$ echo mem > /sys/power/state
[   16.099018] PM: Syncing filesystems ... done.
[   16.141010] Freezing user space processes ... (elapsed 0.002 seconds) done.
[   16.150840] Freezing remaining freezable tasks ... (elapsed 0.003 seconds) done.
[   16.199438] random: nonblocking pool is initialized
[   16.229639] PM: suspend of devices complete after 64.793 msecs
[   16.235488] PM: suspend devices took 0.070 seconds
[   16.245301] PM: late suspend of devices complete after 4.968 msecs
[   16.257063] PM: noirq suspend of devices complete after 5.538 msecs
[   16.263425] Disabling non-boot CPUs ...
[   16.274666] CPU1: shutdown
[   16.286351] CPU2: shutdown
[   16.294169] CPU3: shutdown
[   16.299551] Enabling non-boot CPUs ...
[   16.304155] CPU1: Booted secondary processor
[   16.305717] CPU1 is up
[   16.313078] CPU2: Booted secondary processor
[   16.313456] CPU2 is up
[   16.320778] CPU3: Booted secondary processor
[   16.321174] CPU3 is up
      (hangs here)
Quoted 7 lines
Implement a workaround for the erratum ERR005723: "PCIe does not support L2 Power Down", which consists in toggling bit 18 (TEST_POWERDOWN) of GPR1 register.
Tested on a mx6qsabresd TO1.2 revC2.
Reported-by Shawn Guo <[email protected]>
Signed-off-by Fabio Estevam <[email protected]>

Quoted 8 lines

Tested-by Bj?rn Erik Nilsen <[email protected]>
I encountered exactly the same problem and can confirm your patch works.
Thank you!
I hope this patch (or one that is considered a proper solution) will make it to the mainline kernel any time soon.

Thanks for testing.

This is a workaround found on FSL kernel. We still need to come up with a proper fix for this PCI suspend/resume issue.

Fabio Estevam

author reviewer

2014-10-22 17:13 UTC #11 lore raw

PATCH 1/1 ARM: mx6: Fix suspend/resume with PCI

Hi Bj?rn,

Quoted 4 lines · Fabio Estevam

Thanks for testing.
This is a workaround found on FSL kernel. We still need to come up with a proper fix for this PCI suspend/resume issue.

Just tried 3.18.0-rc1 and I don't see this PCI suspend/resume hang anymore on a mx6qsabresd revC2 with silicon TO1.2.

It does hang on an old mx6sabresd revB with TO1.1 silicon though.

Can you try with 3.18-rc1?