Skip to contentSkip to conversation

CPU scaling support for msm8996

PATCH Superseded Ilia Lin 6 participants 50 messages May 17, 2018 – May 23, 2018

Subsystems (9)

v8 of 12 Superseded Latest is v12 →

lore
Share & tools
lore

Ilia Lin

author
#0 lore raw

v8 cover CPU scaling support for msm8996

[v8]
* Reordered the patch series into 4 groups
* Addressed comments from Amit about the comments and commit messages
* Addressed comments from Amit and Viresh about the resourses deallocation

[v7]
* Addressed comments from Viresh about resourses deallocation and DT compatible

[v6]
* Addressed comments from Viresh about:
** Comments style
** Kconfig bool instead of tristate
** DT and documentation style
** Resourses deallocation on an error
** Typos

[v5]
* Rebased
* Addressed comments from Bjorn about SPDX style,
functions and parameters naming
* Addressed comments from Viresh DT properties and style, comments style, resourses deallocation, documentation placement
* Addressed comments from Sricharan about unnessesary include
* Addressed comments from Nicolas
* Addressed comments from Rob about the commit messages and acks
* Addressed comments from Mark

[v4]
* Adressed all comments from Stephen
* Added CPU regulator support
* Added qcom-cpufreq-kryo driver

[v3]
* Rebased on top of the latest PLL driver changes
* Addressed comment from Rob Herring for bindings

[v2]
* Addressed comments from Rob Herring for bindings
* Addressed comments from Mark Rutland for memory barrier
* Addressed comments from Julien Thierry for clock reenabling condition
* Tuned the HW configuration for clock frequencies below 600MHz

The series contains 4 groups of patches, which can be merged in the chain order, while each group depends on all the previous groups.
(However, Only SOC, or SOC and Clocks, or SOC and Clocks and Cpufreq will work)

SOC (1/15):
Extracts the kryo l2 accessors driver from the QCOM PMU driver

Clocks (2/15-9/15):
This series adds support for the CPU clocks on msm8996 devices. The driver uses the existing PLL drivers and is required to control the CPU frequency scaling on the MSM8996.

Cpufreq (10/15-12/15):
The qcom-cpufreq-kryo driver is aimed to support different SOC versions. The driver reads eFuse information and chooses the required OPP subset by passing the OPP supported-hw parameter.

Regulators (13/15-15/15):
Added SAW regulator support to the SPMI regulator driver. The SAW regulators will be controlled through special CPU registers instead of direct SPMI accesses.

Ilia Lin (13):
soc: qcom: Separate kryo l2 accessors from PMU driver
clk: Use devm_ in the register fixed factor clock
clk: qcom: Add CPU clock driver for msm8996
dt-bindings: clk: qcom: Add bindings for CPU clock for msm8996
clk: qcom: cpu-8996: Add support to switch below 600Mhz
clk: qcom: Add ACD path to CPU clock driver for msm8996
dt: qcom: Add opp and thermal to the msm8996
cpufreq: Add Kryo CPU scaling driver
dt-bindings: cpufreq: Document operating-points-v2-kryo-cpu
dt: qcom: Add qcom-cpufreq-kryo driver configuration
regulator: qcom_spmi: Add support for SAW
dt-bindings: qcom_spmi: Document SAW support
dt: qcom: Add SAW regulator for 8x96 CPUs

Rajendra Nayak (2):
clk: qcom: Make clk_alpha_pll_configure available to modules
clk: qcom: cpu-8996: Add support to switch to alternate PLL

 .../devicetree/bindings/clock/qcom,kryocc.txt      |  17 +
 .../devicetree/bindings/opp/kryo-cpufreq.txt       | 680 +++++++++++++++++++++
 .../bindings/regulator/qcom,spmi-regulator.txt     |  45 ++
 arch/arm64/boot/dts/qcom/apq8096-db820c.dts        |   2 +-
 arch/arm64/boot/dts/qcom/msm8996.dtsi              | 665 +++++++++++++++++++-
 drivers/clk/clk-fixed-factor.c                     |   2 +-
 drivers/clk/qcom/Kconfig                           |   9 +
 drivers/clk/qcom/Makefile                          |   1 +
 drivers/clk/qcom/clk-alpha-pll.c                   |   1 +
 drivers/clk/qcom/clk-alpha-pll.h                   |   6 +
 drivers/clk/qcom/clk-cpu-8996.c                    | 510 ++++++++++++++++
 drivers/cpufreq/Kconfig.arm                        |  10 +
 drivers/cpufreq/Makefile                           |   1 +
 drivers/cpufreq/cpufreq-dt-platdev.c               |   3 +
 drivers/cpufreq/qcom-cpufreq-kryo.c                | 166 +++++
 drivers/perf/Kconfig                               |   1 +
 drivers/perf/qcom_l2_pmu.c                         |  90 +--
 drivers/regulator/qcom_spmi-regulator.c            | 133 +++-
 drivers/soc/qcom/Kconfig                           |   3 +
 drivers/soc/qcom/Makefile                          |   1 +
 drivers/soc/qcom/kryo-l2-accessors.c               |  56 ++
 include/soc/qcom/kryo-l2-accessors.h               |  12 +
 22 files changed, 2324 insertions(+), 90 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,kryocc.txt
 create mode 100644 Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
 create mode 100644 drivers/clk/qcom/clk-cpu-8996.c
 create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
 create mode 100644 drivers/soc/qcom/kryo-l2-accessors.c
 create mode 100644 include/soc/qcom/kryo-l2-accessors.h
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH v8 1/15 soc: qcom: Separate kryo l2 accessors from PMU driver

The driver provides kernel level API for other drivers to access the MSM8996 L2 cache registers. Separating the L2 access code from the PMU driver and making it public to allow other drivers use it. The accesses must be separated with a single spinlock, maintained in this driver.

---
 drivers/perf/Kconfig                 |  1 +
 drivers/perf/qcom_l2_pmu.c           | 90 ++++++++++--------------------------
 drivers/soc/qcom/Kconfig             |  3 ++
 drivers/soc/qcom/Makefile            |  1 +
 drivers/soc/qcom/kryo-l2-accessors.c | 56 ++++++++++++++++++++++
 include/soc/qcom/kryo-l2-accessors.h | 12 +++++
 6 files changed, 97 insertions(+), 66 deletions(-)
 create mode 100644 drivers/soc/qcom/kryo-l2-accessors.c
 create mode 100644 include/soc/qcom/kryo-l2-accessors.h
drivers/perf/Kconfig +1 -0
index 28bb5a0..561252a 100644
@@ -69,6 +69,7 @@ config HISI_PMU
 config QCOM_L2_PMU
 	bool "Qualcomm Technologies L2-cache PMU"
 	depends on ARCH_QCOM && ARM64 && ACPI
+	select QCOM_KRYO_L2_ACCESSORS
 	  help
 	  Provides support for the L2 cache performance monitor unit (PMU)
 	  in Qualcomm Technologies processors.
drivers/perf/qcom_l2_pmu.c +24 -66

192 lines not on this page · raw message

drivers/soc/qcom/Kconfig +3 -0
index 7093fe7..0567dff 100644
@@ -39,6 +39,9 @@ config QCOM_GSBI
           functions for connecting the underlying serial UART, SPI, and I2C
           devices to the output pins.
 
+config QCOM_KRYO_L2_ACCESSORS
+       bool
+
 config QCOM_MDT_LOADER
 	tristate
 	select QCOM_SCM
drivers/soc/qcom/Makefile +1 -0
index cbf414c..e4d3f5a 100644
@@ -14,3 +14,4 @@ obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
 obj-$(CONFIG_QCOM_SMP2P)	+= smp2p.o
 obj-$(CONFIG_QCOM_SMSM)	+= smsm.o
 obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o
+obj-$(CONFIG_QCOM_KRYO_L2_ACCESSORS) +=	kryo-l2-accessors.o
drivers/soc/qcom/kryo-l2-accessors.c new file +56 -0

56 lines not on this page · raw message

include/soc/qcom/kryo-l2-accessors.h new file +12 -0
new file mode 100644
index 0000000..673c534
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef __SOC_ARCH_QCOM_KRYO_L2_ACCESSORS_H
+#define __SOC_ARCH_QCOM_KRYO_L2_ACCESSORS_H
+
+void kryo_l2_set_indirect_reg(u64 reg, u64 val);
+u64 kryo_l2_get_indirect_reg(u64 reg);
+
+#endif
Signature · 1 line
1.9.1
author
#2 lore raw

PATCH v8 2/15 clk: qcom: Make clk_alpha_pll_configure available to modules

In reply to Ilia Lin

From: Rajendra Nayak <[email protected]>

Allow clk_alpha_pll_configure to be called from loadable kernel modules.

---
 drivers/clk/qcom/clk-alpha-pll.c | 1 +
 1 file changed, 1 insertion(+)
drivers/clk/qcom/clk-alpha-pll.c +1 -0
index 9722b70..57f2084 100644
@@ -228,6 +228,7 @@ void clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
 	if (pll->flags & SUPPORTS_FSM_MODE)
 		qcom_pll_set_fsm_mode(regmap, PLL_MODE(pll), 6, 0);
 }
