Subject: Re: [PATCH v5 3/3] PCI: qcom: add support for IPQ60xx PCIe controller

From: Bjorn Helgaas <[email-masked]>
Date: Thu, 27 Jan 2022 09:16:30 -0600

On Tue, Jan 25, 2022 at 09:41:45AM +0200, Baruch Siach wrote:
> On Mon, Jan 24 2022, Bjorn Helgaas wrote:
> > On Mon, Jan 24, 2022 at 06:27:31PM +0200, Baruch Siach wrote:

> >> +static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
> >> +{
> >> +	struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
> >> +	struct device *dev = pcie->pci->dev;
> >> +	int ret;
> >> +
> >> +	ret = reset_control_assert(res->rst);
> >> +	if (ret) {
> >> +		dev_err(dev, "reset assert failed (%d)\n", ret);
> >> +		return ret;
> >> +	}
> >> +
> >> +	usleep_range(2000, 2500);
> >
> > Where do these sleep durations come from?  If they're specified
> > somewhere by PCIe, can you include a citation, e.g., a section number
> > in the spec.
> 
> As I mentioned before, I have no access to hardware documentation. I'm
> only porting working code from downstream kernel.
> 
> In a comment on v4 Bjorn Andersson mentioned "datasheet stating the
> minimum timing of the operations to be performed to get the PCIe
> controller into a known (clean) state".

Sorry if I'm repeating questions.  We can help avoid that by adding a
brief comment here like "black magic copied from working code at X" or
whatever you *do* know.  Repeated questions are wasted effort for both
the author and reviewers.

Sleeps with unsourced durations are always suspect because nobody
wants to wait longer than necessary, and if all we have is a bare
number and nobody knows how it was derived, we can't be confident
about changing it.

>   https://lore.kernel.org/all/Ydd5Wh0KeADBQ%2Fh1@ripper/
> 
> I have no further details.

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel