Re: [PATCH] rockchip: Add delay after link-training

2020-06-30 Thread Kurt Miller
On Sat, 2020-06-27 at 20:57 +0800, Kever Yang wrote:
> Hi Kurt,
> 
> 
> On 2020/6/4 上午5:17, Peter Geis wrote:
> 
> > 
> > On Tue, Jun 2, 2020 at 11:12 AM Kurt Miller  
> > wrote:
> > > 
> > > On Tue, 2020-06-02 at 10:23 +0800, Shawn Lin wrote:
> > > > 
> > > > 在 2020/6/2 9:59, Kever Yang 写道:
> > > > > 
> > > > > Hi Kurt,
> > > > > 
> > > > > On 2020/6/2 上午4:30, Kurt Miller wrote:
> > > > > > 
> > > > > > On at least the RockPro64, many cards will trip a
> > > > > > synchronous abort when first accessing PCIe config space
> > > > > > during bus scanning. A delay after link training allows
> > > > > > some of these cards to function.
> > > > > > 
> > > > > > Signed-off-by: Kurt Miller 
> > > > > > ---
> > > > > > On the RockPro64, some pci cards trip a synchronous abort when
> > > > > > scanning the
> > > > > > pci bus. For example with HighPoint Rocket Raid 640L which is based 
> > > > > > on
> > > > > > Marvell 88SE9230 I see this:
> > > > > > 
> > > > > > => pci
> > > > > > "Synchronous Abort" handler, esr 0x96000210
> > > > > > elr: 0022d034 lr : 0022cfd0 (reloc)
> > > > > > elr: f4568034 lr : f4567fd0
> > > > > > x0 : 0010 x1 : f800
> > > > > > x2 :  x3 : 0010
> > > > > > x4 : f2559290 x5 : 
> > > > > > x6 : 0001 x7 : f2559860
> > > > > > x8 : 0030 x9 : 0008
> > > > > > x10: 0010 x11: f251fd1c
> > > > > > x12: 1421 x13: 1468
> > > > > > x14: f251fd4c x15: 
> > > > > > x16: 00060001 x17: 001f
> > > > > > x18: f2532dc0 x19: f251fcd0
> > > > > > x20: 0001 x21: 
> > > > > > x22: 0001 x23: f45d4000
> > > > > > x24:  x25: f45bc000
> > > > > > x26:  x27: 
> > > > > > x28: f2541440 x29: f251fc20
> > > > > > 
> > > > > > Code: 54c1 35a5 93407c00 f9400081 (b8616800)
> > > > > > Resetting CPU ...
> > > > > > 
> > > > > > Adding a delay after link training works-around the problem. I 
> > > > > > added this
> > > > > > delay to the OpenBSD rkpcie driver as well:
> > > > > > 
> > > > > > https://github.com/openbsd/src/commit/9857dee3520d8ca5bec68538f4b0708d7e64fc87
> > > > > > 
> > > > > > 
> > > > > > HighPoint Rocket Raid 640L needs a 1.75 sec delay and Crossfield
> > > > > > SAS9211-4i
> > > > > > needs a 1 second delay, so I arbitrarily decided on 2 seconds.
> > > > > > 
> > > > > > The delay work-around was originally discovered by nuumio:
> > > > > > https://github.com/nuumio/linux-kernel/commit/5a65b17686002dc84d461bffa324a2cb68e67aee
> > > > > > 
> > > > > > 
> > > > > >    drivers/pci/pcie_rockchip.c | 8 
> > > > > >    1 file changed, 8 insertions(+)
> > > > > > 
> > > > > > diff --git a/drivers/pci/pcie_rockchip.c 
> > > > > > b/drivers/pci/pcie_rockchip.c
> > > > > > index 0edc2464a8..51cfbf6b18 100644
> > > > > > --- a/drivers/pci/pcie_rockchip.c
> > > > > > +++ b/drivers/pci/pcie_rockchip.c
> > > > > > @@ -288,6 +288,14 @@ static int rockchip_pcie_init_port(struct 
> > > > > > udevice
> > > > > > *dev)
> > > > > >    goto err_power_off_phy;
> > > > > >    }
> > > > > > +/*
> > > > > > + * XXX: On at least the RockPro64, many cards will trip a
> > > > > > + * synchronous abort when first accessing PCIe config space
> > > > > > + * during bus scanning. A delay after link training allows
> > > > > > + * some of these cards to function.
> > > > > > + */
> > > > > > +mdelay(2000);
> > > > > I don't understand what kind of delay for init needs 2 Seconds, root
> > > > > cause will preferred.
> > > Hi Kever,
> > > 
> > > While working on this issue for the OpenBSD PCIe driver I was not
> > > able to determine the root cause. I tested the following adapters:
> > > 
> > > ROCKPro64 2 Port SATA
> > > StarTech PEXSAT32 2 Port SATA
> > > Samsung 970 Evo NVMe w/m.2 adapter
> > > IO CREST SI-PEX40148 2 Port SATA
> > > IO CREST SI-PEX40057 4 port
> > > HighPoint Rocket Raid 640L
> > > Crossfield SAS9211-4i
> > > Del PERC H200
> > > Dell PERC 6/i
> > > Intel Gigabit VT Quad Port Server
> > > 
> > > All of the above adapters successfully link trained, however
> > > three of them would panic upon the first read of the PCI config
> > > space. nuumio's work-around of placing a delay after link-training
> > > allows these cards to work.
> > > 
> > > HighPoint Rocket Raid 640L ~1.75 sec delay
> > > Crossfield SAS9211-4i ~1 sec delay
> > > Dell PERC H200 ~1 sec delay
> > > 
> > > In attempt to determine if there was a way to detect how long
> > > to wait, I compared every status and debug register documented
> > > in the rk3399 TRM part 2 for the PCI controller. I compared the
> > > values pre-delay and post-delay. I was not able to find a value
> > > that would indicate it was safe to proceed.
> > > 

