Re: [PATCH v2] PCI: imx6: Add suspend/resume support for i.MX6QDL

[Frank Li]

On Thu, Oct 10, 2024 at 10:11:21PM +0200, Francesco Dolcini wrote:
> Hello Frank,
>
> On Thu, Oct 10, 2024 at 04:01:21PM -0400, Frank Li wrote:
> > On Wed, Oct 09, 2024 at 03:14:05PM +0200, Stefan Eichenberger wrote:
> > > From: Stefan Eichenberger <stefan.eichenberger@xxxxxxxxxxx>
> > >
> > > The suspend/resume support is broken on the i.MX6QDL platform. This
> > > patch resets the link upon resuming to recover functionality. It shares
> > > most of the sequences with other i.MX devices but does not touch the
> > > critical registers, which might break PCIe. This patch addresses the
> > > same issue as the following downstream commit:
> > > https://github.com/nxp-imx/linux-imx/commit/4e92355e1f79d225ea842511fcfd42b343b32995
> > > In comparison this patch will also reset the device if possible. Without
> > > this patch suspend/resume will not work if a PCIe device is connected.
> > > The kernel will hang on resume and print an error:
> > > ath10k_pci 0000:01:00.0: Unable to change power state from D3hot to D0, device inaccessible
> > > 8<--- cut here ---
> > > Unhandled fault: imprecise external abort (0x1406) at 0x0106f944
> > >
> > > Signed-off-by: Stefan Eichenberger <stefan.eichenberger@xxxxxxxxxxx>
> > > ---
> >
> > Thank you for your patch.
> >
> > But it may conflict with another suspend/resume patch
> > https://lore.kernel.org/imx/1727245477-15961-8-git-send-email-hongxing.zhu@xxxxxxx/
>
> Thanks for the head-up.
>
> Do you see any issue with this patch apart that? Because this patch is
> fixing a crash, so I would expect this to be merged, once ready, and
> such a series rebased afterward.
>
> I am writing this explicitly since you wrote a similar comment on the
> v1 (https://lore.kernel.org/all/ZsNXDq%2FkidZdyhvD@lizhi-Precision-Tower-5810/)
> and I would like to prevent to have this fix starving for long just because
> multiple people is working on the same driver.

My key comment for this patch is use flags IMX_PCIE_FLAG_SKIP_TURN_OFF
in suspend()/resume(), it is up to kw to pick which one firstly.

Frank

>
> Francesco
>