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

Stefan Eichenberger posted 1 patch 1 month, 2 weeks ago
There is a newer version of this series
drivers/pci/controller/dwc/pci-imx6.c | 44 +++++++++++++++++++++++++--
1 file changed, 41 insertions(+), 3 deletions(-)
[PATCH v2] PCI: imx6: Add suspend/resume support for i.MX6QDL
Posted by Stefan Eichenberger 1 month, 2 weeks ago
From: Stefan Eichenberger <stefan.eichenberger@toradex.com>

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@toradex.com>
---
v1 -> v2: Share most code with other i.MX platforms and set suspend
	  support flag for i.MX6QDL. Version 1 can be found here:
	  https://lore.kernel.org/all/20240819090428.17349-1-eichest@gmail.com/

 drivers/pci/controller/dwc/pci-imx6.c | 44 +++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 808d1f1054173..f33bef0aa1071 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1238,8 +1238,23 @@ static int imx_pcie_suspend_noirq(struct device *dev)
 
 	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);
+	/*
+	 * Do not turn off the PCIe controller because of ERR003756, ERR004490, ERR005188,
+	 * they all document issues with LLTSSM and the PCIe controller which
+	 * does not come out of reset properly. Therefore, try to keep the controller enabled
+	 * and only reset the link. However, the reference clock still needs to be turned off,
+	 * else the controller will freeze on resume.
+	 */
+	if (imx_pcie->drvdata->variant == IMX6Q) {
+		/* Reset the PCIe device */
+		gpiod_set_value_cansleep(imx_pcie->reset_gpiod, 1);
+
+		if (imx_pcie->drvdata->enable_ref_clk)
+			imx_pcie->drvdata->enable_ref_clk(imx_pcie, false);
+	} else {
+		imx_pcie_stop_link(imx_pcie->pci);
+		imx_pcie_host_exit(pp);
+	}
 
 	return 0;
 }
@@ -1253,6 +1268,28 @@ static int imx_pcie_resume_noirq(struct device *dev)
 	if (!(imx_pcie->drvdata->flags & IMX_PCIE_FLAG_SUPPORTS_SUSPEND))
 		return 0;
 
+	/*
+	 * Even though the i.MX6Q does not support proper 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 (imx_pcie->drvdata->variant == IMX6Q) {
+		if (imx_pcie->drvdata->enable_ref_clk)
+			imx_pcie->drvdata->enable_ref_clk(imx_pcie, true);
+
+		imx_pcie_deassert_core_reset(imx_pcie);
+
+		/*
+		 * 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(&imx_pcie->pci->pp);
+		imx_pcie_msi_save_restore(imx_pcie, false);
+
+		return 0;
+	}
+
 	ret = imx_pcie_host_init(pp);
 	if (ret)
 		return ret;
@@ -1485,7 +1522,8 @@ 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_SUPPORTS_SUSPEND,
 		.dbi_length = 0x200,
 		.gpr = "fsl,imx6q-iomuxc-gpr",
 		.clk_names = imx6q_clks,
-- 
2.43.0
Re: [PATCH v2] PCI: imx6: Add suspend/resume support for i.MX6QDL
Posted by Manivannan Sadhasivam 1 month, 2 weeks ago
On Wed, Oct 09, 2024 at 03:14:05PM +0200, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> 
> The suspend/resume support is broken on the i.MX6QDL platform. This

You mean the 'system suspend/resume'?

> 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

Looks like the device is turned off during suspend.

> 8<--- cut here ---
> Unhandled fault: imprecise external abort (0x1406) at 0x0106f944
> 
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> ---
> v1 -> v2: Share most code with other i.MX platforms and set suspend
> 	  support flag for i.MX6QDL. Version 1 can be found here:
> 	  https://lore.kernel.org/all/20240819090428.17349-1-eichest@gmail.com/
> 
>  drivers/pci/controller/dwc/pci-imx6.c | 44 +++++++++++++++++++++++++--
>  1 file changed, 41 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 808d1f1054173..f33bef0aa1071 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1238,8 +1238,23 @@ static int imx_pcie_suspend_noirq(struct device *dev)
>  
>  	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);
> +	/*
> +	 * Do not turn off the PCIe controller because of ERR003756, ERR004490, ERR005188,
> +	 * they all document issues with LLTSSM and the PCIe controller which

LTSSM

But LTSSM is for the PCIe link state, not sure how it impacts controller state.
Can you share the link to those erratums?

> +	 * does not come out of reset properly. Therefore, try to keep the controller enabled
> +	 * and only reset the link. However, the reference clock still needs to be turned off,

You are resetting the *device* below, not the link.

> +	 * else the controller will freeze on resume.
> +	 */

Please use 80 columns for comments. Exception is for the code.

> +	if (imx_pcie->drvdata->variant == IMX6Q) {
> +		/* Reset the PCIe device */
> +		gpiod_set_value_cansleep(imx_pcie->reset_gpiod, 1);
> +
> +		if (imx_pcie->drvdata->enable_ref_clk)
> +			imx_pcie->drvdata->enable_ref_clk(imx_pcie, false);
> +	} else {
> +		imx_pcie_stop_link(imx_pcie->pci);
> +		imx_pcie_host_exit(pp);
> +	}
>  
>  	return 0;
>  }
> @@ -1253,6 +1268,28 @@ static int imx_pcie_resume_noirq(struct device *dev)
>  	if (!(imx_pcie->drvdata->flags & IMX_PCIE_FLAG_SUPPORTS_SUSPEND))
>  		return 0;
>  
> +	/*
> +	 * Even though the i.MX6Q does not support proper 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.
> +	 */

This comment is not really needed.

> +	if (imx_pcie->drvdata->variant == IMX6Q) {
> +		if (imx_pcie->drvdata->enable_ref_clk)
> +			imx_pcie->drvdata->enable_ref_clk(imx_pcie, true);
> +
> +		imx_pcie_deassert_core_reset(imx_pcie);

There is no corresponding imx_pcie_assert_core_reset() in suspend.

> +
> +		/*
> +		 * 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
> +		 */

This indicates that the controller state is not preserved. I think we need a bit
more understanding on what is going on during system suspend on this platform.

- Mani

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH v2] PCI: imx6: Add suspend/resume support for i.MX6QDL
Posted by Stefan Eichenberger 1 month, 2 weeks ago
Hi Mani,

Thanks a lot for the comments.

On Sat, Oct 12, 2024 at 09:43:15AM +0530, Manivannan Sadhasivam wrote:
> On Wed, Oct 09, 2024 at 03:14:05PM +0200, Stefan Eichenberger wrote:
> > From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> > 
> > The suspend/resume support is broken on the i.MX6QDL platform. This
> 
> You mean the 'system suspend/resume'?
> 
> > 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
> 
> Looks like the device is turned off during suspend.

Yes, I don't have a deep understanding about PCIe to be honest. However,
my understanding is that the PCIe devices will always try to suspend and
there is no way from the PCIe host controller to prevent this. Or am I
wrong? Currently suspend is not implemented for the i.MX6Dual/Quad
variant in the pci-imx6 driver and the device will still be turned off
by the PCI subsystem. On the other side I don't think it will fix
anything if I can prevent suspend for the devices because the
communication will still fail after resume.

> 
> > 8<--- cut here ---
> > Unhandled fault: imprecise external abort (0x1406) at 0x0106f944
> > 
> > Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> > ---
> > v1 -> v2: Share most code with other i.MX platforms and set suspend
> > 	  support flag for i.MX6QDL. Version 1 can be found here:
> > 	  https://lore.kernel.org/all/20240819090428.17349-1-eichest@gmail.com/
> > 
> >  drivers/pci/controller/dwc/pci-imx6.c | 44 +++++++++++++++++++++++++--
> >  1 file changed, 41 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > index 808d1f1054173..f33bef0aa1071 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -1238,8 +1238,23 @@ static int imx_pcie_suspend_noirq(struct device *dev)
> >  
> >  	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);
> > +	/*
> > +	 * Do not turn off the PCIe controller because of ERR003756, ERR004490, ERR005188,
> > +	 * they all document issues with LLTSSM and the PCIe controller which
> 
> LTSSM
> 
> But LTSSM is for the PCIe link state, not sure how it impacts controller state.
> Can you share the link to those erratums?

The erratas are all from this document:
https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf

Only the i.MX 6Dual/6Quad variant is affected but not the newer Plus
variants.

> 
> > +	 * does not come out of reset properly. Therefore, try to keep the controller enabled
> > +	 * and only reset the link. However, the reference clock still needs to be turned off,
> 
> You are resetting the *device* below, not the link.

Thanks, I will fix this comment.

> > +	 * else the controller will freeze on resume.
> > +	 */
> 
> Please use 80 columns for comments. Exception is for the code.

Thanks, I will fix this.

> > +	if (imx_pcie->drvdata->variant == IMX6Q) {
> > +		/* Reset the PCIe device */
> > +		gpiod_set_value_cansleep(imx_pcie->reset_gpiod, 1);
> > +
> > +		if (imx_pcie->drvdata->enable_ref_clk)
> > +			imx_pcie->drvdata->enable_ref_clk(imx_pcie, false);
> > +	} else {
> > +		imx_pcie_stop_link(imx_pcie->pci);
> > +		imx_pcie_host_exit(pp);
> > +	}
> >  
> >  	return 0;
> >  }
> > @@ -1253,6 +1268,28 @@ static int imx_pcie_resume_noirq(struct device *dev)
> >  	if (!(imx_pcie->drvdata->flags & IMX_PCIE_FLAG_SUPPORTS_SUSPEND))
> >  		return 0;
> >  
> > +	/*
> > +	 * Even though the i.MX6Q does not support proper 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.
> > +	 */

Thanks, I will fix this.