Re: [PATCH] rockchip: Add delay after link-training

2020-06-27 Thread Kever Yang

Hi Kurt,


On 2020/6/4 上午5:17, Peter Geis wrote:


On Tue, Jun 2, 2020 at 11:12 AM Kurt Miller  wrote:

On Tue, 2020-06-02 at 10:23 +0800, Shawn Lin wrote:

在 2020/6/2 9:59, Kever Yang 写道:

Hi Kurt,

On 2020/6/2 上午4:30, Kurt Miller wrote:

On at least the RockPro64, many cards will trip a
synchronous abort when first accessing PCIe config space
during bus scanning. A delay after link training allows
some of these cards to function.

Signed-off-by: Kurt Miller 
---
On the RockPro64, some pci cards trip a synchronous abort when
scanning the
pci bus. For example with HighPoint Rocket Raid 640L which is based on
Marvell 88SE9230 I see this:

=> pci
"Synchronous Abort" handler, esr 0x96000210
elr: 0022d034 lr : 0022cfd0 (reloc)
elr: f4568034 lr : f4567fd0
x0 : 0010 x1 : f800
x2 :  x3 : 0010
x4 : f2559290 x5 : 
x6 : 0001 x7 : f2559860
x8 : 0030 x9 : 0008
x10: 0010 x11: f251fd1c
x12: 1421 x13: 1468
x14: f251fd4c x15: 
x16: 00060001 x17: 001f
x18: f2532dc0 x19: f251fcd0
x20: 0001 x21: 
x22: 0001 x23: f45d4000
x24:  x25: f45bc000
x26:  x27: 
x28: f2541440 x29: f251fc20

Code: 54c1 35a5 93407c00 f9400081 (b8616800)
Resetting CPU ...

Adding a delay after link training works-around the problem. I added this
delay to the OpenBSD rkpcie driver as well:

https://github.com/openbsd/src/commit/9857dee3520d8ca5bec68538f4b0708d7e64fc87


HighPoint Rocket Raid 640L needs a 1.75 sec delay and Crossfield
SAS9211-4i
needs a 1 second delay, so I arbitrarily decided on 2 seconds.

The delay work-around was originally discovered by nuumio:
https://github.com/nuumio/linux-kernel/commit/5a65b17686002dc84d461bffa324a2cb68e67aee


   drivers/pci/pcie_rockchip.c | 8 
   1 file changed, 8 insertions(+)

