[PATCH v3 0/9] RPMPD for QCS404 and MSM8998

[PATCH v3 0/9] RPMPD for QCS404 and MSM8998

[Sibi Sankar]

Re-worked the macros of the rpmpd driver. Add power domains support
for QCS404 and MSM8998.

V3:
* always send level updates to vfc and vfl in set_performance state
* fixup commit messages [Rajendra]
* fixup s-o-b ordering

V2:
* Add rpmpd support for msm8998
* fixup corner/vfc with vlfl/vfl

Bjorn Andersson (4):
  soc: qcom: rpmpd: Modify corner defining macros
  dt-bindings: power: Add rpm power domain bindings for qcs404
  soc: qcom: rpmpd: Add QCS404 power-domains
  arm64: dts: qcom: qcs404: Add rpmpd node

Sibi Sankar (5):
  soc: qcom: rpmpd: fixup rpmpd set performance state
  soc: qcom: rpmpd: Add support to set rpmpd state to max
  dt-bindings: power: Add rpm power domain bindings for msm8998
  soc: qcom: rpmpd: Add MSM8998 power-domains
  arm64: dts: qcom: msm8998: Add rpmpd node

 .../devicetree/bindings/power/qcom,rpmpd.txt  |   2 +
 arch/arm64/boot/dts/qcom/msm8998.dtsi         |  51 +++++++
 arch/arm64/boot/dts/qcom/qcs404.dtsi          |  55 +++++++
 drivers/soc/qcom/rpmpd.c                      | 135 ++++++++++++++----
 include/dt-bindings/power/qcom-rpmpd.h        |  34 +++++
 5 files changed, 252 insertions(+), 25 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 1/9] soc: qcom: rpmpd: fixup rpmpd set performance state

[Sibi Sankar]

Remoteproc q6v5-mss does set_performace_state with INT_MAX on
rpmpd. This is currently ignored since it is greater than the
max supported state. Fixup rpmpd state to max if the required
state is greater than all the supported states.

Fixes: 075d3db8d10d ("Add support for the .set_performace_state() and
.opp_to_performance_state()")

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 drivers/soc/qcom/rpmpd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
index 005326050c23..235d01870dd8 100644
--- a/drivers/soc/qcom/rpmpd.c
+++ b/drivers/soc/qcom/rpmpd.c
@@ -226,7 +226,7 @@ static int rpmpd_set_performance(struct generic_pm_domain *domain,
 	struct rpmpd *pd = domain_to_rpmpd(domain);
 
 	if (state > MAX_RPMPD_STATE)
-		goto out;
+		state = MAX_RPMPD_STATE;
 
 	mutex_lock(&rpmpd_lock);
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 2/9] soc: qcom: rpmpd: Add support to set rpmpd state to max

[Sibi Sankar]

rpmpd max state varies across SoCs and SoC families, add support
in the driver to make it SoC/SoC family specific

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 drivers/soc/qcom/rpmpd.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
index 235d01870dd8..94015ece40e9 100644
--- a/drivers/soc/qcom/rpmpd.c
+++ b/drivers/soc/qcom/rpmpd.c
@@ -25,7 +25,7 @@
 #define KEY_ENABLE		0x6e657773 /* swen */
 #define KEY_FLOOR_CORNER	0x636676   /* vfc */
 
-#define MAX_RPMPD_STATE		6
+#define MAX_8996_RPMPD_STATE	6
 
 #define DEFINE_RPMPD_CORNER_SMPA(_platform, _name, _active, r_id)		\
 	static struct rpmpd _platform##_##_active;			\
@@ -83,12 +83,14 @@ struct rpmpd {
 	const int res_type;
 	const int res_id;
 	struct qcom_smd_rpm *rpm;
+	unsigned int max_state;
 	__le32 key;
 };
 
 struct rpmpd_desc {
 	struct rpmpd **rpmpds;
 	size_t num_pds;
+	unsigned int max_state;
 };
 
 static DEFINE_MUTEX(rpmpd_lock);
@@ -114,6 +116,7 @@ static struct rpmpd *msm8996_rpmpds[] = {
 static const struct rpmpd_desc msm8996_desc = {
 	.rpmpds = msm8996_rpmpds,
 	.num_pds = ARRAY_SIZE(msm8996_rpmpds),
+	.max_state = MAX_8996_RPMPD_STATE,
 };
 
 static const struct of_device_id rpmpd_match_table[] = {
@@ -225,8 +228,8 @@ static int rpmpd_set_performance(struct generic_pm_domain *domain,
 	int ret = 0;
 	struct rpmpd *pd = domain_to_rpmpd(domain);
 
-	if (state > MAX_RPMPD_STATE)
-		state = MAX_RPMPD_STATE;
+	if (state > pd->max_state)
+		state = pd->max_state;
 
 	mutex_lock(&rpmpd_lock);
 
@@ -287,6 +290,7 @@ static int rpmpd_probe(struct platform_device *pdev)
 		}
 
 		rpmpds[i]->rpm = rpm;
+		rpmpds[i]->max_state = desc->max_state;
 		rpmpds[i]->pd.power_off = rpmpd_power_off;
 		rpmpds[i]->pd.power_on = rpmpd_power_on;
 		rpmpds[i]->pd.set_performance_state = rpmpd_set_performance;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 3/9] soc: qcom: rpmpd: Modify corner defining macros

[Sibi Sankar]

From: Bjorn Andersson <bjorn.andersson@linaro.org>

QCS404 uses individual resource type magic for each power-domain, so
adjust the macros slightly to make them reusable for this.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
[sibi: Extend rpmpd corner pair to a generic rpmpd pair]
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 drivers/soc/qcom/rpmpd.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
index 94015ece40e9..ca736ceb969c 100644
--- a/drivers/soc/qcom/rpmpd.c
+++ b/drivers/soc/qcom/rpmpd.c
@@ -17,8 +17,8 @@
 #define domain_to_rpmpd(domain) container_of(domain, struct rpmpd, pd)
 
 /* Resource types */
-#define RPMPD_SMPA 0x61706d73
-#define RPMPD_LDOA 0x616f646c
+#define RPMPD_SMPA 0x61706d73 /* smpa */
+#define RPMPD_LDOA 0x616f646c /* ldoa */
 
 /* Operation Keys */
 #define KEY_CORNER		0x6e726f63 /* corn */
@@ -27,46 +27,41 @@
 
 #define MAX_8996_RPMPD_STATE	6
 
-#define DEFINE_RPMPD_CORNER_SMPA(_platform, _name, _active, r_id)		\
+#define DEFINE_RPMPD_PAIR(_platform, _name, _active, r_type, r_key,	\
+			  r_id)						\
 	static struct rpmpd _platform##_##_active;			\
 	static struct rpmpd _platform##_##_name = {			\
 		.pd = {	.name = #_name,	},				\
 		.peer = &_platform##_##_active,				\
-		.res_type = RPMPD_SMPA,					\
+		.res_type = RPMPD_##r_type,				\
 		.res_id = r_id,						\
-		.key = KEY_CORNER,					\
+		.key = KEY_##r_key,					\
 	};								\
 	static struct rpmpd _platform##_##_active = {			\
 		.pd = { .name = #_active, },				\
 		.peer = &_platform##_##_name,				\
 		.active_only = true,					\
-		.res_type = RPMPD_SMPA,					\
+		.res_type = RPMPD_##r_type,				\
 		.res_id = r_id,						\
-		.key = KEY_CORNER,					\
+		.key = KEY_##r_key,					\
 	}
 
-#define DEFINE_RPMPD_CORNER_LDOA(_platform, _name, r_id)			\
+#define DEFINE_RPMPD_CORNER(_platform, _name, r_type, r_id)		\
 	static struct rpmpd _platform##_##_name = {			\
 		.pd = { .name = #_name, },				\
-		.res_type = RPMPD_LDOA,					\
+		.res_type = RPMPD_##r_type,				\
 		.res_id = r_id,						\
 		.key = KEY_CORNER,					\
 	}
 
-#define DEFINE_RPMPD_VFC(_platform, _name, r_id, r_type)		\
+#define DEFINE_RPMPD_VFC(_platform, _name, r_type, r_id)		\
 	static struct rpmpd _platform##_##_name = {			\
 		.pd = { .name = #_name, },				\
-		.res_type = r_type,					\
+		.res_type = RPMPD_##r_type,				\
 		.res_id = r_id,						\
 		.key = KEY_FLOOR_CORNER,				\
 	}
 
-#define DEFINE_RPMPD_VFC_SMPA(_platform, _name, r_id)			\
-	DEFINE_RPMPD_VFC(_platform, _name, r_id, RPMPD_SMPA)
-
-#define DEFINE_RPMPD_VFC_LDOA(_platform, _name, r_id)			\
-	DEFINE_RPMPD_VFC(_platform, _name, r_id, RPMPD_LDOA)
-
 struct rpmpd_req {
 	__le32 key;
 	__le32 nbytes;
@@ -96,12 +91,12 @@ struct rpmpd_desc {
 static DEFINE_MUTEX(rpmpd_lock);
 
 /* msm8996 RPM Power domains */
-DEFINE_RPMPD_CORNER_SMPA(msm8996, vddcx, vddcx_ao, 1);
-DEFINE_RPMPD_CORNER_SMPA(msm8996, vddmx, vddmx_ao, 2);
-DEFINE_RPMPD_CORNER_LDOA(msm8996, vddsscx, 26);
+DEFINE_RPMPD_PAIR(msm8996, vddcx, vddcx_ao, SMPA, CORNER, 1);
+DEFINE_RPMPD_PAIR(msm8996, vddmx, vddmx_ao, SMPA, CORNER, 2);
+DEFINE_RPMPD_CORNER(msm8996, vddsscx, LDOA, 26);
 
-DEFINE_RPMPD_VFC_SMPA(msm8996, vddcx_vfc, 1);
-DEFINE_RPMPD_VFC_LDOA(msm8996, vddsscx_vfc, 26);
+DEFINE_RPMPD_VFC(msm8996, vddcx_vfc, SMPA, 1);
+DEFINE_RPMPD_VFC(msm8996, vddsscx_vfc, LDOA, 26);
 
 static struct rpmpd *msm8996_rpmpds[] = {
 	[MSM8996_VDDCX] =	&msm8996_vddcx,
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 4/9] dt-bindings: power: Add rpm power domain bindings for qcs404

[Sibi Sankar]

From: Bjorn Andersson <bjorn.andersson@linaro.org>

Add RPM power domain bindings for the qcs404 family of SoC

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
[sibis: Add supported rpmpd states for qcs404]
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---

skip adding Rob's R-b due to some additional changes
https://lore.kernel.org/lkml/20190325223343.GA27389@bogus/

 .../devicetree/bindings/power/qcom,rpmpd.txt  |  1 +
 include/dt-bindings/power/qcom-rpmpd.h        | 22 +++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/qcom,rpmpd.txt b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
index 980e5413d18f..172ccf940c5c 100644
--- a/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
+++ b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
@@ -6,6 +6,7 @@ which then translates it into a corresponding voltage on a rail
 Required Properties:
  - compatible: Should be one of the following
 	* qcom,msm8996-rpmpd: RPM Power domain for the msm8996 family of SoC
+	* qcom,qcs404-rpmpd: RPM Power domain for the qcs404 family of SoC
 	* qcom,sdm845-rpmhpd: RPMh Power domain for the sdm845 family of SoC
  - #power-domain-cells: number of cells in Power domain specifier
 	must be 1.
diff --git a/include/dt-bindings/power/qcom-rpmpd.h b/include/dt-bindings/power/qcom-rpmpd.h
index 87d9c6611682..450378662944 100644
--- a/include/dt-bindings/power/qcom-rpmpd.h
+++ b/include/dt-bindings/power/qcom-rpmpd.h
@@ -36,4 +36,26 @@
 #define MSM8996_VDDSSCX		5
 #define MSM8996_VDDSSCX_VFC	6
 
+/* QCS404 Power Domains */
+#define QCS404_VDDMX		0
+#define QCS404_VDDMX_AO		1
+#define QCS404_VDDMX_VFL	2
+#define QCS404_LPICX		3
+#define QCS404_LPICX_VFL	4
+#define QCS404_LPIMX		5
+#define QCS404_LPIMX_VFL	6
+
+/* RPM SMD Power Domain performance levels */
+#define RPM_SMD_LEVEL_RETENTION       16
+#define RPM_SMD_LEVEL_RETENTION_PLUS  32
+#define RPM_SMD_LEVEL_MIN_SVS         48
+#define RPM_SMD_LEVEL_LOW_SVS         64
+#define RPM_SMD_LEVEL_SVS             128
+#define RPM_SMD_LEVEL_SVS_PLUS        192
+#define RPM_SMD_LEVEL_NOM             256
+#define RPM_SMD_LEVEL_NOM_PLUS        320
+#define RPM_SMD_LEVEL_TURBO           384
+#define RPM_SMD_LEVEL_TURBO_NO_CPR    416
+#define RPM_SMD_LEVEL_BINNING         512
+
 #endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 5/9] soc: qcom: rpmpd: Add QCS404 power-domains

[Sibi Sankar]

From: Bjorn Andersson <bjorn.andersson@linaro.org>

Add the shared cx/mx and the low-power-island's cx and mx power-domains
found on QCS404.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
[sibi: Fixup corner/vfc with vlfl/vfl]
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 drivers/soc/qcom/rpmpd.c | 52 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
index ca736ceb969c..238a9e02e890 100644
--- a/drivers/soc/qcom/rpmpd.c
+++ b/drivers/soc/qcom/rpmpd.c
@@ -19,11 +19,16 @@
 /* Resource types */
 #define RPMPD_SMPA 0x61706d73 /* smpa */
 #define RPMPD_LDOA 0x616f646c /* ldoa */
+#define RPMPD_RWMX 0x786d7772 /* rwmx */
+#define RPMPD_RWLC 0x636c7772 /* rwlc */
+#define RPMPD_RWLM 0x6d6c7772 /* rwlm */
 
 /* Operation Keys */
 #define KEY_CORNER		0x6e726f63 /* corn */
 #define KEY_ENABLE		0x6e657773 /* swen */
 #define KEY_FLOOR_CORNER	0x636676   /* vfc */
+#define KEY_FLOOR_LEVEL		0x6c6676   /* vfl */
+#define KEY_LEVEL		0x6c766c76 /* vlvl */
 
 #define MAX_8996_RPMPD_STATE	6
 
@@ -54,6 +59,14 @@
 		.key = KEY_CORNER,					\
 	}
 
+#define DEFINE_RPMPD_LEVEL(_platform, _name, r_type, r_id)		\
+	static struct rpmpd _platform##_##_name = {			\
+		.pd = { .name = #_name, },				\
+		.res_type = RPMPD_##r_type,				\
+		.res_id = r_id,						\
+		.key = KEY_LEVEL,					\
+	}
+
 #define DEFINE_RPMPD_VFC(_platform, _name, r_type, r_id)		\
 	static struct rpmpd _platform##_##_name = {			\
 		.pd = { .name = #_name, },				\
@@ -62,6 +75,14 @@
 		.key = KEY_FLOOR_CORNER,				\
 	}
 
+#define DEFINE_RPMPD_VFL(_platform, _name, r_type, r_id)		\
+	static struct rpmpd _platform##_##_name = {			\
+		.pd = { .name = #_name, },				\
+		.res_type = RPMPD_##r_type,				\
+		.res_id = r_id,						\
+		.key = KEY_FLOOR_LEVEL,					\
+	}
+
 struct rpmpd_req {
 	__le32 key;
 	__le32 nbytes;
@@ -114,8 +135,35 @@ static const struct rpmpd_desc msm8996_desc = {
 	.max_state = MAX_8996_RPMPD_STATE,
 };
 
+/* qcs404 RPM Power domains */
+DEFINE_RPMPD_PAIR(qcs404, vddmx, vddmx_ao, RWMX, LEVEL, 0);
+DEFINE_RPMPD_VFL(qcs404, vddmx_vfl, RWMX, 0);
+
+DEFINE_RPMPD_LEVEL(qcs404, vdd_lpicx, RWLC, 0);
+DEFINE_RPMPD_VFL(qcs404, vdd_lpicx_vfl, RWLC, 0);
+
+DEFINE_RPMPD_LEVEL(qcs404, vdd_lpimx, RWLM, 0);
+DEFINE_RPMPD_VFL(qcs404, vdd_lpimx_vfl, RWLM, 0);
+
+static struct rpmpd *qcs404_rpmpds[] = {
+	[QCS404_VDDMX] = &qcs404_vddmx,
+	[QCS404_VDDMX_AO] = &qcs404_vddmx_ao,
+	[QCS404_VDDMX_VFL] = &qcs404_vddmx_vfl,
+	[QCS404_LPICX] = &qcs404_vdd_lpicx,
+	[QCS404_LPICX_VFL] = &qcs404_vdd_lpicx_vfl,
+	[QCS404_LPIMX] = &qcs404_vdd_lpimx,
+	[QCS404_LPIMX_VFL] = &qcs404_vdd_lpimx_vfl,
+};
+
+static const struct rpmpd_desc qcs404_desc = {
+	.rpmpds = qcs404_rpmpds,
+	.num_pds = ARRAY_SIZE(qcs404_rpmpds),
+	.max_state = RPM_SMD_LEVEL_BINNING,
+};
+
 static const struct of_device_id rpmpd_match_table[] = {
 	{ .compatible = "qcom,msm8996-rpmpd", .data = &msm8996_desc },
+	{ .compatible = "qcom,qcs404-rpmpd", .data = &qcs404_desc },
 	{ }
 };
 
@@ -230,7 +278,9 @@ static int rpmpd_set_performance(struct generic_pm_domain *domain,
 
 	pd->corner = state;
 
-	if (!pd->enabled && pd->key != KEY_FLOOR_CORNER)
+	/* Always send updates for vfc and vfl */
+	if (!pd->enabled && pd->key != KEY_FLOOR_CORNER &&
+	    pd->key != KEY_FLOOR_LEVEL)
 		goto out;
 
 	ret = rpmpd_aggregate_corner(pd);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 6/9] arm64: dts: qcom: qcs404: Add rpmpd node

[Sibi Sankar]

From: Bjorn Andersson <bjorn.andersson@linaro.org>

Add the rpmpd node on the qcs404 and define the available levels.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
[sibis: fixup available levels]
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/qcs404.dtsi | 55 ++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index e8fd26633d57..a7d46647c416 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -4,6 +4,7 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/clock/qcom,gcc-qcs404.h>
 #include <dt-bindings/clock/qcom,rpmcc.h>
+#include <dt-bindings/power/qcom-rpmpd.h>
 
 / {
 	interrupt-parent = <&intc>;
@@ -230,6 +231,60 @@
 				compatible = "qcom,rpmcc-qcs404";
 				#clock-cells = <1>;
 			};
+
+			rpmpd: power-controller {
+				compatible = "qcom,qcs404-rpmpd";
+				#power-domain-cells = <1>;
+				operating-points-v2 = <&rpmpd_opp_table>;
+
+				rpmpd_opp_table: opp-table {
+					compatible = "operating-points-v2";
+
+					rpmpd_opp_ret: opp1 {
+						opp-level = <RPM_SMD_LEVEL_RETENTION>;
+					};
+
+					rpmpd_opp_ret_plus: opp2 {
+						opp-level = <RPM_SMD_LEVEL_RETENTION_PLUS>;
+					};
+
+					rpmpd_opp_min_svs: opp3 {
+						opp-level = <RPM_SMD_LEVEL_MIN_SVS>;
+					};
+
+					rpmpd_opp_low_svs: opp4 {
+						opp-level = <RPM_SMD_LEVEL_LOW_SVS>;
+					};
+
+					rpmhpd_opp_svs: opp5 {
+						opp-level = <RPM_SMD_LEVEL_SVS>;
+					};
+
+					rpmhpd_opp_svs_plus: opp6 {
+						opp-level = <RPM_SMD_LEVEL_SVS_PLUS>;
+					};
+
+					rpmhpd_opp_nom: opp7 {
+						opp-level = <RPM_SMD_LEVEL_NOM>;
+					};
+
+					rpmhpd_opp_nom_plus: opp8 {
+						opp-level = <RPM_SMD_LEVEL_NOM_PLUS>;
+					};
+
+					rpmhpd_opp_turbo: opp9 {
+						opp-level = <RPM_SMD_LEVEL_TURBO>;
+					};
+
+					rpmhpd_opp_turbo_no_cpr: opp10 {
+						opp-level = <RPM_SMD_LEVEL_TURBO_NO_CPR>;
+					};
+
+					rpmhpd_opp_turbo_plus: opp11 {
+						opp-level = <RPM_SMD_LEVEL_BINNING>;
+					};
+				};
+			};
 		};
 	};
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 7/9] dt-bindings: power: Add rpm power domain bindings for msm8998

