Currently NOC_QOS_MODE_FIXED is defined as 0x0 which makes it the
default option (partial struct initialization). The default option
however should be NOC_QOS_MODE_INVALID.
That results in bogus QoS configurations being sent for port 0 (which
is used for the DRAM endpoint on BIMC, for example) coming from all nodes
with .qos.ap_owned = true and uninitialized .qos.qos_mode. It's also an
issue for newer SoCs where all nodes are treated as if they were ap_owned,
but not all of them have QoS configuration.
The NOC_QOS_MODEs are defined as preprocessor constants and are not used
anywhere outside qcom_icc_set_noc_qos(), which is easily worked around.
Separate the desc->type values from the values sent to msmbus in the
aforementioned function. Make the former an enum for better mainainability.
Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
---
drivers/interconnect/qcom/icc-rpm.c | 24 +++++++++++++-----------
drivers/interconnect/qcom/icc-rpm.h | 10 ++++++----
2 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/interconnect/qcom/icc-rpm.c b/drivers/interconnect/qcom/icc-rpm.c
index 4d0997b210f7..35fd75ae70e3 100644
--- a/drivers/interconnect/qcom/icc-rpm.c
+++ b/drivers/interconnect/qcom/icc-rpm.c
@@ -48,6 +48,9 @@
#define NOC_QOS_MODEn_ADDR(n) (0xc + (n * 0x1000))
#define NOC_QOS_MODEn_MASK 0x3
+#define NOC_QOS_MODE_FIXED_VAL 0x0
+#define NOC_QOS_MODE_BYPASS_VAL 0x2
+
static int qcom_icc_set_qnoc_qos(struct icc_node *src, u64 max_bw)
{
struct icc_provider *provider = src->provider;
@@ -153,7 +156,7 @@ static int qcom_icc_set_noc_qos(struct icc_node *src, u64 max_bw)
struct qcom_icc_provider *qp;
struct qcom_icc_node *qn;
struct icc_provider *provider;
- u32 mode = NOC_QOS_MODE_BYPASS;
+ u32 mode = NOC_QOS_MODE_BYPASS_VAL;
int rc = 0;
qn = src->data;
@@ -167,18 +170,17 @@ static int qcom_icc_set_noc_qos(struct icc_node *src, u64 max_bw)
return 0;
}
- if (qn->qos.qos_mode != NOC_QOS_MODE_INVALID)
- mode = qn->qos.qos_mode;
-
- if (mode == NOC_QOS_MODE_FIXED) {
- dev_dbg(src->provider->dev, "NoC QoS: %s: Set Fixed mode\n",
- qn->name);
+ if (qn->qos.qos_mode == NOC_QOS_MODE_FIXED) {
+ dev_dbg(src->provider->dev, "NoC QoS: %s: Set Fixed mode\n", qn->name);
+ mode = NOC_QOS_MODE_FIXED_VAL;
rc = qcom_icc_noc_set_qos_priority(qp, &qn->qos);
if (rc)
return rc;
- } else if (mode == NOC_QOS_MODE_BYPASS) {
- dev_dbg(src->provider->dev, "NoC QoS: %s: Set Bypass mode\n",
- qn->name);
+ } else if (qn->qos.qos_mode == NOC_QOS_MODE_BYPASS) {
+ dev_dbg(src->provider->dev, "NoC QoS: %s: Set Bypass mode\n", qn->name);
+ mode = NOC_QOS_MODE_BYPASS_VAL;
+ } else {
+ /* How did we get here? */
}
return regmap_update_bits(qp->regmap,
@@ -244,7 +246,7 @@ static int __qcom_icc_set(struct icc_node *n, struct qcom_icc_node *qn,
ret = qcom_icc_rpm_set(qn->mas_rpm_id, qn->slv_rpm_id, sum_bw);
if (ret)
return ret;
- } else if (qn->qos.qos_mode != -1) {
+ } else if (qn->qos.qos_mode != NOC_QOS_MODE_INVALID) {
/* set bandwidth directly from the AP */
ret = qcom_icc_qos_set(n, sum_bw);
if (ret)
diff --git a/drivers/interconnect/qcom/icc-rpm.h b/drivers/interconnect/qcom/icc-rpm.h
index a49af844ab13..8ba1918d7997 100644
--- a/drivers/interconnect/qcom/icc-rpm.h
+++ b/drivers/interconnect/qcom/icc-rpm.h
@@ -97,10 +97,12 @@ struct qcom_icc_desc {
unsigned int qos_offset;
};
-/* Valid for both NoC and BIMC */
-#define NOC_QOS_MODE_INVALID -1
-#define NOC_QOS_MODE_FIXED 0x0
-#define NOC_QOS_MODE_BYPASS 0x2
+/* Valid for all bus types */
+enum qos_mode {
+ NOC_QOS_MODE_INVALID = 0,
+ NOC_QOS_MODE_FIXED,
+ NOC_QOS_MODE_BYPASS,
+};
int qnoc_probe(struct platform_device *pdev);
int qnoc_remove(struct platform_device *pdev);
--
2.39.2