diff --git a/drivers/pci/pcie_rockchip.c b/drivers/pci/pcie_rockchip.c
index 0edc2464a8..51cfbf6b18 100644
--- a/drivers/pci/pcie_rockchip.c
+++ b/drivers/pci/pcie_rockchip.c
@@ -288,6 +288,14 @@ static int rockchip_pcie_init_port(struct udevice
*dev)
   goto err_power_off_phy;
   }
+/*
+ * XXX: On at least the RockPro64, many cards will trip a
+ * synchronous abort when first accessing PCIe config space
+ * during bus scanning. A delay after link training allows
+ * some of these cards to function.
+ */
+mdelay(2000);

I don't understand what kind of delay for init needs 2 Seconds, root
cause will preferred.

Hi Kever,

While working on this issue for the OpenBSD PCIe driver I was not
able to determine the root cause. I tested the following adapters:

ROCKPro64 2 Port SATA
StarTech PEXSAT32 2 Port SATA
Samsung 970 Evo NVMe w/m.2 adapter
IO CREST SI-PEX40148 2 Port SATA
IO CREST SI-PEX40057 4 port
HighPoint Rocket Raid 640L
Crossfield SAS9211-4i
Del PERC H200
Dell PERC 6/i
Intel Gigabit VT Quad Port Server

All of the above adapters successfully link trained, however
three of them would panic upon the first read of the PCI config
space. nuumio's work-around of placing a delay after link-training
allows these cards to work.

HighPoint Rocket Raid 640L ~1.75 sec delay
Crossfield SAS9211-4i ~1 sec delay
Dell PERC H200 ~1 sec delay

In attempt to determine if there was a way to detect how long
to wait, I compared every status and debug register documented
in the rk3399 TRM part 2 for the PCI controller. I compared the
values pre-delay and post-delay. I was not able to find a value
that would indicate it was safe to proceed.


Strictly speaking, how long should we need for this had been provided,
see this:

https://patchwork.kernel.org/patch/11561977/


I can accept the same fix like kernel which is 100ms, but 2 Second is 
really too much for most cases.



Thanks,

- Kever



If you need more delay, I  highly suspect you should check if
the power supply is stable before enabling training.

Hi Shawn,

Thank you for pointing out the need for the 100ms delay before
beginning link-training. I believe this is to correct link
training failures, while the delay after link training is
to work-around post link training reading of PCI config
space on certain cards.

There are similar issues on the Linux driver with various cards
randomly throwing an abort.
If it is power, a 2 second wait to allow cards to stabilize after
power on might be wise for the Linux driver as well.

I imagine some cards take longer to complete power on reset than
others, and attempting to read them immediately after powering them up
could be the issue here.


In my testing I discussed above, I suspected that power
was a likely cause. The HighPoint Rocket Raid 640L is
a good test card because it documents its 3.3v power at
0.7 watts:


Re: [PATCH] rockchip: Add delay after link-training

