LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v2] PCI: qcom: Ensure that PERST is asserted for at least 100 ms
@ 2019-05-29  9:43 Niklas Cassel
  2019-05-29 12:52 ` Bjorn Helgaas
  2019-05-30 15:54 ` Lorenzo Pieralisi
  0 siblings, 2 replies; 4+ messages in thread
From: Niklas Cassel @ 2019-05-29  9:43 UTC (permalink / raw)
  To: Stanimir Varbanov, Andy Gross, David Brown, Lorenzo Pieralisi,
	Bjorn Helgaas
  Cc: Niklas Cassel, stable, linux-pci, linux-arm-msm, linux-kernel

Currently, there is only a 1 ms sleep after asserting PERST.

Reading the datasheets for different endpoints, some require PERST to be
asserted for 10 ms in order for the endpoint to perform a reset, others
require it to be asserted for 50 ms.

Several SoCs using this driver uses PCIe Mini Card, where we don't know
what endpoint will be plugged in.

The PCI Express Card Electromechanical Specification specifies:
"On power up, the deassertion of PERST# is delayed 100 ms (TPVPERL) from
the power rails achieving specified operating limits."

Add a sleep of 100 ms before deasserting PERST, in order to ensure that
we are compliant with the spec.

Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")
Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
Acked-by: Stanimir Varbanov <svarbanov@mm-sol.com>
Cc: stable@vger.kernel.org # 4.5+
---
Changes since v1:
Move the sleep into qcom_ep_reset_deassert()

 drivers/pci/controller/dwc/pcie-qcom.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 0ed235d560e3..5d1713069d14 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -178,6 +178,8 @@ static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
 
 static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
 {
+	/* Ensure that PERST has been asserted for at least 100 ms */
+	msleep(100);
 	gpiod_set_value_cansleep(pcie->reset, 0);
 	usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
 }
-- 
2.21.0


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

* Re: [PATCH v2] PCI: qcom: Ensure that PERST is asserted for at least 100 ms
  2019-05-29  9:43 [PATCH v2] PCI: qcom: Ensure that PERST is asserted for at least 100 ms Niklas Cassel
@ 2019-05-29 12:52 ` Bjorn Helgaas
  2019-05-30 15:49   ` Lorenzo Pieralisi
  2019-05-30 15:54 ` Lorenzo Pieralisi
  1 sibling, 1 reply; 4+ messages in thread
From: Bjorn Helgaas @ 2019-05-29 12:52 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Stanimir Varbanov, Andy Gross, David Brown, Lorenzo Pieralisi,
	stable, linux-pci, linux-arm-msm, linux-kernel

On Wed, May 29, 2019 at 11:43:52AM +0200, Niklas Cassel wrote:
> Currently, there is only a 1 ms sleep after asserting PERST.
> 
> Reading the datasheets for different endpoints, some require PERST to be
> asserted for 10 ms in order for the endpoint to perform a reset, others
> require it to be asserted for 50 ms.
> 
> Several SoCs using this driver uses PCIe Mini Card, where we don't know
> what endpoint will be plugged in.

I think this patch is absolutely the right thing to do.  We don't know
what might be plugged in, so we should support arbitrary endpoints.

One could imagine some sort of DT property to identify closed
environments where there is no need to support arbitrary endpoints and
it might be desirable to reduce the delay.  But that should wait
until there's a need for it and if the need *does* arise, hopefully
the definition of such a property could be generic across all SoCs.

> The PCI Express Card Electromechanical Specification specifies:
> "On power up, the deassertion of PERST# is delayed 100 ms (TPVPERL) from
> the power rails achieving specified operating limits."

It'd be nice to include the complete citation, i.e., "PCI Express Card
Electromechanical Specification r2.0, sec 2.2"

> Add a sleep of 100 ms before deasserting PERST, in order to ensure that
> we are compliant with the spec.
> 
> Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")
> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> Acked-by: Stanimir Varbanov <svarbanov@mm-sol.com>
> Cc: stable@vger.kernel.org # 4.5+
> ---
> Changes since v1:
> Move the sleep into qcom_ep_reset_deassert()
> 
>  drivers/pci/controller/dwc/pcie-qcom.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 0ed235d560e3..5d1713069d14 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -178,6 +178,8 @@ static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
>  
>  static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
>  {
> +	/* Ensure that PERST has been asserted for at least 100 ms */
> +	msleep(100);
>  	gpiod_set_value_cansleep(pcie->reset, 0);
>  	usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
>  }
> -- 
> 2.21.0
> 

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

* Re: [PATCH v2] PCI: qcom: Ensure that PERST is asserted for at least 100 ms
  2019-05-29 12:52 ` Bjorn Helgaas