+EXPORT_SYMBOL_GPL(clk_alpha_pll_configure);
 
 static int clk_alpha_pll_hwfsm_enable(struct clk_hw *hw)
 {
Signature · 1 line
1.9.1
author
#3 lore raw

PATCH v8 3/15 clk: Use devm_ in the register fixed factor clock

In reply to Ilia Lin

Use devm_clk_hw_register instead of clk_hw_register to simplify the usage of this API. This way drivers that call the clk_hw_register_fixed_factor won't need to maintain a data structure for further cleanup.

---
 drivers/clk/clk-fixed-factor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clk/clk-fixed-factor.c +1 -1
index a5d402d..8e39bda 100644
@@ -94,7 +94,7 @@ struct clk_hw *clk_hw_register_fixed_factor(struct device *dev,
 	init.num_parents = 1;
 
 	hw = &fix->hw;
-	ret = clk_hw_register(dev, hw);
+	ret = devm_clk_hw_register(dev, hw);
 	if (ret) {
 		kfree(fix);
 		hw = ERR_PTR(ret);
Signature · 1 line
1.9.1
author
#4 lore raw

PATCH v8 4/15 clk: qcom: Add CPU clock driver for msm8996

In reply to Ilia Lin

Each of the CPU clusters (Power and Perf) on msm8996 are clocked via 2 PLLs, a primary and alternate. There are also 2 Mux'es, a primary and secondary all connected together as shown below

                             +-------+
              XO             |       |
          +------------------>0      |
                             |       |
                   PLL/2     | SMUX  +----+
                     +------->1      |    |
                     |       |       |    |
                     |       +-------+    |    +-------+
                     |                    +---->0      |
                     |                         |       |
+---------------+    |             +----------->1      | CPU clk
|Primary PLL    +----+ PLL_EARLY   |           |       +------>
|               +------+-----------+    +------>2 PMUX |
+---------------+      |                |      |       |
                       |   +------+     |   +-->3      |
                       +--^+  ACD +-----+   |  +-------+
+---------------+          +------+         |
|Alt PLL        |                           |
|               +---------------------------+
+---------------+         PLL_EARLY

The primary PLL is what drives the CPU clk, except for times when we are reprogramming the PLL itself (for rate changes) when we temporarily switch to an alternate PLL. A subsequent patch adds support to switch between primary and alternate PLL during rate changes.

The primary PLL operates on a single VCO range, between 600MHz and 3GHz. However the CPUs do support OPPs with frequencies between 300MHz and 600MHz. In order to support running the CPUs at those frequencies we end up having to lock the PLL at twice the rate and drive the CPU clk via the PLL/2 output and SMUX.

So for frequencies above 600MHz we follow the following path
Primary PLL --> PLL_EARLY --> PMUX(1) --> CPU clk
and for frequencies between 300MHz and 600MHz we follow
Primary PLL --> PLL/2 --> SMUX(1) --> PMUX(0) --> CPU clk
Support for this is added in a subsequent patch as well.

ACD stands for Adaptive Clock Distribution and is used to detect voltage droops.

---
 drivers/clk/qcom/Kconfig         |   9 +
 drivers/clk/qcom/Makefile        |   1 +
 drivers/clk/qcom/clk-alpha-pll.h |   6 +
 drivers/clk/qcom/clk-cpu-8996.c  | 403 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 419 insertions(+)
 create mode 100644 drivers/clk/qcom/clk-cpu-8996.c
drivers/clk/qcom/Kconfig +9 -0
index e42e1af..866ce1f 100644
@@ -33,6 +33,15 @@ config QCOM_CLK_APCS_MSM8916
 	  Say Y if you want to support CPU frequency scaling on devices
 	  such as msm8916.
 
+config QCOM_CLK_APCC_MSM8996
+	tristate "MSM8996 CPU Clock Controller"
+	depends on COMMON_CLK_QCOM
+	select QCOM_KRYO_L2_ACCESSORS
+	help
+	  Support for the CPU clock controller on msm8996 devices.
+	  Say Y if you want to support CPU clock scaling using CPUfreq
+	  drivers for dyanmic power management.
+
 config QCOM_CLK_RPM
 	tristate "RPM based Clock Controller"
 	depends on COMMON_CLK_QCOM && MFD_QCOM_RPM
drivers/clk/qcom/Makefile +1 -0
index 7c09ab1..a822fc8 100644
@@ -36,6 +36,7 @@ obj-$(CONFIG_MSM_MMCC_8974) += mmcc-msm8974.o
 obj-$(CONFIG_MSM_MMCC_8996) += mmcc-msm8996.o
 obj-$(CONFIG_QCOM_A53PLL) += a53-pll.o
 obj-$(CONFIG_QCOM_CLK_APCS_MSM8916) += apcs-msm8916.o
+obj-$(CONFIG_QCOM_CLK_APCC_MSM8996) += clk-cpu-8996.o
 obj-$(CONFIG_QCOM_CLK_RPM) += clk-rpm.o
 obj-$(CONFIG_QCOM_CLK_SMD_RPM) += clk-smd-rpm.o
 obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
drivers/clk/qcom/clk-alpha-pll.h +6 -0
index f981b48..9ce2a32 100644
@@ -50,6 +50,12 @@ struct pll_vco {
 	u32 val;
 };
 
+#define VCO(a, b, c) { \
+	.val = a,\
+	.min_freq = b,\
+	.max_freq = c,\
+}
+
 /**
  * struct clk_alpha_pll - phase locked loop (PLL)
  * @offset: base address of registers
drivers/clk/qcom/clk-cpu-8996.c new file +403 -0

403 lines not on this page · raw message

Signature · 1 line
1.9.1
#5 lore raw

Re: [PATCH v8 04/15] clk: qcom: Add CPU clock driver for msm8996

Thank you for the patch! Yet something to improve:

Hi Ilia,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.17-rc6]
[cannot apply to clk/clk-next next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ilia-Lin/CPU-scaling-support-for-msm8996/20180520-132401
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-allyesconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0

reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm

All errors (new ones prefixed by >>):

>> drivers/soc/qcom/kryo-l2-accessors.c:7:10: fatal error: asm/sysreg.h: No such file or directory
    #include <asm/sysreg.h>
             ^~~~~~~~~~~~~~

compilation terminated.

vim +7 drivers/soc/qcom/kryo-l2-accessors.c

f4e14120 Ilia Lin 2018-05-17 @7 #include <asm/sysreg.h> f4e14120 Ilia Lin 2018-05-17 8 #include <soc/qcom/kryo-l2-accessors.h> f4e14120 Ilia Lin 2018-05-17 9

:::::: The code at line 7 was first introduced by commit
:::::: f4e14120d663ce6e4670516a66bc0158dc692c45 soc: qcom: Separate kryo l2 accessors from PMU driver

:::::: TO: Ilia Lin <[email protected]>
:::::: CC: 0day robot <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 64542 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180522/d0336abb/attachment-0001.gz>

author
#6 lore raw

PATCH v8 5/15 dt-bindings: clk: qcom: Add bindings for CPU clock for msm8996

In reply to Ilia Lin

Each of the CPU clusters (Power and Perf) on msm8996 are clocked via 2 PLLs, a primary and alternate. There are also 2 Mux'es, a primary and secondary all connected together as shown below

                             +-------+
              XO             |       |
          +------------------>0      |
                             |       |
                   PLL/2     | SMUX  +----+
                     +------->1      |    |
                     |       |       |    |
                     |       +-------+    |    +-------+
                     |                    +---->0      |
                     |                         |       |
+---------------+    |             +----------->1      | CPU clk
|Primary PLL    +----+ PLL_EARLY   |           |       +------>
|               +------+-----------+    +------>2 PMUX |
+---------------+      |                |      |       |
                       |   +------+     |   +-->3      |
                       +--^+  ACD +-----+   |  +-------+
+---------------+          +------+         |
|Alt PLL        |                           |
|               +---------------------------+
+---------------+         PLL_EARLY

The primary PLL is what drives the CPU clk, except for times when we are reprogramming the PLL itself (for rate changes) when we temporarily switch to an alternate PLL. A subsequent patch adds support to switch between primary and alternate PLL during rate changes.

The primary PLL operates on a single VCO range, between 600MHz and 3GHz. However the CPUs do support OPPs with frequencies between 300MHz and 600MHz. In order to support running the CPUs at those frequencies we end up having to lock the PLL at twice the rate and drive the CPU clk via the PLL/2 output and SMUX.

---
 Documentation/devicetree/bindings/clock/qcom,kryocc.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,kryocc.txt
Documentation/devicetree/bindings/clock/qcom,kryocc.txt new file +17 -0
new file mode 100644
index 0000000..8458783
@@ -0,0 +1,17 @@
+Qualcomm CPUSS clock controller for Kryo CPUs
+----------------------------------------------------
+
+Required properties :
+- compatible : shall contain only one of the following:
+
+			"qcom,msm8996-apcc"
+
+- reg : shall contain base register location and length
+- #clock-cells : shall contain 1
+
+Example:
+	kryocc: clock-controller at 6400000 {
+		compatible = "qcom,msm8996-apcc";
+		reg = <0x6400000 0x90000>;
+		#clock-cells = <1>;
+	};
Signature · 1 line
1.9.1
author
#7 lore raw

PATCH v8 6/15 clk: qcom: cpu-8996: Add support to switch to alternate PLL

In reply to Ilia Lin

From: Rajendra Nayak <[email protected]>

Each of the CPU clusters on msm8996 are powered via a primary PLL and a secondary PLL. The primary PLL is what drives the CPU clk, except for times when we are reprogramming the PLL itself, when we temporarily switch to an alternate PLL. Use clock rate change notifiers to support this.

---
 drivers/clk/qcom/clk-cpu-8996.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
drivers/clk/qcom/clk-cpu-8996.c +33 -0
index d92cad93..620fdc2 100644
@@ -52,6 +52,7 @@
  * detect voltage droops.
  */
 
+#include <linux/clk.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
@@ -178,10 +179,14 @@ struct clk_cpu_8996_mux {
 	u32	reg;
 	u8	shift;
 	u8	width;
+	struct notifier_block nb;
 	struct clk_hw	*pll;
 	struct clk_regmap clkr;
 };
 
+#define to_clk_cpu_8996_mux_nb(_nb) \
+	container_of(_nb, struct clk_cpu_8996_mux, nb)
+
 static inline
 struct clk_cpu_8996_mux *to_clk_cpu_8996_mux_hw(struct clk_hw *hw)
 {
@@ -227,6 +232,26 @@ static int clk_cpu_8996_mux_set_parent(struct clk_hw *hw, u8 index)
 	return 0;
 }
 
+int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
+			void *data)
+{
+	int ret;
+	struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_nb(nb);
+
+	switch (event) {
+	case PRE_RATE_CHANGE:
+		ret = clk_cpu_8996_mux_set_parent(&cpuclk->clkr.hw, ALT_INDEX);
+		break;
+	case POST_RATE_CHANGE:
+		ret = clk_cpu_8996_mux_set_parent(&cpuclk->clkr.hw, PLL_INDEX);
+		break;
+	default:
+		ret = 0;
+		break;
+	}
+
+	return notifier_from_errno(ret);
+};
 const struct clk_ops clk_cpu_8996_mux_ops = {
 	.set_parent = clk_cpu_8996_mux_set_parent,
 	.get_parent = clk_cpu_8996_mux_get_parent,
@@ -270,6 +295,7 @@ static int clk_cpu_8996_mux_set_parent(struct clk_hw *hw, u8 index)
 	.shift = 0,
 	.width = 2,
 	.pll = &pwrcl_pll.clkr.hw,
+	.nb.notifier_call = cpu_clk_notifier_cb,
 	.clkr.hw.init = &(struct clk_init_data) {
 		.name = "pwrcl_pmux",
 		.parent_names = (const char *[]){
@@ -289,6 +315,7 @@ static int clk_cpu_8996_mux_set_parent(struct clk_hw *hw, u8 index)
 	.shift = 0,
 	.width = 2,
 	.pll = &perfcl_pll.clkr.hw,
+	.nb.notifier_call = cpu_clk_notifier_cb,
 	.clkr.hw.init = &(struct clk_init_data) {
 		.name = "perfcl_pmux",
 		.parent_names = (const char *[]){
@@ -347,6 +374,12 @@ struct clk_regmap *clks[] = {
 	clk_alpha_pll_configure(&perfcl_alt_pll, regmap, &altpll_config);
 	clk_alpha_pll_configure(&pwrcl_alt_pll, regmap, &altpll_config);
 
+	ret = clk_notifier_register(pwrcl_pmux.clkr.hw.clk, &pwrcl_pmux.nb);
+	if (ret)
+		return ret;
+
+	ret = clk_notifier_register(perfcl_pmux.clkr.hw.clk, &perfcl_pmux.nb);
+
 	return ret;
 }
 
Signature · 1 line
1.9.1
author
#8 lore raw

PATCH v8 7/15 clk: qcom: cpu-8996: Add support to switch below 600Mhz

In reply to Ilia Lin

The CPU clock controller's primary PLL operates on a single VCO range, between 600MHz and 3GHz. However the CPUs do support OPPs with frequencies between 300MHz and 600MHz. In order to support running the CPUs at those frequencies we end up having to lock the PLL at twice the rate and drive the CPU clk via the PLL/2 output and SMUX.

So for frequencies above 600MHz we follow the following path
Primary PLL --> PLL_EARLY --> PMUX(1) --> CPU clk
and for frequencies between 300MHz and 600MHz we follow
Primary PLL --> PLL/2 --> SMUX(1) --> PMUX(0) --> CPU clk

---
 drivers/clk/qcom/clk-cpu-8996.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)
drivers/clk/qcom/clk-cpu-8996.c +22 -3
index 620fdc2..ff5c0a5 100644
@@ -68,6 +68,8 @@ enum _pmux_input {
 	NUM_OF_PMUX_INPUTS
 };
 
+#define DIV_2_THRESHOLD		600000000
+
 static const u8 prim_pll_regs[PLL_OFF_MAX_REGS] = {
        [PLL_OFF_L_VAL] = 0x04,
        [PLL_OFF_ALPHA_VAL] = 0x08,
@@ -95,10 +97,11 @@ enum _pmux_input {
 
 static const struct alpha_pll_config hfpll_config = {
 	.l = 60,
-	.config_ctl_val = 0x200d4828,
+	.config_ctl_val = 0x200d4aa8,
 	.config_ctl_hi_val = 0x006,
 	.pre_div_mask = BIT(12),
 	.post_div_mask = 0x3 << 8,
+	.post_div_val = 0x1 << 8,
 	.main_output_mask = BIT(0),
 	.early_output_mask = BIT(3),
 };
@@ -140,7 +143,7 @@ enum _pmux_input {
 	.vco_mask = 0x3 << 20,
 	.config_ctl_val = 0x4001051b,
 	.post_div_mask = 0x3 << 8,
-	.post_div_val = 0x1,
+	.post_div_val = 0x1 << 8,
 	.main_output_mask = BIT(0),
 	.early_output_mask = BIT(3),
 };
@@ -181,6 +184,7 @@ struct clk_cpu_8996_mux {
 	u8	width;
 	struct notifier_block nb;
 	struct clk_hw	*pll;
+	struct clk_hw	*pll_div_2;
 	struct clk_regmap clkr;
 };
 
@@ -226,6 +230,13 @@ static int clk_cpu_8996_mux_set_parent(struct clk_hw *hw, u8 index)
 	struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_hw(hw);
 	struct clk_hw *parent = cpuclk->pll;
 
+	if (cpuclk->pll_div_2 && req->rate < DIV_2_THRESHOLD) {
+		if (req->rate < (DIV_2_THRESHOLD / 2))
+			return -EINVAL;
+
+		parent = cpuclk->pll_div_2;
+	}
+
 	req->best_parent_rate = clk_hw_round_rate(parent, req->rate);
 	req->best_parent_hw = parent;
 
@@ -237,13 +248,19 @@ int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
 {
 	int ret;
 	struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_nb(nb);
+	struct clk_notifier_data *cnd = data;
 
 	switch (event) {
 	case PRE_RATE_CHANGE:
 		ret = clk_cpu_8996_mux_set_parent(&cpuclk->clkr.hw, ALT_INDEX);
 		break;
 	case POST_RATE_CHANGE:
-		ret = clk_cpu_8996_mux_set_parent(&cpuclk->clkr.hw, PLL_INDEX);
+		if (cnd->new_rate < DIV_2_THRESHOLD)
+			ret = clk_cpu_8996_mux_set_parent(&cpuclk->clkr.hw,
+							  DIV_2_INDEX);
+		else
+			ret = clk_cpu_8996_mux_set_parent(&cpuclk->clkr.hw,
+							  PLL_INDEX);
 		break;
 	default:
 		ret = 0;
@@ -295,6 +312,7 @@ int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
 	.shift = 0,
 	.width = 2,
 	.pll = &pwrcl_pll.clkr.hw,
+	.pll_div_2 = &pwrcl_smux.clkr.hw,
 	.nb.notifier_call = cpu_clk_notifier_cb,
 	.clkr.hw.init = &(struct clk_init_data) {
 		.name = "pwrcl_pmux",
@@ -315,6 +333,7 @@ int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
 	.shift = 0,
 	.width = 2,
 	.pll = &perfcl_pll.clkr.hw,
+	.pll_div_2 = &perfcl_smux.clkr.hw,
 	.nb.notifier_call = cpu_clk_notifier_cb,
 	.clkr.hw.init = &(struct clk_init_data) {
 		.name = "perfcl_pmux",
Signature · 1 line
1.9.1
author
#9 lore raw

PATCH v8 8/15 clk: qcom: Add ACD path to CPU clock driver for msm8996

In reply to Ilia Lin

The PMUX for each duplex allows for selection of ACD clock source. The DVM (Dynamic Variation Monitor) will flag an error
when a voltage droop event is detected. This flagged error enables ACD to provide a div-by-2 clock, sourced from the primary PLL. The duplex will be provided the divided clock
until a pre-programmed delay has expired.

This change configures ACD during the probe and switches the PMUXes to the ACD clock source.

---
 drivers/clk/qcom/clk-cpu-8996.c | 75 +++++++++++++++++++++++++++++++++++------
 1 file changed, 65 insertions(+), 10 deletions(-)
drivers/clk/qcom/clk-cpu-8996.c +65 -10
index ff5c0a5..0a908d8 100644
@@ -53,9 +53,11 @@
  */
 
 #include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
+#include <soc/qcom/kryo-l2-accessors.h>
 
 #include "clk-alpha-pll.h"
 #include "clk-regmap.h"
@@ -69,6 +71,11 @@ enum _pmux_input {
 };
 
 #define DIV_2_THRESHOLD		600000000
+#define PWRCL_REG_OFFSET 0x0
+#define PERFCL_REG_OFFSET 0x80000
+#define MUX_OFFSET	0x40
+#define ALT_PLL_OFFSET	0x100
+#define SSSCTL_OFFSET 0x160
 
 static const u8 prim_pll_regs[PLL_OFF_MAX_REGS] = {
        [PLL_OFF_L_VAL] = 0x04,
@@ -107,7 +114,7 @@ enum _pmux_input {
 };
 
 static struct clk_alpha_pll perfcl_pll = {
-	.offset = 0x80000,
+	.offset = PERFCL_REG_OFFSET,
 	.regs = prim_pll_regs,
 	.flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_FSM_MODE,
 	.clkr.hw.init = &(struct clk_init_data){
@@ -119,7 +126,7 @@ enum _pmux_input {
 };
 
 static struct clk_alpha_pll pwrcl_pll = {
-	.offset = 0x0,
+	.offset = PWRCL_REG_OFFSET,
 	.regs = prim_pll_regs,
 	.flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_FSM_MODE,
 	.clkr.hw.init = &(struct clk_init_data){
@@ -149,7 +156,7 @@ enum _pmux_input {
 };
 
 static struct clk_alpha_pll perfcl_alt_pll = {
-	.offset = 0x80100,
+	.offset = PERFCL_REG_OFFSET + ALT_PLL_OFFSET,
 	.regs = alt_pll_regs,
 	.vco_table = alt_pll_vco_modes,
 	.num_vco = ARRAY_SIZE(alt_pll_vco_modes),
@@ -163,7 +170,7 @@ enum _pmux_input {
 };
 
 static struct clk_alpha_pll pwrcl_alt_pll = {
-	.offset = 0x100,
+	.offset = PWRCL_REG_OFFSET + ALT_PLL_OFFSET,
 	.regs = alt_pll_regs,
 	.vco_table = alt_pll_vco_modes,
 	.num_vco = ARRAY_SIZE(alt_pll_vco_modes),
@@ -176,6 +183,9 @@ enum _pmux_input {
 	},
 };
 
+void __iomem *base;
+static void qcom_cpu_clk_msm8996_acd_init(void __iomem *base);
+
 /* Mux'es */
 
 struct clk_cpu_8996_mux {
@@ -253,6 +263,7 @@ int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
 	switch (event) {
 	case PRE_RATE_CHANGE:
 		ret = clk_cpu_8996_mux_set_parent(&cpuclk->clkr.hw, ALT_INDEX);
+		qcom_cpu_clk_msm8996_acd_init(base);
 		break;
 	case POST_RATE_CHANGE:
 		if (cnd->new_rate < DIV_2_THRESHOLD)
@@ -260,7 +271,7 @@ int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
 							  DIV_2_INDEX);
 		else
 			ret = clk_cpu_8996_mux_set_parent(&cpuclk->clkr.hw,
-							  PLL_INDEX);
+							  ACD_INDEX);
 		break;
 	default:
 		ret = 0;
@@ -276,7 +287,7 @@ int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
 };
 
 static struct clk_cpu_8996_mux pwrcl_smux = {
-	.reg = 0x40,
+	.reg = PWRCL_REG_OFFSET + MUX_OFFSET,
 	.shift = 2,
 	.width = 2,
 	.clkr.hw.init = &(struct clk_init_data) {
@@ -292,7 +303,7 @@ int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
 };
 
 static struct clk_cpu_8996_mux perfcl_smux = {
-	.reg = 0x80040,
+	.reg = PERFCL_REG_OFFSET + MUX_OFFSET,
 	.shift = 2,
 	.width = 2,
 	.clkr.hw.init = &(struct clk_init_data) {
@@ -308,7 +319,7 @@ int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
 };
 
 static struct clk_cpu_8996_mux pwrcl_pmux = {
-	.reg = 0x40,
+	.reg = PWRCL_REG_OFFSET + MUX_OFFSET,
 	.shift = 0,
 	.width = 2,
 	.pll = &pwrcl_pll.clkr.hw,
@@ -329,7 +340,7 @@ int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
 };
 
 static struct clk_cpu_8996_mux perfcl_pmux = {
-	.reg = 0x80040,
+	.reg = PERFCL_REG_OFFSET + MUX_OFFSET,
 	.shift = 0,
 	.width = 2,
 	.pll = &perfcl_pll.clkr.hw,
@@ -393,6 +404,10 @@ struct clk_regmap *clks[] = {
 	clk_alpha_pll_configure(&perfcl_alt_pll, regmap, &altpll_config);
 	clk_alpha_pll_configure(&pwrcl_alt_pll, regmap, &altpll_config);
 
+	/* Enable alt PLLs */
+	clk_prepare_enable(pwrcl_alt_pll.clkr.hw.clk);
+	clk_prepare_enable(perfcl_alt_pll.clkr.hw.clk);
+
 	ret = clk_notifier_register(pwrcl_pmux.clkr.hw.clk, &pwrcl_pmux.nb);
 	if (ret)
 		return ret;
@@ -402,10 +417,48 @@ struct clk_regmap *clks[] = {
 	return ret;
 }
 
+#define CPU_AFINITY_MASK 0xFFF
+#define PWRCL_CPU_REG_MASK 0x3
+#define PERFCL_CPU_REG_MASK 0x103
+
+#define L2ACDCR_REG 0x580ULL
+#define L2ACDTD_REG 0x581ULL
+#define L2ACDDVMRC_REG 0x584ULL
+#define L2ACDSSCR_REG 0x589ULL
+
+static DEFINE_SPINLOCK(acd_lock);
+
+static void qcom_cpu_clk_msm8996_acd_init(void __iomem *base)
+{
+	u64 hwid;
+	unsigned long flags;
+
+	spin_lock_irqsave(&acd_lock, flags);
+
+	hwid = read_cpuid_mpidr() & CPU_AFINITY_MASK;
+
+	kryo_l2_set_indirect_reg(L2ACDTD_REG, 0x00006A11);
+	kryo_l2_set_indirect_reg(L2ACDDVMRC_REG, 0x000E0F0F);
+	kryo_l2_set_indirect_reg(L2ACDSSCR_REG, 0x00000601);
+
+	if (PWRCL_CPU_REG_MASK == (hwid | PWRCL_CPU_REG_MASK)) {
+		writel(0xF, base + PWRCL_REG_OFFSET + SSSCTL_OFFSET);
+		wmb();
+		kryo_l2_set_indirect_reg(L2ACDCR_REG, 0x002C5FFD);
+	}
+
+	if (PERFCL_CPU_REG_MASK == (hwid | PERFCL_CPU_REG_MASK)) {
+		kryo_l2_set_indirect_reg(L2ACDCR_REG, 0x002C5FFD);
+		writel(0xF, base + PERFCL_REG_OFFSET + SSSCTL_OFFSET);
+		wmb();
+	}
+
+	spin_unlock_irqrestore(&acd_lock, flags);
+}
+
 static int qcom_cpu_clk_msm8996_driver_probe(struct platform_device *pdev)
 {
 	int ret;
-	void __iomem *base;
 	struct resource *res;
 	struct regmap *regmap;
 	struct clk_hw_onecell_data *data;
@@ -429,6 +482,8 @@ static int qcom_cpu_clk_msm8996_driver_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	qcom_cpu_clk_msm8996_acd_init(base);
+
 	data->hws[0] = &pwrcl_pmux.clkr.hw;
 	data->hws[1] = &perfcl_pmux.clkr.hw;
 	data->num = 2;
Signature · 1 line
1.9.1
author
#10 lore raw

PATCH v8 9/15 dt: qcom: Add opp and thermal to the msm8996

In reply to Ilia Lin
---
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 269 ++++++++++++++++++++++++++++++++--
 1 file changed, 260 insertions(+), 9 deletions(-)
arch/arm64/boot/dts/qcom/msm8996.dtsi +260 -9
index 37b7152c..e6cf290 100644
@@ -14,6 +14,7 @@
 #include <dt-bindings/clock/qcom,gcc-msm8996.h>
 #include <dt-bindings/clock/qcom,mmcc-msm8996.h>
 #include <dt-bindings/clock/qcom,rpmcc.h>
+#include <dt-bindings/thermal/thermal.h>
 
 / {
 	model = "Qualcomm Technologies, Inc. MSM8996";
@@ -97,6 +98,9 @@
 			compatible = "qcom,kryo";
 			reg = <0x0 0x0>;
 			enable-method = "psci";
+			clocks = <&kryocc 0>;
+			operating-points-v2 = <&cluster0_opp>;
+			#cooling-cells = <2>;
 			next-level-cache = <&L2_0>;
 			L2_0: l2-cache {
 			      compatible = "cache";
@@ -109,6 +113,9 @@
 			compatible = "qcom,kryo";
 			reg = <0x0 0x1>;
 			enable-method = "psci";
+			clocks = <&kryocc 0>;
+			operating-points-v2 = <&cluster0_opp>;
+			#cooling-cells = <2>;
 			next-level-cache = <&L2_0>;
 		};
 
@@ -117,6 +124,9 @@
 			compatible = "qcom,kryo";
 			reg = <0x0 0x100>;
 			enable-method = "psci";
+			clocks = <&kryocc 1>;
+			operating-points-v2 = <&cluster1_opp>;
+			#cooling-cells = <2>;
 			next-level-cache = <&L2_1>;
 			L2_1: l2-cache {
 			      compatible = "cache";
@@ -129,6 +139,9 @@
 			compatible = "qcom,kryo";
 			reg = <0x0 0x101>;
 			enable-method = "psci";
+			clocks = <&kryocc 1>;
+			operating-points-v2 = <&cluster1_opp>;
+			#cooling-cells = <2>;
 			next-level-cache = <&L2_1>;
 		};
 
@@ -155,6 +168,182 @@
 		};
 	};
 
+	cluster0_opp: opp_table0 {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp-307200000 {
+			opp-hz = /bits/ 64 <307200000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-422400000 {
+			opp-hz = /bits/ 64 <422400000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-480000000 {
+			opp-hz = /bits/ 64 <480000000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-556800000 {
+			opp-hz = /bits/ 64 <556800000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-652800000 {
+			opp-hz = /bits/ 64 <652800000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-729600000 {
+			opp-hz = /bits/ 64 <729600000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-844800000 {
+			opp-hz = /bits/ 64 <844800000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-960000000 {
+			opp-hz = /bits/ 64 <960000000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1036800000 {
+			opp-hz = /bits/ 64 <1036800000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1113600000 {
+			opp-hz = /bits/ 64 <1113600000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1190400000 {
+			opp-hz = /bits/ 64 <1190400000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1228800000 {
+			opp-hz = /bits/ 64 <1228800000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1324800000 {
+			opp-hz = /bits/ 64 <1324800000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1401600000 {
+			opp-hz = /bits/ 64 <1401600000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1478400000 {
+			opp-hz = /bits/ 64 <1478400000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1593600000 {
+			opp-hz = /bits/ 64 <1593600000>;
+			clock-latency-ns = <200000>;
+		};
+	};
+
+	cluster1_opp: opp_table1 {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp-307200000 {
+			opp-hz = /bits/ 64 <307200000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-403200000 {
+			opp-hz = /bits/ 64 <403200000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-480000000 {
+			opp-hz = /bits/ 64 <480000000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-556800000 {
+			opp-hz = /bits/ 64 <556800000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-652800000 {
+			opp-hz = /bits/ 64 <652800000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-729600000 {
+			opp-hz = /bits/ 64 <729600000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-806400000 {
+			opp-hz = /bits/ 64 <806400000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-883200000 {
+			opp-hz = /bits/ 64 <883200000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-940800000 {
+			opp-hz = /bits/ 64 <940800000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1036800000 {
+			opp-hz = /bits/ 64 <1036800000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1113600000 {
+			opp-hz = /bits/ 64 <1113600000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1190400000 {
+			opp-hz = /bits/ 64 <1190400000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1248000000 {
+			opp-hz = /bits/ 64 <1248000000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1324800000 {
+			opp-hz = /bits/ 64 <1324800000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1401600000 {
+			opp-hz = /bits/ 64 <1401600000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1478400000 {
+			opp-hz = /bits/ 64 <1478400000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1555200000 {
+			opp-hz = /bits/ 64 <1555200000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1632000000 {
+			opp-hz = /bits/ 64 <1632000000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1708800000 {
+			opp-hz = /bits/ 64 <1708800000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1785600000 {
+			opp-hz = /bits/ 64 <1785600000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1824000000 {
+			opp-hz = /bits/ 64 <1824000000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1920000000 {
+			opp-hz = /bits/ 64 <1920000000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1996800000 {
+			opp-hz = /bits/ 64 <1996800000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-2073600000 {
+			opp-hz = /bits/ 64 <2073600000>;
+			clock-latency-ns = <200000>;
+		};
+		opp-2150400000 {
+			opp-hz = /bits/ 64 <2150400000>;
+			clock-latency-ns = <200000>;
+		};
+	};
+
 	thermal-zones {
 		cpu-thermal0 {
 			polling-delay-passive = <250>;
@@ -163,18 +352,34 @@
 			thermal-sensors = <&tsens0 3>;
 
 			trips {
-				cpu_alert0: trip0 {
+				cpu_alert0: cpu_alert0 {
 					temperature = <75000>;
 					hysteresis = <2000>;
+					type = "active";
+				};
+				cpu_warn0: cpu_warn0 {
+					temperature = <90000>;
+					hysteresis = <2000>;
 					type = "passive";
 				};
 
-				cpu_crit0: trip1 {
+				cpu_crit0: cpu_crit0 {
 					temperature = <110000>;
 					hysteresis = <2000>;
 					type = "critical";
 				};
 			};
+
+			cooling-maps {
+				map0 {
+					trip = <&cpu_alert0>;
+					cooling-device = <&CPU0 THERMAL_NO_LIMIT 7>;
+				};
+				map1 {
+					trip = <&cpu_warn0>;
+					cooling-device = <&CPU0 8 THERMAL_NO_LIMIT>;
+				};
+			};
 		};
 
 		cpu-thermal1 {
@@ -184,18 +389,34 @@
 			thermal-sensors = <&tsens0 5>;
 
 			trips {
-				cpu_alert1: trip0 {
+				cpu_alert1: cpu_alert1 {
 					temperature = <75000>;
 					hysteresis = <2000>;
+					type = "active";
+				};
+				cpu_warn1: cpu_warn1 {
+					temperature = <90000>;
+					hysteresis = <2000>;
 					type = "passive";
 				};
 
-				cpu_crit1: trip1 {
+				cpu_crit1: cpu_crit1 {
 					temperature = <110000>;
 					hysteresis = <2000>;
 					type = "critical";
 				};
 			};
+
+			cooling-maps {
+				map0 {
+					trip = <&cpu_alert1>;
+					cooling-device = <&CPU0 THERMAL_NO_LIMIT 7>;
+				};
+				map1 {
+					trip = <&cpu_warn1>;
+					cooling-device = <&CPU0 8 THERMAL_NO_LIMIT>;
+				};
+			};
 		};
 
 		cpu-thermal2 {
@@ -205,18 +426,33 @@
 			thermal-sensors = <&tsens0 8>;
 
 			trips {
-				cpu_alert2: trip0 {
+				cpu_alert2: cpu_alert2 {
 					temperature = <75000>;
 					hysteresis = <2000>;
+					type = "active";
+				};
+				cpu_warn2: cpu_warn2 {
+					temperature = <90000>;
+					hysteresis = <2000>;
 					type = "passive";
 				};
 
-				cpu_crit2: trip1 {
+				cpu_crit2: cpu_crit2 {
 					temperature = <110000>;
 					hysteresis = <2000>;
 					type = "critical";
 				};
 			};
+			cooling-maps {
+				map0 {
+					trip = <&cpu_alert2>;
+					cooling-device = <&CPU2 THERMAL_NO_LIMIT 7>;
+				};
+				map1 {
+					trip = <&cpu_warn2>;
+					cooling-device = <&CPU2 8 THERMAL_NO_LIMIT>;
+				};
+			};
 		};
 
 		cpu-thermal3 {
@@ -226,9 +462,14 @@
 			thermal-sensors = <&tsens0 10>;
 
 			trips {
-				cpu_alert3: trip0 {
+				cpu_alert3: cpu_alert3 {
 					temperature = <75000>;
 					hysteresis = <2000>;
+					type = "active";
+				};
+				cpu_warn3: cpu_warn3 {
+					temperature = <90000>;
+					hysteresis = <2000>;
 					type = "passive";
 				};
 
@@ -238,6 +479,16 @@
 					type = "critical";
 				};
 			};
+			cooling-maps {
+				map0 {
+					trip = <&cpu_alert3>;
+					cooling-device = <&CPU2 THERMAL_NO_LIMIT 7>;
+				};
+				map1 {
+					trip = <&cpu_warn3>;
+					cooling-device = <&CPU2 8 THERMAL_NO_LIMIT>;
+				};
+			};
 		};
 	};
 
@@ -414,7 +665,7 @@
 		};
 
 		kryocc: clock-controller at 6400000 {
-			compatible = "qcom,apcc-msm8996";
+			compatible = "qcom,msm8996-apcc";
 			reg = <0x6400000 0x90000>;
 			#clock-cells = <1>;
 		};
@@ -1001,7 +1252,7 @@
 
 				pinctrl-names = "default", "sleep";
 				pinctrl-0 = <&pcie2_clkreq_default &pcie2_perst_default &pcie2_wake_default>;
-				pinctrl-1 = <&pcie2_clkreq_sleep &pcie2_perst_default &pcie2_wake_sleep >;
+				pinctrl-1 = <&pcie2_clkreq_sleep &pcie2_perst_default &pcie2_wake_sleep>;
 
 				vdda-supply = <&pm8994_l28>;
 
Signature · 1 line
1.9.1
author
#11 lore raw

PATCH v8 10/15 cpufreq: Add Kryo CPU scaling driver

In reply to Ilia Lin

In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, the CPU frequency subset and voltage value of each OPP varies based on the silicon variant in use. Qualcomm 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 to provide the OPP framework with required information.
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.

---
 drivers/cpufreq/Kconfig.arm          |  10 +++
 drivers/cpufreq/Makefile             |   1 +
 drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
 drivers/cpufreq/qcom-cpufreq-kryo.c  | 166 +++++++++++++++++++++++++++++++++++
 4 files changed, 180 insertions(+)
 create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
drivers/cpufreq/Kconfig.arm +10 -0
index de55c7d..0bfd40e 100644
@@ -124,6 +124,16 @@ config ARM_OMAP2PLUS_CPUFREQ
 	depends on ARCH_OMAP2PLUS
 	default ARCH_OMAP2PLUS
 
+config ARM_QCOM_CPUFREQ_KRYO
+	bool "Qualcomm Kryo based CPUFreq"
+	depends on QCOM_QFPROM
+	depends on QCOM_SMEM
+	select PM_OPP
+	help
+	  This adds the CPUFreq driver for Qualcomm Kryo SoC based boards.
+
+	  If in doubt, say N.
+
 config ARM_S3C_CPUFREQ
 	bool
 	help
drivers/cpufreq/Makefile +1 -0
index 8d24ade..fb4a2ec 100644
@@ -65,6 +65,7 @@ obj-$(CONFIG_MACH_MVEBU_V7)		+= mvebu-cpufreq.o
 obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ)	+= omap-cpufreq.o
 obj-$(CONFIG_ARM_PXA2xx_CPUFREQ)	+= pxa2xx-cpufreq.o
 obj-$(CONFIG_PXA3xx)			+= pxa3xx-cpufreq.o
+obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)	+= qcom-cpufreq-kryo.o
 obj-$(CONFIG_ARM_S3C2410_CPUFREQ)	+= s3c2410-cpufreq.o
 obj-$(CONFIG_ARM_S3C2412_CPUFREQ)	+= s3c2412-cpufreq.o
 obj-$(CONFIG_ARM_S3C2416_CPUFREQ)	+= s3c2416-cpufreq.o
drivers/cpufreq/cpufreq-dt-platdev.c +3 -0
index 3b585e4..77d6ab8 100644
@@ -118,6 +118,9 @@
 
 	{ .compatible = "nvidia,tegra124", },
 
+	{ .compatible = "qcom,apq8096", },
+	{ .compatible = "qcom,msm8996", },
+
 	{ .compatible = "st,stih407", },
 	{ .compatible = "st,stih410", },
 
drivers/cpufreq/qcom-cpufreq-kryo.c new file +166 -0

166 lines not on this page · raw message

Signature · 1 line
1.9.1
maintainer
#12 lore raw

Re: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver

drivers/cpufreq/qcom-cpufreq-kryo.c line 85

Quoted 13 lines · Ilia Lin
drivers/cpufreq/qcom-cpufreq-kryo.c new file +13 -0
+static int __init qcom_cpufreq_kryo_driver_init(void)
+{
+	size_t len;
+	int ret = 0;
+	u32 versions;
+	enum _msm8996_version msm8996_version;
+	u8 *speedbin;
+	struct device *cpu_dev_silver, *cpu_dev_gold;
+	struct device_node *np;
+	struct nvmem_cell *speedbin_nvmem;
+	struct platform_device *pdev;
+	struct opp_table *opp_silver = NULL;
+	struct opp_table *opp_gold = NULL;

No need to initialize them and you may want to arrange all above in decreasing order of their length.

Quoted 38 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +38 -0
+
+	cpu_dev_silver = get_cpu_device(SILVER_LEAD);
+	if (IS_ERR_OR_NULL(cpu_dev_silver))
+		return PTR_ERR(cpu_dev_silver);
+
+	cpu_dev_gold = get_cpu_device(SILVER_LEAD);
+	if (IS_ERR_OR_NULL(cpu_dev_gold))
+		return PTR_ERR(cpu_dev_gold);
+
+	msm8996_version = qcom_cpufreq_kryo_get_msm_id();
+	if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
+		dev_err(cpu_dev_silver, "Not Snapdragon 820/821!");
+		return -ENODEV;
+	}
+
+	np = dev_pm_opp_of_get_opp_desc_node(cpu_dev_silver);
+	if (IS_ERR_OR_NULL(np))
+		return PTR_ERR(np);
+
+	if (!of_device_is_compatible(np, "operating-points-v2-kryo-cpu")) {
+		ret = -ENOENT;
+		goto free_np;
+	}
+
+	speedbin_nvmem = of_nvmem_cell_get(np, NULL);
+	if (IS_ERR(speedbin_nvmem)) {
+		ret = PTR_ERR(speedbin_nvmem);
+		dev_err(cpu_dev_silver, "Could not get nvmem cell: %d\n", ret);
+		goto free_np;
+	}
+
+	speedbin = nvmem_cell_read(speedbin_nvmem, &len);
+	nvmem_cell_put(speedbin_nvmem);
+
+	switch (msm8996_version) {
+	case MSM8996_V3:
+		versions = 1 << (unsigned int)(*speedbin);
+		break;

drivers/cpufreq/qcom-cpufreq-kryo.c line 133

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +10 -0
+	case MSM8996_SG:
+		versions = 1 << ((unsigned int)(*speedbin) + 4);
+		break;
+	default:
+		BUG();
+		break;
+	}
+
+	opp_silver = dev_pm_opp_set_supported_hw(cpu_dev_silver,&versions,1);
+	if (IS_ERR_OR_NULL(opp_silver)) {

This API doesn't return NULL and so IS_ERR() would be sufficient.

drivers/cpufreq/qcom-cpufreq-kryo.c line 140

Quoted 7 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +7 -0
+		dev_err(cpu_dev_silver, "Failed to set supported hardware\n");
+		ret = PTR_ERR(opp_silver);
+		goto free_np;
+	}
+
+	opp_gold = dev_pm_opp_set_supported_hw(cpu_dev_gold,&versions,1);
+	if (IS_ERR_OR_NULL(opp_gold)) {

same here.

drivers/cpufreq/qcom-cpufreq-kryo.c line 148

Quoted 8 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +8 -0
+		dev_err(cpu_dev_gold, "Failed to set supported hardware\n");
+		ret = PTR_ERR(opp_gold);
+		goto free_opp_silver;
+	}
+
+	pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
+	if (!IS_ERR_OR_NULL(pdev))
+		goto out;

Simply return from here and remove the useless label out.

Quoted 5 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +5 -0
+
+	ret = PTR_ERR(pdev);
+	dev_err(cpu_dev_silver, "Failed to register platform device\n");
+	dev_pm_opp_put_supported_hw(opp_gold);
+

drivers/cpufreq/qcom-cpufreq-kryo.c line 163

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +10 -0
+free_opp_silver:
+	dev_pm_opp_put_supported_hw(opp_silver);
+
+free_np:
+	of_node_put(np);
+
+out:
+	return ret;
+}
+late_initcall(qcom_cpufreq_kryo_driver_init);

Please resend only this patch now or just paste the new code in a mail here so that I can review it quickly and then you can resend the final version. Most of the patches aren't changing anyway.

Signature · 1 line
viresh
author Acked-by Viresh Kumar
#13 lore raw

RE: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver

commit 4abe2cd7176a43c77e9a462e4f6ec51aa7552e73

Author: Ilia Lin <[email protected]>
Date:   Thu May 17 13:55:12 2018 +0300

cpufreq: Add Kryo CPU scaling driver

In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, the CPU frequency subset and voltage value of each OPP varies based on the silicon variant in use. Qualcomm 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
to provide the OPP framework with required information.
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.

drivers/cpufreq/Kconfig.arm +10 -0
index de55c7d..0bfd40e 100644
@@ -124,6 +124,16 @@ config ARM_OMAP2PLUS_CPUFREQ
        depends on ARCH_OMAP2PLUS
        default ARCH_OMAP2PLUS
 
+config ARM_QCOM_CPUFREQ_KRYO
+       bool "Qualcomm Kryo based CPUFreq"
+       depends on QCOM_QFPROM
+       depends on QCOM_SMEM
+       select PM_OPP
+       help
+         This adds the CPUFreq driver for Qualcomm Kryo SoC based boards.
+
+         If in doubt, say N.
+
 config ARM_S3C_CPUFREQ
        bool
        help
drivers/cpufreq/Makefile +1 -0
index 8d24ade..fb4a2ec 100644
@@ -65,6 +65,7 @@ obj-$(CONFIG_MACH_MVEBU_V7)           += mvebu-cpufreq.o
 obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ)    += omap-cpufreq.o
 obj-$(CONFIG_ARM_PXA2xx_CPUFREQ)       += pxa2xx-cpufreq.o
 obj-$(CONFIG_PXA3xx)                   += pxa3xx-cpufreq.o
+obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)    += qcom-cpufreq-kryo.o
 obj-$(CONFIG_ARM_S3C2410_CPUFREQ)      += s3c2410-cpufreq.o
 obj-$(CONFIG_ARM_S3C2412_CPUFREQ)      += s3c2412-cpufreq.o
 obj-$(CONFIG_ARM_S3C2416_CPUFREQ)      += s3c2416-cpufreq.o
(quoted diff) +0 -0

b/drivers/cpufreq/cpufreq-dt-platdev.c
index 3b585e4..77d6ab8 100644

drivers/cpufreq/cpufreq-dt-platdev.c +3 -0
@@ -118,6 +118,9 @@
 
        { .compatible = "nvidia,tegra124", },
 
+       { .compatible = "qcom,apq8096", },
+       { .compatible = "qcom,msm8996", },
+
        { .compatible = "st,stih407", },
        { .compatible = "st,stih410", },
 
(quoted diff) +0 -0

b/drivers/cpufreq/qcom-cpufreq-kryo.c
new file mode 100644
index 0000000..b024b23

drivers/cpufreq/qcom-cpufreq-kryo.c new file +9 -0
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+/*
+ * In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors,
+ * the CPU frequency subset and voltage value of each OPP varies
+ * based on the silicon variant in use. Qualcomm 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
+ * to provide the OPP framework with required information.
+ * 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.
+ */
+
+#include <linux/cpu.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
+#include <linux/slab.h>
+#include <linux/soc/qcom/smem.h>
+

+#define MSM_ID_SMEM    137
+#define SILVER_LEAD    0
+#define GOLD_LEAD      2
+
+enum _msm_id {
+       MSM8996V3 = 0xF6ul,
+       APQ8096V3 = 0x123ul,
+       MSM8996SG = 0x131ul,
+       APQ8096SG = 0x138ul,
+};
+
+enum _msm8996_version {
+       MSM8996_V3,
+       MSM8996_SG,
+       NUM_OF_MSM8996_VERSIONS,

+};
+
+static enum _msm8996_version __init qcom_cpufreq_kryo_get_msm_id(void) +{

+       size_t len;
+       u32 *msm_id;
+       enum _msm8996_version version;

+
+ msm_id = qcom_smem_get(QCOM_SMEM_HOST_ANY, MSM_ID_SMEM, &len);
+ /* The first 4 bytes are format, next to them is the actual msm-id
*/
+ msm_id++;
+

+       switch ((enum _msm_id)*msm_id) {
+       case MSM8996V3:
+       case APQ8096V3:

+ version = MSM8996_V3;
+ break;
+ case MSM8996SG:
+ case APQ8096SG:
+ version = MSM8996_SG;
+ break;
+ default:
+ version = NUM_OF_MSM8996_VERSIONS;
+ }
+
+ return version;
+}
+
+static int __init qcom_cpufreq_kryo_driver_init(void) +{

+       struct device *cpu_dev_silver, *cpu_dev_gold;
+       struct opp_table *opp_silver, *opp_gold;
+       enum _msm8996_version msm8996_version;
+       struct nvmem_cell *speedbin_nvmem;
+       struct platform_device *pdev;
+       struct device_node *np;
+       u8 *speedbin;
+       u32 versions;
+       size_t len;
+       int ret;

+
+ cpu_dev_silver = get_cpu_device(SILVER_LEAD);
+ if (IS_ERR_OR_NULL(cpu_dev_silver))
+ return PTR_ERR(cpu_dev_silver);
+
+ cpu_dev_gold = get_cpu_device(SILVER_LEAD);
+ if (IS_ERR_OR_NULL(cpu_dev_gold))
+ return PTR_ERR(cpu_dev_gold);
+
+ msm8996_version = qcom_cpufreq_kryo_get_msm_id();
+ if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
+ dev_err(cpu_dev_silver, "Not Snapdragon 820/821!");
+ return -ENODEV;
+ }
+
+ np = dev_pm_opp_of_get_opp_desc_node(cpu_dev_silver);
+ if (IS_ERR_OR_NULL(np))
+ return PTR_ERR(np);
+
+ if (!of_device_is_compatible(np, "operating-points-v2-kryo-cpu")) {
+ ret = -ENOENT;
+ goto free_np;
+ }
+
+ speedbin_nvmem = of_nvmem_cell_get(np, NULL);
+ if (IS_ERR(speedbin_nvmem)) {
+ ret = PTR_ERR(speedbin_nvmem);
+ dev_err(cpu_dev_silver, "Could not get nvmem cell: %d\n",
ret);
+ goto free_np;
+ }
+
+ speedbin = nvmem_cell_read(speedbin_nvmem, &len);
+ nvmem_cell_put(speedbin_nvmem);
+
+ switch (msm8996_version) {
+ case MSM8996_V3:
+ versions = 1 << (unsigned int)(*speedbin);
+ break;
+ case MSM8996_SG:
+ versions = 1 << ((unsigned int)(*speedbin) + 4);
+ break;
+ default:
+ BUG();
+ break;
+ }
+
+ opp_silver =
dev_pm_opp_set_supported_hw(cpu_dev_silver,&versions,1);
+ if (IS_ERR_OR_NULL(opp_silver)) {
+ dev_err(cpu_dev_silver, "Failed to set supported
hardware\n");
+ ret = PTR_ERR(opp_silver);
+ goto free_np;
+ }
+
+ opp_gold = dev_pm_opp_set_supported_hw(cpu_dev_gold,&versions,1);
+ if (IS_ERR_OR_NULL(opp_gold)) {

+               dev_err(cpu_dev_gold, "Failed to set supported hardware\n");
+               ret = PTR_ERR(opp_gold);
+               goto free_opp_silver;

+ }
+
+ pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
+ if (!IS_ERR_OR_NULL(pdev))
+ return 0;
+

+       ret = PTR_ERR(pdev);
+       dev_err(cpu_dev_silver, "Failed to register platform device\n");
+       dev_pm_opp_put_supported_hw(opp_gold);

+
+free_opp_silver:
+ dev_pm_opp_put_supported_hw(opp_silver);
+
+free_np:
+ of_node_put(np);
+
+ return ret;
+}
+late_initcall(qcom_cpufreq_kryo_driver_init);
+
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Kryo CPUfreq driver"); +MODULE_LICENSE("GPL v2");

Quoted 120 lines

-----Original Message-----
From: Viresh Kumar <[email protected]>
Sent: Friday, May 18, 2018 04:46
To: Ilia Lin <[email protected]>
Cc: mturquette at baylibre.com; sboyd at kernel.org; robh at kernel.org; mark.rutland at arm.com; nm at ti.com; lgirdwood at gmail.com; broonie at kernel.org; andy.gross at linaro.org; david.brown at linaro.org; catalin.marinas at arm.com; will.deacon at arm.com; rjw at rjwysocki.net; linux- clk at vger.kernel.org; devicetree at vger.kernel.org; linux- kernel at vger.kernel.org; linux-pm at vger.kernel.org; linux-arm- msm at vger.kernel.org; linux-soc at vger.kernel.org; linux-arm- kernel at lists.infradead.org; rnayak at codeaurora.org; amit.kucheria at linaro.org; nicolas.dechesne at linaro.org; celster at codeaurora.org; tfinkel at codeaurora.org
Subject: Re: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver

Review

Quoted 13 lines · Ilia Lin
(quoted diff) +13 -0
+static int __init qcom_cpufreq_kryo_driver_init(void)
+{
+	size_t len;
+	int ret = 0;
+	u32 versions;
+	enum _msm8996_version msm8996_version;
+	u8 *speedbin;
+	struct device *cpu_dev_silver, *cpu_dev_gold;
+	struct device_node *np;
+	struct nvmem_cell *speedbin_nvmem;
+	struct platform_device *pdev;
+	struct opp_table *opp_silver = NULL;
+	struct opp_table *opp_gold = NULL;

No need to initialize them and you may want to arrange all above in decreasing order of their length.

Quoted 18 lines
(quoted diff) +18 -0
+
+	cpu_dev_silver = get_cpu_device(SILVER_LEAD);
+	if (IS_ERR_OR_NULL(cpu_dev_silver))
+		return PTR_ERR(cpu_dev_silver);
+
+	cpu_dev_gold = get_cpu_device(SILVER_LEAD);
+	if (IS_ERR_OR_NULL(cpu_dev_gold))
+		return PTR_ERR(cpu_dev_gold);
+
+	msm8996_version = qcom_cpufreq_kryo_get_msm_id();
+	if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
+		dev_err(cpu_dev_silver, "Not Snapdragon 820/821!");
+		return -ENODEV;
+	}
+
+	np = dev_pm_opp_of_get_opp_desc_node(cpu_dev_silver);
+	if (IS_ERR_OR_NULL(np))
+		return PTR_ERR(np);

Review

Quoted 10 lines
(quoted diff) +10 -0
+
+	if (!of_device_is_compatible(np, "operating-points-v2-kryo-cpu")) {
+		ret = -ENOENT;
+		goto free_np;
+	}
+
+	speedbin_nvmem = of_nvmem_cell_get(np, NULL);
+	if (IS_ERR(speedbin_nvmem)) {
+		ret = PTR_ERR(speedbin_nvmem);
+		dev_err(cpu_dev_silver, "Could not get nvmem cell: %d\n",

ret);

Quoted 9 lines
(quoted diff) +9 -0
+		goto free_np;
+	}
+
+	speedbin = nvmem_cell_read(speedbin_nvmem, &len);
+	nvmem_cell_put(speedbin_nvmem);
+
+	switch (msm8996_version) {
+	case MSM8996_V3:
+		versions = 1 << (unsigned int)(*speedbin);

Review

Quoted 10 lines
(quoted diff) +10 -0
+		break;
+	case MSM8996_SG:
+		versions = 1 << ((unsigned int)(*speedbin) + 4);
+		break;
+	default:
+		BUG();
+		break;
+	}
+
+	opp_silver =

dev_pm_opp_set_supported_hw(cpu_dev_silver,&versions,1);

Review

Quoted 1 line
(quoted diff) +1 -0
+	if (IS_ERR_OR_NULL(opp_silver)) {

This API doesn't return NULL and so IS_ERR() would be sufficient.

Review

Quoted 1 line
(quoted diff) +1 -0
+		dev_err(cpu_dev_silver, "Failed to set supported

hardware\n");

Review

Quoted 5 lines
(quoted diff) +5 -0
+		ret = PTR_ERR(opp_silver);
+		goto free_np;
+	}
+
+	opp_gold =

dev_pm_opp_set_supported_hw(cpu_dev_gold,&versions,1);

Review

Quoted 1 line
(quoted diff) +1 -0
+	if (IS_ERR_OR_NULL(opp_gold)) {

same here.

Review

Quoted 1 line
(quoted diff) +1 -0
+		dev_err(cpu_dev_gold, "Failed to set supported

hardware\n");

Review

Quoted 7 lines
(quoted diff) +7 -0
+		ret = PTR_ERR(opp_gold);
+		goto free_opp_silver;
+	}
+
+	pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
+	if (!IS_ERR_OR_NULL(pdev))
+		goto out;

Simply return from here and remove the useless label out.

Quoted 9 lines
(quoted diff) +9 -0
+
+	ret = PTR_ERR(pdev);
+	dev_err(cpu_dev_silver, "Failed to register platform device\n");
+	dev_pm_opp_put_supported_hw(opp_gold);
+
+free_opp_silver:
+	dev_pm_opp_put_supported_hw(opp_silver);
+
+free_np:
Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+	of_node_put(np);
+
+out:
+	return ret;
+}
+late_initcall(qcom_cpufreq_kryo_driver_init);

Please resend only this patch now or just paste the new code in a mail

here

Quoted 1 line

so that I can review it quickly and then you can resend the final version.

Most

Quoted 4 lines

of the patches aren't changing anyway.

--
viresh

#14 lore raw

Re: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver

Quoted 20 lines · ilialin at codeaurora.org
(quoted diff) +20 -0
+static int __init qcom_cpufreq_kryo_driver_init(void)
+{
+       struct device *cpu_dev_silver, *cpu_dev_gold;
+       struct opp_table *opp_silver, *opp_gold;
+       enum _msm8996_version msm8996_version;
+       struct nvmem_cell *speedbin_nvmem;
+       struct platform_device *pdev;
+       struct device_node *np;
+       u8 *speedbin;
+       u32 versions;
+       size_t len;
+       int ret;
+
+       cpu_dev_silver = get_cpu_device(SILVER_LEAD);
+       if (IS_ERR_OR_NULL(cpu_dev_silver))
+               return PTR_ERR(cpu_dev_silver);
+
+       cpu_dev_gold = get_cpu_device(SILVER_LEAD);
+       if (IS_ERR_OR_NULL(cpu_dev_gold))
+               return PTR_ERR(cpu_dev_gold);

Review

Quoted 10 lines
(quoted diff) +10 -0
+
+       msm8996_version = qcom_cpufreq_kryo_get_msm_id();
+       if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
+               dev_err(cpu_dev_silver, "Not Snapdragon 820/821!");
+               return -ENODEV;
+       }
+
+       np = dev_pm_opp_of_get_opp_desc_node(cpu_dev_silver);
+       if (IS_ERR_OR_NULL(np))
+               return PTR_ERR(np);

This function (qcom_cpufreq_kryo_driver_init) returns zero on success. You are checking "np" here for being an error pointer, or NULL. What value do you think PTR_ERR() returns in the case of PTR_ERR(NULL)?

IS_ERR_OR_NULL() is considered by some (me included) as being _very_ harmful because programmers generally fail to look at linux/err.h and consider what it means when used as above.

Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
author Acked-by Viresh Kumar
#15 lore raw

RE: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver

In reply to Viresh Kumar
Quoted 1 line

From c5804e1d17578a63ca87cc8fd839bf756cfe3567 Mon Sep 17 00:00:00 2001

In-Reply-To: <[email protected]>
References: <[email protected]>
From: Ilia Lin <[email protected]>
Date: Thu, 17 May 2018 13:55:12 +0300
Subject: [PATCH] cpufreq: Add Kryo CPU scaling driver

In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, the CPU frequency subset and voltage value of each OPP varies based on the silicon variant in use. Qualcomm 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 to provide the OPP framework with required information.
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.

---
 drivers/cpufreq/Kconfig.arm          |  10 +++
 drivers/cpufreq/Makefile             |   1 +
 drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
 drivers/cpufreq/qcom-cpufreq-kryo.c  | 164
+++++++++++++++++++++++++++++++++++
 4 files changed, 178 insertions(+)
 create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
drivers/cpufreq/Kconfig.arm +10 -0
index de55c7d..0bfd40e 100644
@@ -124,6 +124,16 @@ config ARM_OMAP2PLUS_CPUFREQ
        depends on ARCH_OMAP2PLUS
        default ARCH_OMAP2PLUS
 
+config ARM_QCOM_CPUFREQ_KRYO
+       bool "Qualcomm Kryo based CPUFreq"
+       depends on QCOM_QFPROM
+       depends on QCOM_SMEM
+       select PM_OPP
+       help
+         This adds the CPUFreq driver for Qualcomm Kryo SoC based boards.
+
+         If in doubt, say N.
+
 config ARM_S3C_CPUFREQ
        bool
        help
drivers/cpufreq/Makefile +1 -0
index 8d24ade..fb4a2ec 100644
@@ -65,6 +65,7 @@ obj-$(CONFIG_MACH_MVEBU_V7)           += mvebu-cpufreq.o
 obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ)    += omap-cpufreq.o
 obj-$(CONFIG_ARM_PXA2xx_CPUFREQ)       += pxa2xx-cpufreq.o
 obj-$(CONFIG_PXA3xx)                   += pxa3xx-cpufreq.o
+obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)    += qcom-cpufreq-kryo.o
 obj-$(CONFIG_ARM_S3C2410_CPUFREQ)      += s3c2410-cpufreq.o
 obj-$(CONFIG_ARM_S3C2412_CPUFREQ)      += s3c2412-cpufreq.o
 obj-$(CONFIG_ARM_S3C2416_CPUFREQ)      += s3c2416-cpufreq.o
(quoted diff) +0 -0

b/drivers/cpufreq/cpufreq-dt-platdev.c
index 3b585e4..77d6ab8 100644

drivers/cpufreq/cpufreq-dt-platdev.c +3 -0
@@ -118,6 +118,9 @@
 
        { .compatible = "nvidia,tegra124", },
 
+       { .compatible = "qcom,apq8096", },
+       { .compatible = "qcom,msm8996", },
+
        { .compatible = "st,stih407", },
        { .compatible = "st,stih410", },
 
(quoted diff) +0 -0

b/drivers/cpufreq/qcom-cpufreq-kryo.c
new file mode 100644
index 0000000..ae2d1b9

drivers/cpufreq/qcom-cpufreq-kryo.c new file +9 -0
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+/*
+ * In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors,
+ * the CPU frequency subset and voltage value of each OPP varies
+ * based on the silicon variant in use. Qualcomm 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
+ * to provide the OPP framework with required information.
+ * 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.
+ */
+
+#include <linux/cpu.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
+#include <linux/slab.h>
+#include <linux/soc/qcom/smem.h>
+

+#define MSM_ID_SMEM    137
+#define SILVER_LEAD    0
+#define GOLD_LEAD      2
+
+enum _msm_id {
+       MSM8996V3 = 0xF6ul,
+       APQ8096V3 = 0x123ul,
+       MSM8996SG = 0x131ul,
+       APQ8096SG = 0x138ul,
+};
+
+enum _msm8996_version {
+       MSM8996_V3,
+       MSM8996_SG,
+       NUM_OF_MSM8996_VERSIONS,

+};
+
+static enum _msm8996_version __init qcom_cpufreq_kryo_get_msm_id(void) +{

+       size_t len;
+       u32 *msm_id;
+       enum _msm8996_version version;

+
+ msm_id = qcom_smem_get(QCOM_SMEM_HOST_ANY, MSM_ID_SMEM, &len);
+ /* The first 4 bytes are format, next to them is the actual msm-id
*/
+ msm_id++;
+

+       switch ((enum _msm_id)*msm_id) {
+       case MSM8996V3:
+       case APQ8096V3:

+ version = MSM8996_V3;
+ break;
+ case MSM8996SG:
+ case APQ8096SG:
+ version = MSM8996_SG;
+ break;
+ default:
+ version = NUM_OF_MSM8996_VERSIONS;
+ }
+
+ return version;
+}
+
+static int __init qcom_cpufreq_kryo_driver_init(void) +{

+       struct device *cpu_dev_silver, *cpu_dev_gold;
+       struct opp_table *opp_silver, *opp_gold;
+       enum _msm8996_version msm8996_version;
+       struct nvmem_cell *speedbin_nvmem;
+       struct platform_device *pdev;
+       struct device_node *np;
+       u8 *speedbin;
+       u32 versions;
+       size_t len;
+       int ret;

+
+ cpu_dev_silver = get_cpu_device(SILVER_LEAD);
+ if (IS_ERR_OR_NULL(cpu_dev_silver))
+ return PTR_ERR(cpu_dev_silver);
+
+ cpu_dev_gold = get_cpu_device(SILVER_LEAD);
+ if (IS_ERR_OR_NULL(cpu_dev_gold))
+ return PTR_ERR(cpu_dev_gold);
+
+ msm8996_version = qcom_cpufreq_kryo_get_msm_id();
+ if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
+ dev_err(cpu_dev_silver, "Not Snapdragon 820/821!");
+ return -ENODEV;
+ }
+
+ np = dev_pm_opp_of_get_opp_desc_node(cpu_dev_silver);
+ if (IS_ERR_OR_NULL(np))
+ return PTR_ERR(np);
+
+ if (!of_device_is_compatible(np, "operating-points-v2-kryo-cpu")) {
+ ret = -ENOENT;
+ goto free_np;
+ }
+
+ speedbin_nvmem = of_nvmem_cell_get(np, NULL);
+ if (IS_ERR(speedbin_nvmem)) {
+ ret = PTR_ERR(speedbin_nvmem);
+ dev_err(cpu_dev_silver, "Could not get nvmem cell: %d\n",
ret);
+ goto free_np;
+ }
+
+ speedbin = nvmem_cell_read(speedbin_nvmem, &len);
+ nvmem_cell_put(speedbin_nvmem);
+
+ switch (msm8996_version) {
+ case MSM8996_V3:
+ versions = 1 << (unsigned int)(*speedbin);
+ break;
+ case MSM8996_SG:
+ versions = 1 << ((unsigned int)(*speedbin) + 4);
+ break;
+ default:
+ BUG();
+ break;
+ }
+
+ opp_silver =
dev_pm_opp_set_supported_hw(cpu_dev_silver,&versions,1);
+ if (IS_ERR(opp_silver)) {
+ dev_err(cpu_dev_silver, "Failed to set supported
hardware\n");
+ ret = PTR_ERR(opp_silver);
+ goto free_np;
+ }
+
+ opp_gold = dev_pm_opp_set_supported_hw(cpu_dev_gold,&versions,1);
+ if (IS_ERR(opp_gold)) {

+               dev_err(cpu_dev_gold, "Failed to set supported hardware\n");
+               ret = PTR_ERR(opp_gold);
+               goto free_opp_silver;

+ }
+
+ pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
+ if (!IS_ERR_OR_NULL(pdev))
+ return 0;
+

+       ret = PTR_ERR(pdev);
+       dev_err(cpu_dev_silver, "Failed to register platform device\n");
+       dev_pm_opp_put_supported_hw(opp_gold);

+
+free_opp_silver:
+ dev_pm_opp_put_supported_hw(opp_silver);
+
+free_np:
+ of_node_put(np);
+
+ return ret;
+}
+late_initcall(qcom_cpufreq_kryo_driver_init);
+
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Kryo CPUfreq driver"); +MODULE_LICENSE("GPL v2");
--
1.9.1

Quoted 120 lines

-----Original Message-----
From: Viresh Kumar <[email protected]>
Sent: Friday, May 18, 2018 04:46
To: Ilia Lin <[email protected]>
Cc: mturquette at baylibre.com; sboyd at kernel.org; robh at kernel.org; mark.rutland at arm.com; nm at ti.com; lgirdwood at gmail.com; broonie at kernel.org; andy.gross at linaro.org; david.brown at linaro.org; catalin.marinas at arm.com; will.deacon at arm.com; rjw at rjwysocki.net; linux- clk at vger.kernel.org; devicetree at vger.kernel.org; linux- kernel at vger.kernel.org; linux-pm at vger.kernel.org; linux-arm- msm at vger.kernel.org; linux-soc at vger.kernel.org; linux-arm- kernel at lists.infradead.org; rnayak at codeaurora.org; amit.kucheria at linaro.org; nicolas.dechesne at linaro.org; celster at codeaurora.org; tfinkel at codeaurora.org
Subject: Re: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver

Review

Quoted 13 lines · Ilia Lin
(quoted diff) +13 -0
+static int __init qcom_cpufreq_kryo_driver_init(void)
+{
+	size_t len;
+	int ret = 0;
+	u32 versions;
+	enum _msm8996_version msm8996_version;
+	u8 *speedbin;
+	struct device *cpu_dev_silver, *cpu_dev_gold;
+	struct device_node *np;
+	struct nvmem_cell *speedbin_nvmem;
+	struct platform_device *pdev;
+	struct opp_table *opp_silver = NULL;
+	struct opp_table *opp_gold = NULL;

No need to initialize them and you may want to arrange all above in decreasing order of their length.

Quoted 18 lines
(quoted diff) +18 -0
+
+	cpu_dev_silver = get_cpu_device(SILVER_LEAD);
+	if (IS_ERR_OR_NULL(cpu_dev_silver))
+		return PTR_ERR(cpu_dev_silver);
+
+	cpu_dev_gold = get_cpu_device(SILVER_LEAD);
+	if (IS_ERR_OR_NULL(cpu_dev_gold))
+		return PTR_ERR(cpu_dev_gold);
+
+	msm8996_version = qcom_cpufreq_kryo_get_msm_id();
+	if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
+		dev_err(cpu_dev_silver, "Not Snapdragon 820/821!");
+		return -ENODEV;
+	}
+
+	np = dev_pm_opp_of_get_opp_desc_node(cpu_dev_silver);
+	if (IS_ERR_OR_NULL(np))
+		return PTR_ERR(np);

Review

Quoted 10 lines
(quoted diff) +10 -0
+
+	if (!of_device_is_compatible(np, "operating-points-v2-kryo-cpu")) {
+		ret = -ENOENT;
+		goto free_np;
+	}
+
+	speedbin_nvmem = of_nvmem_cell_get(np, NULL);
+	if (IS_ERR(speedbin_nvmem)) {
+		ret = PTR_ERR(speedbin_nvmem);
+		dev_err(cpu_dev_silver, "Could not get nvmem cell: %d\n",

ret);

Quoted 9 lines
(quoted diff) +9 -0
+		goto free_np;
+	}
+
+	speedbin = nvmem_cell_read(speedbin_nvmem, &len);
+	nvmem_cell_put(speedbin_nvmem);
+
+	switch (msm8996_version) {
+	case MSM8996_V3:
+		versions = 1 << (unsigned int)(*speedbin);

Review

Quoted 10 lines
(quoted diff) +10 -0
+		break;
+	case MSM8996_SG:
+		versions = 1 << ((unsigned int)(*speedbin) + 4);
+		break;
+	default:
+		BUG();
+		break;
+	}
+
+	opp_silver =

dev_pm_opp_set_supported_hw(cpu_dev_silver,&versions,1);

Review

Quoted 1 line
(quoted diff) +1 -0
+	if (IS_ERR_OR_NULL(opp_silver)) {

This API doesn't return NULL and so IS_ERR() would be sufficient.

Review

Quoted 1 line
(quoted diff) +1 -0
+		dev_err(cpu_dev_silver, "Failed to set supported

hardware\n");

Review

Quoted 5 lines
(quoted diff) +5 -0
+		ret = PTR_ERR(opp_silver);
+		goto free_np;
+	}
+
+	opp_gold =

dev_pm_opp_set_supported_hw(cpu_dev_gold,&versions,1);

Review

Quoted 1 line
(quoted diff) +1 -0
+	if (IS_ERR_OR_NULL(opp_gold)) {

same here.

Review

Quoted 1 line
(quoted diff) +1 -0
+		dev_err(cpu_dev_gold, "Failed to set supported

hardware\n");

Review

Quoted 7 lines
(quoted diff) +7 -0
+		ret = PTR_ERR(opp_gold);
+		goto free_opp_silver;
+	}
+
+	pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
+	if (!IS_ERR_OR_NULL(pdev))
+		goto out;

Simply return from here and remove the useless label out.

Quoted 9 lines
(quoted diff) +9 -0
+
+	ret = PTR_ERR(pdev);
+	dev_err(cpu_dev_silver, "Failed to register platform device\n");
+	dev_pm_opp_put_supported_hw(opp_gold);
+
+free_opp_silver:
+	dev_pm_opp_put_supported_hw(opp_silver);
+
+free_np:
Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+	of_node_put(np);
+
+out:
+	return ret;
+}
+late_initcall(qcom_cpufreq_kryo_driver_init);

Please resend only this patch now or just paste the new code in a mail

here

Quoted 1 line

so that I can review it quickly and then you can resend the final version.

Most

Quoted 4 lines

of the patches aren't changing anyway.

--
viresh

#16 lore raw

RE: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver

In reply to Viresh Kumar

Hi Viresh,

If I send patches in reply, it will produce new patches, instead of answers in the thread. Please find below the file dump.

->cat drivers/cpufreq/qcom-cpufreq-kryo.c
// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (c) 2018, The Linux Foundation. All rights reserved. */

/*
* In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors,
* the CPU frequency subset and voltage value of each OPP varies
* based on the silicon variant in use. Qualcomm 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
* to provide the OPP framework with required information.
* 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. */

#include <linux/cpu.h>
#include <linux/err.h>
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/nvmem-consumer.h>
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/pm_opp.h>
#include <linux/slab.h>
#include <linux/soc/qcom/smem.h>

#define MSM_ID_SMEM     137
#define SILVER_LEAD     0
#define GOLD_LEAD       2
enum _msm_id {
        MSM8996V3 = 0xF6ul,
        APQ8096V3 = 0x123ul,
        MSM8996SG = 0x131ul,
        APQ8096SG = 0x138ul,
};
enum _msm8996_version {
        MSM8996_V3,
        MSM8996_SG,
        NUM_OF_MSM8996_VERSIONS,
};

static enum _msm8996_version __init qcom_cpufreq_kryo_get_msm_id(void)

{
        size_t len;
        u32 *msm_id;
        enum _msm8996_version version;

        msm_id = qcom_smem_get(QCOM_SMEM_HOST_ANY, MSM_ID_SMEM, &len);
        /* The first 4 bytes are format, next to them is the actual msm-id
*/
        msm_id++;

        switch ((enum _msm_id)*msm_id) {
        case MSM8996V3:
        case APQ8096V3:
                version = MSM8996_V3;
                break;
        case MSM8996SG:
        case APQ8096SG:
                version = MSM8996_SG;
                break;
        default:
                version = NUM_OF_MSM8996_VERSIONS;
        }

        return version;
}

static int __init qcom_cpufreq_kryo_driver_init(void)

{
        struct device *cpu_dev_silver, *cpu_dev_gold;
        struct opp_table *opp_silver, *opp_gold;
        enum _msm8996_version msm8996_version;
        struct nvmem_cell *speedbin_nvmem;
        struct platform_device *pdev;
        struct device_node *np;
        u8 *speedbin;
        u32 versions;
        size_t len;
        int ret;

        cpu_dev_silver = get_cpu_device(SILVER_LEAD);
        if (IS_ERR_OR_NULL(cpu_dev_silver))
                return PTR_ERR(cpu_dev_silver);

        cpu_dev_gold = get_cpu_device(SILVER_LEAD);
        if (IS_ERR_OR_NULL(cpu_dev_gold))
                return PTR_ERR(cpu_dev_gold);

        msm8996_version = qcom_cpufreq_kryo_get_msm_id();
        if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
                dev_err(cpu_dev_silver, "Not Snapdragon 820/821!");
                return -ENODEV;
        }

        np = dev_pm_opp_of_get_opp_desc_node(cpu_dev_silver);
        if (IS_ERR_OR_NULL(np))
                return PTR_ERR(np);

        if (!of_device_is_compatible(np, "operating-points-v2-kryo-cpu")) {
                ret = -ENOENT;
                goto free_np;
        }

        speedbin_nvmem = of_nvmem_cell_get(np, NULL);
        if (IS_ERR(speedbin_nvmem)) {
                ret = PTR_ERR(speedbin_nvmem);
                dev_err(cpu_dev_silver, "Could not get nvmem cell: %d\n",
ret);
                goto free_np;
        }

        speedbin = nvmem_cell_read(speedbin_nvmem, &len);
        nvmem_cell_put(speedbin_nvmem);

        switch (msm8996_version) {
        case MSM8996_V3:
                versions = 1 << (unsigned int)(*speedbin);
                break;
        case MSM8996_SG:
                versions = 1 << ((unsigned int)(*speedbin) + 4);
                break;
        default:
                BUG();
                break;
        }

        opp_silver =
dev_pm_opp_set_supported_hw(cpu_dev_silver,&versions,1);
        if (IS_ERR(opp_silver)) {
                dev_err(cpu_dev_silver, "Failed to set supported
hardware\n");
                ret = PTR_ERR(opp_silver);
                goto free_np;
        }

        opp_gold = dev_pm_opp_set_supported_hw(cpu_dev_gold,&versions,1);
        if (IS_ERR(opp_gold)) {
                dev_err(cpu_dev_gold, "Failed to set supported hardware\n");
                ret = PTR_ERR(opp_gold);
                goto free_opp_silver;
        }

        pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
        if (!IS_ERR_OR_NULL(pdev))
                return 0;

        ret = PTR_ERR(pdev);
        dev_err(cpu_dev_silver, "Failed to register platform device\n");
        dev_pm_opp_put_supported_hw(opp_gold);

free_opp_silver:

        dev_pm_opp_put_supported_hw(opp_silver);

free_np:

        of_node_put(np);

        return ret;
}

late_initcall(qcom_cpufreq_kryo_driver_init);

MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Kryo CPUfreq driver"); MODULE_LICENSE("GPL v2");

Quoted 120 lines

-----Original Message-----
From: Viresh Kumar <[email protected]>
Sent: Friday, May 18, 2018 04:46
To: Ilia Lin <[email protected]>
Cc: mturquette at baylibre.com; sboyd at kernel.org; robh at kernel.org; mark.rutland at arm.com; nm at ti.com; lgirdwood at gmail.com; broonie at kernel.org; andy.gross at linaro.org; david.brown at linaro.org; catalin.marinas at arm.com; will.deacon at arm.com; rjw at rjwysocki.net; linux- clk at vger.kernel.org; devicetree at vger.kernel.org; linux- kernel at vger.kernel.org; linux-pm at vger.kernel.org; linux-arm- msm at vger.kernel.org; linux-soc at vger.kernel.org; linux-arm- kernel at lists.infradead.org; rnayak at codeaurora.org; amit.kucheria at linaro.org; nicolas.dechesne at linaro.org; celster at codeaurora.org; tfinkel at codeaurora.org
Subject: Re: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver

drivers/cpufreq/qcom-cpufreq-kryo.c line 85

Quoted 13 lines · Ilia Lin
drivers/cpufreq/qcom-cpufreq-kryo.c new file +13 -0
+static int __init qcom_cpufreq_kryo_driver_init(void)
+{
+	size_t len;
+	int ret = 0;
+	u32 versions;
+	enum _msm8996_version msm8996_version;
+	u8 *speedbin;
+	struct device *cpu_dev_silver, *cpu_dev_gold;
+	struct device_node *np;
+	struct nvmem_cell *speedbin_nvmem;
+	struct platform_device *pdev;
+	struct opp_table *opp_silver = NULL;
+	struct opp_table *opp_gold = NULL;

No need to initialize them and you may want to arrange all above in decreasing order of their length.

Quoted 18 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +18 -0
+
+	cpu_dev_silver = get_cpu_device(SILVER_LEAD);
+	if (IS_ERR_OR_NULL(cpu_dev_silver))
+		return PTR_ERR(cpu_dev_silver);
+
+	cpu_dev_gold = get_cpu_device(SILVER_LEAD);
+	if (IS_ERR_OR_NULL(cpu_dev_gold))
+		return PTR_ERR(cpu_dev_gold);
+
+	msm8996_version = qcom_cpufreq_kryo_get_msm_id();
+	if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
+		dev_err(cpu_dev_silver, "Not Snapdragon 820/821!");
+		return -ENODEV;
+	}
+
+	np = dev_pm_opp_of_get_opp_desc_node(cpu_dev_silver);
+	if (IS_ERR_OR_NULL(np))
+		return PTR_ERR(np);

drivers/cpufreq/qcom-cpufreq-kryo.c

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +10 -0
+
+	if (!of_device_is_compatible(np, "operating-points-v2-kryo-cpu")) {
+		ret = -ENOENT;
+		goto free_np;
+	}
+
+	speedbin_nvmem = of_nvmem_cell_get(np, NULL);
+	if (IS_ERR(speedbin_nvmem)) {
+		ret = PTR_ERR(speedbin_nvmem);
+		dev_err(cpu_dev_silver, "Could not get nvmem cell: %d\n",

ret);

Quoted 9 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +9 -0
+		goto free_np;
+	}
+
+	speedbin = nvmem_cell_read(speedbin_nvmem, &len);
+	nvmem_cell_put(speedbin_nvmem);
+
+	switch (msm8996_version) {
+	case MSM8996_V3:
+		versions = 1 << (unsigned int)(*speedbin);

drivers/cpufreq/qcom-cpufreq-kryo.c

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +10 -0
+		break;
+	case MSM8996_SG:
+		versions = 1 << ((unsigned int)(*speedbin) + 4);
+		break;
+	default:
+		BUG();
+		break;
+	}
+
+	opp_silver =

dev_pm_opp_set_supported_hw(cpu_dev_silver,&versions,1);

drivers/cpufreq/qcom-cpufreq-kryo.c line 133

Quoted 1 line
drivers/cpufreq/qcom-cpufreq-kryo.c new file +1 -0
+	if (IS_ERR_OR_NULL(opp_silver)) {

This API doesn't return NULL and so IS_ERR() would be sufficient.

Review

Quoted 1 line
(quoted diff) +1 -0
+		dev_err(cpu_dev_silver, "Failed to set supported

hardware\n");

drivers/cpufreq/qcom-cpufreq-kryo.c

Quoted 5 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +5 -0
+		ret = PTR_ERR(opp_silver);
+		goto free_np;
+	}
+
+	opp_gold =

dev_pm_opp_set_supported_hw(cpu_dev_gold,&versions,1);

drivers/cpufreq/qcom-cpufreq-kryo.c line 140

Quoted 1 line
drivers/cpufreq/qcom-cpufreq-kryo.c new file +1 -0
+	if (IS_ERR_OR_NULL(opp_gold)) {

same here.

Review

Quoted 1 line
(quoted diff) +1 -0
+		dev_err(cpu_dev_gold, "Failed to set supported

hardware\n");

drivers/cpufreq/qcom-cpufreq-kryo.c line 148

Quoted 7 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +7 -0
+		ret = PTR_ERR(opp_gold);
+		goto free_opp_silver;
+	}
+
+	pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
+	if (!IS_ERR_OR_NULL(pdev))
+		goto out;

Simply return from here and remove the useless label out.

Quoted 9 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +9 -0
+
+	ret = PTR_ERR(pdev);
+	dev_err(cpu_dev_silver, "Failed to register platform device\n");
+	dev_pm_opp_put_supported_hw(opp_gold);
+
+free_opp_silver:
+	dev_pm_opp_put_supported_hw(opp_silver);
+
+free_np:
Quoted 8 lines

drivers/cpufreq/qcom-cpufreq-kryo.c line 163

Quoted 6 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +6 -0
+	of_node_put(np);
+
+out:
+	return ret;
+}
+late_initcall(qcom_cpufreq_kryo_driver_init);

Please resend only this patch now or just paste the new code in a mail

here

Quoted 1 line

so that I can review it quickly and then you can resend the final version.

Most

Quoted 4 lines

of the patches aren't changing anyway.

--
viresh

maintainer
#17 lore raw

Re: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver

Quoted 4 lines · ilialin at codeaurora.org

Hi Viresh,

If I send patches in reply, it will produce new patches, instead of answers in the thread. Please find below the file dump.

There is one email from you which appears to be just fine and appears to be in reply to this thread only. Maybe its your email client that screwed it up for you ? Things look good in mutt.

Signature · 1 line
viresh
#18 lore raw

RE: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver

Final version (addressing Russel's comment as well):

// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (c) 2018, The Linux Foundation. All rights reserved. */

/*
* In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors,
* the CPU frequency subset and voltage value of each OPP varies
* based on the silicon variant in use. Qualcomm 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
* to provide the OPP framework with required information.
* 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. */

#include <linux/cpu.h>
#include <linux/err.h>
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/nvmem-consumer.h>
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/pm_opp.h>
#include <linux/slab.h>
#include <linux/soc/qcom/smem.h>

#define MSM_ID_SMEM 137
#define SILVER_LEAD 0
#define GOLD_LEAD 2

enum _msm_id {
	MSM8996V3 = 0xF6ul,
	APQ8096V3 = 0x123ul,
	MSM8996SG = 0x131ul,
	APQ8096SG = 0x138ul,
};
enum _msm8996_version {
	MSM8996_V3,
	MSM8996_SG,
	NUM_OF_MSM8996_VERSIONS,
};

static enum _msm8996_version __init qcom_cpufreq_kryo_get_msm_id(void)

{
	size_t len;
	u32 *msm_id;
	enum _msm8996_version version;

	msm_id = qcom_smem_get(QCOM_SMEM_HOST_ANY, MSM_ID_SMEM, &len);
	/* The first 4 bytes are format, next to them is the actual msm-id
*/
	msm_id++;

	switch ((enum _msm_id)*msm_id) {
	case MSM8996V3:
	case APQ8096V3:
		version = MSM8996_V3;
		break;
	case MSM8996SG:
	case APQ8096SG:
		version = MSM8996_SG;
		break;
	default:
		version = NUM_OF_MSM8996_VERSIONS;
	}

	return version;
}

static int __init qcom_cpufreq_kryo_driver_init(void)

{
	struct device *cpu_dev_silver, *cpu_dev_gold;
	struct opp_table *opp_silver, *opp_gold;
	enum _msm8996_version msm8996_version;
	struct nvmem_cell *speedbin_nvmem;
	struct platform_device *pdev;
	struct device_node *np;
	u8 *speedbin;
	u32 versions;
	size_t len;
	int ret;

	cpu_dev_silver = get_cpu_device(SILVER_LEAD);
	if (NULL == cpu_dev_silver)
		return -ENODEV;

	cpu_dev_gold = get_cpu_device(SILVER_LEAD);
	if (NULL == cpu_dev_gold)
		return -ENODEV;

	msm8996_version = qcom_cpufreq_kryo_get_msm_id();
	if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
		dev_err(cpu_dev_silver, "Not Snapdragon 820/821!");
		return -ENODEV;
	}

	np = dev_pm_opp_of_get_opp_desc_node(cpu_dev_silver);
	if (IS_ERR(np))
		return PTR_ERR(np);

	if (!of_device_is_compatible(np, "operating-points-v2-kryo-cpu")) {
		ret = -ENOENT;
		goto free_np;
	}

	speedbin_nvmem = of_nvmem_cell_get(np, NULL);
	if (IS_ERR(speedbin_nvmem)) {
		ret = PTR_ERR(speedbin_nvmem);
		dev_err(cpu_dev_silver, "Could not get nvmem cell: %d\n",
ret);
		goto free_np;
	}

	speedbin = nvmem_cell_read(speedbin_nvmem, &len);
	nvmem_cell_put(speedbin_nvmem);

	switch (msm8996_version) {
	case MSM8996_V3:
		versions = 1 << (unsigned int)(*speedbin);
		break;
	case MSM8996_SG:
		versions = 1 << ((unsigned int)(*speedbin) + 4);
		break;
	default:
		BUG();
		break;
	}

	opp_silver =
dev_pm_opp_set_supported_hw(cpu_dev_silver,&versions,1);
	if (IS_ERR(opp_silver)) {
		dev_err(cpu_dev_silver, "Failed to set supported
hardware\n");
		ret = PTR_ERR(opp_silver);
		goto free_np;
	}

	opp_gold = dev_pm_opp_set_supported_hw(cpu_dev_gold,&versions,1);
	if (IS_ERR(opp_gold)) {
		dev_err(cpu_dev_gold, "Failed to set supported hardware\n");
		ret = PTR_ERR(opp_gold);
		goto free_opp_silver;
	}

	pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
	if (!IS_ERR(pdev))
		return 0;

	ret = PTR_ERR(pdev);
	dev_err(cpu_dev_silver, "Failed to register platform device\n");
	dev_pm_opp_put_supported_hw(opp_gold);

free_opp_silver:

	dev_pm_opp_put_supported_hw(opp_silver);

free_np:

	of_node_put(np);

	return ret;
}

late_initcall(qcom_cpufreq_kryo_driver_init);

MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Kryo CPUfreq driver"); MODULE_LICENSE("GPL v2");

Quoted 16 lines

-----Original Message-----
From: Viresh Kumar <[email protected]>
Sent: Monday, May 21, 2018 07:50
To: ilialin at codeaurora.org
Cc: mturquette at baylibre.com; sboyd at kernel.org; robh at kernel.org; mark.rutland at arm.com; nm at ti.com; lgirdwood at gmail.com; broonie at kernel.org; andy.gross at linaro.org; david.brown at linaro.org; catalin.marinas at arm.com; will.deacon at arm.com; rjw at rjwysocki.net; linux- clk at vger.kernel.org; devicetree at vger.kernel.org; linux- kernel at vger.kernel.org; linux-pm at vger.kernel.org; linux-arm- msm at vger.kernel.org; linux-soc at vger.kernel.org; linux-arm- kernel at lists.infradead.org; rnayak at codeaurora.org; amit.kucheria at linaro.org; nicolas.dechesne at linaro.org; celster at codeaurora.org; tfinkel at codeaurora.org
Subject: Re: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver

Quoted 7 lines
Quoted 4 lines · ilialin at codeaurora.org

Hi Viresh,

If I send patches in reply, it will produce new patches, instead of answers in the thread. Please find below the file dump.

There is one email from you which appears to be just fine and appears to

be

Quoted 1 line

in reply to this thread only. Maybe its your email client that screwed it

up for

Quoted 4 lines

you ? Things look good in mutt.

--
viresh

maintainer
#19 lore raw

Re: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver

Quoted 1 line · ilialin at codeaurora.org

Final version (addressing Russel's comment as well):

Sorry, can't review it like this. At least you should be posting your diff here and you also need to do that from a sane email client like mutt, which wouldn't auto-fix/update the code. Doing it from gmail or thunderbird will ofcourse screw it.

Or a simple way is to send the patch again (alone) using in-reply-to field.

Signature · 1 line
viresh
author
#20 lore raw

PATCH 1/1 cpufreq: Add Kryo CPU scaling driver

In reply to Ilia Lin

In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, the CPU frequency subset and voltage value of each OPP varies based on the silicon variant in use. Qualcomm 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 to provide the OPP framework with required information.
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.

---
 drivers/cpufreq/Kconfig.arm          |  10 +++
 drivers/cpufreq/Makefile             |   1 +
 drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
 drivers/cpufreq/qcom-cpufreq-kryo.c  | 164 +++++++++++++++++++++++++++++++++++
 4 files changed, 178 insertions(+)
 create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
drivers/cpufreq/Kconfig.arm +10 -0
index de55c7d..0bfd40e 100644
@@ -124,6 +124,16 @@ config ARM_OMAP2PLUS_CPUFREQ
 	depends on ARCH_OMAP2PLUS
 	default ARCH_OMAP2PLUS
 
+config ARM_QCOM_CPUFREQ_KRYO
+	bool "Qualcomm Kryo based CPUFreq"
+	depends on QCOM_QFPROM
+	depends on QCOM_SMEM
+	select PM_OPP
+	help
+	  This adds the CPUFreq driver for Qualcomm Kryo SoC based boards.
+
+	  If in doubt, say N.
+
 config ARM_S3C_CPUFREQ
 	bool
 	help
drivers/cpufreq/Makefile +1 -0
index 8d24ade..fb4a2ec 100644
@@ -65,6 +65,7 @@ obj-$(CONFIG_MACH_MVEBU_V7)		+= mvebu-cpufreq.o
 obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ)	+= omap-cpufreq.o
 obj-$(CONFIG_ARM_PXA2xx_CPUFREQ)	+= pxa2xx-cpufreq.o
 obj-$(CONFIG_PXA3xx)			+= pxa3xx-cpufreq.o
+obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)	+= qcom-cpufreq-kryo.o
 obj-$(CONFIG_ARM_S3C2410_CPUFREQ)	+= s3c2410-cpufreq.o
 obj-$(CONFIG_ARM_S3C2412_CPUFREQ)	+= s3c2412-cpufreq.o
 obj-$(CONFIG_ARM_S3C2416_CPUFREQ)	+= s3c2416-cpufreq.o
drivers/cpufreq/cpufreq-dt-platdev.c +3 -0
index 3b585e4..77d6ab8 100644
@@ -118,6 +118,9 @@
 
 	{ .compatible = "nvidia,tegra124", },
 
+	{ .compatible = "qcom,apq8096", },
+	{ .compatible = "qcom,msm8996", },
+
 	{ .compatible = "st,stih407", },
 	{ .compatible = "st,stih410", },
 
drivers/cpufreq/qcom-cpufreq-kryo.c new file +164 -0

164 lines not on this page · raw message

Signature · 1 line
1.9.1
maintainer
#21 lore raw

Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

More comments after Russell's reply.

Quoted 5 lines · Ilia Lin
drivers/cpufreq/qcom-cpufreq-kryo.c new file +5 -0
+static int __init qcom_cpufreq_kryo_driver_init(void)
+{
+	struct device *cpu_dev_silver, *cpu_dev_gold;
+	struct opp_table *opp_silver, *opp_gold;
+	enum _msm8996_version msm8996_version;

drivers/cpufreq/qcom-cpufreq-kryo.c line 87

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +10 -0
+	struct nvmem_cell *speedbin_nvmem;
+	struct platform_device *pdev;
+	struct device_node *np;
+	u8 *speedbin;
+	u32 versions;
+	size_t len;
+	int ret;
+
+	cpu_dev_silver = get_cpu_device(SILVER_LEAD);
+	if (IS_ERR_OR_NULL(cpu_dev_silver))

get_cpu_device() returns only NULL on error.

Quoted 4 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +4 -0
+		return PTR_ERR(cpu_dev_silver);
+
+	cpu_dev_gold = get_cpu_device(SILVER_LEAD);
+	if (IS_ERR_OR_NULL(cpu_dev_gold))

drivers/cpufreq/qcom-cpufreq-kryo.c line 101

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +10 -0
+		return PTR_ERR(cpu_dev_gold);
+
+	msm8996_version = qcom_cpufreq_kryo_get_msm_id();
+	if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
+		dev_err(cpu_dev_silver, "Not Snapdragon 820/821!");
+		return -ENODEV;
+	}
+
+	np = dev_pm_opp_of_get_opp_desc_node(cpu_dev_silver);
+	if (IS_ERR_OR_NULL(np))

same here.

Quoted 34 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +34 -0
+		return PTR_ERR(np);
+
+	if (!of_device_is_compatible(np, "operating-points-v2-kryo-cpu")) {
+		ret = -ENOENT;
+		goto free_np;
+	}
+
+	speedbin_nvmem = of_nvmem_cell_get(np, NULL);
+	if (IS_ERR(speedbin_nvmem)) {
+		ret = PTR_ERR(speedbin_nvmem);
+		dev_err(cpu_dev_silver, "Could not get nvmem cell: %d\n", ret);
+		goto free_np;
+	}
+
+	speedbin = nvmem_cell_read(speedbin_nvmem, &len);
+	nvmem_cell_put(speedbin_nvmem);
+
+	switch (msm8996_version) {
+	case MSM8996_V3:
+		versions = 1 << (unsigned int)(*speedbin);
+		break;
+	case MSM8996_SG:
+		versions = 1 << ((unsigned int)(*speedbin) + 4);
+		break;
+	default:
+		BUG();
+		break;
+	}
+
+	opp_silver = dev_pm_opp_set_supported_hw(cpu_dev_silver,&versions,1);
+	if (IS_ERR(opp_silver)) {
+		dev_err(cpu_dev_silver, "Failed to set supported hardware\n");
+		ret = PTR_ERR(opp_silver);
+		goto free_np;

drivers/cpufreq/qcom-cpufreq-kryo.c line 145

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +10 -0
+	}
+
+	opp_gold = dev_pm_opp_set_supported_hw(cpu_dev_gold,&versions,1);
+	if (IS_ERR(opp_gold)) {
+		dev_err(cpu_dev_gold, "Failed to set supported hardware\n");
+		ret = PTR_ERR(opp_gold);
+		goto free_opp_silver;
+	}
+
+	pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);

and this only returns ERR_PTR() on error.

Quoted 21 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +19 -0
+	if (!IS_ERR_OR_NULL(pdev))
+		return 0;
+
+	ret = PTR_ERR(pdev);
+	dev_err(cpu_dev_silver, "Failed to register platform device\n");
+	dev_pm_opp_put_supported_hw(opp_gold);
+
+free_opp_silver:
+	dev_pm_opp_put_supported_hw(opp_silver);
+
+free_np:
+	of_node_put(np);
+
+	return ret;
+}
+late_initcall(qcom_cpufreq_kryo_driver_init);
+
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Kryo CPUfreq driver");
+MODULE_LICENSE("GPL v2");
Signature · 1 line
1.9.1
Signature · 1 line
viresh
#22 lore raw

Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

In reply to Ilia Lin
Quoted 20 lines · Ilia Lin

In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, the CPU frequency subset and voltage value of each OPP varies based on the silicon variant in use. Qualcomm 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 to provide the OPP framework with required information.
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.

---
 drivers/cpufreq/Kconfig.arm          |  10 +++
 drivers/cpufreq/Makefile             |   1 +
 drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
 drivers/cpufreq/qcom-cpufreq-kryo.c  | 164 +++++++++++++++++++++++++++++++++++
 4 files changed, 178 insertions(+)
 create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c

[..]

Quoted 19 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +19 -0
+
+/*
+ * In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors,
+ * the CPU frequency subset and voltage value of each OPP varies
+ * based on the silicon variant in use. Qualcomm 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
+ * to provide the OPP framework with required information.
+ * 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.
+ */
+
+#include <linux/cpu.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>

drivers/cpufreq/qcom-cpufreq-kryo.c line 33

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +10 -0
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
+#include <linux/slab.h>
+#include <linux/soc/qcom/smem.h>
+
+#define MSM_ID_SMEM	137
+#define SILVER_LEAD	0
+#define GOLD_LEAD	2
+

So I gather form other emails, that these are physical cpu number(not even unique identifier like MPIDR). Will this work on parts or platforms that need to boot in GOLD LEAD cpus.

[...]

Quoted 9 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +9 -0
+
+static int __init qcom_cpufreq_kryo_driver_init(void)
+{
+	struct device *cpu_dev_silver, *cpu_dev_gold;
+	struct opp_table *opp_silver, *opp_gold;
+	enum _msm8996_version msm8996_version;
+	struct nvmem_cell *speedbin_nvmem;
+	struct platform_device *pdev;
+	struct device_node *np;

drivers/cpufreq/qcom-cpufreq-kryo.c line 90

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +10 -0
+	u8 *speedbin;
+	u32 versions;
+	size_t len;
+	int ret;
+
+	cpu_dev_silver = get_cpu_device(SILVER_LEAD);
+	if (IS_ERR_OR_NULL(cpu_dev_silver))
+		return PTR_ERR(cpu_dev_silver);
+
+	cpu_dev_gold = get_cpu_device(SILVER_LEAD);

s/SILVER/GOLD/ ?

Signature · 2 lines
Regards,
Sudeep
#23 lore raw

RE: [PATCH] cpufreq: Add Kryo CPU scaling driver

Quoted 77 lines

-----Original Message-----
From: Sudeep Holla <[email protected]>
Sent: Monday, May 21, 2018 15:50
To: Ilia Lin <[email protected]>; mturquette at baylibre.com; sboyd at kernel.org; robh at kernel.org; mark.rutland at arm.com; viresh.kumar at linaro.org; nm at ti.com; lgirdwood at gmail.com; broonie at kernel.org; andy.gross at linaro.org; david.brown at linaro.org; catalin.marinas at arm.com; will.deacon at arm.com; rjw at rjwysocki.net; linux- clk at vger.kernel.org
Cc: Sudeep Holla <[email protected]>; devicetree at vger.kernel.org; linux-kernel at vger.kernel.org; linux-pm at vger.kernel.org; linux-arm- msm at vger.kernel.org; linux-soc at vger.kernel.org; linux-arm- kernel at lists.infradead.org; rnayak at codeaurora.org; amit.kucheria at linaro.org; nicolas.dechesne at linaro.org; celster at codeaurora.org; tfinkel at codeaurora.org
Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

Quoted 22 lines · Ilia Lin

In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, the CPU frequency subset and voltage value of each OPP varies based on the silicon variant in use. Qualcomm 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 to provide the OPP framework with required information. 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.

---
 drivers/cpufreq/Kconfig.arm          |  10 +++
 drivers/cpufreq/Makefile             |   1 +
 drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
 drivers/cpufreq/qcom-cpufreq-kryo.c  | 164
+++++++++++++++++++++++++++++++++++
 4 files changed, 178 insertions(+)
 create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c

[..]

Quoted 30 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +30 -0
+
+/*
+ * In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO
+processors,
+ * the CPU frequency subset and voltage value of each OPP varies
+ * based on the silicon variant in use. Qualcomm 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
+ * to provide the OPP framework with required information.
+ * 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.
+ */
+
+#include <linux/cpu.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
+#include <linux/slab.h>
+#include <linux/soc/qcom/smem.h>
+
Quoted 8 lines

drivers/cpufreq/qcom-cpufreq-kryo.c line 33

Quoted 4 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +4 -0
+#define MSM_ID_SMEM	137
+#define SILVER_LEAD	0
+#define GOLD_LEAD	2
+

So I gather form other emails, that these are physical cpu number(not even unique identifier like MPIDR). Will this work on parts or platforms that need to boot in GOLD LEAD cpus.

The driver is for Kryo CPU, which (and AFAIK all multicore MSMs) always boots on the CPU0.

Quoted 16 lines

[...]

Quoted 13 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +13 -0
+
+static int __init qcom_cpufreq_kryo_driver_init(void)
+{
+	struct device *cpu_dev_silver, *cpu_dev_gold;
+	struct opp_table *opp_silver, *opp_gold;
+	enum _msm8996_version msm8996_version;
+	struct nvmem_cell *speedbin_nvmem;
+	struct platform_device *pdev;
+	struct device_node *np;
+	u8 *speedbin;
+	u32 versions;
+	size_t len;
+	int ret;
Quoted 8 lines

drivers/cpufreq/qcom-cpufreq-kryo.c line 90

Quoted 6 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +6 -0
+
+	cpu_dev_silver = get_cpu_device(SILVER_LEAD);
+	if (IS_ERR_OR_NULL(cpu_dev_silver))
+		return PTR_ERR(cpu_dev_silver);
+
+	cpu_dev_gold = get_cpu_device(SILVER_LEAD);

s/SILVER/GOLD/ ?

Yes, you are right. This is already fixed in the respin.

Quoted 4 lines

--
Regards,
Sudeep

#24 lore raw

Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

Quoted 1 line · ilialin at codeaurora.org

[...]

Quoted 21 lines
Quoted 12 lines
Quoted 12 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +12 -0
+#include <linux/cpu.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
+#include <linux/slab.h>
+#include <linux/soc/qcom/smem.h>
+
Quoted 8 lines

drivers/cpufreq/qcom-cpufreq-kryo.c line 33

Quoted 4 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +4 -0
+#define MSM_ID_SMEM	137
+#define SILVER_LEAD	0
+#define GOLD_LEAD	2
+

So I gather form other emails, that these are physical cpu number(not even unique identifier like MPIDR). Will this work on parts or platforms that need to boot in GOLD LEAD cpus.

Quoted 2 lines

The driver is for Kryo CPU, which (and AFAIK all multicore MSMs) always boots on the CPU0.

That may be true and I am not that bothered about it. But assuming physical ordering from the logical cpu number is *incorrect* and will break if kernel decides to change the allocation algorithm. Kernel provides no guarantee on that, so you need to depend on some physical ID or may be DT to achieve what your want. But the current code as it stands is wrong.

Signature · 2 lines
Regards,
Sudeep
#25 lore raw

RE: [PATCH] cpufreq: Add Kryo CPU scaling driver

Quoted 49 lines

-----Original Message-----
From: Sudeep Holla <[email protected]>
Sent: Monday, May 21, 2018 16:05
To: ilialin at codeaurora.org; mturquette at baylibre.com; sboyd at kernel.org; robh at kernel.org; mark.rutland at arm.com; viresh.kumar at linaro.org; nm at ti.com; lgirdwood at gmail.com; broonie at kernel.org; andy.gross at linaro.org; david.brown at linaro.org; catalin.marinas at arm.com; will.deacon at arm.com; rjw at rjwysocki.net; linux-clk at vger.kernel.org
Cc: Sudeep Holla <[email protected]>; devicetree at vger.kernel.org; linux-kernel at vger.kernel.org; linux-pm at vger.kernel.org; linux-arm- msm at vger.kernel.org; linux-soc at vger.kernel.org; linux-arm- kernel at lists.infradead.org; rnayak at codeaurora.org; amit.kucheria at linaro.org; nicolas.dechesne at linaro.org; celster at codeaurora.org; tfinkel at codeaurora.org
Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

Quoted 1 line · ilialin at codeaurora.org

[...]

Quoted 21 lines
Quoted 12 lines
Quoted 12 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +12 -0
+#include <linux/cpu.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
+#include <linux/slab.h>
+#include <linux/soc/qcom/smem.h>
+
Quoted 8 lines

drivers/cpufreq/qcom-cpufreq-kryo.c line 33

Quoted 4 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +4 -0
+#define MSM_ID_SMEM	137
+#define SILVER_LEAD	0
+#define GOLD_LEAD	2
+

So I gather form other emails, that these are physical cpu number(not even unique identifier like MPIDR). Will this work on parts or platforms that need to boot in GOLD LEAD cpus.

Quoted 2 lines

The driver is for Kryo CPU, which (and AFAIK all multicore MSMs) always boots on the CPU0.

Quoted 5 lines

That may be true and I am not that bothered about it. But assuming physical ordering from the logical cpu number is *incorrect* and will break if kernel decides to change the allocation algorithm. Kernel provides no guarantee on that, so you need to depend on some physical ID or may be DT to achieve what your want. But the current code as it stands is wrong.

Got your point. In fact CPUs are numbered 0-3 and ordered into 2 clusters in the DT:

cpus {
	#address-cells = <2>;
	#size-cells = <0>;

	CPU0: cpu at 0 {
		...
		reg = <0x0 0x0>;
		...
	};

	CPU1: cpu at 1 {
		...
		reg = <0x0 0x1>;
		...
	};

	CPU2: cpu at 100 {
		...
		reg = <0x0 0x100>;
		...
	};

	CPU3: cpu at 101 {
		...
		reg = <0x0 0x101>;
		...
	};

	cpu-map {
		cluster0 {
			core0 {
				cpu = <&CPU0>;
			};

			core1 {
				cpu = <&CPU1>;
			};
		};

		cluster1 {
			core0 {
				cpu = <&CPU2>;
			};

			core1 {
				cpu = <&CPU3>;
			};
		};
	};
};

As far, as I understand, they are probed in the same order. However, to be certain that the physical CPU is the one I intend to configure, I have to fetch the device structure pointer for the cpu-map -> clusterX -> core0 -> cpu path. Could you suggest a kernel API to do that?

Quoted 4 lines

--
Regards,
Sudeep

#26 lore raw

Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

Quoted 5 lines · ilialin at codeaurora.org
Quoted 5 lines

-----Original Message-----
From: Sudeep Holla <[email protected]>
Sent: Monday, May 21, 2018 16:05

[...]

Quoted 61 lines
Quoted 7 lines

That may be true and I am not that bothered about it. But assuming physical ordering from the logical cpu number is *incorrect* and will break if kernel decides to change the allocation algorithm. Kernel provides no guarantee on that, so you need to depend on some physical ID or may be DT to achieve what your want. But the current code as it stands is wrong.

Got your point. In fact CPUs are numbered 0-3 and ordered into 2 clusters in the DT:

cpus {
	#address-cells = <2>;
	#size-cells = <0>;

	CPU0: cpu at 0 {
		...
		reg = <0x0 0x0>;
		...
	};

	CPU1: cpu at 1 {
		...
		reg = <0x0 0x1>;
		...
	};

	CPU2: cpu at 100 {
		...
		reg = <0x0 0x100>;
		...
	};

	CPU3: cpu at 101 {
		...
		reg = <0x0 0x101>;
		...
	};

	cpu-map {
		cluster0 {
			core0 {
				cpu = <&CPU0>;
			};

			core1 {
				cpu = <&CPU1>;
			};
		};

		cluster1 {
			core0 {
				cpu = <&CPU2>;
			};

			core1 {
				cpu = <&CPU3>;
			};
		};
	};
};
Quoted 1 line

As far, as I understand, they are probed in the same order.

Yes that's correct today, will that have to remain same for ever ? No it's not user ABI and kernel can decide to change the allocation order. What if for some reason one/more CPUs fails to boot or even configured not to boot ?

Quoted 5 lines

However, to be certain that the physical CPU is the one I intend to configure, I have to fetch the device structure pointer for the cpu-map -> clusterX -> core0 -> cpu path. Could you suggest a kernel API to do that?

Let's rewind a bit. Do you supply OPPs only on CPU0 and CPU2 ? If yes, that's again wrong. Simple solution is to parse all logical CPUs and skip if the share OPPs with some other CPUs. I think that logic already exists in OPP library IIRC.

Signature · 2 lines
Regards,
Sudeep
#27 lore raw

RE: [PATCH] cpufreq: Add Kryo CPU scaling driver

In reply to Sudeep Holla

OK, I think I found out the way. Would this be correct?
-----------------------------------------------------------------------------------------------
extern struct cpu_topology cpu_topology[NR_CPUS];

static struct device *qcom_cpufreq_kryo_get_cluster_lead(int cluster)

{
	unsigned cpu;

	for_each_possible_cpu(cpu) {
		if ((cluster == cpu_topology[cpu].cluster_id) &&
			(0 == cpu_topology[cpu].core_id))
			return get_cpu_device(cpu);
	}

	return NULL;
}

-----------------------------------------------------------------------------------------------

Quoted 147 lines

-----Original Message-----
From: ilialin at codeaurora.org <[email protected]>
Sent: Tuesday, May 22, 2018 09:56
To: 'Sudeep Holla' <[email protected]>; 'mturquette at baylibre.com' <[email protected]>; 'sboyd at kernel.org' <[email protected]>; 'robh at kernel.org' <[email protected]>; 'mark.rutland at arm.com' <[email protected]>; 'viresh.kumar at linaro.org' <[email protected]>; 'nm at ti.com' <[email protected]>; 'lgirdwood at gmail.com' <[email protected]>; 'broonie at kernel.org' <[email protected]>; 'andy.gross at linaro.org' <[email protected]>; 'david.brown at linaro.org' <[email protected]>; 'catalin.marinas at arm.com' <[email protected]>; 'will.deacon at arm.com' <[email protected]>; 'rjw at rjwysocki.net' <[email protected]>; 'linux-clk at vger.kernel.org' <linux- clk at vger.kernel.org>
Cc: 'devicetree at vger.kernel.org' <[email protected]>; 'linux- kernel at vger.kernel.org' <[email protected]>; 'linux- pm at vger.kernel.org' <[email protected]>; 'linux-arm- msm at vger.kernel.org' <[email protected]>; 'linux- soc at vger.kernel.org' <[email protected]>; 'linux-arm- kernel at lists.infradead.org' <[email protected]>; 'rnayak at codeaurora.org' <[email protected]>; 'amit.kucheria at linaro.org' <[email protected]>; 'nicolas.dechesne at linaro.org' <[email protected]>; 'celster at codeaurora.org' <[email protected]>; 'tfinkel at codeaurora.org' <[email protected]>
Subject: RE: [PATCH] cpufreq: Add Kryo CPU scaling driver

Quoted 45 lines

-----Original Message-----
From: Sudeep Holla <[email protected]>
Sent: Monday, May 21, 2018 16:05
To: ilialin at codeaurora.org; mturquette at baylibre.com; sboyd at kernel.org; robh at kernel.org; mark.rutland at arm.com; viresh.kumar at linaro.org; nm at ti.com; lgirdwood at gmail.com; broonie at kernel.org; andy.gross at linaro.org; david.brown at linaro.org; catalin.marinas at arm.com; will.deacon at arm.com; rjw at rjwysocki.net; linux-clk at vger.kernel.org
Cc: Sudeep Holla <[email protected]>; devicetree at vger.kernel.org; linux-kernel at vger.kernel.org; linux-pm at vger.kernel.org; linux-arm- msm at vger.kernel.org; linux-soc at vger.kernel.org; linux-arm- kernel at lists.infradead.org; rnayak at codeaurora.org; amit.kucheria at linaro.org; nicolas.dechesne at linaro.org; celster at codeaurora.org; tfinkel at codeaurora.org
Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

Quoted 1 line · ilialin at codeaurora.org

[...]

Quoted 18 lines
Quoted 9 lines
Quoted 9 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +9 -0
+#include <linux/cpu.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h> #include <linux/of.h> #include
+<linux/platform_device.h> #include <linux/pm_opp.h> #include
+<linux/slab.h> #include <linux/soc/qcom/smem.h>
+
Quoted 8 lines

drivers/cpufreq/qcom-cpufreq-kryo.c line 33

Quoted 4 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +4 -0
+#define MSM_ID_SMEM	137
+#define SILVER_LEAD	0
+#define GOLD_LEAD	2
+

So I gather form other emails, that these are physical cpu number(not even unique identifier like MPIDR). Will this work on parts or platforms that need to boot in GOLD LEAD cpus.

Quoted 2 lines

The driver is for Kryo CPU, which (and AFAIK all multicore MSMs) always boots on the CPU0.

Quoted 5 lines

That may be true and I am not that bothered about it. But assuming physical ordering from the logical cpu number is *incorrect* and will break if kernel decides to change the allocation algorithm. Kernel provides no guarantee on that, so you need to depend on some physical ID or may be DT to achieve what your want. But the current code as it

stands is wrong.

Got your point. In fact CPUs are numbered 0-3 and ordered into 2 clusters in the DT:

cpus {
	#address-cells = <2>;
	#size-cells = <0>;

	CPU0: cpu at 0 {
		...
		reg = <0x0 0x0>;
		...
	};

	CPU1: cpu at 1 {
		...
		reg = <0x0 0x1>;
		...
	};

	CPU2: cpu at 100 {
		...
		reg = <0x0 0x100>;
		...
	};

	CPU3: cpu at 101 {
		...
		reg = <0x0 0x101>;
		...
	};

	cpu-map {
		cluster0 {
			core0 {
				cpu = <&CPU0>;
			};

			core1 {
				cpu = <&CPU1>;
			};
		};

		cluster1 {
			core0 {
				cpu = <&CPU2>;
			};

			core1 {
				cpu = <&CPU3>;
			};
		};
	};
};

As far, as I understand, they are probed in the same order. However, to be certain that the physical CPU is the one I intend to configure, I have to fetch the device structure pointer for the cpu-map -> clusterX -> core0 -> cpu path. Could you suggest a kernel API to do that?

Quoted 4 lines

--
Regards,
Sudeep

#28 lore raw

Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

Quoted 1 line · ilialin at codeaurora.org

OK, I think I found out the way. Would this be correct?

No.

Quoted 9 lines

-----------------------------------------------------------------------------------------------
extern struct cpu_topology cpu_topology[NR_CPUS];

static struct device *qcom_cpufreq_kryo_get_cluster_lead(int cluster)

{
	unsigned cpu;

	for_each_possible_cpu(cpu) {
		if ((cluster == cpu_topology[cpu].cluster_id) &&

cluster_id is going away soon, so avoid relying on that. IIUC there's a way already something like opp_of_get_shared_cpus.

Signature · 2 lines
Regards,
Sudeep
maintainer
#29 lore raw

Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

In reply to [email protected]
Quoted 5 lines · ilialin at codeaurora.org

OK, I think I found out the way. Would this be correct?
-----------------------------------------------------------------------------------------------
extern struct cpu_topology cpu_topology[NR_CPUS];

static struct device *qcom_cpufreq_kryo_get_cluster_lead(int cluster)

Quoted 11 lines
{
	unsigned cpu;

	for_each_possible_cpu(cpu) {
		if ((cluster == cpu_topology[cpu].cluster_id) &&
			(0 == cpu_topology[cpu].core_id))
			return get_cpu_device(cpu);
	}

	return NULL;
}

Okay, this is what you should do IMHO.

for_each_possible_cpu(cpu) {
        cpu_dev = xxx..
        ret = dev_pm_opp_set_supported_hw(cpu_dev, xxx, xxx);
        if (ret && ret != -EBUSY)
                error-out.
}

This would require a trivial patch for the OPP core to not throw an error message with -EBUSY. I can do that separately.

Signature · 1 line
viresh
author
#30 lore raw

PATCH 1/1 cpufreq: Add Kryo CPU scaling driver

In reply to Ilia Lin

In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, the CPU frequency subset and voltage value of each OPP varies based on the silicon variant in use. Qualcomm 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 to provide the OPP framework with required information.
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.

---
 drivers/cpufreq/Kconfig.arm          |  10 +++
 drivers/cpufreq/Makefile             |   1 +
 drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
 drivers/cpufreq/qcom-cpufreq-kryo.c  | 163 +++++++++++++++++++++++++++++++++++
 4 files changed, 177 insertions(+)
 create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
drivers/cpufreq/Kconfig.arm +10 -0
index de55c7d..0bfd40e 100644
@@ -124,6 +124,16 @@ config ARM_OMAP2PLUS_CPUFREQ
 	depends on ARCH_OMAP2PLUS
 	default ARCH_OMAP2PLUS
 
+config ARM_QCOM_CPUFREQ_KRYO
+	bool "Qualcomm Kryo based CPUFreq"
+	depends on QCOM_QFPROM
+	depends on QCOM_SMEM
+	select PM_OPP
+	help
+	  This adds the CPUFreq driver for Qualcomm Kryo SoC based boards.
+
+	  If in doubt, say N.
+
 config ARM_S3C_CPUFREQ
 	bool
 	help
drivers/cpufreq/Makefile +1 -0
index 8d24ade..fb4a2ec 100644
@@ -65,6 +65,7 @@ obj-$(CONFIG_MACH_MVEBU_V7)		+= mvebu-cpufreq.o
 obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ)	+= omap-cpufreq.o
 obj-$(CONFIG_ARM_PXA2xx_CPUFREQ)	+= pxa2xx-cpufreq.o
 obj-$(CONFIG_PXA3xx)			+= pxa3xx-cpufreq.o
+obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)	+= qcom-cpufreq-kryo.o
 obj-$(CONFIG_ARM_S3C2410_CPUFREQ)	+= s3c2410-cpufreq.o
 obj-$(CONFIG_ARM_S3C2412_CPUFREQ)	+= s3c2412-cpufreq.o
 obj-$(CONFIG_ARM_S3C2416_CPUFREQ)	+= s3c2416-cpufreq.o
drivers/cpufreq/cpufreq-dt-platdev.c +3 -0
index 3b585e4..77d6ab8 100644
@@ -118,6 +118,9 @@
 
 	{ .compatible = "nvidia,tegra124", },
 
+	{ .compatible = "qcom,apq8096", },
+	{ .compatible = "qcom,msm8996", },
+
 	{ .compatible = "st,stih407", },
 	{ .compatible = "st,stih410", },
 
drivers/cpufreq/qcom-cpufreq-kryo.c new file +163 -0

163 lines not on this page · raw message

Signature · 1 line
1.9.1
#31 lore raw

Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

Quoted 12 lines · Ilia Lin

In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, the CPU frequency subset and voltage value of each OPP varies based on the silicon variant in use. Qualcomm 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 to provide the OPP framework with required information.
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.

[...]

Quoted 12 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +12 -0
+
+	switch (msm8996_version) {
+	case MSM8996_V3:
+		versions = 1 << (unsigned int)(*speedbin);
+		break;
+	case MSM8996_SG:
+		versions = 1 << ((unsigned int)(*speedbin) + 4);
+		break;
+	default:
+		BUG();
+		break;
+	}

drivers/cpufreq/qcom-cpufreq-kryo.c line 134

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +10 -0
+
+	for_each_possible_cpu(cpu) {
+		cpu_dev = get_cpu_device(cpu);
+		if (NULL == cpu_dev) {
+			ret = -ENODEV;
+			goto free_opp;
+		}
+
+		opp_tables[cpu] = dev_pm_opp_set_supported_hw(cpu_dev,
+							      &versions, 1);

Will be not get NULL for all CPUs except 0 ?
I haven't seen the patches from Viresh yet, if that prevents getting NULL or not.

--
Regards,
Sudeep

maintainer
#32 lore raw

Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

Quoted 32 lines · Sudeep Holla
Quoted 12 lines · Ilia Lin

In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, the CPU frequency subset and voltage value of each OPP varies based on the silicon variant in use. Qualcomm 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 to provide the OPP framework with required information.
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.

[...]

Quoted 16 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +16 -0
+
+	switch (msm8996_version) {
+	case MSM8996_V3:
+		versions = 1 << (unsigned int)(*speedbin);
+		break;
+	case MSM8996_SG:
+		versions = 1 << ((unsigned int)(*speedbin) + 4);
+		break;
+	default:
+		BUG();
+		break;
+	}
+
+	for_each_possible_cpu(cpu) {
+		cpu_dev = get_cpu_device(cpu);
+		if (NULL == cpu_dev) {
Quoted 8 lines

drivers/cpufreq/qcom-cpufreq-kryo.c line 134

Quoted 6 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +6 -0
+			ret = -ENODEV;
+			goto free_opp;
+		}
+
+		opp_tables[cpu] = dev_pm_opp_set_supported_hw(cpu_dev,
+							      &versions, 1);

Will be not get NULL for all CPUs except 0 ?

With my patches, we will get the OPP table again with refcount incremented. And on failures, we need to call put-supported-hw helper only for the CPUs for which it passed previously.

Signature · 1 line
viresh
author
#33 lore raw

PATCH 1/1 cpufreq: Add Kryo CPU scaling driver

In reply to Ilia Lin

In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, the CPU frequency subset and voltage value of each OPP varies based on the silicon variant in use. Qualcomm 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 to provide the OPP framework with required information.
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.

---
 drivers/cpufreq/Kconfig.arm          |  10 +++
 drivers/cpufreq/Makefile             |   1 +
 drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
 drivers/cpufreq/qcom-cpufreq-kryo.c  | 163 +++++++++++++++++++++++++++++++++++
 4 files changed, 177 insertions(+)
 create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
drivers/cpufreq/Kconfig.arm +10 -0
index de55c7d..0bfd40e 100644
@@ -124,6 +124,16 @@ config ARM_OMAP2PLUS_CPUFREQ
 	depends on ARCH_OMAP2PLUS
 	default ARCH_OMAP2PLUS
 
+config ARM_QCOM_CPUFREQ_KRYO
+	bool "Qualcomm Kryo based CPUFreq"
+	depends on QCOM_QFPROM
+	depends on QCOM_SMEM
+	select PM_OPP
+	help
+	  This adds the CPUFreq driver for Qualcomm Kryo SoC based boards.
+
+	  If in doubt, say N.
+
 config ARM_S3C_CPUFREQ
 	bool
 	help
drivers/cpufreq/Makefile +1 -0
index 8d24ade..fb4a2ec 100644
@@ -65,6 +65,7 @@ obj-$(CONFIG_MACH_MVEBU_V7)		+= mvebu-cpufreq.o
 obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ)	+= omap-cpufreq.o
 obj-$(CONFIG_ARM_PXA2xx_CPUFREQ)	+= pxa2xx-cpufreq.o
 obj-$(CONFIG_PXA3xx)			+= pxa3xx-cpufreq.o
+obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)	+= qcom-cpufreq-kryo.o
 obj-$(CONFIG_ARM_S3C2410_CPUFREQ)	+= s3c2410-cpufreq.o
 obj-$(CONFIG_ARM_S3C2412_CPUFREQ)	+= s3c2412-cpufreq.o
 obj-$(CONFIG_ARM_S3C2416_CPUFREQ)	+= s3c2416-cpufreq.o
drivers/cpufreq/cpufreq-dt-platdev.c +3 -0
index 3b585e4..77d6ab8 100644
@@ -118,6 +118,9 @@
 
 	{ .compatible = "nvidia,tegra124", },
 
+	{ .compatible = "qcom,apq8096", },
+	{ .compatible = "qcom,msm8996", },
+
 	{ .compatible = "st,stih407", },
 	{ .compatible = "st,stih410", },
 
drivers/cpufreq/qcom-cpufreq-kryo.c new file +163 -0

163 lines not on this page · raw message

Signature · 1 line
1.9.1
maintainer Acked-by on v1 1/1
#34 lore raw

Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

Quoted 10 lines · Ilia Lin

In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, the CPU frequency subset and voltage value of each OPP varies based on the silicon variant in use. Qualcomm 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 to provide the OPP framework with required information.
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.

Quoted 8 lines
---
 drivers/cpufreq/Kconfig.arm          |  10 +++
 drivers/cpufreq/Makefile             |   1 +
 drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
 drivers/cpufreq/qcom-cpufreq-kryo.c  | 163 +++++++++++++++++++++++++++++++++++
 4 files changed, 177 insertions(+)
 create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c

Hi Ilia,

So the patch looks good now. But I don't see how this series is going to get merged. You are touching all parts of the kernel and no single maintainer would be able to get these merged easily.

What I would suggest is that you divided the series in at least 3 parts.

- Patch 10 and 11, just the cpufreq stuff. So that Rafael can apply those independently. Also mention the dependency on my OPP patches in the cover letter for those two patches.

- All clk patches in another series, so that Stephen can apply them.

- And finally everything else to go via ARM-Soc.

Maybe things wouldn't be that simple, but that's what you would need to do to get things merged. And please add all Acked by tags you got to the patches.

For this patch:

--
viresh

#35 lore raw

Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

In reply to Ilia Lin
Quoted 3 lines · Ilia Lin
drivers/cpufreq/qcom-cpufreq-kryo.c new file +3 -0
+	np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
+	if (IS_ERR(np))
+		return PTR_ERR(np);

...

drivers/cpufreq/qcom-cpufreq-kryo.c line 143

Quoted 3 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +3 -0
+
+	pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
+	if (!IS_ERR(pdev))

Do you need to hold a reference to `np' here?

Quoted 1 line
drivers/cpufreq/qcom-cpufreq-kryo.c new file +1 -0
+		return 0;
Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
maintainer
#36 lore raw

Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

Quoted 10 lines · Russell King - ARM Linux
Quoted 3 lines · Ilia Lin
drivers/cpufreq/qcom-cpufreq-kryo.c new file +3 -0
+	np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
+	if (IS_ERR(np))
+		return PTR_ERR(np);

...

drivers/cpufreq/qcom-cpufreq-kryo.c line 143

Quoted 3 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +3 -0
+
+	pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
+	if (!IS_ERR(pdev))

Do you need to hold a reference to `np' here?

I am starting to feel bad for Ilia now. The problem is that there was a lot of stuff wrong with the patch and even with so many reviewers it wasn't easy to notice all the problems it had.

But you are right, this reference needs to be dropped.

Signature · 1 line
viresh
author
#37 lore raw

PATCH 1/1 cpufreq: Add Kryo CPU scaling driver

In reply to Ilia Lin

In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, the CPU frequency subset and voltage value of each OPP varies based on the silicon variant in use. Qualcomm 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 to provide the OPP framework with required information.
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.

---
 drivers/cpufreq/Kconfig.arm          |  10 +++
 drivers/cpufreq/Makefile             |   1 +
 drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
 drivers/cpufreq/qcom-cpufreq-kryo.c  | 164 +++++++++++++++++++++++++++++++++++
 4 files changed, 178 insertions(+)
 create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
drivers/cpufreq/Kconfig.arm +10 -0
index de55c7d..0bfd40e 100644
@@ -124,6 +124,16 @@ config ARM_OMAP2PLUS_CPUFREQ
 	depends on ARCH_OMAP2PLUS
 	default ARCH_OMAP2PLUS
 
+config ARM_QCOM_CPUFREQ_KRYO
+	bool "Qualcomm Kryo based CPUFreq"
+	depends on QCOM_QFPROM
+	depends on QCOM_SMEM
+	select PM_OPP
+	help
+	  This adds the CPUFreq driver for Qualcomm Kryo SoC based boards.
+
+	  If in doubt, say N.
+
 config ARM_S3C_CPUFREQ
 	bool
 	help
drivers/cpufreq/Makefile +1 -0
index 8d24ade..fb4a2ec 100644
@@ -65,6 +65,7 @@ obj-$(CONFIG_MACH_MVEBU_V7)		+= mvebu-cpufreq.o
 obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ)	+= omap-cpufreq.o
 obj-$(CONFIG_ARM_PXA2xx_CPUFREQ)	+= pxa2xx-cpufreq.o
 obj-$(CONFIG_PXA3xx)			+= pxa3xx-cpufreq.o
+obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)	+= qcom-cpufreq-kryo.o
 obj-$(CONFIG_ARM_S3C2410_CPUFREQ)	+= s3c2410-cpufreq.o
 obj-$(CONFIG_ARM_S3C2412_CPUFREQ)	+= s3c2412-cpufreq.o
 obj-$(CONFIG_ARM_S3C2416_CPUFREQ)	+= s3c2416-cpufreq.o
drivers/cpufreq/cpufreq-dt-platdev.c +3 -0
index 3b585e4..77d6ab8 100644
@@ -118,6 +118,9 @@
 
 	{ .compatible = "nvidia,tegra124", },
 
+	{ .compatible = "qcom,apq8096", },
+	{ .compatible = "qcom,msm8996", },
+
 	{ .compatible = "st,stih407", },
 	{ .compatible = "st,stih410", },
 
drivers/cpufreq/qcom-cpufreq-kryo.c new file +164 -0

164 lines not on this page · raw message

Signature · 1 line
1.9.1
maintainer Acked-by on v1 1/1
#38 lore raw

Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

Would have been better if you would have updated the subject as:

[PATCH v10 10/15] cpufreq: Add Kryo CPU scaling driver

Quoted 10 lines · Ilia Lin

In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors, the CPU frequency subset and voltage value of each OPP varies based on the silicon variant in use. Qualcomm 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 to provide the OPP framework with required information.
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.

Quoted 2 lines

Yes, I acked it earlier, but then comments came back. You should drop the tags in such cases normally.

Anyway, the patch looks fine now.

Signature · 1 line
viresh
#39 lore raw

Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

In reply to Ilia Lin

drivers/cpufreq/qcom-cpufreq-kryo.c line 32

Quoted 2 lines · Ilia Lin
drivers/cpufreq/qcom-cpufreq-kryo.c new file +2 -0
+#define SILVER_LEAD	0
+#define GOLD_LEAD	2

Okay, two different values here, but "GOLD_LEAD" appears unused.

drivers/cpufreq/qcom-cpufreq-kryo.c line 92

Quoted 7 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +7 -0
+	cpu_dev_silver = get_cpu_device(SILVER_LEAD);
+	if (NULL == cpu_dev_silver)
+		return -ENODEV;
+
+	cpu_dev_gold = get_cpu_device(SILVER_LEAD);
+	if (NULL == cpu_dev_gold)
+		return -ENODEV;

get_cpu_device() takes the logical CPU number. So the above gets CPU 0 each time, and so cpu_dev_silver == cpu_dev_gold here. So what's the point of the second get_cpu_device() ? If it's supposed to be:

	cpu_dev_gold = get_cpu_device(GOLD_LEAD);

That would get CPU 2, but in terms of these defines, it doesn't make that much sense. What exactly does "silver lead" and "gold lead" refer to in these definitions?

drivers/cpufreq/qcom-cpufreq-kryo.c line 143

Quoted 13 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +13 -0
+	opp_silver = dev_pm_opp_set_supported_hw(cpu_dev_silver,&versions,1);
+	if (IS_ERR(opp_silver)) {
+		dev_err(cpu_dev_silver, "Failed to set supported hardware\n");
+		ret = PTR_ERR(opp_silver);
+		goto free_np;
+	}
+
+	opp_gold = dev_pm_opp_set_supported_hw(cpu_dev_gold,&versions,1);
+	if (IS_ERR(opp_gold)) {
+		dev_err(cpu_dev_gold, "Failed to set supported hardware\n");
+		ret = PTR_ERR(opp_gold);
+		goto free_opp_silver;
+	}

Given that cpu_dev_silver == cpu_dev_gold, doesn't the second call to dev_pm_opp_set_supported_hw() always fail, as opp_table->supported_hw will be set by the first call?

To me, this driver looks completely useless as it will always fail to initialise, and I question whether this code has even been runtime tested.

Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
#40 lore raw

RE: [PATCH] cpufreq: Add Kryo CPU scaling driver

You are right.
cpu_dev_silver != cpu_dev_gold, and I found this with my tests as well. Thank you.

Quoted 28 lines

-----Original Message-----
From: Russell King - ARM Linux <[email protected]>
Sent: Monday, May 21, 2018 13:54
To: Ilia Lin <[email protected]>
Cc: viresh.kumar at linaro.org; devicetree at vger.kernel.org; linux- pm at vger.kernel.org; linux-arm-msm at vger.kernel.org; linux- kernel at vger.kernel.org; linux-soc at vger.kernel.org; linux- clk at vger.kernel.org; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

drivers/cpufreq/qcom-cpufreq-kryo.c line 32

Quoted 2 lines · Ilia Lin
drivers/cpufreq/qcom-cpufreq-kryo.c new file +2 -0
+#define SILVER_LEAD	0
+#define GOLD_LEAD	2

Okay, two different values here, but "GOLD_LEAD" appears unused.

drivers/cpufreq/qcom-cpufreq-kryo.c line 92

Quoted 7 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +7 -0
+	cpu_dev_silver = get_cpu_device(SILVER_LEAD);
+	if (NULL == cpu_dev_silver)
+		return -ENODEV;
+
+	cpu_dev_gold = get_cpu_device(SILVER_LEAD);
+	if (NULL == cpu_dev_gold)
+		return -ENODEV;

get_cpu_device() takes the logical CPU number. So the above gets CPU 0 each time, and so cpu_dev_silver == cpu_dev_gold here. So what's the point of the second get_cpu_device() ? If it's supposed to be:

Quoted 4 lines
	cpu_dev_gold = get_cpu_device(GOLD_LEAD);

That would get CPU 2, but in terms of these defines, it doesn't make that much sense. What exactly does "silver lead" and "gold lead" refer to in

these

Quoted 22 lines

definitions?

Review

Quoted 1 line
(quoted diff) +1 -0
+	opp_silver =

dev_pm_opp_set_supported_hw(cpu_dev_silver,&versions,1);

Review

Quoted 2 lines
(quoted diff) +2 -0
+	if (IS_ERR(opp_silver)) {
+		dev_err(cpu_dev_silver, "Failed to set supported

hardware\n");

drivers/cpufreq/qcom-cpufreq-kryo.c

Quoted 5 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +5 -0
+		ret = PTR_ERR(opp_silver);
+		goto free_np;
+	}
+
+	opp_gold =

dev_pm_opp_set_supported_hw(cpu_dev_gold,&versions,1);

Review

Quoted 2 lines
(quoted diff) +2 -0
+	if (IS_ERR(opp_gold)) {
+		dev_err(cpu_dev_gold, "Failed to set supported

hardware\n");

drivers/cpufreq/qcom-cpufreq-kryo.c line 143

Quoted 3 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +3 -0
+		ret = PTR_ERR(opp_gold);
+		goto free_opp_silver;
+	}

Given that cpu_dev_silver == cpu_dev_gold, doesn't the second call to dev_pm_opp_set_supported_hw() always fail, as opp_table-

Quoted 3 lines
Quoted 1 line

supported_hw will be set by the first call?

To me, this driver looks completely useless as it will always fail to

initialise,

Quoted 6 lines

and I question whether this code has even been runtime tested.

--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up

#41 lore raw

Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

Quoted 32 lines · ilialin at codeaurora.org

You are right.
cpu_dev_silver != cpu_dev_gold, and I found this with my tests as well. Thank you.

Quoted 28 lines

-----Original Message-----
From: Russell King - ARM Linux <[email protected]>
Sent: Monday, May 21, 2018 13:54
To: Ilia Lin <[email protected]>
Cc: viresh.kumar at linaro.org; devicetree at vger.kernel.org; linux- pm at vger.kernel.org; linux-arm-msm at vger.kernel.org; linux- kernel at vger.kernel.org; linux-soc at vger.kernel.org; linux- clk at vger.kernel.org; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

drivers/cpufreq/qcom-cpufreq-kryo.c line 32

Quoted 2 lines · Ilia Lin
drivers/cpufreq/qcom-cpufreq-kryo.c new file +2 -0
+#define SILVER_LEAD	0
+#define GOLD_LEAD	2

Okay, two different values here, but "GOLD_LEAD" appears unused.

drivers/cpufreq/qcom-cpufreq-kryo.c line 92

Quoted 7 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +7 -0
+	cpu_dev_silver = get_cpu_device(SILVER_LEAD);
+	if (NULL == cpu_dev_silver)
+		return -ENODEV;
+
+	cpu_dev_gold = get_cpu_device(SILVER_LEAD);
+	if (NULL == cpu_dev_gold)
+		return -ENODEV;

get_cpu_device() takes the logical CPU number. So the above gets CPU 0 each time, and so cpu_dev_silver == cpu_dev_gold here. So what's the point of the second get_cpu_device() ? If it's supposed to be:

Quoted 6 lines
Quoted 4 lines
	cpu_dev_gold = get_cpu_device(GOLD_LEAD);

That would get CPU 2, but in terms of these defines, it doesn't make that much sense. What exactly does "silver lead" and "gold lead" refer to in

these

Quoted 1 line

definitions?

I think you still need to explain this.

Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
#42 lore raw

RE: [PATCH] cpufreq: Add Kryo CPU scaling driver

There are 2 CPU clusters in the Kryo, CPU 0 and 1 are called Silver Cluster and CPU 2 and 3 - Gold Cluster. Each cluster has single clock. The clusters differ in terms of speed capabilities, computing power and power consumption. Therefore, I define separate OPP table for each cluster, and my driver will choose the appropriate OPP subset for each cluster. Lead refers to first CPU in the cluster.

Quoted 10 lines

-----Original Message-----
From: Russell King - ARM Linux <[email protected]>
Sent: Monday, May 21, 2018 15:12
To: ilialin at codeaurora.org
Cc: viresh.kumar at linaro.org; devicetree at vger.kernel.org; linux- pm at vger.kernel.org; linux-arm-msm at vger.kernel.org; linux- kernel at vger.kernel.org; linux-soc at vger.kernel.org; linux- clk at vger.kernel.org; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

Quoted 32 lines
Quoted 31 lines · ilialin at codeaurora.org

You are right.
cpu_dev_silver != cpu_dev_gold, and I found this with my tests as well. Thank you.

Quoted 27 lines

-----Original Message-----
From: Russell King - ARM Linux <[email protected]>
Sent: Monday, May 21, 2018 13:54
To: Ilia Lin <[email protected]>
Cc: viresh.kumar at linaro.org; devicetree at vger.kernel.org; linux- pm at vger.kernel.org; linux-arm-msm at vger.kernel.org; linux- kernel at vger.kernel.org; linux-soc at vger.kernel.org; linux- clk at vger.kernel.org; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

drivers/cpufreq/qcom-cpufreq-kryo.c line 32

Quoted 2 lines · Ilia Lin
drivers/cpufreq/qcom-cpufreq-kryo.c new file +2 -0
+#define SILVER_LEAD	0
+#define GOLD_LEAD	2

Okay, two different values here, but "GOLD_LEAD" appears unused.

drivers/cpufreq/qcom-cpufreq-kryo.c line 92

Quoted 7 lines
drivers/cpufreq/qcom-cpufreq-kryo.c new file +7 -0
+	cpu_dev_silver = get_cpu_device(SILVER_LEAD);
+	if (NULL == cpu_dev_silver)
+		return -ENODEV;
+
+	cpu_dev_gold = get_cpu_device(SILVER_LEAD);
+	if (NULL == cpu_dev_gold)
+		return -ENODEV;

get_cpu_device() takes the logical CPU number. So the above gets CPU 0 each time, and so cpu_dev_silver == cpu_dev_gold here. So what's the point of the second get_cpu_device() ? If it's supposed to

be:

Quoted 15 lines
Quoted 8 lines
Quoted 6 lines
	cpu_dev_gold = get_cpu_device(GOLD_LEAD);

That would get CPU 2, but in terms of these defines, it doesn't make that much sense. What exactly does "silver lead" and "gold lead" refer to in

these

Quoted 1 line

definitions?

I think you still need to explain this.

--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up

#43 lore raw

Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

Quoted 6 lines · ilialin at codeaurora.org

There are 2 CPU clusters in the Kryo, CPU 0 and 1 are called Silver Cluster and CPU 2 and 3 - Gold Cluster. Each cluster has single clock. The clusters differ in terms of speed capabilities, computing power and power consumption. Therefore, I define separate OPP table for each cluster, and my driver will choose the appropriate OPP subset for each cluster. Lead refers to first CPU in the cluster.

Ah, that is really confusing. Lead can means many things. Maybe a little more verbosity with the names such as SILVER_CLUSTER_LEAD_CPU would help?

Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
author
#44 lore raw

PATCH v8 11/15 dt-bindings: cpufreq: Document operating-points-v2-kryo-cpu

In reply to Ilia Lin

The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC to provide the OPP framework with required information.
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.

This change adds documentation for the DT bindings.
The "operating-points-v2-kryo-cpu" DT extends the "operating-points-v2" with following parameters:
- nvmem-cells (NVMEM area containig the speedbin information)
- opp-supported-hw: A single 32 bit bitmap value, representing compatible HW:

			0:	MSM8996 V3, speedbin 0
			1:	MSM8996 V3, speedbin 1
			2:	MSM8996 V3, speedbin 2
			3:	unused
			4:	MSM8996 SG, speedbin 0
			5:	MSM8996 SG, speedbin 1
			6:	MSM8996 SG, speedbin 2
			7-31:	unused
---
 .../devicetree/bindings/opp/kryo-cpufreq.txt       | 680 +++++++++++++++++++++
 1 file changed, 680 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
Documentation/devicetree/bindings/opp/kryo-cpufreq.txt new file +680 -0
new file mode 100644
index 0000000..c2127b9
@@ -0,0 +1,680 @@
+Qualcomm Technologies, Inc. KRYO 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.
+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
+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.
+
+Required properties:
+--------------------
+In 'cpus' nodes:
+- operating-points-v2: Phandle to the operating-points-v2 table to use.
+
+In 'operating-points-v2' table:
+- compatible: Should be
+	- 'operating-points-v2-kryo-cpu' for apq8096 and msm8996.
+- 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
+		value pair.
+		Please refer the for nvmem-cells
+		bindings Documentation/devicetree/bindings/nvmem/nvmem.txt
+		and also examples below.
+
+In every OPP node:
+- opp-supported-hw: A single 32 bit bitmap value, representing compatible HW.
+		    Bitmap:
+			0:	MSM8996 V3, speedbin 0
+			1:	MSM8996 V3, speedbin 1
+			2:	MSM8996 V3, speedbin 2
+			3:	unused
+			4:	MSM8996 SG, speedbin 0
+			5:	MSM8996 SG, speedbin 1
+			6:	MSM8996 SG, speedbin 2
+			7-31:	unused
+
+Example 1:
+---------
+
+	cpus {
+		#address-cells = <2>;
+		#size-cells = <0>;
+
+		CPU0: cpu at 0 {
+			device_type = "cpu";
+			compatible = "qcom,kryo";
+			reg = <0x0 0x0>;
+			enable-method = "psci";
+			clocks = <&kryocc 0>;
+			cpu-supply = <&pm8994_s11_saw>;
+			operating-points-v2 = <&cluster0_opp>;
+			#cooling-cells = <2>;
+			next-level-cache = <&L2_0>;
+			L2_0: l2-cache {
+			      compatible = "cache";
+			      cache-level = <2>;
+			};
+		};
+
+		CPU1: cpu at 1 {
+			device_type = "cpu";
+			compatible = "qcom,kryo";
+			reg = <0x0 0x1>;
+			enable-method = "psci";
+			clocks = <&kryocc 0>;
+			cpu-supply = <&pm8994_s11_saw>;
+			operating-points-v2 = <&cluster0_opp>;
+			#cooling-cells = <2>;
+			next-level-cache = <&L2_0>;
+		};
+
+		CPU2: cpu at 100 {
+			device_type = "cpu";
+			compatible = "qcom,kryo";
+			reg = <0x0 0x100>;
+			enable-method = "psci";
+			clocks = <&kryocc 1>;
+			cpu-supply = <&pm8994_s11_saw>;
+			operating-points-v2 = <&cluster1_opp>;
+			#cooling-cells = <2>;
+			next-level-cache = <&L2_1>;
+			L2_1: l2-cache {
+			      compatible = "cache";
+			      cache-level = <2>;
+			};
+		};
+
+		CPU3: cpu at 101 {
+			device_type = "cpu";
+			compatible = "qcom,kryo";
+			reg = <0x0 0x101>;
+			enable-method = "psci";
+			clocks = <&kryocc 1>;
+			cpu-supply = <&pm8994_s11_saw>;
+			operating-points-v2 = <&cluster1_opp>;
+			#cooling-cells = <2>;
+			next-level-cache = <&L2_1>;
+		};
+
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&CPU0>;
+				};
+
+				core1 {
+					cpu = <&CPU1>;
+				};
+			};
+
+			cluster1 {
+				core0 {
+					cpu = <&CPU2>;
+				};
+
+				core1 {
+					cpu = <&CPU3>;
+				};
+			};
+		};
+	};
+
+	cluster0_opp: opp_table0 {
+		compatible = "operating-points-v2-kryo-cpu";
+		nvmem-cells = <&speedbin_efuse>;
+		opp-shared;
+
+		opp-307200000 {
+			opp-hz = /bits/ 64 <307200000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x77>;
+			clock-latency-ns = <200000>;
+		};
+		opp-384000000 {
+			opp-hz = /bits/ 64 <384000000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-422400000 {
+			opp-hz = /bits/ 64 <422400000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-460800000 {
+			opp-hz = /bits/ 64 <460800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-480000000 {
+			opp-hz = /bits/ 64 <480000000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-537600000 {
+			opp-hz = /bits/ 64 <537600000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-556800000 {
+			opp-hz = /bits/ 64 <556800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-614400000 {
+			opp-hz = /bits/ 64 <614400000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-652800000 {
+			opp-hz = /bits/ 64 <652800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-691200000 {
+			opp-hz = /bits/ 64 <691200000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-729600000 {
+			opp-hz = /bits/ 64 <729600000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-768000000 {
+			opp-hz = /bits/ 64 <768000000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-844800000 {
+			opp-hz = /bits/ 64 <844800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x77>;
+			clock-latency-ns = <200000>;
+		};
+		opp-902400000 {
+			opp-hz = /bits/ 64 <902400000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-960000000 {
+			opp-hz = /bits/ 64 <960000000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-979200000 {
+			opp-hz = /bits/ 64 <979200000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1036800000 {
+			opp-hz = /bits/ 64 <1036800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1056000000 {
+			opp-hz = /bits/ 64 <1056000000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1113600000 {
+			opp-hz = /bits/ 64 <1113600000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1132800000 {
+			opp-hz = /bits/ 64 <1132800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1190400000 {
+			opp-hz = /bits/ 64 <1190400000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1209600000 {
+			opp-hz = /bits/ 64 <1209600000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1228800000 {
+			opp-hz = /bits/ 64 <1228800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1286400000 {
+			opp-hz = /bits/ 64 <1286400000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1324800000 {
+			opp-hz = /bits/ 64 <1324800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x5>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1363200000 {
+			opp-hz = /bits/ 64 <1363200000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x72>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1401600000 {
+			opp-hz = /bits/ 64 <1401600000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x5>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1440000000 {
+			opp-hz = /bits/ 64 <1440000000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1478400000 {
+			opp-hz = /bits/ 64 <1478400000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x1>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1497600000 {
+			opp-hz = /bits/ 64 <1497600000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x4>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1516800000 {
+			opp-hz = /bits/ 64 <1516800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1593600000 {
+			opp-hz = /bits/ 64 <1593600000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x71>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1996800000 {
+			opp-hz = /bits/ 64 <1996800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x20>;
+			clock-latency-ns = <200000>;
+		};
+		opp-2188800000 {
+			opp-hz = /bits/ 64 <2188800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x10>;
+			clock-latency-ns = <200000>;
+		};
+	};
+
+	cluster1_opp: opp_table1 {
+		compatible = "operating-points-v2-kryo-cpu";
+		nvmem-cells = <&speedbin_efuse>;
+		opp-shared;
+
+		opp-307200000 {
+			opp-hz = /bits/ 64 <307200000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x77>;
+			clock-latency-ns = <200000>;
+		};
+		opp-384000000 {
+			opp-hz = /bits/ 64 <384000000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-403200000 {
+			opp-hz = /bits/ 64 <403200000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-460800000 {
+			opp-hz = /bits/ 64 <460800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-480000000 {
+			opp-hz = /bits/ 64 <480000000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-537600000 {
+			opp-hz = /bits/ 64 <537600000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-556800000 {
+			opp-hz = /bits/ 64 <556800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-614400000 {
+			opp-hz = /bits/ 64 <614400000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-652800000 {
+			opp-hz = /bits/ 64 <652800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-691200000 {
+			opp-hz = /bits/ 64 <691200000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-729600000 {
+			opp-hz = /bits/ 64 <729600000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-748800000 {
+			opp-hz = /bits/ 64 <748800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-806400000 {
+			opp-hz = /bits/ 64 <806400000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-825600000 {
+			opp-hz = /bits/ 64 <825600000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-883200000 {
+			opp-hz = /bits/ 64 <883200000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-902400000 {
+			opp-hz = /bits/ 64 <902400000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-940800000 {
+			opp-hz = /bits/ 64 <940800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-979200000 {
+			opp-hz = /bits/ 64 <979200000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1036800000 {
+			opp-hz = /bits/ 64 <1036800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1056000000 {
+			opp-hz = /bits/ 64 <1056000000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1113600000 {
+			opp-hz = /bits/ 64 <1113600000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1132800000 {
+			opp-hz = /bits/ 64 <1132800000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1190400000 {
+			opp-hz = /bits/ 64 <1190400000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1209600000 {
+			opp-hz = /bits/ 64 <1209600000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1248000000 {
+			opp-hz = /bits/ 64 <1248000000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1286400000 {
+			opp-hz = /bits/ 64 <1286400000>;
+			opp-microvolt = <905000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1324800000 {
+			opp-hz = /bits/ 64 <1324800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1363200000 {
+			opp-hz = /bits/ 64 <1363200000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1401600000 {
+			opp-hz = /bits/ 64 <1401600000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1440000000 {
+			opp-hz = /bits/ 64 <1440000000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1478400000 {
+			opp-hz = /bits/ 64 <1478400000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1516800000 {
+			opp-hz = /bits/ 64 <1516800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1555200000 {
+			opp-hz = /bits/ 64 <1555200000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1593600000 {
+			opp-hz = /bits/ 64 <1593600000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1632000000 {
+			opp-hz = /bits/ 64 <1632000000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1670400000 {
+			opp-hz = /bits/ 64 <1670400000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1708800000 {
+			opp-hz = /bits/ 64 <1708800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1747200000 {
+			opp-hz = /bits/ 64 <1747200000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x70>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1785600000 {
+			opp-hz = /bits/ 64 <1785600000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x7>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1804800000 {
+			opp-hz = /bits/ 64 <1804800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x6>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1824000000 {
+			opp-hz = /bits/ 64 <1824000000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x71>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1900800000 {
+			opp-hz = /bits/ 64 <1900800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x74>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1920000000 {
+			opp-hz = /bits/ 64 <1920000000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x1>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1977600000 {
+			opp-hz = /bits/ 64 <1977600000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x30>;
+			clock-latency-ns = <200000>;
+		};
+		opp-1996800000 {
+			opp-hz = /bits/ 64 <1996800000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x1>;
+			clock-latency-ns = <200000>;
+		};
+		opp-2054400000 {
+			opp-hz = /bits/ 64 <2054400000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x30>;
+			clock-latency-ns = <200000>;
+		};
+		opp-2073600000 {
+			opp-hz = /bits/ 64 <2073600000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x1>;
+			clock-latency-ns = <200000>;
+		};
+		opp-2150400000 {
+			opp-hz = /bits/ 64 <2150400000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x31>;
+			clock-latency-ns = <200000>;
+		};
+		opp-2246400000 {
+			opp-hz = /bits/ 64 <2246400000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x10>;
+			clock-latency-ns = <200000>;
+		};
+		opp-2342400000 {
+			opp-hz = /bits/ 64 <2342400000>;
+			opp-microvolt = <1140000 905000 1140000>;
+			opp-supported-hw = <0x10>;
+			clock-latency-ns = <200000>;
+		};
+	};
+
+....
+
+reserved-memory {
+	#address-cells = <2>;
+	#size-cells = <2>;
+	ranges;
+....
+	smem_mem: smem-mem at 86000000 {
+		reg = <0x0 0x86000000 0x0 0x200000>;
+		no-map;
+	};
+....
+};
+
+smem {
+	compatible = "qcom,smem";
+	memory-region = <&smem_mem>;
+	hwlocks = <&tcsr_mutex 3>;
+};
+
+soc {
+....
+	qfprom: qfprom at 74000 {
+		compatible = "qcom,qfprom";
+		reg = <0x00074000 0x8ff>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		....
+		speedbin_efuse: speedbin at 133 {
+			reg = <0x133 0x1>;
+			bits = <5 3>;
+		};
+	};
+};
Signature · 1 line
1.9.1
maintainer Reviewed-by on v8 11/15
#45 lore raw

Re: [PATCH v8 11/15] dt-bindings: cpufreq: Document operating-points-v2-kryo-cpu

Quoted 20 lines · Ilia Lin

The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC to provide the OPP framework with required information.
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.

This change adds documentation for the DT bindings.
The "operating-points-v2-kryo-cpu" DT extends the "operating-points-v2" with following parameters:
- nvmem-cells (NVMEM area containig the speedbin information)
- opp-supported-hw: A single 32 bit bitmap value, representing compatible HW:

			0:	MSM8996 V3, speedbin 0
			1:	MSM8996 V3, speedbin 1
			2:	MSM8996 V3, speedbin 2
			3:	unused
			4:	MSM8996 SG, speedbin 0
			5:	MSM8996 SG, speedbin 1
			6:	MSM8996 SG, speedbin 2
			7-31:	unused
Quoted 6 lines
---
 .../devicetree/bindings/opp/kryo-cpufreq.txt       | 680 +++++++++++++++++++++
 1 file changed, 680 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
author
#46 lore raw

PATCH v8 12/15 dt: qcom: Add qcom-cpufreq-kryo driver configuration

In reply to Ilia Lin

1. Add NVMEM node for the speedbin
2. Add definitions for all possible MSM8996 CPU OPPs. The qcom-cpufreq-kryo driver will select the appropriate subset.

---
 arch/arm64/boot/dts/qcom/apq8096-db820c.dts |   2 +-
 arch/arm64/boot/dts/qcom/msm8996.dtsi       | 281 ++++++++++++++++++++++++++--
 2 files changed, 270 insertions(+), 13 deletions(-)
arch/arm64/boot/dts/qcom/apq8096-db820c.dts +1 -1
index 230e9c8..da23bda 100644
@@ -17,5 +17,5 @@
 
 / {
 	model = "Qualcomm Technologies, Inc. DB820c";
-	compatible = "arrow,apq8096-db820c", "qcom,apq8096-sbc";
+	compatible = "arrow,apq8096-db820c", "qcom,apq8096-sbc", "qcom,apq8096";
 };
arch/arm64/boot/dts/qcom/msm8996.dtsi +269 -12

465 lines not on this page · raw message

Signature · 1 line
1.9.1
author
#47 lore raw

PATCH v8 13/15 regulator: qcom_spmi: Add support for SAW

In reply to Ilia Lin

Add support for SAW controlled regulators.
The regulators defined as SAW controlled in the device tree will be controlled through special CPU registers instead of direct SPMI accesses.
This is required especially for CPU supply regulators to synchronize with clock scaling and for Automatic Voltage Switching.

---
 drivers/regulator/qcom_spmi-regulator.c | 133 +++++++++++++++++++++++++++++++-
 1 file changed, 130 insertions(+), 3 deletions(-)
drivers/regulator/qcom_spmi-regulator.c +130 -3
index 63c7a0c..9817f1a 100644
@@ -25,6 +25,8 @@
 #include <linux/regulator/driver.h>
 #include <linux/regmap.h>
 #include <linux/list.h>
+#include <linux/mfd/syscon.h>
+#include <linux/io.h>
 
 /* Pin control enable input pins. */
 #define SPMI_REGULATOR_PIN_CTRL_ENABLE_NONE		0x00
@@ -181,6 +183,23 @@ enum spmi_boost_byp_registers {
 	SPMI_BOOST_BYP_REG_CURRENT_LIMIT	= 0x4b,
 };
 
+enum spmi_saw3_registers {
+	SAW3_SECURE				= 0x00,
+	SAW3_ID					= 0x04,
+	SAW3_SPM_STS				= 0x0C,
+	SAW3_AVS_STS				= 0x10,
+	SAW3_PMIC_STS				= 0x14,
+	SAW3_RST				= 0x18,
+	SAW3_VCTL				= 0x1C,
+	SAW3_AVS_CTL				= 0x20,
+	SAW3_AVS_LIMIT				= 0x24,
+	SAW3_AVS_DLY				= 0x28,
+	SAW3_AVS_HYSTERESIS			= 0x2C,
+	SAW3_SPM_STS2				= 0x38,
+	SAW3_SPM_PMIC_DATA_3			= 0x4C,
+	SAW3_VERSION				= 0xFD0,
+};
+
 /* Used for indexing into ctrl_reg.  These are offets from 0x40 */
 enum spmi_common_control_register_index {
 	SPMI_COMMON_IDX_VOLTAGE_RANGE		= 0,
@@ -1035,6 +1054,89 @@ static irqreturn_t spmi_regulator_vs_ocp_isr(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+#define SAW3_VCTL_DATA_MASK	0xFF
+#define SAW3_VCTL_CLEAR_MASK	0x700FF
+#define SAW3_AVS_CTL_EN_MASK	0x1
+#define SAW3_AVS_CTL_TGGL_MASK	0x8000000
+#define SAW3_AVS_CTL_CLEAR_MASK	0x7efc00
+
+static struct regmap *saw_regmap = NULL;
+
+static void spmi_saw_set_vdd(void *data)
+{
+	u32 vctl, data3, avs_ctl, pmic_sts;
+	bool avs_enabled = false;
+	unsigned long timeout;
+	u8 voltage_sel = *(u8 *)data;
+
+	regmap_read(saw_regmap, SAW3_AVS_CTL, &avs_ctl);
+	regmap_read(saw_regmap, SAW3_VCTL, &vctl);
+	regmap_read(saw_regmap, SAW3_SPM_PMIC_DATA_3, &data3);
+
+	/* select the band */
+	vctl &= ~SAW3_VCTL_CLEAR_MASK;
+	vctl |= (u32)voltage_sel;
+
+	data3 &= ~SAW3_VCTL_CLEAR_MASK;
+	data3 |= (u32)voltage_sel;
+
+	/* If AVS is enabled, switch it off during the voltage change */
+	avs_enabled = SAW3_AVS_CTL_EN_MASK & avs_ctl;
+	if (avs_enabled) {
+		avs_ctl &= ~SAW3_AVS_CTL_TGGL_MASK;
+		regmap_write(saw_regmap, SAW3_AVS_CTL, avs_ctl);
+	}
+
+	regmap_write(saw_regmap, SAW3_RST, 1);
+	regmap_write(saw_regmap, SAW3_VCTL, vctl);
+	regmap_write(saw_regmap, SAW3_SPM_PMIC_DATA_3, data3);
+
+	timeout = jiffies + usecs_to_jiffies(100);
+	do {
+		regmap_read(saw_regmap, SAW3_PMIC_STS, &pmic_sts);
+		pmic_sts &= SAW3_VCTL_DATA_MASK;
+		if (pmic_sts == (u32)voltage_sel)
+			break;
+
+		cpu_relax();
+
+	} while (time_before(jiffies, timeout));
+
+	/* After successful voltage change, switch the AVS back on */
+	if (avs_enabled) {
+		pmic_sts &= 0x3f;
+		avs_ctl &= ~SAW3_AVS_CTL_CLEAR_MASK;
+		avs_ctl |= ((pmic_sts - 4) << 10);
+		avs_ctl |= (pmic_sts << 17);
+		avs_ctl |= SAW3_AVS_CTL_TGGL_MASK;
+		regmap_write(saw_regmap, SAW3_AVS_CTL, avs_ctl);
+	}
+}
+
+static int
+spmi_regulator_saw_set_voltage(struct regulator_dev *rdev, unsigned selector)
+{
+	struct spmi_regulator *vreg = rdev_get_drvdata(rdev);
+	int ret;
+	u8 range_sel, voltage_sel;
+
+	ret = spmi_sw_selector_to_hw(vreg, selector, &range_sel, &voltage_sel);
+	if (ret)
+		return ret;
+
+	if (0 != range_sel) {
+		dev_dbg(&rdev->dev, "range_sel = %02X voltage_sel = %02X", \
+			range_sel, voltage_sel);
+		return -EINVAL;
+	}
+
+	/* Always do the SAW register writes on the first CPU */
+	return smp_call_function_single(0, spmi_saw_set_vdd, \
+					&voltage_sel, true);
+}
+
+static struct regulator_ops spmi_saw_ops = {};
+
 static struct regulator_ops spmi_smps_ops = {
 	.enable			= regulator_enable_regmap,
 	.disable		= regulator_disable_regmap,
@@ -1250,6 +1352,7 @@ static int spmi_regulator_match(struct spmi_regulator *vreg, u16 force_type)
 	}
 	dig_major_rev	= version[SPMI_COMMON_REG_DIG_MAJOR_REV
 					- SPMI_COMMON_REG_DIG_MAJOR_REV];
+
 	if (!force_type) {
 		type		= version[SPMI_COMMON_REG_TYPE -
 					  SPMI_COMMON_REG_DIG_MAJOR_REV];
@@ -1648,7 +1751,9 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
 	struct regmap *regmap;
 	const char *name;
 	struct device *dev = &pdev->dev;
-	int ret;
+	struct device_node *node = pdev->dev.of_node;
+	struct device_node *syscon;
+	int ret, lenp;
 	struct list_head *vreg_list;
 
 	vreg_list = devm_kzalloc(dev, sizeof(*vreg_list), GFP_KERNEL);
@@ -1665,7 +1770,22 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
 	if (!match)
 		return -ENODEV;
 
+	if (of_find_property(node, "qcom,saw-reg", &lenp)) {
+		syscon = of_parse_phandle(node, "qcom,saw-reg", 0);
+		saw_regmap = syscon_node_to_regmap(syscon);
+		of_node_put(syscon);
+		if (IS_ERR(regmap))
+			dev_err(dev, "ERROR reading SAW regmap\n");
+	}
+
 	for (reg = match->data; reg->name; reg++) {
+
+		if (saw_regmap && \
+		    of_find_property(of_find_node_by_name(node, reg->name), \
+				     "qcom,saw-slave", &lenp)) {
+			continue;
+		}
+
 		vreg = devm_kzalloc(dev, sizeof(*vreg), GFP_KERNEL);
 		if (!vreg)
 			return -ENOMEM;
@@ -1673,7 +1793,6 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
 		vreg->dev = dev;
 		vreg->base = reg->base;
 		vreg->regmap = regmap;
-
 		if (reg->ocp) {
 			vreg->ocp_irq = platform_get_irq_byname(pdev, reg->ocp);
 			if (vreg->ocp_irq < 0) {
@@ -1681,7 +1800,6 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
 				goto err;
 			}
 		}
-
 		vreg->desc.id = -1;
 		vreg->desc.owner = THIS_MODULE;
 		vreg->desc.type = REGULATOR_VOLTAGE;
@@ -1698,6 +1816,15 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
 		if (ret)
 			continue;
 
+		if (saw_regmap && \
+		    of_find_property(of_find_node_by_name(node, reg->name), \
+				     "qcom,saw-leader", &lenp)) {
+			spmi_saw_ops = *(vreg->desc.ops);
+			spmi_saw_ops.set_voltage_sel = \
+				spmi_regulator_saw_set_voltage;
+			vreg->desc.ops = &spmi_saw_ops;
+		}
+
 		config.dev = dev;
 		config.driver_data = vreg;
 		config.regmap = regmap;
Signature · 1 line
1.9.1
author
#48 lore raw

PATCH v8 14/15 dt-bindings: qcom_spmi: Document SAW support

In reply to Ilia Lin

Document the DT bindings for the SAW regulators.

The saw-leader is the only property that is configurable in DT.

The saw-slave property allows ganging (grouping) of several regulators so that their outputs can be combined.

---
 .../bindings/regulator/qcom,spmi-regulator.txt     | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)
Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt +45 -0
index 57d2c65..406f2e5 100644
@@ -110,6 +110,11 @@ Qualcomm SPMI Regulators
 	Definition: Reference to regulator supplying the input pin, as
 		    described in the data sheet.
 
+- qcom,saw-reg:
+	Usage: optional
+	Value type: <phandle>
+	Description: Reference to syscon node defining the SAW registers.
+
 
 The regulator node houses sub-nodes for each regulator within the device. Each
 sub-node is identified using the node's name, with valid values listed for each
@@ -201,6 +206,17 @@ see regulator.txt - with additional custom properties described below:
 			2 = 0.55 uA
 			3 = 0.75 uA
 
+- qcom,saw-slave:
+	Usage: optional
+	Value type: <boo>
+	Description: SAW controlled gang slave. Will not be configured.
+
+- qcom,saw-leader:
+	Usage: optional
+	Value type: <boo>
+	Description: SAW controlled gang leader. Will be configured as
+		     SAW regulator.
+
 Example:
 
 	regulators {
@@ -221,3 +237,32 @@ Example:
 
 		....
 	};
+
+Example 2:
+
+	saw3: syscon at 9A10000 {
+		compatible = "syscon";
+		reg = <0x9A10000 0x1000>;
+	};
+
+	...
+
+	spm-regulators {
+		compatible = "qcom,pm8994-regulators";
+		qcom,saw-reg = <&saw3>;
+		s8 {
+			qcom,saw-slave;
+		};
+		s9 {
+			qcom,saw-slave;
+		};
+		s10 {
+			qcom,saw-slave;
+		};
+		pm8994_s11_saw: s11 {
+			qcom,saw-leader;
+			regulator-always-on;
+			regulator-min-microvolt = <900000>;
+			regulator-max-microvolt = <1140000>;
+		};
+	};
Signature · 1 line
1.9.1
author
#49 lore raw

PATCH v8 15/15 dt: qcom: Add SAW regulator for 8x96 CPUs

In reply to Ilia Lin

1. Add syscon node for the SAW CPU registers
2. Add SAW regulators gang definition for s8-s11
3. Add voltages to the OPP tables
4. Add the s11 SAW regulator as CPU regulator

---
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 119 ++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)
arch/arm64/boot/dts/qcom/msm8996.dtsi +119 -0
index d96a112..871bfac 100644
@@ -8,6 +8,7 @@
 #include <dt-bindings/clock/qcom,mmcc-msm8996.h>
 #include <dt-bindings/clock/qcom,rpmcc.h>
 #include <dt-bindings/thermal/thermal.h>
+#include <dt-bindings/spmi/spmi.h>
 
 / {
 	model = "Qualcomm Technologies, Inc. MSM8996";
@@ -92,6 +93,7 @@
 			reg = <0x0 0x0>;
 			enable-method = "psci";
 			clocks = <&kryocc 0>;
+			cpu-supply = <&pm8994_s11_saw>;
 			operating-points-v2 = <&cluster0_opp>;
 			#cooling-cells = <2>;
 			next-level-cache = <&L2_0>;
@@ -107,6 +109,7 @@
 			reg = <0x0 0x1>;
 			enable-method = "psci";
 			clocks = <&kryocc 0>;
+			cpu-supply = <&pm8994_s11_saw>;
 			operating-points-v2 = <&cluster0_opp>;
 			#cooling-cells = <2>;
 			next-level-cache = <&L2_0>;
@@ -118,6 +121,7 @@
 			reg = <0x0 0x100>;
 			enable-method = "psci";
 			clocks = <&kryocc 1>;
+			cpu-supply = <&pm8994_s11_saw>;
 			operating-points-v2 = <&cluster1_opp>;
 			#cooling-cells = <2>;
 			next-level-cache = <&L2_1>;
@@ -133,6 +137,7 @@
 			reg = <0x0 0x101>;
 			enable-method = "psci";
 			clocks = <&kryocc 1>;
+			cpu-supply = <&pm8994_s11_saw>;
 			operating-points-v2 = <&cluster1_opp>;
 			#cooling-cells = <2>;
 			next-level-cache = <&L2_1>;
@@ -169,171 +174,205 @@
 
 		opp-307200000 {
 			opp-hz = /bits/ 64 <307200000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x77>;
 			clock-latency-ns = <200000>;
 		};
 		opp-384000000 {
 			opp-hz = /bits/ 64 <384000000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-422400000 {
 			opp-hz = /bits/ 64 <422400000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-460800000 {
 			opp-hz = /bits/ 64 <460800000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-480000000 {
 			opp-hz = /bits/ 64 <480000000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-537600000 {
 			opp-hz = /bits/ 64 <537600000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-556800000 {
 			opp-hz = /bits/ 64 <556800000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-614400000 {
 			opp-hz = /bits/ 64 <614400000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-652800000 {
 			opp-hz = /bits/ 64 <652800000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-691200000 {
 			opp-hz = /bits/ 64 <691200000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-729600000 {
 			opp-hz = /bits/ 64 <729600000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-768000000 {
 			opp-hz = /bits/ 64 <768000000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-844800000 {
 			opp-hz = /bits/ 64 <844800000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x77>;
 			clock-latency-ns = <200000>;
 		};
 		opp-902400000 {
 			opp-hz = /bits/ 64 <902400000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-960000000 {
 			opp-hz = /bits/ 64 <960000000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-979200000 {
 			opp-hz = /bits/ 64 <979200000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1036800000 {
 			opp-hz = /bits/ 64 <1036800000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1056000000 {
 			opp-hz = /bits/ 64 <1056000000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1113600000 {
 			opp-hz = /bits/ 64 <1113600000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1132800000 {
 			opp-hz = /bits/ 64 <1132800000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1190400000 {
 			opp-hz = /bits/ 64 <1190400000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1209600000 {
 			opp-hz = /bits/ 64 <1209600000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1228800000 {
 			opp-hz = /bits/ 64 <1228800000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1286400000 {
 			opp-hz = /bits/ 64 <1286400000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1324800000 {
 			opp-hz = /bits/ 64 <1324800000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x5>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1363200000 {
 			opp-hz = /bits/ 64 <1363200000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x72>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1401600000 {
 			opp-hz = /bits/ 64 <1401600000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x5>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1440000000 {
 			opp-hz = /bits/ 64 <1440000000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1478400000 {
 			opp-hz = /bits/ 64 <1478400000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x1>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1497600000 {
 			opp-hz = /bits/ 64 <1497600000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x4>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1516800000 {
 			opp-hz = /bits/ 64 <1516800000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1593600000 {
 			opp-hz = /bits/ 64 <1593600000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x71>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1996800000 {
 			opp-hz = /bits/ 64 <1996800000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x20>;
 			clock-latency-ns = <200000>;
 		};
 		opp-2188800000 {
 			opp-hz = /bits/ 64 <2188800000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x10>;
 			clock-latency-ns = <200000>;
 		};
@@ -346,251 +385,301 @@
 
 		opp-307200000 {
 			opp-hz = /bits/ 64 <307200000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x77>;
 			clock-latency-ns = <200000>;
 		};
 		opp-384000000 {
 			opp-hz = /bits/ 64 <384000000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-403200000 {
 			opp-hz = /bits/ 64 <403200000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-460800000 {
 			opp-hz = /bits/ 64 <460800000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-480000000 {
 			opp-hz = /bits/ 64 <480000000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-537600000 {
 			opp-hz = /bits/ 64 <537600000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-556800000 {
 			opp-hz = /bits/ 64 <556800000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-614400000 {
 			opp-hz = /bits/ 64 <614400000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-652800000 {
 			opp-hz = /bits/ 64 <652800000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-691200000 {
 			opp-hz = /bits/ 64 <691200000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-729600000 {
 			opp-hz = /bits/ 64 <729600000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-748800000 {
 			opp-hz = /bits/ 64 <748800000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-806400000 {
 			opp-hz = /bits/ 64 <806400000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-825600000 {
 			opp-hz = /bits/ 64 <825600000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-883200000 {
 			opp-hz = /bits/ 64 <883200000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-902400000 {
 			opp-hz = /bits/ 64 <902400000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-940800000 {
 			opp-hz = /bits/ 64 <940800000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-979200000 {
 			opp-hz = /bits/ 64 <979200000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1036800000 {
 			opp-hz = /bits/ 64 <1036800000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1056000000 {
 			opp-hz = /bits/ 64 <1056000000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1113600000 {
 			opp-hz = /bits/ 64 <1113600000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1132800000 {
 			opp-hz = /bits/ 64 <1132800000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1190400000 {
 			opp-hz = /bits/ 64 <1190400000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1209600000 {
 			opp-hz = /bits/ 64 <1209600000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1248000000 {
 			opp-hz = /bits/ 64 <1248000000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1286400000 {
 			opp-hz = /bits/ 64 <1286400000>;
+			opp-microvolt = <905000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1324800000 {
 			opp-hz = /bits/ 64 <1324800000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1363200000 {
 			opp-hz = /bits/ 64 <1363200000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1401600000 {
 			opp-hz = /bits/ 64 <1401600000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1440000000 {
 			opp-hz = /bits/ 64 <1440000000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1478400000 {
 			opp-hz = /bits/ 64 <1478400000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1516800000 {
 			opp-hz = /bits/ 64 <1516800000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1555200000 {
 			opp-hz = /bits/ 64 <1555200000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1593600000 {
 			opp-hz = /bits/ 64 <1593600000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1632000000 {
 			opp-hz = /bits/ 64 <1632000000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1670400000 {
 			opp-hz = /bits/ 64 <1670400000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1708800000 {
 			opp-hz = /bits/ 64 <1708800000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1747200000 {
 			opp-hz = /bits/ 64 <1747200000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x70>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1785600000 {
 			opp-hz = /bits/ 64 <1785600000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1804800000 {
 			opp-hz = /bits/ 64 <1804800000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x6>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1824000000 {
 			opp-hz = /bits/ 64 <1824000000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x71>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1900800000 {
 			opp-hz = /bits/ 64 <1900800000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x74>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1920000000 {
 			opp-hz = /bits/ 64 <1920000000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x1>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1977600000 {
 			opp-hz = /bits/ 64 <1977600000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x30>;
 			clock-latency-ns = <200000>;
 		};
 		opp-1996800000 {
 			opp-hz = /bits/ 64 <1996800000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x1>;
 			clock-latency-ns = <200000>;
 		};
 		opp-2054400000 {
 			opp-hz = /bits/ 64 <2054400000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x30>;
 			clock-latency-ns = <200000>;
 		};
 		opp-2073600000 {
 			opp-hz = /bits/ 64 <2073600000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x1>;
 			clock-latency-ns = <200000>;
 		};
 		opp-2150400000 {
 			opp-hz = /bits/ 64 <2150400000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x31>;
 			clock-latency-ns = <200000>;
 		};
 		opp-2246400000 {
 			opp-hz = /bits/ 64 <2246400000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x10>;
 			clock-latency-ns = <200000>;
 		};
 		opp-2342400000 {
 			opp-hz = /bits/ 64 <2342400000>;
+			opp-microvolt = <1140000 905000 1140000>;
 			opp-supported-hw = <0x10>;
 			clock-latency-ns = <200000>;
 		};
@@ -908,6 +997,11 @@
 			#mbox-cells = <1>;
 		};
 
+		saw3: syscon at 9A10000 {
+			compatible = "syscon";
+			reg = <0x9A10000 0x1000>;
+		};
+
 		gcc: clock-controller at 300000 {
 			compatible = "qcom,gcc-msm8996";
 			#clock-cells = <1>;
@@ -1134,6 +1228,31 @@
 			#size-cells = <0>;
 			interrupt-controller;
 			#interrupt-cells = <4>;
+			pmic at 1 {
+				compatible = "qcom,pm8994", "qcom,spmi-pmic";
+				reg = <0x1 SPMI_USID>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				spm-regulators {
+					compatible = "qcom,pm8994-regulators";
+					qcom,saw-reg = <&saw3>;
+					s8 {
+						qcom,saw-slave;
+					};
+					s9 {
+						qcom,saw-slave;
+					};
+					s10 {
+						qcom,saw-slave;
+					};
+					pm8994_s11_saw: s11 {
+						qcom,saw-leader;
+						regulator-always-on;
+						regulator-min-microvolt = <905000>;
+						regulator-max-microvolt = <1140000>;
+					};
+				};
+			};
 		};
 
 		mmcc: clock-controller at 8c0000 {
Signature · 1 line
1.9.1