2020-06-03 Thread Peter Geis
On Tue, Jun 2, 2020 at 11:12 AM Kurt Miller  wrote:
>
> On Tue, 2020-06-02 at 10:23 +0800, Shawn Lin wrote:
> >
> > 在 2020/6/2 9:59, Kever Yang 写道:
> > >
> > > Hi Kurt,
> > >
> > > On 2020/6/2 上午4:30, Kurt Miller wrote:
> > > >
> > > > On at least the RockPro64, many cards will trip a
> > > > synchronous abort when first accessing PCIe config space
> > > > during bus scanning. A delay after link training allows
> > > > some of these cards to function.
> > > >
> > > > Signed-off-by: Kurt Miller 
> > > > ---
> > > > On the RockPro64, some pci cards trip a synchronous abort when
> > > > scanning the
> > > > pci bus. For example with HighPoint Rocket Raid 640L which is based on
> > > > Marvell 88SE9230 I see this:
> > > >
> > > > => pci
> > > > "Synchronous Abort" handler, esr 0x96000210
> > > > elr: 0022d034 lr : 0022cfd0 (reloc)
> > > > elr: f4568034 lr : f4567fd0
> > > > x0 : 0010 x1 : f800
> > > > x2 :  x3 : 0010
> > > > x4 : f2559290 x5 : 
> > > > x6 : 0001 x7 : f2559860
> > > > x8 : 0030 x9 : 0008
> > > > x10: 0010 x11: f251fd1c
> > > > x12: 1421 x13: 1468
> > > > x14: f251fd4c x15: 
> > > > x16: 00060001 x17: 001f
> > > > x18: f2532dc0 x19: f251fcd0
> > > > x20: 0001 x21: 
> > > > x22: 0001 x23: f45d4000
> > > > x24:  x25: f45bc000
> > > > x26:  x27: 
> > > > x28: f2541440 x29: f251fc20
> > > >
> > > > Code: 54c1 35a5 93407c00 f9400081 (b8616800)
> > > > Resetting CPU ...
> > > >
> > > > Adding a delay after link training works-around the problem. I added 
> > > > this
> > > > delay to the OpenBSD rkpcie driver as well:
> > > >
> > > > https://github.com/openbsd/src/commit/9857dee3520d8ca5bec68538f4b0708d7e64fc87
> > > >
> > > >
> > > > HighPoint Rocket Raid 640L needs a 1.75 sec delay and Crossfield
> > > > SAS9211-4i
> > > > needs a 1 second delay, so I arbitrarily decided on 2 seconds.
> > > >
> > > > The delay work-around was originally discovered by nuumio:
> > > > https://github.com/nuumio/linux-kernel/commit/5a65b17686002dc84d461bffa324a2cb68e67aee
> > > >
> > > >
> > > >   drivers/pci/pcie_rockchip.c | 8 
> > > >   1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/pcie_rockchip.c b/drivers/pci/pcie_rockchip.c
> > > > index 0edc2464a8..51cfbf6b18 100644
> > > > --- a/drivers/pci/pcie_rockchip.c
> > > > +++ b/drivers/pci/pcie_rockchip.c
> > > > @@ -288,6 +288,14 @@ static int rockchip_pcie_init_port(struct udevice
> > > > *dev)
> > > >   goto err_power_off_phy;
> > > >   }
> > > > +/*
> > > > + * XXX: On at least the RockPro64, many cards will trip a
> > > > + * synchronous abort when first accessing PCIe config space
> > > > + * during bus scanning. A delay after link training allows
> > > > + * some of these cards to function.
> > > > + */
> > > > +mdelay(2000);
> > > I don't understand what kind of delay for init needs 2 Seconds, root
> > > cause will preferred.
>
> Hi Kever,
>
> While working on this issue for the OpenBSD PCIe driver I was not
> able to determine the root cause. I tested the following adapters:
>
> ROCKPro64 2 Port SATA
> StarTech PEXSAT32 2 Port SATA
> Samsung 970 Evo NVMe w/m.2 adapter
> IO CREST SI-PEX40148 2 Port SATA
> IO CREST SI-PEX40057 4 port
> HighPoint Rocket Raid 640L
> Crossfield SAS9211-4i
> Del PERC H200
> Dell PERC 6/i
> Intel Gigabit VT Quad Port Server
>
> All of the above adapters successfully link trained, however
> three of them would panic upon the first read of the PCI config
> space. nuumio's work-around of placing a delay after link-training
> allows these cards to work.
>
> HighPoint Rocket Raid 640L ~1.75 sec delay
> Crossfield SAS9211-4i ~1 sec delay
> Dell PERC H200 ~1 sec delay
>
> In attempt to determine if there was a way to detect how long
> to wait, I compared every status and debug register documented
> in the rk3399 TRM part 2 for the PCI controller. I compared the
> values pre-delay and post-delay. I was not able to find a value
> that would indicate it was safe to proceed.
>
> > Strictly speaking, how long should we need for this had been provided,
> > see this:
> >
> > https://patchwork.kernel.org/patch/11561977/
> >
> > If you need more delay, I  highly suspect you should check if
> > the power supply is stable before enabling training.
>
> Hi Shawn,
>
> Thank you for pointing out the need for the 100ms delay before
> beginning link-training. I believe this is to correct link
> training failures, while the delay after link training is
> to work-around post link training reading of PCI config
> space on certain cards.

There are similar issues on the Linux driver with various cards
randomly 

Re: [PATCH] rockchip: Add delay after link-training

2020-06-02 Thread Kurt Miller
On Tue, 2020-06-02 at 10:23 +0800, Shawn Lin wrote:
> 
> 在 2020/6/2 9:59, Kever Yang 写道:
> > 
> > Hi Kurt,
> > 
> > On 2020/6/2 上午4:30, Kurt Miller wrote:
> > > 
> > > On at least the RockPro64, many cards will trip a
> > > synchronous abort when first accessing PCIe config space
> > > during bus scanning. A delay after link training allows
> > > some of these cards to function.
> > > 
> > > Signed-off-by: Kurt Miller 
> > > ---
> > > On the RockPro64, some pci cards trip a synchronous abort when 
> > > scanning the
> > > pci bus. For example with HighPoint Rocket Raid 640L which is based on
> > > Marvell 88SE9230 I see this:
> > > 
> > > => pci
> > > "Synchronous Abort" handler, esr 0x96000210
> > > elr: 0022d034 lr : 0022cfd0 (reloc)
> > > elr: f4568034 lr : f4567fd0
> > > x0 : 0010 x1 : f800
> > > x2 :  x3 : 0010
> > > x4 : f2559290 x5 : 
> > > x6 : 0001 x7 : f2559860
> > > x8 : 0030 x9 : 0008
> > > x10: 0010 x11: f251fd1c
> > > x12: 1421 x13: 1468
> > > x14: f251fd4c x15: 
> > > x16: 00060001 x17: 001f
> > > x18: f2532dc0 x19: f251fcd0
> > > x20: 0001 x21: 
> > > x22: 0001 x23: f45d4000
> > > x24:  x25: f45bc000
> > > x26:  x27: 
> > > x28: f2541440 x29: f251fc20
> > > 
> > > Code: 54c1 35a5 93407c00 f9400081 (b8616800)
> > > Resetting CPU ...
> > > 
> > > Adding a delay after link training works-around the problem. I added this
> > > delay to the OpenBSD rkpcie driver as well:
> > > 
> > > https://github.com/openbsd/src/commit/9857dee3520d8ca5bec68538f4b0708d7e64fc87
> > >  
> > > 
> > > 
> > > HighPoint Rocket Raid 640L needs a 1.75 sec delay and Crossfield 
> > > SAS9211-4i
> > > needs a 1 second delay, so I arbitrarily decided on 2 seconds.
> > > 
> > > The delay work-around was originally discovered by nuumio:
> > > https://github.com/nuumio/linux-kernel/commit/5a65b17686002dc84d461bffa324a2cb68e67aee
> > >  
> > > 
> > > 
> > >   drivers/pci/pcie_rockchip.c | 8 
> > >   1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/drivers/pci/pcie_rockchip.c b/drivers/pci/pcie_rockchip.c
> > > index 0edc2464a8..51cfbf6b18 100644
> > > --- a/drivers/pci/pcie_rockchip.c
> > > +++ b/drivers/pci/pcie_rockchip.c
> > > @@ -288,6 +288,14 @@ static int rockchip_pcie_init_port(struct udevice 
> > > *dev)
> > >   goto err_power_off_phy;
> > >   }
> > > +    /*
> > > + * XXX: On at least the RockPro64, many cards will trip a
> > > + * synchronous abort when first accessing PCIe config space
> > > + * during bus scanning. A delay after link training allows
> > > + * some of these cards to function.
> > > + */
> > > +    mdelay(2000);
> > I don't understand what kind of delay for init needs 2 Seconds, root 
> > cause will preferred.

Hi Kever,

While working on this issue for the OpenBSD PCIe driver I was not
able to determine the root cause. I tested the following adapters:

ROCKPro64 2 Port SATA
StarTech PEXSAT32 2 Port SATA
Samsung 970 Evo NVMe w/m.2 adapter
IO CREST SI-PEX40148 2 Port SATA
IO CREST SI-PEX40057 4 port 
HighPoint Rocket Raid 640L
Crossfield SAS9211-4i
Del PERC H200
Dell PERC 6/i
Intel Gigabit VT Quad Port Server

All of the above adapters successfully link trained, however
three of them would panic upon the first read of the PCI config
space. nuumio's work-around of placing a delay after link-training
allows these cards to work.

HighPoint Rocket Raid 640L ~1.75 sec delay
Crossfield SAS9211-4i ~1 sec delay
Dell PERC H200 ~1 sec delay

In attempt to determine if there was a way to detect how long
to wait, I compared every status and debug register documented
in the rk3399 TRM part 2 for the PCI controller. I compared the
values pre-delay and post-delay. I was not able to find a value
that would indicate it was safe to proceed.

> Strictly speaking, how long should we need for this had been provided,
> see this:
> 
> https://patchwork.kernel.org/patch/11561977/
> 
> If you need more delay, I  highly suspect you should check if
> the power supply is stable before enabling training.

Hi Shawn,

Thank you for pointing out the need for the 100ms delay before
beginning link-training. I believe this is to correct link
training failures, while the delay after link training is
to work-around post link training reading of PCI config
space on certain cards.

In my testing I discussed above, I suspected that power
was a likely cause. The HighPoint Rocket Raid 640L is
a good test card because it documents its 3.3v power at
0.7 watts:

https://highpoint-tech.com/USA_new/series_r600-specifications.htm

How can I check if the power supply is stable?

Regards,
-Kurt

> > 
> > 
> > 
> > Thanks,
> 

Re: [PATCH] rockchip: Add delay after link-training

2020-06-01 Thread Shawn Lin




在 2020/6/2 9:59, Kever Yang 写道:

Hi Kurt,

On 2020/6/2 上午4:30, Kurt Miller wrote:

On at least the RockPro64, many cards will trip a
synchronous abort when first accessing PCIe config space
during bus scanning. A delay after link training allows
some of these cards to function.

Signed-off-by: Kurt Miller 
---
On the RockPro64, some pci cards trip a synchronous abort when 
scanning the

pci bus. For example with HighPoint Rocket Raid 640L which is based on
Marvell 88SE9230 I see this:

=> pci
"Synchronous Abort" handler, esr 0x96000210
elr: 0022d034 lr : 0022cfd0 (reloc)
elr: f4568034 lr : f4567fd0
x0 : 0010 x1 : f800
x2 :  x3 : 0010
x4 : f2559290 x5 : 
x6 : 0001 x7 : f2559860
x8 : 0030 x9 : 0008
x10: 0010 x11: f251fd1c
x12: 1421 x13: 1468
x14: f251fd4c x15: 
x16: 00060001 x17: 001f
x18: f2532dc0 x19: f251fcd0
x20: 0001 x21: 
x22: 0001 x23: f45d4000
x24:  x25: f45bc000
x26:  x27: 
x28: f2541440 x29: f251fc20

Code: 54c1 35a5 93407c00 f9400081 (b8616800)
Resetting CPU ...

Adding a delay after link training works-around the problem. I added this
delay to the OpenBSD rkpcie driver as well:

https://github.com/openbsd/src/commit/9857dee3520d8ca5bec68538f4b0708d7e64fc87 



HighPoint Rocket Raid 640L needs a 1.75 sec delay and Crossfield 
SAS9211-4i

needs a 1 second delay, so I arbitrarily decided on 2 seconds.

The delay work-around was originally discovered by nuumio:
https://github.com/nuumio/linux-kernel/commit/5a65b17686002dc84d461bffa324a2cb68e67aee 



  drivers/pci/pcie_rockchip.c | 8 
  1 file changed, 8 insertions(+)

diff --git a/drivers/pci/pcie_rockchip.c b/drivers/pci/pcie_rockchip.c
index 0edc2464a8..51cfbf6b18 100644
--- a/drivers/pci/pcie_rockchip.c
+++ b/drivers/pci/pcie_rockchip.c
@@ -288,6 +288,14 @@ static int rockchip_pcie_init_port(struct udevice 
*dev)

  goto err_power_off_phy;
  }
+    /*
+ * XXX: On at least the RockPro64, many cards will trip a
+ * synchronous abort when first accessing PCIe config space
+ * during bus scanning. A delay after link training allows
+ * some of these cards to function.
+ */
+    mdelay(2000);


