Re: [PATCH] arch: arm64: dts: apq8016-dbc: Add missing cpu opps

[Stephan Gerhold <stephan@gerhold.net>]

On Fri, May 08, 2020 at 02:08:09PM +0200, Ulf Hansson wrote:
> + Lina
> 
> On Thu, 7 May 2020 at 07:33, Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
> >
> > On Wed 06 May 14:18 PDT 2020, Stephan Gerhold wrote:
> >
> > Viresh, Ulf,
> >
> > Stephan is trying to describe the relationship between the CPU rail and
> > the memory rail on db410c (where the performance state of the memory
> > rail needs to be kept above the performance state of the CPU supply.
> >
> > The latter is modelled as a power-domain and the performance state
> > changes as expected, but no one enables the power-domain.
> 
> Just to make one thing clear, from a genpd framework point of view,
> power on/off of a genpd is orthogonal to setting/aggregating
> performance states for it.
> 
> It's instead up to the genpd provider to deal with this (as I
> understand, that seems to be the issue from the below discussions).
> 
> >
> > What's the appropriate method for ensuring the power-domain is
> > enabled/disabled as needed? Should it be referenced in the hierarchical
> > power domain for the CPUs perhaps?
> 
> If I understand the dependency correctly, perhaps you are right that
> there needs to be a subdomain assigned. Although, I don't know if this
> ever has been tested to work for a real use case, when it comes to
> performance state propagations upwards in the hierarchy.
> 

What I don't understand is why this is only happening for me,
and not also for CPR on QCS404 (or does that have the same problem?).
See commit 04aadcaadd39 ("arm64: dts: qcom: qcs404: Add CPR and populate OPP table").

Essentially that works exactly like the "mx" power domain I'm trying to add,
it references the "cpr" power domain performance states from the CPU opps.
Someone also needs to power on the CPR power domain, and AFAICT there is
nothing that does that at the moment.

> Viresh?
> 
> If you also need to manage genpd power on/off, that should be managed
> by using runtime PM reference counting on those devices that are
> attached to the genpd in question.
> 

For testing I have added a pm_runtime_get_sync() for the virtual devices
returned by dev_pm_opp_attach_genpd() in qcom-cpufreq-nvmem.c.

That works fine, but it means the power domain is powered on
permanently. (Which might be the correct behavior in this case,
not sure powering off VDD_MX would ever work correctly...)

