2019-11-29 21:42:47

by Niklas Cassel

[permalink] [raw]
Subject: [PATCH v7 0/5] Add support for QCOM Core Power Reduction

This series adds support for Core Power Reduction (CPR), a form of
Adaptive Voltage Scaling (AVS), found on certain Qualcomm SoCs.

This series is based on top of the qcs404 cpufreq patch series that
hasn't landed yet:
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=207821
as well as that series' matching device tree changes:
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=207831

For testing purposes, this patch series, including the dependencies
listed above, is available on the following git tag:
https://git.linaro.org/people/niklas.cassel/kernel.git/log/?h=cpr-v7

CPR is a technology that reduces core power on a CPU or on other device.
It reads voltage settings from efuses (that have been written in
production), it uses these voltage settings as initial values, for each
OPP.

After moving to a certain OPP, CPR monitors dynamic factors such as
temperature, etc. and adjusts the voltage for that frequency accordingly
to save power and meet silicon characteristic requirements.

This driver has been developed together with Jorge Ramirez-Ortiz, and
is based on an RFC by Stephen Boyd[1], which in turn is based on work
by others on codeaurora.org[2].

[1] https://lkml.org/lkml/2015/9/18/833
[2] https://source.codeaurora.org/quic/la/kernel/msm-4.14/tree/drivers/regulator/cpr-regulator.c?h=msm-4.14

Changes since v6:
(Addressed comments from Ulf Hansson)
-Initialize mutex later.
-Take the mutex in cpr_pd_attach_dev(), in case we ever
implement async attach in the future.
-Add comment regarding why we get the cpu clock rate.
-Add comment how we handle unlisted frequencies.
-Clarify comment regarding why things related to virtual corners
are performed in cpr_pd_attach_dev().
-Removed the internal performance_state variable, the performance
state is instead calculated using the current corner pointer.
-Save a pointer to the first genpd dev that gets attached,
and use that rather than get_cpu_device(0), when getting the CPU
OPP table.

Niklas Cassel (5):
dt-bindings: power: avs: Add support for CPR (Core Power Reduction)
power: avs: Add support for CPR (Core Power Reduction)
arm64: dts: qcom: qcs404: Add CPR and populate OPP table
arm64: defconfig: enable CONFIG_QCOM_CPR
arm64: defconfig: enable CONFIG_ARM_QCOM_CPUFREQ_NVMEM

.../bindings/power/avs/qcom,cpr.txt | 130 ++
MAINTAINERS | 8 +
arch/arm64/boot/dts/qcom/qcs404.dtsi | 132 +-
arch/arm64/configs/defconfig | 2 +
drivers/power/avs/Kconfig | 15 +
drivers/power/avs/Makefile | 1 +
drivers/power/avs/qcom-cpr.c | 1792 +++++++++++++++++
7 files changed, 2072 insertions(+), 8 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
create mode 100644 drivers/power/avs/qcom-cpr.c

--
2.23.0


2019-11-29 21:43:01

by Niklas Cassel

[permalink] [raw]
Subject: [PATCH v7 1/5] dt-bindings: power: avs: Add support for CPR (Core Power Reduction)

Add DT bindings to describe the CPR HW found on certain Qualcomm SoCs.

Co-developed-by: Jorge Ramirez-Ortiz <[email protected]>
Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
Signed-off-by: Niklas Cassel <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
Reviewed-by: Ulf Hansson <[email protected]>
---
Changes since v6:
-Picked up Bjorn's and Ulf's Reviewed-by.

.../bindings/power/avs/qcom,cpr.txt | 130 ++++++++++++++++++
1 file changed, 130 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/avs/qcom,cpr.txt