I don't understand what kind of delay for init needs 2 Seconds, root 
cause will preferred.


Strictly speaking, how long should we need for this had been provided,
see this:

https://patchwork.kernel.org/patch/11561977/

If you need more delay, I  highly suspect you should check if
the power supply is stable before enabling training.




Thanks,

- Kever


+
  /* Initialize Root Complex registers. */
  writel(PCIE_LM_VENDOR_ROCKCHIP, priv->apb_base + 
PCIE_LM_VENDOR_ID);

  writel(PCI_CLASS_BRIDGE_PCI << 16,









Re: [PATCH] rockchip: Add delay after link-training

2020-06-01 Thread Kever Yang

Hi Kurt,

On 2020/6/2 上午4:30, Kurt Miller wrote:

On at least the RockPro64, many cards will trip a
synchronous abort when first accessing PCIe config space
during bus scanning. A delay after link training allows
some of these cards to function.

Signed-off-by: Kurt Miller 
---
On the RockPro64, some pci cards trip a synchronous abort when scanning the
pci bus. For example with HighPoint Rocket Raid 640L which is based on
Marvell 88SE9230 I see this:

=> pci
"Synchronous Abort" handler, esr 0x96000210
elr: 0022d034 lr : 0022cfd0 (reloc)
elr: f4568034 lr : f4567fd0
x0 : 0010 x1 : f800
x2 :  x3 : 0010
x4 : f2559290 x5 : 
x6 : 0001 x7 : f2559860
x8 : 0030 x9 : 0008
x10: 0010 x11: f251fd1c
x12: 1421 x13: 1468
x14: f251fd4c x15: 
x16: 00060001 x17: 001f
x18: f2532dc0 x19: f251fcd0
x20: 0001 x21: 
x22: 0001 x23: f45d4000
x24:  x25: f45bc000
x26:  x27: 
x28: f2541440 x29: f251fc20

Code: 54c1 35a5 93407c00 f9400081 (b8616800)
Resetting CPU ...

Adding a delay after link training works-around the problem. I added this
delay to the OpenBSD rkpcie driver as well:

https://github.com/openbsd/src/commit/9857dee3520d8ca5bec68538f4b0708d7e64fc87

HighPoint Rocket Raid 640L needs a 1.75 sec delay and Crossfield SAS9211-4i
needs a 1 second delay, so I arbitrarily decided on 2 seconds.

The delay work-around was originally discovered by nuumio:
https://github.com/nuumio/linux-kernel/commit/5a65b17686002dc84d461bffa324a2cb68e67aee

  drivers/pci/pcie_rockchip.c | 8 
  1 file changed, 8 insertions(+)

diff --git a/drivers/pci/pcie_rockchip.c b/drivers/pci/pcie_rockchip.c
index 0edc2464a8..51cfbf6b18 100644
--- a/drivers/pci/pcie_rockchip.c
+++ b/drivers/pci/pcie_rockchip.c
@@ -288,6 +288,14 @@ static int rockchip_pcie_init_port(struct udevice *dev)
goto err_power_off_phy;
}
  
+	/*

+* XXX: On at least the RockPro64, many cards will trip a
+* synchronous abort when first accessing PCIe config space
+* during bus scanning. A delay after link training allows
+* some of these cards to function.
+*/
+   mdelay(2000);


I don't understand what kind of delay for init needs 2 Seconds, root 
cause will preferred.



Thanks,

- Kever


