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@xxxxxxxxxx> > > --- > > 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. Kind regards, Niklas
