| Message ID | 20220629130303.3288306-5-bryan.odonoghue@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | qcom-cpufreq-nvmem: Add msm8939 with some fixups | expand |
On 29/06/2022 20:11, Stephan Gerhold wrote: > On Wed, Jun 29, 2022 at 02:03:02PM +0100, Bryan O'Donoghue wrote: >> At the moment the CPR genpd based code is named after the qcs404 however >> msm8936, msm8939 and other antecedent processors of the qcs404 can also >> make use of this data. >> >> Rename it to reflect a more generic use. >> >> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > > There is another power domain that needs to be scaled together with the > CPU frequency on MSM8916 and MSM8939: (VDD)MX. How do you handle that? > Short answer, in another series to enable CPR on 5.x We have code for CPR in a 4.19 tree that works but, it needs more work to be upstream-fit on 5.x. CPR is deliberately omitted here to be submitted later. In this series I'm just switching away from the default cpufreq-dt-platdev which breaks booting to qcom-cpufreq-nvmem. Fair enough ? --- bod
On Thu, Jun 30, 2022 at 05:05:59AM +0100, Bryan O'Donoghue wrote: > On 29/06/2022 20:11, Stephan Gerhold wrote: > > On Wed, Jun 29, 2022 at 02:03:02PM +0100, Bryan O'Donoghue wrote: > > > At the moment the CPR genpd based code is named after the qcs404 however > > > msm8936, msm8939 and other antecedent processors of the qcs404 can also > > > make use of this data. > > > > > > Rename it to reflect a more generic use. > > > > > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > > > > There is another power domain that needs to be scaled together with the > > CPU frequency on MSM8916 and MSM8939: (VDD)MX. How do you handle that? > > > > Short answer, in another series to enable CPR on 5.x > > We have code for CPR in a 4.19 tree that works but, it needs more work to be > upstream-fit on 5.x. > > CPR is deliberately omitted here to be submitted later. > I agree with this decision (doing CPR properly is way too complicated to block the entire msm8939.dtsi with this). > In this series I'm just switching away from the default cpufreq-dt-platdev > which breaks booting to qcom-cpufreq-nvmem. > > Fair enough ? ... but then I don't understand: Why do you need this patch set? You only need to attach the "cpr" power domain in qcom-cpufreq-nvmem if you're actually using CPR. I would recommend adding MSM8939 in a similar way to MSM8916, so without using qcom-cpufreq-nvmem for now. Then we can add CPR on both platforms in a similar way later. Thanks, Stephan
On 29/06/2022 16:03, Bryan O'Donoghue wrote: > At the moment the CPR genpd based code is named after the qcs404 however > msm8936, msm8939 and other antecedent processors of the qcs404 can also > make use of this data. > > Rename it to reflect a more generic use. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > drivers/cpufreq/qcom-cpufreq-nvmem.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c > index 6dfa86971a757..355c8b99e974a 100644 > --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c > +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c > @@ -252,10 +252,10 @@ static const struct qcom_cpufreq_match_data match_data_krait = { > .get_version = qcom_cpufreq_krait_name_version, > }; > > -static const char *qcs404_genpd_names[] = { "cpr", NULL }; > +static const char *cpr_genpd_names[] = { "cpr", NULL }; As a generic comment, as you are touching this piece of code, code you please move cpr_genpd_names above match_data_kryo? So that all match_data instances can use it. > > -static const struct qcom_cpufreq_match_data match_data_qcs404 = { > - .genpd_names = qcs404_genpd_names, > +static const struct qcom_cpufreq_match_data match_data_cpr_genpd = { > + .genpd_names = cpr_genpd_names, > }; > > static int qcom_cpufreq_probe(struct platform_device *pdev) > @@ -454,7 +454,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,msm8996", .data = &match_data_kryo }, > - { .compatible = "qcom,qcs404", .data = &match_data_qcs404 }, > + { .compatible = "qcom,qcs404", .data = &match_data_cpr_genpd }, > { .compatible = "qcom,ipq8064", .data = &match_data_krait }, > { .compatible = "qcom,apq8064", .data = &match_data_krait }, > { .compatible = "qcom,msm8974", .data = &match_data_krait },
On 13/07/2022 14:50, Dmitry Baryshkov wrote: > On 29/06/2022 16:03, Bryan O'Donoghue wrote: >> At the moment the CPR genpd based code is named after the qcs404 however >> msm8936, msm8939 and other antecedent processors of the qcs404 can also >> make use of this data. >> >> Rename it to reflect a more generic use. >> >> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >> --- >> drivers/cpufreq/qcom-cpufreq-nvmem.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c >> b/drivers/cpufreq/qcom-cpufreq-nvmem.c >> index 6dfa86971a757..355c8b99e974a 100644 >> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c >> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c >> @@ -252,10 +252,10 @@ static const struct qcom_cpufreq_match_data >> match_data_krait = { >> .get_version = qcom_cpufreq_krait_name_version, >> }; >> -static const char *qcs404_genpd_names[] = { "cpr", NULL }; >> +static const char *cpr_genpd_names[] = { "cpr", NULL }; > > As a generic comment, as you are touching this piece of code, code you > please move cpr_genpd_names above match_data_kryo? So that all > match_data instances can use it. NP. This has been dropped in V3 per Stephan's preference to not touch anything CPR related until doing the whole thing for 8939. --- bod
diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c index 6dfa86971a757..355c8b99e974a 100644 --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c @@ -252,10 +252,10 @@ static const struct qcom_cpufreq_match_data match_data_krait = { .get_version = qcom_cpufreq_krait_name_version, }; -static const char *qcs404_genpd_names[] = { "cpr", NULL }; +static const char *cpr_genpd_names[] = { "cpr", NULL }; -static const struct qcom_cpufreq_match_data match_data_qcs404 = { - .genpd_names = qcs404_genpd_names, +static const struct qcom_cpufreq_match_data match_data_cpr_genpd = { + .genpd_names = cpr_genpd_names, }; static int qcom_cpufreq_probe(struct platform_device *pdev) @@ -454,7 +454,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,msm8996", .data = &match_data_kryo }, - { .compatible = "qcom,qcs404", .data = &match_data_qcs404 }, + { .compatible = "qcom,qcs404", .data = &match_data_cpr_genpd }, { .compatible = "qcom,ipq8064", .data = &match_data_krait }, { .compatible = "qcom,apq8064", .data = &match_data_krait }, { .compatible = "qcom,msm8974", .data = &match_data_krait },
At the moment the CPR genpd based code is named after the qcs404 however msm8936, msm8939 and other antecedent processors of the qcs404 can also make use of this data. Rename it to reflect a more generic use. Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- drivers/cpufreq/qcom-cpufreq-nvmem.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)