> 
> > +	if (imx_pcie->drvdata->variant == IMX6Q) {
> > +		if (imx_pcie->drvdata->enable_ref_clk)
> > +			imx_pcie->drvdata->enable_ref_clk(imx_pcie, true);
> > +
> > +		imx_pcie_deassert_core_reset(imx_pcie);
> 
> There is no corresponding imx_pcie_assert_core_reset() in suspend.

Thanks, I will try to fix this similar to what they do in the host_init.

> > +
> > +		/*
> > +		 * 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
> > +		 */
> 
> This indicates that the controller state is not preserved. I think we need a bit
> more understanding on what is going on during system suspend on this platform.

Yes I fully agree, but unfortunately I don't really know how to fix it
properly. I just tried to fix the driver by searching for an absolute
minimum that is required to make suspend/resume work when a PCIe device
is connected. As an alternative the downstream patch would do something
similar but I thought also resetting the device would be a better
solution (to stay closer to what the other variants do):
https://github.com/nxp-imx/linux-imx/commit/4e92355e1f79d225ea842511fcfd42b343b32995
I just now saw that I didn't mention ERR005723 at all, sorry for that.
If this would be a more acceptable solution I would do some more tests
with this patch in our setup.

Regards,
Stefan
Re: [PATCH v2] PCI: imx6: Add suspend/resume support for i.MX6QDL
Posted by Frank Li 1 month, 2 weeks ago
On Wed, Oct 09, 2024 at 03:14:05PM +0200, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>
> 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@toradex.com>
> ---

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@nxp.com/

Frank

> v1 -> v2: Share most code with other i.MX platforms and set suspend
> 	  support flag for i.MX6QDL. Version 1 can be found here:
> 	  https://lore.kernel.org/all/20240819090428.17349-1-eichest@gmail.com/
>
>  drivers/pci/controller/dwc/pci-imx6.c | 44 +++++++++++++++++++++++++--
>  1 file changed, 41 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 808d1f1054173..f33bef0aa1071 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1238,8 +1238,23 @@ static int imx_pcie_suspend_noirq(struct device *dev)
>
>  	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);
> +	/*
> +	 * Do not turn off the PCIe controller because of ERR003756, ERR004490, ERR005188,
> +	 * they all document issues with LLTSSM and the PCIe controller which
> +	 * does not come out of reset properly. Therefore, try to keep the controller enabled
> +	 * and only reset the link. However, the reference clock still needs to be turned off,
> +	 * else the controller will freeze on resume.
> +	 */
> +	if (imx_pcie->drvdata->variant == IMX6Q) {

Add new flag: IMX_PCIE_FLAG_SKIP_TURN_OFF.
use imx_check_flag() here.
Maybe other platform don't want to turn PCIe at some usercase.

> +		/* Reset the PCIe device */
> +		gpiod_set_value_cansleep(imx_pcie->reset_gpiod, 1);
> +
> +		if (imx_pcie->drvdata->enable_ref_clk)
> +			imx_pcie->drvdata->enable_ref_clk(imx_pcie, false);
> +	} else {
> +		imx_pcie_stop_link(imx_pcie->pci);
> +		imx_pcie_host_exit(pp);
> +	}
>
>  	return 0;
>  }
> @@ -1253,6 +1268,28 @@ static int imx_pcie_resume_noirq(struct device *dev)
>  	if (!(imx_pcie->drvdata->flags & IMX_PCIE_FLAG_SUPPORTS_SUSPEND))
>  		return 0;
>
> +	/*
> +	 * Even though the i.MX6Q does not support proper 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 (imx_pcie->drvdata->variant == IMX6Q) {
> +		if (imx_pcie->drvdata->enable_ref_clk)
> +			imx_pcie->drvdata->enable_ref_clk(imx_pcie, true);
> +
> +		imx_pcie_deassert_core_reset(imx_pcie);
> +
> +		/*
> +		 * 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(&imx_pcie->pci->pp);
> +		imx_pcie_msi_save_restore(imx_pcie, false);
> +
> +		return 0;
> +	}
> +
>  	ret = imx_pcie_host_init(pp);
>  	if (ret)
>  		return ret;
> @@ -1485,7 +1522,8 @@ 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_SUPPORTS_SUSPEND,

Add IMX_PCIE_FLAG_SKIP_TURN_OFF here
and move comment "Do not turn off the PCIe" to here.

Frank

>  		.dbi_length = 0x200,
>  		.gpr = "fsl,imx6q-iomuxc-gpr",
>  		.clk_names = imx6q_clks,
> --
> 2.43.0
>
Re: [PATCH v2] PCI: imx6: Add suspend/resume support for i.MX6QDL
Posted by Francesco Dolcini 1 month, 2 weeks ago
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@toradex.com>
> >
> > 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@toradex.com>
> > ---
> 
> 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@nxp.com/

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.

Francesco
Re: [PATCH v2] PCI: imx6: Add suspend/resume support for i.MX6QDL
Posted by Frank Li 1 month, 2 weeks ago
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@toradex.com>
> > >
> > > 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@toradex.com>
> > > ---
> >
> > 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@nxp.com/
>
> 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
>
Re: [PATCH v2] PCI: imx6: Add suspend/resume support for i.MX6QDL
Posted by Stefan Eichenberger 1 month, 2 weeks ago
Hi Frank,

On Thu, Oct 10, 2024 at 06:45:17PM -0400, Frank Li wrote:
> 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@toradex.com>
> > > >
> > > > 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@toradex.com>
> > > > ---
> > >
> > > 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@nxp.com/
> >
> > 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.

I will try to implement it as you proposed with the new flag. 

However, what I figured out with kernel v6.12-rc1 I get the following
warning when booting on an i.MX6QDL even without my patch applied:
[    1.901199] PCI: bus0: Fast back to back transfers disabled
[    1.904754] mmc1: SDHCI controller on 2190000.mmc [2190000.mmc] using ADMA
[    1.904914] mmc2: SDHCI controller on 2194000.mmc [2194000.mmc] using ADMA
[    1.910686] pci 0000:01:00.0: [168c:003c] type 00 class 0x028000 PCIe Endpoint
[    1.918390] NET: Registered PF_PACKET protocol family
[    1.918573] mmc0: SDHCI controller on 2198000.mmc [2198000.mmc] using ADMA
[    1.924322] pci 0000:01:00.0: BAR 0 [mem 0x00000000-0x001fffff 64bit]
[    1.931635] Key type dns_resolver registered
[    1.936764] pci 0000:01:00.0: ROM [mem 0x00000000-0x0000ffff pref]
[    1.961043] pci 0000:01:00.0: supports D1 D2
[    1.961526] Registering SWP/SWPB emulation handler
[    1.965601] mmc0: new DDR MMC card at address 0001
[    1.976575] mmcblk0: mmc0:0001 Q2J54A 3.59 GiB
[    1.979794] Loading compiled-in X.509 certificates
[    1.985036] PCI: bus1: Fast back to back transfers disabled
[    1.991531] pci 0000:00:00.0: bridge window [mem 0x01000000-0x011fffff]: assigned
[    1.998742]  mmcblk0: p1 p2 p3
[    1.999163] pci 0000:00:00.0: BAR 0 [mem 0x01200000-0x012fffff]: assigned
[    2.003947] mmcblk0boot0: mmc0:0001 Q2J54A 16.0 MiB
[    2.008990] pci 0000:00:00.0: bridge window [mem 0x01300000-0x013fffff pref]: assigned
[    2.009023] pci 0000:00:00.0: ROM [mem 0x01400000-0x0140ffff pref]: assigned
[    2.009054] pci 0000:01:00.0: BAR 0 [mem 0x01000000-0x011fffff 64bit]: assigned
[    2.017526] mmcblk0boot1: mmc0:0001 Q2J54A 16.0 MiB
[    2.022015] pci 0000:01:00.0: ROM [mem 0x01300000-0x0130ffff pref]: assigned
[    2.032133] mmcblk0rpmb: mmc0:0001 Q2J54A 512 KiB, chardev (242:0)
[    2.036347] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
[    2.036370] pci 0000:00:00.0:   bridge window [mem 0x01000000-0x011fffff]
[    2.036384] pci 0000:00:00.0:   bridge window [mem 0x01300000-0x013fffff pref]
[    2.042552] pps pps0: new PPS source ptp0
[    2.048338] pci_bus 0000:00: resource 4 [io  0x0000-0xffff]
[    2.083626] pci_bus 0000:00: resource 5 [mem 0x01000000-0x01efffff]
[    2.089972] pci_bus 0000:01: resource 1 [mem 0x01000000-0x011fffff]
[    2.093461] fec 2188000.ethernet eth0: registered PHC device 0
[    2.096283] pci_bus 0000:01: resource 2 [mem 0x01300000-0x013fffff pref]
[    2.096352] sysfs: cannot create duplicate filename '/devices/platform/soc/1ffc000.pcie/pci0000:00/0000:00:00.0/resource0'
[    2.096365] CPU: 3 UID: 0 PID: 52 Comm: kworker/u19:2 Not tainted 6.12.0-rc1 #54
[    2.096381] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[    2.096391] Workqueue: async async_run_entry_fn
[    2.103025] imx_thermal 20c8000.anatop:tempmon: Industrial CPU temperature grade - max:105C critical:100C passive:95C
[    2.108932]
[    2.108940] Call trace:
[    2.108950]  unwind_backtrace from show_stack+0x10/0x14
[    2.121391] clk: Disabling unused clocks
[    2.127423]  show_stack from dump_stack_lvl+0x54/0x68
[    2.127451]  dump_stack_lvl from sysfs_warn_dup+0x58/0x64
[    2.134265] PM: genpd: Disabling unused power domains
[    2.138525]  sysfs_warn_dup from sysfs_add_bin_file_mode_ns+0xbc/0xcc
[    2.138547]  sysfs_add_bin_file_mode_ns from sysfs_create_bin_file+0xac/0xb4
[    2.149242] ALSA device list:
[    2.150647]  sysfs_create_bin_file from pci_create_resource_files+0x84/0x148
[    2.153183]   No soundcards found.
[    2.158407]  pci_create_resource_files from pci_bus_add_device+0x24/0xe4
[    2.211699]  pci_bus_add_device from pci_bus_add_devices+0x2c/0x70
[    2.217930]  pci_bus_add_devices from pci_host_probe+0x7c/0xa4
[    2.223806]  pci_host_probe from dw_pcie_host_init+0x4ec/0x71c
[    2.229682]  dw_pcie_host_init from imx_pcie_probe+0x3a8/0x75c
[    2.235559]  imx_pcie_probe from platform_probe+0x5c/0xb0
[    2.241010]  platform_probe from really_probe+0xd0/0x3a4
[    2.246364]  really_probe from __driver_probe_device+0x8c/0x1d4
[    2.252321]  __driver_probe_device from driver_probe_device+0x30/0xc0
[    2.258803]  driver_probe_device from __driver_attach_async_helper+0x50/0xd8
[    2.265892]  __driver_attach_async_helper from async_run_entry_fn+0x30/0x144
[    2.272980]  async_run_entry_fn from process_one_work+0x154/0x2dc
[    2.279115]  process_one_work from worker_thread+0x250/0x3f0
[    2.284811]  worker_thread from kthread+0x110/0x12c
[    2.289726]  kthread from ret_from_fork+0x14/0x28
[    2.294461] Exception stack(0xe6a0dfb0 to 0xe6a0dff8)
[    2.299535] dfa0:                                     00000000 00000000 00000000 00000000
[    2.307740] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    2.315942] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    2.323156] pcieport 0000:00:00.0: PME: Signaling with IRQ 293
[    2.329645] pcieport 0000:00:00.0: AER: enabled with IRQ 293
[    2.335553] sysfs: cannot create duplicate filename '/devices/platform/soc/1ffc000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/resource0'
[    2.347794] CPU: 3 UID: 0 PID: 52 Comm: kworker/u19:2 Not tainted 6.12.0-rc1 #54
[    2.355229] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[    2.361779] Workqueue: async async_run_entry_fn
[    2.366349] Call trace:
[    2.366362]  unwind_backtrace from show_stack+0x10/0x14
[    2.374183]  show_stack from dump_stack_lvl+0x54/0x68
[    2.379273]  dump_stack_lvl from sysfs_warn_dup+0x58/0x64
[    2.384706]  sysfs_warn_dup from sysfs_add_bin_file_mode_ns+0xbc/0xcc
[    2.391183]  sysfs_add_bin_file_mode_ns from sysfs_create_bin_file+0xac/0xb4
[    2.398270]  sysfs_create_bin_file from pci_create_resource_files+0x84/0x148
[    2.405360]  pci_create_resource_files from pci_bus_add_device+0x24/0xe4
[    2.412113]  pci_bus_add_device from pci_bus_add_devices+0x2c/0x70
[    2.418334]  pci_bus_add_devices from pci_bus_add_devices+0x60/0x70
[    2.424642]  pci_bus_add_devices from pci_host_probe+0x7c/0xa4
[    2.430511]  pci_host_probe from dw_pcie_host_init+0x4ec/0x71c
[    2.436384]  dw_pcie_host_init from imx_pcie_probe+0x3a8/0x75c
[    2.442258]  imx_pcie_probe from platform_probe+0x5c/0xb0
[    2.447704]  platform_probe from really_probe+0xd0/0x3a4
[    2.453057]  really_probe from __driver_probe_device+0x8c/0x1d4
[    2.459014]  __driver_probe_device from driver_probe_device+0x30/0xc0
[    2.465493]  driver_probe_device from __driver_attach_async_helper+0x50/0xd8
[    2.472580]  __driver_attach_async_helper from async_run_entry_fn+0x30/0x144
[    2.479666]  async_run_entry_fn from process_one_work+0x154/0x2dc
[    2.485800]  process_one_work from worker_thread+0x250/0x3f0
[    2.491494]  worker_thread from kthread+0x110/0x12c
[    2.496408]  kthread from ret_from_fork+0x14/0x28
[    2.501140] Exception stack(0xe6a0dfb0 to 0xe6a0dff8)
[    2.506214] dfa0:                                     00000000 00000000 00000000 00000000
[    2.514418] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    2.522620] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000

This was not the case with kernel v6.11, do you have an idea where this
comes from? I did not dig into more detail yet but it looks a bit like a
regression. The driver still works, it just prints this duplicate
filename warning.

Regards,
Stefan
Re: [PATCH v2] PCI: imx6: Add suspend/resume support for i.MX6QDL
Posted by Manivannan Sadhasivam 1 month, 2 weeks ago
On Fri, Oct 11, 2024 at 04:36:21PM +0200, Stefan Eichenberger wrote:

[...]

> However, what I figured out with kernel v6.12-rc1 I get the following
> warning when booting on an i.MX6QDL even without my patch applied:
> [    1.901199] PCI: bus0: Fast back to back transfers disabled
> [    1.904754] mmc1: SDHCI controller on 2190000.mmc [2190000.mmc] using ADMA
> [    1.904914] mmc2: SDHCI controller on 2194000.mmc [2194000.mmc] using ADMA
> [    1.910686] pci 0000:01:00.0: [168c:003c] type 00 class 0x028000 PCIe Endpoint
> [    1.918390] NET: Registered PF_PACKET protocol family
> [    1.918573] mmc0: SDHCI controller on 2198000.mmc [2198000.mmc] using ADMA
> [    1.924322] pci 0000:01:00.0: BAR 0 [mem 0x00000000-0x001fffff 64bit]
> [    1.931635] Key type dns_resolver registered
> [    1.936764] pci 0000:01:00.0: ROM [mem 0x00000000-0x0000ffff pref]
> [    1.961043] pci 0000:01:00.0: supports D1 D2
> [    1.961526] Registering SWP/SWPB emulation handler
> [    1.965601] mmc0: new DDR MMC card at address 0001
> [    1.976575] mmcblk0: mmc0:0001 Q2J54A 3.59 GiB
> [    1.979794] Loading compiled-in X.509 certificates
> [    1.985036] PCI: bus1: Fast back to back transfers disabled
> [    1.991531] pci 0000:00:00.0: bridge window [mem 0x01000000-0x011fffff]: assigned
> [    1.998742]  mmcblk0: p1 p2 p3
> [    1.999163] pci 0000:00:00.0: BAR 0 [mem 0x01200000-0x012fffff]: assigned
> [    2.003947] mmcblk0boot0: mmc0:0001 Q2J54A 16.0 MiB
> [    2.008990] pci 0000:00:00.0: bridge window [mem 0x01300000-0x013fffff pref]: assigned
> [    2.009023] pci 0000:00:00.0: ROM [mem 0x01400000-0x0140ffff pref]: assigned
> [    2.009054] pci 0000:01:00.0: BAR 0 [mem 0x01000000-0x011fffff 64bit]: assigned
> [    2.017526] mmcblk0boot1: mmc0:0001 Q2J54A 16.0 MiB
> [    2.022015] pci 0000:01:00.0: ROM [mem 0x01300000-0x0130ffff pref]: assigned
> [    2.032133] mmcblk0rpmb: mmc0:0001 Q2J54A 512 KiB, chardev (242:0)
> [    2.036347] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
> [    2.036370] pci 0000:00:00.0:   bridge window [mem 0x01000000-0x011fffff]
> [    2.036384] pci 0000:00:00.0:   bridge window [mem 0x01300000-0x013fffff pref]
> [    2.042552] pps pps0: new PPS source ptp0
> [    2.048338] pci_bus 0000:00: resource 4 [io  0x0000-0xffff]
> [    2.083626] pci_bus 0000:00: resource 5 [mem 0x01000000-0x01efffff]
> [    2.089972] pci_bus 0000:01: resource 1 [mem 0x01000000-0x011fffff]
> [    2.093461] fec 2188000.ethernet eth0: registered PHC device 0
> [    2.096283] pci_bus 0000:01: resource 2 [mem 0x01300000-0x013fffff pref]
> [    2.096352] sysfs: cannot create duplicate filename '/devices/platform/soc/1ffc000.pcie/pci0000:00/0000:00:00.0/resource0'
> [    2.096365] CPU: 3 UID: 0 PID: 52 Comm: kworker/u19:2 Not tainted 6.12.0-rc1 #54
> [    2.096381] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [    2.096391] Workqueue: async async_run_entry_fn
> [    2.103025] imx_thermal 20c8000.anatop:tempmon: Industrial CPU temperature grade - max:105C critical:100C passive:95C
> [    2.108932]
> [    2.108940] Call trace:
> [    2.108950]  unwind_backtrace from show_stack+0x10/0x14
> [    2.121391] clk: Disabling unused clocks
> [    2.127423]  show_stack from dump_stack_lvl+0x54/0x68
> [    2.127451]  dump_stack_lvl from sysfs_warn_dup+0x58/0x64
> [    2.134265] PM: genpd: Disabling unused power domains
> [    2.138525]  sysfs_warn_dup from sysfs_add_bin_file_mode_ns+0xbc/0xcc
> [    2.138547]  sysfs_add_bin_file_mode_ns from sysfs_create_bin_file+0xac/0xb4
> [    2.149242] ALSA device list:
> [    2.150647]  sysfs_create_bin_file from pci_create_resource_files+0x84/0x148
> [    2.153183]   No soundcards found.
> [    2.158407]  pci_create_resource_files from pci_bus_add_device+0x24/0xe4
> [    2.211699]  pci_bus_add_device from pci_bus_add_devices+0x2c/0x70
> [    2.217930]  pci_bus_add_devices from pci_host_probe+0x7c/0xa4
> [    2.223806]  pci_host_probe from dw_pcie_host_init+0x4ec/0x71c
> [    2.229682]  dw_pcie_host_init from imx_pcie_probe+0x3a8/0x75c
> [    2.235559]  imx_pcie_probe from platform_probe+0x5c/0xb0
> [    2.241010]  platform_probe from really_probe+0xd0/0x3a4
> [    2.246364]  really_probe from __driver_probe_device+0x8c/0x1d4
> [    2.252321]  __driver_probe_device from driver_probe_device+0x30/0xc0
> [    2.258803]  driver_probe_device from __driver_attach_async_helper+0x50/0xd8
> [    2.265892]  __driver_attach_async_helper from async_run_entry_fn+0x30/0x144
> [    2.272980]  async_run_entry_fn from process_one_work+0x154/0x2dc
> [    2.279115]  process_one_work from worker_thread+0x250/0x3f0
> [    2.284811]  worker_thread from kthread+0x110/0x12c
> [    2.289726]  kthread from ret_from_fork+0x14/0x28
> [    2.294461] Exception stack(0xe6a0dfb0 to 0xe6a0dff8)
> [    2.299535] dfa0:                                     00000000 00000000 00000000 00000000
> [    2.307740] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [    2.315942] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [    2.323156] pcieport 0000:00:00.0: PME: Signaling with IRQ 293
> [    2.329645] pcieport 0000:00:00.0: AER: enabled with IRQ 293
> [    2.335553] sysfs: cannot create duplicate filename '/devices/platform/soc/1ffc000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/resource0'
> [    2.347794] CPU: 3 UID: 0 PID: 52 Comm: kworker/u19:2 Not tainted 6.12.0-rc1 #54
> [    2.355229] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [    2.361779] Workqueue: async async_run_entry_fn
> [    2.366349] Call trace:
> [    2.366362]  unwind_backtrace from show_stack+0x10/0x14
> [    2.374183]  show_stack from dump_stack_lvl+0x54/0x68
> [    2.379273]  dump_stack_lvl from sysfs_warn_dup+0x58/0x64
> [    2.384706]  sysfs_warn_dup from sysfs_add_bin_file_mode_ns+0xbc/0xcc
> [    2.391183]  sysfs_add_bin_file_mode_ns from sysfs_create_bin_file+0xac/0xb4
> [    2.398270]  sysfs_create_bin_file from pci_create_resource_files+0x84/0x148
> [    2.405360]  pci_create_resource_files from pci_bus_add_device+0x24/0xe4
> [    2.412113]  pci_bus_add_device from pci_bus_add_devices+0x2c/0x70
> [    2.418334]  pci_bus_add_devices from pci_bus_add_devices+0x60/0x70
> [    2.424642]  pci_bus_add_devices from pci_host_probe+0x7c/0xa4
> [    2.430511]  pci_host_probe from dw_pcie_host_init+0x4ec/0x71c
> [    2.436384]  dw_pcie_host_init from imx_pcie_probe+0x3a8/0x75c
> [    2.442258]  imx_pcie_probe from platform_probe+0x5c/0xb0
> [    2.447704]  platform_probe from really_probe+0xd0/0x3a4
> [    2.453057]  really_probe from __driver_probe_device+0x8c/0x1d4
> [    2.459014]  __driver_probe_device from driver_probe_device+0x30/0xc0
> [    2.465493]  driver_probe_device from __driver_attach_async_helper+0x50/0xd8
> [    2.472580]  __driver_attach_async_helper from async_run_entry_fn+0x30/0x144
> [    2.479666]  async_run_entry_fn from process_one_work+0x154/0x2dc
> [    2.485800]  process_one_work from worker_thread+0x250/0x3f0
> [    2.491494]  worker_thread from kthread+0x110/0x12c
> [    2.496408]  kthread from ret_from_fork+0x14/0x28
> [    2.501140] Exception stack(0xe6a0dfb0 to 0xe6a0dff8)
> [    2.506214] dfa0:                                     00000000 00000000 00000000 00000000
> [    2.514418] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [    2.522620] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> 
> This was not the case with kernel v6.11, do you have an idea where this
> comes from? I did not dig into more detail yet but it looks a bit like a
> regression. The driver still works, it just prints this duplicate
> filename warning.

Could you please do bisect to find the offending commit?

- Mani

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH v2] PCI: imx6: Add suspend/resume support for i.MX6QDL
Posted by Frank Li 1 month, 2 weeks ago
On Fri, Oct 11, 2024 at 04:36:21PM +0200, Stefan Eichenberger wrote:
> Hi Frank,
>
> On Thu, Oct 10, 2024 at 06:45:17PM -0400, Frank Li wrote:
> > 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@toradex.com>
> > > > >
> > > > > 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@toradex.com>
> > > > > ---
> > > >
> > > > 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@nxp.com/
> > >
> > > 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.
>
> I will try to implement it as you proposed with the new flag.

I have not met this problem at arm64 platform. what's your .config?

Frank

>
> However, what I figured out with kernel v6.12-rc1 I get the following
> warning when booting on an i.MX6QDL even without my patch applied:
> [    1.901199] PCI: bus0: Fast back to back transfers disabled
> [    1.904754] mmc1: SDHCI controller on 2190000.mmc [2190000.mmc] using ADMA
> [    1.904914] mmc2: SDHCI controller on 2194000.mmc [2194000.mmc] using ADMA
> [    1.910686] pci 0000:01:00.0: [168c:003c] type 00 class 0x028000 PCIe Endpoint
> [    1.918390] NET: Registered PF_PACKET protocol family
> [    1.918573] mmc0: SDHCI controller on 2198000.mmc [2198000.mmc] using ADMA
> [    1.924322] pci 0000:01:00.0: BAR 0 [mem 0x00000000-0x001fffff 64bit]
> [    1.931635] Key type dns_resolver registered
> [    1.936764] pci 0000:01:00.0: ROM [mem 0x00000000-0x0000ffff pref]
> [    1.961043] pci 0000:01:00.0: supports D1 D2
> [    1.961526] Registering SWP/SWPB emulation handler
> [    1.965601] mmc0: new DDR MMC card at address 0001
> [    1.976575] mmcblk0: mmc0:0001 Q2J54A 3.59 GiB
> [    1.979794] Loading compiled-in X.509 certificates
> [    1.985036] PCI: bus1: Fast back to back transfers disabled
> [    1.991531] pci 0000:00:00.0: bridge window [mem 0x01000000-0x011fffff]: assigned
> [    1.998742]  mmcblk0: p1 p2 p3
> [    1.999163] pci 0000:00:00.0: BAR 0 [mem 0x01200000-0x012fffff]: assigned
> [    2.003947] mmcblk0boot0: mmc0:0001 Q2J54A 16.0 MiB
> [    2.008990] pci 0000:00:00.0: bridge window [mem 0x01300000-0x013fffff pref]: assigned
> [    2.009023] pci 0000:00:00.0: ROM [mem 0x01400000-0x0140ffff pref]: assigned
> [    2.009054] pci 0000:01:00.0: BAR 0 [mem 0x01000000-0x011fffff 64bit]: assigned
> [    2.017526] mmcblk0boot1: mmc0:0001 Q2J54A 16.0 MiB
> [    2.022015] pci 0000:01:00.0: ROM [mem 0x01300000-0x0130ffff pref]: assigned
> [    2.032133] mmcblk0rpmb: mmc0:0001 Q2J54A 512 KiB, chardev (242:0)
> [    2.036347] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
> [    2.036370] pci 0000:00:00.0:   bridge window [mem 0x01000000-0x011fffff]
> [    2.036384] pci 0000:00:00.0:   bridge window [mem 0x01300000-0x013fffff pref]
> [    2.042552] pps pps0: new PPS source ptp0
> [    2.048338] pci_bus 0000:00: resource 4 [io  0x0000-0xffff]
> [    2.083626] pci_bus 0000:00: resource 5 [mem 0x01000000-0x01efffff]
> [    2.089972] pci_bus 0000:01: resource 1 [mem 0x01000000-0x011fffff]
> [    2.093461] fec 2188000.ethernet eth0: registered PHC device 0
> [    2.096283] pci_bus 0000:01: resource 2 [mem 0x01300000-0x013fffff pref]
> [    2.096352] sysfs: cannot create duplicate filename '/devices/platform/soc/1ffc000.pcie/pci0000:00/0000:00:00.0/resource0'
> [    2.096365] CPU: 3 UID: 0 PID: 52 Comm: kworker/u19:2 Not tainted 6.12.0-rc1 #54
> [    2.096381] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [    2.096391] Workqueue: async async_run_entry_fn
> [    2.103025] imx_thermal 20c8000.anatop:tempmon: Industrial CPU temperature grade - max:105C critical:100C passive:95C
> [    2.108932]
> [    2.108940] Call trace:
> [    2.108950]  unwind_backtrace from show_stack+0x10/0x14
> [    2.121391] clk: Disabling unused clocks
> [    2.127423]  show_stack from dump_stack_lvl+0x54/0x68
> [    2.127451]  dump_stack_lvl from sysfs_warn_dup+0x58/0x64
> [    2.134265] PM: genpd: Disabling unused power domains
> [    2.138525]  sysfs_warn_dup from sysfs_add_bin_file_mode_ns+0xbc/0xcc
> [    2.138547]  sysfs_add_bin_file_mode_ns from sysfs_create_bin_file+0xac/0xb4
> [    2.149242] ALSA device list:
> [    2.150647]  sysfs_create_bin_file from pci_create_resource_files+0x84/0x148
> [    2.153183]   No soundcards found.
> [    2.158407]  pci_create_resource_files from pci_bus_add_device+0x24/0xe4
> [    2.211699]  pci_bus_add_device from pci_bus_add_devices+0x2c/0x70
> [    2.217930]  pci_bus_add_devices from pci_host_probe+0x7c/0xa4
> [    2.223806]  pci_host_probe from dw_pcie_host_init+0x4ec/0x71c
> [    2.229682]  dw_pcie_host_init from imx_pcie_probe+0x3a8/0x75c
> [    2.235559]  imx_pcie_probe from platform_probe+0x5c/0xb0
> [    2.241010]  platform_probe from really_probe+0xd0/0x3a4
> [    2.246364]  really_probe from __driver_probe_device+0x8c/0x1d4
> [    2.252321]  __driver_probe_device from driver_probe_device+0x30/0xc0
> [    2.258803]  driver_probe_device from __driver_attach_async_helper+0x50/0xd8
> [    2.265892]  __driver_attach_async_helper from async_run_entry_fn+0x30/0x144
> [    2.272980]  async_run_entry_fn from process_one_work+0x154/0x2dc
> [    2.279115]  process_one_work from worker_thread+0x250/0x3f0
> [    2.284811]  worker_thread from kthread+0x110/0x12c
> [    2.289726]  kthread from ret_from_fork+0x14/0x28
> [    2.294461] Exception stack(0xe6a0dfb0 to 0xe6a0dff8)
> [    2.299535] dfa0:                                     00000000 00000000 00000000 00000000
> [    2.307740] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [    2.315942] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [    2.323156] pcieport 0000:00:00.0: PME: Signaling with IRQ 293
> [    2.329645] pcieport 0000:00:00.0: AER: enabled with IRQ 293
> [    2.335553] sysfs: cannot create duplicate filename '/devices/platform/soc/1ffc000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/resource0'
> [    2.347794] CPU: 3 UID: 0 PID: 52 Comm: kworker/u19:2 Not tainted 6.12.0-rc1 #54
> [    2.355229] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [    2.361779] Workqueue: async async_run_entry_fn
> [    2.366349] Call trace:
> [    2.366362]  unwind_backtrace from show_stack+0x10/0x14
> [    2.374183]  show_stack from dump_stack_lvl+0x54/0x68
> [    2.379273]  dump_stack_lvl from sysfs_warn_dup+0x58/0x64
> [    2.384706]  sysfs_warn_dup from sysfs_add_bin_file_mode_ns+0xbc/0xcc
> [    2.391183]  sysfs_add_bin_file_mode_ns from sysfs_create_bin_file+0xac/0xb4
> [    2.398270]  sysfs_create_bin_file from pci_create_resource_files+0x84/0x148
> [    2.405360]  pci_create_resource_files from pci_bus_add_device+0x24/0xe4
> [    2.412113]  pci_bus_add_device from pci_bus_add_devices+0x2c/0x70
> [    2.418334]  pci_bus_add_devices from pci_bus_add_devices+0x60/0x70
> [    2.424642]  pci_bus_add_devices from pci_host_probe+0x7c/0xa4
> [    2.430511]  pci_host_probe from dw_pcie_host_init+0x4ec/0x71c
> [    2.436384]  dw_pcie_host_init from imx_pcie_probe+0x3a8/0x75c
> [    2.442258]  imx_pcie_probe from platform_probe+0x5c/0xb0
> [    2.447704]  platform_probe from really_probe+0xd0/0x3a4
> [    2.453057]  really_probe from __driver_probe_device+0x8c/0x1d4
> [    2.459014]  __driver_probe_device from driver_probe_device+0x30/0xc0
> [    2.465493]  driver_probe_device from __driver_attach_async_helper+0x50/0xd8
> [    2.472580]  __driver_attach_async_helper from async_run_entry_fn+0x30/0x144
> [    2.479666]  async_run_entry_fn from process_one_work+0x154/0x2dc
> [    2.485800]  process_one_work from worker_thread+0x250/0x3f0
> [    2.491494]  worker_thread from kthread+0x110/0x12c
> [    2.496408]  kthread from ret_from_fork+0x14/0x28
> [    2.501140] Exception stack(0xe6a0dfb0 to 0xe6a0dff8)
> [    2.506214] dfa0:                                     00000000 00000000 00000000 00000000
> [    2.514418] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [    2.522620] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
>
> This was not the case with kernel v6.11, do you have an idea where this
> comes from? I did not dig into more detail yet but it looks a bit like a
> regression. The driver still works, it just prints this duplicate
> filename warning.
>
> Regards,
> Stefan
Re: [PATCH v2] PCI: imx6: Add suspend/resume support for i.MX6QDL
Posted by Fabio Estevam 1 month, 2 weeks ago
On Fri, Oct 11, 2024 at 12:36 PM Frank Li <Frank.li@nxp.com> wrote:

> I have not met this problem at arm64 platform. what's your .config?

Stefan reported the problem on imx6, so imx_v6_v7_defconfig.