All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] soc: qcom: rpmhpd: Rename rpmhpd struct names
@ 2021-11-16  5:26 Rajendra Nayak
  2021-11-16  5:26 ` [PATCH 2/2] soc: qcom: rpmhpd: Make mx as a parent of cx only for sdm845 Rajendra Nayak
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Rajendra Nayak @ 2021-11-16  5:26 UTC (permalink / raw)
  To: agross, bjorn.andersson
  Cc: linux-arm-msm, linux-kernel, swboyd, Rajendra Nayak

The rpmhpd structs were named with a SoC-name prefix, but then
they got reused across multiple SoC families making things confusing.
Rename all the struct names to remove SoC-name prefixes.
No other functional change as part of this patch.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/soc/qcom/rpmhpd.c | 255 +++++++++++++++++++++++-----------------------
 1 file changed, 128 insertions(+), 127 deletions(-)

diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
index 1118345..c71481d 100644
--- a/drivers/soc/qcom/rpmhpd.c
+++ b/drivers/soc/qcom/rpmhpd.c
@@ -63,73 +63,102 @@ struct rpmhpd_desc {
 
 static DEFINE_MUTEX(rpmhpd_lock);
 
-/* SDM845 RPMH powerdomains */
+/* RPMH powerdomains */
 
-static struct rpmhpd sdm845_ebi = {
+static struct rpmhpd ebi = {
 	.pd = { .name = "ebi", },
 	.res_name = "ebi.lvl",
 };
 
-static struct rpmhpd sdm845_lmx = {
+static struct rpmhpd lmx = {
 	.pd = { .name = "lmx", },
 	.res_name = "lmx.lvl",
 };
 
-static struct rpmhpd sdm845_lcx = {
+static struct rpmhpd lcx = {
 	.pd = { .name = "lcx", },
 	.res_name = "lcx.lvl",
 };
 
-static struct rpmhpd sdm845_gfx = {
+static struct rpmhpd gfx = {
 	.pd = { .name = "gfx", },
 	.res_name = "gfx.lvl",
 };
 
-static struct rpmhpd sdm845_mss = {
+static struct rpmhpd mss = {
 	.pd = { .name = "mss", },
 	.res_name = "mss.lvl",
 };
 
-static struct rpmhpd sdm845_mx_ao;
-static struct rpmhpd sdm845_mx = {
+static struct rpmhpd mx_ao;
+static struct rpmhpd mx = {
 	.pd = { .name = "mx", },
-	.peer = &sdm845_mx_ao,
+	.peer = &mx_ao,
 	.res_name = "mx.lvl",
 };
 
-static struct rpmhpd sdm845_mx_ao = {
+static struct rpmhpd mx_ao = {
 	.pd = { .name = "mx_ao", },
 	.active_only = true,
-	.peer = &sdm845_mx,
+	.peer = &mx,
 	.res_name = "mx.lvl",
 };
 
-static struct rpmhpd sdm845_cx_ao;
-static struct rpmhpd sdm845_cx = {
+static struct rpmhpd cx_ao;
+static struct rpmhpd cx = {
 	.pd = { .name = "cx", },
-	.peer = &sdm845_cx_ao,
-	.parent = &sdm845_mx.pd,
+	.peer = &cx_ao,
+	.parent = &mx.pd,
 	.res_name = "cx.lvl",
 };
 
-static struct rpmhpd sdm845_cx_ao = {
+static struct rpmhpd cx_ao = {
 	.pd = { .name = "cx_ao", },
 	.active_only = true,
-	.peer = &sdm845_cx,
-	.parent = &sdm845_mx_ao.pd,
+	.peer = &cx,
+	.parent = &mx_ao.pd,
 	.res_name = "cx.lvl",
 };
 
+static struct rpmhpd mmcx_ao;
+static struct rpmhpd mmcx = {
+	.pd = { .name = "mmcx", },
+	.peer = &mmcx_ao,
+	.res_name = "mmcx.lvl",
+};
+
+static struct rpmhpd mmcx_ao = {
+	.pd = { .name = "mmcx_ao", },
+	.active_only = true,
+	.peer = &mmcx,
+	.res_name = "mmcx.lvl",
+};
+
+static struct rpmhpd mxc_ao;
+static struct rpmhpd mxc = {
+	.pd = { .name = "mxc", },
+	.peer = &mxc_ao,
+	.res_name = "mxc.lvl",
+};
+
+static struct rpmhpd mxc_ao = {
+	.pd = { .name = "mxc_ao", },
+	.active_only = true,
+	.peer = &mxc,
+	.res_name = "mxc.lvl",
+};
+
+/* SDM845 RPMH powerdomains */
 static struct rpmhpd *sdm845_rpmhpds[] = {
-	[SDM845_EBI] = &sdm845_ebi,
-	[SDM845_MX] = &sdm845_mx,
-	[SDM845_MX_AO] = &sdm845_mx_ao,
-	[SDM845_CX] = &sdm845_cx,
-	[SDM845_CX_AO] = &sdm845_cx_ao,
-	[SDM845_LMX] = &sdm845_lmx,
-	[SDM845_LCX] = &sdm845_lcx,
-	[SDM845_GFX] = &sdm845_gfx,
-	[SDM845_MSS] = &sdm845_mss,
+	[SDM845_EBI] = &ebi,
+	[SDM845_MX] = &mx,
+	[SDM845_MX_AO] = &mx_ao,
+	[SDM845_CX] = &cx,
+	[SDM845_CX_AO] = &cx_ao,
+	[SDM845_LMX] = &lmx,
+	[SDM845_LCX] = &lcx,
+	[SDM845_GFX] = &gfx,
+	[SDM845_MSS] = &mss,
 };
 
 static const struct rpmhpd_desc sdm845_desc = {
@@ -139,9 +168,9 @@ static const struct rpmhpd_desc sdm845_desc = {
 
 /* SDX55 RPMH powerdomains */
 static struct rpmhpd *sdx55_rpmhpds[] = {
-	[SDX55_MSS] = &sdm845_mss,
-	[SDX55_MX] = &sdm845_mx,
-	[SDX55_CX] = &sdm845_cx,
+	[SDX55_MSS] = &mss,
+	[SDX55_MX] = &mx,
+	[SDX55_CX] = &cx,
 };
 
 static const struct rpmhpd_desc sdx55_desc = {
@@ -151,12 +180,12 @@ static const struct rpmhpd_desc sdx55_desc = {
 
 /* SM6350 RPMH powerdomains */
 static struct rpmhpd *sm6350_rpmhpds[] = {
-	[SM6350_CX] = &sdm845_cx,
-	[SM6350_GFX] = &sdm845_gfx,
-	[SM6350_LCX] = &sdm845_lcx,
-	[SM6350_LMX] = &sdm845_lmx,
-	[SM6350_MSS] = &sdm845_mss,
-	[SM6350_MX] = &sdm845_mx,
+	[SM6350_CX] = &cx,
+	[SM6350_GFX] = &gfx,
+	[SM6350_LCX] = &lcx,
+	[SM6350_LMX] = &lmx,
+	[SM6350_MSS] = &mss,
+	[SM6350_MX] = &mx,
 };
 
 static const struct rpmhpd_desc sm6350_desc = {
@@ -165,33 +194,18 @@ static const struct rpmhpd_desc sm6350_desc = {
 };
 
 /* SM8150 RPMH powerdomains */
-
-static struct rpmhpd sm8150_mmcx_ao;
-static struct rpmhpd sm8150_mmcx = {
-	.pd = { .name = "mmcx", },
-	.peer = &sm8150_mmcx_ao,
-	.res_name = "mmcx.lvl",
-};
-
-static struct rpmhpd sm8150_mmcx_ao = {
-	.pd = { .name = "mmcx_ao", },
-	.active_only = true,
-	.peer = &sm8150_mmcx,
-	.res_name = "mmcx.lvl",
-};
-
 static struct rpmhpd *sm8150_rpmhpds[] = {
-	[SM8150_MSS] = &sdm845_mss,
-	[SM8150_EBI] = &sdm845_ebi,
-	[SM8150_LMX] = &sdm845_lmx,
-	[SM8150_LCX] = &sdm845_lcx,
-	[SM8150_GFX] = &sdm845_gfx,
-	[SM8150_MX] = &sdm845_mx,
-	[SM8150_MX_AO] = &sdm845_mx_ao,
-	[SM8150_CX] = &sdm845_cx,
-	[SM8150_CX_AO] = &sdm845_cx_ao,
-	[SM8150_MMCX] = &sm8150_mmcx,
-	[SM8150_MMCX_AO] = &sm8150_mmcx_ao,
+	[SM8150_MSS] = &mss,
+	[SM8150_EBI] = &ebi,
+	[SM8150_LMX] = &lmx,
+	[SM8150_LCX] = &lcx,
+	[SM8150_GFX] = &gfx,
+	[SM8150_MX] = &mx,
+	[SM8150_MX_AO] = &mx_ao,
+	[SM8150_CX] = &cx,
+	[SM8150_CX_AO] = &cx_ao,
+	[SM8150_MMCX] = &mmcx,
+	[SM8150_MMCX_AO] = &mmcx_ao,
 };
 
 static const struct rpmhpd_desc sm8150_desc = {
@@ -199,17 +213,18 @@ static const struct rpmhpd_desc sm8150_desc = {
 	.num_pds = ARRAY_SIZE(sm8150_rpmhpds),
 };
 
+/* SM8250 RPMH powerdomains */
 static struct rpmhpd *sm8250_rpmhpds[] = {
-	[SM8250_CX] = &sdm845_cx,
-	[SM8250_CX_AO] = &sdm845_cx_ao,
-	[SM8250_EBI] = &sdm845_ebi,
-	[SM8250_GFX] = &sdm845_gfx,
-	[SM8250_LCX] = &sdm845_lcx,
-	[SM8250_LMX] = &sdm845_lmx,
-	[SM8250_MMCX] = &sm8150_mmcx,
-	[SM8250_MMCX_AO] = &sm8150_mmcx_ao,
-	[SM8250_MX] = &sdm845_mx,
-	[SM8250_MX_AO] = &sdm845_mx_ao,
+	[SM8250_CX] = &cx,
+	[SM8250_CX_AO] = &cx_ao,
+	[SM8250_EBI] = &ebi,
+	[SM8250_GFX] = &gfx,
+	[SM8250_LCX] = &lcx,
+	[SM8250_LMX] = &lmx,
+	[SM8250_MMCX] = &mmcx,
+	[SM8250_MMCX_AO] = &mmcx_ao,
+	[SM8250_MX] = &mx,
+	[SM8250_MX_AO] = &mx_ao,
 };
 
 static const struct rpmhpd_desc sm8250_desc = {
@@ -218,34 +233,20 @@ static const struct rpmhpd_desc sm8250_desc = {
 };
 
 /* SM8350 Power domains */
-static struct rpmhpd sm8350_mxc_ao;
-static struct rpmhpd sm8350_mxc = {
-	.pd = { .name = "mxc", },
-	.peer = &sm8350_mxc_ao,
-	.res_name = "mxc.lvl",
-};
-
-static struct rpmhpd sm8350_mxc_ao = {
-	.pd = { .name = "mxc_ao", },
-	.active_only = true,
-	.peer = &sm8350_mxc,
-	.res_name = "mxc.lvl",
-};
-
 static struct rpmhpd *sm8350_rpmhpds[] = {
-	[SM8350_CX] = &sdm845_cx,
-	[SM8350_CX_AO] = &sdm845_cx_ao,
-	[SM8350_EBI] = &sdm845_ebi,
-	[SM8350_GFX] = &sdm845_gfx,
-	[SM8350_LCX] = &sdm845_lcx,
-	[SM8350_LMX] = &sdm845_lmx,
-	[SM8350_MMCX] = &sm8150_mmcx,
-	[SM8350_MMCX_AO] = &sm8150_mmcx_ao,
-	[SM8350_MX] = &sdm845_mx,
-	[SM8350_MX_AO] = &sdm845_mx_ao,
-	[SM8350_MXC] = &sm8350_mxc,
-	[SM8350_MXC_AO] = &sm8350_mxc_ao,
-	[SM8350_MSS] = &sdm845_mss,
+	[SM8350_CX] = &cx,
+	[SM8350_CX_AO] = &cx_ao,
+	[SM8350_EBI] = &ebi,
+	[SM8350_GFX] = &gfx,
+	[SM8350_LCX] = &lcx,
+	[SM8350_LMX] = &lmx,
+	[SM8350_MMCX] = &mmcx,
+	[SM8350_MMCX_AO] = &mmcx_ao,
+	[SM8350_MX] = &mx,
+	[SM8350_MX_AO] = &mx_ao,
+	[SM8350_MXC] = &mxc,
+	[SM8350_MXC_AO] = &mxc_ao,
+	[SM8350_MSS] = &mss,
 };
 
 static const struct rpmhpd_desc sm8350_desc = {
@@ -255,14 +256,14 @@ static const struct rpmhpd_desc sm8350_desc = {
 
 /* SC7180 RPMH powerdomains */
 static struct rpmhpd *sc7180_rpmhpds[] = {
-	[SC7180_CX] = &sdm845_cx,
-	[SC7180_CX_AO] = &sdm845_cx_ao,
-	[SC7180_GFX] = &sdm845_gfx,
-	[SC7180_MX] = &sdm845_mx,
-	[SC7180_MX_AO] = &sdm845_mx_ao,
-	[SC7180_LMX] = &sdm845_lmx,
-	[SC7180_LCX] = &sdm845_lcx,
-	[SC7180_MSS] = &sdm845_mss,
+	[SC7180_CX] = &cx,
+	[SC7180_CX_AO] = &cx_ao,
+	[SC7180_GFX] = &gfx,
+	[SC7180_MX] = &mx,
+	[SC7180_MX_AO] = &mx_ao,
+	[SC7180_LMX] = &lmx,
+	[SC7180_LCX] = &lcx,
+	[SC7180_MSS] = &mss,
 };
 
 static const struct rpmhpd_desc sc7180_desc = {
@@ -272,15 +273,15 @@ static const struct rpmhpd_desc sc7180_desc = {
 
 /* SC7280 RPMH powerdomains */
 static struct rpmhpd *sc7280_rpmhpds[] = {
-	[SC7280_CX] = &sdm845_cx,
-	[SC7280_CX_AO] = &sdm845_cx_ao,
-	[SC7280_EBI] = &sdm845_ebi,
-	[SC7280_GFX] = &sdm845_gfx,
-	[SC7280_MX] = &sdm845_mx,
-	[SC7280_MX_AO] = &sdm845_mx_ao,
-	[SC7280_LMX] = &sdm845_lmx,
-	[SC7280_LCX] = &sdm845_lcx,
-	[SC7280_MSS] = &sdm845_mss,
+	[SC7280_CX] = &cx,
+	[SC7280_CX_AO] = &cx_ao,
+	[SC7280_EBI] = &ebi,
+	[SC7280_GFX] = &gfx,
+	[SC7280_MX] = &mx,
+	[SC7280_MX_AO] = &mx_ao,
+	[SC7280_LMX] = &lmx,
+	[SC7280_LCX] = &lcx,
+	[SC7280_MSS] = &mss,
 };
 
 static const struct rpmhpd_desc sc7280_desc = {
@@ -290,17 +291,17 @@ static const struct rpmhpd_desc sc7280_desc = {
 
 /* SC8180x RPMH powerdomains */
 static struct rpmhpd *sc8180x_rpmhpds[] = {
-	[SC8180X_CX] = &sdm845_cx,
-	[SC8180X_CX_AO] = &sdm845_cx_ao,
-	[SC8180X_EBI] = &sdm845_ebi,
-	[SC8180X_GFX] = &sdm845_gfx,
-	[SC8180X_LCX] = &sdm845_lcx,
-	[SC8180X_LMX] = &sdm845_lmx,
-	[SC8180X_MMCX] = &sm8150_mmcx,
-	[SC8180X_MMCX_AO] = &sm8150_mmcx_ao,
-	[SC8180X_MSS] = &sdm845_mss,
-	[SC8180X_MX] = &sdm845_mx,
-	[SC8180X_MX_AO] = &sdm845_mx_ao,
+	[SC8180X_CX] = &cx,
+	[SC8180X_CX_AO] = &cx_ao,
+	[SC8180X_EBI] = &ebi,
+	[SC8180X_GFX] = &gfx,
+	[SC8180X_LCX] = &lcx,
+	[SC8180X_LMX] = &lmx,
+	[SC8180X_MMCX] = &mmcx,
+	[SC8180X_MMCX_AO] = &mmcx_ao,
+	[SC8180X_MSS] = &mss,
+	[SC8180X_MX] = &mx,
+	[SC8180X_MX_AO] = &mx_ao,
 };
 
 static const struct rpmhpd_desc sc8180x_desc = {
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] soc: qcom: rpmhpd: Make mx as a parent of cx only for sdm845
  2021-11-16  5:26 [PATCH 1/2] soc: qcom: rpmhpd: Rename rpmhpd struct names Rajendra Nayak
@ 2021-11-16  5:26 ` Rajendra Nayak
  2021-11-19  3:19   ` Bjorn Andersson
  2021-11-18 18:26 ` [PATCH 1/2] soc: qcom: rpmhpd: Rename rpmhpd struct names Matthias Kaehlcke
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Rajendra Nayak @ 2021-11-16  5:26 UTC (permalink / raw)
  To: agross, bjorn.andersson
  Cc: linux-arm-msm, linux-kernel, swboyd, Rajendra Nayak

