linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186
@ 2022-04-22  7:52 Rex-BC Chen
  2022-04-22  7:52 ` [PATCH V4 01/14] dt-bindings: cpufreq: mediatek: Add MediaTek CCI property Rex-BC Chen
                   ` (14 more replies)
  0 siblings, 15 replies; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-22  7:52 UTC (permalink / raw)
  To: rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group, Rex-BC Chen

Cpufreq is a DVFS driver used for power saving to scale the clock frequency
and supply the voltage for CPUs. This series do some cleanup for MediaTek
cpufreq drivers and add support for MediaTek SVS[2] and MediaTek CCI
devfreq[3] which are supported in MT8183 and MT8186.

Changes for V4:
1. Revise drivers from reviewers' suggestion.
2. Fix name of opp table issue.

Changes for V3:
1. Rebased to linux-next-20220414.
2. Drop accepted patches.
3. Drop "cpufreq: mediatek: Use maximum voltage in init stage" because we
   make sure the voltage we set is safe for both mediatek cci and cpufreq.
4. Rename cci property to mediatek,cci.
5. Adjust order of cleanup patches.
6. Add new patches for cleanup, handle infinite loop and MT8183 dts.
7. Revise drivers from reviewers' suggestion.
8. Revise commit message of some patches to avoid confusion and misunderstand.
9. Revise "cpufreq: mediatek: Link CCI device to CPU".
   We do not return successful to pretend we set the target frequency done
   when cci is not ready. Instead, we find and set a safe voltage so that we
   can set the target cpufrequency.

Changes for V2:
1. Drop the modification of transforming cpufreq-mediatek into yaml and
   only add the MediaTek CCI property for MediaTek cpufreq.
2. Split the original patches into several patches.

Reference series:
[1]: V1 of this series is present by Jia-Wei Chang.
     message-id:20220307122151.11666-1-jia-wei.chang@mediatek.com

[2]: The MediaTek CCI devfreq driver is introduced in another series.
     message-id:20220408052150.22536-1-johnson.wang@mediatek.com

[3]: The MediaTek SVS driver is introduced in another series.
     message-id:20220221063939.14969-1-roger.lu@mediatek.com

Andrew-sh.Cheng (1):
  cpufreq: mediatek: Add opp notification support

Jia-Wei Chang (6):
  cpufreq: mediatek: Record previous target vproc value
  cpufreq: mediatek: Move voltage limits to platform data
  cpufreq: mediatek: Add .get function
  cpufreq: mediatek: Make sram regulator optional
  cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()
  cpufreq: mediatek: Add support for MT8186

Rex-BC Chen (7):
  dt-bindings: cpufreq: mediatek: Add MediaTek CCI property
  cpufreq: mediatek: Use device print to show logs
  cpufreq: mediatek: Replace old_* with pre_*
  cpufreq: mediatek: Link CCI device to CPU
  arm64: dts: mediatek: Add opp table and clock property for MT8183 cpufreq
  arm64: dts: mediatek: Add MediaTek CCI node for MT8183
  arm64: dts: mediatek: Add mediatek,cci property for MT8183 cpufreq

 .../bindings/cpufreq/cpufreq-mediatek.txt     |   5 +
 arch/arm64/boot/dts/mediatek/mt8183-evb.dts   |  36 ++
 .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi |   4 +
 arch/arm64/boot/dts/mediatek/mt8183.dtsi      | 285 ++++++++++
 drivers/cpufreq/mediatek-cpufreq.c            | 504 ++++++++++++------
 5 files changed, 670 insertions(+), 164 deletions(-)

