pci: mx6: fix occasional link failures

PATCH New Tim Harvey 4 participants 10 messages Aug 8, 2014 – Aug 20, 2014

Subsystems (2)

Arm i.MX and Layerscape PCI

lore

Share & tools

lore

Tim Harvey

author

2014-08-08 05:57 UTC #0 lore raw

PATCH 1/1 pci: mx6: fix occasional link failures

According to the IMX6 reference manuals, REF_SSP_EN (Reference clock enable for SS function) must remain deasserted until the reference clock is running at the appropriate frequency.

Without this patch we find a high link failure rate (>5%) on certain IMX6 boards at various temperatures.

Signed-off-by Tim Harvey <[email protected]>

---
 drivers/pci/pcie_imx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/pci/pcie_imx.c +4 -4

index c48737e..a3982c4 100644
@@ -509,10 +509,6 @@ static int imx6_pcie_deassert_core_reset(void)
 
 	imx6_pcie_toggle_power();
 
-	/* Enable PCIe */
-	clrbits_le32(&iomuxc_regs->gpr[1], IOMUXC_GPR1_TEST_POWERDOWN);
-	setbits_le32(&iomuxc_regs->gpr[1], IOMUXC_GPR1_REF_SSP_EN);
-
 	enable_pcie_clock();
 
 	/*

@@ -521,6 +517,10 @@ static int imx6_pcie_deassert_core_reset(void)
 	 */
 	mdelay(50);
 
+	/* Enable PCIe */
+	clrbits_le32(&iomuxc_regs->gpr[1], IOMUXC_GPR1_TEST_POWERDOWN);
+	setbits_le32(&iomuxc_regs->gpr[1], IOMUXC_GPR1_REF_SSP_EN);
+
 	imx6_pcie_toggle_reset();
 
 	return 0;

Signature · 1 line

1.8.3.2

Marek Vasut

2014-08-08 12:27 UTC #1 lore raw

PATCH 1/1 pci: mx6: fix occasional link failures

Quoted 8 lines · Tim Harvey

According to the IMX6 reference manuals, REF_SSP_EN (Reference clock enable for SS function) must remain deasserted until the reference clock is running at the appropriate frequency.
Without this patch we find a high link failure rate (>5%) on certain IMX6 boards at various temperatures.
Signed-off-by Tim Harvey <[email protected]>

Acked-by Marek Vasut <[email protected]>

Best regards,
Marek Vasut

Fabio Estevam

reviewer

2014-08-08 13:02 UTC #2 lore raw

PATCH 1/1 pci: mx6: fix occasional link failures

In reply to Tim Harvey

Hi Tim,

Quoted 8 lines · Tim Harvey

According to the IMX6 reference manuals, REF_SSP_EN (Reference clock enable for SS function) must remain deasserted until the reference clock is running at the appropriate frequency.
Without this patch we find a high link failure rate (>5%) on certain IMX6 boards at various temperatures.
Signed-off-by Tim Harvey <[email protected]>

Looks good, thanks.

Reviewed-by Fabio Estevam <[email protected]>

Also Cc Richard and Rogerio in case they have any comment.

Quoted 30 lines

---
 drivers/pci/pcie_imx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/pci/pcie_imx.c +4 -4

index c48737e..a3982c4 100644
@@ -509,10 +509,6 @@ static int imx6_pcie_deassert_core_reset(void)
 
        imx6_pcie_toggle_power();
 
-       /* Enable PCIe */
-       clrbits_le32(&iomuxc_regs->gpr[1], IOMUXC_GPR1_TEST_POWERDOWN);
-       setbits_le32(&iomuxc_regs->gpr[1], IOMUXC_GPR1_REF_SSP_EN);
-
        enable_pcie_clock();
 
        /*

@@ -521,6 +517,10 @@ static int imx6_pcie_deassert_core_reset(void)
         */
        mdelay(50);
 
