linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] PCI: aardvark: Update comment about disabling link training
@ 2020-09-24  8:46 Pali Rohár
  2020-09-24 15:11 ` Bjorn Helgaas
  2020-12-02 18:46 ` [PATCH v2] " Pali Rohár
  0 siblings, 2 replies; 9+ messages in thread
From: Pali Rohár @ 2020-09-24  8:46 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas; +Cc: linux-pci, linux-kernel

It is not HW bug or workaround for some cards but it is requirement by PCI
Express spec. After fundamental reset is needed 100ms delay prior enabling
link training. So update comment in code to reflect this requirement.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pci/controller/pci-aardvark.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index 50ab6d7519ae..19b9b79226e5 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -259,7 +259,12 @@ static void advk_pcie_issue_perst(struct advk_pcie *pcie)
 	if (!pcie->reset_gpio)
 		return;
 
-	/* PERST does not work for some cards when link training is enabled */
+	/*
+	 * As required by PCI Express spec a delay for at least 100ms after
+	 * de-asserting PERST# signal is needed before link training is enabled.
+	 * So ensure that link training is disabled prior de-asserting PERST#
+	 * signal to fulfill that PCI Express spec requirement.
+	 */
 	reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
 	reg &= ~LINK_TRAINING_EN;
 	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
-- 
2.20.1


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

* Re: [PATCH] PCI: aardvark: Update comment about disabling link training
  2020-09-24  8:46 [PATCH] PCI: aardvark: Update comment about disabling link training Pali Rohár
@ 2020-09-24 15:11 ` Bjorn Helgaas
  2020-09-24 15:22   ` Pali Rohár
  2020-12-02 18:46 ` [PATCH v2] " Pali Rohár
  1 sibling, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2020-09-24 15:11 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, linux-pci, linux-kernel

On Thu, Sep 24, 2020 at 10:46:18AM +0200, Pali Rohár wrote:
> It is not HW bug or workaround for some cards but it is requirement by PCI
> Express spec. After fundamental reset is needed 100ms delay prior enabling
> link training. So update comment in code to reflect this requirement.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  drivers/pci/controller/pci-aardvark.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index 50ab6d7519ae..19b9b79226e5 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -259,7 +259,12 @@ static void advk_pcie_issue_perst(struct advk_pcie *pcie)
>  	if (!pcie->reset_gpio)
>  		return;
>  
> -	/* PERST does not work for some cards when link training is enabled */
> +	/*
> +	 * As required by PCI Express spec a delay for at least 100ms after
> +	 * de-asserting PERST# signal is needed before link training is enabled.
> +	 * So ensure that link training is disabled prior de-asserting PERST#
> +	 * signal to fulfill that PCI Express spec requirement.

Can you please include the spec citation here?  In the PCIe base spec,
PERST# is only mentioned in PCIe r5.0, sec 6.6.1, and I don't see the
connection there to 100ms between de-assert of PERST# and enabling
link training.

Sec 6.1.1 does talk about 100ms before sending config requests (for
ports that support <= 5 GT/s), and 100ms after link training completes
(for ports that support > 5 GT/s).

Maybe there's more language in a form-factor spec or something?

> +	 */
>  	reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
>  	reg &= ~LINK_TRAINING_EN;
>  	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
> -- 
> 2.20.1
> 

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

* Re: [PATCH] PCI: aardvark: Update comment about disabling link training
  2020-09-24 15:11 ` Bjorn Helgaas
@ 2020-09-24 15:22   ` Pali Rohár
  2020-10-11 17:21     ` Pali Rohár
  0 siblings, 1 reply; 9+ messages in thread
From: Pali Rohár @ 2020-09-24 15:22 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, linux-pci, linux-kernel

