| Message ID | 20230217-topic-cpr3h-v10-0-67aed8fdfa61@linaro.org |
|---|---|
| Headers | show |
| Series | Add support for Core Power Reduction v3, v4 and Hardened | expand |
On Fri, Feb 17, 2023 at 12:08:25PM +0100, Konrad Dybcio wrote: > CPR3 and newer can be fed per-OPP voltage adjustment values for both > open- and closed-loop paths to make better decisions about settling > on the final voltage offset target. Document these properties. > > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > --- > .../devicetree/bindings/opp/opp-v2-qcom-level.yaml | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml b/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml > index a30ef93213c0..93cc88434dfe 100644 > --- a/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml > +++ b/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml > @@ -34,6 +34,20 @@ patternProperties: > minItems: 1 > maxItems: 2 > > + qcom,opp-cloop-vadj: > + description: | > + A value representing the closed-loop voltage adjustment value A value? > + associated with this OPP node. > + $ref: /schemas/types.yaml#/definitions/int32-array > + maxItems: 2 Or 2 values? > + > + qcom,opp-oloop-vadj: > + description: | > + A value representing the open-loop voltage adjustment value > + associated with this OPP node. > + $ref: /schemas/types.yaml#/definitions/int32-array > + maxItems: 2 > + > required: > - opp-level > - qcom,opp-fuse-level > > -- > 2.39.1 >
Il 18/02/23 01:26, Konrad Dybcio ha scritto: > > > On 18.02.2023 00:13, Rob Herring wrote: >> On Fri, Feb 17, 2023 at 12:08:25PM +0100, Konrad Dybcio wrote: >>> CPR3 and newer can be fed per-OPP voltage adjustment values for both >>> open- and closed-loop paths to make better decisions about settling >>> on the final voltage offset target. Document these properties. >>> >>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >>> --- >>> .../devicetree/bindings/opp/opp-v2-qcom-level.yaml | 14 ++++++++++++++ >>> 1 file changed, 14 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml b/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml >>> index a30ef93213c0..93cc88434dfe 100644 >>> --- a/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml >>> +++ b/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml >>> @@ -34,6 +34,20 @@ patternProperties: >>> minItems: 1 >>> maxItems: 2 >>> >>> + qcom,opp-cloop-vadj: >>> + description: | >>> + A value representing the closed-loop voltage adjustment value >> >> A value? >> >>> + associated with this OPP node. >>> + $ref: /schemas/types.yaml#/definitions/int32-array >>> + maxItems: 2 >> >> Or 2 values? > Right, this description doesn't make any sense if you're just > looking at the documentation without looking at the driver.. > > Generally, each CPR3 instance can have multiple "threads" > (each one of which regulates voltage for some on-SoC IP or > part of it). The nth entry in the qcom,opp-[co]loop-vadj > array corresponds to a voltage offset for the nth thread. > > If the nth entry in the array is missing, the driver assumes > the arr[0] one is "global" to this CPR3 instance at this OPP > level and applies it to all threads. ...and looking at it > again, this is sorta just bad design, especially if you > take into account that there's no known user of CPR3 that > employs more than 2 threads. > > I'll remove that from the driver and make the description clearer. > description: Represents the closed-loop voltage adjustment associated with this OPP node. P.S.: Drop '|' here and on oloop! This binding is intended to support either single or multiple CPR threads; the driver's behavior is unimportant as bindings describe the hardware, not the driver. Regards, Angelo > > Also, only noticed now.. "qcom,sdm630-cprh" was not documented, > so that's to be fixed for the next submission as well! > > > Konrad >> >>> + >>> + qcom,opp-oloop-vadj: >>> + description: | >>> + A value representing the open-loop voltage adjustment value >>> + associated with this OPP node. >>> + $ref: /schemas/types.yaml#/definitions/int32-array >>> + maxItems: 2 >>> + >>> required: >>> - opp-level >>> - qcom,opp-fuse-level >>> >>> -- >>> 2.39.1 >>>
On 20.02.2023 12:27, AngeloGioacchino Del Regno wrote: > Il 18/02/23 01:26, Konrad Dybcio ha scritto: >> >> >> On 18.02.2023 00:13, Rob Herring wrote: >>> On Fri, Feb 17, 2023 at 12:08:25PM +0100, Konrad Dybcio wrote: >>>> CPR3 and newer can be fed per-OPP voltage adjustment values for both >>>> open- and closed-loop paths to make better decisions about settling >>>> on the final voltage offset target. Document these properties. >>>> >>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >>>> --- >>>> .../devicetree/bindings/opp/opp-v2-qcom-level.yaml | 14 ++++++++++++++ >>>> 1 file changed, 14 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml b/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml >>>> index a30ef93213c0..93cc88434dfe 100644 >>>> --- a/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml >>>> +++ b/Documentation/devicetree/bindings/opp/opp-v2-qcom-level.yaml >>>> @@ -34,6 +34,20 @@ patternProperties: >>>> minItems: 1 >>>> maxItems: 2 >>>> + qcom,opp-cloop-vadj: >>>> + description: | >>>> + A value representing the closed-loop voltage adjustment value >>> >>> A value? >>> >>>> + associated with this OPP node. >>>> + $ref: /schemas/types.yaml#/definitions/int32-array >>>> + maxItems: 2 >>> >>> Or 2 values? >> Right, this description doesn't make any sense if you're just >> looking at the documentation without looking at the driver.. >> >> Generally, each CPR3 instance can have multiple "threads" >> (each one of which regulates voltage for some on-SoC IP or >> part of it). The nth entry in the qcom,opp-[co]loop-vadj >> array corresponds to a voltage offset for the nth thread. >> >> If the nth entry in the array is missing, the driver assumes >> the arr[0] one is "global" to this CPR3 instance at this OPP >> level and applies it to all threads. ...and looking at it >> again, this is sorta just bad design, especially if you >> take into account that there's no known user of CPR3 that >> employs more than 2 threads. >> >> I'll remove that from the driver and make the description clearer. >> > > description: > Represents the closed-loop voltage adjustment associated with > this OPP node. > > P.S.: Drop '|' here and on oloop! > > This binding is intended to support either single or multiple CPR threads; > the driver's behavior is unimportant as bindings describe the hardware, > not the driver. Correct, but specifying just one value regardless of the number of threads is not in the spirit of representing things clearly. These properties do not describe the hardware. They let us pass configuration values that are specific to the SoC hosting the CPR, not to the CPR itself. Konrad > > Regards, > Angelo > >> >> Also, only noticed now.. "qcom,sdm630-cprh" was not documented, >> so that's to be fixed for the next submission as well! >> >> >> Konrad >>> >>>> + >>>> + qcom,opp-oloop-vadj: >>>> + description: | >>>> + A value representing the open-loop voltage adjustment value >>>> + associated with this OPP node. >>>> + $ref: /schemas/types.yaml#/definitions/int32-array >>>> + maxItems: 2 >>>> + >>>> required: >>>> - opp-level >>>> - qcom,opp-fuse-level >>>> >>>> -- >>>> 2.39.1 >>>> > > >
Il 27/02/23 03:55, Dmitry Baryshkov ha scritto: > On 17/02/2023 13:08, Konrad Dybcio wrote: >> From: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> >> >> This commit introduces a new driver, based on the one for cpr v1, >> to enable support for the newer Qualcomm Core Power Reduction >> hardware, known downstream as CPR3, CPR4 and CPRh, and support >> for MSM8998 and SDM630 CPU power reduction. >> >> In these new versions of the hardware, support for various new >> features was introduced, including voltage reduction for the GPU, >> security hardening and a new way of controlling CPU DVFS, >> consisting in internal communication between microcontrollers, >> specifically the CPR-Hardened and the Operating State Manager. >> >> The CPR v3, v4 and CPRh are present in a broad range of SoCs, >> from the mid-range to the high end ones including, but not limited >> to, MSM8953/8996/8998, SDM630/636/660/845. >> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> >> [Konrad: rebase, apply review comments] >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >> --- >> drivers/soc/qcom/Kconfig | 22 + >> drivers/soc/qcom/Makefile | 4 +- >> drivers/soc/qcom/cpr-common.h | 2 + >> drivers/soc/qcom/cpr3.c | 2923 +++++++++++++++++++++++++++++++++++++++++ >> include/soc/qcom/cpr.h | 17 + >> 5 files changed, 2967 insertions(+), 1 deletion(-) >> >> diff --git a/include/soc/qcom/cpr.h b/include/soc/qcom/cpr.h >> new file mode 100644 >> index 000000000000..2ba4324d18f6 >> --- /dev/null >> +++ b/include/soc/qcom/cpr.h >> @@ -0,0 +1,17 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * Copyright (c) 2013-2020, The Linux Foundation. All rights reserved. >> + * Copyright (c) 2019 Linaro Limited >> + * Copyright (c) 2021, AngeloGioacchino Del Regno >> + * <angelogioacchino.delregno@somainline.org> >> + */ >> + >> +#ifndef __CPR_H__ >> +#define __CPR_H__ >> + >> +struct cpr_ext_data { >> + int mem_acc_threshold_uV; >> + int apm_threshold_uV; >> +}; > > Who is going to use this? Is it the cpufreq driver or some other driver? > We are adding an API without a clean user, can we drop it for now? > This is mandatory: qcom-cpufreq-hw is supposed to program the OSM before starting. From SDM845 onwards, the OSM is programmed by the bootloader before booting Linux; In MSM8996/98, SDM630/636/660, others, the bootloader does not program the OSM uC, so this has to be done in Linux - specifically, in the CPUFREQ driver (qcom-cpufreq-hw), otherwise this driver is completely pointless to have. CPU DVFS requires three uC to be correctly programmed in order to work: - SAW (for sleep states) - CPR-Hardened (voltage control, mandatory for stability) - OSM (for cpufreq-hw frequency steps [1..N]) Failing to *correctly* program either of the three will render CPU DVFS unusable. That clarified, my opinion is: No, you can't drop this. It's an essential piece for functionality. I agree in that this commit introduces a header that has only an internal (as in cpr3.c) user and no external ones, but I think that Konrad didn't want to include the qcom-cpufreq-hw.c commits in this series because it's already huge and pretty difficult to review; adding the cpufreq-hw commits would make the situation worse. Konrad, perhaps you can send the cpufreq-hw commits in a separate series, in which cover letter you mention a dependency on this one? That would *clearly* show the full picture to reviewers. I remember that when I sent the cpufreq-hw series along with this one (~2 years ago, I think?) that code had positive reviews from Bjorn, so it should be OK. It wasn't picked just-only-because of the cpr3 dependency. Regards, Angelo >> + >> +#endif /* __CPR_H__ */ >> >
On 27/02/2023 11:13, AngeloGioacchino Del Regno wrote: > Il 27/02/23 03:55, Dmitry Baryshkov ha scritto: >> On 17/02/2023 13:08, Konrad Dybcio wrote: >>> From: AngeloGioacchino Del Regno >>> <angelogioacchino.delregno@somainline.org> >>> >>> This commit introduces a new driver, based on the one for cpr v1, >>> to enable support for the newer Qualcomm Core Power Reduction >>> hardware, known downstream as CPR3, CPR4 and CPRh, and support >>> for MSM8998 and SDM630 CPU power reduction. >>> >>> In these new versions of the hardware, support for various new >>> features was introduced, including voltage reduction for the GPU, >>> security hardening and a new way of controlling CPU DVFS, >>> consisting in internal communication between microcontrollers, >>> specifically the CPR-Hardened and the Operating State Manager. >>> >>> The CPR v3, v4 and CPRh are present in a broad range of SoCs, >>> from the mid-range to the high end ones including, but not limited >>> to, MSM8953/8996/8998, SDM630/636/660/845. >>> >>> Signed-off-by: AngeloGioacchino Del Regno >>> <angelogioacchino.delregno@somainline.org> >>> [Konrad: rebase, apply review comments] >>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >>> --- >>> drivers/soc/qcom/Kconfig | 22 + >>> drivers/soc/qcom/Makefile | 4 +- >>> drivers/soc/qcom/cpr-common.h | 2 + >>> drivers/soc/qcom/cpr3.c | 2923 >>> +++++++++++++++++++++++++++++++++++++++++ >>> include/soc/qcom/cpr.h | 17 + >>> 5 files changed, 2967 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/soc/qcom/cpr.h b/include/soc/qcom/cpr.h >>> new file mode 100644 >>> index 000000000000..2ba4324d18f6 >>> --- /dev/null >>> +++ b/include/soc/qcom/cpr.h >>> @@ -0,0 +1,17 @@ >>> +/* SPDX-License-Identifier: GPL-2.0-only */ >>> +/* >>> + * Copyright (c) 2013-2020, The Linux Foundation. All rights reserved. >>> + * Copyright (c) 2019 Linaro Limited >>> + * Copyright (c) 2021, AngeloGioacchino Del Regno >>> + * <angelogioacchino.delregno@somainline.org> >>> + */ >>> + >>> +#ifndef __CPR_H__ >>> +#define __CPR_H__ >>> + >>> +struct cpr_ext_data { >>> + int mem_acc_threshold_uV; >>> + int apm_threshold_uV; >>> +}; >> >> Who is going to use this? Is it the cpufreq driver or some other driver? >> We are adding an API without a clean user, can we drop it for now? >> > > This is mandatory: qcom-cpufreq-hw is supposed to program the OSM before > starting. Thanks for the explanation! > > From SDM845 onwards, the OSM is programmed by the bootloader before > booting > Linux; > In MSM8996/98, SDM630/636/660, others, the bootloader does not program > the OSM > uC, so this has to be done in Linux - specifically, in the CPUFREQ driver > (qcom-cpufreq-hw), otherwise this driver is completely pointless to have. > > CPU DVFS requires three uC to be correctly programmed in order to work: > - SAW (for sleep states) I believe this is handled by the PCSI for all mentioned platforms. > - CPR-Hardened (voltage control, mandatory for stability) This driver (nit: 8996 has cpr3) > - OSM (for cpufreq-hw frequency steps [1..N]) I think this is valid only for the CPRh targets. And for OSM programming the driver populates OPP tables with voltage levels (which should then be handled by the cpufreq-hw). I'd toss another coin into the list: for 8996 we also have to program APM and cluster (kryo) regulators _manually_. > > Failing to *correctly* program either of the three will render CPU DVFS > unusable. > > > That clarified, my opinion is: > No, you can't drop this. It's an essential piece for functionality. > > I agree in that this commit introduces a header that has only an > internal (as in > cpr3.c) user and no external ones, but I think that Konrad didn't want > to include > the qcom-cpufreq-hw.c commits in this series because it's already huge > and pretty > difficult to review; adding the cpufreq-hw commits would make the > situation worse. Perhaps we misunderstand each other here. I suggest dropping the header from _this_ patchset only and submit/merge corresponding code together with the cpufreq-hw changes. This might sound like a complication, but in reality it allows one to assess corresponding code separately. (Moreover, please correct me if I'm wrong, I think this header will be used only with the CPRh, and so this has no use for CPR3/4. Is this correct?) I took a glance at the 'cpufreq: qcom-hw: Implement CPRh aware OSM programming' patch, it doesn't seem to use the header (maybe I checked the older version of the patch). As for me, this is another signal that cpr_ext_data should come together with the LUT programming rather than with the CPRh itself. > Konrad, perhaps you can send the cpufreq-hw commits in a separate > series, in > which cover letter you mention a dependency on this one? > That would *clearly* show the full picture to reviewers. Yes, that would be great. A small note regarding those patches. I see that you patched the qcom-cpufreq-hw.c. This way first the driver programs the LUT, then it reads it back to setup the OPPs. Would it be easier to split OSM-not-programmed driver? > > I remember that when I sent the cpufreq-hw series along with this one > (~2 years > ago, I think?) that code had positive reviews from Bjorn, so it should > be OK. > It wasn't picked just-only-because of the cpr3 dependency. > > Regards, > Angelo > >>> + >>> +#endif /* __CPR_H__ */ >>> >> > > >
On Mon, 27 Feb 2023 at 15:06, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > Il 27/02/23 13:01, Dmitry Baryshkov ha scritto: > > > > I took a glance at the 'cpufreq: qcom-hw: Implement CPRh aware OSM programming' > > patch, it doesn't seem to use the header (maybe I checked the older version of the > > patch). As for me, this is another signal that cpr_ext_data should come together > > with the LUT programming rather than with the CPRh itself. > > > >> Konrad, perhaps you can send the cpufreq-hw commits in a separate series, in > >> which cover letter you mention a dependency on this one? > >> That would *clearly* show the full picture to reviewers. > > > > Yes, that would be great. A small note regarding those patches. I see that you > > patched the qcom-cpufreq-hw.c. This way first the driver programs the LUT, then it > > reads it back to setup the OPPs. Would it be easier to split OSM-not-programmed > > driver? > > > > When I engineered that solution, I kept the cpufreq-hw reading *again* the values > from OSM to keep the driver *fully* compatible with the bootloader-programmed OSM > flow, which makes one thing (in my opinion) perfectly clear: that programming > sequence is exactly the same as what happens "under the hood" on SDM845 (and later) > but performed here-instead-of-there (linux instead of bootloader), with the actual > scaling driver being 100% the same between the two flows in the end. > > Having two drivers as you suggested would indeed achieve the same, but wouldn't be > any easier... if you do that, you'd have to *somehow* make sure that the > programming driver does its job before the cpufreq driver tries to read the OSM > status, adding one more link to an already long chain. > > Besides, I remember that this question got asked a while ago on the mailing lists > and there was a short discussion about it: > > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2555580.html Ack, I see. Maybe splitting LUT programming to a separate source file would emphasise the fact that it is only required for some (older) SoCs. Other than that, I have no additional comments for that series.
On 27.02.2023 04:09, Dmitry Baryshkov wrote: > On 17/02/2023 13:08, Konrad Dybcio wrote: >> From: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> >> >> In preparation for implementing a new driver that will be handling >> CPRv3, CPRv4 and CPR-Hardened, format out common functions to a new >> file. >> >> Update cpr_get_fuses in preparation for CPR3 implementation, change >> parameters where necessary to not take cpr.c private data structures. >> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> >> [Konrad: rebase, apply review comments, don't break backwards compat, improve msg] >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >> --- >> drivers/soc/qcom/Makefile | 2 +- >> drivers/soc/qcom/cpr-common.c | 363 +++++++++++++++++++++++++++++++++++++++ >> drivers/soc/qcom/cpr-common.h | 108 ++++++++++++ >> drivers/soc/qcom/cpr.c | 386 +++--------------------------------------- >> 4 files changed, 494 insertions(+), 365 deletions(-) >> > > [skipped] > >> diff --git a/drivers/soc/qcom/cpr-common.h b/drivers/soc/qcom/cpr-common.h >> new file mode 100644 >> index 000000000000..2cd15f7eac90 >> --- /dev/null >> +++ b/drivers/soc/qcom/cpr-common.h >> @@ -0,0 +1,108 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> + >> +#include <linux/clk.h> >> +#include <linux/platform_device.h> >> +#include <linux/pm_opp.h> >> +#include <linux/regulator/consumer.h> >> + >> +enum voltage_change_dir { >> + NO_CHANGE, >> + DOWN, >> + UP, >> +}; >> + >> +struct fuse_corner_data { >> + int ref_uV; >> + int max_uV; >> + int min_uV; >> + int range_uV; >> + /* fuse volt: closed/open loop */ >> + int volt_cloop_adjust; >> + int volt_oloop_adjust; > > For CPR3 these values are per-fusing-rev. > (for 8996 tables list per-fusing-rev values for min_uV, volt_cloop_adjust and volt_oloop_adjust) Yes they are per-fuse-rev on other SoCs as well.. Angelo didn't implement this in the original revision of the driver and I think it'd be good to add it incrementally since we already consume the necessary fuse.. Otherwise -ETOOFAT! > > Another option, of course, might be to have a per-SoC code that uses fusing_rev to update the fuse_corner_data, but it would mean making it non-const. Hm.. a const array sounds better to me.. > >> + int max_volt_scale; >> + int max_quot_scale; > > Any reason for these limitations? I'd assume that's a safety feature Qualcomm implemented to avoid burning chips if cosmic rays poke at DRAM or some chips are fused incorrectly.. Preferably, I'd keep it! > >> + /* fuse quot */ >> + int quot_offset; >> + int quot_scale; >> + int quot_adjust; > > I see that quot_offset/quot_scale/quot_adjust are set to 0/1/0 for all the platforms I can assess at this moment (8996/8998/sdm660). Can we drop them? If we need them later, we can readd them later. I was about to do it, but noticed 8956 sets scaling to 10.. Guess we can leave it since it's already there! Konrad > >> + /* fuse quot_offset */ >> + int quot_offset_scale; >> + int quot_offset_adjust; >> +}; >> + >> +struct cpr_fuse { >> + char *ring_osc; >> + char *init_voltage; >> + char *quotient; >> + char *quotient_offset; >> +}; >> + >> +struct fuse_corner { >> + int min_uV; >> + int max_uV; >> + int uV; >> + int quot; >> + int step_quot; >> + const struct reg_sequence *accs; >> + int num_accs; >> + unsigned long max_freq; >> + u8 ring_osc_idx; >> +}; >> + >> +struct corner { >> + int min_uV; >> + int max_uV; >> + int uV; >> + int last_uV; >> + int quot_adjust; >> + u32 save_ctl; >> + u32 save_irq; >> + unsigned long freq; >> + bool is_open_loop; >> + struct fuse_corner *fuse_corner; >> +}; >> + >> +struct corner_data { >> + unsigned int fuse_corner; >> + unsigned long freq; >> +}; >> + >> +struct acc_desc { >> + unsigned int enable_reg; >> + u32 enable_mask; >> + >> + struct reg_sequence *config; >> + struct reg_sequence *settings; >> + int num_regs_per_fuse; >> +}; >> + >> +struct cpr_acc_desc { >> + const struct cpr_desc *cpr_desc; >> + const struct acc_desc *acc_desc; >> +}; >> + > > [skipped the rest] >
Changes in v10: - Skip "Let qcom,opp-fuse-level be a 2-long array" (Applied by Viresh) - Use b4 (it may be the first time you're receiving this if git send-email omitted you before..) - +Cc Robert Marko (expressed interest in previous revisions) - Add "Document CPR3 open/closed loop volt adjustment" CPR: - %hhu -> %u (checkpatch) CPR BINDINGS: - Drop QCS404 fuse set (it doesn't use this driver, what did I even think..) but leave the allOf:if: block for expansion (sdm660, msm8996, ipqABCD should follow soon..) - Drop Rob's R-b (as things changed *again*, please take one more look to make sure you're okay with this file, Rob..) Link to v9: https://lore.kernel.org/linux-arm-msm/20230116093845.72621-1-konrad.dybcio@linaro.org/ Changes in v9: - Restore forgotten MAINTAINERS patch (oops) CPR: - Include the missing header (big oops!) - Fix kconfig dependencies CPR bindings: - Fix cpu reg in example (why didn't dt_binding_check scream at that) - Add newlines between nodes in example - Change opp table node names to opp-table-cpu[04] - Change opp table labels to cpu[04]_opp_table - Change CPRh opp subnode names to opp-N from oppN - Remove some stray newlines - Bring back nvmem-cell-names and add the 8998's set - Allow power-domains for VDDCX_AO voting - Remove Rob's r-b, there's been quite a bit of changes.. CPR DT: - Send the correct revision of the patch this time around.. OPP bindings: - Add Rob's ack Link to v8: https://lore.kernel.org/linux-arm-msm/20230110175605.1240188-1-konrad.dybcio@linaro.org/ Changes in v8: - Overtake this series from AGdR - Apply all review comments from v7 except Vladimir's request to not create the include/ header; it will be strictly necessary for OSM-aware cpufreq_hw programming, which this series was more or less created just for.. - Drop QCS404 dtsi change, account for not breaking backwards compat in [3/5] - Add type phandle type reference to acc-syscon in [1/5] - Update AGdR's email addresses for maintainer entries - Add [2/5] to make dt_binding_check happy - Separate the CPRh DT addition from cpufreq_hw addition, sort and properly indent new nodes - Drop CPR yaml conversion, that happened in meantime - Reorder the patches to make a bit more sense - Tested again on MSM8998 Xperia XZ Premium (Maple) - I take no responsibility for AGdR's cheeky jokes, only the code! Link to v7: https://lore.kernel.org/lkml/20210901155735.629282-1-angelogioacchino.delregno@somainline.org/ Changes in v7: - Rebased on linux-next as of 210901 - Changed cpr_read_efuse calls to nvmem_cell_read_variable_le_u32, following what was done in commit c77634b9d916 Changes in v6: - Fixes from Bjorn's review - After a conversation with Viresh, it turned out I was abusing the OPP API to pass the APM and MEM-ACC thresholds to qcom-cpufreq-hw, so now the driver is using the genpd created virtual device and passing drvdata instead to stop the abuse - Since the CPR commonization was ignored for more than 6 months, it is now included in the CPRv3/4/h series, as there is no point in commonizing without having this driver - Rebased on v5.13 Changes in v5: - Fixed getting OPP table when not yet installed by the caller of power domain attachment Changes in v4: - Huge patch series has been split for better reviewability, as suggested by Bjorn Changes in v3: - Fixed YAML doc issues - Removed unused variables and redundant if branch Changes in v2: - Implemented dynamic Memory Accelerator corners support, needed by MSM8998 - Added MSM8998 Silver/Gold parameters This commit introduces a new driver, based on the one for cpr v1, to enable support for the newer Qualcomm Core Power Reduction hardware, known downstream as CPR3, CPR4 and CPRh, and support for MSM8998 and SDM630 CPU power reduction. In these new versions of the hardware, support for various new features was introduced, including voltage reduction for the GPU, security hardening and a new way of controlling CPU DVFS, consisting in internal communication between microcontrollers, specifically the CPR-Hardened and the Operating State Manager. The CPR v3, v4 and CPRh are present in a broad range of SoCs, from the mid-range to the high end ones including, but not limited to, MSM8953/8996/8998, SDM630/636/660/845. As to clarify, SDM845 does the CPR/SAW/OSM setup in TZ firmware, but this is limited to the CPU context; despite GPU CPR support being not implemented in this series, it is planned for the future, and some SDM845 need the CPR (in the context of GPU CPR) to be configured from this driver. It is also planned to add the CPR data for MSM8996, since this driver does support the CPRv4 found on that SoC, but I currently have no time to properly test that on a real device, so I prefer getting this big implementation merged before adding more things on top. As for MSM8953, we (read: nobody from SoMainline) have no device with this chip: since we are unable to test the cpr data and the entire driver on that one, we currently have no plans to do this addition in the future. This is left to other nice developers: I'm sure that somebody will come up with that, sooner or later Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> --- AngeloGioacchino Del Regno (5): MAINTAINERS: Add entry for Qualcomm CPRv3/v4/Hardened driver dt-bindings: soc: qcom: cpr3: Add bindings for CPR3 driver soc: qcom: cpr: Move common functions to new file soc: qcom: Add support for Core Power Reduction v3, v4 and Hardened arm64: dts: qcom: msm8998: Configure CPRh Konrad Dybcio (1): dt-bindings: opp: v2-qcom-level: Document CPR3 open/closed loop volt adjustment .../devicetree/bindings/opp/opp-v2-qcom-level.yaml | 14 + .../devicetree/bindings/soc/qcom/qcom,cpr3.yaml | 299 ++ MAINTAINERS | 6 + arch/arm64/boot/dts/qcom/msm8998.dtsi | 873 ++++++ drivers/soc/qcom/Kconfig | 22 + drivers/soc/qcom/Makefile | 2 + drivers/soc/qcom/cpr-common.c | 363 +++ drivers/soc/qcom/cpr-common.h | 110 + drivers/soc/qcom/cpr.c | 386 +-- drivers/soc/qcom/cpr3.c | 2923 ++++++++++++++++++++ include/soc/qcom/cpr.h | 17 + 11 files changed, 4651 insertions(+), 364 deletions(-) --- base-commit: c068f40300a0eaa34f7105d137a5560b86951aa9 change-id: 20230217-topic-cpr3h-de232bfb47ec Best regards,