diff --git a/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt b/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
new file mode 100644
index 000000000000..ab0d5ebbad4e
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
@@ -0,0 +1,130 @@
+QCOM CPR (Core Power Reduction)
+
+CPR (Core Power Reduction) is a technology to reduce core power on a CPU
+or other device. Each OPP of a device corresponds to a "corner" that has
+a range of valid voltages for a particular frequency. While the device is
+running at a particular frequency, CPR monitors dynamic factors such as
+temperature, etc. and suggests adjustments to the voltage to save power
+and meet silicon characteristic requirements.
+
+- compatible:
+ Usage: required
+ Value type: <string>
+ Definition: should be "qcom,qcs404-cpr", "qcom,cpr" for qcs404
+
+- reg:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: base address and size of the rbcpr register region
+
+- interrupts:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: should specify the CPR interrupt
+
+- clocks:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: phandle to the reference clock
+
+- clock-names:
+ Usage: required
+ Value type: <stringlist>
+ Definition: must be "ref"
+
+- vdd-apc-supply:
+ Usage: required
+ Value type: <phandle>
+ Definition: phandle to the vdd-apc-supply regulator
+
+- #power-domain-cells:
+ Usage: required
+ Value type: <u32>
+ Definition: should be 0
+
+- operating-points-v2:
+ Usage: required
+ Value type: <phandle>
+ Definition: A phandle to the OPP table containing the
+ performance states supported by the CPR
+ power domain
+
+- acc-syscon:
+ Usage: optional
+ Value type: <phandle>
+ Definition: phandle to syscon for writing ACC settings
+
+- nvmem-cells:
+ Usage: required
+ Value type: <phandle>
+ Definition: phandle to nvmem cells containing the data
+ that makes up a fuse corner, for each fuse corner.
+ As well as the CPR fuse revision.
+
+- nvmem-cell-names:
+ Usage: required
+ Value type: <stringlist>
+ Definition: should be "cpr_quotient_offset1", "cpr_quotient_offset2",
+ "cpr_quotient_offset3", "cpr_init_voltage1",
+ "cpr_init_voltage2", "cpr_init_voltage3", "cpr_quotient1",
+ "cpr_quotient2", "cpr_quotient3", "cpr_ring_osc1",
+ "cpr_ring_osc2", "cpr_ring_osc3", "cpr_fuse_revision"
+ for qcs404.
+
+Example:
+
+ cpr_opp_table: cpr-opp-table {
+ compatible = "operating-points-v2-qcom-level";
+
+ cpr_opp1: opp1 {
+ opp-level = <1>;
+ qcom,opp-fuse-level = <1>;
+ };
+ cpr_opp2: opp2 {
+ opp-level = <2>;
+ qcom,opp-fuse-level = <2>;
+ };
+ cpr_opp3: opp3 {
+ opp-level = <3>;
+ qcom,opp-fuse-level = <3>;
+ };
+ };
+
+ power-controller@b018000 {
+ compatible = "qcom,qcs404-cpr", "qcom,cpr";
+ reg = <0x0b018000 0x1000>;
+ interrupts = <0 15 IRQ_TYPE_EDGE_RISING>;
+ clocks = <&xo_board>;
+ clock-names = "ref";
+ vdd-apc-supply = <&pms405_s3>;
+ #power-domain-cells = <0>;
+ operating-points-v2 = <&cpr_opp_table>;
+ acc-syscon = <&tcsr>;
+
+ nvmem-cells = <&cpr_efuse_quot_offset1>,
+ <&cpr_efuse_quot_offset2>,
+ <&cpr_efuse_quot_offset3>,
+ <&cpr_efuse_init_voltage1>,
+ <&cpr_efuse_init_voltage2>,
+ <&cpr_efuse_init_voltage3>,
+ <&cpr_efuse_quot1>,
+ <&cpr_efuse_quot2>,
+ <&cpr_efuse_quot3>,
+ <&cpr_efuse_ring1>,
+ <&cpr_efuse_ring2>,
+ <&cpr_efuse_ring3>,
+ <&cpr_efuse_revision>;
+ nvmem-cell-names = "cpr_quotient_offset1",
+ "cpr_quotient_offset2",
+ "cpr_quotient_offset3",
+ "cpr_init_voltage1",
+ "cpr_init_voltage2",
+ "cpr_init_voltage3",
+ "cpr_quotient1",
+ "cpr_quotient2",
+ "cpr_quotient3",
+ "cpr_ring_osc1",
+ "cpr_ring_osc2",
+ "cpr_ring_osc3",
+ "cpr_fuse_revision";
+ };
--
2.23.0

2019-11-29 21:43:27

by Niklas Cassel

[permalink] [raw]
Subject: [PATCH v7 3/5] arm64: dts: qcom: qcs404: Add CPR and populate OPP table

Add CPR and populate OPP table.

Co-developed-by: Jorge Ramirez-Ortiz <[email protected]>
Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
Signed-off-by: Niklas Cassel <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
Reviewed-by: Ulf Hansson <[email protected]>
---
Changes since v6:
-Picked up Bjorn's and Ulf's Reviewed-by.

arch/arm64/boot/dts/qcom/qcs404.dtsi | 132 +++++++++++++++++++++++++--
1 file changed, 124 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index 03aa80f2814a..ca255d98d007 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -44,7 +44,8 @@
#cooling-cells = <2>;
clocks = <&apcs_glb>;
operating-points-v2 = <&cpu_opp_table>;
- cpu-supply = <&pms405_s3>;
+ power-domains = <&cpr>;
+ power-domain-names = "cpr";
};

CPU1: cpu@101 {
@@ -57,7 +58,8 @@
#cooling-cells = <2>;
clocks = <&apcs_glb>;
operating-points-v2 = <&cpu_opp_table>;
- cpu-supply = <&pms405_s3>;
+ power-domains = <&cpr>;
+ power-domain-names = "cpr";
};

CPU2: cpu@102 {
@@ -70,7 +72,8 @@
#cooling-cells = <2>;
clocks = <&apcs_glb>;
operating-points-v2 = <&cpu_opp_table>;
- cpu-supply = <&pms405_s3>;
+ power-domains = <&cpr>;
+ power-domain-names = "cpr";
};

CPU3: cpu@103 {
@@ -83,7 +86,8 @@
#cooling-cells = <2>;
clocks = <&apcs_glb>;
operating-points-v2 = <&cpu_opp_table>;
- cpu-supply = <&pms405_s3>;
+ power-domains = <&cpr>;
+ power-domain-names = "cpr";
};

L2_0: l2-cache {
@@ -107,20 +111,37 @@
};

cpu_opp_table: cpu-opp-table {
- compatible = "operating-points-v2";
+ compatible = "operating-points-v2-kryo-cpu";
opp-shared;

opp-1094400000 {
opp-hz = /bits/ 64 <1094400000>;
- opp-microvolt = <1224000 1224000 1224000>;
+ required-opps = <&cpr_opp1>;
};
opp-1248000000 {
opp-hz = /bits/ 64 <1248000000>;
- opp-microvolt = <1288000 1288000 1288000>;
+ required-opps = <&cpr_opp2>;
};
opp-1401600000 {
opp-hz = /bits/ 64 <1401600000>;
- opp-microvolt = <1384000 1384000 1384000>;
+ required-opps = <&cpr_opp3>;
+ };
+ };
+
+ cpr_opp_table: cpr-opp-table {
+ compatible = "operating-points-v2-qcom-level";
+
+ cpr_opp1: opp1 {
+ opp-level = <1>;
+ qcom,opp-fuse-level = <1>;
+ };
+ cpr_opp2: opp2 {
+ opp-level = <2>;
+ qcom,opp-fuse-level = <2>;
+ };
+ cpr_opp3: opp3 {
+ opp-level = <3>;
+ qcom,opp-fuse-level = <3>;
};
};

@@ -310,6 +331,62 @@
tsens_caldata: caldata@d0 {
reg = <0x1f8 0x14>;
};
+ cpr_efuse_speedbin: speedbin@13c {
+ reg = <0x13c 0x4>;
+ bits = <2 3>;
+ };
+ cpr_efuse_quot_offset1: qoffset1@231 {
+ reg = <0x231 0x4>;
+ bits = <4 7>;
+ };
+ cpr_efuse_quot_offset2: qoffset2@232 {
+ reg = <0x232 0x4>;
+ bits = <3 7>;
+ };
+ cpr_efuse_quot_offset3: qoffset3@233 {
+ reg = <0x233 0x4>;
+ bits = <2 7>;
+ };
+ cpr_efuse_init_voltage1: ivoltage1@229 {
+ reg = <0x229 0x4>;
+ bits = <4 6>;
+ };
+ cpr_efuse_init_voltage2: ivoltage2@22a {
+ reg = <0x22a 0x4>;
+ bits = <2 6>;
+ };
+ cpr_efuse_init_voltage3: ivoltage3@22b {
+ reg = <0x22b 0x4>;
+ bits = <0 6>;
+ };
+ cpr_efuse_quot1: quot1@22b {
+ reg = <0x22b 0x4>;
+ bits = <6 12>;
+ };
+ cpr_efuse_quot2: quot2@22d {
+ reg = <0x22d 0x4>;
+ bits = <2 12>;
+ };
+ cpr_efuse_quot3: quot3@230 {
+ reg = <0x230 0x4>;
+ bits = <0 12>;
+ };
+ cpr_efuse_ring1: ring1@228 {
+ reg = <0x228 0x4>;
+ bits = <0 3>;
+ };
+ cpr_efuse_ring2: ring2@228 {
+ reg = <0x228 0x4>;
+ bits = <4 3>;
+ };
+ cpr_efuse_ring3: ring3@229 {
+ reg = <0x229 0x4>;
+ bits = <0 3>;
+ };
+ cpr_efuse_revision: revision@218 {
+ reg = <0x218 0x4>;
+ bits = <3 3>;
+ };
};