+       /* Enable PCIe */
+       clrbits_le32(&iomuxc_regs->gpr[1], IOMUXC_GPR1_TEST_POWERDOWN);
+       setbits_le32(&iomuxc_regs->gpr[1], IOMUXC_GPR1_REF_SSP_EN);
+
        imx6_pcie_toggle_reset();
 
        return 0;

Marek Vasut

2014-08-08 13:33 UTC #3 lore raw

PATCH 1/1 pci: mx6: fix occasional link failures

Quoted 10 lines · Fabio Estevam

Hi Tim,
Quoted 7 lines · Tim Harvey
According to the IMX6 reference manuals, REF_SSP_EN (Reference clock enable for SS function) must remain deasserted until the reference clock is running at the appropriate frequency.
Without this patch we find a high link failure rate (>5%) on certain IMX6 boards at various temperatures.

Quoted 7 lines

Quoted 1 line
Signed-off-by Tim Harvey <[email protected]>
Looks good, thanks.
Reviewed-by Fabio Estevam <[email protected]>
Also Cc Richard and Rogerio in case they have any comment.

Can you guys test it on FSL hardware ?

Best regards,
Marek Vasut

Fabio Estevam

reviewer

2014-08-08 15:35 UTC #4 lore raw

PATCH 1/1 pci: mx6: fix occasional link failures

Quoted 1 line · Marek Vasut

Can you guys test it on FSL hardware ?

Sure, I am running u-boot-imx with Tim's patch applied and this additional debug patch that shows the number of reboots and PCI linkups:

 drivers/pci/pcie_imx.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

drivers/pci/pcie_imx.c +12 -0

index a3982c4..1ee77f9 100644
@@ -20,6 +20,8 @@
 #include <linux/sizes.h>
 #include <errno.h>
 
+#define DEBUG
+
 #define PCI_ACCESS_READ  0
 #define PCI_ACCESS_WRITE 1

@@ -573,6 +575,10 @@ void imx_pcie_init(void)
     static struct pci_controller    pcc;
     struct pci_controller        *hose = &pcc;
     int ret;
+#ifdef DEBUG
+    u32 dbg_reg_addr = 0x020cc068;
+    u32 dbg_reg = readl(dbg_reg_addr) + 1;
+#endif
 
     memset(&pcc, 0, sizeof(pcc));

@@ -607,7 +613,14 @@ void imx_pcie_init(void)
     if (!ret) {
         pci_register_hose(hose);
         hose->last_busno = pci_hose_scan(hose);
+#ifdef DEBUG
+        dbg_reg += 1<<16;
+#endif
     }
+#ifdef DEBUG
+    writel(dbg_reg, dbg_reg_addr);
+    printf("PCIe Successes/Attempts: %d/%d\n", dbg_reg >> 16, dbg_reg

& 0xffff);
+#endif
}

Then I do:

=> setenv bootcmd reset
=> save
=> reboot

,connect an Intel Wifi 7260HMW card to the mx6qsabresd and let the system rebooting in loop.

It has been running with no PCI linkup failures here:

U-Boot 2014.07-16451-g7496820-dirty (Aug 08 2014 - 11:34:13)

CPU:   Freescale i.MX6Q rev1.1 at 792 MHz
Reset cause: WDOG
Board: MX6-SabreSD
I2C:   ready
DRAM:  1 GiB
MMC:   FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2
  00:01.0     - 16c3:abcd - Bridge device
   01:00.0    - 8086:08b1 - Network controller
PCIe Successes/Attempts: 2350/2350
No panel detected: default to Hannstar-XGA
Display: Hannstar-XGA (1024x768)
In:    serial
Out:   serial
Err:   serial
PMIC:  PFUZE100 ID=0x10
Net:   FEC [PRIME]

So here goes my:

Tested-by Fabio Estevam <[email protected]>

Regards,

Fabio Estevam

Marek Vasut

2014-08-09 11:51 UTC #5 lore raw