On Thursday 24 September 2020 10:11:06 Bjorn Helgaas wrote:
> On Thu, Sep 24, 2020 at 10:46:18AM +0200, Pali Rohár wrote:
> > It is not HW bug or workaround for some cards but it is requirement by PCI
> > Express spec. After fundamental reset is needed 100ms delay prior enabling
> > link training. So update comment in code to reflect this requirement.
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  drivers/pci/controller/pci-aardvark.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> > index 50ab6d7519ae..19b9b79226e5 100644
> > --- a/drivers/pci/controller/pci-aardvark.c
> > +++ b/drivers/pci/controller/pci-aardvark.c
> > @@ -259,7 +259,12 @@ static void advk_pcie_issue_perst(struct advk_pcie *pcie)
> >  	if (!pcie->reset_gpio)
> >  		return;
> >  
> > -	/* PERST does not work for some cards when link training is enabled */
> > +	/*
> > +	 * As required by PCI Express spec a delay for at least 100ms after
> > +	 * de-asserting PERST# signal is needed before link training is enabled.
> > +	 * So ensure that link training is disabled prior de-asserting PERST#
> > +	 * signal to fulfill that PCI Express spec requirement.
> 
> Can you please include the spec citation here?  In the PCIe base spec,
> PERST# is only mentioned in PCIe r5.0, sec 6.6.1, and I don't see the
> connection there to 100ms between de-assert of PERST# and enabling
> link training.

Hello! I copied this "comment" from other place in pci-aardvark.c where
that timeout 100ms is already applied. Timeout with explanation comment
was introduced in following commit:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f4c7d053d7f7

Here are links to discussions about that patch:

https://lore.kernel.org/linux-pci/20190313213752.1246-1-repk@triplefau.lt/T/#u
https://lore.kernel.org/linux-pci/20190522213351.21366-2-repk@triplefau.lt/T/#u

> Sec 6.1.1 does talk about 100ms before sending config requests (for
> ports that support <= 5 GT/s), and 100ms after link training completes
> (for ports that support > 5 GT/s).
> 
> Maybe there's more language in a form-factor spec or something?
> 
> > +	 */
> >  	reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
> >  	reg &= ~LINK_TRAINING_EN;
> >  	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
> > -- 
> > 2.20.1
> > 

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

* Re: [PATCH] PCI: aardvark: Update comment about disabling link training
  2020-09-24 15:22   ` Pali Rohár
@ 2020-10-11 17:21     ` Pali Rohár
  2020-11-29 23:17       ` Pali Rohár
  0 siblings, 1 reply; 9+ messages in thread
From: Pali Rohár @ 2020-10-11 17:21 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, linux-pci, linux-kernel

On Thursday 24 September 2020 17:22:32 Pali Rohár wrote:
> On Thursday 24 September 2020 10:11:06 Bjorn Helgaas wrote:
> > On Thu, Sep 24, 2020 at 10:46:18AM +0200, Pali Rohár wrote:
> > > It is not HW bug or workaround for some cards but it is requirement by PCI
> > > Express spec. After fundamental reset is needed 100ms delay prior enabling
> > > link training. So update comment in code to reflect this requirement.
> > > 
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > ---
> > >  drivers/pci/controller/pci-aardvark.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> > > index 50ab6d7519ae..19b9b79226e5 100644
> > > --- a/drivers/pci/controller/pci-aardvark.c
> > > +++ b/drivers/pci/controller/pci-aardvark.c
> > > @@ -259,7 +259,12 @@ static void advk_pcie_issue_perst(struct advk_pcie *pcie)
> > >  	if (!pcie->reset_gpio)
> > >  		return;
> > >  
> > > -	/* PERST does not work for some cards when link training is enabled */
> > > +	/*
> > > +	 * As required by PCI Express spec a delay for at least 100ms after
> > > +	 * de-asserting PERST# signal is needed before link training is enabled.
> > > +	 * So ensure that link training is disabled prior de-asserting PERST#
> > > +	 * signal to fulfill that PCI Express spec requirement.
> > 
> > Can you please include the spec citation here?  In the PCIe base spec,
> > PERST# is only mentioned in PCIe r5.0, sec 6.6.1, and I don't see the
> > connection there to 100ms between de-assert of PERST# and enabling
> > link training.
> 
> Hello! I copied this "comment" from other place in pci-aardvark.c where
> that timeout 100ms is already applied. Timeout with explanation comment
> was introduced in following commit:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f4c7d053d7f7
> 
> Here are links to discussions about that patch:
> 
> https://lore.kernel.org/linux-pci/20190313213752.1246-1-repk@triplefau.lt/T/#u
> https://lore.kernel.org/linux-pci/20190522213351.21366-2-repk@triplefau.lt/T/#u

Bjorn or Lorenzo, do you need something else for this patch? It just
updates comment and basically clarify why PERST does not work for some
cards when link training is enabled.

> > Sec 6.1.1 does talk about 100ms before sending config requests (for
> > ports that support <= 5 GT/s), and 100ms after link training completes
> > (for ports that support > 5 GT/s).
> > 
> > Maybe there's more language in a form-factor spec or something?
> > 
> > > +	 */
> > >  	reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
> > >  	reg &= ~LINK_TRAINING_EN;
> > >  	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
> > > -- 
> > > 2.20.1
> > > 

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

* Re: [PATCH] PCI: aardvark: Update comment about disabling link training
  2020-10-11 17:21     ` Pali Rohár
