2019-01-25 23:27:12

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH] PCI: qcom: Don't deassert reset GPIO during probe

Acquiring the reset GPIO low means that reset is being deasserted, this
is followed almost immediately with qcom_pcie_host_init() asserting it,
initializing it and then finally deasserting it again, for the link to
come up.

Some PCIe devices requires a minimum time between the initial deassert
and subsequent reset cycles. In a platform that boots with the reset
GPIO asserted this requirement is being violated by this deassert/assert
pulse.

Acquiring the reset GPIO high will prevent this by matching the state to
the subsequent asserted state.

Cc: [email protected]
Signed-off-by: Bjorn Andersson <[email protected]>
---
drivers/pci/controller/dwc/pcie-qcom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index d185ea5fe996..a7f703556790 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1228,7 +1228,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)

pcie->ops = of_device_get_match_data(dev);

- pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_LOW);
+ pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH);
if (IS_ERR(pcie->reset)) {
ret = PTR_ERR(pcie->reset);
goto err_pm_runtime_put;
--
2.18.0



2019-02-08 15:03:04

by Stanimir Varbanov

[permalink] [raw]
Subject: Re: [PATCH] PCI: qcom: Don't deassert reset GPIO during probe

Hi Bjorn,

Thanks for the patch!

On 1/26/19 1:26 AM, Bjorn Andersson wrote:
> Acquiring the reset GPIO low means that reset is being deasserted, this
> is followed almost immediately with qcom_pcie_host_init() asserting it,
> initializing it and then finally deasserting it again, for the link to
> come up.
>
> Some PCIe devices requires a minimum time between the initial deassert
> and subsequent reset cycles. In a platform that boots with the reset
> GPIO asserted this requirement is being violated by this deassert/assert
> pulse.
>
> Acquiring the reset GPIO high will prevent this by matching the state to
> the subsequent asserted state.
>
> Cc: [email protected]
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index d185ea5fe996..a7f703556790 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1228,7 +1228,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>
> pcie->ops = of_device_get_match_data(dev);
>
> - pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_LOW);
> + pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH);
> if (IS_ERR(pcie->reset)) {
> ret = PTR_ERR(pcie->reset);
> goto err_pm_runtime_put;
>

Acked-by: Stanimir Varbanov <[email protected]>

--
regards,
Stan

2019-02-13 15:56:50

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH] PCI: qcom: Don't deassert reset GPIO during probe

On Fri, Jan 25, 2019 at 03:26:16PM -0800, Bjorn Andersson wrote:
> Acquiring the reset GPIO low means that reset is being deasserted, this
> is followed almost immediately with qcom_pcie_host_init() asserting it,
> initializing it and then finally deasserting it again, for the link to
> come up.
>
> Some PCIe devices requires a minimum time between the initial deassert
> and subsequent reset cycles. In a platform that boots with the reset
> GPIO asserted this requirement is being violated by this deassert/assert
> pulse.
>
> Acquiring the reset GPIO high will prevent this by matching the state to
> the subsequent asserted state.
>
> Cc: [email protected]

Missing Fixes: tag, please provide me one so that I can proceed.

Lorenzo

> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index d185ea5fe996..a7f703556790 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1228,7 +1228,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>
> pcie->ops = of_device_get_match_data(dev);
>
> - pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_LOW);
> + pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH);
> if (IS_ERR(pcie->reset)) {
> ret = PTR_ERR(pcie->reset);
> goto err_pm_runtime_put;
> --
> 2.18.0
>

2019-02-19 05:16:44

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH] PCI: qcom: Don't deassert reset GPIO during probe

On Wed 13 Feb 07:23 PST 2019, Lorenzo Pieralisi wrote:

> On Fri, Jan 25, 2019 at 03:26:16PM -0800, Bjorn Andersson wrote:
> > Acquiring the reset GPIO low means that reset is being deasserted, this
> > is followed almost immediately with qcom_pcie_host_init() asserting it,
> > initializing it and then finally deasserting it again, for the link to
> > come up.
> >
> > Some PCIe devices requires a minimum time between the initial deassert
> > and subsequent reset cycles. In a platform that boots with the reset
> > GPIO asserted this requirement is being violated by this deassert/assert
> > pulse.
> >
> > Acquiring the reset GPIO high will prevent this by matching the state to
> > the subsequent asserted state.
> >
> > Cc: [email protected]
>
> Missing Fixes: tag, please provide me one so that I can proceed.
>

This applies to the original commit introducing this driver, so:

Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")

Regards,
Bjorn

2019-02-19 11:11:13

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH] PCI: qcom: Don't deassert reset GPIO during probe

On Mon, Feb 18, 2019 at 09:16:03PM -0800, Bjorn Andersson wrote:
> On Wed 13 Feb 07:23 PST 2019, Lorenzo Pieralisi wrote:
>
> > On Fri, Jan 25, 2019 at 03:26:16PM -0800, Bjorn Andersson wrote:
> > > Acquiring the reset GPIO low means that reset is being deasserted, this
> > > is followed almost immediately with qcom_pcie_host_init() asserting it,
> > > initializing it and then finally deasserting it again, for the link to
> > > come up.
> > >
> > > Some PCIe devices requires a minimum time between the initial deassert
> > > and subsequent reset cycles. In a platform that boots with the reset
> > > GPIO asserted this requirement is being violated by this deassert/assert
> > > pulse.
> > >
> > > Acquiring the reset GPIO high will prevent this by matching the state to
> > > the subsequent asserted state.
> > >
> > > Cc: [email protected]
> >
> > Missing Fixes: tag, please provide me one so that I can proceed.
> >
>
> This applies to the original commit introducing this driver, so:
>
> Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")

Applied to pci/dwc for v5.1, thanks.

Lorenzo