The requirement to specify the active + sleep and active-only MX power
domains as the parents of the corresponding CX power domains is applicable
only on the sdm845 SoC. With the same struct definition reused for all the
SoCs this condition was wrongly applied to all those SoCs as well, which
isn't needed. Define new sdm845 specific structures to manage this
dependency and remove the parent assignements from the common structure.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/soc/qcom/rpmhpd.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
index c71481d..12d8ce9 100644
--- a/drivers/soc/qcom/rpmhpd.c
+++ b/drivers/soc/qcom/rpmhpd.c
@@ -108,7 +108,6 @@ static struct rpmhpd cx_ao;
 static struct rpmhpd cx = {
 	.pd = { .name = "cx", },
 	.peer = &cx_ao,
-	.parent = &mx.pd,
 	.res_name = "cx.lvl",
 };
 
@@ -116,7 +115,6 @@ static struct rpmhpd cx_ao = {
 	.pd = { .name = "cx_ao", },
 	.active_only = true,
 	.peer = &cx,
-	.parent = &mx_ao.pd,
 	.res_name = "cx.lvl",
 };
 
@@ -149,12 +147,28 @@ static struct rpmhpd mxc_ao = {
 };
 
 /* SDM845 RPMH powerdomains */
+static struct rpmhpd sdm845_cx_ao;
+static struct rpmhpd sdm845_cx = {
+	.pd = { .name = "cx", },
+	.peer = &sdm845_cx_ao,
+	.parent = &mx.pd,
+	.res_name = "cx.lvl",
+};
+
+static struct rpmhpd sdm845_cx_ao = {
+	.pd = { .name = "cx_ao", },
+	.active_only = true,
+	.peer = &sdm845_cx,
+	.parent = &mx_ao.pd,
+	.res_name = "cx.lvl",
+};
+
 static struct rpmhpd *sdm845_rpmhpds[] = {
 	[SDM845_EBI] = &ebi,
 	[SDM845_MX] = &mx,
 	[SDM845_MX_AO] = &mx_ao,
-	[SDM845_CX] = &cx,
-	[SDM845_CX_AO] = &cx_ao,
+	[SDM845_CX] = &sdm845_cx,
+	[SDM845_CX_AO] = &sdm845_cx_ao,
 	[SDM845_LMX] = &lmx,
 	[SDM845_LCX] = &lcx,
 	[SDM845_GFX] = &gfx,
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] soc: qcom: rpmhpd: Rename rpmhpd struct names
  2021-11-16  5:26 [PATCH 1/2] soc: qcom: rpmhpd: Rename rpmhpd struct names Rajendra Nayak
  2021-11-16  5:26 ` [PATCH 2/2] soc: qcom: rpmhpd: Make mx as a parent of cx only for sdm845 Rajendra Nayak
@ 2021-11-18 18:26 ` Matthias Kaehlcke
  2021-11-19  3:11 ` Bjorn Andersson
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Matthias Kaehlcke @ 2021-11-18 18:26 UTC (permalink / raw)
  To: Rajendra Nayak
  Cc: agross, bjorn.andersson, linux-arm-msm, linux-kernel, swboyd

On Tue, Nov 16, 2021 at 10:56:21AM +0530, Rajendra Nayak wrote:
> The rpmhpd structs were named with a SoC-name prefix, but then
> they got reused across multiple SoC families making things confusing.
> Rename all the struct names to remove SoC-name prefixes.
> No other functional change as part of this patch.
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  drivers/soc/qcom/rpmhpd.c | 255 +++++++++++++++++++++++-----------------------
>  1 file changed, 128 insertions(+), 127 deletions(-)
> 
> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
> index 1118345..c71481d 100644
> --- a/drivers/soc/qcom/rpmhpd.c
> +++ b/drivers/soc/qcom/rpmhpd.c
> @@ -63,73 +63,102 @@ struct rpmhpd_desc {
>  
>  static DEFINE_MUTEX(rpmhpd_lock);
>  
> -/* SDM845 RPMH powerdomains */
> +/* RPMH powerdomains */
>  
> -static struct rpmhpd sdm845_ebi = {
> +static struct rpmhpd ebi = {
>  	.pd = { .name = "ebi", },
>  	.res_name = "ebi.lvl",
>  };
>  
> -static struct rpmhpd sdm845_lmx = {
> +static struct rpmhpd lmx = {
>  	.pd = { .name = "lmx", },
>  	.res_name = "lmx.lvl",
>  };
>  
> -static struct rpmhpd sdm845_lcx = {
> +static struct rpmhpd lcx = {
>  	.pd = { .name = "lcx", },
>  	.res_name = "lcx.lvl",
>  };
>  
> -static struct rpmhpd sdm845_gfx = {
> +static struct rpmhpd gfx = {
>  	.pd = { .name = "gfx", },
>  	.res_name = "gfx.lvl",
>  };
>  
> -static struct rpmhpd sdm845_mss = {
> +static struct rpmhpd mss = {
>  	.pd = { .name = "mss", },
>  	.res_name = "mss.lvl",
>  };
>  
> -static struct rpmhpd sdm845_mx_ao;
> -static struct rpmhpd sdm845_mx = {
> +static struct rpmhpd mx_ao;
> +static struct rpmhpd mx = {
>  	.pd = { .name = "mx", },
> -	.peer = &sdm845_mx_ao,
> +	.peer = &mx_ao,
>  	.res_name = "mx.lvl",
>  };
>  
> -static struct rpmhpd sdm845_mx_ao = {
> +static struct rpmhpd mx_ao = {
>  	.pd = { .name = "mx_ao", },
>  	.active_only = true,
> -	.peer = &sdm845_mx,
> +	.peer = &mx,
>  	.res_name = "mx.lvl",
>  };
>  
> -static struct rpmhpd sdm845_cx_ao;
> -static struct rpmhpd sdm845_cx = {
> +static struct rpmhpd cx_ao;
> +static struct rpmhpd cx = {
>  	.pd = { .name = "cx", },
> -	.peer = &sdm845_cx_ao,
> -	.parent = &sdm845_mx.pd,
> +	.peer = &cx_ao,
> +	.parent = &mx.pd,
>  	.res_name = "cx.lvl",
>  };
>  
> -static struct rpmhpd sdm845_cx_ao = {
> +static struct rpmhpd cx_ao = {
>  	.pd = { .name = "cx_ao", },
>  	.active_only = true,
> -	.peer = &sdm845_cx,
> -	.parent = &sdm845_mx_ao.pd,
> +	.peer = &cx,
> +	.parent = &mx_ao.pd,
>  	.res_name = "cx.lvl",
>  };
>  
> +static struct rpmhpd mmcx_ao;
> +static struct rpmhpd mmcx = {
> +	.pd = { .name = "mmcx", },
> +	.peer = &mmcx_ao,
> +	.res_name = "mmcx.lvl",
> +};
> +
> +static struct rpmhpd mmcx_ao = {
> +	.pd = { .name = "mmcx_ao", },
> +	.active_only = true,
> +	.peer = &mmcx,
> +	.res_name = "mmcx.lvl",
> +};
> +
> +static struct rpmhpd mxc_ao;
> +static struct rpmhpd mxc = {
> +	.pd = { .name = "mxc", },
> +	.peer = &mxc_ao,
> +	.res_name = "mxc.lvl",
> +};
> +
> +static struct rpmhpd mxc_ao = {
> +	.pd = { .name = "mxc_ao", },
> +	.active_only = true,
> +	.peer = &mxc,
> +	.res_name = "mxc.lvl",
> +};
> +
> +/* SDM845 RPMH powerdomains */
>  static struct rpmhpd *sdm845_rpmhpds[] = {
> -	[SDM845_EBI] = &sdm845_ebi,
> -	[SDM845_MX] = &sdm845_mx,
> -	[SDM845_MX_AO] = &sdm845_mx_ao,
> -	[SDM845_CX] = &sdm845_cx,
> -	[SDM845_CX_AO] = &sdm845_cx_ao,
> -	[SDM845_LMX] = &sdm845_lmx,
> -	[SDM845_LCX] = &sdm845_lcx,
> -	[SDM845_GFX] = &sdm845_gfx,
> -	[SDM845_MSS] = &sdm845_mss,
> +	[SDM845_EBI] = &ebi,
> +	[SDM845_MX] = &mx,
> +	[SDM845_MX_AO] = &mx_ao,
> +	[SDM845_CX] = &cx,
> +	[SDM845_CX_AO] = &cx_ao,
> +	[SDM845_LMX] = &lmx,
> +	[SDM845_LCX] = &lcx,
> +	[SDM845_GFX] = &gfx,
> +	[SDM845_MSS] = &mss,
>  };

nit: some PD lists are ordered alphabetically, others aren't, since you are
already changing them you could use alphabetical order for all of them.

Just a nit though, the change generally looks good to me, so:

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] soc: qcom: rpmhpd: Rename rpmhpd struct names
  2021-11-16  5:26 [PATCH 1/2] soc: qcom: rpmhpd: Rename rpmhpd struct names Rajendra Nayak
  2021-11-16  5:26 ` [PATCH 2/2] soc: qcom: rpmhpd: Make mx as a parent of cx only for sdm845 Rajendra Nayak
  2021-11-18 18:26 ` [PATCH 1/2] soc: qcom: rpmhpd: Rename rpmhpd struct names Matthias Kaehlcke
@ 2021-11-19  3:11 ` Bjorn Andersson
  2021-12-01 20:44 ` Stephen Boyd
  2021-12-15 22:27 ` Bjorn Andersson
  4 siblings, 0 replies; 9+ messages in thread
From: Bjorn Andersson @ 2021-11-19  3:11 UTC (permalink / raw)
  To: Rajendra Nayak; +Cc: agross, linux-arm-msm, linux-kernel, swboyd

On Mon 15 Nov 23:26 CST 2021, Rajendra Nayak wrote:

> The rpmhpd structs were named with a SoC-name prefix, but then
> they got reused across multiple SoC families making things confusing.
> Rename all the struct names to remove SoC-name prefixes.
> No other functional change as part of this patch.
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>

This has bothered me for a while, thanks for sorting it out!

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
>  drivers/soc/qcom/rpmhpd.c | 255 +++++++++++++++++++++++-----------------------
>  1 file changed, 128 insertions(+), 127 deletions(-)
> 
> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
> index 1118345..c71481d 100644
> --- a/drivers/soc/qcom/rpmhpd.c
> +++ b/drivers/soc/qcom/rpmhpd.c
> @@ -63,73 +63,102 @@ struct rpmhpd_desc {
>  
>  static DEFINE_MUTEX(rpmhpd_lock);
>  
> -/* SDM845 RPMH powerdomains */
> +/* RPMH powerdomains */
>  
> -static struct rpmhpd sdm845_ebi = {
> +static struct rpmhpd ebi = {
>  	.pd = { .name = "ebi", },
>  	.res_name = "ebi.lvl",
>  };
>  
> -static struct rpmhpd sdm845_lmx = {
> +static struct rpmhpd lmx = {
>  	.pd = { .name = "lmx", },
>  	.res_name = "lmx.lvl",
>  };
>  
> -static struct rpmhpd sdm845_lcx = {
> +static struct rpmhpd lcx = {
>  	.pd = { .name = "lcx", },
>  	.res_name = "lcx.lvl",
>  };
>  
> -static struct rpmhpd sdm845_gfx = {
> +static struct rpmhpd gfx = {
>  	.pd = { .name = "gfx", },
>  	.res_name = "gfx.lvl",
>  };
>  
> -static struct rpmhpd sdm845_mss = {
> +static struct rpmhpd mss = {
>  	.pd = { .name = "mss", },
>  	.res_name = "mss.lvl",
>  };
>  
> -static struct rpmhpd sdm845_mx_ao;
> -static struct rpmhpd sdm845_mx = {
> +static struct rpmhpd mx_ao;
> +static struct rpmhpd mx = {
>  	.pd = { .name = "mx", },
> -	.peer = &sdm845_mx_ao,
> +	.peer = &mx_ao,
>  	.res_name = "mx.lvl",
>  };
>  
> -static struct rpmhpd sdm845_mx_ao = {
> +static struct rpmhpd mx_ao = {
>  	.pd = { .name = "mx_ao", },
>  	.active_only = true,
> -	.peer = &sdm845_mx,
> +	.peer = &mx,
>  	.res_name = "mx.lvl",
>  };
>  
> -static struct rpmhpd sdm845_cx_ao;
> -static struct rpmhpd sdm845_cx = {
> +static struct rpmhpd cx_ao;
> +static struct rpmhpd cx = {
>  	.pd = { .name = "cx", },
> -	.peer = &sdm845_cx_ao,
> -	.parent = &sdm845_mx.pd,
> +	.peer = &cx_ao,
> +	.parent = &mx.pd,
>  	.res_name = "cx.lvl",
>  };
>  
> -static struct rpmhpd sdm845_cx_ao = {
> +static struct rpmhpd cx_ao = {
>  	.pd = { .name = "cx_ao", },
>  	.active_only = true,
> -	.peer = &sdm845_cx,
> -	.parent = &sdm845_mx_ao.pd,
> +	.peer = &cx,
> +	.parent = &mx_ao.pd,
>  	.res_name = "cx.lvl",
>  };
>  
> +static struct rpmhpd mmcx_ao;
> +static struct rpmhpd mmcx = {
> +	.pd = { .name = "mmcx", },
> +	.peer = &mmcx_ao,
> +	.res_name = "mmcx.lvl",
> +};
> +
> +static struct rpmhpd mmcx_ao = {
> +	.pd = { .name = "mmcx_ao", },
> +	.active_only = true,
> +	.peer = &mmcx,
> +	.res_name = "mmcx.lvl",
> +};
> +
> +static struct rpmhpd mxc_ao;
> +static struct rpmhpd mxc = {
> +	.pd = { .name = "mxc", },
> +	.peer = &mxc_ao,
> +	.res_name = "mxc.lvl",
> +};
> +
> +static struct rpmhpd mxc_ao = {
> +	.pd = { .name = "mxc_ao", },
> +	.active_only = true,
> +	.peer = &mxc,
> +	.res_name = "mxc.lvl",
> +};
> +
> +/* SDM845 RPMH powerdomains */
>  static struct rpmhpd *sdm845_rpmhpds[] = {
> -	[SDM845_EBI] = &sdm845_ebi,
> -	[SDM845_MX] = &sdm845_mx,
> -	[SDM845_MX_AO] = &sdm845_mx_ao,
> -	[SDM845_CX] = &sdm845_cx,
> -	[SDM845_CX_AO] = &sdm845_cx_ao,
> -	[SDM845_LMX] = &sdm845_lmx,
> -	[SDM845_LCX] = &sdm845_lcx,
> -	[SDM845_GFX] = &sdm845_gfx,
> -	[SDM845_MSS] = &sdm845_mss,
> +	[SDM845_EBI] = &ebi,
> +	[SDM845_MX] = &mx,
> +	[SDM845_MX_AO] = &mx_ao,
> +	[SDM845_CX] = &cx,
> +	[SDM845_CX_AO] = &cx_ao,
> +	[SDM845_LMX] = &lmx,
> +	[SDM845_LCX] = &lcx,
> +	[SDM845_GFX] = &gfx,
> +	[SDM845_MSS] = &mss,
>  };
>  
>  static const struct rpmhpd_desc sdm845_desc = {
> @@ -139,9 +168,9 @@ static const struct rpmhpd_desc sdm845_desc = {
>  
>  /* SDX55 RPMH powerdomains */
>  static struct rpmhpd *sdx55_rpmhpds[] = {
> -	[SDX55_MSS] = &sdm845_mss,
> -	[SDX55_MX] = &sdm845_mx,
> -	[SDX55_CX] = &sdm845_cx,
> +	[SDX55_MSS] = &mss,
> +	[SDX55_MX] = &mx,
> +	[SDX55_CX] = &cx,
>  };
>  
>  static const struct rpmhpd_desc sdx55_desc = {
> @@ -151,12 +180,12 @@ static const struct rpmhpd_desc sdx55_desc = {
>  
>  /* SM6350 RPMH powerdomains */
>  static struct rpmhpd *sm6350_rpmhpds[] = {
> -	[SM6350_CX] = &sdm845_cx,
> -	[SM6350_GFX] = &sdm845_gfx,
> -	[SM6350_LCX] = &sdm845_lcx,
> -	[SM6350_LMX] = &sdm845_lmx,
> -	[SM6350_MSS] = &sdm845_mss,
> -	[SM6350_MX] = &sdm845_mx,
> +	[SM6350_CX] = &cx,
> +	[SM6350_GFX] = &gfx,
> +	[SM6350_LCX] = &lcx,
> +	[SM6350_LMX] = &lmx,
> +	[SM6350_MSS] = &mss,
> +	[SM6350_MX] = &mx,
>  };
>  
>  static const struct rpmhpd_desc sm6350_desc = {
> @@ -165,33 +194,18 @@ static const struct rpmhpd_desc sm6350_desc = {
>  };
>  
>  /* SM8150 RPMH powerdomains */
> -
> -static struct rpmhpd sm8150_mmcx_ao;
> -static struct rpmhpd sm8150_mmcx = {
> -	.pd = { .name = "mmcx", },
> -	.peer = &sm8150_mmcx_ao,
> -	.res_name = "mmcx.lvl",
> -};
> -
> -static struct rpmhpd sm8150_mmcx_ao = {
> -	.pd = { .name = "mmcx_ao", },
> -	.active_only = true,
> -	.peer = &sm8150_mmcx,
> -	.res_name = "mmcx.lvl",
> -};
> -
>  static struct rpmhpd *sm8150_rpmhpds[] = {
> -	[SM8150_MSS] = &sdm845_mss,
> -	[SM8150_EBI] = &sdm845_ebi,
> -	[SM8150_LMX] = &sdm845_lmx,
> -	[SM8150_LCX] = &sdm845_lcx,
> -	[SM8150_GFX] = &sdm845_gfx,
> -	[SM8150_MX] = &sdm845_mx,
> -	[SM8150_MX_AO] = &sdm845_mx_ao,
> -	[SM8150_CX] = &sdm845_cx,
> -	[SM8150_CX_AO] = &sdm845_cx_ao,
> -	[SM8150_MMCX] = &sm8150_mmcx,
> -	[SM8150_MMCX_AO] = &sm8150_mmcx_ao,
> +	[SM8150_MSS] = &mss,
> +	[SM8150_EBI] = &ebi,
> +	[SM8150_LMX] = &lmx,
> +	[SM8150_LCX] = &lcx,
> +	[SM8150_GFX] = &gfx,
> +	[SM8150_MX] = &mx,
> +	[SM8150_MX_AO] = &mx_ao,
> +	[SM8150_CX] = &cx,
> +	[SM8150_CX_AO] = &cx_ao,
> +	[SM8150_MMCX] = &mmcx,
> +	[SM8150_MMCX_AO] = &mmcx_ao,
>  };
>  
>  static const struct rpmhpd_desc sm8150_desc = {
> @@ -199,17 +213,18 @@ static const struct rpmhpd_desc sm8150_desc = {
>  	.num_pds = ARRAY_SIZE(sm8150_rpmhpds),
>  };
>  
> +/* SM8250 RPMH powerdomains */
>  static struct rpmhpd *sm8250_rpmhpds[] = {
> -	[SM8250_CX] = &sdm845_cx,
> -	[SM8250_CX_AO] = &sdm845_cx_ao,
> -	[SM8250_EBI] = &sdm845_ebi,
> -	[SM8250_GFX] = &sdm845_gfx,
> -	[SM8250_LCX] = &sdm845_lcx,
> -	[SM8250_LMX] = &sdm845_lmx,
> -	[SM8250_MMCX] = &sm8150_mmcx,
> -	[SM8250_MMCX_AO] = &sm8150_mmcx_ao,
> -	[SM8250_MX] = &sdm845_mx,
> -	[SM8250_MX_AO] = &sdm845_mx_ao,
> +	[SM8250_CX] = &cx,
> +	[SM8250_CX_AO] = &cx_ao,
> +	[SM8250_EBI] = &ebi,
> +	[SM8250_GFX] = &gfx,
> +	[SM8250_LCX] = &lcx,
> +	[SM8250_LMX] = &lmx,
> +	[SM8250_MMCX] = &mmcx,
> +	[SM8250_MMCX_AO] = &mmcx_ao,
> +	[SM8250_MX] = &mx,
> +	[SM8250_MX_AO] = &mx_ao,
>  };
>  
>  static const struct rpmhpd_desc sm8250_desc = {
> @@ -218,34 +233,20 @@ static const struct rpmhpd_desc sm8250_desc = {
>  };
>  
>  /* SM8350 Power domains */
> -static struct rpmhpd sm8350_mxc_ao;
> -static struct rpmhpd sm8350_mxc = {
> -	.pd = { .name = "mxc", },
> -	.peer = &sm8350_mxc_ao,
> -	.res_name = "mxc.lvl",
> -};
> -
> -static struct rpmhpd sm8350_mxc_ao = {
> -	.pd = { .name = "mxc_ao", },
> -	.active_only = true,
> -	.peer = &sm8350_mxc,
> -	.res_name = "mxc.lvl",
> -};
> -
>  static struct rpmhpd *sm8350_rpmhpds[] = {
> -	[SM8350_CX] = &sdm845_cx,
> -	[SM8350_CX_AO] = &sdm845_cx_ao,
> -	[SM8350_EBI] = &sdm845_ebi,
> -	[SM8350_GFX] = &sdm845_gfx,
> -	[SM8350_LCX] = &sdm845_lcx,
> -	[SM8350_LMX] = &sdm845_lmx,
> -	[SM8350_MMCX] = &sm8150_mmcx,
> -	[SM8350_MMCX_AO] = &sm8150_mmcx_ao,
> -	[SM8350_MX] = &sdm845_mx,
> -	[SM8350_MX_AO] = &sdm845_mx_ao,
> -	[SM8350_MXC] = &sm8350_mxc,
> -	[SM8350_MXC_AO] = &sm8350_mxc_ao,
> -	[SM8350_MSS] = &sdm845_mss,
> +	[SM8350_CX] = &cx,
> +	[SM8350_CX_AO] = &cx_ao,
> +	[SM8350_EBI] = &ebi,
> +	[SM8350_GFX] = &gfx,
> +	[SM8350_LCX] = &lcx,
> +	[SM8350_LMX] = &lmx,
> +	[SM8350_MMCX] = &mmcx,
> +	[SM8350_MMCX_AO] = &mmcx_ao,
> +	[SM8350_MX] = &mx,
> +	[SM8350_MX_AO] = &mx_ao,
> +	[SM8350_MXC] = &mxc,
> +	[SM8350_MXC_AO] = &mxc_ao,
> +	[SM8350_MSS] = &mss,
>  };
>  
>  static const struct rpmhpd_desc sm8350_desc = {
> @@ -255,14 +256,14 @@ static const struct rpmhpd_desc sm8350_desc = {
>  
>  /* SC7180 RPMH powerdomains */
>  static struct rpmhpd *sc7180_rpmhpds[] = {
> -	[SC7180_CX] = &sdm845_cx,
> -	[SC7180_CX_AO] = &sdm845_cx_ao,
> -	[SC7180_GFX] = &sdm845_gfx,
> -	[SC7180_MX] = &sdm845_mx,
> -	[SC7180_MX_AO] = &sdm845_mx_ao,
> -	[SC7180_LMX] = &sdm845_lmx,
> -	[SC7180_LCX] = &sdm845_lcx,
> -	[SC7180_MSS] = &sdm845_mss,
> +	[SC7180_CX] = &cx,
> +	[SC7180_CX_AO] = &cx_ao,
> +	[SC7180_GFX] = &gfx,
> +	[SC7180_MX] = &mx,
> +	[SC7180_MX_AO] = &mx_ao,
> +	[SC7180_LMX] = &lmx,
> +	[SC7180_LCX] = &lcx,
> +	[SC7180_MSS] = &mss,
>  };
>  
>  static const struct rpmhpd_desc sc7180_desc = {
> @@ -272,15 +273,15 @@ static const struct rpmhpd_desc sc7180_desc = {
>  
>  /* SC7280 RPMH powerdomains */
>  static struct rpmhpd *sc7280_rpmhpds[] = {
> -	[SC7280_CX] = &sdm845_cx,
> -	[SC7280_CX_AO] = &sdm845_cx_ao,
> -	[SC7280_EBI] = &sdm845_ebi,
> -	[SC7280_GFX] = &sdm845_gfx,
> -	[SC7280_MX] = &sdm845_mx,
> -	[SC7280_MX_AO] = &sdm845_mx_ao,
> -	[SC7280_LMX] = &sdm845_lmx,
> -	[SC7280_LCX] = &sdm845_lcx,
> -	[SC7280_MSS] = &sdm845_mss,
> +	[SC7280_CX] = &cx,
> +	[SC7280_CX_AO] = &cx_ao,
> +	[SC7280_EBI] = &ebi,
> +	[SC7280_GFX] = &gfx,
> +	[SC7280_MX] = &mx,
> +	[SC7280_MX_AO] = &mx_ao,
> +	[SC7280_LMX] = &lmx,
> +	[SC7280_LCX] = &lcx,
> +	[SC7280_MSS] = &mss,
>  };
>  
>  static const struct rpmhpd_desc sc7280_desc = {
> @@ -290,17 +291,17 @@ static const struct rpmhpd_desc sc7280_desc = {
>  
>  /* SC8180x RPMH powerdomains */
>  static struct rpmhpd *sc8180x_rpmhpds[] = {
> -	[SC8180X_CX] = &sdm845_cx,
> -	[SC8180X_CX_AO] = &sdm845_cx_ao,
> -	[SC8180X_EBI] = &sdm845_ebi,
> -	[SC8180X_GFX] = &sdm845_gfx,
> -	[SC8180X_LCX] = &sdm845_lcx,
> -	[SC8180X_LMX] = &sdm845_lmx,
> -	[SC8180X_MMCX] = &sm8150_mmcx,
> -	[SC8180X_MMCX_AO] = &sm8150_mmcx_ao,
> -	[SC8180X_MSS] = &sdm845_mss,
> -	[SC8180X_MX] = &sdm845_mx,
> -	[SC8180X_MX_AO] = &sdm845_mx_ao,
> +	[SC8180X_CX] = &cx,
> +	[SC8180X_CX_AO] = &cx_ao,
> +	[SC8180X_EBI] = &ebi,
> +	[SC8180X_GFX] = &gfx,
> +	[SC8180X_LCX] = &lcx,
> +	[SC8180X_LMX] = &lmx,
> +	[SC8180X_MMCX] = &mmcx,
> +	[SC8180X_MMCX_AO] = &mmcx_ao,
> +	[SC8180X_MSS] = &mss,
> +	[SC8180X_MX] = &mx,
> +	[SC8180X_MX_AO] = &mx_ao,
>  };
>  
>  static const struct rpmhpd_desc sc8180x_desc = {
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] soc: qcom: rpmhpd: Make mx as a parent of cx only for sdm845
  2021-11-16  5:26 ` [PATCH 2/2] soc: qcom: rpmhpd: Make mx as a parent of cx only for sdm845 Rajendra Nayak
@ 2021-11-19  3:19   ` Bjorn Andersson
  2021-11-23  7:15     ` Rajendra Nayak
  0 siblings, 1 reply; 9+ messages in thread
From: Bjorn Andersson @ 2021-11-19  3:19 UTC (permalink / raw)
  To: Rajendra Nayak; +Cc: agross, linux-arm-msm, linux-kernel, swboyd

On Mon 15 Nov 23:26 CST 2021, Rajendra Nayak wrote:

> The requirement to specify the active + sleep and active-only MX power
> domains as the parents of the corresponding CX power domains is applicable
> only on the sdm845 SoC. With the same struct definition reused for all the
> SoCs this condition was wrongly applied to all those SoCs as well, which
> isn't needed. Define new sdm845 specific structures to manage this
> dependency and remove the parent assignements from the common structure.
> 

Looking at the downstream sm8150 dts I see that both cx and mmcx
specifies mx as parent "supply".

Is this not needed or should we instead name these resources
"cx_with_mx_parent" and have sm8150 opt in as well?

Regards,
Bjorn

> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  drivers/soc/qcom/rpmhpd.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
> index c71481d..12d8ce9 100644
> --- a/drivers/soc/qcom/rpmhpd.c
> +++ b/drivers/soc/qcom/rpmhpd.c
> @@ -108,7 +108,6 @@ static struct rpmhpd cx_ao;
>  static struct rpmhpd cx = {
>  	.pd = { .name = "cx", },
>  	.peer = &cx_ao,
> -	.parent = &mx.pd,
>  	.res_name = "cx.lvl",
>  };
>  
> @@ -116,7 +115,6 @@ static struct rpmhpd cx_ao = {
>  	.pd = { .name = "cx_ao", },
>  	.active_only = true,
>  	.peer = &cx,
> -	.parent = &mx_ao.pd,
>  	.res_name = "cx.lvl",
>  };
>  
> @@ -149,12 +147,28 @@ static struct rpmhpd mxc_ao = {
>  };
>  
>  /* SDM845 RPMH powerdomains */
> +static struct rpmhpd sdm845_cx_ao;
> +static struct rpmhpd sdm845_cx = {
> +	.pd = { .name = "cx", },
> +	.peer = &sdm845_cx_ao,
> +	.parent = &mx.pd,
> +	.res_name = "cx.lvl",
> +};
> +
> +static struct rpmhpd sdm845_cx_ao = {
> +	.pd = { .name = "cx_ao", },
> +	.active_only = true,
> +	.peer = &sdm845_cx,
> +	.parent = &mx_ao.pd,
> +	.res_name = "cx.lvl",
> +};
> +
>  static struct rpmhpd *sdm845_rpmhpds[] = {
>  	[SDM845_EBI] = &ebi,
>  	[SDM845_MX] = &mx,
>  	[SDM845_MX_AO] = &mx_ao,
> -	[SDM845_CX] = &cx,
> -	[SDM845_CX_AO] = &cx_ao,
> +	[SDM845_CX] = &sdm845_cx,
> +	[SDM845_CX_AO] = &sdm845_cx_ao,
>  	[SDM845_LMX] = &lmx,
>  	[SDM845_LCX] = &lcx,
>  	[SDM845_GFX] = &gfx,
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] soc: qcom: rpmhpd: Make mx as a parent of cx only for sdm845
  2021-11-19  3:19   ` Bjorn Andersson
@ 2021-11-23  7:15     ` Rajendra Nayak
  2021-12-02  7:48       ` Rajendra Nayak
  0 siblings, 1 reply; 9+ messages in thread
From: Rajendra Nayak @ 2021-11-23  7:15 UTC (permalink / raw)
  To: Bjorn Andersson; +Cc: agross, linux-arm-msm, linux-kernel, swboyd


On 11/19/2021 8:49 AM, Bjorn Andersson wrote:
> On Mon 15 Nov 23:26 CST 2021, Rajendra Nayak wrote:
> 
>> The requirement to specify the active + sleep and active-only MX power
>> domains as the parents of the corresponding CX power domains is applicable
>> only on the sdm845 SoC. With the same struct definition reused for all the
>> SoCs this condition was wrongly applied to all those SoCs as well, which
>> isn't needed. Define new sdm845 specific structures to manage this
>> dependency and remove the parent assignements from the common structure.
>>
> 
> Looking at the downstream sm8150 dts I see that both cx and mmcx
> specifies mx as parent "supply".
> 
> Is this not needed or should we instead name these resources
> "cx_with_mx_parent" and have sm8150 opt in as well?

Right, looks like these are needed, after talking to some more folks
I was told RPMh does not really enforce any dependencies on any of the
SoCs, so my earlier statement was wrong that this was managed by RPMh.
Some SoCs just have some digital domain requirements which need these
dependencies to be managed (not all SoCs) and when we end up with such
a situation its almost always expected to be managed by the RPMh masters
(APPS running hlos in this case)
This is not just across cx/mx but others as well like mmcx/mxc/gfx etc.

Unfortunately I could not find this very well documented at an SoC level,
so perhaps the best way to go about is to look at downstream dependencies
and try to match them upstream :/
I will respin this to add the 8150 dependencies back (and if I see any more
for the others)
  
> 
> Regards,
> Bjorn
> 
>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>> ---
>>   drivers/soc/qcom/rpmhpd.c | 22 ++++++++++++++++++----
>>   1 file changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
>> index c71481d..12d8ce9 100644
>> --- a/drivers/soc/qcom/rpmhpd.c
>> +++ b/drivers/soc/qcom/rpmhpd.c
>> @@ -108,7 +108,6 @@ static struct rpmhpd cx_ao;
>>   static struct rpmhpd cx = {
>>   	.pd = { .name = "cx", },
>>   	.peer = &cx_ao,
>> -	.parent = &mx.pd,
>>   	.res_name = "cx.lvl",
>>   };
>>   
>> @@ -116,7 +115,6 @@ static struct rpmhpd cx_ao = {
>>   	.pd = { .name = "cx_ao", },
>>   	.active_only = true,
>>   	.peer = &cx,
>> -	.parent = &mx_ao.pd,
>>   	.res_name = "cx.lvl",
>>   };
>>   
>> @@ -149,12 +147,28 @@ static struct rpmhpd mxc_ao = {
>>   };
>>   
>>   /* SDM845 RPMH powerdomains */
>> +static struct rpmhpd sdm845_cx_ao;
>> +static struct rpmhpd sdm845_cx = {
>> +	.pd = { .name = "cx", },
>> +	.peer = &sdm845_cx_ao,
>> +	.parent = &mx.pd,
>> +	.res_name = "cx.lvl",
>> +};
>> +
>> +static struct rpmhpd sdm845_cx_ao = {
>> +	.pd = { .name = "cx_ao", },
>> +	.active_only = true,
>> +	.peer = &sdm845_cx,
>> +	.parent = &mx_ao.pd,
>> +	.res_name = "cx.lvl",
>> +};
>> +
>>   static struct rpmhpd *sdm845_rpmhpds[] = {
>>   	[SDM845_EBI] = &ebi,
>>   	[SDM845_MX] = &mx,
>>   	[SDM845_MX_AO] = &mx_ao,
>> -	[SDM845_CX] = &cx,
>> -	[SDM845_CX_AO] = &cx_ao,
>> +	[SDM845_CX] = &sdm845_cx,
>> +	[SDM845_CX_AO] = &sdm845_cx_ao,
>>   	[SDM845_LMX] = &lmx,
>>   	[SDM845_LCX] = &lcx,
>>   	[SDM845_GFX] = &gfx,
>> -- 
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
>> of Code Aurora Forum, hosted by The Linux Foundation
>>

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] soc: qcom: rpmhpd: Rename rpmhpd struct names
  2021-11-16  5:26 [PATCH 1/2] soc: qcom: rpmhpd: Rename rpmhpd struct names Rajendra Nayak
                   ` (2 preceding siblings ...)
  2021-11-19  3:11 ` Bjorn Andersson
@ 2021-12-01 20:44 ` Stephen Boyd
  2021-12-15 22:27 ` Bjorn Andersson
  4 siblings, 0 replies; 9+ messages in thread
From: Stephen Boyd @ 2021-12-01 20:44 UTC (permalink / raw)
  To: Rajendra Nayak, agross, bjorn.andersson; +Cc: linux-arm-msm, linux-kernel

Quoting Rajendra Nayak (2021-11-15 21:26:21)
> The rpmhpd structs were named with a SoC-name prefix, but then
> they got reused across multiple SoC families making things confusing.
> Rename all the struct names to remove SoC-name prefixes.
> No other functional change as part of this patch.
>
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] soc: qcom: rpmhpd: Make mx as a parent of cx only for sdm845
  2021-11-23  7:15     ` Rajendra Nayak
@ 2021-12-02  7:48       ` Rajendra Nayak
  0 siblings, 0 replies; 9+ messages in thread
From: Rajendra Nayak @ 2021-12-02  7:48 UTC (permalink / raw)
  To: Bjorn Andersson; +Cc: agross, linux-arm-msm, linux-kernel, swboyd


On 11/23/2021 12:45 PM, Rajendra Nayak wrote:
> 
> On 11/19/2021 8:49 AM, Bjorn Andersson wrote:
>> On Mon 15 Nov 23:26 CST 2021, Rajendra Nayak wrote:
>>
>>> The requirement to specify the active + sleep and active-only MX power
>>> domains as the parents of the corresponding CX power domains is applicable
>>> only on the sdm845 SoC. With the same struct definition reused for all the
>>> SoCs this condition was wrongly applied to all those SoCs as well, which
>>> isn't needed. Define new sdm845 specific structures to manage this
>>> dependency and remove the parent assignements from the common structure.
>>>
>>
>> Looking at the downstream sm8150 dts I see that both cx and mmcx
>> specifies mx as parent "supply".
>>
>> Is this not needed or should we instead name these resources
>> "cx_with_mx_parent" and have sm8150 opt in as well?
> 
> Right, looks like these are needed, after talking to some more folks
> I was told RPMh does not really enforce any dependencies on any of the
> SoCs, so my earlier statement was wrong that this was managed by RPMh.
> Some SoCs just have some digital domain requirements which need these
> dependencies to be managed (not all SoCs) and when we end up with such
> a situation its almost always expected to be managed by the RPMh masters
> (APPS running hlos in this case)
> This is not just across cx/mx but others as well like mmcx/mxc/gfx etc.
> 
> Unfortunately I could not find this very well documented at an SoC level,
> so perhaps the best way to go about is to look at downstream dependencies
> and try to match them upstream :/
> I will respin this to add the 8150 dependencies back (and if I see any more
> for the others)

Looking through this more in downstream files, I see atleast the mx being a
parent of cx is needed on pretty much all upstream supported SoCs, except the
sc7280. There seem to be more complex dependencies that downstream models
across other rails, mainly for 8150/8250/8350 and the most recent 8450
but looks like we have been able to live without those upstream so I plan
to leave them for now and just re-post this with an additional cx_with_mx_parent
and a cx to distinguish between these 2 cases.

> 
>>
>> Regards,
>> Bjorn
>>
>>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>>> ---
>>>   drivers/soc/qcom/rpmhpd.c | 22 ++++++++++++++++++----
>>>   1 file changed, 18 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
>>> index c71481d..12d8ce9 100644
>>> --- a/drivers/soc/qcom/rpmhpd.c
>>> +++ b/drivers/soc/qcom/rpmhpd.c
>>> @@ -108,7 +108,6 @@ static struct rpmhpd cx_ao;
>>>   static struct rpmhpd cx = {
>>>       .pd = { .name = "cx", },
>>>       .peer = &cx_ao,
>>> -    .parent = &mx.pd,
>>>       .res_name = "cx.lvl",
>>>   };
>>> @@ -116,7 +115,6 @@ static struct rpmhpd cx_ao = {
>>>       .pd = { .name = "cx_ao", },
>>>       .active_only = true,
>>>       .peer = &cx,
>>> -    .parent = &mx_ao.pd,
>>>       .res_name = "cx.lvl",
>>>   };
>>> @@ -149,12 +147,28 @@ static struct rpmhpd mxc_ao = {
>>>   };
>>>   /* SDM845 RPMH powerdomains */
>>> +static struct rpmhpd sdm845_cx_ao;
>>> +static struct rpmhpd sdm845_cx = {
>>> +    .pd = { .name = "cx", },
>>> +    .peer = &sdm845_cx_ao,
>>> +    .parent = &mx.pd,
>>> +    .res_name = "cx.lvl",
>>> +};
>>> +
>>> +static struct rpmhpd sdm845_cx_ao = {
>>> +    .pd = { .name = "cx_ao", },
>>> +    .active_only = true,
>>> +    .peer = &sdm845_cx,
>>> +    .parent = &mx_ao.pd,
>>> +    .res_name = "cx.lvl",
>>> +};
>>> +
>>>   static struct rpmhpd *sdm845_rpmhpds[] = {
>>>       [SDM845_EBI] = &ebi,
>>>       [SDM845_MX] = &mx,
>>>       [SDM845_MX_AO] = &mx_ao,
>>> -    [SDM845_CX] = &cx,
>>> -    [SDM845_CX_AO] = &cx_ao,
>>> +    [SDM845_CX] = &sdm845_cx,
>>> +    [SDM845_CX_AO] = &sdm845_cx_ao,
>>>       [SDM845_LMX] = &lmx,
>>>       [SDM845_LCX] = &lcx,
>>>       [SDM845_GFX] = &gfx,
>>> -- 
>>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
>>> of Code Aurora Forum, hosted by The Linux Foundation
>>>
> 

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] soc: qcom: rpmhpd: Rename rpmhpd struct names
  2021-11-16  5:26 [PATCH 1/2] soc: qcom: rpmhpd: Rename rpmhpd struct names Rajendra Nayak
                   ` (3 preceding siblings ...)
  2021-12-01 20:44 ` Stephen Boyd
@ 2021-12-15 22:27 ` Bjorn Andersson
  4 siblings, 0 replies; 9+ messages in thread
From: Bjorn Andersson @ 2021-12-15 22:27 UTC (permalink / raw)
  To: agross, Rajendra Nayak; +Cc: linux-arm-msm, linux-kernel, swboyd

On Tue, 16 Nov 2021 10:56:21 +0530, Rajendra Nayak wrote:
> The rpmhpd structs were named with a SoC-name prefix, but then
> they got reused across multiple SoC families making things confusing.
> Rename all the struct names to remove SoC-name prefixes.
> No other functional change as part of this patch.
> 
> 

Applied, thanks!

[1/2] soc: qcom: rpmhpd: Rename rpmhpd struct names
      commit: 8f3d4dd65abd03a5edcf7b5d5a7a3e2a4866db15
[2/2] soc: qcom: rpmhpd: Make mx as a parent of cx only for sdm845
      (no commit info)

Best regards,
-- 
Bjorn Andersson <bjorn.andersson@linaro.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-12-15 22:28 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-16  5:26 [PATCH 1/2] soc: qcom: rpmhpd: Rename rpmhpd struct names Rajendra Nayak
2021-11-16  5:26 ` [PATCH 2/2] soc: qcom: rpmhpd: Make mx as a parent of cx only for sdm845 Rajendra Nayak
2021-11-19  3:19   ` Bjorn Andersson
2021-11-23  7:15     ` Rajendra Nayak
2021-12-02  7:48       ` Rajendra Nayak
2021-11-18 18:26 ` [PATCH 1/2] soc: qcom: rpmhpd: Rename rpmhpd struct names Matthias Kaehlcke
2021-11-19  3:11 ` Bjorn Andersson
2021-12-01 20:44 ` Stephen Boyd
2021-12-15 22:27 ` Bjorn Andersson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.