@ 2020-11-29 23:17       ` Pali Rohár
  2020-11-30 10:52         ` Lorenzo Pieralisi
  0 siblings, 1 reply; 9+ messages in thread
From: Pali Rohár @ 2020-11-29 23:17 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, linux-pci, linux-kernel

On Sunday 11 October 2020 19:21:49 Pali Rohár wrote:
> On Thursday 24 September 2020 17:22:32 Pali Rohár wrote:
> > On Thursday 24 September 2020 10:11:06 Bjorn Helgaas wrote:
> > > On Thu, Sep 24, 2020 at 10:46:18AM +0200, Pali Rohár wrote:
> > > > It is not HW bug or workaround for some cards but it is requirement by PCI
> > > > Express spec. After fundamental reset is needed 100ms delay prior enabling
> > > > link training. So update comment in code to reflect this requirement.
> > > > 
> > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > ---
> > > >  drivers/pci/controller/pci-aardvark.c | 7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> > > > index 50ab6d7519ae..19b9b79226e5 100644
> > > > --- a/drivers/pci/controller/pci-aardvark.c
> > > > +++ b/drivers/pci/controller/pci-aardvark.c
> > > > @@ -259,7 +259,12 @@ static void advk_pcie_issue_perst(struct advk_pcie *pcie)
> > > >  	if (!pcie->reset_gpio)
> > > >  		return;
> > > >  
> > > > -	/* PERST does not work for some cards when link training is enabled */
> > > > +	/*
> > > > +	 * As required by PCI Express spec a delay for at least 100ms after
> > > > +	 * de-asserting PERST# signal is needed before link training is enabled.
> > > > +	 * So ensure that link training is disabled prior de-asserting PERST#
> > > > +	 * signal to fulfill that PCI Express spec requirement.
> > > 
> > > Can you please include the spec citation here?  In the PCIe base spec,
> > > PERST# is only mentioned in PCIe r5.0, sec 6.6.1, and I don't see the
> > > connection there to 100ms between de-assert of PERST# and enabling
> > > link training.
> > 
> > Hello! I copied this "comment" from other place in pci-aardvark.c where
> > that timeout 100ms is already applied. Timeout with explanation comment
> > was introduced in following commit:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f4c7d053d7f7
> > 
> > Here are links to discussions about that patch:
> > 
> > https://lore.kernel.org/linux-pci/20190313213752.1246-1-repk@triplefau.lt/T/#u
> > https://lore.kernel.org/linux-pci/20190522213351.21366-2-repk@triplefau.lt/T/#u
> 
> Bjorn or Lorenzo, do you need something else for this patch? It just
> updates comment and basically clarify why PERST does not work for some
> cards when link training is enabled.

PING?

> > > Sec 6.1.1 does talk about 100ms before sending config requests (for
> > > ports that support <= 5 GT/s), and 100ms after link training completes
> > > (for ports that support > 5 GT/s).
> > > 
> > > Maybe there's more language in a form-factor spec or something?
> > > 
> > > > +	 */
> > > >  	reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
> > > >  	reg &= ~LINK_TRAINING_EN;
> > > >  	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
> > > > -- 
> > > > 2.20.1
> > > > 

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

