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.hdiff --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