[PATCH 00/13] Add support for QCOM Core Power Reduction

[PATCH 00/13] Add support for QCOM Core Power Reduction

[Niklas Cassel]

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=137809

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://www.codeaurora.org/cgit/quic/la/kernel/msm-3.10/tree/drivers/regulator/cpr-regulator.c?h=msm-3.10

Jorge Ramirez-Ortiz (1):
  cpufreq: Add qcs404 to cpufreq-dt-platdev blacklist

Niklas Cassel (10):
  dt-bindings: cpufreq: qcom-nvmem: Make speedbin related properties
    optional
  cpufreq: qcom: Refactor the driver to make it easier to extend
  dt-bindings: cpufreq: qcom-nvmem: Support pstates provided by a power
    domain
  cpufreq: qcom: Add support for qcs404 on nvmem driver
  dt-bindings: opp: Add qcom-opp bindings with properties needed for CPR
  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

Sricharan R (2):
  dt-bindings: cpufreq: Re-organise kryo cpufreq to use it for other
    nvmem based qcom socs
  cpufreq: qcom: Re-organise kryo cpufreq to use it for other nvmem
    based qcom socs

 ...ryo-cpufreq.txt => qcom-nvmem-cpufreq.txt} |  125 +-
 .../devicetree/bindings/opp/qcom-opp.txt      |   19 +
 .../bindings/power/avs/qcom,cpr.txt           |  193 ++
 MAINTAINERS                                   |   13 +-
 arch/arm64/boot/dts/qcom/qcs404.dtsi          |  145 +-
 arch/arm64/configs/defconfig                  |    2 +
 drivers/cpufreq/Kconfig.arm                   |    4 +-
 drivers/cpufreq/Makefile                      |    2 +-
 drivers/cpufreq/cpufreq-dt-platdev.c          |    1 +
 drivers/cpufreq/qcom-cpufreq-kryo.c           |  249 ---
 drivers/cpufreq/qcom-cpufreq-nvmem.c          |  351 ++++
 drivers/power/avs/Kconfig                     |   15 +
 drivers/power/avs/Makefile                    |    1 +
 drivers/power/avs/qcom-cpr.c                  | 1839 +++++++++++++++++
 14 files changed, 2692 insertions(+), 267 deletions(-)
 rename Documentation/devicetree/bindings/opp/{kryo-cpufreq.txt => qcom-nvmem-cpufreq.txt} (87%)
 create mode 100644 Documentation/devicetree/bindings/opp/qcom-opp.txt
 create mode 100644 Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
 delete mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
 create mode 100644 drivers/cpufreq/qcom-cpufreq-nvmem.c
 create mode 100644 drivers/power/avs/qcom-cpr.c

-- 
2.21.0

[PATCH 01/13] dt-bindings: cpufreq: Re-organise kryo cpufreq to use it for other nvmem based qcom socs

[Niklas Cassel]

From: Sricharan R <sricharan@codeaurora.org>

The kryo cpufreq driver reads the nvmem cell and uses that data to
populate the opps. There are other qcom cpufreq socs like krait which
does similar thing. Except for the interpretation of the read data,
rest of the driver is same for both the cases. So pull the common things
out for reuse.

Signed-off-by: Sricharan R <sricharan@codeaurora.org>
[niklas.cassel@linaro.org: split dt-binding into a separate patch and
do not rename the compatible string.]
Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
---
Changes since RFC:
-Made DT bindings a separate patch.
-Keep the original compatible string, since renaming it breaks DT
backwards compatibility.

 .../opp/{kryo-cpufreq.txt => qcom-nvmem-cpufreq.txt}   | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
 rename Documentation/devicetree/bindings/opp/{kryo-cpufreq.txt => qcom-nvmem-cpufreq.txt} (98%)

diff --git a/Documentation/devicetree/bindings/opp/kryo-cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
similarity index 98%
rename from Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
rename to Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
index c2127b96805a..198441e80ba8 100644
--- a/Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
+++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
@@ -1,13 +1,13 @@
-Qualcomm Technologies, Inc. KRYO CPUFreq and OPP bindings
+Qualcomm Technologies, Inc. NVMEM CPUFreq and OPP bindings
 ===================================
 
-In Certain Qualcomm Technologies, Inc. SoCs like apq8096 and msm8996
-that have KRYO processors, the CPU ferequencies subset and voltage value
-of each OPP varies based on the silicon variant in use.
+In Certain Qualcomm Technologies, Inc. SoCs like apq8096 and msm8996,
+the CPU frequencies subset and voltage value of each OPP varies based on
+the silicon variant in use.
 Qualcomm Technologies, Inc. Process Voltage Scaling Tables
 defines the voltage and frequency value based on the msm-id in SMEM
 and speedbin blown in the efuse combination.
-The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC
+The qcom-cpufreq-nvmem driver reads the msm-id and efuse value from the SoC
 to provide the OPP framework with required information (existing HW bitmap).
 This is used to determine the voltage and frequency value for each OPP of
 operating-points-v2 table when it is parsed by the OPP framework.
-- 
2.21.0

[PATCH 03/13] dt-bindings: cpufreq: qcom-nvmem: Make speedbin related properties optional

[Niklas Cassel]

Not all Qualcomm platforms need to care about the speedbin efuse,
nor the value blown into the speedbin efuse.
Therefore, make the nvmem-cells and opp-supported-hw properties
optional.

Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
---
 Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