* Re: [PATCH] PCI: aardvark: Update comment about disabling link training
  2020-11-29 23:17       ` Pali Rohár
@ 2020-11-30 10:52         ` Lorenzo Pieralisi
  2020-12-02 18:41           ` Pali Rohár
  0 siblings, 1 reply; 9+ messages in thread
From: Lorenzo Pieralisi @ 2020-11-30 10:52 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Bjorn Helgaas, Rob Herring, Bjorn Helgaas, linux-pci, linux-kernel

On Mon, Nov 30, 2020 at 12:17:41AM +0100, Pali Rohár wrote:
> On Sunday 11 October 2020 19:21:49 Pali Rohár wrote:
> > On Thursday 24 September 2020 17:22:32 Pali Rohár wrote:
> > > On Thursday 24 September 2020 10:11:06 Bjorn Helgaas wrote:
> > > > On Thu, Sep 24, 2020 at 10:46:18AM +0200, Pali Rohár wrote:
> > > > > It is not HW bug or workaround for some cards but it is requirement by PCI
> > > > > Express spec. After fundamental reset is needed 100ms delay prior enabling
> > > > > link training. So update comment in code to reflect this requirement.
> > > > > 
> > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > ---
> > > > >  drivers/pci/controller/pci-aardvark.c | 7 ++++++-
> > > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> > > > > index 50ab6d7519ae..19b9b79226e5 100644
> > > > > --- a/drivers/pci/controller/pci-aardvark.c
> > > > > +++ b/drivers/pci/controller/pci-aardvark.c
> > > > > @@ -259,7 +259,12 @@ static void advk_pcie_issue_perst(struct advk_pcie *pcie)
> > > > >  	if (!pcie->reset_gpio)
> > > > >  		return;
> > > > >  
> > > > > -	/* PERST does not work for some cards when link training is enabled */
> > > > > +	/*
> > > > > +	 * As required by PCI Express spec a delay for at least 100ms after
> > > > > +	 * de-asserting PERST# signal is needed before link training is enabled.
> > > > > +	 * So ensure that link training is disabled prior de-asserting PERST#
> > > > > +	 * signal to fulfill that PCI Express spec requirement.
> > > > 
> > > > Can you please include the spec citation here?  In the PCIe base spec,
> > > > PERST# is only mentioned in PCIe r5.0, sec 6.6.1, and I don't see the
> > > > connection there to 100ms between de-assert of PERST# and enabling
> > > > link training.
> > > 
> > > Hello! I copied this "comment" from other place in pci-aardvark.c where
> > > that timeout 100ms is already applied. Timeout with explanation comment
> > > was introduced in following commit:
> > > 
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f4c7d053d7f7
> > > 
> > > Here are links to discussions about that patch:
> > > 
> > > https://lore.kernel.org/linux-pci/20190313213752.1246-1-repk@triplefau.lt/T/#u
> > > https://lore.kernel.org/linux-pci/20190522213351.21366-2-repk@triplefau.lt/T/#u
> > 
> > Bjorn or Lorenzo, do you need something else for this patch? It just
> > updates comment and basically clarify why PERST does not work for some
> > cards when link training is enabled.
> 
> PING?

Apologies, I marked it as "changes requested" following Bjorn's reply.
Would you mind please adding a link to the relevant PCI specs in the
comment ?

I understood you copied the comment, it is worth adding that link to all
of them if you don't mind, it can be a preparation patch if you wish.

Thanks,
Lorenzo

> > > > Sec 6.1.1 does talk about 100ms before sending config requests (for
> > > > ports that support <= 5 GT/s), and 100ms after link training completes
> > > > (for ports that support > 5 GT/s).
> > > > 
> > > > Maybe there's more language in a form-factor spec or something?
> > > > 
> > > > > +	 */
> > > > >  	reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
> > > > >  	reg &= ~LINK_TRAINING_EN;
> > > > >  	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
> > > > > -- 
> > > > > 2.20.1
> > > > > 

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

* Re: [PATCH] PCI: aardvark: Update comment about disabling link training
  2020-11-30 10:52         ` Lorenzo Pieralisi
