linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] PCI: imx6: Allow to probe when dw_pcie_wait_for_link() fails
@ 2021-11-04  0:02 Fabio Estevam
  2021-11-04  0:58 ` Hongxing Zhu
  0 siblings, 1 reply; 6+ messages in thread
From: Fabio Estevam @ 2021-11-04  0:02 UTC (permalink / raw)
  To: hongxing.zhu
  Cc: l.stach, lorenzo.pieralisi, robh, bhelgaas, linux-pci, Fabio Estevam

The intention of commit 886a9c134755 ("PCI: dwc: Move link handling into
common code") was to standardize the behavior of link down as explained
in its commit log:

"The behavior for a link down was inconsistent as some drivers would fail
probe in that case while others succeed. Let's standardize this to
succeed as there are usecases where devices (and the link) appear later
even without hotplug. For example, a reconfigured FPGA device."

The pci-imx6 still fails to probe when the link is not present, which
causes the following warning:

[    9.199175] imx6q-pcie 8ffc000.pcie: Phy link never came up
[    9.208701] imx6q-pcie: probe of 8ffc000.pcie failed with error -110
[    9.216214] ------------[ cut here ]------------
[    9.222057] WARNING: CPU: 0 PID: 30 at drivers/regulator/core.c:2257 _regulator_put.part.0+0x1b8/0x1dc
[    9.232411] Modules linked in:
[    9.236201] CPU: 0 PID: 30 Comm: kworker/u2:2 Not tainted 5.15.0-next-20211103 #1
[    9.244086] Hardware name: Freescale i.MX6 SoloX (Device Tree)
[    9.250284] Workqueue: events_unbound async_run_entry_fn
[    9.256067] [<c0111730>] (unwind_backtrace) from [<c010bb74>] (show_stack+0x10/0x14)
[    9.264258] [<c010bb74>] (show_stack) from [<c0f90290>] (dump_stack_lvl+0x58/0x70)
[    9.272245] [<c0f90290>] (dump_stack_lvl) from [<c012631c>] (__warn+0xd4/0x154)
[    9.279969] [<c012631c>] (__warn) from [<c0f87b00>] (warn_slowpath_fmt+0x74/0xa8)
[    9.287882] [<c0f87b00>] (warn_slowpath_fmt) from [<c076b4bc>] (_regulator_put.part.0+0x1b8/0x1dc)
[    9.297273] [<c076b4bc>] (_regulator_put.part.0) from [<c076b574>] (regulator_put+0x2c/0x3c)
[    9.306122] [<c076b574>] (regulator_put) from [<c08c3740>] (release_nodes+0x50/0x178)

Fix this problem by ignoring the dw_pcie_wait_for_link() error like
it is done on the other dwc drivers.

Tested on imx6sx-sdb and imx6q-sabresd boards.

Fixes: 886a9c134755 ("PCI: dwc: Move link handling into common code")
Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 26f49f797b0f..bbc3a46549f8 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -779,9 +779,7 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
 	/* Start LTSSM. */
 	imx6_pcie_ltssm_enable(dev);
 
-	ret = dw_pcie_wait_for_link(pci);
-	if (ret)
-		goto err_reset_phy;
+	dw_pcie_wait_for_link(pci);
 
 	if (pci->link_gen == 2) {
 		/* Allow Gen2 mode after the link is up. */
@@ -817,11 +815,7 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
 		}
 
 		/* Make sure link training is finished as well! */
-		ret = dw_pcie_wait_for_link(pci);
-		if (ret) {
-			dev_err(dev, "Failed to bring link up!\n");
-			goto err_reset_phy;
-		}
+		dw_pcie_wait_for_link(pci);
 	} else {
 		dev_info(dev, "Link: Gen2 disabled\n");
 	}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH] PCI: imx6: Allow to probe when dw_pcie_wait_for_link() fails
  2021-11-04  0:02 [PATCH] PCI: imx6: Allow to probe when dw_pcie_wait_for_link() fails Fabio Estevam
@ 2021-11-04  0:58 ` Hongxing Zhu
  2021-11-04  1:44   ` Fabio Estevam
  2021-12-04 15:34   ` Fabio Estevam
  0 siblings, 2 replies; 6+ messages in thread
