Thread (16 messages) 16 messages, 5 authors, 2020-03-04

Re: [PATCH v3 4/6] interconnect: qcom: Consolidate interconnect RPMh support

From: Georgi Djakov <hidden>
Date: 2020-02-27 14:47:03
Also in: linux-arm-msm, linux-pm, lkml

Hi Sibi,

Thank you for refreshing this patchset!

On 2/9/20 20:34, Sibi Sankar wrote:
quoted hunk ↗ jump to hunk
From: David Dai <redacted>

Add bcm voter driver and add support for RPMh specific interconnect
providers which implements the set and aggregate functionalities that
translates bandwidth requests into RPMh messages. These modules provide
a common set of functionalities for all Qualcomm RPMh based interconnect
providers and should help reduce code duplication when adding new
providers.

Signed-off-by: David Dai <redacted>
Signed-off-by: Odelu Kukatla <redacted>
Signed-off-by: Sibi Sankar <redacted>
---
 drivers/interconnect/qcom/Kconfig     |  13 +-
 drivers/interconnect/qcom/Makefile    |   4 +
 drivers/interconnect/qcom/bcm-voter.c | 366 ++++++++++++++++++++++++++
 drivers/interconnect/qcom/bcm-voter.h |  27 ++
 drivers/interconnect/qcom/icc-rpmh.c  | 147 +++++++++++
 drivers/interconnect/qcom/icc-rpmh.h  | 149 +++++++++++
 6 files changed, 705 insertions(+), 1 deletion(-)
 create mode 100644 drivers/interconnect/qcom/bcm-voter.c
 create mode 100644 drivers/interconnect/qcom/bcm-voter.h
 create mode 100644 drivers/interconnect/qcom/icc-rpmh.c
 create mode 100644 drivers/interconnect/qcom/icc-rpmh.h
diff --git a/drivers/interconnect/qcom/Kconfig b/drivers/interconnect/qcom/Kconfig
index 76938ece1658e..08cfd676b4299 100644
--- a/drivers/interconnect/qcom/Kconfig
+++ b/drivers/interconnect/qcom/Kconfig
@@ -5,6 +5,11 @@ config INTERCONNECT_QCOM
 	help
 	  Support for Qualcomm's Network-on-Chip interconnect hardware.
 
+config INTERCONNECT_QCOM_BCM_VOTER
+	tristate
+	depends on INTERCONNECT_QCOM || COMPILE_TEST
+	depends on QCOM_RPMH && OF
This is not user-selectable, but has a "depends on" lines...
quoted hunk ↗ jump to hunk
+
 config INTERCONNECT_QCOM_MSM8916
 	tristate "Qualcomm MSM8916 interconnect driver"
 	depends on INTERCONNECT_QCOM
@@ -32,10 +37,16 @@ config INTERCONNECT_QCOM_QCS404
 	  This is a driver for the Qualcomm Network-on-Chip on qcs404-based
 	  platforms.
 
+config INTERCONNECT_QCOM_RPMH
+	tristate
+	depends on INTERCONNECT_QCOM || COMPILE_TEST
+	depends on QCOM_COMMAND_DB
+
 config INTERCONNECT_QCOM_SDM845
 	tristate "Qualcomm SDM845 interconnect driver"
 	depends on INTERCONNECT_QCOM
-	depends on (QCOM_RPMH && QCOM_COMMAND_DB && OF) || COMPILE_TEST
+	select INTERCONNECT_QCOM_RPMH
+	select INTERCONNECT_QCOM_BCM_VOTER
...and these selects will force a symbol to a value without checking its
dependencies. So i was wondering whether this will cause any problems.
quoted hunk ↗ jump to hunk
 	help
 	  This is a driver for the Qualcomm Network-on-Chip on sdm845-based
 	  platforms.
