linux-kernel - Re: [PATCH v6 7/7] PCI: dwc: qcom: Add support for IPQ8074 PCIe controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 17 Aug 2017 14:01:17 +0530
From:   Varadarajan Narayanan <varada@...eaurora.org>
To:     Stanimir Varbanov <svarbanov@...sol.com>
Cc:     bhelgaas@...gle.com, robh+dt@...nel.org, mark.rutland@....com,
        kishon@...com, sboyd@...eaurora.org, vivek.gautam@...eaurora.org,
        fengguang.wu@...el.com, weiyongjun1@...wei.com,
        linux-pci@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        smuthayy <smuthayy@...eaurora.org>
Subject: Re: [PATCH v6 7/7] PCI: dwc: qcom: Add support for IPQ8074 PCIe
 controller

Stanimir,

> Hi,
>
> Thanks for the patch.
>
> On 31.07.2017 09:34, Varadarajan Narayanan wrote:
> >Add support for the IPQ8074 PCIe controller.  IPQ8074 supports
> >Gen 1/2, one lane, two PCIe root complex with support for MSI and
> >legacy interrupts, and it conforms to PCI Express Base 2.1
> >specification.
> >
> >The core init is the similar to the existing SoC, however the
> >clocks and reset lines differ.
> >
> >Signed-off-by: smuthayy <smuthayy@...eaurora.org>
> >Signed-off-by: Varadarajan Narayanan <varada@...eaurora.org>

<snip>

> >+static int qcom_pcie_2_3_3_reset(struct qcom_pcie *pcie)
> >+{
> >+	struct qcom_pcie_resources_2_3_3 *res = &pcie->res.v2_3_3;
> >+	int i, ret;
> >+
> >+	for (i = 0; i < ARRAY_SIZE(res->rst); i++) {
> >+		ret = reset_control_assert(res->rst[i]);
> >+		if (ret) {
> >+			dev_err(pcie->pci->dev,
> >+				"%s: reset assert failed for %d\n",
> >+				__func__, i);
> >+			return ret;
> >+		}
> >+	}
> >+
> >+	msleep(20);
>
> Could you explain why we need to wait for 20ms.
>
> >+
> >+	for (i = 0; i < ARRAY_SIZE(res->rst); i++) {
> >+		ret = reset_control_deassert(res->rst[i]);
> >+		if (ret) {
> >+			dev_err(pcie->pci->dev,
> >+				"%s: reset deassert failed for %d\n",
> >+				__func__, i);
> >+			return ret;
> >+		}
> >+	}
> >+
> >+	msleep(20);
>
> Same comment as above.

<snip>

Sorry about the delay. I tried to contact the hardware folks
to get more clarity about these delays. However, I haven't
received any response from them till now. Unfortunately, the
PCIe link doesn't come up without these delays.

I was able to get the PCIe link with the above delays
reduced to 2ms. I have posted v7 of these patches addressing
your other comments and the above delays reduced to 2ms. Can
you please review and provide your feedback.

If everything else (other than these delays) is ok, can this
patch be accepted? Meanwhile, I will follow up with the
hardware folks and based on their response post a patch that
removes the delay or provides a proper explanation for these
delays.

Please let me know.

Thanks
Varada

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation