* [PATCH] interconnect: qcom: sdm660: Correct NOC_QOS_PRIORITY shift and mask
@ 2021-08-23 7:54 Shawn Guo
2021-09-01 14:31 ` Dmitry Baryshkov
2021-09-01 14:51 ` AngeloGioacchino Del Regno
0 siblings, 2 replies; 3+ messages in thread
From: Shawn Guo @ 2021-08-23 7:54 UTC (permalink / raw)
To: Georgi Djakov
Cc: AngeloGioacchino Del Regno, Bjorn Andersson, linux-arm-msm,
linux-pm, Shawn Guo
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@linaro.org>
---
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
#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;
return regmap_update_bits(rmap, NOC_QOS_PRIORITYn_ADDR(qos->qos_port),
- NOC_QOS_PRIORITY_MASK, val);
+ NOC_QOS_PRIORITY_P0_MASK, val);
}
static int qcom_icc_set_noc_qos(struct icc_node *src, u64 max_bw)
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] interconnect: qcom: sdm660: Correct NOC_QOS_PRIORITY shift and mask
2021-08-23 7:54 [PATCH] interconnect: qcom: sdm660: Correct NOC_QOS_PRIORITY shift and mask Shawn Guo
@ 2021-09-01 14:31 ` Dmitry Baryshkov
2021-09-01 14:51 ` AngeloGioacchino Del Regno
1 sibling, 0 replies; 3+ messages in thread
From: Dmitry Baryshkov @ 2021-09-01 14:31 UTC (permalink / raw)
To: Shawn Guo, Georgi Djakov
Cc: AngeloGioacchino Del Regno, Bjorn Andersson, linux-arm-msm, linux-pm
On 23/08/2021 10:54, Shawn Guo wrote:
> 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@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> 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
>
> #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;
> return regmap_update_bits(rmap, NOC_QOS_PRIORITYn_ADDR(qos->qos_port),
> - NOC_QOS_PRIORITY_MASK, val);
> + NOC_QOS_PRIORITY_P0_MASK, val);
> }
>
> static int qcom_icc_set_noc_qos(struct icc_node *src, u64 max_bw)
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] interconnect: qcom: sdm660: Correct NOC_QOS_PRIORITY shift and mask
2021-08-23 7:54 [PATCH] interconnect: qcom: sdm660: Correct NOC_QOS_PRIORITY shift and mask Shawn Guo
2021-09-01 14:31 ` Dmitry Baryshkov
@ 2021-09-01 14:51 ` AngeloGioacchino Del Regno
1 sibling, 0 replies; 3+ messages in thread
From: AngeloGioacchino Del Regno @ 2021-09-01 14:51 UTC (permalink / raw)
To: Shawn Guo, Georgi Djakov
Cc: AngeloGioacchino Del Regno, Bjorn Andersson, linux-arm-msm, linux-pm
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@linaro.org>
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)
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-09-01 14:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-23 7:54 [PATCH] interconnect: qcom: sdm660: Correct NOC_QOS_PRIORITY shift and mask Shawn Guo
2021-09-01 14:31 ` Dmitry Baryshkov
2021-09-01 14:51 ` AngeloGioacchino Del Regno
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).