@ 2020-12-02 18:41           ` Pali Rohár
  0 siblings, 0 replies; 9+ messages in thread
From: Pali Rohár @ 2020-12-02 18:41 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Bjorn Helgaas, Rob Herring, Bjorn Helgaas, linux-pci, linux-kernel

On Monday 30 November 2020 10:52:00 Lorenzo Pieralisi wrote:
> On Mon, Nov 30, 2020 at 12:17:41AM +0100, Pali Rohár wrote:
> > On Sunday 11 October 2020 19:21:49 Pali Rohár wrote:
> > > On Thursday 24 September 2020 17:22:32 Pali Rohár wrote:
> > > > On Thursday 24 September 2020 10:11:06 Bjorn Helgaas wrote:
> > > > > On Thu, Sep 24, 2020 at 10:46:18AM +0200, Pali Rohár wrote:
> > > > > > It is not HW bug or workaround for some cards but it is requirement by PCI
> > > > > > Express spec. After fundamental reset is needed 100ms delay prior enabling
> > > > > > link training. So update comment in code to reflect this requirement.
> > > > > > 
> > > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > > ---
> > > > > >  drivers/pci/controller/pci-aardvark.c | 7 ++++++-
> > > > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> > > > > > index 50ab6d7519ae..19b9b79226e5 100644
> > > > > > --- a/drivers/pci/controller/pci-aardvark.c
> > > > > > +++ b/drivers/pci/controller/pci-aardvark.c
> > > > > > @@ -259,7 +259,12 @@ static void advk_pcie_issue_perst(struct advk_pcie *pcie)
> > > > > >  	if (!pcie->reset_gpio)
> > > > > >  		return;
> > > > > >  
> > > > > > -	/* PERST does not work for some cards when link training is enabled */
> > > > > > +	/*
> > > > > > +	 * As required by PCI Express spec a delay for at least 100ms after
> > > > > > +	 * de-asserting PERST# signal is needed before link training is enabled.
> > > > > > +	 * So ensure that link training is disabled prior de-asserting PERST#
> > > > > > +	 * signal to fulfill that PCI Express spec requirement.
> > > > > 
> > > > > Can you please include the spec citation here?  In the PCIe base spec,
> > > > > PERST# is only mentioned in PCIe r5.0, sec 6.6.1, and I don't see the
> > > > > connection there to 100ms between de-assert of PERST# and enabling
> > > > > link training.
> > > > 
> > > > Hello! I copied this "comment" from other place in pci-aardvark.c where
> > > > that timeout 100ms is already applied. Timeout with explanation comment
> > > > was introduced in following commit:
> > > > 
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f4c7d053d7f7
> > > > 
> > > > Here are links to discussions about that patch:
> > > > 
> > > > https://lore.kernel.org/linux-pci/20190313213752.1246-1-repk@triplefau.lt/T/#u
> > > > https://lore.kernel.org/linux-pci/20190522213351.21366-2-repk@triplefau.lt/T/#u
> > > 
> > > Bjorn or Lorenzo, do you need something else for this patch? It just
> > > updates comment and basically clarify why PERST does not work for some
> > > cards when link training is enabled.
> > 
> > PING?
> 
> Apologies, I marked it as "changes requested" following Bjorn's reply.
> Would you mind please adding a link to the relevant PCI specs in the
> comment ?

Of course, no problem. I will copy reference to PCI spec as Remi wrote
it in linked email, to the comment in source code.

> I understood you copied the comment, it is worth adding that link to all
> of them if you don't mind, it can be a preparation patch if you wish.
> 
> Thanks,
> Lorenzo
> 
> > > > > Sec 6.1.1 does talk about 100ms before sending config requests (for
> > > > > ports that support <= 5 GT/s), and 100ms after link training completes
> > > > > (for ports that support > 5 GT/s).
> > > > > 
> > > > > Maybe there's more language in a form-factor spec or something?
> > > > > 
> > > > > > +	 */
> > > > > >  	reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
> > > > > >  	reg &= ~LINK_TRAINING_EN;
> > > > > >  	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
> > > > > > -- 
> > > > > > 2.20.1
> > > > > > 

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

* [PATCH v2] PCI: aardvark: Update comment about disabling link training
  2020-09-24  8:46 [PATCH] PCI: aardvark: Update comment about disabling link training Pali Rohár
  2020-09-24 15:11 ` Bjorn Helgaas
@ 2020-12-02 18:46 ` Pali Rohár
  2020-12-07 16:42   ` Lorenzo Pieralisi
  1 sibling, 1 reply; 9+ messages in thread
From: Pali Rohár @ 2020-12-02 18:46 UTC (permalink / raw)
  To: Lorenzo Pieralisi; +Cc: Bjorn Helgaas, Rob Herring, linux-pci, linux-kernel

It is not HW bug or workaround for some cards but it is requirement by PCI
Express spec. After fundamental reset is needed 100ms delay prior enabling
link training. So update comment in code to reflect this requirement.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
Changes in v2:
* Add reference to the PCI Express spec
---
 drivers/pci/controller/pci-aardvark.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index 0be485a25327..f742da2a20ee 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -259,7 +259,14 @@ static void advk_pcie_issue_perst(struct advk_pcie *pcie)
 	if (!pcie->reset_gpio)
 		return;
 
-	/* PERST does not work for some cards when link training is enabled */
+	/*
+	 * As required by PCI Express spec (PCI Express Base Specification, REV.
+	 * 4.0 PCI Express, February 19 2014, 6.6.1 Conventional Reset) a delay
+	 * for at least 100ms after de-asserting PERST# signal is needed before
+	 * link training is enabled. So ensure that link training is disabled
+	 * prior de-asserting PERST# signal to fulfill that PCI Express spec
+	 * requirement.
+	 */
 	reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
 	reg &= ~LINK_TRAINING_EN;
 	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
-- 
2.20.1


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

* Re: [PATCH v2] PCI: aardvark: Update comment about disabling link training
  2020-12-02 18:46 ` [PATCH v2] " Pali Rohár
@ 2020-12-07 16:42   ` Lorenzo Pieralisi
  0 siblings, 0 replies; 9+ messages in thread
From: Lorenzo Pieralisi @ 2020-12-07 16:42 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Lorenzo Pieralisi, Bjorn Helgaas, Rob Herring, linux-kernel, linux-pci

On Wed, 2 Dec 2020 19:46:59 +0100, Pali Rohár wrote:
> It is not HW bug or workaround for some cards but it is requirement by PCI
> Express spec. After fundamental reset is needed 100ms delay prior enabling
> link training. So update comment in code to reflect this requirement.

Applied to pci/aardvark, thanks!

[1/1] PCI: aardvark: Update comment about disabling link training
      https://git.kernel.org/lpieralisi/pci/c/1d1cd163d0

Thanks,
Lorenzo

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

end of thread, other threads:[~2020-12-07 16:43 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-24  8:46 [PATCH] PCI: aardvark: Update comment about disabling link training Pali Rohár
2020-09-24 15:11 ` Bjorn Helgaas
2020-09-24 15:22   ` Pali Rohár
2020-10-11 17:21     ` Pali Rohár
2020-11-29 23:17       ` Pali Rohár
2020-11-30 10:52         ` Lorenzo Pieralisi
2020-12-02 18:41           ` Pali Rohár
2020-12-02 18:46 ` [PATCH v2] " Pali Rohár
2020-12-07 16:42   ` 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).