rng: rng@e3000 {
@@ -952,6 +1029,45 @@
clocks = <&sleep_clk>;
};

+ cpr: power-controller@b018000 {
+ compatible = "qcom,qcs404-cpr", "qcom,cpr";
+ reg = <0x0b018000 0x1000>;
+ interrupts = <0 15 IRQ_TYPE_EDGE_RISING>;
+ clocks = <&xo_board>;
+ clock-names = "ref";
+ vdd-apc-supply = <&pms405_s3>;
+ #power-domain-cells = <0>;
+ operating-points-v2 = <&cpr_opp_table>;
+ acc-syscon = <&tcsr>;
+
+ nvmem-cells = <&cpr_efuse_quot_offset1>,
+ <&cpr_efuse_quot_offset2>,
+ <&cpr_efuse_quot_offset3>,
+ <&cpr_efuse_init_voltage1>,
+ <&cpr_efuse_init_voltage2>,
+ <&cpr_efuse_init_voltage3>,
+ <&cpr_efuse_quot1>,
+ <&cpr_efuse_quot2>,
+ <&cpr_efuse_quot3>,
+ <&cpr_efuse_ring1>,
+ <&cpr_efuse_ring2>,
+ <&cpr_efuse_ring3>,
+ <&cpr_efuse_revision>;
+ nvmem-cell-names = "cpr_quotient_offset1",
+ "cpr_quotient_offset2",
+ "cpr_quotient_offset3",
+ "cpr_init_voltage1",
+ "cpr_init_voltage2",
+ "cpr_init_voltage3",
+ "cpr_quotient1",
+ "cpr_quotient2",
+ "cpr_quotient3",
+ "cpr_ring_osc1",
+ "cpr_ring_osc2",
+ "cpr_ring_osc3",
+ "cpr_fuse_revision";
+ };
+
timer@b120000 {
#address-cells = <1>;
#size-cells = <1>;
--
2.23.0

2019-12-20 09:32:50

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v7 1/5] dt-bindings: power: avs: Add support for CPR (Core Power Reduction)

