Messages in this thread | | | Subject | Re: [PATCH] PCI: qcom: Ensure that PERST is asserted for at least 100 ms | From | Stanimir Varbanov <> | Date | Tue, 28 May 2019 23:36:12 +0300 |
| |
Hi Niklas,
On 27.05.19 г. 20:15 ч., Niklas Cassel wrote: > On Fri, May 24, 2019 at 02:43:00PM +0200, Marc Gonzalez wrote: >> On 23/05/2019 21:44, 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. >>> >>> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org> >>> --- >>> 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..cae24376237c 100644 >>> --- a/drivers/pci/controller/dwc/pcie-qcom.c >>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c >>> @@ -1110,6 +1110,8 @@ static int qcom_pcie_host_init(struct pcie_port *pp) >>> if (IS_ENABLED(CONFIG_PCI_MSI)) >>> dw_pcie_msi_init(pp); >>> >>> + /* Ensure that PERST has been asserted for at least 100 ms */ >>> + msleep(100); >>> qcom_ep_reset_deassert(pcie); >>> >>> ret = qcom_pcie_establish_link(pcie); >> >> Currently, qcom_ep_reset_assert() and qcom_ep_reset_deassert() both include >> a call to usleep_range() of 1.0 to 1.5 ms >> >> Can we git rid of both if we sleep 100 ms before qcom_ep_reset_deassert? > > These two sleeps after asserting/deasserting reset in qcom_ep_reset_assert()/ > qcom_ep_reset_deassert() matches the sleeps in: > https://source.codeaurora.org/quic/la/kernel/msm-4.14/tree/drivers/pci/host/pci-msm.c?h=LA.UM.7.1.r1-14000-sm8150.0#n1942 > > and > > https://source.codeaurora.org/quic/la/kernel/msm-4.14/tree/drivers/pci/host/pci-msm.c?h=LA.UM.7.1.r1-14000-sm8150.0#n1949 > > I would rather not remove these since that might affect existing devices. > > > This new sleep matches matches the sleep in: > https://source.codeaurora.org/quic/la/kernel/msm-4.14/tree/drivers/pci/host/pci-msm.c?h=LA.UM.7.1.r1-14000-sm8150.0#n3926 > >> >> Should the msleep() call be included in one of the two wrappers? > > This new sleep could be moved into qcom_ep_reset_deassert(), > added before the gpiod_set_value_cansleep(pcie->reset, 0) call, > if Stanimir prefers it to be placed there instead.
yes, please move the sleep in qcom_ep_reset_deassert()
with that:
Acked-by: Stanimir Varbanov <svarbanov@mm-sol.com>
regards, Stan
| |