> Kind regards
> Uffe
> 
> 
> >
> > Regards,
> > Bjorn
> >
> > > Hi,
> > >
> > > On Sun, Apr 26, 2020 at 02:31:48PM +0200, Stephan Gerhold wrote:
> > > > On Wed, Apr 22, 2020 at 09:55:06PM -0700, Bjorn Andersson wrote:
> > > > > On Fri 03 Apr 11:00 PDT 2020, Stephan Gerhold wrote:
> > > > >
> > > > > > On Fri, Apr 03, 2020 at 12:09:25PM +0200, Stephan Gerhold wrote:
> > > > > > > On Thu, Apr 02, 2020 at 06:31:19PM -0700, Bjorn Andersson wrote:
> > > > > > > > On Thu 02 Apr 01:13 PDT 2020, Stephan Gerhold wrote:
> > > > > > > >
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > On Wed, Apr 01, 2020 at 07:50:59PM +0200, Loic Poulain wrote:
> > > > > > > > > > The highest cpu frequency opps have been dropped because CPR is not
> > > > > > > > > > supported. However, we can simply specify operating voltage so that
> > > > > > > > > > they match the max corner voltages for each freq. With that, we can
> > > > > > > > > > support up to 1.36Ghz. Ideally, msm8916 CPR should be implemented to
> > > > > > > > > > fine tune operating voltages and optimize power consumption.
> > > > > > > > >
> > > > > > > > > Thanks for the patch! I was wondering how to enable the higher CPU
> > > > > > > > > frequencies for a while now...
> > > > > > > > >
> > > > > > > > > I was actually quite excited to see CPR being mainlined for QCS404.
> > > > > > > > > If we are trying to add such a workaround (rather than CPR) for MSM8916
> > > > > > > > > now, does that mean it's unlikely to see CPR working for MSM8916
> > > > > > > > > anytime soon?
> > > > > > > > >
> > > > > > > > > AFAICT, there is a WIP branch from Niklas Cassel here:
> > > > > > > > > https://git.linaro.org/people/nicolas.dechesne/niklas.cassel/kernel.git/log/?h=cpr-msm8916-mainline
> > > > > > > > > but it hasn't been updated for a while. (Not sure if it was working
> > > > > > > > > already...)
> > > > > > > > >
> > > > > > > > > Can someone explain what is missing to make CPR work for MSM8916?
> > > > > > > > >
> > > > > > > >
> > > > > > > > CPR needs to control 3 things; VDD_APC, VDD_MX and MEMACC. On QCS404 it
> > > > > > > > seems we don't have to adjust VDD_MX, so the code for this is missing
> > > > > > > > from the driver.
> > > > > > > >
> > > > > > > > So, afaict, what's missing is that rpmpd.c needs to gain support for
> > > > > > > > 8916, then the CPR driver needs to gain a cpr_acc_desc and compatible
> > > > > > > > for 8916, it needs to reference the VDDMX power domain and before/after
> > > > > > > > we're adjusting the corner of the CPR we need to adjust the MX according
> > > > > > > > to the mapping specified in the downstream kernel (i.e.  1->4, 2->5 and
> > > > > > > > 3->7).
> > > > > > > >
> > > > > > > >
> > > > > > > > Unfortunately, the requirement that VDDMX (VDD_MEM I presume) must be
> > > > > > > > higher than VDD_APC most likely needs to be taken into consideration for
> > > > > > > > Loic's proposed static voltage scaling as well. Unless VDD_MEM is left
> > > > > > > > in Turbo mode from the boot loader I think we need to take VDDMX to
> > > > > > > > corner 7 for speeds 998MHz and above (i.e. where we pull VDD_APC to
> > > > > > > > 1.35V).
> > > > > > > >
> > > > > > >
> > > > > > > I see! I wonder how hard it would be to add MSM8916 to rpmpd,
> > > > > > > looking at previous commits it's mainly setting up a few defines?
> > > > > > >
> > > > > > > If I understand it correctly, the OPPs from rpmpd could then be
> > > > > > > referenced as "required-opps" in the CPU OPP table so that VDD_MX is
> > > > > > > scaled together with the CPU frequency, and doesn't need to stay at
> > > > > > > turbo mode (like in v3 from Loic) the whole time.
> > > > > > >
> > > > > >
> > > > > > I have been thinking about this some more and I think I came up with
> > > > > > some changes that make sense (but not entirely sure).
> > > > > >
> > > > > > Based on the available downstream sources I guessed the defines to add
> > > > > > for MSM8916 to the rpmpd driver. Then I added the VDD_MX OPPs as
> > > > > > "required-opps" to the CPU OPP table so it would vote for the appopriate
> > > > > > corners (with the mapping you mentioned above).
> > > > > >
> > > > >
> > > > > I was not aware it was possible to describe the dependency between the
> > > > > CPU opp table and MX in this fashion. If that's the case then this looks
> > > > > really good and it should be straight forward to add MSM8916 support to
> > > > > the CPR driver as well.
> > > > >
> > > >
> > > > Indeed!
> > > >
> > > > > > I haven't tested it yet, maybe I can get some feedback first if the code
> > > > > > seems reasonable or if I'm missing something obvious? :)
> > > > > >
> > > > >
> > > > > Have you tested this yet?
> > > > >
> > > >
> > > > I just did. It does not fully work, yet:
> > > >
> > > > rpmpd_set_performance() is indeed called as necessary when switching the
> > > > CPU frequency. When I set 200 MHz it sets corner 3, with 533 MHz corner 4
> > > > and starting with 998 MHz corner 6. So far so good :)
> > > >
> > > > However, there is never actually anything sent to the RPM. :(
> > > > It bails out in rpmpd_set_performance() before calling rpmpd_aggregate_corner():
> > > >
> > > >     /* Always send updates for vfc and vfl */
> > > >     if (!pd->enabled && pd->key != KEY_FLOOR_CORNER &&
> > > >         pd->key != KEY_FLOOR_LEVEL)
> > > >             goto out;
> > > >
> > > > Seems like we just try to set performance states, but never actually
> > > > enable (rpmpd_power_on()) the power domain (pd->enabled == false).
> > > >
> > > > I'm not sure which of the components involved here should handle that.
> > > > The OPP core when setting required OPPs, the genpd core etc.
> > > >
> > > > Any ideas?
> > > >
> > > > Thanks,
> > > > Stephan
> > > >
> > >
> > > For now I have added something to qcom-cpufreq-nvmem.c to power on/enable
> > > the power domain (not really sure if it belongs there...):
> > >
> > > diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> > > index a1b8238872a2..ed352ead037e 100644
> > > --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> > > +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> > > @@ -26,6 +26,7 @@
> > >  #include <linux/platform_device.h>
> > >  #include <linux/pm_domain.h>
> > >  #include <linux/pm_opp.h>
> > > +#include <linux/pm_runtime.h>
> > >  #include <linux/slab.h>
> > >  #include <linux/soc/qcom/smem.h>
> > >
> > > @@ -370,10 +371,13 @@ static int qcom_cpufreq_probe(struct platform_device *pdev)
> > >               }
> > >
> > >               if (drv->data->genpd_names) {
> > > +                     struct device **pd_dev;
> > > +                     const char **name = drv->data->genpd_names;
> > > +
> > >                       drv->genpd_opp_tables[cpu] =
> > >                               dev_pm_opp_attach_genpd(cpu_dev,
> > >                                                       drv->data->genpd_names,
> > > -                                                     NULL);
> > > +                                                     &pd_dev);
> > >                       if (IS_ERR(drv->genpd_opp_tables[cpu])) {
> > >                               ret = PTR_ERR(drv->genpd_opp_tables[cpu]);
> > >                               if (ret != -EPROBE_DEFER)
> > > @@ -382,6 +386,12 @@ static int qcom_cpufreq_probe(struct platform_device *pdev)
> > >                                               ret);
> > >                               goto free_genpd_opp;
> > >                       }
> > > +
> > > +                     while (*name) {
> > > +                             pm_runtime_get_sync(*pd_dev);
> > > +                             name++;
> > > +                             pd_dev++;
> > > +                     }
> > >               }
> > >       }
> > >
> > > I really wonder why this doesn't affect CPR (or does it?).
> > > So far I was not able to find anything that would power on/enable
> > > the "cpr" power domain either. But I don't have a qcs404 to verify.
> > >
> > > As far as I can tell from the log, the corner votes are correctly sent
> > > to the RPM now when the CPU frequency is changed.
> > >
> > > However...
> > >
> > > > > > Also: Is there a good way to validate these changes?
> > > > > > I suppose I could check the genpd state but that wouldn't tell me if the
> > > > > > corner was applied correctly. Maybe I can check the actual voltage
> > > > > > through the SPMI interface, hm...
> > > > > >
> > > > >
> > > > > Validating that S2 and VDD_MX changes appropriately in Linux would be a
> > > > > pretty good test.
> > > > >
> > >
> > > Unfortunately I was not able to see any change in the voltage of L3 yet.
> > > On samsung-a5u, /sys/class/regulator/<l3 spmi regulator>/microvolts
> > > permanently reports 1300000 uV, even after a few different corner votes.
> > >
> > > I'm not sure if:
> > >
> > >   - This is normal (maybe some other remoteproc has a higher vote?)
> > >     - I tried to disable wcnss, venus and hexagon without difference
> > >
> > >   - I'm just missing something in the code
> > >
> > >   - This is some peculiarity of the RPM firmware on samsung-a5u.
> > >     (Although that seems quite unlikely to me...)
> > >
> > > Any ideas? :/
> > >
> > > Thanks,
> > > Stephan
> > >
> > > > > > If this seems like a good approach I can split up the changes in
> > > > > > reasonable patches and post it separately. For now the full diff below.
> > > > > >
> > > > >
> > > > > Please do
> > > > >
> > > > > Regards,
> > > > > Bjorn
> > > > >
> > > > > > Stephan
> > > > > >
> > > > > >  arch/arm64/boot/dts/qcom/msm8916.dtsi  | 65 +++++++++++++++++++++++++++++-----
> > > > > >  drivers/cpufreq/cpufreq-dt-platdev.c   |  1 +
> > > > > >  drivers/cpufreq/qcom-cpufreq-nvmem.c   | 11 ++++++
> > > > > >  drivers/soc/qcom/rpmpd.c               | 21 +++++++++++
> > > > > >  include/dt-bindings/power/qcom-rpmpd.h |  7 ++++
> > > > > >  5 files changed, 96 insertions(+), 9 deletions(-)
> > > > > >
> > > > > > diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> > > > > > index b0a82447976a..5b8fce8609d0 100644
> > > > > > --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> > > > > > +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> > > > > > @@ -10,6 +10,7 @@
> > > > > >  #include <dt-bindings/soc/qcom,apr.h>
> > > > > >  #include <dt-bindings/sound/qcom,q6afe.h>
> > > > > >  #include <dt-bindings/thermal/thermal.h>
> > > > > > +#include <dt-bindings/power/qcom-rpmpd.h>
> > > > > >
> > > > > >  / {
> > > > > >         interrupt-parent = <&intc>;
> > > > > > @@ -108,8 +109,8 @@ CPU0: cpu@0 {
> > > > > >                         cpu-supply = <&pm8916_spmi_s2>;
> > > > > >                         operating-points-v2 = <&cpu_opp_table>;
> > > > > >                         #cooling-cells = <2>;
> > > > > > -                       power-domains = <&CPU_PD0>;
> > > > > > -                       power-domain-names = "psci";
> > > > > > +                       power-domains = <&CPU_PD0>, <&rpmpd MSM8916_VDDMX_AO>;
> > > > > > +                       power-domain-names = "psci", "mx";
> > > > > >                 };
> > > > > >
> > > > > >                 CPU1: cpu@1 {
> > > > > > @@ -122,8 +123,8 @@ CPU1: cpu@1 {
> > > > > >                         cpu-supply = <&pm8916_spmi_s2>;
> > > > > >                         operating-points-v2 = <&cpu_opp_table>;
> > > > > >                         #cooling-cells = <2>;
> > > > > > -                       power-domains = <&CPU_PD1>;
> > > > > > -                       power-domain-names = "psci";
> > > > > > +                       power-domains = <&CPU_PD1>, <&rpmpd MSM8916_VDDMX_AO>;
> > > > > > +                       power-domain-names = "psci", "mx";
> > > > > >                 };
> > > > > >
> > > > > >                 CPU2: cpu@2 {
> > > > > > @@ -136,8 +137,8 @@ CPU2: cpu@2 {
> > > > > >                         cpu-supply = <&pm8916_spmi_s2>;
> > > > > >                         operating-points-v2 = <&cpu_opp_table>;
> > > > > >                         #cooling-cells = <2>;
> > > > > > -                       power-domains = <&CPU_PD2>;
> > > > > > -                       power-domain-names = "psci";
> > > > > > +                       power-domains = <&CPU_PD2>, <&rpmpd MSM8916_VDDMX_AO>;
> > > > > > +                       power-domain-names = "psci", "mx";
> > > > > >                 };
> > > > > >
> > > > > >                 CPU3: cpu@3 {
> > > > > > @@ -150,8 +151,8 @@ CPU3: cpu@3 {
> > > > > >                         cpu-supply = <&pm8916_spmi_s2>;
> > > > > >                         operating-points-v2 = <&cpu_opp_table>;
> > > > > >                         #cooling-cells = <2>;
> > > > > > -                       power-domains = <&CPU_PD3>;
> > > > > > -                       power-domain-names = "psci";
> > > > > > +                       power-domains = <&CPU_PD3>, <&rpmpd MSM8916_VDDMX_AO>;
> > > > > > +                       power-domain-names = "psci", "mx";
> > > > > >                 };
> > > > > >
> > > > > >                 L2_0: l2-cache {
> > > > > > @@ -343,40 +344,52 @@ modem_alert0: trip-point@0 {
> > > > > >         };
> > > > > >
> > > > > >         cpu_opp_table: cpu_opp_table {
> > > > > > -               compatible = "operating-points-v2";
> > > > > > +               /*
> > > > > > +                * FIXME: The naming here is really weird, since MSM8916 does
> > > > > > +                * not have kyro. Maybe we should add a more generic compatible?
> > > > > > +                */
> > > > > > +               compatible = "operating-points-v2-kryo-cpu";
> > > > > >                 opp-shared;
> > > > > >
> > > > > >                 opp-200000000 {
> > > > > >                         opp-hz = /bits/ 64 <200000000>;
> > > > > >                         opp-microvolt = <1050000>;
> > > > > > +                       required-opps = <&rpmpd_opp_svs_soc>;
> > > > > >                 };
> > > > > >                 opp-400000000 {
> > > > > >                         opp-hz = /bits/ 64 <400000000>;
> > > > > >                         opp-microvolt = <1050000>;
> > > > > > +                       required-opps = <&rpmpd_opp_svs_soc>;
> > > > > >                 };
> > > > > >                 opp-533330000 {
> > > > > >                         opp-hz = /bits/ 64 <533330000>;
> > > > > >                         opp-microvolt = <1150000>;
> > > > > > +                       required-opps = <&rpmpd_opp_nom>;
> > > > > >                 };
> > > > > >                 opp-800000000 {
> > > > > >                         opp-hz = /bits/ 64 <800000000>;
> > > > > >                         opp-microvolt = <1150000>;
> > > > > > +                       required-opps = <&rpmpd_opp_nom>;
> > > > > >                 };
> > > > > >                 opp-998400000 {
> > > > > >                         opp-hz = /bits/ 64 <998400000>;
> > > > > >                         opp-microvolt = <1350000>;
> > > > > > +                       required-opps = <&rpmpd_opp_super_turbo>;
> > > > > >                 };
> > > > > >                 opp-1094400000 {
> > > > > >                         opp-hz = /bits/ 64 <1094400000>;
> > > > > >                         opp-microvolt = <1350000>;
> > > > > > +                       required-opps = <&rpmpd_opp_super_turbo>;
> > > > > >                 };
> > > > > >                 opp-1152000000 {
> > > > > >                         opp-hz = /bits/ 64 <1152000000>;
> > > > > >                         opp-microvolt = <1350000>;
> > > > > > +                       required-opps = <&rpmpd_opp_super_turbo>;
> > > > > >                 };
> > > > > >                 opp-1209600000 {
> > > > > >                         opp-hz = /bits/ 64 <1209600000>;
> > > > > >                         opp-microvolt = <1350000>;
> > > > > > +                       required-opps = <&rpmpd_opp_super_turbo>;
> > > > > >                 };
> > > > > >         };
> > > > > >
> > > > > > @@ -1710,6 +1723,40 @@ rpmcc: qcom,rpmcc {
> > > > > >                                         #clock-cells = <1>;
> > > > > >                                 };
> > > > > >
> > > > > > +                               rpmpd: power-controller {
> > > > > > +                                       compatible = "qcom,msm8916-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 = <1>;
> > > > > > +                                               };
> > > > > > +
> > > > > > +                                               rpmpd_opp_svs: opp2 {
> > > > > > +                                                       opp-level = <2>;
> > > > > > +                                               };
> > > > > > +
> > > > > > +                                               rpmpd_opp_svs_soc: opp3 {
> > > > > > +                                                       opp-level = <3>;
> > > > > > +                                               };
> > > > > > +
> > > > > > +                                               rpmpd_opp_nom: opp4 {
> > > > > > +                                                       opp-level = <4>;
> > > > > > +                                               };
> > > > > > +
> > > > > > +                                               rpmpd_opp_turbo: opp5 {
> > > > > > +                                                       opp-level = <5>;
> > > > > > +                                               };
> > > > > > +
> > > > > > +                                               rpmpd_opp_super_turbo: opp6 {
> > > > > > +                                                       opp-level = <6>;
> > > > > > +                                               };
> > > > > > +                                       };
> > > > > > +                               };
> > > > > > +
> > > > > >                                 smd_rpm_regulators: pm8916-regulators {
> > > > > >                                         compatible = "qcom,rpm-pm8916-regulators";
> > > > > >
> > > > > > diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
> > > > > > index f2ae9cd455c1..b0f6bd0fffc1 100644
> > > > > > --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> > > > > > +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> > > > > > @@ -128,6 +128,7 @@ static const struct of_device_id blacklist[] __initconst = {
> > > > > >         { .compatible = "nvidia,tegra210", },
> > > > > >
> > > > > >         { .compatible = "qcom,apq8096", },
> > > > > > +       { .compatible = "qcom,msm8916", },
> > > > > >         { .compatible = "qcom,msm8996", },
> > > > > >         { .compatible = "qcom,qcs404", },
> > > > > >
> > > > > > diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> > > > > > index f0d2d5035413..c77a30349d08 100644
> > > > > > --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> > > > > > +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> > > > > > @@ -129,6 +129,16 @@ static const struct qcom_cpufreq_match_data match_data_kryo = {
> > > > > >         .get_version = qcom_cpufreq_kryo_name_version,
> > > > > >  };
> > > > > >
> > > > > > +static const char *msm8916_genpd_names[] = { "mx", NULL };
> > > > > > +
> > > > > > +static const struct qcom_cpufreq_match_data match_data_msm8916 = {
> > > > > > +       /*
> > > > > > +        * FIXME: Might need to implement .get_version here to handle
> > > > > > +        * different frequencies depending on speedbin/pvs version.
> > > > > > +        */
> > > > > > +       .genpd_names = msm8916_genpd_names,
> > > > > > +};
> > > > > > +
> > > > > >  static const char *qcs404_genpd_names[] = { "cpr", NULL };
> > > > > >
> > > > > >  static const struct qcom_cpufreq_match_data match_data_qcs404 = {
> > > > > > @@ -301,6 +311,7 @@ static struct platform_driver qcom_cpufreq_driver = {
> > > > > >
> > > > > >  static const struct of_device_id qcom_cpufreq_match_list[] __initconst = {
> > > > > >         { .compatible = "qcom,apq8096", .data = &match_data_kryo },
> > > > > > +       { .compatible = "qcom,msm8916", .data = &match_data_msm8916 },
> > > > > >         { .compatible = "qcom,msm8996", .data = &match_data_kryo },
> > > > > >         { .compatible = "qcom,qcs404", .data = &match_data_qcs404 },
> > > > > >         {},
> > > > > > diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
> > > > > > index 2b1834c5609a..192ba9099964 100644
> > > > > > --- a/drivers/soc/qcom/rpmpd.c
> > > > > > +++ b/drivers/soc/qcom/rpmpd.c
> > > > > > @@ -115,6 +115,26 @@ struct rpmpd_desc {
> > > > > >
> > > > > >  static DEFINE_MUTEX(rpmpd_lock);
> > > > > >
> > > > > > +/* msm8916 RPM Power Domains */
> > > > > > +DEFINE_RPMPD_PAIR(msm8916, vddcx, vddcx_ao, SMPA, CORNER, 1);
> > > > > > +DEFINE_RPMPD_PAIR(msm8916, vddmx, vddmx_ao, LDOA, CORNER, 3);
> > > > > > +
> > > > > > +DEFINE_RPMPD_VFC(msm8916, vddcx_vfc, SMPA, 1);
> > > > > > +
> > > > > > +static struct rpmpd *msm8916_rpmpds[] = {
> > > > > > +       [MSM8916_VDDCX] =       &msm8916_vddcx,
> > > > > > +       [MSM8916_VDDCX_AO] =    &msm8916_vddcx_ao,
> > > > > > +       [MSM8916_VDDCX_VFC] =   &msm8916_vddcx_vfc,
> > > > > > +       [MSM8916_VDDMX] =       &msm8916_vddmx,
> > > > > > +       [MSM8916_VDDMX_AO] =    &msm8916_vddmx_ao,
> > > > > > +};
> > > > > > +
> > > > > > +static const struct rpmpd_desc msm8916_desc = {
> > > > > > +       .rpmpds = msm8916_rpmpds,
> > > > > > +       .num_pds = ARRAY_SIZE(msm8916_rpmpds),
> > > > > > +       .max_state = MAX_8996_RPMPD_STATE,
> > > > > > +};
> > > > > > +
> > > > > >  /* msm8976 RPM Power Domains */
> > > > > >  DEFINE_RPMPD_PAIR(msm8976, vddcx, vddcx_ao, SMPA, LEVEL, 2);
> > > > > >  DEFINE_RPMPD_PAIR(msm8976, vddmx, vddmx_ao, SMPA, LEVEL, 6);
> > > > > > @@ -220,6 +240,7 @@ static const struct rpmpd_desc qcs404_desc = {
> > > > > >  };
> > > > > >
> > > > > >  static const struct of_device_id rpmpd_match_table[] = {
> > > > > > +       { .compatible = "qcom,msm8916-rpmpd", .data = &msm8916_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 3f74096d5a7c..70d304a2deae 100644
> > > > > > --- a/include/dt-bindings/power/qcom-rpmpd.h
> > > > > > +++ b/include/dt-bindings/power/qcom-rpmpd.h
> > > > > > @@ -51,6 +51,13 @@
> > > > > >  #define RPMH_REGULATOR_LEVEL_TURBO     384
> > > > > >  #define RPMH_REGULATOR_LEVEL_TURBO_L1  416
> > > > > >
> > > > > > +/* MSM8916 Power Domain Indexes */
> > > > > > +#define MSM8916_VDDCX          0
> > > > > > +#define MSM8916_VDDCX_AO       1
> > > > > > +#define MSM8916_VDDCX_VFC      2
> > > > > > +#define MSM8916_VDDMX          3
> > > > > > +#define MSM8916_VDDMX_AO       4
> > > > > > +
> > > > > >  /* MSM8976 Power Domain Indexes */
> > > > > >  #define MSM8976_VDDCX          0
> > > > > >  #define MSM8976_VDDCX_AO       1