[Sibi Sankar]

Add RPM power domain bindings for the msm8998 family of SoC

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 .../devicetree/bindings/power/qcom,rpmpd.txt         |  1 +
 include/dt-bindings/power/qcom-rpmpd.h               | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/qcom,rpmpd.txt b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
index 172ccf940c5c..eb35b22f9e23 100644
--- a/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
+++ b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
@@ -6,6 +6,7 @@ which then translates it into a corresponding voltage on a rail
 Required Properties:
  - compatible: Should be one of the following
 	* qcom,msm8996-rpmpd: RPM Power domain for the msm8996 family of SoC
+	* qcom,msm8998-rpmpd: RPM Power domain for the msm8998 family of SoC
 	* qcom,qcs404-rpmpd: RPM Power domain for the qcs404 family of SoC
 	* qcom,sdm845-rpmhpd: RPMh Power domain for the sdm845 family of SoC
  - #power-domain-cells: number of cells in Power domain specifier
diff --git a/include/dt-bindings/power/qcom-rpmpd.h b/include/dt-bindings/power/qcom-rpmpd.h
index 450378662944..93e36d011527 100644
--- a/include/dt-bindings/power/qcom-rpmpd.h
+++ b/include/dt-bindings/power/qcom-rpmpd.h
@@ -36,6 +36,18 @@
 #define MSM8996_VDDSSCX		5
 #define MSM8996_VDDSSCX_VFC	6
 
+/* MSM8998 Power Domain Indexes */
+#define MSM8998_VDDCX		0
+#define MSM8998_VDDCX_AO	1
+#define MSM8998_VDDCX_VFL	2
+#define MSM8998_VDDMX		3
+#define MSM8998_VDDMX_AO	4
+#define MSM8998_VDDMX_VFL	5
+#define MSM8998_SSCCX		6
+#define MSM8998_SSCCX_VFL	7
+#define MSM8998_SSCMX		8
+#define MSM8998_SSCMX_VFL	9
+
 /* QCS404 Power Domains */
 #define QCS404_VDDMX		0
 #define QCS404_VDDMX_AO		1
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 8/9] soc: qcom: rpmpd: Add MSM8998 power-domains

[Sibi Sankar]

Add the shared cx/mx and sensor sub-system's cx and mx
power-domains found on MSM8998.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 drivers/soc/qcom/rpmpd.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
index 238a9e02e890..706a3f63038e 100644
--- a/drivers/soc/qcom/rpmpd.c
+++ b/drivers/soc/qcom/rpmpd.c
@@ -19,9 +19,12 @@
 /* Resource types */
 #define RPMPD_SMPA 0x61706d73 /* smpa */
 #define RPMPD_LDOA 0x616f646c /* ldoa */
+#define RPMPD_RWCX 0x78637772 /* rwcx */
 #define RPMPD_RWMX 0x786d7772 /* rwmx */
 #define RPMPD_RWLC 0x636c7772 /* rwlc */
 #define RPMPD_RWLM 0x6d6c7772 /* rwlm */
+#define RPMPD_RWSC 0x63737772 /* rwsc */
+#define RPMPD_RWSM 0x6d737772 /* rwsm */
 
 /* Operation Keys */
 #define KEY_CORNER		0x6e726f63 /* corn */
@@ -135,6 +138,38 @@ static const struct rpmpd_desc msm8996_desc = {
 	.max_state = MAX_8996_RPMPD_STATE,
 };
 
+/* msm8998 RPM Power domains */
+DEFINE_RPMPD_PAIR(msm8998, vddcx, vddcx_ao, RWCX, LEVEL, 0);
+DEFINE_RPMPD_VFL(msm8998, vddcx_vfl, RWCX, 0);
+
+DEFINE_RPMPD_PAIR(msm8998, vddmx, vddmx_ao, RWMX, LEVEL, 0);
+DEFINE_RPMPD_VFL(msm8998, vddmx_vfl, RWMX, 0);
+
+DEFINE_RPMPD_LEVEL(msm8998, vdd_ssccx, RWSC, 0);
+DEFINE_RPMPD_VFL(msm8998, vdd_ssccx_vfl, RWSC, 0);
+
+DEFINE_RPMPD_LEVEL(msm8998, vdd_sscmx, RWSM, 0);
+DEFINE_RPMPD_VFL(msm8998, vdd_sscmx_vfl, RWSM, 0);
+
+static struct rpmpd *msm8998_rpmpds[] = {
+	[MSM8998_VDDCX] =		&msm8998_vddcx,
+	[MSM8998_VDDCX_AO] =		&msm8998_vddcx_ao,
+	[MSM8998_VDDCX_VFL] =		&msm8998_vddcx_vfl,
+	[MSM8998_VDDMX] =		&msm8998_vddmx,
+	[MSM8998_VDDMX_AO] =		&msm8998_vddmx_ao,
+	[MSM8998_VDDMX_VFL] =		&msm8998_vddmx_vfl,
+	[MSM8998_SSCCX] =		&msm8998_vdd_ssccx,
+	[MSM8998_SSCCX_VFL] =		&msm8998_vdd_ssccx_vfl,
+	[MSM8998_SSCMX] =		&msm8998_vdd_sscmx,
+	[MSM8998_SSCMX_VFL] =		&msm8998_vdd_sscmx_vfl,
+};
+
+static const struct rpmpd_desc msm8998_desc = {
+	.rpmpds = msm8998_rpmpds,
+	.num_pds = ARRAY_SIZE(msm8998_rpmpds),
+	.max_state = RPM_SMD_LEVEL_BINNING,
+};
+
 /* qcs404 RPM Power domains */
 DEFINE_RPMPD_PAIR(qcs404, vddmx, vddmx_ao, RWMX, LEVEL, 0);
 DEFINE_RPMPD_VFL(qcs404, vddmx_vfl, RWMX, 0);
@@ -163,6 +198,7 @@ static const struct rpmpd_desc qcs404_desc = {
 
 static const struct of_device_id rpmpd_match_table[] = {
 	{ .compatible = "qcom,msm8996-rpmpd", .data = &msm8996_desc },
+	{ .compatible = "qcom,msm8998-rpmpd", .data = &msm8998_desc },
 	{ .compatible = "qcom,qcs404-rpmpd", .data = &qcs404_desc },
 	{ }
 };
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 9/9] arm64: dts: qcom: msm8998: Add rpmpd node

[Sibi Sankar]

Add the rpmpd node on the msm8998 and define the available levels.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/msm8998.dtsi | 51 +++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index 3fd0769fe648..b7e3e0646b9b 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -4,6 +4,7 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/clock/qcom,gcc-msm8998.h>
 #include <dt-bindings/clock/qcom,rpmcc.h>
+#include <dt-bindings/power/qcom-rpmpd.h>
 #include <dt-bindings/gpio/gpio.h>
 
 / {
@@ -272,6 +273,56 @@
 				compatible = "qcom,rpmcc-msm8998", "qcom,rpmcc";
 				#clock-cells = <1>;
 			};
+
+			rpmpd: power-controller {
+				compatible = "qcom,msm8998-rpmpd";
+				#power-domain-cells = <1>;
+				operating-points-v2 = <&rpmpd_opp_table>;
+
+				rpmpd_opp_table: opp-table {
+					compatible = "operating-points-v2";
+
+					rpmpd_opp_ret: opp1 {
+						opp-level = <RPM_SMD_LEVEL_RETENTION>;
+					};
+
+					rpmpd_opp_ret_plus: opp2 {
+						opp-level = <RPM_SMD_LEVEL_RETENTION_PLUS>;
+					};
+
+					rpmpd_opp_min_svs: opp3 {
+						opp-level = <RPM_SMD_LEVEL_MIN_SVS>;
+					};
+
+					rpmpd_opp_low_svs: opp4 {
+						opp-level = <RPM_SMD_LEVEL_LOW_SVS>;
+					};
+
+					rpmhpd_opp_svs: opp5 {
+						opp-level = <RPM_SMD_LEVEL_SVS>;
+					};
+
+					rpmhpd_opp_svs_plus: opp6 {
+						opp-level = <RPM_SMD_LEVEL_SVS_PLUS>;
+					};
+
+					rpmhpd_opp_nom: opp7 {
+						opp-level = <RPM_SMD_LEVEL_NOM>;
+					};
+
+					rpmhpd_opp_nom_plus: opp8 {
+						opp-level = <RPM_SMD_LEVEL_NOM_PLUS>;
+					};
+
+					rpmhpd_opp_turbo: opp9 {
+						opp-level = <RPM_SMD_LEVEL_TURBO>;
+					};
+
+					rpmhpd_opp_turbo_plus: opp10 {
+						opp-level = <RPM_SMD_LEVEL_BINNING>;
+					};
+				};
+			};
 		};
 	};
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 4/9] dt-bindings: power: Add rpm power domain bindings for qcs404

[Rob Herring]

On Wed, 27 Mar 2019 18:08:27 +0530, Sibi Sankar wrote:
> From: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> Add RPM power domain bindings for the qcs404 family of SoC
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> [sibis: Add supported rpmpd states for qcs404]
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> 
> skip adding Rob's R-b due to some additional changes
> https://lore.kernel.org/lkml/20190325223343.GA27389@bogus/
> 
>  .../devicetree/bindings/power/qcom,rpmpd.txt  |  1 +
>  include/dt-bindings/power/qcom-rpmpd.h        | 22 +++++++++++++++++++
>  2 files changed, 23 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 7/9] dt-bindings: power: Add rpm power domain bindings for msm8998