+
/* Initialize Root Complex registers. */
writel(PCIE_LM_VENDOR_ROCKCHIP, priv->apb_base + PCIE_LM_VENDOR_ID);
writel(PCI_CLASS_BRIDGE_PCI << 16,





Re: [PATCH] rockchip: Add delay after link-training

2020-06-01 Thread Kurt Miller
On Tue, 2020-06-02 at 02:16 +0530, Jagan Teki wrote:
> On Tue, Jun 2, 2020 at 2:00 AM Kurt Miller  wrote:
> > 
> > 
> > On at least the RockPro64, many cards will trip a
> > synchronous abort when first accessing PCIe config space
> > during bus scanning. A delay after link training allows
> > some of these cards to function.
> Can you check does the SoC has external PCIe pwr-pin GPIO?
> 
> I did see unstable SSD behavior on rock960 but fixed with this.
> https://github.com/radxa/u-boot/blob/stable-4.4-rockpi4/board/rockchip/evb_rk3399/evb-rk3399.c#L168

The schematic has:

GPIO1_D0/TCPD_VBUS_SOURCE2_d ---L26>>PCIE_PWR

and arch/arm/dts/rk3399-rockpro64.dtsi has:

 {
        pcie {
                pcie_pwr_en: pcie-pwr-en {
rockchip,pins = <1 RK_PD0 RK_FUNC_GPIO _pull_none>;
};
};
};

Does that answer your question? I'm rather new at this so
I may need more guidance if I miss understood your question.

Thanks,
-Kurt


Re: [PATCH] rockchip: Add delay after link-training

2020-06-01 Thread Jagan Teki
On Tue, Jun 2, 2020 at 2:00 AM Kurt Miller  wrote:
>
> On at least the RockPro64, many cards will trip a
> synchronous abort when first accessing PCIe config space
> during bus scanning. A delay after link training allows
> some of these cards to function.

Can you check does the SoC has external PCIe pwr-pin GPIO?

I did see unstable SSD behavior on rock960 but fixed with this.
https://github.com/radxa/u-boot/blob/stable-4.4-rockpi4/board/rockchip/evb_rk3399/evb-rk3399.c#L168


[PATCH] rockchip: Add delay after link-training

2020-06-01 Thread Kurt Miller
On at least the RockPro64, many cards will trip a
synchronous abort when first accessing PCIe config space
during bus scanning. A delay after link training allows
some of these cards to function.

Signed-off-by: Kurt Miller 
---
On the RockPro64, some pci cards trip a synchronous abort when scanning the
pci bus. For example with HighPoint Rocket Raid 640L which is based on
Marvell 88SE9230 I see this:

=> pci
"Synchronous Abort" handler, esr 0x96000210
elr: 0022d034 lr : 0022cfd0 (reloc)
elr: f4568034 lr : f4567fd0
x0 : 0010 x1 : f800
x2 :  x3 : 0010
x4 : f2559290 x5 : 
x6 : 0001 x7 : f2559860
x8 : 0030 x9 : 0008
x10: 0010 x11: f251fd1c
x12: 1421 x13: 1468
x14: f251fd4c x15: 
x16: 00060001 x17: 001f
x18: f2532dc0 x19: f251fcd0
x20: 0001 x21: 
x22: 0001 x23: f45d4000
x24:  x25: f45bc000
x26:  x27: 
x28: f2541440 x29: f251fc20

Code: 54c1 35a5 93407c00 f9400081 (b8616800)
Resetting CPU ...

Adding a delay after link training works-around the problem. I added this
delay to the OpenBSD rkpcie driver as well:

https://github.com/openbsd/src/commit/9857dee3520d8ca5bec68538f4b0708d7e64fc87

HighPoint Rocket Raid 640L needs a 1.75 sec delay and Crossfield SAS9211-4i
needs a 1 second delay, so I arbitrarily decided on 2 seconds.

The delay work-around was originally discovered by nuumio:
https://github.com/nuumio/linux-kernel/commit/5a65b17686002dc84d461bffa324a2cb68e67aee

 drivers/pci/pcie_rockchip.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/pci/pcie_rockchip.c b/drivers/pci/pcie_rockchip.c
index 0edc2464a8..51cfbf6b18 100644
--- a/drivers/pci/pcie_rockchip.c
+++ b/drivers/pci/pcie_rockchip.c
@@ -288,6 +288,14 @@ static int rockchip_pcie_init_port(struct udevice *dev)
goto err_power_off_phy;
}
 
+   /*
+* XXX: On at least the RockPro64, many cards will trip a
+* synchronous abort when first accessing PCIe config space
+* during bus scanning. A delay after link training allows
+* some of these cards to function.
+*/
+   mdelay(2000);
+
/* Initialize Root Complex registers. */
writel(PCIE_LM_VENDOR_ROCKCHIP, priv->apb_base + PCIE_LM_VENDOR_ID);
writel(PCI_CLASS_BRIDGE_PCI << 16,
-- 
2.26.2