index 198441e80ba8..c5ea8b90e35d 100644
--- a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
+++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
@@ -20,6 +20,10 @@ In 'cpus' nodes:
 In 'operating-points-v2' table:
 - compatible: Should be
 	- 'operating-points-v2-kryo-cpu' for apq8096 and msm8996.
+
+Optional properties:
+--------------------
+In 'operating-points-v2' table:
 - nvmem-cells: A phandle pointing to a nvmem-cells node representing the
 		efuse registers that has information about the
 		speedbin that is used to select the right frequency/voltage
-- 
2.21.0

[PATCH 05/13] dt-bindings: cpufreq: qcom-nvmem: Support pstates provided by a power domain

[Niklas Cassel]

Some Qualcomm SoCs have support for Core Power Reduction (CPR).
On these platforms, we need to attach to the power domain provider
providing the performance states, so that the leaky device (the CPU)
can configure the performance states (which represent different
CPU clock frequencies).

Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
---
 .../bindings/opp/qcom-nvmem-cpufreq.txt       | 111 ++++++++++++++++++
 1 file changed, 111 insertions(+)

diff --git a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
index c5ea8b90e35d..e19a95318e98 100644
--- a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
+++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
@@ -23,6 +23,15 @@ In 'operating-points-v2' table:
 
 Optional properties:
 --------------------
+In 'cpus' nodes:
+- power-domains: A phandle pointing to the PM domain specifier which provides
+		the performance states available for active state management.
+		Please refer to the power-domains bindings
+		Documentation/devicetree/bindings/power/power_domain.txt
+		and also examples below.
+- power-domain-names: Should be
+	- 'cpr' for qcs404.
+
 In 'operating-points-v2' table:
 - nvmem-cells: A phandle pointing to a nvmem-cells node representing the
 		efuse registers that has information about the
@@ -682,3 +691,105 @@ soc {
 		};
 	};
 };
+
+Example 2:
+---------
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		CPU0: cpu@100 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x100>;
+			....
+			clocks = <&apcs_glb>;
+			operating-points-v2 = <&cpu_opp_table>;
+			power-domains = <&cprpd>;
+			power-domain-names = "cpr";
+		};
+
+		CPU1: cpu@101 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x101>;
+			....
+			clocks = <&apcs_glb>;
+			operating-points-v2 = <&cpu_opp_table>;
+			power-domains = <&cprpd>;
+			power-domain-names = "cpr";
+		};
+
+		CPU2: cpu@102 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x102>;
+			....
+			clocks = <&apcs_glb>;
+			operating-points-v2 = <&cpu_opp_table>;
+			power-domains = <&cprpd>;
+			power-domain-names = "cpr";
+		};
+
+		CPU3: cpu@103 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x103>;
+			....
+			clocks = <&apcs_glb>;
+			operating-points-v2 = <&cpu_opp_table>;
+			power-domains = <&cprpd>;
+			power-domain-names = "cpr";
+		};
+	};
+
+	cpu_opp_table: cpu-opp-table {
+		compatible = "operating-points-v2-kryo-cpu";
+		opp-shared;
+
+		opp-1094400000 {
+			opp-hz = /bits/ 64 <1094400000>;
+			required-opps = <&cpr_opp1>;
+		};
+		opp-1248000000 {
+			opp-hz = /bits/ 64 <1248000000>;
+			required-opps = <&cpr_opp2>;
+		};
+		opp-1401600000 {
+			opp-hz = /bits/ 64 <1401600000>;
+			required-opps = <&cpr_opp3>;
+		};
+	};
+
+	cpr_opp_table: cpr-opp-table {
+		compatible = "operating-points-v2-qcom-level";
+
+		cpr_opp1: opp1 {
+			opp-level = <1>;
+			....
+		};
+		cpr_opp2: opp2 {
+			opp-level = <2>;
+			....
+		};
+		cpr_opp3: opp3 {
+			opp-level = <3>;
+			....
+		};
+	};
+
+....
+
+soc {
+....
+	cprpd: cpr@b018000 {
+		compatible = "qcom,qcs404-cpr", "qcom,cpr";
+		reg = <0x0b018000 0x1000>;
+		....
+		vdd-apc-supply = <&pms405_s3>;
+		#power-domain-cells = <0>;
+		operating-points-v2 = <&cpr_opp_table>;
+		....
+	};
+};
-- 
2.21.0

[PATCH 08/13] dt-bindings: opp: Add qcom-opp bindings with properties needed for CPR

[Niklas Cassel]

Add qcom-opp bindings with properties needed for Core Power Reduction
(CPR).

CPR is included in a great variety of Qualcomm SoCs, e.g. msm8916 and
msm8996. CPR was first introduced in msm8974.

Changes since RFC:
-Removed opp-hz. It is already an optional property in opp.txt
so no need to specify it with the same definition here.

Co-developed-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
---
 .../devicetree/bindings/opp/qcom-opp.txt      | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/opp/qcom-opp.txt

diff --git a/Documentation/devicetree/bindings/opp/qcom-opp.txt b/Documentation/devicetree/bindings/opp/qcom-opp.txt
new file mode 100644
index 000000000000..f204685d029c
--- /dev/null
+++ b/Documentation/devicetree/bindings/opp/qcom-opp.txt
@@ -0,0 +1,19 @@
+Qualcomm OPP bindings to describe OPP nodes
+
+The bindings are based on top of the operating-points-v2 bindings
+described in Documentation/devicetree/bindings/opp/opp.txt
+Additional properties are described below.
+
+* OPP Table Node
+
+Required properties:
+- compatible: Allow OPPs to express their compatibility. It should be:
+  "operating-points-v2-qcom-level"
+
+* OPP Node
+
+Optional properties:
+- qcom,opp-fuse-level: A positive value representing the fuse corner/level
+  associated with this OPP node. Sometimes several corners/levels shares
+  a certain fuse corner/level. A fuse corner/level contains e.g. ref uV,
+  min uV, and max uV.
-- 
2.21.0

[PATCH 09/13] dt-bindings: power: avs: Add support for CPR (Core Power Reduction)

[Niklas Cassel]

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

Changes since RFC:
-Make compatible string SoC specific.
-Changed interrupt definition.
-Use clock binding for reference clock.
-Clarified qcom,vdd-apc-step-up-limit description.
-Added missing properties.
-Updated the example.

Co-developed-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
---
 .../bindings/power/avs/qcom,cpr.txt           | 193 ++++++++++++++++++
 1 file changed, 193 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..93be67fa8f38
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
@@ -0,0 +1,193 @@
+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.
+
+- qcom,cpr-timer-delay-us:
+	Usage: required
+	Value type: <u32>
+	Definition: delay in uS for the timer interval
+
+- qcom,cpr-timer-cons-up:
+	Usage: required
+	Value type: <u32>
+	Definition: Consecutive number of timer intervals, or units of
+		    qcom,cpr-timer-delay-us, that occur before issuing an up
+		    interrupt
+
+- qcom,cpr-timer-cons-down:
+	Usage: required
+	Value type: <u32>
+	Definition: Consecutive number of timer intervals, or units of
+		    qcom,cpr-timer-delay-us, that occur before issuing a down
+		    interrupt
+
+- qcom,cpr-up-threshold:
+	Usage: optional
+	Value type: <u32>
+	Definition: The threshold for CPR to issue interrupt when error_steps
+		    is greater than it when stepping up
+
+- qcom,cpr-down-threshold:
+	Usage: optional
+	Value type: <u32>
+	Definition: The threshold for CPR to issue interrupt when error_steps
+		    is greater than it when stepping down
+
+- qcom,cpr-idle-clocks:
+	Usage: optional
+	Value type: <u32>
+	Definition: Idle clock cycles ring oscillator can be in
+
+- qcom,cpr-gcnt-us:
+	Usage: required
+	Value type: <u32>
+	Definition: The time for gate count in uS
+
+- qcom,vdd-apc-step-up-limit:
+	Usage: required
+	Value type: <u32>
+	Definition: Limit of number of vdd-apc-supply regulator steps for
+		    scaling up
+
+- qcom,vdd-apc-step-down-limit:
+	Usage: required
+	Value type: <u32>
+	Definition: Limit of number of vdd-apc-supply regulator steps for
+		    scaling down
+
+Example:
+
+	cpr_opp_table: cpr-opp-table {
+		compatible = "operating-points-v2-qcom-level";
+
+		cpr_opp1: opp1 {
+			opp-level = <1>;
+			....
+		};
+		cpr_opp2: opp2 {
+			opp-level = <2>;
+			....
+		};
+		cpr_opp3: opp3 {
+			opp-level = <3>;
+			....
+		};
+	};
+
+	cpr@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";
+
+		qcom,cpr-timer-delay-us = <5000>;
+		qcom,cpr-timer-cons-up = <0>;
+		qcom,cpr-timer-cons-down = <2>;
+		qcom,cpr-up-threshold = <1>;
+		qcom,cpr-down-threshold = <3>;
+		qcom,cpr-idle-clocks = <15>;
+		qcom,cpr-gcnt-us = <1>;
+		qcom,vdd-apc-step-up-limit = <1>;
+		qcom,vdd-apc-step-down-limit = <1>;
+	};
-- 
2.21.0

[PATCH 11/13] arm64: dts: qcom: qcs404: Add CPR and populate OPP table

[Niklas Cassel]

Add CPR and populate OPP table.

Co-developed-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
---
 arch/arm64/boot/dts/qcom/qcs404.dtsi | 145 +++++++++++++++++++++++++--
 1 file changed, 137 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index ff9198740431..5b6276c3ec42 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -38,7 +38,8 @@
 			#cooling-cells = <2>;
 			clocks = <&apcs_glb>;
 			operating-points-v2 = <&cpu_opp_table>;
-			cpu-supply = <&pms405_s3>;
+			power-domains = <&cprpd>;
+			power-domain-names = "cpr";
 		};
 
 		CPU1: cpu@101 {
@@ -51,7 +52,8 @@
 			#cooling-cells = <2>;
 			clocks = <&apcs_glb>;
 			operating-points-v2 = <&cpu_opp_table>;
-			cpu-supply = <&pms405_s3>;
+			power-domains = <&cprpd>;
+			power-domain-names = "cpr";
 		};
 
 		CPU2: cpu@102 {
@@ -64,7 +66,8 @@
 			#cooling-cells = <2>;
 			clocks = <&apcs_glb>;
 			operating-points-v2 = <&cpu_opp_table>;
-			cpu-supply = <&pms405_s3>;
+			power-domains = <&cprpd>;
+			power-domain-names = "cpr";
 		};
 
 		CPU3: cpu@103 {
@@ -77,7 +80,8 @@
 			#cooling-cells = <2>;
 			clocks = <&apcs_glb>;
 			operating-points-v2 = <&cpu_opp_table>;
-			cpu-supply = <&pms405_s3>;
+			power-domains = <&cprpd>;
+			power-domain-names = "cpr";
 		};
 
 		L2_0: l2-cache {
@@ -101,20 +105,40 @@
 	};
 
 	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>;
+			opp-hz = /bits/ 64 <1094400000>;
+		};
+		cpr_opp2: opp2 {
+			opp-level = <2>;
+			qcom,opp-fuse-level = <2>;
+			opp-hz = /bits/ 64 <1248000000>;
+		};
+		cpr_opp3: opp3 {
+			opp-level = <3>;
+			qcom,opp-fuse-level = <3>;
+			opp-hz = /bits/ 64 <1401600000>;
 		};
 	};
 
@@ -294,6 +318,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 {
@@ -901,6 +981,55 @@
 			clock-names = "xo";
 		};
 
+		cprpd: cpr@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";
+
+			qcom,cpr-timer-delay-us = <5000>;
+			qcom,cpr-timer-cons-up = <0>;
+			qcom,cpr-timer-cons-down = <2>;
+			qcom,cpr-up-threshold = <1>;
+			qcom,cpr-down-threshold = <3>;
+			qcom,cpr-idle-clocks = <15>;
+			qcom,cpr-gcnt-us = <1>;
+			qcom,vdd-apc-step-up-limit = <1>;
+			qcom,vdd-apc-step-down-limit = <1>;
+		};
+
 		timer@b120000 {
 			#address-cells = <1>;
 			#size-cells = <1>;
-- 
2.21.0

Re: [PATCH 01/13] dt-bindings: cpufreq: Re-organise kryo cpufreq to use it for other nvmem based qcom socs

[Ilia Lin]

Reviewed-by: Ilia Lin <ilia.lin@kernel.org>

On Fri, Jul 5, 2019 at 12:57 PM Niklas Cassel <niklas.cassel@linaro.org> wrote:
>
> From: Sricharan R <sricharan@codeaurora.org>
>
> The kryo cpufreq driver reads the nvmem cell and uses that data to
> populate the opps. There are other qcom cpufreq socs like krait which
> does similar thing. Except for the interpretation of the read data,
> rest of the driver is same for both the cases. So pull the common things
> out for reuse.
>
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> [niklas.cassel@linaro.org: split dt-binding into a separate patch and
> do not rename the compatible string.]
> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> ---
> Changes since RFC:
> -Made DT bindings a separate patch.
> -Keep the original compatible string, since renaming it breaks DT
> backwards compatibility.
>
>  .../opp/{kryo-cpufreq.txt => qcom-nvmem-cpufreq.txt}   | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>  rename Documentation/devicetree/bindings/opp/{kryo-cpufreq.txt => qcom-nvmem-cpufreq.txt} (98%)
>
> diff --git a/Documentation/devicetree/bindings/opp/kryo-cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> similarity index 98%
> rename from Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
> rename to Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> index c2127b96805a..198441e80ba8 100644
> --- a/Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
> +++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> @@ -1,13 +1,13 @@
> -Qualcomm Technologies, Inc. KRYO CPUFreq and OPP bindings
> +Qualcomm Technologies, Inc. NVMEM CPUFreq and OPP bindings
>  ===================================
>
> -In Certain Qualcomm Technologies, Inc. SoCs like apq8096 and msm8996
> -that have KRYO processors, the CPU ferequencies subset and voltage value
> -of each OPP varies based on the silicon variant in use.
> +In Certain Qualcomm Technologies, Inc. SoCs like apq8096 and msm8996,
> +the CPU frequencies subset and voltage value of each OPP varies based on
> +the silicon variant in use.
>  Qualcomm Technologies, Inc. Process Voltage Scaling Tables
>  defines the voltage and frequency value based on the msm-id in SMEM
>  and speedbin blown in the efuse combination.
> -The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC
> +The qcom-cpufreq-nvmem driver reads the msm-id and efuse value from the SoC
>  to provide the OPP framework with required information (existing HW bitmap).
>  This is used to determine the voltage and frequency value for each OPP of
>  operating-points-v2 table when it is parsed by the OPP framework.
> --
> 2.21.0
>

Re: [PATCH 03/13] dt-bindings: cpufreq: qcom-nvmem: Make speedbin related properties optional

[Ilia Lin]

Reviewed-by: Ilia Lin <ilia.lin@kernel.org>

On Fri, Jul 5, 2019 at 12:58 PM Niklas Cassel <niklas.cassel@linaro.org> wrote:
>
> Not all Qualcomm platforms need to care about the speedbin efuse,
> nor the value blown into the speedbin efuse.
> Therefore, make the nvmem-cells and opp-supported-hw properties
> optional.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> ---
>  Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> index 198441e80ba8..c5ea8b90e35d 100644
> --- a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> +++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> @@ -20,6 +20,10 @@ In 'cpus' nodes:
>  In 'operating-points-v2' table:
>  - compatible: Should be
>         - 'operating-points-v2-kryo-cpu' for apq8096 and msm8996.
> +
> +Optional properties:
> +--------------------
> +In 'operating-points-v2' table:
>  - nvmem-cells: A phandle pointing to a nvmem-cells node representing the
>                 efuse registers that has information about the
>                 speedbin that is used to select the right frequency/voltage
> --
> 2.21.0
>

Re: [PATCH 11/13] arm64: dts: qcom: qcs404: Add CPR and populate OPP table

[Viresh Kumar]

On 05-07-19, 11:57, Niklas Cassel wrote:
> diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
>  	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>;
> +			opp-hz = /bits/ 64 <1094400000>;
> +		};
> +		cpr_opp2: opp2 {
> +			opp-level = <2>;
> +			qcom,opp-fuse-level = <2>;
> +			opp-hz = /bits/ 64 <1248000000>;
> +		};
> +		cpr_opp3: opp3 {
> +			opp-level = <3>;
> +			qcom,opp-fuse-level = <3>;
> +			opp-hz = /bits/ 64 <1401600000>;
>  		};
>  	};

- Do we ever have cases more complex than this for this version of CPR ?

- What about multiple devices with same CPR table, not big LITTLE
  CPUs, but other devices like two different type of IO devices ? What
  will we do with opp-hz in those cases ?

- If there are no such cases, can we live without opp-hz being used
  here and reverse-engineer the highest frequency by looking directly
  at CPUs OPP table ? i.e. by looking at required-opps field.

-- 
viresh

Re: [PATCH 11/13] arm64: dts: qcom: qcs404: Add CPR and populate OPP table

[Niklas Cassel]

On Wed, Jul 10, 2019 at 02:33:03PM +0530, Viresh Kumar wrote:
> On 05-07-19, 11:57, Niklas Cassel wrote:
> > diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> >  	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>;
> > +			opp-hz = /bits/ 64 <1094400000>;
> > +		};
> > +		cpr_opp2: opp2 {
> > +			opp-level = <2>;
> > +			qcom,opp-fuse-level = <2>;
> > +			opp-hz = /bits/ 64 <1248000000>;
> > +		};
> > +		cpr_opp3: opp3 {
> > +			opp-level = <3>;
> > +			qcom,opp-fuse-level = <3>;
> > +			opp-hz = /bits/ 64 <1401600000>;
> >  		};
> >  	};
> 
> - Do we ever have cases more complex than this for this version of CPR ?

For e.g. CPR on msm8916, we will have 7 different frequencies in the CPU
OPP table, but only 3 OPPs in the CPR OPP table.

Each of the 7 OPPs in the CPU OPP table will have a required-opps that
points to an OPP in the CPR OPP table.

On certain msm8916:s, the speedbin efuse will limit us to only have 6
OPPs in the CPU OPP table, but the required-opps are still the same.

So I would say that it is just slightly more complex..

> 
> - What about multiple devices with same CPR table, not big LITTLE
>   CPUs, but other devices like two different type of IO devices ? What
>   will we do with opp-hz in those cases ?

On all SoCs where there is a CPR for e.g. GPU, there is an additional
CPR hardware block, so then there will also be an additional CPR OPP
table. So I don't think that this will be a problem.

> 
> - If there are no such cases, can we live without opp-hz being used
>   here and reverse-engineer the highest frequency by looking directly
>   at CPUs OPP table ? i.e. by looking at required-opps field.

This was actually my initial thought when talking to you 6+ months ago.
However, the problem was that, from the CPR drivers' perspective, it
only sees the CPR OPP table.


So this is the order things are called,
from qcom-cpufreq-nvmem.c perspective:

1) dev_pm_opp_set_supported_hw()

2) dev_pm_opp_attach_genpd() ->
which results in
int cpr_pd_attach_dev(struct generic_pm_domain *domain,
		      struct device *dev)
being called.
This callback is inside the CPR driver, and here we have the
CPU's (genpd virtual) struct device, and this is where we would like to
know the opp-hz.
The problem here is that:
[    3.114979] cpr_pd_attach_dev: dev_pm_opp_get_opp_count for dev: genpd:0:cpu0: -19
[    3.119610] cpr_pd_attach_dev: dev_pm_opp_get_opp_count for dev: cpu0: 0
[    3.126489] cpr_pd_attach_dev: dev_pm_opp_get_opp_count for dev: cpr@b018000: 3

While we have the CPR OPP table in the attach callback, we don't
have the CPU OPP table, neither in the CPU struct device or the genpd virtual
struct device.

Since we have called dev_pm_opp_attach_genpd(.., .., &virt_devs) which
attaches an OPP table to the CPU, I would have expected one of them to
be >= 0.
Especially since dev_name(virt_devs[0]) == genpd:0:cpu0

I guess it should still be possible to parse the required-opps manually here,
by iterating the OF nodes, however, we won't be able to use the CPU's struct
opp_table (which is the nice representation of the OF nodes).

Any suggestions?


Kind regards,
Niklas

Re: [PATCH 11/13] arm64: dts: qcom: qcs404: Add CPR and populate OPP table

[Viresh Kumar]

On 15-07-19, 15:24, Niklas Cassel wrote:
> This was actually my initial thought when talking to you 6+ months ago.
> However, the problem was that, from the CPR drivers' perspective, it
> only sees the CPR OPP table.
> 
> 
> So this is the order things are called,
> from qcom-cpufreq-nvmem.c perspective:
> 
> 1) dev_pm_opp_set_supported_hw()
> 
> 2) dev_pm_opp_attach_genpd() ->
> which results in
> int cpr_pd_attach_dev(struct generic_pm_domain *domain,
> 		      struct device *dev)
> being called.
> This callback is inside the CPR driver, and here we have the
> CPU's (genpd virtual) struct device, and this is where we would like to
> know the opp-hz.
> The problem here is that:
> [    3.114979] cpr_pd_attach_dev: dev_pm_opp_get_opp_count for dev: genpd:0:cpu0: -19
> [    3.119610] cpr_pd_attach_dev: dev_pm_opp_get_opp_count for dev: cpu0: 0
> [    3.126489] cpr_pd_attach_dev: dev_pm_opp_get_opp_count for dev: cpr@b018000: 3
> 
> While we have the CPR OPP table in the attach callback, we don't
> have the CPU OPP table, neither in the CPU struct device or the genpd virtual
> struct device.

If you can find CPU's physical number from the virtual device, then
you can do get_cpu_device(X) and then life will be easy ?

> Since we have called dev_pm_opp_attach_genpd(.., .., &virt_devs) which
> attaches an OPP table to the CPU, I would have expected one of them to
> be >= 0.
> Especially since dev_name(virt_devs[0]) == genpd:0:cpu0
> 
> I guess it should still be possible to parse the required-opps manually here,
> by iterating the OF nodes, however, we won't be able to use the CPU's struct
> opp_table (which is the nice representation of the OF nodes).
> 
> Any suggestions?

-- 
viresh

Re: [PATCH 11/13] arm64: dts: qcom: qcs404: Add CPR and populate OPP table

[Niklas Cassel]

On Tue, Jul 16, 2019 at 04:04:36PM +0530, Viresh Kumar wrote:
> On 15-07-19, 15:24, Niklas Cassel wrote:
> > This was actually my initial thought when talking to you 6+ months ago.
> > However, the problem was that, from the CPR drivers' perspective, it
> > only sees the CPR OPP table.
> > 
> > 
> > So this is the order things are called,
> > from qcom-cpufreq-nvmem.c perspective:
> > 
> > 1) dev_pm_opp_set_supported_hw()
> > 
> > 2) dev_pm_opp_attach_genpd() ->
> > which results in
> > int cpr_pd_attach_dev(struct generic_pm_domain *domain,
> > 		      struct device *dev)
> > being called.
> > This callback is inside the CPR driver, and here we have the
> > CPU's (genpd virtual) struct device, and this is where we would like to
> > know the opp-hz.
> > The problem here is that:
> > [    3.114979] cpr_pd_attach_dev: dev_pm_opp_get_opp_count for dev: genpd:0:cpu0: -19
> > [    3.119610] cpr_pd_attach_dev: dev_pm_opp_get_opp_count for dev: cpu0: 0

Here I cheated and simply used get_cpu_device(0).

Since I cheated, I used get_cpu_device(0) always,
so even when CPU1,CPU2,CPU3 is attached, dev_pm_opp_get_opp_count(cpu0) is
still 0.

I added a print in
[    3.836533] cpr_set_performance: number of OPPs for dev: cpu0: 3

And there I can see that OPP count is 3, so it appears that with the
current code, we need to wait until cpufreq-dt.c:cpufreq_init()
has been called, maybe dev_pm_opp_of_cpumask_add_table() needs
to be called before dev_pm_opp_get_opp_count(cpu0) actually returns 3.

cpufreq_init() is called by platform_device_register_simple("cpufreq-dt", -1,
                                                          NULL, 0);
which is called after dev_pm_opp_attach_genpd().

What I don't understand is that dev_pm_opp_attach_genpd() actually returns
a OPP table. So why do we need to wait for dev_pm_opp_of_cpumask_add_table(),
before either dev_pm_opp_get_opp_count(cpu0) or
dev_pm_opp_get_opp_count(genpd_virtdev_for_cpu0) returns 3?



> > [    3.126489] cpr_pd_attach_dev: dev_pm_opp_get_opp_count for dev: cpr@b018000: 3
> > 
> > While we have the CPR OPP table in the attach callback, we don't
> > have the CPU OPP table, neither in the CPU struct device or the genpd virtual
> > struct device.
> 
> If you can find CPU's physical number from the virtual device, then
> you can do get_cpu_device(X) and then life will be easy ?
> 
> > Since we have called dev_pm_opp_attach_genpd(.., .., &virt_devs) which
> > attaches an OPP table to the CPU, I would have expected one of them to
> > be >= 0.
> > Especially since dev_name(virt_devs[0]) == genpd:0:cpu0
> > 
> > I guess it should still be possible to parse the required-opps manually here,
> > by iterating the OF nodes, however, we won't be able to use the CPU's struct
> > opp_table (which is the nice representation of the OF nodes).
> > 
> > Any suggestions?
> 
> -- 
> viresh

Re: [PATCH 11/13] arm64: dts: qcom: qcs404: Add CPR and populate OPP table

[Viresh Kumar]

On 16-07-19, 12:53, Niklas Cassel wrote:
> Here I cheated and simply used get_cpu_device(0).
> 
> Since I cheated, I used get_cpu_device(0) always,
> so even when CPU1,CPU2,CPU3 is attached, dev_pm_opp_get_opp_count(cpu0) is
> still 0.
> 
> I added a print in
> [    3.836533] cpr_set_performance: number of OPPs for dev: cpu0: 3
> 
> And there I can see that OPP count is 3, so it appears that with the
> current code, we need to wait until cpufreq-dt.c:cpufreq_init()
> has been called, maybe dev_pm_opp_of_cpumask_add_table() needs
> to be called before dev_pm_opp_get_opp_count(cpu0) actually returns 3.
> 
> cpufreq_init() is called by platform_device_register_simple("cpufreq-dt", -1,
>                                                           NULL, 0);
> which is called after dev_pm_opp_attach_genpd().
> 
> What I don't understand is that dev_pm_opp_attach_genpd() actually returns
> a OPP table. So why do we need to wait for dev_pm_opp_of_cpumask_add_table(),
> before either dev_pm_opp_get_opp_count(cpu0) or
> dev_pm_opp_get_opp_count(genpd_virtdev_for_cpu0) returns 3?

Ah, I see the problems now. No, cpufreq table can't be available at
this point of time and we aren't going to change that. It is the right
thing to do.

Now, even if the kernel isn't written in a way which works for you, it
isn't right to put more things in DT than required. DT is (should be)
very much independent of the Linux kernel.

So we have to parse DT to find highest frequency for each
required-opp. Best is to put that code in the OPP core and use it from
your driver.

-- 
viresh

Re: [PATCH 11/13] arm64: dts: qcom: qcs404: Add CPR and populate OPP table

[Niklas Cassel]

On Wed, Jul 17, 2019 at 10:19:23AM +0530, Viresh Kumar wrote:
> On 16-07-19, 12:53, Niklas Cassel wrote:
> > Here I cheated and simply used get_cpu_device(0).
> > 
> > Since I cheated, I used get_cpu_device(0) always,
> > so even when CPU1,CPU2,CPU3 is attached, dev_pm_opp_get_opp_count(cpu0) is
> > still 0.
> > 
> > I added a print in
> > [    3.836533] cpr_set_performance: number of OPPs for dev: cpu0: 3
> > 
> > And there I can see that OPP count is 3, so it appears that with the
> > current code, we need to wait until cpufreq-dt.c:cpufreq_init()
> > has been called, maybe dev_pm_opp_of_cpumask_add_table() needs
> > to be called before dev_pm_opp_get_opp_count(cpu0) actually returns 3.
> > 
> > cpufreq_init() is called by platform_device_register_simple("cpufreq-dt", -1,
> >                                                           NULL, 0);
> > which is called after dev_pm_opp_attach_genpd().
> > 
> > What I don't understand is that dev_pm_opp_attach_genpd() actually returns
> > a OPP table. So why do we need to wait for dev_pm_opp_of_cpumask_add_table(),
> > before either dev_pm_opp_get_opp_count(cpu0) or
> > dev_pm_opp_get_opp_count(genpd_virtdev_for_cpu0) returns 3?
> 
> Ah, I see the problems now. No, cpufreq table can't be available at
> this point of time and we aren't going to change that. It is the right
> thing to do.
> 
> Now, even if the kernel isn't written in a way which works for you, it
> isn't right to put more things in DT than required. DT is (should be)
> very much independent of the Linux kernel.
> 
> So we have to parse DT to find highest frequency for each
> required-opp. Best is to put that code in the OPP core and use it from
> your driver.

Hello Viresh,

Could you please have a look at the last two patches here:
https://git.linaro.org/people/niklas.cassel/kernel.git/log/?h=cpr-opp-hz

If you like my proposal then I could send out the first patch (the one to
OPP core) as a real patch (with an improved commit message), and
incorporate the second patch into my CPR patch series when I send out a V2.


Kind regards,
Niklas

Re: [PATCH 11/13] arm64: dts: qcom: qcs404: Add CPR and populate OPP table

[Viresh Kumar]

On 19-07-19, 17:45, Niklas Cassel wrote:
> Hello Viresh,
> 
> Could you please have a look at the last two patches here:
> https://git.linaro.org/people/niklas.cassel/kernel.git/log/?h=cpr-opp-hz

There is no sane way of providing review comments with a link to the
git tree :)

I still had a look and I see that you don't search for max frequency
but just any OPP that has required-opps set to the level u want. Also,
can't there be multiple phandles in required-opps in your case ?

> If you like my proposal then I could send out the first patch (the one to
> OPP core) as a real patch (with an improved commit message), and
> incorporate the second patch into my CPR patch series when I send out a V2.

Send them both in your series only, otherwise the first one is useless
anyway.

-- 
viresh

Re: [PATCH 01/13] dt-bindings: cpufreq: Re-organise kryo cpufreq to use it for other nvmem based qcom socs

[Rob Herring]

On Fri,  5 Jul 2019 11:57:12 +0200, Niklas Cassel wrote:
> From: Sricharan R <sricharan@codeaurora.org>
> 
> The kryo cpufreq driver reads the nvmem cell and uses that data to
> populate the opps. There are other qcom cpufreq socs like krait which
> does similar thing. Except for the interpretation of the read data,
> rest of the driver is same for both the cases. So pull the common things
> out for reuse.
> 
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> [niklas.cassel@linaro.org: split dt-binding into a separate patch and
> do not rename the compatible string.]
> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> ---
> Changes since RFC:
> -Made DT bindings a separate patch.
> -Keep the original compatible string, since renaming it breaks DT
> backwards compatibility.
> 
>  .../opp/{kryo-cpufreq.txt => qcom-nvmem-cpufreq.txt}   | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>  rename Documentation/devicetree/bindings/opp/{kryo-cpufreq.txt => qcom-nvmem-cpufreq.txt} (98%)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 03/13] dt-bindings: cpufreq: qcom-nvmem: Make speedbin related properties optional

[Rob Herring]

On Fri,  5 Jul 2019 11:57:14 +0200, Niklas Cassel wrote:
> Not all Qualcomm platforms need to care about the speedbin efuse,
> nor the value blown into the speedbin efuse.
> Therefore, make the nvmem-cells and opp-supported-hw properties
> optional.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> ---
>  Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 05/13] dt-bindings: cpufreq: qcom-nvmem: Support pstates provided by a power domain

[Rob Herring]

On Fri, Jul 05, 2019 at 11:57:16AM +0200, Niklas Cassel wrote:
> Some Qualcomm SoCs have support for Core Power Reduction (CPR).
> On these platforms, we need to attach to the power domain provider
> providing the performance states, so that the leaky device (the CPU)
> can configure the performance states (which represent different
> CPU clock frequencies).
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> ---
>  .../bindings/opp/qcom-nvmem-cpufreq.txt       | 111 ++++++++++++++++++
>  1 file changed, 111 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> index c5ea8b90e35d..e19a95318e98 100644
> --- a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> +++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> @@ -23,6 +23,15 @@ In 'operating-points-v2' table:
>  
>  Optional properties:
>  --------------------
> +In 'cpus' nodes:

In 'cpus' node or 'cpu' nodes?

> +- power-domains: A phandle pointing to the PM domain specifier which provides
> +		the performance states available for active state management.
> +		Please refer to the power-domains bindings
> +		Documentation/devicetree/bindings/power/power_domain.txt
> +		and also examples below.
> +- power-domain-names: Should be
> +	- 'cpr' for qcs404.
> +
>  In 'operating-points-v2' table:
>  - nvmem-cells: A phandle pointing to a nvmem-cells node representing the
>  		efuse registers that has information about the

Re: [PATCH 08/13] dt-bindings: opp: Add qcom-opp bindings with properties needed for CPR

[Rob Herring]

On Fri, Jul 05, 2019 at 11:57:19AM +0200, Niklas Cassel wrote:
> Add qcom-opp bindings with properties needed for Core Power Reduction
> (CPR).
> 
> CPR is included in a great variety of Qualcomm SoCs, e.g. msm8916 and
> msm8996. CPR was first introduced in msm8974.
> 
> Changes since RFC:
> -Removed opp-hz. It is already an optional property in opp.txt
> so no need to specify it with the same definition here.
> 
> Co-developed-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> ---
>  .../devicetree/bindings/opp/qcom-opp.txt      | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/opp/qcom-opp.txt
> 
> diff --git a/Documentation/devicetree/bindings/opp/qcom-opp.txt b/Documentation/devicetree/bindings/opp/qcom-opp.txt
> new file mode 100644
> index 000000000000..f204685d029c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/opp/qcom-opp.txt
> @@ -0,0 +1,19 @@
> +Qualcomm OPP bindings to describe OPP nodes
> +
> +The bindings are based on top of the operating-points-v2 bindings
> +described in Documentation/devicetree/bindings/opp/opp.txt
> +Additional properties are described below.
> +
> +* OPP Table Node
> +
> +Required properties:
> +- compatible: Allow OPPs to express their compatibility. It should be:
> +  "operating-points-v2-qcom-level"
> +
> +* OPP Node
> +
> +Optional properties:
> +- qcom,opp-fuse-level: A positive value representing the fuse corner/level
> +  associated with this OPP node. Sometimes several corners/levels shares
> +  a certain fuse corner/level. A fuse corner/level contains e.g. ref uV,
> +  min uV, and max uV.

Doesn't match the example in patch 5.

> -- 
> 2.21.0
> 

Re: [PATCH 09/13] dt-bindings: power: avs: Add support for CPR (Core Power Reduction)

[Rob Herring]

On Fri,  5 Jul 2019 11:57:20 +0200, Niklas Cassel wrote:
> Add DT bindings to describe the CPR HW found on certain Qualcomm SoCs.
> 
> Changes since RFC:
> -Make compatible string SoC specific.
> -Changed interrupt definition.
> -Use clock binding for reference clock.
> -Clarified qcom,vdd-apc-step-up-limit description.
> -Added missing properties.
> -Updated the example.
> 
> Co-developed-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
> ---
>  .../bindings/power/avs/qcom,cpr.txt           | 193 ++++++++++++++++++
>  1 file changed, 193 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 11/13] arm64: dts: qcom: qcs404: Add CPR and populate OPP table

[Niklas Cassel]

On Tue, Jul 23, 2019 at 07:26:35AM +0530, Viresh Kumar wrote:
> On 19-07-19, 17:45, Niklas Cassel wrote:
> > Hello Viresh,
> > 
> > Could you please have a look at the last two patches here:
> > https://git.linaro.org/people/niklas.cassel/kernel.git/log/?h=cpr-opp-hz
> 
> There is no sane way of providing review comments with a link to the
> git tree :)
> 
> I still had a look and I see that you don't search for max frequency
> but just any OPP that has required-opps set to the level u want. Also,
> can't there be multiple phandles in required-opps in your case ?

For each OPP in the CPR OPP table, we need three things,
opp-level, qcom,fuse-level and opp-hz.
The first two can simply be parsed from the OPP node
itself while iterating the CPR OPP table.
The opp-hz has to be fetched from the CPU OPP table.

Several OPPs might have the same qcom,fuse-level value.
However, they will have unique opp-level values and unique
opp-hz values. Each opp-level has a matching opp-hz.

required-opps is basically a connection between a opp-hz
(CPU OPP table) and and a opp-level (CPR OPP table).

So there will be only one match. No need to search for
max frequency.

I think you are confusing this with something else.
The CPR hardware has to be programmed with the highest
frequency for each qcom,fuse-corner.
This is done here:
https://git.linaro.org/people/niklas.cassel/kernel.git/tree/drivers/power/avs/qcom-cpr.c?h=cpr-full#n1219
by saving the highest frequency for each fuse level
while iterating the OPP table.


There can be only one phandle in the required-opps in my case,
this is one of the reasons why I prefer implementing it in the
CPR driver. If it were to be implemented in OPP core, it probably
has to handle multiple phandles.

> 
> > If you like my proposal then I could send out the first patch (the one to
> > OPP core) as a real patch (with an improved commit message), and
> > incorporate the second patch into my CPR patch series when I send out a V2.
> 
> Send them both in your series only, otherwise the first one is useless
> anyway.

Ok, will do.


Kind regards,
Niklas