[Rob Herring]

On Wed, 27 Mar 2019 18:08:30 +0530, Sibi Sankar wrote:
> Add RPM power domain bindings for the msm8998 family of SoC
> 
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
>  .../devicetree/bindings/power/qcom,rpmpd.txt         |  1 +
>  include/dt-bindings/power/qcom-rpmpd.h               | 12 ++++++++++++
>  2 files changed, 13 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 0/9] RPMPD for QCS404 and MSM8998

[Marc Gonzalez]

On 27/03/2019 13:38, Sibi Sankar wrote:

> Re-worked the macros of the rpmpd driver. Add power domains support
> for QCS404 and MSM8998.
> 
> V3:
> * always send level updates to vfc and vfl in set_performance state
> * fixup commit messages [Rajendra]
> * fixup s-o-b ordering
> 
> V2:
> * Add rpmpd support for msm8998
> * fixup corner/vfc with vlfl/vfl
> 
> Bjorn Andersson (4):
>   soc: qcom: rpmpd: Modify corner defining macros
>   dt-bindings: power: Add rpm power domain bindings for qcs404
>   soc: qcom: rpmpd: Add QCS404 power-domains
>   arm64: dts: qcom: qcs404: Add rpmpd node
> 
> Sibi Sankar (5):
>   soc: qcom: rpmpd: fixup rpmpd set performance state
>   soc: qcom: rpmpd: Add support to set rpmpd state to max
>   dt-bindings: power: Add rpm power domain bindings for msm8998
>   soc: qcom: rpmpd: Add MSM8998 power-domains
>   arm64: dts: qcom: msm8998: Add rpmpd node
> 
>  .../devicetree/bindings/power/qcom,rpmpd.txt  |   2 +
>  arch/arm64/boot/dts/qcom/msm8998.dtsi         |  51 +++++++
>  arch/arm64/boot/dts/qcom/qcs404.dtsi          |  55 +++++++
>  drivers/soc/qcom/rpmpd.c                      | 135 ++++++++++++++----
>  include/dt-bindings/power/qcom-rpmpd.h        |  34 +++++
>  5 files changed, 252 insertions(+), 25 deletions(-)

Hello Sibi,

Please note that you sent this series to linux-arm-msm-owner@vger.kernel.org
instead of linux-arm-msm@vger.kernel.org (the admin instead of the list).

This seems to be the case for other patches as well.

Regards.

Re: [PATCH v3 1/9] soc: qcom: rpmpd: fixup rpmpd set performance state

[Marc Gonzalez]

On 27/03/2019 13:38, Sibi Sankar wrote:

> Remoteproc q6v5-mss does set_performace_state with INT_MAX on

Remoteproc q6v5-mss calls set_performance_state()

> rpmpd. This is currently ignored since it is greater than the
> max supported state. Fixup rpmpd state to max if the required
> state is greater than all the supported states.
> 
> Fixes: 075d3db8d10d ("Add support for the .set_performace_state() and
> .opp_to_performance_state()")
> 
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>

AFAIU, the Fixes tag should be on a single line, and quote the commit subject.
(And no blank line between Fixes and Signed-off-by tags.)

Fixes: 075d3db8d10d ("soc: qcom: rpmpd: Add support for get/set performance state")

> ---
>  drivers/soc/qcom/rpmpd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
> index 005326050c23..235d01870dd8 100644
> --- a/drivers/soc/qcom/rpmpd.c
> +++ b/drivers/soc/qcom/rpmpd.c
> @@ -226,7 +226,7 @@ static int rpmpd_set_performance(struct generic_pm_domain *domain,
>  	struct rpmpd *pd = domain_to_rpmpd(domain);
>  
>  	if (state > MAX_RPMPD_STATE)
> -		goto out;
> +		state = MAX_RPMPD_STATE;
>  
>  	mutex_lock(&rpmpd_lock);
>  

With these two nits taken care of:

Reviewed-by: Marc Gonzalez <marc.w.gonzalez@free.fr>

Regards.

Re: [PATCH v3 8/9] soc: qcom: rpmpd: Add MSM8998 power-domains

[Marc Gonzalez]

On 27/03/2019 13:38, Sibi Sankar wrote:

> Add the shared cx/mx and sensor sub-system's cx and mx
> power-domains found on MSM8998.
> 
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
>  drivers/soc/qcom/rpmpd.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
> index 238a9e02e890..706a3f63038e 100644
> --- a/drivers/soc/qcom/rpmpd.c
> +++ b/drivers/soc/qcom/rpmpd.c
> @@ -19,9 +19,12 @@
>  /* Resource types */
>  #define RPMPD_SMPA 0x61706d73 /* smpa */
>  #define RPMPD_LDOA 0x616f646c /* ldoa */
> +#define RPMPD_RWCX 0x78637772 /* rwcx */
>  #define RPMPD_RWMX 0x786d7772 /* rwmx */
>  #define RPMPD_RWLC 0x636c7772 /* rwlc */
>  #define RPMPD_RWLM 0x6d6c7772 /* rwlm */
> +#define RPMPD_RWSC 0x63737772 /* rwsc */
> +#define RPMPD_RWSM 0x6d737772 /* rwsm */

I do not see any value in the comments. Maybe remove them?

I will take a closer look at patches 7-9 on Monday.

Regards.

Re: [PATCH v3 6/9] arm64: dts: qcom: qcs404: Add rpmpd node

[Sibi Sankar]

On 2019-03-27 18:08, Sibi Sankar wrote:
> From: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> Add the rpmpd node on the qcs404 and define the available levels.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> [sibis: fixup available levels]
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/qcs404.dtsi | 55 ++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi
> b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> index e8fd26633d57..a7d46647c416 100644
> --- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
> +++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> @@ -4,6 +4,7 @@
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/clock/qcom,gcc-qcs404.h>
>  #include <dt-bindings/clock/qcom,rpmcc.h>
> +#include <dt-bindings/power/qcom-rpmpd.h>
> 
>  / {
>  	interrupt-parent = <&intc>;
> @@ -230,6 +231,60 @@
>  				compatible = "qcom,rpmcc-qcs404";
>  				#clock-cells = <1>;
>  			};
> +
> +			rpmpd: power-controller {
> +				compatible = "qcom,qcs404-rpmpd";
> +				#power-domain-cells = <1>;
> +				operating-points-v2 = <&rpmpd_opp_table>;
> +
> +				rpmpd_opp_table: opp-table {
> +					compatible = "operating-points-v2";
> +
> +					rpmpd_opp_ret: opp1 {
> +						opp-level = <RPM_SMD_LEVEL_RETENTION>;
> +					};
> +
> +					rpmpd_opp_ret_plus: opp2 {
> +						opp-level = <RPM_SMD_LEVEL_RETENTION_PLUS>;
> +					};
> +
> +					rpmpd_opp_min_svs: opp3 {
> +						opp-level = <RPM_SMD_LEVEL_MIN_SVS>;
> +					};
> +
> +					rpmpd_opp_low_svs: opp4 {
> +						opp-level = <RPM_SMD_LEVEL_LOW_SVS>;
> +					};
> +

typo in opp5 - opp11 rpmhpd/rpmpd
will fix it in the next re-spin

> +					rpmhpd_opp_svs: opp5 {
> +						opp-level = <RPM_SMD_LEVEL_SVS>;
> +					};
> +
> +					rpmhpd_opp_svs_plus: opp6 {
> +						opp-level = <RPM_SMD_LEVEL_SVS_PLUS>;
> +					};
> +
> +					rpmhpd_opp_nom: opp7 {
> +						opp-level = <RPM_SMD_LEVEL_NOM>;
> +					};
> +
> +					rpmhpd_opp_nom_plus: opp8 {
> +						opp-level = <RPM_SMD_LEVEL_NOM_PLUS>;
> +					};
> +
> +					rpmhpd_opp_turbo: opp9 {
> +						opp-level = <RPM_SMD_LEVEL_TURBO>;
> +					};
> +
> +					rpmhpd_opp_turbo_no_cpr: opp10 {
> +						opp-level = <RPM_SMD_LEVEL_TURBO_NO_CPR>;
> +					};
> +
> +					rpmhpd_opp_turbo_plus: opp11 {
> +						opp-level = <RPM_SMD_LEVEL_BINNING>;
> +					};
> +				};
> +			};
>  		};
>  	};

