PCI: imx6: Allow to probe when dw_pcie_wait_for_link() fails

PATCH Applied Fabio Estevam 4 participants 6 messages Jan 6, 2022 – Feb 3, 2022

Subsystems (5)

Stable branch Arm i.MX and Layerscape PCI PCI native host bridge and endpoint drivers +1 more subsystems

lore

Share & tools

lore

Fabio Estevam

author reviewer

2022-01-06 10:36 UTC #0 lore raw

PATCH v2 1/1 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:

imx6q-pcie 8ffc000.pcie: Phy link never came up
imx6q-pcie: probe of 8ffc000.pcie failed with error -110

------------[ cut here ]------------
WARNING: CPU: 0 PID: 30 at drivers/regulator/core.c:2257 _regulator_put.part.0+0x1b8/0x1dc
Modules linked in:
CPU: 0 PID: 30 Comm: kworker/u2:2 Not tainted 5.15.0-next-20211103 #1
Hardware name: Freescale i.MX6 SoloX (Device Tree)
Workqueue: events_unbound async_run_entry_fn
[<c0111730>] (unwind_backtrace) from [<c010bb74>] (show_stack+0x10/0x14)
[<c010bb74>] (show_stack) from [<c0f90290>] (dump_stack_lvl+0x58/0x70)
[<c0f90290>] (dump_stack_lvl) from [<c012631c>] (__warn+0xd4/0x154)
[<c012631c>] (__warn) from [<c0f87b00>] (warn_slowpath_fmt+0x74/0xa8)
[<c0f87b00>] (warn_slowpath_fmt) from [<c076b4bc>] (_regulator_put.part.0+0x1b8/0x1dc)
[<c076b4bc>] (_regulator_put.part.0) from [<c076b574>] (regulator_put+0x2c/0x3c)
[<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.

Cc <[email protected]>

Fixes 886a9c134755 ("PCI: dwc: Move link handling into common code")

Signed-off-by Fabio Estevam <[email protected]>

---
Changes since v1:
- Remove the printk timestamp from the kernel warning log (Richard).

 drivers/pci/controller/dwc/pci-imx6.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

drivers/pci/controller/dwc/pci-imx6.c +2 -8

index 2ac081510632..5e8a03061b31 100644
@@ -807,9 +807,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. */

@@ -845,11 +843,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");
 	}

Signature · 1 line

2.25.1

Hongxing Zhu

maintainer

2022-01-07 03:12 UTC #1 lore raw

Quoted 42 lines

-----Original Message-----
From: Fabio Estevam <[email protected]>
Sent: Thursday, January 6, 2022 6:37 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; Hongxing Zhu <[email protected]>; [email protected]; Fabio Estevam <[email protected]>; [email protected]
Subject: [PATCH v2] 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:
imx6q-pcie 8ffc000.pcie: Phy link never came up
imx6q-pcie: probe of 8ffc000.pcie failed with error -110 ------------[ cut here ]------------
WARNING: CPU: 0 PID: 30 at drivers/regulator/core.c:2257
_regulator_put.part.0+0x1b8/0x1dc Modules linked in:
CPU: 0 PID: 30 Comm: kworker/u2:2 Not tainted 5.15.0-next-20211103
#1 Hardware name: Freescale i.MX6 SoloX (Device Tree)
Workqueue: events_unbound async_run_entry_fn [<c0111730>]
(unwind_backtrace) from [<c010bb74>] (show_stack+0x10/0x14)
[<c010bb74>] (show_stack) from [<c0f90290>] (dump_stack_lvl+0x58/0x70) [<c0f90290>] (dump_stack_lvl) from [<c012631c>] (__warn+0xd4/0x154) [<c012631c>] (__warn) from [<c0f87b00>] (warn_slowpath_fmt+0x74/0xa8) [<c0f87b00>] (warn_slowpath_fmt) from [<c076b4bc>] (_regulator_put.part.0+0x1b8/0x1dc)
[<c076b4bc>] (_regulator_put.part.0) from [<c076b574>] (regulator_put+0x2c/0x3c) [<c076b574>] (regulator_put) from [<c08c3740>] (release_nodes+0x50/0x178)

Quoted 8 lines

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.
Cc <[email protected]>
Fixes 886a9c134755 ("PCI: dwc: Move link handling into common code")
Signed-off-by Fabio Estevam <[email protected]>

[Richard Zhu] Reviewed-by: Richard Zhu <[email protected]> Thanks.

Best Regards
Richard Zhu

Quoted 39 lines

---
Changes since v1:
- Remove the printk timestamp from the kernel warning log (Richard).

 drivers/pci/controller/dwc/pci-imx6.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

drivers/pci/controller/dwc/pci-imx6.c +1 -3

index 2ac081510632..5e8a03061b31 100644
@@ -807,9 +807,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. */ @@ -845,11 +843,7
@@ static int imx6_pcie_start_link(struct dw_pcie *pci)

drivers/pci/controller/dwc/pci-imx6.c +1 -6

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

Fabio Estevam

author reviewer

2022-01-31 13:08 UTC #2 lore raw

Quoted 46 lines · Hongxing Zhu

Quoted 46 lines
-----Original Message-----
From: Fabio Estevam <[email protected]>
Sent: Thursday, January 6, 2022 6:37 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; Hongxing Zhu <[email protected]>; [email protected]; Fabio Estevam <[email protected]>; [email protected]
Subject: [PATCH v2] 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:
imx6q-pcie 8ffc000.pcie: Phy link never came up
imx6q-pcie: probe of 8ffc000.pcie failed with error -110 ------------[ cut here ]------------
WARNING: CPU: 0 PID: 30 at drivers/regulator/core.c:2257
_regulator_put.part.0+0x1b8/0x1dc Modules linked in:
CPU: 0 PID: 30 Comm: kworker/u2:2 Not tainted 5.15.0-next-20211103
#1 Hardware name: Freescale i.MX6 SoloX (Device Tree)
Workqueue: events_unbound async_run_entry_fn [<c0111730>]
(unwind_backtrace) from [<c010bb74>] (show_stack+0x10/0x14)
[<c010bb74>] (show_stack) from [<c0f90290>] (dump_stack_lvl+0x58/0x70) [<c0f90290>] (dump_stack_lvl) from [<c012631c>] (__warn+0xd4/0x154) [<c012631c>] (__warn) from [<c0f87b00>] (warn_slowpath_fmt+0x74/0xa8) [<c0f87b00>] (warn_slowpath_fmt) from [<c076b4bc>] (_regulator_put.part.0+0x1b8/0x1dc)
[<c076b4bc>] (_regulator_put.part.0) from [<c076b574>] (regulator_put+0x2c/0x3c) [<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.

Quoted 6 lines

Quoted 5 lines
Tested on imx6sx-sdb and imx6q-sabresd boards.
Cc <[email protected]>
Fixes 886a9c134755 ("PCI: dwc: Move link handling into common code")
Signed-off-by Fabio Estevam <[email protected]>
[Richard Zhu] Reviewed-by: Richard Zhu <[email protected]>

A gentle ping on this one.

Lorenzo Pieralisi

2022-02-03 17:10 UTC #3 lore raw

In reply to Hongxing Zhu

Quoted 45 lines · Hongxing Zhu

Quoted 45 lines
-----Original Message-----
From: Fabio Estevam <[email protected]>
Sent: Thursday, January 6, 2022 6:37 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; Hongxing Zhu <[email protected]>; [email protected]; Fabio Estevam <[email protected]>; [email protected]
Subject: [PATCH v2] 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:
imx6q-pcie 8ffc000.pcie: Phy link never came up
imx6q-pcie: probe of 8ffc000.pcie failed with error -110 ------------[ cut here ]------------
WARNING: CPU: 0 PID: 30 at drivers/regulator/core.c:2257
_regulator_put.part.0+0x1b8/0x1dc Modules linked in:
CPU: 0 PID: 30 Comm: kworker/u2:2 Not tainted 5.15.0-next-20211103
#1 Hardware name: Freescale i.MX6 SoloX (Device Tree)
Workqueue: events_unbound async_run_entry_fn [<c0111730>]
(unwind_backtrace) from [<c010bb74>] (show_stack+0x10/0x14)
[<c010bb74>] (show_stack) from [<c0f90290>] (dump_stack_lvl+0x58/0x70) [<c0f90290>] (dump_stack_lvl) from [<c012631c>] (__warn+0xd4/0x154) [<c012631c>] (__warn) from [<c0f87b00>] (warn_slowpath_fmt+0x74/0xa8) [<c0f87b00>] (warn_slowpath_fmt) from [<c076b4bc>] (_regulator_put.part.0+0x1b8/0x1dc)
[<c076b4bc>] (_regulator_put.part.0) from [<c076b574>] (regulator_put+0x2c/0x3c) [<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.

Quoted 6 lines

Quoted 5 lines
Tested on imx6sx-sdb and imx6q-sabresd boards.
Cc <[email protected]>
Fixes 886a9c134755 ("PCI: dwc: Move link handling into common code")
Signed-off-by Fabio Estevam <[email protected]>
[Richard Zhu] Reviewed-by: Richard Zhu <[email protected]>

Please, there is no need to add [Richard Zhu] to let us understand it is you, this breaks tools that pick up tags automatically so refrain from using it, thanks.

Lorenzo

Quoted 39 lines

Quoted 38 lines

Changes since v1:
- Remove the printk timestamp from the kernel warning log (Richard).

 drivers/pci/controller/dwc/pci-imx6.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

drivers/pci/controller/dwc/pci-imx6.c +1 -3

index 2ac081510632..5e8a03061b31 100644
@@ -807,9 +807,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. */ @@ -845,11 +843,7
@@ static int imx6_pcie_start_link(struct dw_pcie *pci)

drivers/pci/controller/dwc/pci-imx6.c +1 -6

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

Rob Herring

reviewer Reviewed-by on v2 1/1

2022-02-01 13:16 UTC #4 lore raw

In reply to Fabio Estevam

Quoted 37 lines · 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:
imx6q-pcie 8ffc000.pcie: Phy link never came up
imx6q-pcie: probe of 8ffc000.pcie failed with error -110
------------[ cut here ]------------
WARNING: CPU: 0 PID: 30 at drivers/regulator/core.c:2257 _regulator_put.part.0+0x1b8/0x1dc
Modules linked in:
CPU: 0 PID: 30 Comm: kworker/u2:2 Not tainted 5.15.0-next-20211103 #1
Hardware name: Freescale i.MX6 SoloX (Device Tree)
Workqueue: events_unbound async_run_entry_fn
[<c0111730>] (unwind_backtrace) from [<c010bb74>] (show_stack+0x10/0x14)
[<c010bb74>] (show_stack) from [<c0f90290>] (dump_stack_lvl+0x58/0x70)
[<c0f90290>] (dump_stack_lvl) from [<c012631c>] (__warn+0xd4/0x154)
[<c012631c>] (__warn) from [<c0f87b00>] (warn_slowpath_fmt+0x74/0xa8)
[<c0f87b00>] (warn_slowpath_fmt) from [<c076b4bc>] (_regulator_put.part.0+0x1b8/0x1dc)
[<c076b4bc>] (_regulator_put.part.0) from [<c076b574>] (regulator_put+0x2c/0x3c)
[<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.
Cc <[email protected]>
Fixes 886a9c134755 ("PCI: dwc: Move link handling into common code")
Signed-off-by Fabio Estevam <[email protected]>

Quoted 6 lines

---
Changes since v1:
- Remove the printk timestamp from the kernel warning log (Richard).
 drivers/pci/controller/dwc/pci-imx6.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

Reviewed-by Rob Herring <[email protected]>

Lorenzo Pieralisi

Applied

2022-02-03 17:16 UTC #5 lore raw

In reply to Fabio Estevam

Quoted 10 lines · 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."
[...]

Applied to pci/imx6, thanks!

[1/1] PCI: imx6: Allow to probe when dw_pcie_wait_for_link() fails

      https://git.kernel.org/lpieralisi/pci/c/f81f095e87

Thanks,
Lorenzo