Il 23/08/21 09:54, Shawn Guo ha scritto:
The NOC_QOS_PRIORITY shift and mask do not match what vendor kernel
defines [1]. Correct them per vendor kernel.
[1] https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/drivers/soc/qcom/msm_bus/msm_bus_noc_adhoc.c?h=LA.UM.8.2.r1-04800-sdm660.0#n37
Signed-off-by: Shawn Guo <shawn.guo@xxxxxxxxxx>
Hello Shawn,
the patch looks great and I'm sorry for getting it wrong in the first place;
luckily there was no regression due to this mistake.
However, a few considerations:
---
drivers/interconnect/qcom/sdm660.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/interconnect/qcom/sdm660.c b/drivers/interconnect/qcom/sdm660.c
index ac13046537e8..c89c991a80a0 100644
--- a/drivers/interconnect/qcom/sdm660.c
+++ b/drivers/interconnect/qcom/sdm660.c
@@ -44,9 +44,10 @@
#define NOC_PERM_MODE_BYPASS (1 << NOC_QOS_MODE_BYPASS)
#define NOC_QOS_PRIORITYn_ADDR(n) (0x8 + (n * 0x1000))
-#define NOC_QOS_PRIORITY_MASK 0xf
+#define NOC_QOS_PRIORITY_P1_MASK 0xc
+#define NOC_QOS_PRIORITY_P0_MASK 0x3
#define NOC_QOS_PRIORITY_P1_SHIFT 0x2
-#define NOC_QOS_PRIORITY_P0_SHIFT 0x3
+#define NOC_QOS_PRIORITY_P0_SHIFT 0x0
Defining NOC_QOS_PRIORITY_P0_SHIFT as 0x0 means that there's no shifting action,
which means that we can avoid defining this at all.
#define NOC_QOS_MODEn_ADDR(n) (0xc + (n * 0x1000))
#define NOC_QOS_MODEn_MASK 0x3
@@ -624,13 +625,13 @@ static int qcom_icc_noc_set_qos_priority(struct regmap *rmap,
/* Must be updated one at a time, P1 first, P0 last */
val = qos->areq_prio << NOC_QOS_PRIORITY_P1_SHIFT;
rc = regmap_update_bits(rmap, NOC_QOS_PRIORITYn_ADDR(qos->qos_port),
- NOC_QOS_PRIORITY_MASK, val);
+ NOC_QOS_PRIORITY_P1_MASK, val);
if (rc)
return rc;
val = qos->prio_level << NOC_QOS_PRIORITY_P0_SHIFT;
... so this would be `qos->prio_level << 0`, so if we simply not define any
NOC_QOS_PRIORITY_P0_SHIFT, this can be simplified as:
val = qos->prio_level;
...but that would also eliminate the need to assign it to `val`, so...
return regmap_update_bits(rmap, NOC_QOS_PRIORITYn_ADDR(qos->qos_port),
- NOC_QOS_PRIORITY_MASK, val);
+ NOC_QOS_PRIORITY_P0_MASK, val);
Here we can instead do:
return regmap_update_bits(rmap, NOC_QOS_PRIORITYn_ADDR(qos->qos_port),
NOC_QOS_PRIORITY_MASK, qos->prio_level);
NOC_QOS_PRIORITY_P0_MASK, qos->prio_level);
Cheers,
- Angelo
}
static int qcom_icc_set_noc_qos(struct icc_node *src, u64 max_bw)
