On Mon, Aug 19, 2024 at 11:03:19AM +0200, Stefan Eichenberger wrote: > From: Stefan Eichenberger <stefan.eichenberger@xxxxxxxxxxx> > > According to the https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf errata, Can you show errata number here? > the i.MX6Q PCIe controller does not support suspend/resume. So suspend > and resume was omitted. However, this does not seem to work because it > looks like the PCIe link is still expecting a reset. If we do not reset > the link, we end up with a frozen system after resume. The last message > we see is: > ath10k_pci 0000:01:00.0: Unable to change power state from D3hot to D0, > device inaccessible > > Besides resetting the link, we also need to enable msi again, otherwise > DMA access will not work and we can still end up with a frozen system. > With these changes we can suspend and resume the system properly with a > PCIe device attached. This was tested with a Compex WLE900VX miniPCIe > Wifi module. > > Signed-off-by: Stefan Eichenberger <stefan.eichenberger@xxxxxxxxxxx> > --- > drivers/pci/controller/dwc/pci-imx6.c | 45 ++++++++++++++++++++++++++- > 1 file changed, 44 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > index f17561791e35a..751243f4c519e 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -1213,14 +1213,57 @@ static int imx6_pcie_suspend_noirq(struct device *dev) > return 0; > } > > +static int imx6_pcie_reset_link(struct imx6_pcie *imx6_pcie) > +{ > + int ret; > + > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, > + IMX6Q_GPR1_PCIE_TEST_PD, 1 << 18); > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, > + IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16); > + > + /* Reset the PCIe device */ > + gpiod_set_value_cansleep(imx6_pcie->reset_gpiod, 1); > + > + ret = imx6_pcie_enable_ref_clk(imx6_pcie); > + if (ret) { > + dev_err(imx6_pcie->pci->dev, "unable to enable pcie ref clock\n"); > + return ret; > + } > + > + imx6_pcie_deassert_reset_gpio(imx6_pcie); In my patch https://lore.kernel.org/linux-pci/Zr4XG6r+HnbIlu8S@lizhi-Precision-Tower-5810/T/#mc5f38934b6cef95eca90f1a6a63b3193e45179de imx6qp_pcie_core_reset() and imx6q_pcie_core_reset() is not symatic for assert/desert() to match origin code. I plan fix it after above patch merged. Does it work if make above code symatic? > + > + /* > + * Setup the root complex again and enable msi. Without this PCIe will > + * not work in msi mode and drivers will crash if they try to access > + * the device memory area > + */ > + dw_pcie_setup_rc(&imx6_pcie->pci->pp); > + if (pci_msi_enabled()) { > + u32 val; > + u8 offset = dw_pcie_find_capability(imx6_pcie->pci, PCI_CAP_ID_MSI); > + > + val = dw_pcie_readw_dbi(imx6_pcie->pci, offset + PCI_MSI_FLAGS); > + val |= PCI_MSI_FLAGS_ENABLE; > + dw_pcie_writew_dbi(imx6_pcie->pci, offset + PCI_MSI_FLAGS, val); > + } there are already have imx6_pcie_msi_save_restore(imx6_pcie, true); in suspend/resume, why need addtional one here? > + > + return 0; > +} > + > static int imx6_pcie_resume_noirq(struct device *dev) > { > int ret; > struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev); > struct dw_pcie_rp *pp = &imx6_pcie->pci->pp; > > + /* > + * Even though the i.MX6Q does not support suspend/resume, we need to > + * reset the link after resume or the memory mapped PCIe I/O space will > + * be inaccessible. This will cause the system to freeze. > + */ > if (!(imx6_pcie->drvdata->flags & IMX6_PCIE_FLAG_SUPPORTS_SUSPEND)) > - return 0; > + return imx6_pcie_reset_link(imx6_pcie); If reset everything, I supposed we can add IMX6_PCIE_FLAG_SUPPORTS_SUSPEND at driver data. > > ret = imx6_pcie_host_init(pp); > if (ret) > -- > 2.43.0 >
