| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20241104151202.k6v6fxwyb44n7qw6@thinkpad>
Date: Mon, 4 Nov 2024 20:42:02 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Stefan Eichenberger <eichest@...il.com>
Cc: hongxing.zhu@....com, l.stach@...gutronix.de, lpieralisi@...nel.org,
kw@...ux.com, robh@...nel.org, bhelgaas@...gle.com,
shawnguo@...nel.org, s.hauer@...gutronix.de, kernel@...gutronix.de,
festevam@...il.com, francesco.dolcini@...adex.com, Frank.li@....com,
linux-pci@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
imx@...ts.linux.dev, linux-kernel@...r.kernel.org,
Stefan Eichenberger <stefan.eichenberger@...adex.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@...adex.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@...adex.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.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
>
--
மணிவண்ணன் சதாசிவம்
Powered by blists - more mailing lists