@ 2019-05-30 15:49   ` Lorenzo Pieralisi
  0 siblings, 0 replies; 4+ messages in thread
From: Lorenzo Pieralisi @ 2019-05-30 15:49 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Niklas Cassel, Stanimir Varbanov, Andy Gross, David Brown,
	stable, linux-pci, linux-arm-msm, linux-kernel

On Wed, May 29, 2019 at 07:52:32AM -0500, Bjorn Helgaas wrote:
> On Wed, May 29, 2019 at 11:43:52AM +0200, Niklas Cassel wrote:
> > Currently, there is only a 1 ms sleep after asserting PERST.
> > 
> > Reading the datasheets for different endpoints, some require PERST to be
> > asserted for 10 ms in order for the endpoint to perform a reset, others
> > require it to be asserted for 50 ms.
> > 
> > Several SoCs using this driver uses PCIe Mini Card, where we don't know
> > what endpoint will be plugged in.
> 
> I think this patch is absolutely the right thing to do.  We don't know
> what might be plugged in, so we should support arbitrary endpoints.
> 
> One could imagine some sort of DT property to identify closed
> environments where there is no need to support arbitrary endpoints and
> it might be desirable to reduce the delay.  But that should wait
> until there's a need for it and if the need *does* arise, hopefully
> the definition of such a property could be generic across all SoCs.
> 
> > The PCI Express Card Electromechanical Specification specifies:
> > "On power up, the deassertion of PERST# is delayed 100 ms (TPVPERL) from
> > the power rails achieving specified operating limits."
> 
> It'd be nice to include the complete citation, i.e., "PCI Express Card
> Electromechanical Specification r2.0, sec 2.2"

I will add it and merge the patch.

Thanks,
Lorenzo

> > Add a sleep of 100 ms before deasserting PERST, in order to ensure that
> > we are compliant with the spec.
> > 
> > Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")
> > Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> > Acked-by: Stanimir Varbanov <svarbanov@mm-sol.com>
> > Cc: stable@vger.kernel.org # 4.5+
> > ---
> > Changes since v1:
> > Move the sleep into qcom_ep_reset_deassert()
> > 
> >  drivers/pci/controller/dwc/pcie-qcom.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 0ed235d560e3..5d1713069d14 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -178,6 +178,8 @@ static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
> >  
> >  static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
> >  {
> > +	/* Ensure that PERST has been asserted for at least 100 ms */
> > +	msleep(100);
> >  	gpiod_set_value_cansleep(pcie->reset, 0);
> >  	usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
> >  }
> > -- 
> > 2.21.0
> > 

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

* Re: [PATCH v2] PCI: qcom: Ensure that PERST is asserted for at least 100 ms
  2019-05-29  9:43 [PATCH v2] PCI: qcom: Ensure that PERST is asserted for at least 100 ms Niklas Cassel
  2019-05-29 12:52 ` Bjorn Helgaas
@ 2019-05-30 15:54 ` Lorenzo Pieralisi
  1 sibling, 0 replies; 4+ messages in thread
From: Lorenzo Pieralisi @ 2019-05-30 15:54 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Stanimir Varbanov, Andy Gross, David Brown, Bjorn Helgaas,
	stable, linux-pci, linux-arm-msm, linux-kernel

On Wed, May 29, 2019 at 11:43:52AM +0200, Niklas Cassel wrote:
> Currently, there is only a 1 ms sleep after asserting PERST.
> 
> Reading the datasheets for different endpoints, some require PERST to be
> asserted for 10 ms in order for the endpoint to perform a reset, others
> require it to be asserted for 50 ms.
> 
> Several SoCs using this driver uses PCIe Mini Card, where we don't know
> what endpoint will be plugged in.
> 
> The PCI Express Card Electromechanical Specification specifies:
> "On power up, the deassertion of PERST# is delayed 100 ms (TPVPERL) from
> the power rails achieving specified operating limits."
> 
> Add a sleep of 100 ms before deasserting PERST, in order to ensure that
> we are compliant with the spec.
> 
> Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")
> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> Acked-by: Stanimir Varbanov <svarbanov@mm-sol.com>
> Cc: stable@vger.kernel.org # 4.5+
> ---
> Changes since v1:
> Move the sleep into qcom_ep_reset_deassert()
> 
>  drivers/pci/controller/dwc/pcie-qcom.c | 2 ++
>  1 file changed, 2 insertions(+)

Applied to pci/qcom for v5.3, thanks.

Lorenzo

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 0ed235d560e3..5d1713069d14 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -178,6 +178,8 @@ static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
>  
>  static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
>  {
> +	/* Ensure that PERST has been asserted for at least 100 ms */
> +	msleep(100);
>  	gpiod_set_value_cansleep(pcie->reset, 0);
>  	usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
>  }
> -- 
> 2.21.0
> 

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

end of thread, other threads:[~2019-05-30 15:55 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-29  9:43 [PATCH v2] PCI: qcom: Ensure that PERST is asserted for at least 100 ms Niklas Cassel
2019-05-29 12:52 ` Bjorn Helgaas
2019-05-30 15:49   ` Lorenzo Pieralisi
2019-05-30 15:54 ` 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).