From: Hongxing Zhu @ 2021-11-04  0:58 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: l.stach, lorenzo.pieralisi, robh, bhelgaas, linux-pci


> -----Original Message-----
> From: Fabio Estevam <festevam@gmail.com>
> Sent: Thursday, November 4, 2021 8:02 AM
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: l.stach@pengutronix.de; lorenzo.pieralisi@arm.com;
> robh@kernel.org; bhelgaas@google.com; linux-pci@vger.kernel.org;
> Fabio Estevam <festevam@gmail.com>
> Subject: [PATCH] PCI: imx6: Allow to probe when dw_pcie_wait_for_link()
> fails
> 
> The intention of commit 886a9c134755 ("PCI: dwc: Move link handling
> into common code") was to standardize the behavior of link down as
> explained in its commit log:
> 
> "The behavior for a link down was inconsistent as some drivers would fail
> probe in that case while others succeed. Let's standardize this to succeed
> as there are usecases where devices (and the link) appear later even
> without hotplug. For example, a reconfigured FPGA device."
> 
> The pci-imx6 still fails to probe when the link is not present, which causes
> the following warning:
> 
> [    9.199175] imx6q-pcie 8ffc000.pcie: Phy link never came up
> [    9.208701] imx6q-pcie: probe of 8ffc000.pcie failed with error -110
> [    9.216214] ------------[ cut here ]------------
> [    9.222057] WARNING: CPU: 0 PID: 30 at
> drivers/regulator/core.c:2257 _regulator_put.part.0+0x1b8/0x1dc
> [    9.232411] Modules linked in:
> [    9.236201] CPU: 0 PID: 30 Comm: kworker/u2:2 Not tainted
> 5.15.0-next-20211103 #1
> [    9.244086] Hardware name: Freescale i.MX6 SoloX (Device Tree)
> [    9.250284] Workqueue: events_unbound async_run_entry_fn
> [    9.256067] [<c0111730>] (unwind_backtrace) from [<c010bb74>]
> (show_stack+0x10/0x14)
> [    9.264258] [<c010bb74>] (show_stack) from [<c0f90290>]
> (dump_stack_lvl+0x58/0x70)
> [    9.272245] [<c0f90290>] (dump_stack_lvl) from [<c012631c>]
> (__warn+0xd4/0x154)
> [    9.279969] [<c012631c>] (__warn) from [<c0f87b00>]
> (warn_slowpath_fmt+0x74/0xa8)
> [    9.287882] [<c0f87b00>] (warn_slowpath_fmt) from [<c076b4bc>]
> (_regulator_put.part.0+0x1b8/0x1dc)
> [    9.297273] [<c076b4bc>] (_regulator_put.part.0) from [<c076b574>]
> (regulator_put+0x2c/0x3c)
> [    9.306122] [<c076b574>] (regulator_put) from [<c08c3740>]
> (release_nodes+0x50/0x178)
> 
> Fix this problem by ignoring the dw_pcie_wait_for_link() error like it is
> done on the other dwc drivers.
> 
> Tested on imx6sx-sdb and imx6q-sabresd boards.
> 
> Fixes: 886a9c134755 ("PCI: dwc: Move link handling into common code")
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
[Richard Zhu] Hi Fabio:
First of all, thanks for your help to care this bug.
This dump is planned to be fixed in the #5 patch of
 '[v4,0/6] PCI: imx6: refine codes and add compliance tests mode support'
"https://patchwork.kernel.org/project/linux-arm-kernel/cover/1635747478-25562-1-git-send-email-hongxing.zhu@nxp.com/"

As we know that the hot-plug is not supported by i.MX PCIe. i.MX PCIe would
not have any chances to be functional after one link down.

To save power consumption as much as possible, we should do the error
 handling after link is down.

Ignore the the dw_pcie_wait_for_link() error, PCIe would keep consuming
 the power, this is not friendly for the system power saving.

Best Regards
Richard Zhu
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> b/drivers/pci/controller/dwc/pci-imx6.c
> index 26f49f797b0f..bbc3a46549f8 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -779,9 +779,7 @@ static int imx6_pcie_start_link(struct dw_pcie
> *pci)
>  	/* Start LTSSM. */
>  	imx6_pcie_ltssm_enable(dev);
> 
> -	ret = dw_pcie_wait_for_link(pci);
> -	if (ret)
> -		goto err_reset_phy;
> +	dw_pcie_wait_for_link(pci);
> 
>  	if (pci->link_gen == 2) {
>  		/* Allow Gen2 mode after the link is up. */ @@ -817,11 +815,7
> @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
>  		}
> 
>  		/* Make sure link training is finished as well! */
> -		ret = dw_pcie_wait_for_link(pci);
> -		if (ret) {
> -			dev_err(dev, "Failed to bring link up!\n");
> -			goto err_reset_phy;
> -		}
> +		dw_pcie_wait_for_link(pci);
>  	} else {
>  		dev_info(dev, "Link: Gen2 disabled\n");
>  	}
> --
> 2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] PCI: imx6: Allow to probe when dw_pcie_wait_for_link() fails
  2021-11-04  0:58 ` Hongxing Zhu
@ 2021-11-04  1:44   ` Fabio Estevam
  2021-11-05  8:18     ` Hongxing Zhu
  2021-12-04 15:34   ` Fabio Estevam
  1 sibling, 1 reply; 6+ messages in thread
