Re: [PATCH v6 3/6] PCI: qcom: Add missing icc bandwidth vote for cpu to PCIe path

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 1/12/2024 9:29 PM, Johan Hovold wrote:
On Fri, Jan 12, 2024 at 07:52:02PM +0530, Krishna chaitanya chundru wrote:
CPU-PCIe path consits for registers PCIe BAR space, config space.

consits?

As there is less access on this path compared to pcie to mem path
add minimum vote i.e GEN1x1 bandwidth always.

gen1 bandwidth can't be right.

There is no recommended value we need vote for this path, as there is
BAR and config space in this path we are voting for GEN1x1.
Please suggest a recommended value for this path if the GEN1x1 is high.
In suspend remove the cpu vote after register space access is done.

Fixes: c4860af88d0c ("PCI: qcom: Add basic interconnect support")
cc: stable@xxxxxxxxxxxxxxx

This does not look like a fix so drop the above.

The commit you refer to explicitly left this path unconfigured for now
and only added support for the configuring the mem path as needed on
sc8280xp which otherwise would crash.

Without this path vote BAR and config space can result NOC timeout
errors, we are surviving because of other driver vote for this path.
For that reason we added a fix tag.
@@ -1573,7 +1588,7 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
  	 */
  	ret = icc_set_bw(pcie->icc_mem, 0, kBps_to_icc(1));
  	if (ret) {
-		dev_err(dev, "Failed to set interconnect bandwidth: %d\n", ret);
+		dev_err(dev, "Failed to set interconnect bandwidth for pcie-mem: %d\n", ret);
  		return ret;
  	}
@@ -1597,6 +1612,12 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
  		pcie->suspended = true;
  	}
+ /* Remove cpu path vote after all the register access is done */
+	ret = icc_set_bw(pcie->icc_cpu, 0, 0);

I believe you should use icc_disable() here.

+	if (ret) {
+		dev_err(dev, "failed to set interconnect bandwidth for cpu-pcie: %d\n", ret);
+		return ret;

And you need to unwind before returning on errors.

+	}
  	return 0;
  }
@@ -1605,6 +1626,12 @@ static int qcom_pcie_resume_noirq(struct device *dev)
  	struct qcom_pcie *pcie = dev_get_drvdata(dev);
  	int ret;
+ ret = icc_set_bw(pcie->icc_cpu, 0, QCOM_PCIE_LINK_SPEED_TO_BW(1));

icc_enable()

I was not aware of these API's, I will add them in next patch.

- Krishna Chaitanya.
+	if (ret) {
+		dev_err(dev, "failed to set interconnect bandwidth for cpu-pcie: %d\n", ret);
+		return ret;
+	}

Johan




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux