linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] PCI: amlogic: meson: Don't use FAST_LINK_MODE to set up link
@ 2020-04-29 16:42 Marc Zyngier
  2020-05-06 10:43 ` Marc Zyngier
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Marc Zyngier @ 2020-04-29 16:42 UTC (permalink / raw)
  To: linux-pci, linux-amlogic, linux-arm-kernel, linux-kernel
  Cc: Yue Wang, Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, Kevin Hilman

My vim3l board stubbornly refuses to play ball with a bog
standard PCIe switch (ASM1184e), spitting all kind of errors
ranging from link never coming up to crazy things like downstream
ports falling off the face of the planet.

Upon investigating how the PCIe RC is configured, I found the
following nugget: the Sysnopsys DWC PCIe Reference Manual, in the
section dedicated to the PLCR register, describes bit 7 (FAST_LINK_MODE)
as:

"Sets all internal timers to fast mode for simulation purposes."

I completely understand the need for setting this bit from a simulation
perspective, but what I have on my desk is actual silicon, which
expects timers to have a nominal value (and I expect this is the
case for most people).

Making sure the FAST_LINK_MODE bit is cleared when configuring the RC
solves this problem.

Fixes: 9c0ef6d34fdb ("PCI: amlogic: Add the Amlogic Meson PCIe controller driver")
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/controller/dwc/pci-meson.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
index 3715dceca1bf..ca59ba9e0ecd 100644
--- a/drivers/pci/controller/dwc/pci-meson.c
+++ b/drivers/pci/controller/dwc/pci-meson.c
@@ -289,11 +289,11 @@ static void meson_pcie_init_dw(struct meson_pcie *mp)
 	meson_cfg_writel(mp, val, PCIE_CFG0);
 
 	val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
-	val &= ~LINK_CAPABLE_MASK;
+	val &= ~(LINK_CAPABLE_MASK | FAST_LINK_MODE);
 	meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
 
 	val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
-	val |= LINK_CAPABLE_X1 | FAST_LINK_MODE;
+	val |= LINK_CAPABLE_X1;
 	meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
 
 	val = meson_elb_readl(mp, PCIE_GEN2_CTRL_OFF);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] PCI: amlogic: meson: Don't use FAST_LINK_MODE to set up link
  2020-04-29 16:42 [PATCH] PCI: amlogic: meson: Don't use FAST_LINK_MODE to set up link Marc Zyngier
@ 2020-05-06 10:43 ` Marc Zyngier
  2020-05-07 21:03 ` Rob Herring
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Marc Zyngier @ 2020-05-06 10:43 UTC (permalink / raw)
  To: linux-pci, linux-amlogic, linux-arm-kernel, linux-kernel,
	Kevin Hilman, Yue Wang
  Cc: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi

On Wed, 29 Apr 2020 17:42:30 +0100
Marc Zyngier <maz@kernel.org> wrote:

> My vim3l board stubbornly refuses to play ball with a bog
> standard PCIe switch (ASM1184e), spitting all kind of errors
> ranging from link never coming up to crazy things like downstream
> ports falling off the face of the planet.
> 
> Upon investigating how the PCIe RC is configured, I found the
> following nugget: the Sysnopsys DWC PCIe Reference Manual, in the
> section dedicated to the PLCR register, describes bit 7 (FAST_LINK_MODE)
> as:
> 
> "Sets all internal timers to fast mode for simulation purposes."
> 
> I completely understand the need for setting this bit from a simulation
> perspective, but what I have on my desk is actual silicon, which
> expects timers to have a nominal value (and I expect this is the
> case for most people).
> 
> Making sure the FAST_LINK_MODE bit is cleared when configuring the RC
> solves this problem.
> 
> Fixes: 9c0ef6d34fdb ("PCI: amlogic: Add the Amlogic Meson PCIe controller driver")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/pci/controller/dwc/pci-meson.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> index 3715dceca1bf..ca59ba9e0ecd 100644
> --- a/drivers/pci/controller/dwc/pci-meson.c
> +++ b/drivers/pci/controller/dwc/pci-meson.c
> @@ -289,11 +289,11 @@ static void meson_pcie_init_dw(struct meson_pcie *mp)
>  	meson_cfg_writel(mp, val, PCIE_CFG0);
>  
>  	val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
> -	val &= ~LINK_CAPABLE_MASK;
> +	val &= ~(LINK_CAPABLE_MASK | FAST_LINK_MODE);
>  	meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
>  
>  	val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
> -	val |= LINK_CAPABLE_X1 | FAST_LINK_MODE;
> +	val |= LINK_CAPABLE_X1;
>  	meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
>  
>  	val = meson_elb_readl(mp, PCIE_GEN2_CTRL_OFF);

Yue, Kevin: any comment on this?

I found that the issue is reproducible even without a PCIe switch,
depending on the single device I plug in this machine (an Intel SSD
works fine, while a Marvell Ethernet adapter never shows up) as the
LTSSM times out much earlier than it really should (HW timers running
too quickly). Applying this patch makes every single device I have
lying around work fine.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] PCI: amlogic: meson: Don't use FAST_LINK_MODE to set up link
  2020-04-29 16:42 [PATCH] PCI: amlogic: meson: Don't use FAST_LINK_MODE to set up link Marc Zyngier
  2020-05-06 10:43 ` Marc Zyngier