From: Fabio Estevam @ 2021-11-04  1:44 UTC (permalink / raw)
  To: Hongxing Zhu; +Cc: l.stach, lorenzo.pieralisi, robh, bhelgaas, linux-pci

Hi Richard,

On Wed, Nov 3, 2021 at 9:58 PM Hongxing Zhu <hongxing.zhu@nxp.com> wrote:

> [Richard Zhu] Hi Fabio:
> First of all, thanks for your help to care this bug.
> This dump is planned to be fixed in the #5 patch of
>  '[v4,0/6] PCI: imx6: refine codes and add compliance tests mode support'
> "https://patchwork.kernel.org/project/linux-arm-kernel/cover/1635747478-25562-1-git-send-email-hongxing.zhu@nxp.com/"

Ok, great. Since this patch 5/6 is a bug fix, could you re-order this
series so that the bug fix is the first one in the series?

This makes it easier for the backport to stable.

Patch 1/6 is just a cleanup and could go further.

Thanks

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] PCI: imx6: Allow to probe when dw_pcie_wait_for_link() fails
  2021-11-04  1:44   ` Fabio Estevam
@ 2021-11-05  8:18     ` Hongxing Zhu
  0 siblings, 0 replies; 6+ messages in thread
From: Hongxing Zhu @ 2021-11-05  8:18 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: l.stach, lorenzo.pieralisi, robh, bhelgaas, linux-pci

> -----Original Message-----
> From: Fabio Estevam <festevam@gmail.com>
> Sent: Thursday, November 4, 2021 9:45 AM
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: l.stach@pengutronix.de; lorenzo.pieralisi@arm.com;
> robh@kernel.org; bhelgaas@google.com; linux-pci@vger.kernel.org
> Subject: Re: [PATCH] PCI: imx6: Allow to probe when
> dw_pcie_wait_for_link() fails
> 
> Hi Richard,
> 
> On Wed, Nov 3, 2021 at 9:58 PM Hongxing Zhu <hongxing.zhu@nxp.com>
> wrote:
> 
> > [Richard Zhu] Hi Fabio:
> > First of all, thanks for your help to care this bug.
> > This dump is planned to be fixed in the #5 patch of  '[v4,0/6] PCI:
> > imx6: refine codes and add compliance tests mode support'
> >
> "https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> atchwork.kernel.org%2Fproject%2Flinux-arm-kernel%2Fcover%2F163574
> 7478-25562-1-git-send-email-hongxing.zhu%40nxp.com%2F&amp;data=
> 04%7C01%7Chongxing.zhu%40nxp.com%7Ca96f456b0a004a4da69008d9
> 9f34bb8b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C6377
> 15871101762967%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;
> sdata=bZInvFrlHI8VmWiIS2oxfraSCFAEhWmPG%2BtHLwNHD7A%3D&am
> p;reserved=0"
> 
> Ok, great. Since this patch 5/6 is a bug fix, could you re-order this series so
> that the bug fix is the first one in the series?
> 
> This makes it easier for the backport to stable.
> 
> Patch 1/6 is just a cleanup and could go further.
[Richard Zhu] Hi Fabio: The #5 patch relies on the changes of #2-4 patches.
And the #1 patch make the changes of #2 easier.
It's better to keep the sequence of the v4 series.