-- 
-- Sibi Sankar --
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH v3 9/9] arm64: dts: qcom: msm8998: Add rpmpd node

[Sibi Sankar]

On 2019-03-27 18:08, Sibi Sankar wrote:
> Add the rpmpd node on the msm8998 and define the available levels.
> 
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/msm8998.dtsi | 51 +++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> index 3fd0769fe648..b7e3e0646b9b 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> @@ -4,6 +4,7 @@
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/clock/qcom,gcc-msm8998.h>
>  #include <dt-bindings/clock/qcom,rpmcc.h>
> +#include <dt-bindings/power/qcom-rpmpd.h>
>  #include <dt-bindings/gpio/gpio.h>
> 
>  / {
> @@ -272,6 +273,56 @@
>  				compatible = "qcom,rpmcc-msm8998", "qcom,rpmcc";
>  				#clock-cells = <1>;
>  			};
> +
> +			rpmpd: power-controller {
> +				compatible = "qcom,msm8998-rpmpd";
> +				#power-domain-cells = <1>;
> +				operating-points-v2 = <&rpmpd_opp_table>;
> +
> +				rpmpd_opp_table: opp-table {
> +					compatible = "operating-points-v2";
> +
> +					rpmpd_opp_ret: opp1 {
> +						opp-level = <RPM_SMD_LEVEL_RETENTION>;
> +					};
> +
> +					rpmpd_opp_ret_plus: opp2 {
> +						opp-level = <RPM_SMD_LEVEL_RETENTION_PLUS>;
> +					};
> +
> +					rpmpd_opp_min_svs: opp3 {
> +						opp-level = <RPM_SMD_LEVEL_MIN_SVS>;
> +					};
> +
> +					rpmpd_opp_low_svs: opp4 {
> +						opp-level = <RPM_SMD_LEVEL_LOW_SVS>;
> +					};
> +


typo in opp5 - opp10 rpmhpd/rpmpd
will fix it in the next re-spin

> +					rpmhpd_opp_svs: opp5 {
> +						opp-level = <RPM_SMD_LEVEL_SVS>;
> +					};
> +
> +					rpmhpd_opp_svs_plus: opp6 {
> +						opp-level = <RPM_SMD_LEVEL_SVS_PLUS>;
> +					};
> +
> +					rpmhpd_opp_nom: opp7 {
> +						opp-level = <RPM_SMD_LEVEL_NOM>;
> +					};
> +
> +					rpmhpd_opp_nom_plus: opp8 {
> +						opp-level = <RPM_SMD_LEVEL_NOM_PLUS>;
> +					};
> +
> +					rpmhpd_opp_turbo: opp9 {
> +						opp-level = <RPM_SMD_LEVEL_TURBO>;
> +					};
> +
> +					rpmhpd_opp_turbo_plus: opp10 {
> +						opp-level = <RPM_SMD_LEVEL_BINNING>;
> +					};
> +				};
> +			};
>  		};
>  	};

-- 
-- Sibi Sankar --
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH v3 8/9] soc: qcom: rpmpd: Add MSM8998 power-domains

[Sibi Sankar]

