Re: [PATCH V3 2/8] soc: qcom: geni: Support for ICC voting

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

 



Hi Matthias,

On 3/31/2020 11:22 PM, Matthias Kaehlcke wrote:
Hi Akash,

On Tue, Mar 31, 2020 at 04:39:30PM +0530, Akash Asthana wrote:
Add necessary macros and structure variables to support ICC BW
voting from individual SE drivers.

Signed-off-by: Akash Asthana <akashast@xxxxxxxxxxxxxx>
---
Changes in V2:
  - As per Bjorn's comment dropped enums for ICC paths, given the three
    paths individual members

Changes in V3:
  - Add geni_icc_get, geni_icc_vote_on and geni_icc_vote_off as helper API.
  - Add geni_icc_path structure in common header

  drivers/soc/qcom/qcom-geni-se.c | 98 +++++++++++++++++++++++++++++++++++++++++
  include/linux/qcom-geni-se.h    | 36 +++++++++++++++
  2 files changed, 134 insertions(+)

diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index 7d622ea..9344c14 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -720,6 +720,104 @@ void geni_se_rx_dma_unprep(struct geni_se *se, dma_addr_t iova, size_t len)
  }
  EXPORT_SYMBOL(geni_se_rx_dma_unprep);
+int geni_icc_get(struct geni_se *se, const char *icc_core, const char *icc_cpu,
+		const char *icc_ddr)
+{
+	if (icc_core) {
+		se->to_core.path = devm_of_icc_get(se->dev, "qup-core");
+		if (IS_ERR(se->to_core.path))
+			return PTR_ERR(se->to_core.path);
+	}
+
+	if (icc_cpu) {
+		se->from_cpu.path = devm_of_icc_get(se->dev, "qup-config");
+		if (IS_ERR(se->from_cpu.path))
+			return PTR_ERR(se->from_cpu.path);
+	}
+
+	if (icc_ddr) {
+		se->to_ddr.path = devm_of_icc_get(se->dev, "qup-memory");
+		if (IS_ERR(se->to_ddr.path))
+			return PTR_ERR(se->to_ddr.path);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(geni_icc_get);
+
+int geni_icc_vote_on(struct geni_se *se)
+{
+	int ret;
+
+	if (se->to_core.path) {
+		ret = icc_set_bw(se->to_core.path, se->to_core.avg_bw,
+			se->to_core.peak_bw);
+		if (ret) {
+			dev_err_ratelimited(se->dev, "%s: ICC BW voting failed for core\n",
+						__func__);
+			return ret;
+		}
+	}
+
+	if (se->from_cpu.path) {
+		ret = icc_set_bw(se->from_cpu.path, se->from_cpu.avg_bw,
+			se->from_cpu.peak_bw);
+		if (ret) {
+			dev_err_ratelimited(se->dev, "%s: ICC BW voting failed for cpu\n",
+						__func__);
+			return ret;
+		}
+	}
+
+	if (se->to_ddr.path) {
+		ret = icc_set_bw(se->to_ddr.path, se->to_ddr.avg_bw,
+			se->to_ddr.peak_bw);
+		if (ret) {
+			dev_err_ratelimited(se->dev, "%s: ICC BW voting failed for ddr\n",
+						__func__);
+			return ret;
+		}
+	}

With an array of 'struct geni_icc_path' pointers the above could be
reduced to:

	for (i = 0; i < ARRAY_SIZE(se->icc_paths); i++) {
		if (!se->icc_paths[i])
			continue;

		ret = icc_set_bw(se->icc_paths[i]->path, se->icc_paths[i]->avg_bw,
			se->icc_paths[i]->peak_bw);
		if (ret) {
			dev_err_ratelimited(se->dev, "%s: ICC BW voting failed\n",
						__func__);
			return ret;
		}
	}

similar for geni_icc_vote_off()

It's just a suggestion, looks also good to me as is.

I thought giving individual path name will increase readability. But that doesn't seems to be adding much value on cost of repeated code.

So, I will make the suggested change in next version.

Thanks,

Akash


Reviewed-by: Matthias Kaehlcke <mka@xxxxxxxxxxxx>

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project



[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