@ 2020-05-07 21:03 ` Rob Herring
       [not found] ` <2020050709433087691414@amlogic.com>
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Rob Herring @ 2020-05-07 21:03 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-pci, linux-amlogic, linux-arm-kernel, linux-kernel,
	Bjorn Helgaas, Lorenzo Pieralisi, Kevin Hilman, Yue Wang

On Wed, 29 Apr 2020 17:42:30 +0100, Marc Zyngier wrote:
> My vim3l board stubbornly refuses to play ball with a bog
> standard PCIe switch (ASM1184e), spitting all kind of errors
> ranging from link never coming up to crazy things like downstream
> ports falling off the face of the planet.
> 
> Upon investigating how the PCIe RC is configured, I found the
> following nugget: the Sysnopsys DWC PCIe Reference Manual, in the
> section dedicated to the PLCR register, describes bit 7 (FAST_LINK_MODE)
> as:
> 
> "Sets all internal timers to fast mode for simulation purposes."
> 
> I completely understand the need for setting this bit from a simulation
> perspective, but what I have on my desk is actual silicon, which
> expects timers to have a nominal value (and I expect this is the
> case for most people).
> 
> Making sure the FAST_LINK_MODE bit is cleared when configuring the RC
> solves this problem.
> 
> Fixes: 9c0ef6d34fdb ("PCI: amlogic: Add the Amlogic Meson PCIe controller driver")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/pci/controller/dwc/pci-meson.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] PCI: amlogic: meson: Don't use FAST_LINK_MODE to set up link
       [not found] ` <2020050709433087691414@amlogic.com>
@ 2020-05-09 13:07   ` Marc Zyngier
  0 siblings, 0 replies; 6+ messages in thread
From: Marc Zyngier @ 2020-05-09 13:07 UTC (permalink / raw)
  To: yue.wang
  Cc: linux-pci, linux-amlogic, linux-arm-kernel, linux-kernel,
	Kevin Hilman, Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi

Hi Yue,

On Thu, 07 May 2020 02:43:31 +0100,
"yue.wang@amlogic.com" <yue.wang@amlogic.com> wrote:
> 
> [1  <text/plain; utf-8 (base64)>]
> Marc,
> 
> This patch looks all right. I tested in my meson board and pcie
> EP(QCA9888) worked well.
> 
> Fast link mode is enabled for simulation purposes, it should be
> disabled in the real hardware.

Thanks for confirming my reading of the manual and having tested it.
Can I take this as an "Acked-by:" and a "Tested-by:"?

Cheers,

	M.

>
> yue.wang@amlogic.com
>  
> From: Marc Zyngier
> Date: 2020-05-06 18:43
> To: linux-pci; linux-amlogic; linux-arm-kernel; linux-kernel; Kevin Hilman; Yue Wang
> CC: Bjorn Helgaas; Rob Herring; Lorenzo Pieralisi
> Subject: Re: [PATCH] PCI: amlogic: meson: Don't use FAST_LINK_MODE to set up link
> On Wed, 29 Apr 2020 17:42:30 +0100
> Marc Zyngier <maz@kernel.org> wrote:
>  
> > My vim3l board stubbornly refuses to play ball with a bog
> > standard PCIe switch (ASM1184e), spitting all kind of errors
> > ranging from link never coming up to crazy things like downstream
> > ports falling off the face of the planet.
> > 
> > Upon investigating how the PCIe RC is configured, I found the
> > following nugget: the Sysnopsys DWC PCIe Reference Manual, in the
> > section dedicated to the PLCR register, describes bit 7 (FAST_LINK_MODE)
> > as:
> > 
> > "Sets all internal timers to fast mode for simulation purposes."
> > 
> > I completely understand the need for setting this bit from a simulation
> > perspective, but what I have on my desk is actual silicon, which
> > expects timers to have a nominal value (and I expect this is the
> > case for most people).
> > 
> > Making sure the FAST_LINK_MODE bit is cleared when configuring the RC
> > solves this problem.
> > 
> > Fixes: 9c0ef6d34fdb ("PCI: amlogic: Add the Amlogic Meson PCIe controller driver")
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  drivers/pci/controller/dwc/pci-meson.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> > index 3715dceca1bf..ca59ba9e0ecd 100644
> > --- a/drivers/pci/controller/dwc/pci-meson.c
> > +++ b/drivers/pci/controller/dwc/pci-meson.c
> > @@ -289,11 +289,11 @@ static void meson_pcie_init_dw(struct meson_pcie *mp)
> >  meson_cfg_writel(mp, val, PCIE_CFG0);
> >  
> >  val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
> > - val &= ~LINK_CAPABLE_MASK;
> > + val &= ~(LINK_CAPABLE_MASK | FAST_LINK_MODE);
> >  meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
> >  
> >  val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
> > - val |= LINK_CAPABLE_X1 | FAST_LINK_MODE;
> > + val |= LINK_CAPABLE_X1;
> >  meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
> >  
> >  val = meson_elb_readl(mp, PCIE_GEN2_CTRL_OFF);
>  
> Yue, Kevin: any comment on this?
>  
> I found that the issue is reproducible even without a PCIe switch,
> depending on the single device I plug in this machine (an Intel SSD
> works fine, while a Marvell Ethernet adapter never shows up) as the
> LTSSM times out much earlier than it really should (HW timers running
> too quickly). Applying this patch makes every single device I have
> lying around work fine.
>  
> Thanks,
>  
> M.
> -- 
> Jazz is not dead. It just smells funny...
>  
> [2  <text/html; utf-8 (quoted-printable)>]

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] PCI: amlogic: meson: Don't use FAST_LINK_MODE to set up link
  2020-04-29 16:42 [PATCH] PCI: amlogic: meson: Don't use FAST_LINK_MODE to set up link Marc Zyngier
                   ` (2 preceding siblings ...)
       [not found] ` <2020050709433087691414@amlogic.com>
@ 2020-05-11  8:58 ` Neil Armstrong
  2020-05-11 10:22 ` Lorenzo Pieralisi
  4 siblings, 0 replies; 6+ messages in thread
From: Neil Armstrong @ 2020-05-11  8:58 UTC (permalink / raw)
  To: Marc Zyngier, linux-pci, linux-amlogic, linux-arm-kernel, linux-kernel
  Cc: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi, Kevin Hilman, Yue Wang

On 29/04/2020 18:42, Marc Zyngier wrote:
> My vim3l board stubbornly refuses to play ball with a bog
> standard PCIe switch (ASM1184e), spitting all kind of errors
> ranging from link never coming up to crazy things like downstream
> ports falling off the face of the planet.
> 
> Upon investigating how the PCIe RC is configured, I found the
> following nugget: the Sysnopsys DWC PCIe Reference Manual, in the
> section dedicated to the PLCR register, describes bit 7 (FAST_LINK_MODE)
> as:
> 
> "Sets all internal timers to fast mode for simulation purposes."
> 
> I completely understand the need for setting this bit from a simulation
> perspective, but what I have on my desk is actual silicon, which
> expects timers to have a nominal value (and I expect this is the
> case for most people).
> 
> Making sure the FAST_LINK_MODE bit is cleared when configuring the RC
> solves this problem.
> 
> Fixes: 9c0ef6d34fdb ("PCI: amlogic: Add the Amlogic Meson PCIe controller driver")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/pci/controller/dwc/pci-meson.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> index 3715dceca1bf..ca59ba9e0ecd 100644
> --- a/drivers/pci/controller/dwc/pci-meson.c
> +++ b/drivers/pci/controller/dwc/pci-meson.c
> @@ -289,11 +289,11 @@ static void meson_pcie_init_dw(struct meson_pcie *mp)
>  	meson_cfg_writel(mp, val, PCIE_CFG0);
>  
>  	val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
> -	val &= ~LINK_CAPABLE_MASK;
> +	val &= ~(LINK_CAPABLE_MASK | FAST_LINK_MODE);
>  	meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
>  
>  	val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
> -	val |= LINK_CAPABLE_X1 | FAST_LINK_MODE;
> +	val |= LINK_CAPABLE_X1;
>  	meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
>  
>  	val = meson_elb_readl(mp, PCIE_GEN2_CTRL_OFF);
> 