PATCH 1/1 pci: mx6: fix occasional link failures

Quoted 6 lines · Fabio Estevam

Quoted 1 line · Marek Vasut
Can you guys test it on FSL hardware ?
Sure, I am running u-boot-imx with Tim's patch applied and this additional debug patch that shows the number of reboots and PCI linkups:

Well you do realize that this addition changes the timing of the code and also generates writes on the AXI bus, right? I would be much fonder of your testing if you did it with a pristine code and monitored the number of successful cases with plain logging of the serial console and a grep. This should be easy and would avoid the problems with modified code.

Best regards,
Marek Vasut

Fabio Estevam

reviewer

2014-08-09 15:37 UTC #6 lore raw

PATCH 1/1 pci: mx6: fix occasional link failures

Hi Marek,

Quoted 5 lines · Marek Vasut

Well you do realize that this addition changes the timing of the code and also generates writes on the AXI bus, right? I would be much fonder of your testing if you did it with a pristine code and monitored the number of successful cases with plain logging of the serial console and a grep. This should be easy and would avoid the problems with modified code.

Ok, so with the debug patch applied I let it running overnight and it ran 39k+ times without PCI linkup failures.

I agree with your comments and now I am doing as you suggested:

Running top of tree u-boot-imx with only Tim's patch applied.

Then I do:

sudo minicom | tee pci.txt to collect the logs.

After one hour of testing:

/tmp$ cat pci.txt | grep CPU | wc -l
2251

/tmp$ cat pci.txt | grep Bridge | wc -l
2251

Could you help testing the kernel patch?

Regards,

Fabio Estevam

reviewer

2014-08-09 19:11 UTC #7 lore raw

PATCH 1/1 pci: mx6: fix occasional link failures

Quoted 11 lines · Fabio Estevam

Ok, so with the debug patch applied I let it running overnight and it ran 39k+ times without PCI linkup failures.
I agree with your comments and now I am doing as you suggested:
Running top of tree u-boot-imx with only Tim's patch applied.
Then I do:
sudo minicom | tee pci.txt to collect the logs.

Quoted 7 lines

After one hour of testing:
/tmp$ cat pci.txt | grep CPU | wc -l
2251
/tmp$ cat pci.txt | grep Bridge | wc -l
2251

The test has been running more than 11k times without a failure.

Will finish the tests now and let my board to rest a little bit :-)

Marek Vasut

2014-08-10 20:41 UTC #8 lore raw

PATCH 1/1 pci: mx6: fix occasional link failures

Quoted 17 lines · Fabio Estevam

Quoted 16 lines · Fabio Estevam
Ok, so with the debug patch applied I let it running overnight and it ran 39k+ times without PCI linkup failures.
I agree with your comments and now I am doing as you suggested:
Running top of tree u-boot-imx with only Tim's patch applied.
Then I do:
sudo minicom | tee pci.txt to collect the logs.
After one hour of testing:
/tmp$ cat pci.txt | grep CPU | wc -l
2251

Quoted 6 lines

Quoted 2 lines
/tmp$ cat pci.txt | grep Bridge | wc -l
2251
The test has been running more than 11k times without a failure.
Will finish the tests now and let my board to rest a little bit :-)

Thanks!

Acked-by Marek Vasut <[email protected]>

Best regards,
Marek Vasut

Stefano Babic

2014-08-20 10:38 UTC #9 lore raw

PATCH 1/1 pci: mx6: fix occasional link failures

In reply to Tim Harvey

Hi Tim,

Quoted 9 lines · Tim Harvey

According to the IMX6 reference manuals, REF_SSP_EN (Reference clock enable for SS function) must remain deasserted until the reference clock is running at the appropriate frequency.
Without this patch we find a high link failure rate (>5%) on certain IMX6 boards at various temperatures.
Signed-off-by Tim Harvey <[email protected]>
---

Applied to u-boot-imx, thanks !

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================