[5/6] PCI: imx6: Force Gen1 operation

Commit Message

Marek Vasut Oct. 15, 2013, 4:06 p.m. UTC

Without forcing the PCIe core into Gen1 operation, the PCIe switch
attached directly to the PCIe port is not recognised at all. The
PCIe switch is Gen2 capable to make this issue even more puzzling.

Signed-off-by: Marek Vasut <[e-mail]>
Cc: Bjorn Helgaas <[e-mail]>
Cc: Frank Li <[e-mail]>
Cc: Jingoo Han <[e-mail]>
Cc: Mohit KUMAR <[e-mail]>
Cc: Pratyush Anand <[e-mail]>
Cc: Richard Zhu <[e-mail]>
Cc: Sascha Hauer <[e-mail]>
Cc: Sean Cross <[e-mail]>
Cc: Shawn Guo <[e-mail]>
Cc: Siva Reddy Kallam <[e-mail]>
Cc: Srikanth T Shivanand <[e-mail]>
Cc: Tim Harvey <[e-mail]>
Cc: Troy Kisky <[e-mail]>
Cc: Yinghai Lu <[e-mail]>
---
 drivers/pci/host/pci-imx6.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Pratyush ANAND Oct. 16, 2013, 5:54 a.m. UTC | #1

On Wed, Oct 16, 2013 at 12:06:39AM +0800, Marek Vasut wrote:
> Without forcing the PCIe core into Gen1 operation, the PCIe switch
> attached directly to the PCIe port is not recognised at all. The
> PCIe switch is Gen2 capable to make this issue even more puzzling.
> 
> Signed-off-by: Marek Vasut <[e-mail]>
> Cc: Bjorn Helgaas <[e-mail]>
> Cc: Frank Li <[e-mail]>
> Cc: Jingoo Han <[e-mail]>
> Cc: Mohit KUMAR <[e-mail]>
> Cc: Pratyush Anand <[e-mail]>
> Cc: Richard Zhu <[e-mail]>
> Cc: Sascha Hauer <[e-mail]>
> Cc: Sean Cross <[e-mail]>
> Cc: Shawn Guo <[e-mail]>
> Cc: Siva Reddy Kallam <[e-mail]>
> Cc: Srikanth T Shivanand <[e-mail]>
> Cc: Tim Harvey <[e-mail]>
> Cc: Troy Kisky <[e-mail]>
> Cc: Yinghai Lu <[e-mail]>
> ---
>  drivers/pci/host/pci-imx6.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index ca8c5de..8402e9a 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -321,6 +321,7 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
>  {
>  	int count = 0;
>  	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
> +	uint32_t tmp;
>  
>  	imx6_pcie_assert_core_reset(pp);
>  
> @@ -330,13 +331,23 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
>  
>  	dw_pcie_setup_rc(pp);
>  
> +	/*
> +	 * FIXME:
> +	 * Force Gen1 operation. In case the IP block is in Gen2 operation
> +	 * mode, it does not detect the PCIe switch at all.
> +	 */
> +	tmp = readl(pp->dbi_base + 0x7c);
> +	tmp &= ~0xf;
> +	tmp |= 0x1;
> +	writel(tmp, pp->dbi_base + 0x7c);
> +

Since you are forcing RC to work in GEN1, so you will not be able to
connect any device at GEN2 now.

Yes, There are some buggy PCIe devices which works with GEN1 only host.

So better solution should be to initialize RC by default at GEN2 or
highest speed. Further, a parameter say gen_only can be passed from
DT to force GEN1 only mode.

What do you say?

Regards
Pratyush

>  	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  			IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
>  
>  	while (!dw_pcie_link_up(pp)) {
>  		usleep_range(100, 1000);
>  		count++;
> -		if (count >= 10) {
> +		if (count >= 200) {
>  			dev_err(pp->dev, "phy link never came up\n");
>  			dev_dbg(pp->dev,
>  				"DEBUG_R0: 0x%08x, DEBUG_R1: 0x%08x\n",
> -- 
> 1.8.4.rc3
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Marek Vasut Oct. 16, 2013, 1:57 p.m. UTC | #2

Dear Pratyush Anand,

> On Wed, Oct 16, 2013 at 12:06:39AM +0800, Marek Vasut wrote:
> > Without forcing the PCIe core into Gen1 operation, the PCIe switch
> > attached directly to the PCIe port is not recognised at all. The
> > PCIe switch is Gen2 capable to make this issue even more puzzling.
> > 
> > Signed-off-by: Marek Vasut <[e-mail]>
> > Cc: Bjorn Helgaas <[e-mail]>
> > Cc: Frank Li <[e-mail]>
> > Cc: Jingoo Han <[e-mail]>
> > Cc: Mohit KUMAR <[e-mail]>
> > Cc: Pratyush Anand <[e-mail]>
> > Cc: Richard Zhu <[e-mail]>
> > Cc: Sascha Hauer <[e-mail]>
> > Cc: Sean Cross <[e-mail]>
> > Cc: Shawn Guo <[e-mail]>
> > Cc: Siva Reddy Kallam <[e-mail]>
> > Cc: Srikanth T Shivanand <[e-mail]>
> > Cc: Tim Harvey <[e-mail]>
> > Cc: Troy Kisky <[e-mail]>
> > Cc: Yinghai Lu <[e-mail]>
> > ---
> > 
> >  drivers/pci/host/pci-imx6.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> > index ca8c5de..8402e9a 100644
> > --- a/drivers/pci/host/pci-imx6.c
> > +++ b/drivers/pci/host/pci-imx6.c
> > @@ -321,6 +321,7 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
> > 
> >  {
> >  
> >  	int count = 0;
> >  	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
> > 
> > +	uint32_t tmp;
> > 
> >  	imx6_pcie_assert_core_reset(pp);
> > 
> > @@ -330,13 +331,23 @@ static void imx6_pcie_host_init(struct pcie_port
> > *pp)
> > 
> >  	dw_pcie_setup_rc(pp);
> > 
> > +	/*
> > +	 * FIXME:
> > +	 * Force Gen1 operation. In case the IP block is in Gen2 operation
> > +	 * mode, it does not detect the PCIe switch at all.
> > +	 */
> > +	tmp = readl(pp->dbi_base + 0x7c);
> > +	tmp &= ~0xf;
> > +	tmp |= 0x1;
> > +	writel(tmp, pp->dbi_base + 0x7c);
> > +
> 
> Since you are forcing RC to work in GEN1, so you will not be able to
> connect any device at GEN2 now.

I'd rather prefer to know WHY if the RC is operating in Gen2 mode by default, I 
cannot see the PCIe switch (Pericom PI7C9X2G303) connected to it. I was unable 
to figure this out. Tim pointed out this thread [1] , so it might be related in 
some way.

[1] https://community.freescale.com/message/316162#316162

I tried switching the PCIe RC to Gen2 operation right after LinkUp by writing 
this register with 0x2 again and re-issuing the LinkUp check, which passed and 
the status register reported Gen2 operation, but then the PCIe Intel NIC 
connected to the downstream port of the Pericom switch still reported Gen1 
operation for some reason.

> Yes, There are some buggy PCIe devices which works with GEN1 only host.

The pericom one is Gen2 according to it's datasheet.

> So better solution should be to initialize RC by default at GEN2 or
> highest speed. Further, a parameter say gen_only can be passed from
> DT to force GEN1 only mode.
> 
> What do you say?

I say I'd like to know the root cause of this problem. This Gen2 fix was pulled 
from one of the myriad variants of the FSL PCIe driver, so apparently this issue 
happens to other people as well. But why and how to properly fix this so the 
whole PCIe bus does operate in Gen2 mode, I cannot tell :-(
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Richard Zhu Oct. 17, 2013, 7:02 a.m. UTC | #3

Hi Marek:

> -----Original Message-----
[...]
> > > ---
> > >
> > >  drivers/pci/host/pci-imx6.c | 13 ++++++++++++-
> > >  1 file changed, 12 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/pci/host/pci-imx6.c
> > > b/drivers/pci/host/pci-imx6.c index ca8c5de..8402e9a 100644
> > > --- a/drivers/pci/host/pci-imx6.c
> > > +++ b/drivers/pci/host/pci-imx6.c
> > > @@ -321,6 +321,7 @@ static void imx6_pcie_host_init(struct pcie_port
> > > *pp)
> > >
> > >  {
> > >
> > >  	int count = 0;
> > >  	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
> > >
> > > +	uint32_t tmp;
> > >
> > >  	imx6_pcie_assert_core_reset(pp);
> > >
> > > @@ -330,13 +331,23 @@ static void imx6_pcie_host_init(struct
> > > pcie_port
> > > *pp)
> > >
> > >  	dw_pcie_setup_rc(pp);
> > >
> > > +	/*
> > > +	 * FIXME:
> > > +	 * Force Gen1 operation. In case the IP block is in Gen2 operation
> > > +	 * mode, it does not detect the PCIe switch at all.
> > > +	 */
> > > +	tmp = readl(pp->dbi_base + 0x7c);
> > > +	tmp &= ~0xf;
> > > +	tmp |= 0x1;
> > > +	writel(tmp, pp->dbi_base + 0x7c);
> > > +
> >
> > Since you are forcing RC to work in GEN1, so you will not be able to
> > connect any device at GEN2 now.
> 
> I'd rather prefer to know WHY if the RC is operating in Gen2 mode by default,
> I cannot see the PCIe switch (Pericom PI7C9X2G303) connected to it. I was
> unable to figure this out. Tim pointed out this thread [1] , so it might be
> related in some way.
> 
> [1] https://community.freescale.com/message/316162#316162
> 
> I tried switching the PCIe RC to Gen2 operation right after LinkUp by writing
> this register with 0x2 again and re-issuing the LinkUp check, which passed and
> the status register reported Gen2 operation, but then the PCIe Intel NIC
> connected to the downstream port of the Pericom switch still reported Gen1
> operation for some reason.
> 
 [Richard] Is your Inetl NIC GEN1 EP device?

> > Yes, There are some buggy PCIe devices which works with GEN1 only host.
> 
> The pericom one is Gen2 according to it's datasheet.
[Richard] Yes, it is. Pericom's PI7C9X2G303EL is GEN2 PCIe switch.
It had been verified on FSL's 3.0.35 releases.

> 
> > So better solution should be to initialize RC by default at GEN2 or
> > highest speed. Further, a parameter say gen_only can be passed from DT
> > to force GEN1 only mode.
> >
> > What do you say?
> 
> I say I'd like to know the root cause of this problem. This Gen2 fix was
> pulled from one of the myriad variants of the FSL PCIe driver, so apparently
> this issue happens to other people as well. But why and how to properly fix
> this so the whole PCIe bus does operate in Gen2 mode, I cannot tell :-(
[Richard] GEN2 mode had been verified in FSL 3.0.35 kernel releases before.
I don't know why it can't work in GEN2 mode in the latest kernel.
BTW, Based on for-next branch of shawn's git reops and applied Merak's patch-set,
 my PI7C9X2G303EL with intel e1000e network card plugged can't be detected at all. :(.

> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in the
> body of a message to [e-mail] More majordomo info at
> http://vger.kernel.org/majordomo-info.html


Best Regards
Richard Zhu

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Marek Vasut Oct. 17, 2013, 5:34 p.m. UTC | #4

Hi Richard,

> Hi Marek:
> > -----Original Message-----
> 
> [...]
> 
> > > > ---
> > > > 
> > > >  drivers/pci/host/pci-imx6.c | 13 ++++++++++++-
> > > >  1 file changed, 12 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/pci/host/pci-imx6.c
> > > > b/drivers/pci/host/pci-imx6.c index ca8c5de..8402e9a 100644
> > > > --- a/drivers/pci/host/pci-imx6.c
> > > > +++ b/drivers/pci/host/pci-imx6.c
> > > > @@ -321,6 +321,7 @@ static void imx6_pcie_host_init(struct pcie_port
> > > > *pp)
> > > > 
> > > >  {
> > > >  
> > > >  	int count = 0;
> > > >  	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
> > > > 
> > > > +	uint32_t tmp;
> > > > 
> > > >  	imx6_pcie_assert_core_reset(pp);
> > > > 
> > > > @@ -330,13 +331,23 @@ static void imx6_pcie_host_init(struct
> > > > pcie_port
> > > > *pp)
> > > > 
> > > >  	dw_pcie_setup_rc(pp);
> > > > 
> > > > +	/*
> > > > +	 * FIXME:
> > > > +	 * Force Gen1 operation. In case the IP block is in Gen2 
operation
> > > > +	 * mode, it does not detect the PCIe switch at all.
> > > > +	 */
> > > > +	tmp = readl(pp->dbi_base + 0x7c);
> > > > +	tmp &= ~0xf;
> > > > +	tmp |= 0x1;
> > > > +	writel(tmp, pp->dbi_base + 0x7c);
> > > > +
> > > 
> > > Since you are forcing RC to work in GEN1, so you will not be able to
> > > connect any device at GEN2 now.
> > 
> > I'd rather prefer to know WHY if the RC is operating in Gen2 mode by
> > default, I cannot see the PCIe switch (Pericom PI7C9X2G303) connected to
> > it. I was unable to figure this out. Tim pointed out this thread [1] ,
> > so it might be related in some way.
> > 
> > [1] https://community.freescale.com/message/316162#316162
> > 
> > I tried switching the PCIe RC to Gen2 operation right after LinkUp by
> > writing this register with 0x2 again and re-issuing the LinkUp check,
> > which passed and the status register reported Gen2 operation, but then
> > the PCIe Intel NIC connected to the downstream port of the Pericom
> > switch still reported Gen1 operation for some reason.
> 
>  [Richard] Is your Inetl NIC GEN1 EP device?

Yes, the i210 is PCIe v2.1 (2.5GT/s) x1 device.

> > > Yes, There are some buggy PCIe devices which works with GEN1 only host.
> > 
> > The pericom one is Gen2 according to it's datasheet.
> 
> [Richard] Yes, it is. Pericom's PI7C9X2G303EL is GEN2 PCIe switch.
> It had been verified on FSL's 3.0.35 releases.

Verified how? What does it mean?

Note that when I followed [1] in this manner:

1) Force GEN1 mode
2) Wait for PHY link up
3) Allow GEN2 mode
4) Start GEN2 speed change
5) Wait for PHY link up

my devices were detected and the link between pericom and mx6 was operating in 
gen2 mode. Thus I suspect we might need to start the port in Gen1 mode and 
switch to Gen2 only after link is up. Does that make sense?

https://community.freescale.com/thread/304284

> > > So better solution should be to initialize RC by default at GEN2 or
> > > highest speed. Further, a parameter say gen_only can be passed from DT
> > > to force GEN1 only mode.
> > > 
> > > What do you say?
> > 
> > I say I'd like to know the root cause of this problem. This Gen2 fix was
> > pulled from one of the myriad variants of the FSL PCIe driver, so
> > apparently this issue happens to other people as well. But why and how
> > to properly fix this so the whole PCIe bus does operate in Gen2 mode, I
> > cannot tell :-(
> 
> [Richard] GEN2 mode had been verified in FSL 3.0.35 kernel releases before.
> I don't know why it can't work in GEN2 mode in the latest kernel.
> BTW, Based on for-next branch of shawn's git reops and applied Merak's
> patch-set, my PI7C9X2G303EL with intel e1000e network card plugged can't
> be detected at all. :(.

I think we're still fighting some issue with resource mappings here. We also 
agreed with Tim that my patches are incomplete. I will send out a new version 
ASAP.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Richard Zhu Oct. 18, 2013, 2:12 a.m. UTC | #5

Hi Merak:
Thanks for your great help on the imx6 pcie switch support on the latest kernel fristly.

> -----Original Message-----
> From: Marek Vasut [mailto:[e-mail]]
> Sent: Friday, October 18, 2013 1:35 AM
> To: Zhu Richard-R65037
> Cc: Pratyush Anand; Bjorn Helgaas; [e-mail]; linux-arm-
> [e-mail]; Frank Li; Jingoo Han; Mohit KUMAR DCG; Sascha
> Hauer; Sean Cross; Shawn Guo; Siva Reddy Kallam; Srikanth T Shivanand; Tim
> Harvey; Troy Kisky; Yinghai Lu
> Subject: Re: [PATCH 5/6] PCI: imx6: Force Gen1 operation
> 
> Hi Richard,
> 
> > Hi Marek:
> > > -----Original Message-----
> >
> > [...]
> >
> > > > > ---
> > > > >
> > > > >  drivers/pci/host/pci-imx6.c | 13 ++++++++++++-
> > > > >  1 file changed, 12 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/pci/host/pci-imx6.c
> > > > > b/drivers/pci/host/pci-imx6.c index ca8c5de..8402e9a 100644
> > > > > --- a/drivers/pci/host/pci-imx6.c
> > > > > +++ b/drivers/pci/host/pci-imx6.c
> > > > > @@ -321,6 +321,7 @@ static void imx6_pcie_host_init(struct
> > > > > pcie_port
> > > > > *pp)
> > > > >
> > > > >  {
> > > > >
> > > > >  	int count = 0;
> > > > >  	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
> > > > >
> > > > > +	uint32_t tmp;
> > > > >
> > > > >  	imx6_pcie_assert_core_reset(pp);
> > > > >
> > > > > @@ -330,13 +331,23 @@ static void imx6_pcie_host_init(struct
> > > > > pcie_port
> > > > > *pp)
> > > > >
> > > > >  	dw_pcie_setup_rc(pp);
> > > > >
> > > > > +	/*
> > > > > +	 * FIXME:
> > > > > +	 * Force Gen1 operation. In case the IP block is in Gen2
> operation
> > > > > +	 * mode, it does not detect the PCIe switch at all.
> > > > > +	 */
> > > > > +	tmp = readl(pp->dbi_base + 0x7c);
> > > > > +	tmp &= ~0xf;
> > > > > +	tmp |= 0x1;
> > > > > +	writel(tmp, pp->dbi_base + 0x7c);
> > > > > +
> > > >
> > > > Since you are forcing RC to work in GEN1, so you will not be able
> > > > to connect any device at GEN2 now.
> > >
> > > I'd rather prefer to know WHY if the RC is operating in Gen2 mode by
> > > default, I cannot see the PCIe switch (Pericom PI7C9X2G303)
> > > connected to it. I was unable to figure this out. Tim pointed out
> > > this thread [1] , so it might be related in some way.
> > >
> > > [1] https://community.freescale.com/message/316162#316162
> > >
> > > I tried switching the PCIe RC to Gen2 operation right after LinkUp
> > > by writing this register with 0x2 again and re-issuing the LinkUp
> > > check, which passed and the status register reported Gen2 operation,
> > > but then the PCIe Intel NIC connected to the downstream port of the
> > > Pericom switch still reported Gen1 operation for some reason.
> >
> >  [Richard] Is your Inetl NIC GEN1 EP device?
> 
> Yes, the i210 is PCIe v2.1 (2.5GT/s) x1 device.
> 
> > > > Yes, There are some buggy PCIe devices which works with GEN1 only host.
> > >
> > > The pericom one is Gen2 according to it's datasheet.
> >
> > [Richard] Yes, it is. Pericom's PI7C9X2G303EL is GEN2 PCIe switch.
> > It had been verified on FSL's 3.0.35 releases.
> 
> Verified how? What does it mean?
> 
> Note that when I followed [1] in this manner:
> 
> 1) Force GEN1 mode
> 2) Wait for PHY link up
> 3) Allow GEN2 mode
> 4) Start GEN2 speed change
> 5) Wait for PHY link up
> 
> my devices were detected and the link between pericom and mx6 was operating in
> gen2 mode. Thus I suspect we might need to start the port in Gen1 mode and
> switch to Gen2 only after link is up. Does that make sense?
> 
> https://community.freescale.com/thread/304284
> 
[Richard] Yes, it make sense regarding to the https://community.freescale.com/thread/304284.

The verification I said above means that the Pericom's PI7C9X2G303EL PCIe GEN2 switch can 
work well on 3.0.35 kernel in GEN2 mode(without GEN1 force), and hand been tested by kinds
 of PCIe EP device(GEN1: Intel e1000e network card, GEN2 xHCI USB3.0 PCIe x1 card).

Regarding to your comments why FORCE GEN1 mode in RC " Force Gen1 operation. In case the IP block
is in Gen2 operation mode, it does not detect the PCIe switch at all.",
 does the gen2 link can be set-up without force gen1 mode?

About the errata mentioned in https://community.freescale.com/thread/304284,
 the errata-SW-workaround of initialization is contained in previous Sean Cross' codes, like this.
        /*
         * From L0, initiate MAC entry to gen2 if EP/RC supports gen2.
         * Wait 2ms (LTSSM timeout is 24ms, PHY lock is ~5us in gen2).
         * If (MAC/LTSSM.state == Recovery.RcvrLock)
         * && (PHY/rx_valid==0) then pulse PHY/rx_reset. Transition
         * to gen2 is stuck
         */

> > > > So better solution should be to initialize RC by default at GEN2
> > > > or highest speed. Further, a parameter say gen_only can be passed
> > > > from DT to force GEN1 only mode.
> > > >
> > > > What do you say?
> > >
> > > I say I'd like to know the root cause of this problem. This Gen2 fix
> > > was pulled from one of the myriad variants of the FSL PCIe driver,
> > > so apparently this issue happens to other people as well. But why
> > > and how to properly fix this so the whole PCIe bus does operate in
> > > Gen2 mode, I cannot tell :-(
> >
> > [Richard] GEN2 mode had been verified in FSL 3.0.35 kernel releases before.
> > I don't know why it can't work in GEN2 mode in the latest kernel.
> > BTW, Based on for-next branch of shawn's git reops and applied Merak's
> > patch-set, my PI7C9X2G303EL with intel e1000e network card plugged
> > can't be detected at all. :(.
> 
> I think we're still fighting some issue with resource mappings here. We also
> agreed with Tim that my patches are incomplete. I will send out a new version
> ASAP.
[Richard] What's I said above is just curious that I encounter different phenomena at my side
With the almost same codes.

Thanks again.

Best Regards
Richard Zhu


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Tim Harvey Oct. 18, 2013, 5:04 a.m. UTC | #6

On Tue, Oct 15, 2013 at 9:06 AM, Marek Vasut <[e-mail]> wrote:
> Without forcing the PCIe core into Gen1 operation, the PCIe switch
> attached directly to the PCIe port is not recognised at all. The
> PCIe switch is Gen2 capable to make this issue even more puzzling.
>
[...]
>
>         while (!dw_pcie_link_up(pp)) {
>                 usleep_range(100, 1000);
>                 count++;
> -               if (count >= 10) {
> +               if (count >= 200) {
>                         dev_err(pp->dev, "phy link never came up\n");
>                         dev_dbg(pp->dev,
>                                 "DEBUG_R0: 0x%08x, DEBUG_R1: 0x%08x\n",
> --
> 1.8.4.rc3
>

Marek,

Can you split this patch out?  I found that with the PLX PEX860x PCIe
switches I have on the i.MX6 RC I needed this longer wait for link as
well, but I don't think this is related to gen1/gen2.

Thanks,

Tim
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Marek Vasut Oct. 19, 2013, 5:07 a.m. UTC | #7

Hi Richard,

> Hi Merak:
> Thanks for your great help on the imx6 pcie switch support on the latest
> kernel fristly.

Let's get this PCIe thing working well :)

[...]

> > > > > Since you are forcing RC to work in GEN1, so you will not be able
> > > > > to connect any device at GEN2 now.
> > > > 
> > > > I'd rather prefer to know WHY if the RC is operating in Gen2 mode by
> > > > default, I cannot see the PCIe switch (Pericom PI7C9X2G303)
> > > > connected to it. I was unable to figure this out. Tim pointed out
> > > > this thread [1] , so it might be related in some way.
> > > > 
> > > > [1] https://community.freescale.com/message/316162#316162
> > > > 
> > > > I tried switching the PCIe RC to Gen2 operation right after LinkUp
> > > > by writing this register with 0x2 again and re-issuing the LinkUp
> > > > check, which passed and the status register reported Gen2 operation,
> > > > but then the PCIe Intel NIC connected to the downstream port of the
> > > > Pericom switch still reported Gen1 operation for some reason.
> > >  
> > >  [Richard] Is your Inetl NIC GEN1 EP device?
> > 
> > Yes, the i210 is PCIe v2.1 (2.5GT/s) x1 device.
> > 
> > > > > Yes, There are some buggy PCIe devices which works with GEN1 only
> > > > > host.
> > > > 
> > > > The pericom one is Gen2 according to it's datasheet.
> > > 
> > > [Richard] Yes, it is. Pericom's PI7C9X2G303EL is GEN2 PCIe switch.
> > > It had been verified on FSL's 3.0.35 releases.
> > 
> > Verified how? What does it mean?
> > 
> > Note that when I followed [1] in this manner:
> > 
> > 1) Force GEN1 mode
> > 2) Wait for PHY link up
> > 3) Allow GEN2 mode
> > 4) Start GEN2 speed change
> > 5) Wait for PHY link up
> > 
> > my devices were detected and the link between pericom and mx6 was
> > operating in gen2 mode. Thus I suspect we might need to start the port
> > in Gen1 mode and switch to Gen2 only after link is up. Does that make
> > sense?
> > 
> > https://community.freescale.com/thread/304284
> 
> [Richard] Yes, it make sense regarding to the
> https://community.freescale.com/thread/304284.
> 
> The verification I said above means that the Pericom's PI7C9X2G303EL PCIe
> GEN2 switch can work well on 3.0.35 kernel in GEN2 mode(without GEN1
> force), and hand been tested by kinds of PCIe EP device(GEN1: Intel e1000e
> network card, GEN2 xHCI USB3.0 PCIe x1 card).

Oh, thanks for verifying this!

> Regarding to your comments why FORCE GEN1 mode in RC " Force Gen1
> operation. In case the IP block is in Gen2 operation mode, it does not
> detect the PCIe switch at all.", does the gen2 link can be set-up without
> force gen1 mode?

What I did in the end for testing purpose was I followed the thread at FSL 
community in the link above, forced the link into Gen1 mode , then waited for 
link up, then started directed gen2 switch and waited for linkup again. This 
worked and the PCIe indicated the link is in Gen2 mode.

Interestingly enough, I also implemented PCIe driver for MX6 for U-Boot (will 
submit it shortly to U-Boot ML) and use it with the Intel i210 ethernet NIC now. 
When I use the PCIe in U-Boot, I don't need the Force-Gen1 thing above. I 
suspect though, that the Gen1 setting is preserved from U-Boot, where I have 
this as well.

> About the errata mentioned in
> https://community.freescale.com/thread/304284, the errata-SW-workaround of
> initialization is contained in previous Sean Cross' codes, like this. /*
>          * From L0, initiate MAC entry to gen2 if EP/RC supports gen2.
>          * Wait 2ms (LTSSM timeout is 24ms, PHY lock is ~5us in gen2).
>          * If (MAC/LTSSM.state == Recovery.RcvrLock)
>          * && (PHY/rx_valid==0) then pulse PHY/rx_reset. Transition
>          * to gen2 is stuck
>          */

Is this not handling only the PHY part ? Honestly, I'm quite lost in the flurry 
of different opinions on the MX6 PCIe. Maybe you as a FSL guy could scrounge 
some internal knowledge ?

> > > > > So better solution should be to initialize RC by default at GEN2
> > > > > or highest speed. Further, a parameter say gen_only can be passed
> > > > > from DT to force GEN1 only mode.
> > > > > 
> > > > > What do you say?
> > > > 
> > > > I say I'd like to know the root cause of this problem. This Gen2 fix
> > > > was pulled from one of the myriad variants of the FSL PCIe driver,
> > > > so apparently this issue happens to other people as well. But why
> > > > and how to properly fix this so the whole PCIe bus does operate in
> > > > Gen2 mode, I cannot tell :-(
> > > 
> > > [Richard] GEN2 mode had been verified in FSL 3.0.35 kernel releases
> > > before. I don't know why it can't work in GEN2 mode in the latest
> > > kernel. BTW, Based on for-next branch of shawn's git reops and applied
> > > Merak's patch-set, my PI7C9X2G303EL with intel e1000e network card
> > > plugged can't be detected at all. :(.
> > 
> > I think we're still fighting some issue with resource mappings here. We
> > also agreed with Tim that my patches are incomplete. I will send out a
> > new version ASAP.
> 
> [Richard] What's I said above is just curious that I encounter different
> phenomena at my side With the almost same codes.

I talked to Tim about the mappings some more. It seems if the IOspace is 
disabled, everything works without hangs. If the IOspace is not disabled on the 
ethernet NICs, we get a hang.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Richard Zhu Oct. 21, 2013, 6:33 a.m. UTC | #8

Hi Merak:
> Hi Richard,
> 
> > Hi Merak:
> > Thanks for your great help on the imx6 pcie switch support on the
> > latest kernel fristly.
> 
> Let's get this PCIe thing working well :)
> 
> [...]
> 
> 
> > Regarding to your comments why FORCE GEN1 mode in RC " Force Gen1
> > operation. In case the IP block is in Gen2 operation mode, it does not
> > detect the PCIe switch at all.", does the gen2 link can be set-up
> > without force gen1 mode?
> 
> What I did in the end for testing purpose was I followed the thread at FSL
> community in the link above, forced the link into Gen1 mode , then waited for
> link up, then started directed gen2 switch and waited for linkup again. This
> worked and the PCIe indicated the link is in Gen2 mode.
> 
> Interestingly enough, I also implemented PCIe driver for MX6 for U-Boot (will
> submit it shortly to U-Boot ML) and use it with the Intel i210 ethernet NIC
> now.
> When I use the PCIe in U-Boot, I don't need the Force-Gen1 thing above. I
> suspect though, that the Gen1 setting is preserved from U-Boot, where I have
> this as well.
> 
[Richard] It's great, thanks.
> > About the errata mentioned in
> > https://community.freescale.com/thread/304284, the
> > errata-SW-workaround of initialization is contained in previous Sean Cross'
> codes, like this. /*
> >          * From L0, initiate MAC entry to gen2 if EP/RC supports gen2.
> >          * Wait 2ms (LTSSM timeout is 24ms, PHY lock is ~5us in gen2).
> >          * If (MAC/LTSSM.state == Recovery.RcvrLock)
> >          * && (PHY/rx_valid==0) then pulse PHY/rx_reset. Transition
> >          * to gen2 is stuck
> >          */
> 
> Is this not handling only the PHY part ? Honestly, I'm quite lost in the
> flurry of different opinions on the MX6 PCIe. Maybe you as a FSL guy could
> scrounge some internal knowledge ?
> 
[Richard] Take it easy, maybe there is a misunderstand between you and me.
It is handling only the PHY part.
The errata "PCIe: Clock pointers can lose sync during clock rate changes",
 I mentioned previously is discussed in 
(https://community.freescale.com/message/316162#316162) too.

Charies Powe think that the SW workaround contained in FSL BSP is not complete for the errata.
Because I couldn't reproduce this issue at my side, the complete version of the SW workaround
of this errata is not included in the BSP yet.
That's all. Thanks.

> > > > > > So better solution should be to initialize RC by default at
> > > > > > GEN2 or highest speed. Further, a parameter say gen_only can
> > > > > > be passed from DT to force GEN1 only mode.
> > > > > >
> > > > > > What do you say?
> > > > >
> > > > > I say I'd like to know the root cause of this problem. This Gen2
> > > > > fix was pulled from one of the myriad variants of the FSL PCIe
> > > > > driver, so apparently this issue happens to other people as
> > > > > well. But why and how to properly fix this so the whole PCIe bus
> > > > > does operate in
> > > > > Gen2 mode, I cannot tell :-(
> > > >
> > > > [Richard] GEN2 mode had been verified in FSL 3.0.35 kernel
> > > > releases before. I don't know why it can't work in GEN2 mode in
> > > > the latest kernel. BTW, Based on for-next branch of shawn's git
> > > > reops and applied Merak's patch-set, my PI7C9X2G303EL with intel
> > > > e1000e network card plugged can't be detected at all. :(.
> > >
> > > I think we're still fighting some issue with resource mappings here.
> > > We also agreed with Tim that my patches are incomplete. I will send
> > > out a new version ASAP.
> >
> > [Richard] What's I said above is just curious that I encounter
> > different phenomena at my side With the almost same codes.
> 
> I talked to Tim about the mappings some more. It seems if the IOspace is
> disabled, everything works without hangs. If the IOspace is not disabled on
> the ethernet NICs, we get a hang.
[Richard] Ok, I see. Thanks.


Best Regards
Richard.


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index ca8c5de..8402e9a 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -321,6 +321,7 @@  static void imx6_pcie_host_init(struct pcie_port *pp)
 {
 	int count = 0;
 	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
+	uint32_t tmp;
 
 	imx6_pcie_assert_core_reset(pp);
 
@@ -330,13 +331,23 @@  static void imx6_pcie_host_init(struct pcie_port *pp)
 
 	dw_pcie_setup_rc(pp);
 
+	/*
+	 * FIXME:
+	 * Force Gen1 operation. In case the IP block is in Gen2 operation
+	 * mode, it does not detect the PCIe switch at all.
+	 */
+	tmp = readl(pp->dbi_base + 0x7c);
+	tmp &= ~0xf;
+	tmp |= 0x1;
+	writel(tmp, pp->dbi_base + 0x7c);
+
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 			IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
 
 	while (!dw_pcie_link_up(pp)) {
 		usleep_range(100, 1000);
 		count++;
-		if (count >= 10) {
+		if (count >= 200) {
 			dev_err(pp->dev, "phy link never came up\n");
 			dev_dbg(pp->dev,
 				"DEBUG_R0: 0x%08x, DEBUG_R1: 0x%08x\n",