diff mbox series

[v2] soc: qcom: rpmpd: Add MSM8939 power-domains

Message ID 20200916074924.20637-1-jun.nie@linaro.org (mailing list archive)
State New, archived
Headers show
Series [v2] soc: qcom: rpmpd: Add MSM8939 power-domains | expand

Commit Message

Jun Nie Sept. 16, 2020, 7:49 a.m. UTC
Add the shared modemcx/cx/mx power-domains found on MSM8939.

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 .../devicetree/bindings/power/qcom,rpmpd.yaml |  1 +
 drivers/soc/qcom/rpmpd.c                      | 27 +++++++++++++++++++
 include/dt-bindings/power/qcom-rpmpd.h        | 10 +++++++
 3 files changed, 38 insertions(+)

Comments

Jun Nie Sept. 16, 2020, 7:52 a.m. UTC | #1
Jun Nie <jun.nie@linaro.org> 于2020年9月16日周三 下午3:49写道:
>
> Add the shared modemcx/cx/mx power-domains found on MSM8939.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  .../devicetree/bindings/power/qcom,rpmpd.yaml |  1 +
>  drivers/soc/qcom/rpmpd.c                      | 27 +++++++++++++++++++
>  include/dt-bindings/power/qcom-rpmpd.h        | 10 +++++++
>  3 files changed, 38 insertions(+)
>

Fix max state and alphabetically order per comments from Stephan, vs
the version 1 patch.

Jun
Stephan Gerhold Sept. 16, 2020, 9:15 p.m. UTC | #2
Hi,

Thanks for sending a v2!

On Wed, Sep 16, 2020 at 03:49:24PM +0800, Jun Nie wrote:
> Add the shared modemcx/cx/mx power-domains found on MSM8939.
> 
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  .../devicetree/bindings/power/qcom,rpmpd.yaml |  1 +
>  drivers/soc/qcom/rpmpd.c                      | 27 +++++++++++++++++++
>  include/dt-bindings/power/qcom-rpmpd.h        | 10 +++++++
>  3 files changed, 38 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml b/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml
> index 8058955fb3b9..ce9b556ac155 100644
> --- a/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml
> +++ b/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml
> @@ -16,6 +16,7 @@ description:
>  properties:
>    compatible:
>      enum:
> +      - qcom,msm8939-rpmpd
>        - qcom,msm8976-rpmpd
>        - qcom,msm8996-rpmpd
>        - qcom,msm8998-rpmpd
> diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
> index f2168e4259b2..829f51881e41 100644
> --- a/drivers/soc/qcom/rpmpd.c
> +++ b/drivers/soc/qcom/rpmpd.c
> @@ -220,7 +220,34 @@ static const struct rpmpd_desc qcs404_desc = {
>  	.max_state = RPM_SMD_LEVEL_BINNING,
>  };
>  
> +/* msm8939 RPM Power Domains */
> +DEFINE_RPMPD_PAIR(msm8939, vddmd, vddmd_ao, SMPA, CORNER, 1);
> +DEFINE_RPMPD_VFC(msm8939, vddmd_vfc, SMPA, 1);
> +
> +DEFINE_RPMPD_PAIR(msm8939, vddcx, vddcx_ao, SMPA, CORNER, 2);
> +DEFINE_RPMPD_VFC(msm8939, vddcx_vfc, SMPA, 2);
> +
> +DEFINE_RPMPD_PAIR(msm8939, vddmx, vddmx_ao, LDOA, CORNER, 3);
> +
> +static struct rpmpd *msm8939_rpmpds[] = {
> +	[MSM8939_VDDMDCX] =	&msm8939_vddmd,
> +	[MSM8939_VDDMDCX_AO] =	&msm8939_vddmd_ao,
> +	[MSM8939_VDDMDCX_VFC] =	&msm8939_vddmd_vfc,
> +	[MSM8939_VDDCX] =	&msm8939_vddcx,
> +	[MSM8939_VDDCX_AO] =	&msm8939_vddcx_ao,
> +	[MSM8939_VDDCX_VFC] =	&msm8939_vddcx_vfc,
> +	[MSM8939_VDDMX] =	&msm8939_vddmx,
> +	[MSM8939_VDDMX_AO] =	&msm8939_vddmx_ao,
> +};
> +
> +static const struct rpmpd_desc msm8939_desc = {
> +	.rpmpds = msm8939_rpmpds,
> +	.num_pds = ARRAY_SIZE(msm8939_rpmpds),
> +	.max_state = MSM8939_VDDMX_AO,
> +};

For consistent ordering this block should be also above
/* msm8976 RPM Power Domains */

Also: .max_state still looks wrong to me. It has nothing to do with the
number of power domains (MSM8939_VDDMX_AO). Instead, it is supposed to
be the maximum possible peformance state (in your case: voltage corner).

As far as I know, all SoCs using voltage corners use the same set of
corners, a value from 0-6 for the following corners [1]:

    0. None
    1. Retention
    2. SVS Krait
    3. SVS SoC
    4. Normal/Nominal
    5. Turbo
    6. Super Turbo

(The corners are shifted by one in the msm-3.10 driver...)

Super Turbo (= 6) is the .max_state you want to set here.
That's why I suggested setting .max_state to MAX_8996_RPMPD_STATE (= 6).

In my patch series for MSM8916, I renamed MAX_8996_RPMPD_STATE to
MAX_CORNER_RPMPD_STATE [2]. Maybe that makes it a bit more clear.

Thanks!
Stephan

[1]: https://source.codeaurora.org/quic/la/kernel/msm-3.10/tree/include/linux/regulator/rpm-smd-regulator.h?h=LA.BR.1.2.9.1-02310-8x16.0#n31
[2]: https://lore.kernel.org/linux-arm-msm/20200916104135.25085-2-stephan@gerhold.net/
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml b/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml
index 8058955fb3b9..ce9b556ac155 100644
--- a/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml
+++ b/Documentation/devicetree/bindings/power/qcom,rpmpd.yaml
@@ -16,6 +16,7 @@  description:
 properties:
   compatible:
     enum:
+      - qcom,msm8939-rpmpd
       - qcom,msm8976-rpmpd
       - qcom,msm8996-rpmpd
       - qcom,msm8998-rpmpd
diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
index f2168e4259b2..829f51881e41 100644
--- a/drivers/soc/qcom/rpmpd.c
+++ b/drivers/soc/qcom/rpmpd.c
@@ -220,7 +220,34 @@  static const struct rpmpd_desc qcs404_desc = {
 	.max_state = RPM_SMD_LEVEL_BINNING,
 };
 
+/* msm8939 RPM Power Domains */
+DEFINE_RPMPD_PAIR(msm8939, vddmd, vddmd_ao, SMPA, CORNER, 1);
+DEFINE_RPMPD_VFC(msm8939, vddmd_vfc, SMPA, 1);
+
+DEFINE_RPMPD_PAIR(msm8939, vddcx, vddcx_ao, SMPA, CORNER, 2);
+DEFINE_RPMPD_VFC(msm8939, vddcx_vfc, SMPA, 2);
+
+DEFINE_RPMPD_PAIR(msm8939, vddmx, vddmx_ao, LDOA, CORNER, 3);
+
+static struct rpmpd *msm8939_rpmpds[] = {
+	[MSM8939_VDDMDCX] =	&msm8939_vddmd,
+	[MSM8939_VDDMDCX_AO] =	&msm8939_vddmd_ao,
+	[MSM8939_VDDMDCX_VFC] =	&msm8939_vddmd_vfc,
+	[MSM8939_VDDCX] =	&msm8939_vddcx,
+	[MSM8939_VDDCX_AO] =	&msm8939_vddcx_ao,
+	[MSM8939_VDDCX_VFC] =	&msm8939_vddcx_vfc,
+	[MSM8939_VDDMX] =	&msm8939_vddmx,
+	[MSM8939_VDDMX_AO] =	&msm8939_vddmx_ao,
+};
+
+static const struct rpmpd_desc msm8939_desc = {
+	.rpmpds = msm8939_rpmpds,
+	.num_pds = ARRAY_SIZE(msm8939_rpmpds),
+	.max_state = MSM8939_VDDMX_AO,
+};
+
 static const struct of_device_id rpmpd_match_table[] = {
+	{ .compatible = "qcom,msm8939-rpmpd", .data = &msm8939_desc },
 	{ .compatible = "qcom,msm8976-rpmpd", .data = &msm8976_desc },
 	{ .compatible = "qcom,msm8996-rpmpd", .data = &msm8996_desc },
 	{ .compatible = "qcom,msm8998-rpmpd", .data = &msm8998_desc },
diff --git a/include/dt-bindings/power/qcom-rpmpd.h b/include/dt-bindings/power/qcom-rpmpd.h
index 5e61eaf73bdd..fe207860456e 100644
--- a/include/dt-bindings/power/qcom-rpmpd.h
+++ b/include/dt-bindings/power/qcom-rpmpd.h
@@ -64,6 +64,16 @@ 
 #define RPMH_REGULATOR_LEVEL_TURBO	384
 #define RPMH_REGULATOR_LEVEL_TURBO_L1	416
 
+/* MSM8939 Power Domains */
+#define MSM8939_VDDMDCX		0
+#define MSM8939_VDDMDCX_AO	1
+#define MSM8939_VDDMDCX_VFC	2
+#define MSM8939_VDDCX		3
+#define MSM8939_VDDCX_AO	4
+#define MSM8939_VDDCX_VFC	5
+#define MSM8939_VDDMX		6
+#define MSM8939_VDDMX_AO	7
+
 /* MSM8976 Power Domain Indexes */
 #define MSM8976_VDDCX		0
 #define MSM8976_VDDCX_AO	1