On Friday, November 29, 2019 10:39:11 PM CET Niklas Cassel wrote:
> Add DT bindings to describe the CPR HW found on certain Qualcomm SoCs.
>
> Co-developed-by: Jorge Ramirez-Ortiz <[email protected]>
> Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
> Signed-off-by: Niklas Cassel <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> Reviewed-by: Bjorn Andersson <[email protected]>
> Reviewed-by: Ulf Hansson <[email protected]>
> ---
> Changes since v6:
> -Picked up Bjorn's and Ulf's Reviewed-by.
>
> .../bindings/power/avs/qcom,cpr.txt | 130 ++++++++++++++++++
> 1 file changed, 130 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
>
> diff --git a/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt b/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
> new file mode 100644
> index 000000000000..ab0d5ebbad4e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
> @@ -0,0 +1,130 @@
> +QCOM CPR (Core Power Reduction)
> +
> +CPR (Core Power Reduction) is a technology to reduce core power on a CPU
> +or other device. Each OPP of a device corresponds to a "corner" that has
> +a range of valid voltages for a particular frequency. While the device is
> +running at a particular frequency, CPR monitors dynamic factors such as
> +temperature, etc. and suggests adjustments to the voltage to save power
> +and meet silicon characteristic requirements.
> +
> +- compatible:
> + Usage: required
> + Value type: <string>
> + Definition: should be "qcom,qcs404-cpr", "qcom,cpr" for qcs404
> +
> +- reg:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: base address and size of the rbcpr register region
> +
> +- interrupts:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: should specify the CPR interrupt
> +
> +- clocks:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: phandle to the reference clock
> +
> +- clock-names:
> + Usage: required
> + Value type: <stringlist>
> + Definition: must be "ref"
> +
> +- vdd-apc-supply:
> + Usage: required
> + Value type: <phandle>
> + Definition: phandle to the vdd-apc-supply regulator
> +
> +- #power-domain-cells:
> + Usage: required
> + Value type: <u32>
> + Definition: should be 0
> +
> +- operating-points-v2:
> + Usage: required
> + Value type: <phandle>
> + Definition: A phandle to the OPP table containing the
> + performance states supported by the CPR
> + power domain
> +
> +- acc-syscon:
> + Usage: optional
> + Value type: <phandle>
> + Definition: phandle to syscon for writing ACC settings
> +
> +- nvmem-cells:
> + Usage: required
> + Value type: <phandle>
> + Definition: phandle to nvmem cells containing the data
> + that makes up a fuse corner, for each fuse corner.
> + As well as the CPR fuse revision.
> +
> +- nvmem-cell-names:
> + Usage: required
> + Value type: <stringlist>
> + Definition: should be "cpr_quotient_offset1", "cpr_quotient_offset2",
> + "cpr_quotient_offset3", "cpr_init_voltage1",
> + "cpr_init_voltage2", "cpr_init_voltage3", "cpr_quotient1",
> + "cpr_quotient2", "cpr_quotient3", "cpr_ring_osc1",
> + "cpr_ring_osc2", "cpr_ring_osc3", "cpr_fuse_revision"
> + for qcs404.
> +
> +Example:
> +
> + cpr_opp_table: cpr-opp-table {
> + compatible = "operating-points-v2-qcom-level";
> +
> + cpr_opp1: opp1 {
> + opp-level = <1>;
> + qcom,opp-fuse-level = <1>;
> + };
> + cpr_opp2: opp2 {
> + opp-level = <2>;
> + qcom,opp-fuse-level = <2>;
> + };
> + cpr_opp3: opp3 {
> + opp-level = <3>;
> + qcom,opp-fuse-level = <3>;
> + };
> + };
> +
> + power-controller@b018000 {
> + compatible = "qcom,qcs404-cpr", "qcom,cpr";
> + reg = <0x0b018000 0x1000>;
> + interrupts = <0 15 IRQ_TYPE_EDGE_RISING>;
> + clocks = <&xo_board>;
> + clock-names = "ref";
> + vdd-apc-supply = <&pms405_s3>;
> + #power-domain-cells = <0>;
> + operating-points-v2 = <&cpr_opp_table>;
> + acc-syscon = <&tcsr>;
> +
> + nvmem-cells = <&cpr_efuse_quot_offset1>,
> + <&cpr_efuse_quot_offset2>,
> + <&cpr_efuse_quot_offset3>,
> + <&cpr_efuse_init_voltage1>,
> + <&cpr_efuse_init_voltage2>,
> + <&cpr_efuse_init_voltage3>,
> + <&cpr_efuse_quot1>,
> + <&cpr_efuse_quot2>,
> + <&cpr_efuse_quot3>,
> + <&cpr_efuse_ring1>,
> + <&cpr_efuse_ring2>,
> + <&cpr_efuse_ring3>,
> + <&cpr_efuse_revision>;
> + nvmem-cell-names = "cpr_quotient_offset1",
> + "cpr_quotient_offset2",
> + "cpr_quotient_offset3",
> + "cpr_init_voltage1",
> + "cpr_init_voltage2",
> + "cpr_init_voltage3",
> + "cpr_quotient1",
> + "cpr_quotient2",
> + "cpr_quotient3",
> + "cpr_ring_osc1",
> + "cpr_ring_osc2",
> + "cpr_ring_osc3",
> + "cpr_fuse_revision";
> + };
>

I have queued up this one and the [2/5] for 5.6, but if you'd rather want them
to go in via a different patch, please let me know and I'll drop them.

Thanks!



2019-12-20 14:42:07

by Niklas Cassel

[permalink] [raw]
Subject: Re: [PATCH v7 1/5] dt-bindings: power: avs: Add support for CPR (Core Power Reduction)

