| Message ID | 20241030103250.83640-1-eichest@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | [v4] PCI: imx6: Add suspend/resume support for i.MX6QDL | expand |
On Wed, Oct 30, 2024 at 11:32:45AM +0100, Stefan Eichenberger wrote: > From: Stefan Eichenberger <stefan.eichenberger@toradex.com> > > 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 <stefan.eichenberger@toradex.com> 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. > --- > 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(-) > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > index 808d1f1054173..c8d5c90aa4d45 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -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, > -- > 2.43.0 >
Hello, [...] > > 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
> -----Original Message----- > From: Bjorn Helgaas <helgaas@kernel.org> > Sent: 2024年11月2日 3:34 > To: Stefan Eichenberger <eichest@gmail.com>; Hongxing Zhu > <hongxing.zhu@nxp.com>; Lucas Stach <l.stach@pengutronix.de> > Cc: lpieralisi@kernel.org; kw@linux.com; manivannan.sadhasivam@linaro.org; > robh@kernel.org; bhelgaas@google.com; shawnguo@kernel.org; > s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com; > Francesco Dolcini <francesco.dolcini@toradex.com>; Frank Li > <frank.li@nxp.com>; linux-pci@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; imx@lists.linux.dev; > linux-kernel@vger.kernel.org; Stefan Eichenberger > <stefan.eichenberger@toradex.com> > Subject: Re: [PATCH v4] PCI: imx6: Add suspend/resume support for i.MX6QDL > > On Wed, Oct 30, 2024 at 11:32:45AM +0100, Stefan Eichenberger wrote: > > From: Stefan Eichenberger <stefan.eichenberger@toradex.com> > > > > 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 > > > u%40nxp.com%7C9039460e3cf54773efcd08dcfaac2bc7%7C686ea1d3bc2b4c6f > a92cd > > > 99c5c301635%7C0%7C0%7C638660864604098652%7CUnknown%7CTWFpbGZs > b3d8eyJWI > > > joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0% > 7C%7 > > > C%7C&sdata=WACKwvUPlTZJcxjkLe7YI3ZnITtxEj6wULhMndh8zNE%3D&reserv > ed=0 > > > > 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 > > > b343b32995&data=05%7C02%7Chongxing.zhu%40nxp.com%7C9039460e3cf54 > 773efc > > > d08dcfaac2bc7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63866 > 086460 > > > 4121904%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2lu > MzIiLC > > > JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=tafoT8ug4EehGXrE > NzQOb% > > 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 > > > > Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com> > > 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 <hongxing.zhu@nxp.com> Best Regards Richard Zhu > > --- > > 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(-) > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c > > b/drivers/pci/controller/dwc/pci-imx6.c > > index 808d1f1054173..c8d5c90aa4d45 100644 > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > @@ -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, > > -- > > 2.43.0 > >
On Wed, Oct 30, 2024 at 11:32:45AM +0100, Stefan Eichenberger wrote: > From: Stefan Eichenberger <stefan.eichenberger@toradex.com> > > 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 <stefan.eichenberger@toradex.com> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> - Mani > --- > 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(-) > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > index 808d1f1054173..c8d5c90aa4d45 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -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, > -- > 2.43.0 >
On Sat, Nov 02, 2024 at 09:04:03PM +0900, Krzysztof Wilczyński wrote: > Hello, > > [...] > > > 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. > LGTM, feel free to pick it up. - Mani
Hello, > > [...] > > > > 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. > > > > LGTM, feel free to pick it up. Sounds good! Thank you! Krzysztof
Hello, > 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 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
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index 808d1f1054173..c8d5c90aa4d45 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c @@ -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,