diff --git a/drivers/interconnect/qcom/Makefile b/drivers/interconnect/qcom/Makefile
index e8271575e3d8a..d591bb56273b1 100644
[..]
+static const struct of_device_id bcm_voter_of_match[] = {
+	{ .compatible = "qcom,bcm-voter" },
+	{ }
+};
+
+static struct platform_driver qcom_icc_bcm_voter_driver = {
+	.probe = qcom_icc_bcm_voter_probe,
+	.driver = {
+		.name		= "sdm845_bcm_voter",
Nit: This is not just about sdm845 anymore, so maybe qcom_bcm_voter.
quoted hunk ↗ jump to hunk
+		.of_match_table = bcm_voter_of_match,
+	},
+};
+module_platform_driver(qcom_icc_bcm_voter_driver);
+
+MODULE_AUTHOR("David Dai [off-list ref]");
+MODULE_DESCRIPTION("Qualcomm BCM Voter interconnect driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/interconnect/qcom/bcm-voter.h b/drivers/interconnect/qcom/bcm-voter.h
new file mode 100644
index 0000000000000..3436673e9f104
--- /dev/null
[.]
quoted hunk ↗ jump to hunk
diff --git a/drivers/interconnect/qcom/icc-rpmh.c b/drivers/interconnect/qcom/icc-rpmh.c
new file mode 100644
index 0000000000000..e9443b50e0b48
--- /dev/null
+++ b/drivers/interconnect/qcom/icc-rpmh.c
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/interconnect.h>
+#include <linux/interconnect-provider.h>
+
+#include "bcm-voter.h"
+#include "icc-rpmh.h"
+
+/**
+ * qcom_icc_pre_aggregate - cleans up stale values from prior icc_set
+ * @node: icc node to operate on
+ */
+void qcom_icc_pre_aggregate(struct icc_node *node)
+{
+	size_t i;
+	struct qcom_icc_node *qn;
+
+	qn = node->data;
+
+	for (i = 0; i < QCOM_ICC_NUM_BUCKETS; i++) {
+		qn->sum_avg[i] = 0;
+		qn->max_peak[i] = 0;
+	}
+}
+EXPORT_SYMBOL_GPL(qcom_icc_pre_aggregate);
+
+/**
+ * qcom_icc_aggregate - aggregate bw for buckets indicated by tag
+ * @node: node to aggregate
+ * @tag: tag to indicate which buckets to aggregate
+ * @avg_bw: new bw to sum aggregate
+ * @peak_bw: new bw to max aggregate
+ * @agg_avg: existing aggregate avg bw val
+ * @agg_peak: existing aggregate peak bw val
+ */
+int qcom_icc_aggregate(struct icc_node *node, u32 tag, u32 avg_bw,
+		       u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
+{
+	size_t i;
+	struct qcom_icc_node *qn;
+	struct qcom_icc_provider *qp;
+
+	qn = node->data;
+	qp = to_qcom_provider(node->provider);
+
+	if (!tag)
+		tag = QCOM_ICC_TAG_ALWAYS;
+
+	for (i = 0; i < QCOM_ICC_NUM_BUCKETS; i++) {
+		if (tag & BIT(i)) {
+			qn->sum_avg[i] += avg_bw;
+			qn->max_peak[i] = max_t(u32, qn->max_peak[i], peak_bw);
+		}
+	}
+
+	*agg_avg += avg_bw;
+	*agg_peak = max_t(u32, *agg_peak, peak_bw);
+
+	for (i = 0; i < qn->num_bcms; i++)
+		qcom_icc_bcm_voter_add(qp->voter, qn->bcms[i]);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(qcom_icc_aggregate);
+
+/**
+ * qcom_icc_set - set the constraints based on path
+ * @src: source node for the path to set constraints on
+ * @dst: destination node for the path to set constraints on
+ *
+ * Return: 0 on success, or an error code otherwise
+ */
+int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+	struct qcom_icc_provider *qp;
+	struct icc_node *node;
+
+	if (!src)
+		node = dst;
+	else
+		node = src;
+
+	qp = to_qcom_provider(node->provider);
+
+	qcom_icc_bcm_voter_commit(qp->voter);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(qcom_icc_set);
+
+/**
+ * qcom_icc_bcm_init - populates bcm aux data and connect qnodes
+ * @bcm: bcm to be initialized
+ * @dev: associated provider device
+ *
+ * Return: 0 on success, or an error code otherwise
+ */
+int qcom_icc_bcm_init(struct qcom_icc_bcm *bcm, struct device *dev)
+{
+	struct qcom_icc_node *qn;
+	const struct bcm_db *data;
+	size_t data_count;
+	int i;
+
+	/* BCM is already initialised*/
+	if (bcm->addr)
+		return 0;
+
+	bcm->addr = cmd_db_read_addr(bcm->name);
+	if (!bcm->addr) {
+		dev_err(dev, "%s could not find RPMh address\n",
+			bcm->name);
+		return -EINVAL;
+	}
+
+	data = cmd_db_read_aux_data(bcm->name, &data_count);
+	if (IS_ERR(data)) {
+		dev_err(dev, "%s command db read error (%ld)\n",
+			bcm->name, PTR_ERR(data));
+		return PTR_ERR(data);
+	}
+	if (!data_count) {
+		dev_err(dev, "%s command db missing or partial aux data\n",
+			bcm->name);
+		return -EINVAL;
+	}
+
+	bcm->aux_data.unit = le32_to_cpu(data->unit);
+	bcm->aux_data.width = le16_to_cpu(data->width);
+	bcm->aux_data.vcd = data->vcd;
+	bcm->aux_data.reserved = data->reserved;
+	INIT_LIST_HEAD(&bcm->list);
+	INIT_LIST_HEAD(&bcm->ws_list);
+
+	/* Link Qnodes to their respective BCMs */
+	for (i = 0; i < bcm->num_nodes; i++) {
+		qn = bcm->nodes[i];
+		qn->bcms[qn->num_bcms] = bcm;
+		qn->num_bcms++;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(qcom_icc_bcm_init);
When i tried building this as a module i see the following warning:
WARNING: modpost: missing MODULE_LICENSE() in drivers/interconnect/qcom/icc-rpmh.o

So we may want to include module.h and add MODULE_LICENSE("GPL v2") as specified
in the SPDX tag.

Thanks,
Georgi
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help