-- 
2.18.0


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH V4 01/14] dt-bindings: cpufreq: mediatek: Add MediaTek CCI property
  2022-04-22  7:52 [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Rex-BC Chen
@ 2022-04-22  7:52 ` Rex-BC Chen
  2022-04-22  8:21   ` AngeloGioacchino Del Regno
  2022-04-22 17:26   ` Krzysztof Kozlowski
  2022-04-22  7:52 ` [PATCH V4 02/14] cpufreq: mediatek: Use device print to show logs Rex-BC Chen
                   ` (13 subsequent siblings)
  14 siblings, 2 replies; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-22  7:52 UTC (permalink / raw)
  To: rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group, Rex-BC Chen

MediaTek Cache Coherent Interconnect (CCI) uses software devfreq module
for scaling clock frequency and adjust voltage.
The phandle could be linked between CPU and MediaTek CCI for some
MediaTek SoCs, like MT8183 and MT8186.
Therefore, we add this property in cpufreq-mediatek.txt.

Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
---
 .../devicetree/bindings/cpufreq/cpufreq-mediatek.txt         | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
index b8233ec91d3d..3387e1e2a2df 100644
--- a/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
@@ -20,6 +20,11 @@ Optional properties:
 	       Vsram to fit SoC specific needs. When absent, the voltage scaling
 	       flow is handled by hardware, hence no software "voltage tracking" is
 	       needed.
+- mediatek,cci:
+	MediaTek Cache Coherent Interconnect (CCI) uses the software devfreq module to
+	scale the clock frequency and adjust the voltage.
+	For details, please refer to
+	Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml
 - #cooling-cells:
 	For details, please refer to
 	Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH V4 02/14] cpufreq: mediatek: Use device print to show logs
  2022-04-22  7:52 [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Rex-BC Chen
  2022-04-22  7:52 ` [PATCH V4 01/14] dt-bindings: cpufreq: mediatek: Add MediaTek CCI property Rex-BC Chen
@ 2022-04-22  7:52 ` Rex-BC Chen
  2022-04-25  5:10   ` Viresh Kumar
  2022-04-22  7:52 ` [PATCH V4 03/14] cpufreq: mediatek: Replace old_* with pre_* Rex-BC Chen
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-22  7:52 UTC (permalink / raw)
  To: rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group, Rex-BC Chen

- Replace pr_* with dev_* to show logs.
- Remove usage of __func__.

Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/cpufreq/mediatek-cpufreq.c | 54 ++++++++++++++++--------------
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
index dc4a87e68940..e040f3574af9 100644
--- a/drivers/cpufreq/mediatek-cpufreq.c
+++ b/drivers/cpufreq/mediatek-cpufreq.c
@@ -65,7 +65,8 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 
 	old_vproc = regulator_get_voltage(proc_reg);
 	if (old_vproc < 0) {
-		pr_err("%s: invalid Vproc value: %d\n", __func__, old_vproc);
+		dev_err(info->cpu_dev,
+			"invalid Vproc value: %d\n", old_vproc);
 		return old_vproc;
 	}
 	/* Vsram should not exceed the maximum allowed voltage of SoC. */
@@ -81,14 +82,14 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 		do {
 			old_vsram = regulator_get_voltage(sram_reg);
 			if (old_vsram < 0) {
-				pr_err("%s: invalid Vsram value: %d\n",
-				       __func__, old_vsram);
+				dev_err(info->cpu_dev,
+					"invalid Vsram value: %d\n", old_vsram);
 				return old_vsram;
 			}
 			old_vproc = regulator_get_voltage(proc_reg);
 			if (old_vproc < 0) {
-				pr_err("%s: invalid Vproc value: %d\n",
-				       __func__, old_vproc);
+				dev_err(info->cpu_dev,
+					"invalid Vproc value: %d\n", old_vproc);
 				return old_vproc;
 			}
 
@@ -136,14 +137,14 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 		do {
 			old_vproc = regulator_get_voltage(proc_reg);
 			if (old_vproc < 0) {
-				pr_err("%s: invalid Vproc value: %d\n",
-				       __func__, old_vproc);
+				dev_err(info->cpu_dev,
+					"invalid Vproc value: %d\n", old_vproc);
 				return old_vproc;
 			}
 			old_vsram = regulator_get_voltage(sram_reg);
 			if (old_vsram < 0) {
-				pr_err("%s: invalid Vsram value: %d\n",
-				       __func__, old_vsram);
+				dev_err(info->cpu_dev,
+					"invalid Vsram value: %d\n", old_vsram);
 				return old_vsram;
 			}
 
@@ -214,7 +215,7 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
 	old_freq_hz = clk_get_rate(cpu_clk);
 	old_vproc = regulator_get_voltage(info->proc_reg);
 	if (old_vproc < 0) {
-		pr_err("%s: invalid Vproc value: %d\n", __func__, old_vproc);
+		dev_err(cpu_dev, "invalid Vproc value: %d\n", old_vproc);
 		return old_vproc;
 	}
 
@@ -222,8 +223,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
 
 	opp = dev_pm_opp_find_freq_ceil(cpu_dev, &freq_hz);
 	if (IS_ERR(opp)) {
-		pr_err("cpu%d: failed to find OPP for %ld\n",
-		       policy->cpu, freq_hz);
+		dev_err(cpu_dev, "cpu%d: failed to find OPP for %ld\n",
+			policy->cpu, freq_hz);
 		return PTR_ERR(opp);
 	}
 	vproc = dev_pm_opp_get_voltage(opp);
@@ -237,8 +238,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
 	if (old_vproc < target_vproc) {
 		ret = mtk_cpufreq_set_voltage(info, target_vproc);
 		if (ret) {
-			pr_err("cpu%d: failed to scale up voltage!\n",
-			       policy->cpu);
+			dev_err(cpu_dev,
+				"cpu%d: failed to scale up voltage!\n", policy->cpu);
 			mtk_cpufreq_set_voltage(info, old_vproc);
 			return ret;
 		}
@@ -247,8 +248,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
 	/* Reparent the CPU clock to intermediate clock. */
 	ret = clk_set_parent(cpu_clk, info->inter_clk);
 	if (ret) {
-		pr_err("cpu%d: failed to re-parent cpu clock!\n",
-		       policy->cpu);
+		dev_err(cpu_dev,
+			"cpu%d: failed to re-parent cpu clock!\n", policy->cpu);
 		mtk_cpufreq_set_voltage(info, old_vproc);
 		WARN_ON(1);
 		return ret;
@@ -257,8 +258,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
 	/* Set the original PLL to target rate. */
 	ret = clk_set_rate(armpll, freq_hz);
 	if (ret) {
-		pr_err("cpu%d: failed to scale cpu clock rate!\n",
-		       policy->cpu);
+		dev_err(cpu_dev,
+			"cpu%d: failed to scale cpu clock rate!\n", policy->cpu);
 		clk_set_parent(cpu_clk, armpll);
 		mtk_cpufreq_set_voltage(info, old_vproc);
 		return ret;
@@ -267,8 +268,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
 	/* Set parent of CPU clock back to the original PLL. */
 	ret = clk_set_parent(cpu_clk, armpll);
 	if (ret) {
-		pr_err("cpu%d: failed to re-parent cpu clock!\n",
-		       policy->cpu);
+		dev_err(cpu_dev,
+			"cpu%d: failed to re-parent cpu clock!\n", policy->cpu);
 		mtk_cpufreq_set_voltage(info, inter_vproc);
 		WARN_ON(1);
 		return ret;
@@ -281,8 +282,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
 	if (vproc < inter_vproc || vproc < old_vproc) {
 		ret = mtk_cpufreq_set_voltage(info, vproc);
 		if (ret) {
-			pr_err("cpu%d: failed to scale down voltage!\n",
-			       policy->cpu);
+			dev_err(cpu_dev,
+				"cpu%d: failed to scale down voltage!\n", policy->cpu);
 			clk_set_parent(cpu_clk, info->inter_clk);
 			clk_set_rate(armpll, old_freq_hz);
 			clk_set_parent(cpu_clk, armpll);
@@ -448,15 +449,16 @@ static int mtk_cpufreq_init(struct cpufreq_policy *policy)
 
 	info = mtk_cpu_dvfs_info_lookup(policy->cpu);
 	if (!info) {
-		pr_err("dvfs info for cpu%d is not initialized.\n",
-		       policy->cpu);
+		dev_err(info->cpu_dev,
+			"dvfs info for cpu%d is not initialized.\n", policy->cpu);
 		return -EINVAL;
 	}
 
 	ret = dev_pm_opp_init_cpufreq_table(info->cpu_dev, &freq_table);
 	if (ret) {
-		pr_err("failed to init cpufreq table for cpu%d: %d\n",
-		       policy->cpu, ret);
+		dev_err(info->cpu_dev,
+			"failed to init cpufreq table for cpu%d: %d\n",
+			policy->cpu, ret);
 		return ret;
 	}
 
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH V4 03/14] cpufreq: mediatek: Replace old_* with pre_*
  2022-04-22  7:52 [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Rex-BC Chen
  2022-04-22  7:52 ` [PATCH V4 01/14] dt-bindings: cpufreq: mediatek: Add MediaTek CCI property Rex-BC Chen
  2022-04-22  7:52 ` [PATCH V4 02/14] cpufreq: mediatek: Use device print to show logs Rex-BC Chen
@ 2022-04-22  7:52 ` Rex-BC Chen
  2022-04-25  5:11   ` Viresh Kumar
  2022-04-22  7:52 ` [PATCH V4 04/14] cpufreq: mediatek: Record previous target vproc value Rex-BC Chen
                   ` (11 subsequent siblings)
  14 siblings, 1 reply; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-22  7:52 UTC (permalink / raw)
  To: rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group, Rex-BC Chen

To make driver more readable, replace old_* with pre_*.

Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/cpufreq/mediatek-cpufreq.c | 84 +++++++++++++++---------------
 1 file changed, 42 insertions(+), 42 deletions(-)

diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
index e040f3574af9..ff27f77e8ee6 100644
--- a/drivers/cpufreq/mediatek-cpufreq.c
+++ b/drivers/cpufreq/mediatek-cpufreq.c
@@ -61,18 +61,18 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 {
 	struct regulator *proc_reg = info->proc_reg;
 	struct regulator *sram_reg = info->sram_reg;
-	int old_vproc, old_vsram, new_vsram, vsram, vproc, ret;
+	int pre_vproc, pre_vsram, new_vsram, vsram, vproc, ret;
 
-	old_vproc = regulator_get_voltage(proc_reg);
-	if (old_vproc < 0) {
+	pre_vproc = regulator_get_voltage(proc_reg);
+	if (pre_vproc < 0) {
 		dev_err(info->cpu_dev,
-			"invalid Vproc value: %d\n", old_vproc);
-		return old_vproc;
+			"invalid Vproc value: %d\n", pre_vproc);
+		return pre_vproc;
 	}
 	/* Vsram should not exceed the maximum allowed voltage of SoC. */
 	new_vsram = min(new_vproc + MIN_VOLT_SHIFT, MAX_VOLT_LIMIT);
 
-	if (old_vproc < new_vproc) {
+	if (pre_vproc < new_vproc) {
 		/*
 		 * When scaling up voltages, Vsram and Vproc scale up step
 		 * by step. At each step, set Vsram to (Vproc + 200mV) first,
@@ -80,20 +80,20 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 		 * Keep doing it until Vsram and Vproc hit target voltages.
 		 */
 		do {
-			old_vsram = regulator_get_voltage(sram_reg);
-			if (old_vsram < 0) {
+			pre_vsram = regulator_get_voltage(sram_reg);
+			if (pre_vsram < 0) {
 				dev_err(info->cpu_dev,
-					"invalid Vsram value: %d\n", old_vsram);
-				return old_vsram;
+					"invalid Vsram value: %d\n", pre_vsram);
+				return pre_vsram;
 			}
-			old_vproc = regulator_get_voltage(proc_reg);
-			if (old_vproc < 0) {
+			pre_vproc = regulator_get_voltage(proc_reg);
+			if (pre_vproc < 0) {
 				dev_err(info->cpu_dev,
-					"invalid Vproc value: %d\n", old_vproc);
-				return old_vproc;
+					"invalid Vproc value: %d\n", pre_vproc);
+				return pre_vproc;
 			}
 
-			vsram = min(new_vsram, old_vproc + MAX_VOLT_SHIFT);
+			vsram = min(new_vsram, pre_vproc + MAX_VOLT_SHIFT);
 
 			if (vsram + VOLT_TOL >= MAX_VOLT_LIMIT) {
 				vsram = MAX_VOLT_LIMIT;
@@ -122,12 +122,12 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 			ret = regulator_set_voltage(proc_reg, vproc,
 						    vproc + VOLT_TOL);
 			if (ret) {
-				regulator_set_voltage(sram_reg, old_vsram,
-						      old_vsram);
+				regulator_set_voltage(sram_reg, pre_vsram,
+						      pre_vsram);
 				return ret;
 			}
 		} while (vproc < new_vproc || vsram < new_vsram);
-	} else if (old_vproc > new_vproc) {
+	} else if (pre_vproc > new_vproc) {
 		/*
 		 * When scaling down voltages, Vsram and Vproc scale down step
 		 * by step. At each step, set Vproc to (Vsram - 200mV) first,
@@ -135,20 +135,20 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 		 * Keep doing it until Vsram and Vproc hit target voltages.
 		 */
 		do {
-			old_vproc = regulator_get_voltage(proc_reg);
-			if (old_vproc < 0) {
+			pre_vproc = regulator_get_voltage(proc_reg);
+			if (pre_vproc < 0) {
 				dev_err(info->cpu_dev,
-					"invalid Vproc value: %d\n", old_vproc);
-				return old_vproc;
+					"invalid Vproc value: %d\n", pre_vproc);
+				return pre_vproc;
 			}
-			old_vsram = regulator_get_voltage(sram_reg);
-			if (old_vsram < 0) {
+			pre_vsram = regulator_get_voltage(sram_reg);
+			if (pre_vsram < 0) {
 				dev_err(info->cpu_dev,
-					"invalid Vsram value: %d\n", old_vsram);
-				return old_vsram;
+					"invalid Vsram value: %d\n", pre_vsram);
+				return pre_vsram;
 			}
 
-			vproc = max(new_vproc, old_vsram - MAX_VOLT_SHIFT);
+			vproc = max(new_vproc, pre_vsram - MAX_VOLT_SHIFT);
 			ret = regulator_set_voltage(proc_reg, vproc,
 						    vproc + VOLT_TOL);
 			if (ret)
@@ -178,8 +178,8 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 			}
 
 			if (ret) {
-				regulator_set_voltage(proc_reg, old_vproc,
-						      old_vproc);
+				regulator_set_voltage(proc_reg, pre_vproc,
+						      pre_vproc);
 				return ret;
 			}
 		} while (vproc > new_vproc + VOLT_TOL ||
@@ -207,16 +207,16 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
 	struct mtk_cpu_dvfs_info *info = policy->driver_data;
 	struct device *cpu_dev = info->cpu_dev;
 	struct dev_pm_opp *opp;
-	long freq_hz, old_freq_hz;
-	int vproc, old_vproc, inter_vproc, target_vproc, ret;
+	long freq_hz, pre_freq_hz;
+	int vproc, pre_vproc, inter_vproc, target_vproc, ret;
 
 	inter_vproc = info->intermediate_voltage;
 
-	old_freq_hz = clk_get_rate(cpu_clk);
-	old_vproc = regulator_get_voltage(info->proc_reg);
-	if (old_vproc < 0) {
-		dev_err(cpu_dev, "invalid Vproc value: %d\n", old_vproc);
-		return old_vproc;
+	pre_freq_hz = clk_get_rate(cpu_clk);
+	pre_vproc = regulator_get_voltage(info->proc_reg);
+	if (pre_vproc < 0) {
+		dev_err(cpu_dev, "invalid Vproc value: %d\n", pre_vproc);
+		return pre_vproc;
 	}
 
 	freq_hz = freq_table[index].frequency * 1000;
@@ -235,12 +235,12 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
 	 * current voltage, scale up voltage first.
 	 */
 	target_vproc = (inter_vproc > vproc) ? inter_vproc : vproc;
-	if (old_vproc < target_vproc) {
+	if (pre_vproc < target_vproc) {
 		ret = mtk_cpufreq_set_voltage(info, target_vproc);
 		if (ret) {
 			dev_err(cpu_dev,
 				"cpu%d: failed to scale up voltage!\n", policy->cpu);
-			mtk_cpufreq_set_voltage(info, old_vproc);
+			mtk_cpufreq_set_voltage(info, pre_vproc);
 			return ret;
 		}
 	}
@@ -250,7 +250,7 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
 	if (ret) {
 		dev_err(cpu_dev,
 			"cpu%d: failed to re-parent cpu clock!\n", policy->cpu);
-		mtk_cpufreq_set_voltage(info, old_vproc);
+		mtk_cpufreq_set_voltage(info, pre_vproc);
 		WARN_ON(1);
 		return ret;
 	}
@@ -261,7 +261,7 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
 		dev_err(cpu_dev,
 			"cpu%d: failed to scale cpu clock rate!\n", policy->cpu);
 		clk_set_parent(cpu_clk, armpll);
-		mtk_cpufreq_set_voltage(info, old_vproc);
+		mtk_cpufreq_set_voltage(info, pre_vproc);
 		return ret;
 	}
 
@@ -279,13 +279,13 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
 	 * If the new voltage is lower than the intermediate voltage or the
 	 * original voltage, scale down to the new voltage.
 	 */
-	if (vproc < inter_vproc || vproc < old_vproc) {
+	if (vproc < inter_vproc || vproc < pre_vproc) {
 		ret = mtk_cpufreq_set_voltage(info, vproc);
 		if (ret) {
 			dev_err(cpu_dev,
 				"cpu%d: failed to scale down voltage!\n", policy->cpu);
 			clk_set_parent(cpu_clk, info->inter_clk);
-			clk_set_rate(armpll, old_freq_hz);
+			clk_set_rate(armpll, pre_freq_hz);
 			clk_set_parent(cpu_clk, armpll);
 			return ret;
 		}
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH V4 04/14] cpufreq: mediatek: Record previous target vproc value
  2022-04-22  7:52 [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Rex-BC Chen
                   ` (2 preceding siblings ...)
  2022-04-22  7:52 ` [PATCH V4 03/14] cpufreq: mediatek: Replace old_* with pre_* Rex-BC Chen
@ 2022-04-22  7:52 ` Rex-BC Chen
  2022-04-22  8:21   ` AngeloGioacchino Del Regno
  2022-04-22  7:52 ` [PATCH V4 06/14] cpufreq: mediatek: Move voltage limits to platform data Rex-BC Chen
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-22  7:52 UTC (permalink / raw)
  To: rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group, Andrew-sh . Cheng,
	Rex-BC Chen

From: Jia-Wei Chang <jia-wei.chang@mediatek.com>

We found the buck voltage may not be exactly the same with what we set
because CPU may share the same buck with other module.
Therefore, we need to record the previous desired value instead of reading
it from regulators.

Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com>
Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
---
 drivers/cpufreq/mediatek-cpufreq.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
index ff27f77e8ee6..1688cf68849c 100644
--- a/drivers/cpufreq/mediatek-cpufreq.c
+++ b/drivers/cpufreq/mediatek-cpufreq.c
@@ -40,6 +40,7 @@ struct mtk_cpu_dvfs_info {
 	struct list_head list_head;
 	int intermediate_voltage;
 	bool need_voltage_tracking;
+	int pre_vproc;
 };
 
 static LIST_HEAD(dvfs_info_list);
@@ -191,11 +192,17 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 
 static int mtk_cpufreq_set_voltage(struct mtk_cpu_dvfs_info *info, int vproc)
 {
+	int ret;
+
 	if (info->need_voltage_tracking)
-		return mtk_cpufreq_voltage_tracking(info, vproc);
+		ret = mtk_cpufreq_voltage_tracking(info, vproc);
 	else
-		return regulator_set_voltage(info->proc_reg, vproc,
-					     vproc + VOLT_TOL);
+		ret = regulator_set_voltage(info->proc_reg, vproc,
+					    MAX_VOLT_LIMIT);
+	if (!ret)
+		info->pre_vproc = vproc;
+
+	return ret;
 }
 
 static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
@@ -213,7 +220,12 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
 	inter_vproc = info->intermediate_voltage;
 
 	pre_freq_hz = clk_get_rate(cpu_clk);
-	pre_vproc = regulator_get_voltage(info->proc_reg);
+
+	if (unlikely(info->pre_vproc <= 0))
+		pre_vproc = regulator_get_voltage(info->proc_reg);
+	else
+		pre_vproc = info->pre_vproc;
+
 	if (pre_vproc < 0) {
 		dev_err(cpu_dev, "invalid Vproc value: %d\n", pre_vproc);
 		return pre_vproc;
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH V4 06/14] cpufreq: mediatek: Move voltage limits to platform data
  2022-04-22  7:52 [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Rex-BC Chen
                   ` (3 preceding siblings ...)
  2022-04-22  7:52 ` [PATCH V4 04/14] cpufreq: mediatek: Record previous target vproc value Rex-BC Chen
@ 2022-04-22  7:52 ` Rex-BC Chen
  2022-04-22  7:52 ` [PATCH V4 07/14] cpufreq: mediatek: Add .get function Rex-BC Chen
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-22  7:52 UTC (permalink / raw)
  To: rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group, Rex-BC Chen

From: Jia-Wei Chang <jia-wei.chang@mediatek.com>

Voltages and shifts are defined as macros originally.
There are different requirements of these values for each MediaTek SoCs.
Therefore, we add the platform data and move these values into it.

Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com>
Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/cpufreq/mediatek-cpufreq.c | 90 ++++++++++++++++++++----------
 1 file changed, 61 insertions(+), 29 deletions(-)

diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
index 37c02785a01f..e070a2619bcb 100644
--- a/drivers/cpufreq/mediatek-cpufreq.c
+++ b/drivers/cpufreq/mediatek-cpufreq.c
@@ -10,15 +10,21 @@
 #include <linux/cpumask.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/pm_opp.h>
 #include <linux/regulator/consumer.h>
 
-#define MIN_VOLT_SHIFT		(100000)
-#define MAX_VOLT_SHIFT		(200000)
-#define MAX_VOLT_LIMIT		(1150000)
 #define VOLT_TOL		(10000)
 
+struct mtk_cpufreq_platform_data {
+	int min_volt_shift;
+	int max_volt_shift;
+	int proc_max_volt;
+	int sram_min_volt;
+	int sram_max_volt;
+};
+
 /*
  * The struct mtk_cpu_dvfs_info holds necessary information for doing CPU DVFS
  * on each CPU power/clock domain of Mediatek SoCs. Each CPU cluster in
@@ -46,8 +52,11 @@ struct mtk_cpu_dvfs_info {
 	struct notifier_block opp_nb;
 	unsigned int opp_cpu;
 	unsigned long opp_freq;
+	const struct mtk_cpufreq_platform_data *soc_data;
 };
 
+static struct platform_device *cpufreq_pdev;
+
 static LIST_HEAD(dvfs_info_list);
 
 static struct mtk_cpu_dvfs_info *mtk_cpu_dvfs_info_lookup(int cpu)
@@ -65,6 +74,7 @@ static struct mtk_cpu_dvfs_info *mtk_cpu_dvfs_info_lookup(int cpu)
 static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 					int new_vproc)
 {
+	const struct mtk_cpufreq_platform_data *soc_data = info->soc_data;
 	struct regulator *proc_reg = info->proc_reg;
 	struct regulator *sram_reg = info->sram_reg;
 	int pre_vproc, pre_vsram, new_vsram, vsram, vproc, ret;
@@ -76,7 +86,8 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 		return pre_vproc;
 	}
 	/* Vsram should not exceed the maximum allowed voltage of SoC. */
-	new_vsram = min(new_vproc + MIN_VOLT_SHIFT, MAX_VOLT_LIMIT);
+	new_vsram = min(new_vproc + soc_data->min_volt_shift,
+			soc_data->sram_max_volt);
 
 	if (pre_vproc < new_vproc) {
 		/*
@@ -99,10 +110,11 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 				return pre_vproc;
 			}
 
-			vsram = min(new_vsram, pre_vproc + MAX_VOLT_SHIFT);
+			vsram = min(new_vsram,
+				    pre_vproc + soc_data->min_volt_shift);
 
-			if (vsram + VOLT_TOL >= MAX_VOLT_LIMIT) {
-				vsram = MAX_VOLT_LIMIT;
+			if (vsram + VOLT_TOL >= soc_data->sram_max_volt) {
+				vsram = soc_data->sram_max_volt;
 
 				/*
 				 * If the target Vsram hits the maximum voltage,
@@ -120,7 +132,7 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 				ret = regulator_set_voltage(sram_reg, vsram,
 							    vsram + VOLT_TOL);
 
-				vproc = vsram - MIN_VOLT_SHIFT;
+				vproc = vsram - soc_data->min_volt_shift;
 			}
 			if (ret)
 				return ret;
@@ -154,7 +166,8 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 				return pre_vsram;
 			}
 
-			vproc = max(new_vproc, pre_vsram - MAX_VOLT_SHIFT);
+			vproc = max(new_vproc,
+				    pre_vsram - soc_data->max_volt_shift);
 			ret = regulator_set_voltage(proc_reg, vproc,
 						    vproc + VOLT_TOL);
 			if (ret)
@@ -163,10 +176,11 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 			if (vproc == new_vproc)
 				vsram = new_vsram;
 			else
-				vsram = max(new_vsram, vproc + MIN_VOLT_SHIFT);
+				vsram = max(new_vsram,
+					    vproc + soc_data->min_volt_shift);
 
-			if (vsram + VOLT_TOL >= MAX_VOLT_LIMIT) {
-				vsram = MAX_VOLT_LIMIT;
+			if (vsram + VOLT_TOL >= soc_data->sram_max_volt) {
+				vsram = soc_data->sram_max_volt;
 
 				/*
 				 * If the target Vsram hits the maximum voltage,
@@ -197,13 +211,14 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 
 static int mtk_cpufreq_set_voltage(struct mtk_cpu_dvfs_info *info, int vproc)
 {
+	const struct mtk_cpufreq_platform_data *soc_data = info->soc_data;
 	int ret;
 
 	if (info->need_voltage_tracking)
 		ret = mtk_cpufreq_voltage_tracking(info, vproc);
 	else
 		ret = regulator_set_voltage(info->proc_reg, vproc,
-					    MAX_VOLT_LIMIT);
+					    soc_data->proc_max_volt);
 	if (!ret)
 		info->pre_vproc = vproc;
 
@@ -583,9 +598,17 @@ static struct cpufreq_driver mtk_cpufreq_driver = {
 
 static int mtk_cpufreq_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *match;
 	struct mtk_cpu_dvfs_info *info, *tmp;
 	int cpu, ret;
 
+	match = dev_get_platdata(&pdev->dev);
+	if (!match || !match->data) {
+		dev_err(&pdev->dev,
+			"failed to get mtk cpufreq platform data\n");
+		return -ENODEV;
+	}
+
 	for_each_possible_cpu(cpu) {
 		info = mtk_cpu_dvfs_info_lookup(cpu);
 		if (info)
@@ -597,6 +620,7 @@ static int mtk_cpufreq_probe(struct platform_device *pdev)
 			goto release_dvfs_info_list;
 		}
 
+		info->soc_data = match->data;
 		ret = mtk_cpu_dvfs_info_init(info, cpu);
 		if (ret) {
 			dev_err(&pdev->dev,
@@ -632,20 +656,27 @@ static struct platform_driver mtk_cpufreq_platdrv = {
 	.probe		= mtk_cpufreq_probe,
 };
 
+static const struct mtk_cpufreq_platform_data mt2701_platform_data = {
+	.min_volt_shift = 100000,
+	.max_volt_shift = 200000,
+	.proc_max_volt = 1150000,
+	.sram_min_volt = 0,
+	.sram_max_volt = 1150000,
+};
+
 /* List of machines supported by this driver */
 static const struct of_device_id mtk_cpufreq_machines[] __initconst = {
-	{ .compatible = "mediatek,mt2701", },
-	{ .compatible = "mediatek,mt2712", },
-	{ .compatible = "mediatek,mt7622", },
-	{ .compatible = "mediatek,mt7623", },
-	{ .compatible = "mediatek,mt8167", },
-	{ .compatible = "mediatek,mt817x", },
-	{ .compatible = "mediatek,mt8173", },
-	{ .compatible = "mediatek,mt8176", },
-	{ .compatible = "mediatek,mt8183", },
-	{ .compatible = "mediatek,mt8365", },
-	{ .compatible = "mediatek,mt8516", },
-
+	{ .compatible = "mediatek,mt2701", .data = &mt2701_platform_data },
+	{ .compatible = "mediatek,mt2712", .data = &mt2701_platform_data },
+	{ .compatible = "mediatek,mt7622", .data = &mt2701_platform_data },
+	{ .compatible = "mediatek,mt7623", .data = &mt2701_platform_data },
+	{ .compatible = "mediatek,mt8167", .data = &mt2701_platform_data },
+	{ .compatible = "mediatek,mt817x", .data = &mt2701_platform_data },
+	{ .compatible = "mediatek,mt8173", .data = &mt2701_platform_data },
+	{ .compatible = "mediatek,mt8176", .data = &mt2701_platform_data },
+	{ .compatible = "mediatek,mt8183", .data = &mt2701_platform_data },
+	{ .compatible = "mediatek,mt8365", .data = &mt2701_platform_data },
+	{ .compatible = "mediatek,mt8516", .data = &mt2701_platform_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, mtk_cpufreq_machines);
@@ -654,7 +685,6 @@ static int __init mtk_cpufreq_driver_init(void)
 {
 	struct device_node *np;
 	const struct of_device_id *match;
-	struct platform_device *pdev;
 	int err;
 
 	np = of_find_node_by_path("/");
@@ -678,11 +708,12 @@ static int __init mtk_cpufreq_driver_init(void)
 	 * and the device registration codes are put here to handle defer
 	 * probing.
 	 */
-	pdev = platform_device_register_simple("mtk-cpufreq", -1, NULL, 0);
-	if (IS_ERR(pdev)) {
+	cpufreq_pdev = platform_device_register_data(NULL, "mtk-cpufreq", -1,
+						     match, sizeof(*match));
+	if (IS_ERR(cpufreq_pdev)) {
 		pr_err("failed to register mtk-cpufreq platform device\n");
 		platform_driver_unregister(&mtk_cpufreq_platdrv);
-		return PTR_ERR(pdev);
+		return PTR_ERR(cpufreq_pdev);
 	}
 
 	return 0;
@@ -691,6 +722,7 @@ module_init(mtk_cpufreq_driver_init)
 
 static void __exit mtk_cpufreq_driver_exit(void)
 {
+	platform_device_unregister(cpufreq_pdev);
 	platform_driver_unregister(&mtk_cpufreq_platdrv);
 }
 module_exit(mtk_cpufreq_driver_exit)
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH V4 07/14] cpufreq: mediatek: Add .get function
  2022-04-22  7:52 [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Rex-BC Chen
                   ` (4 preceding siblings ...)
  2022-04-22  7:52 ` [PATCH V4 06/14] cpufreq: mediatek: Move voltage limits to platform data Rex-BC Chen
@ 2022-04-22  7:52 ` Rex-BC Chen
  2022-04-22  8:21   ` AngeloGioacchino Del Regno
  2022-04-25  5:35   ` Viresh Kumar
  2022-04-22  7:52 ` [PATCH V4 08/14] cpufreq: mediatek: Make sram regulator optional Rex-BC Chen
                   ` (8 subsequent siblings)
  14 siblings, 2 replies; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-22  7:52 UTC (permalink / raw)
  To: rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group, Rex-BC Chen

From: Jia-Wei Chang <jia-wei.chang@mediatek.com>

We want to get opp frequency via opp table. Therefore, we add the function
"mtk_cpufreq_get()" to do this.

Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com>
Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
---
 drivers/cpufreq/mediatek-cpufreq.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
index e070a2619bcb..0b2ca0c8eddc 100644
--- a/drivers/cpufreq/mediatek-cpufreq.c
+++ b/drivers/cpufreq/mediatek-cpufreq.c
@@ -71,6 +71,15 @@ static struct mtk_cpu_dvfs_info *mtk_cpu_dvfs_info_lookup(int cpu)
 	return NULL;
 }
 
+static unsigned int mtk_cpufreq_get(unsigned int cpu)
+{
+	struct mtk_cpu_dvfs_info *info;
+
+	info = mtk_cpu_dvfs_info_lookup(cpu);
+
+	return !info ? 0 : (info->opp_freq / 1000);
+}
+
 static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 					int new_vproc)
 {
@@ -588,7 +597,7 @@ static struct cpufreq_driver mtk_cpufreq_driver = {
 		 CPUFREQ_IS_COOLING_DEV,
 	.verify = cpufreq_generic_frequency_table_verify,
 	.target_index = mtk_cpufreq_set_target,
-	.get = cpufreq_generic_get,
+	.get = mtk_cpufreq_get,
 	.init = mtk_cpufreq_init,
 	.exit = mtk_cpufreq_exit,
 	.register_em = cpufreq_register_em_with_opp,
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH V4 08/14] cpufreq: mediatek: Make sram regulator optional
  2022-04-22  7:52 [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Rex-BC Chen
                   ` (5 preceding siblings ...)
  2022-04-22  7:52 ` [PATCH V4 07/14] cpufreq: mediatek: Add .get function Rex-BC Chen
@ 2022-04-22  7:52 ` Rex-BC Chen
  2022-04-25  5:36   ` Viresh Kumar
  2022-04-22  7:52 ` [PATCH V4 09/14] cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking() Rex-BC Chen
                   ` (7 subsequent siblings)
  14 siblings, 1 reply; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-22  7:52 UTC (permalink / raw)
  To: rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group, Rex-BC Chen

From: Jia-Wei Chang <jia-wei.chang@mediatek.com>

For some MediaTek SoCs, like MT8186, it's possible that the sram regulator
is shared between CPU and CCI.
We hope regulator framework can return error for error handling rather
than a dummy handler from regulator_get api.
Therefore, we choose to use regulator_get_optional.

Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com>
Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/cpufreq/mediatek-cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
index 0b2ca0c8eddc..97ce96421241 100644
--- a/drivers/cpufreq/mediatek-cpufreq.c
+++ b/drivers/cpufreq/mediatek-cpufreq.c
@@ -438,7 +438,7 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)
 	}
 
 	/* Both presence and absence of sram regulator are valid cases. */
-	info->sram_reg = regulator_get_exclusive(cpu_dev, "sram");
+	info->sram_reg = regulator_get_optional(cpu_dev, "sram");
 	if (IS_ERR(info->sram_reg))
 		info->sram_reg = NULL;
 	else {
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH V4 09/14] cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()
  2022-04-22  7:52 [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Rex-BC Chen
                   ` (6 preceding siblings ...)
  2022-04-22  7:52 ` [PATCH V4 08/14] cpufreq: mediatek: Make sram regulator optional Rex-BC Chen
@ 2022-04-22  7:52 ` Rex-BC Chen
  2022-04-22  8:20   ` AngeloGioacchino Del Regno
  2022-04-22  7:52 ` [PATCH V4 10/14] cpufreq: mediatek: Link CCI device to CPU Rex-BC Chen
                   ` (6 subsequent siblings)
  14 siblings, 1 reply; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-22  7:52 UTC (permalink / raw)
  To: rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group, Rex-BC Chen

From: Jia-Wei Chang <jia-wei.chang@mediatek.com>

Because the difference of sram and proc should in a range of min_volt_shift
and max_volt_shift. We need to adjust the sram and proc step by step.

We replace VOLT_TOL (voltage tolerance) with the platform data and update the
logic to determine the voltage boundary and invoking regulator_set_voltage.

- Use 'sram_min_volt' and 'sram_max_volt' to determine the voltage boundary
  of sram regulator.
- Use (sram_min_volt - min_volt_shift) and 'proc_max_volt' to determine the
  voltage boundary of vproc regulator.

Moreover, to prevent infinite loop when tracking voltage, we calculate the
maximum value for each platform data.
We assume min voltage is 0 and tracking target voltage using
min_volt_shift for each iteration.
The retry_max is 3 times of expeted iteration count.

Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com>
Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
---
 drivers/cpufreq/mediatek-cpufreq.c | 147 ++++++++++-------------------
 1 file changed, 51 insertions(+), 96 deletions(-)

diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
index 97ce96421241..c96cfd50af92 100644
--- a/drivers/cpufreq/mediatek-cpufreq.c
+++ b/drivers/cpufreq/mediatek-cpufreq.c
@@ -8,6 +8,7 @@
 #include <linux/cpu.h>
 #include <linux/cpufreq.h>
 #include <linux/cpumask.h>
+#include <linux/minmax.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
@@ -15,8 +16,6 @@
 #include <linux/pm_opp.h>
 #include <linux/regulator/consumer.h>
 
-#define VOLT_TOL		(10000)
-
 struct mtk_cpufreq_platform_data {
 	int min_volt_shift;
 	int max_volt_shift;
@@ -53,6 +52,7 @@ struct mtk_cpu_dvfs_info {
 	unsigned int opp_cpu;
 	unsigned long opp_freq;
 	const struct mtk_cpufreq_platform_data *soc_data;
+	int vtrack_max;
 };
 
 static struct platform_device *cpufreq_pdev;
@@ -87,6 +87,7 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 	struct regulator *proc_reg = info->proc_reg;
 	struct regulator *sram_reg = info->sram_reg;
 	int pre_vproc, pre_vsram, new_vsram, vsram, vproc, ret;
+	int retry = info->vtrack_max;
 
 	pre_vproc = regulator_get_voltage(proc_reg);
 	if (pre_vproc < 0) {
@@ -94,91 +95,44 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 			"invalid Vproc value: %d\n", pre_vproc);
 		return pre_vproc;
 	}
-	/* Vsram should not exceed the maximum allowed voltage of SoC. */
-	new_vsram = min(new_vproc + soc_data->min_volt_shift,
-			soc_data->sram_max_volt);
-
-	if (pre_vproc < new_vproc) {
-		/*
-		 * When scaling up voltages, Vsram and Vproc scale up step
-		 * by step. At each step, set Vsram to (Vproc + 200mV) first,
-		 * then set Vproc to (Vsram - 100mV).
-		 * Keep doing it until Vsram and Vproc hit target voltages.
-		 */
-		do {
-			pre_vsram = regulator_get_voltage(sram_reg);
-			if (pre_vsram < 0) {
-				dev_err(info->cpu_dev,
-					"invalid Vsram value: %d\n", pre_vsram);
-				return pre_vsram;
-			}
-			pre_vproc = regulator_get_voltage(proc_reg);
-			if (pre_vproc < 0) {
-				dev_err(info->cpu_dev,
-					"invalid Vproc value: %d\n", pre_vproc);
-				return pre_vproc;
-			}
-
-			vsram = min(new_vsram,
-				    pre_vproc + soc_data->min_volt_shift);
 
-			if (vsram + VOLT_TOL >= soc_data->sram_max_volt) {
-				vsram = soc_data->sram_max_volt;
+	pre_vsram = regulator_get_voltage(sram_reg);
+	if (pre_vsram < 0) {
+		dev_err(info->cpu_dev, "invalid Vsram value: %d\n", pre_vsram);
+		return pre_vsram;
+	}
 
-				/*
-				 * If the target Vsram hits the maximum voltage,
-				 * try to set the exact voltage value first.
-				 */
-				ret = regulator_set_voltage(sram_reg, vsram,
-							    vsram);
-				if (ret)
-					ret = regulator_set_voltage(sram_reg,
-							vsram - VOLT_TOL,
-							vsram);
+	new_vsram = clamp(new_vproc + soc_data->min_volt_shift,
+			  soc_data->sram_min_volt, soc_data->sram_max_volt);
 
-				vproc = new_vproc;
-			} else {
-				ret = regulator_set_voltage(sram_reg, vsram,
-							    vsram + VOLT_TOL);
+	do {
+		if (pre_vproc <= new_vproc) {
+			vsram = clamp(pre_vproc + soc_data->max_volt_shift,
+				      soc_data->sram_min_volt, new_vsram);
+			ret = regulator_set_voltage(sram_reg, vsram,
+						    soc_data->sram_max_volt);
 
-				vproc = vsram - soc_data->min_volt_shift;
-			}
 			if (ret)
 				return ret;
 
+			if (vsram == soc_data->sram_max_volt ||
+			    new_vsram == soc_data->sram_min_volt)
+				vproc = new_vproc;
+			else
+				vproc = vsram - soc_data->min_volt_shift;
+
 			ret = regulator_set_voltage(proc_reg, vproc,
-						    vproc + VOLT_TOL);
+						    soc_data->proc_max_volt);
 			if (ret) {
 				regulator_set_voltage(sram_reg, pre_vsram,
-						      pre_vsram);
+						      soc_data->sram_max_volt);
 				return ret;
 			}
-		} while (vproc < new_vproc || vsram < new_vsram);
-	} else if (pre_vproc > new_vproc) {
-		/*
-		 * When scaling down voltages, Vsram and Vproc scale down step
-		 * by step. At each step, set Vproc to (Vsram - 200mV) first,
-		 * then set Vproc to (Vproc + 100mV).
-		 * Keep doing it until Vsram and Vproc hit target voltages.
-		 */
-		do {
-			pre_vproc = regulator_get_voltage(proc_reg);
-			if (pre_vproc < 0) {
-				dev_err(info->cpu_dev,
-					"invalid Vproc value: %d\n", pre_vproc);
-				return pre_vproc;
-			}
-			pre_vsram = regulator_get_voltage(sram_reg);
-			if (pre_vsram < 0) {
-				dev_err(info->cpu_dev,
-					"invalid Vsram value: %d\n", pre_vsram);
-				return pre_vsram;
-			}
-
+		} else if (pre_vproc > new_vproc) {
 			vproc = max(new_vproc,
 				    pre_vsram - soc_data->max_volt_shift);
 			ret = regulator_set_voltage(proc_reg, vproc,
-						    vproc + VOLT_TOL);
+						    soc_data->proc_max_volt);
 			if (ret)
 				return ret;
 
@@ -188,32 +142,24 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 				vsram = max(new_vsram,
 					    vproc + soc_data->min_volt_shift);
 
-			if (vsram + VOLT_TOL >= soc_data->sram_max_volt) {
-				vsram = soc_data->sram_max_volt;
-
-				/*
-				 * If the target Vsram hits the maximum voltage,
-				 * try to set the exact voltage value first.
-				 */
-				ret = regulator_set_voltage(sram_reg, vsram,
-							    vsram);
-				if (ret)
-					ret = regulator_set_voltage(sram_reg,
-							vsram - VOLT_TOL,
-							vsram);
-			} else {
-				ret = regulator_set_voltage(sram_reg, vsram,
-							    vsram + VOLT_TOL);
-			}
-
+			ret = regulator_set_voltage(sram_reg, vsram,
+						    soc_data->sram_max_volt);
 			if (ret) {
 				regulator_set_voltage(proc_reg, pre_vproc,
-						      pre_vproc);
+						      soc_data->proc_max_volt);
 				return ret;
 			}
-		} while (vproc > new_vproc + VOLT_TOL ||
-			 vsram > new_vsram + VOLT_TOL);
-	}
+		}
+
+		pre_vproc = vproc;
+		pre_vsram = vsram;
+
+		if (--retry < 0) {
+			dev_err(info->cpu_dev,
+				"over loop count, failed to set voltage\n");
+			return -EINVAL;
+		}
+	} while (vproc != new_vproc || vsram != new_vsram);
 
 	return 0;
 }
@@ -277,8 +223,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
 	 * If the new voltage or the intermediate voltage is higher than the
 	 * current voltage, scale up voltage first.
 	 */
-	target_vproc = (inter_vproc > vproc) ? inter_vproc : vproc;
-	if (pre_vproc < target_vproc) {
+	target_vproc = max(inter_vproc, vproc);
+	if (pre_vproc <= target_vproc) {
 		ret = mtk_cpufreq_set_voltage(info, target_vproc);
 		if (ret) {
 			dev_err(cpu_dev,
@@ -499,6 +445,15 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)
 	 */
 	info->need_voltage_tracking = (info->sram_reg != NULL);
 
+	/*
+	 * We assume min voltage is 0 and tracking target voltage using
+	 * min_volt_shift for each iteration.
+	 * The vtrack_max is 3 times of expeted iteration count.
+	 */
+	info->vtrack_max = 3 * DIV_ROUND_UP(max(info->soc_data->sram_max_volt,
+						info->soc_data->proc_max_volt),
+					    info->soc_data->min_volt_shift);
+
 	return 0;
 
 out_disable_inter_clock:
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH V4 10/14] cpufreq: mediatek: Link CCI device to CPU
  2022-04-22  7:52 [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Rex-BC Chen
                   ` (7 preceding siblings ...)
  2022-04-22  7:52 ` [PATCH V4 09/14] cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking() Rex-BC Chen
@ 2022-04-22  7:52 ` Rex-BC Chen
  2022-04-22  7:52 ` [PATCH V4 11/14] cpufreq: mediatek: Add support for MT8186 Rex-BC Chen
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-22  7:52 UTC (permalink / raw)
  To: rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group, Rex-BC Chen

In some MediaTek SoCs, like MT8183, CPU and CCI share the same power
supplies. Cpufreq needs to check if CCI devfreq exists and wait until
CCI devfreq ready before scaling frequency.

Before CCI devfreq is ready, we record the voltage when booting to
kernel and use the max(cpu target voltage, booting voltage) to
prevent cpufreq adjust to the lower voltage which will cause the CCI
crash because of high frequency and low voltage.

- Add is_ccifreq_ready() to link CCI device to CPI, and CPU will start
  DVFS when CCI is ready.
- Add platform data for MT8183.

Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com>
Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Kevin Hilman <khilman@baylibre.com>
---
 drivers/cpufreq/mediatek-cpufreq.c | 82 +++++++++++++++++++++++++++++-
 1 file changed, 81 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
index c96cfd50af92..4c1329fe92c9 100644
--- a/drivers/cpufreq/mediatek-cpufreq.c
+++ b/drivers/cpufreq/mediatek-cpufreq.c
@@ -22,6 +22,7 @@ struct mtk_cpufreq_platform_data {
 	int proc_max_volt;
 	int sram_min_volt;
 	int sram_max_volt;
+	bool ccifreq_supported;
 };
 
 /*
@@ -38,6 +39,7 @@ struct mtk_cpufreq_platform_data {
 struct mtk_cpu_dvfs_info {
 	struct cpumask cpus;
 	struct device *cpu_dev;
+	struct device *cci_dev;
 	struct regulator *proc_reg;
 	struct regulator *sram_reg;
 	struct clk *cpu_clk;
@@ -45,6 +47,7 @@ struct mtk_cpu_dvfs_info {
 	struct list_head list_head;
 	int intermediate_voltage;
 	bool need_voltage_tracking;
+	int vproc_on_boot;
 	int pre_vproc;
 	/* Avoid race condition for regulators between notify and policy */
 	struct mutex reg_lock;
@@ -53,6 +56,7 @@ struct mtk_cpu_dvfs_info {
 	unsigned long opp_freq;
 	const struct mtk_cpufreq_platform_data *soc_data;
 	int vtrack_max;
+	bool ccifreq_bound;
 };
 
 static struct platform_device *cpufreq_pdev;
@@ -180,6 +184,28 @@ static int mtk_cpufreq_set_voltage(struct mtk_cpu_dvfs_info *info, int vproc)
 	return ret;
 }
 
+static bool is_ccifreq_ready(struct mtk_cpu_dvfs_info *info)
+{
+	struct device_link *sup_link;
+
+	if (info->ccifreq_bound)
+		return true;
+
+	sup_link = device_link_add(info->cpu_dev, info->cci_dev,
+				   DL_FLAG_AUTOREMOVE_CONSUMER);
+	if (!sup_link) {
+		dev_err(info->cpu_dev, "cpu%d: sup_link is NULL\n", info->opp_cpu);
+		return false;
+	}
+
+	if (sup_link->supplier->links.status != DL_DEV_DRIVER_BOUND)
+		return false;
+
+	info->ccifreq_bound = true;
+
+	return true;
+}
+
 static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
 				  unsigned int index)
 {
@@ -219,6 +245,14 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
 	vproc = dev_pm_opp_get_voltage(opp);
 	dev_pm_opp_put(opp);
 
+	/*
+	 * If MediaTek cci is supported but is not ready, we will use the value
+	 * of max(target cpu voltage, booting voltage) to prevent high freqeuncy
+	 * low voltage crash.
+	 */
+	if (info->soc_data->ccifreq_supported && !is_ccifreq_ready(info))
+		vproc = max(vproc, info->vproc_on_boot);
+
 	/*
 	 * If the new voltage or the intermediate voltage is higher than the
 	 * current voltage, scale up voltage first.
@@ -340,6 +374,23 @@ static int mtk_cpufreq_opp_notifier(struct notifier_block *nb,
 	return notifier_from_errno(ret);
 }
 
+static struct device *of_get_cci(struct device *cpu_dev)
+{
+	struct device_node *np;
+	struct platform_device *pdev;
+
+	np = of_parse_phandle(cpu_dev->of_node, "mediatek,cci", 0);
+	if (IS_ERR_OR_NULL(np))
+		return NULL;
+
+	pdev = of_find_device_by_node(np);
+	of_node_put(np);
+	if (IS_ERR_OR_NULL(pdev))
+		return NULL;
+
+	return &pdev->dev;
+}
+
 static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)
 {
 	struct device *cpu_dev;
@@ -354,6 +405,16 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)
 	}
 	info->cpu_dev = cpu_dev;
 
+	info->ccifreq_bound = false;
+	if (info->soc_data->ccifreq_supported) {
+		info->cci_dev = of_get_cci(info->cpu_dev);
+		if (IS_ERR_OR_NULL(info->cci_dev)) {
+			ret = PTR_ERR(info->cci_dev);
+			dev_err(cpu_dev, "cpu%d: failed to get cci device\n", cpu);
+			return -ENODEV;
+		}
+	}
+
 	info->cpu_clk = clk_get(cpu_dev, "cpu");
 	if (IS_ERR(info->cpu_clk)) {
 		ret = PTR_ERR(info->cpu_clk);
@@ -417,6 +478,15 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)
 	if (ret)
 		goto out_disable_mux_clock;
 
+	if (info->soc_data->ccifreq_supported) {
+		info->vproc_on_boot = regulator_get_voltage(info->proc_reg);
+		if (info->vproc_on_boot < 0) {
+			dev_err(info->cpu_dev,
+				"invalid Vproc value: %d\n", info->vproc_on_boot);
+			goto out_disable_inter_clock;
+		}
+	}
+
 	/* Search a safe voltage for intermediate frequency. */
 	rate = clk_get_rate(info->inter_clk);
 	opp = dev_pm_opp_find_freq_ceil(cpu_dev, &rate);
@@ -626,6 +696,16 @@ static const struct mtk_cpufreq_platform_data mt2701_platform_data = {
 	.proc_max_volt = 1150000,
 	.sram_min_volt = 0,
 	.sram_max_volt = 1150000,
+	.ccifreq_supported = false,
+};
+
+static const struct mtk_cpufreq_platform_data mt8183_platform_data = {
+	.min_volt_shift = 100000,
+	.max_volt_shift = 200000,
+	.proc_max_volt = 1150000,
+	.sram_min_volt = 0,
+	.sram_max_volt = 1150000,
+	.ccifreq_supported = true,
 };
 
 /* List of machines supported by this driver */
@@ -638,7 +718,7 @@ static const struct of_device_id mtk_cpufreq_machines[] __initconst = {
 	{ .compatible = "mediatek,mt817x", .data = &mt2701_platform_data },
 	{ .compatible = "mediatek,mt8173", .data = &mt2701_platform_data },
 	{ .compatible = "mediatek,mt8176", .data = &mt2701_platform_data },
-	{ .compatible = "mediatek,mt8183", .data = &mt2701_platform_data },
+	{ .compatible = "mediatek,mt8183", .data = &mt8183_platform_data },
 	{ .compatible = "mediatek,mt8365", .data = &mt2701_platform_data },
 	{ .compatible = "mediatek,mt8516", .data = &mt2701_platform_data },
 	{ }
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH V4 11/14] cpufreq: mediatek: Add support for MT8186
  2022-04-22  7:52 [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Rex-BC Chen
                   ` (8 preceding siblings ...)
  2022-04-22  7:52 ` [PATCH V4 10/14] cpufreq: mediatek: Link CCI device to CPU Rex-BC Chen
@ 2022-04-22  7:52 ` Rex-BC Chen
  2022-04-22  7:52 ` [PATCH V4 12/14] arm64: dts: mediatek: Add opp table and clock property for MT8183 cpufreq Rex-BC Chen
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-22  7:52 UTC (permalink / raw)
  To: rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group, Rex-BC Chen

From: Jia-Wei Chang <jia-wei.chang@mediatek.com>

The platform data of MT8186 is different from previous MediaTek SoCs,
so we add a new compatible and platform data for it.

Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com>
Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/cpufreq/mediatek-cpufreq.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
index 4c1329fe92c9..9fc793453231 100644
--- a/drivers/cpufreq/mediatek-cpufreq.c
+++ b/drivers/cpufreq/mediatek-cpufreq.c
@@ -708,6 +708,15 @@ static const struct mtk_cpufreq_platform_data mt8183_platform_data = {
 	.ccifreq_supported = true,
 };
 
+static const struct mtk_cpufreq_platform_data mt8186_platform_data = {
+	.min_volt_shift = 100000,
+	.max_volt_shift = 250000,
+	.proc_max_volt = 1118750,
+	.sram_min_volt = 850000,
+	.sram_max_volt = 1118750,
+	.ccifreq_supported = true,
+};
+
 /* List of machines supported by this driver */
 static const struct of_device_id mtk_cpufreq_machines[] __initconst = {
 	{ .compatible = "mediatek,mt2701", .data = &mt2701_platform_data },
@@ -719,6 +728,7 @@ static const struct of_device_id mtk_cpufreq_machines[] __initconst = {
 	{ .compatible = "mediatek,mt8173", .data = &mt2701_platform_data },
 	{ .compatible = "mediatek,mt8176", .data = &mt2701_platform_data },
 	{ .compatible = "mediatek,mt8183", .data = &mt8183_platform_data },
+	{ .compatible = "mediatek,mt8186", .data = &mt8186_platform_data },
 	{ .compatible = "mediatek,mt8365", .data = &mt2701_platform_data },
 	{ .compatible = "mediatek,mt8516", .data = &mt2701_platform_data },
 	{ }
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH V4 12/14] arm64: dts: mediatek: Add opp table and clock property for MT8183 cpufreq
  2022-04-22  7:52 [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Rex-BC Chen
                   ` (9 preceding siblings ...)
  2022-04-22  7:52 ` [PATCH V4 11/14] cpufreq: mediatek: Add support for MT8186 Rex-BC Chen
@ 2022-04-22  7:52 ` Rex-BC Chen
  2022-04-22  8:20   ` AngeloGioacchino Del Regno
  2022-04-22  7:52 ` [PATCH V4 13/14] arm64: dts: mediatek: Add MediaTek CCI node for MT8183 Rex-BC Chen
                   ` (3 subsequent siblings)
  14 siblings, 1 reply; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-22  7:52 UTC (permalink / raw)
  To: rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group, Rex-BC Chen,
	Andrew-sh . Cheng

- Add cpufreq opp table.
- Add MediaTek cci opp table.
- Add property of opp table and clock fro cpufreq.

Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183-evb.dts |  32 +++
 arch/arm64/boot/dts/mediatek/mt8183.dtsi    | 270 ++++++++++++++++++++
 2 files changed, 302 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
index f3fd3cca23e9..8953dbf84f3e 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
@@ -412,6 +412,38 @@
 
 };
 
+&cpu0 {
+	proc-supply = <&mt6358_vproc12_reg>;
+};
+
+&cpu1 {
+	proc-supply = <&mt6358_vproc12_reg>;
+};
+
+&cpu2 {
+	proc-supply = <&mt6358_vproc12_reg>;
+};
+
+&cpu3 {
+	proc-supply = <&mt6358_vproc12_reg>;
+};
+
+&cpu4 {
+	proc-supply = <&mt6358_vproc11_reg>;
+};
+
+&cpu5 {
+	proc-supply = <&mt6358_vproc11_reg>;
+};
+
+&cpu6 {
+	proc-supply = <&mt6358_vproc11_reg>;
+};
+
+&cpu7 {
+	proc-supply = <&mt6358_vproc11_reg>;
+};
+
 &uart0 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 151877fa6bfd..1db3322f9daa 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -42,6 +42,244 @@
 		rdma1 = &rdma1;
 	};
 
+	cluster0_opp: opp-table-cluster0 {
+		compatible = "operating-points-v2";
+		opp-shared;
+		opp0-793000000 {
+			opp-hz = /bits/ 64 <793000000>;
+			opp-microvolt = <650000>;
+			required-opps = <&opp2_00>;
+		};
+		opp0-910000000 {
+			opp-hz = /bits/ 64 <910000000>;
+			opp-microvolt = <687500>;
+			required-opps = <&opp2_01>;
+		};
+		opp0-1014000000 {
+			opp-hz = /bits/ 64 <1014000000>;
+			opp-microvolt = <718750>;
+			required-opps = <&opp2_02>;
+		};
+		opp0-1131000000 {
+			opp-hz = /bits/ 64 <1131000000>;
+			opp-microvolt = <756250>;
+			required-opps = <&opp2_03>;
+		};
+		opp0-1248000000 {
+			opp-hz = /bits/ 64 <1248000000>;
+			opp-microvolt = <800000>;
+			required-opps = <&opp2_04>;
+		};
+		opp0-1326000000 {
+			opp-hz = /bits/ 64 <1326000000>;
+			opp-microvolt = <818750>;
+			required-opps = <&opp2_05>;
+		};
+		opp0-1417000000 {
+			opp-hz = /bits/ 64 <1417000000>;
+			opp-microvolt = <850000>;
+			required-opps = <&opp2_06>;
+		};
+		opp0-1508000000 {
+			opp-hz = /bits/ 64 <1508000000>;
+			opp-microvolt = <868750>;
+			required-opps = <&opp2_07>;
+		};
+		opp0-1586000000 {
+			opp-hz = /bits/ 64 <1586000000>;
+			opp-microvolt = <893750>;
+			required-opps = <&opp2_08>;
+		};
+		opp0-1625000000 {
+			opp-hz = /bits/ 64 <1625000000>;
+			opp-microvolt = <906250>;
+			required-opps = <&opp2_09>;
+		};
+		opp0-1677000000 {
+			opp-hz = /bits/ 64 <1677000000>;
+			opp-microvolt = <931250>;
+			required-opps = <&opp2_10>;
+		};
+		opp0-1716000000 {
+			opp-hz = /bits/ 64 <1716000000>;
+			opp-microvolt = <943750>;
+			required-opps = <&opp2_11>;
+		};
+		opp0-1781000000 {
+			opp-hz = /bits/ 64 <1781000000>;
+			opp-microvolt = <975000>;
+			required-opps = <&opp2_12>;
+		};
+		opp0-1846000000 {
+			opp-hz = /bits/ 64 <1846000000>;
+			opp-microvolt = <1000000>;
+			required-opps = <&opp2_13>;
+		};
+		opp0-1924000000 {
+			opp-hz = /bits/ 64 <1924000000>;
+			opp-microvolt = <1025000>;
+			required-opps = <&opp2_14>;
+		};
+		opp0-1989000000 {
+			opp-hz = /bits/ 64 <1989000000>;
+			opp-microvolt = <1050000>;
+			required-opps = <&opp2_15>;
+		};	};
+
+	cluster1_opp: opp-table-cluster1 {
+		compatible = "operating-points-v2";
+		opp-shared;
+		opp1-793000000 {
+			opp-hz = /bits/ 64 <793000000>;
+			opp-microvolt = <700000>;
+			required-opps = <&opp2_00>;
+		};
+		opp1-910000000 {
+			opp-hz = /bits/ 64 <910000000>;
+			opp-microvolt = <725000>;
+			required-opps = <&opp2_01>;
+		};
+		opp1-1014000000 {
+			opp-hz = /bits/ 64 <1014000000>;
+			opp-microvolt = <750000>;
+			required-opps = <&opp2_02>;
+		};
+		opp1-1131000000 {
+			opp-hz = /bits/ 64 <1131000000>;
+			opp-microvolt = <775000>;
+			required-opps = <&opp2_03>;
+		};
+		opp1-1248000000 {
+			opp-hz = /bits/ 64 <1248000000>;
+			opp-microvolt = <800000>;
+			required-opps = <&opp2_04>;
+		};
+		opp1-1326000000 {
+			opp-hz = /bits/ 64 <1326000000>;
+			opp-microvolt = <825000>;
+			required-opps = <&opp2_05>;
+		};
+		opp1-1417000000 {
+			opp-hz = /bits/ 64 <1417000000>;
+			opp-microvolt = <850000>;
+			required-opps = <&opp2_06>;
+		};
+		opp1-1508000000 {
+			opp-hz = /bits/ 64 <1508000000>;
+			opp-microvolt = <875000>;
+			required-opps = <&opp2_07>;
+		};
+		opp1-1586000000 {
+			opp-hz = /bits/ 64 <1586000000>;
+			opp-microvolt = <900000>;
+			required-opps = <&opp2_08>;
+		};
+		opp1-1625000000 {
+			opp-hz = /bits/ 64 <1625000000>;
+			opp-microvolt = <912500>;
+			required-opps = <&opp2_09>;
+		};
+		opp1-1677000000 {
+			opp-hz = /bits/ 64 <1677000000>;
+			opp-microvolt = <931250>;
+			required-opps = <&opp2_10>;
+		};
+		opp1-1716000000 {
+			opp-hz = /bits/ 64 <1716000000>;
+			opp-microvolt = <950000>;
+			required-opps = <&opp2_11>;
+		};
+		opp1-1781000000 {
+			opp-hz = /bits/ 64 <1781000000>;
+			opp-microvolt = <975000>;
+			required-opps = <&opp2_12>;
+		};
+		opp1-1846000000 {
+			opp-hz = /bits/ 64 <1846000000>;
+			opp-microvolt = <1000000>;
+			required-opps = <&opp2_13>;
+		};
+		opp1-1924000000 {
+			opp-hz = /bits/ 64 <1924000000>;
+			opp-microvolt = <1025000>;
+			required-opps = <&opp2_14>;
+		};
+		opp1-1989000000 {
+			opp-hz = /bits/ 64 <1989000000>;
+			opp-microvolt = <1050000>;
+			required-opps = <&opp2_15>;
+		};
+	};
+
+	cci_opp: opp-table-cci {
+		compatible = "operating-points-v2";
+		opp-shared;
+		opp2_00: opp-273000000 {
+			opp-hz = /bits/ 64 <273000000>;
+			opp-microvolt = <650000>;
+		};
+		opp2_01: opp-338000000 {
+			opp-hz = /bits/ 64 <338000000>;
+			opp-microvolt = <687500>;
+		};
+		opp2_02: opp-403000000 {
+			opp-hz = /bits/ 64 <403000000>;
+			opp-microvolt = <718750>;
+		};
+		opp2_03: opp-463000000 {
+			opp-hz = /bits/ 64 <463000000>;
+			opp-microvolt = <756250>;
+		};
+		opp2_04: opp-546000000 {
+			opp-hz = /bits/ 64 <546000000>;
+			opp-microvolt = <800000>;
+		};
+		opp2_05: opp-624000000 {
+			opp-hz = /bits/ 64 <624000000>;
+			opp-microvolt = <818750>;
+		};
+		opp2_06: opp-689000000 {
+			opp-hz = /bits/ 64 <689000000>;
+			opp-microvolt = <850000>;
+		};
+		opp2_07: opp-767000000 {
+			opp-hz = /bits/ 64 <767000000>;
+			opp-microvolt = <868750>;
+		};
+		opp2_08: opp-845000000 {
+			opp-hz = /bits/ 64 <845000000>;
+			opp-microvolt = <893750>;
+		};
+		opp2_09: opp-871000000 {
+			opp-hz = /bits/ 64 <871000000>;
+			opp-microvolt = <906250>;
+		};
+		opp2_10: opp-923000000 {
+			opp-hz = /bits/ 64 <923000000>;
+			opp-microvolt = <931250>;
+		};
+		opp2_11: opp-962000000 {
+			opp-hz = /bits/ 64 <962000000>;
+			opp-microvolt = <943750>;
+		};
+		opp2_12: opp-1027000000 {
+			opp-hz = /bits/ 64 <1027000000>;
+			opp-microvolt = <975000>;
+		};
+		opp2_13: opp-1092000000 {
+			opp-hz = /bits/ 64 <1092000000>;
+			opp-microvolt = <1000000>;
+		};
+		opp2_14: opp-1144000000 {
+			opp-hz = /bits/ 64 <1144000000>;
+			opp-microvolt = <1025000>;
+		};
+		opp2_15: opp-1196000000 {
+			opp-hz = /bits/ 64 <1196000000>;
+			opp-microvolt = <1050000>;
+		};
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
@@ -85,6 +323,10 @@
 			enable-method = "psci";
 			capacity-dmips-mhz = <741>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP0>;
+			clocks = <&mcucfg CLK_MCU_MP0_SEL>,
+				 <&topckgen CLK_TOP_ARMPLL_DIV_PLL1>;
+			clock-names = "cpu", "intermediate";
+			operating-points-v2 = <&cluster0_opp>;
 			dynamic-power-coefficient = <84>;
 			#cooling-cells = <2>;
 		};
@@ -96,6 +338,10 @@
 			enable-method = "psci";
 			capacity-dmips-mhz = <741>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP0>;
+			clocks = <&mcucfg CLK_MCU_MP0_SEL>,
+				 <&topckgen CLK_TOP_ARMPLL_DIV_PLL1>;
+			clock-names = "cpu", "intermediate";
+			operating-points-v2 = <&cluster0_opp>;
 			dynamic-power-coefficient = <84>;
 			#cooling-cells = <2>;
 		};
@@ -107,6 +353,10 @@
 			enable-method = "psci";
 			capacity-dmips-mhz = <741>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP0>;
+			clocks = <&mcucfg CLK_MCU_MP0_SEL>,
+				 <&topckgen CLK_TOP_ARMPLL_DIV_PLL1>;
+			clock-names = "cpu", "intermediate";
+			operating-points-v2 = <&cluster0_opp>;
 			dynamic-power-coefficient = <84>;
 			#cooling-cells = <2>;
 		};
@@ -118,6 +368,10 @@
 			enable-method = "psci";
 			capacity-dmips-mhz = <741>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP0>;
+			clocks = <&mcucfg CLK_MCU_MP0_SEL>,
+				 <&topckgen CLK_TOP_ARMPLL_DIV_PLL1>;
+			clock-names = "cpu", "intermediate";
+			operating-points-v2 = <&cluster0_opp>;
 			dynamic-power-coefficient = <84>;
 			#cooling-cells = <2>;
 		};
@@ -129,6 +383,10 @@
 			enable-method = "psci";
 			capacity-dmips-mhz = <1024>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP1>;
+			clocks = <&mcucfg CLK_MCU_MP2_SEL>,
+				 <&topckgen CLK_TOP_ARMPLL_DIV_PLL1>;
+			clock-names = "cpu", "intermediate";
+			operating-points-v2 = <&cluster1_opp>;
 			dynamic-power-coefficient = <211>;
 			#cooling-cells = <2>;
 		};
@@ -140,6 +398,10 @@
 			enable-method = "psci";
 			capacity-dmips-mhz = <1024>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP1>;
+			clocks = <&mcucfg CLK_MCU_MP2_SEL>,
+				 <&topckgen CLK_TOP_ARMPLL_DIV_PLL1>;
+			clock-names = "cpu", "intermediate";
+			operating-points-v2 = <&cluster1_opp>;
 			dynamic-power-coefficient = <211>;
 			#cooling-cells = <2>;
 		};
@@ -151,6 +413,10 @@
 			enable-method = "psci";
 			capacity-dmips-mhz = <1024>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP1>;
+			clocks = <&mcucfg CLK_MCU_MP2_SEL>,
+				 <&topckgen CLK_TOP_ARMPLL_DIV_PLL1>;
+			clock-names = "cpu", "intermediate";
+			operating-points-v2 = <&cluster1_opp>;
 			dynamic-power-coefficient = <211>;
 			#cooling-cells = <2>;
 		};
@@ -162,6 +428,10 @@
 			enable-method = "psci";
 			capacity-dmips-mhz = <1024>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP1>;
+			clocks = <&mcucfg CLK_MCU_MP2_SEL>,
+				 <&topckgen CLK_TOP_ARMPLL_DIV_PLL1>;
+			clock-names = "cpu", "intermediate";
+			operating-points-v2 = <&cluster1_opp>;
 			dynamic-power-coefficient = <211>;
 			#cooling-cells = <2>;
 		};
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH V4 13/14] arm64: dts: mediatek: Add MediaTek CCI node for MT8183
  2022-04-22  7:52 [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Rex-BC Chen
                   ` (10 preceding siblings ...)
  2022-04-22  7:52 ` [PATCH V4 12/14] arm64: dts: mediatek: Add opp table and clock property for MT8183 cpufreq Rex-BC Chen
@ 2022-04-22  7:52 ` Rex-BC Chen
  2022-04-22  7:52 ` [PATCH V4 14/14] arm64: dts: mediatek: Add mediatek,cci property for MT8183 cpufreq Rex-BC Chen
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-22  7:52 UTC (permalink / raw)
  To: rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group, Rex-BC Chen,
	Andrew-sh . Cheng

Add MediaTek CCI devfreq node for MT8183.

Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 arch/arm64/boot/dts/mediatek/mt8183-evb.dts    | 4 ++++
 arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 4 ++++
 arch/arm64/boot/dts/mediatek/mt8183.dtsi       | 7 +++++++
 3 files changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
index 8953dbf84f3e..7ac9864db9de 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
@@ -412,6 +412,10 @@
 
 };
 
+&cci {
+	proc-supply = <&mt6358_vproc12_reg>;
+};
+
 &cpu0 {
 	proc-supply = <&mt6358_vproc12_reg>;
 };
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index 8d5bf73a9099..b035e06840e6 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -230,6 +230,10 @@
 	status = "okay";
 };
 
+&cci {
+	proc-supply = <&mt6358_vproc12_reg>;
+};
+
 &cpu0 {
 	proc-supply = <&mt6358_vproc12_reg>;
 };
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 1db3322f9daa..c95eec5fb704 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -280,6 +280,13 @@
 		};
 	};
 
+	cci: cci {
+		compatible = "mediatek,mt8183-cci";
+		clocks = <&apmixedsys CLK_APMIXED_CCIPLL>;
+		clock-names = "cci_clock";
+		operating-points-v2 = <&cci_opp>;
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH V4 14/14] arm64: dts: mediatek: Add mediatek,cci property for MT8183 cpufreq
  2022-04-22  7:52 [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Rex-BC Chen
                   ` (11 preceding siblings ...)
  2022-04-22  7:52 ` [PATCH V4 13/14] arm64: dts: mediatek: Add MediaTek CCI node for MT8183 Rex-BC Chen
@ 2022-04-22  7:52 ` Rex-BC Chen
  2022-04-22 17:23 ` [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Krzysztof Kozlowski
       [not found] ` <20220422075239.16437-6-rex-bc.chen@mediatek.com>
  14 siblings, 0 replies; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-22  7:52 UTC (permalink / raw)
  To: rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group, Rex-BC Chen

Add mediatek,cci property to support MediaTek CCI feature.

Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index c95eec5fb704..c34d4733e4ea 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -336,6 +336,7 @@
 			operating-points-v2 = <&cluster0_opp>;
 			dynamic-power-coefficient = <84>;
 			#cooling-cells = <2>;
+			mediatek,cci = <&cci>;
 		};
 
 		cpu1: cpu@1 {
@@ -351,6 +352,7 @@
 			operating-points-v2 = <&cluster0_opp>;
 			dynamic-power-coefficient = <84>;
 			#cooling-cells = <2>;
+			mediatek,cci = <&cci>;
 		};
 
 		cpu2: cpu@2 {
@@ -366,6 +368,7 @@
 			operating-points-v2 = <&cluster0_opp>;
 			dynamic-power-coefficient = <84>;
 			#cooling-cells = <2>;
+			mediatek,cci = <&cci>;
 		};
 
 		cpu3: cpu@3 {
@@ -381,6 +384,7 @@
 			operating-points-v2 = <&cluster0_opp>;
 			dynamic-power-coefficient = <84>;
 			#cooling-cells = <2>;
+			mediatek,cci = <&cci>;
 		};
 
 		cpu4: cpu@100 {
@@ -396,6 +400,7 @@
 			operating-points-v2 = <&cluster1_opp>;
 			dynamic-power-coefficient = <211>;
 			#cooling-cells = <2>;
+			mediatek,cci = <&cci>;
 		};
 
 		cpu5: cpu@101 {
@@ -411,6 +416,7 @@
 			operating-points-v2 = <&cluster1_opp>;
 			dynamic-power-coefficient = <211>;
 			#cooling-cells = <2>;
+			mediatek,cci = <&cci>;
 		};
 
 		cpu6: cpu@102 {
@@ -426,6 +432,7 @@
 			operating-points-v2 = <&cluster1_opp>;
 			dynamic-power-coefficient = <211>;
 			#cooling-cells = <2>;
+			mediatek,cci = <&cci>;
 		};
 
 		cpu7: cpu@103 {
@@ -441,6 +448,7 @@
 			operating-points-v2 = <&cluster1_opp>;
 			dynamic-power-coefficient = <211>;
 			#cooling-cells = <2>;
+			mediatek,cci = <&cci>;
 		};
 
 		idle-states {
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 12/14] arm64: dts: mediatek: Add opp table and clock property for MT8183 cpufreq
  2022-04-22  7:52 ` [PATCH V4 12/14] arm64: dts: mediatek: Add opp table and clock property for MT8183 cpufreq Rex-BC Chen
@ 2022-04-22  8:20   ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 45+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-04-22  8:20 UTC (permalink / raw)
  To: Rex-BC Chen, rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group, Andrew-sh . Cheng

Il 22/04/22 09:52, Rex-BC Chen ha scritto:
> - Add cpufreq opp table.
> - Add MediaTek cci opp table.
> - Add property of opp table and clock fro cpufreq.
> 
> Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 09/14] cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()
  2022-04-22  7:52 ` [PATCH V4 09/14] cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking() Rex-BC Chen
@ 2022-04-22  8:20   ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 45+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-04-22  8:20 UTC (permalink / raw)
  To: Rex-BC Chen, rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

Il 22/04/22 09:52, Rex-BC Chen ha scritto:
> From: Jia-Wei Chang <jia-wei.chang@mediatek.com>
> 
> Because the difference of sram and proc should in a range of min_volt_shift
> and max_volt_shift. We need to adjust the sram and proc step by step.
> 
> We replace VOLT_TOL (voltage tolerance) with the platform data and update the
> logic to determine the voltage boundary and invoking regulator_set_voltage.
> 
> - Use 'sram_min_volt' and 'sram_max_volt' to determine the voltage boundary
>    of sram regulator.
> - Use (sram_min_volt - min_volt_shift) and 'proc_max_volt' to determine the
>    voltage boundary of vproc regulator.
> 
> Moreover, to prevent infinite loop when tracking voltage, we calculate the
> maximum value for each platform data.
> We assume min voltage is 0 and tracking target voltage using
> min_volt_shift for each iteration.
> The retry_max is 3 times of expeted iteration count.
> 
> Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com>
> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>


Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function
  2022-04-22  7:52 ` [PATCH V4 07/14] cpufreq: mediatek: Add .get function Rex-BC Chen
@ 2022-04-22  8:21   ` AngeloGioacchino Del Regno
  2022-04-25  5:35   ` Viresh Kumar
  1 sibling, 0 replies; 45+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-04-22  8:21 UTC (permalink / raw)
  To: Rex-BC Chen, rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

Il 22/04/22 09:52, Rex-BC Chen ha scritto:
> From: Jia-Wei Chang <jia-wei.chang@mediatek.com>
> 
> We want to get opp frequency via opp table. Therefore, we add the function
> "mtk_cpufreq_get()" to do this.
> 
> Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com>
> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>



^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 04/14] cpufreq: mediatek: Record previous target vproc value
  2022-04-22  7:52 ` [PATCH V4 04/14] cpufreq: mediatek: Record previous target vproc value Rex-BC Chen
@ 2022-04-22  8:21   ` AngeloGioacchino Del Regno
  2022-04-25  5:12     ` Viresh Kumar
  0 siblings, 1 reply; 45+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-04-22  8:21 UTC (permalink / raw)
  To: Rex-BC Chen, rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group, Andrew-sh . Cheng

Il 22/04/22 09:52, Rex-BC Chen ha scritto:
> From: Jia-Wei Chang <jia-wei.chang@mediatek.com>
> 
> We found the buck voltage may not be exactly the same with what we set
> because CPU may share the same buck with other module.
> Therefore, we need to record the previous desired value instead of reading
> it from regulators.
> 
> Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
> Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com>
> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 01/14] dt-bindings: cpufreq: mediatek: Add MediaTek CCI property
  2022-04-22  7:52 ` [PATCH V4 01/14] dt-bindings: cpufreq: mediatek: Add MediaTek CCI property Rex-BC Chen
@ 2022-04-22  8:21   ` AngeloGioacchino Del Regno
  2022-04-22 17:26   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 45+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-04-22  8:21 UTC (permalink / raw)
  To: Rex-BC Chen, rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

Il 22/04/22 09:52, Rex-BC Chen ha scritto:
> MediaTek Cache Coherent Interconnect (CCI) uses software devfreq module
> for scaling clock frequency and adjust voltage.
> The phandle could be linked between CPU and MediaTek CCI for some
> MediaTek SoCs, like MT8183 and MT8186.
> Therefore, we add this property in cpufreq-mediatek.txt.
> 
> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186
  2022-04-22  7:52 [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Rex-BC Chen
                   ` (12 preceding siblings ...)
  2022-04-22  7:52 ` [PATCH V4 14/14] arm64: dts: mediatek: Add mediatek,cci property for MT8183 cpufreq Rex-BC Chen
@ 2022-04-22 17:23 ` Krzysztof Kozlowski
  2022-04-25  6:20   ` Rex-BC Chen
       [not found] ` <20220422075239.16437-6-rex-bc.chen@mediatek.com>
  14 siblings, 1 reply; 45+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-22 17:23 UTC (permalink / raw)
  To: Rex-BC Chen, rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On 22/04/2022 09:52, Rex-BC Chen wrote:
> 
> Reference series:
> [1]: V1 of this series is present by Jia-Wei Chang.
>      message-id:20220307122151.11666-1-jia-wei.chang@mediatek.com
> 
> [2]: The MediaTek CCI devfreq driver is introduced in another series.
>      message-id:20220408052150.22536-1-johnson.wang@mediatek.com
> 
> [3]: The MediaTek SVS driver is introduced in another series.
>      message-id:20220221063939.14969-1-roger.lu@mediatek.com

These are not proper links. Please use lore references.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 01/14] dt-bindings: cpufreq: mediatek: Add MediaTek CCI property
  2022-04-22  7:52 ` [PATCH V4 01/14] dt-bindings: cpufreq: mediatek: Add MediaTek CCI property Rex-BC Chen
  2022-04-22  8:21   ` AngeloGioacchino Del Regno
@ 2022-04-22 17:26   ` Krzysztof Kozlowski
  2022-04-22 17:34     ` Krzysztof Kozlowski
  1 sibling, 1 reply; 45+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-22 17:26 UTC (permalink / raw)
  To: Rex-BC Chen, rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On 22/04/2022 09:52, Rex-BC Chen wrote:
> MediaTek Cache Coherent Interconnect (CCI) uses software devfreq module
> for scaling clock frequency and adjust voltage.
> The phandle could be linked between CPU and MediaTek CCI for some
> MediaTek SoCs, like MT8183 and MT8186.
> Therefore, we add this property in cpufreq-mediatek.txt.
> 
> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> ---
>  .../devicetree/bindings/cpufreq/cpufreq-mediatek.txt         | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
> index b8233ec91d3d..3387e1e2a2df 100644
> --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
> @@ -20,6 +20,11 @@ Optional properties:
>  	       Vsram to fit SoC specific needs. When absent, the voltage scaling
>  	       flow is handled by hardware, hence no software "voltage tracking" is
>  	       needed.
> +- mediatek,cci:
> +	MediaTek Cache Coherent Interconnect (CCI) uses the software devfreq module to
> +	scale the clock frequency and adjust the voltage.

Devfreq is a SW mechanism, it should not be part of bindings description.

> +	For details, please refer to
> +	Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml

Since the file does not exist, I have troubles reviewing it. First of
all, you already have "mediatek,cci-control" property in DT, so why
using different name?

Second, it looks like you want to put devfreq into bindings instead of
using proper interconnect bindings.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 01/14] dt-bindings: cpufreq: mediatek: Add MediaTek CCI property
  2022-04-22 17:26   ` Krzysztof Kozlowski
@ 2022-04-22 17:34     ` Krzysztof Kozlowski
  2022-04-25  6:19       ` Rex-BC Chen
  0 siblings, 1 reply; 45+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-22 17:34 UTC (permalink / raw)
  To: Rex-BC Chen, rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On 22/04/2022 19:26, Krzysztof Kozlowski wrote:
> On 22/04/2022 09:52, Rex-BC Chen wrote:
>> MediaTek Cache Coherent Interconnect (CCI) uses software devfreq module
>> for scaling clock frequency and adjust voltage.
>> The phandle could be linked between CPU and MediaTek CCI for some
>> MediaTek SoCs, like MT8183 and MT8186.
>> Therefore, we add this property in cpufreq-mediatek.txt.
>>
>> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
>> ---
>>  .../devicetree/bindings/cpufreq/cpufreq-mediatek.txt         | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
>> index b8233ec91d3d..3387e1e2a2df 100644
>> --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
>> @@ -20,6 +20,11 @@ Optional properties:
>>  	       Vsram to fit SoC specific needs. When absent, the voltage scaling
>>  	       flow is handled by hardware, hence no software "voltage tracking" is
>>  	       needed.
>> +- mediatek,cci:
>> +	MediaTek Cache Coherent Interconnect (CCI) uses the software devfreq module to
>> +	scale the clock frequency and adjust the voltage.
> 
> Devfreq is a SW mechanism, it should not be part of bindings description.
> 
>> +	For details, please refer to
>> +	Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml
> 
> Since the file does not exist, I have troubles reviewing it. First of
> all, you already have "mediatek,cci-control" property in DT, so why
> using different name?
> 
> Second, it looks like you want to put devfreq into bindings instead of
> using proper interconnect bindings.

Actually judging by the driver this looks like some
device-boot-time-ordering, so I wonder whether this is a proper way to
express it.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 02/14] cpufreq: mediatek: Use device print to show logs
  2022-04-22  7:52 ` [PATCH V4 02/14] cpufreq: mediatek: Use device print to show logs Rex-BC Chen
@ 2022-04-25  5:10   ` Viresh Kumar
  0 siblings, 0 replies; 45+ messages in thread
From: Viresh Kumar @ 2022-04-25  5:10 UTC (permalink / raw)
  To: Rex-BC Chen
  Cc: rafael, robh+dt, krzk+dt, matthias.bgg, jia-wei.chang, roger.lu,
	hsinyi, khilman, angelogioacchino.delregno, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On 22-04-22, 15:52, Rex-BC Chen wrote:
> - Replace pr_* with dev_* to show logs.
> - Remove usage of __func__.
> 
> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/cpufreq/mediatek-cpufreq.c | 54 ++++++++++++++++--------------
>  1 file changed, 28 insertions(+), 26 deletions(-)

Applied. Thanks.

-- 
viresh

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 03/14] cpufreq: mediatek: Replace old_* with pre_*
  2022-04-22  7:52 ` [PATCH V4 03/14] cpufreq: mediatek: Replace old_* with pre_* Rex-BC Chen
@ 2022-04-25  5:11   ` Viresh Kumar
  0 siblings, 0 replies; 45+ messages in thread
From: Viresh Kumar @ 2022-04-25  5:11 UTC (permalink / raw)
  To: Rex-BC Chen
  Cc: rafael, robh+dt, krzk+dt, matthias.bgg, jia-wei.chang, roger.lu,
	hsinyi, khilman, angelogioacchino.delregno, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On 22-04-22, 15:52, Rex-BC Chen wrote:
> To make driver more readable, replace old_* with pre_*.
> 
> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/cpufreq/mediatek-cpufreq.c | 84 +++++++++++++++---------------
>  1 file changed, 42 insertions(+), 42 deletions(-)

Applied. Thanks.

-- 
viresh

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 04/14] cpufreq: mediatek: Record previous target vproc value
  2022-04-22  8:21   ` AngeloGioacchino Del Regno
@ 2022-04-25  5:12     ` Viresh Kumar
  0 siblings, 0 replies; 45+ messages in thread
From: Viresh Kumar @ 2022-04-25  5:12 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: Rex-BC Chen, rafael, robh+dt, krzk+dt, matthias.bgg,
	jia-wei.chang, roger.lu, hsinyi, khilman, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group, Andrew-sh . Cheng

On 22-04-22, 10:21, AngeloGioacchino Del Regno wrote:
> Il 22/04/22 09:52, Rex-BC Chen ha scritto:
> > From: Jia-Wei Chang <jia-wei.chang@mediatek.com>
> > 
> > We found the buck voltage may not be exactly the same with what we set
> > because CPU may share the same buck with other module.
> > Therefore, we need to record the previous desired value instead of reading
> > it from regulators.
> > 
> > Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
> > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com>
> > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Applied. Thanks.

-- 
viresh

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 05/14] cpufreq: mediatek: Add opp notification support
       [not found] ` <20220422075239.16437-6-rex-bc.chen@mediatek.com>
@ 2022-04-25  5:20   ` Viresh Kumar
  0 siblings, 0 replies; 45+ messages in thread
From: Viresh Kumar @ 2022-04-25  5:20 UTC (permalink / raw)
  To: Rex-BC Chen
  Cc: rafael, robh+dt, krzk+dt, matthias.bgg, jia-wei.chang, roger.lu,
	hsinyi, khilman, angelogioacchino.delregno, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group, Andrew-sh.Cheng

On 22-04-22, 15:52, Rex-BC Chen wrote:
> From: "Andrew-sh.Cheng" <andrew-sh.cheng@mediatek.com>
> 
> >From this opp notifier, cpufreq should listen to opp notification and do

Why the extra ">" here ?

> proper actions when receiving events of disable and voltage adjustment.
> 
> One of the user for this opp notifier is MediaTek SVS.
> The MediaTek Smart Voltage Scaling (SVS) is a hardware which calculates
> suitable SVS bank voltages to OPP voltage table.
> 
> Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
> Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com>
> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/cpufreq/mediatek-cpufreq.c | 92 +++++++++++++++++++++++++++---
>  1 file changed, 84 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
> +static int mtk_cpufreq_opp_notifier(struct notifier_block *nb,
> +				    unsigned long event, void *data)
> +{
> +	struct dev_pm_opp *opp = data;
> +	struct dev_pm_opp *new_opp;
> +	struct mtk_cpu_dvfs_info *info;
> +	unsigned long freq, volt;
> +	struct cpufreq_policy *policy;
> +	int ret = 0;
> +
> +	info = container_of(nb, struct mtk_cpu_dvfs_info, opp_nb);
> +
> +	if (event == OPP_EVENT_ADJUST_VOLTAGE) {

I don't see any call to dev_pm_opp_adjust_voltage() for your platform, how is
this ever going to get called ?

> +		freq = dev_pm_opp_get_freq(opp);
> +
> +		mutex_lock(&info->reg_lock);
> +		if (info->opp_freq == freq) {
> +			volt = dev_pm_opp_get_voltage(opp);
> +			ret = mtk_cpufreq_set_voltage(info, volt);
> +			if (ret)
> +				dev_err(info->cpu_dev,
> +					"failed to scale voltage: %d\n", ret);
> +		}
> +		mutex_unlock(&info->reg_lock);
> +	} else if (event == OPP_EVENT_DISABLE) {
> +		freq = dev_pm_opp_get_freq(opp);
> +
> +		/* case of current opp item is disabled */
> +		if (info->opp_freq == freq) {
> +			freq = 1;
> +			new_opp = dev_pm_opp_find_freq_ceil(info->cpu_dev,
> +							    &freq);
> +			if (IS_ERR(new_opp)) {
> +				dev_err(info->cpu_dev,
> +					"all opp items are disabled\n");
> +				ret = PTR_ERR(new_opp);
> +				return notifier_from_errno(ret);
> +			}
> +
> +			dev_pm_opp_put(new_opp);
> +			policy = cpufreq_cpu_get(info->opp_cpu);
> +			if (policy) {
> +				cpufreq_driver_target(policy, freq / 1000,
> +						      CPUFREQ_RELATION_L);
> +				cpufreq_cpu_put(policy);

IIUC, then you are trying to change the frequency if a currently used OPP is
getting removed ? In that case, this problem is generic enough not to be fixed
in a end driver. This should be fixed in core somehow.

-- 
viresh

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function
  2022-04-22  7:52 ` [PATCH V4 07/14] cpufreq: mediatek: Add .get function Rex-BC Chen
  2022-04-22  8:21   ` AngeloGioacchino Del Regno
@ 2022-04-25  5:35   ` Viresh Kumar
  2022-04-25  9:34     ` Rex-BC Chen
  1 sibling, 1 reply; 45+ messages in thread
From: Viresh Kumar @ 2022-04-25  5:35 UTC (permalink / raw)
  To: Rex-BC Chen
  Cc: rafael, robh+dt, krzk+dt, matthias.bgg, jia-wei.chang, roger.lu,
	hsinyi, khilman, angelogioacchino.delregno, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On 22-04-22, 15:52, Rex-BC Chen wrote:
> From: Jia-Wei Chang <jia-wei.chang@mediatek.com>
> 
> We want to get opp frequency via opp table. Therefore, we add the function
> "mtk_cpufreq_get()" to do this.
> 
> Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com>
> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> ---
>  drivers/cpufreq/mediatek-cpufreq.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
> index e070a2619bcb..0b2ca0c8eddc 100644
> --- a/drivers/cpufreq/mediatek-cpufreq.c
> +++ b/drivers/cpufreq/mediatek-cpufreq.c
> @@ -71,6 +71,15 @@ static struct mtk_cpu_dvfs_info *mtk_cpu_dvfs_info_lookup(int cpu)
>  	return NULL;
>  }
>  
> +static unsigned int mtk_cpufreq_get(unsigned int cpu)
> +{
> +	struct mtk_cpu_dvfs_info *info;
> +
> +	info = mtk_cpu_dvfs_info_lookup(cpu);
> +
> +	return !info ? 0 : (info->opp_freq / 1000);
> +}
> +
>  static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
>  					int new_vproc)
>  {
> @@ -588,7 +597,7 @@ static struct cpufreq_driver mtk_cpufreq_driver = {
>  		 CPUFREQ_IS_COOLING_DEV,
>  	.verify = cpufreq_generic_frequency_table_verify,
>  	.target_index = mtk_cpufreq_set_target,
> -	.get = cpufreq_generic_get,
> +	.get = mtk_cpufreq_get,

The .get callback should read the real frequency from hardware instead of
fetching a cached freq value.

>  	.init = mtk_cpufreq_init,
>  	.exit = mtk_cpufreq_exit,
>  	.register_em = cpufreq_register_em_with_opp,
> -- 
> 2.18.0

-- 
viresh

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 08/14] cpufreq: mediatek: Make sram regulator optional
  2022-04-22  7:52 ` [PATCH V4 08/14] cpufreq: mediatek: Make sram regulator optional Rex-BC Chen
@ 2022-04-25  5:36   ` Viresh Kumar
  0 siblings, 0 replies; 45+ messages in thread
From: Viresh Kumar @ 2022-04-25  5:36 UTC (permalink / raw)
  To: Rex-BC Chen
  Cc: rafael, robh+dt, krzk+dt, matthias.bgg, jia-wei.chang, roger.lu,
	hsinyi, khilman, angelogioacchino.delregno, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On 22-04-22, 15:52, Rex-BC Chen wrote:
> From: Jia-Wei Chang <jia-wei.chang@mediatek.com>
> 
> For some MediaTek SoCs, like MT8186, it's possible that the sram regulator
> is shared between CPU and CCI.
> We hope regulator framework can return error for error handling rather
> than a dummy handler from regulator_get api.
> Therefore, we choose to use regulator_get_optional.
> 
> Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com>
> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/cpufreq/mediatek-cpufreq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
> index 0b2ca0c8eddc..97ce96421241 100644
> --- a/drivers/cpufreq/mediatek-cpufreq.c
> +++ b/drivers/cpufreq/mediatek-cpufreq.c
> @@ -438,7 +438,7 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)
>  	}
>  
>  	/* Both presence and absence of sram regulator are valid cases. */
> -	info->sram_reg = regulator_get_exclusive(cpu_dev, "sram");
> +	info->sram_reg = regulator_get_optional(cpu_dev, "sram");
>  	if (IS_ERR(info->sram_reg))
>  		info->sram_reg = NULL;
>  	else {

Applied. Thanks.

-- 
viresh

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 01/14] dt-bindings: cpufreq: mediatek: Add MediaTek CCI property
  2022-04-22 17:34     ` Krzysztof Kozlowski
@ 2022-04-25  6:19       ` Rex-BC Chen
  2022-04-25  8:55         ` Krzysztof Kozlowski
  0 siblings, 1 reply; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-25  6:19 UTC (permalink / raw)
  To: Krzysztof Kozlowski, rafael, viresh.kumar, robh+dt, krzk+dt,
	matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On Fri, 2022-04-22 at 19:34 +0200, Krzysztof Kozlowski wrote:
> On 22/04/2022 19:26, Krzysztof Kozlowski wrote:
> > On 22/04/2022 09:52, Rex-BC Chen wrote:
> > > MediaTek Cache Coherent Interconnect (CCI) uses software devfreq
> > > module
> > > for scaling clock frequency and adjust voltage.
> > > The phandle could be linked between CPU and MediaTek CCI for some
> > > MediaTek SoCs, like MT8183 and MT8186.
> > > Therefore, we add this property in cpufreq-mediatek.txt.
> > > 
> > > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> > > ---
> > >  .../devicetree/bindings/cpufreq/cpufreq-mediatek.txt         | 5
> > > +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-
> > > mediatek.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-
> > > mediatek.txt
> > > index b8233ec91d3d..3387e1e2a2df 100644
> > > --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-
> > > mediatek.txt
> > > +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-
> > > mediatek.txt
> > > @@ -20,6 +20,11 @@ Optional properties:
> > >  	       Vsram to fit SoC specific needs. When absent, the
> > > voltage scaling
> > >  	       flow is handled by hardware, hence no software "voltage
> > > tracking" is
> > >  	       needed.
> > > +- mediatek,cci:
> > > +	MediaTek Cache Coherent Interconnect (CCI) uses the software
> > > devfreq module to
> > > +	scale the clock frequency and adjust the voltage.
> > 
> > Devfreq is a SW mechanism, it should not be part of bindings
> > description.

Hello Krzysztof,

The reason we want to get the "mediatek,cci":
We need to check the mediatek cci is ready and probed done.
Because cpufreq and mediatek cci are sharing the same regulator in
little core cpus.
Therefore, to prevent high frequency low voltage issue, we need to make
sure the mediatek cci is ready.

If mediatek cci is ready, cpufreq and mediatek cci will register the
same regulator and from regulator's implementation, if there are two
device using the same regulator, the framwork will make sure it's using
the max voltage.
For example:
mediatek cci set 1.2V originally. When cpufreq want to adjust lower
frequency adn set voltage to 1.0V.
The framework will remain using 1.2V to prevent crash of mediatek cci.

Therefore, we need to confirm the mediatek cci is ready and register
the regulator.

> > 
> > > +	For details, please refer to
> > > +	Documentation/devicetree/bindings/interconnect/mediatek,cci.yam
> > > l
> > 
> > Since the file does not exist, I have troubles reviewing it. First
> > of
> > all, you already have "mediatek,cci-control" property in DT, so why
> > using different name?

I am not sure where is "mediatek,cci-control". I think this name is not
used before.

> > 
> > Second, it looks like you want to put devfreq into bindings instead
> > of
> > using proper interconnect bindings.
> 
> Actually judging by the driver this looks like some
> device-boot-time-ordering, so I wonder whether this is a proper way
> to
> express it.

Yes, we need to get the mediatek cci node and let cpufreq and mediatek
cci link succefully. In that case, we can know the mediatek cci is
ready. And we can set the voltage using the regulator framwork.

[1]: 
https://patchwork.kernel.org/project/linux-mediatek/patch/20220422075239.16437-11-rex-bc.chen@mediatek.com/

BRs,
Rex
> 
> 
> Best regards,
> Krzysztof


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186
  2022-04-22 17:23 ` [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Krzysztof Kozlowski
@ 2022-04-25  6:20   ` Rex-BC Chen
  0 siblings, 0 replies; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-25  6:20 UTC (permalink / raw)
  To: Krzysztof Kozlowski, rafael, viresh.kumar, robh+dt, krzk+dt,
	matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On Fri, 2022-04-22 at 19:23 +0200, Krzysztof Kozlowski wrote:
> On 22/04/2022 09:52, Rex-BC Chen wrote:
> > 
> > Reference series:
> > [1]: V1 of this series is present by Jia-Wei Chang.
> >      message-id:20220307122151.11666-1-jia-wei.chang@mediatek.com
> > 
> > [2]: The MediaTek CCI devfreq driver is introduced in another
> > series.
> >      message-id:20220408052150.22536-1-johnson.wang@mediatek.com
> > 
> > [3]: The MediaTek SVS driver is introduced in another series.
> >      message-id:20220221063939.14969-1-roger.lu@mediatek.com
> 
> These are not proper links. Please use lore references.
> 
> 
> Best regards,
> Krzysztof

Hello Krzysztof,

I will use lore references in next version.
Thanks.

BRs,
Rex


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 01/14] dt-bindings: cpufreq: mediatek: Add MediaTek CCI property
  2022-04-25  6:19       ` Rex-BC Chen
@ 2022-04-25  8:55         ` Krzysztof Kozlowski
  2022-04-25 10:20           ` Rex-BC Chen
  0 siblings, 1 reply; 45+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-25  8:55 UTC (permalink / raw)
  To: Rex-BC Chen, rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On 25/04/2022 08:19, Rex-BC Chen wrote:
> On Fri, 2022-04-22 at 19:34 +0200, Krzysztof Kozlowski wrote:
>> On 22/04/2022 19:26, Krzysztof Kozlowski wrote:
>>> On 22/04/2022 09:52, Rex-BC Chen wrote:
>>>> MediaTek Cache Coherent Interconnect (CCI) uses software devfreq
>>>> module
>>>> for scaling clock frequency and adjust voltage.
>>>> The phandle could be linked between CPU and MediaTek CCI for some
>>>> MediaTek SoCs, like MT8183 and MT8186.
>>>> Therefore, we add this property in cpufreq-mediatek.txt.
>>>>
>>>> Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
>>>> ---
>>>>  .../devicetree/bindings/cpufreq/cpufreq-mediatek.txt         | 5
>>>> +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-
>>>> mediatek.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-
>>>> mediatek.txt
>>>> index b8233ec91d3d..3387e1e2a2df 100644
>>>> --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-
>>>> mediatek.txt
>>>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-
>>>> mediatek.txt
>>>> @@ -20,6 +20,11 @@ Optional properties:
>>>>  	       Vsram to fit SoC specific needs. When absent, the
>>>> voltage scaling
>>>>  	       flow is handled by hardware, hence no software "voltage
>>>> tracking" is
>>>>  	       needed.
>>>> +- mediatek,cci:
>>>> +	MediaTek Cache Coherent Interconnect (CCI) uses the software
>>>> devfreq module to
>>>> +	scale the clock frequency and adjust the voltage.
>>>
>>> Devfreq is a SW mechanism, it should not be part of bindings
>>> description.
> 
> Hello Krzysztof,
> 
> The reason we want to get the "mediatek,cci":
> We need to check the mediatek cci is ready and probed done.
> Because cpufreq and mediatek cci are sharing the same regulator in
> little core cpus.
> Therefore, to prevent high frequency low voltage issue, we need to make
> sure the mediatek cci is ready.
> 
> If mediatek cci is ready, cpufreq and mediatek cci will register the
> same regulator and from regulator's implementation, if there are two
> device using the same regulator, the framwork will make sure it's using
> the max voltage.

Thanks for explanation. The property should be described with what you
said here. The property and description should match hardware, so there
is no place for devfreq. Instead mention that power rail is shared or
voltage regulators are common.

However I am not sure if you solved your problem... see below:

> For example:
> mediatek cci set 1.2V originally. When cpufreq want to adjust lower
> frequency adn set voltage to 1.0V.
> The framework will remain using 1.2V to prevent crash of mediatek cci.

No, regulator_set_voltage() for proc_reg says:
"NOTE: If the regulator is shared between several devices then the lowest
 request voltage that meets the system constraints will be used."

Not the highest. So when your devfreq and cpufreq boots, calling
regulator_set_voltage will still cause high frequency and low voltage.

> 
> Therefore, we need to confirm the mediatek cci is ready and register
> the regulator.
>
>>>
>>>> +	For details, please refer to
>>>> +	Documentation/devicetree/bindings/interconnect/mediatek,cci.yam
>>>> l
>>>
>>> Since the file does not exist, I have troubles reviewing it. First
>>> of
>>> all, you already have "mediatek,cci-control" property in DT, so why
>>> using different name?
> 
> I am not sure where is "mediatek,cci-control". I think this name is not
> used before.
> 

Documentation/devicetree/bindings/net/mediatek-net.txt

>>>
>>> Second, it looks like you want to put devfreq into bindings instead
>>> of
>>> using proper interconnect bindings.
>>
>> Actually judging by the driver this looks like some
>> device-boot-time-ordering, so I wonder whether this is a proper way
>> to
>> express it.
> 
> Yes, we need to get the mediatek cci node and let cpufreq and mediatek
> cci link succefully. In that case, we can know the mediatek cci is
> ready. And we can set the voltage using the regulator framwork.
> 
> [1]: 
> https://patchwork.kernel.org/project/linux-mediatek/patch/20220422075239.16437-11-rex-bc.chen@mediatek.com/

Yes, I see the use case. I am not convinced yet whether this is proper
approach...


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function
  2022-04-25  5:35   ` Viresh Kumar
@ 2022-04-25  9:34     ` Rex-BC Chen
  2022-04-25 10:00       ` Viresh Kumar
  0 siblings, 1 reply; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-25  9:34 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rafael, robh+dt, krzk+dt, matthias.bgg, jia-wei.chang, roger.lu,
	hsinyi, khilman, angelogioacchino.delregno, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On Mon, 2022-04-25 at 11:05 +0530, Viresh Kumar wrote:
> On 22-04-22, 15:52, Rex-BC Chen wrote:
> > From: Jia-Wei Chang <jia-wei.chang@mediatek.com>
> > 
> > We want to get opp frequency via opp table. Therefore, we add the
> > function
> > "mtk_cpufreq_get()" to do this.
> > 
> > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com>
> > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> > ---
> >  drivers/cpufreq/mediatek-cpufreq.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/cpufreq/mediatek-cpufreq.c
> > b/drivers/cpufreq/mediatek-cpufreq.c
> > index e070a2619bcb..0b2ca0c8eddc 100644
> > --- a/drivers/cpufreq/mediatek-cpufreq.c
> > +++ b/drivers/cpufreq/mediatek-cpufreq.c
> > @@ -71,6 +71,15 @@ static struct mtk_cpu_dvfs_info
> > *mtk_cpu_dvfs_info_lookup(int cpu)
> >  	return NULL;
> >  }
> >  
> > +static unsigned int mtk_cpufreq_get(unsigned int cpu)
> > +{
> > +	struct mtk_cpu_dvfs_info *info;
> > +
> > +	info = mtk_cpu_dvfs_info_lookup(cpu);
> > +
> > +	return !info ? 0 : (info->opp_freq / 1000);
> > +}
> > +
> >  static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info
> > *info,
> >  					int new_vproc)
> >  {
> > @@ -588,7 +597,7 @@ static struct cpufreq_driver mtk_cpufreq_driver
> > = {
> >  		 CPUFREQ_IS_COOLING_DEV,
> >  	.verify = cpufreq_generic_frequency_table_verify,
> >  	.target_index = mtk_cpufreq_set_target,
> > -	.get = cpufreq_generic_get,
> > +	.get = mtk_cpufreq_get,
> 
> The .get callback should read the real frequency from hardware
> instead of
> fetching a cached freq value.
> 

Hello Viresh,

We found that the pulses of cpu voltage could be observed when
frequency is fixed (scaling_max_freq == scaling_min_freq) if using
cpufreq_generic_get as '.get' callback in MT8186.
cpufreq framework will constantly (~ 1 sec) call 'update' if the policy
frequency is NOT equal to hardware frequency in
cpufreq_verify_current_freq.
The problem is that there might be a tiny difference between the policy
frequency and the hardware frequency even they are very close.
e.g. policy frequency is 500,000,000 Hz however, hardware frequency is
499,999,726 Hz for MT8186 opp15.

To prevent the voltage pulses, we currently use the software cached
values as you pointed out.
I wonder is it possible to add a tolerence for checking difference
between policy frequency and hardware frequency in cpufreq framework so
that we can use cpufreq_generic_get as callback without pulse issue.
Or any suggestion would be appreciated.

Thanks.

BRs,
Rex
> >  	.init = mtk_cpufreq_init,
> >  	.exit = mtk_cpufreq_exit,
> >  	.register_em = cpufreq_register_em_with_opp,
> > -- 
> > 2.18.0
> 
> 


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function
  2022-04-25  9:34     ` Rex-BC Chen
@ 2022-04-25 10:00       ` Viresh Kumar
  2022-04-26 11:13         ` Rex-BC Chen
  0 siblings, 1 reply; 45+ messages in thread
From: Viresh Kumar @ 2022-04-25 10:00 UTC (permalink / raw)
  To: Rex-BC Chen
  Cc: rafael, robh+dt, krzk+dt, matthias.bgg, jia-wei.chang, roger.lu,
	hsinyi, khilman, angelogioacchino.delregno, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On 25-04-22, 17:34, Rex-BC Chen wrote:
> We found that the pulses of cpu voltage could be observed when
> frequency is fixed (scaling_max_freq == scaling_min_freq) if using
> cpufreq_generic_get as '.get' callback in MT8186.
> cpufreq framework will constantly (~ 1 sec) call 'update' if the policy

Which function gets called here in that case ? I would expect
cpufreq_driver_target() to not make a call to MTK driver in that case, after it
finds that new and old frequency are same (it will check the corresponding freq
from cpufreq table).

> frequency is NOT equal to hardware frequency in
> cpufreq_verify_current_freq.
> The problem is that there might be a tiny difference between the policy
> frequency and the hardware frequency even they are very close.
> e.g. policy frequency is 500,000,000 Hz however, hardware frequency is
> 499,999,726 Hz for MT8186 opp15.
> 
> To prevent the voltage pulses, we currently use the software cached
> values as you pointed out.
> I wonder is it possible to add a tolerence for checking difference
> between policy frequency and hardware frequency in cpufreq framework so
> that we can use cpufreq_generic_get as callback without pulse issue.
> Or any suggestion would be appreciated.

-- 
viresh

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 01/14] dt-bindings: cpufreq: mediatek: Add MediaTek CCI property
  2022-04-25  8:55         ` Krzysztof Kozlowski
@ 2022-04-25 10:20           ` Rex-BC Chen
  2022-04-25 10:52             ` Krzysztof Kozlowski
  0 siblings, 1 reply; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-25 10:20 UTC (permalink / raw)
  To: Krzysztof Kozlowski, rafael, viresh.kumar, robh+dt, krzk+dt,
	matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On Mon, 2022-04-25 at 10:55 +0200, Krzysztof Kozlowski wrote:
> On 25/04/2022 08:19, Rex-BC Chen wrote:
> > On Fri, 2022-04-22 at 19:34 +0200, Krzysztof Kozlowski wrote:
> > > On 22/04/2022 19:26, Krzysztof Kozlowski wrote:
> > > > On 22/04/2022 09:52, Rex-BC Chen wrote:
> > > > > MediaTek Cache Coherent Interconnect (CCI) uses software
> > > > > devfreq
> > > > > module
> > > > > for scaling clock frequency and adjust voltage.
> > > > > The phandle could be linked between CPU and MediaTek CCI for
> > > > > some
> > > > > MediaTek SoCs, like MT8183 and MT8186.
> > > > > Therefore, we add this property in cpufreq-mediatek.txt.
> > > > > 
> > > > > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com>
> > > > > ---
> > > > >  .../devicetree/bindings/cpufreq/cpufreq-
> > > > > mediatek.txt         | 5
> > > > > +++++
> > > > >  1 file changed, 5 insertions(+)
> > > > > 
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/cpufreq/cpufreq-
> > > > > mediatek.txt
> > > > > b/Documentation/devicetree/bindings/cpufreq/cpufreq-
> > > > > mediatek.txt
> > > > > index b8233ec91d3d..3387e1e2a2df 100644
> > > > > --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-
> > > > > mediatek.txt
> > > > > +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-
> > > > > mediatek.txt
> > > > > @@ -20,6 +20,11 @@ Optional properties:
> > > > >  	       Vsram to fit SoC specific needs. When absent,
> > > > > the
> > > > > voltage scaling
> > > > >  	       flow is handled by hardware, hence no software
> > > > > "voltage
> > > > > tracking" is
> > > > >  	       needed.
> > > > > +- mediatek,cci:
> > > > > +	MediaTek Cache Coherent Interconnect (CCI) uses the
> > > > > software
> > > > > devfreq module to
> > > > > +	scale the clock frequency and adjust the voltage.
> > > > 
> > > > Devfreq is a SW mechanism, it should not be part of bindings
> > > > description.
> > 
> > Hello Krzysztof,
> > 
> > The reason we want to get the "mediatek,cci":
> > We need to check the mediatek cci is ready and probed done.
> > Because cpufreq and mediatek cci are sharing the same regulator in
> > little core cpus.
> > Therefore, to prevent high frequency low voltage issue, we need to
> > make
> > sure the mediatek cci is ready.
> > 
> > If mediatek cci is ready, cpufreq and mediatek cci will register
> > the
> > same regulator and from regulator's implementation, if there are
> > two
> > device using the same regulator, the framwork will make sure it's
> > using
> > the max voltage.
> 
> Thanks for explanation. The property should be described with what
> you
> said here. The property and description should match hardware, so
> there
> is no place for devfreq. Instead mention that power rail is shared or
> voltage regulators are common.
> 

Hello Krzysztof,

I will modify the description to the reason why we need mediatek,cci.

> However I am not sure if you solved your problem... see below:
> 
> > For example:
> > mediatek cci set 1.2V originally. When cpufreq want to adjust lower
> > frequency adn set voltage to 1.0V.
> > The framework will remain using 1.2V to prevent crash of mediatek
> > cci.
> 
> No, regulator_set_voltage() for proc_reg says:
> "NOTE: If the regulator is shared between several devices then the
> lowest
>  request voltage that meets the system constraints will be used."
> 
> Not the highest. So when your devfreq and cpufreq boots, calling
> regulator_set_voltage will still cause high frequency and low
> voltage.
> 

From the driver comment, I think it still needs to match "meets the
system constraints".

From drivers, we can trace the driver and it finally to
regulator_get_optimal_voltage().
In [1], the framework will get max voltage while finding each device's
voltage.

[1]: 
https://elixir.bootlin.com/linux/latest/source/drivers/regulator/core.c#L3815

> > 
> > Therefore, we need to confirm the mediatek cci is ready and
> > register
> > the regulator.
> > 
> > > > 
> > > > > +	For details, please refer to
> > > > > +	Documentation/devicetree/bindings/interconnect/mediatek
> > > > > ,cci.yam
> > > > > l
> > > > 
> > > > Since the file does not exist, I have troubles reviewing it.
> > > > First
> > > > of
> > > > all, you already have "mediatek,cci-control" property in DT, so
> > > > why
> > > > using different name?
> > 
> > I am not sure where is "mediatek,cci-control". I think this name is
> > not
> > used before.
> > 
> 
> Documentation/devicetree/bindings/net/mediatek-net.txt
> 
> > > > 
> > > > Second, it looks like you want to put devfreq into bindings
> > > > instead
> > > > of
> > > > using proper interconnect bindings.
> > > 
> > > Actually judging by the driver this looks like some
> > > device-boot-time-ordering, so I wonder whether this is a proper
> > > way
> > > to
> > > express it.
> > 
> > Yes, we need to get the mediatek cci node and let cpufreq and
> > mediatek
> > cci link succefully. In that case, we can know the mediatek cci is
> > ready. And we can set the voltage using the regulator framwork.
> > 
> > [1]: 
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20220422075239.16437-11-rex-bc.chen@mediatek.com/__;!!CTRNKA9wMg0ARbw!xOuKKyjBosmRcUseQXU9SiPu8msBXrrQAASdxwVbR0SU2inuXUtO180Y0Erkpy-JmOwu$
> >  
> 
> Yes, I see the use case. I am not convinced yet whether this is
> proper
> approach...
> 

When mediatek cci is ready (probe done and register regulator done), we
can confirm that regulator framwork will make sure the voltage setting
is safe.

BRs,
Rex

> 
> Best regards,
> Krzysztof


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 01/14] dt-bindings: cpufreq: mediatek: Add MediaTek CCI property
  2022-04-25 10:20           ` Rex-BC Chen
@ 2022-04-25 10:52             ` Krzysztof Kozlowski
  2022-04-26  8:26               ` Rex-BC Chen
  0 siblings, 1 reply; 45+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-25 10:52 UTC (permalink / raw)
  To: Rex-BC Chen, rafael, viresh.kumar, robh+dt, krzk+dt, matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On 25/04/2022 12:20, Rex-BC Chen wrote:
>> However I am not sure if you solved your problem... see below:
>>
>>> For example:
>>> mediatek cci set 1.2V originally. When cpufreq want to adjust lower
>>> frequency adn set voltage to 1.0V.
>>> The framework will remain using 1.2V to prevent crash of mediatek
>>> cci.
>>
>> No, regulator_set_voltage() for proc_reg says:
>> "NOTE: If the regulator is shared between several devices then the
>> lowest
>>  request voltage that meets the system constraints will be used."
>>
>> Not the highest. So when your devfreq and cpufreq boots, calling
>> regulator_set_voltage will still cause high frequency and low
>> voltage.
>>
> 
> From the driver comment, I think it still needs to match "meets the
> system constraints".
> 
> From drivers, we can trace the driver and it finally to
> regulator_get_optimal_voltage().
> In [1], the framework will get max voltage while finding each device's
> voltage.
> 
> [1]: 
> https://elixir.bootlin.com/linux/latest/source/drivers/regulator/core.c#L3815

Right, actually in your case it's the regulator_check_consumers() above
that line, because you

I think it's quite generic problem, so would be worth solving for more
regulator consumers, but your approach is fine. At least I do not have
anything smarter, at the moment.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 01/14] dt-bindings: cpufreq: mediatek: Add MediaTek CCI property
  2022-04-25 10:52             ` Krzysztof Kozlowski
@ 2022-04-26  8:26               ` Rex-BC Chen
  0 siblings, 0 replies; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-26  8:26 UTC (permalink / raw)
  To: Krzysztof Kozlowski, rafael, viresh.kumar, robh+dt, krzk+dt,
	matthias.bgg
  Cc: jia-wei.chang, roger.lu, hsinyi, khilman,
	angelogioacchino.delregno, linux-pm, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On Mon, 2022-04-25 at 12:52 +0200, Krzysztof Kozlowski wrote:
> On 25/04/2022 12:20, Rex-BC Chen wrote:
> > > However I am not sure if you solved your problem... see below:
> > > 
> > > > For example:
> > > > mediatek cci set 1.2V originally. When cpufreq want to adjust
> > > > lower
> > > > frequency adn set voltage to 1.0V.
> > > > The framework will remain using 1.2V to prevent crash of
> > > > mediatek
> > > > cci.
> > > 
> > > No, regulator_set_voltage() for proc_reg says:
> > > "NOTE: If the regulator is shared between several devices then
> > > the
> > > lowest
> > >  request voltage that meets the system constraints will be used."
> > > 
> > > Not the highest. So when your devfreq and cpufreq boots, calling
> > > regulator_set_voltage will still cause high frequency and low
> > > voltage.
> > > 
> > 
> > From the driver comment, I think it still needs to match "meets the
> > system constraints".
> > 
> > From drivers, we can trace the driver and it finally to
> > regulator_get_optimal_voltage().
> > In [1], the framework will get max voltage while finding each
> > device's
> > voltage.
> > 
> > [1]: 
> > 
https://urldefense.com/v3/__https://elixir.bootlin.com/linux/latest/source/drivers/regulator/core.c*L3815__;Iw!!CTRNKA9wMg0ARbw!35L9ISPWDpbGPsLEc0935D2nWTLtaLSNfPipvteddPaxwO2i_KaN0wfWegpkyaPjjagW$
> >  
> 
> Right, actually in your case it's the regulator_check_consumers()
> above
> that line, because you
> 
> I think it's quite generic problem, so would be worth solving for
> more
> regulator consumers, but your approach is fine. At least I do not
> have
> anything smarter, at the moment.
> 
> Best regards,
> Krzysztof

Hello Krzysztof,

Thanks for your review!
I will send next version to modify the description of mediatek,cci when
the driver part is explained clear.

BRs,
Rex


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function
  2022-04-25 10:00       ` Viresh Kumar
@ 2022-04-26 11:13         ` Rex-BC Chen
  2022-04-27  3:11           ` Viresh Kumar
  0 siblings, 1 reply; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-26 11:13 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rafael, robh+dt, krzk+dt, matthias.bgg, jia-wei.chang, roger.lu,
	hsinyi, khilman, angelogioacchino.delregno, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On Mon, 2022-04-25 at 15:30 +0530, Viresh Kumar wrote:
> On 25-04-22, 17:34, Rex-BC Chen wrote:
> > We found that the pulses of cpu voltage could be observed when
> > frequency is fixed (scaling_max_freq == scaling_min_freq) if using
> > cpufreq_generic_get as '.get' callback in MT8186.
> > cpufreq framework will constantly (~ 1 sec) call 'update' if the
> > policy
> 
> Which function gets called here in that case ? I would expect
> cpufreq_driver_target() to not make a call to MTK driver in that
> case, after it
> finds that new and old frequency are same (it will check the
> corresponding freq
> from cpufreq table).
> 
> > frequency is NOT equal to hardware frequency in
> > cpufreq_verify_current_freq.
> > The problem is that there might be a tiny difference between the
> > policy
> > frequency and the hardware frequency even they are very close.
> > e.g. policy frequency is 500,000,000 Hz however, hardware frequency
> > is
> > 499,999,726 Hz for MT8186 opp15.
> > 
> > To prevent the voltage pulses, we currently use the software cached
> > values as you pointed out.
> > I wonder is it possible to add a tolerence for checking difference
> > between policy frequency and hardware frequency in cpufreq
> > framework so
> > that we can use cpufreq_generic_get as callback without pulse
> > issue.
> > Or any suggestion would be appreciated.
> 
> 

Hello Viresh,

We have a non-upstream driver which tries to get frequency by
'cpufreq_get'.
When we use that non-upstream driver, 'cpufreq_verify_current_freq'
will be further invoked by 'cpufreq_get' and it would cause voltage
pulse issue as I described previously.
Therefore, we apply the solution in this series.

Recently, we found that using 'cpufreq_generic_get' directly in our
non-upstream driver can do the same thing without pulse issue.
It can meet your request as well.

So here, for cpufreq, I think it is proper to drop this patch and I
will do it in the next version.

Thanks for your review.

BRs,
Rex


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function
  2022-04-26 11:13         ` Rex-BC Chen
@ 2022-04-27  3:11           ` Viresh Kumar
  2022-04-28 11:16             ` Rex-BC Chen
  0 siblings, 1 reply; 45+ messages in thread
From: Viresh Kumar @ 2022-04-27  3:11 UTC (permalink / raw)
  To: Rex-BC Chen
  Cc: rafael, robh+dt, krzk+dt, matthias.bgg, jia-wei.chang, roger.lu,
	hsinyi, khilman, angelogioacchino.delregno, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On 26-04-22, 19:13, Rex-BC Chen wrote:
> We have a non-upstream driver which tries to get frequency by
> 'cpufreq_get'.

This is the right thing to do there.

> When we use that non-upstream driver, 'cpufreq_verify_current_freq'
> will be further invoked by 'cpufreq_get' and it would cause voltage
> pulse issue as I described previously.

I see this will eventually resolve to __cpufreq_driver_target(), which
should return without any frequency updates.

What do you mean by "voltage pulse" here? What actually happens which
you want to avoid.

> Therefore, we apply the solution in this series.

I won't call it a solution but a Bug as .get() is supposed to read
real freq of the hardware.

> Recently, we found that using 'cpufreq_generic_get' directly in our
> non-upstream driver can do the same thing without pulse issue.

That would be an abuse of the cpufreq_generic_get() API. It is ONLY
allowed to be used while setting .get callback in the driver.

-- 
viresh

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function
  2022-04-27  3:11           ` Viresh Kumar
@ 2022-04-28 11:16             ` Rex-BC Chen
  2022-04-28 11:48               ` Viresh Kumar
  0 siblings, 1 reply; 45+ messages in thread
From: Rex-BC Chen @ 2022-04-28 11:16 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rafael, robh+dt, krzk+dt, matthias.bgg, jia-wei.chang, roger.lu,
	hsinyi, khilman, angelogioacchino.delregno, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On Wed, 2022-04-27 at 08:41 +0530, Viresh Kumar wrote:
> On 26-04-22, 19:13, Rex-BC Chen wrote:
> > We have a non-upstream driver which tries to get frequency by
> > 'cpufreq_get'.
> 
> This is the right thing to do there.
> 
> > When we use that non-upstream driver, 'cpufreq_verify_current_freq'
> > will be further invoked by 'cpufreq_get' and it would cause voltage
> > pulse issue as I described previously.
> 
> I see this will eventually resolve to __cpufreq_driver_target(),
> which
> should return without any frequency updates.
> 

Hello Viresh,

Yes, the call stack will eventually go to __cpufreq_driver_target.
However, we can observe the mismatch between target_freq and policy-cur 
with a tiny difference.
e.g.
[ 553.065356] cpufreq: target for CPU 0: 500000 kHz, relation 0,
requested 500000 kHz
[ 553.066366] cpufreq: target_freq/policy->cur: 500000/499999 kHz

We check the assignment of policy->cur could be either from
cpufreq_driver->get_intermediate or from cpufreq_driver->get.
But it is strange to have the frequency value like 499999 kHz.
Is the result of tiny frequency difference expected from your point of
view?

> What do you mean by "voltage pulse" here? What actually happens which
> you want to avoid.
> 

When cpufreq is fixed to lowest opp, "voltage pulse" is a quick voltage
rising and falling phenomenon which can be observed if 'cpufreq_get' is
invoked.
From top of view, if 'cpufreq_get' is NOT invoked in that condition,
the voltage pulse will no longer occur.
That's why we add this patch for this series.

> > Therefore, we apply the solution in this series.
> 
> I won't call it a solution but a Bug as .get() is supposed to read
> real freq of the hardware.
> 
> > Recently, we found that using 'cpufreq_generic_get' directly in our
> > non-upstream driver can do the same thing without pulse issue.
> 
> That would be an abuse of the cpufreq_generic_get() API. It is ONLY
> allowed to be used while setting .get callback in the driver.
> 

Thank you for sharing the correct information.
Is it possible to get frequency (API) a simple way, like get current
opp frequency?

BRs,
Rex


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function
  2022-04-28 11:16             ` Rex-BC Chen
@ 2022-04-28 11:48               ` Viresh Kumar
  2022-05-03 11:33                 ` Rex-BC Chen
  0 siblings, 1 reply; 45+ messages in thread
From: Viresh Kumar @ 2022-04-28 11:48 UTC (permalink / raw)
  To: Rex-BC Chen
  Cc: rafael, robh+dt, krzk+dt, matthias.bgg, jia-wei.chang, roger.lu,
	hsinyi, khilman, angelogioacchino.delregno, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On 28-04-22, 19:16, Rex-BC Chen wrote:
> Yes, the call stack will eventually go to __cpufreq_driver_target.
> However, we can observe the mismatch between target_freq and policy-cur 
> with a tiny difference.
> e.g.
> [ 553.065356] cpufreq: target for CPU 0: 500000 kHz, relation 0,
> requested 500000 kHz
> [ 553.066366] cpufreq: target_freq/policy->cur: 500000/499999 kHz

So you are trying to set the frequency to 500 MHz now, but policy->cur says it
is 499 MHz.

> We check the assignment of policy->cur could be either from
> cpufreq_driver->get_intermediate or from cpufreq_driver->get.

policy->cur is set only at two places, in your case:
- CPUFREQ_POSTCHANGE
- cpufreq_online()

From what I understand, it is possible that cpufreq_online() is setting your
frequency to 499999 (once at boot), but as soon as a frequency change has
happened after that, policy->cur should be set to 500 MHz and you should see
this problem only once.

From CPUFREQ_POSTCHANGE notifier, we always set policy->cur from the table
itself, which should be 500000 MHz.

I wonder how you see policy->cur to be 499999 here. Does this happen only once ?
Or repeatedly ?

> But it is strange to have the frequency value like 499999 kHz.
> Is the result of tiny frequency difference expected from your point of
> view?

Clock driver can give this value, that is fine.

> > What do you mean by "voltage pulse" here? What actually happens which
> > you want to avoid.
> > 
> 
> When cpufreq is fixed to lowest opp, "voltage pulse" is a quick voltage
> rising and falling phenomenon which can be observed if 'cpufreq_get' is
> invoked.

Do check if the call is reaching your driver's ->target_index(), it should be
which it should not, ideally.

> Thank you for sharing the correct information.
> Is it possible to get frequency (API) a simple way, like get current
> opp frequency?

Lets dig/debug a bit further and fix this if a real problem exists.

-- 
viresh

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function
  2022-04-28 11:48               ` Viresh Kumar
@ 2022-05-03 11:33                 ` Rex-BC Chen
  2022-05-04  8:22                   ` Viresh Kumar
  0 siblings, 1 reply; 45+ messages in thread
From: Rex-BC Chen @ 2022-05-03 11:33 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rafael, robh+dt, krzk+dt, matthias.bgg, jia-wei.chang, roger.lu,
	hsinyi, khilman, angelogioacchino.delregno, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On Thu, 2022-04-28 at 17:18 +0530, Viresh Kumar wrote:
> On 28-04-22, 19:16, Rex-BC Chen wrote:
> > Yes, the call stack will eventually go to __cpufreq_driver_target.
> > However, we can observe the mismatch between target_freq and
> > policy-cur 
> > with a tiny difference.
> > e.g.
> > [ 553.065356] cpufreq: target for CPU 0: 500000 kHz, relation 0,
> > requested 500000 kHz
> > [ 553.066366] cpufreq: target_freq/policy->cur: 500000/499999 kHz
> 
> So you are trying to set the frequency to 500 MHz now, but policy-
> >cur says it
> is 499 MHz.
> 
Hello Viresh,

Yes.

> > We check the assignment of policy->cur could be either from
> > cpufreq_driver->get_intermediate or from cpufreq_driver->get.
> 
> policy->cur is set only at two places, in your case:
> - CPUFREQ_POSTCHANGE
> - cpufreq_online()
> 
> From what I understand, it is possible that cpufreq_online() is
> setting your
> frequency to 499999 (once at boot), but as soon as a frequency change
> has
> happened after that, policy->cur should be set to 500 MHz and you
> should see
> this problem only once.
> 

Our observation tells us cpufreq_online is setting only once at boot
for one cpu cluster.
But we can see the problem repeatly occurs once cpufreq_get is invoked.

e.g.
[ 71.154560] cpufreq: Warning: CPU frequency out of sync: cpufreq and
timing core thinks of 500000, is 499999 kHz
[ 71.155880] cpufreq: notification 0 of frequency transition to 499999
kHz
[ 71.156777] cpufreq: notification 1 of frequency transition to 499999
kHz
[ 71.187241] cpufreq: target for CPU 0: 500000 kHz, relation 0,
requested 500000 kHz
[ 71.188246] cpufreq: target_freq/policy->cur: 500000/499999 kHz

> From CPUFREQ_POSTCHANGE notifier, we always set policy->cur from the
> table
> itself, which should be 500000 MHz.
> 

Our observation tells me it can be either 499999 kHz or 500000 kHz.
This can be printed at the last line of CPUFREQ_POSTCHANGE within
'cpufreq_notify_transition'

> I wonder how you see policy->cur to be 499999 here. Does this happen
> only once ?
> Or repeatedly ?
> 

It repeatly happens.

> > But it is strange to have the frequency value like 499999 kHz.
> > Is the result of tiny frequency difference expected from your point
> > of
> > view?
> 
> Clock driver can give this value, that is fine.
> 

Thanks for your answer.

> > > What do you mean by "voltage pulse" here? What actually happens
> > > which
> > > you want to avoid.
> > > 
> > 
> > When cpufreq is fixed to lowest opp, "voltage pulse" is a quick
> > voltage
> > rising and falling phenomenon which can be observed if
> > 'cpufreq_get' is
> > invoked.
> 
> Do check if the call is reaching your driver's ->target_index(), it
> should be
> which it should not, ideally.
> 

Yes, 'cpufreq_get' will eventually go to '->target_index()' because of
inequality between target_freq and policy->cur.

And we realized that the "voltage pulse" is generated by quick
switching voltage from 500 MHz to intermediate voltage and back to 500
MHz in current mediatek-cpufreq.c.
To fix it, we think two possible ways to approach.
One is from cpufreq framework side. Is it expected to update the
cpufreq platform driver repeatly for this case?
If it is expected, then from platform driver side, mediatek-cpufreq
should handle a condition to avoid unnecessary intermediate voltage
switching.

BRs,
Rex

> > Thank you for sharing the correct information.
> > Is it possible to get frequency (API) a simple way, like get
> > current
> > opp frequency?
> 
> Lets dig/debug a bit further and fix this if a real problem exists.
> 


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function
  2022-05-03 11:33                 ` Rex-BC Chen
@ 2022-05-04  8:22                   ` Viresh Kumar
  2022-05-04 11:57                     ` Rex-BC Chen
  0 siblings, 1 reply; 45+ messages in thread
From: Viresh Kumar @ 2022-05-04  8:22 UTC (permalink / raw)
  To: Rex-BC Chen
  Cc: rafael, robh+dt, krzk+dt, matthias.bgg, jia-wei.chang, roger.lu,
	hsinyi, khilman, angelogioacchino.delregno, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On 03-05-22, 19:33, Rex-BC Chen wrote:
> Our observation tells us cpufreq_online is setting only once at boot
> for one cpu cluster.
> But we can see the problem repeatly occurs once cpufreq_get is invoked.
> 
> e.g.
> [ 71.154560] cpufreq: Warning: CPU frequency out of sync: cpufreq and
> timing core thinks of 500000, is 499999 kHz
> [ 71.155880] cpufreq: notification 0 of frequency transition to 499999
> kHz
> [ 71.156777] cpufreq: notification 1 of frequency transition to 499999
> kHz
> [ 71.187241] cpufreq: target for CPU 0: 500000 kHz, relation 0,
> requested 500000 kHz
> [ 71.188246] cpufreq: target_freq/policy->cur: 500000/499999 kHz

Lemme know if this helps:

https://lore.kernel.org/lkml/39e39a7d30c8ee6af81fb64670a330abeb87402e.1651652493.git.viresh.kumar@linaro.org/

-- 
viresh

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function
  2022-05-04  8:22                   ` Viresh Kumar
@ 2022-05-04 11:57                     ` Rex-BC Chen
  2022-05-04 11:58                       ` Viresh Kumar
  0 siblings, 1 reply; 45+ messages in thread
From: Rex-BC Chen @ 2022-05-04 11:57 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rafael, robh+dt, krzk+dt, matthias.bgg, jia-wei.chang, roger.lu,
	hsinyi, khilman, angelogioacchino.delregno, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On Wed, 2022-05-04 at 13:52 +0530, Viresh Kumar wrote:
> On 03-05-22, 19:33, Rex-BC Chen wrote:
> > Our observation tells us cpufreq_online is setting only once at
> > boot
> > for one cpu cluster.
> > But we can see the problem repeatly occurs once cpufreq_get is
> > invoked.
> > 
> > e.g.
> > [ 71.154560] cpufreq: Warning: CPU frequency out of sync: cpufreq
> > and
> > timing core thinks of 500000, is 499999 kHz
> > [ 71.155880] cpufreq: notification 0 of frequency transition to
> > 499999
> > kHz
> > [ 71.156777] cpufreq: notification 1 of frequency transition to
> > 499999
> > kHz
> > [ 71.187241] cpufreq: target for CPU 0: 500000 kHz, relation 0,
> > requested 500000 kHz
> > [ 71.188246] cpufreq: target_freq/policy->cur: 500000/499999 kHz
> 
> Lemme know if this helps:
> 
> 
https://lore.kernel.org/lkml/39e39a7d30c8ee6af81fb64670a330abeb87402e.1651652493.git.viresh.kumar@linaro.org/
> 

Hello Viresh,

Thanks a lot! It helps to fix this issue.
And I will drop this patch in the next version.

BRs,
Rex


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function
  2022-05-04 11:57                     ` Rex-BC Chen
@ 2022-05-04 11:58                       ` Viresh Kumar
  2022-05-04 12:03                         ` Rex-BC Chen
  0 siblings, 1 reply; 45+ messages in thread
From: Viresh Kumar @ 2022-05-04 11:58 UTC (permalink / raw)
  To: Rex-BC Chen
  Cc: rafael, robh+dt, krzk+dt, matthias.bgg, jia-wei.chang, roger.lu,
	hsinyi, khilman, angelogioacchino.delregno, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On 04-05-22, 19:57, Rex-BC Chen wrote:
> Thanks a lot! It helps to fix this issue.
> And I will drop this patch in the next version.

Please provide your Tested-by in that thread then, it will help.

-- 
viresh

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function
  2022-05-04 11:58                       ` Viresh Kumar
@ 2022-05-04 12:03                         ` Rex-BC Chen
  0 siblings, 0 replies; 45+ messages in thread
From: Rex-BC Chen @ 2022-05-04 12:03 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: rafael, robh+dt, krzk+dt, matthias.bgg, jia-wei.chang, roger.lu,
	hsinyi, khilman, angelogioacchino.delregno, linux-pm, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On Wed, 2022-05-04 at 17:28 +0530, Viresh Kumar wrote:
> On 04-05-22, 19:57, Rex-BC Chen wrote:
> > Thanks a lot! It helps to fix this issue.
> > And I will drop this patch in the next version.
> 
> Please provide your Tested-by in that thread then, it will help.
> 

Hello Viresh,

The verification is done by Jia-wei.
I will help him to provide Tested-by.

Thanks.

BRs,
Rex


^ permalink raw reply	[flat|nested] 45+ messages in thread

end of thread, other threads:[~2022-05-04 12:03 UTC | newest]

Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-22  7:52 [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Rex-BC Chen
2022-04-22  7:52 ` [PATCH V4 01/14] dt-bindings: cpufreq: mediatek: Add MediaTek CCI property Rex-BC Chen
2022-04-22  8:21   ` AngeloGioacchino Del Regno
2022-04-22 17:26   ` Krzysztof Kozlowski
2022-04-22 17:34     ` Krzysztof Kozlowski
2022-04-25  6:19       ` Rex-BC Chen
2022-04-25  8:55         ` Krzysztof Kozlowski
2022-04-25 10:20           ` Rex-BC Chen
2022-04-25 10:52             ` Krzysztof Kozlowski
2022-04-26  8:26               ` Rex-BC Chen
2022-04-22  7:52 ` [PATCH V4 02/14] cpufreq: mediatek: Use device print to show logs Rex-BC Chen
2022-04-25  5:10   ` Viresh Kumar
2022-04-22  7:52 ` [PATCH V4 03/14] cpufreq: mediatek: Replace old_* with pre_* Rex-BC Chen
2022-04-25  5:11   ` Viresh Kumar
2022-04-22  7:52 ` [PATCH V4 04/14] cpufreq: mediatek: Record previous target vproc value Rex-BC Chen
2022-04-22  8:21   ` AngeloGioacchino Del Regno
2022-04-25  5:12     ` Viresh Kumar
2022-04-22  7:52 ` [PATCH V4 06/14] cpufreq: mediatek: Move voltage limits to platform data Rex-BC Chen
2022-04-22  7:52 ` [PATCH V4 07/14] cpufreq: mediatek: Add .get function Rex-BC Chen
2022-04-22  8:21   ` AngeloGioacchino Del Regno
2022-04-25  5:35   ` Viresh Kumar
2022-04-25  9:34     ` Rex-BC Chen
2022-04-25 10:00       ` Viresh Kumar
2022-04-26 11:13         ` Rex-BC Chen
2022-04-27  3:11           ` Viresh Kumar
2022-04-28 11:16             ` Rex-BC Chen
2022-04-28 11:48               ` Viresh Kumar
2022-05-03 11:33                 ` Rex-BC Chen
2022-05-04  8:22                   ` Viresh Kumar
2022-05-04 11:57                     ` Rex-BC Chen
2022-05-04 11:58                       ` Viresh Kumar
2022-05-04 12:03                         ` Rex-BC Chen
2022-04-22  7:52 ` [PATCH V4 08/14] cpufreq: mediatek: Make sram regulator optional Rex-BC Chen
2022-04-25  5:36   ` Viresh Kumar
2022-04-22  7:52 ` [PATCH V4 09/14] cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking() Rex-BC Chen
2022-04-22  8:20   ` AngeloGioacchino Del Regno
2022-04-22  7:52 ` [PATCH V4 10/14] cpufreq: mediatek: Link CCI device to CPU Rex-BC Chen
2022-04-22  7:52 ` [PATCH V4 11/14] cpufreq: mediatek: Add support for MT8186 Rex-BC Chen
2022-04-22  7:52 ` [PATCH V4 12/14] arm64: dts: mediatek: Add opp table and clock property for MT8183 cpufreq Rex-BC Chen
2022-04-22  8:20   ` AngeloGioacchino Del Regno
2022-04-22  7:52 ` [PATCH V4 13/14] arm64: dts: mediatek: Add MediaTek CCI node for MT8183 Rex-BC Chen
2022-04-22  7:52 ` [PATCH V4 14/14] arm64: dts: mediatek: Add mediatek,cci property for MT8183 cpufreq Rex-BC Chen
2022-04-22 17:23 ` [PATCH V4 00/14] cpufreq: mediatek: Cleanup and support MT8183 and MT8186 Krzysztof Kozlowski
2022-04-25  6:20   ` Rex-BC Chen
     [not found] ` <20220422075239.16437-6-rex-bc.chen@mediatek.com>
2022-04-25  5:20   ` [PATCH V4 05/14] cpufreq: mediatek: Add opp notification support Viresh Kumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).