[U-Boot] [PATCH] pci: mx6: fix occasional link failures

[U-Boot] [PATCH] pci: mx6: fix occasional link failures

[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 <tharvey@gateworks.com>
---
 drivers/pci/pcie_imx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pcie_imx.c b/drivers/pci/pcie_imx.c
index c48737e..a3982c4 100644
--- a/drivers/pci/pcie_imx.c
+++ b/drivers/pci/pcie_imx.c
@@ -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;
-- 
1.8.3.2

[U-Boot] [PATCH] pci: mx6: fix occasional link failures

[Marek Vasut]

On Friday, August 08, 2014 at 07:57:29 AM, Tim Harvey wrote:
> 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 <tharvey@gateworks.com>

Acked-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

[U-Boot] [PATCH] pci: mx6: fix occasional link failures

[Fabio Estevam]

Hi Tim,

On Fri, Aug 8, 2014 at 2:57 AM, Tim Harvey <tharvey@gateworks.com> wrote:
> 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 <tharvey@gateworks.com>

Looks good, thanks.

Reviewed-by: Fabio Estevam <fabio.estevam@freescale.com>

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

> ---
>  drivers/pci/pcie_imx.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/pcie_imx.c b/drivers/pci/pcie_imx.c
> index c48737e..a3982c4 100644
> --- a/drivers/pci/pcie_imx.c
> +++ b/drivers/pci/pcie_imx.c
> @@ -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;

[U-Boot] [PATCH] pci: mx6: fix occasional link failures

[Marek Vasut]

On Friday, August 08, 2014 at 03:02:27 PM, Fabio Estevam wrote:
> Hi Tim,
> 
> On Fri, Aug 8, 2014 at 2:57 AM, Tim Harvey <tharvey@gateworks.com> wrote:
> > 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 <tharvey@gateworks.com>
> 
> Looks good, thanks.
> 
> Reviewed-by: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Also Cc Richard and Rogerio in case they have any comment.

Can you guys test it on FSL hardware ?

Best regards,
Marek Vasut

[U-Boot] [PATCH] pci: mx6: fix occasional link failures

[Fabio Estevam]

On Fri, Aug 8, 2014 at 10:33 AM, Marek Vasut <marex@denx.de> wrote:

> 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(+)

diff --git a/drivers/pci/pcie_imx.c b/drivers/pci/pcie_imx.c
index a3982c4..1ee77f9 100644
--- a/drivers/pci/pcie_imx.c
+++ b/drivers/pci/pcie_imx.c
@@ -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 <fabio.estevam@freescale.com>

Regards,

Fabio Estevam

[U-Boot] [PATCH] pci: mx6: fix occasional link failures

[Marek Vasut]

On Friday, August 08, 2014 at 05:35:23 PM, Fabio Estevam wrote:
> On Fri, Aug 8, 2014 at 10:33 AM, Marek Vasut <marex@denx.de> wrote:
> > 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

[U-Boot] [PATCH] pci: mx6: fix occasional link failures

[Fabio Estevam]

Hi Marek,

On Sat, Aug 9, 2014 at 8:51 AM, Marek Vasut <marex@denx.de> wrote:

> 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

[U-Boot] [PATCH] pci: mx6: fix occasional link failures

[Fabio Estevam]

On Sat, Aug 9, 2014 at 12:37 PM, Fabio Estevam <festevam@gmail.com> wrote:

> 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

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 :-)

[U-Boot] [PATCH] pci: mx6: fix occasional link failures

[Marek Vasut]

On Saturday, August 09, 2014 at 09:11:49 PM, Fabio Estevam wrote:
> On Sat, Aug 9, 2014 at 12:37 PM, Fabio Estevam <festevam@gmail.com> wrote:
> > 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
> 
> 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 <marex@denx.de>

Best regards,
Marek Vasut

[U-Boot] [PATCH] pci: mx6: fix occasional link failures

[Stefano Babic]

Hi Tim,

On 08/08/2014 07:57, Tim Harvey wrote:
> 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 <tharvey@gateworks.com>
> ---

Applied to u-boot-imx, thanks !

Best regards,
Stefano Babic



-- 
=====================================================================
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
=====================================================================