On Fri, Dec 20, 2019 at 10:31:53AM +0100, Rafael J. Wysocki wrote:
> On Friday, November 29, 2019 10:39:11 PM CET Niklas Cassel wrote:
> > Add DT bindings to describe the CPR HW found on certain Qualcomm SoCs.
> >
> > Co-developed-by: Jorge Ramirez-Ortiz <[email protected]>
> > Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
> > Signed-off-by: Niklas Cassel <[email protected]>
> > Reviewed-by: Rob Herring <[email protected]>
> > Reviewed-by: Bjorn Andersson <[email protected]>
> > Reviewed-by: Ulf Hansson <[email protected]>
> > ---
> > Changes since v6:
> > -Picked up Bjorn's and Ulf's Reviewed-by.
> >
> > .../bindings/power/avs/qcom,cpr.txt | 130 ++++++++++++++++++
> > 1 file changed, 130 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
> >
> > diff --git a/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt b/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
> > new file mode 100644
> > index 000000000000..ab0d5ebbad4e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
> > @@ -0,0 +1,130 @@
> > +QCOM CPR (Core Power Reduction)
> > +
> > +CPR (Core Power Reduction) is a technology to reduce core power on a CPU
> > +or other device. Each OPP of a device corresponds to a "corner" that has
> > +a range of valid voltages for a particular frequency. While the device is
> > +running at a particular frequency, CPR monitors dynamic factors such as
> > +temperature, etc. and suggests adjustments to the voltage to save power
> > +and meet silicon characteristic requirements.
> > +
> > +- compatible:
> > + Usage: required
> > + Value type: <string>
> > + Definition: should be "qcom,qcs404-cpr", "qcom,cpr" for qcs404
> > +
> > +- reg:
> > + Usage: required
> > + Value type: <prop-encoded-array>
> > + Definition: base address and size of the rbcpr register region
> > +
> > +- interrupts:
> > + Usage: required
> > + Value type: <prop-encoded-array>
> > + Definition: should specify the CPR interrupt
> > +
> > +- clocks:
> > + Usage: required
> > + Value type: <prop-encoded-array>
> > + Definition: phandle to the reference clock
> > +
> > +- clock-names:
> > + Usage: required
> > + Value type: <stringlist>
> > + Definition: must be "ref"
> > +
> > +- vdd-apc-supply:
> > + Usage: required
> > + Value type: <phandle>
> > + Definition: phandle to the vdd-apc-supply regulator
> > +
> > +- #power-domain-cells:
> > + Usage: required
> > + Value type: <u32>
> > + Definition: should be 0
> > +
> > +- operating-points-v2:
> > + Usage: required
> > + Value type: <phandle>
> > + Definition: A phandle to the OPP table containing the
> > + performance states supported by the CPR
> > + power domain
> > +
> > +- acc-syscon:
> > + Usage: optional
> > + Value type: <phandle>
> > + Definition: phandle to syscon for writing ACC settings
> > +
> > +- nvmem-cells:
> > + Usage: required
> > + Value type: <phandle>
> > + Definition: phandle to nvmem cells containing the data
> > + that makes up a fuse corner, for each fuse corner.
> > + As well as the CPR fuse revision.
> > +
> > +- nvmem-cell-names:
> > + Usage: required
> > + Value type: <stringlist>
> > + Definition: should be "cpr_quotient_offset1", "cpr_quotient_offset2",
> > + "cpr_quotient_offset3", "cpr_init_voltage1",
> > + "cpr_init_voltage2", "cpr_init_voltage3", "cpr_quotient1",
> > + "cpr_quotient2", "cpr_quotient3", "cpr_ring_osc1",
> > + "cpr_ring_osc2", "cpr_ring_osc3", "cpr_fuse_revision"
> > + for qcs404.
> > +
> > +Example:
> > +
> > + cpr_opp_table: cpr-opp-table {
> > + compatible = "operating-points-v2-qcom-level";
> > +
> > + cpr_opp1: opp1 {
> > + opp-level = <1>;
> > + qcom,opp-fuse-level = <1>;
> > + };
> > + cpr_opp2: opp2 {
> > + opp-level = <2>;
> > + qcom,opp-fuse-level = <2>;
> > + };
> > + cpr_opp3: opp3 {
> > + opp-level = <3>;
> > + qcom,opp-fuse-level = <3>;
> > + };
> > + };
> > +
> > + power-controller@b018000 {
> > + compatible = "qcom,qcs404-cpr", "qcom,cpr";
> > + reg = <0x0b018000 0x1000>;
> > + interrupts = <0 15 IRQ_TYPE_EDGE_RISING>;
> > + clocks = <&xo_board>;
> > + clock-names = "ref";
> > + vdd-apc-supply = <&pms405_s3>;
> > + #power-domain-cells = <0>;
> > + operating-points-v2 = <&cpr_opp_table>;
> > + acc-syscon = <&tcsr>;
> > +
> > + nvmem-cells = <&cpr_efuse_quot_offset1>,
> > + <&cpr_efuse_quot_offset2>,
> > + <&cpr_efuse_quot_offset3>,
> > + <&cpr_efuse_init_voltage1>,
> > + <&cpr_efuse_init_voltage2>,
> > + <&cpr_efuse_init_voltage3>,
> > + <&cpr_efuse_quot1>,
> > + <&cpr_efuse_quot2>,
> > + <&cpr_efuse_quot3>,
> > + <&cpr_efuse_ring1>,
> > + <&cpr_efuse_ring2>,
> > + <&cpr_efuse_ring3>,
> > + <&cpr_efuse_revision>;
> > + nvmem-cell-names = "cpr_quotient_offset1",
> > + "cpr_quotient_offset2",
> > + "cpr_quotient_offset3",
> > + "cpr_init_voltage1",
> > + "cpr_init_voltage2",
> > + "cpr_init_voltage3",
> > + "cpr_quotient1",
> > + "cpr_quotient2",
> > + "cpr_quotient3",
> > + "cpr_ring_osc1",
> > + "cpr_ring_osc2",
> > + "cpr_ring_osc3",
> > + "cpr_fuse_revision";
> > + };
> >
>
> I have queued up this one and the [2/5] for 5.6, but if you'd rather want them
> to go in via a different patch, please let me know and I'll drop them.
>