On 2019-04-05 20:38, Marc Gonzalez wrote:
> On 27/03/2019 13:38, Sibi Sankar wrote:
> 
>> Add the shared cx/mx and sensor sub-system's cx and mx
>> power-domains found on MSM8998.
>> 
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>>  drivers/soc/qcom/rpmpd.c | 36 ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 36 insertions(+)
>> 
>> diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
>> index 238a9e02e890..706a3f63038e 100644
>> --- a/drivers/soc/qcom/rpmpd.c
>> +++ b/drivers/soc/qcom/rpmpd.c
>> @@ -19,9 +19,12 @@
>>  /* Resource types */
>>  #define RPMPD_SMPA 0x61706d73 /* smpa */
>>  #define RPMPD_LDOA 0x616f646c /* ldoa */
>> +#define RPMPD_RWCX 0x78637772 /* rwcx */
>>  #define RPMPD_RWMX 0x786d7772 /* rwmx */
>>  #define RPMPD_RWLC 0x636c7772 /* rwlc */
>>  #define RPMPD_RWLM 0x6d6c7772 /* rwlm */
>> +#define RPMPD_RWSC 0x63737772 /* rwsc */
>> +#define RPMPD_RWSM 0x6d737772 /* rwsm */
> 
> I do not see any value in the comments. Maybe remove them?

comments were included to add value
though, however I guess the comments
were definitely not clear enough.
The magic values for the resources
are calculated as follows:

ascii to hex in reverse order
eg: smpa -> 0x61706d73

0x61 0x70 0x6d 0x73
   a    p    m    s

> 
> I will take a closer look at patches 7-9 on Monday.

Thanks for taking time to review
this series.

> 
> Regards.

-- 
-- Sibi Sankar --
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH v3 8/9] soc: qcom: rpmpd: Add MSM8998 power-domains

[Marc Gonzalez]

On 08/04/2019 10:30, Sibi Sankar wrote:
> On 2019-04-05 20:38, Marc Gonzalez wrote:
>> On 27/03/2019 13:38, Sibi Sankar wrote:
>>
>>> Add the shared cx/mx and sensor sub-system's cx and mx
>>> power-domains found on MSM8998.
>>>
>>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>>> ---
>>>  drivers/soc/qcom/rpmpd.c | 36 ++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 36 insertions(+)
>>>
>>> diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
>>> index 238a9e02e890..706a3f63038e 100644
>>> --- a/drivers/soc/qcom/rpmpd.c
>>> +++ b/drivers/soc/qcom/rpmpd.c
>>> @@ -19,9 +19,12 @@
>>>  /* Resource types */
>>>  #define RPMPD_SMPA 0x61706d73 /* smpa */
>>>  #define RPMPD_LDOA 0x616f646c /* ldoa */
>>> +#define RPMPD_RWCX 0x78637772 /* rwcx */
>>>  #define RPMPD_RWMX 0x786d7772 /* rwmx */
>>>  #define RPMPD_RWLC 0x636c7772 /* rwlc */
>>>  #define RPMPD_RWLM 0x6d6c7772 /* rwlm */
>>> +#define RPMPD_RWSC 0x63737772 /* rwsc */
>>> +#define RPMPD_RWSM 0x6d737772 /* rwsm */
>>
>> I do not see any value in the comments. Maybe remove them?
> 
> comments were included to add value
> though, however I guess the comments
> were definitely not clear enough.
> The magic values for the resources
> are calculated as follows:
> 
> ascii to hex in reverse order
> eg: smpa -> 0x61706d73
> 
> 0x61 0x70 0x6d 0x73
>    a    p    m    s

Ah... I see now.

I agree that explaining *why* e.g. RPMPD_SMPA is defined as 0x61706d73
is worthwhile indeed.

What I meant is that adding /* smpa */ to a macro named RPMPD_SMPA does
not really bring any new information ;-)

How about prefixing the whole block with a small blurb, for example

/* The value of RPMPD_X is X encoded as a little-endian, lower-case, ASCII string */

Regards.

Re: [PATCH v3 8/9] soc: qcom: rpmpd: Add MSM8998 power-domains

[Sibi Sankar]

Hey Marc,
Thanks for the review!

On 2019-04-08 14:24, Marc Gonzalez wrote:
> On 08/04/2019 10:30, Sibi Sankar wrote:
>> On 2019-04-05 20:38, Marc Gonzalez wrote:
>>> On 27/03/2019 13:38, Sibi Sankar wrote:
>>> 
>>>> Add the shared cx/mx and sensor sub-system's cx and mx
>>>> power-domains found on MSM8998.
>>>> 
>>>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>>>> ---
>>>>  drivers/soc/qcom/rpmpd.c | 36 ++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 36 insertions(+)
>>>> 
>>>> diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
>>>> index 238a9e02e890..706a3f63038e 100644
>>>> --- a/drivers/soc/qcom/rpmpd.c
>>>> +++ b/drivers/soc/qcom/rpmpd.c
>>>> @@ -19,9 +19,12 @@
>>>>  /* Resource types */
>>>>  #define RPMPD_SMPA 0x61706d73 /* smpa */
>>>>  #define RPMPD_LDOA 0x616f646c /* ldoa */
>>>> +#define RPMPD_RWCX 0x78637772 /* rwcx */
>>>>  #define RPMPD_RWMX 0x786d7772 /* rwmx */
>>>>  #define RPMPD_RWLC 0x636c7772 /* rwlc */
>>>>  #define RPMPD_RWLM 0x6d6c7772 /* rwlm */
>>>> +#define RPMPD_RWSC 0x63737772 /* rwsc */
>>>> +#define RPMPD_RWSM 0x6d737772 /* rwsm */
>>> 
>>> I do not see any value in the comments. Maybe remove them?
>> 
>> comments were included to add value
>> though, however I guess the comments
>> were definitely not clear enough.
>> The magic values for the resources
>> are calculated as follows:
>> 
>> ascii to hex in reverse order
>> eg: smpa -> 0x61706d73
>> 
>> 0x61 0x70 0x6d 0x73
>>    a    p    m    s
> 
> Ah... I see now.
> 
> I agree that explaining *why* e.g. RPMPD_SMPA is defined as 0x61706d73
> is worthwhile indeed.
> 
> What I meant is that adding /* smpa */ to a macro named RPMPD_SMPA does
> not really bring any new information ;-)

yeah I got that

> 
> How about prefixing the whole block with a small blurb, for example
> 
> /* The value of RPMPD_X is X encoded as a little-endian, lower-case,
> ASCII string */

sure will add this ^^ instead in the next
re-spin

> 
> Regards.

-- 
-- Sibi Sankar --
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.