Re: [PATCH v3 8/9] soc: qcom: rpmpd: Add MSM8998 power-domains
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 --- 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 apms 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.
Re: [PATCH v3 8/9] soc: qcom: rpmpd: Add MSM8998 power-domains
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 >>> --- >>> 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 >apms 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
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 --- 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 apms 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
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 > --- > 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.
[PATCH v3 8/9] soc: qcom: rpmpd: Add MSM8998 power-domains
Add the shared cx/mx and sensor sub-system's cx and mx
power-domains found on MSM8998.
Signed-off-by: Sibi Sankar
---
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] = _vddcx,
+ [MSM8998_VDDCX_AO] =_vddcx_ao,
+ [MSM8998_VDDCX_VFL] = _vddcx_vfl,
+ [MSM8998_VDDMX] = _vddmx,
+ [MSM8998_VDDMX_AO] =_vddmx_ao,
+ [MSM8998_VDDMX_VFL] = _vddmx_vfl,
+ [MSM8998_SSCCX] = _vdd_ssccx,
+ [MSM8998_SSCCX_VFL] = _vdd_ssccx_vfl,
+ [MSM8998_SSCMX] = _vdd_sscmx,
+ [MSM8998_SSCMX_VFL] = _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 = _desc },
+ { .compatible = "qcom,msm8998-rpmpd", .data = _desc },
{ .compatible = "qcom,qcs404-rpmpd", .data = _desc },
{ }
};
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