I don't have HW to test on non NVMe, but I'm reading the same as you in the
DWC PCIe Reference Manual, and it seems coherent.

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Neil

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] PCI: amlogic: meson: Don't use FAST_LINK_MODE to set up link
  2020-04-29 16:42 [PATCH] PCI: amlogic: meson: Don't use FAST_LINK_MODE to set up link Marc Zyngier
                   ` (3 preceding siblings ...)
  2020-05-11  8:58 ` Neil Armstrong
@ 2020-05-11 10:22 ` Lorenzo Pieralisi
  4 siblings, 0 replies; 6+ messages in thread
From: Lorenzo Pieralisi @ 2020-05-11 10:22 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-pci, linux-amlogic, linux-arm-kernel, linux-kernel,
	Yue Wang, Rob Herring, Bjorn Helgaas, Kevin Hilman

On Wed, Apr 29, 2020 at 05:42:30PM +0100, Marc Zyngier wrote:
> My vim3l board stubbornly refuses to play ball with a bog
> standard PCIe switch (ASM1184e), spitting all kind of errors
> ranging from link never coming up to crazy things like downstream
> ports falling off the face of the planet.
> 
> Upon investigating how the PCIe RC is configured, I found the
> following nugget: the Sysnopsys DWC PCIe Reference Manual, in the
> section dedicated to the PLCR register, describes bit 7 (FAST_LINK_MODE)
> as:
> 
> "Sets all internal timers to fast mode for simulation purposes."
> 
> I completely understand the need for setting this bit from a simulation
> perspective, but what I have on my desk is actual silicon, which
> expects timers to have a nominal value (and I expect this is the
> case for most people).
> 
> Making sure the FAST_LINK_MODE bit is cleared when configuring the RC
> solves this problem.
> 
> Fixes: 9c0ef6d34fdb ("PCI: amlogic: Add the Amlogic Meson PCIe controller driver")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/pci/controller/dwc/pci-meson.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reworded the commit log (even if yours was more fun :)) and applied
to pci/dwc, thanks !

Lorenzo

> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> index 3715dceca1bf..ca59ba9e0ecd 100644
> --- a/drivers/pci/controller/dwc/pci-meson.c
> +++ b/drivers/pci/controller/dwc/pci-meson.c
> @@ -289,11 +289,11 @@ static void meson_pcie_init_dw(struct meson_pcie *mp)
>  	meson_cfg_writel(mp, val, PCIE_CFG0);
>  
>  	val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
> -	val &= ~LINK_CAPABLE_MASK;
> +	val &= ~(LINK_CAPABLE_MASK | FAST_LINK_MODE);
>  	meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
>  
>  	val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
> -	val |= LINK_CAPABLE_X1 | FAST_LINK_MODE;
> +	val |= LINK_CAPABLE_X1;
>  	meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
>  
>  	val = meson_elb_readl(mp, PCIE_GEN2_CTRL_OFF);
> -- 
> 2.26.2
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-05-11 10:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-29 16:42 [PATCH] PCI: amlogic: meson: Don't use FAST_LINK_MODE to set up link Marc Zyngier
2020-05-06 10:43 ` Marc Zyngier
2020-05-07 21:03 ` Rob Herring
     [not found] ` <2020050709433087691414@amlogic.com>
2020-05-09 13:07   ` Marc Zyngier
2020-05-11  8:58 ` Neil Armstrong
2020-05-11 10:22 ` Lorenzo Pieralisi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).