Thanks a lot Rafael!

I would very much prefer them to go via your tree.

Unfortunately it seems like kbuild test robot
found an incorrect printk format specifier in
one of the debug prints.

Line 838
dev_dbg(dev, "efuse read(%s) = %x, bytes %ld\n", cname, *data, len);

should be
dev_dbg(dev, "efuse read(%s) = %x, bytes %zd\n", cname, *data, len);

So %zd rather than %ld.

This was obviously an error, but didn't show when
compiling on arm64 or x86_64.

Sorry for this inconvenience.

Could you fix up the commit or do I need to do a respin?


Kind regards,
Niklas

2019-12-23 14:59:58

by Niklas Cassel

[permalink] [raw]
Subject: Re: [PATCH v7 1/5] dt-bindings: power: avs: Add support for CPR (Core Power Reduction)

On Fri, Dec 20, 2019 at 03:33:56PM +0100, Niklas Cassel wrote:
> On Fri, Dec 20, 2019 at 10:31:53AM +0100, Rafael J. Wysocki wrote:
> > On Friday, November 29, 2019 10:39:11 PM CET Niklas Cassel wrote:
> > > Add DT bindings to describe the CPR HW found on certain Qualcomm SoCs.
> > >
> > > Co-developed-by: Jorge Ramirez-Ortiz <[email protected]>
> > > Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
> > > Signed-off-by: Niklas Cassel <[email protected]>
> > > Reviewed-by: Rob Herring <[email protected]>
> > > Reviewed-by: Bjorn Andersson <[email protected]>
> > > Reviewed-by: Ulf Hansson <[email protected]>
> > > ---
> > > Changes since v6:
> > > -Picked up Bjorn's and Ulf's Reviewed-by.
> > >
> > > .../bindings/power/avs/qcom,cpr.txt | 130 ++++++++++++++++++
> > > 1 file changed, 130 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt b/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
> > > new file mode 100644
> > > index 000000000000..ab0d5ebbad4e
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
> > > @@ -0,0 +1,130 @@
> > > +QCOM CPR (Core Power Reduction)
> > > +
> > > +CPR (Core Power Reduction) is a technology to reduce core power on a CPU
> > > +or other device. Each OPP of a device corresponds to a "corner" that has
> > > +a range of valid voltages for a particular frequency. While the device is
> > > +running at a particular frequency, CPR monitors dynamic factors such as
> > > +temperature, etc. and suggests adjustments to the voltage to save power
> > > +and meet silicon characteristic requirements.
> > > +
> > > +- compatible:
> > > + Usage: required
> > > + Value type: <string>
> > > + Definition: should be "qcom,qcs404-cpr", "qcom,cpr" for qcs404
> > > +
> > > +- reg:
> > > + Usage: required
> > > + Value type: <prop-encoded-array>
> > > + Definition: base address and size of the rbcpr register region
> > > +
> > > +- interrupts:
> > > + Usage: required
> > > + Value type: <prop-encoded-array>
> > > + Definition: should specify the CPR interrupt
> > > +
> > > +- clocks:
> > > + Usage: required
> > > + Value type: <prop-encoded-array>
> > > + Definition: phandle to the reference clock
> > > +
> > > +- clock-names:
> > > + Usage: required
> > > + Value type: <stringlist>
> > > + Definition: must be "ref"
> > > +
> > > +- vdd-apc-supply:
> > > + Usage: required
> > > + Value type: <phandle>
> > > + Definition: phandle to the vdd-apc-supply regulator
> > > +
> > > +- #power-domain-cells:
> > > + Usage: required
> > > + Value type: <u32>
> > > + Definition: should be 0
> > > +
> > > +- operating-points-v2:
> > > + Usage: required
> > > + Value type: <phandle>
> > > + Definition: A phandle to the OPP table containing the
> > > + performance states supported by the CPR
> > > + power domain
> > > +
> > > +- acc-syscon:
> > > + Usage: optional
> > > + Value type: <phandle>
> > > + Definition: phandle to syscon for writing ACC settings
> > > +
> > > +- nvmem-cells:
> > > + Usage: required
> > > + Value type: <phandle>
> > > + Definition: phandle to nvmem cells containing the data
> > > + that makes up a fuse corner, for each fuse corner.
> > > + As well as the CPR fuse revision.
> > > +
> > > +- nvmem-cell-names:
> > > + Usage: required
> > > + Value type: <stringlist>
> > > + Definition: should be "cpr_quotient_offset1", "cpr_quotient_offset2",
> > > + "cpr_quotient_offset3", "cpr_init_voltage1",
> > > + "cpr_init_voltage2", "cpr_init_voltage3", "cpr_quotient1",
> > > + "cpr_quotient2", "cpr_quotient3", "cpr_ring_osc1",
> > > + "cpr_ring_osc2", "cpr_ring_osc3", "cpr_fuse_revision"
> > > + for qcs404.
> > > +
> > > +Example:
> > > +
> > > + cpr_opp_table: cpr-opp-table {
> > > + compatible = "operating-points-v2-qcom-level";
> > > +
> > > + cpr_opp1: opp1 {
> > > + opp-level = <1>;
> > > + qcom,opp-fuse-level = <1>;
> > > + };
> > > + cpr_opp2: opp2 {
> > > + opp-level = <2>;
> > > + qcom,opp-fuse-level = <2>;
> > > + };
> > > + cpr_opp3: opp3 {
> > > + opp-level = <3>;
> > > + qcom,opp-fuse-level = <3>;
> > > + };
> > > + };
> > > +
> > > + power-controller@b018000 {
> > > + compatible = "qcom,qcs404-cpr", "qcom,cpr";
> > > + reg = <0x0b018000 0x1000>;
> > > + interrupts = <0 15 IRQ_TYPE_EDGE_RISING>;
> > > + clocks = <&xo_board>;
> > > + clock-names = "ref";
> > > + vdd-apc-supply = <&pms405_s3>;
> > > + #power-domain-cells = <0>;
> > > + operating-points-v2 = <&cpr_opp_table>;
> > > + acc-syscon = <&tcsr>;
> > > +
> > > + nvmem-cells = <&cpr_efuse_quot_offset1>,
> > > + <&cpr_efuse_quot_offset2>,
> > > + <&cpr_efuse_quot_offset3>,
> > > + <&cpr_efuse_init_voltage1>,
> > > + <&cpr_efuse_init_voltage2>,
> > > + <&cpr_efuse_init_voltage3>,
> > > + <&cpr_efuse_quot1>,
> > > + <&cpr_efuse_quot2>,
> > > + <&cpr_efuse_quot3>,
> > > + <&cpr_efuse_ring1>,
> > > + <&cpr_efuse_ring2>,
> > > + <&cpr_efuse_ring3>,
> > > + <&cpr_efuse_revision>;
> > > + nvmem-cell-names = "cpr_quotient_offset1",
> > > + "cpr_quotient_offset2",
> > > + "cpr_quotient_offset3",
> > > + "cpr_init_voltage1",
> > > + "cpr_init_voltage2",
> > > + "cpr_init_voltage3",
> > > + "cpr_quotient1",
> > > + "cpr_quotient2",
> > > + "cpr_quotient3",
> > > + "cpr_ring_osc1",
> > > + "cpr_ring_osc2",
> > > + "cpr_ring_osc3",
> > > + "cpr_fuse_revision";
> > > + };
> > >
> >
> > I have queued up this one and the [2/5] for 5.6, but if you'd rather want them
> > to go in via a different patch, please let me know and I'll drop them.
> >
>
> Thanks a lot Rafael!
>
> I would very much prefer them to go via your tree.
>
> Unfortunately it seems like kbuild test robot
> found an incorrect printk format specifier in
> one of the debug prints.
>
> Line 838
> dev_dbg(dev, "efuse read(%s) = %x, bytes %ld\n", cname, *data, len);
>
> should be
> dev_dbg(dev, "efuse read(%s) = %x, bytes %zd\n", cname, *data, len);
>
> So %zd rather than %ld.
>
> This was obviously an error, but didn't show when
> compiling on arm64 or x86_64.
>
> Sorry for this inconvenience.
>
> Could you fix up the commit or do I need to do a respin?

Hello Rafael,

Since the intel test robot found another problem,
I decided to cook up a series that can be applied
(or squashed) on top of your bleeding-edge branch
here instead:

https://patchwork.kernel.org/project/linux-pm/list/?series=220955

Again, sorry for the inconvenience.

Kind regards,
Niklas