[PATCH v2 0/3] soc: qcom: rpmhpd: Cleanups and mx/cx fixup for sc7280

[PATCH v2 0/3] soc: qcom: rpmhpd: Cleanups and mx/cx fixup for sc7280

[Rajendra Nayak]

v2:
* Fixed the wrong assumption in v1 that only sdm845 needed mx to be
parent of cx, turned out all existing upstream SoCs need it except sc7280
* Added another cleanup patch to sort power-domain defines and lists in
alphabetical order as suggested by Matthias

Mostly cleanups, with a fixup to remove the parent/child relationship
across mx/cx for sc7280 SoC.

Rajendra Nayak (3):
  soc: qcom: rpmhpd: Rename rpmhpd struct names
  soc: qcom: rpmhpd: Remove mx/cx relationship on sc7280
  soc: qcom: rpmhpd: Sort power-domain definitions and lists

 drivers/soc/qcom/rpmhpd.c | 287 ++++++++++++++++++++++++----------------------
 1 file changed, 152 insertions(+), 135 deletions(-)

-- 
2.7.4

[PATCH v2 1/3] soc: qcom: rpmhpd: Rename rpmhpd struct names

[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 <quic_rjendra@quicinc.com>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.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 = {
-- 
2.7.4

[PATCH v2 2/3] soc: qcom: rpmhpd: Remove mx/cx relationship on sc7280

[Rajendra Nayak]

While the requirement to specify the active + sleep and active-only MX
power-domains as the parents of the corresponding CX power domains is
applicable for most SoCs, we have some like the sc7280 where this 
dependency is not applicable.
Define new rpmhpd structs for cx and cx_ao without the mx as
parent and use them for sc7280.

Signed-off-by: Rajendra Nayak <quic_rjendra@quicinc.com>
---
 drivers/soc/qcom/rpmhpd.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
index c71481d..4599efe 100644
--- a/drivers/soc/qcom/rpmhpd.c
+++ b/drivers/soc/qcom/rpmhpd.c
@@ -120,6 +120,20 @@ static struct rpmhpd cx_ao = {
 	.res_name = "cx.lvl",
 };
 
+static struct rpmhpd cx_ao_no_parent;
+static struct rpmhpd cx_no_parent = {
+	.pd = { .name = "cx", },
+	.peer = &cx_ao_no_parent,
+	.res_name = "cx.lvl",
+};
+
+static struct rpmhpd cx_ao_no_parent = {
+	.pd = { .name = "cx_ao", },
+	.active_only = true,
+	.peer = &cx_no_parent,
+	.res_name = "cx.lvl",
+};
+
 static struct rpmhpd mmcx_ao;
 static struct rpmhpd mmcx = {
 	.pd = { .name = "mmcx", },
@@ -273,8 +287,8 @@ static const struct rpmhpd_desc sc7180_desc = {
 
 /* SC7280 RPMH powerdomains */
 static struct rpmhpd *sc7280_rpmhpds[] = {
-	[SC7280_CX] = &cx,
-	[SC7280_CX_AO] = &cx_ao,
+	[SC7280_CX] = &cx_no_parent,
+	[SC7280_CX_AO] = &cx_ao_no_parent,
 	[SC7280_EBI] = &ebi,
 	[SC7280_GFX] = &gfx,
 	[SC7280_MX] = &mx,
-- 
2.7.4

Re: [PATCH v2 2/3] soc: qcom: rpmhpd: Remove mx/cx relationship on sc7280

[Bjorn Andersson]

On Tue 07 Dec 04:08 CST 2021, Rajendra Nayak wrote:

> While the requirement to specify the active + sleep and active-only MX
> power-domains as the parents of the corresponding CX power domains is
> applicable for most SoCs, we have some like the sc7280 where this 
> dependency is not applicable.
> Define new rpmhpd structs for cx and cx_ao without the mx as
> parent and use them for sc7280.
> 
> Signed-off-by: Rajendra Nayak <quic_rjendra@quicinc.com>
> ---
>  drivers/soc/qcom/rpmhpd.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
> index c71481d..4599efe 100644
> --- a/drivers/soc/qcom/rpmhpd.c
> +++ b/drivers/soc/qcom/rpmhpd.c
> @@ -120,6 +120,20 @@ static struct rpmhpd cx_ao = {
>  	.res_name = "cx.lvl",
>  };
>  
> +static struct rpmhpd cx_ao_no_parent;
> +static struct rpmhpd cx_no_parent = {

There are multiple variations of how each of these can be parented, but
only one way they can be without a parent. So how about we turn this the
other way around?

I.e. let's name this one "cx" and the existing one "cx_w_mx_parent".


This will be particularly useful when you look at mmcx, which on
8150/8180 has mx as parent and on 8450 has cx as parent.


PS. Unfortunately I had merged 8450 since you wrote this series, I tried
to just fix it up as I applied your patch, but noticed 8450_cx and
8450_mmcx and wanted to get your opinion on this first.

Regards,
Bjorn

> +	.pd = { .name = "cx", },
> +	.peer = &cx_ao_no_parent,
> +	.res_name = "cx.lvl",
> +};
> +
> +static struct rpmhpd cx_ao_no_parent = {
> +	.pd = { .name = "cx_ao", },
> +	.active_only = true,
> +	.peer = &cx_no_parent,
> +	.res_name = "cx.lvl",
> +};
> +
>  static struct rpmhpd mmcx_ao;
>  static struct rpmhpd mmcx = {
>  	.pd = { .name = "mmcx", },
> @@ -273,8 +287,8 @@ static const struct rpmhpd_desc sc7180_desc = {
>  
>  /* SC7280 RPMH powerdomains */
>  static struct rpmhpd *sc7280_rpmhpds[] = {
> -	[SC7280_CX] = &cx,
> -	[SC7280_CX_AO] = &cx_ao,
> +	[SC7280_CX] = &cx_no_parent,
> +	[SC7280_CX_AO] = &cx_ao_no_parent,
>  	[SC7280_EBI] = &ebi,
>  	[SC7280_GFX] = &gfx,
>  	[SC7280_MX] = &mx,
> -- 
> 2.7.4
> 

Re: [PATCH v2 2/3] soc: qcom: rpmhpd: Remove mx/cx relationship on sc7280

[Rajendra Nayak]

On 12/9/2021 2:12 AM, Bjorn Andersson wrote:
> On Tue 07 Dec 04:08 CST 2021, Rajendra Nayak wrote:
> 
>> While the requirement to specify the active + sleep and active-only MX
>> power-domains as the parents of the corresponding CX power domains is
>> applicable for most SoCs, we have some like the sc7280 where this
>> dependency is not applicable.
>> Define new rpmhpd structs for cx and cx_ao without the mx as
>> parent and use them for sc7280.
>>
>> Signed-off-by: Rajendra Nayak <quic_rjendra@quicinc.com>
>> ---
>>   drivers/soc/qcom/rpmhpd.c | 18 ++++++++++++++++--
>>   1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
>> index c71481d..4599efe 100644
>> --- a/drivers/soc/qcom/rpmhpd.c
>> +++ b/drivers/soc/qcom/rpmhpd.c
>> @@ -120,6 +120,20 @@ static struct rpmhpd cx_ao = {
>>   	.res_name = "cx.lvl",
>>   };
>>   
>> +static struct rpmhpd cx_ao_no_parent;
>> +static struct rpmhpd cx_no_parent = {
> 
> There are multiple variations of how each of these can be parented, but
> only one way they can be without a parent. So how about we turn this the
> other way around?
> 
> I.e. let's name this one "cx" and the existing one "cx_w_mx_parent".
> 
> 
> This will be particularly useful when you look at mmcx, which on
> 8150/8180 has mx as parent and on 8450 has cx as parent.
> 
> 
> PS. Unfortunately I had merged 8450 since you wrote this series, I tried
> to just fix it up as I applied your patch, but noticed 8450_cx and
> 8450_mmcx and wanted to get your opinion on this first.

I agree that sounds like a reasonable thing to do, I hadn't looked at 8450
so did not notice it, I will rebase my patches on top and repost.

> 
> Regards,
> Bjorn
> 
>> +	.pd = { .name = "cx", },
>> +	.peer = &cx_ao_no_parent,
>> +	.res_name = "cx.lvl",
>> +};
>> +
>> +static struct rpmhpd cx_ao_no_parent = {
>> +	.pd = { .name = "cx_ao", },
>> +	.active_only = true,
>> +	.peer = &cx_no_parent,
>> +	.res_name = "cx.lvl",
>> +};
>> +
>>   static struct rpmhpd mmcx_ao;
>>   static struct rpmhpd mmcx = {
>>   	.pd = { .name = "mmcx", },
>> @@ -273,8 +287,8 @@ static const struct rpmhpd_desc sc7180_desc = {
>>   
>>   /* SC7280 RPMH powerdomains */
>>   static struct rpmhpd *sc7280_rpmhpds[] = {
>> -	[SC7280_CX] = &cx,
>> -	[SC7280_CX_AO] = &cx_ao,
>> +	[SC7280_CX] = &cx_no_parent,
>> +	[SC7280_CX_AO] = &cx_ao_no_parent,
>>   	[SC7280_EBI] = &ebi,
>>   	[SC7280_GFX] = &gfx,
>>   	[SC7280_MX] = &mx,
>> -- 
>> 2.7.4
>>

Re: [PATCH v2 2/3] soc: qcom: rpmhpd: Remove mx/cx relationship on sc7280

[Rajendra Nayak]

On 12/9/2021 12:29 PM, Rajendra Nayak wrote:
> 
> On 12/9/2021 2:12 AM, Bjorn Andersson wrote:
>> On Tue 07 Dec 04:08 CST 2021, Rajendra Nayak wrote:
>>
>>> While the requirement to specify the active + sleep and active-only MX
>>> power-domains as the parents of the corresponding CX power domains is
>>> applicable for most SoCs, we have some like the sc7280 where this
>>> dependency is not applicable.
>>> Define new rpmhpd structs for cx and cx_ao without the mx as
>>> parent and use them for sc7280.
>>>
>>> Signed-off-by: Rajendra Nayak <quic_rjendra@quicinc.com>
>>> ---
>>>   drivers/soc/qcom/rpmhpd.c | 18 ++++++++++++++++--
>>>   1 file changed, 16 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
>>> index c71481d..4599efe 100644
>>> --- a/drivers/soc/qcom/rpmhpd.c
>>> +++ b/drivers/soc/qcom/rpmhpd.c
>>> @@ -120,6 +120,20 @@ static struct rpmhpd cx_ao = {
>>>       .res_name = "cx.lvl",
>>>   };
>>> +static struct rpmhpd cx_ao_no_parent;
>>> +static struct rpmhpd cx_no_parent = {
>>
>> There are multiple variations of how each of these can be parented, but
>> only one way they can be without a parent. So how about we turn this the
>> other way around?
>>
>> I.e. let's name this one "cx" and the existing one "cx_w_mx_parent".
>>
>>
>> This will be particularly useful when you look at mmcx, which on
>> 8150/8180 has mx as parent and on 8450 has cx as parent.

I noticed mmcx on 8150/8180 does not have mx as parent, nevertheless
I went ahead and moved to the _w_<parent-name>_parent suffix because
it made sense if we did run into a situation like this in the future.

>>
>>
>> PS. Unfortunately I had merged 8450 since you wrote this series, I tried
>> to just fix it up as I applied your patch, but noticed 8450_cx and
>> 8450_mmcx and wanted to get your opinion on this first.
> 
> I agree that sounds like a reasonable thing to do, I hadn't looked at 8450
> so did not notice it, I will rebase my patches on top and repost.
> 
>>
>> Regards,
>> Bjorn
>>
>>> +    .pd = { .name = "cx", },
>>> +    .peer = &cx_ao_no_parent,
>>> +    .res_name = "cx.lvl",
>>> +};
>>> +
>>> +static struct rpmhpd cx_ao_no_parent = {
>>> +    .pd = { .name = "cx_ao", },
>>> +    .active_only = true,
>>> +    .peer = &cx_no_parent,
>>> +    .res_name = "cx.lvl",
>>> +};
>>> +
>>>   static struct rpmhpd mmcx_ao;
>>>   static struct rpmhpd mmcx = {
>>>       .pd = { .name = "mmcx", },
>>> @@ -273,8 +287,8 @@ static const struct rpmhpd_desc sc7180_desc = {
>>>   /* SC7280 RPMH powerdomains */
>>>   static struct rpmhpd *sc7280_rpmhpds[] = {
>>> -    [SC7280_CX] = &cx,
>>> -    [SC7280_CX_AO] = &cx_ao,
>>> +    [SC7280_CX] = &cx_no_parent,
>>> +    [SC7280_CX_AO] = &cx_ao_no_parent,
>>>       [SC7280_EBI] = &ebi,
>>>       [SC7280_GFX] = &gfx,
>>>       [SC7280_MX] = &mx,
>>> -- 
>>> 2.7.4
>>>

Re: [PATCH v2 2/3] soc: qcom: rpmhpd: Remove mx/cx relationship on sc7280

[Bjorn Andersson]

On Thu 09 Dec 07:37 PST 2021, Rajendra Nayak wrote:

> 
> On 12/9/2021 12:29 PM, Rajendra Nayak wrote:
> > 
> > On 12/9/2021 2:12 AM, Bjorn Andersson wrote:
> > > On Tue 07 Dec 04:08 CST 2021, Rajendra Nayak wrote:
> > > 
> > > > While the requirement to specify the active + sleep and active-only MX
> > > > power-domains as the parents of the corresponding CX power domains is
> > > > applicable for most SoCs, we have some like the sc7280 where this
> > > > dependency is not applicable.
> > > > Define new rpmhpd structs for cx and cx_ao without the mx as
> > > > parent and use them for sc7280.
> > > > 
> > > > Signed-off-by: Rajendra Nayak <quic_rjendra@quicinc.com>
> > > > ---
> > > >   drivers/soc/qcom/rpmhpd.c | 18 ++++++++++++++++--
> > > >   1 file changed, 16 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
> > > > index c71481d..4599efe 100644
> > > > --- a/drivers/soc/qcom/rpmhpd.c
> > > > +++ b/drivers/soc/qcom/rpmhpd.c
> > > > @@ -120,6 +120,20 @@ static struct rpmhpd cx_ao = {
> > > >       .res_name = "cx.lvl",
> > > >   };
> > > > +static struct rpmhpd cx_ao_no_parent;
> > > > +static struct rpmhpd cx_no_parent = {
> > > 
> > > There are multiple variations of how each of these can be parented, but
> > > only one way they can be without a parent. So how about we turn this the
> > > other way around?
> > > 
> > > I.e. let's name this one "cx" and the existing one "cx_w_mx_parent".
> > > 
> > > 
> > > This will be particularly useful when you look at mmcx, which on
> > > 8150/8180 has mx as parent and on 8450 has cx as parent.
> 
> I noticed mmcx on 8150/8180 does not have mx as parent, nevertheless
> I went ahead and moved to the _w_<parent-name>_parent suffix because
> it made sense if we did run into a situation like this in the future.
> 

You're correct, the 8150/8180 mmcx are wrong, let's fix that once your
patches has settled (probably later today).

Thanks for cleaning up the driver!

Regards,
Bjorn

> > > 
> > > 
> > > PS. Unfortunately I had merged 8450 since you wrote this series, I tried
> > > to just fix it up as I applied your patch, but noticed 8450_cx and
> > > 8450_mmcx and wanted to get your opinion on this first.
> > 
> > I agree that sounds like a reasonable thing to do, I hadn't looked at 8450
> > so did not notice it, I will rebase my patches on top and repost.
> > 
> > > 
> > > Regards,
> > > Bjorn
> > > 
> > > > +    .pd = { .name = "cx", },
> > > > +    .peer = &cx_ao_no_parent,
> > > > +    .res_name = "cx.lvl",
> > > > +};
> > > > +
> > > > +static struct rpmhpd cx_ao_no_parent = {
> > > > +    .pd = { .name = "cx_ao", },
> > > > +    .active_only = true,
> > > > +    .peer = &cx_no_parent,
> > > > +    .res_name = "cx.lvl",
> > > > +};
> > > > +
> > > >   static struct rpmhpd mmcx_ao;
> > > >   static struct rpmhpd mmcx = {
> > > >       .pd = { .name = "mmcx", },
> > > > @@ -273,8 +287,8 @@ static const struct rpmhpd_desc sc7180_desc = {
> > > >   /* SC7280 RPMH powerdomains */
> > > >   static struct rpmhpd *sc7280_rpmhpds[] = {
> > > > -    [SC7280_CX] = &cx,
> > > > -    [SC7280_CX_AO] = &cx_ao,
> > > > +    [SC7280_CX] = &cx_no_parent,
> > > > +    [SC7280_CX_AO] = &cx_ao_no_parent,
> > > >       [SC7280_EBI] = &ebi,
> > > >       [SC7280_GFX] = &gfx,
> > > >       [SC7280_MX] = &mx,
> > > > -- 
> > > > 2.7.4
> > > > 

[PATCH v2 3/3] soc: qcom: rpmhpd: Sort power-domain definitions and lists

[Rajendra Nayak]

Sort all power-domain defines and the SoC specific lists in
alphabetical order for better readability.
No functional changes.

Signed-off-by: Rajendra Nayak <quic_rjendra@quicinc.com>
---
 drivers/soc/qcom/rpmhpd.c | 120 +++++++++++++++++++++++-----------------------
 1 file changed, 61 insertions(+), 59 deletions(-)

diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
index 4599efe..4df851f 100644
--- a/drivers/soc/qcom/rpmhpd.c
+++ b/drivers/soc/qcom/rpmhpd.c
@@ -65,46 +65,9 @@ static DEFINE_MUTEX(rpmhpd_lock);
 
 /* RPMH powerdomains */
 
-static struct rpmhpd ebi = {
-	.pd = { .name = "ebi", },
-	.res_name = "ebi.lvl",
-};
-
-static struct rpmhpd lmx = {
-	.pd = { .name = "lmx", },
-	.res_name = "lmx.lvl",
-};
-
-static struct rpmhpd lcx = {
-	.pd = { .name = "lcx", },
-	.res_name = "lcx.lvl",
-};
-
-static struct rpmhpd gfx = {
-	.pd = { .name = "gfx", },
-	.res_name = "gfx.lvl",
-};
-
-static struct rpmhpd mss = {
-	.pd = { .name = "mss", },
-	.res_name = "mss.lvl",
-};
-
-static struct rpmhpd mx_ao;
-static struct rpmhpd mx = {
-	.pd = { .name = "mx", },
-	.peer = &mx_ao,
-	.res_name = "mx.lvl",
-};
-
-static struct rpmhpd mx_ao = {
-	.pd = { .name = "mx_ao", },
-	.active_only = true,
-	.peer = &mx,
-	.res_name = "mx.lvl",
-};
-
 static struct rpmhpd cx_ao;
+static struct rpmhpd mx;
+static struct rpmhpd mx_ao;
 static struct rpmhpd cx = {
 	.pd = { .name = "cx", },
 	.peer = &cx_ao,
@@ -134,6 +97,26 @@ static struct rpmhpd cx_ao_no_parent = {
 	.res_name = "cx.lvl",
 };
 
+static struct rpmhpd ebi = {
+	.pd = { .name = "ebi", },
+	.res_name = "ebi.lvl",
+};
+
+static struct rpmhpd gfx = {
+	.pd = { .name = "gfx", },
+	.res_name = "gfx.lvl",
+};
+
+static struct rpmhpd lcx = {
+	.pd = { .name = "lcx", },
+	.res_name = "lcx.lvl",
+};
+
+static struct rpmhpd lmx = {
+	.pd = { .name = "lmx", },
+	.res_name = "lmx.lvl",
+};
+
 static struct rpmhpd mmcx_ao;
 static struct rpmhpd mmcx = {
 	.pd = { .name = "mmcx", },
@@ -148,6 +131,25 @@ static struct rpmhpd mmcx_ao = {
 	.res_name = "mmcx.lvl",
 };
 
+static struct rpmhpd mss = {
+	.pd = { .name = "mss", },
+	.res_name = "mss.lvl",
+};
+
+static struct rpmhpd mx_ao;
+static struct rpmhpd mx = {
+	.pd = { .name = "mx", },
+	.peer = &mx_ao,
+	.res_name = "mx.lvl",
+};
+
+static struct rpmhpd mx_ao = {
+	.pd = { .name = "mx_ao", },
+	.active_only = true,
+	.peer = &mx,
+	.res_name = "mx.lvl",
+};
+
 static struct rpmhpd mxc_ao;
 static struct rpmhpd mxc = {
 	.pd = { .name = "mxc", },
@@ -164,15 +166,15 @@ static struct rpmhpd mxc_ao = {
 
 /* SDM845 RPMH powerdomains */
 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_LMX] = &lmx,
-	[SDM845_LCX] = &lcx,
+	[SDM845_EBI] = &ebi,
 	[SDM845_GFX] = &gfx,
+	[SDM845_LCX] = &lcx,
+	[SDM845_LMX] = &lmx,
 	[SDM845_MSS] = &mss,
+	[SDM845_MX] = &mx,
+	[SDM845_MX_AO] = &mx_ao,
 };
 
 static const struct rpmhpd_desc sdm845_desc = {
@@ -182,9 +184,9 @@ static const struct rpmhpd_desc sdm845_desc = {
 
 /* SDX55 RPMH powerdomains */
 static struct rpmhpd *sdx55_rpmhpds[] = {
+	[SDX55_CX] = &cx,
 	[SDX55_MSS] = &mss,
 	[SDX55_MX] = &mx,
-	[SDX55_CX] = &cx,
 };
 
 static const struct rpmhpd_desc sdx55_desc = {
@@ -209,17 +211,17 @@ static const struct rpmhpd_desc sm6350_desc = {
 
 /* SM8150 RPMH powerdomains */
 static struct rpmhpd *sm8150_rpmhpds[] = {
-	[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_EBI] = &ebi,
+	[SM8150_GFX] = &gfx,
+	[SM8150_LCX] = &lcx,
+	[SM8150_LMX] = &lmx,
 	[SM8150_MMCX] = &mmcx,
 	[SM8150_MMCX_AO] = &mmcx_ao,
+	[SM8150_MSS] = &mss,
+	[SM8150_MX] = &mx,
+	[SM8150_MX_AO] = &mx_ao,
 };
 
 static const struct rpmhpd_desc sm8150_desc = {
@@ -256,11 +258,11 @@ static struct rpmhpd *sm8350_rpmhpds[] = {
 	[SM8350_LMX] = &lmx,
 	[SM8350_MMCX] = &mmcx,
 	[SM8350_MMCX_AO] = &mmcx_ao,
+	[SM8350_MSS] = &mss,
 	[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 = {
@@ -273,11 +275,11 @@ static struct rpmhpd *sc7180_rpmhpds[] = {
 	[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_LMX] = &lmx,
 	[SC7180_MSS] = &mss,
+	[SC7180_MX] = &mx,
+	[SC7180_MX_AO] = &mx_ao,
 };
 
 static const struct rpmhpd_desc sc7180_desc = {
@@ -291,11 +293,11 @@ static struct rpmhpd *sc7280_rpmhpds[] = {
 	[SC7280_CX_AO] = &cx_ao_no_parent,
 	[SC7280_EBI] = &ebi,
 	[SC7280_GFX] = &gfx,
-	[SC7280_MX] = &mx,
-	[SC7280_MX_AO] = &mx_ao,
-	[SC7280_LMX] = &lmx,
 	[SC7280_LCX] = &lcx,
+	[SC7280_LMX] = &lmx,
 	[SC7280_MSS] = &mss,
+	[SC7280_MX] = &mx,
+	[SC7280_MX_AO] = &mx_ao,
 };
 
 static const struct rpmhpd_desc sc7280_desc = {
-- 
2.7.4

Re: [PATCH v2 0/3] soc: qcom: rpmhpd: Cleanups and mx/cx fixup for sc7280

[Bjorn Andersson]

On Tue, 7 Dec 2021 15:38:29 +0530, Rajendra Nayak wrote:
> v2:
> * Fixed the wrong assumption in v1 that only sdm845 needed mx to be
> parent of cx, turned out all existing upstream SoCs need it except sc7280
> * Added another cleanup patch to sort power-domain defines and lists in
> alphabetical order as suggested by Matthias
> 
> Mostly cleanups, with a fixup to remove the parent/child relationship
> across mx/cx for sc7280 SoC.
> 
> [...]

Applied, thanks!

[1/3] soc: qcom: rpmhpd: Rename rpmhpd struct names
      commit: 8f3d4dd65abd03a5edcf7b5d5a7a3e2a4866db15
[2/3] soc: qcom: rpmhpd: Remove mx/cx relationship on sc7280
      commit: 65e7b31cc48581e32bee4546b59cea404252a138
[3/3] soc: qcom: rpmhpd: Sort power-domain definitions and lists
      commit: 8beb290d17f280f690857897af11dc90ac2e1f3d

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