BR
Richard

> 
> Thanks

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] PCI: imx6: Allow to probe when dw_pcie_wait_for_link() fails
  2021-11-04  0:58 ` Hongxing Zhu
  2021-11-04  1:44   ` Fabio Estevam
@ 2021-12-04 15:34   ` Fabio Estevam
  2021-12-06  2:03     ` Hongxing Zhu
  1 sibling, 1 reply; 6+ messages in thread
From: Fabio Estevam @ 2021-12-04 15:34 UTC (permalink / raw)
  To: Hongxing Zhu; +Cc: l.stach, lorenzo.pieralisi, robh, bhelgaas, linux-pci

Hi Richard,

On Wed, Nov 3, 2021 at 9:58 PM Hongxing Zhu <hongxing.zhu@nxp.com> wrote:

> [Richard Zhu] Hi Fabio:
> First of all, thanks for your help to care this bug.
> This dump is planned to be fixed in the #5 patch of
>  '[v4,0/6] PCI: imx6: refine codes and add compliance tests mode support'
> "https://patchwork.kernel.org/project/linux-arm-kernel/cover/1635747478-25562-1-git-send-email-hongxing.zhu@nxp.com/"

Any progress with this?

Since this problem affects the stable kernels, would your series be
applied to the stable tree too?

Could you please resend your series so we can get this issue resolved?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] PCI: imx6: Allow to probe when dw_pcie_wait_for_link() fails
  2021-12-04 15:34   ` Fabio Estevam
@ 2021-12-06  2:03     ` Hongxing Zhu
  0 siblings, 0 replies; 6+ messages in thread
From: Hongxing Zhu @ 2021-12-06  2:03 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: l.stach, lorenzo.pieralisi, robh, bhelgaas, linux-pci


> -----Original Message-----
> From: Fabio Estevam <festevam@gmail.com>
> Sent: Saturday, December 4, 2021 11:35 PM
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: l.stach@pengutronix.de; lorenzo.pieralisi@arm.com;
> robh@kernel.org; bhelgaas@google.com; linux-pci@vger.kernel.org
> Subject: Re: [PATCH] PCI: imx6: Allow to probe when
> dw_pcie_wait_for_link() fails
> 
> Hi Richard,
> 
> On Wed, Nov 3, 2021 at 9:58 PM Hongxing Zhu <hongxing.zhu@nxp.com>
> wrote:
> 
> > [Richard Zhu] Hi Fabio:
> > First of all, thanks for your help to care this bug.
> > This dump is planned to be fixed in the #5 patch of  '[v4,0/6] PCI:
> > imx6: refine codes and add compliance tests mode support'
> >
> "https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> atchwork.kernel.org%2Fproject%2Flinux-arm-kernel%2Fcover%2F163574
> 7478-25562-1-git-send-email-hongxing.zhu%40nxp.com%2F&amp;data=
> 04%7C01%7Chongxing.zhu%40nxp.com%7C5ead6c09df204e65988908d9
> b73ba44d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637
> 742289061098040%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&am
> p;sdata=%2FbJokqkG1jyolTO1%2FmZhxLhmzcBKUQqU4fNx%2BoKnhGg%
> 3D&amp;reserved=0"
> 
> Any progress with this?
> 
> Since this problem affects the stable kernels, would your series be applied
> to the stable tree too?
> 
> Could you please resend your series so we can get this issue resolved?
[Richard Zhu] I'm glad if this series can be applied to the stable tree too.
And I can re-send this patch-set again include the stable kernel tree.

Unfortunately, I can't get contact and response from Lucas for a while.
Without his ack, I'm afraid that this series wouldn't be merged.
Lucas might be on his vocation and limited to access the email.
I will ping him from time to time. 

Best Regards
Richard

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-12-06  2:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-04  0:02 [PATCH] PCI: imx6: Allow to probe when dw_pcie_wait_for_link() fails Fabio Estevam
2021-11-04  0:58 ` Hongxing Zhu
2021-11-04  1:44   ` Fabio Estevam
2021-11-05  8:18     ` Hongxing Zhu
2021-12-04 15:34   ` Fabio Estevam
2021-12-06  2:03     ` Hongxing Zhu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).